Launchpad itself

Merge ~cjwatson/launchpad:stormify-account into launchpad:master

  1. Git
  2. lp:~cjwatson/launchpad
  3. stormify-account
  4. Merge into master
Proposed by Colin Watson
Status: Merged
Approved by: Colin Watson
Approved revision: 5056ef4fb894065f60b17ddab78af0dc0766c8ed
Merge reported by: Otto Co-Pilot
Merged at revision: not available
Proposed branch: ~cjwatson/launchpad:stormify-account
Merge into: launchpad:master
Diff against target: 445 lines (+77/-42)
19 files modified
lib/lp/registry/doc/person.rst (+1/-1)
lib/lp/registry/interfaces/person.py (+1/-1)
lib/lp/registry/model/mailinglist.py (+3/-3)
lib/lp/registry/model/person.py (+8/-7)
lib/lp/registry/personmerge.py (+1/-1)
lib/lp/registry/security.py (+1/-1)
lib/lp/registry/tests/test_person_merge_job.py (+1/-1)
lib/lp/registry/tests/test_personset.py (+3/-1)
lib/lp/registry/vocabularies.py (+1/-1)
lib/lp/services/auth/tests/test_browser.py (+1/-1)
lib/lp/services/identity/configure.zcml (+1/-1)
lib/lp/services/identity/doc/account.rst (+7/-2)
lib/lp/services/identity/interfaces/account.py (+1/-1)
lib/lp/services/identity/model/account.py (+39/-12)
lib/lp/services/openid/security.py (+1/-1)
lib/lp/services/webapp/authentication.py (+2/-2)
lib/lp/services/webapp/servers.py (+1/-1)
lib/lp/services/webhooks/tests/test_browser.py (+1/-1)
lib/lp/testing/factory.py (+3/-3)
Reviewer Review Type Date Requested Status
Jürgen Gmach Approve
Review via email: mp+446388@code.launchpad.net

Commit message

Convert Account to Storm

