launchpad-reviewers team mailing list archive

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

Thiago F. Pappacena has proposed merging ~pappacena/launchpad:bugfix-delete-oci-recipe into launchpad:master.

Commit message:
Remove OCIFiles from OCIRecipeBuild when deleting OCIRecipe

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~pappacena/launchpad/+git/launchpad/+merge/387712
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~pappacena/launchpad:bugfix-delete-oci-recipe into launchpad:master.

diff --git a/lib/lp/oci/browser/tests/test_ocirecipe.py b/lib/lp/oci/browser/tests/test_ocirecipe.py
index e25d463..3918bed 100644
--- a/lib/lp/oci/browser/tests/test_ocirecipe.py
+++ b/lib/lp/oci/browser/tests/test_ocirecipe.py
@@ -25,6 +25,7 @@ from testtools.matchers import (
 from zope.component import getUtility
 from zope.publisher.interfaces import NotFound
 from zope.security.interfaces import Unauthorized
+from zope.security.proxy import removeSecurityProxy
 from zope.testbrowser.browser import LinkNotFoundError
 
 from lp.app.interfaces.launchpad import ILaunchpadCelebrities
@@ -41,6 +42,7 @@ from lp.oci.interfaces.ocirecipe import (
     OCI_RECIPE_ALLOW_CREATE,
     )
 from lp.services.database.constants import UTC_NOW
+from lp.services.database.interfaces import IStore
 from lp.services.features.testing import FeatureFixture
 from lp.services.propertycache import get_property_cache
 from lp.services.webapp import canonical_url
@@ -561,9 +563,16 @@ class TestOCIRecipeDeleteView(BaseTestOCIRecipeView):
         # An OCI recipe with builds can be deleted.
         recipe = self.factory.makeOCIRecipe(
             registrant=self.person, owner=self.person)
-        self.factory.makeOCIRecipeBuild(recipe=recipe)
-        # XXX cjwatson 2020-02-19: This should also add a file to the build
-        # once that works.
+        ocibuild = removeSecurityProxy(
+            self.factory.makeOCIRecipeBuild(recipe=recipe))
+        ocifile = removeSecurityProxy(
+            self.factory.makeOCIFile(build=ocibuild))
+
+        unrelated_build = removeSecurityProxy(
+            self.factory.makeOCIRecipeBuild())
+        unrelated_file = removeSecurityProxy(
+            self.factory.makeOCIFile(build=unrelated_build))
+
         recipe_url = canonical_url(recipe)
         oci_project_url = canonical_url(recipe.oci_project)
         browser = self.getViewBrowser(recipe, user=self.person)
@@ -572,6 +581,17 @@ class TestOCIRecipeDeleteView(BaseTestOCIRecipeView):
         self.assertEqual(oci_project_url, browser.url)
         self.assertRaises(NotFound, browser.open, recipe_url)
 
+        # Checks that only the related artifacts were deleted too.
+        def obj_exists(obj):
+            store = IStore(obj)
+            cls = obj.__class__
+            return not store.find(cls, cls.id == obj.id).is_empty()
+        self.assertFalse(obj_exists(ocibuild))
+        self.assertFalse(obj_exists(ocifile))
+
+        self.assertTrue(obj_exists(unrelated_build))
+        self.assertTrue(obj_exists(unrelated_file))
+
 
 class TestOCIRecipeView(BaseTestOCIRecipeView):
 
diff --git a/lib/lp/oci/model/ocirecipe.py b/lib/lp/oci/model/ocirecipe.py
index 38e5a9a..0b5459f 100644
--- a/lib/lp/oci/model/ocirecipe.py
+++ b/lib/lp/oci/model/ocirecipe.py
@@ -68,7 +68,10 @@ from lp.oci.interfaces.ociregistrycredentials import (
     IOCIRegistryCredentialsSet,
     )
 from lp.oci.model.ocipushrule import OCIPushRule
-from lp.oci.model.ocirecipebuild import OCIRecipeBuild
+from lp.oci.model.ocirecipebuild import (
+    OCIFile,
+    OCIRecipeBuild,
+    )
 from lp.registry.interfaces.distribution import IDistributionSet
 from lp.registry.interfaces.person import IPersonSet
 from lp.registry.interfaces.role import IPersonRoles
@@ -180,6 +183,17 @@ class OCIRecipe(Storm, WebhookTargetMixin):
         """See `IOCIProject.setOfficialRecipe` method."""
         return self._official
 
+    def _destroyRelatedBuilds(self):
+        """Remove builds associated with this OCIRecipe and its files."""
+        store = IStore(OCIRecipe)
+        recipe_build_filter = (OCIRecipeBuild.recipe == self)
+        builds = store.find(OCIRecipeBuild, recipe_build_filter)
+        build_ids = store.find(OCIRecipeBuild.id, recipe_build_filter)
+        ocifiles = store.find(
+            OCIFile, OCIFile.build_id.is_in(build_ids))
+        ocifiles.remove()
+        builds.remove()
+
     def destroySelf(self):
         """See `IOCIRecipe`."""
         # XXX twom 2019-11-26 This needs to expand as more build artifacts
@@ -194,7 +208,9 @@ class OCIRecipe(Storm, WebhookTargetMixin):
             buildqueue_record.destroySelf()
         build_farm_job_ids = list(store.find(
             OCIRecipeBuild.build_farm_job_id, OCIRecipeBuild.recipe == self))
-        store.find(OCIRecipeBuild, OCIRecipeBuild.recipe == self).remove()
+
+        self._destroyRelatedBuilds()
+
         getUtility(IWebhookSet).delete(self.webhooks)
         store.remove(self)
         store.find(