Merge pull request #471 from TimeWarpEngineering/Cramer/2024-08-13/St…

…ateReadOnlyPublicPropertiesAnalyzer

Add analyzer to check for ICloneable or parameterless constructor

Directory.Build.props

@@ -1,7 +1,7 @@
 <Project>
   <!-- Set common properties regarding assembly information and nuget packages -->
   <PropertyGroup>
-    <TimeWarpStateVersion>11.0.0-beta.85+8.0.303</TimeWarpStateVersion>
+    <TimeWarpStateVersion>11.0.0-beta.86+8.0.400</TimeWarpStateVersion>
     <Authors>Steven T. Cramer</Authors>
     <Product>TimeWarp State</Product>
     <PackageVersion>$(TimeWarpStateVersion)</PackageVersion>

Source/TimeWarp.State.Analyzer/AnalyzerReleases.Unshipped.md

@@ -10,3 +10,4 @@ StateReadOnlyPublicPropertiesRule | Design | Error | StateReadOnlyPublicProperti
 StateSealedClassRule | Design | Warning | StateInheritanceAnalyzer
 TW0001 | TimeWarp.State | Error | TimeWarpStateActionAnalyzer
 TWD001 | Debug | Info | TimeWarpStateActionAnalyzer
+TWS001 | Design | Error | StateImplementationAnalyzer

Source/TimeWarp.State.Analyzer/StateImplementationAnalyzer.cs

@@ -0,0 +1,56 @@
+namespace TimeWarp.State.Analyzer;
+
+[DiagnosticAnalyzer(LanguageNames.CSharp)]
+public class StateImplementationAnalyzer : DiagnosticAnalyzer
+{
+  public const string DiagnosticId = "TWS001";
+
+  private static readonly LocalizableString Title = "State implementation must implement ICloneable or have a parameterless constructor";
+  private static readonly LocalizableString MessageFormat = "The state implementation '{0}' must implement ICloneable or have a parameterless constructor";
+  private static readonly LocalizableString Description = "State implementations should either implement ICloneable for proper cloning or have a parameterless constructor for serialization purposes.";
+  private const string Category = "Design";
+
+  private static readonly DiagnosticDescriptor Rule = new(DiagnosticId, Title, MessageFormat, Category, DiagnosticSeverity.Error, isEnabledByDefault: true, description: Description);
+
+  public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics { get { return ImmutableArray.Create(Rule); } }
+
+  public override void Initialize(AnalysisContext context)
+  {
+    context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.None);
+    context.EnableConcurrentExecution();
+
+    context.RegisterSymbolAction(AnalyzeSymbol, SymbolKind.NamedType);
+  }
+
+  private static void AnalyzeSymbol(SymbolAnalysisContext context)
+  {
+    var namedTypeSymbol = (INamedTypeSymbol)context.Symbol;
+
+    if (IsStateImplementation(namedTypeSymbol))
+    {
+      if (!ImplementsICloneable(namedTypeSymbol) && !HasParameterlessConstructor(namedTypeSymbol))
+      {
+        var diagnostic = Diagnostic.Create(Rule, namedTypeSymbol.Locations[0], namedTypeSymbol.Name);
+        context.ReportDiagnostic(diagnostic);
+      }
+    }
+  }
+
+  private static bool IsStateImplementation(INamedTypeSymbol symbol)
+  {
+    if (symbol.BaseType == null)
+      return false;
+
+    return symbol.BaseType.Name == "State" && symbol.BaseType.TypeArguments.Length == 1;
+  }
+
+  private static bool ImplementsICloneable(INamedTypeSymbol symbol)
+  {
+    return symbol.AllInterfaces.Any(i => i.Name == "ICloneable");
+  }
+
+  private static bool HasParameterlessConstructor(INamedTypeSymbol symbol)
+  {
+    return symbol.Constructors.Any(c => c.Parameters.Length == 0);
+  }
+}

Source/TimeWarp.State.Plus/EventIds.cs

@@ -16,6 +16,7 @@ internal class EventIds
 
   // Feature - StateInitializedNotificationHandler
   public static readonly EventId StateInitializedNotificationHandler_Handling = new(1200, nameof(StateInitializedNotificationHandler_Handling));
+  public static readonly EventId StateInitializedNotificationHandler_LoadActionSetNotFound = new(1201, nameof(StateInitializedNotificationHandler_LoadActionSetNotFound));
 
   // Feature - PersistenceService
   public static readonly EventId PersistenceService_LoadState = new(1300, nameof(PersistenceService_LoadState));

