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

Add support for file extension-based content type overriding #5

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

isoschiz
Copy link

Small patch to add support for checking the file extension of the request path and using that as an override (cf. using a request parameter like "format=json").

Support for this behaviour is switched on in the view class using a new class configuration variable "_format_override_extension", so there is no effect on existing users.

We use this to provide cleaner and more proxy-friendly URLs for our APIs: rather than:

http://api.example.com/foo/bar?format=json

this supports:

http://api.example.com/foo/bar.json

@alexdutton
Copy link
Contributor

Thanks for contributing! Looks sane to me, but I'm wondering if it shouldn't happen somewhere on the View.request() chain, at which point it can remove the extension from the path before it gets to the view code. (This may be an unnecessary requirement, as most people won't be looking at the original URL once it's gotten through the resolver.)

Another approach might be to have View.request() pop the format from **kwargs, as matched by the resolver. For example:

urlpatterns = patterns('',
    url(r'^/book/(?P\d+)(?:.(P[a-z\d]+))?$', views.BookDetailView.as_view(), name='book-detail'),
    …
)

class BookView(HTMLView, JSONView):
    _format_extension_parameter = 'format'
    template_name = 'book-detail'

    def get(request, id):
        …
        return self.render()

This would be a bit more explicit (albeit at the cost of verbosity). However, I imagine one's urlconf will want to match the extension part explicitly anyway. (Also, if some soul wants to do http://api.example.com/xml/foo/bar, they'd be able to.)

@isoschiz
Copy link
Author

This is effectively done in the dispatch chain of the View (I assume this is what you meant?), so it could strip off the extension at this point before passing it to the parent dispatch, and from there onto the user-defined handlers. However, I'm not sure I completely approve of this, and as you say it's unlikely to be necessary. So could add a new flag to do that (though personally would consider it a separate patch/feature).

I considered doing something at the URL dispatcher level, but decided against because it was more of a change, and is more fragile (in the sense the user has to do several things in concert to get the feature to work).

This patch won for me because it's so self contained, simple (both the change and the new option it provides), backwards compatible in every way and offers full flexibility for doing whatever you need...

(...within the confines of using a file extension to determine the type. :-) For your last suggestion, I would probably just have multiple URL dispatcher lines (one per format), each mapping onto the same sub patterns but with a different view class type:

class ForceContentType(HTMLView, JSONView):
    def __init__(self, force_type=None):
        super().__init__()
        self._force_fallback_format = force_type

def api_urls(type=None):
    return patterns('', url(r'^foo/bar', ForceContentType.as_view(force_type=type)), ...)

urlpatterns = patterns('', url(r'^xml/', include(api_urls('xml'))))

This is more explicit and robust, and could already be done today.)

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

Successfully merging this pull request may close these issues.

2 participants