From c2eaa6adb9575811dcb67d2135eaed981c99a568 Mon Sep 17 00:00:00 2001 From: Steve Sanderson Date: Wed, 26 Jun 2024 15:55:52 +0100 Subject: [PATCH 1/2] Limit MaxItemCount in Virtualize --- .../Web/src/Virtualization/Virtualize.cs | 57 ++++++++++++++++--- .../test/E2ETest/Tests/VirtualizationTest.cs | 19 +++++++ .../test/testassets/BasicTestApp/Index.razor | 1 + .../VirtualizationMaxItemCount.razor | 48 ++++++++++++++++ 4 files changed, 117 insertions(+), 8 deletions(-) create mode 100644 src/Components/test/testassets/BasicTestApp/VirtualizationMaxItemCount.razor diff --git a/src/Components/Web/src/Virtualization/Virtualize.cs b/src/Components/Web/src/Virtualization/Virtualize.cs index 61f22da4a73a..b0873be320cd 100644 --- a/src/Components/Web/src/Virtualization/Virtualize.cs +++ b/src/Components/Web/src/Virtualization/Virtualize.cs @@ -25,6 +25,14 @@ public sealed class Virtualize : ComponentBase, IVirtualizeJsCallbacks, I private int _visibleItemCapacity; + // If the client reports a viewport so large that it could show more than MaxItemCount items, + // we keep track of the "unused" capacity, which is the amount of blank space we want to leave + // at the bottom of the viewport (as a number of items). If we didn't leave this blank space, + // then the bottom spacer would always stay visible and the client would request more items in an + // infinite (but asynchronous) loop, as it would believe there are more items to render and + // enough space to render them into. + private int _unusedItemCapacity; + private int _itemCount; private int _loadedItemsStartIndex; @@ -118,6 +126,22 @@ public sealed class Virtualize : ComponentBase, IVirtualizeJsCallbacks, I [Parameter] public string SpacerElement { get; set; } = "div"; + /* + This API will be added in .NET 9 but cannot be added in a .NET 8 or earlier patch, + as we can't change public API in patches. + + /// + /// Gets or sets the maximum number of items that will be rendered, even if the client reports + /// that its viewport is large enough to show more. The default value is 100. + /// + /// This should only be used as a safeguard against excessive memory usage or large data loads. + /// Do not set this to a smaller number than you expect to fit on a realistic-sized window, because + /// that will leave a blank gap below and the user may not be able to see the rest of the content. + /// + [Parameter] + public int MaxItemCount { get; set; } = 100; + */ + /// /// Instructs the component to re-request data from its . /// This is useful if external data may have changed. There is no need to call this @@ -264,18 +288,23 @@ protected override void BuildRenderTree(RenderTreeBuilder builder) var itemsAfter = Math.Max(0, _itemCount - _visibleItemCapacity - _itemsBefore); builder.OpenElement(7, SpacerElement); - builder.AddAttribute(8, "style", GetSpacerStyle(itemsAfter)); + builder.AddAttribute(8, "style", GetSpacerStyle(itemsAfter, _unusedItemCapacity)); builder.AddElementReferenceCapture(9, elementReference => _spacerAfter = elementReference); builder.CloseElement(); } + private string GetSpacerStyle(int itemsInSpacer, int numItemsGapAbove) + => numItemsGapAbove == 0 + ? GetSpacerStyle(itemsInSpacer) + : $"height: {(itemsInSpacer * _itemSize).ToString(CultureInfo.InvariantCulture)}px; flex-shrink: 0; transform: translateY({(numItemsGapAbove * _itemSize).ToString(CultureInfo.InvariantCulture)}px);"; + private string GetSpacerStyle(int itemsInSpacer) => $"height: {(itemsInSpacer * _itemSize).ToString(CultureInfo.InvariantCulture)}px; flex-shrink: 0;"; void IVirtualizeJsCallbacks.OnBeforeSpacerVisible(float spacerSize, float spacerSeparation, float containerSize) { - CalcualteItemDistribution(spacerSize, spacerSeparation, containerSize, out var itemsBefore, out var visibleItemCapacity); + CalcualteItemDistribution(spacerSize, spacerSeparation, containerSize, out var itemsBefore, out var visibleItemCapacity, out var unusedItemCapacity); // Since we know the before spacer is now visible, we absolutely have to slide the window up // by at least one element. If we're not doing that, the previous item size info we had must @@ -286,12 +315,12 @@ void IVirtualizeJsCallbacks.OnBeforeSpacerVisible(float spacerSize, float spacer itemsBefore--; } - UpdateItemDistribution(itemsBefore, visibleItemCapacity); + UpdateItemDistribution(itemsBefore, visibleItemCapacity, unusedItemCapacity); } void IVirtualizeJsCallbacks.OnAfterSpacerVisible(float spacerSize, float spacerSeparation, float containerSize) { - CalcualteItemDistribution(spacerSize, spacerSeparation, containerSize, out var itemsAfter, out var visibleItemCapacity); + CalcualteItemDistribution(spacerSize, spacerSeparation, containerSize, out var itemsAfter, out var visibleItemCapacity, out var unusedItemCapacity); var itemsBefore = Math.Max(0, _itemCount - itemsAfter - visibleItemCapacity); @@ -304,7 +333,7 @@ void IVirtualizeJsCallbacks.OnAfterSpacerVisible(float spacerSize, float spacerS itemsBefore++; } - UpdateItemDistribution(itemsBefore, visibleItemCapacity); + UpdateItemDistribution(itemsBefore, visibleItemCapacity, unusedItemCapacity); } private void CalcualteItemDistribution( @@ -312,7 +341,8 @@ private void CalcualteItemDistribution( float spacerSeparation, float containerSize, out int itemsInSpacer, - out int visibleItemCapacity) + out int visibleItemCapacity, + out int unusedItemCapacity) { if (_lastRenderedItemCount > 0) { @@ -326,11 +356,21 @@ private void CalcualteItemDistribution( _itemSize = ItemSize; } + // This AppContext data exists as a stopgap for .NET 8 and earlier, since this is being added in a patch + // where we can't add new public API. + var maxItemCount = AppContext.GetData("Microsoft.AspNetCore.Components.Web.Virtualization.Virtualize.MaxItemCount") switch + { + int val => val, // In .NET 9, this will be Math.Min(val, MaxItemCount) + _ => 1000 // In .NET 9, this will be MaxItemCount + }; + itemsInSpacer = Math.Max(0, (int)Math.Floor(spacerSize / _itemSize) - OverscanCount); visibleItemCapacity = (int)Math.Ceiling(containerSize / _itemSize) + 2 * OverscanCount; + unusedItemCapacity = Math.Max(0, visibleItemCapacity - maxItemCount); + visibleItemCapacity -= unusedItemCapacity; } - private void UpdateItemDistribution(int itemsBefore, int visibleItemCapacity) + private void UpdateItemDistribution(int itemsBefore, int visibleItemCapacity, int unusedItemCapacity) { // If the itemcount just changed to a lower number, and we're already scrolled past the end of the new // reduced set of items, clamp the scroll position to the new maximum @@ -340,10 +380,11 @@ private void UpdateItemDistribution(int itemsBefore, int visibleItemCapacity) } // If anything about the offset changed, re-render - if (itemsBefore != _itemsBefore || visibleItemCapacity != _visibleItemCapacity) + if (itemsBefore != _itemsBefore || visibleItemCapacity != _visibleItemCapacity || unusedItemCapacity != _unusedItemCapacity) { _itemsBefore = itemsBefore; _visibleItemCapacity = visibleItemCapacity; + _unusedItemCapacity = unusedItemCapacity; var refreshTask = RefreshDataCoreAsync(renderOnSuccess: true); if (!refreshTask.IsCompleted) diff --git a/src/Components/test/E2ETest/Tests/VirtualizationTest.cs b/src/Components/test/E2ETest/Tests/VirtualizationTest.cs index d9a29aa8b6c2..11e9d901b29b 100644 --- a/src/Components/test/E2ETest/Tests/VirtualizationTest.cs +++ b/src/Components/test/E2ETest/Tests/VirtualizationTest.cs @@ -262,6 +262,25 @@ public void CanRenderHtmlTable() Assert.Contains(expectedInitialSpacerStyle, bottomSpacer.GetAttribute("style")); } + [Fact] + public void CanLimitMaxItemsRendered() + { + Browser.MountTestComponent(); + + // Despite having a 600px tall scroll area and 30px high items (600/30=20), + // we only render 10 items due to the MaxItemCount setting + var scrollArea = Browser.Exists(By.Id("virtualize-scroll-area")); + var getItems = () => scrollArea.FindElements(By.ClassName("my-item")); + Browser.Equal(10, () => getItems().Count); + Browser.Equal("Id: 0; Name: Thing 0", () => getItems().First().Text); + + // Scrolling still works and loads new data, though there's no guarantee about + // exactly how many items will show up at any one time + Browser.ExecuteJavaScript("document.getElementById('virtualize-scroll-area').scrollTop = 300;"); + Browser.NotEqual("Id: 0; Name: Thing 0", () => getItems().First().Text); + Browser.True(() => getItems().Count > 3 && getItems().Count <= 10); + } + [Fact] public void CanMutateDataInPlace_Sync() { diff --git a/src/Components/test/testassets/BasicTestApp/Index.razor b/src/Components/test/testassets/BasicTestApp/Index.razor index e437aa9811d2..d782b2e7fe68 100644 --- a/src/Components/test/testassets/BasicTestApp/Index.razor +++ b/src/Components/test/testassets/BasicTestApp/Index.razor @@ -109,6 +109,7 @@ + diff --git a/src/Components/test/testassets/BasicTestApp/VirtualizationMaxItemCount.razor b/src/Components/test/testassets/BasicTestApp/VirtualizationMaxItemCount.razor new file mode 100644 index 000000000000..3c3d793829f0 --- /dev/null +++ b/src/Components/test/testassets/BasicTestApp/VirtualizationMaxItemCount.razor @@ -0,0 +1,48 @@ +@implements IDisposable +

