Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Move Head/BodyResourceLinks insertion to compile-time #1585

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
using System.Collections.Immutable;
using System.Security.Cryptography;
using System.Text;
using DotVVM.Framework.Configuration;

namespace DotVVM.Framework.Compilation.Directives
{
Expand All @@ -22,15 +23,17 @@ public class BaseTypeDirectiveCompiler : DirectiveCompiler<IAbstractBaseTypeDire

private readonly string fileName;
private readonly ImmutableList<NamespaceImport> imports;
private readonly DotvvmConfiguration configuration;

public override string DirectiveName => ParserConstants.BaseTypeDirective;

public BaseTypeDirectiveCompiler(
IReadOnlyDictionary<string, IReadOnlyList<DothtmlDirectiveNode>> directiveNodesByName, IAbstractTreeBuilder treeBuilder, string fileName, ImmutableList<NamespaceImport> imports)
IReadOnlyDictionary<string, IReadOnlyList<DothtmlDirectiveNode>> directiveNodesByName, IAbstractTreeBuilder treeBuilder, string fileName, ImmutableList<NamespaceImport> imports, DotvvmConfiguration configuration)
: base(directiveNodesByName, treeBuilder)
{
this.fileName = fileName;
this.imports = imports;
this.configuration = configuration;
}

protected override IAbstractBaseTypeDirective Resolve(DothtmlDirectiveNode directiveNode)
Expand Down Expand Up @@ -113,7 +116,14 @@ IEnumerable<DothtmlDirectiveNode> propertyDirectives
/// </summary>
private ITypeDescriptor GetDefaultWrapperType()
{
if (fileName.EndsWith(".dotcontrol", StringComparison.Ordinal))
var isControl =
fileName.EndsWith(".dotcontrol", StringComparison.Ordinal) ||
configuration.Markup.Controls.Any(c => fileName.Equals(c.Src, StringComparison.OrdinalIgnoreCase)) ||
this.DirectiveNodesByName.ContainsKey("property") ||
this.DirectiveNodesByName.ContainsKey("wrapperTag") ||
this.DirectiveNodesByName.ContainsKey("noWrapperTag");

if (isControl)
{
return new ResolvedTypeDescriptor(typeof(DotvvmMarkupControl));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
using System;
using System.Linq;
using System.Collections.Generic;
using DotVVM.Framework.Configuration;

namespace DotVVM.Framework.Compilation.Directives
{
Expand All @@ -14,12 +15,14 @@ public class MarkupDirectiveCompilerPipeline : IMarkupDirectiveCompilerPipeline
private readonly IAbstractTreeBuilder treeBuilder;
private readonly IControlBuilderFactory controlBuilderFactory;
private readonly DotvvmResourceRepository resourceRepository;
private readonly DotvvmConfiguration configuration;

public MarkupDirectiveCompilerPipeline(IAbstractTreeBuilder treeBuilder, IControlBuilderFactory controlBuilderFactory, DotvvmResourceRepository resourceRepository)
public MarkupDirectiveCompilerPipeline(IAbstractTreeBuilder treeBuilder, IControlBuilderFactory controlBuilderFactory, DotvvmResourceRepository resourceRepository, DotvvmConfiguration configuration)
{
this.treeBuilder = treeBuilder;
this.controlBuilderFactory = controlBuilderFactory;
this.resourceRepository = resourceRepository;
this.configuration = configuration;
}

public MarkupPageMetadata Compile(DothtmlRootNode dothtmlRoot, string fileName)
Expand Down Expand Up @@ -50,7 +53,7 @@ public MarkupPageMetadata Compile(DothtmlRootNode dothtmlRoot, string fileName)
var injectedServicesResult = serviceCompiler.Compile();
resolvedDirectives.AddIfAny(serviceCompiler.DirectiveName, injectedServicesResult.Directives);

var baseTypeCompiler = new BaseTypeDirectiveCompiler(directivesByName, treeBuilder, fileName, imports);
var baseTypeCompiler = new BaseTypeDirectiveCompiler(directivesByName, treeBuilder, fileName, imports, configuration);
var baseTypeResult = baseTypeCompiler.Compile();
var baseType = baseTypeResult.Artefact;
resolvedDirectives.AddIfAny(baseTypeCompiler.DirectiveName, baseTypeResult.Directives);
Expand Down
122 changes: 122 additions & 0 deletions src/Framework/Framework/Compilation/ResourceLinksVisitor.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,122 @@
using System;
using System.Collections.Generic;
using System.Text;
using DotVVM.Framework.Compilation.ControlTree.Resolved;
using DotVVM.Framework.Controls;
using DotVVM.Framework.Utils;
using DotVVM.Framework.Binding;
using DotVVM.Framework.Binding.Properties;
using DotVVM.Framework.Binding.Expressions;
using System.Collections.Immutable;
using DotVVM.Framework.Compilation.ControlTree;
using System.Linq;

namespace DotVVM.Framework.Compilation
{
/// <summary> Inserts <see cref="BodyResourceLinks" /> and <see cref="HeadResourceLinks"/> into head or body element </summary>
public class ResourceLinksVisitor : ResolvedControlTreeVisitor
{
private readonly IControlResolver controlResolver;

private bool headLinksFound = false;
private bool bodyLinksFound = false;

public ResourceLinksVisitor(IControlResolver controlResolver)
{
this.controlResolver = controlResolver;
}


public override void VisitPropertyTemplate(ResolvedPropertyTemplate propertyTemplate)
{
// skip
}

public override void VisitControl(ResolvedControl control)
{
if (typeof(HeadResourceLinks).IsAssignableFrom(control.Metadata.Type))
{
headLinksFound = true;
}
else if (typeof(BodyResourceLinks).IsAssignableFrom(control.Metadata.Type))
{
bodyLinksFound = true;
}
else
{
base.VisitControl(control);
}
}

private ResolvedControl? TryFindElement(ResolvedControl control, string tagName)
{
// BFS to find the outermost element
// in case somebody mistypes <body> element into a <table>, we don't want to insert resources into it
var queue = new Queue<ResolvedControl>();
queue.Enqueue(control);

while (queue.Count > 0)
{
var current = queue.Dequeue();
if (current.Metadata.Type == typeof(HtmlGenericControl) && current.ConstructorParameters?[0] is string controlTagName && tagName.Equals(controlTagName, StringComparison.OrdinalIgnoreCase))
{
return current;
}
foreach (var child in current.Content)
{
queue.Enqueue(child);
}
}
return null;
}

private ResolvedControl CreateLinksControl(Type type, DataContextStack dataContext)
{
var metadata = controlResolver.ResolveControl(new ResolvedTypeDescriptor(type));
var control = new ResolvedControl((ControlResolverMetadata)metadata, null, dataContext);
control.SetProperty(new ResolvedPropertyValue(Internal.UniqueIDProperty, "cAuto" + type.Name));
return control;
}

public override void VisitView(ResolvedTreeRoot view)
{
if (view.MasterPage is {})
{
// if there is a masterpage, this visitor has already inserted the links into it
return;
}
if (!typeof(Controls.Infrastructure.DotvvmView).IsAssignableFrom(view.ControlBuilderDescriptor.ControlType))
{
// markup controls
return;
}
if (!headLinksFound)
{
var headElement = TryFindElement(view, "head");
if (headElement is {})
{
headElement.Content.Add(CreateLinksControl(typeof(HeadResourceLinks), headElement.DataContextTypeStack));
}
else
{
// no head element found, and no masterpage -> insert it at the document start
view.Content.Insert(0, CreateLinksControl(typeof(HeadResourceLinks), view.DataContextTypeStack));
}
}
if (!bodyLinksFound)
{
var bodyElement = TryFindElement(view, "body");
if (bodyElement is {})
{
bodyElement.Content.Add(CreateLinksControl(typeof(BodyResourceLinks), bodyElement.DataContextTypeStack));
}
else
{
// no body element found, and no masterpage -> insert it at the document end
view.Content.Add(CreateLinksControl(typeof(BodyResourceLinks), view.DataContextTypeStack));
}
}
base.VisitView(view);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ public class ViewCompilingVisitor : ResolvedControlTreeVisitor

protected int currentTemplateIndex;
protected string? controlName;
int controlIdIndex = 0;

public Func<IControlBuilderFactory, IServiceProvider, DotvvmControl> BuildCompiledView { get; set; }

Expand Down Expand Up @@ -247,8 +248,13 @@ protected string CreateControl(ResolvedControl control)
// RawLiterals don't need these helper properties unless in root
if (control.Metadata.Type != typeof(RawLiteral) || control.Parent is ResolvedTreeRoot)
{
// set unique id
emitter.EmitSetDotvvmProperty(name, Internal.UniqueIDProperty, name);
if (!control.Properties.ContainsKey(Internal.UniqueIDProperty))
{
var uniqueId = "c" + controlIdIndex;
controlIdIndex++;
// set unique id
emitter.EmitSetDotvvmProperty(name, Internal.UniqueIDProperty, uniqueId);
}

if (control.DothtmlNode != null && control.DothtmlNode.Tokens.Count > 0)
{
Expand Down
6 changes: 0 additions & 6 deletions src/Framework/Framework/Controls/HtmlGenericControl.cs
Original file line number Diff line number Diff line change
Expand Up @@ -286,12 +286,6 @@ protected override void RenderBeginTag(IHtmlWriter writer, IDotvvmRequestContext
/// </summary>
protected override void RenderEndTag(IHtmlWriter writer, IDotvvmRequestContext context)
{
// render resource link. If the Render is invoked multiple times the resources are rendered on the first invocation.
if (TagName == "head")
new HeadResourceLinks().Render(writer, context);
else if (TagName == "body")
new BodyResourceLinks().Render(writer, context);

if (RendersHtmlTag)
{
writer.RenderEndTag();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,7 @@ public static IServiceCollection RegisterDotVVMServices(IServiceCollection servi
var requiredResourceControl = controlResolver.ResolveControl(new ResolvedTypeDescriptor(typeof(RequiredResource)));
o.TreeVisitors.Add(() => new StyleTreeShufflingVisitor(controlResolver));
o.TreeVisitors.Add(() => new ControlPrecompilationVisitor(s));
o.TreeVisitors.Add(() => new ResourceLinksVisitor(controlResolver));
o.TreeVisitors.Add(() => new LiteralOptimizationVisitor());
o.TreeVisitors.Add(() => new BindingRequiredResourceVisitor((ControlResolverMetadata)requiredResourceControl));
var requiredGlobalizeControl = controlResolver.ResolveControl(new ResolvedTypeDescriptor(typeof(GlobalizeResource)));
Expand Down
2 changes: 1 addition & 1 deletion src/Framework/Framework/Testing/TestHttpRequest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ public TestHttpRequest(IHttpContext context)

public string? Path { get; set; }

public string? PathBase { get; set; }
public string? PathBase { get; set; } = "";
IPathString IHttpRequest.Path => new TestPathString(this.Path);

IPathString IHttpRequest.PathBase => new TestPathString(this.PathBase);
Expand Down
10 changes: 10 additions & 0 deletions src/Framework/Testing/ControlTestHelper.cs
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,16 @@ public async Task<PageRunResult> RunPage(
markup = "<tc:FakeHeadResourceLink />" + markup;
}
markup = $"@viewModel {viewModel.ToString().Replace("+", ".")}\n{directives}\n\n{markup}";
return await RunPageRaw(markup, markupFiles, fileName, user, culture);
}
/// <summary> Run page with the exact markup, without any modifications </summary>
public async Task<PageRunResult> RunPageRaw(
string markup,
Dictionary<string, string>? markupFiles = null,
[CallerMemberName] string? fileName = null,
ClaimsPrincipal? user = null,
CultureInfo? culture = null)
{
var request = PreparePage(markup, markupFiles, fileName, user, culture);
await presenter.ProcessRequest(request);
return CreatePageResult(request);
Expand Down
5 changes: 4 additions & 1 deletion src/Framework/Testing/DotvvmTestHelper.cs
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,10 @@ public static ResolvedTreeRoot ParseResolvedTree(string markup, string fileName
if (checkErrors) CheckForErrors(root.DothtmlNode);
foreach (var v in visitors)
{
v().VisitView(root);
var visitor = v();
if (visitor is ResourceLinksVisitor)
continue; // not wanted in parser tests
visitor.VisitView(root);
}
if (checkErrors)
validator.VisitAndAssert(root);
Expand Down
73 changes: 73 additions & 0 deletions src/Tests/ControlTests/ResourceLinksTests.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,73 @@
using System;
using System.Collections.Generic;
using System.ComponentModel.DataAnnotations;
using System.Globalization;
using System.Threading.Tasks;
using CheckTestOutput;
using DotVVM.Framework.Compilation;
using DotVVM.Framework.Controls;
using DotVVM.Framework.Tests.Binding;
using DotVVM.Framework.ViewModel;
using Microsoft.VisualStudio.TestTools.UnitTesting;
using DotVVM.Framework.Testing;
using System.Security.Claims;

namespace DotVVM.Framework.Tests.ControlTests
{
[TestClass]
public class ResourceLinksTests
{
static readonly ControlTestHelper cth = new ControlTestHelper(config: config => {
});
OutputChecker check = new OutputChecker("testoutputs");

[TestMethod]
public async Task OnlyLiteralInBody()
{
// There was a bug which collapsed the body contents into a `data-bind='text: ...'` binding
var r = await cth.RunPageRaw("""
@viewModel DotVVM.Framework.Tests.ControlTests.ResourceLinksTests.BasicTestViewModel
<head></head>
<body>
Id: {{value: Integer}}
</body>
""");

check.CheckString(r.OutputString, fileExtension: "html");
}
[TestMethod]
public async Task MultipleBodyElements()
{
// You can put body into the document by accident. In such case, DotVVM should try to select the outermost one
var r = await cth.RunPageRaw("""
@viewModel DotVVM.Framework.Tests.ControlTests.ResourceLinksTests.BasicTestViewModel
<head></head>
<body>
<table>
<body> {{value: Integer}} </body>
</table>
</body>
""");

check.CheckString(r.OutputString, fileExtension: "html");
}
[TestMethod]
public async Task NoHeadBodyElements()
{
// No body and head elements, DotVVM should put the resources at the start and end of the document,
// the browser will hopefully correctly infer the head and body elements
var r = await cth.RunPageRaw("""
@viewModel DotVVM.Framework.Tests.ControlTests.ResourceLinksTests.BasicTestViewModel

<div class-a={value: Integer >= 0}>{{value: Integer}}</div>
""");

check.CheckString(r.OutputString, fileExtension: "html");
}
public class BasicTestViewModel: DotvvmViewModelBase
{
[Bind(Name = "int")]
public int Integer { get; set; } = 10;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -6,18 +6,18 @@
<tbody>
<tr data-bind="dotvvm-validation: Email, dotvvm-validationOptions: {}">
<td class="autoui-label autoui-required">
<label data-bind="attr: { for: &quot;c7&quot;+'_'+$index()+'_'+&quot;Email__input&quot; }">Email</label>
<label data-bind="attr: { for: &quot;c5&quot;+'_'+$index()+'_'+&quot;Email__input&quot; }">Email</label>
</td>
<td class="autoui-editor">
<input data-bind="attr: { id: &quot;c7&quot;+'_'+$index()+'_'+&quot;Email__input&quot; }, dotvvm-textbox-text: Email" required="required" type="email">
<input data-bind="attr: { id: &quot;c5&quot;+'_'+$index()+'_'+&quot;Email__input&quot; }, dotvvm-textbox-text: Email" required="required" type="email">
</td>
</tr>
<tr data-bind="dotvvm-validation: Name, dotvvm-validationOptions: {}">
<td class="autoui-label">
<label data-bind="attr: { for: &quot;c7&quot;+'_'+$index()+'_'+&quot;Name__input&quot; }">Name</label>
<label data-bind="attr: { for: &quot;c5&quot;+'_'+$index()+'_'+&quot;Name__input&quot; }">Name</label>
</td>
<td class="autoui-editor">
<input data-bind="attr: { id: &quot;c7&quot;+'_'+$index()+'_'+&quot;Name__input&quot; }, dotvvm-textbox-text: Name" type="text">
<input data-bind="attr: { id: &quot;c5&quot;+'_'+$index()+'_'+&quot;Name__input&quot; }, dotvvm-textbox-text: Name" type="text">
</td>
</tr>
</tbody>
Expand Down
Loading