Schemas should automatically respect API root location #4401

nick-allen · 2016-08-14T18:14:36Z

Creating a schema should automatically provide any path prefixes to the registered router urls instead of defaulting to /.

It's possible to manually set this prefix using the DefaultRouter schema_url paramter, but this requires knowing where the router will be wired into Django URLs.

Steps to reproduce

Install django, rest framework, and coreapi. Configure normal Django boilerplate

Create a viewset

# viewsets.py

from rest_framework.viewsets import ModelViewSet
from rest_framework.serializers import ModelSerializer
from django.contrib.auth import get_user_model

User = get_user_model()


class UserSerializer(ModelSerializer):
    class Meta:
        model = User


class UserViewSet(ModelViewSet):

    queryset = User.objects.all()
    serializer_class = UserSerializer

Create a router with the schema_title argument to setup automatic CoreAPI renderer and register viewsets.
Include router URLs under a path not at root /

# urls.py
from django.conf.urls import url, include

from rest_framework.routers import DefaultRouter

from .viewsets import UserViewSet

v1_router = DefaultRouter(
    schema_title='Example API v1',
)

v1_router.register('users', UserViewSet)

urlpatterns = [
    url(r'^api/v1/', include(v1_router.urls, namespace='v1')),
]

Get schema document and verify url does not include API path prefix

$ curl http://localhost:8000/api/v1/?format=corejson
{
    "_type": "document",
    "_meta": {
        "title": "Example API v1"
    },
    "users": {
        "create": {
            "_type": "link",
            "url": "/users/",
            "action": "post",
            "encoding": "application/json",
            "fields": [
                {
                    "name": "password",
                    "required": true,
                    "location": "form"
                },
                {
                    "name": "last_login",
                    "location": "form"
                },
                {
                    "name": "is_superuser",
                    "location": "form",
                    "description": "Designates that this user has all permissions without explicitly assigning them."
                },
                {
                    "name": "username",
                    "required": true,
                    "location": "form",
                    "description": "Required. 30 characters or fewer. Letters, digits and @/./+/-/_ only."
                },
                {
                    "name": "first_name",
                    "location": "form"
                },
                {
                    "name": "last_name",
                    "location": "form"
                },
                {
                    "name": "email",
                    "location": "form"
                },
                {
                    "name": "is_staff",
                    "location": "form",
                    "description": "Designates whether the user can log into this admin site."
                },
                {
                    "name": "is_active",
                    "location": "form",
                    "description": "Designates whether this user should be treated as active. Unselect this instead of deleting accounts."
                },
                {
                    "name": "date_joined",
                    "location": "form"
                },
                {
                    "name": "groups",
                    "location": "form",
                    "description": "The groups this user belongs to. A user will get all permissions granted to each of their groups."
                },
                {
                    "name": "user_permissions",
                    "location": "form",
                    "description": "Specific permissions for this user."
                }
            ]
        },
        "destroy": {
            "_type": "link",
            "url": "/users/{pk}/",
            "action": "delete",
            "fields": [
                {
                    "name": "pk",
                    "required": true,
                    "location": "path"
                }
            ]
        },
        "list": {
            "_type": "link",
            "url": "/users/",
            "action": "get",
            "fields": [
                {
                    "name": "page",
                    "location": "query"
                }
            ]
        },
        "partial_update": {
            "_type": "link",
            "url": "/users/{pk}/",
            "action": "patch",
            "encoding": "application/json",
            "fields": [
                {
                    "name": "pk",
                    "required": true,
                    "location": "path"
                },
                {
                    "name": "password",
                    "location": "form"
                },
                {
                    "name": "last_login",
                    "location": "form"
                },
                {
                    "name": "is_superuser",
                    "location": "form",
                    "description": "Designates that this user has all permissions without explicitly assigning them."
                },
                {
                    "name": "username",
                    "location": "form",
                    "description": "Required. 30 characters or fewer. Letters, digits and @/./+/-/_ only."
                },
                {
                    "name": "first_name",
                    "location": "form"
                },
                {
                    "name": "last_name",
                    "location": "form"
                },
                {
                    "name": "email",
                    "location": "form"
                },
                {
                    "name": "is_staff",
                    "location": "form",
                    "description": "Designates whether the user can log into this admin site."
                },
                {
                    "name": "is_active",
                    "location": "form",
                    "description": "Designates whether this user should be treated as active. Unselect this instead of deleting accounts."
                },
                {
                    "name": "date_joined",
                    "location": "form"
                },
                {
                    "name": "groups",
                    "location": "form",
                    "description": "The groups this user belongs to. A user will get all permissions granted to each of their groups."
                },
                {
                    "name": "user_permissions",
                    "location": "form",
                    "description": "Specific permissions for this user."
                }
            ]
        },
        "retrieve": {
            "_type": "link",
            "url": "/users/{pk}/",
            "action": "get",
            "fields": [
                {
                    "name": "pk",
                    "required": true,
                    "location": "path"
                }
            ]
        },
        "update": {
            "_type": "link",
            "url": "/users/{pk}/",
            "action": "put",
            "encoding": "application/json",
            "fields": [
                {
                    "name": "pk",
                    "required": true,
                    "location": "path"
                },
                {
                    "name": "password",
                    "required": true,
                    "location": "form"
                },
                {
                    "name": "last_login",
                    "location": "form"
                },
                {
                    "name": "is_superuser",
                    "location": "form",
                    "description": "Designates that this user has all permissions without explicitly assigning them."
                },
                {
                    "name": "username",
                    "required": true,
                    "location": "form",
                    "description": "Required. 30 characters or fewer. Letters, digits and @/./+/-/_ only."
                },
                {
                    "name": "first_name",
                    "location": "form"
                },
                {
                    "name": "last_name",
                    "location": "form"
                },
                {
                    "name": "email",
                    "location": "form"
                },
                {
                    "name": "is_staff",
                    "location": "form",
                    "description": "Designates whether the user can log into this admin site."
                },
                {
                    "name": "is_active",
                    "location": "form",
                    "description": "Designates whether this user should be treated as active. Unselect this instead of deleting accounts."
                },
                {
                    "name": "date_joined",
                    "location": "form"
                },
                {
                    "name": "groups",
                    "location": "form",
                    "description": "The groups this user belongs to. A user will get all permissions granted to each of their groups."
                },
                {
                    "name": "user_permissions",
                    "location": "form",
                    "description": "Specific permissions for this user."
                }
            ]
        }
    }
}

