sebastienros / fluid

Fluid is an open-source .NET template engine based on the Liquid template language.
MIT License
1.37k stars 174 forks source link

Include/Render Statement Cache Not Refreshed When File Content Changed #602

Open MrUpsideDown opened 8 months ago

MrUpsideDown commented 8 months ago

Firstly, thanks to all contributors for this excellent framework. We are using Fluid extensively in our open source project (Rock RMS), and it is a huge asset.

We are trying to resolve an issue with our Fluid implementation (v2.5), and would appreciate any thoughts on how to best address this problem.

The Include/Render Statements currently use an internal caching mechanism to avoid reloading content from the included file each time the statement is executed. https://github.com/sebastienros/fluid/blob/main/Fluid/Ast/IncludeStatement.cs#L52 This makes good sense for performance, but in the current implementation the internal cache can only be refreshed if the name of the included file is changed. However, we would like to also refresh the cache if the file content is changed.

One way to achieve this could be to add a TemplateOptions.ContentProvider (superseding the existing TemplateOptions.FileProvider) that is tasked with supplying template content based on the provided key/filepath. The default implementation of the ContentProvider could mirror the existing implementation offered by the FileProvider, but could also (in our case) be swapped out for a different caching mechanism which can respond to other changes, such as file content modifications. Extending the provider in this way could also allow different content to be supplied for different rendering contexts.

Is this something that could/should be added to Fluid?

MrUpsideDown commented 7 months ago

@sebastienros - I wonder if you have any thoughts on how we might move this forward? Is it something that would require a PR on our part, or is this kind of change best handled internally by you? Any comments appreciated!

MrUpsideDown commented 5 months ago

@sebastienros - Just checking in to see if you have had a chance to consider this? We are very interested in finding a resolution for our project and would like to confirm if a PR would be the best way forward - thanks again for all your great work!

sebastienros commented 5 months ago

Sorry for the delay. Here is my suggestion.

We still want the cache, and the parsed template cache is important. We could expose a TemplateCache class that has a ConcurrentDictionary<string, TemplateInfo> whith

public sealed class TemplateCache
{
  private ConcurrentDictionary<string, TemplateInfo> _templateInfos = [];

  public bool TryGetTemplate(string name, out var TemplateInfo);
  public TemplateInfo GetOrAddTemplate(string name, Func<string, IFluidTemplate> templateInfoBuilder);
  public bool TryRemoveTemplate(string name);
}

public sealed class TemplateInfo
{
  public string Name { get; set; }
  public IFluidTemplate Template { get; set; }
}

Then have this in the general options (each TemplateContext copies this instance when created, there are other properties doing it).

Finally IncludeStatement and RenderStatement will use if through TemplateContext to get/set the cached items. Check if other classes use some similar code maybe, I don't think so.