launchpad-reviewers team mailing list archive

["Thiago F. Pappacena" <pappacena@xxxxxxxxx>]

Thiago F. Pappacena has proposed merging ~pappacena/launchpad:ociproject-project-pillar into launchpad:master.

Commit message:
Model changes to allow OCIProject pillar to be either a Distribution or a Product.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~pappacena/launchpad/+git/launchpad/+merge/383817
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~pappacena/launchpad:ociproject-project-pillar into launchpad:master.

diff --git a/lib/lp/registry/browser/ociproject.py b/lib/lp/registry/browser/ociproject.py
index 700af68..a170528 100644
--- a/lib/lp/registry/browser/ociproject.py
+++ b/lib/lp/registry/browser/ociproject.py
@@ -80,7 +80,7 @@ class OCIProjectAddView(LaunchpadFormView):
         oci_project_name = getUtility(
             IOCIProjectNameSet).getOrCreateByName(name)
 
-        oci_project = getUtility(IOCIProjectSet).getByDistributionAndName(
+        oci_project = getUtility(IOCIProjectSet).getByPillarAndName(
             self.context, oci_project_name.name)
         if oci_project:
             self.setFieldError(
@@ -183,7 +183,7 @@ class OCIProjectEditView(LaunchpadEditFormView):
         distribution = data.get('distribution')
         name = data.get('name')
         if distribution and name:
-            oci_project = getUtility(IOCIProjectSet).getByDistributionAndName(
+            oci_project = getUtility(IOCIProjectSet).getByPillarAndName(
                 distribution, name)
             if oci_project is not None and oci_project != self.context:
                 self.setFieldError(
diff --git a/lib/lp/registry/interfaces/ociproject.py b/lib/lp/registry/interfaces/ociproject.py
index 7394079..517e1ff 100644
--- a/lib/lp/registry/interfaces/ociproject.py
+++ b/lib/lp/registry/interfaces/ociproject.py
@@ -47,6 +47,7 @@ from lp.code.interfaces.gitref import IGitRef
 from lp.code.interfaces.hasgitrepositories import IHasGitRepositories
 from lp.registry.interfaces.distribution import IDistribution
 from lp.registry.interfaces.ociprojectname import IOCIProjectName
+from lp.registry.interfaces.product import IProduct
 from lp.registry.interfaces.series import SeriesStatus
 from lp.services.database.constants import DEFAULT
 from lp.services.fields import (
@@ -54,7 +55,6 @@ from lp.services.fields import (
     PublicPersonChoice,
     )
 
-
 # XXX: pappacena 2020-04-20: It is ok to remove the feature flag since we
 # already have in place the correct permission check for this feature.
 OCI_PROJECT_ALLOW_CREATE = 'oci.project.create.enabled'
@@ -96,7 +96,11 @@ class IOCIProjectEditableAttributes(IBugTarget):
     distribution = exported(ReferenceChoice(
         title=_("The distribution that this OCI project is associated with."),
         schema=IDistribution, vocabulary="Distribution",
-        required=True, readonly=False))
+        required=False, readonly=False))
+    product = exported(ReferenceChoice(
+        title=_('The project that this OCI project is associated with.'),
+        schema=IProduct, vocabulary='Product',
+        required=False, readonly=False))
     name = exported(TextLine(
         title=_("Name"), required=True, readonly=False,
         constraint=name_validator,
@@ -110,8 +114,9 @@ class IOCIProjectEditableAttributes(IBugTarget):
         title=_("The description for this OCI project."),
         required=True, readonly=False))
     pillar = exported(Reference(
-        IDistribution,
-        title=_("The pillar containing this target."), readonly=True))
+        Interface,
+        title=_("The pillar containing this target."),
+        required=True, readonly=False))
 
 
 class IOCIProjectEdit(Interface):
@@ -174,8 +179,13 @@ class IOCIProjectSet(Interface):
             bugfiling_duplicate_search=False):
         """Create an `IOCIProject`."""
 
-    def getByDistributionAndName(distribution, name):
-        """Get the OCIProjects for a given distribution."""
+    def getByPillarAndName(pillar, name):
+        """Get the OCIProjects for a given distribution.
+
+        :param pillar: An instance of Distribution or Product.
+        :param name: The OCIProject name to find.
+        :return: The OCIProject found.
+        """
 
 
 @error_status(http_client.UNAUTHORIZED)