Expected behavior

Using coreapi cli, use any actions following schema tutorial in docs (http://www.django-rest-framework.org/tutorial/7-schemas-and-client-libraries/)

Get schema document

$ coreapi get http://localhost:8000/api/v1/
<Example API v1 "http://localhost:8000/api/v1/">
    users: {
        create(password, username, [last_login], [is_superuser], [first_name], [last_name], [email], [is_staff], [is_active], [date_joined], [groups], [user_permissions])
        destroy(pk)
        list([page])
        partial_update(pk, [password], [last_login], [is_superuser], [username], [first_name], [last_name], [email], [is_staff], [is_active], [date_joined], [groups], [user_permissions])
        retrieve(pk)
        update(pk, password, username, [last_login], [is_superuser], [first_name], [last_name], [email], [is_staff], [is_active], [date_joined], [groups], [user_permissions])
    }

Run list action

$ coreapi action users list
[
    {
        "id": 1,
        "password": "!YOnSaMeJECJU0oeULffq6tp6TB3WAlTLTngqXBpT",
        "last_login": null,
        "is_superuser": false,
        "username": "user",
        "first_name": "",
        "last_name": "",
        "email": "",
        "is_staff": false,
        "is_active": true,
        "date_joined": "2016-08-14T18:10:37.880469Z",
        "groups": [],
        "user_permissions": []
    }
]

Actual behavior

Get schema document

$ coreapi get http://localhost:8000/api/v1/

<Example API v1 "http://localhost:8000/api/v1/">
    users: {
        create(password, username, [last_login], [is_superuser], [first_name], [last_name], [email], [is_staff], [is_active], [date_joined], [groups], [user_permissions])
        destroy(pk)
        list([page])
        partial_update(pk, [password], [last_login], [is_superuser], [username], [first_name], [last_name], [email], [is_staff], [is_active], [date_joined], [groups], [user_permissions])
        retrieve(pk)
        update(pk, password, username, [last_login], [is_superuser], [first_name], [last_name], [email], [is_staff], [is_active], [date_joined], [groups], [user_permissions])
    }

Run list action

$ coreapi action users list
<Error: Not Found>
    message: "
              <!DOCTYPE html>
              <html lang=\"en\">
              <head>
                <meta http-equiv=\"content-type\" content=\"text/html; charset=utf-8\">
                <title>Page not found at /users/</title>
                <meta name=\"robots\" content=\"NONE,NOARCHIVE\">
                <style type=\"text/css\">
                  html * { padding:0; margin:0; }
                  body * { padding:10px 20px; }
                  body * * { padding:0; }
                  body { font:small sans-serif; background:#eee; }
                  body>div { border-bottom:1px solid #ddd; }
                  h1 { font-weight:normal; margin-bottom:.4em; }
                  h1 span { font-size:60%; color:#666; font-weight:normal; }
                  table { border:none; border-collapse: collapse; width:100%; }
                  td, th { vertical-align:top; padding:2px 3px; }
                  th { width:12em; text-align:right; color:#666; padding-right:.5em; }
                  #info { background:#f6f6f6; }
                  #info ol { margin: 0.5em 4em; }
                  #info ol li { font-family: monospace; }
                  #summary { background: #ffc; }
                  #explanation { background:#eee; border-bottom: 0px none; }
                </style>
              </head>
              <body>
                <div id=\"summary\">
                  <h1>Page not found <span>(404)</span></h1>
                  <table class=\"meta\">
                    <tr>
                      <th>Request Method:</th>
                      <td>GET</td>
                    </tr>
                    <tr>
                      <th>Request URL:</th>
                      <td>http://localhost:8000/users/</td>
                    </tr>

                  </table>
                </div>
                <div id=\"info\">

                    <p>
                    Using the URLconf defined in <code>drfbug.urls</code>,
                    Django tried these URL patterns, in this order:
                    </p>
                    <ol>

                        <li>

                              ^api/v1/


                        </li>

                    </ol>
                    <p>The current URL, <code>users/</code>, didn't match any of these.</p>

                </div>

                <div id=\"explanation\">
                  <p>
                    You're seeing this error because you have <code>DEBUG = True</code> in
                    your Django settings file. Change that to <code>False</code>, and Django
                    will display a standard 404 page.
                  </p>
                </div>
              </body>
              </html>
              "

The text was updated successfully, but these errors were encountered:

tomchristie · 2016-08-19T11:23:34Z

It's possible to manually set this prefix using the DefaultRouter schema_url paramter, but this requires knowing where the router will be wired into Django URLs.

I'm not sure that we can automagically determine this in a nice way.

Setting the path explicitly is the right thing for you to do, at least for now.

Perhaps we'll revisit this at some point in the future.

klada · 2016-10-07T11:26:38Z

I would like to bring this up again, as the bug is not fixed.

The issue actually is that the URLs in the schema are wrong. Why is it not possible to simply use reverse() for generating the URLs in the schema? This way the URLs would always be correct. Everything else violates the DRY principle and is error-prone

tomchristie · 2016-10-07T11:36:00Z

@klada - Be specific. There's a whole bunch of fixes in the incoming 3-5 branch. If you believe there's still an issue on that branch then show me an example case that we can use to help resolve things.

tomchristie · 2016-10-07T11:38:39Z

Okay, have read through this issue again.

Going to re-iterate this:

Setting the path explicitly is the right thing for you to do, at least for now.

Yes, that means making sure you know where it's located. Them's the breaks. I'll take another review of this prior to the 3.5 release, but I doubt that aspect is going to change.

nick-allen · 2016-10-07T12:57:34Z

@tomchristie, to @klada's point, this does violate DRY. I think I gave a pretty detailed example case in the description, if there's something else you'd like to see, I'm happy to put something together.

As far as this not changing, is there something specific blocking this? I dug through the schema URL setup code, only issue I found was that I wasn't able to inspect the api_root dynamically if it wasn't provided due to when the URL setup was run.

tomchristie · 2016-10-07T13:12:31Z

Okay, on review can confirm that this issue is resolved in the upcoming 3.5 release.

And you're right, the example was plenty specific enough.

There's a few further improvements that I can see are also required based on the example here, which I'll work towards resolving prior to the release.

tomchristie · 2016-10-07T13:13:50Z

Reopening to leave as a placeholder for resolving schema layout with URL prefixes.

tomchristie · 2016-10-07T14:32:51Z

Resolved via 69b4acd (incoming in 3.5)

Thanks for having highlighted this & apologies for not having fully understood the issue on first pass!

* Start test case * Added 'requests' test client * Address typos * Graceful fallback if requests is not installed. * Add cookie support * Tests for auth and CSRF * Py3 compat * py3 compat * py3 compat * Add get_requests_client * Added SchemaGenerator.should_include_link * add settings for html cutoff on related fields * Router doesn't work if prefix is blank, though project urls.py handles prefix * Fix Django 1.10 to-many deprecation * Add django.core.urlresolvers compatibility * Update django-filter & django-guardian * Check for empty router prefix; adjust URL accordingly It's easiest to fix this issue after we have made the regex. To try to fix it before would require doing something different for List vs Detail, which means we'd have to know which type of url we're constructing before acting accordingly. * Fix misc django deprecations * Use TOC extension instead of header * Fix deprecations for py3k * Add py3k compatibility to is_simple_callable * Add is_simple_callable tests * Drop python 3.2 support (EOL, Dropped by Django) * schema_renderers= should *set* the renderers, not append to them. * API client (#4424) * Fix release notes * Add note about 'User account is disabled.' vs 'Unable to log in' * Clean up schema generation (#4527) * Handle multiple methods on custom action (#4529) * RequestsClient, CoreAPIClient * exclude_from_schema * Added 'get_schema_view()' shortcut * Added schema descriptions * Better descriptions for schemas * Add type annotation to schema generation * Coerce schema 'pk' in path to actual field name * Deprecations move into assertion errors * Use get_schema_view in tests * Updte CoreJSON media type * Handle schema structure correctly when path prefixs exist. Closes #4401 * Add PendingDeprecation to Router schema generation. * Added SCHEMA_COERCE_PATH_PK and SCHEMA_COERCE_METHOD_NAMES * Renamed and documented 'get_schema_fields' interface.

klada · 2016-11-10T09:35:05Z

This issue is still present in 3.5. Please see this demo project for a quick demonstration of the issue.

I have also included a README containing the current (wrong) and the expected corejson output.

moorchegue · 2017-08-25T14:09:18Z

As far as I'm concerned it's still present in the latest release. It's especially annoying when coreapi client is being used, because there's no way to add a prefix manually on the client side.

Is this considered not an issue anymore? Any hints for non-hacky solutions?

carltongibson · 2017-08-25T17:07:07Z

So there's a test added for this

69b4acd#diff-42ea969a60c167b848d4c4910d590060R294

I'd very happily look at a PR that included/added a failing example showing what the issue is meant to be.

moorchegue · 2017-08-26T02:21:13Z

Thanks for pointing out! I guess the test case I'm looking for is settings.BASE_URL = '/some/prefix/', which the test doesn't cover.

carltongibson · 2017-08-26T06:45:28Z

@moorchegue Does the url parameter on SchemaGenerator not give you what you need?

moorchegue · 2017-08-27T11:38:16Z

Oh, yes, it does. Thanks a lot!

tomchristie added the Needs further review label Aug 15, 2016

tomchristie modified the milestone: 3.4.5 Release Aug 15, 2016

tomchristie closed this as completed Aug 19, 2016

tomchristie mentioned this issue Oct 7, 2016

Version 3.5 #4525

Merged

26 tasks

tomchristie added this to the 3.5.0 Release milestone Oct 7, 2016

tomchristie reopened this Oct 7, 2016

tomchristie added a commit that referenced this issue Oct 7, 2016

Handle schema structure correctly when path prefixs exist. Closes #4401

69b4acd

tomchristie closed this as completed Oct 7, 2016

tomchristie mentioned this issue Oct 10, 2016

Version 3.5 #4549

Closed

34 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Schemas should automatically respect API root location #4401

Schemas should automatically respect API root location #4401

nick-allen commented Aug 14, 2016

tomchristie commented Aug 19, 2016

klada commented Oct 7, 2016

tomchristie commented Oct 7, 2016

tomchristie commented Oct 7, 2016 •

edited

Loading

nick-allen commented Oct 7, 2016

tomchristie commented Oct 7, 2016

tomchristie commented Oct 7, 2016

tomchristie commented Oct 7, 2016 •

edited

Loading

klada commented Nov 10, 2016

moorchegue commented Aug 25, 2017

carltongibson commented Aug 25, 2017

moorchegue commented Aug 26, 2017

carltongibson commented Aug 26, 2017

moorchegue commented Aug 27, 2017

Schemas should automatically respect API root location #4401

Schemas should automatically respect API root location #4401

Comments

nick-allen commented Aug 14, 2016

Steps to reproduce

Expected behavior

Actual behavior

tomchristie commented Aug 19, 2016

klada commented Oct 7, 2016

tomchristie commented Oct 7, 2016

tomchristie commented Oct 7, 2016 • edited Loading

nick-allen commented Oct 7, 2016

tomchristie commented Oct 7, 2016

tomchristie commented Oct 7, 2016

tomchristie commented Oct 7, 2016 • edited Loading

klada commented Nov 10, 2016

moorchegue commented Aug 25, 2017

carltongibson commented Aug 25, 2017

moorchegue commented Aug 26, 2017

carltongibson commented Aug 26, 2017

moorchegue commented Aug 27, 2017

tomchristie commented Oct 7, 2016 •

edited

Loading

tomchristie commented Oct 7, 2016 •

edited

Loading