From 9bd06a5dfc78cef90d402c296eb4d428b497c1b3 Mon Sep 17 00:00:00 2001 From: Ryan Nowak Date: Fri, 12 Feb 2016 18:11:57 -0800 Subject: [PATCH] React to changes in HTML abstractions This change implements the new API for flattening content in Razor. Also, some optimizations to avoid allocations on paths where we need to encode HTML attribute values. --- .../TagHelpers/DefaultTagHelperContent.cs | 78 +++++++ .../TagHelpers/TagHelperContent.cs | 6 + .../TagHelpers/TagHelperOutput.cs | 198 +++++++++++++++++- .../TagHelpers/DefaultTagHelperContentTest.cs | 103 +++++++++ .../TagHelpers/TagHelperOutputTest.cs | 78 ++++--- 5 files changed, 422 insertions(+), 41 deletions(-) diff --git a/src/Microsoft.AspNetCore.Razor.Runtime/TagHelpers/DefaultTagHelperContent.cs b/src/Microsoft.AspNetCore.Razor.Runtime/TagHelpers/DefaultTagHelperContent.cs index f84f825297..b92607a290 100644 --- a/src/Microsoft.AspNetCore.Razor.Runtime/TagHelpers/DefaultTagHelperContent.cs +++ b/src/Microsoft.AspNetCore.Razor.Runtime/TagHelpers/DefaultTagHelperContent.cs @@ -148,6 +148,84 @@ namespace Microsoft.AspNetCore.Razor.TagHelpers return this; } + /// + public override void CopyTo(IHtmlContentBuilder destination) + { + if (destination == null) + { + throw new ArgumentNullException(nameof(destination)); + } + + if (_buffer == null) + { + return; + } + + for (var i = 0; i < Buffer.Count; i++) + { + var entry = Buffer[i]; + if (entry == null) + { + continue; + } + + string entryAsString; + IHtmlContentContainer entryAsContainer; + if ((entryAsString = entry as string) != null) + { + destination.Append(entryAsString); + } + else if ((entryAsContainer = entry as IHtmlContentContainer) != null) + { + entryAsContainer.CopyTo(destination); + } + else + { + destination.AppendHtml((IHtmlContent)entry); + } + } + } + + /// + public override void MoveTo(IHtmlContentBuilder destination) + { + if (destination == null) + { + throw new ArgumentNullException(nameof(destination)); + } + + if (_buffer == null) + { + return; + } + + for (var i = 0; i < Buffer.Count; i++) + { + var entry = Buffer[i]; + if (entry == null) + { + continue; + } + + string entryAsString; + IHtmlContentContainer entryAsContainer; + if ((entryAsString = entry as string) != null) + { + destination.Append(entryAsString); + } + else if ((entryAsContainer = entry as IHtmlContentContainer) != null) + { + entryAsContainer.MoveTo(destination); + } + else + { + destination.AppendHtml((IHtmlContent)entry); + } + } + + Buffer.Clear(); + } + /// public override TagHelperContent Clear() { diff --git a/src/Microsoft.AspNetCore.Razor.Runtime/TagHelpers/TagHelperContent.cs b/src/Microsoft.AspNetCore.Razor.Runtime/TagHelpers/TagHelperContent.cs index 45a8160444..507ddbf535 100644 --- a/src/Microsoft.AspNetCore.Razor.Runtime/TagHelpers/TagHelperContent.cs +++ b/src/Microsoft.AspNetCore.Razor.Runtime/TagHelpers/TagHelperContent.cs @@ -128,6 +128,12 @@ namespace Microsoft.AspNetCore.Razor.TagHelpers /// A reference to this instance after the clear operation has completed. public abstract TagHelperContent Clear(); + /// + public abstract void CopyTo(IHtmlContentBuilder destination); + + /// + public abstract void MoveTo(IHtmlContentBuilder destination); + /// /// Gets the content. /// diff --git a/src/Microsoft.AspNetCore.Razor.Runtime/TagHelpers/TagHelperOutput.cs b/src/Microsoft.AspNetCore.Razor.Runtime/TagHelpers/TagHelperOutput.cs index 029ce1a9c0..3baadfdcd5 100644 --- a/src/Microsoft.AspNetCore.Razor.Runtime/TagHelpers/TagHelperOutput.cs +++ b/src/Microsoft.AspNetCore.Razor.Runtime/TagHelpers/TagHelperOutput.cs @@ -12,7 +12,7 @@ namespace Microsoft.AspNetCore.Razor.TagHelpers /// /// Class used to represent the output of an . /// - public class TagHelperOutput : IHtmlContent + public class TagHelperOutput : IHtmlContentContainer { private readonly Func> _getChildContentAsync; private TagHelperAttributeList _attributes; @@ -276,7 +276,100 @@ namespace Microsoft.AspNetCore.Razor.TagHelpers return _getChildContentAsync(useCachedResult, encoder); } - /// + void IHtmlContentContainer.CopyTo(IHtmlContentBuilder destination) + { + if (destination == null) + { + throw new ArgumentNullException(nameof(destination)); + } + + _preElement?.CopyTo(destination); + + var isTagNameNullOrWhitespace = string.IsNullOrWhiteSpace(TagName); + + if (!isTagNameNullOrWhitespace) + { + destination.AppendHtml("<"); + destination.AppendHtml(TagName); + + CopyAttributesTo(destination); + + if (TagMode == TagMode.SelfClosing) + { + destination.AppendHtml(" /"); + } + + destination.AppendHtml(">"); + } + + if (isTagNameNullOrWhitespace || TagMode == TagMode.StartTagAndEndTag) + { + _preContent?.CopyTo(destination); + + _content?.CopyTo(destination); + + _postContent?.CopyTo(destination); + } + + if (!isTagNameNullOrWhitespace && TagMode == TagMode.StartTagAndEndTag) + { + destination.AppendHtml(""); + } + + _postElement?.CopyTo(destination); + } + + void IHtmlContentContainer.MoveTo(IHtmlContentBuilder destination) + { + if (destination == null) + { + throw new ArgumentNullException(nameof(destination)); + } + + _preElement?.MoveTo(destination); + + var isTagNameNullOrWhitespace = string.IsNullOrWhiteSpace(TagName); + + if (!isTagNameNullOrWhitespace) + { + destination.AppendHtml("<"); + destination.AppendHtml(TagName); + + CopyAttributesTo(destination); + + if (TagMode == TagMode.SelfClosing) + { + destination.AppendHtml(" /"); + } + + destination.AppendHtml(">"); + } + + if (isTagNameNullOrWhitespace || TagMode == TagMode.StartTagAndEndTag) + { + _preContent?.MoveTo(destination); + _content?.MoveTo(destination); + _postContent?.MoveTo(destination); + } + + if (!isTagNameNullOrWhitespace && TagMode == TagMode.StartTagAndEndTag) + { + destination.AppendHtml(""); + } + + _postElement?.MoveTo(destination); + + // Depending on the code path we took, these might need to be cleared. + _preContent?.Clear(); + _content?.Clear(); + _postContent?.Clear(); + _attributes?.Clear(); + } + public void WriteTo(TextWriter writer, HtmlEncoder encoder) { if (writer == null) @@ -284,6 +377,11 @@ namespace Microsoft.AspNetCore.Razor.TagHelpers throw new ArgumentNullException(nameof(writer)); } + if (encoder == null) + { + throw new ArgumentNullException(nameof(encoder)); + } + _preElement?.WriteTo(writer, encoder); var isTagNameNullOrWhitespace = string.IsNullOrWhiteSpace(TagName); @@ -310,16 +408,25 @@ namespace Microsoft.AspNetCore.Razor.TagHelpers var htmlContent = value as IHtmlContent; if (htmlContent != null) { - // There's no way of tracking the attribute value quotations in the Razor source. Therefore, we - // must escape any IHtmlContent double quote values in the case that a user wrote: - //

