Fix: Remove that EqualsAndHashCode of Message only uses the name, Change MessageHelper to not expose the TreeSet that is used for deduplication because a TreeSet breaks equals

Author: sam0r040

springwolf-core/src/main/java/io/github/stavshamir/springwolf/asyncapi/MessageHelper.java

@@ -4,7 +4,13 @@
 import io.github.stavshamir.springwolf.asyncapi.types.channel.operation.message.Message;
 import lombok.extern.slf4j.Slf4j;
 
-import java.util.*;
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.Comparator;
+import java.util.HashSet;
+import java.util.Map;
+import java.util.Set;
+import java.util.TreeSet;
 import java.util.function.Supplier;
 import java.util.stream.Collectors;
 
@@ -13,6 +19,10 @@ public class MessageHelper {
     private static final String ONE_OF = "oneOf";
 
     private static final Comparator<Message> byMessageName = Comparator.comparing(Message::getName);
+
+    // TODO Why do we need a SortedSet here? Using a comparator with only the message name will break deep equals on the Set
+    // Unfortunately there are Tests relying on deep equals
+    // see https://docs.oracle.com/javase/7/docs/api/java/util/TreeSet.html
     private static final Supplier<Set<Message>> messageSupplier = () -> new TreeSet<>(byMessageName);
 
     public static Object toMessageObjectOrComposition(Set<Message> messages) {
@@ -22,14 +32,14 @@ public static Object toMessageObjectOrComposition(Set<Message> messages) {
             case 1:
                 return messages.toArray()[0];
             default:
-                return ImmutableMap.of(ONE_OF, messages.stream().collect(Collectors.toCollection(messageSupplier)));
+                return ImmutableMap.of(ONE_OF, new HashSet<>(messages.stream().collect(Collectors.toCollection(messageSupplier))));
         }
     }
 
     @SuppressWarnings("unchecked")
     public static Set<Message> messageObjectToSet(Object messageObject) {
         if (messageObject instanceof Message) {
-            return new HashSet<>(Arrays.asList((Message) messageObject));
+            return new HashSet<>(Collections.singletonList((Message) messageObject));
         }
 
         if (messageObject instanceof Map) {

springwolf-core/src/main/java/io/github/stavshamir/springwolf/asyncapi/types/channel/operation/message/Message.java

@@ -10,7 +10,8 @@
  */
 @Data
 @Builder
-@EqualsAndHashCode(of = {"name"})
+// TODO Why ignore other fields?
+//@EqualsAndHashCode(of = {"name"})
 @NoArgsConstructor
 @AllArgsConstructor
 public class Message {

springwolf-core/src/test/java/io/github/stavshamir/springwolf/asyncapi/MessageHelperTest.java

@@ -53,6 +53,60 @@ public void toMessageObjectOrComposition_multipleMessages() {
                 .isEqualTo(ImmutableMap.of("oneOf", ImmutableSet.of(message1, message2)));
     }
 
+    @Test
+    public void toMessageObjectOrComposition_multipleMessages_remove_duplicates() {
+        Message message1 = Message.builder()
+                .name("foo")
+                .description("This is message 1")
+                .build();
+
+        Message message2 = Message.builder()
+                .name("bar")
+                .description("This is message 2")
+                .build();
+
+        Message message3 = Message.builder()
+                .name("bar")
+                .description("This is message 3, but in essence the same payload type as message 2")
+                .build();
+
+        Object asObject = toMessageObjectOrComposition(ImmutableSet.of(message1, message2, message3));
+
+        // Message3 is not included as it is identical in terms of payload type (Message#name) to message 2
+        assertThat(asObject)
+                .isInstanceOf(Map.class)
+                .isEqualTo(ImmutableMap.of("oneOf", ImmutableSet.of(message1, message2)));
+    }
+
+    @Test
+    public void toMessageObjectOrComposition_multipleMessages_should_not_break_deep_equals() {
+        Message actualMessage1 = Message.builder()
+                .name("foo")
+                .description("This is actual message 1")
+                .build();
+
+        Message actualMessage2 = Message.builder()
+                .name("bar")
+                .description("This is actual message 2")
+                .build();
+
+        Object actualObject = toMessageObjectOrComposition(ImmutableSet.of(actualMessage1, actualMessage2));
+
+        Message expectedMessage1 = Message.builder()
+                .name("foo")
+                .description("This is expected message 1")
+                .build();
+
+        Message expectedMessage2 = Message.builder()
+                .name("bar")
+                .description("This is expected message 2")
+                .build();
+
+        Object expectedObject = toMessageObjectOrComposition(ImmutableSet.of(expectedMessage1, expectedMessage2));
+
+        assertThat(actualObject).isNotEqualTo(expectedObject);
+    }
+
     @Test
     public void messageObjectToSet_notAMessageOrAMap() {
         Object string = "foo";
@@ -94,4 +148,4 @@ public void messageObjectToSet_SetOfMessage() {
                 .containsExactlyInAnyOrder(message1, message2);
     }
 
-}
+}

springwolf-core/src/test/java/io/github/stavshamir/springwolf/asyncapi/scanners/channels/ChannelMergerTest.java

@@ -110,11 +110,12 @@ public void shouldMergeDifferentMessageForSameOperation() {
 
         // then expectedMessage only includes message1 and message2.
         // Message3 is not included as it is identical in terms of payload type (Message#name) to message 2
-        Object expectedMessages = MessageHelper.toMessageObjectOrComposition(Sets.newHashSet(message1, message2));
+        // TODO Is it really expected that the first occurrence is used? (no test in Message Helper...)
+        Object expectedMessages = MessageHelper.toMessageObjectOrComposition(Sets.newHashSet(message1, message3));
         assertThat(mergedChannels).hasSize(1)
                 .hasEntrySatisfying(channelName, it -> {
                     assertThat(it.getPublish()).isEqualTo(Operation.builder().operationId("publisher1").message(expectedMessages).build());
                     assertThat(it.getSubscribe()).isNull();
                 });
     }
-}
+}

springwolf-core/src/test/java/io/github/stavshamir/springwolf/asyncapi/scanners/channels/operationdata/annotation/AsyncListenerAnnotationScannerTest.java

@@ -66,7 +66,7 @@ public void scan_componentHasListenerMethod() {
         Message message = Message.builder()
                 .name(SimpleFoo.class.getName())
                 .title(SimpleFoo.class.getSimpleName())
-                .description(null)
+                .description("")
                 .payload(PayloadReference.fromModelName(SimpleFoo.class.getSimpleName()))
                 .headers(HeaderReference.fromModelName(AsyncHeaders.NOT_DOCUMENTED.getSchemaName()))
                 .build();
@@ -164,4 +164,4 @@ private static class SimpleFoo {
         private String s;
         private boolean b;
     }
-}
+}

