launchpad-reviewers team mailing list archive

["Thiago F. Pappacena" <pappacena@xxxxxxxxx>]

Thiago F. Pappacena has proposed merging ~pappacena/launchpad:stormify-bugmessage into launchpad:master.

Commit message:
Stormify BugMessage

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~pappacena/launchpad/+git/launchpad/+merge/394993
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~pappacena/launchpad:stormify-bugmessage into launchpad:master.

diff --git a/lib/lp/bugs/browser/bugcomment.py b/lib/lp/bugs/browser/bugcomment.py
index 56f33e3..2b61552 100644
--- a/lib/lp/bugs/browser/bugcomment.py
+++ b/lib/lp/bugs/browser/bugcomment.py
@@ -1,4 +1,4 @@
-# Copyright 2006-2012 Canonical Ltd.  This software is licensed under the
+# Copyright 2006-2020 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Bug comment browser view classes."""
@@ -90,7 +90,7 @@ def build_comments_from_chunks(
             # has already loaded all the bug watches. If we start lazy loading
             # those, or not needing them we will need to batch lookup watches
             # here.
-            if bugmessage.bugwatchID is not None:
+            if bugmessage.bugwatch_id is not None:
                 bug_comment.bugwatch = bugmessage.bugwatch
                 bug_comment.synchronized = (
                     bugmessage.remote_comment_id is not None)
diff --git a/lib/lp/bugs/browser/tests/test_bugtarget_filebug.py b/lib/lp/bugs/browser/tests/test_bugtarget_filebug.py
index d3efbe2..77bc21c 100644
--- a/lib/lp/bugs/browser/tests/test_bugtarget_filebug.py
+++ b/lib/lp/bugs/browser/tests/test_bugtarget_filebug.py
@@ -1,4 +1,4 @@
-# Copyright 2010-2019 Canonical Ltd.  This software is licensed under the
+# Copyright 2010-2020 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 from __future__ import absolute_import, print_function, unicode_literals
@@ -588,7 +588,7 @@ class FileBugViewBaseExtraDataTestCase(FileBugViewMixin, TestCaseWithFactory):
             'Added to the description.',
             bug.description)
         self.assertEqual(2, bug.messages.count())
