refactor: Moved business-logic from `ChatRoomService` into `ChatRoom`

- Some essential business-logic -- the identification of mutated messages --
  was buried in `InMemoryChatRoomService`.
- This business-logic was moved into `ChatRoom`, becaus otherwise, it
  would have been necessary, to reproduce this logic in each and every
  new implementation of `ChatRoomService`, which would have been exhausting
  and errorprone.
- This allowed also cleaner code in `InMemoryChatRoomService`, that
  can focus on the persistence-logic.
- The implementation of `MessageMutationException` and the look of the
  accompanying problem-details hat to be refined accordingly.

diff --git a/src/main/java/de/juplo/kafka/chat/backend/api/ChatBackendController.java b/src/main/java/de/juplo/kafka/chat/backend/api/ChatBackendController.java
index 68d056b..51ed6a2 100644 (file)
--- a/src/main/java/de/juplo/kafka/chat/backend/api/ChatBackendController.java
+++ b/src/main/java/de/juplo/kafka/chat/backend/api/ChatBackendController.java
@@ -73,7 +73,6 @@ public class ChatBackendController
                 messageId,
                 username,
                 text)
-            .switchIfEmpty(chatroom.getMessage(username, messageId))
             .map(message -> MessageTo.from(message));
   }
 

diff --git a/src/main/java/de/juplo/kafka/chat/backend/api/ChatBackendControllerAdvice.java b/src/main/java/de/juplo/kafka/chat/backend/api/ChatBackendControllerAdvice.java
index ab8c13d..fa3a0e1 100644 (file)
--- a/src/main/java/de/juplo/kafka/chat/backend/api/ChatBackendControllerAdvice.java
+++ b/src/main/java/de/juplo/kafka/chat/backend/api/ChatBackendControllerAdvice.java
@@ -77,10 +77,10 @@ public class ChatBackendControllerAdvice
     stringBuilder.append(" cannot be mutated!");
     problem.setDetail(stringBuilder.toString());
 
-    problem.setProperty("mutatedMessage", MessageTo.from(e.getMutated()));
-
     problem.setProperty("existingMessage", MessageTo.from(e.getExisting()));
 
+    problem.setProperty("mutatedText", e.getMutatedText());
+
     return problem;
   }
 }

diff --git a/src/main/java/de/juplo/kafka/chat/backend/domain/ChatRoom.java b/src/main/java/de/juplo/kafka/chat/backend/domain/ChatRoom.java
index 22eebff..58efa54 100644 (file)
--- a/src/main/java/de/juplo/kafka/chat/backend/domain/ChatRoom.java
+++ b/src/main/java/de/juplo/kafka/chat/backend/domain/ChatRoom.java
@@ -48,16 +48,23 @@ public class ChatRoom
       String user,
       String text)
   {
+    Message.MessageKey key = Message.MessageKey.of(user, id);
     return service
-        .persistMessage(Message.MessageKey.of(user, id), LocalDateTime.now(clock), text)
-        .doOnNext(message ->
-        {
-          Sinks.EmitResult result = sink.tryEmitNext(message);
-          if (result.isFailure())
-          {
-            log.warn("Emitting of message failed with {} for {}", result.name(), message);
-          }
-        });
+        .getMessage(key)
+        .flatMap(existing -> text.equals(existing.getMessageText())
+            ? Mono.just(existing)
+            : Mono.error(() -> new MessageMutationException(existing, text)))
+        .switchIfEmpty(
+            Mono
+                .just(service.persistMessage(key, LocalDateTime.now(clock), text))
+                .doOnNext(m ->
+                {
+                  Sinks.EmitResult result = sink.tryEmitNext(m);
+                  if (result.isFailure())
+                  {
+                    log.warn("Emitting of message failed with {} for {}", result.name(), m);
+                  }
+                }));
   }
 
 

