Merge lp:~al-maisan/launchpad/partner-expiry-535030 into lp:launchpad

[Muharem Hrnjadovic]

Hello there!

This branch revises the former PPA source/binary expiration script to include
partner archives as well.

Most of the changes are due to the refactoring of the test code making it
possible to "reuse" the majority of the original tests.

Tests to run:

    bin/test -vv test_expire_archive_files

No "make lint" issues.

[Jeroen T. Vermeulen (jtv)]

Looks good.

We discussed a few minor changes in IRC:

 * Use a dict for parameter substitution in those big queries; less brittle as the code continues to evolve.

 * Use the "common tests" class as a mixin rather than a fully-functional base class. That way you don't need to tell the test loader not to run it on its own.

Jeroen

review: Approve (code)

=== renamed file 'cronscripts/expire-ppa-files.py' => 'cronscripts/expire-archive-files.py'
--- cronscripts/expire-ppa-files.py	2010-01-20 10:32:32 +0000
+++ cronscripts/expire-archive-files.py	2010-03-10 12:10:46 +0000
@@ -12,11 +12,11 @@
 import _pythonpath
 
 from canonical.config import config
-from lp.soyuz.scripts.expire_ppa_binaries import PPABinaryExpirer
+from lp.soyuz.scripts.expire_archive_files import ArchiveExpirer
 
 
 if __name__ == '__main__':
-    script = PPABinaryExpirer(
-        'expire-ppa-files', dbuser=config.binaryfile_expire.dbuser)
+    script = ArchiveExpirer(
+        'expire-archive-files', dbuser=config.binaryfile_expire.dbuser)
     script.lock_and_run()
 

=== renamed file 'lib/lp/soyuz/scripts/expire_ppa_binaries.py' => 'lib/lp/soyuz/scripts/expire_archive_files.py'
--- lib/lp/soyuz/scripts/expire_ppa_binaries.py	2010-01-31 19:36:27 +0000
+++ lib/lp/soyuz/scripts/expire_archive_files.py	2010-03-10 12:10:46 +0000
@@ -34,7 +34,7 @@
 """.split()
 
 
-class PPABinaryExpirer(LaunchpadCronScript):
+class ArchiveExpirer(LaunchpadCronScript):
     """Helper class for expiring old PPA binaries.
 
     Any PPA binary older than 30 days that is superseded or deleted
@@ -60,6 +60,7 @@
         from lp.soyuz.interfaces.archive import ArchivePurpose
 
         stay_of_execution = '%d days' % num_days
+        archive_types = (ArchivePurpose.PPA, ArchivePurpose.PARTNER)
 
         # The subquery here has to repeat the checks for privacy and
         # blacklisting on *other* publications that are also done in
@@ -79,7 +80,7 @@
                 AND spph.dateremoved < (
                     CURRENT_TIMESTAMP AT TIME ZONE 'UTC' - interval %s)
                 AND spph.archive = archive.id
-                AND archive.purpose = %s
+                AND archive.purpose IN %s
                 AND lfa.expires IS NULL
             EXCEPT
             SELECT sprf.libraryfile