springwolf-core/src/test/java/io/github/stavshamir/springwolf/asyncapi/scanners/channels/operationdata/annotation/AsyncPublisherAnnotationScannerTest.java

@@ -66,7 +66,7 @@ public void scan_componentHasPublisherMethod() {
         Message message = Message.builder()
                 .name(SimpleFoo.class.getName())
                 .title(SimpleFoo.class.getSimpleName())
-                .description(null)
+                .description("")
                 .payload(PayloadReference.fromModelName(SimpleFoo.class.getSimpleName()))
                 .headers(HeaderReference.fromModelName(AsyncHeaders.NOT_DOCUMENTED.getSchemaName()))
                 .build();
@@ -164,4 +164,4 @@ private static class SimpleFoo {
         private String s;
         private boolean b;
     }
-}
+}

springwolf-plugins/springwolf-kafka-plugin/src/test/java/io/github/stavshamir/springwolf/asyncapi/scanners/channels/annotation/ClassLevelKafkaListenerScannerTest.java

@@ -77,12 +77,14 @@ public void scan_componentWithMultipleKafkaListenersAndHandlers() {
                 .name(SimpleFoo.class.getName())
                 .title(SimpleFoo.class.getSimpleName())
                 .payload(PayloadReference.fromModelName(SimpleFoo.class.getSimpleName()))
+                .headers(HeaderReference.fromModelName("SpringKafkaDefaultHeaders-" + SimpleFoo.class.getSimpleName()))
                 .build();
 
         Message barMessage = Message.builder()
                 .name(SimpleBar.class.getName())
                 .title(SimpleBar.class.getSimpleName())
                 .payload(PayloadReference.fromModelName(SimpleBar.class.getSimpleName()))
+                .headers(HeaderReference.fromModelName("SpringKafkaDefaultHeaders-" + SimpleBar.class.getSimpleName()))
                 .build();
 
         Operation operation = Operation.builder()