Skip to content

Commit

Permalink
Add custom serializers for "_artifact" vs "_artifacts"
Browse files Browse the repository at this point in the history
Remove boilerplate from the plugins by defining new, more specialized
serializers for the single-artifact-per-content case, the
multiple-artifacts-per-content case, and the no-artifacts-per-content
case.

Required PR: pulp/pulp_file#159
Required PR: pulp/pulp-smash#1164
Required PR: pulp/pulpcore-plugin#38

re: #4366
https://pulp.plan.io/issues/4366
  • Loading branch information
dralley committed Jan 31, 2019
1 parent aaec68d commit 8c793f8
Show file tree
Hide file tree
Showing 7 changed files with 74 additions and 13 deletions.
7 changes: 6 additions & 1 deletion pulpcore/app/serializers/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,12 @@
AsyncOperationResponseSerializer
)
from .fields import BaseURLField, ContentRelatedField, LatestVersionField # noqa
from .content import ArtifactSerializer, ContentSerializer # noqa
from .content import ( # noqa
ArtifactSerializer,
NoArtifactContentSerializer,
SingleArtifactContentSerializer,
MultipleArtifactContentSerializer,
)
from .progress import ProgressReportSerializer # noqa
from .publication import ( # noqa
ContentGuardSerializer,
Expand Down
25 changes: 24 additions & 1 deletion pulpcore/app/serializers/content.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,32 @@
UNIQUE_ALGORITHMS = ['sha256', 'sha384', 'sha512']


class ContentSerializer(base.MasterModelSerializer):
class BaseContentSerializer(base.MasterModelSerializer):
_href = base.DetailIdentityField()

class Meta:
model = models.Content
fields = base.MasterModelSerializer.Meta.fields


class NoArtifactContentSerializer(BaseContentSerializer):

class Meta:
model = models.Content
fields = base.MasterModelSerializer.Meta.fields


class SingleArtifactContentSerializer(BaseContentSerializer):
_artifact = fields.SingleContentArtifactField(
help_text=_("Artifact file representing the physical content"),
)

class Meta:
model = models.Content
fields = base.MasterModelSerializer.Meta.fields + ('_artifact',)


class MultipleArtifactContentSerializer(BaseContentSerializer):
_artifacts = fields.ContentArtifactsField(
help_text=_("A dict mapping relative paths inside the Content to the corresponding"
"Artifact URLs. E.g.: {'relative/path': "
Expand Down
33 changes: 33 additions & 0 deletions pulpcore/app/serializers/fields.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,39 @@ class ContentRelatedField(DetailRelatedField):
queryset = models.Content.objects.all()


class SingleContentArtifactField(RelatedField):
"""
A serializer field for the '_artifacts' ManyToManyField on the Content model (single-artifact).
"""
lookup_field = 'pk'
view_name = 'artifacts-detail'
queryset = models.Artifact.objects.all()
allow_null = True

def get_attribute(self, instance):
"""
Returns the field from the instance that should be serialized using this serializer field.
This serializer looks up the list of artifacts and returns only one, if any exist. If more
than one exist, it throws and exception because this serializer is being used in an
improper context.
Args:
instance (:class:`pulpcore.app.models.Content`): An instance of Content being
serialized.
Returns:
A single Artifact model related to the instance of Content.
"""
try:
return instance._artifacts.get()
except models.Artifact.DoesNotExist:
return None
except models.Artifact.MultipleObjectsReturned:
raise ValueError(_("SingleContentArtifactField should not be used in a context where "
"multiple artifacts for one content is possible."))


class ContentArtifactsField(serializers.DictField):
"""
A serializer field for the '_artifacts' ManyToManyField on the Content model.
Expand Down
8 changes: 4 additions & 4 deletions pulpcore/app/viewsets/content.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
from rest_framework.response import Response

from pulpcore.app.models import Artifact, Content, ContentArtifact
from pulpcore.app.serializers import ArtifactSerializer, ContentSerializer
from pulpcore.app.serializers import ArtifactSerializer, MultipleArtifactContentSerializer
from pulpcore.app.viewsets.base import BaseFilterSet, NamedModelViewSet

from .custom_filters import (
Expand Down Expand Up @@ -80,7 +80,6 @@ class ContentFilter(BaseFilterSet):
class Meta:
model = Content
fields = {
'_type': ['exact', 'in'],
'repository_version': ['exact'],
'repository_version_added': ['exact'],
'repository_version_removed': ['exact'],
Expand All @@ -92,9 +91,10 @@ class ContentViewSet(NamedModelViewSet,
mixins.RetrieveModelMixin,
mixins.ListModelMixin):
endpoint_name = 'content'
queryset = Content.objects.all()
serializer_class = ContentSerializer
filterset_class = ContentFilter
# These are just placeholders, the plugin writer would replace them with the actual
queryset = Content.objects.all()
serializer_class = MultipleArtifactContentSerializer

@transaction.atomic
def create(self, request):
Expand Down
2 changes: 1 addition & 1 deletion pulpcore/tests/functional/api/using_plugin/test_orphans.py
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ def test_clean_orphan_content_unit(self):
)

# Verify that the artifact is present on disk.
artifact_path = self.api_client.get(content['artifact'])['file']
artifact_path = self.api_client.get(content['_artifact'])['file']
cmd = ('ls', artifact_path)
self.cli_client.run(cmd, sudo=True)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -324,7 +324,7 @@ def test_delete_first_version(self):
get_content(self.repo, self.repo_version_hrefs[0])
for repo_version_href in self.repo_version_hrefs[1:]:
artifact_paths = get_artifact_paths(self.repo, repo_version_href)
self.assertIn(self.content[0]['artifact'], artifact_paths)
self.assertIn(self.content[0]['_artifact'], artifact_paths)

def test_delete_last_version(self):
"""Delete the last repository version.
Expand All @@ -346,8 +346,8 @@ def test_delete_last_version(self):
self.repo = self.client.get(self.repo['_href'])
artifact_paths = get_artifact_paths(self.repo)

self.assertNotIn(self.content[-2]['artifact'], artifact_paths)
self.assertIn(self.content[-1]['artifact'], artifact_paths)
self.assertNotIn(self.content[-2]['_artifact'], artifact_paths)
self.assertIn(self.content[-1]['_artifact'], artifact_paths)

def test_delete_middle_version(self):
"""Delete a middle version."""
Expand All @@ -359,7 +359,7 @@ def test_delete_middle_version(self):

for repo_version_href in self.repo_version_hrefs[index + 1:]:
artifact_paths = get_artifact_paths(self.repo, repo_version_href)
self.assertIn(self.content[index]['artifact'], artifact_paths)
self.assertIn(self.content[index]['_artifact'], artifact_paths)

def test_delete_publication(self):
"""Delete a publication.
Expand Down
4 changes: 2 additions & 2 deletions pulpcore/tests/unit/content/test_handler.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,9 @@ class HandlerSaveContentTestCase(TestCase):

def setUp(self):
self.f1 = FileContent.objects.create(relative_path='f1', digest='1')
self.f1.artifact = None
ContentArtifact.objects.create(artifact=None, content=self.f1, relative_path='f1')
self.f2 = FileContent.objects.create(relative_path='f2', digest='1')
self.f2.artifact = None
ContentArtifact.objects.create(artifact=None, content=self.f2, relative_path='f2')

def download_result_mock(self, path):
dr = Mock()
Expand Down

0 comments on commit 8c793f8

Please sign in to comment.