+ MaxItemCount is a safeguard against the client reporting a giant viewport and causing the server to perform a + correspondingly giant data load and then tracking a lot of render state. +

+ +

+ If MaxItemCount is exceeded (which it never should be for a well-behaved client), we don't offer any guarantees + that the behavior will be nice for the end user. We just guarantee to limit the .NET-side workload. As such this + E2E test deliberately does a bad thing of setting MaxItemCount to a low value for test purposes. Applications + should not do this. +

+ +
+ @* In .NET 8 and earlier, the E2E test uses an AppContext.SetData call to set MaxItemCount *@ + @* In .NET 9 onwards, it's a Virtualize component parameter *@ + +
+ Id: @context.Id; Name: @context.Name +
+ +
+ +@code { + protected override void OnInitialized() + { + // This relies on Xunit's default behavior of running tests in the same collection sequentially, + // not in parallel. From .NET 9 onwards this can be removed in favour of a Virtualize parameter. + AppContext.SetData("Microsoft.AspNetCore.Components.Web.Virtualization.Virtualize.MaxItemCount", 10); + } + + private async ValueTask> GetItems(ItemsProviderRequest request) + { + const int numThings = 100000; + + await Task.Delay(100); + return new ItemsProviderResult( + Enumerable.Range(request.StartIndex, request.Count).Select(i => new MyThing(i, $"Thing {i}")), + numThings); + } + + record MyThing(int Id, string Name); + + public void Dispose() + { + AppContext.SetData("Microsoft.AspNetCore.Components.Web.Virtualization.Virtualize.MaxItemCount", null); + } +} From 3061fe394b40452d7afa34d4e3d6d0ee2667e01c Mon Sep 17 00:00:00 2001 From: Steve Sanderson Date: Mon, 19 Aug 2024 11:52:33 +0100 Subject: [PATCH 2/2] Add public API for MaxItemCount --- .../Web/src/PublicAPI.Unshipped.txt | 2 + .../Web/src/Virtualization/Virtualize.cs | 14 +++---- .../test/E2ETest/Tests/VirtualizationTest.cs | 17 ++++++-- .../test/testassets/BasicTestApp/Index.razor | 1 + .../VirtualizationMaxItemCount.razor | 19 +-------- ...irtualizationMaxItemCount_AppContext.razor | 41 +++++++++++++++++++ 6 files changed, 65 insertions(+), 29 deletions(-) create mode 100644 src/Components/test/testassets/BasicTestApp/VirtualizationMaxItemCount_AppContext.razor diff --git a/src/Components/Web/src/PublicAPI.Unshipped.txt b/src/Components/Web/src/PublicAPI.Unshipped.txt index 5c174688d13c..551aa355b90b 100644 --- a/src/Components/Web/src/PublicAPI.Unshipped.txt +++ b/src/Components/Web/src/PublicAPI.Unshipped.txt @@ -3,5 +3,7 @@ Microsoft.AspNetCore.Components.Web.Internal.IInternalWebJSInProcessRuntime Microsoft.AspNetCore.Components.Web.Internal.IInternalWebJSInProcessRuntime.InvokeJS(string! identifier, string? argsJson, Microsoft.JSInterop.JSCallResultType resultType, long targetInstanceId) -> string! Microsoft.AspNetCore.Components.Web.KeyboardEventArgs.IsComposing.get -> bool Microsoft.AspNetCore.Components.Web.KeyboardEventArgs.IsComposing.set -> void +Microsoft.AspNetCore.Components.Web.Virtualization.Virtualize.MaxItemCount.get -> int +Microsoft.AspNetCore.Components.Web.Virtualization.Virtualize.MaxItemCount.set -> void override Microsoft.AspNetCore.Components.HtmlRendering.Infrastructure.StaticHtmlRenderer.RendererInfo.get -> Microsoft.AspNetCore.Components.RendererInfo! override Microsoft.AspNetCore.Components.Routing.FocusOnNavigate.BuildRenderTree(Microsoft.AspNetCore.Components.Rendering.RenderTreeBuilder! builder) -> void diff --git a/src/Components/Web/src/Virtualization/Virtualize.cs b/src/Components/Web/src/Virtualization/Virtualize.cs index b0873be320cd..da5de296d8c6 100644 --- a/src/Components/Web/src/Virtualization/Virtualize.cs +++ b/src/Components/Web/src/Virtualization/Virtualize.cs @@ -126,10 +126,6 @@ public sealed class Virtualize : ComponentBase, IVirtualizeJsCallbacks, I [Parameter] public string SpacerElement { get; set; } = "div"; - /* - This API will be added in .NET 9 but cannot be added in a .NET 8 or earlier patch, - as we can't change public API in patches. - /// /// Gets or sets the maximum number of items that will be rendered, even if the client reports /// that its viewport is large enough to show more. The default value is 100. @@ -140,7 +136,6 @@ as we can't change public API in patches. /// [Parameter] public int MaxItemCount { get; set; } = 100; - */ /// /// Instructs the component to re-request data from its . @@ -356,12 +351,13 @@ private void CalcualteItemDistribution( _itemSize = ItemSize; } - // This AppContext data exists as a stopgap for .NET 8 and earlier, since this is being added in a patch - // where we can't add new public API. + // This AppContext data was added as a stopgap for .NET 8 and earlier, since it was added in a patch + // where we couldn't add new public API. For backcompat we still support the AppContext setting, but + // new applications should use the much more convenient MaxItemCount parameter. var maxItemCount = AppContext.GetData("Microsoft.AspNetCore.Components.Web.Virtualization.Virtualize.MaxItemCount") switch { - int val => val, // In .NET 9, this will be Math.Min(val, MaxItemCount) - _ => 1000 // In .NET 9, this will be MaxItemCount + int val => Math.Min(val, MaxItemCount), + _ => MaxItemCount }; itemsInSpacer = Math.Max(0, (int)Math.Floor(spacerSize / _itemSize) - OverscanCount); diff --git a/src/Components/test/E2ETest/Tests/VirtualizationTest.cs b/src/Components/test/E2ETest/Tests/VirtualizationTest.cs index 11e9d901b29b..f7256289a388 100644 --- a/src/Components/test/E2ETest/Tests/VirtualizationTest.cs +++ b/src/Components/test/E2ETest/Tests/VirtualizationTest.cs @@ -262,10 +262,21 @@ public void CanRenderHtmlTable() Assert.Contains(expectedInitialSpacerStyle, bottomSpacer.GetAttribute("style")); } - [Fact] - public void CanLimitMaxItemsRendered() + [Theory] + [InlineData(true)] + [InlineData(false)] + public void CanLimitMaxItemsRendered(bool useAppContext) { - Browser.MountTestComponent(); + if (useAppContext) + { + // This is to test back-compat with the switch added in a .NET 8 patch. + // Newer applications shouldn't use this technique. + Browser.MountTestComponent(); + } + else + { + Browser.MountTestComponent(); + } // Despite having a 600px tall scroll area and 30px high items (600/30=20), // we only render 10 items due to the MaxItemCount setting diff --git a/src/Components/test/testassets/BasicTestApp/Index.razor b/src/Components/test/testassets/BasicTestApp/Index.razor index d782b2e7fe68..be5417871d06 100644 --- a/src/Components/test/testassets/BasicTestApp/Index.razor +++ b/src/Components/test/testassets/BasicTestApp/Index.razor @@ -110,6 +110,7 @@ + diff --git a/src/Components/test/testassets/BasicTestApp/VirtualizationMaxItemCount.razor b/src/Components/test/testassets/BasicTestApp/VirtualizationMaxItemCount.razor index 3c3d793829f0..181e6eeec1bd 100644 --- a/src/Components/test/testassets/BasicTestApp/VirtualizationMaxItemCount.razor +++ b/src/Components/test/testassets/BasicTestApp/VirtualizationMaxItemCount.razor @@ -1,5 +1,4 @@ -@implements IDisposable -

+

MaxItemCount is a safeguard against the client reporting a giant viewport and causing the server to perform a correspondingly giant data load and then tracking a lot of render state.

@@ -12,9 +11,7 @@

- @* In .NET 8 and earlier, the E2E test uses an AppContext.SetData call to set MaxItemCount *@ - @* In .NET 9 onwards, it's a Virtualize component parameter *@ - +
Id: @context.Id; Name: @context.Name
@@ -22,13 +19,6 @@
@code { - protected override void OnInitialized() - { - // This relies on Xunit's default behavior of running tests in the same collection sequentially, - // not in parallel. From .NET 9 onwards this can be removed in favour of a Virtualize parameter. - AppContext.SetData("Microsoft.AspNetCore.Components.Web.Virtualization.Virtualize.MaxItemCount", 10); - } - private async ValueTask> GetItems(ItemsProviderRequest request) { const int numThings = 100000; @@ -40,9 +30,4 @@ } record MyThing(int Id, string Name); - - public void Dispose() - { - AppContext.SetData("Microsoft.AspNetCore.Components.Web.Virtualization.Virtualize.MaxItemCount", null); - } } diff --git a/src/Components/test/testassets/BasicTestApp/VirtualizationMaxItemCount_AppContext.razor b/src/Components/test/testassets/BasicTestApp/VirtualizationMaxItemCount_AppContext.razor new file mode 100644 index 000000000000..d272859d70b6 --- /dev/null +++ b/src/Components/test/testassets/BasicTestApp/VirtualizationMaxItemCount_AppContext.razor @@ -0,0 +1,41 @@ +@implements IDisposable +

+ This is a variation of the VirtualizationMaxItemCount test case in which the max count is set using AppContext. + This E2E test exists only to verify back-compatibility. +

+ +
+ @* In .NET 8 and earlier, the E2E test uses an AppContext.SetData call to set MaxItemCount *@ + @* In .NET 9 onwards, it's a Virtualize component parameter *@ + +
+ Id: @context.Id; Name: @context.Name +
+ +
+ +@code { + protected override void OnInitialized() + { + // This relies on Xunit's default behavior of running tests in the same collection sequentially, + // not in parallel. From .NET 9 onwards this can be removed in favour of a Virtualize parameter. + AppContext.SetData("Microsoft.AspNetCore.Components.Web.Virtualization.Virtualize.MaxItemCount", 10); + } + + private async ValueTask> GetItems(ItemsProviderRequest request) + { + const int numThings = 100000; + + await Task.Delay(100); + return new ItemsProviderResult( + Enumerable.Range(request.StartIndex, request.Count).Select(i => new MyThing(i, $"Thing {i}")), + numThings); + } + + record MyThing(int Id, string Name); + + public void Dispose() + { + AppContext.SetData("Microsoft.AspNetCore.Components.Web.Virtualization.Virtualize.MaxItemCount", null); + } +}