launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Colin Watson has proposed merging lp:~cjwatson/launchpad/close-account-deleted-ppa into lp:launchpad.

Commit message:
Make close-account purge deleted PPAs in simple cases.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/close-account-deleted-ppa/+merge/371119

Doing this in more general cases would need considerably more thought, but this is good enough for trivial but real cases (e.g. erasure requests for people who created PPAs but never uploaded anything to them).
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/close-account-deleted-ppa into lp:launchpad.

=== modified file 'database/schema/security.cfg'
--- database/schema/security.cfg	2019-05-07 17:33:45 +0000
+++ database/schema/security.cfg	2019-08-09 13:11:43 +0000
@@ -334,6 +334,7 @@
 [launchpad]
 groups=launchpad_main
 type=user
+public.archive                          = SELECT, INSERT, UPDATE, DELETE
 public.archivesubscriber                = SELECT, INSERT, UPDATE, DELETE
 public.hwsubmission                     = SELECT, INSERT, UPDATE, DELETE
 public.hwsubmissiondevice               = SELECT, DELETE

=== modified file 'lib/lp/registry/scripts/closeaccount.py'
--- lib/lp/registry/scripts/closeaccount.py	2019-08-08 11:13:44 +0000
+++ lib/lp/registry/scripts/closeaccount.py	2019-08-09 13:11:43 +0000
@@ -6,6 +6,7 @@
 __metaclass__ = type
 __all__ = ['CloseAccountScript']
 
+from storm.exceptions import IntegrityError
 from storm.expr import (
     LeftJoin,
     Lower,
@@ -40,8 +41,12 @@
     LaunchpadScript,
     LaunchpadScriptFailure,
     )
-from lp.soyuz.enums import ArchiveSubscriberStatus
+from lp.soyuz.enums import (
+    ArchiveStatus,
+    ArchiveSubscriberStatus,
+    )
 from lp.soyuz.interfaces.archivesubscriber import IArchiveSubscriberSet
+from lp.soyuz.model.archive import Archive
 from lp.soyuz.model.archivesubscriber import ArchiveSubscriber
 
 
@@ -364,6 +369,22 @@
     table_notification('HWSubmission')
     store.find(HWSubmission, HWSubmission.ownerID == person.id).remove()
 
+    # Purge deleted PPAs.  This is safe because the archive can only be in
+    # the DELETED status if the publisher has removed it from disk and set
+    # all its publications to DELETED.
+    # XXX cjwatson 2019-08-09: This will fail if anything non-trivial has
+    # been done in this person's PPAs; and it's not obvious what to do in
+    # more complicated cases such as builds having been copied out
+    # elsewhere.  It's good enough for some simple cases, though.
+    try:
+        store.find(
+            Archive,
+            Archive.owner == person,
+            Archive.status == ArchiveStatus.DELETED).remove()
+    except IntegrityError:
+        raise LaunchpadScriptFailure(
+            "Can't delete non-trivial PPAs for user %s" % person_name)
+
     has_references = False
 
     # Check for active related projects, and skip inactive ones.

=== modified file 'lib/lp/registry/scripts/tests/test_closeaccount.py'
--- lib/lp/registry/scripts/tests/test_closeaccount.py	2019-08-08 11:13:44 +0000
+++ lib/lp/registry/scripts/tests/test_closeaccount.py	2019-08-09 13:11:43 +0000
@@ -22,6 +22,8 @@
 from lp.answers.enums import QuestionStatus
 from lp.app.enums import InformationType
 from lp.app.interfaces.launchpad import ILaunchpadCelebrities
+from lp.archivepublisher.config import getPubConfig
+from lp.archivepublisher.publishing import Publisher
 from lp.bugs.model.bugsummary import BugSummary
 from lp.code.enums import TargetRevisionControlSystems
 from lp.code.tests.helpers import GitHostingFixture
@@ -53,6 +55,7 @@
     ArchiveSubscriberStatus,
     PackagePublishingStatus,
     )
+from lp.soyuz.model.archive import Archive
 from lp.soyuz.tests.test_publishing import SoyuzTestPublisher
 from lp.testing import TestCaseWithFactory
 from lp.testing.dbuser import dbuser
@@ -540,3 +543,62 @@
         self.assertRemoved(account_id, person_id)
         self.assertRaises(
             KeyError, login_token_set.__getitem__, plaintext_token)
+
+    def test_fails_on_undeleted_ppa(self):
+        person = self.factory.makePerson()
+        ppa = self.factory.makeArchive(owner=person)
+        procs = [self.factory.makeProcessor() for _ in range(2)]
+        ppa.setProcessors(procs)
+        person_id = person.id
+        account_id = person.account.id
+        script = self.makeScript([six.ensure_str(person.name)])
+        with dbuser('launchpad'):
+            self.assertRaisesWithContent(
+                LaunchpadScriptFailure,
+                'User %s is still referenced' % person.name,
+                self.runScript, script)
+        self.assertIn(
+            'ERROR User %s is still referenced by 1 archive.owner values' % (
+                person.name),
+            script.logger.getLogBuffer())
+        self.assertNotRemoved(account_id, person_id)
+
+    def test_fails_on_deleted_ppa_with_builds(self):
+        # XXX cjwatson 2019-08-09: A PPA that has ever had builds can't
+        # currently be purged.  It's not clear what to do about this case.
+        person = self.factory.makePerson()
+        ppa = self.factory.makeArchive(owner=person)
+        self.factory.makeBinaryPackageBuild(archive=ppa)
+        ppa.delete(person)
+        Publisher(
+            DevNullLogger(), getPubConfig(ppa), None, ppa).deleteArchive()
+        person_id = person.id
+        account_id = person.account.id
+        script = self.makeScript([six.ensure_str(person.name)])
+        with dbuser('launchpad'):
+            self.assertRaisesWithContent(
+                LaunchpadScriptFailure,
+                "Can't delete non-trivial PPAs for user %s" % person.name,
+                self.runScript, script)
+        self.assertNotRemoved(account_id, person_id)
+
+    def test_handles_empty_deleted_ppa(self):
+        person = self.factory.makePerson()
+        ppa = self.factory.makeArchive(owner=person)
+        ppa_id = ppa.id
+        other_ppa = self.factory.makeArchive()
+        other_ppa_id = other_ppa.id
+        procs = [self.factory.makeProcessor() for _ in range(2)]
+        ppa.setProcessors(procs)
+        ppa.delete(person)
+        Publisher(
+            DevNullLogger(), getPubConfig(ppa), None, ppa).deleteArchive()
+        store = Store.of(ppa)
+        person_id = person.id
+        account_id = person.account.id
+        script = self.makeScript([six.ensure_str(person.name)])
+        with dbuser('launchpad'):
+            self.runScript(script)
+        self.assertRemoved(account_id, person_id)
+        self.assertIsNone(store.get(Archive, ppa_id))
+        self.assertEqual(other_ppa, store.get(Archive, other_ppa_id))