Page MenuHomePhabricator

A cached server-side HTML template should update when you change a partial template which it includes
Open, LowPublic

Description

https://gerrit.wikimedia.org/r/#/c/223165/ has a top-level template Skin.mustache that includes the partial template sidebar.mustache using {{>sidebar}}.

I notice on my local wiki if I edit Skin.mustache and view a page with the skin, I see the change to it (good!), but if I edit the partial sidebar.mustache, I don't see the change to it, even if I use ?action=purge (bug!).

Looking at includes/TemplateParser.php in core, it does a simple

// Read the template file
$fileContents = file_get_contents( $filename );

// Generate a quick hash for cache invalidation
$fastHash = md5( $fileContents );

this doesn't notice changes to partials included by $filename.

(This is probably hard to fix. A workaround is to make cosmetic changes to all parent templates that include the edited partial template. The bug and workaround is documented in https://www.mediawiki.org/wiki/Manual:HTML_templates#Caching.)

Event Timeline

Spage updated the task description. (Show Details)
Spage raised the priority of this task from to Needs Triage.
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptSep 18 2015, 8:35 PM

Less solved this issue with imports. @ori do you remember how we did that?

Jdlrobson triaged this task as Normal priority.

May need to write a spike to explore ways to fix this.

ovasileva lowered the priority of this task from Normal to Low.Sep 28 2016, 4:45 PM
ovasileva added a subscriber: ovasileva.

@Jdlrobson - wondering why you moved this to needs analysis, seems we have a current solution - are there any other news around this?

Someone should go talk to @ori or someone who has similar knowledge (@Krinkle? @EBernhardson) about how we do this for LESS files (which has a similar problem) and flesh this card out with an actionable plan.

Krinkle added a comment.EditedSep 28 2016, 6:50 PM

For LESS, we cache both the CSS output of the compiler and the meta data generated by the compilation process. Part of that meta data is a list of related files. Related files being, other Less files parsed by run-time imports, as well as any mentioned image files (for CSSMin embed).

Then, when considering to use the cached output, we validate the mtime of the main Less file (the entry point), as well as the mtimes of the related files.

Related source code: ResourceLoaderFileModule::compileLessFile

This particular method actually uses md5 hashes instead of timestamps. Either will work for your purpose, though hashes will be more efficient considering that all our source files come from Git, which does not track mtimes. As such, every week when a new WMF branch is created, the mtimes reset to "now" on the app servers.

Thanks @Krinkle! ^ @olga with that in mind this seems to be "triaged but future"

This proposal is selected for the Developer-Wishlist voting round and will be added to a MediaWiki page very soon. To the subscribers, or proposer of this task: please help modify the task description: add a brief summary (10-12 lines) of the problem that this proposal raises, topics discussed in the comments, and a proposed solution (if there is any yet). Remember to add a header with a title "Description," to your content. Please do so before February 5th, 12:00 pm UTC.

Tgr updated the task description. (Show Details)Feb 5 2017, 4:52 AM
Tgr renamed this task from A cached server-side HTML template doesn't update when you change a partial template which it includes. to A cached server-side HTML template should update when you change a partial template which it includes.Feb 5 2017, 9:24 AM
Tgr updated the task description. (Show Details)

I'd recommend this proposal to be open to other solutions (a proposal to solve a problem instead of implementing a specific solution). While this could be considered a bug in the caching logic and could be solved the same way we solved it for LESS and CSSMin, I'd argue that HTML-templates could be considered a very different system and may be better suited using a different approach. For example, by injecting the template instead of hardcoding it in the source.

This would mean that the place where the sub-template is rendered, an HTML (or text) parameter is used instead and the value set to to the expanded template. This could lead to better separation of concerns and better re-use. For general usage, a common interface (function, method or class) may still provide the same interface of a common entry point where needed. This would naturally cache each portion as its own template and therefore invalidate it accordingly.