REST: Remove '_url' suffixes

This was a design decision made when implementing the REST API. The
idea was that these items were URLs to related objects and should be
indicated as such. However, this was a faulty assumption as the
Patchwork API, unlike other some other APIs (GitHub), does not also
include a full representation of said objects, like so:

    {
      "url": "http://localhost:8000/api/1.0/patches/1/",
      ...
      "delegate_url": "http://localhost:8000/api/1.0/users/1",
      "delegate": {
        "url": "http://localhost:8000/api/1.0/users/1/",
        "username": "admin",
        "first_name": "",
        "last_name": "",
        "email": ""
      }
    }

Since there is no intention to support this design yet, there isn't
really any reason to fight django-rest-framework in appending these
suffixes. Simply remove them. This changes the output for most endpoints
from something like this:

    {
      "url": "http://localhost:8000/api/1.0/patches/1",
      ...
      "delegate_url": "http://localhost:8000/api/1.0/users/1"
    }

to:

    {
      "url": "http://localhost:8000/api/1.0/patches/1",
      ...
      "delegate": "http://localhost:8000/api/1.0/users/1"
    }

This will affect all endpoints with nested resources.

Note that the API version is not bumped as the API is still considered
unreleased.

Signed-off-by: Stephen Finucane <stephen@that.guru>
Reviewed-by: Andy Doan <andy.doan@linaro.org>
Reviewed-by: Daniel Axtens <dja@axtens.net>

diff --git a/patchwork/api/base.py b/patchwork/api/base.py
index 7333a7f26e4e179900d35f22af5ac6cf905c4e05..3639ebe487f494661f795e764b40470cea2e440b 100644 (file)
--- a/patchwork/api/base.py
+++ b/patchwork/api/base.py
@@ -21,22 +21,9 @@ from django.conf import settings
 from rest_framework import permissions
 from rest_framework.pagination import PageNumberPagination
 from rest_framework.response import Response
-from rest_framework.serializers import HyperlinkedModelSerializer
-from rest_framework.serializers import HyperlinkedRelatedField
 from rest_framework.viewsets import ModelViewSet
 
 
