[PATCH v3 03/16] REST: Remove '_url' suffixes
Daniel Axtens
dja at axtens.net
Wed Nov 30 08:58:01 AEDT 2016
Stephen Finucane <stephen at that.guru> writes:
> 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"
> }
Thanks!
Reviewed-by: Daniel Axtens <dja at axtens.net>
Regards,
Daniel
>
> 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
>
> _______________________________________________
> Patchwork mailing list
> Patchwork at lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/patchwork
More information about the Patchwork
mailing list