Skip to content

Avoid overwriting data in SchemaGenerator.get_schena.content #4478

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

Closed
dilyanpalauzov opened this issue Sep 9, 2016 · 2 comments
Closed

Avoid overwriting data in SchemaGenerator.get_schena.content #4478

dilyanpalauzov opened this issue Sep 9, 2016 · 2 comments

Comments

@dilyanpalauzov
Copy link

SchemaGenerator.get_schema contains content[category][action] = link, which might delete another objects stored there, e.g. in cases of "custom actions", but also when having URL conf like:

/groups
/users
/users/groups/

The latter URL pattern has overwritten the former one, because they shared the same category "groups". The patch below fixes the problem, be generating unique (category,action) for inclusion in the content dictionary, so that no collisions happen and no data there is overwritten. I verified the output using django-rest-swagger and it shows exactly what it is supposed to do.

Caveats:
In our configuration, the URLs for the application reside under http://fsafd/rest/urls-patterns (https://fsafd/rest/orange, http://fsafd/rest/peper, etc.), hence I took the constants 1 and 2 below to generate the categories "orange" and "peper"), but in other configurations this constants might be unsuitable. However I found no way to substract the prefix (here /rest/) from the URL and to use the first part of the remainder for the category. I have not invested too much time, maybe someone reading this will have a ready answer.

diff --git a/rest_framework/schemas.py b/rest_framework/schemas.py
index 1b89945..54fb1ea 100644
--- a/rest_framework/schemas.py
+++ b/rest_framework/schemas.py
@@ -129,7 +129,8 @@ class SchemaGenerator(object):
                 if self.should_include_endpoint(path, callback):
                     for method in self.get_allowed_methods(callback):
                         action = self.get_action(path, method, callback)
-                        category = self.get_category(path, method, callback, action)
+                        category, action = self.get_category(path, method,
+                                                             callback, action)
                         endpoint = (path, method, category, action, callback)
                         api_endpoints.append(endpoint)

@@ -186,32 +187,32 @@ class SchemaGenerator(object):

     def get_category(self, path, method, callback, action):
         """
-        Return a descriptive category string for the endpoint, eg. 'users'.
+        Return a tuple with a descriptive category string for the endpoint, eg.
+        'users', and the new action.  The returned tuple is designed to avoid
+        collisions, when inserted to the content dictionary.

         Examples of category/action pairs that should be generated for various
         endpoints:

+        /groups/                    [groups][list], [groups][create]
         /users/                     [users][list], [users][create]
         /users/{pk}/                [users][read], [users][update], [users][destroy]
-        /users/enabled/             [users][enabled]  (custom action)
-        /users/{pk}/star/           [users][star]     (custom action)
-        /users/{pk}/groups/         [groups][list], [groups][create]
-        /users/{pk}/groups/{pk}/    [groups][read], [groups][update], [groups][destroy]
+        /users/enabled/             [users][enabled]  (custom actions)
+        /users/{pk}/star/           [users][star]     (custom actions)
+        /users/{pk}/groups/         [users][groups/list], [users][groups/create]
+        /users/{pk}/groups/{pk}/    [users][groups/read], [users[groups/update],
+                                    [users][groups/destroy]
         path_components = path.strip('/').split('/')
         path_components = [
             component for component in path_components
             if '{' not in component
         ]
-        if action in self.known_actions:
-            # Default action, eg "/users/", "/users/{pk}/"
-            idx = -1
-        else:
-            # Custom action, eg "/users/{pk}/activate/", "/users/active/"
-            idx = -2

+        new_action = path_components[2:]
+        new_action.append(action if action in self.known_actions else method)
         try:
-            return path_components[idx]
+            return path_components[1], "/".join(new_action)
         except IndexError:
             return None

         """

@dilyanpalauzov
Copy link
Author

Here is addition to the code above, that considers the url= parameter of the SchemaGenerator constructor to calculate the prefix to the application and substract it from the generated category. Otherwise it uses the first part of the path as category:

--- a/rest_framework/schemas.py
+++ b/rest_framework/schemas.py
@@ -64,6 +64,7 @@ class SchemaGenerator(object):

         self.title = title
         self.url = url
+        self.url_prefix = urlparse.urlparse(url).path
         self.endpoints = None

     def get_schema(self, request=None):
@@ -148,6 +152,8 @@ class SchemaGenerator(object):
         """
         path = simplify_regex(path_regex)
         path = path.replace('<', '{').replace('>', '}')
+        if self.url:
+            return path.replace(self.url_prefix, '', 1)
         return path

     def should_include_endpoint(self, path, callback):

@tomchristie tomchristie added this to the 3.4.7 Release milestone Sep 12, 2016
@tomchristie tomchristie modified the milestones: 3.4.7 Release, 3.4.8 Release Sep 21, 2016
@tomchristie tomchristie modified the milestones: 3.4.8 Release, 3.5.0 Release Sep 29, 2016
@tomchristie
Copy link
Member

Resolved via #4527 (now uses a nested style for categories, which avoids clashes). Now in the version-3-5 branch.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants