Launchpad itself

Merge lp:~jtv/launchpad/bug-994410-stop-using-latest_message into lp:launchpad

  1. bug-994410-stop-using-latest_message
  2. Merge into devel
Proposed by Jeroen T. Vermeulen
Status: Merged
Approved by: Jeroen T. Vermeulen
Approved revision: no longer in the source branch.
Merged at revision: 15218
Proposed branch: lp:~jtv/launchpad/bug-994410-stop-using-latest_message
Merge into: lp:launchpad
Diff against target: 424 lines (+75/-202)
7 files modified
lib/lp/translations/doc/translationmessage-destroy.txt (+4/-11)
lib/lp/translations/interfaces/pofiletranslator.py (+6/-10)
lib/lp/translations/model/pofiletranslator.py (+7/-15)
lib/lp/translations/tests/pofiletranslator.txt (+0/-99)
lib/lp/translations/tests/test_pofiletranslator.py (+54/-8)
lib/lp/translations/tests/test_translationmerger.py (+0/-55)
lib/lp/translations/translationmerger.py (+4/-4)
To merge this branch: bzr merge lp:~jtv/launchpad/bug-994410-stop-using-latest_message
Reviewer Review Type Date Requested Status
Stuart Bishop (community) Approve
Review via email: mp+104703@code.launchpad.net

Commit message

Stop using POFileTranslator.latest_message.

Description of the change

This field in POFileTranslator can be costly to compute, and can restrict the way we move records around in scripts, but it's only used in one place and not a critical one at that: the translations-merging script tries to fold POFileTranslator records together when merging translations. An approximation of the correct move is sort of good enough. The POFileTranslator table isn't very exact.

More precision for POFileTranslator would be nice, but if that's what we want, we have much bigger fish to fry. Such as the breakage of the Ubuntu translations-copying script, which we risk exactly because of the need to maintain POFileTranslator, and which in a twist of irony caused us to lose a bunch of records in the Precise opening. We should maintain its integrity asynchronously with a scrubber script later (so that it's still inexact but at least *eventually* consistent) and drop the database trigger and other hoop-jumping that we do to keep this table in shape.

This branch contains no lint (you may notice I removed some), it updates copyright notices, and it passes all translations tests.

Jeroen

To post a comment you must log in.
Revision history for this message
Stuart Bishop (stub) wrote :

Looks fine.

review: Approve
Revision history for this message
Jeroen T. Vermeulen (jtv) wrote :

