launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #26860
[Merge] ~ilasc/launchpad:close-account-celery-job into launchpad:master
Ioana Lasc has proposed merging ~ilasc/launchpad:close-account-celery-job into launchpad:master.
Commit message:
Add a close account celery job
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~ilasc/launchpad/+git/launchpad/+merge/400878
--
Your team Launchpad code reviewers is requested to review the proposed merge of ~ilasc/launchpad:close-account-celery-job into launchpad:master.
diff --git a/lib/lp/registry/configure.zcml b/lib/lp/registry/configure.zcml
index 7a7a396..ac3d82b 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.IPersonCloseAccountJobSource">
+ <allow interface=".interfaces.persontransferjob.IPersonCloseAccountJobSource"/>
+ </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..e445553 100644
--- a/lib/lp/registry/interfaces/persontransferjob.py
+++ b/lib/lp/registry/interfaces/persontransferjob.py
@@ -9,6 +9,8 @@ __all__ = [
'IExpiringMembershipNotificationJobSource',
'IMembershipNotificationJob',
'IMembershipNotificationJobSource',
+ 'IPersonCloseAccountJob',
+ 'IPersonCloseAccountJobSource',
'IPersonDeactivateJob',
'IPersonDeactivateJobSource',
'IPersonMergeJob',
@@ -171,6 +173,34 @@ class IPersonDeactivateJobSource(IJobSource):
"""
+class IPersonCloseAccountJob(IPersonTransferJob):
+ """A Job that closes the account for a person."""
+
+ person = PublicPersonChoice(
+ title=_('Alias for person attribute'), vocabulary='ValidPersonOrTeam',
+ required=True)
+
+ def getErrorRecipients(self):
+ """See `BaseRunnableJob`."""
+
+
+class IPersonCloseAccountJobSource(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..5cef604 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',
]
@@ -15,8 +16,11 @@ from lazr.delegates import delegate_to
import pytz
import simplejson
import six
+from storm.exceptions import IntegrityError
from storm.expr import (
And,
+ LeftJoin,
+ Lower,
Or,
)
from storm.locals import (
@@ -24,24 +28,32 @@ from storm.locals import (
Reference,
Unicode,
)
+import transaction
from zope.component import getUtility
from zope.interface import (
implementer,
provider,
)
+from zope.security.proxy import removeSecurityProxy
+from lp.answers.enums import QuestionStatus
+from lp.answers.model.question import Question
from lp.app.interfaces.launchpad import ILaunchpadCelebrities
+from lp.bugs.model.bugtask import BugTask
from lp.registry.enums import PersonTransferJobType
from lp.registry.interfaces.person import (
IPerson,
IPersonSet,
ITeam,
+ PersonCreationRationale,
)
from lp.registry.interfaces.persontransferjob import (
IExpiringMembershipNotificationJob,
IExpiringMembershipNotificationJobSource,
IMembershipNotificationJob,
IMembershipNotificationJobSource,
+ IPersonCloseAccountJob,
+ IPersonCloseAccountJobSource,
IPersonDeactivateJob,
IPersonDeactivateJobSource,
IPersonMergeJob,
@@ -57,23 +69,45 @@ from lp.registry.interfaces.persontransferjob import (
)
from lp.registry.interfaces.teammembership import TeamMembershipStatus
from lp.registry.mail.teammembership import TeamMembershipMailer
-from lp.registry.model.person import Person
+from lp.registry.model.person import (
+ Person,
+ PersonSettings,
+ )
+from lp.registry.model.product import Product
+from lp.registry.model.productseries import ProductSeries
from lp.registry.personmerge import merge_people
from lp.services.config import config
+from lp.services.database import postgresql
+from lp.services.database.constants import DEFAULT
from lp.services.database.decoratedresultset import DecoratedResultSet
from lp.services.database.enumcol import EnumCol
from lp.services.database.interfaces import (
IMasterStore,
IStore,
)
+from lp.services.database.sqlbase import cursor
from lp.services.database.stormbase import StormBase
+from lp.services.identity.interfaces.account import (
+ AccountCreationRationale,
+ AccountStatus,
+ )
+from lp.services.identity.model.emailaddress import EmailAddress
from lp.services.job.model.job import (
EnumeratedSubclass,
Job,
)
from lp.services.job.runner import BaseRunnableJob
from lp.services.mail.sendmail import format_address_for_person
+from lp.services.openid.model.openididentifier import OpenIdIdentifier
from lp.services.scripts import log
+from lp.services.scripts.base import LaunchpadScriptFailure
+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
@implementer(IPersonTransferJob)
@@ -437,6 +471,491 @@ class PersonDeactivateJob(PersonTransferJobDerived):
return 'deactivating ~%s' % self.person.name
+@implementer(IPersonCloseAccountJob)
+@provider(IPersonCloseAccountJobSource)
+class PersonCloseAccountJob(PersonTransferJobDerived):
+ """A Job that closes account for a person."""
+
+ class_job_type = PersonTransferJobType.CLOSE_ACCOUNT
+
+ config = config.IPersonCloseAccountJobSource
+
+ @classmethod
+ def create(cls, username):
+ """See `IPersonCloseAccountJobSource`."""
+ # Minor person has to be not null, so use the janitor.
+ store = IMasterStore(Person)
+ janitor = getUtility(ILaunchpadCelebrities).janitor
+
+ person = store.using(
+ Person,
+ LeftJoin(EmailAddress, Person.id == EmailAddress.personID)
+ ).find(
+ Person,
+ Or(Person.name == username,
+ Lower(EmailAddress.email) == Lower(username))
+ ).order_by(Person.id).config(distinct=True).one()
+
+ if person is None:
+ raise TypeError("User %s does not exist" % username)
+ person_name = person.name
+
+ # We don't do teams
+ if person.is_team:
+ raise TypeError("%s is a team" % person_name)
+
+ log.info("Closing %s's account" % person_name)
+
+ return super(PersonCloseAccountJob, cls).create(
+ minor_person=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:
+ self.close_account(self.person)
+ except Exception:
+ log.error(
+ "%s Account clossure 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
+
+ def close_account(self, person):
+ """Close a person's account.
+
+ Return True on success, or log an error message and return False
+ """
+ store = IMasterStore(Person)
+ janitor = getUtility(ILaunchpadCelebrities).janitor
+ cur = cursor()
+ references = list(postgresql.listReferences(cur, 'person', 'id'))
+ postgresql.check_indirect_references(references)
+ username = self.person.name
+
+ def table_notification(table):
+ log.debug("Handling the %s table" % table)
+
+ # All names starting with 'removed' are blacklisted,
+ # so this will always succeed.
+ new_name = 'removed%d' % person.id
+
+ # Some references can safely remain in place and link
+ # to the cleaned-out Person row.
+ skip = {
+ # These references express some kind of audit trail.
+ # The actions in question still happened, and in some cases
+ # the rows may still have functional significance
+ # (e.g. subscriptions or access grants), but we no longer
+ # identify the actor.
+ ('accessartifactgrant', 'grantor'),
+ ('accesspolicygrant', 'grantor'),
+ ('binarypackagepublishinghistory', 'removed_by'),
+ ('branch', 'registrant'),
+ ('branchmergeproposal', 'merge_reporter'),
+ ('branchmergeproposal', 'merger'),
+ ('branchmergeproposal', 'queuer'),
+ ('branchmergeproposal', 'registrant'),
+ ('branchmergeproposal', 'reviewer'),
+ ('branchsubscription', 'subscribed_by'),
+ ('bug', 'owner'),
+ ('bug', 'who_made_private'),
+ ('bugactivity', 'person'),
+ ('bugnomination', 'decider'),
+ ('bugnomination', 'owner'),
+ ('bugtask', 'owner'),
+ ('bugsubscription', 'subscribed_by'),
+ ('codeimport', 'owner'),
+ ('codeimport', 'registrant'),
+ ('codeimportjob', 'requesting_user'),
+ ('codeimportevent', 'person'),
+ ('codeimportresult', 'requesting_user'),
+ ('distroarchseriesfilter', 'creator'),
+ ('faq', 'last_updated_by'),
+ ('featureflagchangelogentry', 'person'),
+ ('gitactivity', 'changee'),
+ ('gitactivity', 'changer'),
+ ('gitrepository', 'registrant'),
+ ('gitrule', 'creator'),
+ ('gitrulegrant', 'grantor'),
+ ('gitsubscription', 'subscribed_by'),
+ ('job', 'requester'),
+ ('message', 'owner'),
+ ('messageapproval', 'disposed_by'),
+ ('messageapproval', 'posted_by'),
+ ('packagecopyrequest', 'requester'),
+ ('packagediff', 'requester'),
+ ('packageupload', 'signing_key_owner'),
+ ('personlocation', 'last_modified_by'),
+ ('persontransferjob', 'major_person'),
+ ('persontransferjob', 'minor_person'),
+ ('poexportrequest', 'person'),
+ ('pofile', 'lasttranslator'),
+ ('pofiletranslator', 'person'),
+ ('product', 'registrant'),
+ ('question', 'answerer'),
+ ('questionreopening', 'answerer'),
+ ('questionreopening', 'reopener'),
+ ('snapbuild', 'requester'),
+ ('sourcepackagepublishinghistory', 'creator'),
+ ('sourcepackagepublishinghistory', 'removed_by'),
+ ('sourcepackagepublishinghistory', 'sponsor'),
+ ('sourcepackagerecipebuild', 'requester'),
+ ('sourcepackagerelease', 'creator'),
+ ('sourcepackagerelease', 'maintainer'),
+ ('sourcepackagerelease', 'signing_key_owner'),
+ ('specification', 'approver'),
+ ('specification', 'completer'),
+ ('specification', 'drafter'),
+ ('specification', 'goal_decider'),
+ ('specification', 'goal_proposer'),
+ ('specification', 'last_changed_by'),
+ ('specification', 'owner'),
+ ('specification', 'starter'),
+ ('structuralsubscription', 'subscribed_by'),
+ ('teammembership', 'acknowledged_by'),
+ ('teammembership', 'last_changed_by'),
+ ('teammembership', 'proposed_by'),
+ ('teammembership', 'reviewed_by'),
+ ('translationimportqueueentry', 'importer'),
+ ('translationmessage', 'reviewer'),
+ ('translationmessage', 'submitter'),
+ ('translationrelicensingagreement', 'person'),
+ ('usertouseremail', 'recipient'),
+ ('usertouseremail', 'sender'),
+ ('xref', 'creator'),
+
+ # This is maintained by trigger functions and a garbo job. It
+ # doesn't need to be updated immediately.
+ ('bugsummary', 'viewed_by'),
+
+ # XXX cjwatson 2019-05-02 bug=1827399: This is suboptimal because
+ # it does retain some personal information, but it's currently hard
+ # to deal with due to the size and complexity of references to it.
+ # We can hopefully provide a garbo job for this eventually.
+ ('revisionauthor', 'person'),
+ }
+
+ # If all the teams that the user owns
+ # have been deleted (not just one) skip Person.teamowner
+ teams = store.find(Person, Person.teamowner == person)
+ if all(team.merged is not None for team in teams):
+ skip.add(('person', 'teamowner'))
+
+ reference_names = {
+ (src_tab, src_col) for src_tab, src_col, _, _, _, _ in references}
+ for src_tab, src_col in skip:
+ if (src_tab, src_col) not in reference_names:
+ raise AssertionError(
+ "%s.%s is not a Person reference; possible typo?" %
+ (src_tab, src_col))
+
+ # XXX cjwatson 2018-11-29: Registrants could possibly be left as-is,
+ # but perhaps we should pretend that the registrant was ~registry in
+ # that case instead?
+
+ # Remove the EmailAddress. This is the most important step, as
+ # people requesting account removal seem to primarily be interested
+ # in ensuring we no longer store this information.
+ table_notification('EmailAddress')
+ store.find(EmailAddress, EmailAddress.personID == person.id).remove()
+
+ # Clean out personal details from the Person table
+ table_notification('Person')
+ person.display_name = 'Removed by request'
+ person.name = new_name
+ person.homepage_content = None
+ person.icon = None
+ person.mugshot = None
+ person.hide_email_addresses = False
+ person.registrant = None
+ person.logo = None
+ person.creation_rationale = PersonCreationRationale.UNKNOWN
+ person.creation_comment = None
+
+ # Keep the corresponding PersonSettings row, but reset everything to
+ # the defaults.
+ table_notification('PersonSettings')
+ store.find(PersonSettings, PersonSettings.personID == person.id).set(
+ selfgenerated_bugnotifications=DEFAULT,
+ # XXX cjwatson 2018-11-29: These two columns have NULL defaults,
+ # but perhaps shouldn't?
+ expanded_notification_footers=False,
+ require_strong_email_authentication=False)
+ skip.add(('personsettings', 'person'))
+
+ # Remove almost everything from the Account row and the corresponding
+ # OpenIdIdentifier rows, preserving only a minimal audit trail.
+ if person.account is not None:
+ table_notification('Account')
+ account = removeSecurityProxy(person.account)
+ account.displayname = 'Removed by request'
+ account.creation_rationale = AccountCreationRationale.UNKNOWN
+ person.setAccountStatus(
+ AccountStatus.CLOSED, janitor, "Closed using close-account.")
+
+ table_notification('OpenIdIdentifier')
+ store.find(
+ OpenIdIdentifier,
+ OpenIdIdentifier.account_id == account.id).remove()
+
+ # Reassign their bugs
+ table_notification('BugTask')
+ store.find(
+ BugTask, BugTask.assignee_id == person.id).set(assignee_id=None)
+
+ # Reassign questions assigned to the user, and close all their
+ # questions in non-final states since nobody else can.
+ table_notification('Question')
+ store.find(Question, Question.assignee_id == person.id).set(
+ assignee_id=None)
+ owned_non_final_questions = store.find(
+ Question, Question.owner_id == person.id,
+ Question.status.is_in([
+ QuestionStatus.OPEN, QuestionStatus.NEEDSINFO,
+ QuestionStatus.ANSWERED,
+ ]))
+ owned_non_final_questions.set(
+ status=QuestionStatus.SOLVED,
+ whiteboard=(
+ u'Closed by Launchpad due to owner requesting '
+ u'account removal'))
+ skip.add(('question', 'owner'))
+
+ # Remove rows from tables in simple cases in the given order
+ removals = [
+ # Trash their email addresses. People who request complete account
+ # removal would be unhappy if they reregistered with their old
+ # email address and this resurrected their deleted account, as the
+ # email address is probably the piece of data we store that they
+ # were most concerned with being removed from our systems.
+ ('EmailAddress', 'person'),
+
+ # Login and OAuth tokens are no longer interesting if the user can
+ # no longer log in.
+ ('LoginToken', 'requester'),
+ ('OAuthAccessToken', 'person'),
+ ('OAuthRequestToken', 'person'),
+
+ # Trash their codes of conduct and GPG keys
+ ('SignedCodeOfConduct', 'owner'),
+ ('GpgKey', 'owner'),
+
+ # Subscriptions and notifications
+ ('BranchSubscription', 'person'),
+ ('BugMute', 'person'),
+ ('BugNotificationRecipient', 'person'),
+ ('BugSubscription', 'person'),
+ ('BugSubscriptionFilterMute', 'person'),
+ ('GitSubscription', 'person'),
+ ('MailingListSubscription', 'person'),
+ ('QuestionSubscription', 'person'),
+ ('SpecificationSubscription', 'person'),
+ ('StructuralSubscription', 'subscriber'),
+
+ # Personal stuff, freeing up the namespace for others who want
+ # to play or just to remove any fingerprints identifying the user.
+ ('IrcId', 'person'),
+ ('JabberId', 'person'),
+ ('WikiName', 'person'),
+ ('PersonLanguage', 'person'),
+ ('PersonLocation', 'person'),
+ ('SshKey', 'person'),
+
+ # Karma
+ ('Karma', 'person'),
+ ('KarmaCache', 'person'),
+ ('KarmaTotalCache', 'person'),
+
+ # Team memberships
+ ('TeamMembership', 'person'),
+ ('TeamParticipation', 'person'),
+
+ # Contacts
+ ('AnswerContact', 'person'),
+
+ # Pending items in queues
+ ('POExportRequest', 'person'),
+
+ # Access grants
+ ('AccessArtifactGrant', 'grantee'),
+ ('AccessPolicyGrant', 'grantee'),
+ ('ArchivePermission', 'person'),
+ ('GitRuleGrant', 'grantee'),
+ ('SharingJob', 'grantee'),
+
+ # Soyuz reporting
+ ('LatestPersonSourcePackageReleaseCache', 'creator'),
+ ('LatestPersonSourcePackageReleaseCache', 'maintainer'),
+
+ # "Affects me too" information
+ ('BugAffectsPerson', 'person'),
+ ]
+ for table, person_id_column in removals:
+ table_notification(table)
+ store.execute("""
+ DELETE FROM %(table)s WHERE %(person_id_column)s = ?
+ """ % {
+ 'table': table,
+ 'person_id_column': person_id_column,
+ },
+ (person.id,))
+
+ # Trash Sprint Attendance records in the future.
+ table_notification('SprintAttendance')
+ store.execute("""
+ DELETE FROM SprintAttendance
+ USING Sprint
+ WHERE Sprint.id = SprintAttendance.sprint
+ AND attendee = ?
+ AND Sprint.time_starts > CURRENT_TIMESTAMP AT TIME ZONE 'UTC'
+ """, (person.id,))
+ # Any remaining past sprint attendance records can harmlessly refer to
+ # the placeholder person row.
+ skip.add(('sprintattendance', 'attendee'))
+
+ # generate_ppa_htaccess currently relies on seeing active
+ # ArchiveAuthToken rows so that it knows which ones to remove from
+ # .htpasswd files on disk in response to the cancellation of the
+ # corresponding ArchiveSubscriber rows; but even once PPA authorisation
+ # is handled dynamically, we probably still want to have the per-person
+ # audit trail here.
+ archive_subscriber_ids = set(store.find(
+ ArchiveSubscriber.id,
+ ArchiveSubscriber.subscriber_id == person.id,
+ ArchiveSubscriber.status == ArchiveSubscriberStatus.CURRENT))
+ if archive_subscriber_ids:
+ getUtility(IArchiveSubscriberSet).cancel(
+ archive_subscriber_ids, janitor)
+ skip.add(('archivesubscriber', 'subscriber'))
+ skip.add(('archiveauthtoken', 'person'))
+
+ # Remove hardware submissions.
+ table_notification('HWSubmissionDevice')
+ store.execute("""
+ DELETE FROM HWSubmissionDevice
+ USING HWSubmission
+ WHERE HWSubmission.id = HWSubmissionDevice.submission
+ AND owner = ?
+ """, (person.id,))
+ table_notification('HWSubmission')
+ store.execute("""
+ DELETE FROM HWSubmission
+ WHERE HWSubmission.owner = ?
+ """, (person.id,))
+
+ # 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:
+ log.error(
+ "%s Can't delete non-trivial PPAs for user %s", self.log_name,
+ username)
+ raise IntegrityError(
+ "Can't delete non-trivial PPAs for user %s" % username)
+
+ has_references = False
+
+ # Check for active related projects, and skip inactive ones.
+ for col in 'bug_supervisor', 'driver', 'owner':
+ # Raw SQL because otherwise using Product._owner while displaying
+ # it as Product.owner is too fiddly.
+ result = store.execute("""
+ SELECT COUNT(*) FROM product WHERE active AND %(col)s = ?
+ """ % {'col': col},
+ (person.id,))
+ count = result.get_one()[0]
+ if count:
+ log.error(
+ "User %s is still referenced by %d product.%s values" %
+ (username, count, col))
+ has_references = True
+ skip.add(('product', col))
+ for col in 'driver', 'owner':
+ count = store.find(
+ ProductSeries,
+ ProductSeries.product == Product.id, Product.active,
+ getattr(ProductSeries, col) == person).count()
+ if count:
+ log.error(
+ "User %s is still referenced by %d productseries.%s values"
+ % (username, count, col))
+ has_references = True
+ skip.add(('productseries', col))
+
+ # Closing the account will only work if all references have been
+ # handled by this point. If not, it's safer to bail out. It's OK if
+ # this doesn't work in all conceivable situations, since some of them
+ # may require careful thought and decisions by a human administrator.
+ for src_tab, src_col, ref_tab, ref_col, updact, delact in references:
+ if (src_tab, src_col) in skip:
+ continue
+ result = store.execute("""
+ SELECT COUNT(*) FROM %(src_tab)s WHERE %(src_col)s = ?
+ """ % {
+ 'src_tab': src_tab,
+ 'src_col': src_col,
+ },
+ (person.id,))
+ count = result.get_one()[0]
+ if count:
+ log.error(
+ "User %s is still referenced by %d %s.%s values" %
+ (username, count, src_tab, src_col))
+ has_references = True
+ if has_references:
+ log.error(
+ "%s User %s is still referenced", self.log_name,
+ username)
+ raise LaunchpadScriptFailure(
+ "User %s is still referenced" % username)
+
+ return True
+
+
@implementer(ITeamInvitationNotificationJob)
@provider(ITeamInvitationNotificationJobSource)
class TeamInvitationNotificationJob(PersonTransferJobDerived):
diff --git a/lib/lp/registry/scripts/closeaccount.py b/lib/lp/registry/scripts/closeaccount.py
index 6fa8585..77c7990 100644
--- a/lib/lp/registry/scripts/closeaccount.py
+++ b/lib/lp/registry/scripts/closeaccount.py
@@ -1,3 +1,4 @@
+<<<<<<< lib/lp/registry/scripts/closeaccount.py
# Copyright 2009-2020 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
@@ -492,3 +493,5 @@ class CloseAccountScript(LaunchpadScript):
else:
self.logger.debug("Committing changes")
self.txn.commit()
+=======
+>>>>>>> lib/lp/registry/scripts/closeaccount.py
diff --git a/lib/lp/registry/scripts/tests/test_closeaccount.py b/lib/lp/registry/tests/test_person_close_account_job.py
similarity index 54%
rename from lib/lp/registry/scripts/tests/test_closeaccount.py
rename to lib/lp/registry/tests/test_person_close_account_job.py
index 33ccd7a..5d73e7a 100644
--- a/lib/lp/registry/scripts/tests/test_closeaccount.py
+++ b/lib/lp/registry/tests/test_person_close_account_job.py
@@ -1,13 +1,10 @@
-# Copyright 2018-2020 Canonical Ltd. This software is licensed under the
+# Copyright 2021 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
-"""Test the close-account script."""
-
-from __future__ import absolute_import, print_function, unicode_literals
+"""Tests for `PersonDeactivateJob`."""
__metaclass__ = type
-import six
from storm.store import Store
from testtools.matchers import (
MatchesSetwise,
@@ -25,226 +22,139 @@ 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 (
- CodeImportResultStatus,
- TargetRevisionControlSystems,
- )
+from lp.code.enums import TargetRevisionControlSystems, CodeImportResultStatus
from lp.code.interfaces.codeimportjob import ICodeImportJobWorkflow
from lp.code.tests.helpers import GitHostingFixture
from lp.registry.interfaces.person import IPersonSet
+from lp.registry.interfaces.persontransferjob import (
+ IPersonCloseAccountJobSource,
+ )
from lp.registry.interfaces.teammembership import ITeamMembershipSet
-from lp.registry.scripts.closeaccount import CloseAccountScript
+from lp.registry.model.persontransferjob import PersonCloseAccountJob
from lp.scripts.garbo import PopulateLatestPersonSourcePackageReleaseCache
+from lp.services.config import config
from lp.services.database.interfaces import IStore
-from lp.services.database.sqlbase import (
- flush_database_caches,
- get_transaction_timestamp,
- )
+from lp.services.database.sqlbase import get_transaction_timestamp
+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 JobType
+from lp.services.job.interfaces.job import JobStatus, JobType
from lp.services.job.model.job import Job
-from lp.services.log.logger import (
- BufferLogger,
- DevNullLogger,
- )
+from lp.services.job.runner import JobRunner
+from lp.services.job.tests import block_on_job
+from lp.services.log.logger import BufferLogger, DevNullLogger
+from lp.services.scripts import log
from lp.services.scripts.base import LaunchpadScriptFailure
from lp.services.verification.interfaces.authtoken import LoginTokenType
from lp.services.verification.interfaces.logintoken import ILoginTokenSet
-from lp.soyuz.enums import (
- ArchiveSubscriberStatus,
- PackagePublishingStatus,
- )
+from lp.soyuz.enums import PackagePublishingStatus, ArchiveSubscriberStatus
from lp.soyuz.model.archive import Archive
from lp.soyuz.model.archiveauthtoken import ArchiveAuthToken
from lp.soyuz.tests.test_publishing import SoyuzTestPublisher
-from lp.testing import (
- login_celebrity,
- TestCaseWithFactory,
- )
+from lp.testing import TestCaseWithFactory, login_celebrity
from lp.testing.dbuser import dbuser
-from lp.testing.layers import LaunchpadZopelessLayer
+from lp.testing.layers import (
+ CeleryJobLayer,
+ LaunchpadZopelessLayer,
+ )
from lp.translations.interfaces.pofiletranslator import IPOFileTranslatorSet
from lp.translations.interfaces.translationsperson import ITranslationsPerson
-class TestCloseAccount(TestCaseWithFactory):
- """Test the close-account script.
-
- Unfortunately, we have no way of detecting schema updates containing new
- information that needs to be removed or sanitized on account closure
- apart from reviewers noticing and prompting developers to update this
- script. See Bug #120506 for more details.
- """
+class TestPersonCloseAccountJob(TestCaseWithFactory):
layer = LaunchpadZopelessLayer
- def makeScript(self, test_args):
- script = CloseAccountScript(test_args=test_args)
- script.logger = BufferLogger()
- script.txn = transaction
- return script
-
- def runScript(self, script):
- try:
- script.main()
- finally:
- self.addDetail("log", script.logger.content)
- flush_database_caches()
-
- def makePopulatedUser(self):
- """Return a person and account linked to some personal information."""
- person = self.factory.makePerson(karma=10)
- self.assertEqual(AccountStatus.ACTIVE, person.account.status)
- self.assertNotEqual([], list(person.account.openid_identifiers))
- self.factory.makeBugTask().transitionToAssignee(person, validate=False)
- self.factory.makeQuestion().assignee = person
- self.factory.makeQuestion(owner=person)
- self.factory.makeGPGKey(owner=person)
- self.factory.makeBranchSubscription(person=person)
- self.factory.makeBug().subscribe(person, person)
- self.factory.makeGitSubscription(person=person)
- team = self.factory.makeTeam()
- self.factory.makeMailingList(team, team.teamowner).subscribe(person)
- self.factory.makeQuestionSubscription(person=person)
- self.factory.makeSpecification().subscribe(person)
- person.addLanguage(self.factory.makeLanguage())
- self.factory.makeSSHKey(person=person)
- self.factory.makeAccessArtifactGrant(grantee=person)
- self.factory.makeAccessPolicyGrant(grantee=person)
- self.factory.makeGitRuleGrant(grantee=person)
- person.selfgenerated_bugnotifications = True
- person.expanded_notification_footers = True
- person.require_strong_email_authentication = True
- return person, person.id, person.account.id
-
- def assertRemoved(self, account_id, person_id):
- # The Account row still exists, but has been anonymised, leaving
- # only a minimal audit trail.
- account = getUtility(IAccountSet).get(account_id)
- self.assertEqual('Removed by request', account.displayname)
- self.assertEqual(AccountStatus.CLOSED, account.status)
- self.assertIn('Closed using close-account.', account.status_history)
-
- # The Person row still exists to maintain links with information
- # that won't be removed, such as bug comments, but has been
- # anonymised.
- person = getUtility(IPersonSet).get(person_id)
- self.assertThat(person.name, StartsWith('removed'))
- self.assertEqual('Removed by request', person.display_name)
- self.assertEqual(account, person.account)
-
- # The corresponding PersonSettings row has been reset to the
- # defaults.
- self.assertFalse(person.selfgenerated_bugnotifications)
- self.assertFalse(person.expanded_notification_footers)
- self.assertFalse(person.require_strong_email_authentication)
-
- # EmailAddress and OpenIdIdentifier rows have been removed.
- self.assertEqual(
- [], list(getUtility(IEmailAddressSet).getByPerson(person)))
- self.assertEqual([], list(account.openid_identifiers))
-
- 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))
-
- def test_nonexistent(self):
- script = self.makeScript(['nonexistent-person'])
- with dbuser('launchpad'):
- self.assertRaisesWithContent(
- LaunchpadScriptFailure,
- 'User nonexistent-person does not exist',
- self.runScript, script)
+ def test_close_account_job_nonexistent_username_email(self):
+ self.assertRaisesWithContent(
+ TypeError,
+ "User nonexistent_username does not exist",
+ getUtility(IPersonCloseAccountJobSource).create,
+ u'nonexistent_username')
+
+ self.assertRaisesWithContent(
+ TypeError,
+ "User nonexistent_email@xxxxxxxxxxxxx does not exist",
+ getUtility(IPersonCloseAccountJobSource).create,
+ u'nonexistent_email@xxxxxxxxxxxxx')
+
+ def test_close_account_job_valid_username(self):
+ user_to_delete = self.factory.makePerson(name=u'delete-me')
+ job_source = getUtility(IPersonCloseAccountJobSource)
+ jobs = list(job_source.iterReady())
+
+ # at this point we have no jobs
+ self.assertEqual([], jobs)
+
+ getUtility(IPersonCloseAccountJobSource).create(u'delete-me')
+ jobs = list(job_source.iterReady())
+ jobs[0] = removeSecurityProxy(jobs[0])
+ with dbuser(config.IPersonCloseAccountJobSource.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'delete-me@xxxxxxxxxxx')
+ getUtility(
+ IPersonCloseAccountJobSource).create(u'delete-me@xxxxxxxxxxx')
+ self.assertJobCompletes()
+
+ 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()
- script = self.makeScript([team.name])
- with dbuser('launchpad'):
- self.assertRaisesWithContent(
- LaunchpadScriptFailure,
- '%s is a team' % team.name,
- self.runScript, script)
+ self.assertRaisesWithContent(
+ TypeError,
+ "%s is a team" % team.name,
+ getUtility(IPersonCloseAccountJobSource).create,
+ team.name)
def test_unhandled_reference(self):
- person = self.factory.makePerson()
+ 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
- self.factory.makeProduct(owner=person)
- 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 product.owner values' % (
- person.name),
- script.logger.getLogBuffer())
- self.assertNotRemoved(account_id, person_id)
+ job = PersonCloseAccountJob.create(u'delete-me')
+ logger = BufferLogger()
+ with log.use(logger),\
+ dbuser(config.IPersonCloseAccountJobSource.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',
+ u'ERROR PersonCloseAccountJob User delete-me is still referenced'
+ })
+ self.assertTrue(
+ error_message.issubset(logger.getLogBuffer().splitlines()))
- def test_dry_run(self):
- person, person_id, account_id = self.makePopulatedUser()
- script = self.makeScript(['--dry-run', six.ensure_str(person.name)])
- with dbuser('launchpad'):
- self.runScript(script)
- self.assertIn(
- 'Dry run, so not committing changes', script.logger.getLogBuffer())
self.assertNotRemoved(account_id, person_id)
- def test_single_by_name(self):
- person, person_id, account_id = self.makePopulatedUser()
- script = self.makeScript([six.ensure_str(person.name)])
- with dbuser('launchpad'):
- self.runScript(script)
- self.assertRemoved(account_id, person_id)
-
- def test_single_by_email(self):
- person, person_id, account_id = self.makePopulatedUser()
- script = self.makeScript([six.ensure_str(person.preferredemail.email)])
- with dbuser('launchpad'):
- self.runScript(script)
- self.assertRemoved(account_id, person_id)
-
- def test_multiple(self):
- persons = [self.factory.makePerson() for _ in range(3)]
- person_ids = [person.id for person in persons]
- account_ids = [person.account.id for person in persons]
- script = self.makeScript([persons[0].name, persons[1].name])
- with dbuser('launchpad'):
- self.runScript(script)
- self.assertRemoved(account_ids[0], person_ids[0])
- self.assertRemoved(account_ids[1], person_ids[1])
- self.assertNotRemoved(account_ids[2], person_ids[2])
-
- def test_multiple_email_addresses(self):
- person, person_id, account_id = self.makePopulatedUser()
- self.factory.makeEmail('%s@xxxxxxxxxxxxxxxxxxx' % person.name, person)
- script = self.makeScript([six.ensure_str(person.name)])
- with dbuser('launchpad'):
- self.runScript(script)
- self.assertRemoved(account_id, person_id)
-
def test_unactivated(self):
person = self.factory.makePerson(
account_status=AccountStatus.NOACCOUNT)
person_id = person.id
account_id = person.account.id
- script = self.makeScript([person.guessedemails[0].email])
- with dbuser('launchpad'):
- self.runScript(script)
- self.assertRemoved(account_id, person_id)
+ job = PersonCloseAccountJob.create(person.guessedemails[0].email)
+ logger = BufferLogger()
+ with log.use(logger),\
+ dbuser(config.IPersonCloseAccountJobSource.dbuser):
+ job.run()
+
+ self.assertAccountRemoved(account_id, person_id)
def test_retains_audit_trail(self):
person = self.factory.makePerson()
@@ -255,10 +165,12 @@ class TestCloseAccount(TestCaseWithFactory):
snap = self.factory.makeSnap()
snap_build = self.factory.makeSnapBuild(requester=person, snap=snap)
specification = self.factory.makeSpecification(drafter=person)
- script = self.makeScript([six.ensure_str(person.name)])
- with dbuser('launchpad'):
- self.runScript(script)
- self.assertRemoved(account_id, person_id)
+ job = PersonCloseAccountJob.create(person.name)
+ logger = BufferLogger()
+ with log.use(logger),\
+ dbuser(config.IPersonCloseAccountJobSource.dbuser):
+ job.run()
+ self.assertAccountRemoved(account_id, person_id)
self.assertEqual(person, branch_subscription.subscribed_by)
self.assertEqual(person, snap_build.requester)
self.assertEqual(person, specification.drafter)
@@ -275,10 +187,12 @@ class TestCloseAccount(TestCaseWithFactory):
question.addComment(person, "comment")
removeSecurityProxy(question).status = status
questions.append(question)
- script = self.makeScript([six.ensure_str(person.name)])
- with dbuser('launchpad'):
- self.runScript(script)
- self.assertRemoved(account_id, person_id)
+ job = PersonCloseAccountJob.create(person.name)
+ logger = BufferLogger()
+ with log.use(logger),\
+ dbuser(config.IPersonCloseAccountJobSource.dbuser):
+ job.run()
+ self.assertAccountRemoved(account_id, person_id)
for question in questions:
self.assertEqual(QuestionStatus.SOLVED, question.status)
self.assertEqual(
@@ -297,10 +211,12 @@ class TestCloseAccount(TestCaseWithFactory):
question.addComment(person, "comment")
removeSecurityProxy(question).status = status
questions[status] = question
- script = self.makeScript([six.ensure_str(person.name)])
- with dbuser('launchpad'):
- self.runScript(script)
- self.assertRemoved(account_id, person_id)
+ job = PersonCloseAccountJob.create(person.name)
+ logger = BufferLogger()
+ with log.use(logger),\
+ dbuser(config.IPersonCloseAccountJobSource.dbuser):
+ job.run()
+ self.assertAccountRemoved(account_id, person_id)
for question_status, question in questions.items():
self.assertEqual(question_status, question.status)
self.assertIsNone(question.whiteboard)
@@ -323,10 +239,12 @@ class TestCloseAccount(TestCaseWithFactory):
while not job.isDone():
job(chunk_size=100)
self.assertTrue(person.hasMaintainedPackages())
- script = self.makeScript([six.ensure_str(person.name)])
- with dbuser('launchpad'):
- self.runScript(script)
- self.assertRemoved(account_id, person_id)
+ job = PersonCloseAccountJob.create(person.name)
+ logger = BufferLogger()
+ with log.use(logger),\
+ dbuser(config.IPersonCloseAccountJobSource.dbuser):
+ job.run()
+ self.assertAccountRemoved(account_id, person_id)
self.assertEqual(person, spph.package_maintainer)
self.assertEqual(person, spph.package_creator)
self.assertFalse(person.hasMaintainedPackages())
@@ -337,10 +255,12 @@ class TestCloseAccount(TestCaseWithFactory):
bugtask = self.factory.makeBugTask(bug=bug, owner=person)
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)
+ job = PersonCloseAccountJob.create(person.name)
+ logger = BufferLogger()
+ with log.use(logger),\
+ dbuser(config.IPersonCloseAccountJobSource.dbuser):
+ job.run()
+ self.assertAccountRemoved(account_id, person_id)
self.assertEqual(person, bug.owner)
self.assertEqual(person, bugtask.owner)
@@ -362,10 +282,12 @@ class TestCloseAccount(TestCaseWithFactory):
self.assertTrue(bug.isUserAffected(person))
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)
+ job = PersonCloseAccountJob.create(person.name)
+ logger = BufferLogger()
+ with log.use(logger),\
+ dbuser(config.IPersonCloseAccountJobSource.dbuser):
+ job.run()
+ self.assertAccountRemoved(account_id, person_id)
self.assertFalse(bug.isUserAffected(person))
def test_skips_translation_relicensing_agreements(self):
@@ -374,10 +296,12 @@ class TestCloseAccount(TestCaseWithFactory):
translations_person.translations_relicensing_agreement = True
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)
+ job = PersonCloseAccountJob.create(person.name)
+ logger = BufferLogger()
+ with log.use(logger),\
+ dbuser(config.IPersonCloseAccountJobSource.dbuser):
+ job.run()
+ self.assertAccountRemoved(account_id, person_id)
self.assertTrue(translations_person.translations_relicensing_agreement)
def test_skips_po_file_translators(self):
@@ -391,10 +315,12 @@ class TestCloseAccount(TestCaseWithFactory):
person, pofile))
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)
+ job = PersonCloseAccountJob.create(person.name)
+ logger = BufferLogger()
+ with log.use(logger),\
+ dbuser(config.IPersonCloseAccountJobSource.dbuser):
+ job.run()
+ self.assertAccountRemoved(account_id, person_id)
self.assertIsNotNone(
getUtility(IPOFileTranslatorSet).getForPersonPOFile(
person, pofile))
@@ -410,11 +336,13 @@ class TestCloseAccount(TestCaseWithFactory):
self.assertIsNotNone(ppa.getAuthToken(person))
person_id = person.id
account_id = person.account.id
- script = self.makeScript([six.ensure_str(person.name)])
- with dbuser('launchpad'):
+ job = PersonCloseAccountJob.create(person.name)
+ logger = BufferLogger()
+ with log.use(logger),\
+ dbuser(config.IPersonCloseAccountJobSource.dbuser):
now = get_transaction_timestamp(Store.of(person))
- self.runScript(script)
- self.assertRemoved(account_id, person_id)
+ job.run()
+ self.assertAccountRemoved(account_id, person_id)
self.assertEqual(
ArchiveSubscriberStatus.CANCELLED, subscription.status)
self.assertEqual(now, subscription.date_cancelled)
@@ -538,10 +466,12 @@ class TestCloseAccount(TestCaseWithFactory):
submission_ids[0], get_submission_by_submission_key(keys[0]))
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)
+ job = PersonCloseAccountJob.create(person.name)
+ logger = BufferLogger()
+ with log.use(logger),\
+ dbuser(config.IPersonCloseAccountJobSource.dbuser):
+ job.run()
+ self.assertAccountRemoved(account_id, person_id)
self.assertEqual([], get_submissions_by_owner(person))
self.assertIsNone(get_submission_by_submission_key(keys[0]))
self.assertEqual(
@@ -565,10 +495,12 @@ class TestCloseAccount(TestCaseWithFactory):
MatchesStructure.byEquality(count=1, viewed_by=other_person)))
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)
+ job = PersonCloseAccountJob.create(person.name)
+ logger = BufferLogger()
+ with log.use(logger),\
+ dbuser(config.IPersonCloseAccountJobSource.dbuser):
+ job.run()
+ self.assertAccountRemoved(account_id, person_id)
# BugSummaryJournal has been updated, but BugSummary hasn't yet.
summaries = list(store.find(
BugSummary,
@@ -587,6 +519,20 @@ class TestCloseAccount(TestCaseWithFactory):
self.assertThat(summaries, MatchesSetwise(
MatchesStructure.byEquality(viewed_by=other_person)))
+ def test_skips_inactive_product_owner(self):
+ person = self.factory.makePerson()
+ product = self.factory.makeProduct(owner=person)
+ product.active = False
+ person_id = person.id
+ account_id = person.account.id
+ job = PersonCloseAccountJob.create(person.name)
+ logger = BufferLogger()
+ with log.use(logger),\
+ dbuser(config.IPersonCloseAccountJobSource.dbuser):
+ job.run()
+ self.assertAccountRemoved(account_id, person_id)
+ self.assertEqual(person, product.owner)
+
def test_skips_bug_nomination(self):
person = self.factory.makePerson()
other_person = self.factory.makePerson()
@@ -601,26 +547,16 @@ class TestCloseAccount(TestCaseWithFactory):
MatchesStructure.byEquality(owner=other_person)))
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)
+ job = PersonCloseAccountJob.create(person.name)
+ logger = BufferLogger()
+ with log.use(logger),\
+ dbuser(config.IPersonCloseAccountJobSource.dbuser):
+ job.run()
+ self.assertAccountRemoved(account_id, person_id)
self.assertThat(bug.getNominations(), MatchesSetwise(
MatchesStructure.byEquality(owner=person),
MatchesStructure.byEquality(owner=other_person)))
- def test_skips_inactive_product_owner(self):
- person = self.factory.makePerson()
- product = self.factory.makeProduct(owner=person)
- product.active = False
- 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.assertEqual(person, product.owner)
-
def test_skips_code_import(self):
self.useFixture(GitHostingFixture())
person = self.factory.makePerson()
@@ -633,10 +569,12 @@ class TestCloseAccount(TestCaseWithFactory):
TargetRevisionControlSystems.GIT)]
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)
+ job = PersonCloseAccountJob.create(person.name)
+ logger = BufferLogger()
+ with log.use(logger),\
+ dbuser(config.IPersonCloseAccountJobSource.dbuser):
+ job.run()
+ self.assertAccountRemoved(account_id, person_id)
self.assertEqual(person, code_imports[0].registrant)
self.assertEqual(person, code_imports[1].registrant)
@@ -661,10 +599,12 @@ class TestCloseAccount(TestCaseWithFactory):
result_status=CodeImportResultStatus.SUCCESS)
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)
+ job = PersonCloseAccountJob.create(person.name)
+ logger = BufferLogger()
+ with log.use(logger), \
+ dbuser(config.IPersonCloseAccountJobSource.dbuser):
+ job.run()
+ self.assertAccountRemoved(account_id, person_id)
self.assertEqual(person, code_import.registrant)
self.assertEqual(person, result.requesting_user)
self.assertEqual(person, code_import.import_job.requesting_user)
@@ -689,10 +629,12 @@ class TestCloseAccount(TestCaseWithFactory):
removeSecurityProxy(ppa).owner = other_person
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)
+ closeAccountjob = PersonCloseAccountJob.create(person.name)
+ logger = BufferLogger()
+ with log.use(logger), \
+ dbuser(config.IPersonCloseAccountJobSource.dbuser):
+ closeAccountjob.run()
+ self.assertAccountRemoved(account_id, person_id)
self.assertEqual(person, job.requester)
def test_skips_specification_owner(self):
@@ -700,10 +642,12 @@ class TestCloseAccount(TestCaseWithFactory):
person_id = person.id
account_id = person.account.id
specification = self.factory.makeSpecification(owner=person)
- script = self.makeScript([six.ensure_str(person.name)])
- with dbuser('launchpad'):
- self.runScript(script)
- self.assertRemoved(account_id, person_id)
+ job = PersonCloseAccountJob.create(person.name)
+ logger = BufferLogger()
+ with log.use(logger), \
+ dbuser(config.IPersonCloseAccountJobSource.dbuser):
+ job.run()
+ self.assertAccountRemoved(account_id, person_id)
self.assertEqual(person, specification.owner)
def test_skips_teammembership_last_changed_by(self):
@@ -719,10 +663,12 @@ class TestCloseAccount(TestCaseWithFactory):
person_id = member.id
account_id = member.account.id
- script = self.makeScript([six.ensure_str(member.name)])
- with dbuser('launchpad'):
- self.runScript(script)
- self.assertRemoved(account_id, person_id)
+ job = PersonCloseAccountJob.create(member.name)
+ logger = BufferLogger()
+ with log.use(logger), \
+ dbuser(config.IPersonCloseAccountJobSource.dbuser):
+ job.run()
+ self.assertAccountRemoved(account_id, person_id)
def test_skips_teamowner_merged(self):
person = self.factory.makePerson()
@@ -732,22 +678,27 @@ class TestCloseAccount(TestCaseWithFactory):
owned_team2 = self.factory.makeTeam(name='target2', owner=person)
person_id = person.id
account_id = person.account.id
- script = self.makeScript([six.ensure_str(person.name)])
# Closing account fails as the user still owns team2
- with dbuser('launchpad'):
- self.assertRaises(
- LaunchpadScriptFailure, self.runScript, script)
+ job = PersonCloseAccountJob.create(person.name)
+ logger = BufferLogger()
+ with log.use(logger), \
+ dbuser(config.IPersonCloseAccountJobSource.dbuser):
+ job.run()
+ # self.assertRaises(
+ # LaunchpadScriptFailure, job.run)
+ self.assertNotRemoved(account_id, person_id)
# Account will now close as the user doesn't own
# any other teams at this point
removeSecurityProxy(owned_team2).merged = merged_person
- with dbuser('launchpad'):
- self.runScript(script)
- self.assertRemoved(account_id, person_id)
+ with log.use(logger), \
+ dbuser(config.IPersonCloseAccountJobSource.dbuser):
+ job.run()
+ self.assertAccountRemoved(account_id, person_id)
def test_handles_login_token(self):
- person = self.factory.makePerson()
+ person = self.factory.makePerson(name=u'delete-me')
email = '%s@xxxxxxxxxxxxxxxxxxx' % person.name
login_token_set = getUtility(ILoginTokenSet)
token = login_token_set.new(
@@ -757,10 +708,10 @@ class TestCloseAccount(TestCaseWithFactory):
self.assertEqual(token, login_token_set[plaintext_token])
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)
+ getUtility(
+ IPersonCloseAccountJobSource).create(u'delete-me')
+ self.assertJobCompletes()
+ self.assertAccountRemoved(account_id, person_id)
self.assertRaises(
KeyError, login_token_set.__getitem__, plaintext_token)
@@ -775,10 +726,12 @@ class TestCloseAccount(TestCaseWithFactory):
[other_request_token], other_person.oauth_request_tokens)
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)
+ job = PersonCloseAccountJob.create(person.name)
+ logger = BufferLogger()
+ with log.use(logger), \
+ dbuser(config.IPersonCloseAccountJobSource.dbuser):
+ job.run()
+ self.assertAccountRemoved(account_id, person_id)
self.assertContentEqual([], person.oauth_request_tokens)
self.assertContentEqual(
[other_request_token], other_person.oauth_request_tokens)
@@ -794,10 +747,10 @@ class TestCloseAccount(TestCaseWithFactory):
[other_access_token], other_person.oauth_access_tokens)
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)
+ job = PersonCloseAccountJob.create(person.name)
+ with dbuser(config.IPersonCloseAccountJobSource.dbuser):
+ job.run()
+ self.assertAccountRemoved(account_id, person_id)
self.assertContentEqual([], person.oauth_access_tokens)
self.assertContentEqual(
[other_access_token], other_person.oauth_access_tokens)
@@ -809,16 +762,19 @@ class TestCloseAccount(TestCaseWithFactory):
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())
+ job = PersonCloseAccountJob.create(person.name)
+ logger = BufferLogger()
+ with log.use(logger), \
+ dbuser(config.IPersonCloseAccountJobSource.dbuser):
+ job.run()
+ error_message = (
+ {u'ERROR User %s is still referenced by 1 '
+ u'archive.owner values' % person.name,
+ u'ERROR PersonCloseAccountJob User %s is still referenced'
+ % person.name
+ })
+ self.assertTrue(
+ error_message.issubset(logger.getLogBuffer().splitlines()))
self.assertNotRemoved(account_id, person_id)
def test_fails_on_deleted_ppa_with_builds(self):
@@ -832,12 +788,16 @@ class TestCloseAccount(TestCaseWithFactory):
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)
+ job = PersonCloseAccountJob.create(person.name)
+ logger = BufferLogger()
+ with log.use(logger), \
+ dbuser(config.IPersonCloseAccountJobSource.dbuser):
+ job.run()
+ error_message = (
+ {u"ERROR PersonCloseAccountJob Can\'t delete non-trivial "
+ u"PPAs for user %s" % person.name})
+ self.assertTrue(
+ error_message.issubset(logger.getLogBuffer().splitlines()))
self.assertNotRemoved(account_id, person_id)
def test_handles_empty_deleted_ppa(self):
@@ -854,9 +814,78 @@ class TestCloseAccount(TestCaseWithFactory):
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)
+ job = PersonCloseAccountJob.create(person.name)
+ logger = BufferLogger()
+ with log.use(logger), \
+ dbuser(config.IPersonCloseAccountJobSource.dbuser):
+ job.run()
+ self.assertAccountRemoved(account_id, person_id)
self.assertIsNone(store.get(Archive, ppa_id))
self.assertEqual(other_ppa, store.get(Archive, other_ppa_id))
+
+ def assertJobCompletes(self):
+ job_source = getUtility(IPersonCloseAccountJobSource)
+ jobs = list(job_source.iterReady())
+ jobs[0] = removeSecurityProxy(jobs[0])
+ with dbuser(config.IPersonCloseAccountJobSource.dbuser):
+ JobRunner(jobs).runAll()
+ self.assertEqual(JobStatus.COMPLETED, jobs[0].status)
+
+ def assertAccountRemoved(self, account_id, person_id):
+ # The Account row still exists, but has been anonymised, leaving
+ # only a minimal audit trail.
+ account = getUtility(IAccountSet).get(account_id)
+ self.assertEqual('Removed by request', account.displayname)
+ self.assertEqual(AccountStatus.CLOSED, account.status)
+ self.assertIn('Closed using close-account.', account.status_history)
+
+ # The Person row still exists to maintain links with information
+ # that won't be removed, such as bug comments, but has been
+ # anonymised.
+ person = getUtility(IPersonSet).get(person_id)
+ self.assertThat(person.name, StartsWith('removed'))
+ self.assertEqual('Removed by request', person.display_name)
+ self.assertEqual(account, person.account)
+
+ # The corresponding PersonSettings row has been reset to the
+ # defaults.
+ self.assertFalse(person.selfgenerated_bugnotifications)
+ self.assertFalse(person.expanded_notification_footers)
+ self.assertFalse(person.require_strong_email_authentication)
+
+ # EmailAddress and OpenIdIdentifier rows have been removed.
+ self.assertEqual(
+ [], list(getUtility(IEmailAddressSet).getByPerson(person)))
+ self.assertEqual([], list(account.openid_identifiers))
+
+ 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(name=u'delete-me')
+
+ with block_on_job():
+ job = PersonCloseAccountJob.create(u'delete-me')
+ transaction.commit()
+ person = removeSecurityProxy(
+ getUtility(IPersonSet).getByName(user_to_delete.name))
+ self.assertEqual(person.name, u'removed%d' % user_to_delete.id)
+ self.assertEqual(JobStatus.COMPLETED, job.status)
diff --git a/lib/lp/services/config/schema-lazr.conf b/lib/lp/services/config/schema-lazr.conf
index db22192..50ad59d 100644
--- a/lib/lp/services/config/schema-lazr.conf
+++ b/lib/lp/services/config/schema-lazr.conf
@@ -1992,6 +1992,11 @@ link: IBranchMergeProposalJobSource
module: lp.services.webhooks.interfaces
dbuser: webhookrunner
+[IPersonCloseAccountJobSource]
+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
diff --git a/scripts/close-account.py b/scripts/close-account.py
deleted file mode 100755
index a66c679..0000000
--- a/scripts/close-account.py
+++ /dev/null
@@ -1,15 +0,0 @@
-#!/usr/bin/python2 -S
-#
-# Copyright 2009-2018 Canonical Ltd. This software is licensed under the
-# GNU Affero General Public License version 3 (see the file LICENSE).
-
-"""Remove personal details of a user from the database, leaving a stub."""
-
-import _pythonpath
-
-from lp.registry.scripts.closeaccount import CloseAccountScript
-
-
-if __name__ == '__main__':
- script = CloseAccountScript('close-account', dbuser='launchpad')
- script.run()
Follow ups
