From c400289de5f174accfd7fec3dda7a8c894189726 Mon Sep 17 00:00:00 2001 From: "N. Taylor Mullen" Date: Wed, 30 Mar 2016 12:29:08 -0700 Subject: [PATCH] Change `DefaultTagHelperContent` to be smart about single content entries. - Today `TagHelperContent`s always allocate their underlying buffer even though they typically only ever have a single entry. Added a field to enable us to only allocate the backing buffer when we absolutely need to. - Removed `IsEmpty` from `TagHelperContent` since it was not used in any of our `TagHelper`s for simplification. Changed `IsWhiteSpace` naming to be `IsEmptyOrWhiteSpace` since it can be used to indicate either state. - Updated tests. #621 --- .../TagHelpers/DefaultTagHelperContent.cs | 319 +++++++++--------- .../TagHelpers/TagHelperContent.cs | 9 +- .../TagHelpers/DefaultTagHelperContentTest.cs | 50 +-- .../TagHelpers/TagHelperOutputTest.cs | 10 +- 4 files changed, 200 insertions(+), 188 deletions(-) diff --git a/src/Microsoft.AspNetCore.Razor.Runtime/TagHelpers/DefaultTagHelperContent.cs b/src/Microsoft.AspNetCore.Razor.Runtime/TagHelpers/DefaultTagHelperContent.cs index 458ab751f0..7bcb7db80b 100644 --- a/src/Microsoft.AspNetCore.Razor.Runtime/TagHelpers/DefaultTagHelperContent.cs +++ b/src/Microsoft.AspNetCore.Razor.Runtime/TagHelpers/DefaultTagHelperContent.cs @@ -17,20 +17,29 @@ namespace Microsoft.AspNetCore.Razor.TagHelpers [DebuggerDisplay("{DebuggerToString(),nq}")] public class DefaultTagHelperContent : TagHelperContent { - private List _buffer; + private object _singleContent; + private bool _isSingleContentSet; private bool _isModified; + private bool _hasContent; + private List _buffer; private List Buffer { get { - _isModified = true; - if (_buffer == null) { _buffer = new List(); } + if (_isSingleContentSet) + { + Debug.Assert(_buffer.Count == 0); + + _buffer.Add(_singleContent); + _isSingleContentSet = false; + } + return _buffer; } } @@ -40,39 +49,27 @@ namespace Microsoft.AspNetCore.Razor.TagHelpers /// /// Returns true for a cleared . - public override bool IsWhiteSpace + public override bool IsEmptyOrWhiteSpace { get { - if (!IsModified) + if (!_hasContent) { return true; } using (var writer = new EmptyOrWhiteSpaceWriter()) { - foreach (var entry in _buffer) + if (_isSingleContentSet) { - if (entry == null) - { - continue; - } + return IsEmptyOrWhiteSpaceCore(_singleContent, writer); + } - var stringValue = entry as string; - if (stringValue != null) + for (var i = 0; i < (_buffer?.Count ?? 0); i++) + { + if (!IsEmptyOrWhiteSpaceCore(Buffer[i], writer)) { - if (!string.IsNullOrWhiteSpace(stringValue)) - { - return false; - } - } - else - { - ((IHtmlContent)entry).WriteTo(writer, HtmlEncoder.Default); - if (!writer.IsWhiteSpace) - { - return false; - } + return false; } } } @@ -82,73 +79,20 @@ namespace Microsoft.AspNetCore.Razor.TagHelpers } /// - public override bool IsEmpty - { - get - { - if (!IsModified) - { - return true; - } - - using (var writer = new EmptyOrWhiteSpaceWriter()) - { - foreach (var entry in _buffer) - { - if (entry == null) - { - continue; - } - - var stringValue = entry as string; - if (stringValue != null) - { - if (!string.IsNullOrEmpty(stringValue)) - { - return false; - } - } - else - { - ((IHtmlContent)entry).WriteTo(writer, HtmlEncoder.Default); - if (!writer.IsEmpty) - { - return false; - } - } - } - } - - return true; - } - } + public override TagHelperContent Append(string unencoded) => AppendCore(unencoded); /// - public override TagHelperContent Append(string unencoded) - { - Buffer.Add(unencoded); - return this; - } + public override TagHelperContent AppendHtml(IHtmlContent htmlContent) => AppendCore(htmlContent); /// public override TagHelperContent AppendHtml(string encoded) { if (encoded == null) { - Buffer.Add(null); + return AppendCore(null); } - else - { - Buffer.Add(new HtmlEncodedString(encoded)); - } - return this; - } - /// - public override TagHelperContent AppendHtml(IHtmlContent htmlContent) - { - Buffer.Add(htmlContent); - return this; + return AppendCore(new HtmlEncodedString(encoded)); } /// @@ -159,32 +103,20 @@ namespace Microsoft.AspNetCore.Razor.TagHelpers throw new ArgumentNullException(nameof(destination)); } - if (!IsModified) + if (!_hasContent) { return; } - for (var i = 0; i < Buffer.Count; i++) + if (_isSingleContentSet) { - var entry = Buffer[i]; - if (entry == null) + CopyToCore(_singleContent, destination); + } + else + { + for (var i = 0; i < (_buffer?.Count ?? 0); i++) { - 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); + CopyToCore(Buffer[i], destination); } } } @@ -197,62 +129,50 @@ namespace Microsoft.AspNetCore.Razor.TagHelpers throw new ArgumentNullException(nameof(destination)); } - if (!IsModified) + if (!_hasContent) { return; } - for (var i = 0; i < Buffer.Count; i++) + if (_isSingleContentSet) { - var entry = Buffer[i]; - if (entry == null) + MoveToCore(_singleContent, destination); + } + else + { + for (var i = 0; i < (_buffer?.Count ?? 0); i++) { - 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); + MoveToCore(Buffer[i], destination); } } - Buffer.Clear(); + Clear(); } /// public override TagHelperContent Clear() { - Buffer.Clear(); + _hasContent = false; + _isModified = true; + _isSingleContentSet = false; + _buffer?.Clear(); return this; } /// public override void Reinitialize() { - _buffer?.Clear(); + Clear(); _isModified = false; } /// - public override string GetContent() - { - return GetContent(HtmlEncoder.Default); - } + public override string GetContent() => GetContent(HtmlEncoder.Default); /// public override string GetContent(HtmlEncoder encoder) { - if (_buffer == null) + if (!_hasContent) { return string.Empty; } @@ -277,28 +197,130 @@ namespace Microsoft.AspNetCore.Razor.TagHelpers throw new ArgumentNullException(nameof(encoder)); } - if (!IsModified) + if (!_hasContent) { return; } - foreach (var entry in _buffer) + if (_isSingleContentSet) { - if (entry == null) - { - continue; - } + WriteToCore(_singleContent, writer, encoder); + return; + } - var stringValue = entry as string; - if (stringValue != null) + for (var i = 0; i < (_buffer?.Count ?? 0); i++) + { + WriteToCore(Buffer[i], writer, encoder); + } + } + + private void WriteToCore(object entry, TextWriter writer, HtmlEncoder encoder) + { + if (entry == null) + { + return; + } + + var stringValue = entry as string; + if (stringValue != null) + { + encoder.Encode(writer, stringValue); + } + else + { + ((IHtmlContent)entry).WriteTo(writer, encoder); + } + } + + private void CopyToCore(object entry, IHtmlContentBuilder destination) + { + if (entry == null) + { + return; + } + + 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); + } + } + + private void MoveToCore(object entry, IHtmlContentBuilder destination) + { + if (entry == null) + { + return; + } + + 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); + } + } + + private bool IsEmptyOrWhiteSpaceCore(object entry, EmptyOrWhiteSpaceWriter writer) + { + if (entry == null) + { + return false; + } + + var stringValue = entry as string; + if (stringValue != null) + { + if (!string.IsNullOrWhiteSpace(stringValue)) { - encoder.Encode(writer, stringValue); - } - else - { - ((IHtmlContent)entry).WriteTo(writer, encoder); + return false; } } + else + { + ((IHtmlContent)entry).WriteTo(writer, HtmlEncoder.Default); + if (!writer.IsEmptyOrWhiteSpace) + { + return false; + } + } + + return true; + } + + private TagHelperContent AppendCore(object entry) + { + if (!_hasContent) + { + _isSingleContentSet = true; + _singleContent = entry; + } + else + { + Buffer.Add(entry); + } + + _isModified = true; + _hasContent = true; + + return this; } private string DebuggerToString() @@ -317,9 +339,7 @@ namespace Microsoft.AspNetCore.Razor.TagHelpers } } - public bool IsEmpty { get; private set; } = true; - - public bool IsWhiteSpace { get; private set; } = true; + public bool IsEmptyOrWhiteSpace { get; private set; } = true; #if NETSTANDARD1_5 // This is an abstract method in DNXCore @@ -331,14 +351,9 @@ namespace Microsoft.AspNetCore.Razor.TagHelpers public override void Write(string value) { - if (IsEmpty && !string.IsNullOrEmpty(value)) + if (IsEmptyOrWhiteSpace && !string.IsNullOrWhiteSpace(value)) { - IsEmpty = false; - } - - if (IsWhiteSpace && !string.IsNullOrWhiteSpace(value)) - { - IsWhiteSpace = false; + IsEmptyOrWhiteSpace = false; } } } diff --git a/src/Microsoft.AspNetCore.Razor.Runtime/TagHelpers/TagHelperContent.cs b/src/Microsoft.AspNetCore.Razor.Runtime/TagHelpers/TagHelperContent.cs index 16b81881c8..7a6d380215 100644 --- a/src/Microsoft.AspNetCore.Razor.Runtime/TagHelpers/TagHelperContent.cs +++ b/src/Microsoft.AspNetCore.Razor.Runtime/TagHelpers/TagHelperContent.cs @@ -19,14 +19,9 @@ namespace Microsoft.AspNetCore.Razor.TagHelpers public abstract bool IsModified { get; } /// - /// Gets a value indicating whether the content is empty. + /// Gets a value indicating whether the content is empty or whitespace. /// - public abstract bool IsEmpty { get; } - - /// - /// Gets a value indicating whether the content is whitespace. - /// - public abstract bool IsWhiteSpace { get; } + public abstract bool IsEmptyOrWhiteSpace { get; } /// /// Sets the content. diff --git a/test/Microsoft.AspNetCore.Razor.Runtime.Test/TagHelpers/DefaultTagHelperContentTest.cs b/test/Microsoft.AspNetCore.Razor.Runtime.Test/TagHelpers/DefaultTagHelperContentTest.cs index da919abab9..fca8540731 100644 --- a/test/Microsoft.AspNetCore.Razor.Runtime.Test/TagHelpers/DefaultTagHelperContentTest.cs +++ b/test/Microsoft.AspNetCore.Razor.Runtime.Test/TagHelpers/DefaultTagHelperContentTest.cs @@ -164,7 +164,7 @@ namespace Microsoft.AspNetCore.Razor.TagHelpers source.MoveTo(destination); // Assert - Assert.True(source.IsEmpty); + Assert.Equal(string.Empty, source.GetContent()); Assert.Equal(3, items.Count); Assert.Equal("some-content", Assert.IsType(items[0])); @@ -191,8 +191,8 @@ namespace Microsoft.AspNetCore.Razor.TagHelpers source.MoveTo(destination); // Assert - Assert.True(source.IsEmpty); - Assert.True(nested.IsEmpty); + Assert.Equal(string.Empty, source.GetContent()); + Assert.Equal(string.Empty, nested.GetContent()); Assert.Equal(3, items.Count); Assert.Equal("some-content", Assert.IsType(items[0])); @@ -362,7 +362,7 @@ namespace Microsoft.AspNetCore.Razor.TagHelpers [InlineData("\n")] [InlineData("\t")] [InlineData("\r")] - public void CanIdentifyWhiteSpace(string data) + public void CanIdentifyEmptyOrWhiteSpace(string data) { // Arrange var tagHelperContent = new DefaultTagHelperContent(); @@ -372,7 +372,7 @@ namespace Microsoft.AspNetCore.Razor.TagHelpers tagHelperContent.Append(data); // Assert - Assert.True(tagHelperContent.IsWhiteSpace); + Assert.True(tagHelperContent.IsEmptyOrWhiteSpace); } [Fact] @@ -386,21 +386,22 @@ namespace Microsoft.AspNetCore.Razor.TagHelpers tagHelperContent.Append("Hello"); // Assert - Assert.False(tagHelperContent.IsWhiteSpace); + Assert.True(tagHelperContent.GetContent().Length > 0); + Assert.False(tagHelperContent.IsEmptyOrWhiteSpace); } [Fact] - public void IsEmpty_InitiallyTrue() + public void IsEmptyOrWhiteSpace_InitiallyTrue() { // Arrange var tagHelperContent = new DefaultTagHelperContent(); // Act & Assert - Assert.True(tagHelperContent.IsEmpty); + Assert.True(tagHelperContent.IsEmptyOrWhiteSpace); } [Fact] - public void IsEmpty_TrueAfterSetEmptyContent() + public void IsEmptyOrWhiteSpace_TrueAfterSetEmptyContent() { // Arrange var tagHelperContent = new DefaultTagHelperContent(); @@ -409,11 +410,11 @@ namespace Microsoft.AspNetCore.Razor.TagHelpers tagHelperContent.SetContent(string.Empty); // Assert - Assert.True(tagHelperContent.IsEmpty); + Assert.True(tagHelperContent.IsEmptyOrWhiteSpace); } [Fact] - public void IsEmpty_TrueAfterAppendEmptyContent() + public void IsEmptyOrWhiteSpace_TrueAfterAppendEmptyContent() { // Arrange var tagHelperContent = new DefaultTagHelperContent(); @@ -423,11 +424,11 @@ namespace Microsoft.AspNetCore.Razor.TagHelpers tagHelperContent.Append(string.Empty); // Assert - Assert.True(tagHelperContent.IsEmpty); + Assert.True(tagHelperContent.IsEmptyOrWhiteSpace); } [Fact] - public void IsEmpty_TrueAfterAppendEmptyTagHelperContent() + public void IsEmptyOrWhiteSpace_TrueAfterAppendEmptyTagHelperContent() { // Arrange var tagHelperContent = new DefaultTagHelperContent(); @@ -438,11 +439,11 @@ namespace Microsoft.AspNetCore.Razor.TagHelpers tagHelperContent.Append(string.Empty); // Assert - Assert.True(tagHelperContent.IsEmpty); + Assert.True(tagHelperContent.IsEmptyOrWhiteSpace); } [Fact] - public void IsEmpty_TrueAfterClear() + public void IsEmptyOrWhiteSpace_TrueAfterClear() { // Arrange var tagHelperContent = new DefaultTagHelperContent(); @@ -451,11 +452,12 @@ namespace Microsoft.AspNetCore.Razor.TagHelpers tagHelperContent.Clear(); // Assert - Assert.True(tagHelperContent.IsEmpty); + Assert.Equal(string.Empty, tagHelperContent.GetContent()); + Assert.True(tagHelperContent.IsEmptyOrWhiteSpace); } [Fact] - public void IsEmpty_FalseAfterSetContent() + public void IsEmptyOrWhiteSpace_FalseAfterSetContent() { // Arrange var tagHelperContent = new DefaultTagHelperContent(); @@ -464,11 +466,11 @@ namespace Microsoft.AspNetCore.Razor.TagHelpers tagHelperContent.SetContent("Hello"); // Assert - Assert.False(tagHelperContent.IsEmpty); + Assert.False(tagHelperContent.IsEmptyOrWhiteSpace); } [Fact] - public void IsEmpty_FalseAfterAppend() + public void IsEmptyOrWhiteSpace_FalseAfterAppend() { // Arrange var tagHelperContent = new DefaultTagHelperContent(); @@ -477,11 +479,11 @@ namespace Microsoft.AspNetCore.Razor.TagHelpers tagHelperContent.Append("Hello"); // Assert - Assert.False(tagHelperContent.IsEmpty); + Assert.False(tagHelperContent.IsEmptyOrWhiteSpace); } [Fact] - public void IsEmpty_FalseAfterAppendTagHelper() + public void IsEmptyOrWhiteSpace_FalseAfterAppendTagHelper() { // Arrange var tagHelperContent = new DefaultTagHelperContent(); @@ -492,7 +494,7 @@ namespace Microsoft.AspNetCore.Razor.TagHelpers tagHelperContent.AppendHtml(copiedTagHelperContent); // Assert - Assert.False(tagHelperContent.IsEmpty); + Assert.False(tagHelperContent.IsEmptyOrWhiteSpace); } [Fact] @@ -506,7 +508,7 @@ namespace Microsoft.AspNetCore.Razor.TagHelpers tagHelperContent.Clear(); // Assert - Assert.True(tagHelperContent.IsEmpty); + Assert.True(tagHelperContent.IsEmptyOrWhiteSpace); } [Fact] @@ -632,4 +634,4 @@ namespace Microsoft.AspNetCore.Razor.TagHelpers Assert.Equal("Hi", writer.ToString()); } } -} +} \ No newline at end of file diff --git a/test/Microsoft.AspNetCore.Razor.Runtime.Test/TagHelpers/TagHelperOutputTest.cs b/test/Microsoft.AspNetCore.Razor.Runtime.Test/TagHelpers/TagHelperOutputTest.cs index c5970fb78b..04c947137a 100644 --- a/test/Microsoft.AspNetCore.Razor.Runtime.Test/TagHelpers/TagHelperOutputTest.cs +++ b/test/Microsoft.AspNetCore.Razor.Runtime.Test/TagHelpers/TagHelperOutputTest.cs @@ -1079,11 +1079,11 @@ namespace Microsoft.AspNetCore.Razor.TagHelpers // 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.Equal(string.Empty, output.PreElement.GetContent()); + Assert.Equal(string.Empty, output.PreContent.GetContent()); + Assert.Equal(string.Empty, output.Content.GetContent()); + Assert.Equal(string.Empty, output.PostContent.GetContent()); + Assert.Equal(string.Empty, output.PostElement.GetContent()); Assert.Empty(output.Attributes); Assert.Equal(expected, writer.ToString(), StringComparer.Ordinal);