Merge ~cjwatson/launchpad:archive-file-history-backfill into launchpad:master

[Colin Watson]

Backfill ArchiveFile.date_superseded

We can currently derive this reliably from ArchiveFile.scheduled_deletion_date, and doing so gives us a chance of historical queries being mostly accurate.

[Ioana Lasc (ilasc)]

Looks good.

review: Approve

478e0f4... by Colin Watson on 2023-01-04

Backfill ArchiveFile.date_superseded

We can currently derive this reliably from
ArchiveFile.scheduled_deletion_date, and doing so gives us a chance of
historical queries being mostly accurate.

LP: #1765933

[Colin Watson (cjwatson)]

Self-approving to work around voting restrictions (Ioana was on the Launchpad team at the time of her approval vote).

review: Approve

diff --git a/database/schema/security.cfg b/database/schema/security.cfg
index d5578d7..9021834 100644
--- a/database/schema/security.cfg
+++ b/database/schema/security.cfg
@@ -2491,6 +2491,7 @@ public.account                          = SELECT, DELETE
 public.answercontact                    = SELECT, DELETE
 public.archive                          = SELECT, UPDATE
 public.archiveauthtoken                 = SELECT, UPDATE
+public.archivefile                      = SELECT, UPDATE
 public.archivesubscriber                = SELECT, UPDATE
 public.branch                           = SELECT, UPDATE
 public.branchjob                        = SELECT, DELETE
