Open fastbike opened 10 months ago
OK, this was not too hard as a first cut.
The main thing to get my head around was the idea that the each View Template that is processed by the Execute method has its own list of partials. I initially I thought that the rendering process was throwing that away between calls to the Execute method, although further investigation reveals the underlying SynMustache has a cache which is active across the calls to Execute as well as active across requests. So this should result in a good performance if the template/partial has already been loaded. With that in mind, I check to see if the partial template has been loaded (internally the SynMustache engine does this, but I am saving a call to the disk - which in a busy server is going to be happening many times per second.
I've also taken the decision to make the Partials collection a class field so that the optimisation above persists across the request. I.e. previously a Controller line of code like
LoadView(['page_header', 'contents', 'page_footer']);
would have created the partials object for each template and if a Partial was shared between more than one template it would still have been loaded from the disk for each Execute call.
Unfortunately for each request, the template still has to be loaded from disk, because the underlying SynMustacheCache uses a hash of the contents as the index into its cache, so you can't check the cache to see if it has been loaded without reading if from the disk. Chicken and Egg as they say. However the work involved in parsing the template is avoided because the SynMustacheCache can detect it is already present so does not process it. I may think about adding a separate front end cache for the contents of the templates so they only need to be read from disk once, however that introduces another set of issues around cache lifetimes etc. So not part of this ticket :)
Without wanted to get side tracked I offer the proposed changes. And note that i have not yet merged in the suggested changes for the Mustache Helpers from earlier https://github.com/danieleteti/delphimvcframework/pull/699
type
TSynMustacheAccess = class(TSynMustache)
end;
TSynPartialsAccess = class(TSynMustachePartials)
end;
constructor TMVCMustacheViewEngine.Create(const AEngine: TMVCEngine; const AWebContext: TWebContext;
const AViewModel: TMVCViewDataObject; const AViewDataSets: TObjectDictionary<string, TDataSet>; const AContentType: string);
begin
inherited;
FPartials := TSynMustachePartials.Create;
end;
destructor TMVCMustacheViewEngine.Destroy;
begin
FPartials.Free;
inherited;
end;
procedure TMVCMustacheViewEngine.Execute(const ViewName: string; const OutputStream: TStream);
var
lViewTemplate: RawUTF8;
lViewEngine: TSynMustache;
lSW: TStreamWriter;
lHelpers: TSynMustacheHelpers;
begin
PrepareModels;
lViewTemplate := ReadTemplate(ViewName);
lViewEngine := TSynMustache.Parse(lViewTemplate);
lSW := TStreamWriter.Create(OutputStream);
try
ProcessNestedPartials(lViewEngine);
lHelpers := TSynMustacheAccess.HelpersGetStandardList;
lSW.Write(UTF8Tostring(RenderJSON(lViewEngine, FJSONModel, FPartials, lHelpers, nil, false)));
finally
lSW.Free;
end;
end;
procedure TMVCMustacheViewEngine.ProcessNestedPartials(const ViewEngine: TSynMustache);
var
I: Integer;
lPartialName: string;
lViewTemplate: RawUTF8;
lPartialEngine: TSynMustache;
begin
for I := 0 to Length(TSynMustacheAccess(ViewEngine).fTags) - 1 do
begin
if TSynMustacheAccess(ViewEngine).fTags[I].Kind = mtPartial then
begin
lPartialName := TSynMustacheAccess(ViewEngine).fTags[I].Value;
if TSynPartialsAccess(FPartials).GetPartial(lPartialName) = nil then
begin
lViewTemplate := ReadTemplate(lPartialName);
lPartialEngine := FPartials.Add(lPartialName, lViewTemplate);
ProcessNestedPartials(lPartialEngine);
end;
end;
end;
end;
function TMVCMustacheViewEngine.ReadTemplate(TemplateName: string): RawUTF8;
var
lViewFileName: string;
begin
lViewFileName := GetRealFileName(TemplateName);
if not FileExists(lViewFileName) then
raise EMVCFrameworkViewException.CreateFmt('View [%s] not found', [TemplateName]);
Result := StringToUTF8(TFile.ReadAllText(lViewFileName, TEncoding.UTF8));
end;
A simple (slightly naive) improvement will cache the templates for the first time they are accessed during a request, so if they are accessed subsequently in the same request the code will not need to hit the disk. This will have no effect across requests, as the cache is owned by the instance of the engine which is discarded between requests. I could look at a threadsafe cache later.
TMVCMustacheViewEngine = class(TMVCBaseViewEngine)
private
FTemplateCache: TDictionary<string, RawUTF8>;
// etc
function TMVCMustacheViewEngine.ReadTemplate(TemplateName: string): RawUTF8;
var
lViewFileName: string;
begin
if not FTemplateCache.TryGetValue(TemplateName, Result) then
begin
lViewFileName := GetRealFileName(TemplateName);
if not FileExists(lViewFileName) then
raise EMVCFrameworkViewException.CreateFmt('View [%s] not found', [TemplateName]);
Result := StringToUTF8(TFile.ReadAllText(lViewFileName, TEncoding.UTF8));
FTemplateCache.Add(TemplateName, Result);
end;
end;```
Hi David, did you tried the repo version? Check this sample and let me know: https://github.com/danieleteti/delphimvcframework/tree/master/samples/htmx_mustache
As a side note (not directly related to this issue, but it worth mention in this context) now you can use the following methods to handle SSV:
I've taken a look but I don't see the repo version handling more than one level of Partials. Following on from https://github.com/danieleteti/delphimvcframework/issues/693 I think I need to do a pull from the current development repo and submit a pull request back here with my suggested improvements that build on your work.
I have some mustache templates that can be called in a variety of contexts, to support full page rendering and HTMX (partial page) rendering.
For the initial (full) page rendering, we have a page composed of 3 html fragments: _page_header, contents, pagefooter. When the page is initially loaded the code will be like
Inside the contents template we have some static data followed by a table. The table template sets up an html table, including the thead and then includes another template (_contentdata) to pull in the rows of data.
The _contentdata template iterates over the data collection and then attempts to pull in another template (_tablefooter) to render the footer for the table. (This table footer is used all over the application so it makes sense to keep it as a separate template.)
However as the rendering is now three levels deep the _tablefooter template is never processed as the framework's ViewEngine Execute method does not allow more than one level of nesting.
Note that we have another controller method that the HTMX calls that only renders out the _contentdata template, as it is performing an in place replacement of the table data. Because the _tablefooter template is now only one level deep in this context it gets rendered.
I'm thinking we need to refactor the Execute method to be able to firstly build the data models and then recursively call the template, processing each partial as it is declared.
Any other thoughts before I take a look at this.