From: Stephen Finucane Date: Thu, 24 Nov 2016 09:33:53 +0000 (+0000) Subject: REST: Remove '_url' suffixes X-Git-Tag: v2.0.0-rc1~127 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=d4939247040d07d8b63e3fe65207ee6e3f509c60;p=thirdparty%2Fpatchwork.git 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 Reviewed-by: Andy Doan Reviewed-by: Daniel Axtens --- diff --git a/patchwork/api/base.py b/patchwork/api/base.py index 7333a7f2..3639ebe4 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 31ade075..26a25958 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 c36a11bf..2dc77d16 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 b1168f74..758b21d4 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 44a28599..3be8ecf4 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(