Launchpad itself

Merge lp:~stub/launchpad/garbo-bulk-pruner into lp:launchpad

  1. garbo-bulk-pruner
  2. Merge into devel
Proposed by Stuart Bishop on 2012-01-04
Status: Merged
Approved by: William Grant on 2012-01-04
Approved revision: no longer in the source branch.
Merged at revision: 14630
Proposed branch: lp:~stub/launchpad/garbo-bulk-pruner
Merge into: lp:launchpad
Prerequisite: lp:~stub/launchpad/garbo
Diff against target: 191 lines (+34/-35)
3 files modified
database/schema/security.cfg (+3/-2)
lib/lp/scripts/garbo.py (+29/-1)
lib/lp/scripts/tests/test_garbo.py (+2/-32)
To merge this branch: bzr merge lp:~stub/launchpad/garbo-bulk-pruner
Reviewer Review Type Date Requested Status
William Grant code 2012-01-04 Approve on 2012-01-04
Review via email: mp+87461@code.launchpad.net

Commit Message

[r=wgrant][no-qa] Trash EmailAddress and Account records not linked to a Person.

Description of the Change

Trash EmailAddress and Account records not linked to a Person. With this, we can drop the redundant EmailAddress.account field bringing a little sanity to our data model and poisoning some lurking bugs at the source.

To post a comment you must log in.
William Grant (wgrant) wrote :

22:42:25 < wgrant> stub: What does SELECT COUNT(*) FROM emailaddress JOIN person ON person.id = emailaddress.person WHERE
                   emailaddress.account IS DISTINCT FROM person.account; say on prod?
22:42:55 < wgrant> s/IS DISTINCT FROM/!=/ was 0 yesterday, but this is probably closer to what we actually care about.
22:46:53 < stub> wgrant: 0