-class URLSerializer(HyperlinkedModelSerializer):
-    """Just like parent but puts _url for fields"""
-
-    def to_representation(self, instance):
-        data = super(URLSerializer, self).to_representation(instance)
-        for name, field in self.fields.items():
-            if isinstance(field, HyperlinkedRelatedField) and name != 'url':
-                data[name + '_url'] = data.pop(name)
-        return data
-
-
 class LinkHeaderPagination(PageNumberPagination):
     """Provide pagination based on rfc5988.
 

diff --git a/patchwork/api/check.py b/patchwork/api/check.py
index 31ade0758286419021a3fc1197308bc17db57cbd..26a25958ec0e383dab8d04cc15803257256ddc0f 100644 (file)
--- a/patchwork/api/check.py
+++ b/patchwork/api/check.py
@@ -58,7 +58,6 @@ class CheckSerializer(ModelSerializer):
         url = self.context['request'].build_absolute_uri(reverse(
             'api_1.0:patch-detail', args=[instance.patch.id]))
         data['url'] = url + 'checks/%s/' % instance.id
-        data['users_url'] = data.pop('user')
         return data
 
     class Meta:

diff --git a/patchwork/api/patch.py b/patchwork/api/patch.py
index c36a11bfe2a5105b155e0c24472d96b264f7a06b..2dc77d16e423a135c6ee6adf743b2a231af661ea 100644 (file)
--- a/patchwork/api/patch.py
+++ b/patchwork/api/patch.py
@@ -19,12 +19,12 @@
 
 import email.parser
 
+from rest_framework.serializers import HyperlinkedModelSerializer
 from rest_framework.serializers import ListSerializer
 from rest_framework.serializers import SerializerMethodField
 
 from patchwork.api.base import PatchworkPermission
 from patchwork.api.base import PatchworkViewSet
-from patchwork.api.base import URLSerializer
 from patchwork.models import Patch
 
 
@@ -37,8 +37,8 @@ class PatchListSerializer(ListSerializer):
         return super(PatchListSerializer, self).to_representation(data)
 
 
-class PatchSerializer(URLSerializer):
-    mbox_url = SerializerMethodField()
+class PatchSerializer(HyperlinkedModelSerializer):
+    mbox = SerializerMethodField()
     state = SerializerMethodField()
 
     class Meta:
@@ -53,13 +53,13 @@ class PatchSerializer(URLSerializer):
     def get_state(self, obj):
         return obj.state.name
 
-    def get_mbox_url(self, patch):
+    def get_mbox(self, patch):
         request = self.context.get('request', None)
         return request.build_absolute_uri(patch.get_mbox_url())
 
     def to_representation(self, instance):
         data = super(PatchSerializer, self).to_representation(instance)
-        data['checks_url'] = data['url'] + 'checks/'
+        data['checks'] = data['url'] + 'checks/'
         data['check'] = instance.combined_check_state
         headers = data.get('headers')
         if headers is not None:

diff --git a/patchwork/api/person.py b/patchwork/api/person.py
index b1168f7460cfca6056a89cd3c99b7c7be32c29f3..758b21d46101913535b52c5a1622d73081f1c962 100644 (file)
--- a/patchwork/api/person.py
+++ b/patchwork/api/person.py
@@ -17,13 +17,14 @@
 # along with Patchwork; if not, write to the Free Software
 # Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
 
+from rest_framework.serializers import HyperlinkedModelSerializer
+
 from patchwork.api.base import AuthenticatedReadOnly
 from patchwork.api.base import PatchworkViewSet
-from patchwork.api.base import URLSerializer
 from patchwork.models import Person
 
 
-class PersonSerializer(URLSerializer):
+class PersonSerializer(HyperlinkedModelSerializer):
     class Meta:
         model = Person
         fields = ('email', 'name', 'user')

diff --git a/patchwork/tests/test_rest_api.py b/patchwork/tests/test_rest_api.py
index 44a28599734b17ad8df8269d7337e5b952d089e4..3be8ecf4345bba21d3465b414523ae1321d455f6 100644 (file)
--- a/patchwork/tests/test_rest_api.py
+++ b/patchwork/tests/test_rest_api.py
@@ -175,7 +175,7 @@ class TestPersonAPI(APITestCase):
         self.assertEqual(1, len(resp.data))
         self.assertEqual(user.username, resp.data[0]['name'])
         self.assertEqual(user.email, resp.data[0]['email'])
-        self.assertIn('users/%d/' % user.id, resp.data[0]['user_url'])
+        self.assertIn('users/%d/' % user.id, resp.data[0]['user'])
 
     def test_unlinked_user(self):
         person = create_person()
@@ -187,7 +187,7 @@ class TestPersonAPI(APITestCase):
         self.assertEqual(2, len(resp.data))
         self.assertEqual(person.name,
                          resp.data[0]['name'])
-        self.assertIsNone(resp.data[0]['user_url'])
+        self.assertIsNone(resp.data[0]['user'])
 
     def test_readonly(self):
         user = create_maintainer()
@@ -291,13 +291,13 @@ class TestPatchAPI(APITestCase):
         self.assertEqual(status.HTTP_200_OK, resp.status_code)
         self.assertEqual(patch.name, resp.data['name'])
         self.assertIn(TestProjectAPI.api_url(patch.project.id),
-                      resp.data['project_url'])
+                      resp.data['project'])
         self.assertEqual(patch.msgid, resp.data['msgid'])
         self.assertEqual(patch.diff, resp.data['diff'])
         self.assertIn(TestPersonAPI.api_url(patch.submitter.id),
-                      resp.data['submitter_url'])
+                      resp.data['submitter'])
         self.assertEqual(patch.state.name, resp.data['state'])
-        self.assertIn(patch.get_mbox_url(), resp.data['mbox_url'])
+        self.assertIn(patch.get_mbox_url(), resp.data['mbox'])
 
     def test_detail_tags(self):
         patch = create_patch(