Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Split Distribution into two objects #124

Merged
merged 1 commit into from
May 10, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions docs/release-notes/pulpcore/3.0.x.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
19 changes: 3 additions & 16 deletions pulpcore/app/migrations/0001_initial.py
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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(
Expand All @@ -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',),
),
]
5 changes: 3 additions & 2 deletions pulpcore/app/models/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,11 @@
from .publication import ( # noqa
ContentGuard,
BaseDistribution,
Distribution,
Publication,
PublicationDistribution,
PublishedArtifact,
PublishedMetadata
PublishedMetadata,
RepositoryVersionDistribution,
)
from .repository import ( # noqa
Exporter,
Expand Down
31 changes: 21 additions & 10 deletions pulpcore/app/models/publication.py
Original file line number Diff line number Diff line change
Expand Up @@ -211,9 +211,9 @@ class BaseDistribution(MasterModel):

The `name` must be unique.

The ``base_path`` must have no overlapping components. So if a :term:`Distribution` with
``base_path`` of ``a/path/foo`` existed, you could not make a second :term:`Distribution` with a
``base_path`` of ``a/path`` or ``a`` because both are subpaths of ``a/path/foo``.
The ``base_path`` must have no overlapping components. So if a Distribution with ``base_path``
of ``a/path/foo`` existed, you could not make a second Distribution with a ``base_path`` of
``a/path`` or ``a`` because both are subpaths of ``a/path/foo``.

Fields:
name (models.CharField): The name of the distribution. Examples: "rawhide" and "stable".
Expand All @@ -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 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 RepositoryVersion or 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
3 changes: 2 additions & 1 deletion pulpcore/app/serializers/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,9 @@
from .publication import ( # noqa
BaseDistributionSerializer,
ContentGuardSerializer,
DistributionSerializer,
PublicationDistributionSerializer,
PublicationSerializer,
RepositoryVersionDistributionSerializer,
)
from .repository import ( # noqa
ExporterSerializer,
Expand Down
130 changes: 64 additions & 66 deletions pulpcore/app/serializers/publication.py
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down Expand Up @@ -72,7 +66,6 @@ class Meta:
model = models.Publication
fields = MasterModelSerializer.Meta.fields + (
'publisher',
'_distributions',
'repository_version',
'repository'
)
Expand Down Expand Up @@ -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,
Expand All @@ -123,35 +131,59 @@ class BaseDistributionSerializer(MasterModelSerializer):
)

class Meta:
abstract = True
model = models.BaseDistribution
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.'),
Expand All @@ -170,52 +202,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:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you know what happens if those parameters are set to null? I don't know if they disappear or result in None. If the latter, it is better to use data.get('repository') and data.get('repository_version') here

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

They disappear when I was testing it I was testing with one or the other. I'm also going to write some test issues which will add a functional test for this aspect to give us confidence over time.

msg = _("The attributes 'repository' and 'repository_version' must be used"
"exclusively.")
raise serializers.ValidationError(msg)

return data
2 changes: 1 addition & 1 deletion pulpcore/app/viewsets/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,9 @@
RepoVersionHrefFilter
)
from .publication import ( # noqa
BaseDistributionViewSet,
ContentGuardFilter,
ContentGuardViewSet,
DistributionViewSet,
PublicationViewSet,
)
from .repository import ( # noqa
Expand Down
Loading