launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #30221
[Merge] ~cjwatson/launchpad:stormify-message into launchpad:master
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.bloggs@xxxxxxxxxxx", 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.bar@xxxxxxxxxxxxx")
>>> 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.bar@xxxxxxxxxxxxx")
>>> 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
