launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #16121
[Merge] lp:~wgrant/launchpad/bug-1186610 into lp:launchpad
William Grant has proposed merging lp:~wgrant/launchpad/bug-1186610 into lp:launchpad.
Commit message:
Stop crashing on unusual % sequences in translation message IDs and context strings.
Requested reviews:
William Grant (wgrant): code
Related bugs:
Bug #1186610 in Launchpad itself: "Uploading .PO or .POT file gives "tuple index out of range" error"
https://bugs.launchpad.net/launchpad/+bug/1186610
For more details, see:
https://code.launchpad.net/~wgrant/launchpad/bug-1186610/+merge/191939
Port most of lp.translations.model.potemplate to use native Storm rather than the SQLObject compat layer. It was previously using sqlvalues() in such a way that it could trip over unusual % sequences in translation message IDs or context strings.
--
https://code.launchpad.net/~wgrant/launchpad/bug-1186610/+merge/191939
Your team Launchpad code reviewers is subscribed to branch lp:launchpad.
=== modified file 'lib/lp/translations/browser/potemplate.py'
--- lib/lp/translations/browser/potemplate.py 2013-01-07 02:40:55 +0000
+++ lib/lp/translations/browser/potemplate.py 2013-10-21 06:11:14 +0000
@@ -62,6 +62,7 @@
from lp.registry.model.productseries import ProductSeries
from lp.registry.model.sourcepackagename import SourcePackageName
from lp.services.helpers import is_tar_filename
+from lp.services.propertycache import cachedproperty
from lp.services.webapp import (
canonical_url,
enabled_with_permission,
@@ -309,7 +310,7 @@
return (translation_group is not None and
translation_group.translation_guide_url is not None)
- @property
+ @cachedproperty
def related_templates_by_source(self):
by_source = list(
self.context.relatives_by_source[:self.SHOW_RELATED_TEMPLATES])
=== modified file 'lib/lp/translations/interfaces/potemplate.py'
--- lib/lp/translations/interfaces/potemplate.py 2013-01-07 02:40:55 +0000
+++ lib/lp/translations/interfaces/potemplate.py 2013-10-21 06:11:14 +0000
@@ -667,9 +667,6 @@
def __iter__():
"""Return an iterator over all PO templates."""
- def getByIDs(ids):
- """Return all PO templates with the given IDs."""
-
def getAllByName(name):
"""Return a list with all PO templates with the given name."""
=== modified file 'lib/lp/translations/model/potemplate.py'
--- lib/lp/translations/model/potemplate.py 2013-06-20 05:50:00 +0000
+++ lib/lp/translations/model/potemplate.py 2013-10-21 06:11:14 +0000
@@ -36,6 +36,7 @@
LeftJoin,
Or,
Select,
+ SQL,
)
from storm.info import ClassAlias
from storm.store import (
@@ -55,6 +56,7 @@
from lp.registry.interfaces.person import validate_public_person
from lp.registry.model.packaging import Packaging
from lp.registry.model.sourcepackagename import SourcePackageName
+from lp.services.database.bulk import load_related
from lp.services.database.collection import Collection
from lp.services.database.constants import DEFAULT
from lp.services.database.datetimecol import UtcDateTimeCol
@@ -66,9 +68,7 @@
)
from lp.services.database.sqlbase import (
flush_database_updates,
- quote,
SQLBase,
- sqlvalues,
)
from lp.services.helpers import shortlist
from lp.services.mail.helpers import get_email_template
@@ -362,31 +362,29 @@
def relatives_by_source(self):
"""See `IPOTemplate`."""
if self.productseries is not None:
- return POTemplate.select(
- 'id <> %s AND productseries = %s AND iscurrent' % sqlvalues(
- self, self.productseries), orderBy=['name'])
+ return Store.of(self).find(
+ POTemplate,
+ POTemplate.id != self.id,
+ POTemplate.productseriesID == self.productseries.id,
+ POTemplate.iscurrent).order_by(POTemplate.name)
elif (self.distroseries is not None and
self.sourcepackagename is not None):
- return POTemplate.select('''
- id <> %s AND
- distroseries = %s AND
- sourcepackagename = %s AND
- iscurrent
- ''' % sqlvalues(
- self, self.distroseries, self.sourcepackagename),
- orderBy=['name'])
+ return Store.of(self).find(
+ POTemplate,
+ POTemplate.id != self.id,
+ POTemplate.distroseriesID == self.distroseries.id,
+ POTemplate.sourcepackagenameID == self.sourcepackagename.id,
+ POTemplate.iscurrent).order_by(POTemplate.name)
else:
raise AssertionError('Unknown POTemplate source.')
@property
def language_count(self):
- return Language.select('''
- POFile.language = Language.id AND
- POFile.currentcount + POFile.rosettacount > 0 AND
- POFile.potemplate = %s
- ''' % sqlvalues(self.id),
- clauseTables=['POFile'],
- distinct=True).count()
+ return IStore(Language).find(
+ Language,
+ POFile.language == Language.id,
+ POFile.currentcount + POFile.rosettacount > 0,
+ POFile.potemplateID == self.id).config(distinct=True).count()
@cachedproperty
def sourcepackage(self):
@@ -421,70 +419,62 @@
"""Return SQL clauses for finding POTMsgSets which belong
to this POTemplate."""
clauses = [
- 'TranslationTemplateItem.potemplate = %s' % sqlvalues(self),
- 'TranslationTemplateItem.potmsgset = POTMsgSet.id',
+ TranslationTemplateItem.potemplateID == self.id,
+ TranslationTemplateItem.potmsgsetID == POTMsgSet.id,
]
return clauses
def getPOTMsgSetByMsgIDText(self, singular_text, plural_text=None,
only_current=False, context=None):
"""See `IPOTemplate`."""
- clauses = self._getPOTMsgSetSelectionClauses()
- if only_current:
- clauses.append('TranslationTemplateItem.sequence > 0')
- if context is not None:
- clauses.append('context = %s' % sqlvalues(context))
- else:
- clauses.append('context IS NULL')
-
# Find a message ID with the given text.
try:
singular_msgid = POMsgID.byMsgid(singular_text)
except SQLObjectNotFound:
return None
- clauses.append('msgid_singular = %s' % sqlvalues(singular_msgid))
# Find a message ID for the plural string.
+ plural_msgid = None
if plural_text is not None:
try:
plural_msgid = POMsgID.byMsgid(plural_text)
- clauses.append('msgid_plural = %s' % sqlvalues(plural_msgid))
except SQLObjectNotFound:
return None
- else:
- # You have to be explicit now.
- clauses.append('msgid_plural IS NULL')
# Find a message set with the given message ID.
- return POTMsgSet.selectOne(' AND '.join(clauses),
- clauseTables=['TranslationTemplateItem'])
+ clauses = self._getPOTMsgSetSelectionClauses()
+ clauses.extend([
+ POTMsgSet.msgid_singular == singular_msgid,
+ POTMsgSet.msgid_plural == plural_msgid,
+ POTMsgSet.context == context,
+ ])
+ if only_current:
+ clauses.append(TranslationTemplateItem.sequence > 0)
+ return Store.of(self).find(POTMsgSet, *clauses).one()
def getPOTMsgSetBySequence(self, sequence):
"""See `IPOTemplate`."""
assert sequence > 0, ('%r is out of range')
-
clauses = self._getPOTMsgSetSelectionClauses()
- clauses.append(
- 'TranslationTemplateItem.sequence = %s' % sqlvalues(sequence))
-
- return POTMsgSet.selectOne(' AND '.join(clauses),
- clauseTables=['TranslationTemplateItem'])
+ clauses.append(TranslationTemplateItem.sequence == sequence)
+ return Store.of(self).find(POTMsgSet, *clauses).one()
def getPOTMsgSets(self, current=True, prefetch=True):
"""See `IPOTemplate`."""
clauses = self._getPOTMsgSetSelectionClauses()
-
if current:
# Only count the number of POTMsgSet that are current.
- clauses.append('TranslationTemplateItem.sequence > 0')
+ clauses.append(TranslationTemplateItem.sequence > 0)
+ result = Store.of(self).find(
+ POTMsgSet, *clauses).order_by(TranslationTemplateItem.sequence)
- query = POTMsgSet.select(" AND ".join(clauses),
- clauseTables=['TranslationTemplateItem'],
- orderBy=['TranslationTemplateItem.sequence'])
if prefetch:
- query = query.prejoin(['msgid_singular', 'msgid_plural'])
-
- return query
+ def prefetch_msgids(rows):
+ load_related(
+ POMsgID, rows, ['msgid_singularID', 'msgid_pluralID'])
+ return DecoratedResultSet(result, pre_iter_hook=prefetch_msgids)
+ else:
+ return result
def getTranslationCredits(self):
"""See `IPOTemplate`."""
@@ -511,18 +501,16 @@
def getPOTMsgSetByID(self, id):
"""See `IPOTemplate`."""
clauses = self._getPOTMsgSetSelectionClauses()
- clauses.append('POTMsgSet.id = %s' % sqlvalues(id))
-
- return POTMsgSet.selectOne(' AND '.join(clauses),
- clauseTables=['TranslationTemplateItem'])
+ clauses.append(POTMsgSet.id == id)
+ return Store.of(self).find(POTMsgSet, *clauses).one()
def languages(self):
"""See `IPOTemplate`."""
- return Language.select("POFile.language = Language.id AND "
- "Language.code <> 'en' AND "
- "POFile.potemplate = %d" % self.id,
- clauseTables=['POFile', 'Language'],
- distinct=True)
+ return Store.of(self).find(
+ Language,
+ POFile.languageID == Language.id,
+ Language.code != u'en',
+ POFile.potemplateID == self.id).config(distinct=True)
def getPOFileByPath(self, path):
"""See `IPOTemplate`."""
@@ -538,15 +526,10 @@
# the usual get() followed by "is None"; the dict may contain None
# values to indicate we looked for a POFile and found none.
return self._cached_pofiles_by_language[language_code]
-
- self._cached_pofiles_by_language[language_code] = POFile.selectOne("""
- POFile.potemplate = %d AND
- POFile.language = Language.id AND
- Language.code = %s
- """ % (self.id,
- quote(language_code)),
- clauseTables=['Language'])
-
+ self._cached_pofiles_by_language[language_code] = Store.of(self).find(
+ POFile,
+ POFile.potemplateID == self.id, POFile.languageID == Language.id,
+ Language.code == language_code).one()
return self._cached_pofiles_by_language[language_code]
def getOtherSidePOTemplate(self):
@@ -608,12 +591,6 @@
else:
pofile.unreviewedCount()
- def _null_quote(self, value):
- if value is None:
- return " IS NULL "
- else:
- return " = %s" % sqlvalues(value)
-
def _getPOTMsgSetBy(self, msgid_singular, msgid_plural=None, context=None,
sharing_templates=False):
"""Look for a POTMsgSet by msgid_singular, msgid_plural, context.
@@ -622,35 +599,23 @@
POTMsgSet, look through sharing templates as well.
"""
clauses = [
- 'TranslationTemplateItem.potmsgset = POTMsgSet.id',
+ TranslationTemplateItem.potmsgsetID == POTMsgSet.id,
+ POTMsgSet.msgid_singular == msgid_singular,
+ POTMsgSet.msgid_plural == msgid_plural,
+ POTMsgSet.context == context,
]
- clause_tables = ['TranslationTemplateItem']
if sharing_templates:
clauses.append(
- 'TranslationTemplateItem.potemplate in %s' % sqlvalues(
- self._sharing_ids))
- else:
- clauses.append(
- 'TranslationTemplateItem.potemplate = %s' % sqlvalues(self))
-
- clauses.append(
- 'POTMsgSet.msgid_singular %s' % self._null_quote(msgid_singular))
- clauses.append(
- 'POTMsgSet.msgid_plural %s' % self._null_quote(msgid_plural))
- clauses.append(
- 'POTMsgSet.context %s' % self._null_quote(context))
-
- result = POTMsgSet.select(
- ' AND '.join(clauses),
- clauseTables=clause_tables,
- # If there are multiple messages, make the one from the
- # current POTemplate be returned first.
- orderBy=['(TranslationTemplateItem.POTemplate<>%s)' % (
- sqlvalues(self))])[:2]
- if not result.is_empty():
- return result[0]
- else:
- return None
+ TranslationTemplateItem.potemplateID.is_in(self._sharing_ids))
+ else:
+ clauses.append(TranslationTemplateItem.potemplateID == self.id)
+
+ # If there are multiple messages, make the one from the
+ # current POTemplate be returned first.
+ order_by = SQL(
+ 'TranslationTemplateItem.potemplate <> ?', params=(self.id,))
+ return IStore(POTMsgSet).find(
+ POTMsgSet, *clauses).order_by(order_by).first()
def hasMessageID(self, msgid_singular, msgid_plural, context=None):
"""See `IPOTemplate`."""
@@ -660,11 +625,8 @@
def hasPluralMessage(self):
"""See `IPOTemplate`."""
clauses = self._getPOTMsgSetSelectionClauses()
- clauses.append(
- 'POTMsgSet.msgid_plural IS NOT NULL')
- return bool(POTMsgSet.select(
- ' AND '.join(clauses),
- clauseTables=['TranslationTemplateItem']))
+ clauses.append(POTMsgSet.msgid_plural != None)
+ return not Store.of(self).find(POTMsgSet, *clauses).is_empty()
def export(self):
"""See `IPOTemplate`."""
@@ -1294,13 +1256,6 @@
for potemplate in res:
yield potemplate
- def getByIDs(self, ids):
- """See `IPOTemplateSet`."""
- values = ",".join(sqlvalues(*ids))
- return POTemplate.select("POTemplate.id in (%s)" % values,
- prejoins=["productseries", "distroseries", "sourcepackagename"],
- orderBy=["POTemplate.id"])
-
def getAllByName(self, name):
"""See `IPOTemplateSet`."""
return POTemplate.selectBy(name=name, orderBy=['name', 'id'])
@@ -1401,7 +1356,7 @@
"""See `IPOTemplateSet`."""
rowcount = IMasterStore(POTemplate).execute(
"DELETE FROM SuggestivePOTemplate "
- "WHERE potemplate = %s" % sqlvalues(potemplate)).rowcount
+ "WHERE potemplate = ?", params=(potemplate.id,)).rowcount
return rowcount == 1
def populateSuggestivePOTemplatesCache(self):
@@ -1420,13 +1375,14 @@
Product.id = ProductSeries.product
WHERE
POTemplate.iscurrent AND (
- Distribution.translations_usage IN %(usage)s OR
- Product.translations_usage IN %(usage)s)
+ Distribution.translations_usage IN (?, ?) OR
+ Product.translations_usage IN (?, ?))
ORDER BY POTemplate.id
)
- """ % {
- 'usage': sqlvalues(
- ServiceUsage.LAUNCHPAD, ServiceUsage.EXTERNAL)}
+ """,
+ params=(
+ ServiceUsage.LAUNCHPAD.value, ServiceUsage.EXTERNAL.value,
+ ServiceUsage.LAUNCHPAD.value, ServiceUsage.EXTERNAL.value)
).rowcount
@@ -1569,8 +1525,8 @@
if name_pattern is None:
templatename_clause = True
else:
- templatename_clause = (
- "potemplate.name ~ %s" % sqlvalues(name_pattern))
+ templatename_clause = SQL(
+ "POTemplate.name ~ ?", params=(name_pattern,))
return self._queryPOTemplates(templatename_clause)
=== modified file 'lib/lp/translations/templates/potemplate-index.pt'
--- lib/lp/translations/templates/potemplate-index.pt 2012-08-06 06:09:19 +0000
+++ lib/lp/translations/templates/potemplate-index.pt 2013-10-21 06:11:14 +0000
@@ -83,7 +83,7 @@
<div class="yui-u">
<div class="portlet"
- tal:condition="context/relatives_by_source">
+ tal:condition="view/related_templates_by_source">
<h3>Related templates</h3>
<p id="potemplate-relatives">
<span>Other templates here:</span>
=== modified file 'lib/lp/translations/tests/test_potemplate.py'
--- lib/lp/translations/tests/test_potemplate.py 2012-11-01 15:35:53 +0000
+++ lib/lp/translations/tests/test_potemplate.py 2013-10-21 06:11:14 +0000
@@ -357,7 +357,7 @@
subset = getUtility(IPOTemplateSet).getSharingSubset(
distribution=self.ubuntu)
classes = subset.groupEquivalentPOTemplates(
- name_pattern='krungthepmahanakorn.*-etc')
+ name_pattern=u'krungthepmahanakorn.*-etc')
expected = {
(unique_name, self.package.name): [bangkok_template],
=== modified file 'lib/lp/translations/tests/test_shared_potemplate.py'
--- lib/lp/translations/tests/test_shared_potemplate.py 2013-01-07 02:40:55 +0000
+++ lib/lp/translations/tests/test_shared_potemplate.py 2013-10-21 06:11:14 +0000
@@ -242,13 +242,13 @@
# Baseline test.
self.assertContentEqual(
['foo', 'foo-bar', 'foo-two'],
- self._makeAndFind(['foo', 'foo-bar', 'foo-two'], 'foo.*'))
+ self._makeAndFind(['foo', 'foo-bar', 'foo-two'], u'foo.*'))
def test_getSharingPOTemplatesByRegex_not_all(self):
# A template may not match.
self.assertContentEqual(
['foo-bar', 'foo-two'],
- self._makeAndFind(['foo', 'foo-bar', 'foo-two'], 'foo-.*'))
+ self._makeAndFind(['foo', 'foo-bar', 'foo-two'], u'foo-.*'))
def test_getSharingPOTemplatesByRegex_all(self):
# Not passing a pattern returns all templates.
@@ -260,19 +260,19 @@
# A not matching pattern returns no templates.
self.assertContentEqual(
[],
- self._makeAndFind(['foo', 'foo-bar', 'foo-two'], "doo.+dle"))
+ self._makeAndFind(['foo', 'foo-bar', 'foo-two'], u"doo.+dle"))
def test_getSharingPOTemplatesByRegex_robustness_single_quotes(self):
# Single quotes do not confuse the regex match.
self.assertContentEqual(
[],
- self._makeAndFind(['foo', 'foo-bar', 'foo-two'], "'"))
+ self._makeAndFind(['foo', 'foo-bar', 'foo-two'], u"'"))
def test_getSharingPOTemplatesByRegex_robustness_double_quotes(self):
# Double quotes do not confuse the regex match.
self.assertContentEqual(
[],
- self._makeAndFind(['foo', 'foo-bar', 'foo-two'], '"'))
+ self._makeAndFind(['foo', 'foo-bar', 'foo-two'], u'"'))
def test_getSharingPOTemplatesByRegex_robustness_backslash(self):
# A backslash at the end could escape enclosing quotes without
@@ -282,7 +282,7 @@
product = self.factory.makeProduct()
subset = getUtility(IPOTemplateSet).getSharingSubset(product=product)
self.assertRaises(
- DataError, list, subset.getSharingPOTemplatesByRegex("foo.*\\"))
+ DataError, list, subset.getSharingPOTemplatesByRegex(u"foo.*\\"))
class TestMessageSharingProductPackage(TestCaseWithFactory):
=== modified file 'lib/lp/translations/translationmerger.py'
--- lib/lp/translations/translationmerger.py 2013-06-20 05:50:00 +0000
+++ lib/lp/translations/translationmerger.py 2013-10-21 06:11:14 +0000
@@ -274,7 +274,7 @@
product=product, distribution=distribution,
sourcepackagename=sourcepackagename)
equivalence_classes = subset.groupEquivalentPOTemplates(
- self.options.template_names)
+ self.options.template_names.decode('utf-8'))
class_count = len(equivalence_classes)
log.info("Merging %d template equivalence classes." % class_count)
References