@@ -95,15 +96,15 @@
                 AND spph.archive = a.id
                 AND p.id = a.owner
                 AND (
-                    p.name IN %s
+                    (p.name IN %s AND a.purpose = %s)
                     OR a.private IS TRUE
-                    OR a.purpose != %s
+                    OR a.purpose NOT IN %s
                     OR dateremoved >
                         CURRENT_TIMESTAMP AT TIME ZONE 'UTC' - interval %s
                     OR dateremoved IS NULL);
             """ % sqlvalues(
-                stay_of_execution, ArchivePurpose.PPA, self.blacklist,
-                ArchivePurpose.PPA, stay_of_execution))
+                stay_of_execution, archive_types, self.blacklist,
+                ArchivePurpose.PPA, archive_types, stay_of_execution))
 
         lfa_ids = results.get_all()
         return lfa_ids
@@ -114,6 +115,7 @@
         from lp.soyuz.interfaces.archive import ArchivePurpose
 
         stay_of_execution = '%d days' % num_days
+        archive_types = (ArchivePurpose.PPA, ArchivePurpose.PARTNER)
 
         # The subquery here has to repeat the checks for privacy and
         # blacklisting on *other* publications that are also done in
@@ -131,9 +133,10 @@
                 AND bpr.id = bpf.binarypackagerelease
                 AND bpph.binarypackagerelease = bpr.id
                 AND bpph.dateremoved < (
-                    CURRENT_TIMESTAMP AT TIME ZONE 'UTC' - interval %s)
+                    CURRENT_TIMESTAMP AT TIME ZONE 'UTC' -
+                    interval %(stay_of_execution)s)
                 AND bpph.archive = archive.id
-                AND archive.purpose = %s
+                AND archive.purpose IN %(archive_types)s
                 AND lfa.expires IS NULL
             EXCEPT
             SELECT bpf.libraryfile
@@ -149,15 +152,18 @@
                 AND bpph.archive = a.id
                 AND p.id = a.owner
                 AND (
-                    p.name IN %s
+                    (p.name IN %(blacklist)s AND a.purpose = %(ppa)s)
                     OR a.private IS TRUE
-                    OR a.purpose != %s
-                    OR dateremoved >
-                        CURRENT_TIMESTAMP AT TIME ZONE 'UTC' - interval %s
-                    OR dateremoved IS NULL);
+                    OR a.purpose NOT IN %(archive_types)s
+                    OR dateremoved > (
+                        CURRENT_TIMESTAMP AT TIME ZONE 'UTC' -
+                        interval %(stay_of_execution)s)
+                    OR dateremoved IS NULL)
             """ % sqlvalues(
-                stay_of_execution, ArchivePurpose.PPA, self.blacklist,
-                ArchivePurpose.PPA, stay_of_execution))
+                stay_of_execution=stay_of_execution,
+                archive_types=archive_types,
+                blacklist=self.blacklist,
+                ppa=ArchivePurpose.PPA))
 
         lfa_ids = results.get_all()
         return lfa_ids

=== renamed file 'lib/lp/soyuz/scripts/tests/test_expire_ppa_bins.py' => 'lib/lp/soyuz/scripts/tests/test_expire_archive_files.py'
--- lib/lp/soyuz/scripts/tests/test_expire_ppa_bins.py	2010-02-27 20:20:03 +0000
+++ lib/lp/soyuz/scripts/tests/test_expire_archive_files.py	2010-03-10 12:10:46 +0000
@@ -1,12 +1,10 @@
 # Copyright 2009 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
-"""Test the expire-ppa-binaries.py script. """
+"""Test the expire-archive-files.py script. """
 
+from datetime import datetime, timedelta
 import pytz
-
-from datetime import datetime, timedelta
-
 import unittest
 
 from zope.component import getUtility
@@ -15,35 +13,20 @@
 from canonical.launchpad.scripts import QuietFakeLogger
 from canonical.testing.layers import LaunchpadZopelessLayer
 from lp.registry.interfaces.distribution import IDistributionSet
-from lp.registry.interfaces.person import IPersonSet
-from lp.soyuz.scripts.expire_ppa_binaries import PPABinaryExpirer
+from lp.soyuz.interfaces.archive import ArchivePurpose
+from lp.soyuz.scripts.expire_archive_files import ArchiveExpirer
 from lp.soyuz.tests.test_publishing import SoyuzTestPublisher
 from lp.testing import TestCaseWithFactory
 
 
-class TestPPABinaryExpiry(TestCaseWithFactory):
-    """Test the expire-ppa-binaries.py script."""
-
+class ArchiveExpiryTestBase(TestCaseWithFactory):
+    """base class for the expire-archive-files.py script tests."""
     layer = LaunchpadZopelessLayer
     dbuser = config.binaryfile_expire.dbuser
 