diff --git a/lib/lp/registry/model/distribution.py b/lib/lp/registry/model/distribution.py
index 9c4c96e..50efe68 100644
--- a/lib/lp/registry/model/distribution.py
+++ b/lib/lp/registry/model/distribution.py
@@ -841,7 +841,7 @@ class Distribution(SQLBase, BugTargetBase, MakesAnnouncements,
             """ % sqlvalues(self.id, name))
 
     def getOCIProject(self, name):
-        oci_project = getUtility(IOCIProjectSet).getByDistributionAndName(
+        oci_project = getUtility(IOCIProjectSet).getByPillarAndName(
             self, name)
         return oci_project
 
diff --git a/lib/lp/registry/model/ociproject.py b/lib/lp/registry/model/ociproject.py
index 2967007..ae94433 100644
--- a/lib/lp/registry/model/ociproject.py
+++ b/lib/lp/registry/model/ociproject.py
@@ -32,6 +32,7 @@ from lp.registry.interfaces.ociproject import (
     IOCIProjectSet,
     )
 from lp.registry.interfaces.ociprojectname import IOCIProjectNameSet
+from lp.registry.interfaces.product import IProduct
 from lp.registry.interfaces.series import SeriesStatus
 from lp.registry.model.ociprojectname import OCIProjectName
 from lp.registry.model.ociprojectseries import OCIProjectSeries
@@ -75,6 +76,9 @@ class OCIProject(BugTargetBase, StormBase):
     distribution_id = Int(name="distribution", allow_none=True)
     distribution = Reference(distribution_id, "Distribution.id")
 
+    product_id = Int(name='product', allow_none=True)
+    product = Reference(product_id, 'Product.id')
+
     ociprojectname_id = Int(name="ociprojectname", allow_none=False)
     ociprojectname = Reference(ociprojectname_id, "OCIProjectName.id")
 
@@ -97,7 +101,18 @@ class OCIProject(BugTargetBase, StormBase):
     @property
     def pillar(self):
         """See `IBugTarget`."""
-        return self.distribution
+        return self.product if self.product_id else self.distribution
+
+    @pillar.setter
+    def pillar(self, pillar):
+        if IDistribution.providedBy(pillar):
+            self.distribution = pillar
+        elif IProduct.providedBy(pillar):
+            self.product = pillar
+        else:
+            raise ValueError(
+                'The target of an OCIProject must be an '
+                'IDistribution instance.')
 
     @property
     def display_name(self):
@@ -164,16 +179,7 @@ class OCIProjectSet:
         target = OCIProject()
         target.date_created = date_created
         target.date_last_modified = date_created
-
-        # XXX twom 2019-10-10 This needs to have IProduct support
-        # when the model supports it
-        if IDistribution.providedBy(pillar):
-            target.distribution = pillar
-        else:
-            raise ValueError(
-                'The target of an OCIProject must be an '
-                'IDistribution instance.')
-
+        target.pillar = pillar
         target.registrant = registrant
         target.ociprojectname = name
         target.description = description
@@ -183,11 +189,30 @@ class OCIProjectSet:
         store.add(target)
         return target
 
-    def getByDistributionAndName(self, distribution, name):
+    def _get_pillar_attribute(self, pillar):
+        """Checks if the provided pillar is a valid one for OCIProject,
+        returning the model attribute where this pillar would be stored.
+
+        If pillar is not valid, raises ValueError.
+
+        :param pillar: A Distribution or Product.
+        :return: Storm attribute where the pillar would be stored.
+                 If pillar is not valid, raises ValueError.
+        """
+        if IDistribution.providedBy(pillar):
+            return OCIProject.distribution
+        elif IProduct.providedBy(pillar):
+            return OCIProject.product
+        else:
+            raise ValueError(
+                'The target of an OCIProject must be either an '
+                'IDistribution or an IProduct instance.')
+
+    def getByPillarAndName(self, pillar, name):
         """See `IOCIProjectSet`."""
         target = IStore(OCIProject).find(
             OCIProject,
-            OCIProject.distribution == distribution,
+            self._get_pillar_attribute(pillar) == pillar,
             OCIProject.ociprojectname == OCIProjectName.id,
             OCIProjectName.name == name).one()
         return target
diff --git a/lib/lp/registry/tests/test_ociproject.py b/lib/lp/registry/tests/test_ociproject.py
index fdddb4c..3d3c740 100644
--- a/lib/lp/registry/tests/test_ociproject.py
+++ b/lib/lp/registry/tests/test_ociproject.py
@@ -46,6 +46,11 @@ class TestOCIProject(TestCaseWithFactory):
         with admin_logged_in():
             self.assertProvides(oci_project, IOCIProject)
 
+    def test_product_pillar(self):
+        product = self.factory.makeProduct(name="some-project")
+        oci_project = self.factory.makeOCIProject(pillar=product)
+        self.assertEqual(product, oci_project.pillar)
+
     def test_newSeries(self):
         driver = self.factory.makePerson()
         distribution = self.factory.makeDistribution(driver=driver)
@@ -130,7 +135,7 @@ class TestOCIProjectSet(TestCaseWithFactory):
 
         with person_logged_in(registrant):
             fetched_result = getUtility(
-                IOCIProjectSet).getByDistributionAndName(
+                IOCIProjectSet).getByPillarAndName(
                     distribution, oci_project.ociprojectname.name)
             self.assertEqual(oci_project, fetched_result)
 
@@ -273,6 +278,5 @@ class TestOCIProjectWebservice(TestCaseWithFactory):
             other_user = self.factory.makePerson()
             distro = removeSecurityProxy(self.factory.makeDistribution(
                 owner=other_user))
-            url = api_url(distro)
 
         self.assertCanCreateOCIProject(distro, self.person)