Remove schemas

[tarekziade]

Work in progress

• removed the schema & serialization features
• provide a Colander helper
• Fix the doc
• add a test to show how to do cross location validation (body+query string)
• decide if we release a minor release with a deprecation warnin
• Update changelog

[leplatrem]

querystring ?

[leplatrem]

If we want compatibility of clients that exploit the information of JSON validation responses, the resulting location should be header and not headers (see request.errors.add() below)

[leplatrem]

Now the schemas is cleaner, but we need to provide -- for extensions like sphinx or swagger -- a simple way to introspect the schema defined for a view (method+content-type...).

If we want to make schema validation pluggable with any library (jsonschema, marshmallow, cerberus, colander, ...) then ideally we need to make schema introspection agnostic too.

BTW the method service.schema_for(...) is still there.

A proposition would be to leave the schema argument, make it accept a standard pivot format like JSONSchema as dict.

from cornice.validators import jsonschema_validator

person_schema = {"title": "Person", "fields": [...]}

service = ...

@service.post(schema=person_schema, validators=(jsonschema_validator(person_schema),))
def service_post(request):
....

This way, everything is explicit and easily introspectable. Developers not using swagger/sphinx could omit the schema argument. What do you think ?

[Natim]

cannot we write it like that:

@service.post(schema=person_schema, validators=(jsonschema_validator,))

And schema is always passed to validators?

[leplatrem]

And schema is always passed to validators?

Why not. But I prefer the former because for example, if you use colander and don't need swagger/sphinx, you can just do:

class PostSchema(colander.Mapping):
      body = PersonSchema()

@service.post(validators=(colander_validator(PostSchema),))

And if you need swagger, something like:

class PostSchema(colander.Mapping):
      body = PersonSchema()

@service.post(schema=colander_to_jsonschema(PostSchema),
              validators=(colander_validator(PostSchema),))

Otherwise yes, we could pass every parameters to validators, but I prefer the approach where we don't do too much magic. Related issue #105

[leplatrem]

I migrated the Kinto code to this branch. It was rather easy. But had to change some code here:

--- a/cornice/validators/__init__.py
+++ b/cornice/validators/__init__.py
@@ -11,18 +11,22 @@ DEFAULT_FILTERS = []
 def extract_cstruct(request):
     if request.content_type == 'application/x-www-form-urlencoded':
         body = request.POST.mixed()
-    elif not request.content_type == 'application/json':
+    elif request.content_type and request.content_type != 'application/json':
         body = request.body
     else:
-        try:
-            body = request.json_body
-        except ValueError:
-            request.errors.add('body', '', 'Invalid JSON')
-            return {}
-        else:
-            if not hasattr(body, 'items'):
-                request.errors.add('body', '', 'Should be a JSON object')
+        if request.body:
+            try:
+                body = request.json_body
+            except ValueError as e:
+                error_msg = 'Invalid JSON request body: %s' % e
+                request.errors.add('body', '', error_msg)
                 return {}
