proposal: DJA DefaultRouter

[n2ygk]

Proposal: rest_framework_json_api.routers.DefaultRouter

Current method of declaring URLPatterns

The current typical usage example is like this:

from rest_framework import routers

router = routers.DefaultRouter(trailing_slash=False)

router.register(r'blogs', BlogViewSet)
router.register(r'entries', EntryViewSet)

urlpatterns = [
    # basic collection and item URLs:
    url(r'^', include(router.urls)),
    # relationships URLs:
    url(r'^entries/(?P<pk>[^/.]+)/relationships/(?P<related_field>\w+)',
        EntryRelationshipView.as_view(),
        name='entry-relationships'),
    url(r'^blogs/(?P<pk>[^/.]+)/relationships/(?P<related_field>\w+)',
        BlogRelationshipView.as_view(),
        name='blog-relationships'),
   # related URLs:
    url(r'entries/(?P<entry_pk>[^/.]+)/blog',
        BlogViewSet.as_view({'get': 'retrieve'}),
        name='entry-blog'),
]

URLPatterns created by the Default Router

The above creates the following URLPatterns for router.register(r'blogs', BlogViewSet):

 00 = {URLPattern} <URLPattern '^blogs$' [name='blog-list']>
  default_args = {dict} {}
  lookup_str = {str} 'example.views.BlogViewSet'
  name = {str} 'blog-list'
  pattern = {RegexPattern} ^blogs$
   _is_endpoint = {bool} True
   _regex = {str} '^blogs$'
   _regex_dict = {dict} {}
   converters = {dict} {}
   name = {str} 'blog-list'
   regex = {SRE_Pattern} re.compile('^blogs$')
 01 = {URLPattern} <URLPattern '^blogs\.(?P<format>[a-z0-9]+)/?$' [name='blog-list']>
  default_args = {dict} {}
  lookup_str = {str} 'example.views.BlogViewSet'
  name = {str} 'blog-list'
  pattern = {RegexPattern} ^blogs\.(?P<format>[a-z0-9]+)/?$
   _is_endpoint = {bool} True
   _regex = {str} '^blogs\\.(?P<format>[a-z0-9]+)/?$'
   _regex_dict = {dict} {}
   converters = {dict} {}
   name = {str} 'blog-list'
   regex = {SRE_Pattern} re.compile('^blogs\\.(?P<format>[a-z0-9]+)/?$')
 02 = {URLPattern} <URLPattern '^blogs/(?P<pk>[^/.]+)$' [name='blog-detail']>
  default_args = {dict} {}
  lookup_str = {str} 'example.views.BlogViewSet'
  name = {str} 'blog-detail'
  pattern = {RegexPattern} ^blogs/(?P<pk>[^/.]+)$
   _is_endpoint = {bool} True
   _regex = {str} '^blogs/(?P<pk>[^/.]+)$'
   _regex_dict = {dict} {}
   converters = {dict} {}
   name = {str} 'blog-detail'
   regex = {SRE_Pattern} re.compile('^blogs/(?P<pk>[^/.]+)$')
 03 = {URLPattern} <URLPattern '^blogs/(?P<pk>[^/.]+)\.(?P<format>[a-z0-9]+)/?$' [name='blog-detail']>
  default_args = {dict} {}
  lookup_str = {str} 'example.views.BlogViewSet'
  name = {str} 'blog-detail'
  pattern = {RegexPattern} ^blogs/(?P<pk>[^/.]+)\.(?P<format>[a-z0-9]+)/?$

along with an api-root view that lists the available registry entries. This is a nice feature.

Drawbacks of the current method

• Relationships & related URLs follow a standard pattern yet have to be "manually" declared separately. This is hard to do, error prone and just plain annoying.
• We might not need the .<format> flavor of each ViewSet. (Not a huge concern.)

Proposed: rest_framework_json_api.DefaultRouter

Create a DJA DefaultRouter that extends DRF's DefaultRouter:

1. Add URLPatterns for relationships and their RelationshipView. This seems pretty straightforward. Just add another argument to DefaultRouter.register() (or apply a default naming pattern) to add the RelationshipView.
2. Add URLPatterns for related links. This is a bit harder as each related link has to be enumerated (link name and ViewSet).

