launchpad-reviewers team mailing list archive

[Ian Booth <ian.booth@xxxxxxxxxxxxx>]

Ian Booth has proposed merging lp:~wallyworld/launchpad/translator-licence-checks-531720 into lp:launchpad.

Commit message:
Add caching to some translations views.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #531720 in Launchpad itself: "duplicate code and late evaluation doing translator license checks"
  https://bugs.launchpad.net/launchpad/+bug/531720

For more details, see:
https://code.launchpad.net/~wallyworld/launchpad/translator-licence-checks-531720/+merge/130458

== Implementation ==

Add caching for the model TranslationsPerson translations_relicensing_agreement property.
I also found repeated calls from the TAL to some other properties on the POFileView which I cached.

== Tests ==

Internal changes - existing tests suffice.

== Lint ==

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

-- 
https://code.launchpad.net/~wallyworld/launchpad/translator-licence-checks-531720/+merge/130458
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~wallyworld/launchpad/translator-licence-checks-531720 into lp:launchpad.

=== modified file 'lib/lp/translations/browser/pofile.py'
--- lib/lp/translations/browser/pofile.py	2012-02-15 21:14:05 +0000
+++ lib/lp/translations/browser/pofile.py	2012-10-19 00:46:22 +0000
@@ -275,22 +275,22 @@
     def contributors(self):
         return list(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)
 
-    @property
+    @cachedproperty
     def has_translationgroup(self):
         """Is there a translation group for this translation?"""
         return self.context.potemplate.translationgroups
 
-    @property
+    @cachedproperty
     def is_managed(self):
         """Is a translation group member assigned to this translation?"""
         for group in self.context.potemplate.translationgroups:
@@ -298,7 +298,7 @@
                 return True
         return False
 
-    @property
+    @cachedproperty
     def managers(self):
         """List translation groups and translation teams for this translation.
 

=== modified file 'lib/lp/translations/model/translationsperson.py'
--- lib/lp/translations/model/translationsperson.py	2012-05-24 20:25:54 +0000
+++ lib/lp/translations/model/translationsperson.py	2012-10-19 00:46:22 +0000
@@ -32,6 +32,10 @@
 from lp.registry.model.projectgroup import ProjectGroup
 from lp.registry.model.teammembership import TeamParticipation
 from lp.services.database.sqlbase import sqlvalues
+from lp.services.propertycache import (
+    cachedproperty,
+    get_property_cache,
+    )
 from lp.services.worlddata.model.language import Language
 from lp.translations.enums import TranslationPermission
 from lp.translations.interfaces.translationgroup import ITranslationGroupSet
@@ -95,7 +99,8 @@
         """See `ITranslationsPerson`."""
         return getUtility(ITranslatorSet).getByTranslator(self.person)
 
-    def get_translations_relicensing_agreement(self):
+    @cachedproperty
+    def _translations_relicensing_agreement(self):
         """Return whether translator agrees to relicense their translations.
 
         If she has made no explicit decision yet, return None.
@@ -107,6 +112,9 @@
         else:
             return relicensing_agreement.allow_relicensing
 
+    def get_translations_relicensing_agreement(self):
+        return self._translations_relicensing_agreement
+
     def set_translations_relicensing_agreement(self, value):
         """Set a translations relicensing decision by translator.
 
@@ -120,6 +128,7 @@
                 allow_relicensing=value)
         else:
             relicensing_agreement.allow_relicensing = value
+        del get_property_cache(self)._translations_relicensing_agreement
 
     translations_relicensing_agreement = property(
         get_translations_relicensing_agreement,