diff --git a/docs/plans/2026-01-07-module-result-discriminated-union-implementation.md b/docs/plans/2026-01-07-module-result-discriminated-union-implementation.md new file mode 100644 index 0000000000..e0fad1db7a --- /dev/null +++ b/docs/plans/2026-01-07-module-result-discriminated-union-implementation.md @@ -0,0 +1,656 @@ +# ModuleResult Discriminated Union Implementation Plan + +> **For Claude:** REQUIRED SUB-SKILL: Use superpowers:executing-plans to implement this plan task-by-task. + +**Goal:** Replace exception-throwing `ModuleResult.Value` with a type-safe discriminated union pattern using C# records. + +**Architecture:** Three sealed record variants (`Success`, `Failure`, `Skipped`) inherit from abstract `ModuleResult`. Common metadata lives in the base. Pattern matching replaces try-catch. Timeout is a `Failure` with `ModuleTimeoutException` (pattern match on exception type if needed). + +**Tech Stack:** .NET 10, C# records, System.Text.Json polymorphic serialization + +--- + +## Task 1: Update IModuleResult Interface + +**Files:** +- Modify: `src/ModularPipelines/Models/IModuleResult.cs` + +**Step 1: Update the interface to add safe accessors** + +Replace the file content with: + +```csharp +using ModularPipelines.Enums; + +namespace ModularPipelines.Models; + +/// +/// Non-generic interface for type-erased module result access. +/// +public interface IModuleResult +{ + /// + /// Gets the name of the module. + /// + string ModuleName { get; } + + /// + /// Gets how long the module ran for. + /// + TimeSpan ModuleDuration { get; } + + /// + /// Gets when the module started. + /// + DateTimeOffset ModuleStart { get; } + + /// + /// Gets when the module ended. + /// + DateTimeOffset ModuleEnd { get; } + + /// + /// Gets the status of the module. + /// + Status ModuleStatus { get; } + + /// + /// Gets the type of result that is held. + /// + ModuleResultType ModuleResultType { get; } + + /// + /// Gets whether the result is a success. + /// + bool IsSuccess { get; } + + /// + /// Gets whether the result is a failure. + /// + bool IsFailure { get; } + + /// + /// Gets whether the result was skipped. + /// + bool IsSkipped { get; } + + /// + /// Gets the value if successful, or null/default otherwise. Does not throw. + /// + object? ValueOrDefault { get; } + + /// + /// Gets the exception if failed, or null otherwise. + /// + Exception? ExceptionOrDefault { get; } + + /// + /// Gets the skip decision if skipped, or null otherwise. + /// + SkipDecision? SkipDecisionOrDefault { get; } +} +``` + +**Step 2: Build to verify compilation** + +Run: `dotnet build src/ModularPipelines/ModularPipelines.csproj -c Release` +Expected: Build errors (ModuleResult doesn't implement new members yet) - this is expected + +**Step 3: Commit interface changes** + +```bash +git add src/ModularPipelines/Models/IModuleResult.cs +git commit -m "refactor(IModuleResult): add safe accessor properties for discriminated union pattern" +``` + +--- + +## Task 2: Rewrite ModuleResult as Discriminated Union + +**Files:** +- Modify: `src/ModularPipelines/Models/ModuleResult.cs` + +**Step 1: Replace ModuleResult.cs with discriminated union implementation** + +Replace the entire file with: + +```csharp +using System.Diagnostics.CodeAnalysis; +using System.Text.Json.Serialization; +using ModularPipelines.Engine; +using ModularPipelines.Enums; + +namespace ModularPipelines.Models; + +/// +/// Represents the result of a module execution as a discriminated union. +/// Use pattern matching to handle Success, Failure, and Skipped cases. +/// +/// The type of value returned on success. +/// +/// +/// var result = await myModule; +/// switch (result) +/// { +/// case ModuleResult<string>.Success { Value: var value }: +/// Console.WriteLine($"Got: {value}"); +/// break; +/// case ModuleResult<string>.Failure { Exception: var ex }: +/// Console.WriteLine($"Failed: {ex.Message}"); +/// break; +/// case ModuleResult<string>.Skipped { Decision: var skip }: +/// Console.WriteLine($"Skipped: {skip.Reason}"); +/// break; +/// } +/// +/// +// NOTE: Actual implementation uses a custom JsonConverter instead of JsonDerivedType +// because JsonDerivedType doesn't handle generic types well at runtime. +// The actual attribute is: [JsonConverter(typeof(ModuleResultJsonConverterFactory))] +[JsonConverter(typeof(ModuleResultJsonConverterFactory))] +public abstract record ModuleResult : IModuleResult +{ + // === Metadata (available on all outcomes) === + + /// + [JsonInclude] + public required string ModuleName { get; init; } + + /// + [JsonInclude] + public required TimeSpan ModuleDuration { get; init; } + + /// + [JsonInclude] + public required DateTimeOffset ModuleStart { get; init; } + + /// + [JsonInclude] + public required DateTimeOffset ModuleEnd { get; init; } + + /// + [JsonInclude] + public required Status ModuleStatus { get; init; } + + // === Quick checks === + + /// + [JsonIgnore] + public bool IsSuccess => this is Success; + + /// + [JsonIgnore] + public bool IsFailure => this is Failure; + + /// + [JsonIgnore] + public bool IsSkipped => this is Skipped; + + // === Safe accessors (no exceptions) === + + /// + /// Gets the value if successful, or default(T) otherwise. Does not throw. + /// + [JsonIgnore] + public T? ValueOrDefault => this is Success s ? s.Value : default; + + /// + [JsonIgnore] + object? IModuleResult.ValueOrDefault => ValueOrDefault; + + /// + [JsonIgnore] + public Exception? ExceptionOrDefault => this is Failure f ? f.Exception : null; + + /// + [JsonIgnore] + public SkipDecision? SkipDecisionOrDefault => this is Skipped sk ? sk.Decision : null; + + // === Computed for compatibility === + + /// + [JsonIgnore] + public ModuleResultType ModuleResultType => this switch + { + Success => ModuleResultType.Success, + Failure => ModuleResultType.Failure, + Skipped => ModuleResultType.Skipped, + _ => throw new InvalidOperationException("Unknown result type") + }; + + // === Internal: Module type tracking === + + /// + /// Gets or sets the type of the module that produced this result. + /// + [JsonIgnore] + internal Type? ModuleType { get; init; } + + // === Pattern matching helpers === + + /// + /// Matches the result to one of three functions based on the outcome. + /// + /// The return type of the match functions. + /// Function to call if successful. + /// Function to call if failed. + /// Function to call if skipped. + /// The result of the matched function. + public TResult Match( + Func onSuccess, + Func onFailure, + Func onSkipped) => this switch + { + Success s => onSuccess(s.Value), + Failure f => onFailure(f.Exception), + Skipped sk => onSkipped(sk.Decision), + _ => throw new InvalidOperationException("Unknown result type") + }; + + /// + /// Executes one of three actions based on the outcome. + /// + /// Action to call if successful. + /// Action to call if failed. + /// Action to call if skipped. + public void Switch( + Action onSuccess, + Action onFailure, + Action onSkipped) + { + switch (this) + { + case Success s: + onSuccess(s.Value); + break; + case Failure f: + onFailure(f.Exception); + break; + case Skipped sk: + onSkipped(sk.Decision); + break; + } + } + + // === Discriminated variants === + + /// + /// Represents a successful module execution with a value. + /// + /// The value produced by the module. + public sealed record Success(T Value) : ModuleResult; + + /// + /// Represents a failed module execution with an exception. + /// + /// The exception that caused the failure. + public sealed record Failure(Exception Exception) : ModuleResult; + + /// + /// Represents a skipped module execution. + /// + /// The skip decision containing the reason. + public sealed record Skipped(SkipDecision Decision) : ModuleResult; + + // === Internal factory methods === + + internal static Success CreateSuccess(T value, ModuleExecutionContext ctx) => new(value) + { + ModuleName = ctx.ModuleType.Name, + ModuleDuration = (ctx.EndTime == DateTimeOffset.MinValue ? DateTimeOffset.Now : ctx.EndTime) - + (ctx.StartTime == DateTimeOffset.MinValue ? DateTimeOffset.Now : ctx.StartTime), + ModuleStart = ctx.StartTime == DateTimeOffset.MinValue ? DateTimeOffset.Now : ctx.StartTime, + ModuleEnd = ctx.EndTime == DateTimeOffset.MinValue ? DateTimeOffset.Now : ctx.EndTime, + ModuleStatus = ctx.Status, + ModuleType = ctx.ModuleType + }; + + internal static Failure CreateFailure(Exception exception, ModuleExecutionContext ctx) => new(exception) + { + ModuleName = ctx.ModuleType.Name, + ModuleDuration = (ctx.EndTime == DateTimeOffset.MinValue ? DateTimeOffset.Now : ctx.EndTime) - + (ctx.StartTime == DateTimeOffset.MinValue ? DateTimeOffset.Now : ctx.StartTime), + ModuleStart = ctx.StartTime == DateTimeOffset.MinValue ? DateTimeOffset.Now : ctx.StartTime, + ModuleEnd = ctx.EndTime == DateTimeOffset.MinValue ? DateTimeOffset.Now : ctx.EndTime, + ModuleStatus = ctx.Status, + ModuleType = ctx.ModuleType + }; + + internal static Skipped CreateSkipped(SkipDecision decision, ModuleExecutionContext ctx) => new(decision) + { + ModuleName = ctx.ModuleType.Name, + ModuleDuration = (ctx.EndTime == DateTimeOffset.MinValue ? DateTimeOffset.Now : ctx.EndTime) - + (ctx.StartTime == DateTimeOffset.MinValue ? DateTimeOffset.Now : ctx.StartTime), + ModuleStart = ctx.StartTime == DateTimeOffset.MinValue ? DateTimeOffset.Now : ctx.StartTime, + ModuleEnd = ctx.EndTime == DateTimeOffset.MinValue ? DateTimeOffset.Now : ctx.EndTime, + ModuleStatus = ctx.Status, + ModuleType = ctx.ModuleType + }; + + // Prevent external inheritance - only Success, Failure, Skipped are valid + private protected ModuleResult() + { + } +} +``` + +**Step 2: Build to check for errors** + +Run: `dotnet build src/ModularPipelines/ModularPipelines.csproj -c Release` +Expected: Build errors from files still referencing old ModuleResult pattern - this is expected + +**Step 3: Commit the new discriminated union** + +```bash +git add src/ModularPipelines/Models/ModuleResult.cs +git commit -m "refactor(ModuleResult): implement discriminated union with Success/Failure/Skipped variants" +``` + +--- + +## Task 3: Delete Obsolete Subclasses + +**Files:** +- Delete: `src/ModularPipelines/Models/SkippedModuleResult.cs` +- Delete: `src/ModularPipelines/Models/TimedOutModuleResult.cs` + +**Step 1: Delete the files** + +```bash +rm src/ModularPipelines/Models/SkippedModuleResult.cs +rm src/ModularPipelines/Models/TimedOutModuleResult.cs +``` + +**Step 2: Commit deletions** + +```bash +git add -A +git commit -m "refactor: remove SkippedModuleResult and TimedOutModuleResult (replaced by variants)" +``` + +--- + +## Task 4: Update ModuleResultFactory + +**Files:** +- Modify: `src/ModularPipelines/Engine/Execution/ModuleResultFactory.cs` + +**Step 1: Simplify the factory to use new static methods** + +Replace the file with: + +```csharp +using ModularPipelines.Models; + +namespace ModularPipelines.Engine.Execution; + +/// +/// Factory for creating ModuleResult instances. +/// +internal static class ModuleResultFactory +{ + /// + /// Creates a successful ModuleResult for the specified value. + /// + public static ModuleResult CreateSuccess(T value, ModuleExecutionContext ctx) + { + return ModuleResult.CreateSuccess(value, ctx); + } + + /// + /// Creates a failure ModuleResult for the specified exception. + /// + public static ModuleResult CreateFailure(Exception exception, ModuleExecutionContext ctx) + { + return ModuleResult.CreateFailure(exception, ctx); + } + + /// + /// Creates a skipped ModuleResult for the specified skip decision. + /// + public static ModuleResult CreateSkipped(SkipDecision decision, ModuleExecutionContext ctx) + { + return ModuleResult.CreateSkipped(decision, ctx); + } + + /// + /// Creates a skipped ModuleResult (type-erased version for engine use). + /// + public static IModuleResult CreateSkipped(Type resultType, ModuleExecutionContext executionContext) + { + var method = typeof(ModuleResultFactory) + .GetMethod(nameof(CreateSkippedGeneric), System.Reflection.BindingFlags.NonPublic | System.Reflection.BindingFlags.Static)! + .MakeGenericMethod(resultType); + + return (IModuleResult)method.Invoke(null, [executionContext.SkipResult ?? SkipDecision.DoNotSkip, executionContext])!; + } + + /// + /// Creates a failure ModuleResult (type-erased version for engine use). + /// + public static IModuleResult CreateException(Type resultType, Exception exception, ModuleExecutionContext executionContext) + { + var method = typeof(ModuleResultFactory) + .GetMethod(nameof(CreateFailureGeneric), System.Reflection.BindingFlags.NonPublic | System.Reflection.BindingFlags.Static)! + .MakeGenericMethod(resultType); + + return (IModuleResult)method.Invoke(null, [exception, executionContext])!; + } + + private static IModuleResult CreateSkippedGeneric(SkipDecision decision, ModuleExecutionContext ctx) + { + return ModuleResult.CreateSkipped(decision, ctx); + } + + private static IModuleResult CreateFailureGeneric(Exception exception, ModuleExecutionContext ctx) + { + return ModuleResult.CreateFailure(exception, ctx); + } +} +``` + +**Step 2: Build to verify** + +Run: `dotnet build src/ModularPipelines/ModularPipelines.csproj -c Release` +Expected: More build errors from other files - expected at this stage + +**Step 3: Commit** + +```bash +git add src/ModularPipelines/Engine/Execution/ModuleResultFactory.cs +git commit -m "refactor(ModuleResultFactory): simplify to use ModuleResult static factory methods" +``` + +--- + +## Task 5: Update ModuleExecutionPipeline + +**Files:** +- Modify: `src/ModularPipelines/Engine/ModuleExecutionPipeline.cs` + +**Step 1: Read current implementation** + +Run: `grep -n "TimedOutModuleResult\|new ModuleResult\|SkippedModuleResult" src/ModularPipelines/Engine/ModuleExecutionPipeline.cs` + +**Step 2: Update result creation to use factory methods** + +Find and replace usages: +- Replace `new ModuleResult(value, executionContext)` with `ModuleResult.CreateSuccess(value, executionContext)` +- Replace `new ModuleResult(exception, executionContext)` with `ModuleResult.CreateFailure(exception, executionContext)` +- Replace `new SkippedModuleResult(...)` with `ModuleResult.CreateSkipped(...)` +- Replace `new TimedOutModuleResult(...)` with `ModuleResult.CreateFailure(timeoutException, executionContext)` + +**Step 3: Build and fix any remaining issues** + +Run: `dotnet build src/ModularPipelines/ModularPipelines.csproj -c Release` + +**Step 4: Commit** + +```bash +git add src/ModularPipelines/Engine/ModuleExecutionPipeline.cs +git commit -m "refactor(ModuleExecutionPipeline): use discriminated union factory methods" +``` + +--- + +## Task 6: Update Remaining Engine Files + +**Files to update (fix compilation errors):** +- `src/ModularPipelines/Engine/ModuleExecutor.cs` +- `src/ModularPipelines/Engine/Execution/ModuleResultRegistrar.cs` +- `src/ModularPipelines/Engine/Executors/IgnoredModuleResultRegistrar.cs` +- `src/ModularPipelines/Engine/IModuleResultRegistry.cs` + +**Step 1: Build and identify errors** + +Run: `dotnet build src/ModularPipelines/ModularPipelines.csproj -c Release 2>&1 | head -50` + +**Step 2: Fix each error by updating to new pattern** + +Common fixes: +- Replace `.Value` access with `.ValueOrDefault` or pattern matching +- Replace `.Exception` access with `.ExceptionOrDefault` or pattern matching +- Update any code creating ModuleResult directly to use factory methods + +**Step 3: Build until clean** + +Run: `dotnet build src/ModularPipelines/ModularPipelines.csproj -c Release` +Expected: 0 errors + +**Step 4: Commit** + +```bash +git add -A +git commit -m "refactor(engine): update all engine files to use discriminated union pattern" +``` + +--- + +## Task 7: Delete Exception Classes (Optional - can deprecate instead) + +**Files:** +- Modify: `src/ModularPipelines/Exceptions/ModuleFailedException.cs` (add Obsolete attribute) +- Modify: `src/ModularPipelines/Exceptions/ModuleSkippedException.cs` (add Obsolete attribute) + +**Step 1: Mark as obsolete rather than delete (safer for consumers)** + +Add to ModuleFailedException.cs: +```csharp +[Obsolete("Use pattern matching on ModuleResult.Failure instead. This exception is no longer thrown.")] +``` + +Add to ModuleSkippedException.cs: +```csharp +[Obsolete("Use pattern matching on ModuleResult.Skipped instead. This exception is no longer thrown.")] +``` + +**Step 2: Commit** + +```bash +git add src/ModularPipelines/Exceptions/ModuleFailedException.cs +git add src/ModularPipelines/Exceptions/ModuleSkippedException.cs +git commit -m "deprecate: mark ModuleFailedException and ModuleSkippedException as obsolete" +``` + +--- + +## Task 8: Update Test Helpers + +**Files:** +- Modify: `test/ModularPipelines.TestHelpers/Assertions/ModuleResultAssertions.cs` (if exists) + +**Step 1: Find test helper files** + +Run: `find test -name "*.cs" | xargs grep -l "ModuleResult" | head -10` + +**Step 2: Update assertions to use new pattern** + +Replace `.Value` accesses with `.ValueOrDefault` or pattern matching assertions. + +**Step 3: Commit** + +```bash +git add test/ +git commit -m "test: update test helpers for discriminated union pattern" +``` + +--- + +## Task 9: Update Unit Tests + +**Files:** +- `test/ModularPipelines.UnitTests/ReturnNothingTests.cs` +- Other test files using `.Value` + +**Step 1: Find all test files using .Value on ModuleResult** + +Run: `grep -rn "result\.Value" test/ --include="*.cs"` + +**Step 2: Update each test to use new pattern** + +Example change: +```csharp +// Before +await Assert.That(result.Value).IsNull(); + +// After +await Assert.That(result.IsSuccess).IsTrue(); +await Assert.That(result.ValueOrDefault).IsNull(); +``` + +**Step 3: Run tests** + +Run: `dotnet run --project test/ModularPipelines.UnitTests/ModularPipelines.UnitTests.csproj -c Release` +Expected: All tests pass (486+) + +**Step 4: Commit** + +```bash +git add test/ +git commit -m "test: update unit tests for discriminated union pattern" +``` + +--- + +## Task 10: Build Full Solution and Run All Tests + +**Step 1: Build entire solution** + +Run: `dotnet build ModularPipelines.sln -c Release` +Expected: 0 errors + +**Step 2: Run all tests** + +Run: `dotnet run --project test/ModularPipelines.UnitTests/ModularPipelines.UnitTests.csproj -c Release` +Expected: Same or better pass rate as baseline (486 passing) + +**Step 3: Final commit** + +```bash +git add -A +git commit -m "feat: complete discriminated union implementation for ModuleResult + +BREAKING CHANGE: ModuleResult.Value no longer throws exceptions. +Use pattern matching or IsSuccess/ValueOrDefault instead. + +Closes #1869" +``` + +--- + +## Summary + +| Task | Description | Estimated Steps | +|------|-------------|-----------------| +| 1 | Update IModuleResult interface | 3 | +| 2 | Rewrite ModuleResult as discriminated union | 3 | +| 3 | Delete obsolete subclasses | 2 | +| 4 | Update ModuleResultFactory | 3 | +| 5 | Update ModuleExecutionPipeline | 4 | +| 6 | Update remaining engine files | 4 | +| 7 | Deprecate exception classes | 2 | +| 8 | Update test helpers | 3 | +| 9 | Update unit tests | 4 | +| 10 | Final build and test | 3 | + +**Total: ~31 steps** diff --git a/docs/plans/2026-01-07-module-result-discriminated-union.md b/docs/plans/2026-01-07-module-result-discriminated-union.md index 636a26e65c..8a118383dc 100644 --- a/docs/plans/2026-01-07-module-result-discriminated-union.md +++ b/docs/plans/2026-01-07-module-result-discriminated-union.md @@ -76,8 +76,8 @@ switch (result) case ModuleResult.Failure { Exception: var ex }: Console.WriteLine($"Failed: {ex.Message}"); break; - case ModuleResult.Skipped { Decision.Reason: var reason }: - Console.WriteLine($"Skipped: {reason}"); + case ModuleResult.Skipped { Decision: var skip }: + Console.WriteLine($"Skipped: {skip.Reason}"); break; } ``` @@ -172,11 +172,13 @@ internal static Skipped CreateSkipped(SkipDecision decision, ModuleExecutionCont ### Removed - `ModuleResult.Value` property (the throwing one) -- `ModuleFailedException` class -- `ModuleSkippedException` class - `SkippedModuleResult` subclass - `TimedOutModuleResult` subclass +### Deprecated (not removed) +- `ModuleFailedException` class (marked with [Obsolete]) +- `ModuleSkippedException` class (marked with [Obsolete]) + ### Preserved - `ModuleResultType` property (now computed) - `IModuleResult` interface diff --git a/src/ModularPipelines.Build/Modules/CreateReleaseModule.cs b/src/ModularPipelines.Build/Modules/CreateReleaseModule.cs index 3cd16b9150..a19182c1c2 100644 --- a/src/ModularPipelines.Build/Modules/CreateReleaseModule.cs +++ b/src/ModularPipelines.Build/Modules/CreateReleaseModule.cs @@ -57,9 +57,9 @@ public Task ShouldSkip(IPipelineContext context) } return await context.GitHub().Client.Repository.Release.Create(repositoryId, - new NewRelease($"v{versionInfoResult.Value}") + new NewRelease($"v{versionInfoResult.ValueOrDefault}") { - Name = versionInfoResult.Value, + Name = versionInfoResult.ValueOrDefault, GenerateReleaseNotes = true, }); } diff --git a/src/ModularPipelines.Build/Modules/FindProjectDependenciesModule.cs b/src/ModularPipelines.Build/Modules/FindProjectDependenciesModule.cs index 8d0d620c2e..0e2683899b 100644 --- a/src/ModularPipelines.Build/Modules/FindProjectDependenciesModule.cs +++ b/src/ModularPipelines.Build/Modules/FindProjectDependenciesModule.cs @@ -16,7 +16,7 @@ public class FindProjectDependenciesModule : Module(); - foreach (var file in projects.Value!) + foreach (var file in projects.ValueOrDefault!) { var projectRootElement = ProjectRootElement.Open(file)!; @@ -27,7 +27,7 @@ public class FindProjectDependenciesModule : Module x.Name == name); + var project = projects.ValueOrDefault!.FirstOrDefault(x => x.Name == name); if (project != null) { @@ -36,7 +36,7 @@ public class FindProjectDependenciesModule : Module>, IAlways var projects = context.GetModule>(); - foreach (var project in projects.Value! + foreach (var project in projects.ValueOrDefault! .Where(x => !x.NameWithoutExtension.StartsWith("ModularPipelines.Analyzers"))) { var moduleName = project.NameWithoutExtension; diff --git a/src/ModularPipelines.Build/Modules/LocalMachine/AddLocalNugetSourceModule.cs b/src/ModularPipelines.Build/Modules/LocalMachine/AddLocalNugetSourceModule.cs index fbf56968ed..faa6dda6e3 100644 --- a/src/ModularPipelines.Build/Modules/LocalMachine/AddLocalNugetSourceModule.cs +++ b/src/ModularPipelines.Build/Modules/LocalMachine/AddLocalNugetSourceModule.cs @@ -27,7 +27,7 @@ public Task ShouldIgnoreFailures(IPipelineContext context, Exception excep return await context.DotNet().Nuget.Add.Source(new DotNetNugetAddSourceOptions { Name = "ModularPipelinesLocalNuGet", - Packagesourcepath = localNugetPathResult.Value.AssertExists(), + Packagesourcepath = localNugetPathResult.ValueOrDefault.AssertExists(), }, cancellationToken: cancellationToken); } } \ No newline at end of file diff --git a/src/ModularPipelines.Build/Modules/LocalMachine/UploadPackagesToLocalNuGetModule.cs b/src/ModularPipelines.Build/Modules/LocalMachine/UploadPackagesToLocalNuGetModule.cs index 2383393e95..8a49ccd151 100644 --- a/src/ModularPipelines.Build/Modules/LocalMachine/UploadPackagesToLocalNuGetModule.cs +++ b/src/ModularPipelines.Build/Modules/LocalMachine/UploadPackagesToLocalNuGetModule.cs @@ -21,8 +21,8 @@ public class UploadPackagesToLocalNuGetModule : Module return await NugetUploadHelper.UploadPackagesAsync( context, - packagePaths.Value!, - source: localRepoLocation.Value.AssertExists()!.Path, + packagePaths.ValueOrDefault!, + source: localRepoLocation.ValueOrDefault.AssertExists()!.Path, apiKey: null, cancellationToken); } diff --git a/src/ModularPipelines.Build/Modules/PackProjectsModule.cs b/src/ModularPipelines.Build/Modules/PackProjectsModule.cs index de1fb0e2d9..3d9c417665 100644 --- a/src/ModularPipelines.Build/Modules/PackProjectsModule.cs +++ b/src/ModularPipelines.Build/Modules/PackProjectsModule.cs @@ -28,23 +28,23 @@ public class PackProjectsModule : Module var changedFiles = context.GetModule>(); - var dependencies = await projectFiles.Value!.Dependencies + var dependencies = await projectFiles.ValueOrDefault!.Dependencies .ToAsyncProcessorBuilder() .SelectAsync(async projectFile => await Pack(context, cancellationToken, projectFile, packageVersion)) .ProcessOneAtATime(); var gitVersioningInformation = await context.Git().Versioning.GetGitVersioningInformation(); - var others = await projectFiles.Value!.Others + var others = await projectFiles.ValueOrDefault!.Others .Where(x => { - if (changedFiles.SkipDecision.ShouldSkip) + if (changedFiles.SkipDecisionOrDefault?.ShouldSkip == true) { return true; } return ProjectHasChanged(x, - changedFiles.Value!, context); + changedFiles.ValueOrDefault!, context); }) .ToAsyncProcessorBuilder() .SelectAsync(async projectFile => await Pack(context, cancellationToken, projectFile, packageVersion)) @@ -79,8 +79,8 @@ private static async Task Pack(IModuleContext context, Cancellati NoRestore = true, Properties = new List { - ("PackageVersion", packageVersion.Value!), - ("Version", packageVersion.Value!), + ("PackageVersion", packageVersion.ValueOrDefault!), + ("Version", packageVersion.ValueOrDefault!), }, }, cancellationToken: cancellationToken); } diff --git a/src/ModularPipelines.Build/Modules/PackagePathsParserModule.cs b/src/ModularPipelines.Build/Modules/PackagePathsParserModule.cs index 7c1ef9bfda..80fdfbbd27 100644 --- a/src/ModularPipelines.Build/Modules/PackagePathsParserModule.cs +++ b/src/ModularPipelines.Build/Modules/PackagePathsParserModule.cs @@ -17,7 +17,7 @@ public class PackagePathsParserModule : Module> { var packPackagesModuleResult = context.GetModule(); - return Task.FromResult?>(packPackagesModuleResult.Value! + return Task.FromResult?>(packPackagesModuleResult.ValueOrDefault! .Select(x => x.StandardOutput) .Select(x => x.Split(PackageCreationSuccessPrefix)[1]) .Select(x => x.Split(PackagePathSuffix)[0]) diff --git a/src/ModularPipelines.Build/Modules/PushVersionTagModule.cs b/src/ModularPipelines.Build/Modules/PushVersionTagModule.cs index eba718662f..591b2b2039 100644 --- a/src/ModularPipelines.Build/Modules/PushVersionTagModule.cs +++ b/src/ModularPipelines.Build/Modules/PushVersionTagModule.cs @@ -18,7 +18,7 @@ public Task ShouldIgnoreFailures(IPipelineContext context, Exception excep { var versionInformation = ((IModuleContext) context).GetModule(); - return Task.FromResult(exception.Message.Contains($"tag 'v{versionInformation.Value!}' already exists")); + return Task.FromResult(exception.Message.Contains($"tag 'v{versionInformation.ValueOrDefault!}' already exists")); } public override async Task ExecuteAsync(IModuleContext context, CancellationToken cancellationToken) @@ -27,7 +27,7 @@ public Task ShouldIgnoreFailures(IPipelineContext context, Exception excep await context.Git().Commands.Tag(new GitTagOptions { - Arguments = [$"v{versionInformation.Value!}"], + Arguments = [$"v{versionInformation.ValueOrDefault!}"], }, token: cancellationToken); return await context.Git().Commands.Push(new GitPushOptions diff --git a/src/ModularPipelines.Build/Modules/UploadPackagesToNugetModule.cs b/src/ModularPipelines.Build/Modules/UploadPackagesToNugetModule.cs index 6ee2ef7072..5e1af8e5df 100644 --- a/src/ModularPipelines.Build/Modules/UploadPackagesToNugetModule.cs +++ b/src/ModularPipelines.Build/Modules/UploadPackagesToNugetModule.cs @@ -41,7 +41,7 @@ public Task ShouldSkip(IPipelineContext context) return await NugetUploadHelper.UploadPackagesAsync( context, - packagePaths.Value!, + packagePaths.ValueOrDefault!, source: "https://api.nuget.org/v3/index.json", apiKey: _nugetSettings.Value.ApiKey, cancellationToken); diff --git a/src/ModularPipelines.Examples/Modules/Azure/AssignAccessToBlobStorageModule.cs b/src/ModularPipelines.Examples/Modules/Azure/AssignAccessToBlobStorageModule.cs index 50e0e6669d..29cc896e0d 100644 --- a/src/ModularPipelines.Examples/Modules/Azure/AssignAccessToBlobStorageModule.cs +++ b/src/ModularPipelines.Examples/Modules/Azure/AssignAccessToBlobStorageModule.cs @@ -21,8 +21,8 @@ public class AssignAccessToBlobStorageModule : Module var storageAccount = context.GetModule(); var roleAssignmentResource = await context.Azure().Provisioner.Security.RoleAssignment( - storageAccount.Value!.Id, - new RoleAssignmentCreateOrUpdateContent(WellKnownRoleDefinitions.BlobStorageOwnerDefinitionId, userAssignedIdentity.Value!.Data.PrincipalId!.Value) + storageAccount.ValueOrDefault!.Id, + new RoleAssignmentCreateOrUpdateContent(WellKnownRoleDefinitions.BlobStorageOwnerDefinitionId, userAssignedIdentity.ValueOrDefault!.Data.PrincipalId!.Value) ); return roleAssignmentResource.Value; diff --git a/src/ModularPipelines.Examples/Modules/Azure/ProvisionAzureFunction.cs b/src/ModularPipelines.Examples/Modules/Azure/ProvisionAzureFunction.cs index e2d0e14c2d..ae5a81df60 100644 --- a/src/ModularPipelines.Examples/Modules/Azure/ProvisionAzureFunction.cs +++ b/src/ModularPipelines.Examples/Modules/Azure/ProvisionAzureFunction.cs @@ -31,7 +31,7 @@ public class ProvisionAzureFunction : Module { Identity = new ManagedServiceIdentity(ManagedServiceIdentityType.UserAssigned) { - UserAssignedIdentities = { { userAssignedIdentity.Value!.Id, new UserAssignedIdentity() } }, + UserAssignedIdentities = { { userAssignedIdentity.ValueOrDefault!.Id, new UserAssignedIdentity() } }, }, SiteConfig = new SiteConfigProperties { @@ -40,12 +40,12 @@ public class ProvisionAzureFunction : Module new() { Name = "BlobStorageConnectionString", - Value = storageAccount.Value!.Data.PrimaryEndpoints.BlobUri.AbsoluteUri, + Value = storageAccount.ValueOrDefault!.Data.PrimaryEndpoints.BlobUri.AbsoluteUri, }, new() { Name = "BlobContainerName", - Value = blobContainer.Value!.Data.Name, + Value = blobContainer.ValueOrDefault!.Data.Name, }, }, }, diff --git a/src/ModularPipelines.Examples/Modules/Azure/ProvisionBlobStorageContainerModule.cs b/src/ModularPipelines.Examples/Modules/Azure/ProvisionBlobStorageContainerModule.cs index 1e2c9ee41b..9ea3d4ee0e 100644 --- a/src/ModularPipelines.Examples/Modules/Azure/ProvisionBlobStorageContainerModule.cs +++ b/src/ModularPipelines.Examples/Modules/Azure/ProvisionBlobStorageContainerModule.cs @@ -15,7 +15,7 @@ public class ProvisionBlobStorageContainerModule : Module var blobStorageAccount = context.GetModule(); var blobContainerProvisionResponse = await context.Azure().Provisioner.Storage.BlobContainer( - blobStorageAccount.Value!.Id, + blobStorageAccount.ValueOrDefault!.Id, "MyContainer", new BlobContainerData() ); diff --git a/src/ModularPipelines.GitHub/GitHubMarkdownSummaryGenerator.cs b/src/ModularPipelines.GitHub/GitHubMarkdownSummaryGenerator.cs index b23564645e..c85ef25ac4 100644 --- a/src/ModularPipelines.GitHub/GitHubMarkdownSummaryGenerator.cs +++ b/src/ModularPipelines.GitHub/GitHubMarkdownSummaryGenerator.cs @@ -63,8 +63,8 @@ private async Task GetException(PipelineSummary pipelineSummary) var exception = results .FirstOrDefault(x => x.ModuleStatus == Status.Failed) - ?.Exception - ?? results.Select(x => x.Exception).FirstOrDefault(); + ?.ExceptionOrDefault + ?? results.Select(x => x.ExceptionOrDefault).FirstOrDefault(); if (exception is null) { diff --git a/src/ModularPipelines/Attributes/Events/IModuleEndEventReceiver.cs b/src/ModularPipelines/Attributes/Events/IModuleEndEventReceiver.cs index b9922c0f65..d59c3832bf 100644 --- a/src/ModularPipelines/Attributes/Events/IModuleEndEventReceiver.cs +++ b/src/ModularPipelines/Attributes/Events/IModuleEndEventReceiver.cs @@ -20,5 +20,5 @@ public interface IModuleEndEventReceiver /// The event context providing module information and control flow. /// The result of the module execution. /// A task representing the async operation. - Task OnModuleEndAsync(IModuleEventContext context, ModuleResult result); + Task OnModuleEndAsync(IModuleEventContext context, IModuleResult result); } diff --git a/src/ModularPipelines/Engine/Attributes/AttributeEventInvoker.cs b/src/ModularPipelines/Engine/Attributes/AttributeEventInvoker.cs index ea78c4a434..7c1fdff8b7 100644 --- a/src/ModularPipelines/Engine/Attributes/AttributeEventInvoker.cs +++ b/src/ModularPipelines/Engine/Attributes/AttributeEventInvoker.cs @@ -85,7 +85,7 @@ public async Task InvokeStartReceiversAsync( public async Task InvokeEndReceiversAsync( IEnumerable receivers, IModuleEventContext context, - ModuleResult result) + IModuleResult result) { foreach (var receiver in receivers) { diff --git a/src/ModularPipelines/Engine/Attributes/IAttributeEventInvoker.cs b/src/ModularPipelines/Engine/Attributes/IAttributeEventInvoker.cs index da3d65859a..6a25a269f3 100644 --- a/src/ModularPipelines/Engine/Attributes/IAttributeEventInvoker.cs +++ b/src/ModularPipelines/Engine/Attributes/IAttributeEventInvoker.cs @@ -14,7 +14,7 @@ internal interface IAttributeEventInvoker Task InvokeStartReceiversAsync(IEnumerable receivers, IModuleEventContext context); - Task InvokeEndReceiversAsync(IEnumerable receivers, IModuleEventContext context, ModuleResult result); + Task InvokeEndReceiversAsync(IEnumerable receivers, IModuleEventContext context, IModuleResult result); Task InvokeFailureReceiversAsync(IEnumerable receivers, IModuleEventContext context, Exception exception); diff --git a/src/ModularPipelines/Engine/Execution/ModuleLifecycleEventInvoker.cs b/src/ModularPipelines/Engine/Execution/ModuleLifecycleEventInvoker.cs index 37fc2e4bc1..6e762f6c16 100644 --- a/src/ModularPipelines/Engine/Execution/ModuleLifecycleEventInvoker.cs +++ b/src/ModularPipelines/Engine/Execution/ModuleLifecycleEventInvoker.cs @@ -96,7 +96,7 @@ public async Task InvokeEndEventAsync(ModuleLifecycleContext context, Enums.Stat _metadataRegistry, context.CancellationToken); - await _attributeEventInvoker.InvokeEndReceiversAsync(receivers, eventContext, (ModuleResult)result).ConfigureAwait(false); + await _attributeEventInvoker.InvokeEndReceiversAsync(receivers, eventContext, result).ConfigureAwait(false); } /// diff --git a/src/ModularPipelines/Engine/Execution/ModuleResultFactory.cs b/src/ModularPipelines/Engine/Execution/ModuleResultFactory.cs index b6f04651cb..b4c9b9b84e 100644 --- a/src/ModularPipelines/Engine/Execution/ModuleResultFactory.cs +++ b/src/ModularPipelines/Engine/Execution/ModuleResultFactory.cs @@ -1,110 +1,100 @@ -using System.Collections.Concurrent; -using System.Linq.Expressions; -using System.Reflection; +using ModularPipelines.Enums; using ModularPipelines.Models; -using ModularPipelines.Modules; namespace ModularPipelines.Engine.Execution; /// -/// Factory for creating ModuleResult instances without reflection. +/// Factory for creating ModuleResult instances. /// -/// -/// Replaces reflection-based constructor invocation with compiled expression trees. -/// internal static class ModuleResultFactory { /// - /// Delegate for creating a ModuleResult with a null value (for skipped modules). + /// Creates a successful ModuleResult for the specified value. /// - internal delegate IModuleResult CreateSkippedResultDelegate(ModuleExecutionContext executionContext); + public static ModuleResult CreateSuccess(T value, ModuleExecutionContext ctx) + { + return ModuleResult.CreateSuccess(value, ctx); + } /// - /// Delegate for creating a ModuleResult with an exception. + /// Creates a failure ModuleResult for the specified exception. /// - internal delegate IModuleResult CreateExceptionResultDelegate(Exception exception, ModuleExecutionContext executionContext); + public static ModuleResult CreateFailure(Exception exception, ModuleExecutionContext ctx) + { + return ModuleResult.CreateFailure(exception, ctx); + } - private static readonly ConcurrentDictionary SkippedResultCache = new(); - private static readonly ConcurrentDictionary ExceptionResultCache = new(); + /// + /// Creates a skipped ModuleResult for the specified skip decision. + /// + public static ModuleResult CreateSkipped(SkipDecision decision, ModuleExecutionContext ctx) + { + return ModuleResult.CreateSkipped(decision, ctx); + } /// - /// Creates a skipped ModuleResult for the specified result type. + /// Creates a skipped ModuleResult (type-erased version for engine use). /// public static IModuleResult CreateSkipped(Type resultType, ModuleExecutionContext executionContext) { - var factory = SkippedResultCache.GetOrAdd(resultType, CreateSkippedFactory); - return factory(executionContext); + var method = typeof(ModuleResultFactory) + .GetMethod(nameof(CreateSkippedGeneric), System.Reflection.BindingFlags.NonPublic | System.Reflection.BindingFlags.Static)! + .MakeGenericMethod(resultType); + + return (IModuleResult)method.Invoke(null, [executionContext.SkipResult ?? SkipDecision.DoNotSkip, executionContext])!; } /// - /// Creates an exception ModuleResult for the specified result type. + /// Creates a failure ModuleResult (type-erased version for engine use). /// public static IModuleResult CreateException(Type resultType, Exception exception, ModuleExecutionContext executionContext) { - var factory = ExceptionResultCache.GetOrAdd(resultType, CreateExceptionFactory); - return factory(exception, executionContext); + var method = typeof(ModuleResultFactory) + .GetMethod(nameof(CreateFailureGeneric), System.Reflection.BindingFlags.NonPublic | System.Reflection.BindingFlags.Static)! + .MakeGenericMethod(resultType); + + return (IModuleResult)method.Invoke(null, [exception, executionContext])!; } - private static CreateSkippedResultDelegate CreateSkippedFactory(Type resultType) + private static IModuleResult CreateSkippedGeneric(SkipDecision decision, ModuleExecutionContext ctx) { - var contextParam = Expression.Parameter(typeof(ModuleExecutionContext), "executionContext"); - var resultGenericType = typeof(ModuleResult<>).MakeGenericType(resultType); - var typedContextType = typeof(ModuleExecutionContext<>).MakeGenericType(resultType); + return ModuleResult.CreateSkipped(decision, ctx); + } - // Cast to typed context - var castContext = Expression.Convert(contextParam, typedContextType); + private static IModuleResult CreateFailureGeneric(Exception exception, ModuleExecutionContext ctx) + { + return ModuleResult.CreateFailure(exception, ctx); + } - // Find the internal constructor: ModuleResult(T? value, ModuleExecutionContext context) - var constructor = resultGenericType.GetConstructor( - BindingFlags.NonPublic | BindingFlags.Instance, - null, - new[] { resultType, typeof(ModuleExecutionContext) }, - null); + /// + /// Creates a copy of the result with a different status (type-erased version for engine use). + /// + public static IModuleResult WithStatus(IModuleResult result, Status status) + { + var resultType = result.GetType(); - if (constructor == null) + // Get the generic type argument from ModuleResult + var genericType = resultType; + while (genericType != null && (!genericType.IsGenericType || genericType.GetGenericTypeDefinition() != typeof(ModuleResult<>))) { - throw new InvalidOperationException( - $"Could not find internal constructor for ModuleResult<{resultType.Name}>(T?, ModuleExecutionContext)"); + genericType = genericType.BaseType; } - // Create: new ModuleResult(default(T), executionContext) - var defaultValue = Expression.Default(resultType); - var newResult = Expression.New(constructor, defaultValue, contextParam); - - // Cast to IModuleResult - var castToInterface = Expression.Convert(newResult, typeof(IModuleResult)); - - var lambda = Expression.Lambda(castToInterface, contextParam); - return lambda.Compile(); - } - - private static CreateExceptionResultDelegate CreateExceptionFactory(Type resultType) - { - var exceptionParam = Expression.Parameter(typeof(Exception), "exception"); - var contextParam = Expression.Parameter(typeof(ModuleExecutionContext), "executionContext"); - var resultGenericType = typeof(ModuleResult<>).MakeGenericType(resultType); - - // Find the internal constructor: ModuleResult(Exception exception, ModuleExecutionContext context) - // Note: The constructor takes the base class ModuleExecutionContext, not ModuleExecutionContext - var constructor = resultGenericType.GetConstructor( - BindingFlags.NonPublic | BindingFlags.Instance, - null, - new[] { typeof(Exception), typeof(ModuleExecutionContext) }, - null); - - if (constructor == null) + if (genericType == null) { - throw new InvalidOperationException( - $"Could not find internal constructor for ModuleResult<{resultType.Name}>(Exception, ModuleExecutionContext)"); + throw new InvalidOperationException($"Cannot determine generic type from {resultType}"); } - // Create: new ModuleResult(exception, executionContext) - var newResult = Expression.New(constructor, exceptionParam, contextParam); + var valueType = genericType.GetGenericArguments()[0]; + var method = typeof(ModuleResultFactory) + .GetMethod(nameof(WithStatusGeneric), System.Reflection.BindingFlags.NonPublic | System.Reflection.BindingFlags.Static)! + .MakeGenericMethod(valueType); - // Cast to IModuleResult - var castToInterface = Expression.Convert(newResult, typeof(IModuleResult)); + return (IModuleResult)method.Invoke(null, [result, status])!; + } - var lambda = Expression.Lambda(castToInterface, exceptionParam, contextParam); - return lambda.Compile(); + private static IModuleResult WithStatusGeneric(ModuleResult result, Status status) + { + return result with { ModuleStatus = status }; } } diff --git a/src/ModularPipelines/Engine/Executors/IgnoredModuleResultRegistrar.cs b/src/ModularPipelines/Engine/Executors/IgnoredModuleResultRegistrar.cs index a5e80b3e9a..be006766c3 100644 --- a/src/ModularPipelines/Engine/Executors/IgnoredModuleResultRegistrar.cs +++ b/src/ModularPipelines/Engine/Executors/IgnoredModuleResultRegistrar.cs @@ -49,15 +49,11 @@ public async Task RegisterIgnoredModuleResultsAsync(IReadOnlyList var historicalResult = await TryGetHistoricalResultAsync(module, resultType, pipelineContext).ConfigureAwait(false); if (historicalResult != null) { - // Update the status to UsedHistory since we're using a cached result - if (historicalResult is ModuleResult moduleResult) - { - moduleResult.ModuleStatus = Status.UsedHistory; - } - + // Update the status to UsedHistory using the factory method + var usedHistoryResult = ModuleResultFactory.WithStatus(historicalResult, Status.UsedHistory); _logger.LogDebug("Using historical result for ignored module {ModuleName}", MarkupFormatter.FormatModuleName(moduleType.Name)); - _resultRegistry.RegisterResult(moduleType, historicalResult); + _resultRegistry.RegisterResult(moduleType, usedHistoryResult); continue; } } diff --git a/src/ModularPipelines/Engine/ModuleExecutionPipeline.cs b/src/ModularPipelines/Engine/ModuleExecutionPipeline.cs index 7d4cf2faf4..5a924443f3 100644 --- a/src/ModularPipelines/Engine/ModuleExecutionPipeline.cs +++ b/src/ModularPipelines/Engine/ModuleExecutionPipeline.cs @@ -120,7 +120,7 @@ public async Task> ExecuteAsync( executionContext.RecordEndTime(); executionContext.Status = Status.Successful; - moduleResult = new ModuleResult(result, executionContext); + moduleResult = ModuleResult.CreateSuccess(result, executionContext); module.CompletionSource.TrySetResult(moduleResult!); @@ -141,7 +141,7 @@ public async Task> ExecuteAsync( await _directHookInvoker.InvokeFailedAsync(module, moduleContext, exception, executionContext.ModuleCancellationTokenSource.Token).ConfigureAwait(false); // Create the failed result for OnAfterExecuteAsync to receive - moduleResult = new ModuleResult(exception, executionContext); + moduleResult = ModuleResult.CreateFailure(exception, executionContext); module.CompletionSource.TrySetResult(moduleResult!); @@ -218,9 +218,7 @@ private async Task> HandleSkipped( executionContext.Status = Status.Skipped; executionContext.SkipResult = skipDecision; - module.CompletionSource.TrySetResult(new ModuleResult(new ModuleSkippedException(module.GetType().Name), executionContext)!); - - // Check if we should use historical data + // Check if we should use historical data BEFORE setting completion source // For skipped modules with a history repository configured, check for cached results if (_resultRepository.IsEnabled) { @@ -228,15 +226,19 @@ private async Task> HandleSkipped( if (historicalResult != null) { executionContext.Status = Status.UsedHistory; - historicalResult.ModuleStatus = Status.UsedHistory; - executionContext.SetTypedResult(historicalResult); + + // Create a new result with UsedHistory status using record's 'with' expression + var usedHistoryResult = historicalResult with { ModuleStatus = Status.UsedHistory }; + executionContext.SetTypedResult(usedHistoryResult); + module.CompletionSource.TrySetResult(usedHistoryResult); logger.LogDebug("Using historical result for skipped module"); - return historicalResult; + return usedHistoryResult; } } - var skippedResult = new SkippedModuleResult(executionContext, skipDecision); + var skippedResult = ModuleResult.CreateSkipped(skipDecision, executionContext); executionContext.SetTypedResult(skippedResult); + module.CompletionSource.TrySetResult(skippedResult); logger.LogInformation("Module {ModuleName} skipped: {Reason}", executionContext.ModuleType.Name, @@ -366,7 +368,7 @@ private async Task> HandleException( executionContext.Status = Status.PipelineTerminated; logger.LogInformation("Pipeline has been canceled"); - var cancelledResult = new ModuleResult(exception, executionContext); + var cancelledResult = ModuleResult.CreateFailure(exception, executionContext); executionContext.SetTypedResult(cancelledResult); return cancelledResult; } @@ -383,7 +385,7 @@ private async Task> HandleException( logger.LogDebug("Ignoring failures in this module and continuing..."); executionContext.Status = Status.IgnoredFailure; - var ignoredResult = new ModuleResult(exception, executionContext); + var ignoredResult = ModuleResult.CreateFailure(exception, executionContext); executionContext.SetTypedResult(ignoredResult); await SaveToHistory(module, ignoredResult, moduleContext).ConfigureAwait(false); @@ -392,10 +394,7 @@ private async Task> HandleException( } // Create a failed result before cancelling and throwing - // Use TimedOutModuleResult for timeout exceptions to expose detailed timeout information - ModuleResult failedResult = exception is ModuleTimeoutException timeoutEx - ? new TimedOutModuleResult(executionContext, timeoutEx) - : new ModuleResult(exception, executionContext); + ModuleResult failedResult = ModuleResult.CreateFailure(exception, executionContext); executionContext.SetTypedResult(failedResult); // Cancel the pipeline and propagate diff --git a/src/ModularPipelines/Exceptions/ModuleFailedException.cs b/src/ModularPipelines/Exceptions/ModuleFailedException.cs index ea310b488d..72401ee596 100644 --- a/src/ModularPipelines/Exceptions/ModuleFailedException.cs +++ b/src/ModularPipelines/Exceptions/ModuleFailedException.cs @@ -5,6 +5,7 @@ namespace ModularPipelines.Exceptions; /// /// An exception that occurs when a module execution fails. /// +[Obsolete("Use pattern matching on ModuleResult.Failure instead. The .Value property has been removed; use ValueOrDefault or Match() for safe access.")] public class ModuleFailedException : PipelineException { /// diff --git a/src/ModularPipelines/Exceptions/ModuleSkippedException.cs b/src/ModularPipelines/Exceptions/ModuleSkippedException.cs index b78d087c55..0dfacea8a9 100644 --- a/src/ModularPipelines/Exceptions/ModuleSkippedException.cs +++ b/src/ModularPipelines/Exceptions/ModuleSkippedException.cs @@ -1,5 +1,6 @@ namespace ModularPipelines.Exceptions; +[Obsolete("Use pattern matching on ModuleResult.Skipped instead. The .Value property has been removed; use ValueOrDefault or Match() for safe access.")] public class ModuleSkippedException : PipelineException { public string ModuleName { get; } diff --git a/src/ModularPipelines/Models/IModuleResult.cs b/src/ModularPipelines/Models/IModuleResult.cs index 8c351a5351..c8dcf34d22 100644 --- a/src/ModularPipelines/Models/IModuleResult.cs +++ b/src/ModularPipelines/Models/IModuleResult.cs @@ -1,46 +1,69 @@ -using ModularPipelines.Enums; +using ModularPipelines.Enums; namespace ModularPipelines.Models; +/// +/// Non-generic interface for type-erased module result access. +/// public interface IModuleResult { /// /// Gets the name of the module. /// - public string ModuleName { get; } + string ModuleName { get; } /// /// Gets how long the module ran for. /// - public TimeSpan ModuleDuration { get; } + TimeSpan ModuleDuration { get; } /// /// Gets when the module started. /// - public DateTimeOffset ModuleStart { get; } + DateTimeOffset ModuleStart { get; } /// /// Gets when the module ended. /// - public DateTimeOffset ModuleEnd { get; } + DateTimeOffset ModuleEnd { get; } /// - /// Gets the exception that occurred in the module, if one was thrown. + /// Gets the status of the module. /// - public Exception? Exception { get; } + Status ModuleStatus { get; } /// - /// Gets the Skip Decision of the module. + /// Gets the type of result that is held. /// - public SkipDecision SkipDecision { get; } + ModuleResultType ModuleResultType { get; } /// - /// Gets the type of result that is held. + /// Gets whether the result is a success. /// - public ModuleResultType ModuleResultType { get; } + bool IsSuccess { get; } /// - /// Gets the status of the module. + /// Gets whether the result is a failure. + /// + bool IsFailure { get; } + + /// + /// Gets whether the result was skipped. + /// + bool IsSkipped { get; } + + /// + /// Gets the value if successful, or null/default otherwise. Does not throw. + /// + object? ValueOrDefault { get; } + + /// + /// Gets the exception if failed, or null otherwise. + /// + Exception? ExceptionOrDefault { get; } + + /// + /// Gets the skip decision if skipped, or null otherwise. /// - public Status ModuleStatus { get; } -} \ No newline at end of file + SkipDecision? SkipDecisionOrDefault { get; } +} diff --git a/src/ModularPipelines/Models/ModuleResult.cs b/src/ModularPipelines/Models/ModuleResult.cs index e682f8db28..130be9358c 100644 --- a/src/ModularPipelines/Models/ModuleResult.cs +++ b/src/ModularPipelines/Models/ModuleResult.cs @@ -1,177 +1,522 @@ -using System.Diagnostics.CodeAnalysis; +using System.Text.Json; using System.Text.Json.Serialization; using ModularPipelines.Engine; using ModularPipelines.Enums; -using ModularPipelines.Exceptions; -using ModularPipelines.Modules; -using ModularPipelines.Serialization; namespace ModularPipelines.Models; -public class ModuleResult : ModuleResult +/// +/// Represents the result of a module execution as a discriminated union. +/// Use pattern matching to handle Success, Failure, and Skipped cases. +/// +/// The type of value returned on success. +/// +/// +/// var result = await myModule; +/// switch (result) +/// { +/// case ModuleResult<string>.Success { Value: var value }: +/// Console.WriteLine($"Got: {value}"); +/// break; +/// case ModuleResult<string>.Failure { Exception: var ex }: +/// Console.WriteLine($"Failed: {ex.Message}"); +/// break; +/// case ModuleResult<string>.Skipped { Decision: var skip }: +/// Console.WriteLine($"Skipped: {skip.Reason}"); +/// break; +/// } +/// +/// +[JsonConverter(typeof(ModuleResultJsonConverterFactory))] +public abstract record ModuleResult : IModuleResult { - private T? _value; + // === Metadata (available on all outcomes) === + + /// + [JsonInclude] + public required string ModuleName { get; init; } + + /// + [JsonInclude] + public required TimeSpan ModuleDuration { get; init; } + + /// + [JsonInclude] + public required DateTimeOffset ModuleStart { get; init; } + + /// + [JsonInclude] + public required DateTimeOffset ModuleEnd { get; init; } + + /// + [JsonInclude] + public required Status ModuleStatus { get; init; } + + // === Quick checks === + + /// + [JsonIgnore] + public bool IsSuccess => this is Success; + + /// + [JsonIgnore] + public bool IsFailure => this is Failure; + + /// + [JsonIgnore] + public bool IsSkipped => this is Skipped; + + // === Safe accessors (no exceptions) === /// - /// Initialises a new instance of the class. - /// Creates a result from execution context (composition-based modules). + /// Gets the value if successful, or default(T) otherwise. Does not throw. /// - internal ModuleResult(Exception exception, ModuleExecutionContext executionContext) - : base(exception, executionContext) + [JsonIgnore] + public T? ValueOrDefault => this is Success s ? s.Value : default; + + /// + [JsonIgnore] + object? IModuleResult.ValueOrDefault => ValueOrDefault; + + /// + [JsonIgnore] + public Exception? ExceptionOrDefault => this is Failure f ? f.Exception : null; + + /// + [JsonIgnore] + public SkipDecision? SkipDecisionOrDefault => this is Skipped sk ? sk.Decision : null; + + // === Computed for compatibility === + + /// + [JsonIgnore] + public ModuleResultType ModuleResultType => this switch { + Success => ModuleResultType.Success, + Failure => ModuleResultType.Failure, + Skipped => ModuleResultType.Skipped, + _ => throw new InvalidOperationException("Unknown result type") + }; + + // === Internal: Module type tracking === + + /// + /// Gets or sets the type of the module that produced this result. + /// + [JsonIgnore] + internal Type? ModuleType { get; init; } + + // === Pattern matching helpers === + + /// + /// Matches the result to one of three functions based on the outcome. + /// + /// The return type of the match functions. + /// Function to call if successful. + /// Function to call if failed. + /// Function to call if skipped. + /// The result of the matched function. + public TResult Match( + Func onSuccess, + Func onFailure, + Func onSkipped) => this switch + { + Success s => onSuccess(s.Value), + Failure f => onFailure(f.Exception), + Skipped sk => onSkipped(sk.Decision), + _ => throw new InvalidOperationException("Unknown result type") + }; + + /// + /// Executes one of three actions based on the outcome. + /// + /// Action to call if successful. + /// Action to call if failed. + /// Action to call if skipped. + public void Switch( + Action onSuccess, + Action onFailure, + Action onSkipped) + { + switch (this) + { + case Success s: + onSuccess(s.Value); + break; + case Failure f: + onFailure(f.Exception); + break; + case Skipped sk: + onSkipped(sk.Decision); + break; + } } + // === Discriminated variants === + /// - /// Initialises a new instance of the class. - /// Creates a result from execution context (composition-based modules). + /// Represents a successful module execution with a value. /// - internal ModuleResult(T? value, ModuleExecutionContext executionContext) - : base(executionContext) + /// The value produced by the module. + public sealed record Success(T Value) : ModuleResult; + + /// + /// Represents a failed module execution with an exception. + /// + /// The exception that caused the failure. + public sealed record Failure(Exception Exception) : ModuleResult; + + /// + /// Represents a skipped module execution. + /// + /// The skip decision containing the reason. + public sealed record Skipped(SkipDecision Decision) : ModuleResult; + + // === Internal factory methods === + + internal static Success CreateSuccess(T value, ModuleExecutionContext ctx) { - _value = value; + var (start, end, duration) = GetTimingInfo(ctx); + return new(value) + { + ModuleName = ctx.ModuleType.Name, + ModuleDuration = duration, + ModuleStart = start, + ModuleEnd = end, + ModuleStatus = ctx.Status, + ModuleType = ctx.ModuleType + }; } - [ExcludeFromCodeCoverage] - [JsonConstructor] - private ModuleResult() + internal static Failure CreateFailure(Exception exception, ModuleExecutionContext ctx) { + var (start, end, duration) = GetTimingInfo(ctx); + return new(exception) + { + ModuleName = ctx.ModuleType.Name, + ModuleDuration = duration, + ModuleStart = start, + ModuleEnd = end, + ModuleStatus = ctx.Status, + ModuleType = ctx.ModuleType + }; + } + + internal static Skipped CreateSkipped(SkipDecision decision, ModuleExecutionContext ctx) + { + var (start, end, duration) = GetTimingInfo(ctx); + return new(decision) + { + ModuleName = ctx.ModuleType.Name, + ModuleDuration = duration, + ModuleStart = start, + ModuleEnd = end, + ModuleStatus = ctx.Status, + ModuleType = ctx.ModuleType + }; } /// - /// Gets the value held in the result. + /// Gets consistent timing information from the execution context. + /// If either start or end time is MinValue, returns TimeSpan.Zero for duration + /// to avoid inconsistent results from calling DateTimeOffset.Now multiple times. /// - /// Thrown if the module failed. - /// Thrown if the module was skipped. - [JsonInclude] - public T? Value + private static (DateTimeOffset Start, DateTimeOffset End, TimeSpan Duration) GetTimingInfo(ModuleExecutionContext ctx) { - get + var now = DateTimeOffset.Now; + var start = ctx.StartTime == DateTimeOffset.MinValue ? now : ctx.StartTime; + var end = ctx.EndTime == DateTimeOffset.MinValue ? now : ctx.EndTime; + + // If either time was originally MinValue, duration is unreliable - use Zero + var duration = (ctx.StartTime == DateTimeOffset.MinValue || ctx.EndTime == DateTimeOffset.MinValue) + ? TimeSpan.Zero + : end - start; + + return (start, end, duration); + } + + // Prevent external inheritance - only Success, Failure, Skipped are valid + private protected ModuleResult() + { + } +} + +/// +/// JSON converter for Exception objects. Serializes essential exception data +/// and deserializes to a wrapper exception preserving the message. +/// +/// +/// Security Considerations: +/// +/// This converter intentionally serializes limited exception information: +/// +/// +/// +/// Type: Only the full type name (not AssemblyQualifiedName) is serialized +/// to avoid leaking internal assembly version and culture information. +/// +/// +/// Message: Exception messages may contain sensitive data (file paths, +/// user input, etc.). Consumers should sanitize exception messages before serialization +/// if they may contain sensitive information. +/// +/// +/// StackTrace: Stack traces may reveal internal file paths and code structure. +/// Consider whether this is acceptable for your use case. For production logging to external +/// systems, you may want to omit or truncate stack traces. +/// +/// +/// +/// On deserialization, only well-known exception types from the System namespace are +/// reconstructed. Unknown types fall back to a generic Exception to prevent type injection. +/// +/// +internal sealed class ExceptionJsonConverter : JsonConverter +{ + public override Exception? Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options) + { + if (reader.TokenType == JsonTokenType.Null) { - if (ModuleResultType == ModuleResultType.Failure) + return null; + } + + string? typeName = null; + string? message = null; + string? stackTrace = null; + + while (reader.Read()) + { + if (reader.TokenType == JsonTokenType.EndObject) { - throw new ModuleFailedException(ModuleType!, Exception!); + break; } - if (ModuleResultType == ModuleResultType.Skipped) + if (reader.TokenType == JsonTokenType.PropertyName) { - throw new ModuleSkippedException(ModuleName); - } + var propertyName = reader.GetString(); + reader.Read(); - return _value; + switch (propertyName) + { + case "Type": + typeName = reader.GetString(); + break; + case "Message": + message = reader.GetString(); + break; + case "StackTrace": + stackTrace = reader.GetString(); + break; + } + } } - private set + // Try to reconstruct the original exception type if possible + // Security: Only allow well-known exception types from System namespace + if (typeName != null) { - _value = value; + var exceptionType = Type.GetType(typeName); + if (exceptionType != null && + typeof(Exception).IsAssignableFrom(exceptionType) && + (exceptionType.Namespace?.StartsWith("System", StringComparison.Ordinal) == true)) + { + try + { + var ex = Activator.CreateInstance(exceptionType, message) as Exception; + if (ex != null) + { + return ex; + } + } + catch + { + // Fall through to default + } + } } + + // NOTE: StackTrace is intentionally not restored during deserialization. + // Setting the StackTrace property via reflection is fragile and can cause issues. + // The original stack trace is preserved in the JSON for diagnostic purposes, + // but deserialized exceptions will have a new stack trace from deserialization. + return new Exception(message ?? "Deserialized exception"); } - /// - /// Gets a value indicating whether gets whether the result holds a value. - /// - public bool HasValue => !EqualityComparer.Default.Equals(_value, default); + public override void Write(Utf8JsonWriter writer, Exception value, JsonSerializerOptions options) + { + writer.WriteStartObject(); + // Security: Use FullName instead of AssemblyQualifiedName to avoid leaking + // assembly version, culture, and public key token information + writer.WriteString("Type", value.GetType().FullName); + writer.WriteString("Message", value.Message); + writer.WriteString("StackTrace", value.StackTrace); + writer.WriteEndObject(); + } } -[JsonConverter(typeof(TypeDiscriminatorConverter))] -public class ModuleResult : IModuleResult, ITypeDiscriminator +/// +/// JSON converter factory that creates typed converters for ModuleResult<T>. +/// +internal sealed class ModuleResultJsonConverterFactory : JsonConverterFactory { - /// - [JsonInclude] - public string ModuleName { get; private set; } - - /// - [JsonInclude] - public TimeSpan ModuleDuration { get; private set; } - - /// - [JsonInclude] - public DateTimeOffset ModuleStart { get; private set; } - - /// - [JsonInclude] - public DateTimeOffset ModuleEnd { get; private set; } - - /// - /// Initialises a new instance of the class. - /// Creates a result from execution context (composition-based modules). - /// - internal ModuleResult(Exception exception, ModuleExecutionContext executionContext) - : this(executionContext) + public override bool CanConvert(Type typeToConvert) { - Exception = exception; + return typeToConvert.IsGenericType && + typeToConvert.GetGenericTypeDefinition() == typeof(ModuleResult<>); } - /// - /// Initialises a new instance of the class. - /// Creates a result from execution context (composition-based modules). - /// - internal ModuleResult(ModuleExecutionContext executionContext) + public override JsonConverter? CreateConverter(Type typeToConvert, JsonSerializerOptions options) { - ModuleName = executionContext.ModuleType.Name; - ModuleType = executionContext.ModuleType; - ModuleStart = executionContext.StartTime == DateTimeOffset.MinValue ? DateTimeOffset.Now : executionContext.StartTime; - ModuleEnd = executionContext.EndTime == DateTimeOffset.MinValue ? DateTimeOffset.Now : executionContext.EndTime; - ModuleDuration = ModuleEnd - ModuleStart; - SkipDecision = executionContext.SkipResult; - TypeDiscriminator = GetType().FullName!; - ModuleStatus = executionContext.Status; + var valueType = typeToConvert.GetGenericArguments()[0]; + var converterType = typeof(ModuleResultJsonConverter<>).MakeGenericType(valueType); + return (JsonConverter?)Activator.CreateInstance(converterType); } +} - /// - /// Initialises a new instance of the class. - /// - [ExcludeFromCodeCoverage] - [JsonConstructor] - protected ModuleResult() +/// +/// JSON converter for ModuleResult<T> that handles polymorphic serialization/deserialization. +/// +internal sealed class ModuleResultJsonConverter : JsonConverter> +{ + private static readonly ExceptionJsonConverter ExceptionConverter = new(); + + public override ModuleResult? Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options) { - // Late initialisation via deserialisation - ModuleName = null!; - ModuleDuration = TimeSpan.Zero!; - ModuleStart = DateTimeOffset.MinValue!; - ModuleEnd = DateTimeOffset.MinValue; - SkipDecision = null!; - TypeDiscriminator = GetType().FullName!; - } + if (reader.TokenType == JsonTokenType.Null) + { + return null; + } - /// - [JsonInclude] - public Exception? Exception { get; private set; } + string? discriminator = null; + string? moduleName = null; + TimeSpan moduleDuration = TimeSpan.Zero; + DateTimeOffset moduleStart = DateTimeOffset.MinValue; + DateTimeOffset moduleEnd = DateTimeOffset.MinValue; + Status moduleStatus = Status.NotYetStarted; + T? value = default; + Exception? exception = null; + SkipDecision? skipDecision = null; - /// - [JsonInclude] - public SkipDecision SkipDecision { get; private protected set; } + if (reader.TokenType != JsonTokenType.StartObject) + { + throw new JsonException("Expected StartObject token"); + } - /// - public ModuleResultType ModuleResultType - { - get + while (reader.Read()) { - if (Exception != null) + if (reader.TokenType == JsonTokenType.EndObject) { - return ModuleResultType.Failure; + break; } - if (SkipDecision?.ShouldSkip == true) + if (reader.TokenType == JsonTokenType.PropertyName) { - return ModuleResultType.Skipped; - } + var propertyName = reader.GetString(); + reader.Read(); - return ModuleResultType.Success; + switch (propertyName) + { + case "$type": + discriminator = reader.GetString(); + break; + case "ModuleName": + moduleName = reader.GetString(); + break; + case "ModuleDuration": + moduleDuration = JsonSerializer.Deserialize(ref reader, options); + break; + case "ModuleStart": + moduleStart = reader.GetDateTimeOffset(); + break; + case "ModuleEnd": + moduleEnd = reader.GetDateTimeOffset(); + break; + case "ModuleStatus": + moduleStatus = JsonSerializer.Deserialize(ref reader, options); + break; + case "Value": + value = JsonSerializer.Deserialize(ref reader, options); + break; + case "Exception": + exception = ExceptionConverter.Read(ref reader, typeof(Exception), options); + break; + case "Decision": + skipDecision = JsonSerializer.Deserialize(ref reader, options); + break; + } + } } + + return discriminator switch + { + "Success" => new ModuleResult.Success(value!) + { + ModuleName = moduleName!, + ModuleDuration = moduleDuration, + ModuleStart = moduleStart, + ModuleEnd = moduleEnd, + ModuleStatus = moduleStatus + }, + "Failure" => new ModuleResult.Failure(exception!) + { + ModuleName = moduleName!, + ModuleDuration = moduleDuration, + ModuleStart = moduleStart, + ModuleEnd = moduleEnd, + ModuleStatus = moduleStatus + }, + "Skipped" => new ModuleResult.Skipped(skipDecision!) + { + ModuleName = moduleName!, + ModuleDuration = moduleDuration, + ModuleStart = moduleStart, + ModuleEnd = moduleEnd, + ModuleStatus = moduleStatus + }, + _ => throw new JsonException($"Unknown discriminator: {discriminator}") + }; } - /// - public Status ModuleStatus { get; internal set; } + public override void Write(Utf8JsonWriter writer, ModuleResult value, JsonSerializerOptions options) + { + writer.WriteStartObject(); - /// - /// Gets the type information used to aid in serialization. - /// - [JsonInclude] - public string TypeDiscriminator { get; private set; } + // Write discriminator + var discriminator = value switch + { + ModuleResult.Success => "Success", + ModuleResult.Failure => "Failure", + ModuleResult.Skipped => "Skipped", + _ => throw new JsonException("Unknown ModuleResult type") + }; + writer.WriteString("$type", discriminator); - /// - /// Gets or sets the type of the module that produced this result. - /// - [JsonIgnore] - internal Type? ModuleType { get; set; } + // Write common properties + writer.WriteString("ModuleName", value.ModuleName); + writer.WritePropertyName("ModuleDuration"); + JsonSerializer.Serialize(writer, value.ModuleDuration, options); + writer.WriteString("ModuleStart", value.ModuleStart); + writer.WriteString("ModuleEnd", value.ModuleEnd); + writer.WritePropertyName("ModuleStatus"); + JsonSerializer.Serialize(writer, value.ModuleStatus, options); + + // Write variant-specific properties + switch (value) + { + case ModuleResult.Success success: + writer.WritePropertyName("Value"); + JsonSerializer.Serialize(writer, success.Value, options); + break; + case ModuleResult.Failure failure: + writer.WritePropertyName("Exception"); + ExceptionConverter.Write(writer, failure.Exception, options); + break; + case ModuleResult.Skipped skipped: + writer.WritePropertyName("Decision"); + JsonSerializer.Serialize(writer, skipped.Decision, options); + break; + } + + writer.WriteEndObject(); + } } diff --git a/src/ModularPipelines/Models/PipelineSummary.cs b/src/ModularPipelines/Models/PipelineSummary.cs index 7cc0922893..f3b2d32ede 100644 --- a/src/ModularPipelines/Models/PipelineSummary.cs +++ b/src/ModularPipelines/Models/PipelineSummary.cs @@ -129,7 +129,7 @@ public async Task> GetModuleResultsAsync() } // Fallback: create a cancellation result for modules that haven't completed - return (IModuleResult) new ModuleResult(new TaskCanceledException(), new ModuleExecutionContext(x, x.GetType())); + return ModuleResult.CreateFailure(new TaskCanceledException(), new ModuleExecutionContext(x, x.GetType())); }).ProcessInParallel(); } diff --git a/src/ModularPipelines/Models/SkippedModuleResult.cs b/src/ModularPipelines/Models/SkippedModuleResult.cs deleted file mode 100644 index 2323c178ab..0000000000 --- a/src/ModularPipelines/Models/SkippedModuleResult.cs +++ /dev/null @@ -1,12 +0,0 @@ -using ModularPipelines.Engine; - -namespace ModularPipelines.Models; - -internal class SkippedModuleResult : ModuleResult -{ - internal SkippedModuleResult(ModuleExecutionContext executionContext, SkipDecision skipDecision) - : base(default(T?), executionContext) - { - SkipDecision = skipDecision; - } -} \ No newline at end of file diff --git a/src/ModularPipelines/Models/TimedOutModuleResult.cs b/src/ModularPipelines/Models/TimedOutModuleResult.cs deleted file mode 100644 index 77a812047b..0000000000 --- a/src/ModularPipelines/Models/TimedOutModuleResult.cs +++ /dev/null @@ -1,53 +0,0 @@ -using ModularPipelines.Engine; -using ModularPipelines.Exceptions; - -namespace ModularPipelines.Models; - -/// -/// Represents the result of a module that timed out during execution. -/// -/// The type of result the module would have returned. -/// -/// This result type provides detailed information about the timeout, including: -/// -/// The configured timeout duration -/// How long the module actually ran before timeout -/// Whether the module respected the cancellation token -/// -/// -public class TimedOutModuleResult : ModuleResult -{ - /// - /// Initializes a new instance of the class. - /// - /// The execution context of the timed out module. - /// The timeout exception that occurred. - internal TimedOutModuleResult( - ModuleExecutionContext executionContext, - ModuleTimeoutException timeoutException) - : base(timeoutException, executionContext) - { - ConfiguredTimeout = timeoutException.ConfiguredTimeout; - ElapsedTime = timeoutException.ElapsedTime; - WasCancellationTokenRespected = timeoutException.WasCancellationTokenRespected; - } - - /// - /// Gets the timeout duration that was configured for the module. - /// - public TimeSpan ConfiguredTimeout { get; } - - /// - /// Gets the actual time elapsed before the timeout was enforced. - /// - public TimeSpan ElapsedTime { get; } - - /// - /// Gets a value indicating whether the module properly observed the cancellation token. - /// - /// - /// If false, the module did not respond to the cancellation token and was - /// forcibly terminated by the timeout enforcement mechanism. - /// - public bool WasCancellationTokenRespected { get; } -} diff --git a/test/ModularPipelines.Azure.UnitTests/AzureCommandTests.cs b/test/ModularPipelines.Azure.UnitTests/AzureCommandTests.cs index 01fb7a6669..090cfc75ef 100644 --- a/test/ModularPipelines.Azure.UnitTests/AzureCommandTests.cs +++ b/test/ModularPipelines.Azure.UnitTests/AzureCommandTests.cs @@ -35,7 +35,7 @@ public async Task Azure_Command_Is_Expected_Command() { var result = await await RunModule(); - await Assert.That(result.Value!.CommandInput) + await Assert.That(result.ValueOrDefault!.CommandInput) .IsEqualTo("az account list --all"); } @@ -43,7 +43,7 @@ await Assert.That(result.Value!.CommandInput) public async Task Azure_Command_With_Sub_Command_Group_Is_Expected_Command() { var result = await await RunModule(); - await Assert.That(result.Value!.CommandInput) + await Assert.That(result.ValueOrDefault!.CommandInput) .IsEqualTo("az account management-group list"); } } \ No newline at end of file diff --git a/test/ModularPipelines.TestHelpers/Assertions/ModuleResultAssertions.cs b/test/ModularPipelines.TestHelpers/Assertions/ModuleResultAssertions.cs index dfa2c609f4..3b27148576 100644 --- a/test/ModularPipelines.TestHelpers/Assertions/ModuleResultAssertions.cs +++ b/test/ModularPipelines.TestHelpers/Assertions/ModuleResultAssertions.cs @@ -22,8 +22,8 @@ public static async Task AssertSuccessWithValue(ModuleResult moduleResult using (Assert.Multiple()) { await Assert.That(moduleResult.ModuleResultType).IsEqualTo(ModuleResultType.Success); - await Assert.That(moduleResult.Exception).IsNull(); - await Assert.That(moduleResult.Value).IsNotNull(); + await Assert.That(moduleResult.ExceptionOrDefault).IsNull(); + await Assert.That(moduleResult.ValueOrDefault).IsNotNull(); } } @@ -41,7 +41,7 @@ public static async Task AssertSuccess(ModuleResult moduleResult) using (Assert.Multiple()) { await Assert.That(moduleResult.ModuleResultType).IsEqualTo(ModuleResultType.Success); - await Assert.That(moduleResult.Exception).IsNull(); + await Assert.That(moduleResult.ExceptionOrDefault).IsNull(); } } @@ -55,8 +55,8 @@ public static async Task AssertCommandOutput(ModuleResult module { using (Assert.Multiple()) { - await Assert.That(moduleResult.Value!.StandardError).IsNull().Or.IsEmpty(); - await Assert.That(moduleResult.Value.StandardOutput.Trim()).IsEqualTo(expectedOutput); + await Assert.That(moduleResult.ValueOrDefault!.StandardError).IsNull().Or.IsEmpty(); + await Assert.That(moduleResult.ValueOrDefault.StandardOutput.Trim()).IsEqualTo(expectedOutput); } } } diff --git a/test/ModularPipelines.UnitTests/Attributes/LifecycleEventIntegrationTests.cs b/test/ModularPipelines.UnitTests/Attributes/LifecycleEventIntegrationTests.cs index a6e1386368..ccdb8d3204 100644 --- a/test/ModularPipelines.UnitTests/Attributes/LifecycleEventIntegrationTests.cs +++ b/test/ModularPipelines.UnitTests/Attributes/LifecycleEventIntegrationTests.cs @@ -25,7 +25,7 @@ public Task OnModuleStartAsync(IModuleEventContext context) public class LogEndAttribute : Attribute, IModuleEndEventReceiver { - public Task OnModuleEndAsync(IModuleEventContext context, ModuleResult result) + public Task OnModuleEndAsync(IModuleEventContext context, IModuleResult result) { EventLog.Add($"End:{context.ModuleName}"); return Task.CompletedTask; diff --git a/test/ModularPipelines.UnitTests/Attributes/MetadataCrossPhaseIntegrationTests.cs b/test/ModularPipelines.UnitTests/Attributes/MetadataCrossPhaseIntegrationTests.cs index 90b3aa4df2..67a579489b 100644 --- a/test/ModularPipelines.UnitTests/Attributes/MetadataCrossPhaseIntegrationTests.cs +++ b/test/ModularPipelines.UnitTests/Attributes/MetadataCrossPhaseIntegrationTests.cs @@ -56,7 +56,7 @@ public ReadMetadataOnEndAttribute(string key) _key = key; } - public Task OnModuleEndAsync(IModuleEventContext context, ModuleResult result) + public Task OnModuleEndAsync(IModuleEventContext context, IModuleResult result) { var value = context.GetMetadata(_key); EventLog.Add($"End:ReadMetadata:{_key}={value ?? "null"}"); diff --git a/test/ModularPipelines.UnitTests/ComposableModuleTests.cs b/test/ModularPipelines.UnitTests/ComposableModuleTests.cs index 5875b4de42..d83edebe18 100644 --- a/test/ModularPipelines.UnitTests/ComposableModuleTests.cs +++ b/test/ModularPipelines.UnitTests/ComposableModuleTests.cs @@ -129,8 +129,8 @@ public async Task Skippable_Module_Is_Skipped_When_Condition_True() var resultRegistry = host.RootServices.GetRequiredService(); var moduleResult = resultRegistry.GetResult(typeof(AlwaysSkippedModule))!; - await Assert.That(moduleResult.SkipDecision.ShouldSkip).IsTrue(); - await Assert.That(moduleResult.SkipDecision.Reason).IsEqualTo("Skipped via composition"); + await Assert.That(moduleResult.SkipDecisionOrDefault!.ShouldSkip).IsTrue(); + await Assert.That(moduleResult.SkipDecisionOrDefault.Reason).IsEqualTo("Skipped via composition"); } [Test] @@ -144,7 +144,7 @@ public async Task Skippable_Module_Executes_When_Condition_False() var resultRegistry = host.RootServices.GetRequiredService(); var moduleResult = resultRegistry.GetResult(typeof(NeverSkippedModule))!; - await Assert.That(moduleResult.SkipDecision.ShouldSkip).IsFalse(); + await Assert.That(moduleResult.SkipDecisionOrDefault?.ShouldSkip ?? false).IsFalse(); } [Test] diff --git a/test/ModularPipelines.UnitTests/Helpers/CommandTests.cs b/test/ModularPipelines.UnitTests/Helpers/CommandTests.cs index e7c0f98b15..3f814440a5 100644 --- a/test/ModularPipelines.UnitTests/Helpers/CommandTests.cs +++ b/test/ModularPipelines.UnitTests/Helpers/CommandTests.cs @@ -54,6 +54,6 @@ public async Task Standard_Output_Equals_Foo_Bar_With_Timeout() { var moduleResult = await await RunModule(); - await Assert.That(moduleResult.Value!.Trim()).IsEqualTo(TestConstants.TestString); + await Assert.That(moduleResult.ValueOrDefault!.Trim()).IsEqualTo(TestConstants.TestString); } } diff --git a/test/ModularPipelines.UnitTests/Helpers/DockerTests.cs b/test/ModularPipelines.UnitTests/Helpers/DockerTests.cs index 4b926835d3..c35235dcf2 100644 --- a/test/ModularPipelines.UnitTests/Helpers/DockerTests.cs +++ b/test/ModularPipelines.UnitTests/Helpers/DockerTests.cs @@ -63,6 +63,6 @@ public async Task DockerBuild_CorrectInputCommand() .GetFolder("MyApp") .GetFile("Dockerfile").Path; - await Assert.That(result.Value!.CommandInput).IsEqualTo($"docker image build --build-arg=Arg1=Value1 --build-arg=Arg2=Value2 --build-arg=Arg3=Value3 --tag=mytaggedimage --target=build-env {dockerfilePath}"); + await Assert.That(result.ValueOrDefault!.CommandInput).IsEqualTo($"docker image build --build-arg=Arg1=Value1 --build-arg=Arg2=Value2 --build-arg=Arg3=Value3 --tag=mytaggedimage --target=build-env {dockerfilePath}"); } } diff --git a/test/ModularPipelines.UnitTests/Helpers/EncodingTests.cs b/test/ModularPipelines.UnitTests/Helpers/EncodingTests.cs index 6eebfb2d83..a59a8e1926 100644 --- a/test/ModularPipelines.UnitTests/Helpers/EncodingTests.cs +++ b/test/ModularPipelines.UnitTests/Helpers/EncodingTests.cs @@ -65,7 +65,7 @@ public async Task To_Base64_Works_Correctly() var moduleResult = await await RunModule(); await ModuleResultAssertions.AssertSuccessWithValue(moduleResult); - await Assert.That(moduleResult.Value).IsEqualTo("Rm9vIGJhciE="); + await Assert.That(moduleResult.ValueOrDefault).IsEqualTo("Rm9vIGJhciE="); } [Test] @@ -75,7 +75,7 @@ public async Task From_Base64_Works_Correctly() var moduleResult = await await RunModule(); await ModuleResultAssertions.AssertSuccessWithValue(moduleResult); - await Assert.That(moduleResult.Value).IsEqualTo(TestInput); + await Assert.That(moduleResult.ValueOrDefault).IsEqualTo(TestInput); } [Test] @@ -85,7 +85,7 @@ public async Task To_Hex_Works_Correctly() var moduleResult = await await RunModule(); await ModuleResultAssertions.AssertSuccessWithValue(moduleResult); - await Assert.That(moduleResult.Value).IsEqualTo("466f6f2062617221"); + await Assert.That(moduleResult.ValueOrDefault).IsEqualTo("466f6f2062617221"); } [Test] @@ -95,7 +95,7 @@ public async Task From_Hex_Works_Correctly() var moduleResult = await await RunModule(); await ModuleResultAssertions.AssertSuccessWithValue(moduleResult); - await Assert.That(moduleResult.Value).IsEqualTo(TestInput); + await Assert.That(moduleResult.ValueOrDefault).IsEqualTo(TestInput); } #endregion diff --git a/test/ModularPipelines.UnitTests/Helpers/GitHubRepositoryInfoTests.cs b/test/ModularPipelines.UnitTests/Helpers/GitHubRepositoryInfoTests.cs index 49d0c3409c..1c5cf4ab56 100644 --- a/test/ModularPipelines.UnitTests/Helpers/GitHubRepositoryInfoTests.cs +++ b/test/ModularPipelines.UnitTests/Helpers/GitHubRepositoryInfoTests.cs @@ -22,7 +22,7 @@ public async Task GitHub_Repository_Information_Is_Populated() { var moduleResult = await await RunModule(); - var gitHubRepositoryInfo = moduleResult.Value!; + var gitHubRepositoryInfo = moduleResult.ValueOrDefault!; using (Assert.Multiple()) { diff --git a/test/ModularPipelines.UnitTests/Helpers/GitTests.cs b/test/ModularPipelines.UnitTests/Helpers/GitTests.cs index c1401422ae..a4c5e1ad69 100644 --- a/test/ModularPipelines.UnitTests/Helpers/GitTests.cs +++ b/test/ModularPipelines.UnitTests/Helpers/GitTests.cs @@ -37,8 +37,8 @@ public async Task Standard_Output_Starts_With_Git_Version() using (Assert.Multiple()) { - await Assert.That(moduleResult.Value!.StandardError).IsNull().Or.IsEmpty(); - await Assert.That(moduleResult.Value.StandardOutput).Matches(@"git version \d+.*"); + await Assert.That(moduleResult.ValueOrDefault!.StandardError).IsNull().Or.IsEmpty(); + await Assert.That(moduleResult.ValueOrDefault.StandardOutput).Matches(@"git version \d+.*"); } } diff --git a/test/ModularPipelines.UnitTests/Helpers/NodeTests.cs b/test/ModularPipelines.UnitTests/Helpers/NodeTests.cs index 8694f9878d..fa7efebc3f 100644 --- a/test/ModularPipelines.UnitTests/Helpers/NodeTests.cs +++ b/test/ModularPipelines.UnitTests/Helpers/NodeTests.cs @@ -32,8 +32,8 @@ public async Task Standard_Output_Is_Version_Number() using (Assert.Multiple()) { - await Assert.That(moduleResult.Value!.StandardError).IsNull().Or.IsEmpty(); - await Assert.That(moduleResult.Value.StandardOutput).Matches(@"v\d+"); + await Assert.That(moduleResult.ValueOrDefault!.StandardError).IsNull().Or.IsEmpty(); + await Assert.That(moduleResult.ValueOrDefault.StandardOutput).Matches(@"v\d+"); } } } diff --git a/test/ModularPipelines.UnitTests/Hooks/DirectModuleHooksTests.cs b/test/ModularPipelines.UnitTests/Hooks/DirectModuleHooksTests.cs index f2d274e84c..b751edcb9a 100644 --- a/test/ModularPipelines.UnitTests/Hooks/DirectModuleHooksTests.cs +++ b/test/ModularPipelines.UnitTests/Hooks/DirectModuleHooksTests.cs @@ -299,8 +299,8 @@ public async Task OnAfterExecuteAsync_CalledWhenModuleFails() // The result passed to OnAfterExecuteAsync should contain the exception await Assert.That(module.ReceivedAfterResult).IsNotNull(); - await Assert.That(module.ReceivedAfterResult!.Exception).IsNotNull(); - await Assert.That(module.ReceivedAfterResult.Exception).IsTypeOf(); + await Assert.That(module.ReceivedAfterResult!.ExceptionOrDefault).IsNotNull(); + await Assert.That(module.ReceivedAfterResult.ExceptionOrDefault).IsTypeOf(); } [Test] @@ -341,7 +341,7 @@ public async Task OnAfterExecuteAsync_ExceptionLogged_ResultPreserved() // Module should still succeed despite after hook throwing await Assert.That(moduleResult).IsNotNull(); await Assert.That(moduleResult!.ModuleResultType).IsEqualTo(ModuleResultType.Success); - await Assert.That(moduleResult.Value).IsEqualTo("Success"); + await Assert.That(moduleResult.ValueOrDefault).IsEqualTo("Success"); } [Test] diff --git a/test/ModularPipelines.UnitTests/IgnoredFailureTests.cs b/test/ModularPipelines.UnitTests/IgnoredFailureTests.cs index db5b3efd33..d1b5f425e3 100644 --- a/test/ModularPipelines.UnitTests/IgnoredFailureTests.cs +++ b/test/ModularPipelines.UnitTests/IgnoredFailureTests.cs @@ -36,7 +36,7 @@ public async Task Has_Not_Thrown_Or_Cancelled_Pipeline() using (Assert.Multiple()) { await Assert.That(moduleResult.ModuleResultType).IsEqualTo(ModuleResultType.Failure); - await Assert.That(moduleResult.Exception).IsNotNull(); + await Assert.That(moduleResult.ExceptionOrDefault).IsNotNull(); await Assert.That(engineCancellationToken.IsCancellationRequested).IsFalse(); } } diff --git a/test/ModularPipelines.UnitTests/JsonSerializationTests.cs b/test/ModularPipelines.UnitTests/JsonSerializationTests.cs index 7dc7227d7d..8fce8a3374 100644 --- a/test/ModularPipelines.UnitTests/JsonSerializationTests.cs +++ b/test/ModularPipelines.UnitTests/JsonSerializationTests.cs @@ -83,8 +83,8 @@ public async Task Test1() // Verify the module result values using (Assert.Multiple()) { - await Assert.That(module1Result.Value!["Foo"].ToString()).IsEqualTo("Bar"); - await Assert.That(module1Result.Value!["Hello"].ToString()).IsEqualTo("world!"); + await Assert.That(module1Result.ValueOrDefault!["Foo"].ToString()).IsEqualTo("Bar"); + await Assert.That(module1Result.ValueOrDefault!["Hello"].ToString()).IsEqualTo("world!"); await Assert.That(module1Result.ModuleName).IsEqualTo(typeof(Module1).Name); } } diff --git a/test/ModularPipelines.UnitTests/ModuleHistoryTests.cs b/test/ModularPipelines.UnitTests/ModuleHistoryTests.cs index e93258200f..c315c751ba 100644 --- a/test/ModularPipelines.UnitTests/ModuleHistoryTests.cs +++ b/test/ModularPipelines.UnitTests/ModuleHistoryTests.cs @@ -80,7 +80,7 @@ public Task SaveResultAsync(Module module, ModuleResult moduleResult, I { // Create a result using the module execution context var executionContext = new ModuleExecutionContext(module, module.GetType()); - return Task.FromResult?>(new ModuleResult(default(T?), executionContext)); + return Task.FromResult?>(ModuleResult.CreateSuccess(default!, executionContext)); } } diff --git a/test/ModularPipelines.UnitTests/RetryTests.cs b/test/ModularPipelines.UnitTests/RetryTests.cs index 9eadbbe47f..bab8a60be0 100644 --- a/test/ModularPipelines.UnitTests/RetryTests.cs +++ b/test/ModularPipelines.UnitTests/RetryTests.cs @@ -127,7 +127,7 @@ public async Task When_Successful_Do_Not_Retry() using (Assert.Multiple()) { await Assert.That(module.ExecutionCount).IsEqualTo(ExpectedSingleExecutionCount); - await Assert.That(result.Exception).IsNull(); + await Assert.That(result.ExceptionOrDefault).IsNull(); } } @@ -151,7 +151,7 @@ public async Task When_Error_Then_Retry() using (Assert.Multiple()) { await Assert.That(module.ExecutionCount).IsEqualTo(ExpectedExecutionCountAfterRetries); - await Assert.That(result.Exception).IsNull(); + await Assert.That(result.ExceptionOrDefault).IsNull(); } } @@ -171,7 +171,7 @@ public async Task When_Error_With_Custom_RetryPolicy_Then_Retry() using (Assert.Multiple()) { await Assert.That(module.ExecutionCount).IsEqualTo(ExpectedExecutionCountAfterRetries); - await Assert.That(result.Exception).IsNull(); + await Assert.That(result.ExceptionOrDefault).IsNull(); } } @@ -195,7 +195,7 @@ public async Task When_Error_And_Zero_Retry_Count_Then_Do_Not_Retry() using (Assert.Multiple()) { await Assert.That(module.ExecutionCount).IsEqualTo(ExpectedSingleExecutionCount); - await Assert.That(result.Exception).IsNotNull(); + await Assert.That(result.ExceptionOrDefault).IsNotNull(); } } diff --git a/test/ModularPipelines.UnitTests/ReturnNothingTests.cs b/test/ModularPipelines.UnitTests/ReturnNothingTests.cs index 8506049bd2..3dfbe8350a 100644 --- a/test/ModularPipelines.UnitTests/ReturnNothingTests.cs +++ b/test/ModularPipelines.UnitTests/ReturnNothingTests.cs @@ -44,14 +44,14 @@ public async Task Module3_HasValue_False() await Assert(result); } - private static async Task Assert(ModuleResult result) + private static async Task Assert(ModuleResult result) { using (TUnit.Assertions.Assert.Multiple()) { - await TUnit.Assertions.Assert.That(result.HasValue).IsFalse(); + await TUnit.Assertions.Assert.That(result.IsSuccess).IsTrue(); await TUnit.Assertions.Assert.That(result.ModuleResultType).IsEqualTo(ModuleResultType.Success); - await TUnit.Assertions.Assert.That(result.Value).IsNull(); - await TUnit.Assertions.Assert.That(result.Exception).IsNull(); + await TUnit.Assertions.Assert.That(result.ValueOrDefault).IsNull(); + await TUnit.Assertions.Assert.That(result.ExceptionOrDefault).IsNull(); } } } diff --git a/test/ModularPipelines.UnitTests/SkippedModuleTests.cs b/test/ModularPipelines.UnitTests/SkippedModuleTests.cs index a3a88269aa..470d8edbd0 100644 --- a/test/ModularPipelines.UnitTests/SkippedModuleTests.cs +++ b/test/ModularPipelines.UnitTests/SkippedModuleTests.cs @@ -33,7 +33,7 @@ public async Task Skipped_Result_Is_As_Expected() using (Assert.Multiple()) { await Assert.That(moduleResult.ModuleResultType).IsEqualTo(ModuleResultType.Skipped); - await Assert.That(moduleResult.Exception).IsNull(); + await Assert.That(moduleResult.ExceptionOrDefault).IsNull(); } } }