diff --git a/src/main/java/de/juplo/kafka/chat/backend/domain/ChatRoomService.java b/src/main/java/de/juplo/kafka/chat/backend/domain/ChatRoomService.java
index 374a442..c70ffe4 100644 (file)
--- a/src/main/java/de/juplo/kafka/chat/backend/domain/ChatRoomService.java
+++ b/src/main/java/de/juplo/kafka/chat/backend/domain/ChatRoomService.java
@@ -8,7 +8,7 @@ import java.time.LocalDateTime;
 
 public interface ChatRoomService
 {
-  Mono<Message> persistMessage(
+  Message persistMessage(
       Message.MessageKey key,
       LocalDateTime timestamp,
       String text);

diff --git a/src/main/java/de/juplo/kafka/chat/backend/domain/MessageMutationException.java b/src/main/java/de/juplo/kafka/chat/backend/domain/MessageMutationException.java
index 0408de3..3f02792 100644 (file)
--- a/src/main/java/de/juplo/kafka/chat/backend/domain/MessageMutationException.java
+++ b/src/main/java/de/juplo/kafka/chat/backend/domain/MessageMutationException.java
@@ -5,15 +5,15 @@ import lombok.Getter;
 
 public class MessageMutationException extends RuntimeException
 {
-  @Getter
-  private final Message mutated;
   @Getter
   private final Message existing;
+  @Getter
+  private final String mutatedText;
 
-  public MessageMutationException(Message mutated, Message existing)
+  public MessageMutationException(Message existing, String mutatedText)
   {
     super("Messages are imutable!");
-    this.mutated = mutated;
     this.existing = existing;
+    this.mutatedText = mutatedText;
   }
 }

diff --git a/src/main/java/de/juplo/kafka/chat/backend/persistence/InMemoryChatRoomService.java b/src/main/java/de/juplo/kafka/chat/backend/persistence/InMemoryChatRoomService.java
index 1831037..49d400b 100644 (file)
--- a/src/main/java/de/juplo/kafka/chat/backend/persistence/InMemoryChatRoomService.java
+++ b/src/main/java/de/juplo/kafka/chat/backend/persistence/InMemoryChatRoomService.java
@@ -1,7 +1,6 @@
 package de.juplo.kafka.chat.backend.persistence;
 
 import de.juplo.kafka.chat.backend.domain.Message;
-import de.juplo.kafka.chat.backend.domain.MessageMutationException;
 import de.juplo.kafka.chat.backend.domain.ChatRoomService;
 import lombok.extern.slf4j.Slf4j;
 import reactor.core.publisher.Flux;
@@ -26,32 +25,17 @@ public class InMemoryChatRoomService implements ChatRoomService
   {
     log.debug("Creating InMemoryChatroomService");
     messages = new LinkedHashMap<>();
-    messageFlux.subscribe(message -> persistMessage(message));
+    messageFlux.subscribe(message -> messages.put(message.getKey(), message));
   }
 
   @Override
-  public Mono<Message> persistMessage(
+  public Message persistMessage(
       Message.MessageKey key,
       LocalDateTime timestamp,
       String text)
   {
     Message message = new Message(key, (long)messages.size(), timestamp, text);
-    return Mono.justOrEmpty(persistMessage(message));
-  }
-
-  private Message persistMessage(Message message)
-  {
-    Message.MessageKey key = message.getKey();
-    Message existing = messages.get(key);
-    if (existing != null)
-    {
-      log.info("Message with key {} already exists; {}", key, existing);
-      if (!message.equals(existing))
-        throw new MessageMutationException(message, existing);
-      return null;
-    }
-
-    messages.put(key, message);
+    messages.put(message.getKey(), message);
     return message;
   }
 

diff --git a/src/test/java/de/juplo/kafka/chat/backend/api/ChatBackendControllerTest.java b/src/test/java/de/juplo/kafka/chat/backend/api/ChatBackendControllerTest.java
index 404fab2..8947a03 100644 (file)
--- a/src/test/java/de/juplo/kafka/chat/backend/api/ChatBackendControllerTest.java
+++ b/src/test/java/de/juplo/kafka/chat/backend/api/ChatBackendControllerTest.java
@@ -13,7 +13,6 @@ import org.springframework.test.web.reactive.server.WebTestClient;
 import reactor.core.publisher.Mono;
 
 import java.time.LocalDateTime;
-import java.util.Optional;
 import java.util.UUID;
 
 import static org.mockito.ArgumentMatchers.any;
@@ -167,10 +166,9 @@ public class ChatBackendControllerTest
         .thenReturn(Mono.just(chatRoom));
     Message.MessageKey key = Message.MessageKey.of("foo", 1l);
     LocalDateTime timestamp = LocalDateTime.now();
-    Message mutated = new Message(key, 0l, timestamp, "Mutated!");
     Message existing = new Message(key, 0l, timestamp, "Existing");
     when(chatRoom.addMessage(any(Long.class), any(String.class), any(String.class)))
-        .thenThrow(new MessageMutationException(mutated, existing));
+        .thenReturn(Mono.error(() -> new MessageMutationException(existing, "Mutated!")));
 
     // When
     client
@@ -187,7 +185,7 @@ public class ChatBackendControllerTest
         .expectStatus().is4xxClientError()
         .expectBody()
         .jsonPath("$.type").isEqualTo("/problem/message-mutation")
-        .jsonPath("$.mutatedMessage.text").isEqualTo("Mutated!")
-        .jsonPath("$.existingMessage.text").isEqualTo("Existing");
+        .jsonPath("$.existingMessage.text").isEqualTo("Existing")
+        .jsonPath("$.mutatedText").isEqualTo("Mutated!");
   }
 }

diff --git a/src/test/java/de/juplo/kafka/chat/backend/domain/ChatRoomTest.java b/src/test/java/de/juplo/kafka/chat/backend/domain/ChatRoomTest.java
new file mode 100644 (file)
index 0000000..f513c40
--- /dev/null
+++ b/src/test/java/de/juplo/kafka/chat/backend/domain/ChatRoomTest.java
@@ -0,0 +1,130 @@
+package de.juplo.kafka.chat.backend.domain;
+
+import org.junit.jupiter.api.DisplayName;
+import org.junit.jupiter.api.Test;
+import reactor.core.publisher.Mono;
+
+import java.time.Clock;
+import java.time.Instant;
+import java.time.LocalDateTime;
+import java.time.ZoneId;
+import java.util.UUID;
+
+import static org.mockito.Mockito.*;
+import static pl.rzrz.assertj.reactor.Assertions.assertThat;
+
+
+public class ChatRoomTest
+{
+  @Test
+  @DisplayName("Assert, that Mono emits expected message, if it exists")
+  void testGetExistingMessage()
+  {
+    // Given
+    String user = "foo";
+    Long messageId = 1l;
+    ChatRoomService chatRoomService = mock(ChatRoomService.class);
+    ChatRoom chatRoom = new ChatRoom(UUID.randomUUID(), "Foo", Clock.systemDefaultZone(), chatRoomService, 8);
+    Message.MessageKey key = Message.MessageKey.of(user, messageId);
+    LocalDateTime timestamp = LocalDateTime.now();
+    Message message = new Message(key, 0l, timestamp, "Bar");
+    when(chatRoomService.getMessage(any(Message.MessageKey.class))).thenReturn(Mono.just(message));
+
+    // When
+    Mono<Message> mono = chatRoom.getMessage(user, messageId);
+
+    // Then
+    assertThat(mono).emitsExactly(message);
+  }
+
+  @Test
+  @DisplayName("Assert, that Mono if empty, if message does not exists")
+  void testGetNonExistentMessage()
+  {
+    // Given
+    String user = "foo";
+    Long messageId = 1l;
+    ChatRoomService chatRoomService = mock(ChatRoomService.class);
+    ChatRoom chatRoom = new ChatRoom(UUID.randomUUID(), "Foo", Clock.systemDefaultZone(), chatRoomService, 8);
+    when(chatRoomService.getMessage(any(Message.MessageKey.class))).thenReturn(Mono.empty());
+
+    // When
+    Mono<Message> mono = chatRoom.getMessage(user, messageId);
+
+    // Then
+    assertThat(mono).emitsCount(0);
+  }
+
+  @Test
+  @DisplayName("Assert, that Mono emits expected message, if a new message is added")
+  void testAddNewMessage()
+  {
+    // Given
+    String user = "foo";
+    Long messageId = 1l;
+    ChatRoomService chatRoomService = mock(ChatRoomService.class);
+    ChatRoom chatRoom = new ChatRoom(UUID.randomUUID(), "Foo", Clock.systemDefaultZone(), chatRoomService, 8);
+    Message.MessageKey key = Message.MessageKey.of(user, messageId);
+    Clock now = Clock.fixed(Instant.now(), ZoneId.systemDefault());
+    LocalDateTime timestamp = LocalDateTime.now(now);
+    String messageText = "Bar";
+    Message message = new Message(key, 0l, timestamp, messageText);
+    when(chatRoomService.getMessage(any(Message.MessageKey.class))).thenReturn(Mono.empty());
+    when(chatRoomService.persistMessage(any(Message.MessageKey.class), any(LocalDateTime.class), any(String.class))).thenReturn(message);
+
+    // When
+    Mono<Message> mono = chatRoom.addMessage(messageId, user, messageText);
+
+    // Then
+    assertThat(mono).emitsExactly(message);
+  }
+
+  @Test
+  @DisplayName("Assert, that Mono emits expected message, if an unchanged message is added")
+  void testAddUnchangedMessage()
+  {
+    // Given
+    String user = "foo";
+    Long messageId = 1l;
+    ChatRoomService chatRoomService = mock(ChatRoomService.class);
+    ChatRoom chatRoom = new ChatRoom(UUID.randomUUID(), "Foo", Clock.systemDefaultZone(), chatRoomService, 8);
+    Message.MessageKey key = Message.MessageKey.of(user, messageId);
+    Clock now = Clock.fixed(Instant.now(), ZoneId.systemDefault());
+    LocalDateTime timestamp = LocalDateTime.now(now);
+    String messageText = "Bar";
+    Message message = new Message(key, 0l, timestamp, messageText);
+    when(chatRoomService.getMessage(any(Message.MessageKey.class))).thenReturn(Mono.just(message));
+    when(chatRoomService.persistMessage(any(Message.MessageKey.class), any(LocalDateTime.class), any(String.class))).thenReturn(message);
+
+    // When
+    Mono<Message> mono = chatRoom.addMessage(messageId, user, messageText);
+
+    // Then
+    assertThat(mono).emitsExactly(message);
+  }
+
+  @Test
+  @DisplayName("Assert, that Mono sends an error, if a message is added again with mutated text")
+  void testAddMutatedMessage()
+  {
+    // Given
+    String user = "foo";
+    Long messageId = 1l;
+    ChatRoomService chatRoomService = mock(ChatRoomService.class);
+    ChatRoom chatRoom = new ChatRoom(UUID.randomUUID(), "Foo", Clock.systemDefaultZone(), chatRoomService, 8);
+    Message.MessageKey key = Message.MessageKey.of(user, messageId);
+    Clock now = Clock.fixed(Instant.now(), ZoneId.systemDefault());
+    LocalDateTime timestamp = LocalDateTime.now(now);
+    String messageText = "Bar";
+    String mutatedText = "Boom!";
+    Message message = new Message(key, 0l, timestamp, messageText);
+    when(chatRoomService.getMessage(any(Message.MessageKey.class))).thenReturn(Mono.just(message));
+    when(chatRoomService.persistMessage(any(Message.MessageKey.class), any(LocalDateTime.class), any(String.class))).thenReturn(message);
+
+    // When
+    Mono<Message> mono = chatRoom.addMessage(messageId, user, mutatedText);
+
+    // Then
+    assertThat(mono).sendsError();
+  }
+}