review: Approve (code)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'database/schema/security.cfg'
2--- database/schema/security.cfg 2012-01-03 01:40:09 +0000
3+++ database/schema/security.cfg 2012-01-04 11:57:46 +0000
4@@ -1,4 +1,4 @@
5-# Copyright 2009-2011 Canonical Ltd. This software is licensed under the
6+# Copyright 2009-2012 Canonical Ltd. This software is licensed under the
7 # GNU Affero General Public License version 3 (see the file LICENSE).
8 #
9 # Possible permissions: SELECT, INSERT, UPDATE, EXECUTE
10@@ -2114,6 +2114,7 @@
11
12 [garbo]
13 groups=script,read
14+public.account = SELECT, DELETE
15 public.answercontact = SELECT, DELETE
16 public.branch = SELECT, UPDATE
17 public.branchjob = SELECT, DELETE
18@@ -2141,7 +2142,7 @@
19 public.codeimportevent = SELECT, DELETE
20 public.codeimporteventdata = SELECT, DELETE
21 public.codeimportresult = SELECT, DELETE
22-public.emailaddress = SELECT, UPDATE
23+public.emailaddress = SELECT, UPDATE, DELETE
24 public.hwsubmission = SELECT, UPDATE
25 public.job = SELECT, INSERT, DELETE
26 public.logintoken = SELECT, DELETE
27
28=== modified file 'lib/lp/scripts/garbo.py'
29--- lib/lp/scripts/garbo.py 2011-12-30 06:47:17 +0000
30+++ lib/lp/scripts/garbo.py 2012-01-04 11:57:46 +0000
31@@ -1,4 +1,4 @@
32-# Copyright 2009-2011 Canonical Ltd. This software is licensed under the
33+# Copyright 2009-2012 Canonical Ltd. This software is licensed under the
34 # GNU Affero General Public License version 3 (see the file LICENSE).
35
36 """Database garbage collection."""
37@@ -65,6 +65,7 @@
38 )
39 from lp.services.identity.interfaces.account import AccountStatus
40 from lp.services.identity.interfaces.emailaddress import EmailAddressStatus
41+from lp.services.identity.model.account import Account
42 from lp.services.identity.model.emailaddress import EmailAddress
43 from lp.services.job.model.job import Job
44 from lp.services.librarian.model import TimeLimitedToken
45@@ -310,6 +311,31 @@
46 """
47
48
49+class AccountOnlyEmailAddressPruner(BulkPruner):
50+ """Remove EmailAddress records not linked to a Person."""
51+ target_table_class = EmailAddress
52+ ids_to_prune_query = "SELECT id FROM EmailAddress WHERE person IS NULL"
53+
54+
55+class UnlinkedAccountPruner(BulkPruner):
56+ """Remove Account records not linked to a Person."""
57+ target_table_class = Account
58+ # We join with EmailAddress to ensure we only attempt removal after
59+ # the EmailAddress rows have been removed by
60+ # AccountOnlyEmailAddressPruner. We join with Person to work around
61+ # records with bad crosslinks. These bad crosslinks will be fixed by
62+ # dropping the EmailAddress.account column.
63+ ids_to_prune_query = """
64+ SELECT Account.id
65+ FROM Account
66+ LEFT OUTER JOIN EmailAddress ON Account.id = EmailAddress.account
67+ LEFT OUTER JOIN Person ON Account.id = Person.account
68+ WHERE
69+ EmailAddress.id IS NULL
70+ AND Person.id IS NULL
71+ """
72+
73+
74 class BugSummaryJournalRollup(TunableLoop):
75 """Rollup BugSummaryJournal rows into BugSummary."""
76 maximum_chunk_size = 5000
77@@ -1242,6 +1268,8 @@
78 SuggestiveTemplatesCacheUpdater,
79 POTranslationPruner,
80 UnusedPOTMsgSetPruner,
81+ AccountOnlyEmailAddressPruner,
82+ UnlinkedAccountPruner,
83 ]
84 experimental_tunable_loops = [
85 PersonPruner,
86
87=== modified file 'lib/lp/scripts/tests/test_garbo.py'
88--- lib/lp/scripts/tests/test_garbo.py 2011-12-30 06:47:17 +0000
89+++ lib/lp/scripts/tests/test_garbo.py 2012-01-04 11:57:46 +0000
90@@ -1,4 +1,4 @@
91-# Copyright 2009-2011 Canonical Ltd. This software is licensed under the
92+# Copyright 2009-2012 Canonical Ltd. This software is licensed under the
93 # GNU Affero General Public License version 3 (see the file LICENSE).
94
95 """Test the database garbage collector."""
96@@ -51,10 +51,7 @@
97 )
98 from lp.code.model.codeimportevent import CodeImportEvent
99 from lp.code.model.codeimportresult import CodeImportResult
100-from lp.registry.interfaces.person import (
101- IPersonSet,
102- PersonCreationRationale,
103- )
104+from lp.registry.interfaces.person import IPersonSet
105 from lp.scripts.garbo import (
106 AntiqueSessionPruner,
107 BulkPruner,
108@@ -626,18 +623,14 @@
109 LaunchpadZopelessLayer.switchDbUser('testadmin')
110 rev1 = self.factory.makeRevision('Author 1 <author-1@Example.Org>')
111 rev2 = self.factory.makeRevision('Author 2 <author-2@Example.Org>')
112- rev3 = self.factory.makeRevision('Author 3 <author-3@Example.Org>')
113
114 person1 = self.factory.makePerson(email='Author-1@example.org')
115 person2 = self.factory.makePerson(
116 email='Author-2@example.org',
117 email_address_status=EmailAddressStatus.NEW)
118- account3 = self.factory.makeAccount(
119- 'Author 3', 'Author-3@example.org')
120
121 self.assertEqual(rev1.revision_author.person, None)
122 self.assertEqual(rev2.revision_author.person, None)
123- self.assertEqual(rev3.revision_author.person, None)
124
125 self.runDaily()
126
127@@ -646,7 +639,6 @@
128 LaunchpadZopelessLayer.switchDbUser('testadmin')
129 self.assertEqual(rev1.revision_author.person, person1)
130 self.assertEqual(rev2.revision_author.person, None)
131- self.assertEqual(rev3.revision_author.person, None)
132
133 # Validating an email address creates a linkage.
134 person2.validateAndEnsurePreferredEmail(person2.guessedemails[0])
135@@ -656,33 +648,20 @@
136 LaunchpadZopelessLayer.switchDbUser('testadmin')
137 self.assertEqual(rev2.revision_author.person, person2)
138
139- # Creating a person for an existing account creates a linkage.
140- person3 = account3.createPerson(PersonCreationRationale.UNKNOWN)
141- self.assertEqual(rev3.revision_author.person, None)
142-
143- self.runDaily()
144- LaunchpadZopelessLayer.switchDbUser('testadmin')
145- self.assertEqual(rev3.revision_author.person, person3)
146-
147 def test_HWSubmissionEmailLinker(self):
148 LaunchpadZopelessLayer.switchDbUser('testadmin')
149 sub1 = self.factory.makeHWSubmission(
150 emailaddress='author-1@Example.Org')
151 sub2 = self.factory.makeHWSubmission(
152 emailaddress='author-2@Example.Org')
153- sub3 = self.factory.makeHWSubmission(
154- emailaddress='author-3@Example.Org')
155
156 person1 = self.factory.makePerson(email='Author-1@example.org')
157 person2 = self.factory.makePerson(
158 email='Author-2@example.org',
159 email_address_status=EmailAddressStatus.NEW)
160- account3 = self.factory.makeAccount(
161- 'Author 3', 'Author-3@example.org')
162
163 self.assertEqual(sub1.owner, None)
164 self.assertEqual(sub2.owner, None)
165- self.assertEqual(sub3.owner, None)
166
167 self.runDaily()
168
169@@ -691,7 +670,6 @@
170 LaunchpadZopelessLayer.switchDbUser('testadmin')
171 self.assertEqual(sub1.owner, person1)
172 self.assertEqual(sub2.owner, None)
173- self.assertEqual(sub3.owner, None)
174
175 # Validating an email address creates a linkage.
176 person2.validateAndEnsurePreferredEmail(person2.guessedemails[0])
177@@ -701,14 +679,6 @@
178 LaunchpadZopelessLayer.switchDbUser('testadmin')
179 self.assertEqual(sub2.owner, person2)
180
181- # Creating a person for an existing account creates a linkage.
182- person3 = account3.createPerson(PersonCreationRationale.UNKNOWN)
183- self.assertEqual(sub3.owner, None)
184-
185- self.runDaily()
186- LaunchpadZopelessLayer.switchDbUser('testadmin')
187- self.assertEqual(sub3.owner, person3)
188-
189 def test_PersonPruner(self):
190 personset = getUtility(IPersonSet)
191 # Switch the DB user because the garbo_daily user isn't allowed to