-    # We need to test several cases are handled properly:
-    #  - publications with no "dateremoved" are not expired
-    #  - publications with dateremoved <= 30 days ago are not expired
-    #  - publications with dateremoved > 30 days ago are expired
-    #  - publications with dateremoved > 30 days ago but refer to a
-    #     binary published elsewhere with no dateremoved are not
-    #     expired
-    #  - publications with dateremoved > 30 days ago but refer to a
-    #    binary published elsewhere with dateremoved <= 30 days ago
-    #    are not expired
-    #  - publications with dateremoved > 30 days ago but refer to a
-    #    binary published elsewhere with dateremoved > 30 days ago
-    #    are expired.
-
     def setUp(self):
         """Set up some test publications."""
-        super(TestPPABinaryExpiry, self).setUp()
+        super(ArchiveExpiryTestBase, self).setUp()
         # Configure the test publisher.
         self.layer.switchDbUser("launchpad")
         self.stp = SoyuzTestPublisher()
@@ -54,16 +37,12 @@
         self.under_threshold_date = self.now - timedelta(days=29)
         self.over_threshold_date = self.now - timedelta(days=31)
 
-        # Prepare two PPAs for the tests to use.
-        self.ppa = self.factory.makeArchive()
-        self.ppa2 = self.factory.makeArchive()
-
     def getScript(self, test_args=None):
-        """Return a PPABinaryExpirer instance."""
+        """Return a ArchiveExpirer instance."""
         if test_args is None:
             test_args = []
         test_args.extend(['--expire-after', '30'])
-        script = PPABinaryExpirer("test expirer", test_args=test_args)
+        script = ArchiveExpirer("test expirer", test_args=test_args)
         script.logger = QuietFakeLogger()
         script.txn = self.layer.txn
         return script
@@ -75,6 +54,24 @@
         self.layer.switchDbUser(self.dbuser)
         script.main()
 
+    def _setUpExpirablePublications(self, archive=None):
+        """Helper to set up two publications that are both expirable."""
+        if archive is None:
+            archive = self.archive
+        pkg5 = self.stp.getPubSource(
+            sourcename="pkg5", architecturehintlist="i386", archive=archive,
+            dateremoved=self.over_threshold_date)
+        other_source = pkg5.copyTo(
+            pkg5.distroseries, pkg5.pocket, self.archive2)
+        other_source.dateremoved = self.over_threshold_date
+        [pub] = self.stp.getPubBinaries(
+            pub_source=pkg5, dateremoved=self.over_threshold_date,
+            archive=archive)
+        [other_binary] = pub.copyTo(
+            pub.distroarchseries.distroseries, pub.pocket, self.archive2)
+        other_binary.dateremoved = self.over_threshold_date
+        return pkg5, pub
+
     def assertBinaryExpired(self, publication):
         self.assertNotEqual(
             publication.binarypackagerelease.files[0].libraryfile.expires,
@@ -99,13 +96,19 @@
             None,
             "lfa.expires should be None, but it's not.")
 
+
+class ArchiveExpiryCommonTests(object):
+    """Common source/binary expiration test cases.
+    
+    These will be shared irrespective of archive type (ppa/partner).
+    """
     def testNoExpirationWithNoDateremoved(self):
         """Test that no expiring happens if no dateremoved set."""
         pkg1 = self.stp.getPubSource(
-            sourcename="pkg1", architecturehintlist="i386", archive=self.ppa,
-            dateremoved=None)
+            sourcename="pkg1", architecturehintlist="i386",
+            archive=self.archive, dateremoved=None)
         [pub] = self.stp.getPubBinaries(
-            pub_source=pkg1, dateremoved=None, archive=self.ppa)
+            pub_source=pkg1, dateremoved=None, archive=self.archive)
 
         self.runScript()
         self.assertSourceNotExpired(pkg1)
@@ -114,11 +117,11 @@
     def testNoExpirationWithDateUnderThreshold(self):
         """Test no expiring if dateremoved too recent."""
         pkg2 = self.stp.getPubSource(
-            sourcename="pkg2", architecturehintlist="i386", archive=self.ppa,
-            dateremoved=self.under_threshold_date)
+            sourcename="pkg2", architecturehintlist="i386",
+            archive=self.archive, dateremoved=self.under_threshold_date)
         [pub] = self.stp.getPubBinaries(
             pub_source=pkg2, dateremoved=self.under_threshold_date,
-            archive=self.ppa)
+            archive=self.archive)
 
         self.runScript()
         self.assertSourceNotExpired(pkg2)
