drf-extensions icon indicating copy to clipboard operation
drf-extensions copied to clipboard

Allow optional regex changes for parents query lookups in nested routers

Open frwickst opened this issue 10 years ago • 5 comments

Hi! I noticed that when using a ExtendedSimpleRouter and registering routes that might have a dot "." in their lookup regex will cause the lookup to fail. I presume that the prefix creation here here was made with pk lookups in mind. However, there might be (as in my case) times where dots are a part of the API endpoint. As an example an email address.

I noticed that there is a function here that indicates if dots are to be used in the regex or not. And after DRF 2.4 it seems that dots are not to be matched in URLs (according to that file). However, the way the code is set up right now there is no way of changing the default behavior or the regex lookup. I propose that a optional parameter, parents_query_lookups_regex, be implemented in the register() function, which would give the caller more flexibility when it comes to matching routes.

I'm quite new with the code base so please excuse me if I have overlooked some part of the router setup or if I'm making a complete ass out of myself asking for such implementations :)

frwickst avatar Nov 03 '14 12:11 frwickst

Hi @frwickst.

I think we should somehow reuse default get_lookup_regex. Then you could specify regex of your pk lookup right in the viewset:

class MyModelViewSet(mixins.RetrieveModelMixin, viewsets.GenericViewSet):
    lookup_value_regex = '.+@.+\..+'  # super email regex

Don't hesitate to make a pull request =)

chibisov avatar Nov 04 '14 10:11 chibisov

Hi @chibisov.

I'm very interesting in investing time in this issue, because I'm fighting with a use-case to register two sets of urls using something like:

user_pk_routes = router.register('users', UserViewSet, base_name='user')
user_username_routes = router.register('users', UserViewSet, base_name='user.username')

and then attaching nested routes to the user with lookups by username or pk:

user_pk_routes.register('posts', PostViewSet, base_name='user-post', parents_query_lookups=['user'])
...
user_username_routes.register('posts', PostViewSet, base_name='user.username-post', parents_query_lookups=['user__username'])

In effect I want:

r'^users/(?P<pk>[0-9]+)/posts/$'
r'^users/(?P<username>[^/.]*[a-zA-Z][^/.]*)/posts/$'

By using a simple for loop I would, in theory, be able create two sets of routes for the same viewset.

The issue I have right now is that I have 2 non-conflicting regexes for username and pk (I don't allow to register usernames that would look like pk), but since the parent regex that is used is always [^/.] either the username or the pk lookup fails, depending on which ones goes first in the urlconf.

Then you could specify regex of your pk lookup right in the viewset

The good idea about this approach is that we are reusing functionality from DRF, the bad idea is that I am forced to create a i.e. UserUsernameViewSet and extend from UserViewSet because they have the same logic, but different routing rules, which I would like to avoid, as it's not DRY at all.

maryokhin avatar Dec 11 '14 16:12 maryokhin

Hi @maryokhin

Why do you think you break DRY by extending UserViewSet? For example, you could iteratively create new viewsets based on UserViewSet and register them in router (example below doesn't evaluates base_name correctly):

for _lookup_field, _lookup_value_regex in [
    ('pk', '[0-9]+'), 
    ('username', '[^/.]*[a-zA-Z][^/.]*')
]:
    class ExtendedUserViewSet(UserViewSet):
        lookup_field = _lookup_field
        lookup_value_regex = _lookup_value_regex
    (
        router.register('users', ExtendedUserViewSet, base_name='user')
              .register('posts', PostViewSet, base_name='user.post', parents_query_lookups=['user__username'])
    )

Anyway I think first of all we should implement usage of get_lookup_regex.

chibisov avatar Dec 11 '14 17:12 chibisov

I haven't thought of dynamic view creation, maybe it is a solution. It would become a bit messy with dynamically handling base_name and parents_query_lookups for nested routes, which is why #55 might be in a way related.

I have also tried to propose allowing to specify viewset instances on the router, which would have allowed to make it less awkward, along the lines of:

for _lookup_field, _lookup_value_regex in [
    ('pk', '[0-9]+'), 
    ('username', '[^/.]*[a-zA-Z][^/.]*')
]:
    (
        router.register('users', UserViewSet.as_view(lookup_field = _lookup_field,  lookup_value_regex = _lookup_value_regex), base_name='user')
              .register('posts', PostViewSet, base_name='user.post', parents_query_lookups=['user__username'])
    )

But I'm guessing that's not going to happen.

Anyway I think first of all we should implement usage of get_lookup_regex.

Agree with this as being a good starting point, I will start looking into it.

maryokhin avatar Dec 11 '14 18:12 maryokhin

are the discussed issued solved? or anything remain?

auvipy avatar Mar 30 '16 20:03 auvipy