diff --git a/src/Microsoft.AspNetCore.Mvc.TagHelpers/CacheTagHelper.cs b/src/Microsoft.AspNetCore.Mvc.TagHelpers/CacheTagHelper.cs index f25bd690ae..0e2224f541 100644 --- a/src/Microsoft.AspNetCore.Mvc.TagHelpers/CacheTagHelper.cs +++ b/src/Microsoft.AspNetCore.Mvc.TagHelpers/CacheTagHelper.cs @@ -2,6 +2,7 @@ // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System; +using System.Diagnostics; using System.IO; using System.Text; using System.Text.Encodings.Web; @@ -228,10 +229,13 @@ public void WriteTo(TextWriter writer, HtmlEncoder encoder) for (var i = 0; i < _buffer.Pages.Count; i++) { - var pageLength = Math.Min(length, PagedCharBuffer.PageSize); - writer.Write(_buffer.Pages[i], 0, pageLength); + var page = _buffer.Pages[i]; + var pageLength = Math.Min(length, page.Length); + writer.Write(page, index: 0, count: pageLength); length -= pageLength; } + + Debug.Assert(length == 0); } } } diff --git a/src/Microsoft.AspNetCore.Mvc.ViewFeatures/Internal/PagedBufferedTextWriter.cs b/src/Microsoft.AspNetCore.Mvc.ViewFeatures/Internal/PagedBufferedTextWriter.cs index 9f728e2196..5dce67153b 100644 --- a/src/Microsoft.AspNetCore.Mvc.ViewFeatures/Internal/PagedBufferedTextWriter.cs +++ b/src/Microsoft.AspNetCore.Mvc.ViewFeatures/Internal/PagedBufferedTextWriter.cs @@ -3,6 +3,7 @@ using System; using System.Buffers; +using System.Diagnostics; using System.IO; using System.Text; using System.Threading.Tasks; @@ -39,15 +40,16 @@ public override async Task FlushAsync() for (var i = 0; i < pages.Count; i++) { var page = pages[i]; - - var pageLength = Math.Min(length, PagedCharBuffer.PageSize); + var pageLength = Math.Min(length, page.Length); if (pageLength != 0) { - await _inner.WriteAsync(page, 0, pageLength); + await _inner.WriteAsync(page, index: 0, count: pageLength); } + length -= pageLength; } + Debug.Assert(length == 0); _charBuffer.Clear(); } diff --git a/src/Microsoft.AspNetCore.Mvc.ViewFeatures/Internal/PagedCharBuffer.cs b/src/Microsoft.AspNetCore.Mvc.ViewFeatures/Internal/PagedCharBuffer.cs index 7d13c34768..f086daf67b 100644 --- a/src/Microsoft.AspNetCore.Mvc.ViewFeatures/Internal/PagedCharBuffer.cs +++ b/src/Microsoft.AspNetCore.Mvc.ViewFeatures/Internal/PagedCharBuffer.cs @@ -26,9 +26,9 @@ public int Length get { var length = _charIndex; - if (Pages.Count > 1) + for (var i = 0; i < Pages.Count - 1; i++) { - length += PageSize * (Pages.Count - 1); + length += Pages[i].Length; } return length; @@ -120,8 +120,7 @@ public void Clear() private char[] GetCurrentPage() { - if (CurrentPage == null || - _charIndex == PageSize) + if (CurrentPage == null || _charIndex == CurrentPage.Length) { CurrentPage = NewPage(); _charIndex = 0; diff --git a/test/Microsoft.AspNetCore.Mvc.ViewFeatures.Test/Internal/PagedBufferedTextWriterTest.cs b/test/Microsoft.AspNetCore.Mvc.ViewFeatures.Test/Internal/PagedBufferedTextWriterTest.cs index 1f3db0aefb..42bc170a5d 100644 --- a/test/Microsoft.AspNetCore.Mvc.ViewFeatures.Test/Internal/PagedBufferedTextWriterTest.cs +++ b/test/Microsoft.AspNetCore.Mvc.ViewFeatures.Test/Internal/PagedBufferedTextWriterTest.cs @@ -265,6 +265,66 @@ public async Task FlushAsync_ReturnsPages() Assert.Equal(3, pool.Returned.Count); } + [Fact] + public async Task FlushAsync_FlushesContent() + { + // Arrange + var pool = new TestArrayPool(); + var inner = new StringWriter(); + + var writer = new PagedBufferedTextWriter(pool, inner); + for (var i = 0; i < Content.Length; i++) + { + writer.Write(Content[i]); + } + + // Act + await writer.FlushAsync(); + + // Assert + Assert.Equal(Content, inner.ToString().ToCharArray()); + } + + [Fact] + public async Task FlushAsync_WritesContentToInner() + { + // Arrange + var pool = new TestArrayPool(); + var inner = new StringWriter(); + + var writer = new PagedBufferedTextWriter(pool, inner); + for (var i = 0; i < Content.Length; i++) + { + writer.Write(Content[i]); + } + + // Act + await writer.FlushAsync(); + + // Assert + Assert.Equal(Content, inner.ToString().ToCharArray()); + } + + [Fact] + public async Task FlushAsync_WritesContentToInner_WithLargeArrays() + { + // Arrange + var pool = new RentMoreArrayPool(); + var inner = new StringWriter(); + + var writer = new PagedBufferedTextWriter(pool, inner); + for (var i = 0; i < Content.Length; i++) + { + writer.Write(Content[i]); + } + + // Act + await writer.FlushAsync(); + + // Assert + Assert.Equal(Content, inner.ToString().ToCharArray()); + } + private class TestArrayPool : ArrayPool { public IList Returned { get; } = new List(); @@ -279,5 +339,20 @@ public override void Return(char[] buffer, bool clearArray = false) Returned.Add(buffer); } } + + private class RentMoreArrayPool : ArrayPool + { + public IList Returned { get; } = new List(); + + public override char[] Rent(int minimumLength) + { + return new char[2 * minimumLength]; + } + + public override void Return(char[] buffer, bool clearArray = false) + { + Returned.Add(buffer); + } + } } -} +} \ No newline at end of file diff --git a/test/Microsoft.AspNetCore.Mvc.ViewFeatures.Test/Internal/PagedCharBufferTest.cs b/test/Microsoft.AspNetCore.Mvc.ViewFeatures.Test/Internal/PagedCharBufferTest.cs index ba43cd8ceb..28944150ff 100644 --- a/test/Microsoft.AspNetCore.Mvc.ViewFeatures.Test/Internal/PagedCharBufferTest.cs +++ b/test/Microsoft.AspNetCore.Mvc.ViewFeatures.Test/Internal/PagedCharBufferTest.cs @@ -26,7 +26,7 @@ public void AppendWithChar_AddsCharacterToPage() } [Fact] - public void AppendWithChar_AddsCharacterToTheLastPage() + public void AppendWithChar_AddsCharacterToNewPage() { // Arrange var stringToAppend = new string('a', PagedCharBuffer.PageSize); @@ -62,6 +62,75 @@ public void AppendWithChar_AppendsToTheCurrentPageIfItIsNotFull() Assert.Equal(4, buffer.Length); } + [Fact] + public void AppendWithChar_AppendsLastCharacterToTheCurrentPage() + { + // Arrange + var stringToAppend = new string('a', PagedCharBuffer.PageSize - 1); + var charToAppend = 't'; + var buffer = new PagedCharBuffer(new CharArrayBufferSource()); + + // Act + buffer.Append(stringToAppend); + buffer.Append(charToAppend); + + // Assert + Assert.Equal(PagedCharBuffer.PageSize, buffer.Length); + var page = Assert.Single(buffer.Pages); + Assert.Equal(stringToAppend.ToCharArray(), page.Take(PagedCharBuffer.PageSize - 1)); + Assert.Equal('t', page[PagedCharBuffer.PageSize - 1]); + } + + [Fact] + public void AppendWithChar_AddsCharacterToNewPage_AfterLengthFallback() + { + // Arrange + // Imitate ArrayPool.Shared after it falls back from its default char[1024] Bucket to the next one. + var bufferSource = new Mock(); + bufferSource + .Setup(s => s.Rent(PagedCharBuffer.PageSize)) + .Returns(() => new char[2 * PagedCharBuffer.PageSize]); + var buffer = new PagedCharBuffer(bufferSource.Object); + + var stringToAppend = new string('a', 2 * PagedCharBuffer.PageSize); + var charToAppend = 't'; + + // Act + buffer.Append(stringToAppend); + buffer.Append(charToAppend); + + // Assert + Assert.Equal(2 * PagedCharBuffer.PageSize + 1, buffer.Length); + Assert.Collection(buffer.Pages, + page => Assert.Equal(stringToAppend.ToCharArray(), page), + page => Assert.Equal(charToAppend, page[0])); + } + + [Fact] + public void AppendWithChar_AppendsLastCharacterToTheCurrentPage_AfterLengthFallback() + { + // Arrange + // Imitate ArrayPool.Shared after it falls back from its default char[1024] Bucket to the next one. + var bufferSource = new Mock(); + bufferSource + .Setup(s => s.Rent(PagedCharBuffer.PageSize)) + .Returns(new char[2 * PagedCharBuffer.PageSize]); + var buffer = new PagedCharBuffer(bufferSource.Object); + + var stringToAppend = new string('a', 2 * PagedCharBuffer.PageSize - 1); + var charToAppend = 't'; + + // Act + buffer.Append(stringToAppend); + buffer.Append(charToAppend); + + // Assert + Assert.Equal(2 * PagedCharBuffer.PageSize, buffer.Length); + var page = Assert.Single(buffer.Pages); + Assert.Equal(stringToAppend.ToCharArray(), page.Take(2 * PagedCharBuffer.PageSize - 1)); + Assert.Equal('t', page[2 * PagedCharBuffer.PageSize - 1]); + } + [Fact] public void AppendWithString_AppendsToPage() { @@ -79,7 +148,7 @@ public void AppendWithString_AppendsToPage() } [Fact] - public void AppendWithString_AcrossMultiplePages() + public void AppendWithString_AppendsToMultiplePages() { // Arrange var length = 2 * PagedCharBuffer.PageSize + 1; @@ -99,6 +168,96 @@ public void AppendWithString_AcrossMultiplePages() page => Assert.Equal('d', page[0])); } + [Fact] + public void AppendWithString_AppendsToMultiplePages_AsLengthFallsBack() + { + // Arrange + // Imitate ArrayPool.Shared as it falls back from its default char[1024] Bucket to the next one. + var bufferSource = new Mock(); + bufferSource + .SetupSequence(s => s.Rent(PagedCharBuffer.PageSize)) + .Returns(new char[PagedCharBuffer.PageSize]) + .Returns(new char[2 * PagedCharBuffer.PageSize]); + var buffer = new PagedCharBuffer(bufferSource.Object); + + // Request enough space that transition should occur. + var length = 2 * PagedCharBuffer.PageSize + 1; + var expected1 = Enumerable.Repeat('d', PagedCharBuffer.PageSize); + var expected2 = Enumerable.Repeat('d', PagedCharBuffer.PageSize + 1); + var laterString = new string('d', PagedCharBuffer.PageSize); + + // Act (loop within first Append(string) call). + buffer.Append('d'); + buffer.Append(laterString); + buffer.Append(laterString); + + // Assert + Assert.Equal(length, buffer.Length); + Assert.Collection( + buffer.Pages, + page => Assert.Equal(expected1, page), + page => Assert.Equal(expected2, page.Take(PagedCharBuffer.PageSize + 1))); + } + + [Fact] + public void AppendWithString_AppendsToMultiplePages_AsLengthReturnsToNormal() + { + // Arrange + // Imitate ArrayPool.Shared just after an entry in its default char[1024] Bucket is returned. + var bufferSource = new Mock(); + bufferSource + .SetupSequence(s => s.Rent(PagedCharBuffer.PageSize)) + .Returns(new char[2 * PagedCharBuffer.PageSize]) + .Returns(new char[PagedCharBuffer.PageSize]); + var buffer = new PagedCharBuffer(bufferSource.Object); + + // Request enough space that transition should occur. + var length = 2 * PagedCharBuffer.PageSize + 1; + var expected = Enumerable.Repeat('d', 2 * PagedCharBuffer.PageSize); + var laterString = new string('d', PagedCharBuffer.PageSize); + + // Act (loop within second Append(string) call). + buffer.Append('d'); + buffer.Append(laterString); + buffer.Append(laterString); + + // Assert + Assert.Equal(length, buffer.Length); + Assert.Collection( + buffer.Pages, + page => Assert.Equal(expected, page), + page => Assert.Equal('d', page[0])); + } + + [Fact] + public void AppendWithString_AppendsToMultiplePages_AfterLengthFallback() + { + // Arrange + // Imitate ArrayPool.Shared after it falls back from its default char[1024] Bucket to the next one. + var bufferSource = new Mock(); + bufferSource + .Setup(s => s.Rent(PagedCharBuffer.PageSize)) + .Returns(() => new char[2 * PagedCharBuffer.PageSize]); + var buffer = new PagedCharBuffer(bufferSource.Object); + + // Request enough space that transition should occur. + var length = 2 * PagedCharBuffer.PageSize + 1; + var expected = Enumerable.Repeat('d', 2 * PagedCharBuffer.PageSize); + var laterString = new string('d', PagedCharBuffer.PageSize); + + // Act (loop within second Append(string) call). + buffer.Append('d'); + buffer.Append(laterString); + buffer.Append(laterString); + + // Assert + Assert.Equal(length, buffer.Length); + Assert.Collection( + buffer.Pages, + page => Assert.Equal(expected, page), + page => Assert.Equal('d', page[0])); + } + [Fact] public void AppendWithString_AppendsToTheCurrentPageIfItIsNotEmpty() { @@ -159,6 +318,96 @@ public void AppendWithCharArray_AppendsToMultiplePages() }); } + [Fact] + public void AppendWithCharArray_AppendsToMultiplePages_AsLengthFallsBack() + { + // Arrange + // Imitate ArrayPool.Shared as it falls back from its default char[1024] Bucket to the next one. + var bufferSource = new Mock(); + bufferSource + .SetupSequence(s => s.Rent(PagedCharBuffer.PageSize)) + .Returns(new char[PagedCharBuffer.PageSize]) + .Returns(new char[2 * PagedCharBuffer.PageSize]); + var buffer = new PagedCharBuffer(bufferSource.Object); + + // Request enough space that transition should occur. + var length = 2 * PagedCharBuffer.PageSize + 1; + var expected1 = Enumerable.Repeat('d', PagedCharBuffer.PageSize); + var expected2 = Enumerable.Repeat('d', PagedCharBuffer.PageSize + 1); + var laterChars = new string('d', PagedCharBuffer.PageSize).ToCharArray(); + + // Act (loop within first Append(char[]) call). + buffer.Append('d'); + buffer.Append(laterChars, 0, laterChars.Length); + buffer.Append(laterChars, 0, laterChars.Length); + + // Assert + Assert.Equal(length, buffer.Length); + Assert.Collection( + buffer.Pages, + page => Assert.Equal(expected1, page), + page => Assert.Equal(expected2, page.Take(PagedCharBuffer.PageSize + 1))); + } + + [Fact] + public void AppendWithCharArray_AppendsToMultiplePages_AsLengthReturnsToNormal() + { + // Arrange + // Imitate ArrayPool.Shared just after an entry in its default char[1024] Bucket is returned. + var bufferSource = new Mock(); + bufferSource + .SetupSequence(s => s.Rent(PagedCharBuffer.PageSize)) + .Returns(new char[2 * PagedCharBuffer.PageSize]) + .Returns(new char[PagedCharBuffer.PageSize]); + var buffer = new PagedCharBuffer(bufferSource.Object); + + // Request enough space that transition should occur. + var length = 2 * PagedCharBuffer.PageSize + 1; + var expected = Enumerable.Repeat('d', 2 * PagedCharBuffer.PageSize); + var laterChars = new string('d', PagedCharBuffer.PageSize).ToCharArray(); + + // Act (loop within second Append(char[]) call). + buffer.Append('d'); + buffer.Append(laterChars, 0, laterChars.Length); + buffer.Append(laterChars, 0, laterChars.Length); + + // Assert + Assert.Equal(length, buffer.Length); + Assert.Collection( + buffer.Pages, + page => Assert.Equal(expected, page), + page => Assert.Equal('d', page[0])); + } + + [Fact] + public void AppendWithCharArray_AppendsToMultiplePages_AfterLengthFallback() + { + // Arrange + // Imitate ArrayPool.Shared after it falls back from its default char[1024] Bucket to the next one. + var bufferSource = new Mock(); + bufferSource + .Setup(s => s.Rent(PagedCharBuffer.PageSize)) + .Returns(() => new char[2 * PagedCharBuffer.PageSize]); + var buffer = new PagedCharBuffer(bufferSource.Object); + + // Request enough space that transition should occur. + var length = 2 * PagedCharBuffer.PageSize + 1; + var expected = Enumerable.Repeat('d', 2 * PagedCharBuffer.PageSize); + var laterChars = new string('d', PagedCharBuffer.PageSize).ToCharArray(); + + // Act (loop within second Append(char[]) call). + buffer.Append('d'); + buffer.Append(laterChars, 0, laterChars.Length); + buffer.Append(laterChars, 0, laterChars.Length); + + // Assert + Assert.Equal(length, buffer.Length); + Assert.Collection( + buffer.Pages, + page => Assert.Equal(expected, page), + page => Assert.Equal('d', page[0])); + } + [Fact] public void AppendWithCharArray_AppendsToCurrentPage() {