The goal would be to have a URLPatterns that looks like this:

from rest_framework_json_api import routers

router = routers.DefaultRouter(trailing_slash=False)

router.register(r'blogs', BlogViewSet, BlogRelationshipView)
router.register(r'entries', EntryViewSet, EntryRelationshipView, ??related stuff here??)

urlpatterns = [
    url(r'^', include(router.urls)),
]

For how to do the "related stuff here" take a look at @Anton-Shutik's approach in #451

[sliverc]

I like the idea 👍

I think to register related urls as implemented in #451 will be fairly straight forward. With RelationshipView is going to be a bit harder but playing around with implementation might lead to a good solution. Only thing which also need to be considered is to implement a json api specific SimpleRouter and DefaultRouter as both might be used.

[n2ygk]

I have a naming question: Do we prefix everything with JSONAPI or override existing names in DRF? We now have both styles:

• DJA serializers override DRF's HyperlinkedModelSerializer and so on. The advantage is code written for DRF can upgrade to DJA by simply changing an import. The disadvantage is that it's easy to "forget" to change the import from rest_framework to rest_framework_json_api.
• DJA paginators are named JSONAPIPageNumberPagination instead of overriding DRF's PageNumberPagination. The advantage is that it's very obvious that this is DJA-specific. The disadvantage is that instead of changing one import, you have to change each class use instance.

The approach here could be either:

• DJA DefaultRouter overriding DRF DefaultRouter.
• DJA JSONAPIDefaultRouter

[sliverc]

Yeah I know I have also noticed. As we had to make backwards incompatible changes for pagination we started to add the JSONAPI prefix to work around this. There were also other cases in the past where this was done.

Actually I would prefer not to use JSONAPI prefix as it is already clear by the rest_framework_json_api module that it is imported from DJA. As you outlined this way someone can also easily switch between DRF and DJA version by simply changing the import.

However this won't be easy to accomplish to go that direction to make it consistent and backwards compatible. Potentially we have to break this at some point.

But for new things (actually haven't thought about that when reviewing the filter backends) we can easily leave out the JSONAPI prefix.

What is your preference?

[n2ygk]

As we had to make backwards incompatible changes for pagination

I don't think there was actually a backwards compatibility issue here as if one wants to be backward compatible they would just use rest_framework.pagination.PageNumberPagination. (Likewise for LimitOffset.)

Potentially we have to break this at some point.

I say sooner than later. In the last release (2.5.0), we used the prefix JsonApi for paginators which is already deprecated by #463. The backend filters are not yet in a released version so have never been "seen" by their JSONAPI-prefixed names.

The only other instance I was able to find was rest_framework_json_api.metadata.JSONAPIMetadata which extends rest_framework.metadata.SimpleMetadata; This could easily be renamed (with deprecation) to rest_framework_json_api.metadata.SimpleMetadata which would more clearly show the inheritance as well.

The only other prefix I saw like this is JSONAPIMeta serializer inner class which I think is required to stay that way to distinguish it from the Meta inner class.

What is your preference?

My preference is to put in the effort now:

• drop the JSONAPI prefix from django_filters.DjangoFilterBackend #466 before merging it. We have an open question (django_filters.DjangoFilterBackend #466 (comment)) around naming it as well.
• new PR to back out the prefix changes done in JSONAPI prefix for paginators #463 and eliminate the JsonApi prefix and add deprecation warnings for page vs. page[number] and page_size vs. page[size]. Or just document this as a breaking change: page and page_size were never correct JSON:API usage.
• new PR to rename/deprecate JSONAPIMetadata.

Do you agree?

[sliverc]

There is also JSONAPISettings which we could rename to APISettings.

Also I have noticed that we are kind of getting into the habit to misuse some issues to discuss things which might be related but not actually the same issue.

I think for outsiders to better follow and to better track issues (e.g. a PR should solve one issue and not a comment of an issue) it would be better to create new issues for this (in this case a new issue where discussion happens on JSONAPI prefix).

I will also follow up on a issue for versioning and deprecation policy.

[n2ygk]

I've "moved" this comment thread about naming to it's own issue: #471