From c65d158175228696727fc5a047ef247e67efcf9d Mon Sep 17 00:00:00 2001 From: Daniel Alley Date: Thu, 7 Nov 2019 13:25:50 -0500 Subject: [PATCH] Nested views: Change from generic subclass views to specific views Required PR: https://github.com/pulp/pulp_file/pull/299 Required PR: https://github.com/PulpQE/pulp-smash/pull/1223 re: #5625 https://pulp.plan.io/issues/5625 --- pulpcore/app/apps.py | 7 ++-- pulpcore/app/serializers/__init__.py | 3 ++ pulpcore/app/serializers/fields.py | 56 ++++++++++++++++++++----- pulpcore/app/serializers/publication.py | 16 ++----- pulpcore/app/serializers/repository.py | 28 ++++--------- pulpcore/app/urls.py | 18 +++----- pulpcore/app/util.py | 14 ++++--- pulpcore/plugin/viewsets/__init__.py | 1 + 8 files changed, 79 insertions(+), 64 deletions(-) diff --git a/pulpcore/app/apps.py b/pulpcore/app/apps.py index 485f7351bc..bfcd4f8518 100644 --- a/pulpcore/app/apps.py +++ b/pulpcore/app/apps.py @@ -1,3 +1,4 @@ +from collections import defaultdict from importlib import import_module from django import apps @@ -65,7 +66,7 @@ def __init__(self, app_name, app_module): # Module containing urlpatterns self.urls_module = None - # Mapping of model names to viewsets (viewsets unrelated to models are excluded) + # Mapping of model names to viewset lists (viewsets unrelated to models are excluded) self.named_viewsets = None # Mapping of serializer names to serializers self.named_serializers = None @@ -99,7 +100,7 @@ def import_serializers(self): def import_viewsets(self): # circular import avoidance from pulpcore.app.viewsets import NamedModelViewSet - self.named_viewsets = {} + self.named_viewsets = defaultdict(list) if module_has_submodule(self.module, VIEWSETS_MODULE_NAME): # import the viewsets module and track any interesting viewsets viewsets_module_name = '{name}.{module}'.format( @@ -112,7 +113,7 @@ def import_viewsets(self): # gets registered in the named_viewsets registry. if (obj is not NamedModelViewSet and issubclass(obj, NamedModelViewSet)): model = obj.queryset.model - self.named_viewsets[model] = obj + self.named_viewsets[model].append(obj) except TypeError: # obj isn't a class, issubclass exploded but obj can be safely filtered out continue diff --git a/pulpcore/app/serializers/__init__.py b/pulpcore/app/serializers/__init__.py index 71a2cbe730..371bf07f42 100644 --- a/pulpcore/app/serializers/__init__.py +++ b/pulpcore/app/serializers/__init__.py @@ -25,6 +25,9 @@ LatestVersionField, SecretCharField, SingleContentArtifactField, + RepositoryVersionsIdentityFromRepositoryField, + RepositoryVersionRelatedField, + RepositoryVersionIdentityField, relative_path_validator, ) from .progress import ProgressReportSerializer # noqa diff --git a/pulpcore/app/serializers/fields.py b/pulpcore/app/serializers/fields.py index 32a9122e72..a4dcd1032d 100644 --- a/pulpcore/app/serializers/fields.py +++ b/pulpcore/app/serializers/fields.py @@ -6,10 +6,9 @@ from rest_framework import serializers from rest_framework.fields import empty from rest_framework.reverse import reverse -from rest_framework_nested.relations import NestedHyperlinkedRelatedField from pulpcore.app import models -from pulpcore.app.serializers import RelatedField +from pulpcore.app.serializers import DetailIdentityField, IdentityField, RelatedField def relative_path_validator(relative_path): @@ -174,11 +173,51 @@ def to_representation(self, value): return ret -class LatestVersionField(NestedHyperlinkedRelatedField): - parent_lookup_kwargs = {'repository_pk': 'repository__pk'} - lookup_field = 'number' +class RepositoryVersionsIdentityFromRepositoryField(DetailIdentityField): + view_name = 'repositories-detail' + + def __init__(self, view_name=None, **kwargs): + assert view_name is None, 'The `view_name` must not be set.' + super().__init__(view_name=self.view_name, **kwargs) + + def get_url(self, obj, view_name, request, *args, **kwargs): + return super().get_url(obj, self.view_name, request, *args, **kwargs) + "versions/" + + +class RepositoryVersionFieldGetURLMixin: view_name = 'versions-detail' + def __init__(self, view_name=None, **kwargs): + assert view_name is None, 'The `view_name` must not be set.' + super().__init__(view_name=self.view_name, **kwargs) + + def get_url(self, obj, view_name, request, *args, **kwargs): + rvr_field = RepositoryVersionsIdentityFromRepositoryField() + repo_url = rvr_field.get_url(obj.repository, None, request, *args, **kwargs) + return f"{repo_url}{obj.number}/" + + def use_pk_only_optimization(self): + return False + + +class RepositoryVersionIdentityField(RepositoryVersionFieldGetURLMixin, IdentityField): + pass + + +class RepositoryVersionRelatedField(RepositoryVersionFieldGetURLMixin, RelatedField): + queryset = models.RepositoryVersion.objects.all() + + def get_object(self, view_name, view_args, view_kwargs): + lookup_kwargs = { + 'repository__pk': view_kwargs['repository_pk'], + 'number': view_kwargs['number'] + } + return self.get_queryset().get(**lookup_kwargs) + + +class LatestVersionField(RepositoryVersionRelatedField): + queryset = None # read-only relational fields should not provide a `queryset` argument + def __init__(self, *args, **kwargs): """ Unfortunately you can't just set read_only=True on the class. It has @@ -207,12 +246,7 @@ def get_url(self, obj, view_name, request, format): version = obj.exclude(complete=False).latest() except obj.model.DoesNotExist: return None - - kwargs = { - 'repository_pk': version.repository.pk, - 'number': version.number, - } - return self.reverse(view_name, kwargs=kwargs, request=None, format=format) + return super().get_url(version, view_name, request, format) def get_attribute(self, instance): """ diff --git a/pulpcore/app/serializers/publication.py b/pulpcore/app/serializers/publication.py index cb63c569f2..0a69aac7f4 100644 --- a/pulpcore/app/serializers/publication.py +++ b/pulpcore/app/serializers/publication.py @@ -11,19 +11,15 @@ DetailIdentityField, DetailRelatedField, ModelSerializer, - NestedRelatedField, + RepositoryVersionRelatedField, validate_unknown_fields, ) class PublicationSerializer(ModelSerializer): pulp_href = DetailIdentityField() - repository_version = NestedRelatedField( - view_name='versions-detail', - lookup_field='number', - parent_lookup_kwargs={'repository_pk': 'repository__pk'}, - queryset=models.RepositoryVersion.objects.all(), - required=False, + repository_version = RepositoryVersionRelatedField( + required=False ) repository = DetailRelatedField( help_text=_('A URI of the repository to be published.'), @@ -195,14 +191,10 @@ class RepositoryVersionDistributionSerializer(BaseDistributionSerializer): queryset=models.Repository.objects.all(), allow_null=True ) - repository_version = NestedRelatedField( + repository_version = RepositoryVersionRelatedField( required=False, help_text=_('RepositoryVersion to be served'), - queryset=models.RepositoryVersion.objects.exclude(complete=False), - view_name='versions-detail', allow_null=True, - lookup_field='number', - parent_lookup_kwargs={'repository_pk': 'repository__pk'}, ) class Meta: diff --git a/pulpcore/app/serializers/repository.py b/pulpcore/app/serializers/repository.py index 0601df537a..50474d014d 100644 --- a/pulpcore/app/serializers/repository.py +++ b/pulpcore/app/serializers/repository.py @@ -8,21 +8,18 @@ from pulpcore.app.serializers import ( DetailIdentityField, DetailRelatedField, - IdentityField, LatestVersionField, ModelSerializer, - NestedIdentityField, - NestedRelatedField, SecretCharField, + RepositoryVersionIdentityField, + RepositoryVersionRelatedField, + RepositoryVersionsIdentityFromRepositoryField ) class RepositorySerializer(ModelSerializer): pulp_href = DetailIdentityField() - versions_href = IdentityField( - view_name='versions-list', - lookup_url_kwarg='repository_pk', - ) + versions_href = RepositoryVersionsIdentityFromRepositoryField() latest_version_href = LatestVersionField() name = serializers.CharField( help_text=_('A unique name for this repository.'), @@ -215,21 +212,14 @@ def to_internal_value(self, data): class RepositoryVersionSerializer(ModelSerializer, NestedHyperlinkedModelSerializer): - pulp_href = NestedIdentityField( - view_name='versions-detail', - lookup_field='number', parent_lookup_kwargs={'repository_pk': 'repository__pk'}, - ) + pulp_href = RepositoryVersionIdentityField() number = serializers.IntegerField( read_only=True ) - base_version = NestedRelatedField( + base_version = RepositoryVersionRelatedField( required=False, help_text=_('A repository version whose content was used as the initial set of content ' 'for this repository version'), - queryset=models.RepositoryVersion.objects.all(), - view_name='versions-detail', - lookup_field='number', - parent_lookup_kwargs={'repository_pk': 'repository__pk'}, ) content_summary = ContentSummarySerializer( help_text=_('Various count summaries of the content in the version and the HREF to view ' @@ -259,14 +249,10 @@ class RepositoryAddRemoveContentSerializer(ModelSerializer, NestedHyperlinkedMod write_only=True, required=False ) - base_version = NestedRelatedField( + base_version = RepositoryVersionRelatedField( required=False, help_text=_('A repository version whose content will be used as the initial set of content ' 'for the new repository version'), - queryset=models.RepositoryVersion.objects.all(), - view_name='versions-detail', - lookup_field='number', - parent_lookup_kwargs={'repository_pk': 'repository__pk'}, ) def validate_remove_content_units(self, value): diff --git a/pulpcore/app/urls.py b/pulpcore/app/urls.py index 967e6e7451..66d8383030 100644 --- a/pulpcore/app/urls.py +++ b/pulpcore/app/urls.py @@ -26,9 +26,9 @@ class ViewSetNode: RootNode ├─ RepositoryViewSet │ ├─ PluginPublisherViewSet (non-nested) - │ │ └─ DistributionViewSet + │ │ └─ PluginPublisherVDistributionViewSet │ ├─ AnotherPluginPublisherViewSet - │ │ └─ DistributionViewSet (This node is attached to all Publisher Detail parents) + │ │ └─ AnotherPluginDistributionViewSet │ └─ FileRemoteViewSet └─ some-non-nested viewset """ @@ -48,11 +48,6 @@ def add_decendent(self, node): """ Add a VSNode to the tree. If node is not a direct child, attempt to add the to each child. - Some nodes must be added more than once if they have a Master/Detail parent. Using - Distributions as an example, DistributionViewset.parent_viewset is PublisherViewSet, which - is a MasterViewset. Each of the publisher detail viewsets like FilePublisherViewSEt will - have its own router, and the DistributionViewSet must be registered with each. - Args: node (ViewSetNode): A node that represents a viewset and its decendents. """ @@ -62,9 +57,8 @@ def add_decendent(self, node): # Non-nested viewsets are attached to the root node if not node.viewset.parent_viewset: self.children.append(node) - # The node is a direct child if the child.parent_viewset == self.viewset and also - # if child.viewset is a master viewset and self.viewset is one of its detail viewsets. - elif self.viewset and issubclass(self.viewset, node.viewset.parent_viewset): + # The node is a direct child if the child.parent_viewset is self.viewset. + elif self.viewset and self.viewset is node.viewset.parent_viewset: self.children.append(node) else: for child in self.children: @@ -104,8 +98,8 @@ def __repr__(self): plugin_patterns = [] # Iterate over each app, including pulpcore and the plugins. for app_config in pulp_plugin_configs(): - for viewset in app_config.named_viewsets.values(): - all_viewsets.append(viewset) + for viewsets in app_config.named_viewsets.values(): + all_viewsets.extend(viewsets) if app_config.urls_module: plugin_patterns.append(app_config.urls_module) diff --git a/pulpcore/app/util.py b/pulpcore/app/util.py index 394aab1275..1b10af75b1 100644 --- a/pulpcore/app/util.py +++ b/pulpcore/app/util.py @@ -22,11 +22,15 @@ def get_viewset_for_model(model_obj): model_viewset = None # go through the viewset registry to find the viewset for the passed-in model for app in pulp_plugin_configs(): - for model, viewset in app.named_viewsets.items(): - _model_viewset_cache.setdefault(model, viewset) - if model is model_class: - model_viewset = viewset - break + for model, viewsets in app.named_viewsets.items(): + # There may be multiple viewsets for a model. In this + # case, we can't reverse the mapping. + if len(viewsets) == 1: + viewset = viewsets[0] + _model_viewset_cache.setdefault(model, viewset) + if model is model_class: + model_viewset = viewset + break if model_viewset is not None: break diff --git a/pulpcore/plugin/viewsets/__init__.py b/pulpcore/plugin/viewsets/__init__.py index c4e212c386..bef6e57126 100644 --- a/pulpcore/plugin/viewsets/__init__.py +++ b/pulpcore/plugin/viewsets/__init__.py @@ -17,6 +17,7 @@ RemoteFilter, RemoteViewSet, RepositoryViewSet, + RepositoryVersionViewSet, ) # Import custom filters that are potentially useful to plugin writers