Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support for custom type dispatcher #48

Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 1 addition & 4 deletions cornice_swagger/converters/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,8 @@
from cornice_swagger.converters.parameters import ParameterConversionDispatcher


def convert_schema(schema_node):

dispatcher = TypeConversionDispatcher()
def convert_schema(schema_node, dispatcher=TypeConversionDispatcher()):
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now we have references to the dispatchers in the CorniceSwagger class, I would just remove these two and call the dispatchers directly from there.

converted = dispatcher(schema_node)

return converted


Expand Down
28 changes: 16 additions & 12 deletions cornice_swagger/converters/schema.py
Original file line number Diff line number Diff line change
Expand Up @@ -237,24 +237,28 @@ def convert_type(self, schema_node):

class TypeConversionDispatcher(object):

converters = {
colander.Boolean: BooleanTypeConverter,
colander.Date: DateTypeConverter,
colander.DateTime: DateTimeTypeConverter,
colander.Float: NumberTypeConverter,
colander.Integer: IntegerTypeConverter,
colander.Mapping: ObjectTypeConverter,
colander.Sequence: ArrayTypeConverter,
colander.String: StringTypeConverter,
colander.Time: TimeTypeConverter,
}
def __init__(self, custom_converters=None, default_converter=None):
self.converters = {
colander.Boolean: BooleanTypeConverter,
colander.Date: DateTypeConverter,
colander.DateTime: DateTimeTypeConverter,
colander.Float: NumberTypeConverter,
colander.Integer: IntegerTypeConverter,
colander.Mapping: ObjectTypeConverter,
colander.Sequence: ArrayTypeConverter,
colander.String: StringTypeConverter,
colander.Time: TimeTypeConverter,
}
if custom_converters:
self.converters.update(custom_converters)
self.default_converter = default_converter

def __call__(self, schema_node):

schema_type = schema_node.typ
schema_type = type(schema_type)

converter_class = self.converters.get(schema_type)
converter_class = self.converters.get(schema_type, self.default_converter)
if converter_class is None:
raise NoSuchConverter

Expand Down
25 changes: 19 additions & 6 deletions cornice_swagger/swagger.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
import six

from cornice_swagger.util import body_schema_transformer, merge_dicts, trim
from cornice_swagger.converters import TypeConversionDispatcher
from cornice_swagger.converters import convert_schema, convert_parameter


Expand All @@ -16,7 +17,8 @@ class CorniceSwaggerException(Exception):
class CorniceSwagger(object):
"""Handles the creation of a swagger document from a cornice application."""

def __init__(self, services, def_ref_depth=0, param_ref=False, resp_ref=False):
def __init__(self, services, def_ref_depth=0, param_ref=False, resp_ref=False,
custom_type_converters=None, default_type_converter=None):
"""
:param services:
List of cornice services to document. You may use
Expand All @@ -33,11 +35,18 @@ def __init__(self, services, def_ref_depth=0, param_ref=False, resp_ref=False):
Defines if swagger responses should be put inline on the operation
or on the responses section and referenced by JSON pointers.
Default is inline.
:param custom_type_converters:
A dictionary mapping user defined cornice types to callables
implementing cornice_swagger.converters.schema.TypeConverter iface
:param default_type_converter:
Default TypeConverter to use when there is no type registered for a
cornice type. If it's not given and the type is not registered,
cornice_swagger.converters.exceptions.NoSuchConverter is raised
"""

self.services = services

self.definitions = DefinitionHandler(ref=def_ref_depth)
typ_dispatcher = TypeConversionDispatcher(custom_type_converters, default_type_converter)
Copy link
Collaborator

@gabisurita gabisurita Jan 24, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My feel is to do the same for the parameter dispatcher here, even if it doesn't take any additional parameters related to it on the __init__ call. Just to keep semantics.

self.definitions = DefinitionHandler(ref=def_ref_depth, typ_dispatcher=typ_dispatcher)
self.parameters = ParameterHandler(self.definitions,
ref=param_ref)
self.responses = ResponseHandler(self.definitions,
Copy link
Collaborator

@gabisurita gabisurita Jan 24, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The response handler might also use custom type dispatchers.

Expand Down Expand Up @@ -309,14 +318,18 @@ class DefinitionHandler(object):

json_pointer = '#/definitions/'

def __init__(self, ref=0):
def __init__(self, ref=0, typ_dispatcher=TypeConversionDispatcher()):
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a good reason to have to overwrite the whole TypeConversionDispatcher class? Can't we just pass a dict mapping the user defined cornice types to a callable implementing the cornice_swagger.converters.schema.TypeConverter interface?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The reason is that I needed to change the behaviour of the TypeConversionDispatcher and not only add some custom type to fit our use case. Since our custom types always inherits from colander.String, I've adopted a solution like this:

class TypeConversionDispatcher(object):
    converters = defaultdict(lambda: StringTypeConverter)

    converters.update({
        colander.Boolean: BooleanTypeConverter,
        colander.Date: DateTypeConverter,
        colander.DateTime: DateTimeTypeConverter,
        colander.Float: NumberTypeConverter,
        colander.Integer: IntegerTypeConverter,
        colander.Mapping: ObjectTypeConverter,
        colander.Sequence: ArrayTypeConverter,
        colander.String: StringTypeConverter,
        colander.Time: TimeTypeConverter,
    })
...

I do realize that's not a common use case, so it's ok for me to go with a dict of type:converter

Copy link
Collaborator

@gabisurita gabisurita Jan 22, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, I see. But I still think that passing the dispatcher is a bit extreme.

One solution that i think is a little more simple is to have custom_type_converters={} that update the default converters and default_type_converter=None that is used when we don't have an entry for that type instead of raising an exception. Thoughts?

Edit: I would probably use a default converter as well :)

