launchpad-reviewers team mailing list archive

[Colin Watson <mp+437053@xxxxxxxxxxxxxxxxxx>]

Colin Watson has proposed merging ~cjwatson/launchpad:fix-archive-api-tests into launchpad:master.

Commit message:
Fix ArchiveAPI tests to not create bad ArchiveFile combinations

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/+git/launchpad/+merge/437053

DB patch 2211-15-0 introduces a unique index to ensure that there may be only one non-superseded `ArchiveFile` row for a given path in a given archive at any one time.  The `ArchiveAPI` tests unintentionally violated this.

This situation shouldn't happen on production, and I've checked that there are no violations in the most recent production dump that was loaded into staging.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~cjwatson/launchpad:fix-archive-api-tests into launchpad:master.

diff --git a/lib/lp/soyuz/xmlrpc/tests/test_archive.py b/lib/lp/soyuz/xmlrpc/tests/test_archive.py
index 3d725ce..1dffd43 100644
--- a/lib/lp/soyuz/xmlrpc/tests/test_archive.py
+++ b/lib/lp/soyuz/xmlrpc/tests/test_archive.py
@@ -11,11 +11,11 @@ from zope.security.proxy import removeSecurityProxy
 
 from lp.buildmaster.enums import BuildStatus
 from lp.services.database.interfaces import IStore
+from lp.services.database.sqlbase import get_transaction_timestamp
 from lp.services.features.testing import FeatureFixture
 from lp.services.macaroons.interfaces import IMacaroonIssuer
 from lp.soyuz.enums import ArchiveRepositoryFormat, PackagePublishingStatus
 from lp.soyuz.interfaces.archive import NAMED_AUTH_TOKEN_FEATURE_FLAG
-from lp.soyuz.interfaces.archivefile import IArchiveFileSet
 from lp.soyuz.xmlrpc.archive import ArchiveAPI
 from lp.testing import TestCaseWithFactory, person_logged_in
 from lp.testing.layers import LaunchpadFunctionalLayer
@@ -324,10 +324,13 @@ class TestArchiveAPI(TestCaseWithFactory):
 
     def test_translatePath_by_hash_checksum_found(self):
         archive = removeSecurityProxy(self.factory.makeArchive())
+        now = get_transaction_timestamp(IStore(archive))
         self.factory.makeArchiveFile(
             archive=archive,
             container="release:jammy",
             path="dists/jammy/InRelease",
+            date_superseded=now,
+            scheduled_deletion_date=now + timedelta(days=1),
         )
         archive_file = self.factory.makeArchiveFile(
             archive=archive,
@@ -349,10 +352,13 @@ class TestArchiveAPI(TestCaseWithFactory):
 
     def test_translatePath_by_hash_checksum_found_private(self):
         archive = removeSecurityProxy(self.factory.makeArchive(private=True))
+        now = get_transaction_timestamp(IStore(archive))
         self.factory.makeArchiveFile(
             archive=archive,
             container="release:jammy",
             path="dists/jammy/InRelease",
+            date_superseded=now,
+            scheduled_deletion_date=now + timedelta(days=1),
         )
         archive_file = self.factory.makeArchiveFile(
             archive=archive,
@@ -385,15 +391,18 @@ class TestArchiveAPI(TestCaseWithFactory):
 
     def test_translatePath_non_pool_found(self):
         archive = removeSecurityProxy(self.factory.makeArchive())
+        now = get_transaction_timestamp(IStore(archive))
         self.factory.makeArchiveFile(archive=archive)
         path = "dists/focal/InRelease"
         archive_files = [
-            self.factory.makeArchiveFile(archive=archive, path=path)
-            for _ in range(2)
+            self.factory.makeArchiveFile(
+                archive=archive,
+                path=path,
+                date_superseded=now,
+                scheduled_deletion_date=now + timedelta(days=1),
+            ),
+            self.factory.makeArchiveFile(archive=archive, path=path),
         ]
-        getUtility(IArchiveFileSet).scheduleDeletion(
-            [archive_files[0]], timedelta(days=1)
-        )
         self.assertEqual(
             archive_files[1].library_file.getURL(),
             self.archive_api.translatePath(archive.reference, path),
@@ -409,15 +418,18 @@ class TestArchiveAPI(TestCaseWithFactory):
 
     def test_translatePath_non_pool_found_private(self):
         archive = removeSecurityProxy(self.factory.makeArchive(private=True))
+        now = get_transaction_timestamp(IStore(archive))
         self.factory.makeArchiveFile(archive=archive)
         path = "dists/focal/InRelease"
         archive_files = [
-            self.factory.makeArchiveFile(archive=archive, path=path)
-            for _ in range(2)
+            self.factory.makeArchiveFile(
+                archive=archive,
+                path=path,
+                date_superseded=now,
+                scheduled_deletion_date=now + timedelta(days=1),
+            ),
+            self.factory.makeArchiveFile(archive=archive, path=path),
         ]
-        getUtility(IArchiveFileSet).scheduleDeletion(
-            [archive_files[0]], timedelta(days=1)
-        )
         self.assertStartsWith(
             archive_files[1].library_file.getURL() + "?token=",
             self.archive_api.translatePath(archive.reference, path),
diff --git a/lib/lp/testing/factory.py b/lib/lp/testing/factory.py
index 9a672aa..62cd634 100644
--- a/lib/lp/testing/factory.py
+++ b/lib/lp/testing/factory.py
@@ -3795,6 +3795,7 @@ class LaunchpadObjectFactory(ObjectFactory):
         container=None,
         path=None,
         library_file=None,
+        date_superseded=None,
         scheduled_deletion_date=None,
     ):
         if archive is None:
@@ -3811,6 +3812,8 @@ class LaunchpadObjectFactory(ObjectFactory):
             path=path,
             library_file=library_file,
         )
+        if date_superseded is not None:
+            removeSecurityProxy(archive_file).date_superseded = date_superseded
         if scheduled_deletion_date is not None:
             removeSecurityProxy(
                 archive_file