Source/TimeWarp.State.Plus/Features/FeatureFlags/FeatureFlagState/FeatureFlagState.cs

@@ -4,5 +4,8 @@ public sealed class FeatureFlagState : State<FeatureFlagState>
 {
   public FeatureFlagState(ISender sender) : base(sender) {}
 
+  [JsonConstructor]
+  public FeatureFlagState() {}
+
   public override void Initialize() => throw new NotImplementedException();
 }

Source/TimeWarp.State.Plus/Features/Persistentence/Services/PersistenceService.cs

@@ -24,9 +24,9 @@ ILogger<PersistenceService> logger
   {
     string typeName =
       stateType.Name ??
-      throw new InvalidOperationException("The type provided has a null full name, which is not supported for persistence operations.");
+      throw new InvalidOperationException(message: "The type provided has a null full name, which is not supported for persistence operations.");
 
-    Logger.LogInformation(EventIds.PersistenceService_LoadState, "Loading State for {stateType}", stateType);
+    Logger.LogInformation(EventIds.PersistenceService_LoadState, message: "Loading State for {stateType}", stateType);
 
     string? serializedState = persistentStateMethod switch
     {
@@ -40,7 +40,7 @@ ILogger<PersistenceService> logger
     Logger.LogTrace
     (
       EventIds.PersistenceService_LoadState_SerializedState,
-      "Serialized State: {serializedState}",
+      message: "Serialized State: {serializedState}",
       serializedState
     );
 
@@ -51,9 +51,15 @@ ILogger<PersistenceService> logger
       {
         result = JsonSerializer.Deserialize(serializedState, stateType, JsonSerializerOptions);
       }
-      catch (JsonException ex)
+      catch (JsonException jsonException)
       {
-        Logger.LogError(EventIds.PersistenceService_LoadState_DeserializationError, ex, "Error deserializing state for {stateType}", stateType);
+        Logger.LogError
+        (
+          EventIds.PersistenceService_LoadState_DeserializationError, 
+          jsonException, 
+          message: "Error deserializing state for {stateType}",
+          stateType
+        );
         throw;
       }
     }

Source/TimeWarp.State.Plus/Features/Persistentence/StateInitializedNotificationHandler.cs

@@ -20,13 +20,29 @@ public async Task Handle(StateInitializedNotification stateInitializedNotificati
     string assemblyQualifiedName = stateInitializedNotification.StateType.AssemblyQualifiedName ?? throw new InvalidOperationException();
 
     string typeName = assemblyQualifiedName.Replace(fullName, $"{fullName}+LoadActionSet+Action");
-    Logger.LogDebug(EventIds.StateInitializedNotificationHandler_Handling, "StateInitializedNotificationHandler: {StateTypeName}", stateInitializedNotification.StateType.Name);
+
+    Logger.LogDebug
+    (
+      EventIds.StateInitializedNotificationHandler_Handling, 
+      message: "StateInitializedNotificationHandler: {StateTypeName}", 
+      stateInitializedNotification.StateType.Name
+    );
+
     var actionType = Type.GetType(typeName);
 
     if (actionType != null)
     {
       object action = Activator.CreateInstance(actionType) ?? throw new InvalidOperationException();
       await Sender.Send(action, cancellationToken);
     }
+    else
+    {
+      Logger.LogDebug
+      (
+        EventIds.StateInitializedNotificationHandler_LoadActionSetNotFound, 
+        message: "StateInitializedNotificationHandler: {LoadActionSetTypeName} not found.", 
+        typeName
+      );
+    }
   }
 }

Source/TimeWarp.State.Plus/Features/Routing/RouteState/RouteState.cs

@@ -1,7 +1,5 @@
 namespace TimeWarp.Features.Routing;
 
-using System.Text.Json.Serialization;
-
 /// <summary>
 /// Maintain the Route in TimeWarp.State
 /// </summary>

Source/TimeWarp.State.Plus/Features/Theme/ThemeState/ThemeState.cs

@@ -5,6 +5,10 @@ public sealed partial class ThemeState : State<ThemeState>
   public Theme CurrentTheme { get; private set; }
 
   public ThemeState(ISender sender) : base(sender) {}
+
+  [JsonConstructor]
+  public ThemeState() {}
+
   public override void Initialize() => CurrentTheme = Theme.System;
 
   /// <summary>

Source/TimeWarp.State.Plus/Features/Timers/TimerState/TimerState.cs

