Spring/spring-ai
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

GH-2165: Simplify VectorStore delete method to return void

Browse Source 
- Change VectorStore.delete() and related implementations to return void instead of Optional<Boolean>
- Remove unnecessary boolean return values and success status checks across all vector store implementations
- Clean up tests by removing redundant assertions
- Implementations continue using runtime exceptions for error signaling

Signed-off-by: Soby Chacko <soby.chacko@broadcom.com>
This commit is contained in:
Soby Chacko
2025-02-03 20:31:50 -05:00
committed by Ilayaperumal Gopinathan
parent b46615921d
commit 6035516044
26 changed files with 43 additions and 127 deletions
Download Patch File Download Diff File Expand all files Collapse all files

3
spring-ai-core/src/main/java/org/springframework/ai/vectorstore/SimpleVectorStore.java
View File

@@ -106,11 +106,10 @@ public class SimpleVectorStore extends AbstractObservationVectorStore {
}
@Override
public Optional<Boolean> doDelete(List<String> idList) {
public void doDelete(List<String> idList) {
for (String id : idList) {
this.store.remove(id);
}
return Optional.of(true);
}
@Override

4
spring-ai-core/src/main/java/org/springframework/ai/vectorstore/VectorStore.java
View File

@@ -59,10 +59,8 @@ public interface VectorStore extends DocumentWriter {
/**
* Deletes documents from the vector store.
* @param idList list of document ids for which documents will be removed.
* @return Returns true if the documents were successfully deleted.
*/
@Nullable
Optional<Boolean> delete(List<String> idList);
void delete(List<String> idList);
/**
* Deletes documents from the vector store based on filter criteria.

8
spring-ai-core/src/main/java/org/springframework/ai/vectorstore/observation/AbstractObservationVectorStore.java
View File

@@ -87,14 +87,13 @@ public abstract class AbstractObservationVectorStore implements VectorStore {
}
@Override
@Nullable
public Optional<Boolean> delete(List<String> deleteDocIds) {
public void delete(List<String> deleteDocIds) {
VectorStoreObservationContext observationContext = this
.createObservationContextBuilder(VectorStoreObservationContext.Operation.DELETE.value())
.build();
return VectorStoreObservationDocumentation.AI_VECTOR_STORE
VectorStoreObservationDocumentation.AI_VECTOR_STORE
.observation(this.customObservationConvention, DEFAULT_OBSERVATION_CONVENTION, () -> observationContext,
this.observationRegistry)
.observe(() -> this.doDelete(deleteDocIds));
@@ -140,9 +139,8 @@ public abstract class AbstractObservationVectorStore implements VectorStore {
/**
* Perform the actual delete operation.
* @param idList the list of document IDs to delete
* @return true if the documents were successfully deleted
*/
public abstract Optional<Boolean> doDelete(List<String> idList);
public abstract void doDelete(List<String> idList);
/**
* Template method for concrete implementations to provide filter-based deletion

5
spring-ai-core/src/test/java/org/springframework/ai/vectorstore/SimpleVectorStoreTests.java
View File

@@ -38,6 +38,7 @@ import org.springframework.core.io.Resource;
import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatThrownBy;
import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;
@@ -112,8 +113,8 @@ class SimpleVectorStoreTests {
@Test
void shouldHandleDeleteOfNonexistentDocument() {
this.vectorStore.delete(List.of("nonexistent-id"));
// Should not throw exception and return true
assertThat(this.vectorStore.delete(List.of("nonexistent-id")).get()).isTrue();
// Should not throw exception
assertDoesNotThrow(() -> this.vectorStore.delete(List.of("nonexistent-id")));
}
@Test

5
vector-stores/spring-ai-azure-cosmos-db-store/src/main/java/org/springframework/ai/vectorstore/cosmosdb/CosmosDBVectorStore.java
View File

@@ -271,7 +271,7 @@ public class CosmosDBVectorStore extends AbstractObservationVectorStore implemen
}
@Override
public Optional<Boolean> doDelete(List<String> idList) {
public void doDelete(List<String> idList) {
try {
// Convert the list of IDs into bulk delete operations
List<CosmosItemOperation> itemOperations = idList.stream()
@@ -285,12 +285,9 @@ public class CosmosDBVectorStore extends AbstractObservationVectorStore implemen
response.getResponse().getStatusCode()))
.doOnError(error -> logger.error("Error deleting document: {}", error.getMessage()))
.blockLast(); // This will block until all operations have finished
return Optional.of(true);
}
catch (Exception e) {
logger.error("Exception while deleting documents: {}", e.getMessage());
return Optional.of(false);
}
}

18
vector-stores/spring-ai-azure-store/src/main/java/org/springframework/ai/vectorstore/azure/AzureVectorStore.java
View File

@@ -188,12 +188,9 @@ public class AzureVectorStore extends AbstractObservationVectorStore implements
}
@Override
public Optional<Boolean> doDelete(List<String> documentIds) {
public void doDelete(List<String> documentIds) {
Assert.notNull(documentIds, "The document ID list should not be null.");
if (CollectionUtils.isEmpty(documentIds)) {
return Optional.of(true); // nothing to do;
}
final var searchDocumentIds = documentIds.stream().map(documentId -> {
SearchDocument searchDocument = new SearchDocument();
@@ -201,18 +198,7 @@ public class AzureVectorStore extends AbstractObservationVectorStore implements
return searchDocument;
}).toList();
var results = this.searchClient.deleteDocuments(searchDocumentIds);
boolean resSuccess = true;
for (IndexingResult result : results.getResults()) {
if (!result.isSucceeded()) {
resSuccess = false;
break;
}
}
return Optional.of(resSuccess);
this.searchClient.deleteDocuments(searchDocumentIds);
}
@Override

11
vector-stores/spring-ai-cassandra-store/src/main/java/org/springframework/ai/vectorstore/cassandra/CassandraVectorStore.java
View File

@@ -305,7 +305,7 @@ public class CassandraVectorStore extends AbstractObservationVectorStore impleme
}
@Override
public Optional<Boolean> doDelete(List<String> idList) {
public void doDelete(List<String> idList) {
CompletableFuture[] futures = new CompletableFuture[idList.size()];
int i = 0;
for (String id : idList) {
@@ -314,7 +314,6 @@ public class CassandraVectorStore extends AbstractObservationVectorStore impleme
futures[i++] = this.session.executeAsync(s).toCompletableFuture();
}
CompletableFuture.allOf(futures).join();
return Optional.of(Boolean.TRUE);
}
@Override
@@ -339,13 +338,7 @@ public class CassandraVectorStore extends AbstractObservationVectorStore impleme
if (!matchingDocs.isEmpty()) {
// Then delete those documents by ID
List<String> idsToDelete = matchingDocs.stream().map(Document::getId).collect(Collectors.toList());
Optional<Boolean> result = delete(idsToDelete);
if (result.isPresent() && !result.get()) {
throw new IllegalStateException("Failed to delete some documents");
}
delete(idsToDelete);
logger.debug("Deleted {} documents matching filter expression", idsToDelete.size());
}
}

5
vector-stores/spring-ai-chroma-store/src/main/java/org/springframework/ai/chroma/vectorstore/ChromaVectorStore.java
View File

@@ -158,10 +158,9 @@ public class ChromaVectorStore extends AbstractObservationVectorStore implements
}
@Override
public Optional<Boolean> doDelete(@NonNull List<String> idList) {
public void doDelete(List<String> idList) {
Assert.notNull(idList, "Document id list must not be null");
int status = this.chromaApi.deleteEmbeddings(this.collectionId, new DeleteEmbeddingsRequest(idList));
return Optional.of(status == 200);
this.chromaApi.deleteEmbeddings(this.collectionId, new DeleteEmbeddingsRequest(idList));
}
@Override

5
vector-stores/spring-ai-chroma-store/src/test/java/org/springframework/ai/chroma/vectorstore/ChromaVectorStoreIT.java
View File

@@ -88,8 +88,7 @@ public class ChromaVectorStoreIT {
assertThat(resultDoc.getMetadata()).containsKeys("meta2", DocumentMetadata.DISTANCE.value());
// Remove all documents from the store
assertThat(vectorStore.delete(this.documents.stream().map(doc -> doc.getId()).toList()))
.isEqualTo(Optional.of(Boolean.TRUE));
vectorStore.delete(this.documents.stream().map(doc -> doc.getId()).toList());
List<Document> results2 = vectorStore
.similaritySearch(SearchRequest.builder().query("Great").topK(1).build());
@@ -118,7 +117,7 @@ public class ChromaVectorStoreIT {
assertThat(resultDoc.getText()).isEqualTo("The sky is blue because of Rayleigh scattering.");
// Remove all documents from the store
assertThat(vectorStore.delete(List.of(document.getId()))).isEqualTo(Optional.of(Boolean.TRUE));
vectorStore.delete(List.of(document.getId()));
results = vectorStore.similaritySearch(SearchRequest.builder().query("Why is the sky blue?").build());
assertThat(results).hasSize(0);

10
vector-stores/spring-ai-coherence-store/src/main/java/org/springframework/ai/vectorstore/coherence/CoherenceVectorStore.java
View File

@@ -178,21 +178,15 @@ public class CoherenceVectorStore extends AbstractObservationVectorStore impleme
}
@Override
public Optional<Boolean> doDelete(final List<String> idList) {
public void doDelete(final List<String> idList) {
var chunkIds = idList.stream().map(this::toChunkId).toList();
Map<DocumentChunk.Id, Boolean> results = this.documentChunks.invokeAll(chunkIds, entry -> {
this.documentChunks.invokeAll(chunkIds, entry -> {
if (entry.isPresent()) {
entry.remove(false);
return true;
}
return false;
});
for (boolean r : results.values()) {
if (!r) {
return Optional.of(false);
}
}
return Optional.of(true);
}
@Override

3
vector-stores/spring-ai-elasticsearch-store/src/main/java/org/springframework/ai/vectorstore/elasticsearch/ElasticsearchVectorStore.java
View File

@@ -209,7 +209,7 @@ public class ElasticsearchVectorStore extends AbstractObservationVectorStore imp
}
@Override
public Optional<Boolean> doDelete(List<String> idList) {
public void doDelete(List<String> idList) {
BulkRequest.Builder bulkRequestBuilder = new BulkRequest.Builder();
// For the index to be present, either it must be pre-created or set the
// initializeSchema to true.
@@ -219,7 +219,6 @@ public class ElasticsearchVectorStore extends AbstractObservationVectorStore imp
for (String id : idList) {
bulkRequestBuilder.operations(op -> op.delete(idx -> idx.index(this.options.getIndexName()).id(id)));
}
return Optional.of(bulkRequest(bulkRequestBuilder.build()).errors());
}
@Override

4
vector-stores/spring-ai-gemfire-store/src/main/java/org/springframework/ai/vectorstore/gemfire/GemFireVectorStore.java
View File

@@ -231,7 +231,7 @@ public class GemFireVectorStore extends AbstractObservationVectorStore implement
}
@Override
public Optional<Boolean> doDelete(List<String> idList) {
public void doDelete(List<String> idList) {
try {
this.client.method(HttpMethod.DELETE)
.uri("/" + this.indexName + EMBEDDINGS)
@@ -242,9 +242,7 @@ public class GemFireVectorStore extends AbstractObservationVectorStore implement
}
catch (Exception e) {
logger.warn("Error removing embedding: {}", e.getMessage(), e);
return Optional.of(false);
}
return Optional.of(true);
}
@Override

3
vector-stores/spring-ai-hanadb-store/src/main/java/org/springframework/ai/vectorstore/hanadb/HanaCloudVectorStore.java
View File

@@ -126,10 +126,9 @@ public class HanaCloudVectorStore extends AbstractObservationVectorStore {
}
@Override
public Optional<Boolean> doDelete(List<String> idList) {
public void doDelete(List<String> idList) {
int deleteCount = this.repository.deleteEmbeddingsById(this.tableName, idList);
logger.info("{} embeddings deleted", deleteCount);
return Optional.of(deleteCount == idList.size());
}
public int purgeEmbeddings() {

4
vector-stores/spring-ai-mariadb-store/src/main/java/org/springframework/ai/vectorstore/mariadb/MariaDBVectorStore.java
View File

@@ -318,15 +318,13 @@ public class MariaDBVectorStore extends AbstractObservationVectorStore implement
}
@Override
public Optional<Boolean> doDelete(List<String> idList) {
public void doDelete(List<String> idList) {
int updateCount = 0;
for (String id : idList) {
int count = this.jdbcTemplate.update(
String.format("DELETE FROM %s WHERE %s = ?", getFullyQualifiedTableName(), this.idFieldName), id);
updateCount = updateCount + count;
}
return Optional.of(updateCount == idList.size());
}
@Override

4
vector-stores/spring-ai-milvus-store/src/main/java/org/springframework/ai/vectorstore/milvus/MilvusVectorStore.java
View File

@@ -274,7 +274,7 @@ public class MilvusVectorStore extends AbstractObservationVectorStore implements
}
@Override
public Optional<Boolean> doDelete(List<String> idList) {
public void doDelete(List<String> idList) {
Assert.notNull(idList, "Document id list must not be null");
String deleteExpression = String.format("%s in [%s]", this.idFieldName,
@@ -290,8 +290,6 @@ public class MilvusVectorStore extends AbstractObservationVectorStore implements
if (deleteCount != idList.size()) {
logger.warn(String.format("Deleted only %s entries from requested %s ", deleteCount, idList.size()));
}
return Optional.of(status.getStatus() == Status.Success.getCode());
}
@Override

8
vector-stores/spring-ai-mongodb-atlas-store/src/main/java/org/springframework/ai/vectorstore/mongodb/atlas/MongoDBAtlasVectorStore.java
View File

@@ -268,13 +268,9 @@ public class MongoDBAtlasVectorStore extends AbstractObservationVectorStore impl
}
@Override
public Optional<Boolean> doDelete(List<String> idList) {
public void doDelete(List<String> idList) {
Query query = new Query(org.springframework.data.mongodb.core.query.Criteria.where(ID_FIELD_NAME).in(idList));
var deleteRes = this.mongoTemplate.remove(query, this.collectionName);
long deleteCount = deleteRes.getDeletedCount();
return Optional.of(deleteCount == idList.size());
this.mongoTemplate.remove(query, this.collectionName);
}
@Override

5
vector-stores/spring-ai-neo4j-store/src/main/java/org/springframework/ai/vectorstore/neo4j/Neo4jVectorStore.java
View File

@@ -221,20 +221,19 @@ public class Neo4jVectorStore extends AbstractObservationVectorStore implements
}
@Override
public Optional<Boolean> doDelete(List<String> idList) {
public void doDelete(List<String> idList) {
try (var session = this.driver.session(this.sessionConfig)) {
// Those queries with internal, cypher based transaction management cannot be
// run with executeWrite
var summary = session
session
.run("""
MATCH (n:%s) WHERE n.%s IN $ids
CALL { WITH n DETACH DELETE n } IN TRANSACTIONS OF $transactionSize ROWS
""".formatted(this.label, this.idProperty),
Map.of("ids", idList, "transactionSize", DEFAULT_TRANSACTION_SIZE))
.consume();
return Optional.of(idList.size() == summary.counters().nodesDeleted());
}
}

3
vector-stores/spring-ai-opensearch-store/src/main/java/org/springframework/ai/vectorstore/opensearch/OpenSearchVectorStore.java
View File

@@ -217,12 +217,11 @@ public class OpenSearchVectorStore extends AbstractObservationVectorStore implem
}
@Override
public Optional<Boolean> doDelete(List<String> idList) {
public void doDelete(List<String> idList) {
BulkRequest.Builder bulkRequestBuilder = new BulkRequest.Builder();
for (String id : idList) {
bulkRequestBuilder.operations(op -> op.delete(idx -> idx.index(this.index).id(id)));
}
return Optional.of(bulkRequest(bulkRequestBuilder.build()).errors());
}
private BulkResponse bulkRequest(BulkRequest bulkRequest) {

6
vector-stores/spring-ai-oracle-store/src/main/java/org/springframework/ai/vectorstore/oracle/OracleVectorStore.java
View File

@@ -286,7 +286,7 @@ public class OracleVectorStore extends AbstractObservationVectorStore implements
}
@Override
public Optional<Boolean> doDelete(final List<String> idList) {
public void doDelete(final List<String> idList) {
final String sql = String.format("delete from %s where id=?", this.tableName);
final int[] argTypes = { Types.VARCHAR };
@@ -297,19 +297,15 @@ public class OracleVectorStore extends AbstractObservationVectorStore implements
final int[] deleteCounts = this.jdbcTemplate.batchUpdate(sql, batchArgs, argTypes);
int deleteCount = 0;
for (int detailedResult : deleteCounts) {
switch (detailedResult) {
case Statement.EXECUTE_FAILED:
break;
case 1:
case Statement.SUCCESS_NO_INFO:
deleteCount++;
break;
}
}
return Optional.of(deleteCount == idList.size());
}
@Override

4
vector-stores/spring-ai-pgvector-store/src/main/java/org/springframework/ai/vectorstore/pgvector/PgVectorStore.java
View File

@@ -318,15 +318,13 @@ public class PgVectorStore extends AbstractObservationVectorStore implements Ini
}
@Override
public Optional<Boolean> doDelete(List<String> idList) {
public void doDelete(List<String> idList) {
int updateCount = 0;
for (String id : idList) {
int count = this.jdbcTemplate.update("DELETE FROM " + getFullyQualifiedTableName() + " WHERE id = ?",
UUID.fromString(id));
updateCount = updateCount + count;
}
return Optional.of(updateCount == idList.size());
}
@Override

4
vector-stores/spring-ai-pgvector-store/src/test/java/org/springframework/ai/vectorstore/pgvector/PgVectorStoreIT.java
View File

@@ -449,9 +449,7 @@ public class PgVectorStoreIT {
assertThat(results).hasSize(3);
// Delete two documents by ID
Optional<Boolean> deleteResult = vectorStore.delete(List.of(bgDocument.getId(), nlDocument.getId()));
assertThat(deleteResult).isPresent().contains(true);
vectorStore.delete(List.of(bgDocument.getId(), nlDocument.getId()));
// Verify deletion
results = vectorStore.similaritySearch(searchRequest);

19
vector-stores/spring-ai-pinecone-store/src/main/java/org/springframework/ai/vectorstore/pinecone/PineconeVectorStore.java
View File

@@ -224,9 +224,8 @@ public class PineconeVectorStore extends AbstractObservationVectorStore {
* Deletes a list of documents by their IDs based on the namespace.
* @param documentIds The list of document IDs to be deleted.
* @param namespace The namespace of the document IDs.
* @return An optional boolean indicating the deletion status.
*/
public Optional<Boolean> delete(List<String> documentIds, String namespace) {
public void delete(List<String> documentIds, String namespace) {
DeleteRequest deleteRequest = DeleteRequest.newBuilder()
.setNamespace(namespace) // ignored for free tier.
@@ -235,19 +234,15 @@ public class PineconeVectorStore extends AbstractObservationVectorStore {
.build();
this.pineconeConnection.getBlockingStub().delete(deleteRequest);
// The Pinecone delete API does not provide deletion status info.
return Optional.of(true);
}
/**
* Deletes a list of documents by their IDs.
* @param documentIds The list of document IDs to be deleted.
* @return An optional boolean indicating the deletion status.
*/
@Override
public Optional<Boolean> doDelete(List<String> documentIds) {
return delete(documentIds, this.pineconeNamespace);
public void doDelete(List<String> documentIds) {
delete(documentIds, this.pineconeNamespace);
}
public List<Document> similaritySearch(SearchRequest request, String namespace) {
@@ -309,13 +304,7 @@ public class PineconeVectorStore extends AbstractObservationVectorStore {
if (!matchingDocs.isEmpty()) {
// Then delete those documents by ID
List<String> idsToDelete = matchingDocs.stream().map(Document::getId).collect(Collectors.toList());
Optional<Boolean> result = delete(idsToDelete, this.pineconeNamespace);
if (result.isPresent() && !result.get()) {
throw new IllegalStateException("Failed to delete some documents");
}
delete(idsToDelete, this.pineconeNamespace);
logger.debug("Deleted {} documents matching filter expression", idsToDelete.size());
}
}

10
vector-stores/spring-ai-qdrant-store/src/main/java/org/springframework/ai/vectorstore/qdrant/QdrantVectorStore.java
View File

@@ -198,20 +198,16 @@ public class QdrantVectorStore extends AbstractObservationVectorStore implements
/**
* Deletes a list of documents by their IDs.
* @param documentIds The list of document IDs to be deleted.
* @return An optional boolean indicating the deletion status.
*/
@Override
public Optional<Boolean> doDelete(List<String> documentIds) {
public void doDelete(List<String> documentIds) {
try {
List<PointId> ids = documentIds.stream()
.map(id -> io.qdrant.client.PointIdFactory.id(UUID.fromString(id)))
.toList();
var result = this.qdrantClient.deleteAsync(this.collectionName, ids)
.get()
.getStatus() == UpdateStatus.Completed;
return Optional.of(result);
this.qdrantClient.deleteAsync(this.collectionName, ids).get();
}
catch (InterruptedException | ExecutionException | IllegalArgumentException e) {
catch (Exception e) {
throw new RuntimeException(e);
}
}

4
vector-stores/spring-ai-redis-store/src/main/java/org/springframework/ai/vectorstore/redis/RedisVectorStore.java
View File

@@ -280,7 +280,7 @@ public class RedisVectorStore extends AbstractObservationVectorStore implements
}
@Override
public Optional<Boolean> doDelete(List<String> idList) {
public void doDelete(List<String> idList) {
try (Pipeline pipeline = this.jedis.pipelined()) {
for (String id : idList) {
pipeline.jsonDel(key(id));
@@ -291,9 +291,7 @@ public class RedisVectorStore extends AbstractObservationVectorStore implements
if (logger.isErrorEnabled()) {
logger.error("Could not delete document: {}", errResponse.get());
}
return Optional.of(false);
}
return Optional.of(true);
}
}

5
vector-stores/spring-ai-typesense-store/src/main/java/org/springframework/ai/vectorstore/typesense/TypesenseVectorStore.java
View File

@@ -167,7 +167,7 @@ public class TypesenseVectorStore extends AbstractObservationVectorStore impleme
}
@Override
public Optional<Boolean> doDelete(List<String> idList) {
public void doDelete(List<String> idList) {
DeleteDocumentsParameters deleteDocumentsParameters = new DeleteDocumentsParameters();
deleteDocumentsParameters.filterBy(DOC_ID_FIELD_NAME + ":=[" + String.join(",", idList) + "]");
@@ -180,12 +180,9 @@ public class TypesenseVectorStore extends AbstractObservationVectorStore impleme
if (deletedDocs < idList.size()) {
logger.warn("Failed to delete all documents");
}
return Optional.of(deletedDocs > 0);
}
catch (Exception e) {
logger.error("Failed to delete documents", e);
return Optional.of(Boolean.FALSE);
}
}

10
vector-stores/spring-ai-weaviate-store/src/main/java/org/springframework/ai/vectorstore/weaviate/WeaviateVectorStore.java
View File

@@ -273,7 +273,7 @@ public class WeaviateVectorStore extends AbstractObservationVectorStore {
}
@Override
public Optional<Boolean> doDelete(List<String> documentIds) {
public void doDelete(List<String> documentIds) {
Result<BatchDeleteResponse> result = this.weaviateClient.batch()
.objectsBatchDeleter()
@@ -294,8 +294,6 @@ public class WeaviateVectorStore extends AbstractObservationVectorStore {
.collect(Collectors.joining(","));
throw new RuntimeException("Failed to delete documents because: \n" + errorMessages);
}
return Optional.of(!result.hasErrors());
}
@Override
@@ -317,11 +315,7 @@ public class WeaviateVectorStore extends AbstractObservationVectorStore {
if (!matchingDocs.isEmpty()) {
List<String> idsToDelete = matchingDocs.stream().map(Document::getId).collect(Collectors.toList());
Optional<Boolean> result = delete(idsToDelete);
if (result.isPresent() && !result.get()) {
throw new IllegalStateException("Failed to delete some documents");
}
delete(idsToDelete);
logger.debug("Deleted {} documents matching filter expression", idsToDelete.size());
}