From 7f4cd2ec3dcf32889765e7ef65efda64dd676ddd Mon Sep 17 00:00:00 2001 From: kuntal1461 Date: Sat, 11 Oct 2025 15:03:59 +0530 Subject: [PATCH] chore: improve readability and fix Sonar suggestions in OpenAiChatModelMutateTests Signed-off-by: Kuntal Maity --- .../autoconfigure/OpenAiPropertiesTests.java | 2 +- .../ai/openai/api/OpenAiApi.java | 45 +++++++++++++++---- .../ai/openai/api/OpenAiFileApi.java | 28 ++++++++++++ .../ai/openai/api/OpenAiApiIT.java | 11 ++--- .../api/OpenAiChatModelMutateTests.java | 7 +-- 5 files changed, 73 insertions(+), 20 deletions(-) diff --git a/auto-configurations/models/spring-ai-autoconfigure-model-openai/src/test/java/org/springframework/ai/model/openai/autoconfigure/OpenAiPropertiesTests.java b/auto-configurations/models/spring-ai-autoconfigure-model-openai/src/test/java/org/springframework/ai/model/openai/autoconfigure/OpenAiPropertiesTests.java index 8b16ce828c2..28378329628 100644 --- a/auto-configurations/models/spring-ai-autoconfigure-model-openai/src/test/java/org/springframework/ai/model/openai/autoconfigure/OpenAiPropertiesTests.java +++ b/auto-configurations/models/spring-ai-autoconfigure-model-openai/src/test/java/org/springframework/ai/model/openai/autoconfigure/OpenAiPropertiesTests.java @@ -377,7 +377,7 @@ public void chatOptionsTest() { "spring.ai.openai.chat.options.topP=0.56", // "spring.ai.openai.chat.options.toolChoice.functionName=toolChoiceFunctionName", - "spring.ai.openai.chat.options.toolChoice=" + ModelOptionsUtils.toJsonString(ToolChoiceBuilder.FUNCTION("toolChoiceFunctionName")), + "spring.ai.openai.chat.options.toolChoice=" + ModelOptionsUtils.toJsonString(ToolChoiceBuilder.function("toolChoiceFunctionName")), "spring.ai.openai.chat.options.tools[0].function.name=myFunction1", "spring.ai.openai.chat.options.tools[0].function.description=function description", diff --git a/models/spring-ai-openai/src/main/java/org/springframework/ai/openai/api/OpenAiApi.java b/models/spring-ai-openai/src/main/java/org/springframework/ai/openai/api/OpenAiApi.java index 8064b070aab..49d42b1e1aa 100644 --- a/models/spring-ai-openai/src/main/java/org/springframework/ai/openai/api/OpenAiApi.java +++ b/models/spring-ai-openai/src/main/java/org/springframework/ai/openai/api/OpenAiApi.java @@ -16,8 +16,10 @@ package org.springframework.ai.openai.api; +import java.util.Arrays; import java.util.List; import java.util.Map; +import java.util.Objects; import java.util.concurrent.atomic.AtomicBoolean; import java.util.function.Consumer; import java.util.function.Predicate; @@ -85,6 +87,12 @@ public static Builder builder() { private static final Predicate SSE_DONE_PREDICATE = "[DONE]"::equals; + private static final String REQUEST_BODY_NULL_MESSAGE = "The request body can not be null."; + + private static final String STREAM_FALSE_MESSAGE = "Request must set the stream property to false."; + + private static final String ADDITIONAL_HEADERS_NULL_MESSAGE = "The additional HTTP headers can not be null."; + // Store config fields for mutate/copy private final String baseUrl; @@ -183,9 +191,9 @@ public ResponseEntity chatCompletionEntity(ChatCompletionRequest public ResponseEntity chatCompletionEntity(ChatCompletionRequest chatRequest, MultiValueMap additionalHttpHeader) { - Assert.notNull(chatRequest, "The request body can not be null."); - Assert.isTrue(!chatRequest.stream(), "Request must set the stream property to false."); - Assert.notNull(additionalHttpHeader, "The additional HTTP headers can not be null."); + Assert.notNull(chatRequest, REQUEST_BODY_NULL_MESSAGE); + Assert.isTrue(!chatRequest.stream(), STREAM_FALSE_MESSAGE); + Assert.notNull(additionalHttpHeader, ADDITIONAL_HEADERS_NULL_MESSAGE); // @formatter:off return this.restClient.post() @@ -221,7 +229,7 @@ public Flux chatCompletionStream(ChatCompletionRequest chat public Flux chatCompletionStream(ChatCompletionRequest chatRequest, MultiValueMap additionalHttpHeader) { - Assert.notNull(chatRequest, "The request body can not be null."); + Assert.notNull(chatRequest, REQUEST_BODY_NULL_MESSAGE); Assert.isTrue(chatRequest.stream(), "Request must set the stream property to true."); AtomicBoolean isInsideTool = new AtomicBoolean(false); @@ -284,7 +292,7 @@ public Flux chatCompletionStream(ChatCompletionRequest chat */ public ResponseEntity> embeddings(EmbeddingRequest embeddingRequest) { - Assert.notNull(embeddingRequest, "The request body can not be null."); + Assert.notNull(embeddingRequest, REQUEST_BODY_NULL_MESSAGE); // Input text to embed, encoded as a string or array of tokens. To embed multiple // inputs in a single @@ -296,7 +304,7 @@ public ResponseEntity> embeddings(EmbeddingRequest< // The input must not exceed the max input tokens for the model (8192 tokens for // text-embedding-ada-002), cannot // be an empty string, and any array must be 2048 dimensions or less. - if (embeddingRequest.input() instanceof List list) { + if (embeddingRequest.input() instanceof List list) { Assert.isTrue(!CollectionUtils.isEmpty(list), "The input list can not be empty."); Assert.isTrue(list.size() <= 2048, "The list must be 2048 dimensions or less"); Assert.isTrue( @@ -1218,7 +1226,7 @@ public static class ToolChoiceBuilder { /** * Specifying a particular function forces the model to call that function. */ - public static Object FUNCTION(String functionName) { + public static Object function(String functionName) { return Map.of("type", "function", "function", Map.of("name", functionName)); } } @@ -1877,7 +1885,9 @@ public record ChunkChoice(// @formatter:off @JsonIgnoreProperties(ignoreUnknown = true) public record Embedding(// @formatter:off @JsonProperty("index") Integer index, - @JsonProperty("embedding") @JsonDeserialize(using = OpenAiEmbeddingDeserializer.class) float[] embedding, + @JsonProperty("embedding") + @JsonDeserialize(using = OpenAiEmbeddingDeserializer.class) + float[] embedding, @JsonProperty("object") String object) { // @formatter:on /** @@ -1891,6 +1901,25 @@ public Embedding(Integer index, float[] embedding) { this(index, embedding, "embedding"); } + @Override + public boolean equals(Object o) { + if (this == o) { + return true; + } + if (!(o instanceof Embedding other)) { + return false; + } + return Objects.equals(this.index, other.index) && Arrays.equals(this.embedding, other.embedding) + && Objects.equals(this.object, other.object); + } + + @Override + public int hashCode() { + int result = Objects.hash(this.index, this.object); + result = 31 * result + Arrays.hashCode(this.embedding); + return result; + } + } /** diff --git a/models/spring-ai-openai/src/main/java/org/springframework/ai/openai/api/OpenAiFileApi.java b/models/spring-ai-openai/src/main/java/org/springframework/ai/openai/api/OpenAiFileApi.java index 8004d8f6686..535b6a9a043 100644 --- a/models/spring-ai-openai/src/main/java/org/springframework/ai/openai/api/OpenAiFileApi.java +++ b/models/spring-ai-openai/src/main/java/org/springframework/ai/openai/api/OpenAiFileApi.java @@ -16,7 +16,9 @@ package org.springframework.ai.openai.api; +import java.util.Arrays; import java.util.List; +import java.util.Objects; import java.util.function.Consumer; import com.fasterxml.jackson.annotation.JsonInclude; @@ -201,6 +203,32 @@ public static Builder builder() { return new Builder(); } + @Override + public boolean equals(Object o) { + if (this == o) { + return true; + } + if (!(o instanceof UploadFileRequest that)) { + return false; + } + return Arrays.equals(this.file, that.file) && Objects.equals(this.fileName, that.fileName) + && Objects.equals(this.purpose, that.purpose); + } + + @Override + public int hashCode() { + int result = Arrays.hashCode(this.file); + result = 31 * result + Objects.hashCode(this.fileName); + result = 31 * result + Objects.hashCode(this.purpose); + return result; + } + + @Override + public String toString() { + return "UploadFileRequest{file=" + Arrays.toString(this.file) + ", fileName=" + + Objects.toString(this.fileName) + ", purpose=" + Objects.toString(this.purpose) + "}"; + } + public static final class Builder { private byte[] file; diff --git a/models/spring-ai-openai/src/test/java/org/springframework/ai/openai/api/OpenAiApiIT.java b/models/spring-ai-openai/src/test/java/org/springframework/ai/openai/api/OpenAiApiIT.java index d050a621034..4a66c37df09 100644 --- a/models/spring-ai-openai/src/test/java/org/springframework/ai/openai/api/OpenAiApiIT.java +++ b/models/spring-ai-openai/src/test/java/org/springframework/ai/openai/api/OpenAiApiIT.java @@ -46,7 +46,7 @@ * @author Alexandros Pappas */ @EnabledIfEnvironmentVariable(named = "OPENAI_API_KEY", matches = ".+") -public class OpenAiApiIT { +class OpenAiApiIT { OpenAiApi openAiApi = OpenAiApi.builder().apiKey(System.getenv("OPENAI_API_KEY")).build(); @@ -117,7 +117,7 @@ void inputAudio() throws IOException { assertThat(response.getBody()).isNotNull(); assertThat(response.getBody().usage().promptTokensDetails().audioTokens()).isGreaterThan(0); - assertThat(response.getBody().usage().completionTokenDetails().audioTokens()).isEqualTo(0); + assertThat(response.getBody().usage().completionTokenDetails().audioTokens()).isZero(); assertThat(response.getBody().choices().get(0).message().content()).containsIgnoringCase("hobbits"); } @@ -135,7 +135,7 @@ void outputAudio() { assertThat(response).isNotNull(); assertThat(response.getBody()).isNotNull(); - assertThat(response.getBody().usage().promptTokensDetails().audioTokens()).isEqualTo(0); + assertThat(response.getBody().usage().promptTokensDetails().audioTokens()).isZero(); assertThat(response.getBody().usage().completionTokenDetails().audioTokens()).isGreaterThan(0); assertThat(response.getBody().choices().get(0).message().audioOutput().data()).isNotNull(); @@ -153,8 +153,9 @@ void streamOutputAudio() { ChatCompletionRequest chatCompletionRequest = new ChatCompletionRequest(List.of(chatCompletionMessage), OpenAiApi.ChatModel.GPT_4_O_AUDIO_PREVIEW.getValue(), audioParameters, true); - assertThatThrownBy(() -> this.openAiApi.chatCompletionStream(chatCompletionRequest).collectList().block()) - .isInstanceOf(RuntimeException.class) + Flux response = this.openAiApi.chatCompletionStream(chatCompletionRequest); + + assertThatThrownBy(response::blockLast).isInstanceOf(RuntimeException.class) .hasMessageContaining("400 Bad Request from POST https://api.openai.com/v1/chat/completions"); } diff --git a/models/spring-ai-openai/src/test/java/org/springframework/ai/openai/api/OpenAiChatModelMutateTests.java b/models/spring-ai-openai/src/test/java/org/springframework/ai/openai/api/OpenAiChatModelMutateTests.java index 706c0ab7f9a..36ca255710e 100644 --- a/models/spring-ai-openai/src/test/java/org/springframework/ai/openai/api/OpenAiChatModelMutateTests.java +++ b/models/spring-ai-openai/src/test/java/org/springframework/ai/openai/api/OpenAiChatModelMutateTests.java @@ -18,7 +18,6 @@ import org.junit.jupiter.api.Test; -import org.springframework.ai.chat.client.ChatClient; import org.springframework.ai.openai.OpenAiChatModel; import org.springframework.ai.openai.OpenAiChatOptions; import org.springframework.util.LinkedMultiValueMap; @@ -52,8 +51,6 @@ void testMutateCreatesDistinctClientsWithDifferentEndpointsAndModels() { .openAiApi(gpt4Api) .defaultOptions(OpenAiChatOptions.builder().model("gpt-4").temperature(0.7).build()) .build(); - ChatClient gpt4Client = ChatClient.builder(gpt4Model).build(); - // Mutate for Llama OpenAiApi llamaApi = this.baseApi.mutate() .baseUrl("https://your-custom-endpoint.com") @@ -63,8 +60,6 @@ void testMutateCreatesDistinctClientsWithDifferentEndpointsAndModels() { .openAiApi(llamaApi) .defaultOptions(OpenAiChatOptions.builder().model("llama-70b").temperature(0.5).build()) .build(); - ChatClient llamaClient = ChatClient.builder(llamaModel).build(); - // Assert endpoints and models are different assertThat(gpt4Model).isNotSameAs(llamaModel); assertThat(gpt4Api).isNotSameAs(llamaApi); @@ -78,7 +73,7 @@ void testMutateCreatesDistinctClientsWithDifferentEndpointsAndModels() { void testCloneCreatesDeepCopy() { OpenAiChatModel clone = this.baseModel.clone(); assertThat(clone).isNotSameAs(this.baseModel); - assertThat(clone.toString()).isEqualTo(this.baseModel.toString()); + assertThat(clone).hasToString(this.baseModel.toString()); } @Test