diff --git a/README.md b/README.md index aa5ea5a543..d61a556024 100644 --- a/README.md +++ b/README.md @@ -91,7 +91,7 @@ public class PublishModule : Module protected override async Task ExecuteAsync(IModuleContext context, CancellationToken cancellationToken) { var buildResult = await context.GetModule(); - var outputPath = buildResult.ValueOrDefault!.OutputPath; // Strongly-typed, compile-time checked + var outputPath = buildResult.Value.OutputPath; // Throws with module context if unavailable // Publish using the build output... return None.Value; } diff --git a/README_Template.md b/README_Template.md index e015f18974..16753ef86d 100644 --- a/README_Template.md +++ b/README_Template.md @@ -91,7 +91,7 @@ public class PublishModule : Module protected override async Task ExecuteAsync(IModuleContext context, CancellationToken cancellationToken) { var buildResult = await context.GetModule(); - var outputPath = buildResult.ValueOrDefault!.OutputPath; // Strongly-typed, compile-time checked + var outputPath = buildResult.Value.OutputPath; // Throws with module context if unavailable // Publish using the build output... return None.Value; } diff --git a/docs/docs/fundamentals.md b/docs/docs/fundamentals.md index e73ec65418..c9bbeed750 100644 --- a/docs/docs/fundamentals.md +++ b/docs/docs/fundamentals.md @@ -51,16 +51,22 @@ Modules are strongly typed, so we can return clear, concrete objects, and other // Get a module's result var myModule = await context.GetModule(); -// Access the value using pattern matching (recommended) -if (myModule is ModuleResult.Success { Value: var result }) +// Access a required dependency value directly. +// This throws with module context if it failed, was skipped, or returned null. +var requiredValue = myModule.Value; +var firstString = requiredValue.MyFirstString; +var secondString = requiredValue.MySecondString; + +// Or use pattern matching when every outcome needs different handling +if (myModule is ModuleResult.Success { Value: var successfulValue }) { - var string1 = result.MyFirstString; - var string2 = result.MySecondString; + Console.WriteLine(successfulValue.MyFirstString); + Console.WriteLine(successfulValue.MySecondString); } -// Or use ValueOrDefault for simpler access -var string1 = myModule.ValueOrDefault?.MyFirstString; -var string2 = myModule.ValueOrDefault?.MySecondString; +// ValueOrDefault remains available when missing data is expected +var optionalFirstString = myModule.ValueOrDefault?.MyFirstString; +var optionalSecondString = myModule.ValueOrDefault?.MySecondString; ``` ## Custom Types diff --git a/docs/docs/how-to/sharing-data.md b/docs/docs/how-to/sharing-data.md index ae48a1cd85..4789b41ba4 100644 --- a/docs/docs/how-to/sharing-data.md +++ b/docs/docs/how-to/sharing-data.md @@ -21,8 +21,9 @@ public class DeployModule : Module // Get the build module's result var buildResult = await context.GetModule(); - // Access the value safely - var artifact = buildResult.ValueOrDefault?.ArtifactPath; + // Access the required dependency value. This throws with module context + // if the module failed, was skipped, or returned null. + var artifact = buildResult.Value.ArtifactPath; return await Deploy(artifact); } @@ -63,9 +64,20 @@ return result.Match( ); ``` +## Accessing Required Values + +Use `Value` when the dependency must have produced a non-null value. It returns `T` +without a null-forgiveness operator. If the module failed, was skipped, or returned +`null`, it throws an `InvalidOperationException` that identifies the module and outcome: + +```csharp +var result = await context.GetModule(); +var value = result.Value; +``` + ## Safe Accessors -For simpler checks, inspect the union through its safe accessors: +When an absent value is expected, inspect the union through its non-throwing accessors: ```csharp var result = await context.GetModule(); diff --git a/src/ModularPipelines/Models/ModuleResult.cs b/src/ModularPipelines/Models/ModuleResult.cs index fd7a24d2f1..4f9e0865ab 100644 --- a/src/ModularPipelines/Models/ModuleResult.cs +++ b/src/ModularPipelines/Models/ModuleResult.cs @@ -1,4 +1,5 @@ using System.Diagnostics.CodeAnalysis; +using System.Text; using System.Text.Json; using System.Text.Json.Serialization; using ModularPipelines.Engine; @@ -218,6 +219,25 @@ private protected ModuleResult() [JsonConverter(typeof(ModuleResultJsonConverterFactory))] public abstract record ModuleResult : ModuleResult { + /// + /// Gets the successful non-null value. + /// + /// + /// The module failed, was skipped, or succeeded with a value. + /// + [JsonIgnore] + public T Value => this switch + { + Success { Value: not null } success => success.Value, + Success => throw new InvalidOperationException($"{ModuleName} succeeded but returned null"), + Failure failure => throw new InvalidOperationException( + $"{ModuleName} failed: {failure.Exception.Message}", + failure.Exception), + Skipped skipped => throw new InvalidOperationException( + $"{ModuleName} was skipped: {skipped.Decision.Reason ?? "No reason was provided"}"), + _ => throw new InvalidOperationException($"{ModuleName} has an unknown result type"), + }; + // === Safe accessors (no exceptions) === /// @@ -298,9 +318,39 @@ public void Switch( /// /// Represents a successful module execution with a value. /// - /// The value produced by the module, which may be null. [JsonConverter(typeof(ModuleResultJsonConverterFactory))] - public sealed record Success(T? Value) : ModuleResult; + public sealed record Success : ModuleResult + { +#pragma warning disable SA1313 // Preserve the public parameter names generated by the former positional record. + /// + /// Initialises a new instance of the class. + /// + /// The value produced by the module, which may be null. + public Success(T? Value) + { + this.Value = Value; + } + + /// + /// Gets the value produced by the module, which may be null. + /// + /// + /// This property intentionally hides the required + /// accessor to preserve the nullable value carried by a known successful result. + /// Access through uses the required accessor instead. + /// + public new T? Value { get; init; } + + /// + /// Deconstructs the result into its successful value. + /// + /// The value produced by the module, which may be null. + public void Deconstruct(out T? Value) + { + Value = this.Value; + } +#pragma warning restore SA1313 + } /// /// Represents a failed module execution with an exception. @@ -407,6 +457,9 @@ internal static Success CreateSuccess(T? value, ModuleExecutionContext ctx) /// protected override object? GetValueOrDefault() => ValueOrDefault; + /// + protected override bool PrintMembers(StringBuilder builder) => base.PrintMembers(builder); + // Prevent external inheritance - only Success, Failure, and Skipped are valid private protected ModuleResult() { @@ -455,8 +508,6 @@ internal sealed class ExceptionJsonConverter : JsonConverter string? typeName = null; string? message = null; - string? stackTrace = null; - while (reader.Read()) { if (reader.TokenType == JsonTokenType.EndObject) @@ -478,7 +529,6 @@ internal sealed class ExceptionJsonConverter : JsonConverter message = reader.GetString(); break; case "StackTrace": - stackTrace = reader.GetString(); break; } } @@ -495,8 +545,7 @@ internal sealed class ExceptionJsonConverter : JsonConverter { try { - var ex = Activator.CreateInstance(exceptionType, message) as Exception; - if (ex != null) + if (Activator.CreateInstance(exceptionType, message) is Exception ex) { return ex; } @@ -618,10 +667,10 @@ internal sealed class ModuleResultNonGenericJsonConverter : JsonConverter : JsonConverter result = success; + success.Deconstruct(Value: out var deconstructedValue); + + using (Assert.Multiple()) + { + await Assert.That(result.Value).IsEqualTo(42); + await Assert.That(success.Value).IsEqualTo(42); + await Assert.That(deconstructedValue).IsEqualTo(42); + } + } + [Test] public async Task Generic_Skipped_Can_Be_Pattern_Matched() { @@ -164,6 +179,127 @@ public async Task Concrete_Generic_Skipped_Serializes_Through_Json() await Assert.That(deserialized!.SkipDecisionOrDefault?.Reason).IsEqualTo("Not needed"); } + [Test] + public async Task Success_Constructor_PreservesValueNamedArgument() + { + var success = new ModuleResult.Success(Value: 42) + { + ModuleName = nameof(IntModule), + ModuleDuration = TimeSpan.Zero, + ModuleStart = DateTimeOffset.UtcNow, + ModuleEnd = DateTimeOffset.UtcNow, + ModuleStatus = Status.Successful, + }; + + await Assert.That(success.Value).IsEqualTo(42); + } + + [Test] + public async Task Success_Value_SurvivesJsonRoundTrip() + { + ModuleResult result = CreateSuccess(42); + + var json = JsonSerializer.Serialize(result); + var deserialized = JsonSerializer.Deserialize>(json); + + await Assert.That(deserialized!.Value).IsEqualTo(42); + } + + [Test] + public async Task Failure_Value_ThrowsWithModuleContext() + { + var failure = new InvalidOperationException("Compilation failed"); + ModuleResult result = CreateFailure(failure); + + var exception = await Assert.That(() => result.Value) + .Throws(); + + using (Assert.Multiple()) + { + await Assert.That(exception!.Message).IsEqualTo("IntModule failed: Compilation failed"); + await Assert.That(exception.InnerException).IsSameReferenceAs(failure); + } + } + + [Test] + public async Task Skipped_Value_ThrowsWithModuleContext() + { + ModuleResult result = CreateSkipped("No source changes"); + + var exception = await Assert.That(() => result.Value) + .Throws(); + + await Assert.That(exception!.Message).IsEqualTo("IntModule was skipped: No source changes"); + } + + [Test] + public async Task NullSuccess_Value_ThrowsWithModuleContext() + { + ModuleResult result = new ModuleResult.Success(null) + { + ModuleName = "NullableModule", + ModuleDuration = TimeSpan.Zero, + ModuleStart = DateTimeOffset.UtcNow, + ModuleEnd = DateTimeOffset.UtcNow, + ModuleStatus = Status.Successful, + }; + + var exception = await Assert.That(() => result.Value) + .Throws(); + + await Assert.That(exception!.Message).IsEqualTo("NullableModule succeeded but returned null"); + } + + [Test] + public async Task Failure_ToString_DoesNotEvaluateRequiredValue() + { + ModuleResult result = CreateFailure(new InvalidOperationException("Compilation failed")); + + var formatted = result.ToString(); + + await Assert.That(formatted).Contains("Compilation failed"); + } + + [Test] + public async Task Skipped_ToString_DoesNotEvaluateRequiredValue() + { + ModuleResult result = CreateSkipped("No source changes"); + + var formatted = result.ToString(); + + await Assert.That(formatted).Contains("No source changes"); + } + + [Test] + public async Task NullSuccess_ToString_DoesNotEvaluateRequiredValue() + { + ModuleResult result = new ModuleResult.Success(null) + { + ModuleName = "NullableModule", + ModuleDuration = TimeSpan.Zero, + ModuleStart = DateTimeOffset.UtcNow, + ModuleEnd = DateTimeOffset.UtcNow, + ModuleStatus = Status.Successful, + }; + + var formatted = result.ToString(); + + await Assert.That(formatted).Contains(nameof(ModuleResult.Success)); + } + + [Test] + public async Task Success_ToString_PrintsValueOnce() + { + ModuleResult result = CreateSuccess(42); + + var formatted = result.ToString(); + var valueOccurrences = formatted + .Split("Value = 42", StringSplitOptions.None) + .Length - 1; + + await Assert.That(valueOccurrences).IsEqualTo(1); + } + [Test] public async Task NullSuccess_TryGetValue_ReturnsTrue() { @@ -202,9 +338,9 @@ private static ModuleResult.Success CreateSuccess(int value) }; } - private static ModuleResult CreateFailure() + private static ModuleResult CreateFailure(Exception? exception = null) { - return new ModuleResult.Failure(new InvalidOperationException("Failed")) + return new ModuleResult.Failure(exception ?? new InvalidOperationException("Failed")) { ModuleName = nameof(IntModule), ModuleTypeName = typeof(IntModule).FullName, @@ -215,6 +351,18 @@ private static ModuleResult CreateFailure() }; } + private static ModuleResult CreateSkipped(string reason) + { + return new ModuleResult.Skipped(SkipDecision.Skip(reason)) + { + ModuleName = nameof(IntModule), + ModuleDuration = TimeSpan.Zero, + ModuleStart = DateTimeOffset.UtcNow, + ModuleEnd = DateTimeOffset.UtcNow, + ModuleStatus = Status.Skipped, + }; + } + private sealed class IntModule : Module { protected internal override Task ExecuteAsync(