- using (var stringWriter = new StringWriter()) + // Perf: static text in a bound attribute go down this path. Avoid allocating if possible (common case). + var htmlEncodedString = value as HtmlEncodedString; + if (htmlEncodedString != null && !htmlEncodedString.Value.Contains("\"")) { - htmlContent.WriteTo(stringWriter, encoder); - stringWriter.GetStringBuilder().Replace("\"", """); + writer.Write(htmlEncodedString.Value); + } + else + { + // There's no way of tracking the attribute value quotations in the Razor source. Therefore, we + // must escape any IHtmlContent double quote values in the case that a user wrote: + //

+ using (var stringWriter = new StringWriter()) + { + htmlContent.WriteTo(stringWriter, encoder); + stringWriter.GetStringBuilder().Replace("\"", """); - var stringValue = stringWriter.ToString(); - writer.Write(stringValue); + var stringValue = stringWriter.ToString(); + writer.Write(stringValue); + } } } else if (value != null) @@ -356,5 +463,76 @@ namespace Microsoft.AspNetCore.Razor.TagHelpers _postElement?.WriteTo(writer, encoder); } + + private void CopyAttributesTo(IHtmlContentBuilder destination) + { + StringWriter stringWriter = null; + + // Perf: Avoid allocating enumerator + for (var i = 0; i < (_attributes?.Count ?? 0); i++) + { + var attribute = _attributes[i]; + destination.AppendHtml(" "); + destination.AppendHtml(attribute.Name); + + if (attribute.Minimized) + { + continue; + } + + destination.AppendHtml("=\""); + var value = attribute.Value; + var htmlContent = value as IHtmlContent; + if (htmlContent != null) + { + // Perf: static text in a bound attribute go down this path. Avoid allocating if possible (common case). + var htmlEncodedString = value as HtmlEncodedString; + if (htmlEncodedString != null && !htmlEncodedString.Value.Contains("\"")) + { + destination.AppendHtml(htmlEncodedString); + } + else + { + // Perf: We'll share this writer implementation for all attributes since + // they can't nest. + stringWriter = stringWriter ?? new StringWriter(); + + destination.AppendHtml(new AttributeContent(htmlContent, stringWriter)); + } + } + else if (value != null) + { + destination.Append(value.ToString()); + } + + destination.AppendHtml("\""); + } + } + + private class AttributeContent : IHtmlContent + { + private readonly IHtmlContent _inner; + private readonly StringWriter _stringWriter; + + public AttributeContent(IHtmlContent inner, StringWriter stringWriter) + { + _inner = inner; + _stringWriter = stringWriter; + } + + public void WriteTo(TextWriter writer, HtmlEncoder encoder) + { + // There's no way of tracking the attribute value quotations in the Razor source. Therefore, we + // must escape any IHtmlContent double quote values in the case that a user wrote: + //

+ _inner.WriteTo(_stringWriter, encoder); + _stringWriter.GetStringBuilder().Replace("\"", """); + + var stringValue = _stringWriter.ToString(); + writer.Write(stringValue); + + _stringWriter.GetStringBuilder().Clear(); + } + } } } diff --git a/test/Microsoft.AspNetCore.Razor.Runtime.Test/TagHelpers/DefaultTagHelperContentTest.cs b/test/Microsoft.AspNetCore.Razor.Runtime.Test/TagHelpers/DefaultTagHelperContentTest.cs index 936c72014a..827983a31c 100644 --- a/test/Microsoft.AspNetCore.Razor.Runtime.Test/TagHelpers/DefaultTagHelperContentTest.cs +++ b/test/Microsoft.AspNetCore.Razor.Runtime.Test/TagHelpers/DefaultTagHelperContentTest.cs @@ -1,8 +1,10 @@ // Copyright (c) .NET Foundation. All rights reserved. // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. +using System.Collections.Generic; using System.Globalization; using System.IO; +using Microsoft.AspNetCore.Html; using Microsoft.Extensions.WebEncoders.Testing; using Xunit; @@ -83,6 +85,107 @@ namespace Microsoft.AspNetCore.Razor.TagHelpers Assert.Equal(expected, copiedTagHelperContent.GetContent(new HtmlTestEncoder())); } + [Fact] + public void CopyTo_CopiesAllItems() + { + // Arrange + var source = new DefaultTagHelperContent(); + source.AppendHtml(new HtmlEncodedString("hello")); + source.Append("Test"); + + var items = new List(); + var destination = new HtmlContentBuilder(items); + destination.Append("some-content"); + + // Act + source.CopyTo(destination); + + // Assert + Assert.Equal(3, items.Count); + + Assert.Equal("some-content", Assert.IsType(items[0])); + Assert.Equal("hello", Assert.IsType(items[1]).Value); + Assert.Equal("Test", Assert.IsType(items[2])); + } + + [Fact] + public void CopyTo_DoesDeepCopy() + { + // Arrange + var source = new DefaultTagHelperContent(); + + var nested = new DefaultTagHelperContent(); + source.AppendHtml(nested); + nested.AppendHtml(new HtmlEncodedString("hello")); + source.Append("Test"); + + var items = new List(); + var destination = new HtmlContentBuilder(items); + destination.Append("some-content"); + + // Act + source.CopyTo(destination); + + // Assert + Assert.Equal(3, items.Count); + + Assert.Equal("some-content", Assert.IsType(items[0])); + Assert.Equal("hello", Assert.IsType(items[1]).Value); + Assert.Equal("Test", Assert.IsType(items[2])); + } + + [Fact] + public void MoveTo_CopiesAllItems_AndClears() + { + // Arrange + var source = new DefaultTagHelperContent(); + source.AppendHtml(new HtmlEncodedString("hello")); + source.Append("Test"); + + var items = new List(); + var destination = new HtmlContentBuilder(items); + destination.Append("some-content"); + + // Act + source.MoveTo(destination); + + // Assert + Assert.True(source.IsEmpty); + Assert.Equal(3, items.Count); + + Assert.Equal("some-content", Assert.IsType(items[0])); + Assert.Equal("hello", Assert.IsType(items[1]).Value); + Assert.Equal("Test", Assert.IsType(items[2])); + } + + [Fact] + public void MoveTo_DoesDeepMove() + { + // Arrange + var source = new DefaultTagHelperContent(); + + var nested = new DefaultTagHelperContent(); + source.AppendHtml(nested); + nested.AppendHtml(new HtmlEncodedString("hello")); + source.Append("Test"); + + var items = new List(); + var destination = new HtmlContentBuilder(items); + destination.Append("some-content"); + + // Act + source.MoveTo(destination); + + // Assert + Assert.True(source.IsEmpty); + Assert.True(nested.IsEmpty); + Assert.Equal(3, items.Count); + + Assert.Equal("some-content", Assert.IsType(items[0])); + Assert.Equal("hello", Assert.IsType(items[1]).Value); + Assert.Equal("Test", Assert.IsType(items[2])); + } + // GetContent - this one relies on the default encoder. [Fact] public void CanGetContent() diff --git a/test/Microsoft.AspNetCore.Razor.Runtime.Test/TagHelpers/TagHelperOutputTest.cs b/test/Microsoft.AspNetCore.Razor.Runtime.Test/TagHelpers/TagHelperOutputTest.cs index e82aae5c96..440a4587db 100644 --- a/test/Microsoft.AspNetCore.Razor.Runtime.Test/TagHelpers/TagHelperOutputTest.cs +++ b/test/Microsoft.AspNetCore.Razor.Runtime.Test/TagHelpers/TagHelperOutputTest.cs @@ -972,16 +972,17 @@ namespace Microsoft.AspNetCore.Razor.TagHelpers Assert.Equal(expected, writer.ToString(), StringComparer.Ordinal); } - // This tests a separate code path that's used by THO when the writer is an HtmlTextWriter. - // The output should be the same, but we do some specific perf optimizations on this path. [Theory] [MemberData(nameof(WriteTagHelper_InputData))] - public void WriteTo_WritesFormattedTagHelper_HtmlTextWriter(TagHelperOutput output, string expected) + public void CopyTo_CopiesToBuilder(TagHelperOutput output, string expected) { // Arrange - var inner = new StringWriter(); + var attributeCount = output.Attributes.Count; + + var writer = new StringWriter(); var testEncoder = new HtmlTestEncoder(); - var writer = new MockHtmlTextWriter(inner, testEncoder); + + var buffer = new HtmlContentBuilder(); var tagHelperExecutionContext = new TagHelperExecutionContext( tagName: output.TagName, @@ -994,10 +995,49 @@ namespace Microsoft.AspNetCore.Razor.TagHelpers tagHelperExecutionContext.Output = output; // Act - output.WriteTo(writer, testEncoder); + ((IHtmlContentContainer)output).CopyTo(buffer); // Assert - Assert.Equal(expected, inner.ToString(), StringComparer.Ordinal); + buffer.WriteTo(writer, testEncoder); + + Assert.Equal(attributeCount, output.Attributes.Count); + Assert.Equal(expected, writer.ToString(), StringComparer.Ordinal); + } + + [Theory] + [MemberData(nameof(WriteTagHelper_InputData))] + public void MoveTo_MovesToBuilder(TagHelperOutput output, string expected) + { + // Arrange + var writer = new StringWriter(); + var testEncoder = new HtmlTestEncoder(); + + var buffer = new HtmlContentBuilder(); + + var tagHelperExecutionContext = new TagHelperExecutionContext( + tagName: output.TagName, + tagMode: output.TagMode, + items: new Dictionary(), + uniqueId: string.Empty, + executeChildContentAsync: () => Task.FromResult(result: true), + startTagHelperWritingScope: _ => { }, + endTagHelperWritingScope: () => new DefaultTagHelperContent()); + tagHelperExecutionContext.Output = output; + + // Act + ((IHtmlContentContainer)output).MoveTo(buffer); + + // Assert + buffer.WriteTo(writer, testEncoder); + + Assert.True(output.PreElement.IsEmpty); + Assert.True(output.PreContent.IsEmpty); + Assert.True(output.Content.IsEmpty); + Assert.True(output.PostContent.IsEmpty); + Assert.True(output.PostElement.IsEmpty); + Assert.Empty(output.Attributes); + + Assert.Equal(expected, writer.ToString(), StringComparer.Ordinal); } private static TagHelperOutput GetTagHelperOutput( @@ -1046,29 +1086,5 @@ namespace Microsoft.AspNetCore.Razor.TagHelpers return output; } - - private class MockHtmlTextWriter : HtmlTextWriter - { - private readonly HtmlEncoder _encoder; - private readonly TextWriter _inner; - - public MockHtmlTextWriter(TextWriter inner, HtmlEncoder encoder) - { - _inner = inner; - _encoder = encoder; - } - - public override Encoding Encoding => _inner.Encoding; - - public override void Write(IHtmlContent value) - { - value.WriteTo(_inner, _encoder); - } - - public override void Write(char value) - { - _inner.Write(value); - } - } } } \ No newline at end of file