launchpad-reviewers team mailing list archive

[Colin Watson <mp+446887@xxxxxxxxxxxxxxxxxx>]

Colin Watson has proposed merging ~cjwatson/launchpad:stormify-potmsgset into launchpad:master.

Commit message:
Convert POTMsgSet to Storm

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/+git/launchpad/+merge/446887

I simplified the default for `msgid_plural` to None; `DEFAULT` means to use the database's default value for the column, but that's NULL.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~cjwatson/launchpad:stormify-potmsgset into launchpad:master.

diff --git a/lib/lp/testing/factory.py b/lib/lp/testing/factory.py
index ea6937e..11d337b 100644
--- a/lib/lp/testing/factory.py
+++ b/lib/lp/testing/factory.py
@@ -4165,7 +4165,7 @@ class LaunchpadObjectFactory(ObjectFactory):
             potmsgset.sourcecomment = sourcecomment
         if flagscomment is not None:
             potmsgset.flagscomment = flagscomment
-        removeSecurityProxy(potmsgset).sync()
+        IStore(potmsgset).flush()
         return ProxyFactory(potmsgset)
 
     def makePOFileAndPOTMsgSet(
diff --git a/lib/lp/translations/browser/tests/pofile-views.rst b/lib/lp/translations/browser/tests/pofile-views.rst
index d8112c0..153f788 100644
--- a/lib/lp/translations/browser/tests/pofile-views.rst
+++ b/lib/lp/translations/browser/tests/pofile-views.rst
@@ -263,8 +263,9 @@ situations, like when this set of actions happen:
 The problem here is that some of the messages on that form are disabled so
 their sequence is 0.
 
+    >>> from lp.services.database.interfaces import IStore
     >>> from lp.translations.model.potmsgset import POTMsgSet
-    >>> potmsgset = POTMsgSet.get(161)
+    >>> potmsgset = IStore(POTMsgSet).get(POTMsgSet, 161)
     >>> item = potmsgset.setSequence(pofile_es.potemplate, 0)
     >>> potmsgset.getSequence(pofile_es.potemplate)
     0
diff --git a/lib/lp/translations/doc/gettext-check-messages.rst b/lib/lp/translations/doc/gettext-check-messages.rst
index 6ac4436..4f2a2ed 100644
--- a/lib/lp/translations/doc/gettext-check-messages.rst
+++ b/lib/lp/translations/doc/gettext-check-messages.rst
@@ -24,7 +24,6 @@ points that would otherwise be in the output.
 
     >>> from zope.security.proxy import removeSecurityProxy
 
-    >>> from lp.services.database.sqlbase import quote
     >>> from lp.translations.scripts.gettext_check_messages import (
     ...     GettextCheckMessages,
     ... )
@@ -103,7 +102,7 @@ The gettext_check_message script goes through a given set of messages
 and re-does the gettext check.  Which messages it checks is specified as
 a plain SQL WHERE clause.
 
-    >>> run_checker(["-vv", "-w id=%s" % quote(ubuntu_message.id)])
+    >>> run_checker(["-vv", "-w", "id=%d" % ubuntu_message.id])
     DEBUG Checking messages matching:  id=...
     DEBUG Checking message ...
     DEBUG Commit point.
@@ -127,7 +126,7 @@ detects the problem when it checks that message.
     >>> from lp.services.propertycache import get_property_cache
     >>> get_property_cache(ubuntu_message).translations = ["%s c"]
 
-    >>> run_checker(["-w id=%s" % quote(ubuntu_message.id)])
+    >>> run_checker(["-w", "id=%d" % ubuntu_message.id])
     DEBUG Checking messages matching:  id=...
     DEBUG Checking message ...
     INFO ... (ubuntu): format specifications ... are not the same
@@ -159,7 +158,7 @@ happens to produce validation errors.
 In this example we'd like to see a nicely predictable ordering, so we
 add a sort order using the -o option.
 
-    >>> run_checker(["-w", "potmsgset=%s" % quote(potmsgset), "-o", "id"])
+    >>> run_checker(["-w", "potmsgset=%d" % potmsgset.id, "-o", "id"])
     DEBUG Checking messages matching:  potmsgset=...
     DEBUG Checking message ...
     INFO ... (unused): format specifications ... are not the same
@@ -179,7 +178,7 @@ The script also notes when a message is shared between upstream and Ubuntu.
 
     >>> upstream_message.is_current_ubuntu = True
     >>> upstream_message.is_current_upstream = True
-    >>> run_checker(["-w id=%s" % quote(upstream_message.id)])
+    >>> run_checker(["-w", "id=%d" % upstream_message.id])
     DEBUG ...
     INFO ... (ubuntu, upstream): number of format specifications ...
 
@@ -191,7 +190,7 @@ The --dry-run option makes the script abort all its database changes.
 
     >>> ubuntu_message.is_current_ubuntu = True
 
-    >>> run_checker(["-w id=%s" % quote(ubuntu_message.id), "--dry-run"])
+    >>> run_checker(["-w", "id=%d" % ubuntu_message.id, "--dry-run"])
     INFO Dry run.  Not making any changes.
     DEBUG Checking messages matching:  id=...
     DEBUG Checking message ...
@@ -216,7 +215,7 @@ purpose of this test we count messages checked.  If we set the commit
 interval to 1, we get a commit after every message plus one at the end
 to close things off neatly.
 
-    >>> run_checker(["-w potmsgset=%s" % quote(potmsgset)], commit_interval=1)
+    >>> run_checker(["-w", "potmsgset=%d" % potmsgset.id], commit_interval=1)
     DEBUG Checking messages matching:  potmsgset=...
     DEBUG Checking message ...
     INFO ... (...): number of format specifications ...
diff --git a/lib/lp/translations/doc/potmsgset.rst b/lib/lp/translations/doc/potmsgset.rst
index 8b418c1..5d8403f 100644
--- a/lib/lp/translations/doc/potmsgset.rst
+++ b/lib/lp/translations/doc/potmsgset.rst
@@ -761,8 +761,12 @@ is a fuzzily matched message" or "this message follows C format-string
 rules."  These flags are set in a comment starting with a comma, and
 flags are separated by further commas.
 
+    >>> from lp.translations.model.pomsgid import POMsgID
     >>> from lp.translations.model.potmsgset import POTMsgSet
-    >>> flagged_potmsgset = POTMsgSet(flagscomment=", fuzzy, c-format")
+    >>> flagged_potmsgset = POTMsgSet(
+    ...     msgid_singular=POMsgID(factory.getUniqueUnicode()),
+    ...     flagscomment=", fuzzy, c-format",
+    ... )
 
 The flags property produces these as a neat list of flags.
 
@@ -779,5 +783,7 @@ The flags property produces these as a neat list of flags.
 
 If the message has no flags, that list is empty.
 
-    >>> print_flags(POTMsgSet())
+    >>> print_flags(
+    ...     POTMsgSet(msgid_singular=POMsgID(factory.getUniqueUnicode()))
+    ... )
     .
diff --git a/lib/lp/translations/model/potemplate.py b/lib/lp/translations/model/potemplate.py
index 85c6f57..a1a52ef 100644
--- a/lib/lp/translations/model/potemplate.py
+++ b/lib/lp/translations/model/potemplate.py
@@ -927,12 +927,6 @@ class POTemplate(SQLBase, RosettaStats):
             context=context,
             msgid_singular=msgid_singular,
             msgid_plural=msgid_plural,
-            sequence=0,
-            potemplate=None,
-            commenttext=None,
-            filereferences=None,
-            sourcecomment=None,
-            flagscomment=None,
         )
 
         potmsgset.setSequence(self, sequence)
