Split Distribution into two objects

The Distribution contained options which should not be mixed.
Specifically the repository and repository_version options go together
and the publication goes by itself.

This PR splits that object into two new objects
RepositoryVersionDistribution and PublicationDistribution. The two
models are not detail objects, and require the plugin writer to declare
a Distribution detail object. It was already this way before this PR.

This also adds a release note about the breaking changes that come with
the switch to Master/Detail.

Required PR: pulp/pulpcore-plugin#97
Required PR: pulp/pulp_file#219

https://pulp.plan.io/issues/4785
closes #4785

docs/release-notes/pulpcore/3.0.x.rst

@@ -20,6 +20,11 @@ example from `pulp_file <https://github.com/pulp/pulp_file>`_ see the URL change
 `here <https://github.com/pulp/pulp_file/pull/205/files#diff-88b99bb28683bd5b7e3a204826ead112R200>`_
 as an example. See plugin docs compatible with 3.0.0rc2 for more details.
 
+Distributions are now Master/Detail which causes the Distribution URL endpoint to change. To give an
+example from `pulp_file <https://github.com/pulp/pulp_file>`_ see the URL changes made
+`in this PR <https://github.com/pulp/pulp_file/pull/219/files>`_ as an example. See plugin docs
+compatible with 3.0.0rc2 for more details.
+
 The semantics of :term:`Remote` attributes ``ssl_ca_certificate``, ``ssl_client_certificate``, and
 ``ssl_client_key`` changed even though the field names didn't. Now these assets are saved directly
 in the database instead of on the filesystem, and they are prevented from being read back out to

pulpcore/app/migrations/0001_initial.py

@@ -1,4 +1,4 @@
-# Generated by Django 2.2.1 on 2019-05-08 18:30
+# Generated by Django 2.2.1 on 2019-05-09 16:49
 
 from django.conf import settings
 import django.core.validators
@@ -278,7 +278,7 @@ class Migration(migrations.Migration):
                 ('version_removed', models.ForeignKey(null=True, on_delete=django.db.models.deletion.CASCADE, related_name='removed_memberships', to='core.RepositoryVersion')),
             ],
             options={
-                'unique_together': {('repository', 'content', 'version_added'), ('repository', 'content', 'version_removed')},
+                'unique_together': {('repository', 'content', 'version_removed'), ('repository', 'content', 'version_added')},
             },
         ),
         migrations.AddField(
@@ -410,7 +410,7 @@ class Migration(migrations.Migration):
             ],
             options={
                 'default_related_name': 'published_metadata',
-                'unique_together': {('publication', 'relative_path'), ('publication', 'file')},
+                'unique_together': {('publication', 'file'), ('publication', 'relative_path')},
             },
         ),
         migrations.CreateModel(
@@ -428,17 +428,4 @@ class Migration(migrations.Migration):
                 'unique_together': {('publication', 'content_artifact'), ('publication', 'relative_path')},
             },
         ),
-        migrations.CreateModel(
-            name='Distribution',
-            fields=[
-                ('basedistribution_ptr', models.OneToOneField(auto_created=True, on_delete=django.db.models.deletion.CASCADE, parent_link=True, primary_key=True, related_name='_distributions', serialize=False, to='core.BaseDistribution')),
-                ('publication', models.ForeignKey(null=True, on_delete=django.db.models.deletion.SET_NULL, related_name='_distributions', to='core.Publication')),
-                ('repository', models.ForeignKey(null=True, on_delete=django.db.models.deletion.SET_NULL, related_name='_distributions', to='core.Repository')),
-                ('repository_version', models.ForeignKey(null=True, on_delete=django.db.models.deletion.SET_NULL, related_name='_distributions', to='core.RepositoryVersion')),
-            ],
-            options={
-                'default_related_name': '_distributions',
-            },
-            bases=('core.basedistribution',),
-        ),
     ]

pulpcore/app/models/__init__.py

