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

Schema generation doesn't work if URL ends with method name #4704

Closed
mlubimow opened this issue Nov 24, 2016 · 17 comments
Closed

Schema generation doesn't work if URL ends with method name #4704

mlubimow opened this issue Nov 24, 2016 · 17 comments

Comments

@mlubimow
Copy link
Contributor

mlubimow commented Nov 24, 2016

Steps to reproduce

If one would use urls like these (not a good practice but possible):

url(r'^test', ExampleView1.as_view()),
url(r'^test/delete/', ExampleView2.as_view()),

Schema generation using rest_framework.schemas.get_schema_view will fail.
I've attached minimal test django project if anyone would like to reproduce the issue.

Expected behavior

Schema should be generated as normally does.

Actual behavior

Throws an exception:

Environment:


Request Method: GET
Request URL: http://127.0.0.1:8001/

Django Version: 1.10.3
Python Version: 2.7.12
Installed Applications:
['django.contrib.admin',
 'django.contrib.auth',
 'django.contrib.contenttypes',
 'django.contrib.sessions',
 'django.contrib.messages',
 'django.contrib.staticfiles',
 'rest_framework',
 'schema_bug_test_app']
Installed Middleware:
['django.middleware.security.SecurityMiddleware',
 'django.contrib.sessions.middleware.SessionMiddleware',
 'django.middleware.common.CommonMiddleware',
 'django.middleware.csrf.CsrfViewMiddleware',
 'django.contrib.auth.middleware.AuthenticationMiddleware',
 'django.contrib.messages.middleware.MessageMiddleware',
 'django.middleware.clickjacking.XFrameOptionsMiddleware']



Traceback:

File "/Users/mlubimow/Projects/test_project/env/lib/python2.7/site-packages/django/core/handlers/exception.py" in inner
  39.             response = get_response(request)

File "/Users/mlubimow/Projects/test_project/env/lib/python2.7/site-packages/django/core/handlers/base.py" in _get_response
  187.                 response = self.process_exception_by_middleware(e, request)

File "/Users/mlubimow/Projects/test_project/env/lib/python2.7/site-packages/django/core/handlers/base.py" in _get_response
  185.                 response = wrapped_callback(request, *callback_args, **callback_kwargs)

File "/Users/mlubimow/Projects/test_project/env/lib/python2.7/site-packages/django/views/decorators/csrf.py" in wrapped_view
  58.         return view_func(*args, **kwargs)

File "/Users/mlubimow/Projects/test_project/env/lib/python2.7/site-packages/django/views/generic/base.py" in view
  68.             return self.dispatch(request, *args, **kwargs)

File "/Users/mlubimow/Projects/test_project/env/lib/python2.7/site-packages/rest_framework/views.py" in dispatch
  477.             response = self.handle_exception(exc)

File "/Users/mlubimow/Projects/test_project/env/lib/python2.7/site-packages/rest_framework/views.py" in handle_exception
  437.             self.raise_uncaught_exception(exc)

File "/Users/mlubimow/Projects/test_project/env/lib/python2.7/site-packages/rest_framework/views.py" in dispatch
  474.             response = handler(request, *args, **kwargs)

File "/Users/mlubimow/Projects/test_project/env/lib/python2.7/site-packages/rest_framework/schemas.py" in get
  593.             schema = generator.get_schema(request)

File "/Users/mlubimow/Projects/test_project/env/lib/python2.7/site-packages/rest_framework/schemas.py" in get_schema
  242.         links = self.get_links(request)

File "/Users/mlubimow/Projects/test_project/env/lib/python2.7/site-packages/rest_framework/schemas.py" in get_links
  276.             insert_into(links, keys, link)

File "/Users/mlubimow/Projects/test_project/env/lib/python2.7/site-packages/rest_framework/schemas.py" in insert_into
  79.     target[keys[-1]] = value

Exception Type: TypeError at /
Exception Value: 'Link' object does not support item assignment
@tomchristie
Copy link
Member

Any chance you could include the traceback of that exception?

@mlubimow
Copy link
Contributor Author

Oh, yeah, sorry for that. I've updated report.

@tomchristie tomchristie added this to the 3.5.4 Release milestone Nov 24, 2016
@eshandas
Copy link

