lookup_field on hyperlinked fields, and overriddable hyperlinked fields. Closes #688

1 files changed, 89 insertions, 58 deletions

diff --git a/rest_framework/relations.py b/rest_framework/relations.py
index abe5203b..6d8deec1 100644
--- a/rest_framework/relations.py
+++ b/rest_framework/relations.py
@@ -288,10 +288,8 @@ class HyperlinkedRelatedField(RelatedField):
     """
     Represents a relationship using hyperlinking.
     """
-    pk_url_kwarg = 'pk'
-    slug_field = 'slug'
-    slug_url_kwarg = None  # Defaults to same as `slug_field` unless overridden
     read_only = False
+    lookup_field = 'pk'
 
     default_error_messages = {
         'no_match': _('Invalid hyperlink - No URL match'),
@@ -301,69 +299,120 @@ class HyperlinkedRelatedField(RelatedField):
         'incorrect_type': _('Incorrect type.  Expected url string, received %s.'),
     }
 
+    # These are all pending deprecation
+    pk_url_kwarg = 'pk'
+    slug_field = 'slug'
+    slug_url_kwarg = None  # Defaults to same as `slug_field` unless overridden
+
     def __init__(self, *args, **kwargs):
         try:
             self.view_name = kwargs.pop('view_name')
         except KeyError:
             raise ValueError("Hyperlinked field requires 'view_name' kwarg")
 
+        self.lookup_field = kwargs.pop('lookup_field', self.lookup_field)
+        self.format = kwargs.pop('format', None)
+
+        # These are pending deprecation
+        self.pk_url_kwarg = kwargs.pop('pk_url_kwarg', self.pk_url_kwarg)
         self.slug_field = kwargs.pop('slug_field', self.slug_field)
         default_slug_kwarg = self.slug_url_kwarg or self.slug_field
-        self.pk_url_kwarg = kwargs.pop('pk_url_kwarg', self.pk_url_kwarg)
         self.slug_url_kwarg = kwargs.pop('slug_url_kwarg', default_slug_kwarg)
 
-        self.format = kwargs.pop('format', None)
         super(HyperlinkedRelatedField, self).__init__(*args, **kwargs)
 
-    def get_slug_field(self):
+    def get_url(self, obj, view_name, request, format):
         """
-        Get the name of a slug field to be used to look up by slug.
-        """
-        return self.slug_field
-
-    def to_native(self, obj):
-        view_name = self.view_name
-        request = self.context.get('request', None)
-        format = self.format or self.context.get('format', None)
-
-        if request is None:
-            warnings.warn("Using `HyperlinkedRelatedField` without including the "
-                          "request in the serializer context is deprecated. "
-                          "Add `context={'request': request}` when instantiating the serializer.",
-                          DeprecationWarning, stacklevel=4)
+        Given an object, return the URL that hyperlinks to the object.
 
-        pk = getattr(obj, 'pk', None)
-        if pk is None:
-            return
-        kwargs = {self.pk_url_kwarg: pk}
+        May raise a `NoReverseMatch` if the `view_name` and `lookup_field`
+        attributes are not configured to correctly match the URL conf.
+        """
+        lookup_field = getattr(obj, self.lookup_field)
+        kwargs = {self.lookup_field: lookup_field}
         try:
             return reverse(view_name, kwargs=kwargs, request=request, format=format)
         except NoReverseMatch:
             pass
 
+        if self.pk_url_kwarg != 'pk':
+            # Only try pk if it has been explicitly set.
+            # Otherwise, the default `lookup_field = 'pk'` has us covered.
+            pk = obj.pk
+            kwargs = {self.pk_url_kwarg: pk}
+            try:
+                return reverse(view_name, kwargs=kwargs, request=request, format=format)
+            except NoReverseMatch:
+                pass
+
         slug = getattr(obj, self.slug_field, None)
+        if slug is not None:
+            # Only try slug if it corresponds to an attribute on the object.
+            kwargs = {self.slug_url_kwarg: slug}
+            try:
+                return reverse(view_name, kwargs=kwargs, request=request, format=format)
+            except NoReverseMatch:
+                pass
 
-        if not slug:
-            raise Exception('Could not resolve URL for field using view name "%s"' % view_name)
+        raise NoReverseMatch()
 
-        kwargs = {self.slug_url_kwarg: slug}
-        try:
-            return reverse(view_name, kwargs=kwargs, request=request, format=format)
-        except NoReverseMatch:
-            pass
+    def get_object(self, queryset, view_name, view_args, view_kwargs):
+        """
+        Return the object corresponding to a matched URL.
 
