Colin Watson has proposed merging ~cjwatson/launchpad:stormify-message into launchpad:master.
Commit message: Convert Message and MessageChunk to Storm Requested reviews: Launchpad code reviewers (launchpad-reviewers) For more details, see: https://code.launchpad.net/~cjwatson/launchpad/+git/launchpad/+merge/446484 -- Your team Launchpad code reviewers is requested to review the proposed merge of ~cjwatson/launchpad:stormify-message into launchpad:master.
diff --git a/lib/lp/answers/model/question.py b/lib/lp/answers/model/question.py index ccf0e74..2665579 100644 --- a/lib/lp/answers/model/question.py +++ b/lib/lp/answers/model/question.py @@ -1580,7 +1580,7 @@ class QuestionTargetMixin: # messages about the bug. question.createBugLink(bug) # Copy the last message that explains why the bug is a question. - message = bug.messages[-1] + message = bug.messages.last() question.addComment( message.owner, message.text_contents, diff --git a/lib/lp/bugs/doc/bugtask-expiration.rst b/lib/lp/bugs/doc/bugtask-expiration.rst index abaa762..097f573 100644 --- a/lib/lp/bugs/doc/bugtask-expiration.rst +++ b/lib/lp/bugs/doc/bugtask-expiration.rst @@ -616,7 +616,7 @@ primary and replica bugtasks were expired. >>> hoary_bugtask.bug.messages.count() 3 - >>> message = hoary_bugtask.bug.messages[-1] + >>> message = hoary_bugtask.bug.messages.last() >>> print(message.owner.name) janitor diff --git a/lib/lp/bugs/doc/externalbugtracker-comment-imports.rst b/lib/lp/bugs/doc/externalbugtracker-comment-imports.rst index bd9e7b1..a861c4c 100644 --- a/lib/lp/bugs/doc/externalbugtracker-comment-imports.rst +++ b/lib/lp/bugs/doc/externalbugtracker-comment-imports.rst @@ -169,7 +169,7 @@ invalid. >>> joe = getUtility(IPersonSet).getByEmail( ... "joe.blo...@example.com", filter_status=False ... ) - >>> bug.messages[-1].owner == joe + >>> bug.messages.last().owner == joe True >>> print(joe.displayname) @@ -201,7 +201,7 @@ is associated with the existing person. >>> bugwatch_updater.importBugComments() INFO Imported 1 comments for remote bug 123456 on ... - >>> print(bug.messages[-1].owner.name) + >>> print(bug.messages.last().owner.name) no-priv It's also possible for Launchpad to create Persons from remote @@ -220,10 +220,10 @@ used to create a Person based on the displayname alone. >>> bugwatch_updater.importBugComments() INFO Imported 1 comments for remote bug 123456 on ... - >>> print(bug.messages[-1].owner.name) + >>> print(bug.messages.last().owner.name) noemail-bugzilla-checkwatches-1 - >>> print(bug.messages[-1].owner.preferredemail) + >>> print(bug.messages.last().owner.preferredemail) None A BugTrackerPerson record will have been created to map the new Person @@ -248,7 +248,7 @@ imported. or display name found. (OOPS-...) INFO Imported 0 comments for remote bug 123456 on ... - >>> print(bug.messages[-1].text_contents) + >>> print(bug.messages.last().text_contents) Yet another comment. Let's delete that comment now so that it doesn't break later tests. diff --git a/lib/lp/bugs/model/bug.py b/lib/lp/bugs/model/bug.py index 4ff9c08..dc2d66b 100644 --- a/lib/lp/bugs/model/bug.py +++ b/lib/lp/bugs/model/bug.py @@ -181,7 +181,6 @@ from lp.services.database.sqlobject import ( IntCol, SQLMultipleJoin, SQLObjectNotFound, - SQLRelatedJoin, StringCol, ) from lp.services.database.stormbase import StormBase @@ -390,14 +389,6 @@ class Bug(SQLBase, InformationTypeMixin): # useful Joins activity = ReferenceSet("id", BugActivity.bug_id, order_by=BugActivity.id) - messages = SQLRelatedJoin( - "Message", - joinColumn="bug_id", - otherColumn="message_id", - intermediateTable="BugMessage", - prejoins=["owner"], - orderBy=["datecreated", "id"], - ) bug_messages = ReferenceSet( "id", BugMessage.bug_id, order_by=BugMessage.index ) @@ -525,6 +516,21 @@ class Bug(SQLBase, InformationTypeMixin): ) @property + def messages(self): + """See `IBug`.""" + return DecoratedResultSet( + IStore(Message) + .find( + (Message, Person), + BugMessage.bug == self, + BugMessage.message_id == Message.id, + Message.owner_id == Person.id, + ) + .order_by(Message.datecreated, Message.id), + result_decorator=operator.itemgetter(0), + ) + + @property def security_related(self): return self.information_type in SECURITY_INFORMATION_TYPES @@ -679,7 +685,7 @@ class Bug(SQLBase, InformationTypeMixin): # in storm with very large bugs by not joining and instead # querying a second time. If this starts to show high db # time, we can left outer join instead. - owner_ids = {message.ownerID for message in messages} + owner_ids = {message.owner_id for message in messages} owner_ids.discard(None) if not owner_ids: return @@ -692,12 +698,12 @@ class Bug(SQLBase, InformationTypeMixin): # for the message content. message_ids = {message.id for message in messages} chunks = store.find( - MessageChunk, MessageChunk.messageID.is_in(message_ids) + MessageChunk, MessageChunk.message_id.is_in(message_ids) ) chunks.order_by(MessageChunk.id) chunk_map = {} for chunk in chunks: - message_chunks = chunk_map.setdefault(chunk.messageID, []) + message_chunks = chunk_map.setdefault(chunk.message_id, []) message_chunks.append(chunk) for message in messages: if message.id not in chunk_map: @@ -1546,7 +1552,7 @@ class Bug(SQLBase, InformationTypeMixin): self, message, bugwatch=None, user=None, remote_comment_id=None ): """See `IBug`.""" - if message not in self.messages: + if IStore(self).find(BugMessage, bug=self, message=message).is_empty(): if user is None: user = message.owner result = BugMessage( @@ -1953,7 +1959,7 @@ class Bug(SQLBase, InformationTypeMixin): # Since "soft-deleted" messages will have 0 chunks, we should use # left join here. message_join = LeftJoin( - Message, MessageChunk, Message.id == MessageChunk.messageID + Message, MessageChunk, Message.id == MessageChunk.message_id ) query = Store.of(self).using(BugMessage, message_join) result = query.find( @@ -1967,7 +1973,7 @@ class Bug(SQLBase, InformationTypeMixin): def eager_load_owners(rows): owners = set() for row in rows: - owners.add(row[1].ownerID) + owners.add(row[1].owner_id) owners.discard(None) if not owners: return @@ -3300,10 +3306,7 @@ class BugSet: datecreated=params.datecreated, ) MessageChunk( - message=params.msg, - sequence=1, - content=params.comment, - blob=None, + message=params.msg, sequence=1, content=params.comment ) # Extract the details needed to create the bug and optional msg. diff --git a/lib/lp/bugs/model/bugnotification.py b/lib/lp/bugs/model/bugnotification.py index 7078cda..1eb6fcc 100644 --- a/lib/lp/bugs/model/bugnotification.py +++ b/lib/lp/bugs/model/bugnotification.py @@ -151,8 +151,8 @@ class BugNotificationSet: last_omitted_notification = notification elif ( last_omitted_notification is not None - and notification.message.ownerID - == last_omitted_notification.message.ownerID + and notification.message.owner_id + == last_omitted_notification.message.owner_id and notification.bug_id == last_omitted_notification.bug_id and last_omitted_notification.message.datecreated - notification.message.datecreated @@ -162,7 +162,7 @@ class BugNotificationSet: if last_omitted_notification != notification: last_omitted_notification = None pending_notifications.append(notification) - people_ids.add(notification.message.ownerID) + people_ids.add(notification.message.owner_id) bug_ids.add(notification.bug_id) # Now we do some calls that are purely for caching. # Converting these into lists forces the queries to execute. diff --git a/lib/lp/bugs/model/tests/test_bugtask.py b/lib/lp/bugs/model/tests/test_bugtask.py index 238d3f9..cca4282 100644 --- a/lib/lp/bugs/model/tests/test_bugtask.py +++ b/lib/lp/bugs/model/tests/test_bugtask.py @@ -2566,7 +2566,7 @@ class TestAutoConfirmBugTasks(TestCaseWithFactory): self.assertEqual( "Status changed to 'Confirmed' because the bug affects " "multiple users.", - bug.messages[-1].text_contents, + bug.messages.last().text_contents, ) def test_do_not_confirm_bugwatch_tasks(self): diff --git a/lib/lp/bugs/stories/bugs/xx-bug-hidden-comments.rst b/lib/lp/bugs/stories/bugs/xx-bug-hidden-comments.rst index f0323c9..3ccfe3c 100644 --- a/lib/lp/bugs/stories/bugs/xx-bug-hidden-comments.rst +++ b/lib/lp/bugs/stories/bugs/xx-bug-hidden-comments.rst @@ -25,7 +25,7 @@ is now set to be hidden. >>> from lp.bugs.interfaces.bugmessage import IBugMessageSet >>> login("foo....@canonical.com") >>> bug_11 = getUtility(IBugSet).get(11) - >>> message = bug_11.messages[-1] + >>> message = bug_11.messages.last() >>> print(message.text_contents) This comment will not be visible when the test completes. >>> bug_message = getUtility(IBugMessageSet).getByBugAndMessage( diff --git a/lib/lp/bugs/stories/bugs/xx-remote-bug-comments.rst b/lib/lp/bugs/stories/bugs/xx-remote-bug-comments.rst index cd7e997..a1ad3c1 100644 --- a/lib/lp/bugs/stories/bugs/xx-remote-bug-comments.rst +++ b/lib/lp/bugs/stories/bugs/xx-remote-bug-comments.rst @@ -86,7 +86,7 @@ the 'awaiting synchronization' mark goes away. >>> from lp.bugs.interfaces.bugmessage import IBugMessageSet >>> login("foo....@canonical.com") >>> bug_15 = getUtility(IBugSet).get(15) - >>> message = bug_15.messages[-1] + >>> message = bug_15.messages.last() >>> print(message.text_contents) A reply comment. >>> bug_message = getUtility(IBugMessageSet).getByBugAndMessage( diff --git a/lib/lp/bugs/tests/bugtarget-questiontarget.rst b/lib/lp/bugs/tests/bugtarget-questiontarget.rst index e70219b..28ba61b 100644 --- a/lib/lp/bugs/tests/bugtarget-questiontarget.rst +++ b/lib/lp/bugs/tests/bugtarget-questiontarget.rst @@ -105,7 +105,9 @@ bug. >>> len(question.messages) == bug.messages.count() - 1 True - >>> question.messages[-1].text_contents == bug.messages[-1].text_contents + >>> question.messages[ + ... -1 + ... ].text_contents == bug.messages.last().text_contents True >>> print(question.messages[-1].text_contents) diff --git a/lib/lp/registry/model/distroseriesdifference.py b/lib/lp/registry/model/distroseriesdifference.py index 030e521..23602af 100644 --- a/lib/lp/registry/model/distroseriesdifference.py +++ b/lib/lp/registry/model/distroseriesdifference.py @@ -220,12 +220,12 @@ def message_chunks(messages): """ store = IStore(MessageChunk) chunks = store.find( - MessageChunk, MessageChunk.messageID.is_in(m.id for m in messages) + MessageChunk, MessageChunk.message_id.is_in(m.id for m in messages) ) grouped = defaultdict(list) for chunk in chunks: - grouped[chunk.messageID].append(chunk) + grouped[chunk.message_id].append(chunk) return grouped @@ -332,7 +332,7 @@ def eager_load_dsds(dsds): # Load DistroSeriesDifferenceComment owners, SourcePackageRecipeBuild # requesters, GPGKey owners, and SourcePackageRelease creators. person_ids = set().union( - (dsdc.message.ownerID for dsdc in latest_comments), + (dsdc.message.owner_id for dsdc in latest_comments), (sprb.requester_id for sprb in sprbs), (spr.creatorID for spr in sprs), (spr.signing_key_owner_id for spr in sprs), diff --git a/lib/lp/services/messages/configure.zcml b/lib/lp/services/messages/configure.zcml index e84df12..2dd4713 100644 --- a/lib/lp/services/messages/configure.zcml +++ b/lib/lp/services/messages/configure.zcml @@ -39,7 +39,7 @@ interface="lp.services.messages.interfaces.messagerevision.IMessageRevisionEdit"/> </class> - <!-- MessageChunk --> + <!-- MessageRevisionChunk --> <class class="lp.services.messages.model.messagerevision.MessageRevisionChunk"> <allow diff --git a/lib/lp/services/messages/model/message.py b/lib/lp/services/messages/model/message.py index 8f4cc26..ade6a5d 100644 --- a/lib/lp/services/messages/model/message.py +++ b/lib/lp/services/messages/model/message.py @@ -22,6 +22,7 @@ import six from lazr.config import as_timedelta from storm.locals import ( And, + Bool, DateTime, Int, Max, @@ -41,17 +42,8 @@ from lp.registry.interfaces.person import ( validate_public_person, ) from lp.services.config import config -from lp.services.database.constants import UTC_NOW -from lp.services.database.datetimecol import UtcDateTimeCol -from lp.services.database.sqlbase import SQLBase -from lp.services.database.sqlobject import ( - BoolCol, - ForeignKey, - IntCol, - SQLMultipleJoin, - SQLRelatedJoin, - StringCol, -) +from lp.services.database.constants import DEFAULT, UTC_NOW +from lp.services.database.interfaces import IStore from lp.services.database.stormbase import StormBase from lp.services.librarian.interfaces import ILibraryFileAliasSet from lp.services.messages.interfaces.message import ( @@ -88,49 +80,64 @@ def utcdatetime_from_field(field_value): @implementer(IMessage) -class Message(SQLBase): +class Message(StormBase): """A message. This is an RFC822-style message, typically it would be coming into the bug system, or coming in from a mailing list. """ - _table = "Message" - _defaultOrder = "-id" - datecreated = UtcDateTimeCol(notNull=True, default=UTC_NOW) - date_deleted = UtcDateTimeCol(notNull=False, default=None) - date_last_edited = UtcDateTimeCol(notNull=False, default=None) - subject = StringCol(notNull=False, default=None) - owner = ForeignKey( - dbName="owner", - foreignKey="Person", - storm_validator=validate_public_person, - notNull=False, + __storm_table__ = "Message" + __storm_order__ = "-id" + id = Int(primary=True) + datecreated = DateTime( + allow_none=False, default=UTC_NOW, tzinfo=timezone.utc ) - parent = ForeignKey( - foreignKey="Message", dbName="parent", notNull=False, default=None + date_deleted = DateTime(allow_none=True, default=None, tzinfo=timezone.utc) + date_last_edited = DateTime( + allow_none=True, default=None, tzinfo=timezone.utc ) - rfc822msgid = StringCol(notNull=True) - bugs = SQLRelatedJoin( - "Bug", - joinColumn="message_id", - otherColumn="bug_id", - intermediateTable="BugMessage", + subject = Unicode(allow_none=True, default=None) + owner_id = Int( + name="owner", validator=validate_public_person, allow_none=True ) - _chunks = SQLMultipleJoin("MessageChunk", joinColumn="message") + owner = Reference(owner_id, "Person.id") + parent_id = Int(name="parent", allow_none=True, default=None) + parent = Reference(parent_id, "Message.id") + rfc822msgid = Unicode(allow_none=False) + bugs = ReferenceSet( + "id", "BugMessage.message_id", "BugMessage.bug_id", "Bug.id" + ) + _chunks = ReferenceSet("id", "MessageChunk.message_id") @cachedproperty def chunks(self): return list(self._chunks) - raw = ForeignKey(foreignKey="LibraryFileAlias", dbName="raw", default=None) - _bugattachments = ReferenceSet( - "<primary key>", "BugAttachment._message_id" - ) + raw_id = Int(name="raw", default=None) + raw = Reference(raw_id, "LibraryFileAlias.id") + _bugattachments = ReferenceSet("id", "BugAttachment._message_id") @cachedproperty def bugattachments(self): return list(self._bugattachments) - visible = BoolCol(notNull=True, default=True) + visible = Bool(allow_none=False, default=True) + + def __init__( + self, + rfc822msgid, + datecreated=DEFAULT, + subject=None, + owner=None, + parent=None, + raw=None, + ): + super().__init__() + self.rfc822msgid = rfc822msgid + self.datecreated = datecreated + self.subject = subject + self.owner = owner + self.parent = parent + self.raw = raw def __repr__(self): return "<Message id=%s>" % self.id @@ -293,7 +300,7 @@ class MessageSet: } def get(self, rfc822msgid): - messages = list(Message.selectBy(rfc822msgid=rfc822msgid)) + messages = list(IStore(Message).find(Message, rfc822msgid=rfc822msgid)) if len(messages) == 0: raise NotFoundError(rfc822msgid) return messages @@ -313,6 +320,7 @@ class MessageSet: owner=owner, datecreated=datecreated, ) + IStore(Message).add(message) MessageChunk(message=message, sequence=1, content=content) # XXX 2008-05-27 jamesh: # Ensure that BugMessages get flushed in same order as they @@ -608,24 +616,30 @@ class MessageSet: @implementer(IMessageChunk) -class MessageChunk(SQLBase): +class MessageChunk(StormBase): """One part of a possibly multipart Message""" - _table = "MessageChunk" - _defaultOrder = "sequence" + __storm_table__ = "MessageChunk" + __storm_order__ = "sequence" - message = ForeignKey(foreignKey="Message", dbName="message", notNull=True) + id = Int(primary=True) - sequence = IntCol(notNull=True) + message_id = Int(name="message", allow_none=False) + message = Reference(message_id, "Message.id") - content = StringCol(notNull=False, default=None) + sequence = Int(allow_none=False) - blob = ForeignKey( - foreignKey="LibraryFileAlias", - dbName="blob", - notNull=False, - default=None, - ) + content = Unicode(allow_none=True, default=None) + + blob_id = Int(name="blob", allow_none=True, default=None) + blob = Reference(blob_id, "LibraryFileAlias.id") + + def __init__(self, message, sequence, content=None, blob=None): + super().__init__() + self.message = message + self.sequence = sequence + self.content = content + self.blob = blob def __str__(self): """Return a text representation of this chunk. diff --git a/lib/lp/soyuz/doc/closing-bugs-from-changelogs.rst b/lib/lp/soyuz/doc/closing-bugs-from-changelogs.rst index 3b2c5cc..8a20b2b 100644 --- a/lib/lp/soyuz/doc/closing-bugs-from-changelogs.rst +++ b/lib/lp/soyuz/doc/closing-bugs-from-changelogs.rst @@ -122,7 +122,7 @@ added as a comment from the janitor. >>> switch_dbuser("launchpad") >>> pmount_bug = getUtility(IBugSet).get(pmount_bug_id) - >>> last_comment = pmount_bug.messages[-1] + >>> last_comment = pmount_bug.messages.last() >>> print(pmount_release.creator.displayname) Mark Shuttleworth >>> print(last_comment.owner.displayname) diff --git a/lib/lp/testing/factory.py b/lib/lp/testing/factory.py index 7329dda..ea6937e 100644 --- a/lib/lp/testing/factory.py +++ b/lib/lp/testing/factory.py @@ -4569,7 +4569,7 @@ class LaunchpadObjectFactory(ObjectFactory): `Message` table already. """ msg_id = make_msgid("launchpad") - while not Message.selectBy(rfc822msgid=msg_id).is_empty(): + while not IStore(Message).find(Message, rfc822msgid=msg_id).is_empty(): msg_id = make_msgid("launchpad") return msg_id
_______________________________________________ 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