Colin Watson has proposed merging
lp:~cjwatson/launchpad/garbo-archivepermission-duplicates into lp:launchpad.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
Related bugs:
Bug #1025441 in Launchpad itself: "Please clean up the duplicates from
archivepermission"
https://bugs.launchpad.net/launchpad/+bug/1025441
For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/garbo-archivepermission-duplicates/+merge/115554
== Summary ==
There are some duplicated rows in ArchivePermission left over from an old
(fixed) bug. These get in the way because Archive.removePackagesetUploader
uses .one().
== Proposed fix ==
Add a BulkPruner-based garbo job to get rid of the duplicates. Previous
analysis showed that the only problems have non-NULL packageset and duplicate
person, archive, packageset, permission, so we'll get rid of all but the first
id for all such groups.
== LOC Rationale ==
+99, but it will all be removed again once the job has completed.
== Tests ==
bin/test -vvct test_DuplicateArchivePermissionPruner
== Demo and Q/A ==
dogfood's database is already in an appropriately broken state. Dump the
archivepermission table, run the garbo job, and dump it again, and then verify
that all the removed IDs (there should be 190, I believe) correspond to
duplicates and that exactly one of each duplicated group remains.
--
https://code.launchpad.net/~cjwatson/launchpad/garbo-archivepermission-duplicates/+merge/115554
Your team Launchpad code reviewers is requested to review the proposed merge of
lp:~cjwatson/launchpad/garbo-archivepermission-duplicates into lp:launchpad.
=== modified file 'database/schema/security.cfg'
--- database/schema/security.cfg 2012-07-17 06:21:47 +0000
+++ database/schema/security.cfg 2012-07-18 13:53:21 +0000
@@ -2201,6 +2201,7 @@
[garbo]
groups=script,read
public.account = SELECT, DELETE
+public.archivepermission = SELECT, DELETE
public.answercontact = SELECT, DELETE
public.branch = SELECT, UPDATE
public.branchjob = SELECT, DELETE
=== modified file 'lib/lp/scripts/garbo.py'
--- lib/lp/scripts/garbo.py 2012-07-10 06:57:47 +0000
+++ lib/lp/scripts/garbo.py 2012-07-18 13:53:21 +0000
@@ -93,6 +93,7 @@
MAIN_STORE,
MASTER_FLAVOR,
)
+from lp.soyuz.model.archivepermission import ArchivePermission
from lp.translations.interfaces.potemplate import IPOTemplateSet
from lp.translations.model.potmsgset import POTMsgSet
from lp.translations.model.potranslation import POTranslation
@@ -990,6 +991,23 @@
transaction.commit()
+class DuplicateArchivePermissionPruner(BulkPruner):
+ """Cleans up duplicate ArchivePermission rows created by bug 887185."""
+ target_table_class = ArchivePermission
+ ids_to_prune_query = """
+ SELECT id FROM (
+ SELECT id, rank() OVER w AS rank
+ FROM ArchivePermission
+ WHERE packageset IS NOT NULL
+ WINDOW w AS (
+ PARTITION BY person, archive, packageset, permission
+ ORDER BY id
+ )
+ ) AS whatever
+ WHERE rank > 1
+ """
+
+
class BaseDatabaseGarbageCollector(LaunchpadCronScript):
"""Abstract base class to run a collection of TunableLoops."""
script_name = None # Script name for locking and database user. Override.
@@ -1267,6 +1285,7 @@
BugWatchActivityPruner,
CodeImportEventPruner,
CodeImportResultPruner,
+ DuplicateArchivePermissionPruner,
HWSubmissionEmailLinker,
LoginTokenPruner,
ObsoleteBugAttachmentPruner,
=== modified file 'lib/lp/scripts/tests/test_garbo.py'
--- lib/lp/scripts/tests/test_garbo.py 2012-07-10 07:08:54 +0000
+++ lib/lp/scripts/tests/test_garbo.py 2012-07-18 13:53:21 +0000
@@ -97,6 +97,9 @@
MASTER_FLAVOR,
)
from lp.services.worlddata.interfaces.language import ILanguageSet
+from lp.soyuz.enums import ArchivePermissionType
+from lp.soyuz.interfaces.archivepermission import IArchivePermissionSet
+from lp.soyuz.model.archivepermission import ArchivePermission
from lp.testing import (
person_logged_in,
TestCase,
@@ -1016,6 +1019,82 @@
self.runHourly()
self.assertNotEqual(old_update, naked_bug.heat_last_updated)
+ def test_DuplicateArchivePermissionPruner_removes_duplicate_rows(self):
+ # DuplicateArchivePermissionPruner removes duplicated packageset
+ # permissions.
+ switch_dbuser('testadmin')
+ archive = self.factory.makeArchive()
+ person = self.factory.makePerson()
+ packageset = self.factory.makePackageset()
+ for _ in range(3):
+ ArchivePermission(
+ archive=archive, person=person, packageset=packageset,
+ permission=ArchivePermissionType.UPLOAD)
+ ap_set = getUtility(IArchivePermissionSet)
+ self.assertEqual(
+ 3, ap_set.uploadersForPackageset(archive, packageset).count())
+ self.runDaily()
+ self.assertEqual(
+ 1, ap_set.uploadersForPackageset(archive, packageset).count())
+
+ def test_DuplicateArchivePermissionPruner_skips_unique_rows(self):
+ # DuplicateArchivePermissionPruner leaves unique packageset
+ # permissions alone.
+ switch_dbuser('testadmin')
+ archive_one = self.factory.makeArchive()
+ archive_two = self.factory.makeArchive()
+ person_one = self.factory.makePerson()
+ person_two = self.factory.makePerson()
+ packageset_one = self.factory.makePackageset()
+ packageset_two = self.factory.makePackageset()
+ for archive, person, packageset in (
+ (archive_one, person_one, packageset_one),
+ (archive_two, person_one, packageset_one),
+ (archive_one, person_two, packageset_one),
+ (archive_one, person_one, packageset_two),
+ ):
+ ArchivePermission(
+ archive=archive, person=person, packageset=packageset,
+ permission=ArchivePermissionType.UPLOAD)
+ ap_set = getUtility(IArchivePermissionSet)
+ self.assertEqual(
+ 2,
+ ap_set.uploadersForPackageset(archive_one, packageset_one).count())
+ self.assertEqual(
+ 1,
+ ap_set.uploadersForPackageset(archive_two, packageset_one).count())
+ self.assertEqual(
+ 1,
+ ap_set.uploadersForPackageset(archive_one, packageset_two).count())
+ self.runDaily()
+ self.assertEqual(
+ 2,
+ ap_set.uploadersForPackageset(archive_one, packageset_one).count())
+ self.assertEqual(
+ 1,
+ ap_set.uploadersForPackageset(archive_two, packageset_one).count())
+ self.assertEqual(
+ 1,
+ ap_set.uploadersForPackageset(archive_one, packageset_two).count())
+
+ def test_DuplicateArchivePermissionPruner_skips_non_packagesets(self):
+ # DuplicateArchivePermissionPruner leaves non-packageset permissions
+ # alone.
+ switch_dbuser('testadmin')
+ archive = self.factory.makeArchive()
+ person = self.factory.makePerson()
+ component = self.factory.makeComponent()
+ for _ in range(3):
+ ArchivePermission(
+ archive=archive, person=person, component=component,
+ permission=ArchivePermissionType.UPLOAD)
+ ap_set = getUtility(IArchivePermissionSet)
+ self.assertEqual(
+ 3, ap_set.uploadersForComponent(archive, component).count())
+ self.runDaily()
+ self.assertEqual(
+ 3, ap_set.uploadersForComponent(archive, component).count())
+
class TestGarboTasks(TestCaseWithFactory):
layer = LaunchpadZopelessLayer
_______________________________________________
Mailing list: https://launchpad.net/~launchpad-reviewers
Post to : [email protected]
Unsubscribe : https://launchpad.net/~launchpad-reviewers
More help : https://help.launchpad.net/ListHelp
