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 8c5f3f9)

Co-authored-by: Bruno Rocha <[email protected]>

CHANGES/139.misc

@@ -0,0 +1 @@
+Inbound repo and distro creation moved to Namespace model manager.

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
 

galaxy_ng/app/api/v3/viewsets/namespace.py

@@ -1,21 +1,14 @@
-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
 from galaxy_ng.app.api import base as api_base
 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)

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}"

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
 

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 protected]",
                 "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)

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