diff --git a/src/main/java/org/example/projectbifrost/BifrostController.java b/src/main/java/org/example/projectbifrost/BifrostController.java index da1ddd3..517a1f6 100644 --- a/src/main/java/org/example/projectbifrost/BifrostController.java +++ b/src/main/java/org/example/projectbifrost/BifrostController.java @@ -19,10 +19,6 @@ public BifrostController(ChatService chatService) { this.chatService = chatService; } - @GetMapping("/bifrost") - public String bifrost() { - return "Welcome to Bifrost, the gateway to the realms!"; - } @PostMapping("/v1/chat") public String sendChatRequest(@Valid @RequestBody ChatRequestDTO dto) { diff --git a/src/main/java/org/example/projectbifrost/dto/ChatRequestDTO.java b/src/main/java/org/example/projectbifrost/dto/ChatRequestDTO.java index db52480..8e2ad5f 100644 --- a/src/main/java/org/example/projectbifrost/dto/ChatRequestDTO.java +++ b/src/main/java/org/example/projectbifrost/dto/ChatRequestDTO.java @@ -8,8 +8,8 @@ * a chat interaction, including the personality of the chatbot, the message * sent by the user, and a session identifier for tracking the conversation. */ -public record ChatRequestDTO(@NotNull Personality personality, - @NotBlank String message, - @NotBlank String sessionId) { +public record ChatRequestDTO(@NotNull(message = "Personality can not be null") Personality personality, + @NotBlank(message = "Message can not be blank") String message, + @NotBlank(message = "sessionId can not be blank") String sessionId) { } diff --git a/src/main/java/org/example/projectbifrost/storage/ChatSessionStorage.java b/src/main/java/org/example/projectbifrost/storage/ChatSessionStorage.java index bc47164..65fedf8 100644 --- a/src/main/java/org/example/projectbifrost/storage/ChatSessionStorage.java +++ b/src/main/java/org/example/projectbifrost/storage/ChatSessionStorage.java @@ -28,8 +28,12 @@ public ChatSession getOrCreateChatSession(String sessionId) { ); } + /** + * Deletes a session by its ID. + * * Note: For production, an automatic cleanup (like a Cache with + * a time limit) would be better to avoid filling up the memory. + */ public void deleteSession(String sessionId) { sessionStorage.remove(sessionId); - //Copilot påpekar att det kan vara bra med en MAX-size på listan, och sköta delete om den blir för stor } } diff --git a/src/test/java/org/example/projectbifrost/controller/BifrostControllerTest.java b/src/test/java/org/example/projectbifrost/controller/BifrostControllerTest.java new file mode 100644 index 0000000..8dfe58e --- /dev/null +++ b/src/test/java/org/example/projectbifrost/controller/BifrostControllerTest.java @@ -0,0 +1,66 @@ +package org.example.projectbifrost.controller; + +import org.example.projectbifrost.BifrostController; +import org.example.projectbifrost.dto.ChatRequestDTO; +import org.example.projectbifrost.service.ChatService; +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Test; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.boot.webmvc.test.autoconfigure.WebMvcTest; +import org.springframework.http.MediaType; +import org.springframework.test.context.bean.override.mockito.MockitoBean; +import org.springframework.test.web.servlet.MockMvc; + +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.when; +import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get; +import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.post; +import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status; + +@WebMvcTest(BifrostController.class) +class BifrostControllerTest { + + @Autowired + private MockMvc mockMvc; + + @MockitoBean + private ChatService chatService; + + @Test + @DisplayName("Should return 200 OK for valid chat request") + void shouldReturn200ForValidRequest() throws Exception { + when(chatService.chatWithLLM(any(ChatRequestDTO.class))).thenReturn("Hello mortal"); + + mockMvc.perform(post("/api/v1/chat") + .contentType(MediaType.APPLICATION_JSON) + .content(""" + { + "personality": "ODIN", + "message": "Hello Odin", + "sessionId": "session123" + } + """)) + .andExpect(status().isOk()); + } + + @Test + @DisplayName("Should return 400 Bad Request when message is missing") + void shouldReturn400WhenMessageIsMissing() throws Exception { + mockMvc.perform(post("/api/v1/chat") + .contentType(MediaType.APPLICATION_JSON) + .content(""" + { + "personality": "ODIN", + "sessionId": "session123" + } + """)) + .andExpect(status().isBadRequest()); // @Valid stops + } + + @Test + @DisplayName("Should return 200 OK when fetching chat history for existing session") + void shouldReturn200ForGetHistory() throws Exception { + mockMvc.perform(get("/api/v1/chat/session123")) + .andExpect(status().isOk()); + } +} diff --git a/src/test/java/org/example/projectbifrost/service/ChatServiceTest.java b/src/test/java/org/example/projectbifrost/service/ChatServiceTest.java index 4121fe0..5547dbf 100644 --- a/src/test/java/org/example/projectbifrost/service/ChatServiceTest.java +++ b/src/test/java/org/example/projectbifrost/service/ChatServiceTest.java @@ -1,7 +1,11 @@ package org.example.projectbifrost.service; import com.github.tomakehurst.wiremock.stubbing.Scenario; +import org.example.projectbifrost.domain.ChatSession; +import org.example.projectbifrost.dto.ChatRequestDTO; import org.example.projectbifrost.dto.OpenRouterRequestDTO; +import org.example.projectbifrost.dto.Personality; +import org.example.projectbifrost.exception.InvalidLLMResponseException; import org.example.projectbifrost.exception.RetryableHttpException; import org.junit.jupiter.api.DisplayName; import org.junit.jupiter.api.Test; @@ -26,6 +30,62 @@ class ChatServiceTest { @Autowired private ChatService chatService; + @Test + @DisplayName("Test complete successful chat flow with session management and message history") + void testSuccessfulChatWithLLM() { + String sessionId = "test-sessionId-1"; + String userMessage = "What is your advice?"; + String llmResponse = "Heed my wisdom, mortal!"; + + stubFor(post(urlEqualTo("/chat/completions")) + .willReturn(aResponse().withStatus(200) + .withHeader("Content-Type", "application/json") + .withBody("{\"choices\": [{\"message\": {\"content\": \"" + llmResponse + "\"}}]}"))); + + String result = chatService.chatWithLLM( + new ChatRequestDTO(Personality.ODIN, userMessage, sessionId) + ); + + // Verify response and history + assertThat(result).isEqualTo(llmResponse); + + ChatSession session = chatService.getSessionHistory(sessionId); + assertThat(session.getChatHistory()).hasSize(2); + assertThat(session.getChatHistory().get(0).getRole()).isEqualTo("user"); + assertThat(session.getChatHistory().get(0).getContent()).isEqualTo(userMessage); + assertThat(session.getChatHistory().get(1).getRole()).isEqualTo("assistant"); + assertThat(session.getChatHistory().get(1).getContent()).isEqualTo(llmResponse); + } + + @Test + @DisplayName("Should throw InvalidLLMResponseException if LLM has empty answer") + void testEmptyAnswerFromLLM() { + //Tell WireMock to send an answer where "choices" is empty [] + stubFor(post(urlEqualTo("/chat/completions")) + .willReturn(aResponse().withStatus(200) + .withHeader("Content-Type", "application/json") + .withBody("{\"choices\": []}"))); + + ChatRequestDTO requestDTO = new ChatRequestDTO(Personality.ODIN, "Hello", "session-empty-llm"); + + assertThatThrownBy(() -> + chatService.chatWithLLM(requestDTO) + ).isInstanceOf(InvalidLLMResponseException.class); + } + + @Test + @DisplayName("Should throw RetryableHttpException if server returns status 500") + void testServerErrorThrowsException() { + stubFor(post(urlEqualTo("/chat/completions")) + .willReturn(aResponse().withStatus(500))); + + ChatRequestDTO requestDTO = new ChatRequestDTO(Personality.ODIN, "Hello", "session-server-error"); + + assertThatThrownBy(() -> + chatService.chatWithLLM(requestDTO) + ).isInstanceOf(RetryableHttpException.class); + } + @Test @DisplayName("Test retry mechanism when LLM service is temporarily unavailable") void testRetry() { @@ -68,6 +128,9 @@ void testRetry() { @Test @DisplayName("Test Circuit Breaker opens after consecutive failures and throws exception") void testCircuitBreakerLogic() throws InterruptedException { + + var messages = List.of(new OpenRouterRequestDTO.Message("user", "Hello")); + stubFor(post("/chat/completions") .willReturn(aResponse().withStatus(500) .withHeader("Content-Type", "application/json") @@ -75,7 +138,7 @@ void testCircuitBreakerLogic() throws InterruptedException { for (int i = 0; i < 10; i++) { try { - chatService.fetchResponseFromLLM(List.of(new OpenRouterRequestDTO.Message("user", "Hello"))); + chatService.fetchResponseFromLLM(messages); } catch (Exception e) { //Ignore exceptions in loop, just let CircuitBreaker register them } @@ -84,8 +147,9 @@ void testCircuitBreakerLogic() throws InterruptedException { // RESET and verify circuit breaker is open, so fallback throws exception resetAllRequests(); + assertThatThrownBy(() -> - chatService.fetchResponseFromLLM(List.of(new OpenRouterRequestDTO.Message("user", "Hello"))) + chatService.fetchResponseFromLLM(messages) ) .isInstanceOf(RetryableHttpException.class) .hasMessageContaining("The Gods are not responding"); diff --git a/src/test/java/org/example/projectbifrost/storage/ChatSessionStorageTest.java b/src/test/java/org/example/projectbifrost/storage/ChatSessionStorageTest.java new file mode 100644 index 0000000..e13e64f --- /dev/null +++ b/src/test/java/org/example/projectbifrost/storage/ChatSessionStorageTest.java @@ -0,0 +1,59 @@ +package org.example.projectbifrost.storage; + +import org.example.projectbifrost.domain.ChatSession; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Test; + +import static org.assertj.core.api.Assertions.assertThat; + +class ChatSessionStorageTest { + private ChatSessionStorage storage; + + @BeforeEach + void setUp() { + storage = new ChatSessionStorage(); + } + + @Test + @DisplayName("Should create a new session if id does not exist") + void shouldCreateNewSessionWhenMissing() { + String sessionId = "new-id"; + + ChatSession session = storage.getOrCreateChatSession(sessionId); + + assertThat(session).isNotNull(); + assertThat(session.getSessionId()).isEqualTo(sessionId); + + assertThat(session.getChatHistory()).isNotNull(); + } + + @Test + @DisplayName("Should return existing session if id exists") + void shouldReturnExistingSession() { + String sessionId = "same-id"; + + ChatSession first = storage.getOrCreateChatSession(sessionId); + ChatSession second = storage.getOrCreateChatSession(sessionId); + + assertThat(second).isSameAs(first); + } + + + @Test + @DisplayName("Should delete session") + void shouldDeleteSession() { + String sessionId = "goodbye-session"; + + storage.getOrCreateChatSession(sessionId); + + storage.deleteSession(sessionId); + + // After delete, when calling getOrCreate it should create a new session, so we can check that it's different from the old one + ChatSession oldSession = storage.getOrCreateChatSession(sessionId); // this session was removed + storage.deleteSession(sessionId); + ChatSession newSession = storage.getOrCreateChatSession(sessionId); + + assertThat(newSession).isNotSameAs(oldSession); + } +}