diff --git a/advisors/spring-ai-advisors-vector-store/src/main/java/org/springframework/ai/chat/client/advisor/vectorstore/VectorStoreChatMemoryAdvisor.java b/advisors/spring-ai-advisors-vector-store/src/main/java/org/springframework/ai/chat/client/advisor/vectorstore/VectorStoreChatMemoryAdvisor.java index a076d1788..16bbe97f3 100644 --- a/advisors/spring-ai-advisors-vector-store/src/main/java/org/springframework/ai/chat/client/advisor/vectorstore/VectorStoreChatMemoryAdvisor.java +++ b/advisors/spring-ai-advisors-vector-store/src/main/java/org/springframework/ai/chat/client/advisor/vectorstore/VectorStoreChatMemoryAdvisor.java @@ -21,10 +21,8 @@ import java.util.HashMap; import java.util.List; import java.util.Map; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; +import org.springframework.util.Assert; import reactor.core.scheduler.Scheduler; -import reactor.core.scheduler.Schedulers; import org.springframework.ai.chat.client.ChatClientRequest; import org.springframework.ai.chat.client.ChatClientResponse; @@ -54,16 +52,13 @@ import org.springframework.ai.vectorstore.VectorStore; */ public class VectorStoreChatMemoryAdvisor implements BaseChatMemoryAdvisor { - public static final String CHAT_MEMORY_RETRIEVE_SIZE_KEY = "chat_memory_response_size"; + public static final String TOP_K = "chat_memory_vector_store_top_k"; private static final String DOCUMENT_METADATA_CONVERSATION_ID = "conversationId"; private static final String DOCUMENT_METADATA_MESSAGE_TYPE = "messageType"; - /** - * The default chat memory retrieve size to use when no retrieve size is provided. - */ - public static final int DEFAULT_TOP_K = 20; + private static final int DEFAULT_TOP_K = 20; private static final PromptTemplate DEFAULT_SYSTEM_PROMPT_TEMPLATE = new PromptTemplate(""" {instructions} @@ -78,7 +73,7 @@ public class VectorStoreChatMemoryAdvisor implements BaseChatMemoryAdvisor { private final PromptTemplate systemPromptTemplate; - protected final int defaultChatMemoryRetrieveSize; + private final int defaultTopK; private final String defaultConversationId; @@ -86,12 +81,17 @@ public class VectorStoreChatMemoryAdvisor implements BaseChatMemoryAdvisor { private final Scheduler scheduler; - private VectorStore vectorStore; + private final VectorStore vectorStore; - public VectorStoreChatMemoryAdvisor(PromptTemplate systemPromptTemplate, int defaultChatMemoryRetrieveSize, + private VectorStoreChatMemoryAdvisor(PromptTemplate systemPromptTemplate, int defaultTopK, String defaultConversationId, int order, Scheduler scheduler, VectorStore vectorStore) { + Assert.notNull(systemPromptTemplate, "systemPromptTemplate cannot be null"); + Assert.isTrue(defaultTopK > 0, "topK must be greater than 0"); + Assert.hasText(defaultConversationId, "defaultConversationId cannot be null or empty"); + Assert.notNull(scheduler, "scheduler cannot be null"); + Assert.notNull(vectorStore, "vectorStore cannot be null"); this.systemPromptTemplate = systemPromptTemplate; - this.defaultChatMemoryRetrieveSize = defaultChatMemoryRetrieveSize; + this.defaultTopK = defaultTopK; this.defaultConversationId = defaultConversationId; this.order = order; this.scheduler = scheduler; @@ -114,7 +114,7 @@ public class VectorStoreChatMemoryAdvisor implements BaseChatMemoryAdvisor { @Override public ChatClientRequest before(ChatClientRequest request, AdvisorChain advisorChain) { - String conversationId = getConversationId(request.context()); + String conversationId = getConversationId(request.context(), this.defaultConversationId); String query = request.prompt().getUserMessage() != null ? request.prompt().getUserMessage().getText() : ""; int topK = getChatMemoryTopK(request.context()); String filter = DOCUMENT_METADATA_CONVERSATION_ID + "=='" + conversationId + "'"; @@ -149,9 +149,7 @@ public class VectorStoreChatMemoryAdvisor implements BaseChatMemoryAdvisor { } private int getChatMemoryTopK(Map context) { - return context.containsKey(CHAT_MEMORY_RETRIEVE_SIZE_KEY) - ? Integer.parseInt(context.get(CHAT_MEMORY_RETRIEVE_SIZE_KEY).toString()) - : this.defaultChatMemoryRetrieveSize; + return context.containsKey(TOP_K) ? Integer.parseInt(context.get(TOP_K).toString()) : this.defaultTopK; } @Override @@ -164,7 +162,8 @@ public class VectorStoreChatMemoryAdvisor implements BaseChatMemoryAdvisor { .map(g -> (Message) g.getOutput()) .toList(); } - this.vectorStore.write(toDocuments(assistantMessages, this.getConversationId(chatClientResponse.context()))); + this.vectorStore.write(toDocuments(assistantMessages, + this.getConversationId(chatClientResponse.context(), this.defaultConversationId))); return chatClientResponse; } @@ -202,11 +201,11 @@ public class VectorStoreChatMemoryAdvisor implements BaseChatMemoryAdvisor { private PromptTemplate systemPromptTemplate = DEFAULT_SYSTEM_PROMPT_TEMPLATE; - private Integer topK = DEFAULT_TOP_K; + private Integer defaultTopK = DEFAULT_TOP_K; private String conversationId = ChatMemory.DEFAULT_CONVERSATION_ID; - private Scheduler scheduler; + private Scheduler scheduler = BaseAdvisor.DEFAULT_SCHEDULER; private int order = Advisor.DEFAULT_CHAT_MEMORY_PRECEDENCE_ORDER; @@ -232,11 +231,11 @@ public class VectorStoreChatMemoryAdvisor implements BaseChatMemoryAdvisor { /** * Set the chat memory retrieve size. - * @param topK the chat memory retrieve size + * @param defaultTopK the chat memory retrieve size * @return this builder */ - public Builder topK(int topK) { - this.topK = topK; + public Builder defaultTopK(int defaultTopK) { + this.defaultTopK = defaultTopK; return this; } @@ -250,16 +249,6 @@ public class VectorStoreChatMemoryAdvisor implements BaseChatMemoryAdvisor { return this; } - /** - * Set whether to protect from blocking. - * @param protectFromBlocking whether to protect from blocking - * @return the builder - */ - public Builder protectFromBlocking(boolean protectFromBlocking) { - this.scheduler = protectFromBlocking ? BaseAdvisor.DEFAULT_SCHEDULER : Schedulers.immediate(); - return this; - } - public Builder scheduler(Scheduler scheduler) { this.scheduler = scheduler; return this; @@ -280,7 +269,7 @@ public class VectorStoreChatMemoryAdvisor implements BaseChatMemoryAdvisor { * @return the advisor */ public VectorStoreChatMemoryAdvisor build() { - return new VectorStoreChatMemoryAdvisor(this.systemPromptTemplate, this.topK, this.conversationId, + return new VectorStoreChatMemoryAdvisor(this.systemPromptTemplate, this.defaultTopK, this.conversationId, this.order, this.scheduler, this.vectorStore); } diff --git a/advisors/spring-ai-advisors-vector-store/src/test/java/org/springframework/ai/chat/client/advisor/vectorstore/VectorStoreChatMemoryAdvisorTests.java b/advisors/spring-ai-advisors-vector-store/src/test/java/org/springframework/ai/chat/client/advisor/vectorstore/VectorStoreChatMemoryAdvisorTests.java new file mode 100644 index 000000000..a8ec91282 --- /dev/null +++ b/advisors/spring-ai-advisors-vector-store/src/test/java/org/springframework/ai/chat/client/advisor/vectorstore/VectorStoreChatMemoryAdvisorTests.java @@ -0,0 +1,77 @@ +package org.springframework.ai.chat.client.advisor.vectorstore; + +import org.junit.jupiter.api.Test; +import org.mockito.Mockito; +import org.springframework.ai.vectorstore.VectorStore; + +import static org.assertj.core.api.Assertions.assertThatThrownBy; + +/** + * Unit tests for {@link VectorStoreChatMemoryAdvisor}. + * + * @author Thomas Vitale + */ +class VectorStoreChatMemoryAdvisorTests { + + @Test + void whenVectorStoreIsNullThenThrow() { + assertThatThrownBy(() -> VectorStoreChatMemoryAdvisor.builder(null).build()) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("vectorStore cannot be null"); + } + + @Test + void whenDefaultConversationIdIsNullThenThrow() { + VectorStore vectorStore = Mockito.mock(VectorStore.class); + + assertThatThrownBy(() -> VectorStoreChatMemoryAdvisor.builder(vectorStore).conversationId(null).build()) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("defaultConversationId cannot be null or empty"); + } + + @Test + void whenDefaultConversationIdIsEmptyThenThrow() { + VectorStore vectorStore = Mockito.mock(VectorStore.class); + + assertThatThrownBy(() -> VectorStoreChatMemoryAdvisor.builder(vectorStore).conversationId(null).build()) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("defaultConversationId cannot be null or empty"); + } + + @Test + void whenSchedulerIsNullThenThrow() { + VectorStore vectorStore = Mockito.mock(VectorStore.class); + + assertThatThrownBy(() -> VectorStoreChatMemoryAdvisor.builder(vectorStore).scheduler(null).build()) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("scheduler cannot be null"); + } + + @Test + void whenSystemPromptTemplateIsNullThenThrow() { + VectorStore vectorStore = Mockito.mock(VectorStore.class); + + assertThatThrownBy(() -> VectorStoreChatMemoryAdvisor.builder(vectorStore).systemPromptTemplate(null).build()) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("systemPromptTemplate cannot be null"); + } + + @Test + void whenDefaultTopKIsZeroThenThrow() { + VectorStore vectorStore = Mockito.mock(VectorStore.class); + + assertThatThrownBy(() -> VectorStoreChatMemoryAdvisor.builder(vectorStore).defaultTopK(0).build()) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("topK must be greater than 0"); + } + + @Test + void whenDefaultTopKIsNegativeThenThrow() { + VectorStore vectorStore = Mockito.mock(VectorStore.class); + + assertThatThrownBy(() -> VectorStoreChatMemoryAdvisor.builder(vectorStore).defaultTopK(-1).build()) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("topK must be greater than 0"); + } + +} diff --git a/spring-ai-client-chat/src/main/java/org/springframework/ai/chat/client/advisor/MessageChatMemoryAdvisor.java b/spring-ai-client-chat/src/main/java/org/springframework/ai/chat/client/advisor/MessageChatMemoryAdvisor.java index 9e68742f1..b2b0278f2 100644 --- a/spring-ai-client-chat/src/main/java/org/springframework/ai/chat/client/advisor/MessageChatMemoryAdvisor.java +++ b/spring-ai-client-chat/src/main/java/org/springframework/ai/chat/client/advisor/MessageChatMemoryAdvisor.java @@ -18,12 +18,9 @@ package org.springframework.ai.chat.client.advisor; import java.util.ArrayList; import java.util.List; -import java.util.Map; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; +import org.springframework.util.Assert; import reactor.core.scheduler.Scheduler; -import reactor.core.scheduler.Schedulers; import org.springframework.ai.chat.client.ChatClientRequest; import org.springframework.ai.chat.client.ChatClientResponse; @@ -40,12 +37,11 @@ import org.springframework.ai.chat.messages.UserMessage; * * @author Christian Tzolov * @author Mark Pollack + * @author Thomas Vitale * @since 1.0.0 */ public class MessageChatMemoryAdvisor implements BaseChatMemoryAdvisor { - private static final Logger logger = LoggerFactory.getLogger(MessageChatMemoryAdvisor.class); - private final ChatMemory chatMemory; private final String defaultConversationId; @@ -56,6 +52,9 @@ public class MessageChatMemoryAdvisor implements BaseChatMemoryAdvisor { private MessageChatMemoryAdvisor(ChatMemory chatMemory, String defaultConversationId, int order, Scheduler scheduler) { + Assert.notNull(chatMemory, "chatMemory cannot be null"); + Assert.hasText(defaultConversationId, "defaultConversationId cannot be null or empty"); + Assert.notNull(scheduler, "scheduler cannot be null"); this.chatMemory = chatMemory; this.defaultConversationId = defaultConversationId; this.order = order; @@ -74,7 +73,7 @@ public class MessageChatMemoryAdvisor implements BaseChatMemoryAdvisor { @Override public ChatClientRequest before(ChatClientRequest chatClientRequest, AdvisorChain advisorChain) { - String conversationId = getConversationId(chatClientRequest.context()); + String conversationId = getConversationId(chatClientRequest.context(), this.defaultConversationId); // 1. Retrieve the chat memory for the current conversation. List memoryMessages = this.chatMemory.get(conversationId); @@ -105,7 +104,8 @@ public class MessageChatMemoryAdvisor implements BaseChatMemoryAdvisor { .map(g -> (Message) g.getOutput()) .toList(); } - this.chatMemory.add(this.getConversationId(chatClientResponse.context()), assistantMessages); + this.chatMemory.add(this.getConversationId(chatClientResponse.context(), this.defaultConversationId), + assistantMessages); return chatClientResponse; } @@ -119,7 +119,7 @@ public class MessageChatMemoryAdvisor implements BaseChatMemoryAdvisor { private int order = Advisor.DEFAULT_CHAT_MEMORY_PRECEDENCE_ORDER; - private Scheduler scheduler; + private Scheduler scheduler = BaseAdvisor.DEFAULT_SCHEDULER; private ChatMemory chatMemory; @@ -137,16 +137,6 @@ public class MessageChatMemoryAdvisor implements BaseChatMemoryAdvisor { return this; } - /** - * Set whether to protect from blocking. - * @param protectFromBlocking whether to protect from blocking - * @return the builder - */ - public Builder protectFromBlocking(boolean protectFromBlocking) { - this.scheduler = protectFromBlocking ? BaseAdvisor.DEFAULT_SCHEDULER : Schedulers.immediate(); - return this; - } - /** * Set the order. * @param order the order diff --git a/spring-ai-client-chat/src/main/java/org/springframework/ai/chat/client/advisor/PromptChatMemoryAdvisor.java b/spring-ai-client-chat/src/main/java/org/springframework/ai/chat/client/advisor/PromptChatMemoryAdvisor.java index b30c8b197..bd5f6b0bd 100644 --- a/spring-ai-client-chat/src/main/java/org/springframework/ai/chat/client/advisor/PromptChatMemoryAdvisor.java +++ b/spring-ai-client-chat/src/main/java/org/springframework/ai/chat/client/advisor/PromptChatMemoryAdvisor.java @@ -23,10 +23,10 @@ import java.util.stream.Collectors; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import org.springframework.util.Assert; import reactor.core.publisher.Flux; import reactor.core.publisher.Mono; import reactor.core.scheduler.Scheduler; -import reactor.core.scheduler.Schedulers; import org.springframework.ai.chat.client.ChatClientMessageAggregator; import org.springframework.ai.chat.client.ChatClientRequest; @@ -80,6 +80,10 @@ public class PromptChatMemoryAdvisor implements BaseChatMemoryAdvisor { private PromptChatMemoryAdvisor(ChatMemory chatMemory, String defaultConversationId, int order, Scheduler scheduler, PromptTemplate systemPromptTemplate) { + Assert.notNull(chatMemory, "chatMemory cannot be null"); + Assert.hasText(defaultConversationId, "defaultConversationId cannot be null or empty"); + Assert.notNull(scheduler, "scheduler cannot be null"); + Assert.notNull(systemPromptTemplate, "systemPromptTemplate cannot be null"); this.chatMemory = chatMemory; this.defaultConversationId = defaultConversationId; this.order = order; @@ -103,7 +107,7 @@ public class PromptChatMemoryAdvisor implements BaseChatMemoryAdvisor { @Override public ChatClientRequest before(ChatClientRequest chatClientRequest, AdvisorChain advisorChain) { - String conversationId = getConversationId(chatClientRequest.context()); + String conversationId = getConversationId(chatClientRequest.context(), this.defaultConversationId); // 1. Retrieve the chat memory for the current conversation. List memoryMessages = this.chatMemory.get(conversationId); logger.debug("[PromptChatMemoryAdvisor.before] Memory before processing for conversationId={}: {}", @@ -151,12 +155,15 @@ public class PromptChatMemoryAdvisor implements BaseChatMemoryAdvisor { } if (!assistantMessages.isEmpty()) { - this.chatMemory.add(this.getConversationId(chatClientResponse.context()), assistantMessages); + this.chatMemory.add(this.getConversationId(chatClientResponse.context(), this.defaultConversationId), + assistantMessages); logger.debug("[PromptChatMemoryAdvisor.after] Added ASSISTANT messages to memory for conversationId={}: {}", - this.getConversationId(chatClientResponse.context()), assistantMessages); - List memoryMessages = this.chatMemory.get(this.getConversationId(chatClientResponse.context())); + this.getConversationId(chatClientResponse.context(), this.defaultConversationId), + assistantMessages); + List memoryMessages = this.chatMemory + .get(this.getConversationId(chatClientResponse.context(), this.defaultConversationId)); logger.debug("[PromptChatMemoryAdvisor.after] Memory after ASSISTANT add for conversationId={}: {}", - this.getConversationId(chatClientResponse.context()), memoryMessages); + this.getConversationId(chatClientResponse.context(), this.defaultConversationId), memoryMessages); } return chatClientResponse; } @@ -215,16 +222,6 @@ public class PromptChatMemoryAdvisor implements BaseChatMemoryAdvisor { return this; } - /** - * Set whether to protect from blocking. - * @param protectFromBlocking whether to protect from blocking - * @return the builder - */ - public Builder protectFromBlocking(boolean protectFromBlocking) { - this.scheduler = protectFromBlocking ? BaseAdvisor.DEFAULT_SCHEDULER : Schedulers.immediate(); - return this; - } - public Builder scheduler(Scheduler scheduler) { this.scheduler = scheduler; return this; diff --git a/spring-ai-client-chat/src/main/java/org/springframework/ai/chat/client/advisor/api/BaseChatMemoryAdvisor.java b/spring-ai-client-chat/src/main/java/org/springframework/ai/chat/client/advisor/api/BaseChatMemoryAdvisor.java index ef20dd3a0..b05cc0963 100644 --- a/spring-ai-client-chat/src/main/java/org/springframework/ai/chat/client/advisor/api/BaseChatMemoryAdvisor.java +++ b/spring-ai-client-chat/src/main/java/org/springframework/ai/chat/client/advisor/api/BaseChatMemoryAdvisor.java @@ -1,5 +1,5 @@ /* - * Copyright 2024 - 2024 the original author or authors. + * Copyright 2024 - 2025 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -18,11 +18,13 @@ package org.springframework.ai.chat.client.advisor.api; import java.util.Map; import org.springframework.ai.chat.memory.ChatMemory; +import org.springframework.util.Assert; /** - * Base interface for {@link ChatMemory} backed advisors. + * Base interface for chat memory advisors. * - * @author Codi + * @author Mark Pollack + * @author Thomas Vitale * @since 1.0 */ public interface BaseChatMemoryAdvisor extends BaseAdvisor { @@ -30,12 +32,13 @@ public interface BaseChatMemoryAdvisor extends BaseAdvisor { /** * Retrieve the conversation ID from the given context or return the default * conversation ID when not found. - * @param context the context to retrieve the conversation ID from. - * @return the conversation ID. */ - default String getConversationId(Map context) { - return context != null && context.containsKey(ChatMemory.CONVERSATION_ID) - ? context.get(ChatMemory.CONVERSATION_ID).toString() : ChatMemory.DEFAULT_CONVERSATION_ID; + default String getConversationId(Map context, String defaultConversationId) { + Assert.notNull(context, "context cannot be null"); + Assert.noNullElements(context.keySet().toArray(), "context cannot contain null keys"); + Assert.hasText(defaultConversationId, "defaultConversationId cannot be null or empty"); + return context.containsKey(ChatMemory.CONVERSATION_ID) ? context.get(ChatMemory.CONVERSATION_ID).toString() + : defaultConversationId; } } diff --git a/spring-ai-client-chat/src/test/java/org/springframework/ai/chat/client/advisor/MessageChatMemoryAdvisorTests.java b/spring-ai-client-chat/src/test/java/org/springframework/ai/chat/client/advisor/MessageChatMemoryAdvisorTests.java index 546d220b2..4e1125aa7 100644 --- a/spring-ai-client-chat/src/test/java/org/springframework/ai/chat/client/advisor/MessageChatMemoryAdvisorTests.java +++ b/spring-ai-client-chat/src/test/java/org/springframework/ai/chat/client/advisor/MessageChatMemoryAdvisorTests.java @@ -21,16 +21,53 @@ import org.springframework.ai.chat.client.advisor.api.Advisor; import org.springframework.ai.chat.memory.ChatMemory; import org.springframework.ai.chat.memory.InMemoryChatMemoryRepository; import org.springframework.ai.chat.memory.MessageWindowChatMemory; +import reactor.core.scheduler.Schedulers; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; /** - * Tests for {@link MessageChatMemoryAdvisor} builder method chaining. + * Unit tests for {@link MessageChatMemoryAdvisor}. * * @author Mark Pollack + * @author Thomas Vitale */ public class MessageChatMemoryAdvisorTests { + @Test + void whenChatMemoryIsNullThenThrow() { + assertThatThrownBy(() -> MessageChatMemoryAdvisor.builder(null).build()) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("chatMemory cannot be null"); + } + + @Test + void whenDefaultConversationIdIsNullThenThrow() { + ChatMemory chatMemory = MessageWindowChatMemory.builder().build(); + + assertThatThrownBy(() -> MessageChatMemoryAdvisor.builder(chatMemory).conversationId(null).build()) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("defaultConversationId cannot be null or empty"); + } + + @Test + void whenDefaultConversationIdIsEmptyThenThrow() { + ChatMemory chatMemory = MessageWindowChatMemory.builder().build(); + + assertThatThrownBy(() -> MessageChatMemoryAdvisor.builder(chatMemory).conversationId(null).build()) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("defaultConversationId cannot be null or empty"); + } + + @Test + void whenSchedulerIsNullThenThrow() { + ChatMemory chatMemory = MessageWindowChatMemory.builder().build(); + + assertThatThrownBy(() -> MessageChatMemoryAdvisor.builder(chatMemory).scheduler(null).build()) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("scheduler cannot be null"); + } + @Test void testBuilderMethodChaining() { // Create a chat memory @@ -41,12 +78,11 @@ public class MessageChatMemoryAdvisorTests { // Test builder method chaining with methods from AbstractBuilder String customConversationId = "test-conversation-id"; int customOrder = 42; - boolean customProtectFromBlocking = false; MessageChatMemoryAdvisor advisor = MessageChatMemoryAdvisor.builder(chatMemory) .conversationId(customConversationId) .order(customOrder) - .protectFromBlocking(customProtectFromBlocking) + .scheduler(Schedulers.immediate()) .build(); // Verify the advisor was built with the correct properties diff --git a/spring-ai-client-chat/src/test/java/org/springframework/ai/chat/client/advisor/PromptChatMemoryAdvisorTests.java b/spring-ai-client-chat/src/test/java/org/springframework/ai/chat/client/advisor/PromptChatMemoryAdvisorTests.java index 5bd4ed567..dbfb380a1 100644 --- a/spring-ai-client-chat/src/test/java/org/springframework/ai/chat/client/advisor/PromptChatMemoryAdvisorTests.java +++ b/spring-ai-client-chat/src/test/java/org/springframework/ai/chat/client/advisor/PromptChatMemoryAdvisorTests.java @@ -22,16 +22,62 @@ import org.springframework.ai.chat.memory.ChatMemory; import org.springframework.ai.chat.memory.InMemoryChatMemoryRepository; import org.springframework.ai.chat.memory.MessageWindowChatMemory; import org.springframework.ai.chat.prompt.PromptTemplate; +import reactor.core.scheduler.Schedulers; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; /** - * Tests for {@link PromptChatMemoryAdvisor} builder method chaining. + * Unit tests for {@link PromptChatMemoryAdvisor}. * * @author Mark Pollack + * @author Thomas Vitale */ public class PromptChatMemoryAdvisorTests { + @Test + void whenChatMemoryIsNullThenThrow() { + assertThatThrownBy(() -> PromptChatMemoryAdvisor.builder(null).build()) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("chatMemory cannot be null"); + } + + @Test + void whenDefaultConversationIdIsNullThenThrow() { + ChatMemory chatMemory = MessageWindowChatMemory.builder().build(); + + assertThatThrownBy(() -> PromptChatMemoryAdvisor.builder(chatMemory).conversationId(null).build()) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("defaultConversationId cannot be null or empty"); + } + + @Test + void whenDefaultConversationIdIsEmptyThenThrow() { + ChatMemory chatMemory = MessageWindowChatMemory.builder().build(); + + assertThatThrownBy(() -> PromptChatMemoryAdvisor.builder(chatMemory).conversationId(null).build()) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("defaultConversationId cannot be null or empty"); + } + + @Test + void whenSchedulerIsNullThenThrow() { + ChatMemory chatMemory = MessageWindowChatMemory.builder().build(); + + assertThatThrownBy(() -> PromptChatMemoryAdvisor.builder(chatMemory).scheduler(null).build()) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("scheduler cannot be null"); + } + + @Test + void whenSystemPromptTemplateIsNullThenThrow() { + ChatMemory chatMemory = MessageWindowChatMemory.builder().build(); + + assertThatThrownBy(() -> PromptChatMemoryAdvisor.builder(chatMemory).systemPromptTemplate(null).build()) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("systemPromptTemplate cannot be null"); + } + @Test void testBuilderMethodChaining() { // Create a chat memory @@ -43,13 +89,12 @@ public class PromptChatMemoryAdvisorTests { // PromptChatMemoryAdvisor.Builder String customConversationId = "test-conversation-id"; int customOrder = 42; - boolean customProtectFromBlocking = false; String customSystemPrompt = "Custom system prompt with {instructions} and {memory}"; PromptChatMemoryAdvisor advisor = PromptChatMemoryAdvisor.builder(chatMemory) .conversationId(customConversationId) // From AbstractBuilder .order(customOrder) // From AbstractBuilder - .protectFromBlocking(customProtectFromBlocking) // From AbstractBuilder + .scheduler(Schedulers.immediate()) // From AbstractBuilder .build(); // Verify the advisor was built with the correct properties diff --git a/spring-ai-docs/src/main/antora/modules/ROOT/pages/upgrade-notes.adoc b/spring-ai-docs/src/main/antora/modules/ROOT/pages/upgrade-notes.adoc index d1a122813..6d9c4982f 100644 --- a/spring-ai-docs/src/main/antora/modules/ROOT/pages/upgrade-notes.adoc +++ b/spring-ai-docs/src/main/antora/modules/ROOT/pages/upgrade-notes.adoc @@ -39,9 +39,10 @@ For details, refer to: * The constant `CHAT_MEMORY_CONVERSATION_ID_KEY` has been renamed to `CONVERSATION_ID` and moved from `AbstractChatMemoryAdvisor` to the `ChatMemory` interface. Update your imports to use `org.springframework.ai.chat.memory.ChatMemory.CONVERSATION_ID`. * In `VectorStoreChatMemoryAdvisor`: ** The constant `DEFAULT_CHAT_MEMORY_RESPONSE_SIZE` (value: 100) has been renamed to `DEFAULT_TOP_K` with a new default value of 20. - ** The builder method `chatMemoryRetrieveSize(int)` has been renamed to `topK(int)`. Update your code to use the new method name: `VectorStoreChatMemoryAdvisor.builder(store).topK(1).build()`. + ** The builder method `chatMemoryRetrieveSize(int)` has been renamed to `defaultTopK(int)`. Update your code to use the new method name: `VectorStoreChatMemoryAdvisor.builder(vectorStore).defaultTopK(1).build()`. ** The `systemTextAdvise(String)` builder method has been removed. Use the `systemPromptTemplate(PromptTemplate)` method instead. * In `PromptChatMemoryAdvisor`, the `systemTextAdvise(String)` builder method has been removed. Use the `systemPromptTemplate(PromptTemplate)` method instead. +* In `MessageChatMemoryAdvisor`, `PromptChatMemoryAdvisor`, and `VectorStoreChatMemoryAdvisor`, the `protectFromBlocking(boolean)` method has been removed. Use the `scheduler()` method instead. By default, the advisors protect from blocking, so you don't need to set this method unless you want to disable the protection or customize the Reactor Scheduler. ==== Self-contained Templates in Advisors diff --git a/vector-stores/spring-ai-pgvector-store/src/test/java/org/springframework/ai/vectorstore/pgvector/PgVectorStoreVectorStoreChatMemoryAdvisorIT.java b/vector-stores/spring-ai-pgvector-store/src/test/java/org/springframework/ai/vectorstore/pgvector/PgVectorStoreVectorStoreChatMemoryAdvisorIT.java index 721d63650..ccc91f22d 100644 --- a/vector-stores/spring-ai-pgvector-store/src/test/java/org/springframework/ai/vectorstore/pgvector/PgVectorStoreVectorStoreChatMemoryAdvisorIT.java +++ b/vector-stores/spring-ai-pgvector-store/src/test/java/org/springframework/ai/vectorstore/pgvector/PgVectorStoreVectorStoreChatMemoryAdvisorIT.java @@ -118,7 +118,7 @@ public class PgVectorStoreVectorStoreChatMemoryAdvisorIT { new Document("Dogs are loyal pets.", java.util.Map.of("conversationId", conversationId)))); ChatClient chatClient = ChatClient.builder(chatModel) - .defaultAdvisors(VectorStoreChatMemoryAdvisor.builder(store).topK(1).build()) + .defaultAdvisors(VectorStoreChatMemoryAdvisor.builder(store).defaultTopK(1).build()) .build(); // Send a semantically related query @@ -154,7 +154,7 @@ public class PgVectorStoreVectorStoreChatMemoryAdvisorIT { .of(new Document("Automobiles are fast.", java.util.Map.of("conversationId", conversationId)))); ChatClient chatClient = ChatClient.builder(chatModel) - .defaultAdvisors(VectorStoreChatMemoryAdvisor.builder(store).topK(1).build()) + .defaultAdvisors(VectorStoreChatMemoryAdvisor.builder(store).defaultTopK(1).build()) .build(); String answer = chatClient.prompt() @@ -186,7 +186,7 @@ public class PgVectorStoreVectorStoreChatMemoryAdvisorIT { new Document("Bananas are yellow.", java.util.Map.of("conversationId", conversationId)))); ChatClient chatClient = ChatClient.builder(chatModel) - .defaultAdvisors(VectorStoreChatMemoryAdvisor.builder(store).topK(2).build()) + .defaultAdvisors(VectorStoreChatMemoryAdvisor.builder(store).defaultTopK(2).build()) .build(); String answer = chatClient.prompt() @@ -220,7 +220,7 @@ public class PgVectorStoreVectorStoreChatMemoryAdvisorIT { new Document("Dogs are loyal pets.", java.util.Map.of("conversationId", conversationId)))); ChatClient chatClient = ChatClient.builder(chatModel) - .defaultAdvisors(VectorStoreChatMemoryAdvisor.builder(store).topK(1).build()) + .defaultAdvisors(VectorStoreChatMemoryAdvisor.builder(store).defaultTopK(1).build()) .build(); String answer = chatClient.prompt() @@ -251,7 +251,7 @@ public class PgVectorStoreVectorStoreChatMemoryAdvisorIT { java.util.Map.of("conversationId", conversationId)))); ChatClient chatClient = ChatClient.builder(chatModel) - .defaultAdvisors(VectorStoreChatMemoryAdvisor.builder(store).topK(1).build()) + .defaultAdvisors(VectorStoreChatMemoryAdvisor.builder(store).defaultTopK(1).build()) .build(); String answer = chatClient.prompt() @@ -283,7 +283,7 @@ public class PgVectorStoreVectorStoreChatMemoryAdvisorIT { new Document("Bananas are yellow.", java.util.Map.of("conversationId", conversationId)))); ChatClient chatClient = ChatClient.builder(chatModel) - .defaultAdvisors(VectorStoreChatMemoryAdvisor.builder(store).topK(2).build()) + .defaultAdvisors(VectorStoreChatMemoryAdvisor.builder(store).defaultTopK(2).build()) .build(); String answer = chatClient.prompt() @@ -315,7 +315,7 @@ public class PgVectorStoreVectorStoreChatMemoryAdvisorIT { .of(new Document("The sun is a star.", java.util.Map.of("conversationId", conversationId)))); ChatClient chatClient = ChatClient.builder(chatModel) - .defaultAdvisors(VectorStoreChatMemoryAdvisor.builder(store).topK(1).build()) + .defaultAdvisors(VectorStoreChatMemoryAdvisor.builder(store).defaultTopK(1).build()) .build(); String answer = chatClient.prompt()