Address review feedback: Use filters directly instead of scenarios in…

… registrations Co-authored-by: javiercn <6995051+javiercn@users.noreply.github.com>
dotnet · Copilot · Jul 1, 2025 · Jul 1, 2025 · Jul 1, 2025 · Jul 1, 2025
commit 5d2bb55b4e0ee70ebc8a39bbf910deefec76bb01
@@ -174,45 +174,16 @@ public RestoringComponentStateSubscription RegisterOnRestoring(
         ArgumentNullException.ThrowIfNull(filter);
         ArgumentNullException.ThrowIfNull(callback);
 
-        // Create a wrapper scenario that uses the filter
-        var filterScenario = new FilterWrapperScenario(filter);
-        var registration = new RestoreComponentStateRegistration(filterScenario, callback);
+        var registration = new RestoreComponentStateRegistration(filter, callback);
         _restoringCallbacks.Add(registration);
 
-        // If we already have a current scenario and it matches, invoke immediately
-        if (CurrentScenario != null && ShouldInvokeCallback(filterScenario, CurrentScenario))
+        // If we already have a current scenario and the filter matches, invoke immediately
+        if (CurrentScenario != null && filter.ShouldRestore(CurrentScenario))
         {
             callback();
         }
 
-        return new RestoringComponentStateSubscription(_restoringCallbacks, filterScenario, callback);
-    }
-
-    /// <summary>
-    /// A scenario wrapper that uses a filter to determine if it should match the current scenario.
-    /// </summary>
-    private sealed class FilterWrapperScenario : IPersistentComponentStateScenario
-    {
-        private readonly IPersistentStateFilter _filter;
-
-        public FilterWrapperScenario(IPersistentStateFilter filter)
-        {
-            _filter = filter;
-        }
-
-        public bool IsRecurring => true; // Filter-based scenarios can be recurring
-
-        public bool ShouldMatchScenario(IPersistentComponentStateScenario currentScenario)
-        {
-            return _filter.ShouldRestore(currentScenario);
-        }
-
-        public override bool Equals(object? obj)
-        {
-            return obj is FilterWrapperScenario other && ReferenceEquals(_filter, other._filter);
-        }
-
-        public override int GetHashCode() => _filter.GetHashCode();
+        return new RestoringComponentStateSubscription(_restoringCallbacks, filter, callback);
     }
 
     /// <summary>
@@ -244,32 +215,13 @@ private void InvokeRestoringCallbacks(IPersistentComponentStateScenario scenario
         {
             var registration = _restoringCallbacks[i];
 
-            if (ShouldInvokeCallback(registration.Scenario, scenario))
+            if (registration.Filter.ShouldRestore(scenario))
             {
                 registration.Callback();
-
-                // Remove non-recurring 
8000
callbacks after invocation
-                if (!registration.Scenario.IsRecurring)
-                {
-                    _restoringCallbacks.RemoveAt(i);
-                }
             }
         }
     }
 
