Fixed issues with schema name collisions (#5486)

* Fixed issues with schema name collisions

* Fixed mutating issues in python 3

* Optimized solution

* Fixed isort

* Removed not needed cast

* Fix for key collision

* Added preferred key to preserve if available

* Add accidently removed test
This commit is contained in:
Marcin 2017-10-16 10:32:48 +01:00 committed by Carlton Gibson
parent c7fb60bcd4
commit 5d7b6e5b2f
2 changed files with 122 additions and 11 deletions

View File

@ -4,7 +4,7 @@ generators.py # Top-down schema generation
See schemas.__init__.py for package overview.
"""
import warnings
from collections import OrderedDict
from collections import Counter, OrderedDict
from importlib import import_module
from django.conf import settings
@ -64,6 +64,25 @@ to customise schema structure.
"""
class LinkNode(OrderedDict):
def __init__(self):
self.links = []
self.methods_counter = Counter()
super(LinkNode, self).__init__()
def get_available_key(self, preferred_key):
if preferred_key not in self:
return preferred_key
while True:
current_val = self.methods_counter[preferred_key]
self.methods_counter[preferred_key] += 1
key = '{}_{}'.format(preferred_key, current_val)
if key not in self:
return key
def insert_into(target, keys, value):
"""
Nested dictionary insertion.
@ -71,14 +90,15 @@ def insert_into(target, keys, value):
>>> example = {}
>>> insert_into(example, ['a', 'b', 'c'], 123)
>>> example
{'a': {'b': {'c': 123}}}
LinkNode({'a': LinkNode({'b': LinkNode({'c': LinkNode(links=[123])}}})))
"""
for key in keys[:-1]:
if key not in target:
target[key] = {}
target[key] = LinkNode()
target = target[key]
try:
target[keys[-1]] = value
target.links.append((keys[-1], value))
except TypeError:
msg = INSERT_INTO_COLLISION_FMT.format(
value_url=value.url,
@ -88,6 +108,15 @@ def insert_into(target, keys, value):
raise ValueError(msg)
def distribute_links(obj):
for key, value in obj.items():
distribute_links(value)
for preferred_key, link in obj.links:
key = obj.get_available_key(preferred_key)
obj[key] = link
def is_custom_action(action):
return action not in set([
'retrieve', 'list', 'create', 'update', 'partial_update', 'destroy'
@ -255,6 +284,7 @@ class SchemaGenerator(object):
if not url and request is not None:
url = request.build_absolute_uri()
distribute_links(links)
return coreapi.Document(
title=self.title, description=self.description,
url=url, content=links
@ -265,7 +295,7 @@ class SchemaGenerator(object):
Return a dictionary containing all the links that should be
included in the API schema.
"""
links = OrderedDict()
links = LinkNode()
# Generate (path, method, view) given (path, method, callback).
paths = []
@ -288,6 +318,7 @@ class SchemaGenerator(object):
subpath = path[len(prefix):]
keys = self.get_keys(subpath, method, view)
insert_into(links, keys, link)
return links
# Methods used when we generate a view instance from the raw callback...

View File

@ -749,6 +749,10 @@ class NamingCollisionView(generics.RetrieveUpdateDestroyAPIView):
serializer_class = BasicModelSerializer
class BasicNamingCollisionView(generics.RetrieveAPIView):
queryset = BasicModel.objects.all()
class NamingCollisionViewSet(GenericViewSet):
"""
Example via: https://stackoverflow.com/questions/43778668/django-rest-framwork-occured-typeerror-link-object-does-not-support-item-ass/
@ -779,9 +783,35 @@ class TestURLNamingCollisions(TestCase):
]
generator = SchemaGenerator(title='Naming Colisions', patterns=patterns)
schema = generator.get_schema()
with pytest.raises(ValueError):
generator.get_schema()
expected = coreapi.Document(
url='',
title='Naming Colisions',
content={
'test': {
'list': {
'list': coreapi.Link(url='/test/list/', action='get')
},
'list_0': coreapi.Link(url='/test', action='get')
}
}
)
assert expected == schema
def _verify_cbv_links(self, loc, url, methods=None, suffixes=None):
if methods is None:
methods = ('read', 'update', 'partial_update', 'delete')
if suffixes is None:
suffixes = (None for m in methods)
for method, suffix in zip(methods, suffixes):
if suffix is not None:
key = '{}_{}'.format(method, suffix)
else:
key = method
assert loc[key].url == url
def test_manually_routing_generic_view(self):
patterns = [
@ -797,8 +827,14 @@ class TestURLNamingCollisions(TestCase):
generator = SchemaGenerator(title='Naming Colisions', patterns=patterns)
with pytest.raises(ValueError):
generator.get_schema()
schema = generator.get_schema()
self._verify_cbv_links(schema['test']['delete'], '/test/delete/')
self._verify_cbv_links(schema['test']['put'], '/test/put/')
self._verify_cbv_links(schema['test']['get'], '/test/get/')
self._verify_cbv_links(schema['test']['update'], '/test/update/')
self._verify_cbv_links(schema['test']['retrieve'], '/test/retrieve/')
self._verify_cbv_links(schema['test'], '/test', suffixes=(None, '0', None, '0'))
def test_from_router(self):
patterns = [
@ -806,9 +842,53 @@ class TestURLNamingCollisions(TestCase):
]
generator = SchemaGenerator(title='Naming Colisions', patterns=patterns)
schema = generator.get_schema()
desc = schema['detail_0'].description # not important here
with pytest.raises(ValueError):
generator.get_schema()
expected = coreapi.Document(
url='',
title='Naming Colisions',
content={
'detail': {
'detail_export': coreapi.Link(
url='/from-routercollision/detail/export/',
action='get',
description=desc)
},
'detail_0': coreapi.Link(
url='/from-routercollision/detail/',
action='get',
description=desc
)
}
)
assert schema == expected
def test_url_under_same_key_not_replaced(self):
patterns = [
url(r'example/(?P<pk>\d+)/$', BasicNamingCollisionView.as_view()),
url(r'example/(?P<slug>\w+)/$', BasicNamingCollisionView.as_view()),
]
generator = SchemaGenerator(title='Naming Colisions', patterns=patterns)
schema = generator.get_schema()
assert schema['example']['read'].url == '/example/{id}/'
assert schema['example']['read_0'].url == '/example/{slug}/'
def test_url_under_same_key_not_replaced_another(self):
patterns = [
url(r'^test/list/', simple_fbv),
url(r'^test/(?P<pk>\d+)/list/', simple_fbv),
]
generator = SchemaGenerator(title='Naming Colisions', patterns=patterns)
schema = generator.get_schema()
assert schema['test']['list']['list'].url == '/test/list/'
assert schema['test']['list']['list_0'].url == '/test/{id}/list/'
def test_is_list_view_recognises_retrieve_view_subclasses():