@@ -127,11 +130,11 @@
     def testExpirationWithDateOverThreshold(self):
         """Test expiring works if dateremoved old enough."""
         pkg3 = self.stp.getPubSource(
-            sourcename="pkg3", architecturehintlist="i386", archive=self.ppa,
-            dateremoved=self.over_threshold_date)
+            sourcename="pkg3", architecturehintlist="i386",
+            archive=self.archive, dateremoved=self.over_threshold_date)
         [pub] = self.stp.getPubBinaries(
             pub_source=pkg3, dateremoved=self.over_threshold_date,
-            archive=self.ppa)
+            archive=self.archive)
 
         self.runScript()
         self.assertSourceExpired(pkg3)
@@ -140,16 +143,16 @@
     def testNoExpirationWithDateOverThresholdAndOtherValidPublication(self):
         """Test no expiry if dateremoved old enough but other publication."""
         pkg4 = self.stp.getPubSource(
-            sourcename="pkg4", architecturehintlist="i386", archive=self.ppa,
-            dateremoved=self.over_threshold_date)
+            sourcename="pkg4", architecturehintlist="i386",
+            archive=self.archive, dateremoved=self.over_threshold_date)
         other_source = pkg4.copyTo(
-            pkg4.distroseries, pkg4.pocket, self.ppa2)
+            pkg4.distroseries, pkg4.pocket, self.archive2)
         other_source.dateremoved = None
         [pub] = self.stp.getPubBinaries(
             pub_source=pkg4, dateremoved=self.over_threshold_date,
-            archive=self.ppa)
+            archive=self.archive)
         [other_binary] = pub.copyTo(
-            pub.distroarchseries.distroseries, pub.pocket, self.ppa2)
+            pub.distroarchseries.distroseries, pub.pocket, self.archive2)
         other_binary.dateremoved = None
 
         self.runScript()
@@ -163,40 +166,22 @@
         not over date threshold.
         """
         pkg5 = self.stp.getPubSource(
-            sourcename="pkg5", architecturehintlist="i386", archive=self.ppa,
-            dateremoved=self.over_threshold_date)
+            sourcename="pkg5", architecturehintlist="i386",
+            archive=self.archive, dateremoved=self.over_threshold_date)
         other_source = pkg5.copyTo(
-            pkg5.distroseries, pkg5.pocket, self.ppa2)
+            pkg5.distroseries, pkg5.pocket, self.archive2)
         other_source.dateremoved = self.under_threshold_date
         [pub] = self.stp.getPubBinaries(
             pub_source=pkg5, dateremoved=self.over_threshold_date,
-            archive=self.ppa)
+            archive=self.archive)
         [other_binary] = pub.copyTo(
-            pub.distroarchseries.distroseries, pub.pocket, self.ppa2)
+            pub.distroarchseries.distroseries, pub.pocket, self.archive2)
         other_binary.dateremoved = self.under_threshold_date
 
         self.runScript()
         self.assertSourceNotExpired(pkg5)
         self.assertBinaryNotExpired(pub)
 
-    def _setUpExpirablePublications(self, archive=None):
-        """Helper to set up two publications that are both expirable."""
-        if archive is None:
-            archive = self.ppa
-        pkg5 = self.stp.getPubSource(
-            sourcename="pkg5", architecturehintlist="i386", archive=archive,
-            dateremoved=self.over_threshold_date)
-        other_source = pkg5.copyTo(
-            pkg5.distroseries, pkg5.pocket, self.ppa2)
-        other_source.dateremoved = self.over_threshold_date
-        [pub] = self.stp.getPubBinaries(
-            pub_source=pkg5, dateremoved=self.over_threshold_date,
-            archive=archive)
-        [other_binary] = pub.copyTo(
-            pub.distroarchseries.distroseries, pub.pocket, self.ppa2)
-        other_binary.dateremoved = self.over_threshold_date
-        return pkg5, pub
-
     def testNoExpirationWithDateOverThresholdAndOtherPubOverThreshold(self):
         """Test expiring works.
 
