launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

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

Commit message:
Convert POTranslation to Storm

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

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

diff --git a/lib/lp/testing/factory.py b/lib/lp/testing/factory.py
index 38b9ffb..b38c7d6 100644
--- a/lib/lp/testing/factory.py
+++ b/lib/lp/testing/factory.py
@@ -3289,7 +3289,7 @@ class BareLaunchpadObjectFactory(ObjectFactory):
         """
         translations = removeSecurityProxy(translations)
         if translations is None:
-            return {0: self.getUniqueString()}
+            return {0: self.getUniqueUnicode()}
         if isinstance(translations, dict):
             return translations
         assert isinstance(translations, (list, tuple)), (
diff --git a/lib/lp/translations/doc/potranslation.txt b/lib/lp/translations/doc/potranslation.txt
index 459ea42..8fc1e94 100644
--- a/lib/lp/translations/doc/potranslation.txt
+++ b/lib/lp/translations/doc/potranslation.txt
@@ -7,11 +7,11 @@ This is the class that holds the text of translations in the database.
 
 Creating one is easy.
 
-    >>> created = POTranslation(translation="This is a launchpad test")
+    >>> created = POTranslation.new("This is a launchpad test")
 
-To get hold of a PO translation, use POTranslation.byTranslation.
+To get hold of a PO translation, use POTranslation.getByTranslation.
 
-    >>> got = POTranslation.byTranslation("This is a launchpad test")
+    >>> got = POTranslation.getByTranslation("This is a launchpad test")
     >>> got == created
     True
     >>> got.translation
@@ -19,10 +19,10 @@ To get hold of a PO translation, use POTranslation.byTranslation.
 
 However, if the translation doesn't already exist, you'll get an error.
 
-    >>> got = POTranslation.byTranslation("In Xanadu did Kubla Khan")
+    >>> got = POTranslation.getByTranslation("In Xanadu did Kubla Khan")
     Traceback (most recent call last):
     ...
-    SQLObjectNotFound: In Xanadu did Kubla Khan
+    NotFoundError: 'In Xanadu did Kubla Khan'
 
 If you want to get hold of one, and have it automatically created if it
 doesn't already exist, use POTranslation.getOrCreateTranslation.
diff --git a/lib/lp/translations/model/pofile.py b/lib/lp/translations/model/pofile.py
index 8179ee2..ff8ac2c 100644
--- a/lib/lp/translations/model/pofile.py
+++ b/lib/lp/translations/model/pofile.py
@@ -167,7 +167,7 @@ class POFileMixIn(RosettaStats):
         # Like in findPOTMsgSetsContaining(), to avoid seqscans on
         # POTranslation table, we do ILIKE comparison on them in a subselect
         # which is first filtered by the POFile.
-        msgstr_column_name = "msgstr%dID" % plural_form
+        msgstr_column_name = "msgstr%d_id" % plural_form
         tm_ids = ClassAlias(TranslationMessage, "tm_ids")
         return Select(
             TranslationMessage.potmsgsetID,
diff --git a/lib/lp/translations/model/potmsgset.py b/lib/lp/translations/model/potmsgset.py
index c742d36..456dc8d 100644
--- a/lib/lp/translations/model/potmsgset.py
+++ b/lib/lp/translations/model/potmsgset.py
@@ -442,7 +442,7 @@ class POTMsgSet(SQLBase):
         # all translated forms.  The Python code can later sort out the
         # distinct translations per form.
         msgstrs = [
-            Coalesce(getattr(TranslationMessage, 'msgstr%dID' % form), -1)
+            Coalesce(getattr(TranslationMessage, 'msgstr%d_id' % form), -1)
             for form in range(TranslationConstants.MAX_PLURAL_FORMS)]
 
         result = IStore(TranslationMessage).with_(msgsets).find(
@@ -597,7 +597,7 @@ class POTMsgSet(SQLBase):
         # more than one, prefer the one that adds the fewest extraneous
         # plural forms.
         order.extend([
-            NullsFirst(getattr(TranslationMessage, 'msgstr%dID' % form))
+            NullsFirst(getattr(TranslationMessage, 'msgstr%d_id' % form))
             for form in remaining_plural_forms])
         matches = list(IStore(TranslationMessage).find(
             TranslationMessage, *clauses).order_by(*order))
diff --git a/lib/lp/translations/model/potranslation.py b/lib/lp/translations/model/potranslation.py
index 37ec5f3..714f9cb 100644
--- a/lib/lp/translations/model/potranslation.py
+++ b/lib/lp/translations/model/potranslation.py
@@ -5,46 +5,56 @@ __metaclass__ = type
 __all__ = ['POTranslation']
 
 import six
-from sqlobject import (
-    SQLObjectNotFound,
-    StringCol,
+from storm.expr import Func
+from storm.locals import (
+    Int,
+    Unicode,
     )
 from zope.interface import implementer
 
-from lp.services.database.sqlbase import (
-    quote,
-    SQLBase,
-    )
+from lp.app.errors import NotFoundError
+from lp.services.database.interfaces import IStore
+from lp.services.database.stormbase import StormBase
 from lp.translations.interfaces.potranslation import IPOTranslation
 
 
 @implementer(IPOTranslation)
-class POTranslation(SQLBase):
+class POTranslation(StormBase):
+
+    __storm_table__ = 'POTranslation'
+
+    id = Int(primary=True)
+    translation = Unicode(name='translation', allow_none=False)
 
-    _table = 'POTranslation'
+    def __init__(self, translation):
+        super(POTranslation, self).__init__()
+        self.translation = translation
 
-    # alternateID=False because we have to select by hash in order to do
-    # index lookups.
-    translation = StringCol(dbName='translation', notNull=True, unique=True,
-        alternateID=False)
+    @classmethod
+    def new(cls, translation):
+        """Return a new POTranslation object for the given translation."""
+        potranslation = cls(translation)
+        IStore(cls).add(potranslation)
+        return potranslation
 
-    def byTranslation(cls, key):
+    @classmethod
+    def getByTranslation(cls, key):
         """Return a POTranslation object for the given translation."""
 
         # We can't search directly on msgid, because this database column
         # contains values too large to index. Instead we search on its
         # hash, which *is* indexed
-        r = cls.selectOne('sha1(translation) = sha1(%s)' % quote(key))
+        r = IStore(POTranslation).find(
+            POTranslation,
+            Func('sha1', POTranslation.translation) ==
+                Func('sha1', six.ensure_text(key))).one()
 
         if r is not None:
             return r
         else:
-            # To be 100% compatible with the alternateID behaviour, we should
-            # raise SQLObjectNotFound instead of KeyError
-            raise SQLObjectNotFound(key)
-
-    byTranslation = classmethod(byTranslation)
+            raise NotFoundError(six.ensure_str(key, errors='replace'))
 
+    @classmethod
     def getOrCreateTranslation(cls, key):
         """Return a POTranslation object for the given translation, or create
         it if it doesn't exist.