@@ -1,13 +1,15 @@
 namespace TimeWarp.State.Plus.Features.Timers;
 
+using Microsoft.Extensions.Options;
 using System.Timers;
 
-public sealed partial class TimerState : State<TimerState>
+public sealed partial class TimerState : State<TimerState>, ICloneable
 {
   private readonly ILogger<TimerState> Logger;
   private readonly IPublisher Publisher;
   private readonly MultiTimerOptions MultiTimerOptions;
   private Dictionary<string, (Timer Timer, TimerConfig TimerConfig)> Timers = new();
+
   public TimerState
   (
     IOptions<MultiTimerOptions> multiTimerOptionsAccessor,
@@ -19,6 +21,31 @@ IPublisher publisher
     Publisher = publisher;
     MultiTimerOptions = multiTimerOptionsAccessor.Value;
   }
+
+  /// <summary>
+  /// Creates a new instance of TimerState with the same configuration and Timer instances.
+  /// </summary>
+  /// <remarks>
+  /// This method performs a shallow clone of the state.
+  /// It reuses the existing Timer instances and configuration.
+  /// If an error occurs in an action, it will not rollback to the previous state.
+  /// This approach is intentional to maintain consistency of timer states across clones.
+  /// Actions are expected to be well-tested and reliable, minimizing the risk of failures.
+  /// </remarks>
+  /// <returns>A new TimerState instance with the same configuration and Timer instances.</returns>
+  public object Clone()
+  {
+    return new TimerState
+    (
+      new OptionsWrapper<MultiTimerOptions>(MultiTimerOptions),
+      Logger,
+      Publisher
+    )
+    {
+      Timers = this.Timers
+    };
+  }
+
   public override void Initialize()
   {
     Timers.Clear();

Source/TimeWarp.State.Plus/GlobalUsings.cs

@@ -11,4 +11,5 @@
 global using Microsoft.JSInterop;
 global using System.Reflection;
 global using System.Text.Json;
+global using System.Text.Json.Serialization;
 global using TimeWarp.State.Plus;

Source/TimeWarp.State/Features/CloneState/Pipeline/StateTransactionBehavior.cs

@@ -37,7 +37,7 @@ IPublisher publisher
     Logger.LogDebug
     (
       EventIds.StateTransactionBehavior_Constructing,
-      "constructing {ClassName}<{RequestType},{ResponseType}>",
+      message: "constructing {ClassName}<{RequestType},{ResponseType}>",
       className,
       typeof(TRequest).Name,
       typeof(TResponse).Name
@@ -53,14 +53,14 @@ CancellationToken cancellationToken
   {
     // Analyzer will ensure the following.  If IAction it has to be nested in a IState implementation.
     Type enclosingStateType = typeof(TRequest).GetEnclosingStateType();
-    var originalState = (IState)Store.GetState(enclosingStateType)!;// Not null because of Analyzer
+    var originalState = (IState)Store.GetState(enclosingStateType);
     IState newState = (originalState is ICloneable cloneable) ?
       (IState)cloneable.Clone() :
       originalState.Clone
       (
         (ex, path, _, _) =>
         {
-          Logger.LogWarning("Cloning error: {path} {Message}", path, ex.Message);
+          Logger.LogWarning(message: "Cloning error: {path} {Message}", path, ex.Message);
         }
       );
 
@@ -75,7 +75,7 @@ CancellationToken cancellationToken
     Logger.LogDebug
     (
       EventIds.StateTransactionBehavior_Cloning,
-      "Cloned State of type {declaringType} originalState.Guid:{originalState_Guid} newState.Guid:{newState_Guid}",
+      message: "Cloned State of type {declaringType} originalState.Guid:{originalState_Guid} newState.Guid:{newState_Guid}",
       enclosingStateType,
       originalState.Guid,
       newState.Guid
@@ -94,15 +94,15 @@ CancellationToken cancellationToken
       (
         EventIds.StateTransactionBehavior_Exception,
         exception,
-        "Error cloning State. Type:{enclosingStateType}",
+        message: "Error cloning State. Type:{enclosingStateType}",
         enclosingStateType
       );
 
       // If something fails we restore system to previous state.
       Logger.LogInformation
       (
         EventIds.StateTransactionBehavior_Restoring,
-        "Attempting to restore State of type: {enclosingStateType}",
+        message: "Attempting to restore State of type: {enclosingStateType}",
         enclosingStateType
       );