+            else:
+                if not hasattr(body, 'items'):
+                    request.errors.add('body', '', 'Should be a JSON object')
+                    return {}
+        else:
+            body = {}

     cstruct = {'method': request.method,
                'url': request.url,

Basically in order to keep the current behaviour for the following situations:

• Empty body and no Content-Type → body = {}
• Empty body and Content-Type: application/json → body = {}

Those are useful to create objects with a simple/empty http PUT :8888/v1/buckets/plop.

Also the JSON validation messages seem to have changed in this branch. It's probably due to switch to request.json_body instead of the former simplejson.loads(request.body).

[tarekziade]

Now the schemas is cleaner, but we need to provide -- for extensions like sphinx or swagger --
a simple way to introspect the schema defined for a view (method+content-type...).

@leplatrem why can't this been done in a validator ? I don't understand why you need to re-introduce a schema key.

[leplatrem]

@leplatrem why can't this been done in a validator ?

How else would you introspect the validator function to obtain the schema and produce a swagger output ?

[tarekziade]

The introspection can happen outside Cornice's logic, by building the information in memory when the modules are read:

class PostSchema(colander.Mapping):
      body = PersonSchema()

@service.post(validators=(some_validator, swaggered(PostSchema)))
def my_service(request)
    return

here, the swaggered function can populate a global dict that describes the service and its linked schema. Or am I missing something ?

The trick is to have the validators be factories e.g. some_validator & swaggered(PostSchema) would return the actual validator that can be then stored and used by the service.

[tarekziade]

full example

class Schema():
    pass



def some_validator():
    def __some_validator():
        print('Validating with some_validator')
    return __some_validator


def swaggered(schema):
    print('Generating docs')
    def _swaggered():
        def __swaggered():
            pass
        return __swaggered
    return _swaggered


def service(validators=None):
    if validators is None:
        _validators = []
    else:
        _validators = [v() for v in validators]

    def _service(func):
        def __service(*args, **kwargs):
            for v in _validators:
                v()
            print('Running the request')
            return func(object())
        return __service
    return _service



@service(validators=(some_validator, swaggered(Schema)))
def my_service(request):
    print('the actual request')
    return


my_service(object)

output

$ python demo.py
Generating docs
Validating with some_validator
Running the request
the actual request

[leplatrem]

I see. Why not indeed! Unless I miss a point, the downside is that swagger cannot be an «external» option anymore. The validators are coupled to the fact that we want swagger enabled on the API. It could become problematic when dealing with several formats outputs (e.g. API-blueprint)..

My original idea was to leave the original schema kwarg as a descriptive attribute storing an agnostic schema for the service — without exploiting it as we do now. But I realize it can be pretty confusing if it's name is just schema as before.

I propose that we come back to this topic in another issue/pull-request, even if it means that we delay the swagger feature.

Getting rid of the current schema approach and pushing towards a flat and simple list of validators makes a lot of sense right now IMO.

[tarekziade]

I propose that we come back to this topic in another issue/pull-request, even if it means that we delay the swagger feature.

doing a factory for validators has a huge impact though - I think we should decide on that design now to avoid refactoring again later if we decide to do it this way

[tarekziade]

Another interesting use case to have a factory there is that we can pass all kind of information to the validators at startup time, for them to prepare stuff in memory that can be reuse. I think it's a valuable pattern.

[tarekziade]

the downside is that swagger cannot be an «external» option anymore. The validators are coupled to the fact that we want swagger enabled on the API. It could become problematic when dealing with several formats outputs (e.g. API-blueprint)..

I don't understand this part. what is an «external» option ? can you describe the problem ?

[leplatrem]

I propose that we come back to this topic in another issue/pull-request, even if it means that we delay the swagger feature.

doing a factory for validators has a huge impact though - I think we should decide on that design now to avoid refactoring again later if we decide to do it this way

Ok I hadn't caught the mandatory factory approach. I hence don't understand why it would be different than what we have now where we can choose between:

def lambda_validator(request):
     # ...

@service(validators=(lambda_validator,))

and

def factory_validator(schema_class):
    schema = schema_class()  # reused instance
    def _validate(request):
         # ...
    return _validate

@service(validators=(factory_validator(Schema),))

the downside is that swagger cannot be an «external» option anymore. The validators are coupled to the fact that we want swagger enabled on the API. It could become problematic when dealing with several formats > > outputs (e.g. API-blueprint)..

I don't understand this part. what is an «external» option ? can you describe the problem ?

In your example, you do:

@service(validators=(some_validator, swaggered(Schema)))
def my_service(request):
    # ...

From what I understand, the swaggered validator is in charge of producing a Swagger output.
The day I want to obtain the Swagger definitions for my API, I have to change the validators of every endpoint with this swaggered one. If tomorrow I want API-blueprint, shall I have to add another api_blueprinted ? Plus, does swaggered validate anything from the request?

In my opinion, the Swagger/OpenAPI/Blueprint thing should be external. In other words, it should be decoupled from my service definitions.

I obtain the swagger of my API by introspecting the declared services and produce the JSON result from that. Like this — as a simplistic example:

from cornice.services import get_services

def generate_swagger():
    result = {
        "swagger": "2.0",
        "paths": {}
    }

    services = get_services()

    for service in services:
        for method, view, kwargs in service.definitions:
            json_schema = kwargs.get('schema')
            # transform agnostic json schema to swagger
            # to build result
            # result["paths"][service.path] = ...

    return result


if __name__ == '__main__':
    print(json.dumps(generate_swagger()))

Since we open the door to any validation library, we can't rely on the fact that there will a colander schema there. And I believe that we shouldn't couple the services definition with a description format like Swagger

I can't really think of a better solution than a descriptive/agnostic service kwarg in order to achieve that...

[tarekziade]

hence don't understand why it would be different than what we have now where we can choose between:

if you look at my example, the validators are executed twice. Once when the service is loaded, and every time the service is called. The first execution is where the introspection can happen. Your example cannot do that. I made a full example for that purpose because it's not obvious.

In my opinion, the Swagger/OpenAPI/Blueprint thing should be external. In other words, it should be decoupled from my service definitions.

ok, I understand your point now. Your solution of using a schema keyword works for me in that case, as long as it's just by convention: e.g. Cornice just accepts keywords and pass them along to whatever want to use them.

[tarekziade]

Also the JSON validation messages seem to have changed in this branch. It's probably due to switch to request.json_body instead of the former simplejson.loads(request.body).

According to https://github.com/Pylons/webob/blob/master/webob/request.py#L8 - webob will use simplejson if available.

The code that's called is

json.loads(self.body.decode(self.charset))

[Natim]

nit: can we use **kwargs for consistency?

[Natim]

I didn't follow enough the refactoring so I will hand the review to @leplatrem but thank you for the effort on this. Cornice is becoming a better place.

[leplatrem]

missing request parameter

[leplatrem]

r+ with doc nit