chore: cleanup Result and AbstractResult (#1656)

Author: paullatzelsperger

spi/core-spi/src/main/java/org/eclipse/dataspaceconnector/spi/result/AbstractResult.java

@@ -15,16 +15,17 @@
 package org.eclipse.dataspaceconnector.spi.result;
 
 import com.fasterxml.jackson.annotation.JsonIgnore;
-import org.jetbrains.annotations.NotNull;
 
 import java.util.List;
-import java.util.Objects;
 
 /**
  * Base result type used by services to indicate success or failure.
+ * <p>
+ * Service operations should generally never throw checked exceptions. Instead, they should return concrete result types
+ * and raise unchecked exceptions only when an unexpected event happens, such as a programming error.
  *
- * Service operations should generally never throw checked exceptions. Instead, they should return concrete result types and raise unchecked exceptions only when an
- * unexpected event happens, such as a programming error.
+ * @param <F> The type of {@link Failure}.
+ * @param <T> The type of the content
  */
 public abstract class AbstractResult<T, F extends Failure> {
 
@@ -36,7 +37,6 @@ protected AbstractResult(T content, F failure) {
         this.failure = failure;
     }
 
-    @NotNull
     public T getContent() {
         return content;
     }
@@ -48,8 +48,7 @@ public F getFailure() {
     //will cause problems during JSON serialization if failure is null
     @JsonIgnore
     public List<String> getFailureMessages() {
-        Objects.requireNonNull(failure);
-        return failure.getMessages();
+        return failure == null ? List.of() : failure.getMessages();
     }
 
     public boolean succeeded() {
@@ -67,6 +66,6 @@ public boolean failed() {
      */
     @JsonIgnore // will cause problems during JSON serialization if failure is null
     public String getFailureDetail() {
-        return String.join(", ", getFailureMessages());
+        return failure == null ? null : String.join(", ", getFailureMessages());
     }
 }

spi/core-spi/src/test/java/org/eclipse/dataspaceconnector/spi/result/ResultTest.java

@@ -20,6 +20,14 @@
 
 class ResultTest {
 
+
+    @Test
+    void verifyFailureMessages_whenSucceeded() {
+        var r = Result.success("Foobar");
+        assertThat(r.getFailureDetail()).isNull();
+        assertThat(r.getFailureMessages()).isEmpty();
+    }
+
     @Test
     void map_appliesFunctionToContent() {
         var result = Result.success("successful").map(it -> it + " and mapped");
@@ -28,6 +36,12 @@ void map_appliesFunctionToContent() {
         assertThat(result.getContent()).isEqualTo("successful and mapped");
     }
 
+    @Test
+    void verifyContent_whenFailed() {
+        var r = Result.failure("random failure");
+        assertThat(r.getContent()).isNull();
+    }
+
     @Test
     void map_doesNothingOnFailedResult() {
         var result = Result.failure("error").map(it -> it + " and mapped");