Page MenuHomePhabricator

QuickSurveys\Hooks::onResourceLoaderRegisterModules should not initialise LocalisationCache
Closed, ResolvedPublic

Description

If needed then we may need to find a way that performs well (e.g. using Memcached/WANObjectCache with a check key that is purged whenever a survey is changed or something).

However from what I can see, no information from localisation cache influences the decision it is trying to make in onResourceLoaderRegisterModules, which is the call to isEnabled(). It's just that it parses the message in the class constructor instead of where it is needed.

Suggestions:

  • Create the Message object in the constructor, but don't call text() yet.
  • Change isInsecure to a lazy-initialised property with a getter method.

https://github.com/wikimedia/mediawiki-extensions-QuickSurveys/blob/aeadd78eb3fde974314139ee8207e736e06c241e/includes/ExternalSurvey.php#L50

Event Timeline

Krinkle created this task.Mar 6 2017, 8:14 PM
Krinkle updated the task description. (Show Details)
Krinkle added subscribers: phuedx, JGirault, Jdlrobson.
Krinkle updated the task description. (Show Details)Mar 6 2017, 8:17 PM

Change 341391 had a related patch set uploaded (by krinkle):
[mediawiki/extensions/QuickSurveys] Defer wfMessage fetch for ExternalSurvey::isInsecure

https://gerrit.wikimedia.org/r/341391

Krinkle claimed this task.Mar 6 2017, 8:42 PM
Krinkle added a project: Performance-Team.
Krinkle moved this task from Inbox to Radar on the Performance-Team board.
pmiazga moved this task from Incoming to 2014-15 Q4 on the Readers-Web-Backlog board.
Krinkle removed Krinkle as the assignee of this task.Mar 7 2017, 5:23 AM

Change 341391 merged by jenkins-bot:
[mediawiki/extensions/QuickSurveys] Defer wfMessage fetch for ExternalSurvey::isInsecure

https://gerrit.wikimedia.org/r/341391

Jdlrobson closed this task as Resolved.Mar 7 2017, 7:16 PM
Jdlrobson claimed this task.

Patch was merged