Ensure failed results always have a Problem

ManagedCode.Communication.AspNetCore/WebApi/Extensions/ControllerExtensions.cs

@@ -10,7 +10,7 @@ public static IActionResult ToActionResult<T>(this Result<T> result)
         if (result.IsSuccess)
             return new OkObjectResult(result.Value);
 
-        var problem = result.Problem ?? Problem.Create("Operation failed", "Unknown error occurred", 500);
+        var problem = result.GetProblemNoFallback() ?? Problem.Create("Operation failed", "Unknown error occurred", 500);
         return new ObjectResult(problem)
         {
             StatusCode = problem.StatusCode
@@ -22,7 +22,7 @@ public static IActionResult ToActionResult(this Result result)
         if (result.IsSuccess)
             return new NoContentResult();
 
-        var problem = result.Problem ?? Problem.Create("Operation failed", "Unknown error occurred", 500);
+        var problem = result.GetProblemNoFallback() ?? Problem.Create("Operation failed", "Unknown error occurred", 500);
         return new ObjectResult(problem)
         {
             StatusCode = problem.StatusCode
@@ -34,7 +34,7 @@ public static Microsoft.AspNetCore.Http.IResult ToHttpResult<T>(this Result<T> r
         if (result.IsSuccess)
             return Results.Ok(result.Value);
 
-        var problem = result.Problem ?? Problem.Create("Operation failed", "Unknown error occurred", 500);
+        var problem = result.GetProblemNoFallback() ?? Problem.Create("Operation failed", "Unknown error occurred", 500);
         return Results.Problem(
             title: problem.Title,
             detail: problem.Detail,
@@ -50,7 +50,7 @@ public static Microsoft.AspNetCore.Http.IResult ToHttpResult(this Result result)
         if (result.IsSuccess)
             return Results.NoContent();
 
-        var problem = result.Problem ?? Problem.Create("Operation failed", "Unknown error occurred", 500);
+        var problem = result.GetProblemNoFallback() ?? Problem.Create("Operation failed", "Unknown error occurred", 500);
         return Results.Problem(
             title: problem.Title,
             detail: problem.Detail,

ManagedCode.Communication.Orleans/Converters/CollectionResultTSurrogateConverter.cs

@@ -9,8 +9,10 @@ public sealed class CollectionResultTSurrogateConverter<T> : IConverter<Collecti
 {
     public CollectionResult<T> ConvertFromSurrogate(in CollectionResultTSurrogate<T> surrogate)
     {
-        return CollectionResult<T>.Create(surrogate.IsSuccess, surrogate.Collection, surrogate.PageNumber, surrogate.PageSize, surrogate.TotalItems,
-            surrogate.Problem);
+        if (surrogate.IsSuccess)
+            return CollectionResult<T>.CreateSuccess(surrogate.Collection, surrogate.PageNumber, surrogate.PageSize, surrogate.TotalItems);
+
+        return CollectionResult<T>.CreateFailed(surrogate.Problem ?? Problem.GenericError(), surrogate.Collection);
     }
 
     public CollectionResultTSurrogate<T> ConvertToSurrogate(in CollectionResult<T> value)

ManagedCode.Communication.Orleans/Converters/ResultSurrogateConverter.cs

@@ -8,7 +8,10 @@ public sealed class ResultSurrogateConverter : IConverter<Result, ResultSurrogat
 {
     public Result ConvertFromSurrogate(in ResultSurrogate surrogate)
     {
-        return Result.Create(surrogate.IsSuccess, surrogate.Problem);
+        if (surrogate.IsSuccess)
+            return Result.Succeed();
+
+        return Result.CreateFailed(surrogate.Problem ?? Problem.GenericError());
     }
 
     public ResultSurrogate ConvertToSurrogate(in Result value)

ManagedCode.Communication.Orleans/Converters/ResultTSurrogateConverter.cs

@@ -8,7 +8,10 @@ public sealed class ResultTSurrogateConverter<T> : IConverter<Result<T>, ResultT
 {
     public Result<T> ConvertFromSurrogate(in ResultTSurrogate<T> surrogate)
     {
-        return Result<T>.Create(surrogate.IsSuccess, surrogate.Value, surrogate.Problem);
+        if (surrogate.IsSuccess)
+            return Result<T>.Succeed(surrogate.Value!);
+
+        return Result<T>.CreateFailed(surrogate.Problem ?? Problem.GenericError(), surrogate.Value);
     }
 
     public ResultTSurrogate<T> ConvertToSurrogate(in Result<T> value)

ManagedCode.Communication.Tests/AspNetCore/Extensions/ControllerExtensionsTests.cs

@@ -86,7 +86,7 @@ public void ToActionResult_WithValidationError_Returns400WithProblemDetails()
     public void ToActionResult_WithNoProblem_ReturnsDefaultError()
     {
         // Arrange - manually create failed result without problem
-        var result = new Result<string> { IsSuccess = false, Problem = null };
+        var result = (Result<string>)default;
 
         // Act
         var actionResult = result.ToActionResult();

ManagedCode.Communication.Tests/CollectionResults/CollectionResultFailMethodsTests.cs

@@ -28,7 +28,7 @@ public void Fail_NoParameters_ShouldCreateFailedResult()
         result.PageSize.Should().Be(0);
         result.TotalItems.Should().Be(0);
         result.TotalPages.Should().Be(0);
-        result.HasProblem.Should().BeFalse();
+        result.HasProblem.Should().BeTrue();
     }
 
     #endregion
@@ -48,7 +48,7 @@ public void Fail_WithEnumerable_ShouldCreateFailedResultWithItems()
         result.IsFailed.Should().BeTrue();
         result.Collection.Should().BeEquivalentTo(items);
         result.Collection.Should().HaveCount(5);
-        result.HasProblem.Should().BeFalse();
+        result.HasProblem.Should().BeTrue();
     }
 
     [Fact]
@@ -84,7 +84,7 @@ public void Fail_WithArray_ShouldCreateFailedResultWithItems()
         result.IsFailed.Should().BeTrue();
         result.Collection.Should().BeEquivalentTo(items);
         result.Collection.Should().HaveCount(3);
-        result.HasProblem.Should().BeFalse();
+        result.HasProblem.Should().BeTrue();
     }
 
     [Fact]

ManagedCode.Communication.Tests/CollectionResults/CollectionResultFromMethodsTests.cs

@@ -460,7 +460,7 @@ public void From_FailedCollectionResultWithoutProblem_ShouldReturnFailedResult()
 
         // Assert
         result.IsFailed.Should().BeTrue();
-        result.HasProblem.Should().BeFalse();
+        result.HasProblem.Should().BeTrue();
     }
 
     #endregion
@@ -507,7 +507,7 @@ public void From_GenericFailedCollectionResultWithoutProblem_ShouldReturnFailedR
 
         // Assert
         result.IsFailed.Should().BeTrue();
-        result.HasProblem.Should().BeFalse();
+        result.HasProblem.Should().BeTrue();
     }
 
     #endregion

ManagedCode.Communication.Tests/Results/CollectionResultTests.cs

@@ -296,7 +296,7 @@ public void InvalidFieldError_WithValidationProblem_ShouldReturnErrorMessage()
     public void AddInvalidMessage_ShouldAddValidationError()
     {
         // Arrange
-        var result = CollectionResult<string>.Empty();
+        var result = CollectionResult<string>.Invalid();
 
         // Act
         result.AddInvalidMessage("email", "Email is required");
@@ -317,7 +317,7 @@ public void AddInvalidMessage_ShouldAddValidationError()
     public void AddInvalidMessage_WithGeneralMessage_ShouldAddToGeneralErrors()
     {
         // Arrange
-        var result = CollectionResult<string>.Empty();
+        var result = CollectionResult<string>.Invalid();
 
         // Act
         result.AddInvalidMessage("General error occurred");
@@ -331,6 +331,30 @@ public void AddInvalidMessage_WithGeneralMessage_ShouldAddToGeneralErrors()
             .Be("General error occurred");
     }
 
+    [Fact]
+    public void AddInvalidMessage_ThrowsOnSuccessfulResult()
+    {
+        // Arrange
+        var result = CollectionResult<string>.Empty();
+
+        // Act & Assert
+        result.Invoking(r => r.AddInvalidMessage("email", "Email is required"))
+            .Should()
+            .ThrowExactly<InvalidOperationException>();
+    }
+
+    [Fact]
+    public void AddInvalidMessage_WithGeneralMessage_ThrowsOnSuccessfulResult()
+    {
+        // Arrange
+        var result = CollectionResult<string>.Empty();
+
+        // Act & Assert
+        result.Invoking(r => r.AddInvalidMessage("General error occurred"))
+            .Should()
+            .ThrowExactly<InvalidOperationException>();
+    }
+
     [Fact]
     public void ThrowIfFail_WithSuccessfulResult_ShouldNotThrow()
     {

ManagedCode.Communication.Tests/Results/ResultFailMethodsTests.cs

@@ -20,7 +20,7 @@ public void Result_Fail_NoParameters_ShouldCreateFailedResult()
         // Assert
         result.IsFailed.Should().BeTrue();
         result.IsSuccess.Should().BeFalse();
-        result.HasProblem.Should().BeFalse();
+        result.HasProblem.Should().BeTrue();
     }
 
     #endregion

ManagedCode.Communication.Tests/Results/ResultStaticHelperMethodsTests.cs

@@ -22,7 +22,7 @@ public void Result_FailT_NoParameters_ShouldCreateFailedResultT()
         result.IsFailed.Should().BeTrue();
         result.IsSuccess.Should().BeFalse();
         result.Value.Should().BeNull();
-        result.HasProblem.Should().BeFalse();
+        result.HasProblem.Should().BeTrue();
     }
 
     [Fact]

ManagedCode.Communication.Tests/Results/ResultTFailMethodsTests.cs

@@ -22,7 +22,7 @@ public void ResultT_Fail_NoParameters_ShouldCreateFailedResult()
         result.IsFailed.Should().BeTrue();
         result.IsSuccess.Should().BeFalse();
         result.Value.Should().BeNull();
-        result.HasProblem.Should().BeFalse();
+        result.HasProblem.Should().BeTrue();
     }
 
     #endregion
@@ -41,7 +41,7 @@ public void ResultT_Fail_WithValue_ShouldCreateFailedResultWithValue()
         // Assert
         result.IsFailed.Should().BeTrue();
         result.Value.Should().Be(value);
-        result.HasProblem.Should().BeFalse();
+        result.HasProblem.Should().BeTrue();
     }
 
     [Fact]
@@ -54,7 +54,7 @@ public void ResultT_Fail_WithNullValue_ShouldCreateFailedResultWithNull()
         // Assert
         result.IsFailed.Should().BeTrue();
         result.Value.Should().BeNull();
-        result.HasProblem.Should().BeFalse();
+        result.HasProblem.Should().BeTrue();
     }
 
     #endregion

ManagedCode.Communication/CollectionResultT/CollectionResult.cs

@@ -18,7 +18,7 @@ private CollectionResult(bool isSuccess, IEnumerable<T>? collection, int pageNum
     {
     }
 
-    internal CollectionResult(bool isSuccess, T[]? collection, int pageNumber, int pageSize, int totalItems, Problem? problem = null)
+    private CollectionResult(bool isSuccess, T[]? collection, int pageNumber, int pageSize, int totalItems, Problem? problem = null)
     {
         IsSuccess = isSuccess;
         Collection = collection ?? [];
@@ -29,19 +29,25 @@ internal CollectionResult(bool isSuccess, T[]? collection, int pageNumber, int p
         Problem = problem;
     }
 
-    internal static CollectionResult<T> Create(bool isSuccess, T[]? collection, int pageNumber, int pageSize, int totalItems, Problem? problem = null)
+    internal static CollectionResult<T> CreateSuccess(T[]? collection, int pageNumber, int pageSize, int totalItems)
     {
-        return new CollectionResult<T>(isSuccess, collection, pageNumber, pageSize, totalItems, problem);
+        return new CollectionResult<T>(true, collection, pageNumber, pageSize, totalItems, null);
     }
 
+    internal static CollectionResult<T> CreateFailed(Problem problem, T[]? collection = null)
+    {
+        return new CollectionResult<T>(false, collection, 0, 0, 0, problem);
+    }
+
+    [JsonInclude]
     [JsonPropertyName("isSuccess")]
     [JsonPropertyOrder(1)]
     [MemberNotNullWhen(true, nameof(Collection))]
     [MemberNotNullWhen(false, nameof(Problem))]
     public bool IsSuccess { get; set; }
 
     [JsonIgnore]
-    public bool IsFailed => !IsSuccess || HasProblem;
+    public bool IsFailed => !IsSuccess;
 
     [JsonPropertyName("collection")]
     [JsonPropertyOrder(2)]
@@ -64,10 +70,24 @@ internal static CollectionResult<T> Create(bool isSuccess, T[]? collection, int
     [JsonPropertyOrder(6)]
     public int TotalPages { get; set; }
 
+    [JsonInclude]
     [JsonPropertyName("problem")]
     [JsonPropertyOrder(7)]
     [JsonIgnore(Condition = JsonIgnoreCondition.WhenWritingNull)]
-    public Problem? Problem { get; set; }
+    private Problem? _problem;
+
+    [JsonIgnore]
+    public Problem? Problem
+    {
+        get
+        {
+            if (_problem is null && !IsSuccess)
+                _problem = Problem.GenericError();
+
+            return _problem;
+        }
+        private init => _problem = value;
+    }
 
     [JsonIgnore]
     public bool IsEmpty => Collection is null || Collection.Length == 0;
@@ -77,15 +97,22 @@ internal static CollectionResult<T> Create(bool isSuccess, T[]? collection, int
 
     [JsonIgnore]
     [MemberNotNullWhen(true, nameof(Problem))]
-    public bool HasProblem => Problem != null;
+    public bool HasProblem => !IsSuccess;
 
     #region IResultProblem Implementation
 
+    /// <summary>
+    ///     Get the Problem assigned to the result without falling back to a generic error if no problem is assigned.
+    ///     Useful if a different default problem is desired.
+    /// </summary>
+    internal Problem? GetProblemNoFallback() => _problem;
+
     public bool ThrowIfFail()
     {
-        if (HasProblem)
+        var problem = Problem;
+        if (problem is not null)
         {
-            throw Problem;
+            throw problem;
         }
 
         return false;
@@ -95,7 +122,7 @@ public bool ThrowIfFail()
     public bool TryGetProblem([MaybeNullWhen(false)] out Problem problem)
     {
         problem = Problem;
-        return HasProblem;
+        return problem is not null;
     }
 
     #endregion
@@ -120,44 +147,38 @@ public string InvalidFieldError(string fieldName)
 
     public void AddInvalidMessage(string message)
     {
-        if (Problem == null)
-        {
-            Problem = Problem.Validation((ProblemConstants.ValidationFields.General, message));
-        }
-        else
+        if (IsSuccess) throw new InvalidOperationException("Cannot add invalid message to a successful result");
+
+        var problem = Problem;
+
+        problem.Extensions[ProblemConstants.ExtensionKeys.Errors] ??= new Dictionary<string, List<string>>();
+        if (problem.Extensions[ProblemConstants.ExtensionKeys.Errors] is Dictionary<string, List<string>> errors)
         {
-            Problem.Extensions[ProblemConstants.ExtensionKeys.Errors] ??= new Dictionary<string, List<string>>();
-            if (Problem.Extensions[ProblemConstants.ExtensionKeys.Errors] is Dictionary<string, List<string>> errors)
+            if (!errors.ContainsKey(ProblemConstants.ValidationFields.General))
             {
-                if (!errors.ContainsKey(ProblemConstants.ValidationFields.General))
-                {
-                    errors[ProblemConstants.ValidationFields.General] = new List<string>();
-                }
-
-                errors[ProblemConstants.ValidationFields.General]
-                    .Add(message);
+                errors[ProblemConstants.ValidationFields.General] = new List<string>();
             }
+
+            errors[ProblemConstants.ValidationFields.General]
+                .Add(message);
         }
     }
 
     public void AddInvalidMessage(string key, string value)
     {
-        if (Problem == null)
-        {
-            Problem = Problem.Validation((key, value));
-        }
-        else
+        if (IsSuccess) throw new InvalidOperationException("Cannot add invalid message to a successful result");
+
+        var problem = Problem;
+
+        problem.Extensions[ProblemConstants.ExtensionKeys.Errors] ??= new Dictionary<string, List<string>>();
+        if (problem.Extensions[ProblemConstants.ExtensionKeys.Errors] is Dictionary<string, List<string>> errors)
         {
-            Problem.Extensions[ProblemConstants.ExtensionKeys.Errors] ??= new Dictionary<string, List<string>>();
-            if (Problem.Extensions[ProblemConstants.ExtensionKeys.Errors] is Dictionary<string, List<string>> errors)
+            if (!errors.ContainsKey(key))
             {
-                if (!errors.ContainsKey(key))
-                {
-                    errors[key] = new List<string>();
-                }
-
-                errors[key].Add(value);
+                errors[key] = new List<string>();
             }
+
+            errors[key].Add(value);
         }
     }
 
@@ -170,7 +191,7 @@ public void AddInvalidMessage(string key, string value)
     /// </summary>
     public static CollectionResult<T> Empty()
     {
-        return Create(true, [], 0, 0, 0);
+        return CreateSuccess([], 0, 0, 0);
     }
 
     #endregion