@@ -208,26 +193,6 @@
         self.assertSourceExpired(source)
         self.assertBinaryExpired(binary)
 
-    def testBlacklistingWorks(self):
-        """Test that blacklisted PPAs are not expired."""
-        source, binary = self._setUpExpirablePublications(archive=self.ppa)
-        script = self.getScript()
-        script.blacklist = [self.ppa.owner.name, ]
-        self.layer.txn.commit()
-        self.layer.switchDbUser(self.dbuser)
-        script.main()
-        self.assertSourceNotExpired(source)
-        self.assertBinaryNotExpired(binary)
-
-    def testPrivatePPAsNotExpired(self):
-        """Test that private PPAs are not expired."""
-        self.ppa.private = True
-        self.ppa.buildd_secret = "foo"
-        source, binary = self._setUpExpirablePublications()
-        self.runScript()
-        self.assertSourceNotExpired(source)
-        self.assertBinaryNotExpired(binary)
-
     def testDryRun(self):
         """Test that when dryrun is specified, nothing is expired."""
         source, binary = self._setUpExpirablePublications()
@@ -240,7 +205,7 @@
         self.assertSourceNotExpired(source)
         self.assertBinaryNotExpired(binary)
 
-    def testDoesNotAffectNonPPA(self):
+    def testDoesNotAffectPrimary(self):
         """Test that expiry does not happen for non-PPA publications."""
         ubuntu_archive = getUtility(IDistributionSet)['ubuntu'].main_archive
         source, binary = self._setUpExpirablePublications(ubuntu_archive)
@@ -249,5 +214,55 @@
         self.assertBinaryNotExpired(binary)
 
 
+class TestPPAExpiry(ArchiveExpiryTestBase, ArchiveExpiryCommonTests):
+    """Test the expire-archive-files.py script.
+    
+    Here we make use of the common test cases defined in the base class but
+    also add tests specific to PPAs (excluding particular PPAs from expiry
+    based on a "black list" or on the fact that PPA is private).
+    """
+
+    def setUp(self):
+        """Set up some test publications."""
+        super(TestPPAExpiry, self).setUp()
+        # Prepare two PPAs for the tests to use.
+        self.archive = self.factory.makeArchive()
+        self.archive2 = self.factory.makeArchive()
+
+    def testBlacklistingWorks(self):
+        """Test that blacklisted PPAs are not expired."""
+        source, binary = self._setUpExpirablePublications(
+            archive=self.archive)
+        script = self.getScript()
+        script.blacklist = [self.archive.owner.name, ]
+        self.layer.txn.commit()
+        self.layer.switchDbUser(self.dbuser)
+        script.main()
+        self.assertSourceNotExpired(source)
+        self.assertBinaryNotExpired(binary)
+
+    def testPrivatePPAsNotExpired(self):
+        """Test that private PPAs are not expired."""
+        self.archive.private = True
+        self.archive.buildd_secret = "foo"
+        source, binary = self._setUpExpirablePublications()
+        self.runScript()
+        self.assertSourceNotExpired(source)
+        self.assertBinaryNotExpired(binary)
+
+
+class TestPartnerExpiry(ArchiveExpiryTestBase, ArchiveExpiryCommonTests):
+    """Test the expire-archive-files.py script on partner archives."""
+
+    def setUp(self):
+        """Set up the partner archives under test."""
+        super(TestPartnerExpiry, self).setUp()
+        # Prepare two partner archives for the tests to use.
+        self.archive = self.factory.makeArchive(
+            purpose=ArchivePurpose.PARTNER)
+        self.archive2 = self.factory.makeArchive(
+            purpose=ArchivePurpose.PARTNER)
+
+
 def test_suite():
     return unittest.TestLoader().loadTestsFromName(__name__)