Skip to content
This repository was archived by the owner on Mar 18, 2019. It is now read-only.

fixing #37 issue support of definitions for nested objects #38

Open
wants to merge 20 commits into
base: master
Choose a base branch
from
Open

fixing #37 issue support of definitions for nested objects #38

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
97724ae
fixing #37 issue support of definitions for nested objects
Jun 11, 2017
a67ba3a
fix unexpected print
Jun 11, 2017
2391f78
py2X<->py3X
Jun 11, 2017
aea9c8c
OrderedDict instead of regular dict
Jun 26, 2017
5a7bb2a
adding py36 to tox envlist
xxmatyuk Jan 15, 2018
532bfbd
preliminary changes
xxmatyuk Jan 17, 2018
5be9182
bugfixing, enhancements, tests update
xxmatyuk Jan 18, 2018
fec8d0e
flake8 warning fix
xxmatyuk Jan 18, 2018
4152782
py2.7 <-> py3.X
xxmatyuk Jan 18, 2018
09a0648
tests run aftermath fixes
xxmatyuk Jan 18, 2018
cd49ed4
small change
xxmatyuk Jan 18, 2018
56dea30
statement enhancement
xxmatyuk Jan 18, 2018
2efbdcc
adding docstrings
xxmatyuk Jan 18, 2018
44ebcbe
more sophisticated checks added
xxmatyuk Jan 18, 2018
836aa93
fix typo
xxmatyuk Jan 18, 2018
efd7516
py2.X <-> py3.X
xxmatyuk Jan 18, 2018
cb565a3
final changes
xxmatyuk Jan 18, 2018
ca20480
trying to fix flake8 issue
xxmatyuk Jan 18, 2018
84a1d7b
fixing tests once again
xxmatyuk Jan 18, 2018
34d9cd5
flake8 fix
xxmatyuk Jan 18, 2018
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
65 changes: 60 additions & 5 deletions openapi_codec/encode.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import coreschema
from collections import OrderedDict
from coreapi.document import Field
from coreapi.compat import urlparse
from openapi_codec.utils import get_method, get_encoding, get_location, get_links_from_document

Expand All @@ -23,15 +24,61 @@ def generate_swagger_object(document):
swagger['schemes'] = [parsed_url.scheme]

swagger['paths'] = _get_paths_object(document)
swagger['definitions'] = _get_definitions(document)

return swagger


def _get_definitions(document):

definitions = OrderedDict()
links = _get_links(document)

def get_field_def_data(field_item, defs):

definition_data = {
'type': 'object',
'properties': {}
}

if isinstance(field_item, coreschema.Object):
props = field_item.properties
Copy link

Choose a reason for hiding this comment

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

since the recursion can also be an array of "simple" types, i would also suggest to add here:

        elif isinstance(field_item, (coreschema.String, coreschema.Boolean, coreschema.Integer, coreschema.Number)):
            return {'type': _get_field_type(field_item), 'description': _get_field_description(field_item)}

else:
props = field_item.schema.properties

for f_name, f_schema in iter(props.items()):

if _get_field_type(f_schema) is 'object':
defs[f_name] = get_field_def_data(f_schema, defs)
definition_data['properties'][f_name] = {
'$ref': '#/definitions/{}'.format(f_name)
}
else:
definition_data['properties'][f_name] = {
'type': _get_field_type(f_schema),
Copy link

Choose a reason for hiding this comment

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

type can also be an array here. Now items is missing and the swagger-ui (version 2) breaks.

Copy link

@tuky tuky Aug 7, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

i suppose it needs something like

            elif _get_field_type(f_schema) == 'array':
                _item_name = '{}_item'.format(f_name)
                defs[_item_name] = get_field_def_data(f_schema.items, defs)
                definition_data['properties'][f_name] = {
                    'type': 'array',
                    'items': {'$ref': '#/definitions/{}'.format(_item_name)}
                }

Copy link

Choose a reason for hiding this comment

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

The usage of f_name confuses me here. There can be multiple fields with the same name (in different paths/objects) but different array member type in a schema, so this approach will definitely fail in such case.

Copy link

Choose a reason for hiding this comment

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

yes. name clashes shoudl be a problem in all of this PR.

Copy link
Author

Choose a reason for hiding this comment

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

Okay, let me figure that out. I thought it was useful and nobody even start it, but as soon I did it turns out everyone, but me knows how to do it better =)

Copy link

Choose a reason for hiding this comment

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

@xxmatyuk No offense =), I tried 2 other PRs for the same issue, and yours is still the best although still not quite ready.

Copy link
Author

Choose a reason for hiding this comment

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

@tuffnatty no problems, that was looong hard day) I'm willing to fix it, so stay tuned. And yeah, thanks )

Copy link
Author

@xxmatyuk xxmatyuk Jan 6, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

@tuky @tuffnatty
guys, I'm making the changes as per you proposed to do, but could you please help me to find an example of the same name for definitions? So that I can write tests and perform changes real quick. Thanks.

Copy link

Choose a reason for hiding this comment

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

Say e.g. you have 4 serializers in 2 different files:

class AuthorSerializer(Serializer):
    name = CharField()

