Launchpad itself

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

  1. Git
  2. lp:~cjwatson/launchpad
  3. built-using-guard-deletion
  4. Merge into master
Proposed by Colin Watson
Status: Needs review
Proposed branch: ~cjwatson/launchpad:built-using-guard-deletion
Merge into: launchpad:master
Prerequisite: ~cjwatson/launchpad:built-using-ui
Diff against target: 393 lines (+293/-1)
5 files modified
lib/lp/soyuz/interfaces/binarysourcereference.py (+15/-0)
lib/lp/soyuz/model/binarysourcereference.py (+29/-1)
lib/lp/soyuz/model/publishing.py (+54/-0)
lib/lp/soyuz/tests/test_binarysourcereference.py (+100/-0)
lib/lp/soyuz/tests/test_publishing.py (+95/-0)
Reviewer Review Type Date Requested Status
Launchpad code reviewers Pending
Review via email: mp+381239@code.launchpad.net

Commit message

Guard removal of sources referenced by Built-Using

Description of the change

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.

To post a comment you must log in.
~cjwatson/launchpad:built-using-guard-deletion updated
c0c4aa2... by Colin Watson

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

Simplify tests using createFromSourcePackageReleases

Unmerged commits

06ccc3e... by Colin Watson

Simplify tests using createFromSourcePackageReleases

