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("ad...@canonical.com") >>> 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 )
_______________________________________________ Mailing list: https://launchpad.net/~launchpad-reviewers Post to : launchpad-reviewers@lists.launchpad.net Unsubscribe : https://launchpad.net/~launchpad-reviewers More help : https://help.launchpad.net/ListHelp