-        self.assertEqual(bug.bug_messages[-1], message_event.object)
+        self.assertEqual(list(bug.bug_messages)[-1], message_event.object)
         notifications = [
             no.message for no in view.request.response.notifications]
         self.assertContentEqual(
diff --git a/lib/lp/bugs/browser/tests/test_bugwatch_views.py b/lib/lp/bugs/browser/tests/test_bugwatch_views.py
index 8eedb3b..7cd8393 100644
--- a/lib/lp/bugs/browser/tests/test_bugwatch_views.py
+++ b/lib/lp/bugs/browser/tests/test_bugwatch_views.py
@@ -1,8 +1,10 @@
-# Copyright 2010-2013 Canonical Ltd.  This software is licensed under the
+# Copyright 2010-2020 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Tests for BugWatch views."""
 
+from __future__ import absolute_import, print_function, unicode_literals
+
 __metaclass__ = type
 
 from zope.component import getUtility
diff --git a/lib/lp/bugs/doc/bugmessage.txt b/lib/lp/bugs/doc/bugmessage.txt
index ac47389..f563143 100644
--- a/lib/lp/bugs/doc/bugmessage.txt
+++ b/lib/lp/bugs/doc/bugmessage.txt
@@ -118,7 +118,7 @@ Note that although the watch was created when the Message was added to
 the bug, the message and the watch are not linked because the message
 was not imported by the bug watch.
 
-    >>> bug_message = bug_one.bug_messages[-1]
+    >>> bug_message = list(bug_one.bug_messages)[-1]
     >>> print(bug_message.message == test_message)
     True
     >>> print(bug_message.bugwatch)
@@ -176,9 +176,9 @@ to compare this value for every bug in a large set.
 
 == Retrieving IMessage.id from IBugMessage ==
 
-Each IBugMessage has a messageID attribute, which allows access
+Each IBugMessage has a message_id attribute, which allows access
 to IBugMessage.IMessage.id without the additional query.
 
     >>> bugmessage_one = bugmessageset.get(1)
-    >>> bugmessage_one.messageID == bugmessage_one.message.id
+    >>> bugmessage_one.message_id == bugmessage_one.message.id
     True
diff --git a/lib/lp/bugs/doc/externalbugtracker-comment-pushing.txt b/lib/lp/bugs/doc/externalbugtracker-comment-pushing.txt
index 48edb29..dbc1c6a 100644
--- a/lib/lp/bugs/doc/externalbugtracker-comment-pushing.txt
+++ b/lib/lp/bugs/doc/externalbugtracker-comment-pushing.txt
@@ -6,6 +6,7 @@ to the remote bug tracker.
 In order to demonstrate this we need to create example Bug, BugTracker,
 BugWatch, Message and BugMessage instances with which to work.
 
+    >>> import six
     >>> from zope.interface import implementer
     >>> from lp.bugs.tests.externalbugtracker import (
     ...     new_bugtracker)
@@ -55,7 +56,7 @@ ISupportsCommentPushing interface.
     ...     remote_comments = {}
     ...
     ...     def addRemoteComment(self, remote_bug, comment_body, rfc822msgid):
-    ...         remote_comment_id = str(self.next_comment_id)
+    ...         remote_comment_id = six.ensure_text(str(self.next_comment_id))
     ...         self.remote_comments[remote_comment_id] = comment_body
     ...
     ...         print("Comment added as remote comment %s" % (
diff --git a/lib/lp/bugs/interfaces/bugmessage.py b/lib/lp/bugs/interfaces/bugmessage.py
index a71fca2..98c18f3 100644
--- a/lib/lp/bugs/interfaces/bugmessage.py
+++ b/lib/lp/bugs/interfaces/bugmessage.py
@@ -1,4 +1,4 @@
-# Copyright 2004-2012 Canonical Ltd.  This software is licensed under the
+# Copyright 2004-2020 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Bug message interfaces."""
@@ -45,14 +45,14 @@ class IBugMessage(IHasBug):
     # change it, rather than arbitrary API scripts.
     index = Int(title=u'The comment number', required=False, readonly=False,
         default=None)
-    messageID = Int(title=u"The message id.", readonly=True)
+    message_id = Int(title=u"The message id.", readonly=True)
     message = Object(schema=IMessage, title=u"The message.")
     bugwatch = Object(schema=IBugWatch,
         title=u"A bugwatch to which the message pertains.")
-    bugwatchID = Int(title=u'The bugwatch id.', readonly=True)
+    bugwatch_id = Int(title=u'The bugwatch id.', readonly=True)
     remote_comment_id = TextLine(
         title=u"The id this comment has in the bugwatch's bug tracker.")
-    ownerID = Attribute("The ID of the owner mirrored from the message")
+    owner_id = Attribute("The ID of the owner mirrored from the message")
     owner = Object(schema=IPerson,
         title=u"The Message owner mirrored from the message.", readonly=True)
 
diff --git a/lib/lp/bugs/model/bug.py b/lib/lp/bugs/model/bug.py
index b20d9bc..0ca6164 100644
--- a/lib/lp/bugs/model/bug.py
+++ b/lib/lp/bugs/model/bug.py
@@ -368,13 +368,13 @@ class Bug(SQLBase, InformationTypeMixin):
 
     # useful Joins
     activity = SQLMultipleJoin('BugActivity', joinColumn='bug', orderBy='id')
-    messages = SQLRelatedJoin('Message', joinColumn='bug',
-                           otherColumn='message',
+    messages = SQLRelatedJoin('Message', joinColumn='bug_id',
+                           otherColumn='message_id',
                            intermediateTable='BugMessage',
                            prejoins=['owner'],
                            orderBy=['datecreated', 'id'])
-    bug_messages = SQLMultipleJoin(
-        'BugMessage', joinColumn='bug', orderBy='index')
+    bug_messages = ReferenceSet(
+        'id', BugMessage.bug_id, order_by=BugMessage.index)
     watches = SQLMultipleJoin(
         'BugWatch', joinColumn='bug', orderBy=['bugtracker', 'remotebug'])
     duplicates = SQLMultipleJoin('Bug', joinColumn='duplicateof', orderBy='id')
@@ -617,25 +617,25 @@ class Bug(SQLBase, InformationTypeMixin):
                 Message,
                 Join(
                     BugMessage,
-                    BugMessage.messageID == Message.id),
+                    BugMessage.message_id == Message.id),
                 LeftJoin(
                     Join(
                         ParentMessage,
                         ParentBugMessage,
-                        ParentMessage.id == ParentBugMessage.messageID),
+                        ParentMessage.id == ParentBugMessage.message_id),
                     And(
                         Message.parent == ParentMessage.id,
-                        ParentBugMessage.bugID == self.id)),
+                        ParentBugMessage.bug_id == self.id)),
                 ]
             results = store.using(*tables).find(
                 (Message, ParentMessage, BugMessage),
-                BugMessage.bugID == self.id,
+                BugMessage.bug_id == self.id,
                 )
         else:
             lookup = Message, BugMessage
             results = store.find(lookup,
-                BugMessage.bugID == self.id,
-                BugMessage.messageID == Message.id,
+                BugMessage.bug_id == self.id,
+                BugMessage.message_id == Message.id,
                 )
         results.order_by(BugMessage.index)
         return DecoratedResultSet(results, index_message,
@@ -1578,7 +1578,7 @@ class Bug(SQLBase, InformationTypeMixin):
         # query seems fine as we have to join out from bugmessage anyway.
         result = Store.of(self).find((BugMessage, Message, MessageChunk),
             Message.id == MessageChunk.messageID,
-            BugMessage.messageID == Message.id,
+            BugMessage.message_id == Message.id,
             BugMessage.bug == self.id, *ranges)
         result.order_by(BugMessage.index, MessageChunk.sequence)
 
diff --git a/lib/lp/bugs/model/bugmessage.py b/lib/lp/bugs/model/bugmessage.py
index 653f893..6da4d70 100644
--- a/lib/lp/bugs/model/bugmessage.py
+++ b/lib/lp/bugs/model/bugmessage.py
@@ -1,4 +1,4 @@
-# Copyright 2009 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2020 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 __metaclass__ = type
@@ -6,11 +6,12 @@ __all__ = ['BugMessage', 'BugMessageSet']
 
 from email.utils import make_msgid
 
-from sqlobject import (
-    ForeignKey,
-    IntCol,
-    StringCol,
+import six
+from storm.properties import (
+    Int,
+    Unicode,
     )
+from storm.references import Reference
 from storm.store import Store
 from zope.interface import implementer
 
@@ -19,10 +20,8 @@ from lp.bugs.interfaces.bugmessage import (
     IBugMessageSet,
     )
 from lp.registry.interfaces.person import validate_public_person
-from lp.services.database.sqlbase import (
-    SQLBase,
-    sqlvalues,
-    )
+from lp.services.database.interfaces import IStore
+from lp.services.database.stormbase import StormBase
 from lp.services.messages.model.message import (
     Message,
     MessageChunk,
@@ -30,30 +29,46 @@ from lp.services.messages.model.message import (
 
 
 @implementer(IBugMessage)
-class BugMessage(SQLBase):
+class BugMessage(StormBase):
     """A table linking bugs and messages."""
 
-    _table = 'BugMessage'
-
-    def __init__(self, *args, **kw):
-        # This is maintained by triggers to ensure validity, but we
-        # also set it here to ensure it is visible to the transaction
-        # creating a BugMessage.
-        kw['owner'] = owner = kw['message'].owner
-        assert owner is not None, "BugMessage's Message must have an owner"
-        super(BugMessage, self).__init__(*args, **kw)
+    __storm_table__ = 'BugMessage'
 
     # db field names
-    bug = ForeignKey(dbName='bug', foreignKey='Bug', notNull=True)
-    message = ForeignKey(dbName='message', foreignKey='Message', notNull=True)
-    bugwatch = ForeignKey(dbName='bugwatch', foreignKey='BugWatch',
-        notNull=False, default=None)
-    remote_comment_id = StringCol(notNull=False, default=None)
+    id = Int(primary=True)
+
+    bug_id = Int(name='bug', allow_none=False)
+    bug = Reference(bug_id, 'Bug.id')
+
+    message_id = Int(name='message', allow_none=False)
+    message = Reference(message_id, 'Message.id')
+
+    bugwatch_id = Int(name='bugwatch', allow_none=True, default=None)
+    bugwatch = Reference(bugwatch_id, 'BugWatch.id')
+
+    remote_comment_id = Unicode(allow_none=True, default=None)
     # -- The index of the message is cached in the DB.
-    index = IntCol(notNull=True)
+    index = Int(name='index', allow_none=False)
     # -- The owner, cached from the message table using triggers.
-    owner = ForeignKey(dbName='owner', foreignKey='Person',
-        storm_validator=validate_public_person, notNull=True)
+    owner_id = Int(
+        name="owner", allow_none=False, validator=validate_public_person)
+    owner = Reference(owner_id, "Person.id")
+
+    def __init__(self, owner=None, index=0, message=None, bug=None,
+                 bugwatch=None, remote_comment_id=None):
+        # This is maintained by triggers to ensure validity, but we
+        # also set it here to ensure it is visible to the transaction
+        # creating a BugMessage.
+        self.owner = message.owner
+        assert self.owner is not None, (
+            "BugMessage's Message must have an owner")
+        self.index = index
+        self.message = message
+        self.bug = bug
+        self.remote_comment_id = (
+            six.ensure_text(remote_comment_id) if remote_comment_id is not None
+            else None)
+        self.bugwatch = bugwatch
 
     def __repr__(self):
         return "<BugMessage at 0x%x message=%s index=%s>" % (
@@ -81,15 +96,20 @@ class BugMessageSet:
 
     def get(self, bugmessageid):
         """See `IBugMessageSet`."""
-        return BugMessage.get(bugmessageid)
+        store = IStore(BugMessage)
+        return store.find(BugMessage, BugMessage.id == bugmessageid).one()
 
     def getByBugAndMessage(self, bug, message):
         """See`IBugMessageSet`."""
-        return BugMessage.selectOneBy(bug=bug, message=message)
+        store = IStore(BugMessage)
+        return store.find(
+            BugMessage,
+            BugMessage.bug == bug, BugMessage.message == message).one()
 
     def getImportedBugMessages(self, bug):
         """See IBugMessageSet."""
-        return BugMessage.select("""
-            BugMessage.bug = %s
-            AND BugMessage.bugwatch IS NOT NULL
-            """ % sqlvalues(bug), orderBy='id')
+        store = IStore(BugMessage)
+        resultset = store.find(
+            BugMessage,
+            BugMessage.bug == bug, BugMessage.bugwatch != None)
+        return resultset.order_by(BugMessage.id)
diff --git a/lib/lp/bugs/model/bugtasksearch.py b/lib/lp/bugs/model/bugtasksearch.py
index 9ceb517..21f7313 100644
--- a/lib/lp/bugs/model/bugtasksearch.py
+++ b/lib/lp/bugs/model/bugtasksearch.py
@@ -631,7 +631,7 @@ def _build_query(params):
         with_clauses.append(convert_storm_clause_to_string(
             With('commented_bug_ids', Union(
                 Select(
-                    BugMessage.bugID, tables=[BugMessage],
+                    BugMessage.bug_id, tables=[BugMessage],
                     where=And(
                         BugMessage.index > 0,
                         BugMessage.owner == params.bug_commenter)),
diff --git a/lib/lp/bugs/model/bugtracker.py b/lib/lp/bugs/model/bugtracker.py
index 22410e1..f4f10c4 100644
--- a/lib/lp/bugs/model/bugtracker.py
+++ b/lib/lp/bugs/model/bugtracker.py
@@ -602,7 +602,7 @@ class BugTracker(SQLBase):
         """See `IBugTracker`."""
         return Store.of(self).find(
             BugMessage,
-            BugMessage.bugwatchID == BugWatch.id,
+            BugMessage.bugwatch_id == BugWatch.id,
             BugWatch.bugtrackerID == self.id).order_by(BugMessage.id)
 
     def getLinkedPersonByName(self, name):
diff --git a/lib/lp/bugs/model/bugwatch.py b/lib/lp/bugs/model/bugwatch.py
index b21ea5d..fca7a1c 100644
--- a/lib/lp/bugs/model/bugwatch.py
+++ b/lib/lp/bugs/model/bugwatch.py
@@ -1,4 +1,4 @@
-# Copyright 2009-2019 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2020 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 __metaclass__ = type
@@ -261,6 +261,8 @@ class BugWatch(SQLBase):
 
     def hasComment(self, comment_id):
         """See `IBugWatch`."""
+        if comment_id is not None:
+            comment_id = six.ensure_text(comment_id)
         store = Store.of(self)
         bug_messages = store.find(
             BugMessage,
diff --git a/lib/lp/bugs/scripts/checkwatches/bugwatchupdater.py b/lib/lp/bugs/scripts/checkwatches/bugwatchupdater.py
index ba24e49..d8cad22 100644
--- a/lib/lp/bugs/scripts/checkwatches/bugwatchupdater.py
+++ b/lib/lp/bugs/scripts/checkwatches/bugwatchupdater.py
@@ -1,8 +1,10 @@
-# Copyright 2010-2011 Canonical Ltd.  This software is licensed under the
+# Copyright 2010-2020 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Classes and logic for the checkwatches BugWatchUpdater."""
 
+from __future__ import absolute_import, print_function, unicode_literals
+
 __metaclass__ = type
 __all__ = [
     'BugWatchUpdater',
@@ -11,6 +13,7 @@ __all__ = [
 import sys
 
 from lazr.lifecycle.event import ObjectCreatedEvent
+import six
 from zope.component import getUtility
 from zope.event import notify
 
@@ -247,7 +250,8 @@ class BugWatchUpdater(WorkingBase):
             assert remote_comment_id is not None, (
                 "A remote_comment_id must be specified.")
             with self.transaction:
-                unpushed_comment.remote_comment_id = remote_comment_id
+                unpushed_comment.remote_comment_id = six.ensure_text(
+                    remote_comment_id)
 
             pushed_comments += 1
 
diff --git a/lib/lp/bugs/scripts/checkwatches/tests/test_bugwatchupdater.py b/lib/lp/bugs/scripts/checkwatches/tests/test_bugwatchupdater.py
index db136d8..9472091 100644
--- a/lib/lp/bugs/scripts/checkwatches/tests/test_bugwatchupdater.py
+++ b/lib/lp/bugs/scripts/checkwatches/tests/test_bugwatchupdater.py
@@ -1,8 +1,10 @@
-# Copyright 2010-2017 Canonical Ltd.  This software is licensed under the
+# Copyright 2010-2020 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Tests for the checkwatches.bugwatchupdater module."""
 
+from __future__ import absolute_import, print_function, unicode_literals
+
 __metaclass__ = type
 
 from datetime import datetime
diff --git a/lib/lp/bugs/tests/test_bugs_webservice.py b/lib/lp/bugs/tests/test_bugs_webservice.py
index 2de90c3..2c87319 100644
--- a/lib/lp/bugs/tests/test_bugs_webservice.py
+++ b/lib/lp/bugs/tests/test_bugs_webservice.py
@@ -1,4 +1,4 @@
-# Copyright 2009-2019 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2020 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Webservice unit tests related to Launchpad Bugs."""
@@ -199,7 +199,7 @@ class TestBugScaling(TestCaseWithFactory):
     def test_attachments_query_counts_constant(self):
         # XXX j.c.sackett 2010-09-02 bug=619017
         # This test was being thrown off by the reference bug. To get around
-        # the problem, flush and reset are called on the bug storm cache
+        # the problem, flush and invalidate are called on the bug storm cache
         # before each call to the webservice. When lp's storm is updated
         # to release the committed fix for this bug, please see about
         # updating this test.
@@ -216,7 +216,7 @@ class TestBugScaling(TestCaseWithFactory):
         url = '/bugs/%d/attachments?ws.size=75' % self.bug.id
         # First request.
         store.flush()
-        store.reset()
+        store.invalidate()
         response = webservice.get(url)
         self.assertThat(collector, HasQueryCount(LessThan(24)))
         with_2_count = collector.count
@@ -227,16 +227,17 @@ class TestBugScaling(TestCaseWithFactory):
         logout()
         # Second request.
         store.flush()
-        store.reset()
+        store.invalidate()
         response = webservice.get(url)
         self.assertThat(collector, HasQueryCount(Equals(with_2_count)))
 
     def test_messages_query_counts_constant(self):
         # XXX Robert Collins 2010-09-15 bug=619017
         # This test may be thrown off by the reference bug. To get around the
-        # problem, flush and reset are called on the bug storm cache before
-        # each call to the webservice. When lp's storm is updated to release
-        # the committed fix for this bug, please see about updating this test.
+        # problem, flush and invalidate are called on the bug storm cache
+        # before each call to the webservice. When lp's storm is updated to
+        # release the committed fix for this bug, please see about updating
+        # this test.
         login(USER_EMAIL)
         bug = self.factory.makeBug()
         store = Store.of(bug)
@@ -251,7 +252,7 @@ class TestBugScaling(TestCaseWithFactory):
         url = '/bugs/%d/messages?ws.size=75' % bug.id
         # First request.
         store.flush()
-        store.reset()
+        store.invalidate()
         response = webservice.get(url)
         self.assertThat(collector, HasQueryCount(LessThan(24)))
         with_2_count = collector.count
@@ -263,7 +264,7 @@ class TestBugScaling(TestCaseWithFactory):
         logout()
         # Second request.
         store.flush()
-        store.reset()
+        store.invalidate()
         response = webservice.get(url)
         self.assertThat(collector, HasQueryCount(Equals(with_2_count)))
 
diff --git a/lib/lp/bugs/tests/test_bugtracker.py b/lib/lp/bugs/tests/test_bugtracker.py
index da07fe6..67c6114 100644
--- a/lib/lp/bugs/tests/test_bugtracker.py
+++ b/lib/lp/bugs/tests/test_bugtracker.py
@@ -1,6 +1,8 @@
-# Copyright 2009-2019 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2020 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
+from __future__ import absolute_import, print_function, unicode_literals
+
 __metaclass__ = type
 
 from datetime import (
diff --git a/lib/lp/services/messages/model/message.py b/lib/lp/services/messages/model/message.py
index 94f91da..05d8a5d 100644
--- a/lib/lp/services/messages/model/message.py
+++ b/lib/lp/services/messages/model/message.py
@@ -1,4 +1,4 @@
-# Copyright 2009-2018 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2020 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 __metaclass__ = type
@@ -108,7 +108,7 @@ class Message(SQLBase):
     parent = ForeignKey(foreignKey='Message', dbName='parent',
         notNull=False, default=None)
     rfc822msgid = StringCol(notNull=True)
-    bugs = SQLRelatedJoin('Bug', joinColumn='message', otherColumn='bug',
+    bugs = SQLRelatedJoin('Bug', joinColumn='message_id', otherColumn='bug_id',
         intermediateTable='BugMessage')
     _chunks = SQLMultipleJoin('MessageChunk', joinColumn='message')