Merge ~cjwatson/launchpad:built-using-guard-deletion into launchpad:master

[Colin Watson]

Guard removal of sources referenced by Built-Using

Prevent SourcePackagePublishingHistory.requestDeletion from deleting source publications that have Built-Using references from active binary publications in the same archive and suite.

This isn't necessarily complete: in particular, it can miss references from other pockets, and in any case it might race with a build still in progress. The intent of this is not to ensure integrity, but to avoid some easily-detectable mistakes that could cause confusion.

c0c4aa2... by Colin Watson on 2020-04-22

Expand deletion guard to other pockets

It now checks all pockets that could legitimately depend on the one from
which the publication is being deleted.

06ccc3e... by Colin Watson on 2020-04-22

Simplify tests using createFromSourcePackageReleases

Unmerged commits

06ccc3e... by Colin Watson on 2020-04-22

Simplify tests using createFromSourcePackageReleases

c0c4aa2... by Colin Watson on 2020-04-22

Expand deletion guard to other pockets

It now checks all pockets that could legitimately depend on the one from
which the publication is being deleted.

d7fbcfd... by Colin Watson on 2020-03-26

Guard removal of sources referenced by Built-Using

Prevent SourcePackagePublishingHistory.requestDeletion from deleting
source publications that have Built-Using references from active binary
publications in the same archive and suite.

This isn't necessarily complete: in particular, it can miss references
from other pockets, and in any case it might race with a build still in
progress. The intent of this is not to ensure integrity, but to avoid
some easily-detectable mistakes that could cause confusion.

LP: #1868558

