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).

a.png (846×1 px, 88 KB)

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 updated the task description. (Show Details)
Krinkle added subscribers: phuedx, JGirault, Jdlrobson.

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

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

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

Jdlrobson claimed this task.

Patch was merged