-        kwargs = {self.pk_url_kwarg: obj.pk, self.slug_url_kwarg: slug}
+        Takes the matched URL conf arguments, and the queryset, and should
+        return an object instance, or raise an `ObjectDoesNotExist` exception.
+        """
+        lookup = view_kwargs.get(self.lookup_field, None)
+        pk = view_kwargs.get(self.pk_url_kwarg, None)
+        slug = view_kwargs.get(self.slug_url_kwarg, None)
+
+        if lookup is not None:
+            filter_kwargs = {self.lookup_field: lookup}
+        elif pk is not None:
+            filter_kwargs = {'pk': pk}
+        elif slug is not None:
+            filter_kwargs = {self.slug_field: slug}
+        else:
+            raise ObjectDoesNotExist()
+
+        return queryset.get(**filter_kwargs)
+
+    def to_native(self, obj):
+        view_name = self.view_name
+        request = self.context.get('request', None)
+        format = self.format or self.context.get('format', None)
+
+        if request is None:
+            msg = (
+                "Using `HyperlinkedRelatedField` without including the request "
+                "in the serializer context is deprecated. "
+                "Add `context={'request': request}` when instantiating "
+                "the serializer."
+            )
+            warnings.warn(msg, DeprecationWarning, stacklevel=4)
+
+        # If the object has not yet been saved then we cannot hyperlink to it.
+        if getattr(obj, 'pk', None) is None:
+            return
+
+        # Return the hyperlink, or error if incorrectly configured.
         try:
-            return reverse(view_name, kwargs=kwargs, request=request, format=format)
+            return self.get_url(obj, view_name, request, format)
         except NoReverseMatch:
-            pass
-
-        raise Exception('Could not resolve URL for field using view name "%s"' % view_name)
+            msg = (
+                'Could not resolve URL for hyperlinked relationship using '
+                'view name "%s". You may have failed to include the related '
+                'model in your API, or incorrectly configured the '
+                '`lookup_field` attribute on this field.'
+            )
+            raise Exception(msg % view_name)
 
     def from_native(self, value):
         # Convert URL -> model instance pk
         # TODO: Use values_list
-        if self.queryset is None:
+        queryset = self.queryset
+        if queryset is None:
             raise Exception('Writable related fields must include a `queryset` argument')
 
         try:
@@ -387,29 +436,11 @@ class HyperlinkedRelatedField(RelatedField):
         if match.view_name != self.view_name:
             raise ValidationError(self.error_messages['incorrect_match'])
 
-        pk = match.kwargs.get(self.pk_url_kwarg, None)
-        slug = match.kwargs.get(self.slug_url_kwarg, None)
-
-        # Try explicit primary key.
-        if pk is not None:
-            queryset = self.queryset.filter(pk=pk)
-        # Next, try looking up by slug.
-        elif slug is not None:
-            slug_field = self.get_slug_field()
-            queryset = self.queryset.filter(**{slug_field: slug})
-        # If none of those are defined, it's probably a configuation error.
-        else:
-            raise ValidationError(self.error_messages['configuration_error'])
-
         try:
-            obj = queryset.get()
-        except ObjectDoesNotExist:
+            return self.get_object(queryset, match.view_name,
+                                   match.args, match.kwargs)
+        except (ObjectDoesNotExist, TypeError, ValueError):
             raise ValidationError(self.error_messages['does_not_exist'])
-        except (TypeError, ValueError):
-            msg = self.error_messages['incorrect_type']
-            raise ValidationError(msg % type(value).__name__)
-
-        return obj
 
 
 class HyperlinkedIdentityField(Field):