class Song(Serializer):
    author = AuthorSerializer()
    co_authors = AuthorSerializer(many=True)
class AuthorSerializer(Serializer):
    birthday = DateField()

class Song(Serializer):
    author = AuthorSerializer()

The 2 different AuthorSerializer should get the same names when generated into CoreAPI format. Then we have a name clash, when reusing them as '#/definitions/author' or '#/definitions/co_authors_item' in OpenAPI.

Copy link
Author

Choose a reason for hiding this comment

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

@tuky, thanks a lot! I wrote a test, reproduced the clash, working on fix.

'description': ''
}

return definition_data

for _, link, _ in links:
for field in link.fields:
field_type = _get_field_type(field)
if field_type == 'object':
definitions[field.name] = get_field_def_data(field, definitions)

if field_type == 'array':
item_name = '{}_item'.format(field.name)
definitions[item_name] = get_field_def_data(field.schema.items, definitions)

return definitions


def _add_tag_prefix(item):
operation_id, link, tags = item
if tags:
operation_id = tags[0] + '_' + operation_id
return (operation_id, link, tags)
return operation_id, link, tags


def _get_links(document):
Expand Down Expand Up @@ -114,8 +161,10 @@ def _get_field_type(field):
# Deprecated
return field.type

if field.schema is None:
return 'string'
if isinstance(field, Field):
cls = field.schema.__class__
else:
cls = field.__class__

return {
coreschema.String: 'string',
Expand All @@ -124,7 +173,7 @@ def _get_field_type(field):
coreschema.Boolean: 'boolean',
coreschema.Array: 'array',
coreschema.Object: 'object',
}.get(field.schema.__class__, 'string')
}.get(cls, 'string')


def _get_parameters(link, encoding):
Expand Down Expand Up @@ -160,8 +209,14 @@ def _get_parameters(link, encoding):
'description': field_description,
'type': field_type,
}

if field_type == 'array':
schema_property['items'] = {'type': 'string'}
item_name = '{}_item'.format(field.name)
schema_property['items'] = {'$ref': '#/definitions/{}'.format(item_name)}

if field_type == 'object':
schema_property = {'$ref': '#/definitions/{}'.format(field.name)}

properties[field.name] = schema_property
if field.required:
required.append(field.name)
Expand Down
92 changes: 91 additions & 1 deletion tests/test_encode.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
import coreapi
import coreschema
from openapi_codec.encode import generate_swagger_object, _get_parameters

from collections import OrderedDict
from openapi_codec.encode import generate_swagger_object, _get_parameters, _get_definitions
from unittest import TestCase


Expand Down Expand Up @@ -32,6 +34,11 @@ def test_schemes(self):
expected = ['https']
self.assertEquals(self.swagger['schemes'], expected)

def test_definitions(self):
self.assertIn('definitions', self.swagger)
expected = dict()
self.assertEquals(self.swagger['definitions'], expected)


class TestPaths(TestCase):
def setUp(self):
Expand Down Expand Up @@ -101,3 +108,86 @@ def test_expected_fields(self):
'type': 'string' # Everything is a string for now.
}
self.assertEquals(self.swagger[0], expected)


class TestDefinitions(TestCase):

def setUp(self):

obj_props = OrderedDict()
obj_props['foo'] = coreschema.String()
obj_props['bar'] = coreschema.Integer()

self.object_field = coreapi.Field(
name='dummy_object',
required=True,
location='form',
schema=coreschema.Object(
properties=obj_props
)
)

self.array_field = coreapi.Field(
name='dummy_array',
location='form',
schema=coreschema.Array(
items=self.object_field
)
)

self.link = coreapi.Link(
action='post',
url='/users/',
fields=[self.object_field, self.array_field]
)

self.document = coreapi.Document(
content={
'users': {
'create': self.link,
}
}
)

self.definitions = _get_definitions(self.document)
self.parameters = _get_parameters(self.link, '')
self.swagger = generate_swagger_object(self.document)

def test_basic_definitions(self):

self.assertIn('definitions', self.swagger)
self.assertIn('dummy_object', self.definitions)
self.assertIn('dummy_array_item', self.definitions)

expected_dummy_object_def = {
'type': 'object',
'properties': {
'foo': {'type': 'string', 'description': ''},
'bar': {'type': 'integer', 'description': ''}
}
}

self.assertEqual(self.definitions.get('dummy_object'), expected_dummy_object_def)
self.assertEqual(self.definitions.get('dummy_array_item'), expected_dummy_object_def)

expected_dummy_parameters = [{
'schema': {
'required': ['dummy_object'],
'type': 'object',
'properties': {
'dummy_array': {
'items': {
'$ref': '#/definitions/dummy_array_item'
},
'type': 'array',
'description': ''
},
'dummy_object': {
'$ref': '#/definitions/dummy_object'
}
}
},
'name': 'data',
'in': 'body'
}]
self.assertEqual(self.parameters, expected_dummy_parameters)
1 change: 0 additions & 1 deletion tests/test_mappings.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -202,7 +202,6 @@ def test_mapping():
coreapi.Field(
name='example',
location='body',

schema=coreschema.String()
)
]
Expand Down