Merge lp:~jtv/launchpad/bug-719267 into lp:launchpad

[Jeroen T. Vermeulen]

[r=lifeless][bug=719267] Fix timeouts on approval of new translations templates.

= Summary =

Timeouts were blocking translations import queue reviews, at least the creation of new templates. The time went into recalculating the statistics of each POFile that is created to match ones in sharing templates. Some large queries are involved.

== Proposed fix ==

A new template is created empty, so there's no need to query the database — all the counts that get constructed directly from the queries are zero.

I considered adding a parameter for "don't bother calculating the statistics," since the problem happens in a known scenario where the statistics don't need calculating. But in the end I just created some shortcuts: if the template is empty, the queries are skipped and zero counts are returned. This doesn't affect any APIs and may even benefit additional invocations.

== Pre-implementation notes ==

Robert looked into optimizing statistics calculation for multiple languages at once. The results are promising, though in this case there's no need to go to the trouble.

== Implementation details ==

The call that led to the unnecessary queries had an unnecessary line break. There is no other reason why I changed it.

== Tests ==

The statistics as calculated by the changed methods are extensively tested by:
{{{
./bin/test -vvc lp.translations.tests.test_pofile
}}}

I'm adding one for the zero case, just in case it's not properly covered: test_updateStatistics_counts_zero_for_empty_template.

== Demo and Q/A ==

Approve the oldest translation template uploads awaiting review. The requests should not time out.

Run cronscripts/rosetta-pofile-stats.py. This will take hours. Afterwards, POFile translation statistics will still be valid (and not all zeroed out as would happen if the change kicked in inappropriately).

= Launchpad lint =

Checking for conflicts and issues in changed files.

Linting changed files:
  lib/lp/translations/model/pofile.py
  lib/lp/translations/model/potemplate.py

[Robert Collins (lifeless)]

review: Approve

=== modified file 'lib/lp/translations/model/pofile.py'
--- lib/lp/translations/model/pofile.py	2011-01-26 23:33:20 +0000
+++ lib/lp/translations/model/pofile.py	2011-02-21 20:08:53 +0000
@@ -825,6 +825,12 @@
 
     def _countTranslations(self):
         """Count `currentcount`, `updatescount`, and `rosettacount`."""
+        if self.potemplate.messageCount() == 0:
+            # Shortcut: if the template is empty, as it is when it is
+            # first created, we know the answers without querying the
+            # database.
+            return 0, 0, 0
+
         side_traits = getUtility(ITranslationSideTraitsSet).getForTemplate(
             self.potemplate)
         has_other_msgstrs = "COALESCE(%s) IS NOT NULL" % ", ".join([
@@ -889,6 +895,12 @@
 
     def _countNewSuggestions(self):
         """Count messages with new suggestions."""
+        if self.potemplate.messageCount() == 0:
+            # Shortcut: if the template is empty, as it is when it is
+            # first created, we know the answers without querying the
+            # database.
+            return 0
+
         flag_name = getUtility(ITranslationSideTraitsSet).getForTemplate(
             self.potemplate).flag_name
         suggestion_nonempty = "COALESCE(%s) IS NOT NULL" % ', '.join([
 
=== modified file 'lib/lp/translations/model/potemplate.py'
--- lib/lp/translations/model/potemplate.py	2011-02-14 17:14:17 +0000
+++ lib/lp/translations/model/potemplate.py	2011-02-21 20:08:53 +0000
@@ -1155,8 +1155,7 @@
             if shared_template is template:
                 continue
             for pofile in shared_template.pofiles:
-                template.newPOFile(
-                    pofile.language.code, False)
+                template.newPOFile(pofile.language.code, create_sharing=False)
             # Do not continue, else it would trigger an existingpo assertion.
             return
 
 
=== modified file 'lib/lp/translations/tests/test_pofile.py'
--- lib/lp/translations/tests/test_pofile.py	2011-02-17 22:06:02 +0000
+++ lib/lp/translations/tests/test_pofile.py	2011-02-21 20:08:53 +0000
@@ -760,6 +760,18 @@
         # A `POFile` starts out with consistent statistics.
         self.assertTrue(self.factory.makePOFile().testStatistics())
 
+    def test_updateStatistics_counts_zero_for_empty_template(self):
+        # Statistics for an empty template are all calculated as zero.
+        pofile = self.factory.makePOFile()
+        pofile.updateStatistics()
+        self.assertEquals(0, self.devel_pofile.messageCount())
+        self.assertEquals(0, self.devel_pofile.translatedCount())
+        self.assertEquals(0, self.devel_pofile.untranslatedCount())
+        self.assertEquals(0, self.devel_pofile.currentCount())
+        self.assertEquals(0, self.devel_pofile.rosettaCount())
+        self.assertEquals(0, self.devel_pofile.updatesCount())
+        self.assertEquals(0, self.devel_pofile.unreviewedCount())
+
     def test_updateStatistics(self):
         # Test that updating statistics keeps working.