Project

Profile

Help

Issue #2814

API for resources that use HyperlinkedRelatedFields in serializers require full URL of related resource

Added by dkliban@redhat.com over 2 years ago. Updated 6 months ago.

Status:
CLOSED - NOTABUG
Priority:
Normal
Assignee:
-
Category:
-
Sprint/Milestone:
Start date:
Due date:
Severity:
2. Medium
Version:
Platform Release:
Blocks Release:
OS:
Backwards Incompatible:
No
Triaged:
Yes
Groomed:
No
Sprint Candidate:
No
Tags:
QA Contact:
Complexity:
Smash Test:
Verified:
No
Verification Required:
No
Sprint:

Description

For example, when creating an Importer, the 'repository' needs to be specifed as a URL:

{
    "type": "file",
    "name": "test-importerr",
    "last_updated": "2017-06-12T19:26:36.472529Z",
    "feed_url": "https://repos.fedorapeople.org/pulp/pulp/demo_repos/test_file_repo",
    "validate": false,
    "ssl_validation": false,
    "proxy_url": "",
    "download_policy": "immediate",
    "last_sync": null,
    "repository": "http://localhost:1234/api/v3/repositories/test-repo/" 
}

It is desirable for the API to return URLs so the client can easily find the the related resource, however, the API should accept a resource Id. In this case it would be 'test-repo'.

History

#1 Updated by ttereshc over 2 years ago

  • Triaged changed from No to Yes

#2 Updated by dkliban@redhat.com over 2 years ago

The patch below modifies the RepositoryRelatedField to accept a repository name from API requests. However, this breaks the nice forms you get from django-rest-framework. The form that drf renders for creating/updating an importer, stores the URL for the value of the 'repository' field. That value is produced by the to_representation() method of the field. We need to somehow produce one value when the value is requested for rendering the form template and another value when generating JSON representation of the resource.

diff --git a/platform/pulpcore/app/serializers/fields.py b/platform/pulpcore/app/serializers/fields.py
index 55e9556..9ec59a7 100644
--- a/platform/pulpcore/app/serializers/fields.py
+++ b/platform/pulpcore/app/serializers/fields.py
@@ -19,6 +19,16 @@ class RepositoryRelatedField(serializers.HyperlinkedRelatedField):
     lookup_field = 'name'
     queryset = models.Repository.objects.all()

+    def __init__(self):
+        super().__init__()
+        self.error_messages['does_not_exist'] = "The id does not match an existing repository." 
+
+    def to_internal_value(self, data):
+        try:
+            return self.get_queryset().get(name=data)
+        except models.Repository.DoesNotExist:
+            self.fail('does_not_exist')
+

#3 Updated by mhrivnak over 2 years ago

This is a common misconception about REST APIs, that is likely a remnant of preconceptions most of us have from using RPC. A critical aspect of REST is that resources must be identified via hypertext. This is the Uniform way that Resources are supposed to be Identified. ;)

This entertaining rant from the original creator for REST is a decent starting point for reading:

http://roy.gbiv.com/untangled/2008/rest-apis-must-be-hypertext-driven

This is a more gentle introduction to these ideas:

http://timelessrepo.com/haters-gonna-hateoas

I boil a lot of those details down to a simple guiding philosophy: the more time an API user has to spend reading documentation about how to construct requests, the less we're achieving REST.

I'm not sure if we can bite off full "level 4" RESTfulness (as the above article describes) right now. But we can make big improvements from Pulp 2, which has more of a "Web API" than a REST API. Allowing resources to be identified solely by their URL is a good start. How we uniquely identify resources internally within Pulp is separate from how API users will identify those resources. That's a mindset that takes some getting used to.

I propose we close this. But I know this is a big topic, so I'm happy to discuss it more if anyone would like.

#4 Updated by dkliban@redhat.com over 2 years ago

I am convinced that using hyperlinks to identify is appropriate. I am happy to close it also.

#5 Updated by bmbouter over 2 years ago

  • Status changed from NEW to CLOSED - NOTABUG

I agree URLs should be the way all resources are referred to. I also agree this should be closed, so I'm closing as NOTABUG.

#6 Updated by daviddavis 6 months ago

  • Sprint/Milestone set to 3.0

#7 Updated by bmbouter 6 months ago

  • Tags deleted (Pulp 3)

Please register to edit this issue

Also available in: Atom PDF