launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #23170
[Merge] lp:~cjwatson/launchpad/close-account-packaging into lp:launchpad
Colin Watson has proposed merging lp:~cjwatson/launchpad/close-account-packaging into lp:launchpad.
Commit message:
Handle packaging-related references in close-account.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/close-account-packaging/+merge/361004
--
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/close-account-packaging into lp:launchpad.
=== modified file 'lib/lp/registry/scripts/closeaccount.py'
--- lib/lp/registry/scripts/closeaccount.py 2018-12-04 13:00:56 +0000
+++ lib/lp/registry/scripts/closeaccount.py 2018-12-17 15:03:37 +0000
@@ -105,6 +105,7 @@
('messageapproval', 'posted_by'),
('packagecopyrequest', 'requester'),
('packagediff', 'requester'),
+ ('packageupload', 'signing_key_owner'),
('personlocation', 'last_modified_by'),
('persontransferjob', 'major_person'),
('persontransferjob', 'minor_person'),
@@ -117,6 +118,9 @@
('sourcepackagepublishinghistory', 'removed_by'),
('sourcepackagepublishinghistory', 'sponsor'),
('sourcepackagerecipebuild', 'requester'),
+ ('sourcepackagerelease', 'creator'),
+ ('sourcepackagerelease', 'maintainer'),
+ ('sourcepackagerelease', 'signing_key_owner'),
('specification', 'approver'),
('specification', 'completer'),
('specification', 'drafter'),
@@ -267,6 +271,10 @@
('ArchivePermission', 'person'),
('GitRuleGrant', 'grantee'),
('SharingJob', 'grantee'),
+
+ # Soyuz reporting
+ ('LatestPersonSourcePackageReleaseCache', 'creator'),
+ ('LatestPersonSourcePackageReleaseCache', 'maintainer'),
]
for table, person_id_column in removals:
table_notification(table)
=== modified file 'lib/lp/registry/scripts/tests/test_closeaccount.py'
--- lib/lp/registry/scripts/tests/test_closeaccount.py 2018-12-04 17:33:38 +0000
+++ lib/lp/registry/scripts/tests/test_closeaccount.py 2018-12-17 15:03:37 +0000
@@ -17,19 +17,26 @@
from zope.security.proxy import removeSecurityProxy
from lp.answers.enums import QuestionStatus
+from lp.app.interfaces.launchpad import ILaunchpadCelebrities
from lp.registry.interfaces.person import IPersonSet
from lp.registry.scripts.closeaccount import CloseAccountScript
+from lp.scripts.garbo import PopulateLatestPersonSourcePackageReleaseCache
from lp.services.database.sqlbase import flush_database_caches
from lp.services.identity.interfaces.account import (
AccountStatus,
IAccountSet,
)
from lp.services.identity.interfaces.emailaddress import IEmailAddressSet
-from lp.services.log.logger import BufferLogger
+from lp.services.log.logger import (
+ BufferLogger,
+ DevNullLogger,
+ )
from lp.services.scripts.base import LaunchpadScriptFailure
+from lp.soyuz.enums import PackagePublishingStatus
+from lp.soyuz.tests.test_publishing import SoyuzTestPublisher
from lp.testing import TestCaseWithFactory
from lp.testing.dbuser import dbuser
-from lp.testing.layers import ZopelessDatabaseLayer
+from lp.testing.layers import LaunchpadZopelessLayer
class TestCloseAccount(TestCaseWithFactory):
@@ -41,7 +48,7 @@
script. See Bug #120506 for more details.
"""
- layer = ZopelessDatabaseLayer
+ layer = LaunchpadZopelessLayer
def makeScript(self, test_args):
script = CloseAccountScript(test_args=test_args)
@@ -248,3 +255,29 @@
for question_status, question in questions.items():
self.assertEqual(question_status, question.status)
self.assertIsNone(question.whiteboard)
+
+ def test_handles_packaging_references(self):
+ person = self.factory.makePerson()
+ person_id = person.id
+ account_id = person.account.id
+ self.factory.makeGPGKey(person)
+ publisher = SoyuzTestPublisher()
+ publisher.person = person
+ ubuntu = getUtility(ILaunchpadCelebrities).ubuntu
+ spph = publisher.getPubSource(
+ status=PackagePublishingStatus.PUBLISHED,
+ distroseries=ubuntu.currentseries,
+ maintainer=person, creator=person)
+ with dbuser('garbo_frequently'):
+ job = PopulateLatestPersonSourcePackageReleaseCache(
+ DevNullLogger())
+ while not job.isDone():
+ job(chunk_size=100)
+ self.assertTrue(person.hasMaintainedPackages())
+ script = self.makeScript([nativeString(person.name)])
+ with dbuser('launchpad'):
+ self.runScript(script)
+ self.assertRemoved(account_id, person_id)
+ self.assertEqual(person, spph.package_maintainer)
+ self.assertEqual(person, spph.package_creator)
+ self.assertFalse(person.hasMaintainedPackages())
=== modified file 'lib/lp/scripts/garbo.py'
--- lib/lp/scripts/garbo.py 2018-05-01 08:59:57 +0000
+++ lib/lp/scripts/garbo.py 2018-12-17 15:03:37 +0000
@@ -648,6 +648,7 @@
def __call__(self, chunk_size):
cache_filter_data = []
new_records = dict()
+ person_ids = set()
# Create a map of new published spr data for creators and maintainers.
# The map is keyed on (creator/maintainer, archive, spn, distroseries).
for new_published_spr_data in self.getPendingUpdates()[:chunk_size]:
@@ -661,8 +662,10 @@
maintainer_id, None, archive_id, distroseries_id, spn_id)
creator_key = (
None, creator_id, archive_id, distroseries_id, spn_id)
- new_records[maintainer_key] = maintainer_key + value
- new_records[creator_key] = creator_key + value
+ new_records[maintainer_key] = list(maintainer_key + value)
+ new_records[creator_key] = list(creator_key + value)
+ person_ids.add(maintainer_id)
+ person_ids.add(creator_id)
self.last_spph_id = spph_id
# Gather all the current cached reporting records corresponding to the
@@ -689,14 +692,32 @@
existing_records[key] = pytz.UTC.localize(
lpsprc_record.dateuploaded)
+ # Gather account statuses for creators and maintainers.
+ # Deactivating or closing an account removes its LPSPRC rows, and we
+ # don't want to resurrect them.
+ account_statuses = dict(self.store.find(
+ (Person.id, Account.status),
+ Person.id.is_in(person_ids), Person.account == Account.id))
+ ignore_statuses = (AccountStatus.DEACTIVATED, AccountStatus.CLOSED)
+ for new_record in new_records.values():
+ if (new_record[0] is not None and
+ account_statuses.get(new_record[0]) in ignore_statuses):
+ new_record[0] = None
+ if (new_record[1] is not None and
+ account_statuses.get(new_record[1]) in ignore_statuses):
+ new_record[1] = None
+
# Figure out what records from the new published spr data need to be
# inserted and updated into the cache table.
inserts = dict()
updates = dict()
for key, new_published_spr_data in new_records.items():
existing_dateuploaded = existing_records.get(key, None)
- new_dateuploaded = new_published_spr_data[7]
- if existing_dateuploaded is None:
+ new_maintainer, new_creator, _, _, _, _, _, new_dateuploaded, _ = (
+ new_published_spr_data)
+ if new_maintainer is None and new_creator is None:
+ continue
+ elif existing_dateuploaded is None:
target = inserts
else:
target = updates
=== modified file 'lib/lp/scripts/tests/test_garbo.py'
--- lib/lp/scripts/tests/test_garbo.py 2018-05-01 08:59:57 +0000
+++ lib/lp/scripts/tests/test_garbo.py 2018-12-17 15:03:37 +0000
@@ -29,9 +29,15 @@
Storm,
)
from storm.store import Store
+from testtools.content import Content
+from testtools.content_type import UTF8_TEXT
from testtools.matchers import (
+ AfterPreprocessing,
Equals,
GreaterThan,
+ Is,
+ MatchesListwise,
+ MatchesStructure,
)
import transaction
from zope.component import getUtility
@@ -417,6 +423,9 @@
self.log_buffer = StringIO()
handler = logging.StreamHandler(self.log_buffer)
self.log.addHandler(handler)
+ self.addDetail(
+ 'garbo-log',
+ Content(UTF8_TEXT, lambda: [self.log_buffer.getvalue()]))
def runFrequently(self, maximum_chunk_size=2, test_args=()):
switch_dbuser('garbo_daily')
@@ -1380,12 +1389,12 @@
def test_PopulateLatestPersonSourcePackageReleaseCache(self):
switch_dbuser('testadmin')
# Make some same test data - we create published source package
- # releases for 2 different creators and maintainers.
+ # releases for three different creators and maintainers.
creators = []
- for _ in range(2):
+ for _ in range(3):
creators.append(self.factory.makePerson())
maintainers = []
- for _ in range(2):
+ for _ in range(3):
maintainers.append(self.factory.makePerson())
spn = self.factory.makeSourcePackageName()
@@ -1415,9 +1424,22 @@
creator=creators[1], maintainer=maintainers[1],
distroseries=distroseries, sourcepackagename=spn,
date_uploaded=datetime(2010, 12, 4, tzinfo=UTC))
+ self.factory.makeSourcePackagePublishingHistory(
+ status=PackagePublishingStatus.PUBLISHED,
+ sourcepackagerelease=spr4)
+ spr5 = self.factory.makeSourcePackageRelease(
+ creator=creators[2], maintainer=maintainers[2],
+ distroseries=distroseries, sourcepackagename=spn,
+ date_uploaded=datetime(2010, 12, 5, tzinfo=UTC))
spph_1 = self.factory.makeSourcePackagePublishingHistory(
status=PackagePublishingStatus.PUBLISHED,
- sourcepackagerelease=spr4)
+ sourcepackagerelease=spr5)
+
+ # Some of the creators and maintainers have inactive accounts.
+ removeSecurityProxy(creators[2].account).status = (
+ AccountStatus.DEACTIVATED)
+ removeSecurityProxy(maintainers[2].account).status = (
+ AccountStatus.CLOSED)
transaction.commit()
self.runFrequently()
@@ -1430,31 +1452,37 @@
params=[u'PopulateLatestPersonSourcePackageReleaseCache']
).get_one())
- def _assert_release_by_creator(creator, spr):
+ def _assert_releases_by_creator(creator, sprs):
release_records = store.find(
LatestPersonSourcePackageReleaseCache,
LatestPersonSourcePackageReleaseCache.creator_id == creator.id)
- [record] = list(release_records)
- self.assertEqual(spr.creator, record.creator)
- self.assertIsNone(record.maintainer_id)
- self.assertEqual(
- spr.dateuploaded, UTC.localize(record.dateuploaded))
+ self.assertThat(list(release_records), MatchesListwise([
+ MatchesStructure(
+ creator=Equals(spr.creator),
+ maintainer_id=Is(None),
+ dateuploaded=AfterPreprocessing(
+ UTC.localize, Equals(spr.dateuploaded)))
+ for spr in sprs]))
- def _assert_release_by_maintainer(maintainer, spr):
+ def _assert_releases_by_maintainer(maintainer, sprs):
release_records = store.find(
LatestPersonSourcePackageReleaseCache,
LatestPersonSourcePackageReleaseCache.maintainer_id ==
maintainer.id)
- [record] = list(release_records)
- self.assertEqual(spr.maintainer, record.maintainer)
- self.assertIsNone(record.creator_id)
- self.assertEqual(
- spr.dateuploaded, UTC.localize(record.dateuploaded))
+ self.assertThat(list(release_records), MatchesListwise([
+ MatchesStructure(
+ maintainer=Equals(spr.maintainer),
+ creator_id=Is(None),
+ dateuploaded=AfterPreprocessing(
+ UTC.localize, Equals(spr.dateuploaded)))
+ for spr in sprs]))
- _assert_release_by_creator(creators[0], spr2)
- _assert_release_by_creator(creators[1], spr4)
- _assert_release_by_maintainer(maintainers[0], spr3)
- _assert_release_by_maintainer(maintainers[1], spr4)
+ _assert_releases_by_creator(creators[0], [spr2])
+ _assert_releases_by_creator(creators[1], [spr4])
+ _assert_releases_by_creator(creators[2], [])
+ _assert_releases_by_maintainer(maintainers[0], [spr3])
+ _assert_releases_by_maintainer(maintainers[1], [spr4])
+ _assert_releases_by_maintainer(maintainers[2], [])
job_data = load_garbo_job_state(
'PopulateLatestPersonSourcePackageReleaseCache')
@@ -1463,21 +1491,23 @@
# Create a newer published source package release and ensure the
# release cache table is correctly updated.
switch_dbuser('testadmin')
- spr5 = self.factory.makeSourcePackageRelease(
+ spr6 = self.factory.makeSourcePackageRelease(
creator=creators[1], maintainer=maintainers[1],
distroseries=distroseries, sourcepackagename=spn,
date_uploaded=datetime(2010, 12, 5, tzinfo=UTC))
spph_2 = self.factory.makeSourcePackagePublishingHistory(
status=PackagePublishingStatus.PUBLISHED,
- sourcepackagerelease=spr5)
+ sourcepackagerelease=spr6)
transaction.commit()
self.runFrequently()
- _assert_release_by_creator(creators[0], spr2)
- _assert_release_by_creator(creators[1], spr5)
- _assert_release_by_maintainer(maintainers[0], spr3)
- _assert_release_by_maintainer(maintainers[1], spr5)
+ _assert_releases_by_creator(creators[0], [spr2])
+ _assert_releases_by_creator(creators[1], [spr6])
+ _assert_releases_by_creator(creators[2], [])
+ _assert_releases_by_maintainer(maintainers[0], [spr3])
+ _assert_releases_by_maintainer(maintainers[1], [spr6])
+ _assert_releases_by_maintainer(maintainers[2], [])
job_data = load_garbo_job_state(
'PopulateLatestPersonSourcePackageReleaseCache')
Follow ups