diff --git a/lib/lp/translations/model/potmsgset.py b/lib/lp/translations/model/potmsgset.py
index ab2e031..e4efa9e 100644
--- a/lib/lp/translations/model/potmsgset.py
+++ b/lib/lp/translations/model/potmsgset.py
@@ -24,7 +24,7 @@ from storm.expr import (
     Select,
     Table,
 )
-from storm.locals import Int, Reference
+from storm.locals import Int, Reference, Unicode
 from storm.store import EmptyResultSet, Store
 from zope.component import getUtility
 from zope.interface import implementer
@@ -33,10 +33,8 @@ from zope.security.proxy import removeSecurityProxy
 from lp.app.errors import NotFoundError
 from lp.app.interfaces.launchpad import ILaunchpadCelebrities
 from lp.services.config import config
-from lp.services.database.constants import DEFAULT
 from lp.services.database.interfaces import IStore
-from lp.services.database.sqlbase import SQLBase, sqlvalues
-from lp.services.database.sqlobject import StringCol
+from lp.services.database.stormbase import StormBase
 from lp.services.database.stormexpr import (
     NullsFirst,
     NullsLast,
@@ -121,28 +119,46 @@ def dictify_translations(translations):
 
 
 @implementer(IPOTMsgSet)
-class POTMsgSet(SQLBase):
-    _table = "POTMsgSet"
+class POTMsgSet(StormBase):
+    __storm_table__ = "POTMsgSet"
 
-    context = StringCol(dbName="context", notNull=False)
+    id = Int(primary=True)
     msgid_singular_id = Int(name="msgid_singular", allow_none=False)
     msgid_singular = Reference(msgid_singular_id, "POMsgID.id")
-    msgid_plural_id = Int(
-        name="msgid_plural", allow_none=True, default=DEFAULT
-    )
+    msgid_plural_id = Int(name="msgid_plural", allow_none=True, default=None)
     msgid_plural = Reference(msgid_plural_id, "POMsgID.id")
-    commenttext = StringCol(dbName="commenttext", notNull=False)
-    filereferences = StringCol(dbName="filereferences", notNull=False)
-    sourcecomment = StringCol(dbName="sourcecomment", notNull=False)
-    flagscomment = StringCol(dbName="flagscomment", notNull=False)
+    context = Unicode(name="context", allow_none=True)
+    commenttext = Unicode(name="commenttext", allow_none=True)
+    filereferences = Unicode(name="filereferences", allow_none=True)
+    sourcecomment = Unicode(name="sourcecomment", allow_none=True)
+    flagscomment = Unicode(name="flagscomment", allow_none=True)
 
     credits_message_ids = credits_message_info.keys()
 
+    def __init__(
+        self,
+        msgid_singular,
+        msgid_plural=None,
+        context=None,
+        commenttext=None,
+        filereferences=None,
+        sourcecomment=None,
+        flagscomment=None,
+    ):
+        super().__init__()
+        self.msgid_singular = msgid_singular
+        self.msgid_plural = msgid_plural
+        self.context = context
+        self.commenttext = commenttext
+        self.filereferences = filereferences
+        self.sourcecomment = sourcecomment
+        self.flagscomment = flagscomment
+
     def clone(self):
         return POTMsgSet(
-            context=self.context,
             msgid_singular=self.msgid_singular,
             msgid_plural=self.msgid_plural,
+            context=self.context,
             commenttext=self.commenttext,
             filereferences=self.filereferences,
             sourcecomment=self.sourcecomment,
@@ -649,9 +665,9 @@ class POTMsgSet(SQLBase):
         if len(matches) > 0:
             if len(matches) > 1:
                 logging.info(
-                    "Translation for POTMsgSet %s into %s "
-                    "matches %s existing translations."
-                    % sqlvalues(self, pofile.language.code, len(matches))
+                    "Translation for POTMsgSet %r into %s "
+                    "matches %d existing translations."
+                    % (self, pofile.language.code, len(matches))
                 )
             return matches[0]
         else:
diff --git a/lib/lp/translations/utilities/translationmerger.py b/lib/lp/translations/utilities/translationmerger.py
index 3963227..e1a8ab1 100644
--- a/lib/lp/translations/utilities/translationmerger.py
+++ b/lib/lp/translations/utilities/translationmerger.py
@@ -481,7 +481,9 @@ class TranslationMerger:
         self.tm.endTransaction(intermediate=True)
 
         for representative_id in representatives.values():
-            representative = POTMsgSet.get(representative_id)
+            representative = IStore(POTMsgSet).get(
+                POTMsgSet, representative_id
+            )
             self._scrubPOTMsgSetTranslations(representative)
             self.tm.endTransaction(intermediate=True)
 
@@ -599,7 +601,7 @@ class TranslationMerger:
                     representative,
                     representative_templates[representative],
                 )
-                removeSecurityProxy(subordinate).destroySelf()
+                Store.of(subordinate).remove(subordinate)
                 potmsgset_deletions += 1
 
                 self.tm.endTransaction(intermediate=True)
@@ -635,7 +637,7 @@ class TranslationMerger:
             order_check.check(template)
             potmsgset_ids = self._getPOTMsgSetIds(template)
             for potmsgset_id in potmsgset_ids:
-                potmsgset = POTMsgSet.get(potmsgset_id)
+                potmsgset = IStore(POTMsgSet).get(POTMsgSet, potmsgset_id)
 
                 tm_ids = self._partitionTranslationMessageIds(potmsgset)
                 before = sum((len(sublist) for sublist in tm_ids), 0)