@@ -6,10 +6,11 @@
 from .publication import (  # noqa
     ContentGuard,
     BaseDistribution,
-    Distribution,
     Publication,
+    PublicationDistribution,
     PublishedArtifact,
-    PublishedMetadata
+    PublishedMetadata,
+    RepositoryVersionDistribution,
 )
 from .repository import (  # noqa
     Exporter,

pulpcore/app/models/publication.py

@@ -232,23 +232,34 @@ class BaseDistribution(MasterModel):
     remote = models.ForeignKey(Remote, null=True, on_delete=models.SET_NULL)
 
 
-class Distribution(BaseDistribution):
+class PublicationDistribution(BaseDistribution):
     """
-    A distribution defines how a publication is distributed by Pulp's content app.
-
-    The use of repository, repository_version, or publication are mutually exclusive, and this
-    model's serializer will raise a ValidationError if they are used together.
+    Define how Pulp's content app will serve a :term:`Publication`.
 
     Relations:
         publication (models.ForeignKey): Publication to be served.
+    """
+
+    publication = models.ForeignKey(Publication, null=True, on_delete=models.SET_NULL)
+
+    class Meta:
+        abstract = True
+
+
+class RepositoryVersionDistribution(BaseDistribution):
+    """
+    Define how Pulp's content app will serve a :term:`RepositoryVersion` or :term:`Repository`.
+
+    The ``repository`` and ``repository_version`` fields cannot be used together.
+
+    Relations:
         repository (models.ForeignKey): The latest RepositoryVersion for this Repository will be
             served.
         repository_version (models.ForeignKey): RepositoryVersion to be served.
     """
 
-    publication = models.ForeignKey(Publication, null=True, on_delete=models.SET_NULL)
     repository = models.ForeignKey(Repository, null=True, on_delete=models.SET_NULL)
     repository_version = models.ForeignKey(RepositoryVersion, null=True, on_delete=models.SET_NULL)
 
     class Meta:
-        default_related_name = '_distributions'
+        abstract = True

pulpcore/app/serializers/__init__.py

@@ -35,8 +35,9 @@
 from .publication import (  # noqa
     BaseDistributionSerializer,
     ContentGuardSerializer,
-    DistributionSerializer,
+    PublicationDistributionSerializer,
     PublicationSerializer,
+    RepositoryVersionDistributionSerializer,
 )
 from .repository import (  # noqa
     ExporterSerializer,

pulpcore/app/serializers/publication.py

@@ -21,12 +21,6 @@
 
 class PublicationSerializer(MasterModelSerializer):
     _href = DetailIdentityField()
-    _distributions = DetailRelatedField(
-       help_text=_('This publication is currently being served as'
-                   'defined by these distributions.'),
-       many=True,
-       read_only=True,
-    )
     repository_version = NestedRelatedField(
         view_name='versions-detail',
         lookup_field='number',
@@ -72,7 +66,6 @@ class Meta:
         model = models.Publication
         fields = MasterModelSerializer.Meta.fields + (
             'publisher',
-            '_distributions',
             'repository_version',
             'repository'
         )
@@ -100,20 +93,35 @@ class Meta:
 
 class BaseDistributionSerializer(MasterModelSerializer):
     _href = DetailIdentityField()
+    base_path = serializers.CharField(
+        help_text=_('The base (relative) path component of the published url. Avoid paths that \
+                    overlap with other distribution base paths (e.g. "foo" and "foo/bar")'),
+        validators=[validators.MaxLengthValidator(
+            models.BaseDistribution._meta.get_field('base_path').max_length,
+            message=_('`base_path` length must be less than {} characters').format(
+                models.BaseDistribution._meta.get_field('base_path').max_length
+            )),
+            UniqueValidator(queryset=models.BaseDistribution.objects.all()),
+        ]
+    )
+    base_url = BaseURLField(
+        source='base_path', read_only=True,
+        help_text=_('The URL for accessing the publication as defined by this distribution.')
+    )
     content_guard = DetailRelatedField(
         required=False,
         help_text=_('An optional content-guard.'),
         queryset=models.ContentGuard.objects.all(),
         allow_null=True
     )
     name = serializers.CharField(
-        help_text=_('A unique distribution name. Ex, `rawhide` and `stable`.'),
+        help_text=_('A unique name. Ex, `rawhide` and `stable`.'),
         validators=[validators.MaxLengthValidator(
-            models.Distribution._meta.get_field('name').max_length,
-            message=_('Distribution name length must be less than {} characters').format(
-                models.Distribution._meta.get_field('name').max_length
+            models.BaseDistribution._meta.get_field('name').max_length,
+            message=_('`name` length must be less than {} characters').format(
+                models.BaseDistribution._meta.get_field('name').max_length
             )),
-            UniqueValidator(queryset=models.Distribution.objects.all())]
+            UniqueValidator(queryset=models.BaseDistribution.objects.all())]
     )
     remote = DetailRelatedField(
         required=False,
@@ -123,35 +131,58 @@ class BaseDistributionSerializer(MasterModelSerializer):
     )
 
     class Meta:
+        abstract = True
         fields = ModelSerializer.Meta.fields + (
+            'base_path',
+            'base_url',
             'content_guard',
             'name',
             'remote',
         )
 
+    def _validate_path_overlap(self, path):
+        # look for any base paths nested in path
+        search = path.split("/")[0]
+        q = Q(base_path=search)
+        for subdir in path.split("/")[1:]:
+            search = "/".join((search, subdir))
+            q |= Q(base_path=search)
 
-class DistributionSerializer(BaseDistributionSerializer):
-    base_path = serializers.CharField(
-        help_text=_('The base (relative) path component of the published url. Avoid paths that \
-                    overlap with other distribution base paths (e.g. "foo" and "foo/bar")'),
-        validators=[validators.MaxLengthValidator(
-            models.Distribution._meta.get_field('base_path').max_length,
-            message=_('Distribution base_path length must be less than {} characters').format(
-                models.Distribution._meta.get_field('base_path').max_length
-            )),
-            UniqueValidator(queryset=models.Distribution.objects.all()),
-        ]
-    )
-    base_url = BaseURLField(
-        source='base_path', read_only=True,
-        help_text=_('The URL for accessing the publication as defined by this distribution.')
-    )
+        # look for any base paths that nest path
+        q |= Q(base_path__startswith='{}/'.format(path))
+        qs = models.BaseDistribution.objects.filter(q)
+
+        if self.instance is not None:
+            qs = qs.exclude(pk=self.instance.pk)
+
+        match = qs.first()
+        if match:
+            raise serializers.ValidationError(detail=_("Overlaps with existing distribution '"
+                                                       "{}'").format(match.name))
+
+        return path
+
+    def validate_base_path(self, path):
+        self._validate_relative_path(path)
+        return self._validate_path_overlap(path)
+
+
+class PublicationDistributionSerializer(BaseDistributionSerializer):
     publication = DetailRelatedField(
         required=False,
         help_text=_('Publication to be served'),
         queryset=models.Publication.objects.exclude(complete=False),
         allow_null=True
     )
+
+    class Meta:
+        abstract = True
+        fields = BaseDistributionSerializer.Meta.fields + (
+            'publication',
+        )
+
+
+class RepositoryVersionDistributionSerializer(BaseDistributionSerializer):
     repository = RelatedField(
         required=False,
         help_text=_('The latest RepositoryVersion for this Repository will be served.'),
@@ -170,52 +201,18 @@ class DistributionSerializer(BaseDistributionSerializer):
     )
 
     class Meta:
-        model = models.Distribution
+        abstract = True
         fields = BaseDistributionSerializer.Meta.fields + (
-            'base_path',
-            'base_url',
-            'publication',
             'repository',
             'repository_version',
         )
 
-    def _validate_path_overlap(self, path):
-        # look for any base paths nested in path
-        search = path.split("/")[0]
-        q = Q(base_path=search)
-        for subdir in path.split("/")[1:]:
-            search = "/".join((search, subdir))
-            q |= Q(base_path=search)
-
-        # look for any base paths that nest path
-        q |= Q(base_path__startswith='{}/'.format(path))
-        qs = models.Distribution.objects.filter(q)
-
-        if self.instance is not None:
-            qs = qs.exclude(pk=self.instance.pk)
-
-        match = qs.first()
-        if match:
-            raise serializers.ValidationError(detail=_("Overlaps with existing distribution '"
-                                                       "{}'").format(match.name))
-
-        return path
-
-    def validate_base_path(self, path):
-        self._validate_relative_path(path)
-        return self._validate_path_overlap(path)
-
     def validate(self, data):
         super().validate(data)
 
-        mutex_keys = ['publication', 'repository', 'repository_version']
-        in_use_keys = []
-        for mkey in mutex_keys:
-            if mkey in data:
-                in_use_keys.append(mkey)
-
-        if len(in_use_keys) > 1:
-            msg = _("The attributes {keys} must be used exclusively.".format(keys=in_use_keys))
+        if 'repository' in data and 'repository_version' in data:
+            msg = _("The attributes 'repository' and 'repository_version' must be used"
+                    "exclusively.")
             raise serializers.ValidationError(msg)
 
         return data

pulpcore/app/viewsets/__init__.py

@@ -16,9 +16,9 @@
     RepoVersionHrefFilter
 )
 from .publication import (  # noqa
+    BaseDistributionViewSet,
     ContentGuardFilter,
     ContentGuardViewSet,
-    DistributionViewSet,
     PublicationViewSet,
 )
 from .repository import (  # noqa

pulpcore/app/viewsets/publication.py

@@ -3,13 +3,13 @@
 from rest_framework.filters import OrderingFilter
 
 from pulpcore.app.models import (
+    BaseDistribution,
     ContentGuard,
-    Distribution,
     Publication,
 )
 from pulpcore.app.serializers import (
+    BaseDistributionSerializer,
     ContentGuardSerializer,
-    DistributionSerializer,
     PublicationSerializer,
 )
 from pulpcore.app.viewsets import (
@@ -65,42 +65,29 @@ class DistributionFilter(BaseFilterSet):
     base_path = filters.CharFilter()
 
     class Meta:
-        model = Distribution
+        model = BaseDistribution
         fields = {
             'name': NAME_FILTER_OPTIONS,
             'base_path': ['exact', 'contains', 'icontains', 'in']
         }
 
 
-class DistributionViewSet(NamedModelViewSet,
-                          mixins.RetrieveModelMixin,
-                          mixins.ListModelMixin,
-                          AsyncCreateMixin,
-                          AsyncRemoveMixin,
-                          AsyncUpdateMixin):
+class BaseDistributionViewSet(NamedModelViewSet,
+                              mixins.RetrieveModelMixin,
+                              mixins.ListModelMixin,
+                              AsyncCreateMixin,
+                              AsyncRemoveMixin,
+                              AsyncUpdateMixin):
     """
     Provides read and list methods and also provides asynchronous CUD methods to dispatch tasks
     with reservation that lock all Distributions preventing race conditions during base_path
     checking.
     """
     endpoint_name = 'distributions'
-    queryset = Distribution.objects.all()
-    serializer_class = DistributionSerializer
+    queryset = BaseDistribution.objects.all()
+    serializer_class = BaseDistributionSerializer
     filterset_class = DistributionFilter
 
     def async_reserved_resources(self, instance):
         """Return resource that locks all Distributions."""
         return "/api/v3/distributions/"
-
-    @classmethod
-    def is_master_viewset(cls):
-        """
-        Declare DistributionViewSet to be a master ViewSet.
-
-        This Viewset is a master ViewSet despite that its associated Model isn't
-        a master model. This prevents registering the ViewSet with a router.
-        """
-        if cls is DistributionViewSet:
-            return True
-        else:
-            return super().is_master_viewset()