@@ -55,8 +65,6 @@ class POTranslation(SQLBase):
         key = six.ensure_text(key)
 
         try:
-            return cls.byTranslation(key)
-        except SQLObjectNotFound:
-            return cls(translation=key)
-
-    getOrCreateTranslation = classmethod(getOrCreateTranslation)
+            return cls.getByTranslation(key)
+        except NotFoundError:
+            return cls.new(key)
diff --git a/lib/lp/translations/model/translationmessage.py b/lib/lp/translations/model/translationmessage.py
index 0753e5c..b323a20 100644
--- a/lib/lp/translations/model/translationmessage.py
+++ b/lib/lp/translations/model/translationmessage.py
@@ -20,7 +20,11 @@ from sqlobject import (
     StringCol,
     )
 from storm.expr import And
-from storm.locals import SQL
+from storm.locals import (
+    Int,
+    Reference,
+    SQL,
+    )
 from storm.store import Store
 from zope.component import getUtility
 from zope.interface import implementer
@@ -262,18 +266,19 @@ class TranslationMessage(SQLBase, TranslationMessageMixIn):
     assert TranslationConstants.MAX_PLURAL_FORMS == 6, (
         "Change this code to support %d plural forms."
         % TranslationConstants.MAX_PLURAL_FORMS)
-    msgstr0 = ForeignKey(foreignKey='POTranslation', dbName='msgstr0',
-                         notNull=False, default=DEFAULT)
-    msgstr1 = ForeignKey(foreignKey='POTranslation', dbName='msgstr1',
-                         notNull=False, default=DEFAULT)
-    msgstr2 = ForeignKey(foreignKey='POTranslation', dbName='msgstr2',
-                         notNull=False, default=DEFAULT)
-    msgstr3 = ForeignKey(foreignKey='POTranslation', dbName='msgstr3',
-                         notNull=False, default=DEFAULT)
-    msgstr4 = ForeignKey(foreignKey='POTranslation', dbName='msgstr4',
-                         notNull=False, default=DEFAULT)
-    msgstr5 = ForeignKey(foreignKey='POTranslation', dbName='msgstr5',
-                         notNull=False, default=DEFAULT)
+
+    msgstr0_id = Int(name='msgstr0', allow_none=True, default=DEFAULT)
+    msgstr0 = Reference(msgstr0_id, 'POTranslation.id')
+    msgstr1_id = Int(name='msgstr1', allow_none=True, default=DEFAULT)
+    msgstr1 = Reference(msgstr1_id, 'POTranslation.id')
+    msgstr2_id = Int(name='msgstr2', allow_none=True, default=DEFAULT)
+    msgstr2 = Reference(msgstr2_id, 'POTranslation.id')
+    msgstr3_id = Int(name='msgstr3', allow_none=True, default=DEFAULT)
+    msgstr3 = Reference(msgstr3_id, 'POTranslation.id')
+    msgstr4_id = Int(name='msgstr4', allow_none=True, default=DEFAULT)
+    msgstr4 = Reference(msgstr4_id, 'POTranslation.id')
+    msgstr5_id = Int(name='msgstr5', allow_none=True, default=DEFAULT)
+    msgstr5 = Reference(msgstr5_id, 'POTranslation.id')
 
     comment = StringCol(
         dbName='comment', notNull=False, default=None)