To post a comment you must log in.
Revision history for this message
Jürgen Gmach (jugmac00) :
review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1diff --git a/lib/lp/registry/doc/person.rst b/lib/lp/registry/doc/person.rst
2index 593fd24..eb3075a 100644
3--- a/lib/lp/registry/doc/person.rst
4+++ b/lib/lp/registry/doc/person.rst
5@@ -147,7 +147,7 @@ using the createPersonAndEmail() method.
6
7 >>> from lp.services.identity.model.account import Account
8 >>> from lp.services.database.interfaces import IPrimaryStore
9- >>> account = IPrimaryStore(Account).get(Account, p.accountID)
10+ >>> account = IPrimaryStore(Account).get(Account, p.account_id)
11 >>> account.reactivate("Activated by doc test.")
12 >>> p.account_status
13 <DBItem AccountStatus.ACTIVE...
14diff --git a/lib/lp/registry/interfaces/person.py b/lib/lp/registry/interfaces/person.py
15index 383d604..7cdc22a 100644
16--- a/lib/lp/registry/interfaces/person.py
17+++ b/lib/lp/registry/interfaces/person.py
18@@ -849,7 +849,7 @@ class IPersonViewRestricted(
19 """IPerson attributes that require launchpad.View permission."""
20
21 account = Object(schema=IAccount)
22- accountID = Int(title=_("Account ID"), required=True, readonly=True)
23+ account_id = Int(title=_("Account ID"), required=True, readonly=True)
24 karma = exported(
25 Int(
26 title=_("Karma"),
27diff --git a/lib/lp/registry/model/mailinglist.py b/lib/lp/registry/model/mailinglist.py
28index ccc31dd..c47b266 100644
29--- a/lib/lp/registry/model/mailinglist.py
30+++ b/lib/lp/registry/model/mailinglist.py
31@@ -646,7 +646,7 @@ class MailingListSet:
32 tables = (
33 EmailAddress,
34 Join(Person, Person.id == EmailAddress.personID),
35- Join(Account, Account.id == Person.accountID),
36+ Join(Account, Account.id == Person.account_id),
37 Join(TeamParticipation, TeamParticipation.personID == Person.id),
38 Join(
39 MailingListSubscription,
40@@ -701,7 +701,7 @@ class MailingListSet:
41 Team = ClassAlias(Person)
42 tables = (
43 Person,
44- Join(Account, Account.id == Person.accountID),
45+ Join(Account, Account.id == Person.account_id),
46 Join(EmailAddress, EmailAddress.personID == Person.id),
47 Join(TeamParticipation, TeamParticipation.personID == Person.id),
48 Join(MailingList, MailingList.team_id == TeamParticipation.teamID),
49@@ -725,7 +725,7 @@ class MailingListSet:
50 # for three global approvals.
51 tables = (
52 Person,
53- Join(Account, Account.id == Person.accountID),
54+ Join(Account, Account.id == Person.account_id),
55 Join(EmailAddress, EmailAddress.personID == Person.id),
56 Join(MessageApproval, MessageApproval.posted_by_id == Person.id),
57 Join(
58diff --git a/lib/lp/registry/model/person.py b/lib/lp/registry/model/person.py
59index 53e2358..a868a2c 100644
60--- a/lib/lp/registry/model/person.py
61+++ b/lib/lp/registry/model/person.py
62@@ -468,7 +468,8 @@ class Person(
63 _visibility_warning_cache_key = None
64 _visibility_warning_cache = None
65
66- account = ForeignKey(dbName="account", foreignKey="Account", default=None)
67+ account_id = Int(name="account", default=None)
68+ account = Reference(account_id, "Account.id")
69
70 def _validate_name(self, attr, value):
71 """Check that rename is allowed."""
72@@ -716,9 +717,9 @@ class Person(
73 )
74 # Teams don't have Account records
75 if self.account is not None:
76- account_id = self.account.id
77+ account = self.account
78 self.account = None
79- Account.delete(account_id)
80+ Store.of(account).remove(account)
81 self.creation_rationale = None
82 self.teamowner = team_owner
83 alsoProvides(self, ITeam)
84@@ -2245,7 +2246,7 @@ class Person(
85 LeftJoin(
86 account_table,
87 And(
88- person_table.accountID == account_table.id,
89+ person_table.account_id == account_table.id,
90 account_table.status == AccountStatus.ACTIVE,
91 ),
92 )
93@@ -4249,7 +4250,7 @@ class PersonSet:
94 person = Person(
95 name=name,
96 display_name=displayname,
97- accountID=account_id,
98+ account_id=account_id,
99 creation_rationale=rationale,
100 creation_comment=comment,
101 hide_email_addresses=hide_email_addresses,
102@@ -4289,7 +4290,7 @@ class PersonSet:
103
104 def getByAccount(self, account):
105 """See `IPersonSet`."""
106- return Person.selectOne(Person.q.accountID == account.id)
107+ return IStore(Person).find(Person, account_id=account.id).one()
108
109 def updateStatistics(self):
110 """See `IPersonSet`."""
111@@ -5502,7 +5503,7 @@ def _get_recipients_for_team(team):
112 EmailAddress.status == EmailAddressStatus.PREFERRED,
113 ),
114 ),
115- LeftJoin(Account, Person.accountID == Account.id),
116+ LeftJoin(Account, Person.account_id == Account.id),
117 )
118 pending_team_ids = [team.id]
119 recipient_ids = set()
120diff --git a/lib/lp/registry/personmerge.py b/lib/lp/registry/personmerge.py
121index 3318c6b..cec406c 100644
122--- a/lib/lp/registry/personmerge.py
123+++ b/lib/lp/registry/personmerge.py
124@@ -1245,7 +1245,7 @@ def merge_people(from_person, to_person, reviewer, delete=False):
125 """
126 UPDATE OpenIdIdentifier SET account=%s WHERE account=%s
127 """
128- % sqlvalues(to_person.accountID, from_person.accountID)
129+ % sqlvalues(to_person.account_id, from_person.account_id)
130 )
131
132 if delete:
133diff --git a/lib/lp/registry/security.py b/lib/lp/registry/security.py
134index a453edc..03692fe 100644
135--- a/lib/lp/registry/security.py
136+++ b/lib/lp/registry/security.py
137@@ -168,7 +168,7 @@ class EditAccountBySelfOrAdmin(AuthorizationBase):
138 usedfor = IAccount
139
140 def checkAuthenticated(self, user):
141- return user.in_admin or user.person.accountID == self.obj.id
142+ return user.in_admin or user.person.account == self.obj
143
144
145 class ViewAccount(EditAccountBySelfOrAdmin):
146diff --git a/lib/lp/registry/tests/test_person_merge_job.py b/lib/lp/registry/tests/test_person_merge_job.py
147index 6db2384..319ed86 100644
148--- a/lib/lp/registry/tests/test_person_merge_job.py
149+++ b/lib/lp/registry/tests/test_person_merge_job.py
150@@ -49,7 +49,7 @@ def transfer_email(job):
151 """
152 from_email = removeSecurityProxy(job.from_person.preferredemail)
153 from_email.personID = job.to_person.id
154- from_email.accountID = job.to_person.accountID
155+ from_email.account_id = job.to_person.account_id
156 from_email.status = EmailAddressStatus.NEW
157 IStore(from_email).flush()
158
159diff --git a/lib/lp/registry/tests/test_personset.py b/lib/lp/registry/tests/test_personset.py
160index 07acc60..dc8f36f 100644
161--- a/lib/lp/registry/tests/test_personset.py
162+++ b/lib/lp/registry/tests/test_personset.py
163@@ -344,13 +344,15 @@ class TestPersonSetCreateByOpenId(TestCaseWithFactory):
164 )
165
166 def makeAccount(self):
167- return self.store.add(
168+ account = self.store.add(
169 Account(
170 displayname="Displayname",
171 creation_rationale=AccountCreationRationale.UNKNOWN,
172 status=AccountStatus.ACTIVE,
173 )
174 )
175+ self.store.flush()
176+ return account
177
178 def makePerson(self, account):
179 return self.store.add(
180diff --git a/lib/lp/registry/vocabularies.py b/lib/lp/registry/vocabularies.py
181index 9391add..a9acabf 100644
182--- a/lib/lp/registry/vocabularies.py
183+++ b/lib/lp/registry/vocabularies.py
184@@ -694,7 +694,7 @@ class ValidPersonOrTeamVocabulary(
185 SQL("MatchingPerson"),
186 Person,
187 LeftJoin(EmailAddress, EmailAddress.person == Person.id),
188- LeftJoin(Account, Account.id == Person.accountID),
189+ LeftJoin(Account, Account.id == Person.account_id),
190 ]
191 tables.extend(self.extra_tables)
192
193diff --git a/lib/lp/services/auth/tests/test_browser.py b/lib/lp/services/auth/tests/test_browser.py
194index 3d237c0..a1aba38 100644
195--- a/lib/lp/services/auth/tests/test_browser.py
196+++ b/lib/lp/services/auth/tests/test_browser.py
197@@ -54,7 +54,7 @@ class TestAccessTokenViewBase:
198 # in create_view instead, but that approach needs care to avoid
199 # adding an extra query to tests that might be sensitive to that.
200 principal = getUtility(IPlacelessAuthUtility).getPrincipal(
201- self.owner.accountID
202+ self.owner.account_id
203 )
204 view = create_view(
205 self.target,
206diff --git a/lib/lp/services/identity/configure.zcml b/lib/lp/services/identity/configure.zcml
207index 690b52f..66aea4e 100644
208--- a/lib/lp/services/identity/configure.zcml
209+++ b/lib/lp/services/identity/configure.zcml
210@@ -18,7 +18,7 @@
211 person
212 personID
213 account
214- accountID
215+ account_id
216 status
217 rdf_sha1"/>
218 <require
219diff --git a/lib/lp/services/identity/doc/account.rst b/lib/lp/services/identity/doc/account.rst
220index 69a3770..735e890 100644
221--- a/lib/lp/services/identity/doc/account.rst
222+++ b/lib/lp/services/identity/doc/account.rst
223@@ -14,6 +14,7 @@ implements the IAccountSet interface.
224
225 >>> from zope.interface.verify import verifyObject
226 >>> from lp.registry.interfaces.person import IPersonSet
227+ >>> from lp.services.database.interfaces import IStore
228 >>> from lp.services.identity.interfaces.account import (
229 ... IAccount,
230 ... IAccountSet,
231@@ -116,8 +117,12 @@ database. Only an admin can change the status.
232 >>> login("admin@canonical.com")
233 >>> account.setStatus(AccountStatus.SUSPENDED, None, "spammer")
234
235- # Shouldn't be necessary with Storm!
236- >>> removeSecurityProxy(account).sync()
237+date_status_set is maintained by a DB trigger, so we need to flush the
238+status change and force the Account row to be reloaded from the database in
239+order to check that the trigger works.
240+
241+ >>> IStore(account).flush()
242+ >>> IStore(account).autoreload(account)
243 >>> account.date_status_set > original_date_status_set
244 True
245
246diff --git a/lib/lp/services/identity/interfaces/account.py b/lib/lp/services/identity/interfaces/account.py
247index eaff4a7..99615be 100644
248--- a/lib/lp/services/identity/interfaces/account.py
249+++ b/lib/lp/services/identity/interfaces/account.py
250@@ -355,7 +355,7 @@ class AccountStatusChoice(Choice):
251 if not IAccount.providedBy(self.context):
252 # Not an account, eg. validating Person.setAccountStatus.
253 return True
254- if removeSecurityProxy(self.context)._SO_creating:
255+ if removeSecurityProxy(self.context)._creating:
256 # This object is initializing.
257 return True
258 if self.context.status == value:
259diff --git a/lib/lp/services/identity/model/account.py b/lib/lp/services/identity/model/account.py
260index 5412f60..9096b01 100644
261--- a/lib/lp/services/identity/model/account.py
262+++ b/lib/lp/services/identity/model/account.py
263@@ -8,17 +8,15 @@ __all__ = [
264 "AccountSet",
265 ]
266
267-import datetime
268+from datetime import datetime, timezone
269
270-from storm.locals import ReferenceSet
271+from storm.locals import DateTime, Int, ReferenceSet, Unicode
272 from zope.interface import implementer
273
274 from lp.services.database.constants import UTC_NOW
275-from lp.services.database.datetimecol import UtcDateTimeCol
276 from lp.services.database.enumcol import DBEnum
277 from lp.services.database.interfaces import IPrimaryStore, IStore
278-from lp.services.database.sqlbase import SQLBase
279-from lp.services.database.sqlobject import StringCol
280+from lp.services.database.stormbase import StormBase
281 from lp.services.helpers import backslashreplace
282 from lp.services.identity.interfaces.account import (
283 AccountCreationRationale,
284@@ -38,12 +36,21 @@ class AccountStatusDBEnum(DBEnum):
285
286
287 @implementer(IAccount)
288-class Account(SQLBase):
289+class Account(StormBase):
290 """An Account."""
291
292- date_created = UtcDateTimeCol(notNull=True, default=UTC_NOW)
293+ __storm_table__ = "Account"
294
295- displayname = StringCol(dbName="displayname", notNull=True)
296+ id = Int(primary=True)
297+
298+ date_created = DateTime(
299+ name="date_created",
300+ allow_none=False,
301+ default=UTC_NOW,
302+ tzinfo=timezone.utc,
303+ )
304+
305+ displayname = Unicode(name="displayname", allow_none=False)
306
307 creation_rationale = DBEnum(
308 name="creation_rationale",
309@@ -51,15 +58,33 @@ class Account(SQLBase):
310 allow_none=False,
311 )
312 status = AccountStatusDBEnum(
313- enum=AccountStatus, default=AccountStatus.NOACCOUNT, allow_none=False
314+ name="status",
315+ enum=AccountStatus,
316+ default=AccountStatus.NOACCOUNT,
317+ allow_none=False,
318 )
319- date_status_set = UtcDateTimeCol(notNull=True, default=UTC_NOW)
320- status_history = StringCol(dbName="status_comment", default=None)
321+ date_status_set = DateTime(
322+ name="date_status_set",
323+ allow_none=False,
324+ default=UTC_NOW,
325+ tzinfo=timezone.utc,
326+ )
327+ status_history = Unicode(name="status_comment", default=None)
328
329 openid_identifiers = ReferenceSet(
330 "Account.id", OpenIdIdentifier.account_id
331 )
332
333+ _creating = False
334+
335+ def __init__(self, displayname, creation_rationale, status):
336+ super().__init__()
337+ self._creating = True
338+ self.displayname = displayname
339+ self.creation_rationale = creation_rationale
340+ self.status = status
341+ del self._creating
342+
343 def __repr__(self):
344 displayname = backslashreplace(self.displayname)
345 return "<%s '%s' (%s)>" % (
346@@ -70,7 +95,7 @@ class Account(SQLBase):
347
348 def addStatusComment(self, user, comment):
349 """See `IAccountModerateRestricted`."""
350- prefix = datetime.datetime.utcnow().strftime("%Y-%m-%d %H:%M:%S")
351+ prefix = datetime.utcnow().strftime("%Y-%m-%d %H:%M:%S")
352 if user is not None:
353 prefix += " %s" % user.name
354 old_lines = (
355@@ -112,6 +137,8 @@ class AccountSet:
356 creation_rationale=rationale,
357 status=status,
358 )
359+ IStore(Account).add(account)
360+ IStore(Account).flush()
361
362 # Create an OpenIdIdentifier record if requested.
363 if openid_identifier is not None:
364diff --git a/lib/lp/services/openid/security.py b/lib/lp/services/openid/security.py
365index 6c9183e..d794603 100644
366--- a/lib/lp/services/openid/security.py
367+++ b/lib/lp/services/openid/security.py
368@@ -14,4 +14,4 @@ class ViewOpenIdIdentifierBySelfOrAdmin(AuthorizationBase):
369 usedfor = IOpenIdIdentifier
370
371 def checkAuthenticated(self, user):
372- return user.in_admin or user.person.accountID == self.obj.accountID
373+ return user.in_admin or user.person.account_id == self.obj.account_id
374diff --git a/lib/lp/services/webapp/authentication.py b/lib/lp/services/webapp/authentication.py
375index b40d4af..e7c84ad 100644
376--- a/lib/lp/services/webapp/authentication.py
377+++ b/lib/lp/services/webapp/authentication.py
378@@ -88,8 +88,8 @@ class PlacelessAuthUtility:
379 person_id = authdata.get("personid")
380 if person_id is not None:
381 person = getUtility(IPersonSet).get(person_id)
382- if person is not None and person.accountID is not None:
383- id = person.accountID
384+ if person is not None and person.account_id is not None:
385+ id = person.account_id
386
387 if id is None:
388 return None
389diff --git a/lib/lp/services/webapp/servers.py b/lib/lp/services/webapp/servers.py
390index ffed710..53db3f4 100644
391--- a/lib/lp/services/webapp/servers.py
392+++ b/lib/lp/services/webapp/servers.py
393@@ -1244,7 +1244,7 @@ class WebServicePublication(
394 alsoProvides(request, IAccessTokenVerifiedRequest)
395 get_interaction_extras().access_token = access_token
396 return getUtility(IPlacelessLoginSource).getPrincipal(
397- access_token.owner.accountID
398+ access_token.owner.account_id
399 )
400
401 def _getPrincipalFromOAuth(self, request):
402diff --git a/lib/lp/services/webhooks/tests/test_browser.py b/lib/lp/services/webhooks/tests/test_browser.py
403index 04f4654..f8a76a7 100644
404--- a/lib/lp/services/webhooks/tests/test_browser.py
405+++ b/lib/lp/services/webhooks/tests/test_browser.py
406@@ -222,7 +222,7 @@ class WebhookTargetViewTestHelpers:
407 # in create_view instead, but that approach needs care to avoid
408 # adding an extra query to tests that might be sensitive to that.
409 principal = getUtility(IPlacelessAuthUtility).getPrincipal(
410- self.owner.accountID
411+ self.owner.account_id
412 )
413 view = create_view(
414 self.target,
415diff --git a/lib/lp/testing/factory.py b/lib/lp/testing/factory.py
416index 8764715..7329dda 100644
417--- a/lib/lp/testing/factory.py
418+++ b/lib/lp/testing/factory.py
419@@ -681,7 +681,7 @@ class LaunchpadObjectFactory(ObjectFactory):
420 # To make the person someone valid in Launchpad, validate the
421 # email.
422 if email_address_status == EmailAddressStatus.PREFERRED:
423- account = IPrimaryStore(Account).get(Account, person.accountID)
424+ account = IPrimaryStore(Account).get(Account, person.account_id)
425 account.status = AccountStatus.ACTIVE
426 person.setPreferredEmail(email)
427
428@@ -757,7 +757,7 @@ class LaunchpadObjectFactory(ObjectFactory):
429 if set_preferred_email:
430 # setPreferredEmail no longer activates the account
431 # automatically.
432- account = IPrimaryStore(Account).get(Account, person.accountID)
433+ account = IPrimaryStore(Account).get(Account, person.account_id)
434 account.reactivate("Activated by factory.makePersonByName")
435 person.setPreferredEmail(email)
436
437@@ -768,7 +768,7 @@ class LaunchpadObjectFactory(ObjectFactory):
438 person.mailing_list_auto_subscribe_policy = (
439 MailingListAutoSubscribePolicy.NEVER
440 )
441- account = IPrimaryStore(Account).get(Account, person.accountID)
442+ account = IPrimaryStore(Account).get(Account, person.account_id)
443 getUtility(IEmailAddressSet).new(
444 alternative_address, person, EmailAddressStatus.VALIDATED
445 )

Subscribers

People subscribed via source and target branches

to status/vote changes: