More tests

Author: javiercn

src/Components/Components/src/PersistentState/ComponentStatePersistenceManager.cs

@@ -94,23 +94,33 @@ async Task PauseAndPersistState()
                 // the next store can start with a clean slate.
                 foreach (var store in compositeStore)
                 {
-                    await PersistState(store);
+                    var result = await TryPersistState(store);
+                    if (!result)
+                    {
+                        break;
+                    }
                     _currentState.Clear();
                 }
             }
             else
             {
-                await PersistState(store);
+                await TryPersistState(store);
             }
 
             State.PersistingState = false;
             _stateIsPersisted = true;
         }
 
-        async Task PersistState(IPersistentComponentStateStore store)
+        async Task<bool> TryPersistState(IPersistentComponentStateStore store)
         {
-            await PauseAsync(store);
+            if (!await TryPauseAsync(store))
+            {
+                _currentState.Clear();
+                return false;
+            }
+
             await store.PersistStateAsync(_currentState);
+            return true;
         }
     }
 
@@ -166,9 +176,9 @@ private void InferRenderModes(Renderer renderer)
         }
     }
 
-    internal Task PauseAsync(IPersistentComponentStateStore store)
+    internal Task<bool> TryPauseAsync(IPersistentComponentStateStore store)
     {
-        List<Task>? pendingCallbackTasks = null;
+        List<Task<bool>>? pendingCallbackTasks = null;
 
         // We are iterating backwards to allow the callbacks to remove themselves from the list.
         // Otherwise, we would have to make a copy of the list to avoid running into situations
@@ -189,31 +199,38 @@ internal Task PauseAsync(IPersistentComponentStateStore store)
                 continue;
             }
 
-            var result = ExecuteCallback(registration.Callback, _logger);
+            var result = TryExecuteCallback(registration.Callback, _logger);
             if (!result.IsCompletedSuccessfully)
             {
-                pendingCallbackTasks ??= new();
+                pendingCallbackTasks ??= [];
                 pendingCallbackTasks.Add(result);
             }
+            else
+            {
+                if (!result.Result)
+                {
+                    return Task.FromResult(false);
+                }
+            }
         }
 
         if (pendingCallbackTasks != null)
         {
-            return Task.WhenAll(pendingCallbackTasks);
+            return AnyTaskFailed(pendingCallbackTasks);
         }
         else
         {
-            return Task.CompletedTask;
+            return Task.FromResult(true);
         }
 
-        static Task ExecuteCallback(Func<Task> callback, ILogger<ComponentStatePersistenceManager> logger)
+        static Task<bool> TryExecuteCallback(Func<Task> callback, ILogger<ComponentStatePersistenceManager> logger)
         {
             try
             {
                 var current = callback();
                 if (current.IsCompletedSuccessfully)
                 {
-                    return current;
+                    return Task.FromResult(true);
                 }
                 else
                 {
@@ -223,21 +240,35 @@ static Task ExecuteCallback(Func<Task> callback, ILogger<ComponentStatePersisten
             catch (Exception ex)
             {
                 logger.LogError(new EventId(1000, "PersistenceCallbackError"), ex, "There was an error executing a callback while pausing the application.");
-                return Task.CompletedTask;
+                return Task.FromResult(false);
             }
 
-            static async Task Awaited(Task task, ILogger<ComponentStatePersistenceManager> logger)
+            static async Task<bool> Awaited(Task task, ILogger<ComponentStatePersistenceManager> logger)
             {
                 try
                 {
                     await task;
+                    return true;
                 }
                 catch (Exception ex)
                 {
                     logger.LogError(new EventId(1000, "PersistenceCallbackError"), ex, "There was an error executing a callback while pausing the application.");
-                    return;
+                    return false;
                 }
             }
         }
+
+        static async Task<bool> AnyTaskFailed(List<Task<bool>> pendingCallbackTasks)
+        {
+            foreach (var result in await Task.WhenAll(pendingCallbackTasks))
+            {
+                if (!result)
+                {
+                    return false;
+                }
+            }
+
+            return true;
+        }
     }
 }

src/Components/Components/src/SupplyParameterFromPersistentComponentStateProviderServiceCollectionExtensions.cs

@@ -45,11 +45,7 @@ public static IServiceCollection AddSupplyValueFromPersistentComponentStateProvi
         // We look for the assembly in the current list of loaded assemblies.
         // We look for the type inside the assembly.
         // We resolve the service from the DI container.
-        // TODO: We can support registering for a specific render mode at this level (that way no info gets sent to the client accidentally 4 example).
-        //       Even as far as defaulting to Server (to avoid disclosing anything confidential to the client, even though is the Developer responsibility).
-        //       We can choose to fail when the service is not registered on DI.
         // We loop through the properties in the type and try to restore the properties that have SupplyParameterFromPersistentComponentState on them.
-
         services.TryAddEnumerable(ServiceDescriptor.Singleton<IPersistentComponentRegistration>(new PersistentComponentRegistration<TService>(componentRenderMode)));
 
         return services;

src/Components/Components/src/SupplyParameterFromPersistentComponentStateValueProvider.cs

@@ -18,6 +18,8 @@ internal sealed class SupplyParameterFromPersistentComponentStateValueProvider(P
     private readonly Dictionary<ComponentState, PersistingComponentStateSubscription> _subscriptions = [];
 
     public bool IsFixed => false;
+    // For testing purposes only
+    internal Dictionary<ComponentState, PersistingComponentStateSubscription> Subscriptions => _subscriptions;
 
     public bool CanSupplyValue(in CascadingParameterInfo parameterInfo)
         => parameterInfo.Attribute is SupplyParameterFromPersistentComponentStateAttribute;
@@ -31,7 +33,7 @@ public bool CanSupplyValue(in CascadingParameterInfo parameterInfo)
         "IL2072:Target parameter argument does not satisfy 'DynamicallyAccessedMembersAttribute' in call to target method. The return value of the source method does not have matching annotations.",
         Justification = "JSON serialization and deserialization might require types that cannot be statically analyzed.")]
     public object? GetCurrentValue(in CascadingParameterInfo parameterInfo) =>