diff --git a/lib/lp/archivepublisher/publishing.py b/lib/lp/archivepublisher/publishing.py
index 07e32dd..e6133f4 100644
--- a/lib/lp/archivepublisher/publishing.py
+++ b/lib/lp/archivepublisher/publishing.py
@@ -2,6 +2,7 @@
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 __all__ = [
+    "BY_HASH_STAY_OF_EXECUTION",
     "cannot_modify_suite",
     "DirectoryHash",
     "FORMAT_TO_SUBCOMPONENT",
diff --git a/lib/lp/scripts/garbo.py b/lib/lp/scripts/garbo.py
index 4531f82..3ec4d53 100644
--- a/lib/lp/scripts/garbo.py
+++ b/lib/lp/scripts/garbo.py
@@ -48,6 +48,7 @@ from zope.component import getUtility
 from zope.security.proxy import removeSecurityProxy
 
 from lp.answers.model.answercontact import AnswerContact
+from lp.archivepublisher.publishing import BY_HASH_STAY_OF_EXECUTION
 from lp.bugs.interfaces.bug import IBugSet
 from lp.bugs.model.bug import Bug
 from lp.bugs.model.bugattachment import BugAttachment
@@ -127,6 +128,7 @@ from lp.soyuz.enums import (
 from lp.soyuz.interfaces.publishing import active_publishing_status
 from lp.soyuz.model.archive import Archive
 from lp.soyuz.model.archiveauthtoken import ArchiveAuthToken
+from lp.soyuz.model.archivefile import ArchiveFile
 from lp.soyuz.model.archivesubscriber import ArchiveSubscriber
 from lp.soyuz.model.binarypackagebuild import BinaryPackageBuild
 from lp.soyuz.model.binarypackagerelease import BinaryPackageRelease
@@ -2243,6 +2245,39 @@ class BinaryPackagePublishingHistorySPNPopulator(BulkPruner):
         transaction.commit()
 
 
+class ArchiveFileDatePopulator(TunableLoop):
+    """Populates ArchiveFile.date_superseded."""
+
+    maximum_chunk_size = 5000
+
+    def __init__(self, log, abort_time=None):
+        super().__init__(log, abort_time)
+        self.start_at = 1
+        self.store = IPrimaryStore(ArchiveFile)
+
+    def findArchiveFiles(self):
+        archive_files = self.store.find(
+            ArchiveFile,
+            ArchiveFile.id >= self.start_at,
+            ArchiveFile.date_superseded == None,
+            ArchiveFile.scheduled_deletion_date != None,
+        )
+        return archive_files.order_by(ArchiveFile.id)
+
+    def isDone(self):
+        return self.findArchiveFiles().is_empty()
+
+    def __call__(self, chunk_size):
+        archive_files = list(self.findArchiveFiles()[:chunk_size])
+        for archive_file in archive_files:
+            archive_file.date_superseded = (
+                archive_file.scheduled_deletion_date
+                - timedelta(days=BY_HASH_STAY_OF_EXECUTION)
+            )
+        self.start_at = archive_files[-1].id + 1
+        transaction.commit()
+
+
 class BaseDatabaseGarbageCollector(LaunchpadCronScript):
     """Abstract base class to run a collection of TunableLoops."""
 
@@ -2556,6 +2591,7 @@ class DailyDatabaseGarbageCollector(BaseDatabaseGarbageCollector):
     tunable_loops = [
         AnswerContactPruner,
         ArchiveArtifactoryColumnsPopulator,
+        ArchiveFileDatePopulator,
         BinaryPackagePublishingHistoryFormatPopulator,
         BinaryPackagePublishingHistorySPNPopulator,
         BranchJobPruner,
diff --git a/lib/lp/scripts/tests/test_garbo.py b/lib/lp/scripts/tests/test_garbo.py
index c93b28e..ade0b76 100644
--- a/lib/lp/scripts/tests/test_garbo.py
+++ b/lib/lp/scripts/tests/test_garbo.py
@@ -72,6 +72,7 @@ from lp.registry.model.commercialsubscription import CommercialSubscription
 from lp.registry.model.teammembership import TeamMembership
 from lp.scripts.garbo import (
     AntiqueSessionPruner,
+    ArchiveFileDatePopulator,
     BulkPruner,
     DailyDatabaseGarbageCollector,
     DuplicateSessionPruner,
@@ -127,6 +128,7 @@ from lp.soyuz.enums import (
     PackagePublishingStatus,
 )
 from lp.soyuz.interfaces.archive import NAMED_AUTH_TOKEN_FEATURE_FLAG
+from lp.soyuz.interfaces.archivefile import IArchiveFileSet
 from lp.soyuz.interfaces.livefs import LIVEFS_FEATURE_FLAG
 from lp.soyuz.interfaces.publishing import IPublishingSet
 from lp.soyuz.model.distributionsourcepackagecache import (
@@ -434,7 +436,11 @@ class TestGarbo(FakeAdapterMixin, TestCaseWithFactory):
         self.log_buffer = io.StringIO()
         handler = logging.StreamHandler(self.log_buffer)
         self.log.addHandler(handler)
-        self.addDetail("garbo-log", text_content(self.log_buffer.getvalue()))
+        self.addCleanup(
+            lambda: self.addDetail(
+                "garbo-log", text_content(self.log_buffer.getvalue())
+            )
+        )
 
     def runFrequently(self, maximum_chunk_size=2, test_args=()):
         switch_dbuser("garbo_daily")
@@ -2559,6 +2565,58 @@ class TestGarbo(FakeAdapterMixin, TestCaseWithFactory):
         # Other publications are left alone.
         self.assertIsNone(bpphs[2].sourcepackagename)
 
+    def test_ArchiveFileDatePopulator(self):
+        switch_dbuser("testadmin")
+        now = datetime.now(UTC)
+        archive_files = [self.factory.makeArchiveFile() for _ in range(2)]
+        removeSecurityProxy(
+            archive_files[1]
+        ).scheduled_deletion_date = now + timedelta(hours=6)
+
+        self.runDaily()
+
+        self.assertThat(
+            archive_files,
+            MatchesListwise(
+                [
+                    MatchesStructure(date_superseded=Is(None)),
+                    MatchesStructure.byEquality(
+                        date_superseded=now - timedelta(hours=18)
+                    ),
+                ]
+            ),
+        )
+
+    def test_ArchiveFileDatePopulator_findArchiveFiles_filters_correctly(self):
+        switch_dbuser("testadmin")
+
+        # Create three ArchiveFiles: one with date_superseded set, one with
+        # date_superseded unset and scheduled_deletion_date set, and one
+        # with both unset.
+        archive_files = [self.factory.makeArchiveFile() for _ in range(3)]
+
+        Store.of(archive_files[0]).flush()
+        getUtility(IArchiveFileSet).scheduleDeletion(
+            [archive_files[0]], timedelta(days=1)
+        )
+        self.assertIsNotNone(archive_files[0].date_superseded)
+
+        removeSecurityProxy(
+            archive_files[1]
+        ).scheduled_deletion_date = datetime.now(UTC) + timedelta(days=1)
+        self.assertIsNone(archive_files[1].date_superseded)
+
+        self.assertIsNone(archive_files[2].date_superseded)
+        self.assertIsNone(archive_files[2].scheduled_deletion_date)
+
+        populator = ArchiveFileDatePopulator(None)
+        # Consider only ArchiveFiles created by this test.
+        populator.start_at = archive_files[0].id
+
+        rs = populator.findArchiveFiles()
+        self.assertEqual(1, rs.count())
+        self.assertEqual(archive_files[1], rs.one())
+
 
 class TestGarboTasks(TestCaseWithFactory):
     layer = LaunchpadZopelessLayer