From a1d7aa8f712b659f9d8302a2d2a098d2538e6c89 Mon Sep 17 00:00:00 2001 From: Paul Melnikow Date: Thu, 2 Jan 2014 17:44:47 -0500 Subject: Allow viewset to specify lookup value regex for routing This patch allows a viewset to define a pattern for its lookup field, which the router will honor. Without this patch, any characters are allowed in the lookup field, and overriding this behavior requires subclassing router and copying and pasting the implementation of get_lookup_regex. It's possible it would be better to remove this functionality from the routers and simply expose a parameter to get_lookup_regex which allows overriding the lookup_regex. That way the viewset config logic could be in the a subclass, which could invoke the super method directly. I'm using this now for PostgreSQL UUID fields using https://github.com/dcramer/django-uuidfield . Without this patch, that field passes the lookup string to the database driver, which raises a DataError to complain about the invalid UUID. It's possible the field ought to signal this error in a different way, which could obviate the need to specify a pattern. --- rest_framework/routers.py | 20 ++++++++++++++------ rest_framework/tests/test_routers.py | 21 +++++++++++++++++++++ 2 files changed, 35 insertions(+), 6 deletions(-) (limited to 'rest_framework') diff --git a/rest_framework/routers.py b/rest_framework/routers.py index 740d58f0..8766ecb2 100644 --- a/rest_framework/routers.py +++ b/rest_framework/routers.py @@ -219,13 +219,21 @@ class SimpleRouter(BaseRouter): https://github.com/alanjds/drf-nested-routers """ - if self.trailing_slash: - base_regex = '(?P<{lookup_prefix}{lookup_field}>[^/]+)' - else: - # Don't consume `.json` style suffixes - base_regex = '(?P<{lookup_prefix}{lookup_field}>[^/.]+)' + base_regex = '(?P<{lookup_prefix}{lookup_field}>{lookup_value})' lookup_field = getattr(viewset, 'lookup_field', 'pk') - return base_regex.format(lookup_field=lookup_field, lookup_prefix=lookup_prefix) + try: + lookup_value = viewset.lookup_value_regex + except AttributeError: + if self.trailing_slash: + lookup_value = '[^/]+' + else: + # Don't consume `.json` style suffixes + lookup_value = '[^/.]+' + return base_regex.format( + lookup_prefix=lookup_prefix, + lookup_field=lookup_field, + lookup_value=lookup_value + ) def get_urls(self): """ diff --git a/rest_framework/tests/test_routers.py b/rest_framework/tests/test_routers.py index 1c34648f..0f6d62c7 100644 --- a/rest_framework/tests/test_routers.py +++ b/rest_framework/tests/test_routers.py @@ -121,6 +121,27 @@ class TestCustomLookupFields(TestCase): ) +class TestLookupValueRegex(TestCase): + """ + Ensure the router honors lookup_value_regex when applied + to the viewset. + """ + def setUp(self): + class NoteViewSet(viewsets.ModelViewSet): + queryset = RouterTestModel.objects.all() + lookup_field = 'uuid' + lookup_value_regex = '[0-9a-f]{32}' + + self.router = SimpleRouter() + self.router.register(r'notes', NoteViewSet) + self.urls = self.router.urls + + def test_urls_limited_by_lookup_value_regex(self): + expected = ['^notes/$', '^notes/(?P[0-9a-f]{32})/$'] + for idx in range(len(expected)): + self.assertEqual(expected[idx], self.urls[idx].regex.pattern) + + class TestTrailingSlashIncluded(TestCase): def setUp(self): class NoteViewSet(viewsets.ModelViewSet): -- cgit v1.2.3 From 3cd15fb1713dfc49e1bf1fd48045ca3ae5654e18 Mon Sep 17 00:00:00 2001 From: Paul Melnikow Date: Sat, 4 Jan 2014 16:57:50 -0500 Subject: Router: Do not automatically adjust lookup_regex when trailing_slash is True BREAKING CHANGE When trailing_slash is set to True, the router no longer will adjust the lookup regex to allow it to include periods. To simulate the old behavior, the programmer should specify `lookup_regex = '[^/]+'` on the viewset. https://github.com/tomchristie/django-rest-framework/pull/1328#issuecomment-31517099 --- rest_framework/routers.py | 7 ++----- rest_framework/tests/test_routers.py | 2 +- 2 files changed, 3 insertions(+), 6 deletions(-) (limited to 'rest_framework') diff --git a/rest_framework/routers.py b/rest_framework/routers.py index 8766ecb2..df1233fd 100644 --- a/rest_framework/routers.py +++ b/rest_framework/routers.py @@ -224,11 +224,8 @@ class SimpleRouter(BaseRouter): try: lookup_value = viewset.lookup_value_regex except AttributeError: - if self.trailing_slash: - lookup_value = '[^/]+' - else: - # Don't consume `.json` style suffixes - lookup_value = '[^/.]+' + # Don't consume `.json` style suffixes + lookup_value = '[^/.]+' return base_regex.format( lookup_prefix=lookup_prefix, lookup_field=lookup_field, diff --git a/rest_framework/tests/test_routers.py b/rest_framework/tests/test_routers.py index 0f6d62c7..e41da57f 100644 --- a/rest_framework/tests/test_routers.py +++ b/rest_framework/tests/test_routers.py @@ -152,7 +152,7 @@ class TestTrailingSlashIncluded(TestCase): self.urls = self.router.urls def test_urls_have_trailing_slash_by_default(self): - expected = ['^notes/$', '^notes/(?P[^/]+)/$'] + expected = ['^notes/$', '^notes/(?P[^/.]+)/$'] for idx in range(len(expected)): self.assertEqual(expected[idx], self.urls[idx].regex.pattern) -- cgit v1.2.3