hello @tomchristie any idea when this will be solved?
Thanks

@tomchristie
Copy link
Member

Not yet. Been focusing on functionality for 3.6 lately.

@alform
Copy link

alform commented Jan 11, 2017

This also happens if an url is (or maybe also ends with? did not try this one) 'list' (a natural verb enough). The reason is that insert_into (in schemas.py) is passed a keys parameter whose last element's value is 'list', which eventually conflicts with the 'list' present in the url.

@ZuSe
Copy link

ZuSe commented Feb 23, 2017

It also occurs when you use drf-extensions with NestedRoutes.
From what I remember it used to worked in 3.4.

@jplock
Copy link

jplock commented Jun 12, 2017

Is it possible to have this resolved in the next patch release?

@ghost
Copy link

ghost commented Jun 26, 2017

This issue is caused by usage of url_path in detail_route decorator. Any chance of fix?

class MyViewSet(mixins.RetrieveModelMixin, mixins.ListModelMixin, viewsets.GenericViewSet):
    ...
    @detail_route(
        methods=['get', ],
        url_name='children-list',
        url_path='children(?:/(?P<child_id>[a-zA-Z0-9]+))?',
        serializer_class=emg_serializers.ChildSerializer
    )
    def children(self, , request, parent_id=None, child_id=None):
        ...

@Sinkler
Copy link

Sinkler commented Jul 26, 2017

You can temporary fix it in your project using a custom SchemaView with a custom SchemaGenerator: https://stackoverflow.com/a/45323639/5157209

@jplock
Copy link

jplock commented Sep 28, 2017

@carltongibson is it possible to get this fixed in 3.7.0?

@carltongibson
Copy link
Collaborator

screenshot 2017-09-28 17 44 03

😀

that's the idea.

I need to look into exactly what the issue is, and then the fix, but it's on the short list

carltongibson added a commit to carltongibson/django-rest-framework that referenced this issue Sep 29, 2017
@carltongibson
Copy link
Collaborator

carltongibson commented Sep 29, 2017

I opened #5464 with test cases for this.

Can I ask all interested to review to see that your case is covered?

@mlubimow I covered your original case.

I also added the viewset case from the Stack Overflow issue. (@ola-t I wasn't entirely sure your case with children matches this 100% — can you confirm?)

@everyone: Are you seeing a case that's a bit different?

I'll take PRs on https://github.com/carltongibson/django-rest-framework on the 37/schema-naming-collisions branch adding extra test cases — just copy the two examples.

Please don't be shy — if your case isn't covered it won't be fixed for v3.7!

@mlubimow
Copy link
Contributor Author

Hey @carltongibson,

I think my case may not be covered, as I've got a problem when URL contained http method name in path (like get/post/delete etc). For example: url(r'^test/delete/', simple_fbv).

Thanks for fixing that issue! 👍

@carltongibson
Copy link
Collaborator

carltongibson commented Sep 29, 2017

@mlubimow Can you then try and replicate the issue in a test case?

Why? Because this does not raise:

    def test_manually_routing_with_delete(self):
        patterns = [
            url(r'^test', simple_fbv),
            url(r'^test/delete/', simple_fbv),  # !!!: Using `delete`
        ]

        generator = SchemaGenerator(title='Naming Colisions', patterns=patterns)
        schema = generator.get_schema()

That means there's info missing from your description. I'm happy to fix it, but I need the reproduce... 👍

(PR to my fork on 37/schema-naming-collisions branch)

@mlubimow
Copy link
Contributor Author

@carltongibson I tried to reproduce it on your branch but I can't - I'm not sure if its because of a different way of getting the list of endpoints (as in my MVP attached to the issue endpoints are read from settings using EndpointInspector) or it has been fixed since then (when I created a ticket PIP version of DRF was 3.5.3).

Installing new version of DRF in my MVP yields another error..

@carltongibson
Copy link
Collaborator

@mlubimow I added a GenericAPIView based example that shows the behaviour your saw. fa87a95

@mlubimow
Copy link
Contributor Author

mlubimow commented Oct 2, 2017

@carltongibson thanks!

carltongibson pushed a commit that referenced this issue Oct 5, 2017
* Add failing tests for #4704

* Add generic view based test case.

* Adjust insert_into to raise ValueError
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

8 participants