From 141fbc52660f607e9bbd3c122ed6422460ec4efb Mon Sep 17 00:00:00 2001 From: Ilayaperumal Gopinathan Date: Fri, 12 Dec 2025 17:35:17 +0000 Subject: [PATCH] Fix ChromaApi exception handling - Uses HttpClientErrorException.NotFound for 404 responses instead of parsing error messages in getCollection, getTenant, and getDatabase methods Signed-off-by: Ilayaperumal Gopinathan --- .../ai/chroma/vectorstore/ChromaApi.java | 23 +-- .../ai/chroma/vectorstore/ChromaApiTest.java | 140 ++++++++++++++++++ 2 files changed, 152 insertions(+), 11 deletions(-) create mode 100644 vector-stores/spring-ai-chroma-store/src/test/java/org/springframework/ai/chroma/vectorstore/ChromaApiTest.java diff --git a/vector-stores/spring-ai-chroma-store/src/main/java/org/springframework/ai/chroma/vectorstore/ChromaApi.java b/vector-stores/spring-ai-chroma-store/src/main/java/org/springframework/ai/chroma/vectorstore/ChromaApi.java index 5c22719d4a0..cdf6292b9dd 100644 --- a/vector-stores/spring-ai-chroma-store/src/main/java/org/springframework/ai/chroma/vectorstore/ChromaApi.java +++ b/vector-stores/spring-ai-chroma-store/src/main/java/org/springframework/ai/chroma/vectorstore/ChromaApi.java @@ -136,11 +136,12 @@ public Tenant getTenant(String tenantName) { .retrieve() .body(Tenant.class); } + catch (HttpClientErrorException.NotFound e) { + // Tenant not found, return null + return null; + } catch (HttpServerErrorException | HttpClientErrorException e) { String msg = this.getErrorMessage(e); - if (String.format("Tenant [%s] not found", tenantName).equals(msg)) { - return null; - } throw new RuntimeException(msg, e); } } @@ -165,11 +166,12 @@ public Database getDatabase(String tenantName, String databaseName) { .retrieve() .body(Database.class); } + catch (HttpClientErrorException.NotFound e) { + // Database not found, return null + return null; + } catch (HttpServerErrorException | HttpClientErrorException e) { String msg = this.getErrorMessage(e); - if (msg.startsWith(String.format("Database [%s] not found.", databaseName))) { - return null; - } throw new RuntimeException(msg, e); } } @@ -226,13 +228,12 @@ public Collection getCollection(String tenantName, String databaseName, String c .retrieve() .body(Collection.class); } + catch (HttpClientErrorException.NotFound e) { + // Collection not found, return null + return null; + } catch (HttpServerErrorException | HttpClientErrorException e) { String msg = this.getErrorMessage(e); - // Handle both "does not exist" and "does not exists" variants from Chroma API - if (String.format("Collection [%s] does not exist", collectionName).equals(msg) - || String.format("Collection [%s] does not exists", collectionName).equals(msg)) { - return null; - } throw new RuntimeException(msg, e); } } diff --git a/vector-stores/spring-ai-chroma-store/src/test/java/org/springframework/ai/chroma/vectorstore/ChromaApiTest.java b/vector-stores/spring-ai-chroma-store/src/test/java/org/springframework/ai/chroma/vectorstore/ChromaApiTest.java new file mode 100644 index 00000000000..0b723ed607a --- /dev/null +++ b/vector-stores/spring-ai-chroma-store/src/test/java/org/springframework/ai/chroma/vectorstore/ChromaApiTest.java @@ -0,0 +1,140 @@ +/* + * Copyright 2025-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. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.ai.chroma.vectorstore; + +import java.io.IOException; + +import okhttp3.mockwebserver.MockResponse; +import okhttp3.mockwebserver.MockWebServer; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +import org.springframework.http.HttpHeaders; +import org.springframework.http.HttpStatus; +import org.springframework.http.MediaType; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; + +/** + * Unit tests for {@link ChromaApi} exception handling. + * + * @author Ilayaperumal Gopinathan + */ +class ChromaApiTest { + + MockWebServer mockWebServer; + + ChromaApi chromaApi; + + @BeforeEach + void setUp() throws IOException { + this.mockWebServer = new MockWebServer(); + this.mockWebServer.start(); + this.chromaApi = ChromaApi.builder().baseUrl(this.mockWebServer.url("/").toString()).build(); + } + + @AfterEach + void tearDown() throws IOException { + this.mockWebServer.shutdown(); + } + + @Test + void getCollectionReturnsNullOn404() { + MockResponse mockResponse = new MockResponse().setResponseCode(HttpStatus.NOT_FOUND.value()) + .addHeader(HttpHeaders.CONTENT_TYPE, MediaType.APPLICATION_JSON_VALUE) + .setBody("{\"error\":\"NotFoundError\",\"message\":\"Collection [test-collection] does not exists\"}"); + this.mockWebServer.enqueue(mockResponse); + + ChromaApi.Collection result = this.chromaApi.getCollection("tenant", "database", "test-collection"); + + assertThat(result).isNull(); + } + + @Test + void getCollectionThrowsOnOtherClientError() { + MockResponse mockResponse = new MockResponse().setResponseCode(HttpStatus.BAD_REQUEST.value()) + .addHeader(HttpHeaders.CONTENT_TYPE, MediaType.APPLICATION_JSON_VALUE) + .setBody("{\"error\":\"BadRequest\",\"message\":\"Invalid request\"}"); + this.mockWebServer.enqueue(mockResponse); + + assertThatThrownBy(() -> this.chromaApi.getCollection("tenant", "database", "test-collection")) + .isInstanceOf(RuntimeException.class) + .hasMessageContaining("Invalid request"); + } + + @Test + void getTenantReturnsNullOn404() { + MockResponse mockResponse = new MockResponse().setResponseCode(HttpStatus.NOT_FOUND.value()) + .addHeader(HttpHeaders.CONTENT_TYPE, MediaType.APPLICATION_JSON_VALUE) + .setBody("{\"error\":\"NotFoundError\",\"message\":\"Tenant [test-tenant] not found\"}"); + this.mockWebServer.enqueue(mockResponse); + + ChromaApi.Tenant result = this.chromaApi.getTenant("test-tenant"); + + assertThat(result).isNull(); + } + + @Test + void getTenantThrowsOnOtherClientError() { + MockResponse mockResponse = new MockResponse().setResponseCode(HttpStatus.FORBIDDEN.value()) + .addHeader(HttpHeaders.CONTENT_TYPE, MediaType.APPLICATION_JSON_VALUE) + .setBody("{\"error\":\"Forbidden\",\"message\":\"Access denied\"}"); + this.mockWebServer.enqueue(mockResponse); + + assertThatThrownBy(() -> this.chromaApi.getTenant("test-tenant")).isInstanceOf(RuntimeException.class) + .hasMessageContaining("Access denied"); + } + + @Test + void getDatabaseReturnsNullOn404() { + MockResponse mockResponse = new MockResponse().setResponseCode(HttpStatus.NOT_FOUND.value()) + .addHeader(HttpHeaders.CONTENT_TYPE, MediaType.APPLICATION_JSON_VALUE) + .setBody("{\"error\":\"NotFoundError\",\"message\":\"Database [test-database] not found.\"}"); + this.mockWebServer.enqueue(mockResponse); + + ChromaApi.Database result = this.chromaApi.getDatabase("tenant", "test-database"); + + assertThat(result).isNull(); + } + + @Test + void getDatabaseThrowsOnOtherClientError() { + MockResponse mockResponse = new MockResponse().setResponseCode(HttpStatus.UNAUTHORIZED.value()) + .addHeader(HttpHeaders.CONTENT_TYPE, MediaType.APPLICATION_JSON_VALUE) + .setBody("{\"error\":\"Unauthorized\",\"message\":\"Authentication required\"}"); + this.mockWebServer.enqueue(mockResponse); + + assertThatThrownBy(() -> this.chromaApi.getDatabase("tenant", "test-database")) + .isInstanceOf(RuntimeException.class) + .hasMessageContaining("Authentication required"); + } + + @Test + void getCollectionThrowsOnServerError() { + MockResponse mockResponse = new MockResponse().setResponseCode(HttpStatus.INTERNAL_SERVER_ERROR.value()) + .addHeader(HttpHeaders.CONTENT_TYPE, MediaType.APPLICATION_JSON_VALUE) + .setBody("{\"error\":\"InternalServerError\",\"message\":\"Internal server error occurred\"}"); + this.mockWebServer.enqueue(mockResponse); + + assertThatThrownBy(() -> this.chromaApi.getCollection("tenant", "database", "test-collection")) + .isInstanceOf(RuntimeException.class) + .hasMessageContaining("Internal server error occurred"); + } + +}