Merge ~ilasc/launchpad:close-account-celery-job into launchpad:master

[Ioana Lasc]

Add a close account celery job

[Ioana Lasc (ilasc)]

Moved back a step in the implementation of PersonCloseAccountJob to a thin wrapper around the existing close account script.
MP ready for a direction review.

[Colin Watson (cjwatson)]

Much better than before, thanks! A few details to fix up, but the direction is fine and this looks pretty close.

review: Approve

diff --git a/lib/lp/app/errors.py b/lib/lp/app/errors.py
index f9df912..af5c4fa 100644
--- a/lib/lp/app/errors.py
+++ b/lib/lp/app/errors.py
@@ -12,6 +12,7 @@ __all__ = [
     'POSTToNonCanonicalURL',
     'ServiceUsageForbidden',
     'SubscriptionPrivacyViolation',
+    'TeamAccountNotClosable',
     'TranslationUnavailable',
     'UnexpectedFormData',
     'UserCannotUnsubscribePerson',
@@ -29,6 +30,10 @@ class TranslationUnavailable(Exception):
     """Translation objects are unavailable."""
 
 
+class TeamAccountNotClosable(Exception):
+    """We do not close team accounts."""
+
+
 @error_status(http_client.NOT_FOUND)
 class NotFoundError(KeyError):
     """Launchpad object not found."""
diff --git a/lib/lp/registry/configure.zcml b/lib/lp/registry/configure.zcml
index 7a7a396..318e09e 100644
--- a/lib/lp/registry/configure.zcml
+++ b/lib/lp/registry/configure.zcml
@@ -105,6 +105,12 @@
     </securedutility>
 
     <securedutility
+        component=".model.persontransferjob.PersonCloseAccountJob"
+        provides=".interfaces.persontransferjob.IPersonCloseAccountJobs">
+      <allow interface=".interfaces.persontransferjob.IPersonCloseAccountJobs"/>
+    </securedutility>
+
+    <securedutility
         component=".model.persontransferjob.TeamInvitationNotificationJob"
         provides=".interfaces.persontransferjob.ITeamInvitationNotificationJobSource">
       <allow interface=".interfaces.persontransferjob.ITeamInvitationNotificationJobSource"/>
diff --git a/lib/lp/registry/enums.py b/lib/lp/registry/enums.py
index b71dec6..0ca6af9 100644
--- a/lib/lp/registry/enums.py
+++ b/lib/lp/registry/enums.py
@@ -380,6 +380,11 @@ class PersonTransferJobType(DBEnumeratedType):
         Notify team admins that a member renewed their own membership.
         """)
 
+    CLOSE_ACCOUNT = DBItem(7, """Close account.
+
+        Close account for a given username.
+        """)
+
 
 class ProductJobType(DBEnumeratedType):
     """Values that IProductJob.job_type can take."""
diff --git a/lib/lp/registry/interfaces/persontransferjob.py b/lib/lp/registry/interfaces/persontransferjob.py
index 3e80aed..66874ec 100644
--- a/lib/lp/registry/interfaces/persontransferjob.py
+++ b/lib/lp/registry/interfaces/persontransferjob.py
@@ -9,6 +9,8 @@ __all__ = [
     'IExpiringMembershipNotificationJobSource',
     'IMembershipNotificationJob',
     'IMembershipNotificationJobSource',
+    'IPersonCloseAccountJob',
+    'IPersonCloseAccountJobs',
     'IPersonDeactivateJob',
     'IPersonDeactivateJobSource',
     'IPersonMergeJob',
@@ -171,6 +173,35 @@ class IPersonDeactivateJobSource(IJobSource):
         """
 
 
+class IPersonCloseAccountJob(IPersonTransferJob):
+    """A Job that closes the account for a person."""
+
+    person = PublicPersonChoice(
+        title=_('Alias for major_person attribute'),
+        vocabulary='ValidPersonOrTeam',
+        required=True)
+
+    def getErrorRecipients(self):
+        """See `BaseRunnableJob`."""
+
+
+class IPersonCloseAccountJobs(IJobSource):
+    """An interface for acquiring ICloseAccountJobs."""
+
+    def create(person):
+        """Create a new IPersonCloseAccountJob.
+
+        :param person: A `IPerson` to close the account for.
+        """
+
+    def find(person=None):
+        """Finds pending close account jobs.
+
+        :param person: Match jobs on `person`, or `None` to ignore.
+        :return: A `ResultSet` yielding `IPersonCloseAccountJob`.
+        """
+
+
 class ITeamInvitationNotificationJob(IPersonTransferJob):
     """A Job to notify about team joining invitations."""
 
diff --git a/lib/lp/registry/model/persontransferjob.py b/lib/lp/registry/model/persontransferjob.py
index 60ec767..010a1af 100644
--- a/lib/lp/registry/model/persontransferjob.py
+++ b/lib/lp/registry/model/persontransferjob.py
@@ -6,6 +6,7 @@
 __metaclass__ = type
 __all__ = [
     'MembershipNotificationJob',
+    'PersonCloseAccountJob',
     'PersonTransferJob',
     ]
 
@@ -24,12 +25,14 @@ from storm.locals import (
     Reference,
     Unicode,
     )
+import transaction
 from zope.component import getUtility
 from zope.interface import (
     implementer,
     provider,
     )
 
+from lp.app.errors import TeamAccountNotClosable
 from lp.app.interfaces.launchpad import ILaunchpadCelebrities
 from lp.registry.enums import PersonTransferJobType
 from lp.registry.interfaces.person import (
@@ -42,6 +45,8 @@ from lp.registry.interfaces.persontransferjob import (
     IExpiringMembershipNotificationJobSource,
     IMembershipNotificationJob,
     IMembershipNotificationJobSource,
+    IPersonCloseAccountJob,
+    IPersonCloseAccountJobs,
     IPersonDeactivateJob,
     IPersonDeactivateJobSource,
     IPersonMergeJob,
@@ -59,6 +64,7 @@ from lp.registry.interfaces.teammembership import TeamMembershipStatus
 from lp.registry.mail.teammembership import TeamMembershipMailer
 from lp.registry.model.person import Person
 from lp.registry.personmerge import merge_people
+from lp.registry.scripts.closeaccount import close_account
 from lp.services.config import config
 from lp.services.database.decoratedresultset import DecoratedResultSet
 from lp.services.database.enumcol import EnumCol
@@ -437,6 +443,75 @@ class PersonDeactivateJob(PersonTransferJobDerived):
         return 'deactivating ~%s' % self.person.name
 
 
+@implementer(IPersonCloseAccountJob)
+@provider(IPersonCloseAccountJobs)
+class PersonCloseAccountJob(PersonTransferJobDerived):
+    """A Job that closes account for a person."""
+
+    class_job_type = PersonTransferJobType.CLOSE_ACCOUNT
+
+    config = config.IPersonCloseAccountJobs
+
+    @classmethod
+    def create(cls, person):
+        """See `IPersonCloseAccountJobs`."""
+
+        # We don't delete teams
+        if person.is_team:
+            raise TeamAccountNotClosable("%s is a team" % person.name)
+
+        return super(PersonCloseAccountJob, cls).create(
+            minor_person=getUtility(ILaunchpadCelebrities).janitor,
+            major_person=person, metadata={})
+
+    @classmethod
+    def find(cls, person=None):
+        """See `IPersonMergeJobSource`."""
+        conditions = [
+            PersonCloseAccountJob.job_type == cls.class_job_type,
+            PersonCloseAccountJob.job_id == Job.id,
+            Job._status.is_in(Job.PENDING_STATUSES)]
+        arg_conditions = []
+        if person:
+            arg_conditions.append(PersonCloseAccountJob.major_person == person)
+        conditions.extend(arg_conditions)
+        return DecoratedResultSet(
+            IStore(PersonCloseAccountJob).find(
+                PersonCloseAccountJob, *conditions), cls)
+
+    @property
+    def person(self):
+        """See `IPersonCloseAccountJob`."""
+        return self.major_person
+
+    @property
+    def log_name(self):
+        return self.__class__.__name__
+
+    def getErrorRecipients(self):
+        """See `IPersonCloseAccountJob`."""
+        return [format_address_for_person(self.person)]
+
+    def run(self):
+        """Perform the account closure."""
+        try:
+            close_account(self.person.name, log)
+            transaction.commit()
+        except Exception:
+            log.error(
+                "%s Account closure failed for user %s", self.log_name,
+                self.person.name)
+            transaction.abort()
+
+    def __repr__(self):
+        return (
+            "<{self.__class__.__name__} to close account "
+            "~{self.person.name}").format(self=self)
+
+    def getOperationDescription(self):
+        return 'closing account for ~%s' % self.person.name
+
+
 @implementer(ITeamInvitationNotificationJob)
 @provider(ITeamInvitationNotificationJobSource)
 class TeamInvitationNotificationJob(PersonTransferJobDerived):
diff --git a/lib/lp/registry/tests/test_person_close_account_job.py b/lib/lp/registry/tests/test_person_close_account_job.py
new file mode 100644
index 0000000..14c6be9
--- /dev/null
+++ b/lib/lp/registry/tests/test_person_close_account_job.py
@@ -0,0 +1,138 @@
+# Copyright 2021 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""Tests for `PersonCloseAccountJob`."""
+
+__metaclass__ = type
+
+from testtools.matchers import (
+    Not,
+    StartsWith,
+    )
+import transaction
+from zope.component import getUtility
+from zope.security.proxy import removeSecurityProxy
+
+from lp.app.errors import TeamAccountNotClosable
+from lp.registry.interfaces.person import IPersonSet
+from lp.registry.interfaces.persontransferjob import IPersonCloseAccountJobs
+from lp.registry.model.persontransferjob import PersonCloseAccountJob
+from lp.services.config import config
+from lp.services.features.testing import FeatureFixture
+from lp.services.identity.interfaces.account import (
+    AccountStatus,
+    IAccountSet,
+    )
+from lp.services.identity.interfaces.emailaddress import IEmailAddressSet
+from lp.services.job.interfaces.job import JobStatus
+from lp.services.job.runner import JobRunner
+from lp.services.job.tests import block_on_job
+from lp.services.log.logger import BufferLogger
+from lp.services.scripts import log
+from lp.testing import TestCaseWithFactory
+from lp.testing.dbuser import dbuser
+from lp.testing.layers import (
+    CeleryJobLayer,
+    LaunchpadZopelessLayer,
+    )
+
+
+class TestPersonCloseAccountJob(TestCaseWithFactory):
+
+    layer = LaunchpadZopelessLayer
+
+    def test_close_account_job_valid_username(self):
+        user_to_delete = self.factory.makePerson(name=u'delete-me')
+        job_source = getUtility(IPersonCloseAccountJobs)
+        jobs = list(job_source.iterReady())
+
+        # at this point we have no jobs
+        self.assertEqual([], jobs)
+
+        getUtility(IPersonCloseAccountJobs).create(user_to_delete)
+        jobs = list(job_source.iterReady())
+        jobs[0] = removeSecurityProxy(jobs[0])
+        with dbuser(config.IPersonCloseAccountJobs.dbuser):
+            JobRunner(jobs).runAll()
+
+        self.assertEqual(JobStatus.COMPLETED, jobs[0].status)
+        person = removeSecurityProxy(
+            getUtility(IPersonSet).getByName(user_to_delete.name))
+        self.assertEqual(person.name, u'removed%d' % user_to_delete.id)
+
+    def test_close_account_job_valid_email(self):
+        user_to_delete = self.factory.makePerson(
+            email=u'[e-mail]')
+        getUtility(
+            IPersonCloseAccountJobs).create(user_to_delete)
+        job_source = getUtility(IPersonCloseAccountJobs)
+        jobs = list(job_source.iterReady())
+        jobs[0] = removeSecurityProxy(jobs[0])
+        with dbuser(config.IPersonCloseAccountJobs.dbuser):
+            JobRunner(jobs).runAll()
+        self.assertEqual(JobStatus.COMPLETED, jobs[0].status)
+        person = removeSecurityProxy(
+            getUtility(IPersonSet).getByName(user_to_delete.name))
+        self.assertEqual(person.name, u'removed%d' % user_to_delete.id)
+
+    def test_team(self):
+        team = self.factory.makeTeam()
+        self.assertRaisesWithContent(
+            TeamAccountNotClosable,
+            "%s is a team" % team.name,
+            getUtility(IPersonCloseAccountJobs).create,
+            team)
+
+    def test_unhandled_reference(self):
+        user_to_delete = self.factory.makePerson(name=u'delete-me')
+        self.factory.makeProduct(owner=user_to_delete)
+        person = removeSecurityProxy(
+            getUtility(IPersonSet).getByName(user_to_delete.name))
+        person_id = person.id
+        account_id = person.account.id
+        job = PersonCloseAccountJob.create(user_to_delete)
+        logger = BufferLogger()
+        with log.use(logger),\
+                dbuser(config.IPersonCloseAccountJobs.dbuser):
+            job.run()
+        error_message = (
+            {u'ERROR User delete-me is still '
+             u'referenced by 1 product.owner values',
+             u'ERROR User delete-me is still '
+             u'referenced by 1 productseries.owner values',
+             })
+        self.assertTrue(
+            error_message.issubset(logger.getLogBuffer().splitlines()))
+        self.assertNotRemoved(account_id, person_id)
+
+    def assertNotRemoved(self, account_id, person_id):
+        account = getUtility(IAccountSet).get(account_id)
+        self.assertNotEqual('Removed by request', account.displayname)
+        self.assertEqual(AccountStatus.ACTIVE, account.status)
+        person = getUtility(IPersonSet).get(person_id)
+        self.assertEqual(account, person.account)
+        self.assertNotEqual('Removed by request', person.display_name)
+        self.assertThat(person.name, Not(StartsWith('removed')))
+        self.assertNotEqual(
+            [], list(getUtility(IEmailAddressSet).getByPerson(person)))
+        self.assertNotEqual([], list(account.openid_identifiers))
+
+
+class TestPersonCloseAccountJobViaCelery(TestCaseWithFactory):
+
+    layer = CeleryJobLayer
+
+    def test_PersonCloseAccountJob(self):
+        """PersonCloseAccountJob runs under Celery."""
+        self.useFixture(FeatureFixture(
+            {'jobs.celery.enabled_classes':
+             'PersonCloseAccountJob'}))
+        user_to_delete = self.factory.makePerson()
+
+        with block_on_job():
+            job = PersonCloseAccountJob.create(user_to_delete)
+            transaction.commit()
+        person = removeSecurityProxy(
+            getUtility(IPersonSet).getByName(user_to_delete.name))
+        self.assertEqual(JobStatus.COMPLETED, job.status)
+        self.assertEqual(person.name, u'removed%d' % user_to_delete.id)
diff --git a/lib/lp/services/config/schema-lazr.conf b/lib/lp/services/config/schema-lazr.conf
index db22192..158a30b 100644
--- a/lib/lp/services/config/schema-lazr.conf
+++ b/lib/lp/services/config/schema-lazr.conf
@@ -1798,6 +1798,7 @@ job_sources:
     IOCIRecipeRequestBuildsJobSource,
     IOCIRegistryUploadJobSource,
     IPackageUploadNotificationJobSource,
+    IPersonCloseAccountJobs,
     IPersonDeactivateJobSource,
     IPersonMergeJobSource,
     IPlainPackageCopyJobSource,
@@ -1992,6 +1993,11 @@ link: IBranchMergeProposalJobSource
 module: lp.services.webhooks.interfaces
 dbuser: webhookrunner
 
+[IPersonCloseAccountJobs]
+module: lp.registry.interfaces.persontransferjob
+dbuser: launchpad
+crontab_group: MAIN
+
 [job_runner_queues]
 # The names of all queues.
 queues: launchpad_job launchpad_job_slow bzrsyncd_job bzrsyncd_job_slow branch_write_job branch_write_job_slow celerybeat