"""
:param ref:
The depth that should be used by self.ref when calling self.from_schema.
"""

self.definition_registry = {}
self.ref = ref
self.typ_dispatcher = typ_dispatcher

def convert_schema(self, schema_node):
return convert_schema(schema_node, self.typ_dispatcher)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just call the type dispatcher here, we don't need to go through convert_schema.


def from_schema(self, schema_node, base_name=None):
"""
Expand All @@ -330,7 +343,7 @@ def from_schema(self, schema_node, base_name=None):
:rtype: dict
Swagger schema.
"""
return self._ref_recursive(convert_schema(schema_node), self.ref, base_name)
return self._ref_recursive(self.convert_schema(schema_node), self.ref, base_name)

def _ref_recursive(self, schema, depth, base_name=None):
"""
Expand Down Expand Up @@ -515,7 +528,7 @@ def from_schema_mapping(self, schema_mapping):
response['schema'] = self.definitions.from_schema(field_schema)

elif location in ('header', 'headers'):
header_schema = convert_schema(field_schema)
header_schema = self.definitions.convert_schema(field_schema)
headers = header_schema.get('properties')
if headers:
# Response headers doesn't accept titles
Expand Down
8 changes: 8 additions & 0 deletions tests/support.py
Original file line number Diff line number Diff line change
Expand Up @@ -55,3 +55,11 @@ class AnotherDeclarativeSchema(colander.MappingSchema):
@colander.instantiate(description='my another body')
class body(colander.MappingSchema):
timestamp = colander.SchemaNode(colander.Int())


class MyType(colander.String):
pass


class CustomTypeQuerySchema(colander.MappingSchema):
foo = colander.SchemaNode(MyType(), missing=colander.drop)
45 changes: 43 additions & 2 deletions tests/test_parameter_handler.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,13 @@

from cornice.validators import colander_body_validator

from cornice_swagger.converters import TypeConversionDispatcher
from cornice_swagger.converters.schema import StringTypeConverter
from cornice_swagger.swagger import ParameterHandler, DefinitionHandler
from cornice_swagger.converters import convert_schema
from cornice_swagger.util import body_schema_transformer
from .support import (BodySchema, PathSchema, QuerySchema, HeaderSchema,
DeclarativeSchema, AnotherDeclarativeSchema)
from .support import (BodySchema, PathSchema, QuerySchema, HeaderSchema, MyType,
DeclarativeSchema, AnotherDeclarativeSchema, CustomTypeQuerySchema)


class SchemaParamConversionTest(unittest.TestCase):
Expand Down Expand Up @@ -57,6 +59,45 @@ class RequestSchema(colander.MappingSchema):
}
self.assertDictEqual(params[0], expected)

def test_covert_query_with_custom_colander_type(self):
converters = {MyType: StringTypeConverter}
typ_dispatcher = TypeConversionDispatcher(custom_converters=converters)
definition_handler = DefinitionHandler(typ_dispatcher=typ_dispatcher)

class RequestSchema(colander.MappingSchema):
querystring = CustomTypeQuerySchema()

node = RequestSchema()
params = ParameterHandler(definition_handler).from_schema(node)
self.assertEquals(len(params), 1)

expected = {
'name': 'foo',
'in': 'query',
'type': 'string',
'required': False
}
self.assertDictEqual(params[0], expected)

def test_covert_query_with_default_colander_type(self):
typ_dispatcher = TypeConversionDispatcher(default_converter=StringTypeConverter)
definition_handler = DefinitionHandler(typ_dispatcher=typ_dispatcher)

class RequestSchema(colander.MappingSchema):
querystring = CustomTypeQuerySchema()

node = RequestSchema()
params = ParameterHandler(definition_handler).from_schema(node)
self.assertEquals(len(params), 1)

expected = {
'name': 'foo',
'in': 'query',
'type': 'string',
'required': False
}
self.assertDictEqual(params[0], expected)

def test_covert_headers_with_request_validator_schema(self):

class RequestSchema(colander.MappingSchema):
Expand Down
8 changes: 8 additions & 0 deletions tests/test_swagger.py
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,14 @@ def test_with_resp_ref(self):
validate(spec)
self.assertIn('responses', spec)

def test_using_provided_dispatcher_params(self):
# todo: improve testing by mocking definitions and ensure that .convert_schema is called
default_typ = object()
swagger = CorniceSwagger([], custom_type_converters={default_typ: default_typ},
default_type_converter=default_typ)
self.assertIn(default_typ, swagger.definitions.typ_dispatcher.converters)
self.assertEqual(default_typ, swagger.definitions.typ_dispatcher.default_converter)

def test_swagger_field_updates_extracted_paths(self):
swagger = CorniceSwagger([self.service])
raw_swagger = {
Expand Down