@@ -430,7 +435,7 @@ class TranslationMessage(SQLBase, TranslationMessageMixIn):
 
         for form in range(TranslationConstants.MAX_PLURAL_FORMS):
             msgstr_name = 'msgstr%d' % form
-            msgstr = getattr(self, 'msgstr%dID' % form)
+            msgstr = getattr(self, 'msgstr%d_id' % form)
             if msgstr is None:
                 form_clause = "%s IS NULL" % msgstr_name
             else:
@@ -497,10 +502,10 @@ class TranslationMessage(SQLBase, TranslationMessageMixIn):
         """See `ITranslationMessage`."""
         store = Store.of(self)
 
-        forms_match = (TranslationMessage.msgstr0ID == self.msgstr0ID)
+        forms_match = (TranslationMessage.msgstr0_id == self.msgstr0_id)
         for form in range(1, TranslationConstants.MAX_PLURAL_FORMS):
             form_name = 'msgstr%d' % form
-            form_value = getattr(self, 'msgstr%dID' % form)
+            form_value = getattr(self, 'msgstr%d_id' % form)
             forms_match = And(
                 forms_match,
                 getattr(TranslationMessage, form_name) == form_value)
@@ -570,7 +575,7 @@ class TranslationMessageSet:
         if need_potranslation:
             load_related(
                 POTranslation, tms,
-                ['msgstr%dID' % form
+                ['msgstr%d_id' % form
                  for form in range(TranslationConstants.MAX_PLURAL_FORMS)])
         if need_potmsgset:
             load_related(POTMsgSet, tms, ['potmsgsetID'])
diff --git a/lib/lp/translations/tests/helpers.py b/lib/lp/translations/tests/helpers.py
index 932c83c..ea982e4 100644
--- a/lib/lp/translations/tests/helpers.py
+++ b/lib/lp/translations/tests/helpers.py
@@ -50,7 +50,7 @@ def make_translationmessage(factory, pofile=None, potmsgset=None,
         potmsgset = factory.makePOTMsgSet(
             potemplate=pofile.potemplate)
     if translations is None:
-        translations = [factory.getUniqueString()]
+        translations = [factory.getUniqueUnicode()]
     if diverged:
         potemplate = pofile.potemplate
     else:
diff --git a/lib/lp/translations/tests/test_translationmerger.py b/lib/lp/translations/tests/test_translationmerger.py
index cd49d51..c7b5cff 100644
--- a/lib/lp/translations/tests/test_translationmerger.py
+++ b/lib/lp/translations/tests/test_translationmerger.py
@@ -743,14 +743,14 @@ class TestSharingMigrationPerformance(TestCaseWithFactory,
         self.assertNoStatementsInvolvingTable(
             POMsgID.__storm_table__, recorder.statements)
         self.assertNoStatementsInvolvingTable(
-            POTranslation._table, recorder.statements)
+            POTranslation.__storm_table__, recorder.statements)
 
         with StormStatementRecorder() as recorder:
             self.merger.mergeTranslationMessages()
         self.assertNoStatementsInvolvingTable(
             POMsgID.__storm_table__, recorder.statements)
         self.assertNoStatementsInvolvingTable(
-            POTranslation._table, recorder.statements)
+            POTranslation.__storm_table__, recorder.statements)
 
 
 class TestFindMergablePackagings(TestCaseWithFactory):
diff --git a/lib/lp/translations/utilities/translationmerger.py b/lib/lp/translations/utilities/translationmerger.py
index 5805eb2..192c32b 100644
--- a/lib/lp/translations/utilities/translationmerger.py
+++ b/lib/lp/translations/utilities/translationmerger.py
@@ -600,7 +600,7 @@ class TranslationMerger:
         """
         tm = removeSecurityProxy(tm)
         msgstr_ids = tuple([
-            getattr(tm, 'msgstr%dID' % form)
+            getattr(tm, 'msgstr%d_id' % form)
             for form in range(TranslationConstants.MAX_PLURAL_FORMS)])
 
         return (tm.potemplateID, tm.languageID) + msgstr_ids