-        state.TryTakeFromJson(parameterInfo.PropertyName, parameterInfo.PropertyType, out var value) ? value : null;
+        throw new InvalidOperationException("Using this provider requires a key.");
 
     [UnconditionalSuppressMessage(
         "ReflectionAnalysis",
@@ -74,7 +76,8 @@ public void Unsubscribe(ComponentState subscriber, in CascadingParameterInfo par
         }
     }
 
-    private static string ComputeKey(ComponentState componentState, string propertyName)
+    // Internal for testing only
+    internal static string ComputeKey(ComponentState componentState, string propertyName)
     {
         // We need to come up with a pseudo-unique key for the storage key.
         // We need to consider the property name, the component type, and its position within the component tree.
@@ -110,20 +113,58 @@ private static string ComputeFinalKey(byte[] preKey, ComponentState componentSta
         {
             Span<byte> keyBuffer = stackalloc byte[1024];
             preKey.CopyTo(keyBuffer);
-            if (key is IUtf8SpanFormattable formattable)
+            if (key is IUtf8SpanFormattable spanFormattable)
             {
-                while (!formattable.TryFormat(keyBuffer[preKey.Length..], out var written, "{0:G}", CultureInfo.InvariantCulture))
+                var wroteKey = false;
+                while (!wroteKey)
                 {
-                    // It is really unlikely that we will enter here, but we need to handle this case
-                    Debug.Assert(written == 0);
-                    var newPool = pool == null ? ArrayPool<byte>.Shared.Rent(2048) : ArrayPool<byte>.Shared.Rent(pool.Length * 2);
-                    keyBuffer[0..preKey.Length].CopyTo(newPool);
-                    keyBuffer = newPool;
-                    if (pool != null)
+                    wroteKey = spanFormattable.TryFormat(keyBuffer[preKey.Length..], out var written, "", CultureInfo.InvariantCulture);
+                    if (!wroteKey)
                     {
-                        ArrayPool<byte>.Shared.Return(pool, clearArray: true);
+                        // It is really unlikely that we will enter here, but we need to handle this case
+                        Debug.Assert(written == 0);
+                        GrowBuffer(preKey, ref pool, ref keyBuffer);
+                    }
+                    else
+                    {
+                        keyBuffer = keyBuffer[..(preKey.Length + written)];
+                    }
+                }
+            }
+            else if (key is IFormattable formattable)
+            {
+                var keyString = formattable.ToString("", CultureInfo.InvariantCulture);
+                var wroteKey = false;
+                while (!wroteKey)
+                {
+                    wroteKey = Encoding.UTF8.TryGetBytes(keyString, keyBuffer[preKey.Length..], out var written);
+                    if (!wroteKey)
+                    {
+                        Debug.Assert(written == 0);
+                        GrowBuffer(preKey, ref pool, ref keyBuffer);
+                    }
+                    else
+                    {
+                        keyBuffer = keyBuffer[..(preKey.Length + written)];
+                    }
+                }
+            }
+            else if (key is IConvertible convertible)
+            {
+                var keyString = convertible.ToString(CultureInfo.InvariantCulture);
+                var wroteKey = false;
+                while (!wroteKey)
+                {
+                    wroteKey = Encoding.UTF8.TryGetBytes(keyString, keyBuffer[preKey.Length..], out var written);
+                    if (!wroteKey)
+                    {
+                        Debug.Assert(written == 0);
+                        GrowBuffer(preKey, ref pool, ref keyBuffer);
+                    }
+                    else
+                    {
+                        keyBuffer = keyBuffer[..(preKey.Length + written)];
                     }
-                    pool = newPool;
                 }
             }
 
@@ -139,6 +180,18 @@ private static string ComputeFinalKey(byte[] preKey, ComponentState componentSta
         }
     }
 
+    private static void GrowBuffer(byte[] preKey, ref byte[]? pool, ref Span<byte> keyBuffer)
+    {
+        var newPool = pool == null ? ArrayPool<byte>.Shared.Rent(2048) : ArrayPool<byte>.Shared.Rent(pool.Length * 2);
+        keyBuffer[0..preKey.Length].CopyTo(newPool);
+        keyBuffer = newPool;
+        if (pool != null)
+        {
+            ArrayPool<byte>.Shared.Return(pool, clearArray: true);
+        }
+        pool = newPool;
+    }
+
     private static object? GetSerializableKey(ComponentState componentState)
     {
         if (componentState.LogicalParentComponentState is not { } parentComponentState)
@@ -173,10 +226,18 @@ private static string GetParentComponentType(ComponentState componentState) =>
     private static byte[] KeyFactory((string parentComponentType, string componentType, string propertyName) parts) =>
         Encoding.UTF8.GetBytes(string.Join(".", parts.parentComponentType, parts.componentType, parts.propertyName));
 
-    private static bool IsSerializableKey(object key) =>
-        key is { } componentKey && componentKey.GetType() is Type type &&
-            (Type.GetTypeCode(type) != TypeCode.Object
-            || type == typeof(Guid)
-            || type == typeof(DateOnly)
-            || type == typeof(TimeOnly));
+    private static bool IsSerializableKey(object key)
+    {
+        if (key == null)
+        {
+            return false;
+        }
+        var keyType = key.GetType();
+        var result = Type.GetTypeCode(keyType) != TypeCode.Object
+            || keyType == typeof(Guid)
+            || keyType == typeof(DateOnly)
+            || keyType == typeof(TimeOnly);
+
+        return result;
+    }
 }

src/Components/Components/test/PersistentState/ComponentStatePersistenceManagerTest.cs

@@ -302,6 +302,55 @@ public async Task PersistStateAsync_ContinuesInvokingPauseCallbacksDuringPersist
         Assert.Equal(LogLevel.Error, log.LogLevel);
     }
 
+    [Fact]
+    public async Task PersistStateAsync_InvokesAllCallbacksEvenIfACallbackIsRemovedAsPartOfRunningIt()
+    {
+        // Arrange
+        var state = new Dictionary<string, byte[]>();
+        var store = new TestStore(state);
+        var persistenceManager = new ComponentStatePersistenceManager(
+            NullLogger<ComponentStatePersistenceManager>.Instance,
+            CreateServiceProvider());
+        var renderer = new TestRenderer();
+
+        var executionSequence = new List<int>();
+
+        persistenceManager.State.RegisterOnPersisting(() =>
+        {
+            executionSequence.Add(1);
+            return Task.CompletedTask;
+        }, new TestRenderMode());
+
+        PersistingComponentStateSubscription subscription2 = default;
+        subscription2 = persistenceManager.State.RegisterOnPersisting(() =>
+        {
+            executionSequence.Add(2);
+            subscription2.Dispose();
+            return Task.CompletedTask;
+        }, new TestRenderMode());
+
+        var tcs = new TaskCompletionSource();
+        persistenceManager.State.RegisterOnPersisting(async () =>
+        {
+            executionSequence.Add(3);
+            await tcs.Task;
+            executionSequence.Add(4);
+        }, new TestRenderMode());
+
+        // Act
+        var persistTask = persistenceManager.PersistStateAsync(store, renderer);
+        tcs.SetResult(); // Allow the async callback to complete
+        await persistTask;
+
+        // Assert
+        Assert.Contains(3, executionSequence);
+        Assert.Contains(2, executionSequence);
+        Assert.Contains(1, executionSequence);
+        Assert.Contains(4, executionSequence);
+
+        Assert.Equal(4, executionSequence.Count);
+    }
+
     private class TestRenderer : Renderer
     {
         public TestRenderer() : base(new ServiceCollection().BuildServiceProvider(), NullLoggerFactory.Instance)