Nested views: Change from generic subclass views to specific views

Required PR: pulp/pulp_file#299
Required PR: pulp/pulp-smash#1223

re: #5625
https://pulp.plan.io/issues/5625

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

pulpcore/app/serializers/__init__.py

@@ -25,6 +25,9 @@
     LatestVersionField,
     SecretCharField,
     SingleContentArtifactField,
+    RepositoryVersionsIdentityFromRepositoryField,
+    RepositoryVersionRelatedField,
+    RepositoryVersionIdentityField,
     relative_path_validator,
 )
 from .progress import ProgressReportSerializer  # noqa

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):
         """

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:

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):

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)
 

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
 

pulpcore/plugin/viewsets/__init__.py

@@ -17,6 +17,7 @@
     RemoteFilter,
     RemoteViewSet,
     RepositoryViewSet,
+    RepositoryVersionViewSet,
 )
 
 # Import custom filters that are potentially useful to plugin writers