From 1ab65c0ab714e46194263be0614e79f6d704a5d6 Mon Sep 17 00:00:00 2001 From: "patchback[bot]" <45432694+patchback[bot]@users.noreply.github.com> Date: Thu, 10 Dec 2020 13:34:11 -0500 Subject: [PATCH] Move creation of inboud repo and distro to Namespace model manager. (#586) (#601) - Creation of inbound repo and distro moved to Namespace model manager - Logic for deletion also moved to namespace manager - Unit tests added Issue: AAH-139 (cherry picked from commit 8c5f3f945435a792504adc97df7bd2d2a94b412d) Co-authored-by: Bruno Rocha --- CHANGES/139.misc | 1 + galaxy_ng/app/api/v3/viewsets/collection.py | 4 +- galaxy_ng/app/api/v3/viewsets/namespace.py | 34 +------------- galaxy_ng/app/constants.py | 2 + galaxy_ng/app/models/namespace.py | 47 +++++++++++++++++++ .../api/test_api_v3_namespace_viewsets.py | 13 +++-- galaxy_ng/tests/unit/test_models.py | 26 +++++++++- 7 files changed, 87 insertions(+), 40 deletions(-) create mode 100644 CHANGES/139.misc diff --git a/CHANGES/139.misc b/CHANGES/139.misc new file mode 100644 index 0000000000..20423b652a --- /dev/null +++ b/CHANGES/139.misc @@ -0,0 +1 @@ +Inbound repo and distro creation moved to Namespace model manager. diff --git a/galaxy_ng/app/api/v3/viewsets/collection.py b/galaxy_ng/app/api/v3/viewsets/collection.py index 081cf1645b..1cb2e6d5c2 100644 --- a/galaxy_ng/app/api/v3/viewsets/collection.py +++ b/galaxy_ng/app/api/v3/viewsets/collection.py @@ -22,9 +22,7 @@ from pulp_ansible.app.models import CollectionImport as PulpCollectionImport from galaxy_ng.app.api import base as api_base -from galaxy_ng.app.constants import DeploymentMode - -from galaxy_ng.app.api.v3.viewsets.namespace import INBOUND_REPO_NAME_FORMAT +from galaxy_ng.app.constants import DeploymentMode, INBOUND_REPO_NAME_FORMAT from galaxy_ng.app import models from galaxy_ng.app.access_control import access_policy diff --git a/galaxy_ng/app/api/v3/viewsets/namespace.py b/galaxy_ng/app/api/v3/viewsets/namespace.py index 6c9ee9bacf..2dc16eef5f 100644 --- a/galaxy_ng/app/api/v3/viewsets/namespace.py +++ b/galaxy_ng/app/api/v3/viewsets/namespace.py @@ -1,10 +1,7 @@ -import logging - from django.db import transaction from django.db.models import Q from django_filters import filters from django_filters.rest_framework import filterset, DjangoFilterBackend -from pulp_ansible.app.models import AnsibleRepository, AnsibleDistribution from galaxy_ng.app import models from galaxy_ng.app.access_control.access_policy import NamespaceAccessPolicy @@ -12,10 +9,6 @@ from galaxy_ng.app.api.v3 import serializers from galaxy_ng.app.exceptions import ConflictError -log = logging.getLogger(__name__) - -INBOUND_REPO_NAME_FORMAT = "inbound-{namespace_name}" - class NamespaceFilter(filterset.FilterSet): keywords = filters.CharFilter(method='keywords_filter') @@ -59,34 +52,11 @@ def get_serializer_class(self): @transaction.atomic def create(self, request, *args, **kwargs): - """Override to also create inbound pulp repository and distribution.""" + """Override to validate for name duplication before serializer validation.""" name = request.data['name'] if models.Namespace.objects.filter(name=name).exists(): + # Ensures error raised is 409, not 400. raise ConflictError( detail={'name': f'A namespace named {name} already exists.'} ) - - inbound_name = INBOUND_REPO_NAME_FORMAT.format(namespace_name=name) - repo = AnsibleRepository.objects.create(name=inbound_name) - AnsibleDistribution.objects.create( - name=inbound_name, - base_path=inbound_name, - repository=repo - ) return super().create(request, *args, **kwargs) - - @transaction.atomic - def destroy(self, request, *args, **kwargs): - """Override to also delete inbound pulp repository and distribution. - RepositoryVersion objects get deleted on delete of AnsibleRepository. - """ - name = INBOUND_REPO_NAME_FORMAT.format(namespace_name=kwargs['name']) - try: - AnsibleRepository.objects.get(name=name).delete() - except AnsibleRepository.DoesNotExist: - pass - try: - AnsibleDistribution.objects.get(name=name).delete() - except AnsibleDistribution.DoesNotExist: - pass - return super().destroy(request, *args, **kwargs) diff --git a/galaxy_ng/app/constants.py b/galaxy_ng/app/constants.py index 995b6a3cdd..a6011e6935 100644 --- a/galaxy_ng/app/constants.py +++ b/galaxy_ng/app/constants.py @@ -14,3 +14,5 @@ class DeploymentMode(enum.Enum): 'galaxy-dev.ansible.com', 'galaxy-qa.ansible.com', ) + +INBOUND_REPO_NAME_FORMAT = "inbound-{namespace_name}" diff --git a/galaxy_ng/app/models/namespace.py b/galaxy_ng/app/models/namespace.py index f857d51e85..d9da5c9e79 100644 --- a/galaxy_ng/app/models/namespace.py +++ b/galaxy_ng/app/models/namespace.py @@ -1,14 +1,58 @@ +import contextlib from django.db import models from django.db import transaction +from django.db import IntegrityError from django_lifecycle import LifecycleModel from pulpcore.plugin.models import AutoDeleteObjPermsMixin +from pulp_ansible.app.models import AnsibleRepository, AnsibleDistribution from galaxy_ng.app.access_control import mixins +from galaxy_ng.app.constants import INBOUND_REPO_NAME_FORMAT __all__ = ("Namespace", "NamespaceLink") +def create_inbound_repo(name): + """Creates inbound repo and inbound distribution for namespace publication.""" + inbound_name = INBOUND_REPO_NAME_FORMAT.format(namespace_name=name) + with contextlib.suppress(IntegrityError): + # IntegrityError is suppressed for when the named repo/distro already exists + # In that cases the error handling is performed on the caller. + repo = AnsibleRepository.objects.create(name=inbound_name) + AnsibleDistribution.objects.create( + name=inbound_name, + base_path=inbound_name, + repository=repo + ) + + +def delete_inbound_repo(name): + """Deletes inbound repo and distro in case of namespace deletion.""" + inbound_name = INBOUND_REPO_NAME_FORMAT.format(namespace_name=name) + with contextlib.suppress(AnsibleRepository.DoesNotExist): + AnsibleRepository.objects.get(name=inbound_name).delete() + with contextlib.suppress(AnsibleDistribution.DoesNotExist): + AnsibleDistribution.objects.get(name=inbound_name).delete() + + +class NamespaceManager(models.Manager): + + def create(self, **kwargs): + """Override to create inbound repo and distro.""" + create_inbound_repo(kwargs['name']) + return super().create(**kwargs) + + def bulk_create(self, objs, **kwargs): + for obj in objs: + create_inbound_repo(obj.name) + return super().bulk_create(objs, **kwargs) + + def delete(self): + delete_inbound_repo(self.name) + return super().delete() + + class Namespace(LifecycleModel, mixins.GroupModelPermissionsMixin, AutoDeleteObjPermsMixin): """ A model representing Ansible content namespace. @@ -27,6 +71,9 @@ class Namespace(LifecycleModel, mixins.GroupModelPermissionsMixin, AutoDeleteObj links: Back reference to related links. """ + # Cutom manager to handle inbound repo and distro + + objects = NamespaceManager() # Fields diff --git a/galaxy_ng/tests/unit/api/test_api_v3_namespace_viewsets.py b/galaxy_ng/tests/unit/api/test_api_v3_namespace_viewsets.py index 7f70a54319..b7829ee837 100644 --- a/galaxy_ng/tests/unit/api/test_api_v3_namespace_viewsets.py +++ b/galaxy_ng/tests/unit/api/test_api_v3_namespace_viewsets.py @@ -8,8 +8,7 @@ from galaxy_ng.app.models import auth as auth_models from galaxy_ng.app.models import Namespace from galaxy_ng.app.api.v3.serializers import NamespaceSerializer -from galaxy_ng.app.api.v3.viewsets.namespace import INBOUND_REPO_NAME_FORMAT -from galaxy_ng.app.constants import DeploymentMode +from galaxy_ng.app.constants import DeploymentMode, INBOUND_REPO_NAME_FORMAT from .base import BaseTestCase @@ -204,8 +203,14 @@ def test_conflict_error_if_already_exists(self): "company": "A company name", "email": "email@companyname.com", "description": "A testing namespace", - "groups": [{"name": "system:partner-engineers"}], - "object_permissions": ["change_namespace", "upload_to_namespace"] + "groups": [ + { + "id": self.pe_group.id, + "name": self.pe_group.name, + "object_permissions": ["change_namespace", "upload_to_namespace"] + } + ] } response = self.client.post(ns_list_url, post_data, format='json') + log.debug('namespace response: %s', response.data) self.assertEqual(response.status_code, status.HTTP_409_CONFLICT) diff --git a/galaxy_ng/tests/unit/test_models.py b/galaxy_ng/tests/unit/test_models.py index 3f52ca57bd..7151220253 100644 --- a/galaxy_ng/tests/unit/test_models.py +++ b/galaxy_ng/tests/unit/test_models.py @@ -1,5 +1,6 @@ +from galaxy_ng.app.constants import INBOUND_REPO_NAME_FORMAT from django.test import TestCase -from pulp_ansible.app.models import Collection +from pulp_ansible.app.models import AnsibleDistribution, AnsibleRepository, Collection from galaxy_ng.app.models import Namespace @@ -27,3 +28,26 @@ def test_existing_namespace_not_changed(self): ) namespace = Namespace.objects.get(name=self.namespace_name) self.assertEquals(namespace.description, description) + + +class TestNamespaceModelManager(TestCase): + namespace_name = 'my_test_ns_2' + + def test_new_namespace_creates_inbound_repo(self): + """When creating a new Namespace the manager should create inbound instances.""" + self.assertFalse(Namespace.objects.filter(name=self.namespace_name)) + Namespace.objects.create(name=self.namespace_name) + self.assertTrue(Namespace.objects.filter(name=self.namespace_name).exists()) + inbound_name = INBOUND_REPO_NAME_FORMAT.format(namespace_name=self.namespace_name) + self.assertTrue(AnsibleRepository.objects.filter(name=inbound_name).exists()) + self.assertTrue(AnsibleDistribution.objects.filter(name=inbound_name).exists()) + + def test_delete_namespace_deletes_inbound_repo(self): + """When deleting a Namespace the manager should delete inbound instances.""" + Namespace.objects.get_or_create(name=self.namespace_name) + Namespace.objects.filter(name=self.namespace_name).delete() + + inbound_name = INBOUND_REPO_NAME_FORMAT.format(namespace_name=self.namespace_name) + self.assertFalse(Namespace.objects.filter(name=self.namespace_name).exists()) + self.assertFalse(AnsibleRepository.objects.filter(name=inbound_name).exists()) + self.assertFalse(AnsibleDistribution.objects.filter(name=inbound_name).exists())