duplicate code and late evaluation doing translator license checks

[Jeroen T. Vermeulen]

Right now we have a smörgåsbord of places in the code that check for a person's relicensing agreement: canAddSuggestions checks twice, canEditTranslations calls canAddSuggestions then checks twice itself; then there's POFileBaseView.user_can_suggest and POFileBaseView.user_can_edit, BaseTranslationView.user_is_official_translator, and CurrentTranslationMessageView. I may have missed one.

So this information is queried from the database lots of times. The +translate page for n messages seems to query it 4n+1 times. It'd be nice if we could sweep these together a bit and cache them where possible (i.e. in browser code—launchbag perhaps?).

[Данило Шеган (danilo)]

I believe it's actually happening only a constant number of times. I've just checked and query was executed exactly 3 times on https://translations.edge.launchpad.net/ubuntu/lucid/+source/checkbox/+pots/checkbox/sr/+translate/++oops++

What makes you think it's an actual problem?

[Jeroen T. Vermeulen (jtv)]

When it comes to performance, it showed up over a hundred times in some timeouts I looked at, taking more than half a second of database time IIRC. Just now I picked an example I hadn't looked at before; that showed >3s of wasted time for this, but I believe that to be a fluke. OOPS-1528G2463

But I also believe the repetitive code a maintenance burden, which is harder to quantify.

Did you try it with an unprivileged user identity? Checks tend to get bypassed for rosetta admins.

[Robert Collins (lifeless)]

4. 107 3108 29 3079 launchpad-main-slave

SELECT TranslationRelicensingAgreement.allow_relicensing,
       TranslationRelicensingAgreement.date_decided,
       TranslationRelicensingAgreement.id,
       TranslationRelicensingAgreement.person
FROM TranslationRelicensingAgreement
WHERE TranslationRelicensingAgreement.person = %s

is the repetition referenced here - 107 lookups; the 29ms each is probably due to cross thread contention which we had lots of back then.

[Curtis Hovey (sinzui)]

Current oops is OOPS-aa85dfbfab513c425b1c8127d59bcfbf

I see that .translations_relicensing_agreement is a property composed of a setter and getter. The property is set exactly once in a view. We can make the getter a cachedproperty, and the setter is can clear the cache.

[Launchpad QA Bot (lpqabot)]

Fixed in stable r16171 <http://bazaar.launchpad.net/~launchpad-pqm/launchpad/stable/revision/16171>.