c0c4aa2... by Colin Watson

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

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

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1diff --git a/lib/lp/soyuz/interfaces/binarysourcereference.py b/lib/lp/soyuz/interfaces/binarysourcereference.py
2index dc21765..e625cf9 100644
3--- a/lib/lp/soyuz/interfaces/binarysourcereference.py
4+++ b/lib/lp/soyuz/interfaces/binarysourcereference.py
5@@ -84,3 +84,18 @@ class IBinarySourceReferenceSet(Interface):
6 :param reference_type: A `BinarySourceReferenceType` to search for.
7 :return: A `ResultSet` of matching `IBinarySourceReference`s.
8 """
9+
10+ def findPublished(archive, distroseries, pockets, reference_type,
11+ source_package_releases=None):
12+ """Find references corresponding to published binary publications.
13+
14+ :param archive: An `IArchive` to search for binary publications.
15+ :param distroseries: An `IDistroSeries` to search for binary
16+ publications.
17+ :param pocket: A collection of `PackagePublishingPocket`s to search
18+ for binary publications.
19+ :param reference_type: A `BinarySourceReferenceType` to search for.
20+ :param source_package_releases: If not None, only return references
21+ pointing to any of this sequence of `ISourcePackageRelease`s.
22+ :return: A `ResultSet` of matching `IBinarySourceReference`s.
23+ """
24diff --git a/lib/lp/soyuz/model/binarysourcereference.py b/lib/lp/soyuz/model/binarysourcereference.py
25index adcfb6f..ca3563b 100644
26--- a/lib/lp/soyuz/model/binarysourcereference.py
27+++ b/lib/lp/soyuz/model/binarysourcereference.py
28@@ -25,12 +25,17 @@ from lp.services.database.enumcol import DBEnum
29 from lp.services.database.interfaces import IStore
30 from lp.services.database.stormbase import StormBase
31 from lp.soyuz.adapters.archivedependencies import pocket_dependencies
32-from lp.soyuz.enums import BinarySourceReferenceType
33+from lp.soyuz.enums import (
34+ BinarySourceReferenceType,
35+ PackagePublishingStatus,
36+ )
37 from lp.soyuz.interfaces.binarysourcereference import (
38 IBinarySourceReference,
39 IBinarySourceReferenceSet,
40 UnparsableBuiltUsing,
41 )
42+from lp.soyuz.model.distroarchseries import DistroArchSeries
43+from lp.soyuz.model.publishing import BinaryPackagePublishingHistory
44
45
46 @implementer(IBinarySourceReference)
47@@ -147,3 +152,26 @@ class BinarySourceReferenceSet:
48 BinarySourceReference,
49 BinarySourceReference.binary_package_release == bpr,
50 BinarySourceReference.reference_type == reference_type)
51+
52+ @classmethod
53+ def findPublished(cls, archive, distroseries, pockets, reference_type,
54+ source_package_releases=None):
55+ """See `IBinarySourceReferenceSet`."""
56+ clauses = [
57+ BinarySourceReference.binary_package_release_id ==
58+ BinaryPackagePublishingHistory.binarypackagereleaseID,
59+ BinarySourceReference.reference_type == reference_type,
60+ BinaryPackagePublishingHistory.status ==
61+ PackagePublishingStatus.PUBLISHED,
62+ BinaryPackagePublishingHistory.archive == archive,
63+ BinaryPackagePublishingHistory.distroarchseries ==
64+ DistroArchSeries.id,
65+ BinaryPackagePublishingHistory.pocket.is_in(pockets),
66+ DistroArchSeries.distroseries == distroseries,
67+ ]
68+ if source_package_releases is not None:
69+ clauses.append(
70+ BinarySourceReference.source_package_release_id.is_in(
71+ spr.id for spr in source_package_releases))
72+ return IStore(BinarySourceReference).find(
73+ BinarySourceReference, *clauses).config(distinct=True)
74diff --git a/lib/lp/soyuz/model/publishing.py b/lib/lp/soyuz/model/publishing.py
75index fafd7f9..3c7ce7d 100644
76--- a/lib/lp/soyuz/model/publishing.py
77+++ b/lib/lp/soyuz/model/publishing.py
78@@ -76,9 +76,11 @@ from lp.services.webapp.errorlog import (
79 ScriptRequest,
80 )
81 from lp.services.worlddata.model.country import Country
82+from lp.soyuz.adapters.archivedependencies import pocket_dependencies
83 from lp.soyuz.adapters.proxiedsourcefiles import ProxiedSourceLibraryFileAlias
84 from lp.soyuz.enums import (
85 BinaryPackageFormat,
86+ BinarySourceReferenceType,
87 PackagePublishingPriority,
88 PackagePublishingStatus,
89 PackageUploadStatus,
90@@ -87,6 +89,9 @@ from lp.soyuz.interfaces.binarypackagebuild import (
91 BuildSetStatus,
92 IBinaryPackageBuildSet,
93 )
94+from lp.soyuz.interfaces.binarysourcereference import (
95+ IBinarySourceReferenceSet,
96+ )
97 from lp.soyuz.interfaces.component import IComponentSet
98 from lp.soyuz.interfaces.distributionjob import (
99 IDistroSeriesDifferenceJobSource,
100@@ -603,6 +608,25 @@ class SourcePackagePublishingHistory(SQLBase, ArchivePublisherBase):
101 "Cannot delete publications from suite '%s'" %
102 self.distroseries.getSuite(self.pocket))
103
104+ # Check for active Built-Using references to this source package
105+ # from any pockets in this archive and series that could
106+ # legitimately refer to it. This isn't necessarily complete: it's
107+ # there to avoid confusing consequences of mistakes, not to ensure
108+ # integrity. (The dominator will reinstate a source publication if
109+ # necessary.)
110+ reverse_pockets = {
111+ source_pocket
112+ for source_pocket, expanded_pockets in pocket_dependencies.items()
113+ if self.pocket in expanded_pockets}
114+ bsrs = getUtility(IBinarySourceReferenceSet).findPublished(
115+ self.archive, self.distroseries, reverse_pockets,
116+ BinarySourceReferenceType.BUILT_USING,
117+ source_package_releases=[self.sourcepackagerelease])
118+ if not bsrs.is_empty():
119+ raise DeletionError(
120+ "Cannot delete source publication with a Built-Using "
121+ "reference from an active binary publication.")
122+
123 self.setDeleted(removed_by, removal_comment)
124 if self.archive.is_main:
125 dsd_job_source = getUtility(IDistroSeriesDifferenceJobSource)
126@@ -1712,6 +1736,36 @@ class PublishingSet:
127 "Cannot delete publications from suite '%s'" %
128 distroseries.getSuite(pocket))
129
130+ # Check for active Built-Using references to these source packages
131+ # from any pockets in this archive and series that could
132+ # legitimately refer to them. This isn't necessarily complete: it's
133+ # there to avoid confusing consequences of mistakes, not to ensure
134+ # integrity. (The dominator will reinstate a source publication if
135+ # necessary.)
136+ if sources:
137+ # XXX cjwatson 2020-03-24: In theory we could consolidate this
138+ # into one query, but it's complex and probably not worth it.
139+ sprs_by_location = defaultdict(list)
140+ for spph in sources:
141+ sprs_by_location[
142+ (spph.archive, spph.distroseries, spph.pocket)].append(
143+ spph.sourcepackagerelease)
144+ for (archive, distroseries, pocket), sprs in (
145+ sprs_by_location.items()):
146+ reverse_pockets = {
147+ source_pocket
148+ for source_pocket, expanded_pockets
149+ in pocket_dependencies.items()
150+ if pocket in expanded_pockets}
151+ bsrs = getUtility(IBinarySourceReferenceSet).findPublished(
152+ archive, distroseries, reverse_pockets,
153+ BinarySourceReferenceType.BUILT_USING,
154+ source_package_releases=sprs)
155+ if not bsrs.is_empty():
156+ raise DeletionError(
157+ "Cannot delete source publication with a Built-Using "
158+ "reference from an active binary publication.")
159+
160 spph_ids = [spph.id for spph in sources]
161 self.setMultipleDeleted(
162 SourcePackagePublishingHistory, spph_ids, removed_by,
163diff --git a/lib/lp/soyuz/tests/test_binarysourcereference.py b/lib/lp/soyuz/tests/test_binarysourcereference.py
164index f682de8..0c4db7f 100644
165--- a/lib/lp/soyuz/tests/test_binarysourcereference.py
166+++ b/lib/lp/soyuz/tests/test_binarysourcereference.py
167@@ -20,6 +20,7 @@ from lp.registry.interfaces.pocket import PackagePublishingPocket
168 from lp.soyuz.enums import (
169 ArchivePurpose,
170 BinarySourceReferenceType,
171+ PackagePublishingStatus,
172 )
173 from lp.soyuz.interfaces.binarysourcereference import (
174 IBinarySourceReferenceSet,
175@@ -206,3 +207,102 @@ class TestBinarySourceReference(TestCaseWithFactory):
176 [],
177 self.reference_set.findByBinaryPackageRelease(
178 other_bpr, BinarySourceReferenceType.BUILT_USING))
179+
180+ def test_findPublished_empty(self):
181+ archive = self.factory.makeArchive()
182+ self.assertContentEqual(
183+ [],
184+ self.reference_set.findPublished(
185+ archive, archive.distribution.currentseries,
186+ {PackagePublishingPocket.RELEASE},
187+ BinarySourceReferenceType.BUILT_USING))
188+
189+ def test_findPublished(self):
190+ # findPublished finds references corresponding to published binary
191+ # publications.
192+ das = self.factory.makeDistroArchSeries()
193+ archive = das.main_archive
194+ bpphs = [
195+ self.factory.makeBinaryPackagePublishingHistory(
196+ archive=archive, distroarchseries=das, pocket=pocket,
197+ status=PackagePublishingStatus.PUBLISHED)
198+ for pocket in (
199+ PackagePublishingPocket.RELEASE,
200+ PackagePublishingPocket.PROPOSED)]
201+ all_sprs = []
202+ bsrs = []
203+ for bpph in bpphs:
204+ spphs = [
205+ self.factory.makeSourcePackagePublishingHistory(
206+ archive=bpph.archive, distroseries=bpph.distroseries,
207+ pocket=bpph.pocket)
208+ for _ in range(2)]
209+ sprs = [spph.sourcepackagerelease for spph in spphs]
210+ all_sprs.extend(sprs)
211+ bsrs.extend(self.reference_set.createFromSourcePackageReleases(
212+ bpph.binarypackagerelease, sprs,
213+ BinarySourceReferenceType.BUILT_USING))
214+
215+ # Create a few more BPPHs with slight mismatches to ensure that
216+ # findPublished matches correctly.
217+ other_bpphs = []
218+ other_bpphs.append(self.factory.makeBinaryPackagePublishingHistory(
219+ archive=archive, pocket=PackagePublishingPocket.RELEASE,
220+ status=PackagePublishingStatus.PUBLISHED))
221+ other_bpphs.append(self.factory.makeBinaryPackagePublishingHistory(
222+ archive=self.factory.makeArchive(
223+ distribution=das.distroseries.distribution),
224+ distroarchseries=das, pocket=PackagePublishingPocket.RELEASE,
225+ status=PackagePublishingStatus.PUBLISHED))
226+ other_bpphs.append(self.factory.makeBinaryPackagePublishingHistory(
227+ archive=archive, distroarchseries=das,
228+ pocket=PackagePublishingPocket.UPDATES,
229+ status=PackagePublishingStatus.PUBLISHED))
230+ other_bpphs.append(self.factory.makeBinaryPackagePublishingHistory(
231+ archive=archive, distroarchseries=das,
232+ pocket=PackagePublishingPocket.RELEASE,
233+ status=PackagePublishingStatus.SUPERSEDED))
234+ for bpph in other_bpphs:
235+ spr = self.factory.makeSourcePackagePublishingHistory(
236+ archive=bpph.archive, distroseries=bpph.distroseries,
237+ pocket=bpph.pocket).sourcepackagerelease
238+ self.reference_set.createFromSourcePackageReleases(
239+ bpph.binarypackagerelease, [spr],
240+ BinarySourceReferenceType.BUILT_USING)
241+
242+ self.assertThat(
243+ self.reference_set.findPublished(
244+ archive, das.distroseries, {PackagePublishingPocket.RELEASE},
245+ BinarySourceReferenceType.BUILT_USING),
246+ MatchesSetwise(*(
247+ MatchesStructure.byEquality(
248+ binary_package_release=bsr.binary_package_release,
249+ source_package_release=bsr.source_package_release,
250+ reference_type=BinarySourceReferenceType.BUILT_USING)
251+ for bsr in bsrs[:2])))
252+ self.assertThat(
253+ self.reference_set.findPublished(
254+ archive, das.distroseries,
255+ {PackagePublishingPocket.RELEASE,
256+ PackagePublishingPocket.PROPOSED},
257+ BinarySourceReferenceType.BUILT_USING),
258+ MatchesSetwise(*(
259+ MatchesStructure.byEquality(
260+ binary_package_release=bsr.binary_package_release,
261+ source_package_release=bsr.source_package_release,
262+ reference_type=BinarySourceReferenceType.BUILT_USING)
263+ for bsr in bsrs)))
264+ self.assertThat(
265+ self.reference_set.findPublished(
266+ archive, das.distroseries,
267+ {PackagePublishingPocket.RELEASE,
268+ PackagePublishingPocket.PROPOSED},
269+ BinarySourceReferenceType.BUILT_USING,
270+ source_package_releases=[
271+ bsr.source_package_release for bsr in [bsrs[0], bsrs[2]]]),
272+ MatchesSetwise(*(
273+ MatchesStructure.byEquality(
274+ binary_package_release=bsr.binary_package_release,
275+ source_package_release=bsr.source_package_release,
276+ reference_type=BinarySourceReferenceType.BUILT_USING)
277+ for bsr in [bsrs[0], bsrs[2]])))
278diff --git a/lib/lp/soyuz/tests/test_publishing.py b/lib/lp/soyuz/tests/test_publishing.py
279index cd7d1ea..43d9b91 100644
280--- a/lib/lp/soyuz/tests/test_publishing.py
281+++ b/lib/lp/soyuz/tests/test_publishing.py
282@@ -38,9 +38,13 @@ from lp.services.log.logger import DevNullLogger
283 from lp.soyuz.enums import (
284 ArchivePurpose,
285 BinaryPackageFormat,
286+ BinarySourceReferenceType,
287 PackageUploadStatus,
288 )
289 from lp.soyuz.interfaces.binarypackagename import IBinaryPackageNameSet
290+from lp.soyuz.interfaces.binarysourcereference import (
291+ IBinarySourceReferenceSet,
292+ )
293 from lp.soyuz.interfaces.component import IComponentSet
294 from lp.soyuz.interfaces.publishing import (
295 active_publishing_status,
296@@ -984,6 +988,97 @@ class TestPublishingSetLite(TestCaseWithFactory):
297 self.assertRaisesWithContent(
298 DeletionError, message, pub.api_requestDeletion, self.person)
299
300+ def test_requestDeletion_singular_disallows_active_built_using_refs(self):
301+ # The singular version of requestDeletion checks that there are no
302+ # active Built-Using references to the source package being deleted.
303+ archive = self.factory.makeArchive(purpose=ArchivePurpose.PRIMARY)
304+ distroseries = self.factory.makeDistroSeries(
305+ distribution=archive.distribution)
306+ das = self.factory.makeDistroArchSeries(distroseries=distroseries)
307+ bpphs = [
308+ self.factory.makeBinaryPackagePublishingHistory(
309+ archive=archive, distroarchseries=das, pocket=pocket,
310+ status=PackagePublishingStatus.PUBLISHED)
311+ for pocket in (
312+ PackagePublishingPocket.RELEASE,
313+ PackagePublishingPocket.PROPOSED)]
314+ spph = self.factory.makeSourcePackagePublishingHistory(
315+ archive=archive, distroseries=distroseries, pocket=bpphs[0].pocket,
316+ status=PackagePublishingStatus.PUBLISHED)
317+ bsr_set = getUtility(IBinarySourceReferenceSet)
318+ for bpph in bpphs:
319+ spr = spph.sourcepackagerelease
320+ bsr_set.createFromSourcePackageReleases(
321+ bpph.binarypackagerelease, [spr],
322+ BinarySourceReferenceType.BUILT_USING)
323+
324+ # As long as the referring binary publications are active, deletion
325+ # is disallowed.
326+ message = (
327+ "Cannot delete source publication with a Built-Using reference "
328+ "from an active binary publication.")
329+ self.assertRaisesWithContent(
330+ DeletionError, message, spph.requestDeletion, self.person)
331+ self.assertRaisesWithContent(
332+ DeletionError, message, spph.api_requestDeletion, self.person)
333+ bpphs[0].requestDeletion(self.person)
334+ self.assertRaisesWithContent(
335+ DeletionError, message, spph.requestDeletion, self.person)
336+ self.assertRaisesWithContent(
337+ DeletionError, message, spph.api_requestDeletion, self.person)
338+ bpphs[1].requestDeletion(self.person)
339+
340+ # Now that all the referring binary publications are deleted, we
341+ # allow deleting the source publication.
342+ spph.requestDeletion(self.person)
343+
344+ def test_requestDeletion_bulk_disallows_active_built_using_refs(self):
345+ # The bulk version of requestDeletion checks that there are no
346+ # active Built-Using references to the source packages being
347+ # deleted.
348+ archive = self.factory.makeArchive(purpose=ArchivePurpose.PRIMARY)
349+ distroseries = self.factory.makeDistroSeries(
350+ distribution=archive.distribution)
351+ das = self.factory.makeDistroArchSeries(distroseries=distroseries)
352+ bpphs = [
353+ self.factory.makeBinaryPackagePublishingHistory(
354+ archive=archive, distroarchseries=das, pocket=pocket,
355+ status=PackagePublishingStatus.PUBLISHED)
356+ for pocket in (
357+ PackagePublishingPocket.RELEASE,
358+ PackagePublishingPocket.PROPOSED)]
359+ spphs = [
360+ self.factory.makeSourcePackagePublishingHistory(
361+ archive=archive, distroseries=distroseries,
362+ pocket=PackagePublishingPocket.RELEASE,
363+ status=PackagePublishingStatus.PUBLISHED)
364+ for bpph in bpphs]
365+ bsr_set = getUtility(IBinarySourceReferenceSet)
366+ for bpph in bpphs:
367+ for spph in spphs:
368+ spr = spph.sourcepackagerelease
369+ bsr_set.createFromSourcePackageReleases(
370+ bpph.binarypackagerelease, [spr],
371+ BinarySourceReferenceType.BUILT_USING)
372+
373+ # As long as the referring binary publications are active, deletion
374+ # is disallowed.
375+ message = (
376+ "Cannot delete source publication with a Built-Using reference "
377+ "from an active binary publication.")
378+ self.assertRaisesWithContent(
379+ DeletionError, message,
380+ getUtility(IPublishingSet).requestDeletion, spphs, self.person)
381+ bpphs[0].requestDeletion(self.person)
382+ self.assertRaisesWithContent(
383+ DeletionError, message,
384+ getUtility(IPublishingSet).requestDeletion, spphs, self.person)
385+ bpphs[1].requestDeletion(self.person)
386+
387+ # Now that all the referring binary publications are deleted, we
388+ # allow deleting the source publications.
389+ getUtility(IPublishingSet).requestDeletion(spphs, self.person)
390+
391 def test_requestDeletion_marks_debug_as_deleted(self):
392 matching_bpph, debug_matching_bpph = (
393 self.factory.makeBinaryPackagePublishingHistory(

Subscribers

People subscribed via source and target branches

to status/vote changes: