From dcd64d09826c8f150ea40ef6512d168a74b48836 Mon Sep 17 00:00:00 2001 From: Marcin Lubimow Date: Mon, 9 Oct 2017 10:14:16 +0100 Subject: [PATCH] Added preferred key to preserve if available --- rest_framework/schemas/generators.py | 35 +++++++---- tests/test_schemas.py | 92 +++++++++++++++------------- 2 files changed, 71 insertions(+), 56 deletions(-) diff --git a/rest_framework/schemas/generators.py b/rest_framework/schemas/generators.py index 77e96b85f..2f4dd63db 100644 --- a/rest_framework/schemas/generators.py +++ b/rest_framework/schemas/generators.py @@ -70,12 +70,15 @@ class LinkNode(OrderedDict): self.methods_counter = Counter() super(LinkNode, self).__init__() - def get_next_key(self, method): - while True: - current_val = self.methods_counter[method] - self.methods_counter[method] += 1 + def get_available_key(self, preferred_key): + if preferred_key not in self: + return preferred_key - key = '{}_{}'.format(method, current_val) + 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 @@ -89,13 +92,13 @@ def insert_into(target, keys, value): >>> example LinkNode({'a': LinkNode({'b': LinkNode({'c': LinkNode(links=[123])}}}))) """ - for key in keys: + for key in keys[:-1]: if key not in target: target[key] = LinkNode() target = target[key] try: - target.links.append(value) + target.links.append((keys[-1], value)) except TypeError: msg = INSERT_INTO_COLLISION_FMT.format( value_url=value.url, @@ -109,8 +112,8 @@ def distribute_links(obj): for key, value in obj.items(): distribute_links(value) - for link in obj.links: - key = obj.get_next_key(link.action) + for preferred_key, link in obj.links: + key = obj.get_available_key(preferred_key) obj[key] = link @@ -436,8 +439,16 @@ class SchemaGenerator(object): if is_custom_action(action): # Custom action, eg "/users/{pk}/activate/", "/users/active/" - if len(view.action_map) == 1: - return named_path_components[:-1] + if len(view.action_map) > 1: + action = self.default_mapping[method.lower()] + if action in self.coerce_method_names: + action = self.coerce_method_names[action] + return named_path_components + [action] + else: + return named_path_components[:-1] + [action] + + if action in self.coerce_method_names: + action = self.coerce_method_names[action] # Default action, eg "/users/", "/users/{pk}/" - return named_path_components + return named_path_components + [action] diff --git a/tests/test_schemas.py b/tests/test_schemas.py index 08e4a023a..eaace9a7b 100644 --- a/tests/test_schemas.py +++ b/tests/test_schemas.py @@ -120,7 +120,7 @@ class TestRouterGeneratedSchema(TestCase): title='Example API', content={ 'example': { - 'get_0': coreapi.Link( + 'list': coreapi.Link( url='/example/', action='get', fields=[ @@ -129,17 +129,17 @@ class TestRouterGeneratedSchema(TestCase): coreapi.Field('ordering', required=False, location='query', schema=coreschema.String(title='Ordering', description='Which field to use when ordering the results.')) ] ), - 'get_1': coreapi.Link( + 'custom_list_action': coreapi.Link( url='/example/custom_list_action/', action='get' ), 'custom_list_action_multiple_methods': { - 'get_0': coreapi.Link( + 'read': coreapi.Link( url='/example/custom_list_action_multiple_methods/', action='get' ) }, - 'get_2': coreapi.Link( + 'read': coreapi.Link( url='/example/{id}/', action='get', fields=[ @@ -150,7 +150,6 @@ class TestRouterGeneratedSchema(TestCase): } } ) - assert response.data == expected def test_authenticated_request(self): @@ -163,7 +162,7 @@ class TestRouterGeneratedSchema(TestCase): title='Example API', content={ 'example': { - 'get_0': coreapi.Link( + 'list': coreapi.Link( url='/example/', action='get', fields=[ @@ -172,7 +171,7 @@ class TestRouterGeneratedSchema(TestCase): coreapi.Field('ordering', required=False, location='query', schema=coreschema.String(title='Ordering', description='Which field to use when ordering the results.')) ] ), - 'post_0': coreapi.Link( + 'create': coreapi.Link( url='/example/', action='post', encoding='application/json', @@ -181,7 +180,7 @@ class TestRouterGeneratedSchema(TestCase): coreapi.Field('b', required=False, location='form', schema=coreschema.String(title='B')) ] ), - 'get_2': coreapi.Link( + 'read': coreapi.Link( url='/example/{id}/', action='get', fields=[ @@ -189,7 +188,7 @@ class TestRouterGeneratedSchema(TestCase): coreapi.Field('ordering', required=False, location='query', schema=coreschema.String(title='Ordering', description='Which field to use when ordering the results.')) ] ), - 'post_1': coreapi.Link( + 'custom_action': coreapi.Link( url='/example/{id}/custom_action/', action='post', encoding='application/json', @@ -200,7 +199,7 @@ class TestRouterGeneratedSchema(TestCase): coreapi.Field('d', required=False, location='form', schema=coreschema.String(title='D')), ] ), - 'post_2': coreapi.Link( + 'custom_action_with_list_fields': coreapi.Link( url='/example/{id}/custom_action_with_list_fields/', action='post', encoding='application/json', @@ -211,21 +210,21 @@ class TestRouterGeneratedSchema(TestCase): coreapi.Field('b', required=True, location='form', schema=coreschema.Array(title='B', items=coreschema.String())), ] ), - 'get_1': coreapi.Link( + 'custom_list_action': coreapi.Link( url='/example/custom_list_action/', action='get' ), 'custom_list_action_multiple_methods': { - 'get_0': coreapi.Link( + 'read': coreapi.Link( url='/example/custom_list_action_multiple_methods/', action='get' ), - 'post_0': coreapi.Link( + 'create': coreapi.Link( url='/example/custom_list_action_multiple_methods/', action='post' ) }, - 'put_0': coreapi.Link( + 'update': coreapi.Link( url='/example/{id}/', action='put', encoding='application/json', @@ -236,7 +235,7 @@ class TestRouterGeneratedSchema(TestCase): coreapi.Field('ordering', required=False, location='query', schema=coreschema.String(title='Ordering', description='Which field to use when ordering the results.')) ] ), - 'patch_0': coreapi.Link( + 'partial_update': coreapi.Link( url='/example/{id}/', action='patch', encoding='application/json', @@ -247,7 +246,7 @@ class TestRouterGeneratedSchema(TestCase): coreapi.Field('ordering', required=False, location='query', schema=coreschema.String(title='Ordering', description='Which field to use when ordering the results.')) ] ), - 'delete_0': coreapi.Link( + 'delete': coreapi.Link( url='/example/{id}/', action='delete', fields=[ @@ -329,17 +328,17 @@ class TestSchemaGenerator(TestCase): title='Example API', content={ 'example': { - 'post_0': coreapi.Link( + 'create': coreapi.Link( url='/example/', action='post', fields=[] ), - 'get_0': coreapi.Link( + 'list': coreapi.Link( url='/example/', action='get', fields=[] ), - 'get_1': coreapi.Link( + 'read': coreapi.Link( url='/example/{id}/', action='get', fields=[ @@ -347,7 +346,7 @@ class TestSchemaGenerator(TestCase): ] ), 'sub': { - 'get_0': coreapi.Link( + 'list': coreapi.Link( url='/example/{id}/sub/', action='get', fields=[ @@ -382,17 +381,17 @@ class TestSchemaGeneratorNotAtRoot(TestCase): title='Example API', content={ 'example': { - 'post_0': coreapi.Link( + 'create': coreapi.Link( url='/api/v1/example/', action='post', fields=[] ), - 'get_0': coreapi.Link( + 'list': coreapi.Link( url='/api/v1/example/', action='get', fields=[] ), - 'get_1': coreapi.Link( + 'read': coreapi.Link( url='/api/v1/example/{id}/', action='get', fields=[ @@ -400,7 +399,7 @@ class TestSchemaGeneratorNotAtRoot(TestCase): ] ), 'sub': { - 'get_0': coreapi.Link( + 'list': coreapi.Link( url='/api/v1/example/{id}/sub/', action='get', fields=[ @@ -437,7 +436,7 @@ class TestSchemaGeneratorWithMethodLimitedViewSets(TestCase): title='Example API', content={ 'example1': { - 'get_0': coreapi.Link( + 'list': coreapi.Link( url='/example1/', action='get', fields=[ @@ -446,17 +445,17 @@ class TestSchemaGeneratorWithMethodLimitedViewSets(TestCase): coreapi.Field('ordering', required=False, location='query', schema=coreschema.String(title='Ordering', description='Which field to use when ordering the results.')) ] ), - 'get_1': coreapi.Link( + 'custom_list_action': coreapi.Link( url='/example1/custom_list_action/', action='get' ), 'custom_list_action_multiple_methods': { - 'get_0': coreapi.Link( + 'read': coreapi.Link( url='/example1/custom_list_action_multiple_methods/', action='get' ) }, - 'get_2': coreapi.Link( + 'read': coreapi.Link( url='/example1/{id}/', action='get', fields=[ @@ -494,7 +493,7 @@ class TestSchemaGeneratorWithRestrictedViewSets(TestCase): title='Example API', content={ 'example': { - 'get_0': coreapi.Link( + 'list': coreapi.Link( url='/example/', action='get', fields=[] @@ -666,7 +665,7 @@ class SchemaGenerationExclusionTests(TestCase): title='Exclusions', content={ 'included-fbv': { - 'get_0': coreapi.Link(url='/included-fbv/', action='get') + 'list': coreapi.Link(url='/included-fbv/', action='get') } } ) @@ -791,21 +790,26 @@ class TestURLNamingCollisions(TestCase): content={ 'test': { 'list': { - 'get_0': coreapi.Link(url='/test/list/', action='get') + 'list': coreapi.Link(url='/test/list/', action='get') }, - 'get_0': coreapi.Link(url='/test', action='get') + 'list_0': coreapi.Link(url='/test', action='get') } } ) assert expected == schema - def _verify_cbv_links(self, loc, url, methods=None, number=0): + def _verify_cbv_links(self, loc, url, methods=None, suffixes=None): if methods is None: - methods = ('get', 'put', 'patch', 'delete') + methods = ('read', 'update', 'partial_update', 'delete') + if suffixes is None: + suffixes = (None for m in methods) - for method in methods: - key = '{}_{}'.format(method, number) + 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): @@ -829,7 +833,7 @@ class TestURLNamingCollisions(TestCase): 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') + self._verify_cbv_links(schema['test'], '/test', suffixes=(None, '0', None, '0')) def test_from_router(self): patterns = [ @@ -838,19 +842,19 @@ class TestURLNamingCollisions(TestCase): generator = SchemaGenerator(title='Naming Colisions', patterns=patterns) schema = generator.get_schema() - desc = schema['get_0'].description # not important here + desc = schema['detail_0'].description # not important here expected = coreapi.Document( url='', title='Naming Colisions', content={ 'detail': { - 'get_0': coreapi.Link( + 'detail_export': coreapi.Link( url='/from-routercollision/detail/export/', action='get', description=desc) }, - 'get_0': coreapi.Link( + 'detail_0': coreapi.Link( url='/from-routercollision/detail/', action='get', description=desc @@ -869,8 +873,8 @@ class TestURLNamingCollisions(TestCase): generator = SchemaGenerator(title='Naming Colisions', patterns=patterns) schema = generator.get_schema() - assert schema['example']['get_0'].url == '/example/{id}/' - assert schema['example']['get_1'].url == '/example/{slug}/' + 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): @@ -882,5 +886,5 @@ class TestURLNamingCollisions(TestCase): generator = SchemaGenerator(title='Naming Colisions', patterns=patterns) schema = generator.get_schema() - assert schema['test']['list']['get_0'].url == '/test/list/' - assert schema['test']['list']['get_1'].url == '/test/{id}/list/' + assert schema['test']['list']['list'].url == '/test/list/' + assert schema['test']['list']['list_0'].url == '/test/{id}/list/'