concentricsky / django-tastypie-swagger

An adapter to use swagger-ui with django-tastypie.
Other
132 stars 144 forks source link

generated urls missing end / #89

Open oppianmatt opened 10 years ago

oppianmatt commented 10 years ago

In views.py we have:

class ResourcesView(TastypieApiMixin, SwaggerApiDataMixin, JSONView):
    """
    Provide a top-level resource listing for swagger

    This JSON must conform to https://github.com/wordnik/swagger-core/wiki/Resource-Listing
    """

    def get_context_data(self, *args, **kwargs):
        context = super(ResourcesView, self).get_context_data(*args, **kwargs)

        # Construct schema endpoints from resources
        apis = [{'path': '/%s' % name} for name in sorted(self.tastypie_api._registry.keys())]
        context.update({
            'basePath': self.request.build_absolute_uri(reverse('%s:schema' % self.kwargs.get('namespace'))),
            'apis': apis,
        })
        return context

it's creating links like /schema/resource

but in urls.py we have

url(r'^schema/(?P<resource>\S+)/$', SchemaView.as_view()),

note that it has a trailing slash

fix is to change the a line to:

apis = [{'path': '/%s/' % name} for name in sorted(self.tastypie_api._registry.keys())]

People aren't seeing the issue because there is usually middleware which adds a slash for when it 404s making it work.

oppianmatt commented 10 years ago

problem with that though is that it's designed to display the endpoints without a slash so when it does have a slash it look like a list of /

but really you shouldn't be relying on a 302 redirect to get to the right place from some middleware that might not be enabled

johnraz commented 10 years ago

Actually this is on purpose to match the swagger specifications The real fix would be I guess to remove the trailing slash from the url conf.

johnraz commented 10 years ago

Its not a matter of middleware btw but just a settings (APPEND_SLASH) edit: well its a combinaison of the 2 but who wouldn't use common middleware in django anyway ?

oppianmatt commented 10 years ago

you sir are correct. I found this issue when I put in a catch all that matches everything not matched by other urlconfs (because we needed to redirect old links) and that broke swagger. I've made it a negative match. If the url matched the url conf it wouldn't be affected. Looking at swagger when it works, you see a bunch of 301 redirects, so fixing this would improve performance slightly (and help others who have the setting off, or urls confs that will match)

johnraz commented 10 years ago

We are open to PR ;)