From f182483acefb5aff74e49efc2355a8210b572b2e Mon Sep 17 00:00:00 2001 From: Tomperez98 Date: Wed, 9 Sep 2026 12:08:17 -0500 Subject: [PATCH] Harden state exploration and validation --- .../ConcurrentTestCaseAlgorithms.cs | 2 +- .../StepFunctionApplicationException.cs | 23 +- Accordant/State/State.cs | 20 +- Accordant/StateGraph.cs | 433 +++++++++++++++--- Accordant/StateGraphExpander.cs | 54 ++- Accordant/StateProfile.cs | 59 ++- .../StepFunction/ContractStepFunction.cs | 93 +++- Accordant/StepFunction/IStepFunction.cs | 99 +++- Accordant/SystemChecker.cs | 214 +++++++-- .../SharedDictionaryReferenceTests.cs | 4 +- .../FailFastValidationTests.cs | 361 +++++++++++++++ .../StateFreezeValidationTests.cs | 78 ++++ .../StateGraphValidationTests.cs | 248 ++++++++++ 13 files changed, 1526 insertions(+), 162 deletions(-) create mode 100644 Tests/Accordant.Tests/FailFastValidationTests.cs create mode 100644 Tests/Accordant.Tests/StateFreezeValidationTests.cs create mode 100644 Tests/Accordant.Tests/StateGraphValidationTests.cs diff --git a/Accordant.Operations/GenerationAlgorithms/ConcurrentTestCaseAlgorithms.cs b/Accordant.Operations/GenerationAlgorithms/ConcurrentTestCaseAlgorithms.cs index f441047..1c7b4c2 100644 --- a/Accordant.Operations/GenerationAlgorithms/ConcurrentTestCaseAlgorithms.cs +++ b/Accordant.Operations/GenerationAlgorithms/ConcurrentTestCaseAlgorithms.cs @@ -103,7 +103,7 @@ void Recurse(StateGraphNode node, HashSet visitedHashes, List /// This is an exception thrown during state graph exploration @@ -22,28 +23,36 @@ public class StepFunctionApplicationException : Exception /// The state graph node at which applying one of its step /// functions lead to the exception. /// - public StateGraphNode ExceptionEncounteringNode { get; set; } + public StateGraphNode ExceptionEncounteringNode { get; } /// /// The path from the root node to the node at which the exception /// was encountered. The initial step function is null for the starting node. /// - public IList<(IStepFunction stepFunction, StateGraphNode node)> PathToNode { get; set; } + public IReadOnlyList<(IStepFunction stepFunction, StateGraphNode node)> PathToNode { get; } /// /// The step function that lead to the exception. /// - public IStepFunction ExceptionEncounteringStepFunction { get; set; } + public IStepFunction ExceptionEncounteringStepFunction { get; } public StepFunctionApplicationException( Exception exception, StateGraphNode node, - IList<(IStepFunction stepFunction, StateGraphNode node)> pathToNode, + IReadOnlyList<(IStepFunction stepFunction, StateGraphNode node)> pathToNode, IStepFunction stepFunction) : base("Encountered an exception when applying a step function at a node", exception) { - ExceptionEncounteringNode = node; - PathToNode = pathToNode; - ExceptionEncounteringStepFunction = stepFunction; + if (exception == null) + { + throw new ArgumentNullException(nameof(exception)); + } + + ExceptionEncounteringNode = node ?? throw new ArgumentNullException(nameof(node)); + PathToNode = (pathToNode ?? throw new ArgumentNullException(nameof(pathToNode))) + .ToList() + .AsReadOnly(); + ExceptionEncounteringStepFunction = stepFunction ?? + throw new ArgumentNullException(nameof(stepFunction)); } } diff --git a/Accordant/State/State.cs b/Accordant/State/State.cs index 2a763e1..dce4ee8 100644 --- a/Accordant/State/State.cs +++ b/Accordant/State/State.cs @@ -20,14 +20,13 @@ namespace Microsoft.Accordant; /// public abstract class State : IState { - public static Random Random { get; } = new Random(); - /// - /// Controls whether performs validation. - /// Set to false to disable validation for performance in production scenarios. + /// Controls whether performs validation + /// for this state instance. Set to false only when the caller explicitly + /// accepts the loss of mutation detection for this state. /// Default is true. /// - public static bool EnableFreezeValidation { get; set; } = true; + public bool EnableFreezeValidation { get; set; } = true; protected string stringRepresentation = null; protected ulong? stateHash = null; @@ -93,6 +92,9 @@ public State Clone(Dictionary clonedMap) if (clonedMap[this] is State state) { + // Validation is an instance option and should follow the state + // when it is cloned, while the clone remains unfrozen. + state.EnableFreezeValidation = EnableFreezeValidation; return state; } else @@ -296,6 +298,14 @@ public void Freeze() public void Freeze(HashSet visited) { + if (IsFrozen) + { + // A frozen state is already baselined. Re-freezing must validate + // it rather than silently accepting a mutation as the new baseline. + ValidateNotMutated(); + return; + } + if (visited.Contains(this)) { return; diff --git a/Accordant/StateGraph.cs b/Accordant/StateGraph.cs index b2b9114..b463fe0 100644 --- a/Accordant/StateGraph.cs +++ b/Accordant/StateGraph.cs @@ -5,9 +5,10 @@ namespace Microsoft.Accordant; using System; using System.Collections.Generic; +using System.Globalization; using System.IO.Hashing; using System.Linq; -using System.Security.Cryptography; +using System.Runtime.ExceptionServices; using System.Text; /// @@ -38,6 +39,8 @@ public static StateGraphNode ExploreStateGraph( Func shouldIncludeStepFunctionResult = null, bool lazy = false) { + ValidateExplorationInputs(steps, startingState, maxDepth); + if (lazy && !generateStateGraph) { throw new ArgumentException( @@ -45,6 +48,8 @@ public static StateGraphNode ExploreStateGraph( nameof(generateStateGraph)); } + EnsureStateFrozen(startingState, nameof(startingState)); + var expander = new StateGraphExpander( maxDepth, stateConstraint, @@ -77,7 +82,10 @@ public static StateGraphNode ExploreStateGraph( // Both the traversal path and the depth handed to expansion are read // from each node (reconstructed from its discovery back-pointers), // exactly as in lazy mode, so the worklist carries only the nodes. - var processed = new HashSet(); + // Node interning is collision-safe, so node references are the + // identity used by the traversal. A compact fingerprint is useful + // for diagnostics but must not decide whether a node was processed. + var processed = new HashSet(); var stack = new Stack(); stack.Push(rootGraphNode); @@ -86,7 +94,7 @@ public static StateGraphNode ExploreStateGraph( { var node = stack.Pop(); - if (!processed.Add(node.GetNodeFingerprint())) + if (!processed.Add(node)) { continue; } @@ -101,7 +109,7 @@ public static StateGraphNode ExploreStateGraph( foreach (var edge in edges) { var child = edge.Target; - if (!processed.Contains(child.GetNodeFingerprint())) + if (!processed.Contains(child)) { stack.Push(child); } @@ -113,6 +121,113 @@ public static StateGraphNode ExploreStateGraph( null; } + private static void ValidateExplorationInputs( + IList steps, + IState startingState, + int maxDepth) + { + if (steps == null) + { + throw new ArgumentNullException(nameof(steps)); + } + + if (startingState == null) + { + throw new ArgumentNullException(nameof(startingState)); + } + + if (maxDepth < -1) + { + throw new ArgumentOutOfRangeException( + nameof(maxDepth), + maxDepth, + "maxDepth must be -1 for unbounded exploration or a non-negative depth."); + } + + ValidateStepFunctionList(steps, "The initial step-function set"); + } + + internal static void EnsureStateFrozen(IState state, string context) + { + if (state == null) + { + throw new ArgumentNullException(nameof(state)); + } + + if (!state.IsFrozen) + { + state.Freeze(); + } + + if (!state.IsFrozen) + { + throw new InvalidOperationException( + $"The {context} state did not become frozen after Freeze(). " + + "IState implementations must make Freeze establish immutability."); + } + } + + internal static void ValidateStepFunctionList( + IEnumerable stepFunctions, + string context) + { + if (stepFunctions == null) + { + throw new ArgumentNullException(nameof(stepFunctions)); + } + + var seenIds = new HashSet(StringComparer.Ordinal); + foreach (var stepFunction in stepFunctions) + { + if (stepFunction == null) + { + throw new ArgumentException( + $"{context} cannot contain null step functions.", + nameof(stepFunctions)); + } + + var stepFunctionId = stepFunction.StepFunctionId; + if (string.IsNullOrEmpty(stepFunctionId)) + { + throw new ArgumentException( + $"{context} contains a step function with a null or empty StepFunctionId.", + nameof(stepFunctions)); + } + + if (!seenIds.Add(stepFunctionId)) + { + throw new ArgumentException( + $"{context} contains duplicate StepFunctionId '{stepFunctionId}'.", + nameof(stepFunctions)); + } + } + } + + internal static void ValidateStepResult( + StepResult stepResult, + IStepFunction sourceStepFunction, + int resultIndex) + { + if (stepResult == null) + { + throw new InvalidOperationException( + $"Step function '{sourceStepFunction.StepFunctionId}' returned a null StepResult at index {resultIndex}."); + } + + if (stepResult.State == null) + { + throw new InvalidOperationException( + $"Step function '{sourceStepFunction.StepFunctionId}' returned a StepResult with a null State at index {resultIndex}."); + } + + if (stepResult.StepFunctions != null) + { + ValidateStepFunctionList( + stepResult.StepFunctions, + $"StepResult {resultIndex} from step function '{sourceStepFunction.StepFunctionId}'"); + } + } + /// /// Generates the raw successors of a node — the single source of truth /// for successor generation shared by eager @@ -145,6 +260,7 @@ public static StateGraphNode ExploreStateGraph( { var state = node.State; var stepFunctions = node.StepFunctions; + ValidateStepFunctionList(stepFunctions, "The node's step-function set"); foreach (var stepFunction in stepFunctions) { @@ -159,7 +275,7 @@ public static StateGraphNode ExploreStateGraph( throw new StepFunctionApplicationException( ex, node, - path.ToList(), + path, stepFunction); } @@ -168,27 +284,66 @@ public static StateGraphNode ExploreStateGraph( continue; } + var resultIndex = 0; foreach (var stepResult in stepResults) { + try + { + ValidateStepResult(stepResult, stepFunction, resultIndex); + EnsureStateFrozen( + stepResult.State, + $"StepResult {resultIndex} from step function '{stepFunction.StepFunctionId}'"); + } + catch (Exception ex) + { + throw new StepFunctionApplicationException( + ex, + node, + path, + stepFunction); + } + if (shouldIncludeStepFunctionResult != null && !shouldIncludeStepFunctionResult(state, stepFunction, stepResult)) { + resultIndex++; continue; } - var newStepFunctions = stepFunctions - .Where(s => s.StepFunctionId != stepFunction.StepFunctionId) - .ToList(); - if (stepResult.StepFunctions != null) + IList orderedStepFunctions; + try + { + var newStepFunctions = stepFunctions + .Where(s => s.StepFunctionId != stepFunction.StepFunctionId) + .ToList(); + if (stepResult.StepFunctions != null) + { + newStepFunctions.AddRange(stepResult.StepFunctions); + } + + ValidateStepFunctionList( + newStepFunctions, + $"Successor step-function set from '{stepFunction.StepFunctionId}'"); + orderedStepFunctions = newStepFunctions + .OrderBy(s => s.StepFunctionId) + .ToList(); + } + catch (Exception ex) { - newStepFunctions.AddRange(stepResult.StepFunctions); + throw new StepFunctionApplicationException( + ex, + node, + path, + stepFunction); } yield return ( stepFunction, stepResult.State, - newStepFunctions.OrderBy(s => s.StepFunctionId).ToList(), + orderedStepFunctions, stepResult.EdgeMetadata); + + resultIndex++; } } } @@ -203,12 +358,33 @@ public class StateGraphNode { private string nodeFingerprint = null; - private static SHA256 SHA256 = SHA256.Create(); + internal StateGraphNode( + IState state, + IList stepFunctions, + StateGraphExpander lazyExpander, + StateGraphNode discoveredFrom, + IStepFunction discoveredVia, + int depth) + { + State = state ?? throw new ArgumentNullException(nameof(state)); + if (stepFunctions == null) + { + throw new ArgumentNullException(nameof(stepFunctions)); + } + + var stepFunctionSnapshot = new List(stepFunctions); + StateGraph.ValidateStepFunctionList(stepFunctionSnapshot, "Node step functions"); + StepFunctions = stepFunctionSnapshot.AsReadOnly(); + LazyExpander = lazyExpander; + DiscoveredFrom = discoveredFrom; + DiscoveredVia = discoveredVia; + Depth = depth; + } /// /// The system state represented by this node. /// - public IState State { get; set; } + public IState State { get; } /// /// The set of step functions that can be applied to this state. @@ -216,21 +392,30 @@ public class StateGraphNode /// updated state (though a step function can produce new step functions that /// are included in the step function list for the updated state). /// - public IList StepFunctions { get; set; } + public IReadOnlyList StepFunctions { get; } private List edges = new List(); - private bool expanded; + private enum ExpansionState + { + NotExpanded, + Expanding, + Expanded, + Failed + } + + private ExpansionState expansionState; + private ExceptionDispatchInfo expansionFailure; /// /// The expander bound to this node in lazy (on-the-fly) exploration, which /// computes the node's outgoing edges the first time is /// accessed. This is the single flag distinguishing the two modes: /// - /// null ⇒ an eager (or manually constructed) node. The - /// eager worklist has already computed and stored its edges via - /// , so the lazy machinery is inert and - /// behaves as a plain list. + /// null ⇒ an eager node. The eager worklist has already + /// computed and stored its edges via , so the + /// lazy machinery is inert and behaves as a plain + /// read-only list. /// non-null ⇒ a lazy node /// (StateGraph.ExploreStateGraph(..., lazy: true)) that materializes /// its edges on first access. @@ -313,37 +498,58 @@ public class StateGraphNode /// visualization, BFS traversals) — the graph materializes only as far /// as it is actually walked. /// - public List Edges + public IReadOnlyList Edges { get { EnsureExpanded(); return edges; } - - set => edges = value; } /// /// Ensures this node's outgoing edges have been computed. A no-op for - /// eager / manually built nodes ( is null) + /// eager nodes ( is null) /// and idempotent for lazy nodes (expansion runs at most once). + /// If lazy expansion fails, the original exception is cached and rethrown + /// on subsequent accesses rather than exposing a partial empty graph. /// internal void EnsureExpanded() { - if (expanded) + if (expansionState == ExpansionState.Expanded) { return; } - // Mark expanded before invoking the expander so that any re-entrant - // access to this node's Edges during expansion returns the - // (currently empty) backing list rather than recursing. - expanded = true; + if (expansionState == ExpansionState.Failed) + { + expansionFailure.Throw(); + return; + } - if (LazyExpander != null) + // Preserve the previous re-entrant behavior: an expansion callback + // reading this node's Edges observes the current backing list instead + // of recursively expanding the same node. + if (expansionState == ExpansionState.Expanding) + { + return; + } + + expansionState = ExpansionState.Expanding; + try + { + if (LazyExpander != null) + { + edges = LazyExpander.ExpandNode(this); + } + + expansionState = ExpansionState.Expanded; + } + catch (Exception ex) { - edges = LazyExpander.ExpandNode(this); + expansionFailure = ExceptionDispatchInfo.Capture(ex); + expansionState = ExpansionState.Failed; + throw; } } @@ -353,7 +559,7 @@ internal void EnsureExpanded() /// than triggering (re)computation. Used by the eager explorer, whose /// worklist has already produced the node's edges. /// - internal void SetExpandedEdges(List computedEdges) + internal void SetExpandedEdges(IReadOnlyList computedEdges) { // Eager and lazy are mutually exclusive per node: an eager node must // never be lazy-bound, or a later Edges access would re-expand it @@ -366,8 +572,28 @@ internal void SetExpandedEdges(List computedEdges) "lazy-bound node (LazyExpander != null)."); } - edges = computedEdges; - expanded = true; + if (computedEdges == null) + { + throw new ArgumentNullException(nameof(computedEdges)); + } + + var edgeSnapshot = new List(computedEdges); + foreach (var edge in edgeSnapshot) + { + if (edge == null) + { + throw new ArgumentException("Expanded edges cannot contain null entries.", nameof(computedEdges)); + } + } + + if (expansionState != ExpansionState.NotExpanded) + { + throw new InvalidOperationException( + "Expanded edges can only be assigned to a node that has not started expansion."); + } + + edges = edgeSnapshot; + expansionState = ExpansionState.Expanded; } /// @@ -379,7 +605,7 @@ public string GetNodeFingerprint() { if (nodeFingerprint == null) { - nodeFingerprint = GetNodeFingerprint(State, StepFunctions); + nodeFingerprint = GetNodeFingerprint(State, StepFunctions.ToList()); } return nodeFingerprint; @@ -411,16 +637,28 @@ public static string GenerateDotFileContent( var edges = new List<(string, string, string)>(); var nodes = new List<(string, string)>(); - var seenSet = new HashSet(); + var seenSet = new HashSet(); + var nodeIds = new Dictionary(); + var nextNodeId = 0; + + string GetNodeId(StateGraphNode node) + { + if (!nodeIds.TryGetValue(node, out var nodeId)) + { + nodeId = $"N{nextNodeId++}"; + nodeIds[node] = nodeId; + } + + return nodeId; + } + void CollectEdges(StateGraphNode node) { - if (seenSet.Contains(node.GetNodeFingerprint())) + if (!seenSet.Add(node)) { return; } - seenSet.Add(node.GetNodeFingerprint()); - var nodeLabel = nodeLabelLambda(node); if (showStepFunctionsInNode) @@ -430,7 +668,7 @@ void CollectEdges(StateGraphNode node) } nodes.Add(( - node.GetNodeFingerprint().Substring(0, 5), + GetNodeId(node), nodeLabel.Replace("\"", "\\\""))); foreach (var edge in node.Edges) @@ -440,8 +678,8 @@ void CollectEdges(StateGraphNode node) edge.StepFunction.StepFunctionId; edges.Add(( - node.GetNodeFingerprint().Substring(0, 5), - edge.Target.GetNodeFingerprint().Substring(0, 5), + GetNodeId(node), + GetNodeId(edge.Target), edgeLabel.Replace("\"", "\\\""))); CollectEdges(edge.Target); @@ -476,19 +714,100 @@ void CollectEdges(StateGraphNode node) return string.Join("\r\n", lines); } + /// + /// Returns the compact diagnostic fingerprint for a state and its enabled + /// step functions. This value is intentionally not used as graph identity: + /// it is a 64-bit display hash and therefore can collide. + /// public static string GetNodeFingerprint( IState state, IList stepFunctions) { - // Combine state hash with step function IDs for node fingerprint - var nodeState = - state.GetStateHash().ToString() + "-" + - string.Join(string.Empty, stepFunctions.OrderBy(s => s.StepFunctionId).Select(s => s.StepFunctionId)); - - // Use XxHash64 for fast node fingerprinting - var bytes = Encoding.UTF8.GetBytes(nodeState); + var fastKey = GetFastNodeKey(state, stepFunctions); + var bytes = Encoding.UTF8.GetBytes(fastKey); var hash = XxHash64.HashToUInt64(bytes); - return hash.ToString("x16"); + return hash.ToString("x16", CultureInfo.InvariantCulture); + } + + /// + /// Returns the cheap lookup key used by graph interning. Equal fast keys + /// are still compared using the canonical state representation before two + /// nodes are considered identical. + /// + internal static string GetFastNodeKey( + IState state, + IList stepFunctions) + { + if (state == null) + { + throw new ArgumentNullException(nameof(state)); + } + + StateGraph.ValidateStepFunctionList(stepFunctions, "Fingerprint step functions"); + + var stepFunctionSignature = new StringBuilder(); + foreach (var stepFunction in stepFunctions.OrderBy( + s => s.StepFunctionId, + StringComparer.Ordinal)) + { + var id = stepFunction.StepFunctionId; + stepFunctionSignature + .Append(id.Length.ToString(CultureInfo.InvariantCulture)) + .Append(':') + .Append(id); + } + + return state.GetStateHash().ToString(CultureInfo.InvariantCulture) + + "-" + + stepFunctionSignature; + } + + /// + /// Returns the exact logical state representation required to resolve a + /// collision in . + /// + internal static string GetCanonicalStateRepresentation(IState state) + { + if (state == null) + { + throw new ArgumentNullException(nameof(state)); + } + + var representation = state.StringRepresentation(); + if (representation == null) + { + throw new InvalidOperationException( + $"State of type '{state.GetType().Name}' returned a null StringRepresentation()."); + } + + return representation; + } + + /// + /// Compares two candidate nodes after their cheap hash/signature keys have + /// matched. The canonical representation is computed only on this slow + /// collision path. + /// + internal static bool HasSameNodeIdentity( + StateGraphNode existingNode, + IState state, + IList stepFunctions) + { + if (existingNode == null) + { + throw new ArgumentNullException(nameof(existingNode)); + } + + if (GetFastNodeKey(existingNode.State, existingNode.StepFunctions.ToList()) != + GetFastNodeKey(state, stepFunctions)) + { + return false; + } + + return string.Equals( + GetCanonicalStateRepresentation(existingNode.State), + GetCanonicalStateRepresentation(state), + StringComparison.Ordinal); } /// @@ -524,19 +843,29 @@ public static Func CreateCountBasedNodeLabelLambda() /// public class StateGraphEdge { + internal StateGraphEdge( + StateGraphNode target, + IStepFunction stepFunction, + object metadata) + { + Target = target ?? throw new ArgumentNullException(nameof(target)); + StepFunction = stepFunction ?? throw new ArgumentNullException(nameof(stepFunction)); + Metadata = metadata; + } + /// /// The target state graph node. /// - public StateGraphNode Target { get; set; } + public StateGraphNode Target { get; } /// /// The step function that takes the system to the target /// state graph node. /// - public IStepFunction StepFunction { get; set; } + public IStepFunction StepFunction { get; } /// /// Metadata associated with the edge. /// - public object Metadata { get; set; } + public object Metadata { get; } } diff --git a/Accordant/StateGraphExpander.cs b/Accordant/StateGraphExpander.cs index 91040c3..5218f98 100644 --- a/Accordant/StateGraphExpander.cs +++ b/Accordant/StateGraphExpander.cs @@ -48,8 +48,11 @@ internal sealed class StateGraphExpander // reaching the same (state, step-functions) fingerprint resolve to the // same node object, so a state's edges are computed at most once and the // graph stays a proper DAG-with-cycles. - private readonly Dictionary nodeMap = - new Dictionary(); + // The compact hash/signature key is used for the normal lookup path. + // Multiple nodes may share it, so each bucket is resolved with the exact + // canonical state representation before interning. + private readonly Dictionary> nodeMap = + new Dictionary>(StringComparer.Ordinal); public StateGraphExpander( int maxDepth, @@ -86,23 +89,33 @@ internal StateGraphNode GetOrCreateNode( IStepFunction discoveredVia, int depth) { - var fingerprint = StateGraphNode.GetNodeFingerprint(state, stepFunctions); + var fastKey = StateGraphNode.GetFastNodeKey(state, stepFunctions); - if (!nodeMap.TryGetValue(fingerprint, out var node)) + if (nodeMap.TryGetValue(fastKey, out var candidates)) { - node = new StateGraphNode + foreach (var candidate in candidates) { - State = state, - StepFunctions = stepFunctions, - LazyExpander = this.lazy ? this : null, - DiscoveredFrom = discoveredFrom, - DiscoveredVia = discoveredVia, - Depth = depth - }; - - nodeMap[fingerprint] = node; + if (StateGraphNode.HasSameNodeIdentity(candidate, state, stepFunctions)) + { + return candidate; + } + } + } + else + { + candidates = new List(); + nodeMap[fastKey] = candidates; } + var node = new StateGraphNode( + state, + stepFunctions, + this.lazy ? this : null, + discoveredFrom, + discoveredVia, + depth); + + candidates.Add(node); return node; } @@ -161,19 +174,16 @@ internal List ExpandNode(StateGraphNode node) var child = GetOrCreateNode(childState, childStepFunctions, node, stepFunction, childDepth); - var childFingerprint = child.GetNodeFingerprint(); var alreadyPresent = edges.Any(e => e.StepFunction.StepFunctionId == stepFunction.StepFunctionId && - e.Target.GetNodeFingerprint() == childFingerprint); + ReferenceEquals(e.Target, child)); if (!alreadyPresent) { - edges.Add(new StateGraphEdge - { - StepFunction = stepFunction, - Target = child, - Metadata = edgeMetadata - }); + edges.Add(new StateGraphEdge( + child, + stepFunction, + edgeMetadata)); } } diff --git a/Accordant/StateProfile.cs b/Accordant/StateProfile.cs index 3a97f18..fab2f27 100644 --- a/Accordant/StateProfile.cs +++ b/Accordant/StateProfile.cs @@ -33,11 +33,21 @@ namespace Microsoft.Accordant; /// public class StateProfile { + private IList<(IState State, IList StepFunctions)> statesAndStepFunctions; + /// /// The set of states the system can be and the set of step functions /// associated with each of those states. + /// + /// The assigned collection is snapshotted and exposed as read-only. This + /// keeps the profile invariant stable after construction while preserving + /// the existing IList-based API. /// - public IList<(IState State, IList StepFunctions)> StatesAndStepFunctions { get; set; } + public IList<(IState State, IList StepFunctions)> StatesAndStepFunctions + { + get => statesAndStepFunctions; + set => statesAndStepFunctions = Normalize(value); + } /// /// Constructs an instance of this class given a single state. @@ -56,6 +66,11 @@ public StateProfile(IState state) /// public StateProfile(IList states) { + if (states == null) + { + throw new ArgumentNullException(nameof(states)); + } + StatesAndStepFunctions = states.Select(s => (s, (IList)Array.Empty())).ToList(); } @@ -67,17 +82,43 @@ public StateProfile(IList states) public StateProfile(IList<(IState, IList)> statesAndStepFunctions) { StatesAndStepFunctions = statesAndStepFunctions; + } + + private static IList<(IState State, IList StepFunctions)> Normalize( + IList<(IState State, IList StepFunctions)> statesAndStepFunctions) + { + if (statesAndStepFunctions == null) + { + throw new ArgumentNullException(nameof(statesAndStepFunctions)); + } + + var snapshot = new List<(IState State, IList StepFunctions)>( + statesAndStepFunctions.Count); - // If any of the step functions is null, then convert that to an empty list, - // while preserving the non-null ones. - if (StatesAndStepFunctions.Any(ssf => ssf.StepFunctions == null)) + foreach (var (state, stepFunctions) in statesAndStepFunctions) { - StatesAndStepFunctions = StatesAndStepFunctions - .Select(ssf => ( - ssf.State, - ssf.StepFunctions == null ? Array.Empty() : ssf.StepFunctions)) - .ToList(); + if (state == null) + { + throw new ArgumentException( + "A state profile cannot contain a null state.", + nameof(statesAndStepFunctions)); + } + + // A null step-function list has historically meant "no enabled + // step functions". Preserve that behavior while still taking a + // defensive snapshot of every non-null collection. + IList normalizedStepFunctions = stepFunctions == null + ? Array.Empty() + : new List(stepFunctions).AsReadOnly(); + + StateGraph.ValidateStepFunctionList( + normalizedStepFunctions, + "State profile step functions"); + + snapshot.Add((state, normalizedStepFunctions)); } + + return snapshot.AsReadOnly(); } /// diff --git a/Accordant/StepFunction/ContractStepFunction.cs b/Accordant/StepFunction/ContractStepFunction.cs index 32d22b0..9b68702 100644 --- a/Accordant/StepFunction/ContractStepFunction.cs +++ b/Accordant/StepFunction/ContractStepFunction.cs @@ -42,7 +42,7 @@ public ContractStepFunction( Request = request; ObservedResponse = observedResponse; Verify = verify ?? throw new ArgumentNullException(nameof(verify)); - _predecessorIds = predecessorIds ?? Array.Empty(); + _predecessorIds = NormalizePredecessorIds(predecessorIds); } /// @@ -51,7 +51,78 @@ public ContractStepFunction( /// public void SetPredecessorIds(IReadOnlyCollection ids) { - _predecessorIds = ids ?? Array.Empty(); + if (HasBeenApplied) + { + throw new InvalidOperationException( + "Predecessor IDs cannot be changed after the contract step function has been applied."); + } + + _predecessorIds = NormalizePredecessorIds(ids); + } + + private static IReadOnlyCollection NormalizePredecessorIds( + IReadOnlyCollection ids) + { + if (ids == null) + { + return Array.Empty(); + } + + var snapshot = ids.ToList(); + var seenIds = new HashSet(StringComparer.Ordinal); + foreach (var id in snapshot) + { + if (string.IsNullOrEmpty(id)) + { + throw new ArgumentException( + "Predecessor IDs cannot contain null or empty values.", + nameof(ids)); + } + + if (!seenIds.Add(id)) + { + throw new ArgumentException( + $"Predecessor IDs contain duplicate ID '{id}'.", + nameof(ids)); + } + } + + return snapshot.AsReadOnly(); + } + + private static void ValidateVerifiedStateProfile(StateProfile stateProfile) + { + if (stateProfile == null) + { + throw new InvalidOperationException( + "A verification result marked valid must include a non-null StateProfile."); + } + + if (stateProfile.StatesAndStepFunctions == null || + stateProfile.StatesAndStepFunctions.Count == 0) + { + throw new InvalidOperationException( + "A valid verification result must include at least one state outcome."); + } + + foreach (var (state, stepFunctions) in stateProfile.StatesAndStepFunctions) + { + if (state == null) + { + throw new InvalidOperationException( + "A valid verification result cannot include a null state outcome."); + } + + if (stepFunctions == null) + { + throw new InvalidOperationException( + "A valid verification result cannot include a null step-function collection."); + } + + StateGraph.ValidateStepFunctionList( + stepFunctions, + "A valid verification result"); + } } protected override IList ApplyInternal( @@ -75,14 +146,14 @@ protected override IList ApplyInternal( { return null; } - else - { - return - stateProfile.StatesAndStepFunctions.Select(stateAndStepFunctions => new StepResult() - { - State = stateAndStepFunctions.State, - StepFunctions = stateAndStepFunctions.StepFunctions - }).ToList(); - } + + ValidateVerifiedStateProfile(stateProfile); + + return + stateProfile.StatesAndStepFunctions.Select(stateAndStepFunctions => new StepResult() + { + State = stateAndStepFunctions.State, + StepFunctions = stateAndStepFunctions.StepFunctions + }).ToList(); } } diff --git a/Accordant/StepFunction/IStepFunction.cs b/Accordant/StepFunction/IStepFunction.cs index 798258e..287cf8f 100644 --- a/Accordant/StepFunction/IStepFunction.cs +++ b/Accordant/StepFunction/IStepFunction.cs @@ -12,12 +12,13 @@ namespace Microsoft.Accordant; public class StepResult { /// - /// The next state. + /// The next state. This must be non-null for every returned result. /// public IState State { get; set; } /// /// The list of step functions that should be applied in the next state. + /// A null list means that no step functions are enabled in the next state. /// public IList StepFunctions { get; set; } @@ -62,10 +63,34 @@ public interface IStepFunction /// public abstract class BaseStepFunction : IStepFunction { - private string stepFunctionId = Guid.NewGuid().ToString(); + private readonly string stepFunctionId; + private bool hasBeenApplied; + + /// + /// Creates a step function with an optional stable identifier. When no ID + /// is supplied, a GUID preserves the existing identity-by-instance + /// behavior. + /// + protected BaseStepFunction(string stepFunctionId = null) + { + this.stepFunctionId = stepFunctionId ?? Guid.NewGuid().ToString(); + if (string.IsNullOrEmpty(this.stepFunctionId)) + { + throw new ArgumentException( + "A step-function ID cannot be null or empty.", + nameof(stepFunctionId)); + } + } public virtual string StepFunctionId => stepFunctionId; + /// + /// Indicates whether this step function has begun application. Derived + /// step functions can use this to reject configuration changes that would + /// alter behavior after graph identity has started being used. + /// + protected bool HasBeenApplied => hasBeenApplied; + /// /// This method locks the state, calls the derived class's /// method @@ -73,21 +98,49 @@ public abstract class BaseStepFunction : IStepFunction /// public IList Apply(IState state, IReadOnlyList<(IStepFunction, StateGraphNode)> path) { + if (state == null) + { + throw new ArgumentNullException(nameof(state)); + } + + if (path == null) + { + throw new ArgumentNullException(nameof(path)); + } + + hasBeenApplied = true; state.Freeze(); - var stepResults = ApplyInternal(state, path); + IList stepResults; + try + { + stepResults = ApplyInternal(state, path); + } + catch + { + // Mutation validation must also run when user code throws. This + // is a defect in the step implementation, not an expected result. + if (state is State stateObj) + { + stateObj.ValidateNotMutated(); + } + + throw; + } // Validate that State inputs were not mutated by user code - if (state is State stateObj) + if (state is State stateObjAfterApply) { - stateObj.ValidateNotMutated(); + stateObjAfterApply.ValidateNotMutated(); } if (stepResults != null) { - foreach (var stepResult in stepResults) + for (var i = 0; i < stepResults.Count; i++) { - stepResult.State?.Freeze(); + var stepResult = stepResults[i]; + StateGraph.ValidateStepResult(stepResult, this, i); + stepResult.State.Freeze(); } } @@ -239,13 +292,39 @@ internal AsyncOperation( string name = null) { _isTerminal = isTerminal ?? throw new ArgumentNullException(nameof(isTerminal)); - _transitions = transitions ?? throw new ArgumentNullException(nameof(transitions)); - if (_transitions.Length == 0) + if (transitions == null) + { + throw new ArgumentNullException(nameof(transitions)); + } + + if (transitions.Length == 0) + { throw new ArgumentException("At least one transition is required.", nameof(transitions)); + } + + _transitions = new Action[transitions.Length]; + for (var i = 0; i < transitions.Length; i++) + { + _transitions[i] = transitions[i] ?? throw new ArgumentException( + $"Transition at index {i} cannot be null.", + nameof(transitions)); + } + _name = name; } - public override Func IsTerminalState => state => _isTerminal((TState)state); + public override Func IsTerminalState => state => + { + if (!(state is TState typedState)) + { + throw new ArgumentException( + $"Async operation '{ToString()}' requires state type '{typeof(TState).FullName}', " + + $"but received '{state?.GetType().FullName ?? "null"}'.", + nameof(state)); + } + + return _isTerminal(typedState); + }; protected override IList GetStepResults(IState state) { diff --git a/Accordant/SystemChecker.cs b/Accordant/SystemChecker.cs index 629c04e..04a7250 100644 --- a/Accordant/SystemChecker.cs +++ b/Accordant/SystemChecker.cs @@ -29,10 +29,16 @@ public static StateProfile Validate( StateProfile stateProfile, Action> hook = null) { + // Validate and snapshot the entire externally-owned sequence before + // invoking a step function or hook. This prevents a malformed later + // batch from causing partially-applied validation. + var sequenceSnapshot = SnapshotAndValidateSequence(sequenceOfConcurrentSteps); + ValidateStateProfile(stateProfile); + try { return ValidateInternal( - sequenceOfConcurrentSteps, + sequenceSnapshot, stateProfile, hook); } @@ -49,6 +55,62 @@ public static StateProfile Validate( } } + private static IList> SnapshotAndValidateSequence( + IList> sequenceOfConcurrentSteps) + { + if (sequenceOfConcurrentSteps == null) + { + throw new ArgumentNullException(nameof(sequenceOfConcurrentSteps)); + } + + var snapshot = new List>(sequenceOfConcurrentSteps.Count); + for (var i = 0; i < sequenceOfConcurrentSteps.Count; i++) + { + var concurrentSteps = sequenceOfConcurrentSteps[i]; + if (concurrentSteps == null) + { + throw new ArgumentException( + $"Concurrent step batch at index {i} cannot be null.", + nameof(sequenceOfConcurrentSteps)); + } + + var batchSnapshot = concurrentSteps.ToList(); + StateGraph.ValidateStepFunctionList( + batchSnapshot, + $"Concurrent step batch at index {i}"); + snapshot.Add(batchSnapshot); + } + + return snapshot; + } + + private static void ValidateStateProfile(StateProfile stateProfile) + { + if (stateProfile == null) + { + throw new ArgumentNullException(nameof(stateProfile)); + } + + if (stateProfile.StatesAndStepFunctions == null) + { + throw new InvalidOperationException( + "A state profile must contain a non-null state collection."); + } + + foreach (var (state, stepFunctions) in stateProfile.StatesAndStepFunctions) + { + if (state == null) + { + throw new InvalidOperationException( + "A state profile cannot contain a null state."); + } + + StateGraph.ValidateStepFunctionList( + stepFunctions, + "State profile step functions"); + } + } + private static StateProfile ValidateInternal( IList> sequenceOfConcurrentSteps, StateProfile stateProfile, @@ -56,62 +118,128 @@ private static StateProfile ValidateInternal( { foreach (var concurrentSteps in sequenceOfConcurrentSteps) { - var updatedStatesAndStepFunctions = new List<(IState, IList)>(); + stateProfile = AdvanceProfile(concurrentSteps, stateProfile, hook); + } - foreach (var (state, stepFunctions) in stateProfile.StatesAndStepFunctions) - { - var allConcurrentStepFunctions = - concurrentSteps.Concat(stepFunctions).ToList(); - - _ = StateGraph.ExploreStateGraph( - allConcurrentStepFunctions, - state, - maxDepth: -1, - generateStateGraph: false, - hook: (node) => + return stateProfile; + } + + /// + /// Applies one concurrent operation batch to every possible state in a + /// profile and returns the deduplicated successor profile. The optional + /// hook observes every explored node but does not participate in choosing + /// the returned profile. + /// + internal static StateProfile AdvanceProfile( + IList concurrentSteps, + StateProfile stateProfile, + Action> hook = null) + { + if (concurrentSteps == null) + { + throw new ArgumentNullException(nameof(concurrentSteps)); + } + + if (stateProfile == null) + { + throw new ArgumentNullException(nameof(stateProfile)); + } + + ValidateStateProfile(stateProfile); + + // Validate all combined sets before exploring the first state. This + // catches duplicate IDs between the current batch and active steps + // without partially advancing the profile. + var profileSnapshot = stateProfile.StatesAndStepFunctions.ToList(); + foreach (var (state, stepFunctions) in profileSnapshot) + { + var allConcurrentStepFunctions = + concurrentSteps.Concat(stepFunctions).ToList(); + StateGraph.ValidateStepFunctionList( + allConcurrentStepFunctions, + "Concurrent and active step functions"); + } + + var updatedStatesAndStepFunctions = new List<(IState, IList)>(); + + foreach (var (state, stepFunctions) in profileSnapshot) + { + var allConcurrentStepFunctions = + concurrentSteps.Concat(stepFunctions).ToList(); + + _ = StateGraph.ExploreStateGraph( + allConcurrentStepFunctions, + state, + maxDepth: -1, + generateStateGraph: false, + hook: node => + { + var updatedState = node.State; + var updatedStepFunctions = node.StepFunctions.ToList(); + + hook?.Invoke(updatedState, updatedStepFunctions); + + if (updatedStepFunctions.Any(sf => sf is ContractStepFunction)) { - var updatedState = node.State; - var stepFunctions = node.StepFunctions; + return; + } - if (hook != null) - { - hook(updatedState, stepFunctions); - } + updatedStatesAndStepFunctions.Add((updatedState, updatedStepFunctions)); + }); + } - if (stepFunctions.Any(sf => sf is ContractStepFunction)) - { - return; - } + if (updatedStatesAndStepFunctions.Count == 0) + { + throw new InvalidSpecException("Model cannot explain the behavior of the system."); + } - updatedStatesAndStepFunctions.Add((updatedState, stepFunctions)); - }); - } + var dedupedUpdatedStatesAndStepFunctions = + DeduplicateStatesAndStepFunctions(updatedStatesAndStepFunctions); + + return new StateProfile(dedupedUpdatedStatesAndStepFunctions); + } + + private static IList<(IState, IList)> DeduplicateStatesAndStepFunctions( + IList<(IState, IList)> statesAndStepFunctions) + { + var deduped = new List<(IState, IList)>(); + var processedBuckets = new Dictionary)>>( + StringComparer.Ordinal); - if (updatedStatesAndStepFunctions.Count == 0) + foreach (var stateAndStepFunctions in statesAndStepFunctions) + { + var fastKey = StateGraphNode.GetFastNodeKey( + stateAndStepFunctions.Item1, + stateAndStepFunctions.Item2); + + if (!processedBuckets.TryGetValue(fastKey, out var candidates)) { - throw new InvalidSpecException("Model cannot explain the behavior of the system."); + candidates = new List<(IState, IList)>(); + processedBuckets[fastKey] = candidates; } - var dedupedUpdatedStatesAndStepFunctions = new List<(IState, IList)>(); - - var processedFingerprints = new HashSet(); - for (int i = 0; i < updatedStatesAndStepFunctions.Count; i++) + if (candidates.Count == 0) { - var ssf = updatedStatesAndStepFunctions[i]; - var fingerprint = StateGraphNode.GetNodeFingerprint(ssf.Item1, ssf.Item2); + candidates.Add(stateAndStepFunctions); + deduped.Add(stateAndStepFunctions); + continue; + } - if (processedFingerprints.Contains(fingerprint)) - { - continue; - } + var currentRepresentation = StateGraphNode.GetCanonicalStateRepresentation( + stateAndStepFunctions.Item1); + var duplicate = candidates.Any(candidate => + string.Equals( + StateGraphNode.GetCanonicalStateRepresentation(candidate.Item1), + currentRepresentation, + StringComparison.Ordinal)); - processedFingerprints.Add(fingerprint); - dedupedUpdatedStatesAndStepFunctions.Add(ssf); + if (!duplicate) + { + candidates.Add(stateAndStepFunctions); + deduped.Add(stateAndStepFunctions); } - - stateProfile = new StateProfile(dedupedUpdatedStatesAndStepFunctions); } - return stateProfile; + return deduped; } } diff --git a/Tests/Accordant.Operations.Tests/SharedDictionaryReferenceTests.cs b/Tests/Accordant.Operations.Tests/SharedDictionaryReferenceTests.cs index fd5e520..8f13292 100644 --- a/Tests/Accordant.Operations.Tests/SharedDictionaryReferenceTests.cs +++ b/Tests/Accordant.Operations.Tests/SharedDictionaryReferenceTests.cs @@ -152,8 +152,8 @@ public void SharedDictionaryReference_WhenModified_ShouldBeDetectedByMutationDet new OperationInput("GetCount", spec["GetCount"]), }; - // Enable mutation detection (should be on by default) - State.EnableFreezeValidation = true; + // Enable mutation detection explicitly on the starting state (it is on by default). + initialState.EnableFreezeValidation = true; // The framework should detect this via mutation detection. // The exception is wrapped in TestCaseGenerationException with StateFrozenException as inner. diff --git a/Tests/Accordant.Tests/FailFastValidationTests.cs b/Tests/Accordant.Tests/FailFastValidationTests.cs new file mode 100644 index 0000000..09bd545 --- /dev/null +++ b/Tests/Accordant.Tests/FailFastValidationTests.cs @@ -0,0 +1,361 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +namespace Microsoft.Accordant.Tests; + +using System; +using System.Collections.Generic; +using Microsoft.Accordant; +using NUnit.Framework; + +[TestFixture] +public class FailFastValidationTests +{ + private sealed class ConstantHashState : IState + { + public int Value { get; set; } + + public bool IsFrozen { get; private set; } + + public void Freeze() + { + IsFrozen = true; + } + + public IState Clone() + { + return new ConstantHashState { Value = Value }; + } + + public ulong GetStateHash() => 42; + + public string StringRepresentation() => Value.ToString(); + } + + private sealed class OtherState : IState + { + public bool IsFrozen { get; private set; } + + public void Freeze() + { + IsFrozen = true; + } + + public IState Clone() + { + return new OtherState(); + } + + public ulong GetStateHash() => 99; + + public string StringRepresentation() => "other"; + } + + private sealed class ProduceStep : BaseStepFunction + { + private readonly string id; + private readonly int value; + private readonly IList nextStepFunctions; + + public ProduceStep( + string id, + int value, + IList nextStepFunctions) + { + this.id = id; + this.value = value; + this.nextStepFunctions = nextStepFunctions; + } + + public override string StepFunctionId => id; + + protected override IList ApplyInternal(IState state) + { + return new[] + { + new StepResult + { + State = new ConstantHashState { Value = value }, + StepFunctions = nextStepFunctions + } + }; + } + } + + private sealed class BranchStep : BaseStepFunction + { + private readonly IStepFunction sharedStep; + + public BranchStep(IStepFunction sharedStep) + { + this.sharedStep = sharedStep; + } + + public override string StepFunctionId => "branch"; + + protected override IList ApplyInternal(IState state) + { + return new[] + { + new StepResult + { + State = new ConstantHashState { Value = 1 }, + StepFunctions = new[] { sharedStep } + }, + new StepResult + { + State = new ConstantHashState { Value = 2 }, + StepFunctions = new[] { sharedStep } + } + }; + } + } + + private sealed class ThrowingStep : BaseStepFunction + { + public override string StepFunctionId => "throwing"; + + protected override IList ApplyInternal(IState state) + { + throw new InvalidOperationException("expected failure"); + } + } + + private sealed class CountingStep : BaseStepFunction + { + private readonly Action onApply; + + public CountingStep(Action onApply) + { + this.onApply = onApply; + } + + public override string StepFunctionId => "counting"; + + protected override IList ApplyInternal(IState state) + { + onApply(); + return null; + } + } + + private sealed class UnfreezableState : IState + { + public bool IsFrozen => false; + + public void Freeze() + { + } + + public IState Clone() => new UnfreezableState(); + + public ulong GetStateHash() => 1; + + public string StringRepresentation() => "unfreezable"; + } + + private sealed class MutatingThrowState : State + { + public int Value { get; set; } + + protected override void CloneInternal(Dictionary clonedMap) + => clonedMap[this] = new MutatingThrowState { Value = Value }; + + protected override string StringRepresentationInternal( + Dictionary objectPaths, + string path, + bool forceRecompute) + => Value.ToString(); + + protected override void FreezeComponents(HashSet visited) + { + } + } + + private sealed class MutatingThrowStep : BaseStepFunction + { + public override string StepFunctionId => "mutating-throwing"; + + protected override IList ApplyInternal(IState state) + { + ((MutatingThrowState)state).Value++; + throw new InvalidOperationException("expected failure"); + } + } + + [Test] + public void StateGraph_DoesNotMergeDistinctStatesWithTheSameHash() + { + var sharedStep = new ProduceStep("shared", 3, Array.Empty()); + var root = StateGraph.ExploreStateGraph( + new IStepFunction[] { new BranchStep(sharedStep) }, + new ConstantHashState()); + + Assert.That(root.Edges, Has.Count.EqualTo(2)); + Assert.That(root.Edges[0].Target, Is.Not.SameAs(root.Edges[1].Target)); + Assert.That(root.Edges[0].Target.State.StringRepresentation(), Is.Not.EqualTo( + root.Edges[1].Target.State.StringRepresentation())); + } + + [Test] + public void NodeFingerprint_DoesNotUseAmbiguousStepFunctionConcatenation() + { + var state = new ConstantHashState(); + var first = new IStepFunction[] + { + new ProduceStep("ab", 1, Array.Empty()), + new ProduceStep("c", 1, Array.Empty()) + }; + var second = new IStepFunction[] + { + new ProduceStep("a", 1, Array.Empty()), + new ProduceStep("bc", 1, Array.Empty()) + }; + + Assert.That( + StateGraphNode.GetNodeFingerprint(state, first), + Is.Not.EqualTo(StateGraphNode.GetNodeFingerprint(state, second))); + } + + [Test] + public void AsyncOperation_CopiesTransitionsBeforeUse() + { + var transitions = new Action[] + { + next => next.Value = 1 + }; + var operation = AsyncOperation.Create( + isTerminal: _ => false, + transitions: transitions); + + transitions[0] = null; + + var results = ((IStepFunction)operation).Apply( + new ConstantHashState(), + Array.Empty<(IStepFunction, StateGraphNode)>()); + + Assert.That(((ConstantHashState)results[0].State).Value, Is.EqualTo(1)); + } + + [Test] + public void AsyncOperation_RejectsNullTransitions() + { + Assert.Throws(() => AsyncOperation.Create( + isTerminal: _ => false, + transitions: new Action[] { null })); + } + + [Test] + public void BaseStepFunction_RejectsNullApplyInputs() + { + var operation = AsyncOperation.Create( + isTerminal: _ => false, + transition: _ => { }); + + Assert.Throws(() => ((IStepFunction)operation).Apply( + null, + Array.Empty<(IStepFunction, StateGraphNode)>())); + Assert.Throws(() => ((IStepFunction)operation).Apply( + new ConstantHashState(), + null)); + } + + [Test] + public void AsyncOperation_ReportsWrongStateTypeClearly() + { + var operation = AsyncOperation.Create( + isTerminal: _ => false, + transition: _ => { }); + + var exception = Assert.Throws(() => ((IStepFunction)operation).Apply( + new OtherState(), + Array.Empty<(IStepFunction, StateGraphNode)>())); + + Assert.That(exception.Message, Does.Contain(nameof(ConstantHashState))); + Assert.That(exception.Message, Does.Contain(nameof(OtherState))); + } + + [Test] + public void StepFunctionApplicationException_SnapshotsDiagnostics() + { + var exception = Assert.Throws(() => + StateGraph.ExploreStateGraph( + new IStepFunction[] { new ThrowingStep() }, + new ConstantHashState())); + + Assert.That(typeof(StepFunctionApplicationException) + .GetProperty(nameof(StepFunctionApplicationException.PathToNode)) + .CanWrite, Is.False); + Assert.That(typeof(StepFunctionApplicationException) + .GetProperty(nameof(StepFunctionApplicationException.ExceptionEncounteringNode)) + .CanWrite, Is.False); + Assert.That(exception.PathToNode, Has.Count.EqualTo(1)); + + var mutablePath = exception.PathToNode as IList<(IStepFunction, StateGraphNode)>; + Assert.That(mutablePath, Is.Not.Null); + Assert.Throws(() => mutablePath.Clear()); + } + + [Test] + public void StateProfile_SnapshotsAndProtectsItsCollections() + { + var states = new List { new ConstantHashState() }; + var profile = new StateProfile(states); + + states.Clear(); + + Assert.That(profile.StatesAndStepFunctions, Has.Count.EqualTo(1)); + Assert.Throws(() => profile.StatesAndStepFunctions.Clear()); + } + + [Test] + public void StateGraph_RejectsStatesThatCannotBeFrozen() + { + var exception = Assert.Throws(() => + StateGraph.ExploreStateGraph( + Array.Empty(), + new UnfreezableState())); + + Assert.That(exception.Message, Does.Contain("did not become frozen")); + } + + [Test] + public void SystemChecker_ValidatesAllBatchesBeforeApplyingAnyStep() + { + var applied = false; + var sequence = new List> + { + new IStepFunction[] { new CountingStep(() => applied = true) }, + new IStepFunction[] { null } + }; + + Assert.Throws(() => + SystemChecker.Validate(sequence, new ConstantHashState())); + Assert.That(applied, Is.False); + } + + [Test] + public void BaseStepFunction_ValidatesMutationWhenApplicationThrows() + { + Assert.Throws(() => + new MutatingThrowStep().Apply( + new MutatingThrowState(), + Array.Empty<(IStepFunction, StateGraphNode)>())); + } + + [Test] + public void ContractStepFunction_RejectsConfigurationChangesAfterApplication() + { + var contract = new ContractStepFunction( + request: null, + observedResponse: null, + verify: (_, _, _) => (false, (StateProfile)null)); + + contract.Apply( + new ConstantHashState(), + Array.Empty<(IStepFunction, StateGraphNode)>()); + + Assert.Throws(() => + contract.SetPredecessorIds(new[] { "predecessor" })); + } +} diff --git a/Tests/Accordant.Tests/StateFreezeValidationTests.cs b/Tests/Accordant.Tests/StateFreezeValidationTests.cs new file mode 100644 index 0000000..0a22797 --- /dev/null +++ b/Tests/Accordant.Tests/StateFreezeValidationTests.cs @@ -0,0 +1,78 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +namespace Microsoft.Accordant.Tests; + +using System.Collections.Generic; +using Microsoft.Accordant; +using NUnit.Framework; + +[TestFixture] +public class StateFreezeValidationTests +{ + private sealed class MutableListState : State + { + public List Items { get; } = new List(); + + protected override void CloneInternal(Dictionary clonedMap) + { + var clone = new MutableListState(); + clone.Items.AddRange(Items); + clonedMap[this] = clone; + } + + protected override string StringRepresentationInternal( + Dictionary objectPaths, + string path, + bool forceRecompute) + { + return string.Join(",", Items); + } + + protected override void FreezeComponents(HashSet visited) + { + } + } + + [Test] + public void Freeze_RejectsMutationAfterTheInitialFreeze() + { + var state = new MutableListState(); + var previousValidationSetting = state.EnableFreezeValidation; + try + { + state.EnableFreezeValidation = true; + state.Items.Add("before-freeze"); + state.Freeze(); + + state.Items.Add("after-freeze"); + + var exception = Assert.Throws(() => state.Freeze()); + + Assert.That(exception.Message, Does.Contain("mutated after freezing")); + } + finally + { + state.EnableFreezeValidation = previousValidationSetting; + } + } + + [Test] + public void FreezeValidation_IsConfiguredPerStateInstance() + { + var validationDisabledState = new MutableListState + { + EnableFreezeValidation = false + }; + var validationEnabledState = new MutableListState(); + + validationDisabledState.Freeze(); + validationEnabledState.Freeze(); + validationDisabledState.Items.Add("mutation"); + validationEnabledState.Items.Add("mutation"); + + Assert.DoesNotThrow(() => validationDisabledState.Freeze()); + Assert.Throws(() => validationEnabledState.Freeze()); + Assert.That(validationDisabledState.Clone().EnableFreezeValidation, Is.False); + } +} diff --git a/Tests/Accordant.Tests/StateGraphValidationTests.cs b/Tests/Accordant.Tests/StateGraphValidationTests.cs new file mode 100644 index 0000000..a0b2658 --- /dev/null +++ b/Tests/Accordant.Tests/StateGraphValidationTests.cs @@ -0,0 +1,248 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +namespace Microsoft.Accordant.Tests; + +using System; +using System.Collections.Generic; +using System.Globalization; +using Microsoft.Accordant; +using NUnit.Framework; + +[TestFixture] +public class StateGraphValidationTests +{ + private sealed class TestState : State + { + public int Value { get; set; } + + protected override void CloneInternal(Dictionary clonedMap) + { + clonedMap[this] = new TestState { Value = Value }; + } + + protected override string StringRepresentationInternal( + Dictionary objectPaths, + string path, + bool forceRecompute) + { + return Value.ToString(CultureInfo.InvariantCulture); + } + + protected override void FreezeComponents(HashSet visited) + { + } + } + + private sealed class TestStep : BaseStepFunction + { + private readonly string id; + private readonly Func> apply; + + public TestStep(string id, Func> apply = null) + { + this.id = id; + this.apply = apply ?? (state => null); + } + + public override string StepFunctionId => id; + + protected override IList ApplyInternal(IState state) + { + return apply(state); + } + } + + private static StateGraphNode Explore( + IList steps, + int maxDepth = 1) + { + return StateGraph.ExploreStateGraph( + steps, + new TestState(), + maxDepth: maxDepth); + } + + [Test] + public void ExploreStateGraph_RejectsInvalidArgumentsAtTheBoundary() + { + Assert.That( + Assert.Throws(() => + StateGraph.ExploreStateGraph(null, new TestState())), + Has.Property(nameof(ArgumentNullException.ParamName)).EqualTo("steps")); + + Assert.That( + Assert.Throws(() => + StateGraph.ExploreStateGraph(Array.Empty(), null)), + Has.Property(nameof(ArgumentNullException.ParamName)).EqualTo("startingState")); + + Assert.That( + Assert.Throws(() => + Explore(Array.Empty(), maxDepth: -2)), + Has.Property(nameof(ArgumentOutOfRangeException.ParamName)).EqualTo("maxDepth")); + + Assert.Throws(() => + Explore(new IStepFunction[] { null })); + + Assert.Throws(() => + Explore(new IStepFunction[] { new TestStep(string.Empty) })); + + var duplicateIdSteps = new IStepFunction[] + { + new TestStep("duplicate"), + new TestStep("duplicate") + }; + + Assert.Throws(() => Explore(duplicateIdSteps)); + } + + [Test] + public void StateGraphNode_SnapshotsCollectionsAndExposesReadOnlyProperties() + { + var step = new TestStep("step"); + var steps = new List { step }; + var root = Explore(steps); + + steps.Clear(); + + Assert.That(root.StepFunctions, Has.Count.EqualTo(1)); + Assert.That(root.StepFunctions[0], Is.SameAs(step)); + Assert.That(typeof(StateGraphNode).GetProperty(nameof(StateGraphNode.State)).CanWrite, Is.False); + Assert.That(typeof(StateGraphNode).GetProperty(nameof(StateGraphNode.StepFunctions)).CanWrite, Is.False); + Assert.That(typeof(StateGraphNode).GetProperty(nameof(StateGraphNode.Edges)).CanWrite, Is.False); + Assert.That(typeof(StateGraphEdge).GetProperty(nameof(StateGraphEdge.Target)).CanWrite, Is.False); + Assert.That(typeof(StateGraphEdge).GetProperty(nameof(StateGraphEdge.StepFunction)).CanWrite, Is.False); + Assert.That(typeof(StateGraphEdge).GetProperty(nameof(StateGraphEdge.Metadata)).CanWrite, Is.False); + } + + [Test] + public void ExploreStateGraph_RejectsNullStepResults() + { + var step = new TestStep("null-result", _ => new List { null }); + + var exception = Assert.Throws(() => Explore(new[] { step })); + + Assert.That(exception.InnerException, Is.TypeOf()); + Assert.That(exception.InnerException.Message, Does.Contain("null StepResult")); + } + + [Test] + public void ExploreStateGraph_RejectsNullResultStates() + { + var step = new TestStep("null-state", _ => new List + { + new StepResult() + }); + + var exception = Assert.Throws(() => Explore(new[] { step })); + + Assert.That(exception.InnerException, Is.TypeOf()); + Assert.That(exception.InnerException.Message, Does.Contain("null State")); + } + + [Test] + public void ExploreStateGraph_RejectsNullSuccessorStepFunctions() + { + var step = new TestStep("null-successor", state => new List + { + new StepResult + { + State = state, + StepFunctions = new IStepFunction[] { null } + } + }); + + var exception = Assert.Throws(() => Explore(new[] { step })); + + Assert.That(exception.InnerException, Is.TypeOf()); + Assert.That(exception.InnerException.Message, Does.Contain("null step functions")); + } + + [Test] + public void ExploreStateGraph_RejectsDuplicateSuccessorStepFunctions() + { + var otherStep = new TestStep("other"); + var step = new TestStep("source", state => new List + { + new StepResult + { + State = state, + StepFunctions = new IStepFunction[] { otherStep } + } + }); + + var exception = Assert.Throws(() => + Explore(new IStepFunction[] { step, otherStep })); + + Assert.That(exception.InnerException, Is.TypeOf()); + Assert.That(exception.InnerException.Message, Does.Contain("duplicate StepFunctionId")); + } + + [Test] + public void LazyExpansion_RethrowsTheOriginalFailureInsteadOfReturningPartialEdges() + { + var step = new TestStep("failing", _ => + throw new InvalidOperationException("expected lazy expansion failure")); + var root = StateGraph.ExploreStateGraph( + new IStepFunction[] { step }, + new TestState(), + lazy: true); + + var firstException = Assert.Throws(() => _ = root.Edges); + var secondException = Assert.Throws(() => _ = root.Edges); + + Assert.That(secondException, Is.SameAs(firstException)); + Assert.That(firstException.InnerException, Is.TypeOf()); + Assert.That(firstException.InnerException.Message, Does.Contain("expected lazy expansion failure")); + } + + [Test] + public void ContractStepFunction_RejectsValidVerificationWithoutAStateProfile() + { + var contract = new ContractStepFunction( + request: null, + observedResponse: null, + verify: (_, _, _) => (true, (StateProfile)null)); + + var exception = Assert.Throws(() => contract.Apply( + new TestState(), + Array.Empty<(IStepFunction, StateGraphNode)>())); + + Assert.That(exception.Message, Does.Contain("non-null StateProfile")); + } + + [Test] + public void ContractStepFunction_RejectsValidVerificationWithNoOutcomes() + { + var emptyProfile = new StateProfile( + new List<(IState, IList)>()); + var contract = new ContractStepFunction( + request: null, + observedResponse: null, + verify: (_, _, _) => (true, emptyProfile)); + + var exception = Assert.Throws(() => contract.Apply( + new TestState(), + Array.Empty<(IStepFunction, StateGraphNode)>())); + + Assert.That(exception.Message, Does.Contain("at least one state outcome")); + } + + [Test] + public void ContractStepFunction_RejectsInvalidPredecessorIds() + { + Assert.Throws(() => new ContractStepFunction( + request: null, + observedResponse: null, + verify: (_, _, _) => (false, (StateProfile)null), + predecessorIds: new[] { "" })); + + var contract = new ContractStepFunction( + request: null, + observedResponse: null, + verify: (_, _, _) => (false, (StateProfile)null)); + + Assert.Throws(() => + contract.SetPredecessorIds(new[] { "duplicate", "duplicate" })); + } +}