[PATCH v3 03/16] REST: Remove '_url' suffixes
Stephen Finucane
stephen at that.guru
Sat Nov 26 05:18:22 AEDT 2016
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 at that.guru>
Reviewed-by: Andy Doan <andy.doan at linaro.org>
---
patchwork/api/base.py | 13 -------------
patchwork/api/check.py | 1 -
patchwork/api/patch.py | 10 +++++-----
patchwork/api/person.py | 5 +++--
patchwork/tests/test_rest_api.py | 10 +++++-----
5 files changed, 13 insertions(+), 26 deletions(-)
diff --git a/patchwork/api/base.py b/patchwork/api/base.py
index 7333a7f..3639ebe 100644
--- 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 31ade07..26a2595 100644
--- 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 c36a11b..2dc77d1 100644
--- 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 b1168f7..758b21d 100644
--- 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 44a2859..3be8ecf 100644
--- 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(
--
2.7.4
More information about the Patchwork
mailing list