launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #30216
[Merge] ~cjwatson/launchpad:stormify-account into launchpad:master
Colin Watson has proposed merging ~cjwatson/launchpad:stormify-account into launchpad:master.
Commit message:
Convert Account to Storm
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/+git/launchpad/+merge/446388
--
Your team Launchpad code reviewers is requested to review the proposed merge of ~cjwatson/launchpad:stormify-account into launchpad:master.
diff --git a/lib/lp/registry/doc/person.rst b/lib/lp/registry/doc/person.rst
index 593fd24..eb3075a 100644
--- a/lib/lp/registry/doc/person.rst
+++ b/lib/lp/registry/doc/person.rst
@@ -147,7 +147,7 @@ using the createPersonAndEmail() method.
>>> from lp.services.identity.model.account import Account
>>> from lp.services.database.interfaces import IPrimaryStore
- >>> account = IPrimaryStore(Account).get(Account, p.accountID)
+ >>> account = IPrimaryStore(Account).get(Account, p.account_id)
>>> account.reactivate("Activated by doc test.")
>>> p.account_status
<DBItem AccountStatus.ACTIVE...
diff --git a/lib/lp/registry/interfaces/person.py b/lib/lp/registry/interfaces/person.py
index 383d604..7cdc22a 100644
--- a/lib/lp/registry/interfaces/person.py
+++ b/lib/lp/registry/interfaces/person.py
@@ -849,7 +849,7 @@ class IPersonViewRestricted(
"""IPerson attributes that require launchpad.View permission."""
account = Object(schema=IAccount)
- accountID = Int(title=_("Account ID"), required=True, readonly=True)
+ account_id = Int(title=_("Account ID"), required=True, readonly=True)
karma = exported(
Int(
title=_("Karma"),
diff --git a/lib/lp/registry/model/mailinglist.py b/lib/lp/registry/model/mailinglist.py
index ccc31dd..c47b266 100644
--- a/lib/lp/registry/model/mailinglist.py
+++ b/lib/lp/registry/model/mailinglist.py
@@ -646,7 +646,7 @@ class MailingListSet:
tables = (
EmailAddress,
Join(Person, Person.id == EmailAddress.personID),
- Join(Account, Account.id == Person.accountID),
+ Join(Account, Account.id == Person.account_id),
Join(TeamParticipation, TeamParticipation.personID == Person.id),
Join(
MailingListSubscription,
@@ -701,7 +701,7 @@ class MailingListSet:
Team = ClassAlias(Person)
tables = (
Person,
- Join(Account, Account.id == Person.accountID),
+ Join(Account, Account.id == Person.account_id),
Join(EmailAddress, EmailAddress.personID == Person.id),
Join(TeamParticipation, TeamParticipation.personID == Person.id),
Join(MailingList, MailingList.team_id == TeamParticipation.teamID),
@@ -725,7 +725,7 @@ class MailingListSet:
# for three global approvals.
tables = (
Person,
- Join(Account, Account.id == Person.accountID),
+ Join(Account, Account.id == Person.account_id),
Join(EmailAddress, EmailAddress.personID == Person.id),
Join(MessageApproval, MessageApproval.posted_by_id == Person.id),
Join(
diff --git a/lib/lp/registry/model/person.py b/lib/lp/registry/model/person.py
index 53e2358..a868a2c 100644
--- a/lib/lp/registry/model/person.py
+++ b/lib/lp/registry/model/person.py
@@ -468,7 +468,8 @@ class Person(
_visibility_warning_cache_key = None
_visibility_warning_cache = None
- account = ForeignKey(dbName="account", foreignKey="Account", default=None)
+ account_id = Int(name="account", default=None)
+ account = Reference(account_id, "Account.id")
def _validate_name(self, attr, value):
"""Check that rename is allowed."""
@@ -716,9 +717,9 @@ class Person(
)
# Teams don't have Account records
if self.account is not None:
- account_id = self.account.id
+ account = self.account
self.account = None
- Account.delete(account_id)
+ Store.of(account).remove(account)
self.creation_rationale = None
self.teamowner = team_owner
alsoProvides(self, ITeam)
@@ -2245,7 +2246,7 @@ class Person(
LeftJoin(
account_table,
And(
- person_table.accountID == account_table.id,
+ person_table.account_id == account_table.id,
account_table.status == AccountStatus.ACTIVE,
),
)
@@ -4249,7 +4250,7 @@ class PersonSet:
person = Person(
name=name,
display_name=displayname,
- accountID=account_id,
+ account_id=account_id,
creation_rationale=rationale,
creation_comment=comment,
hide_email_addresses=hide_email_addresses,
@@ -4289,7 +4290,7 @@ class PersonSet:
def getByAccount(self, account):
"""See `IPersonSet`."""
- return Person.selectOne(Person.q.accountID == account.id)
+ return IStore(Person).find(Person, account_id=account.id).one()
def updateStatistics(self):
"""See `IPersonSet`."""
@@ -5502,7 +5503,7 @@ def _get_recipients_for_team(team):
EmailAddress.status == EmailAddressStatus.PREFERRED,
),
),
- LeftJoin(Account, Person.accountID == Account.id),
+ LeftJoin(Account, Person.account_id == Account.id),
)
pending_team_ids = [team.id]
recipient_ids = set()
diff --git a/lib/lp/registry/personmerge.py b/lib/lp/registry/personmerge.py
index 3318c6b..cec406c 100644
--- a/lib/lp/registry/personmerge.py
+++ b/lib/lp/registry/personmerge.py
@@ -1245,7 +1245,7 @@ def merge_people(from_person, to_person, reviewer, delete=False):
"""
UPDATE OpenIdIdentifier SET account=%s WHERE account=%s
"""
- % sqlvalues(to_person.accountID, from_person.accountID)
+ % sqlvalues(to_person.account_id, from_person.account_id)
)
if delete:
diff --git a/lib/lp/registry/security.py b/lib/lp/registry/security.py
index a453edc..03692fe 100644
--- a/lib/lp/registry/security.py
+++ b/lib/lp/registry/security.py
@@ -168,7 +168,7 @@ class EditAccountBySelfOrAdmin(AuthorizationBase):
usedfor = IAccount
def checkAuthenticated(self, user):
- return user.in_admin or user.person.accountID == self.obj.id
+ return user.in_admin or user.person.account == self.obj
class ViewAccount(EditAccountBySelfOrAdmin):
diff --git a/lib/lp/registry/tests/test_person_merge_job.py b/lib/lp/registry/tests/test_person_merge_job.py
index 6db2384..319ed86 100644
--- a/lib/lp/registry/tests/test_person_merge_job.py
+++ b/lib/lp/registry/tests/test_person_merge_job.py
@@ -49,7 +49,7 @@ def transfer_email(job):
"""
from_email = removeSecurityProxy(job.from_person.preferredemail)
from_email.personID = job.to_person.id
- from_email.accountID = job.to_person.accountID
+ from_email.account_id = job.to_person.account_id
from_email.status = EmailAddressStatus.NEW
IStore(from_email).flush()
diff --git a/lib/lp/registry/tests/test_personset.py b/lib/lp/registry/tests/test_personset.py
index 07acc60..dc8f36f 100644
--- a/lib/lp/registry/tests/test_personset.py
+++ b/lib/lp/registry/tests/test_personset.py
@@ -344,13 +344,15 @@ class TestPersonSetCreateByOpenId(TestCaseWithFactory):
)
def makeAccount(self):
- return self.store.add(
+ account = self.store.add(
Account(
displayname="Displayname",
creation_rationale=AccountCreationRationale.UNKNOWN,
status=AccountStatus.ACTIVE,
)
)
+ self.store.flush()
+ return account
def makePerson(self, account):
return self.store.add(
diff --git a/lib/lp/registry/vocabularies.py b/lib/lp/registry/vocabularies.py
index 9391add..a9acabf 100644
--- a/lib/lp/registry/vocabularies.py
+++ b/lib/lp/registry/vocabularies.py
@@ -694,7 +694,7 @@ class ValidPersonOrTeamVocabulary(
SQL("MatchingPerson"),
Person,
LeftJoin(EmailAddress, EmailAddress.person == Person.id),
- LeftJoin(Account, Account.id == Person.accountID),
+ LeftJoin(Account, Account.id == Person.account_id),
]
tables.extend(self.extra_tables)
diff --git a/lib/lp/services/auth/tests/test_browser.py b/lib/lp/services/auth/tests/test_browser.py
index 3d237c0..a1aba38 100644
--- a/lib/lp/services/auth/tests/test_browser.py
+++ b/lib/lp/services/auth/tests/test_browser.py
@@ -54,7 +54,7 @@ class TestAccessTokenViewBase:
# in create_view instead, but that approach needs care to avoid
# adding an extra query to tests that might be sensitive to that.
principal = getUtility(IPlacelessAuthUtility).getPrincipal(
- self.owner.accountID
+ self.owner.account_id
)
view = create_view(
self.target,
diff --git a/lib/lp/services/identity/configure.zcml b/lib/lp/services/identity/configure.zcml
index 690b52f..66aea4e 100644
--- a/lib/lp/services/identity/configure.zcml
+++ b/lib/lp/services/identity/configure.zcml
@@ -18,7 +18,7 @@
person
personID
account
- accountID
+ account_id
status
rdf_sha1"/>
<require
diff --git a/lib/lp/services/identity/doc/account.rst b/lib/lp/services/identity/doc/account.rst
index 69a3770..735e890 100644
--- a/lib/lp/services/identity/doc/account.rst
+++ b/lib/lp/services/identity/doc/account.rst
@@ -14,6 +14,7 @@ implements the IAccountSet interface.
>>> from zope.interface.verify import verifyObject
>>> from lp.registry.interfaces.person import IPersonSet
+ >>> from lp.services.database.interfaces import IStore
>>> from lp.services.identity.interfaces.account import (
... IAccount,
... IAccountSet,
@@ -116,8 +117,12 @@ database. Only an admin can change the status.
>>> login("admin@xxxxxxxxxxxxx")
>>> account.setStatus(AccountStatus.SUSPENDED, None, "spammer")
- # Shouldn't be necessary with Storm!
- >>> removeSecurityProxy(account).sync()
+date_status_set is maintained by a DB trigger, so we need to flush the
+status change and force the Account row to be reloaded from the database in
+order to check that the trigger works.
+
+ >>> IStore(account).flush()
+ >>> IStore(account).autoreload(account)
>>> account.date_status_set > original_date_status_set
True
diff --git a/lib/lp/services/identity/interfaces/account.py b/lib/lp/services/identity/interfaces/account.py
index eaff4a7..99615be 100644
--- a/lib/lp/services/identity/interfaces/account.py
+++ b/lib/lp/services/identity/interfaces/account.py
@@ -355,7 +355,7 @@ class AccountStatusChoice(Choice):
if not IAccount.providedBy(self.context):
# Not an account, eg. validating Person.setAccountStatus.
return True
- if removeSecurityProxy(self.context)._SO_creating:
+ if removeSecurityProxy(self.context)._creating:
# This object is initializing.
return True
if self.context.status == value:
diff --git a/lib/lp/services/identity/model/account.py b/lib/lp/services/identity/model/account.py
index 5412f60..9096b01 100644
--- a/lib/lp/services/identity/model/account.py
+++ b/lib/lp/services/identity/model/account.py
@@ -8,17 +8,15 @@ __all__ = [
"AccountSet",
]
-import datetime
+from datetime import datetime, timezone
-from storm.locals import ReferenceSet
+from storm.locals import DateTime, Int, ReferenceSet, Unicode
from zope.interface import implementer
from lp.services.database.constants import UTC_NOW
-from lp.services.database.datetimecol import UtcDateTimeCol
from lp.services.database.enumcol import DBEnum
from lp.services.database.interfaces import IPrimaryStore, IStore
-from lp.services.database.sqlbase import SQLBase
-from lp.services.database.sqlobject import StringCol
+from lp.services.database.stormbase import StormBase
from lp.services.helpers import backslashreplace
from lp.services.identity.interfaces.account import (
AccountCreationRationale,
@@ -38,12 +36,21 @@ class AccountStatusDBEnum(DBEnum):
@implementer(IAccount)
-class Account(SQLBase):
+class Account(StormBase):
"""An Account."""
- date_created = UtcDateTimeCol(notNull=True, default=UTC_NOW)
+ __storm_table__ = "Account"
- displayname = StringCol(dbName="displayname", notNull=True)
+ id = Int(primary=True)
+
+ date_created = DateTime(
+ name="date_created",
+ allow_none=False,
+ default=UTC_NOW,
+ tzinfo=timezone.utc,
+ )
+
+ displayname = Unicode(name="displayname", allow_none=False)
creation_rationale = DBEnum(
name="creation_rationale",
@@ -51,15 +58,33 @@ class Account(SQLBase):
allow_none=False,
)
status = AccountStatusDBEnum(
- enum=AccountStatus, default=AccountStatus.NOACCOUNT, allow_none=False
+ name="status",
+ enum=AccountStatus,
+ default=AccountStatus.NOACCOUNT,
+ allow_none=False,
)
- date_status_set = UtcDateTimeCol(notNull=True, default=UTC_NOW)
- status_history = StringCol(dbName="status_comment", default=None)
+ date_status_set = DateTime(
+ name="date_status_set",
+ allow_none=False,
+ default=UTC_NOW,
+ tzinfo=timezone.utc,
+ )
+ status_history = Unicode(name="status_comment", default=None)
openid_identifiers = ReferenceSet(
"Account.id", OpenIdIdentifier.account_id
)
+ _creating = False
+
+ def __init__(self, displayname, creation_rationale, status):
+ super().__init__()
+ self._creating = True
+ self.displayname = displayname
+ self.creation_rationale = creation_rationale
+ self.status = status
+ del self._creating
+
def __repr__(self):
displayname = backslashreplace(self.displayname)
return "<%s '%s' (%s)>" % (
@@ -70,7 +95,7 @@ class Account(SQLBase):
def addStatusComment(self, user, comment):
"""See `IAccountModerateRestricted`."""
- prefix = datetime.datetime.utcnow().strftime("%Y-%m-%d %H:%M:%S")
+ prefix = datetime.utcnow().strftime("%Y-%m-%d %H:%M:%S")
if user is not None:
prefix += " %s" % user.name
old_lines = (
@@ -112,6 +137,8 @@ class AccountSet:
creation_rationale=rationale,
status=status,
)
+ IStore(Account).add(account)
+ IStore(Account).flush()
# Create an OpenIdIdentifier record if requested.
if openid_identifier is not None:
diff --git a/lib/lp/services/openid/security.py b/lib/lp/services/openid/security.py
index 6c9183e..d794603 100644
--- a/lib/lp/services/openid/security.py
+++ b/lib/lp/services/openid/security.py
@@ -14,4 +14,4 @@ class ViewOpenIdIdentifierBySelfOrAdmin(AuthorizationBase):
usedfor = IOpenIdIdentifier
def checkAuthenticated(self, user):
- return user.in_admin or user.person.accountID == self.obj.accountID
+ return user.in_admin or user.person.account_id == self.obj.account_id
diff --git a/lib/lp/services/webapp/authentication.py b/lib/lp/services/webapp/authentication.py
index b40d4af..e7c84ad 100644
--- a/lib/lp/services/webapp/authentication.py
+++ b/lib/lp/services/webapp/authentication.py
@@ -88,8 +88,8 @@ class PlacelessAuthUtility:
person_id = authdata.get("personid")
if person_id is not None:
person = getUtility(IPersonSet).get(person_id)
- if person is not None and person.accountID is not None:
- id = person.accountID
+ if person is not None and person.account_id is not None:
+ id = person.account_id
if id is None:
return None
diff --git a/lib/lp/services/webapp/servers.py b/lib/lp/services/webapp/servers.py
index ffed710..53db3f4 100644
--- a/lib/lp/services/webapp/servers.py
+++ b/lib/lp/services/webapp/servers.py
@@ -1244,7 +1244,7 @@ class WebServicePublication(
alsoProvides(request, IAccessTokenVerifiedRequest)
get_interaction_extras().access_token = access_token
return getUtility(IPlacelessLoginSource).getPrincipal(
- access_token.owner.accountID
+ access_token.owner.account_id
)
def _getPrincipalFromOAuth(self, request):
diff --git a/lib/lp/services/webhooks/tests/test_browser.py b/lib/lp/services/webhooks/tests/test_browser.py
index 04f4654..f8a76a7 100644
--- a/lib/lp/services/webhooks/tests/test_browser.py
+++ b/lib/lp/services/webhooks/tests/test_browser.py
@@ -222,7 +222,7 @@ class WebhookTargetViewTestHelpers:
# in create_view instead, but that approach needs care to avoid
# adding an extra query to tests that might be sensitive to that.
principal = getUtility(IPlacelessAuthUtility).getPrincipal(
- self.owner.accountID
+ self.owner.account_id
)
view = create_view(
self.target,
diff --git a/lib/lp/testing/factory.py b/lib/lp/testing/factory.py
index 8764715..7329dda 100644
--- a/lib/lp/testing/factory.py
+++ b/lib/lp/testing/factory.py
@@ -681,7 +681,7 @@ class LaunchpadObjectFactory(ObjectFactory):
# To make the person someone valid in Launchpad, validate the
# email.
if email_address_status == EmailAddressStatus.PREFERRED:
- account = IPrimaryStore(Account).get(Account, person.accountID)
+ account = IPrimaryStore(Account).get(Account, person.account_id)
account.status = AccountStatus.ACTIVE
person.setPreferredEmail(email)
@@ -757,7 +757,7 @@ class LaunchpadObjectFactory(ObjectFactory):
if set_preferred_email:
# setPreferredEmail no longer activates the account
# automatically.
- account = IPrimaryStore(Account).get(Account, person.accountID)
+ account = IPrimaryStore(Account).get(Account, person.account_id)
account.reactivate("Activated by factory.makePersonByName")
person.setPreferredEmail(email)
@@ -768,7 +768,7 @@ class LaunchpadObjectFactory(ObjectFactory):
person.mailing_list_auto_subscribe_policy = (
MailingListAutoSubscribePolicy.NEVER
)
- account = IPrimaryStore(Account).get(Account, person.accountID)
+ account = IPrimaryStore(Account).get(Account, person.account_id)
getUtility(IEmailAddressSet).new(
alternative_address, person, EmailAddressStatus.VALIDATED
)