-    private static bool ShouldInvokeCallback(IPersistentComponentStateScenario callbackScenario, IPersistentComponentStateScenario currentScenario)
-    {
-        // Special handling for filter wrapper scenarios
-        if (callbackScenario is FilterWrapperScenario filterWrapper)
-        {
-            return filterWrapper.ShouldMatchScenario(currentScenario);
-        }
-
-        // For regular scenarios, match by type and properties
-        return callbackScenario.GetType() == currentScenario.GetType() &&
-               callbackScenario.Equals(currentScenario);
-    }
-
     private bool TryTake(string key, out byte[]? value)
     {
         ArgumentNullException.ThrowIfNull(key);

@@ -17,7 +17,7 @@ public class ComponentStatePersistenceManager
     private bool _stateIsPersisted;
     private readonly PersistentServicesRegistry? _servicesRegistry;
     private readonly Dictionary<string, byte[]> _currentState = new(StringComparer.Ordinal);
-    private int _restoreCallCount;
+    private bool _isFirstRestore = true;
 
     /// <summary>
     /// Initializes a new instance of <see cref="ComponentStatePersistenceManager"/>.
@@ -72,12 +72,11 @@ public async Task RestoreStateAsync(
     {
         var data = await store.GetPersistedStateAsync();
 
-        _restoreCallCount++;
-
-        if (_restoreCallCount == 1)
+        if (_isFirstRestore)
         {
             // First-time initialization
             State.InitializeExistingState(data);
+            _isFirstRestore = false;
         }
         else
         {

@@ -8,12 +8,12 @@ namespace Microsoft.AspNetCore.Components;
 /// </summary>
 internal readonly struct RestoreComponentStateRegistration
 {
-    public RestoreComponentStateRegistration(IPersistentComponentStateScenario scenario, Action callback)
+    public RestoreComponentStateRegistration(IPersistentStateFilter filter, Action callback)
     {
-        Scenario = scenario;
+        Filter = filter;
         Callback = callback;
     }
 
-    public IPersistentComponentStateScenario Scenario { get; }
+    public IPersistentStateFilter Filter { get; }
     public Action Callback { get; }
 }
@@ -9,28 +9,28 @@ namespace Microsoft.AspNetCore.Components;
 public readonly struct RestoringComponentStateSubscription : IDisposable
 {
     private readonly List<RestoreComponentStateRegistration>? _callbacks;
-    private readonly IPersistentComponentStateScenario? _scenario;
+    private readonly IPersistentStateFilter? _filter;
     private readonly Action? _callback;
 
     internal RestoringComponentStateSubscription(
         List<RestoreComponentStateRegistration> callbacks,
-        IPersistentComponentStateScenario scenario,
+        IPersistentStateFilter filter,
         Action callback)
     {
         _callbacks = callbacks;
-        _scenario = scenario;
+        _filter = filter;
         _callback = callback;
     }
 
     /// <inheritdoc />
     public void Dispose()
     {
-        if (_callbacks != null && _scenario != null && _callback != null)
+        if (_callbacks != null && _filter != null && _callback != null)
         {
             for (int i = _callbacks.Count - 1; i >= 0; i--)
             {
                 var registration = _callbacks[i];
-                if (ReferenceEquals(registration.Scenario, _scenario) && ReferenceEquals(registration.Callback, _callback))
+                if (ReferenceEquals(registration.Filter, _filter) && ReferenceEquals(registration.Callback, _callback))
                 {
                     _callbacks.RemoveAt(i);
                     break;

@@ -21,6 +21,7 @@ internal sealed class SupplyParameterFromPersistentComponentStateValueProvider(P
     private static readonly ConcurrentDictionary<(Type, string), PropertyGetter> _propertyGetterCache = new();
 
     private readonly Dictionary<ComponentState, PersistingComponentStateSubscription> _subscriptions = [];
+    private readonly Dictionary<(ComponentState, string), object?> _scenarioRestoredValues = [];
 
     public bool IsFixed => false;
     // For testing purposes only
@@ -40,8 +41,15 @@ public bool CanSupplyValue(in CascadingParameterInfo parameterInfo)
     public object? GetCurrentValue(object? key, in CascadingParameterInfo parameterInfo)
     {
         var componentState = (ComponentState)key!;
+
+        // Check if we have a scenario-restored value first
+        var valueKey = (componentState, parameterInfo.PropertyName);
+        if (_scenarioRestoredValues.TryGetValue(valueKey, out var scenarioValue))
+        {
+            return scenarioValue;
+        }
+
         var storageKey = ComputeKey(componentState, parameterInfo.PropertyName);
-
         return state.TryTakeFromJson(storageKey, parameterInfo.PropertyType, out var value) ? value : null;
     }
 
@@ -293,27 +301,28 @@ private void RegisterScenarioRestorationCallback(ComponentState subscriber, in C
     {
         // Check for IPersistentStateFilter attributes
         var filterAttributes = propertyInfo.GetCustomAttributes(typeof(IPersistentStateFilter), inherit: true);
-        if (filterAttributes.Length == 0)
+
+        // Register restoration callbacks for each filter
+        foreach (IPersistentStateFilter filter in filterAttributes)
         {
-            return; // No filters, no scenario-based restoration needed
+            RegisterRestorationCallback(subscriber, parameterInfo, filter);
         }
+    }
 
+    [UnconditionalSuppressMessage("Trimming", "IL2026:Members annotated with 'RequiresUnreferencedCodeAttribute' require dynamic access otherwise can break functionality when trimming application code", Justification = "Property types of rendered components are preserved through other means and won't get trimmed.")]
+    [UnconditionalSuppressMessage("Trimming", "IL2072:'type' argument does not satisfy 'DynamicallyAccessedMemberTypes.PublicConstructors', 'DynamicallyAccessedMemberTypes.PublicFields', 'DynamicallyAccessedMemberTypes.PublicProperties' in call to target method. The return value of the source method does not have matching annotations.", Justification = "Property types of rendered components are preserved through other means and won't get trimmed.")]
+    private void RegisterRestorationCallback(ComponentState subscriber, in CascadingParameterInfo parameterInfo, IPersistentStateFilter filter)
+    {
         var storageKey = ComputeKey(subscriber, parameterInfo.PropertyName);
         var propertyType = parameterInfo.PropertyType;
-        var component = subscriber.Component;
+        var valueKey = (subscriber, parameterInfo.PropertyName);
 
-        // Register restoration callbacks for each filter
-        foreach (IPersistentStateFilter filter in filterAttributes)
+        state.RegisterOnRestoring(filter, () =>
         {
-            state.RegisterOnRestoring(filter, () =>
+            if (state.TryTakeFromJson(storageKey, propertyType, out var value))
             {
-                if (state.TryTakeFromJson(storageKey, propertyType, out var value))
-                {
-                    // Set the property value on the component
-                    propertyInfo.SetValue(component, value);
-                    // The component will re-render naturally when needed
-                }
-            });
-        }
+                _scenarioRestoredValues[valueKey] = value;
+            }
+        });
     }
 }