Merge lp:~jtv/launchpad/cache-pofile-view-permissions into lp:launchpad

[Jeroen T. Vermeulen]

[r=adeuring][ui=none][bug=297136] Cache permissions across messages on POFile:+translate pages.

= Bug 297136 =

This should strip a few dozen pointlessly repeated queries off the performance-critical POFile:+translate pages. I came across the omissions while working on something else. The queries don't cost that much time, but we'll often see around a hundred of them on a single request and there's just no good reason for that.

My branch effectively hoists a few very similar checks from the individual message views up into the surrounding POFile page view (if any). The checks all depend on the relationship between the user and the POFile, and global system state. Neither will ever differ between messages on the same translate page.

(There's also a very minor cleanup of list handling at the end).

[Abel Deuring (adeuring)]

(10:52:35) adeuring: jtv: IN line 37 of the diff, you run "if self.form_is_writeable:" for each loop iteration. I think you can move the "if" outside the loop
(10:53:06) jtv: adeuring: true… it's a cachedproperty, but with this short a loop I guess that makes sense.
(10:53:41) adeuring: jtv: yeah, that's a real micro-optimisation...
(10:53:42) ***jtv looks again
(10:53:59) jtv: Heh, I had missed how completely nonsensical the loop becomes in one of the two cases. :)
(10:54:50) adeuring: ;)
(10:54:57) jtv: And now it becomes clear that this is just one big list comprehension.
(10:55:48) adeuring: jtv: perhaps I had not have yet enogh coffee -- but where did the "alt_current.setPOFile(alt_pofile)" from line 49 go to? Or is it simply unnecessary?
(10:58:15) jtv: adeuring: that's what made me clean this up—alt_current goes into alt_submissions, which is really just [alt_current] + alt_external, so at the end I just have to loop over alt_submissions instead of alt_external.
(10:58:44) adeuring: jtv: ah, thanks! r=me, then.

review: Approve (code)

[Abel Deuring (adeuring)]

very nice!

review: Approve (code)

=== modified file 'lib/lp/translations/browser/pofile.py'
--- lib/lp/translations/browser/pofile.py	2010-11-16 13:10:43 +0000
+++ lib/lp/translations/browser/pofile.py	2010-11-26 10:03:59 +0000
@@ -289,12 +289,12 @@
     def contributors(self):
         return tuple(self.context.contributors)
 
-    @property
+    @cachedproperty
     def user_can_edit(self):
         """Does the user have full edit rights for this translation?"""
         return self.context.canEditTranslations(self.user)
 
-    @property
+    @cachedproperty
     def user_can_suggest(self):
         """Is the user allowed to make suggestions here?"""
         return self.context.canAddSuggestions(self.user)
@@ -824,11 +824,11 @@
 
     def _buildTranslationMessageViews(self, for_potmsgsets):
         """Build translation message views for all potmsgsets given."""
+        can_edit = self.context.canEditTranslations(self.user)
         for potmsgset in for_potmsgsets:
             translationmessage = (
                 potmsgset.getCurrentTranslationMessageOrDummy(self.context))
             error = self.errors.get(potmsgset)
-            can_edit = self.context.canEditTranslations(self.user)
 
             view = self._prepareView(
                 CurrentTranslationMessageView, translationmessage,
@@ -984,11 +984,11 @@
 
     def _messages_html_id(self):
         order = []
-        for message in self.translationmessage_views:
-            if (message.form_is_writeable):
-                for dictionary in message.translation_dictionaries:
-                    order.append(
-                        dictionary['html_id_translation'] + '_new')
+        if self.form_is_writeable:
+            for message in self.translationmessage_views:
+                order += [
+                    dictionary['html_id_translation'] + '_new'
+                    for dictionary in message.translation_dictionaries]
         return order
 
     @property

=== modified file 'lib/lp/translations/browser/translationmessage.py'
--- lib/lp/translations/browser/translationmessage.py	2010-10-28 09:54:48 +0000
+++ lib/lp/translations/browser/translationmessage.py	2010-11-26 10:03:59 +0000
@@ -1236,14 +1236,12 @@
             alt_current = potmsgset.getCurrentTranslationMessage(
                 self.pofile.potemplate, self.sec_lang)
             if alt_current is not None:
-                alt_current.setPOFile(alt_pofile)
-            if alt_current is not None:
                 alt_submissions.append(alt_current)
             alt_external = list(
                 potmsgset.getExternallyUsedTranslationMessages(self.sec_lang))
-            for suggestion in alt_external:
+            alt_submissions.extend(alt_external)
+            for suggestion in alt_submissions:
                 suggestion.setPOFile(alt_pofile)
-            alt_submissions.extend(alt_external)
             alt_title = self.sec_lang.englishname
 
         # To maintain compatibility with the old DB model as much as possible,