Changes after Tonys suggestion

SharePointRadi · SharePointRadi · commit e542e9f4a6d6 · 2025-03-05T13:45:46.000+02:00
diff --git a/src/OneBitSoftware.Utilities.OperationResult/Errors/IOperationError.cs b/src/OneBitSoftware.Utilities.OperationResult/Errors/IOperationError.cs
@@ -9,15 +9,5 @@ public interface IOperationError
         string? Message { get; set; }
 
         string? Details { get; set; }
-
-        /// <summary>
-        /// Defines if the error is logged or not. Used when merging <cref="OperationResult"/> instances and one of them does not have an ILogger.
-        /// </summary>
-        bool Logged { get; internal set; }
-
-        /// <summary>
-        /// Defines the log level for the error.
-        /// </summary>
-        LogLevel? LogLevel { get; set; }
     }
 }
diff --git a/src/OneBitSoftware.Utilities.OperationResult/Errors/OperationError.cs b/src/OneBitSoftware.Utilities.OperationResult/Errors/OperationError.cs
@@ -5,12 +5,11 @@
 
     public class OperationError : IOperationError
     {
-        public OperationError(string? message = null, int? code = null, string? details = null, LogLevel? logLevel = null)
+        public OperationError(string? message = null, int? code = null, string? details = null)
         {
             this.Message = message;
             this.Code = code;
             this.Details = details;
-            this.LogLevel = logLevel;
         }
 
         public int? Code { get; set; }
@@ -19,20 +18,12 @@ public OperationError(string? message = null, int? code = null, string? details
 
         public string? Details { get; set; }
 
-        /// <inheritdoc />
-        public LogLevel? LogLevel { get; set; }
-
-        /// <inheritdoc />
-        bool IOperationError.Logged { get; set; }
-
         public override string ToString()
         {
             var result = new StringBuilder();
 
             if (this.Code != null) result.AppendLine($"Code: {this.Code}");
 
-            if (this.LogLevel is not null) result.AppendLine($"Severity: {this.LogLevel}"); // TODO: maybe convert to string?
-
             if (!string.IsNullOrWhiteSpace(this.Message)) result.AppendLine($"Message: {this.Message}");
 
             if (!string.IsNullOrWhiteSpace(this.Details)) result.AppendLine($"Trace: {this.Details}");
diff --git a/src/OneBitSoftware.Utilities.OperationResult/OperationResult.cs b/src/OneBitSoftware.Utilities.OperationResult/OperationResult.cs
@@ -13,9 +13,14 @@
     /// </summary>
     public class OperationResult
     {
-        private readonly List<string> _successMessages = new List<string>();
+        /// <summary>
+        /// Contains <see cref="IOperationError"/> instances that have not been logged.
+        /// </summary>
+        private readonly List<(IOperationError Error, LogLevel? LogLevel)> _errorsNotLogged = new();
+
+        private readonly List<string> _successMessages = new();
 
-        protected readonly ILogger? _logger;
+        private readonly ILogger? _logger;
 
         /// <summary>
         /// Gets or sets a value indicating whether the operation is successful or not.
@@ -44,7 +49,7 @@ public IEnumerable<string>? SuccessMessages
         /// Gets an <see cref="List{T}"/> containing the error codes and messages of the <see cref="OperationResult{T}" />.
         /// </summary>
         public List<IOperationError> Errors { get; internal set; } = new List<IOperationError>();
-        
+
         /// <summary>
         /// Gets or sets the first exception that resulted from the operation.
         /// </summary>
@@ -93,18 +98,22 @@ public OperationResult AppendErrors(OperationResult otherOperationResult)
         {
             if (otherOperationResult is null) return this;
 
-            foreach (var error in otherOperationResult.Errors)
+            // store any messages for logging at a later stage, when merged to an OperationResult with a logger.
+            if (this._logger is null) 
+            {
+                this._errorsNotLogged.AddRange(otherOperationResult._errorsNotLogged); 
+            }
+            else
             {
-                this.AppendErrorInternal(error);
-
-                // Logs messages if the other operation result does not have a logger
-                if (this._logger is not null && otherOperationResult._logger is null && !error.Logged)
-                {
-                    this._logger.Log(GetLogLevel(error.LogLevel), error.Message);
-                    error.Logged = true;
-                }
+                foreach (var (error, logLevel) in otherOperationResult._errorsNotLogged)
+                    this.LogInternal(error, logLevel);
+
+                otherOperationResult._errorsNotLogged.Clear();
             }
 
+            // Append the error message without logging (presuming that there is already a log message).
+            foreach (var error in otherOperationResult.Errors) this.AppendErrorInternal(error);
+
             return this;
         }
 
@@ -120,7 +129,7 @@ public OperationResult AppendError(string message, int? code = null, LogLevel? l
         {
             if (string.IsNullOrWhiteSpace(message)) throw new ArgumentNullException(nameof(message));
 
-            var error = new OperationError(message, code) { Details = details, LogLevel = logLevel };
+            var error = new OperationError(message, code) { Details = details };
             this.AppendError(error, logLevel);
 
             return this;
@@ -140,7 +149,7 @@ public OperationResult AppendError<T>(string message, int? code = null, LogLevel
         {
             if (string.IsNullOrWhiteSpace(message)) throw new ArgumentNullException(nameof(message));
 
-            var error = new T() { Message = message, Code = code, Details = details, LogLevel = logLevel };
+            var error = new T() { Message = message, Code = code, Details = details };
             this.AppendError(error, logLevel);
 
             return this;
@@ -155,31 +164,26 @@ public OperationResult AppendError<T>(string message, int? code = null, LogLevel
         public OperationResult AppendError(IOperationError error, LogLevel? logLevel = LogLevel.Error)
         {
             this.AppendErrorInternal(error);
-
-            if (this._logger != null)
-            {
-                this._logger.Log(GetLogLevel(logLevel), error.Message);
-                error.Logged = true;
-            }
+            this.LogInternal(error, logLevel);
 
             return this;
         }
 
         /// <summary>
-        /// Appends an exception to the error message collection and logs the full exception as an Error <see cref="LogEventLevel"/> level. A call to this method will set the Success property to false.
+        /// Appends an exception to the error message collection and logs the full exception as an Error <see cref="LogLevel"/> level. A call to this method will set the Success property to false.
         /// </summary>
         /// <param name="exception">The exception to log.</param>
         /// <param name="errorCode">The error code.</param>
-        /// <param name="logLevel">The <see cref="LogEventLevel"/> logging severity.</param>
+        /// <param name="logLevel">The <see cref="LogLevel"/> logging severity.</param>
         /// <returns>The current instance of the <see cref="OperationResult"/>.</returns>
-        public OperationResult AppendException(Exception exception, int? errorCode = null, LogLevel? logLevel = null)
+        public OperationResult AppendException(Exception exception, int errorCode = 0, LogLevel? logLevel = null)
         {
             if (exception is null) throw new ArgumentNullException(nameof(exception));
 
             // Append the exception as a first if it is not yet set.
             this.InitialException ??= exception;
 
-            var error = new OperationError(exception.ToString(), errorCode, null, LogLevel.Error);
+            var error = new OperationError(exception.ToString(), errorCode);
             this.AppendError(error, logLevel);
 
             return this;
@@ -218,13 +222,28 @@ public static OperationResult FromError(string message, int? code = null, LogLev
             return result.AppendError(message, code, logLevel, details);
         }
 
-        protected static LogLevel GetLogLevel(LogLevel? optionalLevel) => optionalLevel ?? LogLevel.Error;
-
         /// <summary>
         /// Appends an <see cref="IOperationError"/> to the internal errors collection.
         /// </summary>
         /// <param name="error">An instance of <see cref="IOperationError"/> to add to the internal errors collection.</param>
         protected void AppendErrorInternal(IOperationError error) => this.Errors.Add(error);
+
+        /// <summary>
+        /// Logs to the internal logger if it is set, otherwise it will add the error to the internal errors collection.
+        /// </summary>
+        /// <param name="error">The <see cref="IOperationError"/> to log.</param>
+        /// <param name="logLevel">The log level.</param>
+        private void LogInternal(IOperationError error, LogLevel? logLevel)
+        {
+            if (this._logger is null)
+            {
+                this._errorsNotLogged.Add((Error: error, LogLevel: logLevel));
+            }
+            else
+            {
+                this._logger.Log(logLevel ?? LogLevel.Error, error.Message);
+            }
+        }
     }
 
     /// <summary>
@@ -291,41 +310,14 @@ public OperationResult(TResult resultObject) : base()
             return this;
         }
 
-        /// <summary>
-        /// Appends error messages from <paramref name="otherOperationResult"/> to the current instance.
-        /// </summary>
-        /// <param name="otherOperationResult">The <see cref="OperationResult"/> to append from.</param>
-        /// <typeparam name="TOther">A type that inherits from <see cref="OperationResult"/>.</typeparam>
-        /// <returns>The original <see cref="OperationResult"/> with the appended messages from <paramref name="otherOperationResult"/>.</returns>
-        [Obsolete("Please use AppendErrors instead. This method will be removed to avoid confusion.")]
-        public OperationResult<TResult> AppendErrorMessages<TOther>(TOther otherOperationResult)
-            where TOther : OperationResult
-        {
-            base.AppendErrors(otherOperationResult);
-
-            return this;
-        }
-
-        /// <summary>
-        /// Appends error from <paramref name="otherOperationResult"/> to the current instance.
-        /// </summary>
-        /// <param name="otherOperationResult">The <see cref="OperationResult"/> to append from.</param>
-        /// <returns>The original <see cref="OperationResult"/> with the appended messages from <paramref name="otherOperationResult"/>.</returns>
-        public new OperationResult<TResult> AppendErrors(OperationResult otherOperationResult)
-        {
-            base.AppendErrors(otherOperationResult);
-
-            return this;
-        }
-
         /// <summary>
         /// Appends an exception to the error message collection and logs the full exception as an Error <see cref="LogEventLevel"/> level. A call to this method will set the Success property to false.
         /// </summary>
         /// <param name="exception">The exception to log.</param>
         /// <param name="errorCode">The error code.</param>
         /// <param name="logLevel">The <see cref="LogEventLevel"/> logging severity.</param>
         /// <returns>The current instance of the <see cref="OperationResult{TResult}"/>.</returns>
-        public new OperationResult<TResult> AppendException(Exception exception, int? errorCode = null, LogLevel? logLevel = null)
+        public new OperationResult<TResult> AppendException(Exception exception, int errorCode = 0, LogLevel? logLevel = null)
         {
             base.AppendException(exception, errorCode, logLevel);
 
diff --git a/tests/OneBitSoftware.Utilities.OperationResultTests/OperationResultAppendErrorsTests.cs b/tests/OneBitSoftware.Utilities.OperationResultTests/OperationResultAppendErrorsTests.cs
@@ -62,7 +62,7 @@ public void AppendErrorsT_ShouldListAllErrors()
         operationResultBase.AppendError(message2, errorCode2, LogLevel.Debug, detail2);
 
         // Act - AppendErrorMessages is to be removed
-        operationResultBase.AppendErrorMessages(operationResultTarget);
+        operationResultBase.AppendErrors(operationResultTarget);
 
         // Assert
         Assert.False(operationResultBase.Success);
diff --git a/tests/OneBitSoftware.Utilities.OperationResultTests/Serialization/OperationResultSerializationSystemTextJsonTests.cs b/tests/OneBitSoftware.Utilities.OperationResultTests/Serialization/OperationResultSerializationSystemTextJsonTests.cs
@@ -56,7 +56,7 @@ public async Task SerializeWithSystemTextJsonIncludesTypeDiscriminator()
             // Assert
             Assert.Contains(testText, text);
             Assert.Contains(operationErrorText, text);
-            var expectText = @"{""Success"":false,""Errors"":[{""type"":""operation_error"",""Code"":123,""Message"":""Test"",""Details"":""\u0022type\u0022"",""LogLevel"":null}]}";
+            var expectText = @"{""Success"":false,""Errors"":[{""type"":""operation_error"",""Code"":123,""Message"":""Test"",""Details"":""\u0022type\u0022""}]}";
             Assert.Equal(expectText, text);
         }
 
@@ -67,15 +67,15 @@ public async Task SerializeWithSystemTextJsonSetsExpectedJson()
             var testText = "\"type\"";
             var outputStream = new MemoryStream();
             var operationResult = new OperationResult();
-            operationResult.AppendError(new OperationError(message: "Test") { Code = 123, Details = testText, LogLevel = Microsoft.Extensions.Logging.LogLevel.Warning });
+            operationResult.AppendError(new OperationError(message: "Test") { Code = 123, Details = testText });
 
             // Act
             await JsonSerializer.SerializeAsync(outputStream, operationResult, GetSerializationOptions());
             outputStream.Position = 0;
             string text = new StreamReader(outputStream).ReadToEnd();
 
             // Assert
-            var expectText = @"{""Success"":false,""Errors"":[{""type"":""operation_error"",""Code"":123,""Message"":""Test"",""Details"":""\u0022type\u0022"",""LogLevel"":3}]}";
+            var expectText = @"{""Success"":false,""Errors"":[{""type"":""operation_error"",""Code"":123,""Message"":""Test"",""Details"":""\u0022type\u0022""}]}";
             Assert.Equal(expectText, text);
         }
 