diff --git a/lib/lp/soyuz/interfaces/binarysourcereference.py b/lib/lp/soyuz/interfaces/binarysourcereference.py
index dc21765..e625cf9 100644
--- a/lib/lp/soyuz/interfaces/binarysourcereference.py
+++ b/lib/lp/soyuz/interfaces/binarysourcereference.py
@@ -84,3 +84,18 @@ class IBinarySourceReferenceSet(Interface):
         :param reference_type: A `BinarySourceReferenceType` to search for.
         :return: A `ResultSet` of matching `IBinarySourceReference`s.
         """
+
+    def findPublished(archive, distroseries, pockets, reference_type,
+                      source_package_releases=None):
+        """Find references corresponding to published binary publications.
+
+        :param archive: An `IArchive` to search for binary publications.
+        :param distroseries: An `IDistroSeries` to search for binary
+            publications.
+        :param pocket: A collection of `PackagePublishingPocket`s to search
+            for binary publications.
+        :param reference_type: A `BinarySourceReferenceType` to search for.
+        :param source_package_releases: If not None, only return references
+            pointing to any of this sequence of `ISourcePackageRelease`s.
+        :return: A `ResultSet` of matching `IBinarySourceReference`s.
+        """
diff --git a/lib/lp/soyuz/model/binarysourcereference.py b/lib/lp/soyuz/model/binarysourcereference.py
index adcfb6f..ca3563b 100644
--- a/lib/lp/soyuz/model/binarysourcereference.py
+++ b/lib/lp/soyuz/model/binarysourcereference.py
@@ -25,12 +25,17 @@ from lp.services.database.enumcol import DBEnum
 from lp.services.database.interfaces import IStore
 from lp.services.database.stormbase import StormBase
 from lp.soyuz.adapters.archivedependencies import pocket_dependencies
-from lp.soyuz.enums import BinarySourceReferenceType
+from lp.soyuz.enums import (
+    BinarySourceReferenceType,
+    PackagePublishingStatus,
+    )
 from lp.soyuz.interfaces.binarysourcereference import (
     IBinarySourceReference,
     IBinarySourceReferenceSet,
     UnparsableBuiltUsing,
     )
+from lp.soyuz.model.distroarchseries import DistroArchSeries
+from lp.soyuz.model.publishing import BinaryPackagePublishingHistory
 
 
 @implementer(IBinarySourceReference)
@@ -147,3 +152,26 @@ class BinarySourceReferenceSet:
             BinarySourceReference,
             BinarySourceReference.binary_package_release == bpr,
             BinarySourceReference.reference_type == reference_type)
+
+    @classmethod
+    def findPublished(cls, archive, distroseries, pockets, reference_type,
+                      source_package_releases=None):
+        """See `IBinarySourceReferenceSet`."""
+        clauses = [
+            BinarySourceReference.binary_package_release_id ==
+                BinaryPackagePublishingHistory.binarypackagereleaseID,
+            BinarySourceReference.reference_type == reference_type,
+            BinaryPackagePublishingHistory.status ==
+                PackagePublishingStatus.PUBLISHED,
+            BinaryPackagePublishingHistory.archive == archive,
+            BinaryPackagePublishingHistory.distroarchseries ==
+                DistroArchSeries.id,
+            BinaryPackagePublishingHistory.pocket.is_in(pockets),
+            DistroArchSeries.distroseries == distroseries,
+            ]
+        if source_package_releases is not None:
+            clauses.append(
+                BinarySourceReference.source_package_release_id.is_in(
+                    spr.id for spr in source_package_releases))
+        return IStore(BinarySourceReference).find(
+            BinarySourceReference, *clauses).config(distinct=True)
diff --git a/lib/lp/soyuz/model/publishing.py b/lib/lp/soyuz/model/publishing.py
index fafd7f9..3c7ce7d 100644
--- a/lib/lp/soyuz/model/publishing.py
+++ b/lib/lp/soyuz/model/publishing.py
@@ -76,9 +76,11 @@ from lp.services.webapp.errorlog import (
     ScriptRequest,
     )
 from lp.services.worlddata.model.country import Country
+from lp.soyuz.adapters.archivedependencies import pocket_dependencies
 from lp.soyuz.adapters.proxiedsourcefiles import ProxiedSourceLibraryFileAlias
 from lp.soyuz.enums import (
     BinaryPackageFormat,
+    BinarySourceReferenceType,
     PackagePublishingPriority,
     PackagePublishingStatus,
     PackageUploadStatus,
@@ -87,6 +89,9 @@ from lp.soyuz.interfaces.binarypackagebuild import (
     BuildSetStatus,
     IBinaryPackageBuildSet,
     )
+from lp.soyuz.interfaces.binarysourcereference import (
+    IBinarySourceReferenceSet,
+    )
 from lp.soyuz.interfaces.component import IComponentSet
 from lp.soyuz.interfaces.distributionjob import (
     IDistroSeriesDifferenceJobSource,
@@ -603,6 +608,25 @@ class SourcePackagePublishingHistory(SQLBase, ArchivePublisherBase):
                 "Cannot delete publications from suite '%s'" %
                 self.distroseries.getSuite(self.pocket))
 
+        # Check for active Built-Using references to this source package
+        # from any pockets in this archive and series that could
+        # legitimately refer to it.  This isn't necessarily complete: it's
+        # there to avoid confusing consequences of mistakes, not to ensure
+        # integrity.  (The dominator will reinstate a source publication if
+        # necessary.)
+        reverse_pockets = {
+            source_pocket
+            for source_pocket, expanded_pockets in pocket_dependencies.items()
+            if self.pocket in expanded_pockets}
+        bsrs = getUtility(IBinarySourceReferenceSet).findPublished(
+            self.archive, self.distroseries, reverse_pockets,
+            BinarySourceReferenceType.BUILT_USING,
+            source_package_releases=[self.sourcepackagerelease])
+        if not bsrs.is_empty():
+            raise DeletionError(
+                "Cannot delete source publication with a Built-Using "
+                "reference from an active binary publication.")
+
         self.setDeleted(removed_by, removal_comment)
         if self.archive.is_main:
             dsd_job_source = getUtility(IDistroSeriesDifferenceJobSource)
@@ -1712,6 +1736,36 @@ class PublishingSet:
                     "Cannot delete publications from suite '%s'" %
                     distroseries.getSuite(pocket))
 
+        # Check for active Built-Using references to these source packages
+        # from any pockets in this archive and series that could
+        # legitimately refer to them.  This isn't necessarily complete: it's
+        # there to avoid confusing consequences of mistakes, not to ensure
+        # integrity.  (The dominator will reinstate a source publication if
+        # necessary.)
+        if sources:
+            # XXX cjwatson 2020-03-24: In theory we could consolidate this
+            # into one query, but it's complex and probably not worth it.
+            sprs_by_location = defaultdict(list)
+            for spph in sources:
+                sprs_by_location[
+                    (spph.archive, spph.distroseries, spph.pocket)].append(
+                        spph.sourcepackagerelease)
+            for (archive, distroseries, pocket), sprs in (
+                    sprs_by_location.items()):
+                reverse_pockets = {
+                    source_pocket
+                    for source_pocket, expanded_pockets
+                    in pocket_dependencies.items()
+                    if pocket in expanded_pockets}
+                bsrs = getUtility(IBinarySourceReferenceSet).findPublished(
+                    archive, distroseries, reverse_pockets,
+                    BinarySourceReferenceType.BUILT_USING,
+                    source_package_releases=sprs)
+                if not bsrs.is_empty():
+                    raise DeletionError(
+                        "Cannot delete source publication with a Built-Using "
+                        "reference from an active binary publication.")
+
         spph_ids = [spph.id for spph in sources]
         self.setMultipleDeleted(
             SourcePackagePublishingHistory, spph_ids, removed_by,
diff --git a/lib/lp/soyuz/tests/test_binarysourcereference.py b/lib/lp/soyuz/tests/test_binarysourcereference.py
index f682de8..0c4db7f 100644
--- a/lib/lp/soyuz/tests/test_binarysourcereference.py
+++ b/lib/lp/soyuz/tests/test_binarysourcereference.py
@@ -20,6 +20,7 @@ from lp.registry.interfaces.pocket import PackagePublishingPocket
 from lp.soyuz.enums import (
     ArchivePurpose,
     BinarySourceReferenceType,
+    PackagePublishingStatus,
     )
 from lp.soyuz.interfaces.binarysourcereference import (
     IBinarySourceReferenceSet,
@@ -206,3 +207,102 @@ class TestBinarySourceReference(TestCaseWithFactory):
             [],
             self.reference_set.findByBinaryPackageRelease(
                 other_bpr, BinarySourceReferenceType.BUILT_USING))
+
+    def test_findPublished_empty(self):
+        archive = self.factory.makeArchive()
+        self.assertContentEqual(
+            [],
+            self.reference_set.findPublished(
+                archive, archive.distribution.currentseries,
+                {PackagePublishingPocket.RELEASE},
+                BinarySourceReferenceType.BUILT_USING))
+
+    def test_findPublished(self):
+        # findPublished finds references corresponding to published binary
+        # publications.
+        das = self.factory.makeDistroArchSeries()
+        archive = das.main_archive
+        bpphs = [
+            self.factory.makeBinaryPackagePublishingHistory(
+                archive=archive, distroarchseries=das, pocket=pocket,
+                status=PackagePublishingStatus.PUBLISHED)
+            for pocket in (
+                PackagePublishingPocket.RELEASE,
+                PackagePublishingPocket.PROPOSED)]
+        all_sprs = []
+        bsrs = []
+        for bpph in bpphs:
+            spphs = [
+                self.factory.makeSourcePackagePublishingHistory(
+                    archive=bpph.archive, distroseries=bpph.distroseries,
+                    pocket=bpph.pocket)
+                for _ in range(2)]
+            sprs = [spph.sourcepackagerelease for spph in spphs]
+            all_sprs.extend(sprs)
+            bsrs.extend(self.reference_set.createFromSourcePackageReleases(
+                bpph.binarypackagerelease, sprs,
+                BinarySourceReferenceType.BUILT_USING))
+
+        # Create a few more BPPHs with slight mismatches to ensure that
+        # findPublished matches correctly.
+        other_bpphs = []
+        other_bpphs.append(self.factory.makeBinaryPackagePublishingHistory(
+            archive=archive, pocket=PackagePublishingPocket.RELEASE,
+            status=PackagePublishingStatus.PUBLISHED))
+        other_bpphs.append(self.factory.makeBinaryPackagePublishingHistory(
+            archive=self.factory.makeArchive(
+                distribution=das.distroseries.distribution),
+            distroarchseries=das, pocket=PackagePublishingPocket.RELEASE,
+            status=PackagePublishingStatus.PUBLISHED))
+        other_bpphs.append(self.factory.makeBinaryPackagePublishingHistory(
+            archive=archive, distroarchseries=das,
+            pocket=PackagePublishingPocket.UPDATES,
+            status=PackagePublishingStatus.PUBLISHED))
+        other_bpphs.append(self.factory.makeBinaryPackagePublishingHistory(
+            archive=archive, distroarchseries=das,
+            pocket=PackagePublishingPocket.RELEASE,
+            status=PackagePublishingStatus.SUPERSEDED))
+        for bpph in other_bpphs:
+            spr = self.factory.makeSourcePackagePublishingHistory(
+                archive=bpph.archive, distroseries=bpph.distroseries,
+                pocket=bpph.pocket).sourcepackagerelease
+            self.reference_set.createFromSourcePackageReleases(
+                bpph.binarypackagerelease, [spr],
+                BinarySourceReferenceType.BUILT_USING)
+
+        self.assertThat(
+            self.reference_set.findPublished(
+                archive, das.distroseries, {PackagePublishingPocket.RELEASE},
+                BinarySourceReferenceType.BUILT_USING),
+            MatchesSetwise(*(
+                MatchesStructure.byEquality(
+                    binary_package_release=bsr.binary_package_release,
+                    source_package_release=bsr.source_package_release,
+                    reference_type=BinarySourceReferenceType.BUILT_USING)
+                for bsr in bsrs[:2])))
+        self.assertThat(
+            self.reference_set.findPublished(
+                archive, das.distroseries,
+                {PackagePublishingPocket.RELEASE,
+                 PackagePublishingPocket.PROPOSED},
+                BinarySourceReferenceType.BUILT_USING),
+            MatchesSetwise(*(
+                MatchesStructure.byEquality(
+                    binary_package_release=bsr.binary_package_release,
+                    source_package_release=bsr.source_package_release,
+                    reference_type=BinarySourceReferenceType.BUILT_USING)
+                for bsr in bsrs)))
+        self.assertThat(
+            self.reference_set.findPublished(
+                archive, das.distroseries,
+                {PackagePublishingPocket.RELEASE,
+                 PackagePublishingPocket.PROPOSED},
+                BinarySourceReferenceType.BUILT_USING,
+                source_package_releases=[
+                    bsr.source_package_release for bsr in [bsrs[0], bsrs[2]]]),
+            MatchesSetwise(*(
+                MatchesStructure.byEquality(
+                    binary_package_release=bsr.binary_package_release,
+                    source_package_release=bsr.source_package_release,
+                    reference_type=BinarySourceReferenceType.BUILT_USING)
+                for bsr in [bsrs[0], bsrs[2]])))
diff --git a/lib/lp/soyuz/tests/test_publishing.py b/lib/lp/soyuz/tests/test_publishing.py
index cd7d1ea..43d9b91 100644
--- a/lib/lp/soyuz/tests/test_publishing.py
+++ b/lib/lp/soyuz/tests/test_publishing.py
@@ -38,9 +38,13 @@ from lp.services.log.logger import DevNullLogger
 from lp.soyuz.enums import (
     ArchivePurpose,
     BinaryPackageFormat,
+    BinarySourceReferenceType,
     PackageUploadStatus,
     )
 from lp.soyuz.interfaces.binarypackagename import IBinaryPackageNameSet
+from lp.soyuz.interfaces.binarysourcereference import (
+    IBinarySourceReferenceSet,
+    )
 from lp.soyuz.interfaces.component import IComponentSet
 from lp.soyuz.interfaces.publishing import (
     active_publishing_status,
@@ -984,6 +988,97 @@ class TestPublishingSetLite(TestCaseWithFactory):
             self.assertRaisesWithContent(
                 DeletionError, message, pub.api_requestDeletion, self.person)
 
+    def test_requestDeletion_singular_disallows_active_built_using_refs(self):
+        # The singular version of requestDeletion checks that there are no
+        # active Built-Using references to the source package being deleted.
+        archive = self.factory.makeArchive(purpose=ArchivePurpose.PRIMARY)
+        distroseries = self.factory.makeDistroSeries(
+            distribution=archive.distribution)
+        das = self.factory.makeDistroArchSeries(distroseries=distroseries)
+        bpphs = [
+            self.factory.makeBinaryPackagePublishingHistory(
+                archive=archive, distroarchseries=das, pocket=pocket,
+                status=PackagePublishingStatus.PUBLISHED)
+            for pocket in (
+                PackagePublishingPocket.RELEASE,
+                PackagePublishingPocket.PROPOSED)]
+        spph = self.factory.makeSourcePackagePublishingHistory(
+            archive=archive, distroseries=distroseries, pocket=bpphs[0].pocket,
+            status=PackagePublishingStatus.PUBLISHED)
+        bsr_set = getUtility(IBinarySourceReferenceSet)
+        for bpph in bpphs:
+            spr = spph.sourcepackagerelease
+            bsr_set.createFromSourcePackageReleases(
+                bpph.binarypackagerelease, [spr],
+                BinarySourceReferenceType.BUILT_USING)
+
+        # As long as the referring binary publications are active, deletion
+        # is disallowed.
+        message = (
+            "Cannot delete source publication with a Built-Using reference "
+            "from an active binary publication.")
+        self.assertRaisesWithContent(
+            DeletionError, message, spph.requestDeletion, self.person)
+        self.assertRaisesWithContent(
+            DeletionError, message, spph.api_requestDeletion, self.person)
+        bpphs[0].requestDeletion(self.person)
+        self.assertRaisesWithContent(
+            DeletionError, message, spph.requestDeletion, self.person)
+        self.assertRaisesWithContent(
+            DeletionError, message, spph.api_requestDeletion, self.person)
+        bpphs[1].requestDeletion(self.person)
+
+        # Now that all the referring binary publications are deleted, we
+        # allow deleting the source publication.
+        spph.requestDeletion(self.person)
+
+    def test_requestDeletion_bulk_disallows_active_built_using_refs(self):
+        # The bulk version of requestDeletion checks that there are no
+        # active Built-Using references to the source packages being
+        # deleted.
+        archive = self.factory.makeArchive(purpose=ArchivePurpose.PRIMARY)
+        distroseries = self.factory.makeDistroSeries(
+            distribution=archive.distribution)
+        das = self.factory.makeDistroArchSeries(distroseries=distroseries)
+        bpphs = [
+            self.factory.makeBinaryPackagePublishingHistory(
+                archive=archive, distroarchseries=das, pocket=pocket,
+                status=PackagePublishingStatus.PUBLISHED)
+            for pocket in (
+                PackagePublishingPocket.RELEASE,
+                PackagePublishingPocket.PROPOSED)]
+        spphs = [
+            self.factory.makeSourcePackagePublishingHistory(
+                archive=archive, distroseries=distroseries,
+                pocket=PackagePublishingPocket.RELEASE,
+                status=PackagePublishingStatus.PUBLISHED)
+            for bpph in bpphs]
+        bsr_set = getUtility(IBinarySourceReferenceSet)
+        for bpph in bpphs:
+            for spph in spphs:
+                spr = spph.sourcepackagerelease
+                bsr_set.createFromSourcePackageReleases(
+                    bpph.binarypackagerelease, [spr],
+                    BinarySourceReferenceType.BUILT_USING)
+
+        # As long as the referring binary publications are active, deletion
+        # is disallowed.
+        message = (
+            "Cannot delete source publication with a Built-Using reference "
+            "from an active binary publication.")
+        self.assertRaisesWithContent(
+            DeletionError, message,
+            getUtility(IPublishingSet).requestDeletion, spphs, self.person)
+        bpphs[0].requestDeletion(self.person)
+        self.assertRaisesWithContent(
+            DeletionError, message,
+            getUtility(IPublishingSet).requestDeletion, spphs, self.person)
+        bpphs[1].requestDeletion(self.person)
+
+        # Now that all the referring binary publications are deleted, we
+        # allow deleting the source publications.
+        getUtility(IPublishingSet).requestDeletion(spphs, self.person)
+
     def test_requestDeletion_marks_debug_as_deleted(self):
         matching_bpph, debug_matching_bpph = (
             self.factory.makeBinaryPackagePublishingHistory(