Came through EC2 without problems. Now trying to make it land.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'lib/lp/translations/doc/translationmessage-destroy.txt'
--- lib/lp/translations/doc/translationmessage-destroy.txt 2012-01-20 15:42:44 +0000
+++ lib/lp/translations/doc/translationmessage-destroy.txt 2012-05-08 04:45:29 +0000
@@ -33,8 +33,8 @@
33In two sharing POTemplates with one shared POTMsgSet with one shared33In two sharing POTemplates with one shared POTMsgSet with one shared
34translation, we get two POFileTranslator records for each of the POFiles.34translation, we get two POFileTranslator records for each of the POFiles.
3535
36 >>> # We need to be able to create persons and projects so let's just use36 # We need to be able to create persons and projects so let's just use
37 >>> # a global 'postgres' permission which allows everything.37 # a global 'postgres' permission which allows everything.
38 >>> switch_dbuser('postgres')38 >>> switch_dbuser('postgres')
39 >>> from lp.services.database.sqlbase import sqlvalues39 >>> from lp.services.database.sqlbase import sqlvalues
40 >>> from lp.app.enums import ServiceUsage40 >>> from lp.app.enums import ServiceUsage
@@ -63,13 +63,6 @@
63 ... potmsgset=potmsgset,63 ... potmsgset=potmsgset,
64 ... translations=[u"blah"])64 ... translations=[u"blah"])
65 >>> print POFileTranslator.select(65 >>> print POFileTranslator.select(
66 ... "latest_message=%s" % sqlvalues(tm)).count()66 ... "pofile IN (%s, %s)"
67 ... % sqlvalues(devel_sr_pofile, stable_sr_pofile)).count()
67 268 2
68
69Removing a translation message triggers POFileTranslator row removal as well.
70
71 >>> tm.destroySelf()
72 >>> print list(POFileTranslator.select(
73 ... "latest_message=%s" % sqlvalues(tm)))
74 []
75
7669
=== modified file 'lib/lp/translations/interfaces/pofiletranslator.py'
--- lib/lp/translations/interfaces/pofiletranslator.py 2011-12-24 16:54:44 +0000
+++ lib/lp/translations/interfaces/pofiletranslator.py 2012-05-08 04:45:29 +0000
@@ -1,4 +1,4 @@
1# Copyright 2009 Canonical Ltd. This software is licensed under the1# Copyright 2009-2012 Canonical Ltd. This software is licensed under the
2# GNU Affero General Public License version 3 (see the file LICENSE).2# GNU Affero General Public License version 3 (see the file LICENSE).
33
4# pylint: disable-msg=E02134# pylint: disable-msg=E0213
@@ -21,7 +21,6 @@
21from lp import _21from lp import _
22from lp.registry.interfaces.person import IPerson22from lp.registry.interfaces.person import IPerson
23from lp.translations.interfaces.pofile import IPOFile23from lp.translations.interfaces.pofile import IPOFile
24from lp.translations.interfaces.translationmessage import ITranslationMessage
2524
2625
27class IPOFileTranslator(Interface):26class IPOFileTranslator(Interface):
@@ -37,10 +36,6 @@
37 title=_(u"The `POFile` modified by the translator."), required=True,36 title=_(u"The `POFile` modified by the translator."), required=True,
38 readonly=True, schema=IPOFile)37 readonly=True, schema=IPOFile)
3938
40 latest_message = Object(
41 title=_(u"Latest `TranslationMessage` for this person and file."),
42 required=True, readonly=True, schema=ITranslationMessage)
43
44 date_last_touched = Datetime(39 date_last_touched = Datetime(
45 title=_(u"When the latest translation message was added."),40 title=_(u"When the latest translation message was added."),
46 required=True, readonly=True)41 required=True, readonly=True)
@@ -69,9 +64,10 @@
69 person and pofile, or None.64 person and pofile, or None.
70 """65 """
7166
72 def getForPOTMsgSet(potmsgset):67 def getForTemplate(potemplate):
73 """Retrieve `POFileTranslator`s for translations of `potmsgset`.68 """Retrieve `POFileTranslator` objects associated iwth `POTemplate`.
7469
75 :return: a query result of `POFileTranslator`s whose70 :param potemplate: `POTemplate` to look for.
76 `latest_message` are translations of `potmsgset`.71 :return: Result set of `POFileTranslator` records associated with
72 `POFile`s that translate `potemplate`.
77 """73 """
7874
=== modified file 'lib/lp/translations/model/pofiletranslator.py'
--- lib/lp/translations/model/pofiletranslator.py 2011-12-30 06:14:56 +0000
+++ lib/lp/translations/model/pofiletranslator.py 2012-05-08 04:45:29 +0000
@@ -1,4 +1,4 @@
1# Copyright 2009 Canonical Ltd. This software is licensed under the1# Copyright 2009-2012 Canonical Ltd. This software is licensed under the
2# GNU Affero General Public License version 3 (see the file LICENSE).2# GNU Affero General Public License version 3 (see the file LICENSE).
33
4__metaclass__ = type4__metaclass__ = type
@@ -23,7 +23,7 @@
23 IPOFileTranslator,23 IPOFileTranslator,
24 IPOFileTranslatorSet,24 IPOFileTranslatorSet,
25 )25 )
26from lp.translations.model.translationmessage import TranslationMessage26from lp.translations.model.pofile import POFile
2727
2828
29class POFileTranslator(SQLBase):29class POFileTranslator(SQLBase):
@@ -34,9 +34,6 @@
34 person = ForeignKey(34 person = ForeignKey(
35 dbName='person', foreignKey='Person',35 dbName='person', foreignKey='Person',
36 storm_validator=validate_public_person, notNull=True)36 storm_validator=validate_public_person, notNull=True)
37 latest_message = ForeignKey(
38 foreignKey='TranslationMessage', dbName='latest_message',
39 notNull=True)
40 date_last_touched = UtcDateTimeCol(37 date_last_touched = UtcDateTimeCol(
41 dbName='date_last_touched', notNull=False, default=None)38 dbName='date_last_touched', notNull=False, default=None)
4239
@@ -62,10 +59,6 @@
62 'pofile.potemplate.productseries.product',59 'pofile.potemplate.productseries.product',
63 'pofile.potemplate.distroseries',60 'pofile.potemplate.distroseries',
64 'pofile.potemplate.sourcepackagename',61 'pofile.potemplate.sourcepackagename',
65 'latest_message',
66 'latest_message.potmsgset',
67 'latest_message.potmsgset.msgid_singular',
68 'latest_message.msgstr0',
69 ]))62 ]))
7063
71 def getForPersonPOFile(self, person, pofile):64 def getForPersonPOFile(self, person, pofile):
@@ -74,10 +67,9 @@
74 POFileTranslator.person == person.id,67 POFileTranslator.person == person.id,
75 POFileTranslator.pofile == pofile.id)).one()68 POFileTranslator.pofile == pofile.id)).one()
7669
77 def getForPOTMsgSet(self, potmsgset):70 def getForTemplate(self, potemplate):
78 """See `IPOFileTranslatorSet`."""71 """See `IPOFileTranslatorSet`."""
79 store = Store.of(potmsgset)72 return Store.of(potemplate).find(
80 match = And(73 POFileTranslator,
81 POFileTranslator.latest_message == TranslationMessage.id,74 POFileTranslator.pofileID == POFile.id,
82 TranslationMessage.potmsgset == potmsgset)75 POFile.potemplateID == potemplate.id)
83 return store.find(POFileTranslator, match).config(distinct=True)
8476
=== removed file 'lib/lp/translations/tests/pofiletranslator.txt'
--- lib/lp/translations/tests/pofiletranslator.txt 2011-12-30 06:14:56 +0000
+++ lib/lp/translations/tests/pofiletranslator.txt 1970-01-01 00:00:00 +0000
@@ -1,99 +0,0 @@
1The POFileTranslator table is an eager materialized view, containing
2references to the most recent posubmission made by a particular person
3to a particular pofile.
4
5This table is read only to most users, but for this test we connect
6as a superuser so we can poke at it directly.
7
8 >>> from lp.services.database.sqlbase import connect
9 >>> connection = connect(user='testadmin')
10 >>> cur = connection.cursor()
11 >>> def pofiletranslator(person_id, pofile_id):
12 ... cur.execute("""
13 ... SELECT latest_message, date_last_touched
14 ... FROM POFileTranslator
15 ... WHERE person = %(person_id)s AND pofile = %(pofile_id)s
16 ... """, vars())
17 ... result = cur.fetchone()
18 ... if result is None:
19 ... return None
20 ... return list(result)
21
22 >>> stub_id = 22
23 >>> pofile_id = 1
24 >>> language_id = 387
25 >>> potmsgset_id = 1
26
27Note that our oldest TranslationMessage belongs to pofile #1.
28
29Stub has so far not translated anything in this pofile
30
31 >>> pofiletranslator(stub_id, pofile_id) is None
32 True
33
34If we add a message, the cache is updated
35
36 >>> cur.execute("""
37 ... INSERT INTO TranslationMessage(
38 ... potmsgset, msgstr1, origin, submitter, language
39 ... ) VALUES (
40 ... %(potmsgset_id)s, 1, 1, %(stub_id)s, %(language_id)s)
41 ... """, vars())
42 >>> posubmission_id, date_touched = pofiletranslator(stub_id, pofile_id)
43
44
45We now set the last touched timestamp into the past so we can detect that
46it is correctly updated. This is only possible because we are connected to
47the database as the testadmin user.
48
49 >>> cur.execute("""
50 ... UPDATE POFileTranslator
51 ... SET date_last_touched = CURRENT_TIMESTAMP - '1 day'::interval
52 ... WHERE person=%(stub_id)s AND pofile=%(pofile_id)s
53 ... """, vars())
54
55
56If we add a new submission to that pofile, the cache is updated including
57the last touched timestamp.
58
59 >>> cur.execute("""
60 ... INSERT INTO TranslationMessage(
61 ... potmsgset, msgstr1, origin, submitter, language
62 ... ) VALUES (
63 ... %(potmsgset_id)s, 2, 1, %(stub_id)s, %(language_id)s)
64 ... """, vars())
65 >>> new_posubmission_id, new_date_touched = pofiletranslator(
66 ... stub_id, pofile_id)
67 >>> posubmission_id == new_posubmission_id
68 False
69
70
71We should get the same timestamp as before, despite having updated the
72cache manually, as we are in the same database transaction and
73CURRENT_TIMESTAMP will return a constant value throughout the transaction
74and the triggers will have reset the value in the cache.
75
76 >>> date_touched == new_date_touched
77 True
78
79
80If we update the submissin, the cache is updated
81
82 >>> mark_id = 1
83 >>> cur.execute("""
84 ... UPDATE TranslationMessage SET submitter=%(mark_id)s
85 ... WHERE id = %(new_posubmission_id)s
86 ... """, vars())
87 >>> mark_posubmission_id, date_touched = pofiletranslator(
88 ... mark_id, pofile_id)
89 >>> stub_posubmission_id, date_touched = pofiletranslator(
90 ... stub_id, pofile_id)
91 >>> mark_posubmission_id == stub_posubmission_id
92 False
93
94
95The trigger was smart enough to locate the previous submission from stub
96
97 >>> stub_posubmission_id == posubmission_id
98 True
99
1000
=== modified file 'lib/lp/translations/tests/test_pofiletranslator.py'
--- lib/lp/translations/tests/test_pofiletranslator.py 2011-12-28 17:03:06 +0000
+++ lib/lp/translations/tests/test_pofiletranslator.py 2012-05-08 04:45:29 +0000
@@ -1,14 +1,60 @@
1# Copyright 2009 Canonical Ltd. This software is licensed under the1# Copyright 2009-2012 Canonical Ltd. This software is licensed under the
2# GNU Affero General Public License version 3 (see the file LICENSE).2# GNU Affero General Public License version 3 (see the file LICENSE).
33
4"""Runs the POFileTranslator test."""4"""Runs the POFileTranslator test."""
55
6__metaclass__ = type6__metaclass__ = type
77
8from lp.testing.layers import DatabaseLayer8from zope.component import getUtility
9from lp.testing.systemdocs import LayeredDocFileSuite9
1010from lp.testing import TestCaseWithFactory
1111from lp.testing.layers import ZopelessDatabaseLayer
12def test_suite():12from lp.translations.interfaces.pofiletranslator import IPOFileTranslatorSet
13 return LayeredDocFileSuite(13
14 'pofiletranslator.txt', layer=DatabaseLayer)14
15class TestPOFileTranslator(TestCaseWithFactory):
16 layer = ZopelessDatabaseLayer
17
18 def test_getForPersonPOFile_returns_None_if_not_found(self):
19 self.assertIsNone(
20 getUtility(IPOFileTranslatorSet).getForPersonPOFile(
21 self.factory.makePerson(), self.factory.makePOFile()))
22
23 def test_getForPersonPOFile_finds_record(self):
24 pofile = self.factory.makePOFile()
25 potmsgset = self.factory.makePOTMsgSet(pofile.potemplate)
26 tm = self.factory.makeCurrentTranslationMessage(
27 potmsgset=potmsgset, language=pofile.language)
28 poft = getUtility(IPOFileTranslatorSet).getForPersonPOFile(
29 tm.submitter, pofile)
30 self.assertEqual(pofile, poft.pofile)
31 self.assertEqual(tm.submitter, poft.person)
32
33 def test_getForPersonPOFile_ignores_other_persons(self):
34 pofile = self.factory.makePOFile()
35 potmsgset = self.factory.makePOTMsgSet(pofile.potemplate)
36 self.factory.makeCurrentTranslationMessage(
37 potmsgset=potmsgset, language=pofile.language)
38 self.assertIsNone(
39 getUtility(IPOFileTranslatorSet).getForPersonPOFile(
40 self.factory.makePerson(), pofile))
41
42 def test_getForPersonPOFile_ignores_other_POFiles(self):
43 pofile = self.factory.makePOFile('nl')
44 potmsgset = self.factory.makePOTMsgSet(pofile.potemplate)
45 tm = self.factory.makeCurrentTranslationMessage(
46 potmsgset=potmsgset, language=pofile.language)
47 other_pofile = self.factory.makePOFile('de', pofile.potemplate)
48 self.assertIsNone(
49 getUtility(IPOFileTranslatorSet).getForPersonPOFile(
50 tm.submitter, other_pofile))
51
52 def test_getForTemplate_finds_all_for_template(self):
53 pofile = self.factory.makePOFile()
54 potmsgset = self.factory.makePOTMsgSet(pofile.potemplate)
55 tm = self.factory.makeCurrentTranslationMessage(
56 potmsgset=potmsgset, language=pofile.language)
57 [poft] = list(
58 getUtility(IPOFileTranslatorSet).getForTemplate(pofile.potemplate))
59 self.assertEqual(pofile.potemplate, poft.pofile.potemplate)
60 self.assertEqual(tm.submitter, poft.person)
1561
=== modified file 'lib/lp/translations/tests/test_translationmerger.py'
--- lib/lp/translations/tests/test_translationmerger.py 2012-04-27 15:02:49 +0000
+++ lib/lp/translations/tests/test_translationmerger.py 2012-05-08 04:45:29 +0000
@@ -3,7 +3,6 @@
33
4__metaclass__ = type4__metaclass__ = type
55
6from datetime import timedelta
7import gc6import gc
8from logging import ERROR7from logging import ERROR
98
@@ -20,7 +19,6 @@
20 )19 )
21from lp.testing.layers import LaunchpadZopelessLayer20from lp.testing.layers import LaunchpadZopelessLayer
22from lp.testing.sampledata import ADMIN_EMAIL21from lp.testing.sampledata import ADMIN_EMAIL
23from lp.translations.interfaces.pofiletranslator import IPOFileTranslatorSet
24from lp.translations.model.pomsgid import POMsgID22from lp.translations.model.pomsgid import POMsgID
25from lp.translations.model.potemplate import POTemplate23from lp.translations.model.potemplate import POTemplate
26from lp.translations.model.potranslation import POTranslation24from lp.translations.model.potranslation import POTranslation
@@ -510,59 +508,6 @@
510 tms = trunk_message.potmsgset.getAllTranslationMessages()508 tms = trunk_message.potmsgset.getAllTranslationMessages()
511 self.assertEqual(list(tms), [trunk_message])509 self.assertEqual(list(tms), [trunk_message])
512510
513 # XXX: GavinPanella 2011-10-28 bug=883274: Spurious failure in buildbot.
514 def disabled_test_clashingPOFileTranslatorEntries(self):
515 # POFileTranslator is maintained by a trigger on
516 # TranslationMessage. Fiddling with TranslationTemplateItems
517 # directly bypasses it, so the script must make sure that
518 # POFileTranslator respects its unique constraints.
519
520 # In this scenario, "trunk" has a TranslationMessage with a
521 # matching POFileTranslator entry. This message is happy where
522 # it is; it's not changing in any way during the test.
523 poftset = getUtility(IPOFileTranslatorSet)
524
525 translator = self.trunk_template.owner
526 self.trunk_pofile.owner = translator
527 self.stable_pofile.owner = translator
528
529 contented_potmsgset = self.factory.makePOTMsgSet(
530 self.trunk_template, singular='snut', sequence=2)
531 contented_message = self._makeTranslationMessage(
532 self.trunk_pofile, contented_potmsgset, 'druf', False)
533 self.assertEqual(translator, contented_message.submitter)
534 poft = poftset.getForPersonPOFile(translator, self.trunk_pofile)
535 self.assertEqual(poft.latest_message, contented_message)
536
537 # Then there's the pair of POTMsgSets that are identical between
538 # trunk and stable. This one is translated only in stable.
539 # Merging will transfer that TranslationMessage from
540 # stable to trunk (where it becomes the shared message) through
541 # direct manipulation of TranslationTemplateItem.
542 stable_message = self._makeTranslationMessage(
543 self.stable_pofile, self.stable_potmsgset, 'fulb', False)
544 self.assertEqual(
545 stable_message.submitter, contented_message.submitter)
546
547 stable_message = removeSecurityProxy(stable_message)
548
549 # As it happens, this message is more recent than the happy one.
550 # This doesn't matter except it makes our test more predictable.
551 stable_message.date_created += timedelta(0, 0, 1)
552 poft = poftset.getForPersonPOFile(translator, self.stable_pofile)
553 self.assertEqual(poft.latest_message, stable_message)
554 removeSecurityProxy(poft).date_last_touched = (
555 stable_message.date_created)
556
557 # Now the migration script runs. This also carries the
558 # POFileTranslator record for stable_message into trunk_pofile.
559 # The one for contented_message disappears in the process.
560 self.merger.mergePOTMsgSets()
561 self.merger.mergeTranslationMessages()
562
563 poft = poftset.getForPersonPOFile(translator, self.trunk_pofile)
564 self.assertEqual(poft.latest_message, stable_message)
565
566511
567class TestRemoveDuplicates(TestCaseWithFactory, TranslatedProductMixin):512class TestRemoveDuplicates(TestCaseWithFactory, TranslatedProductMixin):
568 """Test _scrubPOTMsgSetTranslations and friends."""513 """Test _scrubPOTMsgSetTranslations and friends."""
569514
=== modified file 'lib/lp/translations/translationmerger.py'
--- lib/lp/translations/translationmerger.py 2012-01-01 02:58:52 +0000
+++ lib/lp/translations/translationmerger.py 2012-05-08 04:45:29 +0000
@@ -1,4 +1,4 @@
1# Copyright 2009-2011 Canonical Ltd. This software is licensed under the1# Copyright 2009-2012 Canonical Ltd. This software is licensed under the
2# GNU Affero General Public License version 3 (see the file LICENSE).2# GNU Affero General Public License version 3 (see the file LICENSE).
33
4__metaclass__ = type4__metaclass__ = type
@@ -56,7 +56,7 @@
56 potmsgset.context)56 potmsgset.context)
5757
5858
59def merge_pofiletranslators(from_potmsgset, to_template):59def merge_pofiletranslators(from_template, to_template):
60 """Merge POFileTranslator entries from one template into another.60 """Merge POFileTranslator entries from one template into another.
61 """61 """
62 # Import here to avoid circular import.62 # Import here to avoid circular import.
@@ -64,7 +64,7 @@
64 IPOFileTranslatorSet)64 IPOFileTranslatorSet)
6565
66 pofiletranslatorset = getUtility(IPOFileTranslatorSet)66 pofiletranslatorset = getUtility(IPOFileTranslatorSet)
67 affected_rows = pofiletranslatorset.getForPOTMsgSet(from_potmsgset)67 affected_rows = pofiletranslatorset.getForTemplate(from_template)
68 for pofiletranslator in affected_rows:68 for pofiletranslator in affected_rows:
69 person = pofiletranslator.person69 person = pofiletranslator.person
70 from_pofile = pofiletranslator.pofile70 from_pofile = pofiletranslator.pofile
@@ -112,7 +112,7 @@
112 item.potmsgset = representative112 item.potmsgset = representative
113 templates.add(item.potemplate)113 templates.add(item.potemplate)
114114
115 merge_pofiletranslators(item.potmsgset, representative_template)115 merge_pofiletranslators(item.potemplate, representative_template)
116116
117117
118def filter_clashes(clashing_ubuntu, clashing_upstream, twin):118def filter_clashes(clashing_ubuntu, clashing_upstream, twin):