From de3838b8309e79c75c89595f5a6f8d04287baf7c Mon Sep 17 00:00:00 2001 From: Aaron Morton Date: Mon, 15 Jun 2026 13:05:21 +1200 Subject: [PATCH 1/8] IndexingDescTest & CreateCollectionCommandResolverTest --- .../CreateCollectionOperation.java | 1229 +++++++++-------- .../CreateCollectionCommandResolver.java | 1 - .../stargate/sgv2/jsonapi/TestConstants.java | 2 + .../model/command/impl/IndexingDescTest.java | 331 +++++ .../CreateCollectionOperationTest.java | 6 - .../CreateCollectionCommandResolverTest.java | 671 ++++----- ...llectionResolverVectorizeDisabledTest.java | 1 + .../util/exception/APIExceptionAssert.java | 39 + .../util/exception/SchemaExceptionAssert.java | 20 + .../profiles}/DisableVectorizeProfile.java | 2 +- .../profiles}/EnabledVectorizeProfile.java | 2 +- 11 files changed, 1346 insertions(+), 958 deletions(-) create mode 100644 src/test/java/io/stargate/sgv2/jsonapi/api/model/command/impl/IndexingDescTest.java create mode 100644 src/test/java/io/stargate/sgv2/jsonapi/util/exception/APIExceptionAssert.java create mode 100644 src/test/java/io/stargate/sgv2/jsonapi/util/exception/SchemaExceptionAssert.java rename src/test/java/io/stargate/sgv2/jsonapi/{service/resolver => util/profiles}/DisableVectorizeProfile.java (90%) rename src/test/java/io/stargate/sgv2/jsonapi/{service/resolver => util/profiles}/EnabledVectorizeProfile.java (90%) diff --git a/src/main/java/io/stargate/sgv2/jsonapi/service/operation/collections/CreateCollectionOperation.java b/src/main/java/io/stargate/sgv2/jsonapi/service/operation/collections/CreateCollectionOperation.java index 7776d6ba74..ee9fbd7a62 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/service/operation/collections/CreateCollectionOperation.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/service/operation/collections/CreateCollectionOperation.java @@ -28,7 +28,6 @@ import io.stargate.sgv2.jsonapi.config.constants.TableCommentConstants; import io.stargate.sgv2.jsonapi.exception.DatabaseException; import io.stargate.sgv2.jsonapi.exception.SchemaException; -import io.stargate.sgv2.jsonapi.service.cqldriver.CQLSessionCache; import io.stargate.sgv2.jsonapi.service.cqldriver.executor.QueryExecutor; import io.stargate.sgv2.jsonapi.service.cqldriver.override.ExtendedCreateIndex; import io.stargate.sgv2.jsonapi.service.cqldriver.override.ExtendedVectorType; @@ -43,627 +42,691 @@ import io.stargate.sgv2.jsonapi.service.schema.collections.CollectionSchemaObject; import io.stargate.sgv2.jsonapi.service.schema.collections.CollectionTableMatcher; import io.stargate.sgv2.jsonapi.service.schema.tables.CQLSAIIndex; + import java.time.Duration; import java.util.*; import java.util.function.Supplier; import java.util.stream.Collectors; + import org.slf4j.Logger; import org.slf4j.LoggerFactory; -public record CreateCollectionOperation( - CommandContext commandContext, - DatabaseLimitsConfig dbLimitsConfig, - CQLSessionCache cqlSessionCache, - CqlIdentifier collectionName, - int ddlDelayMillis, - boolean tooManyIndexesRollbackEnabled, +public class CreateCollectionOperation implements Operation { + + private static final Logger LOGGER = LoggerFactory.getLogger(CreateCollectionOperation.class); + + private static final CollectionTableMatcher COLLECTION_MATCHER = + new CollectionTableMatcher(); + + private static final ObjectMapper OBJECT_MAPPER = new ObjectMapper(); + + private final CommandContext commandContext; + private final DatabaseLimitsConfig dbLimitsConfig; + private final CqlIdentifier collectionName; + private final int ddlDelayMillis; + private final boolean tooManyIndexesRollbackEnabled; // nullable - CreateCollectionCommand.Options.DocIdDesc docIdDesc, + private final CreateCollectionCommand.Options.DocIdDesc docIdDesc; // nullable - CreateCollectionCommand.Options.IndexingDesc indexingDesc, + private final CreateCollectionCommand.Options.IndexingDesc indexingDesc; // nullable - CreateCollectionCommand.Options.VectorSearchDesc vectorDesc, - SchemaHolder lexicalDef, - SchemaHolder rerankDef) - implements Operation { - - private static final Logger LOGGER = LoggerFactory.getLogger(CreateCollectionOperation.class); - - private static final CollectionTableMatcher COLLECTION_MATCHER = new CollectionTableMatcher(); - - private static final ObjectMapper OBJECT_MAPPER = new ObjectMapper(); - - @Override - public Uni> execute( - RequestContext requestContext, QueryExecutor queryExecutor) { - - var initialTableComment = generateTableComment(); - LOGGER.info( - "execute()- createCollection for identifier= {}.{}, initialTableComment={}", - commandContext.schemaObject().identifier().keyspace(), - collectionName, - initialTableComment); - - return queryExecutor - .getDriverMetadata(requestContext) - .map(Metadata::getKeyspaces) - .flatMap( - allKeyspaces -> { - - // Step 1 - does the keyspace exist ? - var targetKeyspace = - allKeyspaces.get(commandContext.schemaObject().identifier().keyspace()); - if (targetKeyspace == null) { - return Uni.createFrom() - .failure( - SchemaException.Code.UNKNOWN_KEYSPACE.get( - errVars(commandContext.schemaObject()))); - } - - // Step 2 - is there an existing table and if not is there enough free indexes ? - var existingTableMetadata = - findTableAndValidateLimits(allKeyspaces, targetKeyspace, collectionName); - - // Step 3 - create the collection if no existing table - // use the runningValue() of lexicalDef this will either be the value from user or - // default - if (existingTableMetadata == null) { - return executeCollectionCreation( - requestContext, - queryExecutor, - initialTableComment, - lexicalDef().runningValue(), - false); - } - - // Step 4- Existing collection, check if the schema from the user is the same as the - // existing - // we need to merge in the current schema if the user did not specify anything - var existingCollectionSettings = - CollectionSchemaObject.getCollectionSettings( - requestContext, existingTableMetadata, OBJECT_MAPPER); - if (LOGGER.isDebugEnabled()) { - LOGGER.debug( - "execute() - existingCollectionSettings: {}", existingCollectionSettings); - } - - // Need to resolve the vector settings so we can use them to create a full - // representation - // of what the new collection will look like - var vectorModelName = - getOrDefault( - vectorDesc, - CreateCollectionCommand.Options.VectorSearchDesc::sourceModel, - null); - var embeddingSourceModel = - EmbeddingSourceModel.fromApiNameOrDefault(vectorModelName) - .orElseThrow( - () -> - EmbeddingSourceModel.getUnknownSourceModelException(vectorModelName)); - - var similarityFunctionName = - getOrDefault( - vectorDesc, CreateCollectionCommand.Options.VectorSearchDesc::metric, null); - var similarityFunction = - SimilarityFunction.fromApiNameOrDefault(similarityFunctionName) - .orElseThrow( - () -> - SimilarityFunction.getUnknownFunctionException( - similarityFunctionName)); - - // OK, we know there is an existing collection, and it is different from the one we - // already have. - // So we will replace the lexical and rerank in the new one with the existing if the - // user did not specify new values. - // NOTE: FROM NOW ON WE NEED TO USE THE OVERRIDEN VALUE, (which may or may not be - // actually overidden) - var overrideLexicalDef = - lexicalDef() - .replaceIfMissing(existingCollectionSettings.lexicalDefSchemaValue()) - .value(); - var overrideRerankDef = - rerankDef() - .replaceIfMissing(existingCollectionSettings.rerankDefSchemaValue()) - .value(); - - var overrideTableComment = - generateTableComment(overrideLexicalDef, overrideRerankDef); - - if (LOGGER.isDebugEnabled()) { - LOGGER.debug("execute() - overrideTableComment: {}", overrideTableComment); - } - - var newCollectionSettings = - CollectionSchemaObject.createCollectionSettings( - requestContext, - existingTableMetadata, - vectorDesc != null, - getOrDefault( - vectorDesc, - CreateCollectionCommand.Options.VectorSearchDesc::dimension, - 0), - similarityFunction, - embeddingSourceModel, - overrideTableComment, - OBJECT_MAPPER); - - boolean settingsAreEqual = existingCollectionSettings.equals(newCollectionSettings); - if (LOGGER.isDebugEnabled()) { - LOGGER.debug( - "execute() - settingsAreEqual: {}, newCollectionSettings={}", - settingsAreEqual, - newCollectionSettings); - } - - if (settingsAreEqual) { - return executeCollectionCreation( - requestContext, - queryExecutor, - overrideTableComment, - overrideLexicalDef.runningValue(), - true); - } - return Uni.createFrom() - .failure( - SchemaException.Code.EXISTING_COLLECTION_DIFFERENT_SETTINGS.get( - Map.of("collectionName", cqlIdentifierToMessageString(collectionName)))); - }); - } - - @VisibleForTesting - String generateTableComment() { - return generateTableComment(lexicalDef(), rerankDef()); - } - - @VisibleForTesting - String generateTableComment( - SchemaHolder overrideLexicalDef, - SchemaHolder overrideRerankDef) { - - var optionsNode = OBJECT_MAPPER.createObjectNode(); - - if (indexingDesc != null) { - optionsNode.putPOJO(TableCommentConstants.COLLECTION_INDEXING_KEY, indexingDesc); + private final CreateCollectionCommand.Options.VectorSearchDesc vectorDesc; + private final SchemaHolder lexicalDef; + private final SchemaHolder rerankDef; + + public CreateCollectionOperation( + CommandContext commandContext, + DatabaseLimitsConfig dbLimitsConfig, + CqlIdentifier collectionName, + int ddlDelayMillis, + boolean tooManyIndexesRollbackEnabled, + // nullable + CreateCollectionCommand.Options.DocIdDesc docIdDesc, + // nullable + CreateCollectionCommand.Options.IndexingDesc indexingDesc, + // nullable + CreateCollectionCommand.Options.VectorSearchDesc vectorDesc, + SchemaHolder lexicalDef, + SchemaHolder rerankDef) { + this.commandContext = commandContext; + this.dbLimitsConfig = dbLimitsConfig; + this.collectionName = collectionName; + this.ddlDelayMillis = ddlDelayMillis; + this.tooManyIndexesRollbackEnabled = tooManyIndexesRollbackEnabled; + this.docIdDesc = docIdDesc; + this.indexingDesc = indexingDesc; + this.vectorDesc = vectorDesc; + this.lexicalDef = lexicalDef; + this.rerankDef = rerankDef; + } + + @VisibleForTesting + public CqlIdentifier collectionName(){ + return collectionName; + } + + @VisibleForTesting + public CommandContext commandContext() { + return commandContext; + } + + @VisibleForTesting + public CreateCollectionCommand.Options.DocIdDesc docIdDesc() { + return docIdDesc; + } + @VisibleForTesting + public CreateCollectionCommand.Options.IndexingDesc indexingDesc() { + return indexingDesc; + } + @VisibleForTesting + public CreateCollectionCommand.Options.VectorSearchDesc vectorDesc() { + return vectorDesc; + } + @VisibleForTesting + public SchemaHolder lexicalDef() { + return lexicalDef; + } + @VisibleForTesting + public SchemaHolder rerankDef() { + return rerankDef; + } + + + @Override + public Uni> execute( + RequestContext requestContext, QueryExecutor queryExecutor) { + + var initialTableComment = generateTableComment(); + LOGGER.info( + "execute()- createCollection for identifier= {}.{}, initialTableComment={}", + commandContext.schemaObject().identifier().keyspace(), + collectionName, + initialTableComment); + + return queryExecutor + .getDriverMetadata(requestContext) + .map(Metadata::getKeyspaces) + .flatMap( + allKeyspaces -> { + + // Step 1 - does the keyspace exist ? + var targetKeyspace = + allKeyspaces.get(commandContext.schemaObject().identifier().keyspace()); + if (targetKeyspace == null) { + return Uni.createFrom() + .failure( + SchemaException.Code.UNKNOWN_KEYSPACE.get( + errVars(commandContext.schemaObject()))); + } + + // Step 2 - is there an existing table and if not is there enough free indexes ? + var existingTableMetadata = + findTableAndValidateLimits(allKeyspaces, targetKeyspace, collectionName); + + // Step 3 - create the collection if no existing table + // use the runningValue() of lexicalDef this will either be the value from user or + // default + if (existingTableMetadata == null) { + return executeCollectionCreation( + requestContext, + queryExecutor, + initialTableComment, + lexicalDef.runningValue(), + false); + } + + // Step 4- Existing collection, check if the schema from the user is the same as the + // existing + // we need to merge in the current schema if the user did not specify anything + var existingCollectionSettings = + CollectionSchemaObject.getCollectionSettings( + requestContext, existingTableMetadata, OBJECT_MAPPER); + if (LOGGER.isDebugEnabled()) { + LOGGER.debug( + "execute() - existingCollectionSettings: {}", existingCollectionSettings); + } + + // Need to resolve the vector settings so we can use them to create a full + // representation + // of what the new collection will look like + var vectorModelName = + getOrDefault( + vectorDesc, + CreateCollectionCommand.Options.VectorSearchDesc::sourceModel, + null); + var embeddingSourceModel = + EmbeddingSourceModel.fromApiNameOrDefault(vectorModelName) + .orElseThrow( + () -> + EmbeddingSourceModel.getUnknownSourceModelException(vectorModelName)); + + var similarityFunctionName = + getOrDefault( + vectorDesc, CreateCollectionCommand.Options.VectorSearchDesc::metric, null); + var similarityFunction = + SimilarityFunction.fromApiNameOrDefault(similarityFunctionName) + .orElseThrow( + () -> + SimilarityFunction.getUnknownFunctionException( + similarityFunctionName)); + + // OK, we know there is an existing collection, and it is different from the one we + // already have. + // So we will replace the lexical and rerank in the new one with the existing if the + // user did not specify new values. + // NOTE: FROM NOW ON WE NEED TO USE THE OVERRIDEN VALUE, (which may or may not be + // actually overidden) + var overrideLexicalDef = + lexicalDef + .replaceIfMissing(existingCollectionSettings.lexicalDefSchemaValue()) + .value(); + var overrideRerankDef = + rerankDef + .replaceIfMissing(existingCollectionSettings.rerankDefSchemaValue()) + .value(); + + var overrideTableComment = + generateTableComment(overrideLexicalDef, overrideRerankDef); + + if (LOGGER.isDebugEnabled()) { + LOGGER.debug("execute() - overrideTableComment: {}", overrideTableComment); + } + + var newCollectionSettings = + CollectionSchemaObject.createCollectionSettings( + requestContext, + existingTableMetadata, + vectorDesc != null, + getOrDefault( + vectorDesc, + CreateCollectionCommand.Options.VectorSearchDesc::dimension, + 0), + similarityFunction, + embeddingSourceModel, + overrideTableComment, + OBJECT_MAPPER); + + boolean settingsAreEqual = existingCollectionSettings.equals(newCollectionSettings); + if (LOGGER.isDebugEnabled()) { + LOGGER.debug( + "execute() - settingsAreEqual: {}, newCollectionSettings={}", + settingsAreEqual, + newCollectionSettings); + } + + if (settingsAreEqual) { + return executeCollectionCreation( + requestContext, + queryExecutor, + overrideTableComment, + overrideLexicalDef.runningValue(), + true); + } + return Uni.createFrom() + .failure( + SchemaException.Code.EXISTING_COLLECTION_DIFFERENT_SETTINGS.get( + Map.of("collectionName", cqlIdentifierToMessageString(collectionName)))); + }); } - if (vectorDesc != null) { - optionsNode.putPOJO(TableCommentConstants.COLLECTION_VECTOR_KEY, vectorDesc); + + @VisibleForTesting + String generateTableComment() { + return generateTableComment(lexicalDef, rerankDef); } - // if default_id is not specified during createCollection, resolve type to empty string - if (docIdDesc != null) { - optionsNode.putPOJO(TableCommentConstants.DEFAULT_ID_KEY, docIdDesc); - } else { - optionsNode.putPOJO( - TableCommentConstants.DEFAULT_ID_KEY, - OBJECT_MAPPER.createObjectNode().putPOJO("type", "")); + + @VisibleForTesting + String generateTableComment( + SchemaHolder overrideLexicalDef, + SchemaHolder overrideRerankDef) { + + var optionsNode = OBJECT_MAPPER.createObjectNode(); + + if (indexingDesc != null) { + optionsNode.putPOJO(TableCommentConstants.COLLECTION_INDEXING_KEY, indexingDesc); + } + if (vectorDesc != null) { + optionsNode.putPOJO(TableCommentConstants.COLLECTION_VECTOR_KEY, vectorDesc); + } + // if default_id is not specified during createCollection, resolve type to empty string + if (docIdDesc != null) { + optionsNode.putPOJO(TableCommentConstants.DEFAULT_ID_KEY, docIdDesc); + } else { + optionsNode.putPOJO( + TableCommentConstants.DEFAULT_ID_KEY, + OBJECT_MAPPER.createObjectNode().putPOJO("type", "")); + } + + // Take the running value, this will either be what the user gave us or the appropriate default + optionsNode.putPOJO( + TableCommentConstants.COLLECTION_LEXICAL_CONFIG_KEY, overrideLexicalDef.runningValue()); + optionsNode.putPOJO( + TableCommentConstants.COLLECTION_RERANKING_CONFIG_KEY, overrideRerankDef.runningValue()); + + var collectionNode = OBJECT_MAPPER.createObjectNode(); + collectionNode.put( + TableCommentConstants.COLLECTION_NAME_KEY, cqlIdentifierToJsonKey(collectionName)); + // Use ordinalValue() to get the integer representation of the enum into the JSON + collectionNode.put( + TableCommentConstants.SCHEMA_VERSION_KEY, + CollectionSchemaVersion.CURRENT_VERSION.ordinalValue()); + collectionNode.putPOJO(TableCommentConstants.OPTIONS_KEY, optionsNode); + + var tableCommentNode = OBJECT_MAPPER.createObjectNode(); + tableCommentNode.putPOJO(TableCommentConstants.TOP_LEVEL_KEY, collectionNode); + + return tableCommentNode.toString(); } - // Take the running value, this will either be what the user gave us or the appropriate default - optionsNode.putPOJO( - TableCommentConstants.COLLECTION_LEXICAL_CONFIG_KEY, overrideLexicalDef.runningValue()); - optionsNode.putPOJO( - TableCommentConstants.COLLECTION_RERANKING_CONFIG_KEY, overrideRerankDef.runningValue()); - - var collectionNode = OBJECT_MAPPER.createObjectNode(); - collectionNode.put( - TableCommentConstants.COLLECTION_NAME_KEY, cqlIdentifierToJsonKey(collectionName)); - // Use ordinalValue() to get the integer representation of the enum into the JSON - collectionNode.put( - TableCommentConstants.SCHEMA_VERSION_KEY, - CollectionSchemaVersion.CURRENT_VERSION.ordinalValue()); - collectionNode.putPOJO(TableCommentConstants.OPTIONS_KEY, optionsNode); - - var tableCommentNode = OBJECT_MAPPER.createObjectNode(); - tableCommentNode.putPOJO(TableCommentConstants.TOP_LEVEL_KEY, collectionNode); - - return tableCommentNode.toString(); - } - - /** - * execute collection creation and indexes creation - * - * @param requestContext DBRequestContext - * @param queryExecutor QueryExecutor instance - * @param collectionLexicalDef Lexical configuration for the collection - * @param collectionExisted boolean that says if collection existed before - * @return Uni> - */ - private Uni> executeCollectionCreation( - RequestContext requestContext, - QueryExecutor queryExecutor, - String tableComment, - CollectionLexicalDef collectionLexicalDef, - boolean collectionExisted) { - - final Uni execCreateTable = - queryExecutor.executeCreateSchemaChange( - requestContext, getCreateTable(tableComment, collectionLexicalDef)); - - final Uni indexResult = - execCreateTable - .onItem() - .delayIt() - .by(Duration.ofMillis(ddlDelayMillis > 0 ? ddlDelayMillis : 100)) - .onItem() - .transformToUni( - res -> { - if (res.wasApplied()) { - final List indexStatements = - getIndexStatements(collectionLexicalDef, collectionExisted); - Multi indexResultMulti; + /** + * execute collection creation and indexes creation + * + * @param requestContext DBRequestContext + * @param queryExecutor QueryExecutor instance + * @param collectionLexicalDef Lexical configuration for the collection + * @param collectionExisted boolean that says if collection existed before + * @return Uni> + */ + private Uni> executeCollectionCreation( + RequestContext requestContext, + QueryExecutor queryExecutor, + String tableComment, + CollectionLexicalDef collectionLexicalDef, + boolean collectionExisted) { + + final Uni execCreateTable = + queryExecutor.executeCreateSchemaChange( + requestContext, getCreateTable(tableComment, collectionLexicalDef)); + + final Uni indexResult = + execCreateTable + .onItem() + .delayIt() + .by(Duration.ofMillis(ddlDelayMillis > 0 ? ddlDelayMillis : 100)) + .onItem() + .transformToUni( + res -> { + if (res.wasApplied()) { + final List indexStatements = + getIndexStatements(collectionLexicalDef, collectionExisted); + Multi indexResultMulti; /* CI will override ddlDelayMillis to 0 using `-Dstargate.jsonapi.operations.database-config.ddl-delay-millis=0` to speed up the test execution This is ok because CI is run as single cassandra instance and there is no need to wait for the schema changes to propagate */ - if (ddlDelayMillis == 0) { - indexResultMulti = - createIndexParallel(queryExecutor, requestContext, indexStatements); - } else { - indexResultMulti = - createIndexOrdered(queryExecutor, requestContext, indexStatements); - } - - return indexResultMulti - .collect() - .asList() - .onItem() - .transform( - results -> { - final Optional first = - results.stream() - .filter(indexRes -> !(indexRes.wasApplied())) - .findFirst(); - return !first.isPresent(); - }); - } else { - return Uni.createFrom().item(false); - } - }); - - return indexResult - .onItem() - .transform( - res -> { - if (!res) { - // amorton - 13 jan 2026 - this is bad, the old code would swallow the error for - // creating the table and indexes, will need to improve later. - - // table creation failure or index creation failure - // HACK - remove when re-writing this class - return commandResultSupplier( - DatabaseException.Code.CORRUPTED_COLLECTION_SCHEMA.get( - errVars( - commandContext.schemaObject(), - map -> - map.put( - "errorMessage", - "Collection creation failure (unable to create table)")))); - } else { - return new SchemaChangeResult(true); - } - }) - .onFailure( - error -> - // InvalidQueryException(DB index limit violation) - error instanceof InvalidQueryException - && (error - .getMessage() - .matches( - ".*Cannot have more than \\d+ indexes, failed to create index on table.*") - || error.getMessage().matches("Index .* already exists"))) - .recoverWithUni( - // this block only handles the case where the index creation fails because of index - // limit or already exists during create collection - error -> { - // if index creation fails and collection not existed before and rollback is enabled, - // then drop the collection - if (!collectionExisted && tooManyIndexesRollbackEnabled) { - return cleanUpCollectionFailedWithTooManyIndex(requestContext, queryExecutor); - } - - if (error.getMessage().matches("Index .* already exists")) { - // if index creation fails because index already exists - return Uni.createFrom() - .item( - () -> - commandResultSupplier( - SchemaException.Code.EXISTING_INDEX_FOR_COLLECTION.get( - errVars(commandContext.schemaObject())))); - } else { - // if index creation violates DB index limit and collection existed before, - // will not drop the collection - return Uni.createFrom() - .item( - () -> - commandResultSupplier( - SchemaException.Code.TOO_MANY_INDEXES_FOR_COLLECTION.get( - errVars( - commandContext.schemaObject(), - map -> - map.put( - "indexesPerCollection", - String.valueOf( - dbLimitsConfig - .indexesNeededPerCollection())))))); - } - }); - } - - private Supplier commandResultSupplier(Throwable throwable) { - return () -> - CommandResult.statusOnlyBuilder(RequestTracing.NO_OP).addThrowable(throwable).build(); - } - - /** Create indexes for collections in ordered. This is to avoid schema change conflicts. */ - private Multi createIndexOrdered( - QueryExecutor queryExecutor, - RequestContext requestContext, - List indexStatements) { - - return Multi.createFrom() - .items(indexStatements.stream()) - .onItem() - .transformToUni( - indexStatement -> - queryExecutor.executeCreateSchemaChange(requestContext, indexStatement)) - .concatenate(); - } - - /** Create indexes for collections in parallel. Only used to speed up the CI actions. */ - private Multi createIndexParallel( - QueryExecutor queryExecutor, - RequestContext requestContext, - List indexStatements) { - - return Multi.createFrom() - .items(indexStatements.stream()) - .onItem() - .transformToUni( - indexStatement -> - queryExecutor.executeCreateSchemaChange(requestContext, indexStatement)) - .merge(); - } - - public Uni> cleanUpCollectionFailedWithTooManyIndex( - RequestContext requestContext, QueryExecutor queryExecutor) { - - // turning the name into asInternal() because DeleteCollectionCollectionOperation stil uses - // string - DeleteCollectionCollectionOperation deleteCollectionCollectionOperation = - new DeleteCollectionCollectionOperation(commandContext, collectionName.asInternal()); - - // amorton - 13 jan 2026 - keeping the existing logic here, where the error was returning in - // two situations - // unsure how the second happens - var exception = - SchemaException.Code.TOO_MANY_INDEXES_FOR_COLLECTION.get( - errVars( - commandContext.schemaObject(), - map -> - map.put( - "indexesPerCollection", - String.valueOf(dbLimitsConfig.indexesNeededPerCollection())))); - return deleteCollectionCollectionOperation - .execute(requestContext, queryExecutor) - .onItem() - .transform( - res -> - (Supplier) - () -> - CommandResult.statusOnlyBuilder(RequestTracing.NO_OP) - .addThrowable(exception) - .build()) - .onFailure() - .recoverWithItem( - e -> - // This is unlikely to happen for delete collection though - // Also return with TOO_MANY_INDEXES exception - () -> - CommandResult.statusOnlyBuilder(RequestTracing.NO_OP) - .addThrowable(exception) - .build()); - } - - /** - * Method for finding existing collection with given name: 1. if valid one exists and returning - * that table; 2. if invalid one exists, error out; 3. if no table exists with given name, verify - * maximum table limit and return null; - * - * @return Existing valid collection with given name, if any; {@code null} if not - */ - TableMetadata findTableAndValidateLimits( - Map allKeyspaces, - KeyspaceMetadata currKeyspace, - CqlIdentifier tableName) { - - // First: do we already have a Table with the same name? - for (TableMetadata table : currKeyspace.getTables().values()) { - if (table.getName().equals(tableName)) { - // If that is not a valid Data API collection, error out the createCollectionCommand - if (!COLLECTION_MATCHER.test(table)) { - throw SchemaException.Code.EXISTING_TABLE_NOT_DATA_API_COLLECTION.get( - Map.of("tableName", cqlIdentifierToMessageString(tableName))); - } - // If that is a valid Data API table, we returned it - return table; - } + if (ddlDelayMillis == 0) { + indexResultMulti = + createIndexParallel(queryExecutor, requestContext, indexStatements); + } else { + indexResultMulti = + createIndexOrdered(queryExecutor, requestContext, indexStatements); + } + + return indexResultMulti + .collect() + .asList() + .onItem() + .transform( + results -> { + final Optional first = + results.stream() + .filter(indexRes -> !(indexRes.wasApplied())) + .findFirst(); + return !first.isPresent(); + }); + } else { + return Uni.createFrom().item(false); + } + }); + + return indexResult + .onItem() + .transform( + res -> { + if (!res) { + // amorton - 13 jan 2026 - this is bad, the old code would swallow the error for + // creating the table and indexes, will need to improve later. + + // table creation failure or index creation failure + // HACK - remove when re-writing this class + return commandResultSupplier( + DatabaseException.Code.CORRUPTED_COLLECTION_SCHEMA.get( + errVars( + commandContext.schemaObject(), + map -> + map.put( + "errorMessage", + "Collection creation failure (unable to create table)")))); + } else { + return new SchemaChangeResult(true); + } + }) + .onFailure( + error -> + // InvalidQueryException(DB index limit violation) + error instanceof InvalidQueryException + && (error + .getMessage() + .matches( + ".*Cannot have more than \\d+ indexes, failed to create index on table.*") + || error.getMessage().matches("Index .* already exists"))) + .recoverWithUni( + // this block only handles the case where the index creation fails because of index + // limit or already exists during create collection + error -> { + // if index creation fails and collection not existed before and rollback is enabled, + // then drop the collection + if (!collectionExisted && tooManyIndexesRollbackEnabled) { + return cleanUpCollectionFailedWithTooManyIndex(requestContext, queryExecutor); + } + + if (error.getMessage().matches("Index .* already exists")) { + // if index creation fails because index already exists + return Uni.createFrom() + .item( + () -> + commandResultSupplier( + SchemaException.Code.EXISTING_INDEX_FOR_COLLECTION.get( + errVars(commandContext.schemaObject())))); + } else { + // if index creation violates DB index limit and collection existed before, + // will not drop the collection + return Uni.createFrom() + .item( + () -> + commandResultSupplier( + SchemaException.Code.TOO_MANY_INDEXES_FOR_COLLECTION.get( + errVars( + commandContext.schemaObject(), + map -> + map.put( + "indexesPerCollection", + String.valueOf( + dbLimitsConfig + .indexesNeededPerCollection())))))); + } + }); } - // Otherwise we need to check if we can create a new Collection based on limits; - // limits are calculated across the whole Database, so all Keyspaces need to be checked. - final List allTables = - allKeyspaces.values().stream() - .map(keyspace -> keyspace.getTables().values()) - .flatMap(Collection::stream) - .toList(); - final long collectionCount = allTables.stream().filter(COLLECTION_MATCHER).count(); - final int MAX_COLLECTIONS = dbLimitsConfig.maxCollections(); - if (collectionCount >= MAX_COLLECTIONS) { - throw SchemaException.Code.TOO_MANY_COLLECTIONS.get( - Map.of( - "table", - cqlIdentifierToMessageString(tableName), - "collectionCount", - String.valueOf(collectionCount), - "collectionMaxCount", - String.valueOf(MAX_COLLECTIONS))); + private Supplier commandResultSupplier(Throwable throwable) { + return () -> + CommandResult.statusOnlyBuilder(RequestTracing.NO_OP).addThrowable(throwable).build(); } - // And then see how many Indexes have been created, how many available - int saisUsed = allTables.stream().mapToInt(table -> table.getIndexes().size()).sum(); - if ((saisUsed + dbLimitsConfig.indexesNeededPerCollection()) - > dbLimitsConfig.indexesAvailablePerDatabase()) { - throw SchemaException.Code.TOO_MANY_INDEXES_FOR_COLLECTION.get( - errVars( - commandContext.schemaObject(), - map -> - map.put( - "indexesPerCollection", - String.valueOf(dbLimitsConfig.indexesNeededPerCollection())))); + /** + * Create indexes for collections in ordered. This is to avoid schema change conflicts. + */ + private Multi createIndexOrdered( + QueryExecutor queryExecutor, + RequestContext requestContext, + List indexStatements) { + + return Multi.createFrom() + .items(indexStatements.stream()) + .onItem() + .transformToUni( + indexStatement -> + queryExecutor.executeCreateSchemaChange(requestContext, indexStatement)) + .concatenate(); } - return null; - } - - private SimpleStatement getCreateTable(String comment, CollectionLexicalDef overrideLexicalDef) { - - var keyspace = commandContext.schemaObject().identifier().keyspace(); - - CreateTable create = - SchemaBuilder.createTable(keyspace, collectionName) - .ifNotExists() - .withPartitionKey("key", DataTypes.tupleOf(DataTypes.TINYINT, DataTypes.TEXT)) - .withColumn("tx_id", DataTypes.TIMEUUID) - .withColumn("doc_json", DataTypes.TEXT) - .withColumn("exist_keys", DataTypes.setOf(DataTypes.TEXT)) - .withColumn("array_size", DataTypes.mapOf(DataTypes.TEXT, DataTypes.INT)) - .withColumn("array_contains", DataTypes.setOf(DataTypes.TEXT)) - .withColumn("query_bool_values", DataTypes.mapOf(DataTypes.TEXT, DataTypes.TINYINT)) - .withColumn("query_dbl_values", DataTypes.mapOf(DataTypes.TEXT, DataTypes.DECIMAL)) - .withColumn("query_text_values", DataTypes.mapOf(DataTypes.TEXT, DataTypes.TEXT)) - .withColumn( - "query_timestamp_values", DataTypes.mapOf(DataTypes.TEXT, DataTypes.TIMESTAMP)) - .withColumn("query_null_values", DataTypes.setOf(DataTypes.TEXT)); - - if (vectorDesc != null) { - create = - create.withColumn( - "query_vector_value", - new ExtendedVectorType(DataTypes.FLOAT, vectorDesc.dimension())); - } - if (overrideLexicalDef.enabled()) { - create = create.withColumn("query_lexical_value", DataTypes.TEXT); + /** + * Create indexes for collections in parallel. Only used to speed up the CI actions. + */ + private Multi createIndexParallel( + QueryExecutor queryExecutor, + RequestContext requestContext, + List indexStatements) { + + return Multi.createFrom() + .items(indexStatements.stream()) + .onItem() + .transformToUni( + indexStatement -> + queryExecutor.executeCreateSchemaChange(requestContext, indexStatement)) + .merge(); } - // adding the comment changes the return into something to deal with options - var statement = comment == null ? create.build() : create.withComment(comment).build(); + public Uni> cleanUpCollectionFailedWithTooManyIndex( + RequestContext requestContext, QueryExecutor queryExecutor) { + + // turning the name into asInternal() because DeleteCollectionCollectionOperation stil uses + // string + DeleteCollectionCollectionOperation deleteCollectionCollectionOperation = + new DeleteCollectionCollectionOperation(commandContext, collectionName.asInternal()); - if (LOGGER.isTraceEnabled()) { - LOGGER.trace("getCreateTable() - created table statement: {}", statement.getQuery()); + // amorton - 13 jan 2026 - keeping the existing logic here, where the error was returning in + // two situations + // unsure how the second happens + var exception = + SchemaException.Code.TOO_MANY_INDEXES_FOR_COLLECTION.get( + errVars( + commandContext.schemaObject(), + map -> + map.put( + "indexesPerCollection", + String.valueOf(dbLimitsConfig.indexesNeededPerCollection())))); + return deleteCollectionCollectionOperation + .execute(requestContext, queryExecutor) + .onItem() + .transform( + res -> + (Supplier) + () -> + CommandResult.statusOnlyBuilder(RequestTracing.NO_OP) + .addThrowable(exception) + .build()) + .onFailure() + .recoverWithItem( + e -> + // This is unlikely to happen for delete collection though + // Also return with TOO_MANY_INDEXES exception + () -> + CommandResult.statusOnlyBuilder(RequestTracing.NO_OP) + .addThrowable(exception) + .build()); } - return statement; - } - - /* - * When a createCollection is done on a table that already exist the index are run with IF NOT EXISTS. - * For a new table they are run without IF NOT EXISTS. - */ - private List getIndexStatements( - CollectionLexicalDef overrideLexicalDef, boolean collectionExisted) { - - List statements = new ArrayList<>(10); - - var denyAllIndexes = - getOrDefault(indexingDesc, CreateCollectionCommand.Options.IndexingDesc::denyAll, false); - - if (!denyAllIndexes) { - statements.add(saiColumn(collectionExisted, "exists_keys", "exist_keys")); - statements.add(saiEntries(collectionExisted, "array_size", "array_size")); - statements.add(saiColumn(collectionExisted, "array_contains", "array_contains")); - statements.add(saiEntries(collectionExisted, "query_bool_values", "query_bool_values")); - statements.add(saiEntries(collectionExisted, "query_dbl_values", "query_dbl_values")); - statements.add(saiEntries(collectionExisted, "query_text_values", "query_text_values")); - statements.add( - saiEntries(collectionExisted, "query_timestamp_values", "query_timestamp_values")); - statements.add(saiColumn(collectionExisted, "query_null_values", "query_null_values")); + + /** + * Method for finding existing collection with given name: 1. if valid one exists and returning + * that table; 2. if invalid one exists, error out; 3. if no table exists with given name, verify + * maximum table limit and return null; + * + * @return Existing valid collection with given name, if any; {@code null} if not + */ + TableMetadata findTableAndValidateLimits( + Map allKeyspaces, + KeyspaceMetadata currKeyspace, + CqlIdentifier tableName) { + + // First: do we already have a Table with the same name? + for (TableMetadata table : currKeyspace.getTables().values()) { + if (table.getName().equals(tableName)) { + // If that is not a valid Data API collection, error out the createCollectionCommand + if (!COLLECTION_MATCHER.test(table)) { + throw SchemaException.Code.EXISTING_TABLE_NOT_DATA_API_COLLECTION.get( + Map.of("tableName", cqlIdentifierToMessageString(tableName))); + } + // If that is a valid Data API table, we returned it + return table; + } + } + + // Otherwise we need to check if we can create a new Collection based on limits; + // limits are calculated across the whole Database, so all Keyspaces need to be checked. + final List allTables = + allKeyspaces.values().stream() + .map(keyspace -> keyspace.getTables().values()) + .flatMap(Collection::stream) + .toList(); + final long collectionCount = allTables.stream().filter(COLLECTION_MATCHER).count(); + final int MAX_COLLECTIONS = dbLimitsConfig.maxCollections(); + if (collectionCount >= MAX_COLLECTIONS) { + throw SchemaException.Code.TOO_MANY_COLLECTIONS.get( + Map.of( + "table", + cqlIdentifierToMessageString(tableName), + "collectionCount", + String.valueOf(collectionCount), + "collectionMaxCount", + String.valueOf(MAX_COLLECTIONS))); + } + + // And then see how many Indexes have been created, how many available + int saisUsed = allTables.stream().mapToInt(table -> table.getIndexes().size()).sum(); + if ((saisUsed + dbLimitsConfig.indexesNeededPerCollection()) + > dbLimitsConfig.indexesAvailablePerDatabase()) { + throw SchemaException.Code.TOO_MANY_INDEXES_FOR_COLLECTION.get( + errVars( + commandContext.schemaObject(), + map -> + map.put( + "indexesPerCollection", + String.valueOf(dbLimitsConfig.indexesNeededPerCollection())))); + } + + return null; } - // NOTE: This is a little sloppy, in normal request the CreateCollectionCommandResolver will - // make sure the vectorDesc is valid and has defaults set. So even though they are strings - // they have been validated as the thing we should use. See - // CreateCollectionCommandResolver.validateVectorOptions() - // it gets the proper CQL names from the Enums, replacing what the user sent in. (kind of - // confusing) - // TODO: create a VectorSearchDef that uses the SimilarityFunction and EmbeddingSourceModel - // enums - if (vectorDesc != null) { - // Sanity checking here, if we pass a null value the map go bang, try to stop bang, bang bad - Map vectorOptions = new HashMap<>(); - if (vectorDesc.metric() != null && !vectorDesc.metric().isBlank()) { - vectorOptions.put("similarity_function", vectorDesc.metric()); - } - if (vectorDesc.sourceModel() != null && !vectorDesc.sourceModel().isBlank()) { - vectorOptions.put("source_model", vectorDesc.sourceModel()); - } - statements.add( - buildSaiIndex( - collectionExisted, "query_vector_value", "query_vector_value", false, vectorOptions)); + private SimpleStatement getCreateTable(String comment, CollectionLexicalDef overrideLexicalDef) { + + var keyspace = commandContext.schemaObject().identifier().keyspace(); + + CreateTable create = + SchemaBuilder.createTable(keyspace, collectionName) + .ifNotExists() + .withPartitionKey("key", DataTypes.tupleOf(DataTypes.TINYINT, DataTypes.TEXT)) + .withColumn("tx_id", DataTypes.TIMEUUID) + .withColumn("doc_json", DataTypes.TEXT) + .withColumn("exist_keys", DataTypes.setOf(DataTypes.TEXT)) + .withColumn("array_size", DataTypes.mapOf(DataTypes.TEXT, DataTypes.INT)) + .withColumn("array_contains", DataTypes.setOf(DataTypes.TEXT)) + .withColumn("query_bool_values", DataTypes.mapOf(DataTypes.TEXT, DataTypes.TINYINT)) + .withColumn("query_dbl_values", DataTypes.mapOf(DataTypes.TEXT, DataTypes.DECIMAL)) + .withColumn("query_text_values", DataTypes.mapOf(DataTypes.TEXT, DataTypes.TEXT)) + .withColumn( + "query_timestamp_values", DataTypes.mapOf(DataTypes.TEXT, DataTypes.TIMESTAMP)) + .withColumn("query_null_values", DataTypes.setOf(DataTypes.TEXT)); + + if (vectorDesc != null) { + create = + create.withColumn( + "query_vector_value", + new ExtendedVectorType(DataTypes.FLOAT, vectorDesc.dimension())); + } + if (overrideLexicalDef.enabled()) { + create = create.withColumn("query_lexical_value", DataTypes.TEXT); + } + + // adding the comment changes the return into something to deal with options + var statement = comment == null ? create.build() : create.withComment(comment).build(); + + if (LOGGER.isTraceEnabled()) { + LOGGER.trace("getCreateTable() - created table statement: {}", statement.getQuery()); + } + return statement; } - if (overrideLexicalDef.enabled()) { - var analyzerDef = overrideLexicalDef.analyzerDefinition(); - var analyzerString = analyzerDef.isTextual() ? analyzerDef.asText() : analyzerDef.toString(); - statements.add( - buildSaiIndex( - collectionExisted, - "query_lexical_value", - "query_lexical_value", - false, - Map.of("index_analyzer", analyzerString))); + /* + * When a createCollection is done on a table that already exist the index are run with IF NOT EXISTS. + * For a new table they are run without IF NOT EXISTS. + */ + private List getIndexStatements( + CollectionLexicalDef overrideLexicalDef, boolean collectionExisted) { + + List statements = new ArrayList<>(10); + + var denyAllIndexes = + getOrDefault(indexingDesc, CreateCollectionCommand.Options.IndexingDesc::denyAll, false); + + if (!denyAllIndexes) { + statements.add(saiColumn(collectionExisted, "exists_keys", "exist_keys")); + statements.add(saiEntries(collectionExisted, "array_size", "array_size")); + statements.add(saiColumn(collectionExisted, "array_contains", "array_contains")); + statements.add(saiEntries(collectionExisted, "query_bool_values", "query_bool_values")); + statements.add(saiEntries(collectionExisted, "query_dbl_values", "query_dbl_values")); + statements.add(saiEntries(collectionExisted, "query_text_values", "query_text_values")); + statements.add( + saiEntries(collectionExisted, "query_timestamp_values", "query_timestamp_values")); + statements.add(saiColumn(collectionExisted, "query_null_values", "query_null_values")); + } + + // NOTE: This is a little sloppy, in normal request the CreateCollectionCommandResolver will + // make sure the vectorDesc is valid and has defaults set. So even though they are strings + // they have been validated as the thing we should use. See + // CreateCollectionCommandResolver.validateVectorOptions() + // it gets the proper CQL names from the Enums, replacing what the user sent in. (kind of + // confusing) + // TODO: create a VectorSearchDef that uses the SimilarityFunction and EmbeddingSourceModel + // enums + if (vectorDesc != null) { + // Sanity checking here, if we pass a null value the map go bang, try to stop bang, bang bad + Map vectorOptions = new HashMap<>(); + if (vectorDesc.metric() != null && !vectorDesc.metric().isBlank()) { + vectorOptions.put("similarity_function", vectorDesc.metric()); + } + if (vectorDesc.sourceModel() != null && !vectorDesc.sourceModel().isBlank()) { + vectorOptions.put("source_model", vectorDesc.sourceModel()); + } + statements.add( + buildSaiIndex( + collectionExisted, "query_vector_value", "query_vector_value", false, vectorOptions)); + } + + if (overrideLexicalDef.enabled()) { + var analyzerDef = overrideLexicalDef.analyzerDefinition(); + var analyzerString = analyzerDef.isTextual() ? analyzerDef.asText() : analyzerDef.toString(); + statements.add( + buildSaiIndex( + collectionExisted, + "query_lexical_value", + "query_lexical_value", + false, + Map.of("index_analyzer", analyzerString))); + } + + if (LOGGER.isTraceEnabled()) { + var cqlStrings = + statements.stream().map(SimpleStatement::getQuery).collect(Collectors.joining("; ")); + LOGGER.trace("getIndexStatements() - created index statements: {}", cqlStrings); + } + return statements; } - if (LOGGER.isTraceEnabled()) { - var cqlStrings = - statements.stream().map(SimpleStatement::getQuery).collect(Collectors.joining("; ")); - LOGGER.trace("getIndexStatements() - created index statements: {}", cqlStrings); + private SimpleStatement saiColumn(boolean ifNotExists, String indexSuffix, String column) { + return buildSaiIndex(ifNotExists, indexSuffix, column, false, Map.of()); } - return statements; - } - - private SimpleStatement saiColumn(boolean ifNotExists, String indexSuffix, String column) { - return buildSaiIndex(ifNotExists, indexSuffix, column, false, Map.of()); - } - - private SimpleStatement saiEntries(boolean ifNotExists, String indexSuffix, String column) { - return buildSaiIndex(ifNotExists, indexSuffix, column, true, Map.of()); - } - - private SimpleStatement buildSaiIndex( - boolean ifNotExists, - String indexSuffix, - String columnName, // aaron - next change will make this a CQLIdentifier - boolean isEntries, - Map options) { - - var keyspace = commandContext.schemaObject().identifier().keyspace(); - var index = CqlIdentifier.fromInternal(collectionName.asInternal() + "_" + indexSuffix); - var column = CqlIdentifier.fromInternal(columnName); - - var start = SchemaBuilder.createIndex(index).custom(CQLSAIIndex.SAI_CLASS_NAME); - if (ifNotExists) { - start = start.ifNotExists(); + + private SimpleStatement saiEntries(boolean ifNotExists, String indexSuffix, String column) { + return buildSaiIndex(ifNotExists, indexSuffix, column, true, Map.of()); } - var onTable = start.onTable(keyspace, collectionName); - var createIndex = isEntries ? onTable.andColumnEntries(column) : onTable.andColumn(column); + private SimpleStatement buildSaiIndex( + boolean ifNotExists, + String indexSuffix, + String columnName, // aaron - next change will make this a CQLIdentifier + boolean isEntries, + Map options) { - if (!options.isEmpty()) { - // in the CQL statement OPTIONS are the things after WITH, and for the `create index` there is - // an option called OPTIONS calling withSASIOptions deals with this. - createIndex = createIndex.withSASIOptions(options); - } + var keyspace = commandContext.schemaObject().identifier().keyspace(); + var index = CqlIdentifier.fromInternal(collectionName.asInternal() + "_" + indexSuffix); + var column = CqlIdentifier.fromInternal(columnName); + + var start = SchemaBuilder.createIndex(index).custom(CQLSAIIndex.SAI_CLASS_NAME); + if (ifNotExists) { + start = start.ifNotExists(); + } - return new ExtendedCreateIndex((DefaultCreateIndex) createIndex).build(); - } + var onTable = start.onTable(keyspace, collectionName); + var createIndex = isEntries ? onTable.andColumnEntries(column) : onTable.andColumn(column); + + if (!options.isEmpty()) { + // in the CQL statement OPTIONS are the things after WITH, and for the `create index` there is + // an option called OPTIONS calling withSASIOptions deals with this. + createIndex = createIndex.withSASIOptions(options); + } + + return new ExtendedCreateIndex((DefaultCreateIndex) createIndex).build(); + } } diff --git a/src/main/java/io/stargate/sgv2/jsonapi/service/resolver/CreateCollectionCommandResolver.java b/src/main/java/io/stargate/sgv2/jsonapi/service/resolver/CreateCollectionCommandResolver.java index 4041163fa8..7666eda66d 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/service/resolver/CreateCollectionCommandResolver.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/service/resolver/CreateCollectionCommandResolver.java @@ -101,7 +101,6 @@ public Operation resolveKeyspaceCommand( return new CreateCollectionOperation( context, dbLimitsConfig, - context.cqlSessionCache(), collectionName, operationsConfig.databaseConfig().ddlDelayMillis(), operationsConfig.tooManyIndexesRollbackEnabled(), diff --git a/src/test/java/io/stargate/sgv2/jsonapi/TestConstants.java b/src/test/java/io/stargate/sgv2/jsonapi/TestConstants.java index e0c42122bc..73b687c3f3 100644 --- a/src/test/java/io/stargate/sgv2/jsonapi/TestConstants.java +++ b/src/test/java/io/stargate/sgv2/jsonapi/TestConstants.java @@ -5,6 +5,7 @@ import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; +import com.fasterxml.jackson.databind.ObjectMapper; import io.micrometer.core.instrument.MeterRegistry; import io.stargate.sgv2.jsonapi.api.model.command.CommandConfig; import io.stargate.sgv2.jsonapi.api.model.command.CommandContext; @@ -56,6 +57,7 @@ public class TestConstants { public final String APP_NAME; public final DatabaseType DATABASE_TYPE; public final TenantFactory SINGLETON_TENANT_FACTORY; + public final ObjectMapper OBJECT_MAPPER = new ObjectMapper(); // ============================================================ // Names diff --git a/src/test/java/io/stargate/sgv2/jsonapi/api/model/command/impl/IndexingDescTest.java b/src/test/java/io/stargate/sgv2/jsonapi/api/model/command/impl/IndexingDescTest.java new file mode 100644 index 0000000000..39b789f42e --- /dev/null +++ b/src/test/java/io/stargate/sgv2/jsonapi/api/model/command/impl/IndexingDescTest.java @@ -0,0 +1,331 @@ +package io.stargate.sgv2.jsonapi.api.model.command.impl; + +import static io.stargate.sgv2.jsonapi.util.exception.SchemaExceptionAssert.assertThatSchemaException; +import static org.assertj.core.api.Assertions.*; + +import com.fasterxml.jackson.core.JsonProcessingException; +import io.stargate.sgv2.jsonapi.TestConstants; +import io.stargate.sgv2.jsonapi.exception.ErrorCode; +import io.stargate.sgv2.jsonapi.exception.SchemaException; +import org.junit.jupiter.api.Test; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +/** Unit tests for the {@link CreateCollectionCommand.Options.IndexingDesc} */ +public class IndexingDescTest { + + private static final Logger LOGGER = LoggerFactory.getLogger(IndexingDescTest.class); + + private final TestConstants TEST_CONSTANTS = new TestConstants(); + + public CreateCollectionCommand.Options.IndexingDesc deserialize(String testName, String json) { + LOGGER.info("deserialize() - testName: {}, json: {}", testName, json); + + try { + return TEST_CONSTANTS.OBJECT_MAPPER.readValue( + json, CreateCollectionCommand.Options.IndexingDesc.class); + } catch (JsonProcessingException e) { + throw new RuntimeException(e); + } + } + + public void assertSchemaException( + String testName, String json, ErrorCode errorCode, String... message) { + + var indexingDesc = deserialize(testName, json); + var throwable = catchThrowable(indexingDesc::validate); + + assertThatSchemaException(throwable) + .as(testName) + .hasCode(errorCode) + .hasMessageSnippets(message); + } + + @Test + public void failAllowAndDeny() { + + var json = + """ + { + "deny": [ + "comment" + ], + "allow": [ + "data" + ] + }"""; + + assertSchemaException( + "failAllowAndDeny()", + json, + SchemaException.Code.INVALID_INDEXING_DEFINITION, + "'createCollection' indexing definition invalid: 'allow' and 'deny' cannot be used together"); + } + + @Test + public void failNeitherAllowNorDeny() { + + var json = + """ + {}"""; + + assertSchemaException( + "failNeitherAllowNorDeny()", + json, + SchemaException.Code.INVALID_INDEXING_DEFINITION, + "'allow' or 'deny' should be provided"); + } + + @Test + public void failAllowDuplicates() { + + var json = + """ + { + "allow": [ + "name", + "name" + ] + }"""; + + assertSchemaException( + "failAllowDuplicates()", + json, + SchemaException.Code.INVALID_INDEXING_DEFINITION, + "'allow' cannot contain duplicates"); + } + + @Test + public void failDenyDuplicates() { + + var json = + """ + { + "deny": [ + "name", + "name" + ] + }"""; + + assertSchemaException( + "failDenyDuplicates()", + json, + SchemaException.Code.INVALID_INDEXING_DEFINITION, + "'deny' cannot contain duplicates"); + } + + @Test + public void failAllowEmptyPath() { + + var json = + """ + { + "allow": [ + "" + ] + }"""; + + assertSchemaException( + "failAllowEmptyPath()", + json, + SchemaException.Code.INVALID_INDEXING_DEFINITION, + "path must be represented as a non-empty string"); + } + + @Test + public void failDenyEmptyPath() { + + var json = + """ + { + "deny": [ + "" + ] + }"""; + + assertSchemaException( + "failDenyEmptyPath()", + json, + SchemaException.Code.INVALID_INDEXING_DEFINITION, + "path must be represented as a non-empty string"); + } + + @Test + public void failAllowDollarPrefix() { + + var json = + """ + { + "allow": [ + "$score" + ] + }"""; + + assertSchemaException( + "failAllowDollarPrefix()", + json, + SchemaException.Code.INVALID_INDEXING_DEFINITION, + "path ('$score') must not start with '$'"); + } + + @Test + public void failDenyDollarPrefix() { + + var json = + """ + { + "deny": [ + "$score" + ] + }"""; + + assertSchemaException( + "failDenyDollarPrefix()", + json, + SchemaException.Code.INVALID_INDEXING_DEFINITION, + "path ('$score') must not start with '$'"); + } + + @Test + public void failAllowInvalidPath() { + + var json = + """ + { + "allow": [ + "bad&path" + ] + }"""; + + assertSchemaException( + "failAllowInvalidPath()", + json, + SchemaException.Code.INVALID_INDEXING_DEFINITION, + "indexing path ('bad&path') is not a valid path"); + } + + @Test + public void failDenyInvalidPath() { + + var json = + """ + { + "deny": [ + "bad&path" + ] + }"""; + + assertSchemaException( + "failDenyInvalidPath()", + json, + SchemaException.Code.INVALID_INDEXING_DEFINITION, + "indexing path ('bad&path') is not a valid path"); + } + + @Test + public void successAllowStar() { + + var json = + """ + { + "allow": [ + "*" + ] + }"""; + + var indexingDesc = deserialize("successAllowStar()", json); + indexingDesc.validate(); + assertThat(indexingDesc.allow()).containsExactly("*"); + assertThat(indexingDesc.deny()).isNull(); + } + + @Test + public void successDenyStar() { + + var json = + """ + { + "deny": [ + "*" + ] + }"""; + + var indexingDesc = deserialize("successDenyStar()", json); + indexingDesc.validate(); + assertThat(indexingDesc.deny()).containsExactly("*"); + assertThat(indexingDesc.denyAll()).as("delayAll true when deny '*'").isTrue(); + assertThat(indexingDesc.allow()).isNull(); + } + + @Test + public void successAllowVector() { + + var json = + """ + { + "allow": [ + "$vector" + ] + }"""; + + var indexingDesc = deserialize("successAllowVector()", json); + indexingDesc.validate(); + assertThat(indexingDesc.allow()).containsExactly("$vector"); + assertThat(indexingDesc.deny()).isNull(); + } + + @Test + public void successDenyVector() { + + var json = + """ + { + "deny": [ + "$vector" + ] + }"""; + + var indexingDesc = deserialize("successDenyVector()", json); + indexingDesc.validate(); + assertThat(indexingDesc.deny()).containsExactly("$vector"); + assertThat(indexingDesc.allow()).isNull(); + } + + @Test + public void successAllowValidPaths() { + + var json = + """ + { + "allow": [ + "name", + "address.city", + "tags" + ] + }"""; + + var indexingDesc = deserialize("successAllowValidPaths()", json); + indexingDesc.validate(); + assertThat(indexingDesc.allow()).containsExactly("name", "address.city", "tags"); + assertThat(indexingDesc.deny()).isNull(); + } + + @Test + public void successDenyValidPaths() { + + var json = + """ + { + "deny": [ + "name", + "address.city", + "tags" + ] + }"""; + + var indexingDesc = deserialize("successDenyValidPaths()", json); + indexingDesc.validate(); + assertThat(indexingDesc.deny()).containsExactly("name", "address.city", "tags"); + assertThat(indexingDesc.allow()).isNull(); + } +} diff --git a/src/test/java/io/stargate/sgv2/jsonapi/service/operation/collections/CreateCollectionOperationTest.java b/src/test/java/io/stargate/sgv2/jsonapi/service/operation/collections/CreateCollectionOperationTest.java index 65853c29e7..b9726996d9 100644 --- a/src/test/java/io/stargate/sgv2/jsonapi/service/operation/collections/CreateCollectionOperationTest.java +++ b/src/test/java/io/stargate/sgv2/jsonapi/service/operation/collections/CreateCollectionOperationTest.java @@ -33,7 +33,6 @@ import io.stargate.sgv2.jsonapi.api.model.command.impl.CreateCollectionCommand; import io.stargate.sgv2.jsonapi.config.DatabaseLimitsConfig; import io.stargate.sgv2.jsonapi.config.constants.TableCommentConstants; -import io.stargate.sgv2.jsonapi.service.cqldriver.CQLSessionCache; import io.stargate.sgv2.jsonapi.service.cqldriver.executor.QueryExecutor; import io.stargate.sgv2.jsonapi.service.cqldriver.executor.VectorColumnDefinition; import io.stargate.sgv2.jsonapi.service.cqldriver.executor.VectorConfig; @@ -183,7 +182,6 @@ public void createCollectionNoVector() { new CreateCollectionOperation( KEYSPACE_CONTEXT, databaseLimitsConfig, - mock(CQLSessionCache.class), TEST_CONSTANTS.COLLECTION_IDENTIFIER.table(), 10, false, @@ -231,7 +229,6 @@ public void createCollectionVector() { new CreateCollectionOperation( KEYSPACE_CONTEXT, databaseLimitsConfig, - mock(CQLSessionCache.class), TEST_CONSTANTS.COLLECTION_IDENTIFIER.table(), 10, false, @@ -290,7 +287,6 @@ public void denyAllCollectionNoVector() { new CreateCollectionOperation( KEYSPACE_CONTEXT, databaseLimitsConfig, - mock(CQLSessionCache.class), TEST_CONSTANTS.COLLECTION_IDENTIFIER.table(), 10, false, @@ -347,7 +343,6 @@ public void denyAllCollectionVector() { new CreateCollectionOperation( KEYSPACE_CONTEXT, databaseLimitsConfig, - mock(CQLSessionCache.class), TEST_CONSTANTS.COLLECTION_IDENTIFIER.table(), 10, false, @@ -416,7 +411,6 @@ public void indexAlreadyDropTable() { new CreateCollectionOperation( KEYSPACE_CONTEXT, databaseLimitsConfig, - mock(CQLSessionCache.class), TEST_CONSTANTS.COLLECTION_IDENTIFIER.table(), 10, true, diff --git a/src/test/java/io/stargate/sgv2/jsonapi/service/resolver/CreateCollectionCommandResolverTest.java b/src/test/java/io/stargate/sgv2/jsonapi/service/resolver/CreateCollectionCommandResolverTest.java index bf2fe808c3..651e0ac469 100644 --- a/src/test/java/io/stargate/sgv2/jsonapi/service/resolver/CreateCollectionCommandResolverTest.java +++ b/src/test/java/io/stargate/sgv2/jsonapi/service/resolver/CreateCollectionCommandResolverTest.java @@ -1,10 +1,12 @@ package io.stargate.sgv2.jsonapi.service.resolver; import static io.stargate.sgv2.jsonapi.util.CqlIdentifierUtil.cqlIdentifierFromUserInput; -import static org.assertj.core.api.Assertions.assertThat; +import static io.stargate.sgv2.jsonapi.util.exception.SchemaExceptionAssert.assertThatSchemaException; +import static org.assertj.core.api.Assertions.*; import static org.assertj.core.api.Assertions.catchThrowable; -import com.fasterxml.jackson.databind.ObjectMapper; +import com.datastax.oss.driver.api.core.CqlIdentifier; +import com.fasterxml.jackson.core.JsonProcessingException; import io.quarkus.test.junit.QuarkusTest; import io.quarkus.test.junit.TestProfile; import io.stargate.sgv2.jsonapi.TestConstants; @@ -15,415 +17,352 @@ import io.stargate.sgv2.jsonapi.service.operation.collections.CreateCollectionOperation; import io.stargate.sgv2.jsonapi.service.schema.EmbeddingSourceModel; import io.stargate.sgv2.jsonapi.service.schema.KeyspaceSchemaObject; +import io.stargate.sgv2.jsonapi.service.schema.SimilarityFunction; +import io.stargate.sgv2.jsonapi.util.profiles.EnabledVectorizeProfile; import jakarta.inject.Inject; import java.util.List; import java.util.Map; import org.apache.commons.lang3.RandomStringUtils; -import org.junit.jupiter.api.BeforeEach; -import org.junit.jupiter.api.Nested; import org.junit.jupiter.api.Test; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; @QuarkusTest @TestProfile(EnabledVectorizeProfile.class) class CreateCollectionCommandResolverTest { - @Inject ObjectMapper objectMapper; - @Inject CreateCollectionCommandResolver resolver; + private static final Logger LOGGER = + LoggerFactory.getLogger(CreateCollectionCommandResolverTest.class); + @Inject private CreateCollectionCommandResolver RESOLVER; private final TestConstants TEST_CONSTANTS = new TestConstants(); - CommandContext commandContext; + // want a single instance for all calls, keyspaceContext() creates a new each call + private final CommandContext COMMAND_CONTEXT = + TEST_CONSTANTS.keyspaceContext(); - @BeforeEach - public void beforeEach() { - commandContext = TEST_CONSTANTS.keyspaceContext(); + private Throwable resolveCommandThrows(String testName, String rawJson) { + return catchThrowable( + () -> resolveCommand(testName, rawJson, TEST_CONSTANTS.COLLECTION_IDENTIFIER.table())); } - @Nested - class CreateCollectionSuccess { + private CreateCollectionOperation resolveCommand(String testName, String rawJson) { + return resolveCommand(testName, rawJson, TEST_CONSTANTS.COLLECTION_IDENTIFIER.table()); + } - @Test - public void happyPath() throws Exception { - var json = - TEST_CONSTANTS.subsRawNames( - """ - { - "createCollection": { - "name" : "${collection}" - } - } - """); - - var command = objectMapper.readValue(json, CreateCollectionCommand.class); - var operation = resolver.resolveCommand(commandContext, command); - - assertThat(operation) - .isInstanceOfSatisfying( - CreateCollectionOperation.class, - op -> { - assertThat(op.collectionName()) - .isEqualTo(TEST_CONSTANTS.COLLECTION_IDENTIFIER.table()); - assertThat(op.commandContext()).isEqualTo(commandContext); - assertThat(op.vectorDesc()).isNull(); - }); - } + private CreateCollectionOperation resolveCommand( + String testName, String rawJson, CqlIdentifier collectionName) { - @Test - public void happyPathVectorSearch() throws Exception { - var json = - TEST_CONSTANTS.subsRawNames( - """ - { - "createCollection": { - "name" : "${collection}", - "options": { - "vector": { - "dimension": 4, - "metric": "cosine" - } - } - } - } - """); - - var command = objectMapper.readValue(json, CreateCollectionCommand.class); - var operation = resolver.resolveCommand(commandContext, command); - - assertThat(operation) - .isInstanceOfSatisfying( - CreateCollectionOperation.class, - op -> { - assertThat(op.collectionName()) - .isEqualTo(TEST_CONSTANTS.COLLECTION_IDENTIFIER.table()); - assertThat(op.commandContext()).isEqualTo(commandContext); - assertThat(op.vectorDesc()).isNotNull(); - assertThat(op.vectorDesc().dimension()).isEqualTo(4); - assertThat(op.vectorDesc().metric()).isEqualTo("cosine"); - }); + var json = TEST_CONSTANTS.subsRawNames(rawJson); + LOGGER.info("resolveCommand() - testName: {}, json: {}", testName, json); + + CreateCollectionCommand command; + try { + command = TEST_CONSTANTS.OBJECT_MAPPER.readValue(json, CreateCollectionCommand.class); + } catch (JsonProcessingException e) { + throw new RuntimeException(e); } - @Test - public void happyPathVectorizeSearch() throws Exception { - var json = - TEST_CONSTANTS.subsRawNames( - """ - { - "createCollection": { - "name": "${collection}", - "options": { - "vector": { - "metric": "cosine", - "dimension": 768, - "service": { - "provider": "azureOpenAI", - "modelName": "text-embedding-3-small", - "parameters": { - "resourceName": "test", - "deploymentId": "test" - } + var operation = RESOLVER.resolveCommand(COMMAND_CONTEXT, command); + + assertThat(operation) + .isInstanceOfSatisfying( + CreateCollectionOperation.class, + op -> { + assertThat(op.collectionName()) + .as("%s - collectionName() matches command", testName) + .isEqualTo(collectionName); + assertThat(op.commandContext()) + .as("%s - commandContext() is same object", testName) + .isSameAs(COMMAND_CONTEXT); + }); + return (CreateCollectionOperation) operation; + } + + @Test + public void successWithDefaults() { + var operation = + resolveCommand( + "successWithDefaults()", + """ + { + "createCollection": { + "name": "${collection}" } } - } - } - } - """); - // NOTE: source model of null turns into DEFAULT - var expectedVectorDesc = - new CreateCollectionCommand.Options.VectorSearchDesc( - 768, - "cosine", - EmbeddingSourceModel.DEFAULT.cqlName(), - new VectorizeConfig( - "azureOpenAI", - "text-embedding-3-small", - null, - Map.of("resourceName", "test", "deploymentId", "test"))); - - var command = objectMapper.readValue(json, CreateCollectionCommand.class); - var operation = resolver.resolveCommand(commandContext, command); - - // NOTE: this used to check the table comment string that was created, that has moved to the - // CreateCollectionOperationTest - assertThat(operation) - .isInstanceOfSatisfying( - CreateCollectionOperation.class, - op -> { - assertThat(op.collectionName()) - .isEqualTo(TEST_CONSTANTS.COLLECTION_IDENTIFIER.table()); - assertThat(op.commandContext()).isEqualTo(commandContext); - assertThat(op.vectorDesc()).isEqualTo(expectedVectorDesc); - }); - } + """); + + assertThat(operation.docIdDesc()).isNull(); + assertThat(operation.indexingDesc()).isNull(); + assertThat(operation.vectorDesc()).isNull(); + assertThat(operation.indexingDesc()).isNull(); + assertThat(operation.lexicalDef()) + .isEqualTo( + COMMAND_CONTEXT.requestContext().schemaRegistry().lexicalDef().currentVersion(null)); + assertThat(operation.rerankDef()) + .isEqualTo( + COMMAND_CONTEXT.requestContext().schemaRegistry().rerankDef().currentVersion(null)); + } - @Test - public void happyPathIndexing() throws Exception { + @Test + public void successWithSupportedNames() { + String[] supportedNames = {"a", "A", "0", "_", "a0", "0a_A", "_0a"}; + for (String name : supportedNames) { var json = - TEST_CONSTANTS.subsRawNames( """ - { - "createCollection": { - "name" : "${collection}", - "options": { - "indexing": { - "deny" : ["comment"] - } - } - } - } - """); - var expectedIndexing = - new CreateCollectionCommand.Options.IndexingDesc(null, List.of("comment")); - - var command = objectMapper.readValue(json, CreateCollectionCommand.class); - var operation = resolver.resolveCommand(commandContext, command); - - assertThat(operation) - .isInstanceOfSatisfying( - CreateCollectionOperation.class, - op -> { - assertThat(op.collectionName()) - .isEqualTo(TEST_CONSTANTS.COLLECTION_IDENTIFIER.table()); - assertThat(op.commandContext()).isEqualTo(commandContext); - assertThat(op.indexingDesc()).isEqualTo(expectedIndexing); - }); + { + "createCollection": { + "name": "%s" + } + } + """ + .formatted(name); + resolveCommand("successWithSupportedNames()", json, cqlIdentifierFromUserInput(name)); } + } - @Test - public void happyPathVectorSearchDefaultFunction() throws Exception { - - var json = - TEST_CONSTANTS.subsRawNames( - """ - { - "createCollection": { - "name" : "${collection}", - "options": { - "vector": { - "dimension": 4 - } - } - } - } - """); - - var command = objectMapper.readValue(json, CreateCollectionCommand.class); - var operation = resolver.resolveCommand(commandContext, command); - - assertThat(operation) - .isInstanceOfSatisfying( - CreateCollectionOperation.class, - op -> { - assertThat(op.collectionName()) - .isEqualTo(TEST_CONSTANTS.COLLECTION_IDENTIFIER.table()); - assertThat(op.commandContext()).isEqualTo(commandContext); - assertThat(op.vectorDesc()).isNotNull(); - assertThat(op.vectorDesc().dimension()).isEqualTo(4); - assertThat(op.vectorDesc().metric()).isEqualTo("COSINE"); - }); - } + @Test + public void successWithVector() { + var operation = + resolveCommand( + "successWithVector()", + """ + { + "createCollection": { + "name": "${collection}", + "options": { + "vector": { + "dimension": 4, + "metric": "cosine", + "sourceModel": "openai-v3-large" + } + } + } + } + """); - @Test - public void createCollectionWithSupportedName() throws Exception { + var expectedVectorDesc = + new CreateCollectionCommand.Options.VectorSearchDesc( + 4, "cosine", EmbeddingSourceModel.OPENAI_V3_LARGE.apiName(), null); - String[] supportedName = {"a", "A", "0", "_", "a0", "0a_A", "_0a"}; - for (String name : supportedName) { - String json = - """ - { - "createCollection": { - "name" : "%s" - } - } - """ - .formatted(name); - - var command = objectMapper.readValue(json, CreateCollectionCommand.class); - var operation = resolver.resolveCommand(commandContext, command); - - assertThat(operation) - .isInstanceOfSatisfying( - CreateCollectionOperation.class, - op -> { - assertThat(op.collectionName()).isEqualTo(cqlIdentifierFromUserInput(name)); - assertThat(op.commandContext()).isEqualTo(commandContext); - assertThat(op.vectorDesc()).isNull(); - }); - } - } + assertThat(operation.vectorDesc()).isEqualTo(expectedVectorDesc); } - @Nested - class CreateCollectionFailure { - - @Test - public void indexingOptionsError() throws Exception { - - var json = - TEST_CONSTANTS.subsRawNames( - """ - { - "createCollection": { - "name" : "${collection}", - "options": { - "vector": { - "dimension": 4, - "metric": "cosine" - }, - "indexing": { - "deny" : ["comment"], - "allow" : ["data"] + @Test + public void successWithVectorDefaultMetric() { + + var operation = + resolveCommand( + "successWithVectorDefaultMetric()", + """ + { + "createCollection": { + "name": "${collection}", + "options": { + "vector": { + "dimension": 4 + } + } + } } - } - } - } - """); - - var command = objectMapper.readValue(json, CreateCollectionCommand.class); - var throwable = catchThrowable(() -> resolver.resolveCommand(commandContext, command)); - - assertThat(throwable) - .isInstanceOf(SchemaException.class) - .satisfies( - e -> { - SchemaException exception = (SchemaException) e; - assertThat(exception.getMessage()) - .containsSequence( - "'createCollection' indexing definition invalid: 'allow' and 'deny' cannot be used together"); - assertThat(exception.code) - .isEqualTo(SchemaException.Code.INVALID_INDEXING_DEFINITION.name()); - }); - } + """); + + // aaron 15 june 26 - not sure why but we need the name of the source model must be the CQL name + // cql cares about capitals, we dont when processing this normally + var expectedVectorDesc = + new CreateCollectionCommand.Options.VectorSearchDesc( + 4, + SimilarityFunction.COSINE.cqlIndexingFunction(), + EmbeddingSourceModel.OTHER.cqlName(), + null); + + assertThat(operation.vectorDesc()).isEqualTo(expectedVectorDesc); + } - @Test - public void createCollectionWithNull() throws Exception { + @Test + public void successWithVectorize() { + var operation = + resolveCommand( + "successWithVector()", + """ + { + "createCollection": { + "name": "${collection}", + "options": { + "vector": { + "metric": "cosine", + "dimension": 768, + "service": { + "provider": "azureOpenAI", + "modelName": "text-embedding-3-small", + "parameters": { + "resourceName": "testResourceName", + "deploymentId": "testResourceName" + } + } + } + } + } + } + """); + + // NOTE: source model of null turns into DEFAULT + // aaron 15 june 26 - not sure why but we need the name of the source model must be the CQL name + // cql cares about capitals, we dont when processing this normally + var expectedVectorDesc = + new CreateCollectionCommand.Options.VectorSearchDesc( + 768, + "cosine", + EmbeddingSourceModel.DEFAULT.cqlName(), + new VectorizeConfig( + "azureOpenAI", + "text-embedding-3-small", + null, + Map.of("resourceName", "testResourceName", "deploymentId", "testResourceName"))); + + assertThat(operation.vectorDesc()).isEqualTo(expectedVectorDesc); + } - var json = - """ - { - "createCollection": { - } - } - """; - - var command = objectMapper.readValue(json, CreateCollectionCommand.class); - var throwable = catchThrowable(() -> resolver.resolveCommand(commandContext, command)); - - verifySchemaException( - throwable, - SchemaException.Code.UNSUPPORTED_SCHEMA_NAME, - "The command attempted to create a Collection with a name that is not supported.", - "The supported Collection names must not be empty, more than 48 characters long, or contain non-alphanumeric-underscore characters.", - "The command used the unsupported Collection name: '(null)'."); - } + @Test + public void successWithDenyIndexing() { + var operation = + resolveCommand( + "successWithDenyIndexing()", + """ + { + "createCollection": { + "name": "${collection}", + "options": { + "indexing": { + "deny": [ + "comment" + ] + } + } + } + } + """); - @Test - public void createCollectionWithEmptyName() throws Exception { + var expectedIndexing = + new CreateCollectionCommand.Options.IndexingDesc(null, List.of("comment")); - var json = - """ - { - "createCollection": { - "name" : "" - } - } - """; - - var command = objectMapper.readValue(json, CreateCollectionCommand.class); - var throwable = catchThrowable(() -> resolver.resolveCommand(commandContext, command)); - - verifySchemaException( - throwable, - SchemaException.Code.UNSUPPORTED_SCHEMA_NAME, - "The command attempted to create a Collection with a name that is not supported.", - "The supported Collection names must not be empty, more than 48 characters long, or contain non-alphanumeric-underscore characters.", - "The command used the unsupported Collection name: ''."); - } + assertThat(operation.indexingDesc()).isEqualTo(expectedIndexing); + } - @Test - public void createCollectionWithBlankName() throws Exception { + @Test + public void failWithUndefinedName() { - var json = - """ - { - "createCollection": { - "name": " " - } - } - """; - - var command = objectMapper.readValue(json, CreateCollectionCommand.class); - var throwable = catchThrowable(() -> resolver.resolveCommand(commandContext, command)); - - verifySchemaException( - throwable, - SchemaException.Code.UNSUPPORTED_SCHEMA_NAME, - "The command attempted to create a Collection with a name that is not supported.", - "The supported Collection names must not be empty, more than 48 characters long, or contain non-alphanumeric-underscore characters.", - "The command used the unsupported Collection name: ' '."); - } + var throwable = + resolveCommandThrows( + "failWithUndefinedName()", + """ + { + "createCollection": {} + } + """); + + assertThatSchemaException(throwable) + .as("failWithUndefinedName()") + .hasCode(SchemaException.Code.UNSUPPORTED_SCHEMA_NAME) + .hasMessageSnippets( + "The command attempted to create a Collection with a name that is not supported.", + "The supported Collection names must not be empty, more than 48 characters long, or contain non-alphanumeric-underscore characters.", + "The command used the unsupported Collection name: '(null)'."); + } - @Test - public void createCollectionWithNameTooLong() throws Exception { + @Test + public void failWithEmptyName() { - var name = RandomStringUtils.insecure().nextAlphabetic(49); - var json = - """ - { - "createCollection": { - "name": "%s" - } - } - """ - .formatted(name); - - var command = objectMapper.readValue(json, CreateCollectionCommand.class); - var throwable = catchThrowable(() -> resolver.resolveCommand(commandContext, command)); + var throwable = + resolveCommandThrows( + "failWithUndefinedName()", + """ + { + "createCollection": { + "name": "" + } + } + """); + + assertThatSchemaException(throwable) + .as("failWithEmptyName()") + .hasCode(SchemaException.Code.UNSUPPORTED_SCHEMA_NAME) + .hasMessageSnippets( + "The command attempted to create a Collection with a name that is not supported.", + "The supported Collection names must not be empty, more than 48 characters long, or contain non-alphanumeric-underscore characters.", + "The command used the unsupported Collection name: ''."); + } - verifySchemaException( - throwable, - SchemaException.Code.UNSUPPORTED_SCHEMA_NAME, - "The command attempted to create a Collection with a name that is not supported.", - "The supported Collection names must not be empty, more than 48 characters long, or contain non-alphanumeric-underscore characters.", - "The command used the unsupported Collection name: '%s'.".formatted(name)); - } + @Test + public void failWithBlankName() { + + // Blank is a white space + var throwable = + resolveCommandThrows( + "failWithBlankName()", + """ + { + "createCollection": { + "name": " " + } + } + """); + + assertThatSchemaException(throwable) + .as("failWithBlankName()") + .hasCode(SchemaException.Code.UNSUPPORTED_SCHEMA_NAME) + .hasMessageSnippets( + "The command attempted to create a Collection with a name that is not supported.", + "The supported Collection names must not be empty, more than 48 characters long, or contain non-alphanumeric-underscore characters.", + "The command used the unsupported Collection name: ' '."); + } - @Test - public void createCollectionWithSpecialCharacter() throws Exception { + @Test + public void failWithNameTooLong() { - var json = - """ - { - "createCollection": { - "name": "!@-" - } - } - """; - - var command = objectMapper.readValue(json, CreateCollectionCommand.class); - var throwable = catchThrowable(() -> resolver.resolveCommand(commandContext, command)); - - verifySchemaException( - throwable, - SchemaException.Code.UNSUPPORTED_SCHEMA_NAME, - "The command attempted to create a Collection with a name that is not supported.", - "The supported Collection names must not be empty, more than 48 characters long, or contain non-alphanumeric-underscore characters.", - "The command used the unsupported Collection name: '!@-'."); - } + var longName = RandomStringUtils.insecure().nextAlphabetic(49); + var throwable = + resolveCommandThrows( + "failWithNameTooLong()", + """ + { + "createCollection": { + "name": "%s" + } + } + """ + .formatted(longName)); + + assertThatSchemaException(throwable) + .as("failWithNameTooLong()") + .hasCode(SchemaException.Code.UNSUPPORTED_SCHEMA_NAME) + .hasMessageSnippets( + "The command attempted to create a Collection with a name that is not supported.", + "The supported Collection names must not be empty, more than 48 characters long, or contain non-alphanumeric-underscore characters.", + "The command used the unsupported Collection name: '%s'.".formatted(longName)); } - private void verifySchemaException( - Throwable throwable, SchemaException.Code expectedErrorCode, String... messageSnippet) { - - assertThat(throwable) - .isInstanceOf(SchemaException.class) - .satisfies( - e -> { - SchemaException exception = (SchemaException) e; - assertThat(exception.code).isEqualTo(expectedErrorCode.name()); - for (String snippet : messageSnippet) { - assertThat(exception.getMessage()).contains(snippet); - } - }); + @Test + public void failWithNameSpecialCharacter() { + + var throwable = + resolveCommandThrows( + "failWithNameSpecialCharacter()", + """ + { + "createCollection": { + "name": "!@-" + } + } + """); + + assertThatSchemaException(throwable) + .as("failWithNameSpecialCharacter()") + .hasCode(SchemaException.Code.UNSUPPORTED_SCHEMA_NAME) + .hasMessageSnippets( + "The command attempted to create a Collection with a name that is not supported.", + "The supported Collection names must not be empty, more than 48 characters long, or contain non-alphanumeric-underscore characters.", + "The command used the unsupported Collection name: '!@-'."); } } diff --git a/src/test/java/io/stargate/sgv2/jsonapi/service/resolver/CreateCollectionResolverVectorizeDisabledTest.java b/src/test/java/io/stargate/sgv2/jsonapi/service/resolver/CreateCollectionResolverVectorizeDisabledTest.java index e42f6d7d7a..310c462ba3 100644 --- a/src/test/java/io/stargate/sgv2/jsonapi/service/resolver/CreateCollectionResolverVectorizeDisabledTest.java +++ b/src/test/java/io/stargate/sgv2/jsonapi/service/resolver/CreateCollectionResolverVectorizeDisabledTest.java @@ -10,6 +10,7 @@ import io.stargate.sgv2.jsonapi.api.model.command.impl.CreateCollectionCommand; import io.stargate.sgv2.jsonapi.api.model.command.impl.FindEmbeddingProvidersCommand; import io.stargate.sgv2.jsonapi.exception.SchemaException; +import io.stargate.sgv2.jsonapi.util.profiles.DisableVectorizeProfile; import jakarta.inject.Inject; import org.junit.jupiter.api.Test; diff --git a/src/test/java/io/stargate/sgv2/jsonapi/util/exception/APIExceptionAssert.java b/src/test/java/io/stargate/sgv2/jsonapi/util/exception/APIExceptionAssert.java new file mode 100644 index 0000000000..fa3bed815f --- /dev/null +++ b/src/test/java/io/stargate/sgv2/jsonapi/util/exception/APIExceptionAssert.java @@ -0,0 +1,39 @@ +package io.stargate.sgv2.jsonapi.util.exception; + +import static org.assertj.core.api.AssertionsForClassTypes.assertThat; + +import io.stargate.sgv2.jsonapi.exception.APIException; +import io.stargate.sgv2.jsonapi.exception.ErrorCode; +import java.util.function.Function; +import org.assertj.core.api.AbstractAssert; + +public abstract class APIExceptionAssert< + SELF extends APIExceptionAssert, ERROR extends APIException> + extends AbstractAssert { + + protected APIExceptionAssert(ERROR actual, Class selfClazz) { + super(actual, selfClazz); + } + + protected static , ERROR extends APIException> + SELF assertThatAPIException( + Function assertCtor, Class errorClass, Throwable throwable) { + + assertThat(throwable).isInstanceOf(errorClass); + return assertCtor.apply(errorClass.cast(throwable)); + } + + public SELF hasCode(ErrorCode expected) { + isNotNull(); + assertThat(actual.code).isEqualTo(expected.name()); + return myself; + } + + public SELF hasMessageSnippets(String... snippets) { + isNotNull(); + for (String snippet : snippets) { + assertThat(actual.getMessage()).contains(snippet); + } + return myself; + } +} diff --git a/src/test/java/io/stargate/sgv2/jsonapi/util/exception/SchemaExceptionAssert.java b/src/test/java/io/stargate/sgv2/jsonapi/util/exception/SchemaExceptionAssert.java new file mode 100644 index 0000000000..c29b2dd4b3 --- /dev/null +++ b/src/test/java/io/stargate/sgv2/jsonapi/util/exception/SchemaExceptionAssert.java @@ -0,0 +1,20 @@ +package io.stargate.sgv2.jsonapi.util.exception; + +import io.stargate.sgv2.jsonapi.exception.SchemaException; + +public class SchemaExceptionAssert + extends APIExceptionAssert { + + private SchemaExceptionAssert(SchemaException actual) { + super(actual, SchemaExceptionAssert.class); + } + + public static SchemaExceptionAssert assertThatSchemaException(SchemaException schemaException) { + return assertThatAPIException( + SchemaExceptionAssert::new, SchemaException.class, schemaException); + } + + public static SchemaExceptionAssert assertThatSchemaException(Throwable throwable) { + return assertThatAPIException(SchemaExceptionAssert::new, SchemaException.class, throwable); + } +} diff --git a/src/test/java/io/stargate/sgv2/jsonapi/service/resolver/DisableVectorizeProfile.java b/src/test/java/io/stargate/sgv2/jsonapi/util/profiles/DisableVectorizeProfile.java similarity index 90% rename from src/test/java/io/stargate/sgv2/jsonapi/service/resolver/DisableVectorizeProfile.java rename to src/test/java/io/stargate/sgv2/jsonapi/util/profiles/DisableVectorizeProfile.java index ddf69aaf18..089362743b 100644 --- a/src/test/java/io/stargate/sgv2/jsonapi/service/resolver/DisableVectorizeProfile.java +++ b/src/test/java/io/stargate/sgv2/jsonapi/util/profiles/DisableVectorizeProfile.java @@ -1,4 +1,4 @@ -package io.stargate.sgv2.jsonapi.service.resolver; +package io.stargate.sgv2.jsonapi.util.profiles; import com.google.common.collect.ImmutableMap; import io.quarkus.test.junit.QuarkusTestProfile; diff --git a/src/test/java/io/stargate/sgv2/jsonapi/service/resolver/EnabledVectorizeProfile.java b/src/test/java/io/stargate/sgv2/jsonapi/util/profiles/EnabledVectorizeProfile.java similarity index 90% rename from src/test/java/io/stargate/sgv2/jsonapi/service/resolver/EnabledVectorizeProfile.java rename to src/test/java/io/stargate/sgv2/jsonapi/util/profiles/EnabledVectorizeProfile.java index 64fe0da76a..5c5b8325bd 100644 --- a/src/test/java/io/stargate/sgv2/jsonapi/service/resolver/EnabledVectorizeProfile.java +++ b/src/test/java/io/stargate/sgv2/jsonapi/util/profiles/EnabledVectorizeProfile.java @@ -1,4 +1,4 @@ -package io.stargate.sgv2.jsonapi.service.resolver; +package io.stargate.sgv2.jsonapi.util.profiles; import com.google.common.collect.ImmutableMap; import io.quarkus.test.junit.QuarkusTestProfile; From ad487855b0a4e8f191d89a64e7a4e0225178c539 Mon Sep 17 00:00:00 2001 From: Aaron Morton Date: Tue, 16 Jun 2026 14:12:53 +1200 Subject: [PATCH 2/8] WIP --- .../model/command/impl/IndexingDescTest.java | 2 +- .../CreateCollectionOperationTest.java | 387 ++++++++++-------- .../CreateCollectionCommandResolverTest.java | 2 +- .../APIExceptionAssert.java | 6 +- .../util/asserts/CommandResultAssert.java | 127 ++++++ .../jsonapi/util/asserts/DataAPIAsserts.java | 20 + .../SchemaExceptionAssert.java | 6 +- 7 files changed, 383 insertions(+), 167 deletions(-) rename src/test/java/io/stargate/sgv2/jsonapi/util/{exception => asserts}/APIExceptionAssert.java (85%) create mode 100644 src/test/java/io/stargate/sgv2/jsonapi/util/asserts/CommandResultAssert.java create mode 100644 src/test/java/io/stargate/sgv2/jsonapi/util/asserts/DataAPIAsserts.java rename src/test/java/io/stargate/sgv2/jsonapi/util/{exception => asserts}/SchemaExceptionAssert.java (67%) diff --git a/src/test/java/io/stargate/sgv2/jsonapi/api/model/command/impl/IndexingDescTest.java b/src/test/java/io/stargate/sgv2/jsonapi/api/model/command/impl/IndexingDescTest.java index 39b789f42e..e326d22b7a 100644 --- a/src/test/java/io/stargate/sgv2/jsonapi/api/model/command/impl/IndexingDescTest.java +++ b/src/test/java/io/stargate/sgv2/jsonapi/api/model/command/impl/IndexingDescTest.java @@ -1,6 +1,6 @@ package io.stargate.sgv2.jsonapi.api.model.command.impl; -import static io.stargate.sgv2.jsonapi.util.exception.SchemaExceptionAssert.assertThatSchemaException; +import static io.stargate.sgv2.jsonapi.util.asserts.DataAPIAsserts.assertThatSchemaException; import static org.assertj.core.api.Assertions.*; import com.fasterxml.jackson.core.JsonProcessingException; diff --git a/src/test/java/io/stargate/sgv2/jsonapi/service/operation/collections/CreateCollectionOperationTest.java b/src/test/java/io/stargate/sgv2/jsonapi/service/operation/collections/CreateCollectionOperationTest.java index b9726996d9..c1e1ae3f74 100644 --- a/src/test/java/io/stargate/sgv2/jsonapi/service/operation/collections/CreateCollectionOperationTest.java +++ b/src/test/java/io/stargate/sgv2/jsonapi/service/operation/collections/CreateCollectionOperationTest.java @@ -1,6 +1,9 @@ package io.stargate.sgv2.jsonapi.service.operation.collections; +import static io.stargate.sgv2.jsonapi.util.asserts.DataAPIAsserts.assertThatCommandResult; +import static io.stargate.sgv2.jsonapi.util.asserts.DataAPIAsserts.assertThatSchemaException; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.catchThrowable; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.argThat; import static org.mockito.ArgumentMatchers.eq; @@ -9,20 +12,13 @@ import com.datastax.oss.driver.api.core.CqlIdentifier; import com.datastax.oss.driver.api.core.cql.AsyncResultSet; -import com.datastax.oss.driver.api.core.cql.ColumnDefinitions; import com.datastax.oss.driver.api.core.cql.Row; import com.datastax.oss.driver.api.core.cql.SimpleStatement; import com.datastax.oss.driver.api.core.metadata.Metadata; import com.datastax.oss.driver.api.core.metadata.Node; -import com.datastax.oss.driver.api.core.metadata.schema.ColumnMetadata; import com.datastax.oss.driver.api.core.metadata.schema.KeyspaceMetadata; import com.datastax.oss.driver.api.core.servererrors.InvalidQueryException; -import com.datastax.oss.driver.api.core.type.DataType; -import com.datastax.oss.driver.internal.core.metadata.schema.DefaultColumnMetadata; import com.datastax.oss.driver.internal.core.metadata.schema.DefaultKeyspaceMetadata; -import com.datastax.oss.driver.internal.core.type.DefaultTupleType; -import com.datastax.oss.driver.internal.core.type.PrimitiveType; -import com.datastax.oss.protocol.internal.ProtocolConstants; import com.fasterxml.jackson.core.JacksonException; import com.fasterxml.jackson.databind.JsonNode; import com.fasterxml.jackson.databind.ObjectMapper; @@ -30,12 +26,14 @@ import io.quarkus.test.junit.TestProfile; import io.smallrye.mutiny.Uni; import io.smallrye.mutiny.helpers.test.UniAssertSubscriber; +import io.stargate.sgv2.jsonapi.api.model.command.CommandStatus; import io.stargate.sgv2.jsonapi.api.model.command.impl.CreateCollectionCommand; import io.stargate.sgv2.jsonapi.config.DatabaseLimitsConfig; import io.stargate.sgv2.jsonapi.config.constants.TableCommentConstants; +import io.stargate.sgv2.jsonapi.exception.APIException; +import io.stargate.sgv2.jsonapi.exception.SchemaException; import io.stargate.sgv2.jsonapi.service.cqldriver.executor.QueryExecutor; import io.stargate.sgv2.jsonapi.service.cqldriver.executor.VectorColumnDefinition; -import io.stargate.sgv2.jsonapi.service.cqldriver.executor.VectorConfig; import io.stargate.sgv2.jsonapi.service.resolver.CreateCollectionCommandResolver; import io.stargate.sgv2.jsonapi.service.schema.EmbeddingSourceModel; import io.stargate.sgv2.jsonapi.service.schema.SimilarityFunction; @@ -49,7 +47,7 @@ import java.util.*; import java.util.concurrent.atomic.AtomicInteger; import java.util.regex.Pattern; -import org.junit.jupiter.api.BeforeEach; + import org.junit.jupiter.api.Test; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -97,87 +95,101 @@ public class CreateCollectionOperationTest extends OperationTestBase { // Comment to extract comment from the crete table cql statement. // Assume it is delineated by single quotes - private static final Pattern COMMENT_PATTERN = Pattern.compile("comment='(.*?)'"); - - private final ColumnDefinitions RESULT_COLUMNS = - buildColumnDefs(OperationTestBase.TestColumn.ofBoolean("[applied]")); + private static final Pattern TABLE_COMMENT_PATTERN = Pattern.compile("comment='(.*?)'"); - private AsyncResultSet mockSuccessSchemaResultset() { - List resultRows = - Arrays.asList(new MockRow(RESULT_COLUMNS, 0, Arrays.asList(byteBufferFrom(true)))); - return new MockAsyncResultSet(RESULT_COLUMNS, resultRows, null); + private SchemaChangeMemento assertOperation(String testName, + CreateCollectionOperation operation, + int expectedCreateTable, + int expectedCreateIndex){ + return assertOperation(testName, operation, expectedCreateTable, expectedCreateIndex, true, false); } - private record SchemaChangeMemento(AtomicInteger counter, List cqlComments) { - SchemaChangeMemento { - counter = counter == null ? new AtomicInteger() : counter; - cqlComments = cqlComments == null ? new ArrayList<>() : cqlComments; + private SchemaChangeMemento assertOperation(String testName, + CreateCollectionOperation operation, + int expectedCreateTable, + int expectedCreateIndex, + boolean mockKeyspaceMetadata, + boolean awaitFailure) { + LOGGER.info("assertOperation() - testName={}, expectedCreateTable={}, expectedCreateIndex={}", testName, expectedCreateTable, expectedCreateIndex); + + // track calls to change the schema and mock the keyspace exists for the new collection + var mockQueryExecutor = mock(QueryExecutor.class); + var schemaChangeMemento = addSchemaChangeMomento(mockQueryExecutor); + addMockKeyspaceMetadata(mockQueryExecutor, mockKeyspaceMetadata); + + // IMPORTANT - we still need to confirm the DB calls even in an error condition + // so catch, check db activity, then rethrow + var subscriber = awaitFailure ? + operation + .execute(requestContext, mockQueryExecutor) + .subscribe() + .withSubscriber(UniAssertSubscriber.create()) + .awaitFailure() + : + operation + .execute(requestContext, mockQueryExecutor) + .subscribe() + .withSubscriber(UniAssertSubscriber.create()) + .awaitItem(); + + LOGGER.info("assertOperation() - testName={}, schemaChangeMemento={}", testName, schemaChangeMemento); + + // Validate the change schema calls + assertThat(schemaChangeMemento.counter.get()) + .as("%s - total schema calls is expected") + .isEqualTo(expectedCreateTable + expectedCreateIndex); + + int actualCreateTables = schemaChangeMemento.queries.stream() + .filter(query -> query.startsWith("CREATE TABLE")) + .toList() + .size(); + assertThat(actualCreateTables) + .as("%s - expected create table calls", testName) + .isEqualTo(expectedCreateTable); + + //create table is always first + if ( expectedCreateTable > 0) { + var collectionComment = schemaChangeMemento.tableComments.getFirst(); + assertThat(collectionComment) + .as("%s - collection comment is not blank", testName) + .isNotBlank().as("Collection comment is not blank"); } - } - - private SchemaChangeMemento addSchemaChangeMomento(QueryExecutor queryExecutor) { - var memento = new SchemaChangeMemento(null, null); - when(queryExecutor.executeCreateSchemaChange(eq(requestContext), any())) - .then( - invocation -> { - memento.counter.incrementAndGet(); - SimpleStatement statement = invocation.getArgument(1); - var matcher = COMMENT_PATTERN.matcher(statement.getQuery()); - memento.cqlComments.add(matcher.find() ? matcher.group(1) : null); - return Uni.createFrom().item(mockSuccessSchemaResultset()); - }); - return memento; - } + int actualCreateIndexes = schemaChangeMemento.queries.stream() + .filter(query -> query.startsWith("CREATE CUSTOM INDEX")) + .toList() + .size(); + assertThat(actualCreateIndexes) + .as("%s - expected create index calls", testName) + .isEqualTo(expectedCreateIndex); + + // if the call failed, then we re-throw + if (awaitFailure) { + var throwable = subscriber.getFailure(); + LOGGER.info("assertOperation() - testName={}, throwable={}", testName, throwable.toString()); + + assertThat(throwable) + .as("%s - expected failure is APIException", testName) + .isInstanceOf(APIException.class); + throw (APIException) throwable; + } - private void addKeyspaceSchema(QueryExecutor queryExecutor) { + // no failure, so check the command result is success + var commandResult = subscriber.getItem().get(); + LOGGER.info("assertOperation() - testName={}, commandResult={}", testName, commandResult); + assertThatCommandResult(commandResult) + .as(testName) + .isDDLSuccess() + .hasOnlyStatus(CommandStatus.OK, 1); - var driverMetadata = mock(Metadata.class); - when(queryExecutor.getDriverMetadata(any())).thenReturn(Uni.createFrom().item(driverMetadata)); - - var allKeyspaces = new HashMap(); - var keyspaceMetadata = - new DefaultKeyspaceMetadata( - TEST_CONSTANTS.KEYSPACE_IDENTIFIER.keyspace(), - false, - false, - new HashMap<>(), - new HashMap<>(), - new HashMap<>(), - new HashMap<>(), - new HashMap<>(), - new HashMap<>()); - allKeyspaces.put(keyspaceMetadata.getName(), keyspaceMetadata); - when(driverMetadata.getKeyspaces()).thenReturn(allKeyspaces); + return schemaChangeMemento; } - private JsonNode collectionNodeFromTableComment(String testName, String tableComment) { - - LOGGER.info("tableCommentToNode() - testName: {}, tableComment: {}", testName, tableComment); - try { - var root = objectMapper.readTree(tableComment); - // we always want the "collection" node, see example at the top - // let the null out, it will cause the calling test to fail loud - return root.get(TableCommentConstants.TOP_LEVEL_KEY); - } catch (JacksonException e) { - throw new RuntimeException( - "Invalid JSON in Table comment for Collection, problem: " + e.getMessage()); - } - } - - @BeforeEach - public void init() {} @Test - public void createCollectionNoVector() { + public void successWithLexicalNoVector() { - var queryExecutor = mock(QueryExecutor.class); - var schemaChangeMemento = addSchemaChangeMomento(queryExecutor); - addKeyspaceSchema(queryExecutor); - - // aaron - 19-nov-2025 - best I can tell the sessionCache is not used but we need to pass it - // :( var operation = new CreateCollectionOperation( KEYSPACE_CONTEXT, @@ -191,19 +203,10 @@ public void createCollectionNoVector() { CollectionLexicalDefSchemaFactory.FOR_TESTING_ENABLED.currentVersion(null), CollectionRerankDefSchemaFactory.FOR_TESTING_ENABLED.currentVersion(null)); - operation - .execute(requestContext, queryExecutor) - .subscribe() - .withSubscriber(UniAssertSubscriber.create()) - .awaitItem(); - // 1 create Table + 8 super shredder indexes + lexical index - assertThat(schemaChangeMemento.counter.get()).isEqualTo(10); + var schemaChangeMemento = assertOperation("successWithLexicalNoVector", operation, 1,9); - var collectionComment = schemaChangeMemento.cqlComments.getFirst(); - assertThat(collectionComment).isNotBlank().as("Collection comment is not blank"); - - var commentNode = collectionNodeFromTableComment("createCollectionNoVector", collectionComment); + var commentNode = collectionNodeFromTableComment("successWithLexicalNoVector", schemaChangeMemento); var optionsNode = commentNode.get(TableCommentConstants.OPTIONS_KEY); assertThat(optionsNode.get(TableCommentConstants.COLLECTION_VECTOR_KEY)) .as("Collection comment must not have a vector key") @@ -211,14 +214,7 @@ public void createCollectionNoVector() { } @Test - public void createCollectionVector() { - - var queryExecutor = mock(QueryExecutor.class); - var schemaChangeMemento = addSchemaChangeMomento(queryExecutor); - addKeyspaceSchema(queryExecutor); - - // aaron - 19-nov-2025 - best I can tell the sessionCache is not used but we need to pass it - // :( + public void successWithLexicalWithVector() { var vectorDesc = new CreateCollectionCommand.Options.VectorSearchDesc(5, "cosine", null, null); // Must use validateVectorOptions() because it will cleanup defaults, the resolver normally does @@ -238,26 +234,19 @@ public void createCollectionVector() { CollectionLexicalDefSchemaFactory.FOR_TESTING_ENABLED.currentVersion(null), CollectionRerankDefSchemaFactory.FOR_TESTING_ENABLED.currentVersion(null)); - operation - .execute(requestContext, queryExecutor) - .subscribe() - .withSubscriber(UniAssertSubscriber.create()) - .awaitItem(); - // 1 create Table + 8 super shredder indexes + 1 vector index + 1 lexical - assertThat(schemaChangeMemento.counter.get()).isEqualTo(11); + var schemaChangeMemento = assertOperation("successWithLexicalWithVector", operation, 1,10); - var collectionComment = schemaChangeMemento.cqlComments.getFirst(); - assertThat(collectionComment).isNotBlank().as("Collection comment is not blank"); - - var commentNode = collectionNodeFromTableComment("createCollectionVector", collectionComment); + var commentNode = collectionNodeFromTableComment("successWithLexicalWithVector", schemaChangeMemento); var optionsNode = commentNode.get(TableCommentConstants.OPTIONS_KEY); + var vectorNode = optionsNode.get(TableCommentConstants.COLLECTION_VECTOR_KEY); - assertThat(vectorNode).as("Collection comment must have a vector key").isNotNull(); + assertThat(vectorNode) + .as("Collection comment must have a vector key") + .isNotNull(); // see CollectionSettingsV1Reader var vectorColumnDefinition = VectorColumnDefinition.fromJson(vectorNode, objectMapper); - var vectorConfig = VectorConfig.fromColumnDefinitions(List.of(vectorColumnDefinition)); assertThat(vectorColumnDefinition.vectorSize()) .as("Vector size from table comment matches") @@ -274,13 +263,7 @@ public void createCollectionVector() { } @Test - public void denyAllCollectionNoVector() { - var queryExecutor = mock(QueryExecutor.class); - var schemaChangeMemento = addSchemaChangeMomento(queryExecutor); - addKeyspaceSchema(queryExecutor); - - // aaron - 19-nov-2025 - best I can tell the sessionCache is not used but we need to pass it - // :( + public void successIndexingDenyAllWithLexical() { var indexingDesc = new CreateCollectionCommand.Options.IndexingDesc(null, List.of("*")); var operation = @@ -296,21 +279,14 @@ public void denyAllCollectionNoVector() { CollectionLexicalDefSchemaFactory.FOR_TESTING_ENABLED.currentVersion(null), CollectionRerankDefSchemaFactory.FOR_TESTING_ENABLED.currentVersion(null)); - operation - .execute(requestContext, queryExecutor) - .subscribe() - .withSubscriber(UniAssertSubscriber.create()) - .awaitItem(); - // 1 create Table + 1 lexical index - assertThat(schemaChangeMemento.counter.get()).isEqualTo(2); - - var collectionComment = schemaChangeMemento.cqlComments.getFirst(); - assertThat(collectionComment).isNotBlank().as("Collection comment is not blank"); + // NOTE: because of deny all we do not need any super shredding, but we do need the lexcial still + // for the $lexcial field + var schemaChangeMemento = assertOperation("successIndexingDenyAllWithLexical", operation, 1,1); // see CollectionSettingsV1Reader var commentNode = - collectionNodeFromTableComment("denyAllCollectionNoVector", collectionComment); + collectionNodeFromTableComment("successIndexingDenyAllWithLexical", schemaChangeMemento); var optionsNode = commentNode.get(TableCommentConstants.OPTIONS_KEY); var indexingNode = optionsNode.get(TableCommentConstants.COLLECTION_INDEXING_KEY); @@ -327,11 +303,7 @@ public void denyAllCollectionNoVector() { } @Test - public void denyAllCollectionVector() { - - var queryExecutor = mock(QueryExecutor.class); - var schemaChangeMemento = addSchemaChangeMomento(queryExecutor); - addKeyspaceSchema(queryExecutor); + public void successIndexingDenyAllWithLexicalWithVector() { var vectorDesc = new CreateCollectionCommand.Options.VectorSearchDesc(5, "cosine", null, null); // Must use validateVectorOptions() because it will cleanup defaults, the resolver normally does @@ -352,28 +324,54 @@ public void denyAllCollectionVector() { CollectionLexicalDefSchemaFactory.FOR_TESTING_ENABLED.currentVersion(null), CollectionRerankDefSchemaFactory.FOR_TESTING_ENABLED.currentVersion(null)); - operation - .execute(requestContext, queryExecutor) - .subscribe() - .withSubscriber(UniAssertSubscriber.create()) - .awaitItem(); - // 1 create Table + 1 vector index + 1 lexical - assertThat(schemaChangeMemento.counter.get()).isEqualTo(3); + // NOTE: because of deny all we do not need any super shredding, but we do need the lexcial still + // for the $lexcial field + var schemaChangeMemento = assertOperation("successIndexingDenyAllWithLexicalWithVector", operation, 1,2); // NOTE: no need to test the table comment again, that is covered above } @Test - public void indexAlreadyDropTable() { + public void failMissingKeyspace(){ + + var operation = + new CreateCollectionOperation( + KEYSPACE_CONTEXT, + databaseLimitsConfig, + TEST_CONSTANTS.COLLECTION_IDENTIFIER.table(), + 10, + false, + null, + null, + null, + CollectionLexicalDefSchemaFactory.FOR_TESTING_ENABLED.currentVersion(null), + CollectionRerankDefSchemaFactory.FOR_TESTING_ENABLED.currentVersion(null)); + + // this should throw, not return commandresult + var exception = catchThrowable(() -> assertOperation("failMissingKeyspace", operation, 0,0, false, true)); + + assertThatSchemaException(exception) + .as("failMissingKeyspace()") + .hasCode(SchemaException.Code.UNKNOWN_KEYSPACE) + .hasMessageSnippets("The keyspace used by the command: %s.".formatted(KEYSPACE_CONTEXT.schemaObject().identifier().keyspace())); + } + + /** + * Test: create table works, but there is an index with the same name, the operation should then try + * to drop the table. + */ + @Test + public void failExistingIndexDropTable() { var queryExecutor = mock(QueryExecutor.class); - var successResultSet = mockSuccessSchemaResultset(); - addKeyspaceSchema(queryExecutor); + var successResultSet = mockSchemaSuccessResultSet(); + addMockKeyspaceMetadata(queryExecutor); - final AtomicInteger schemaChangeCounter = new AtomicInteger(); - final AtomicInteger schemaDropCounter = new AtomicInteger(); + var schemaChangeCounter = new AtomicInteger(); + var schemaDropCounter = new AtomicInteger(); + // count the first create table when(queryExecutor.executeCreateSchemaChange( eq(requestContext), argThat( @@ -385,6 +383,7 @@ public void indexAlreadyDropTable() { return Uni.createFrom().item(successResultSet); }); + // mock existing index from first create index when(queryExecutor.executeCreateSchemaChange( eq(requestContext), argThat( @@ -395,6 +394,7 @@ public void indexAlreadyDropTable() { throw new InvalidQueryException(mock(Node.class), "Index xxxxx already exists"); }); + // count the drop table when(queryExecutor.executeDropSchemaChange( eq(requestContext), argThat( @@ -405,8 +405,6 @@ public void indexAlreadyDropTable() { return Uni.createFrom().item(successResultSet); }); - // aaron - 19-nov-2025 - best I can tell the sessionCache is not used but we need to pass it - // :( var operation = new CreateCollectionOperation( KEYSPACE_CONTEXT, @@ -425,25 +423,94 @@ public void indexAlreadyDropTable() { .subscribe() .withSubscriber(UniAssertSubscriber.create()) .awaitItem(); + // 1 create Table + 1 index failure assertThat(schemaChangeCounter.get()).isEqualTo(2); // 1 drop table assertThat(schemaDropCounter.get()).isEqualTo(1); } - private List createCorrectPartitionColumn() { - List tuple = - Arrays.asList( - new PrimitiveType(ProtocolConstants.DataType.TINYINT), - new PrimitiveType(ProtocolConstants.DataType.VARCHAR)); - List partitionKey = new ArrayList<>(); - partitionKey.add( - new DefaultColumnMetadata( - CqlIdentifier.fromInternal("keyspace"), - CqlIdentifier.fromInternal("collection"), - CqlIdentifier.fromInternal("key"), - new DefaultTupleType(tuple), - false)); - return partitionKey; + private void addMockKeyspaceMetadata(QueryExecutor queryExecutor) { + addMockKeyspaceMetadata(queryExecutor, true); + } + + /** + * Attaches mock KeyspaceMetadata to the queryExecutor so create tests can find the keyspace + */ + private void addMockKeyspaceMetadata(QueryExecutor queryExecutor, boolean addKeyspaceMetadata) { + + var allKeyspaces = new HashMap(); + if (addKeyspaceMetadata) { + var keyspaceMetadata = + new DefaultKeyspaceMetadata( + TEST_CONSTANTS.KEYSPACE_IDENTIFIER.keyspace(), + false, + false, + new HashMap<>(), + new HashMap<>(), + new HashMap<>(), + new HashMap<>(), + new HashMap<>(), + new HashMap<>()); + allKeyspaces.put(keyspaceMetadata.getName(), keyspaceMetadata); + } + + var driverMetadata = mock(Metadata.class); + when(driverMetadata.getKeyspaces()) + .thenReturn(allKeyspaces); + + when(queryExecutor.getDriverMetadata(any())) + .thenReturn(Uni.createFrom().item(driverMetadata)); + } + + private AsyncResultSet mockSchemaSuccessResultSet() { + + var schemaColumns = buildColumnDefs(OperationTestBase.TestColumn.ofBoolean("[applied]")); + List resultRows = List.of(new MockRow(schemaColumns, 0, List.of(byteBufferFrom(true)))); + return new MockAsyncResultSet(schemaColumns, resultRows, null); + } + + private record SchemaChangeMemento(AtomicInteger counter, List queries, List tableComments) { + SchemaChangeMemento { + counter = counter == null ? new AtomicInteger() : counter; + queries = queries == null ? new ArrayList<>() : queries; + tableComments = tableComments == null ? new ArrayList<>() : tableComments; + } + } + + private SchemaChangeMemento addSchemaChangeMomento(QueryExecutor queryExecutor) { + var memento = new SchemaChangeMemento(null, null,null); + + when(queryExecutor.executeCreateSchemaChange(eq(requestContext), any())) + .then( + invocation -> { + memento.counter.incrementAndGet(); + SimpleStatement statement = invocation.getArgument(1); + memento.queries.add(statement.getQuery()); + var matcher = TABLE_COMMENT_PATTERN.matcher(statement.getQuery()); + memento.tableComments.add(matcher.find() ? matcher.group(1) : null); + return Uni.createFrom().item(mockSchemaSuccessResultSet()); + }); + return memento; + } + + private JsonNode collectionNodeFromTableComment(String testName, SchemaChangeMemento memento) { + // create table should be first + return collectionNodeFromTableComment(testName, memento.tableComments.getFirst()); } + + private JsonNode collectionNodeFromTableComment(String testName, String tableComment) { + + LOGGER.info("tableCommentToNode() - testName: {}, tableComment: {}", testName, tableComment); + try { + var root = objectMapper.readTree(tableComment); + // we always want the "collection" node, see example at the top + // let the null out, it will cause the calling test to fail loud + return root.get(TableCommentConstants.TOP_LEVEL_KEY); + } catch (JacksonException e) { + throw new RuntimeException( + "Invalid JSON in Table comment for Collection, problem: " + e.getMessage()); + } + } + } diff --git a/src/test/java/io/stargate/sgv2/jsonapi/service/resolver/CreateCollectionCommandResolverTest.java b/src/test/java/io/stargate/sgv2/jsonapi/service/resolver/CreateCollectionCommandResolverTest.java index 651e0ac469..dff89816f8 100644 --- a/src/test/java/io/stargate/sgv2/jsonapi/service/resolver/CreateCollectionCommandResolverTest.java +++ b/src/test/java/io/stargate/sgv2/jsonapi/service/resolver/CreateCollectionCommandResolverTest.java @@ -1,7 +1,7 @@ package io.stargate.sgv2.jsonapi.service.resolver; import static io.stargate.sgv2.jsonapi.util.CqlIdentifierUtil.cqlIdentifierFromUserInput; -import static io.stargate.sgv2.jsonapi.util.exception.SchemaExceptionAssert.assertThatSchemaException; +import static io.stargate.sgv2.jsonapi.util.asserts.DataAPIAsserts.assertThatSchemaException; import static org.assertj.core.api.Assertions.*; import static org.assertj.core.api.Assertions.catchThrowable; diff --git a/src/test/java/io/stargate/sgv2/jsonapi/util/exception/APIExceptionAssert.java b/src/test/java/io/stargate/sgv2/jsonapi/util/asserts/APIExceptionAssert.java similarity index 85% rename from src/test/java/io/stargate/sgv2/jsonapi/util/exception/APIExceptionAssert.java rename to src/test/java/io/stargate/sgv2/jsonapi/util/asserts/APIExceptionAssert.java index fa3bed815f..38a3451eaa 100644 --- a/src/test/java/io/stargate/sgv2/jsonapi/util/exception/APIExceptionAssert.java +++ b/src/test/java/io/stargate/sgv2/jsonapi/util/asserts/APIExceptionAssert.java @@ -1,4 +1,4 @@ -package io.stargate.sgv2.jsonapi.util.exception; +package io.stargate.sgv2.jsonapi.util.asserts; import static org.assertj.core.api.AssertionsForClassTypes.assertThat; @@ -19,7 +19,9 @@ protected APIExceptionAssert(ERROR actual, Class selfClazz) { SELF assertThatAPIException( Function assertCtor, Class errorClass, Throwable throwable) { - assertThat(throwable).isInstanceOf(errorClass); + assertThat(throwable) + .as("Throwable is instance of " + errorClass.getSimpleName() + "") + .isInstanceOf(errorClass); return assertCtor.apply(errorClass.cast(throwable)); } diff --git a/src/test/java/io/stargate/sgv2/jsonapi/util/asserts/CommandResultAssert.java b/src/test/java/io/stargate/sgv2/jsonapi/util/asserts/CommandResultAssert.java new file mode 100644 index 0000000000..e125ee35b4 --- /dev/null +++ b/src/test/java/io/stargate/sgv2/jsonapi/util/asserts/CommandResultAssert.java @@ -0,0 +1,127 @@ +package io.stargate.sgv2.jsonapi.util.asserts; + +import io.stargate.sgv2.jsonapi.api.model.command.CommandResult; +import io.stargate.sgv2.jsonapi.api.model.command.CommandStatus; +import io.stargate.sgv2.jsonapi.api.v1.ResponseAssertions; +import io.stargate.sgv2.jsonapi.exception.ErrorCode; +import io.stargate.sgv2.jsonapi.service.shredding.DocRowIdentifer; +import org.assertj.core.api.AbstractAssert; + +import java.util.List; + +import static org.assertj.core.api.Assertions.assertThat; + +/** + * Assertions for matching the {@link CommandResult} + *

+ * NOTE: there is some logic duplication with {@link ResponseAssertions} which is used for + * integration tests, it would be good to have some underlying logic but not a big problem + *

+ */ +public class CommandResultAssert extends AbstractAssert { + + private CommandResultAssert(CommandResult commandResult) { + super(commandResult, CommandResultAssert.class); + } + + protected static CommandResultAssert assertThatCommandResult(CommandResult commandResult) { + + // must always be non null + assertThat(commandResult) + .as("CommandResult is not null") + .isNotNull(); + return new CommandResultAssert(commandResult); + } + + public CommandResultAssert isDDLSuccess(){ + + assertThat(actual.data()) + .as("isDDLSuccess() - data is null") + .isNull(); + assertThat(actual.status()) + .as("isError() - status is no empty") + .isNotEmpty(); + assertThat(actual.errors()) + .as("isDDLSuccess() - errors is empty") + .isEmpty(); + return this; + } + + public CommandResultAssert isError(){ + + assertThat(actual.data()) + .as("isError() - data is null") + .isNull(); + assertThat(actual.status()) + .as("isError() - status is empty") + .isEmpty(); + assertThat(actual.errors()) + .as("isError() - errors is not null") + .isNotNull(); + assertThat(actual.errors()) + .as("isError() - errors is not empty") + .isNotEmpty(); + return this; + } + + public CommandResultAssert hasOnlyStatus(CommandStatus key, Object value) { + assertThat(actual.status()) + .as("status is not null") + .isNotNull(); + assertThat(actual.status()) + .as("status is only one") + .hasSize(1); + assertThat(actual.status().get(key)) + .as("status['%s']", key) + .isEqualTo(value); + + return this; + } + + public CommandResultAssert hasNoStatus() { + assertThat(actual.status()) + .as("status is empty") + .isEmpty(); + return this; + } + + public CommandResultAssert hasNoErrors() { + assertThat(actual.errors()) + .as("errors is empty") + .isEmpty(); + return this; + } + + public CommandResultAssert hasErrorCount(int count) { + assertThat(actual.errors()) + .as("errors is not null") + .isNotNull(); + assertThat(actual.errors()) + .as("errors size") + .hasSize(count); + return this; + } + + public CommandResultAssert hasOnlyError(ErrorCode errorCode, String... snippets) { + + assertThat(actual.errors()) + .as("errors is not null") + .isNotNull(); + assertThat(actual.errors()) + .as("errors size") + .hasSize(1); + + var commandError = actual.errors().getFirst(); + assertThat(commandError.errorCode()) + .as("errorCode") + .isEqualTo(errorCode.name()); + + for (var snippet : snippets) { + assertThat(commandError.message()) + .as("message") + .contains(snippet); + } + return this; + } + +} diff --git a/src/test/java/io/stargate/sgv2/jsonapi/util/asserts/DataAPIAsserts.java b/src/test/java/io/stargate/sgv2/jsonapi/util/asserts/DataAPIAsserts.java new file mode 100644 index 0000000000..211354bb51 --- /dev/null +++ b/src/test/java/io/stargate/sgv2/jsonapi/util/asserts/DataAPIAsserts.java @@ -0,0 +1,20 @@ +package io.stargate.sgv2.jsonapi.util.asserts; + +import io.stargate.sgv2.jsonapi.api.model.command.CommandResult; +import io.stargate.sgv2.jsonapi.exception.SchemaException; + +public class DataAPIAsserts { + + private DataAPIAsserts() {} + + public static CommandResultAssert assertThatCommandResult(CommandResult commandResult) { + return CommandResultAssert.assertThatCommandResult(commandResult); + } + public static SchemaExceptionAssert assertThatSchemaException(Throwable throwable) { + return SchemaExceptionAssert.assertThatSchemaException(throwable); + } + + public static SchemaExceptionAssert assertThatSchemaException(SchemaException schemaException) { + return SchemaExceptionAssert.assertThatSchemaException(schemaException); + } +} diff --git a/src/test/java/io/stargate/sgv2/jsonapi/util/exception/SchemaExceptionAssert.java b/src/test/java/io/stargate/sgv2/jsonapi/util/asserts/SchemaExceptionAssert.java similarity index 67% rename from src/test/java/io/stargate/sgv2/jsonapi/util/exception/SchemaExceptionAssert.java rename to src/test/java/io/stargate/sgv2/jsonapi/util/asserts/SchemaExceptionAssert.java index c29b2dd4b3..b567adea4f 100644 --- a/src/test/java/io/stargate/sgv2/jsonapi/util/exception/SchemaExceptionAssert.java +++ b/src/test/java/io/stargate/sgv2/jsonapi/util/asserts/SchemaExceptionAssert.java @@ -1,4 +1,4 @@ -package io.stargate.sgv2.jsonapi.util.exception; +package io.stargate.sgv2.jsonapi.util.asserts; import io.stargate.sgv2.jsonapi.exception.SchemaException; @@ -9,12 +9,12 @@ private SchemaExceptionAssert(SchemaException actual) { super(actual, SchemaExceptionAssert.class); } - public static SchemaExceptionAssert assertThatSchemaException(SchemaException schemaException) { + protected static SchemaExceptionAssert assertThatSchemaException(SchemaException schemaException) { return assertThatAPIException( SchemaExceptionAssert::new, SchemaException.class, schemaException); } - public static SchemaExceptionAssert assertThatSchemaException(Throwable throwable) { + protected static SchemaExceptionAssert assertThatSchemaException(Throwable throwable) { return assertThatAPIException(SchemaExceptionAssert::new, SchemaException.class, throwable); } } From 087f2f20d11e1072d4195318cf5a889c98a26ff6 Mon Sep 17 00:00:00 2001 From: Aaron Morton Date: Wed, 17 Jun 2026 14:37:43 +1200 Subject: [PATCH 3/8] code tidy --- .../CreateCollectionOperation.java | 1290 ++++++++--------- .../model/command/impl/IndexingDescTest.java | 195 +-- .../CreateCollectionOperationTest.java | 220 +-- .../CreateCollectionCommandResolverTest.java | 47 +- .../util/asserts/APIExceptionAssert.java | 4 +- .../util/asserts/CommandResultAssert.java | 161 +- .../jsonapi/util/asserts/DataAPIAsserts.java | 21 +- .../util/asserts/SchemaExceptionAssert.java | 3 +- 8 files changed, 960 insertions(+), 981 deletions(-) diff --git a/src/main/java/io/stargate/sgv2/jsonapi/service/operation/collections/CreateCollectionOperation.java b/src/main/java/io/stargate/sgv2/jsonapi/service/operation/collections/CreateCollectionOperation.java index ee9fbd7a62..51147f5059 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/service/operation/collections/CreateCollectionOperation.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/service/operation/collections/CreateCollectionOperation.java @@ -42,691 +42,687 @@ import io.stargate.sgv2.jsonapi.service.schema.collections.CollectionSchemaObject; import io.stargate.sgv2.jsonapi.service.schema.collections.CollectionTableMatcher; import io.stargate.sgv2.jsonapi.service.schema.tables.CQLSAIIndex; - import java.time.Duration; import java.util.*; import java.util.function.Supplier; import java.util.stream.Collectors; - import org.slf4j.Logger; import org.slf4j.LoggerFactory; public class CreateCollectionOperation implements Operation { - private static final Logger LOGGER = LoggerFactory.getLogger(CreateCollectionOperation.class); - - private static final CollectionTableMatcher COLLECTION_MATCHER = - new CollectionTableMatcher(); - - private static final ObjectMapper OBJECT_MAPPER = new ObjectMapper(); - - private final CommandContext commandContext; - private final DatabaseLimitsConfig dbLimitsConfig; - private final CqlIdentifier collectionName; - private final int ddlDelayMillis; - private final boolean tooManyIndexesRollbackEnabled; - // nullable - private final CreateCollectionCommand.Options.DocIdDesc docIdDesc; - // nullable - private final CreateCollectionCommand.Options.IndexingDesc indexingDesc; - // nullable - private final CreateCollectionCommand.Options.VectorSearchDesc vectorDesc; - private final SchemaHolder lexicalDef; - private final SchemaHolder rerankDef; - - public CreateCollectionOperation( - CommandContext commandContext, - DatabaseLimitsConfig dbLimitsConfig, - CqlIdentifier collectionName, - int ddlDelayMillis, - boolean tooManyIndexesRollbackEnabled, - // nullable - CreateCollectionCommand.Options.DocIdDesc docIdDesc, - // nullable - CreateCollectionCommand.Options.IndexingDesc indexingDesc, - // nullable - CreateCollectionCommand.Options.VectorSearchDesc vectorDesc, - SchemaHolder lexicalDef, - SchemaHolder rerankDef) { - this.commandContext = commandContext; - this.dbLimitsConfig = dbLimitsConfig; - this.collectionName = collectionName; - this.ddlDelayMillis = ddlDelayMillis; - this.tooManyIndexesRollbackEnabled = tooManyIndexesRollbackEnabled; - this.docIdDesc = docIdDesc; - this.indexingDesc = indexingDesc; - this.vectorDesc = vectorDesc; - this.lexicalDef = lexicalDef; - this.rerankDef = rerankDef; - } - - @VisibleForTesting - public CqlIdentifier collectionName(){ - return collectionName; - } - - @VisibleForTesting - public CommandContext commandContext() { - return commandContext; - } - - @VisibleForTesting - public CreateCollectionCommand.Options.DocIdDesc docIdDesc() { - return docIdDesc; + private static final Logger LOGGER = LoggerFactory.getLogger(CreateCollectionOperation.class); + + private static final CollectionTableMatcher COLLECTION_MATCHER = new CollectionTableMatcher(); + + private static final ObjectMapper OBJECT_MAPPER = new ObjectMapper(); + + private final CommandContext commandContext; + private final DatabaseLimitsConfig dbLimitsConfig; + private final CqlIdentifier collectionName; + private final int ddlDelayMillis; + private final boolean tooManyIndexesRollbackEnabled; + // nullable + private final CreateCollectionCommand.Options.DocIdDesc docIdDesc; + // nullable + private final CreateCollectionCommand.Options.IndexingDesc indexingDesc; + // nullable + private final CreateCollectionCommand.Options.VectorSearchDesc vectorDesc; + private final SchemaHolder lexicalDef; + private final SchemaHolder rerankDef; + + public CreateCollectionOperation( + CommandContext commandContext, + DatabaseLimitsConfig dbLimitsConfig, + CqlIdentifier collectionName, + int ddlDelayMillis, + boolean tooManyIndexesRollbackEnabled, + // nullable + CreateCollectionCommand.Options.DocIdDesc docIdDesc, + // nullable + CreateCollectionCommand.Options.IndexingDesc indexingDesc, + // nullable + CreateCollectionCommand.Options.VectorSearchDesc vectorDesc, + SchemaHolder lexicalDef, + SchemaHolder rerankDef) { + this.commandContext = commandContext; + this.dbLimitsConfig = dbLimitsConfig; + this.collectionName = collectionName; + this.ddlDelayMillis = ddlDelayMillis; + this.tooManyIndexesRollbackEnabled = tooManyIndexesRollbackEnabled; + this.docIdDesc = docIdDesc; + this.indexingDesc = indexingDesc; + this.vectorDesc = vectorDesc; + this.lexicalDef = lexicalDef; + this.rerankDef = rerankDef; + } + + @VisibleForTesting + public CqlIdentifier collectionName() { + return collectionName; + } + + @VisibleForTesting + public CommandContext commandContext() { + return commandContext; + } + + @VisibleForTesting + public CreateCollectionCommand.Options.DocIdDesc docIdDesc() { + return docIdDesc; + } + + @VisibleForTesting + public CreateCollectionCommand.Options.IndexingDesc indexingDesc() { + return indexingDesc; + } + + @VisibleForTesting + public CreateCollectionCommand.Options.VectorSearchDesc vectorDesc() { + return vectorDesc; + } + + @VisibleForTesting + public SchemaHolder lexicalDef() { + return lexicalDef; + } + + @VisibleForTesting + public SchemaHolder rerankDef() { + return rerankDef; + } + + @Override + public Uni> execute( + RequestContext requestContext, QueryExecutor queryExecutor) { + + var initialTableComment = generateTableComment(); + LOGGER.info( + "execute()- createCollection for identifier= {}.{}, initialTableComment={}", + commandContext.schemaObject().identifier().keyspace(), + collectionName, + initialTableComment); + + return queryExecutor + .getDriverMetadata(requestContext) + .map(Metadata::getKeyspaces) + .flatMap( + allKeyspaces -> { + + // Step 1 - does the keyspace exist ? + var targetKeyspace = + allKeyspaces.get(commandContext.schemaObject().identifier().keyspace()); + if (targetKeyspace == null) { + return Uni.createFrom() + .failure( + SchemaException.Code.UNKNOWN_KEYSPACE.get( + errVars(commandContext.schemaObject()))); + } + + // Step 2 - is there an existing table and if not is there enough free indexes ? + var existingTableMetadata = + findTableAndValidateLimits(allKeyspaces, targetKeyspace, collectionName); + + // Step 3 - create the collection if no existing table + // use the runningValue() of lexicalDef this will either be the value from user or + // default + if (existingTableMetadata == null) { + return executeCollectionCreation( + requestContext, + queryExecutor, + initialTableComment, + lexicalDef.runningValue(), + false); + } + + // Step 4- Existing collection, check if the schema from the user is the same as the + // existing + // we need to merge in the current schema if the user did not specify anything + var existingCollectionSettings = + CollectionSchemaObject.getCollectionSettings( + requestContext, existingTableMetadata, OBJECT_MAPPER); + if (LOGGER.isDebugEnabled()) { + LOGGER.debug( + "execute() - existingCollectionSettings: {}", existingCollectionSettings); + } + + // Need to resolve the vector settings so we can use them to create a full + // representation + // of what the new collection will look like + var vectorModelName = + getOrDefault( + vectorDesc, + CreateCollectionCommand.Options.VectorSearchDesc::sourceModel, + null); + var embeddingSourceModel = + EmbeddingSourceModel.fromApiNameOrDefault(vectorModelName) + .orElseThrow( + () -> + EmbeddingSourceModel.getUnknownSourceModelException(vectorModelName)); + + var similarityFunctionName = + getOrDefault( + vectorDesc, CreateCollectionCommand.Options.VectorSearchDesc::metric, null); + var similarityFunction = + SimilarityFunction.fromApiNameOrDefault(similarityFunctionName) + .orElseThrow( + () -> + SimilarityFunction.getUnknownFunctionException( + similarityFunctionName)); + + // OK, we know there is an existing collection, and it is different from the one we + // already have. + // So we will replace the lexical and rerank in the new one with the existing if the + // user did not specify new values. + // NOTE: FROM NOW ON WE NEED TO USE THE OVERRIDEN VALUE, (which may or may not be + // actually overidden) + var overrideLexicalDef = + lexicalDef + .replaceIfMissing(existingCollectionSettings.lexicalDefSchemaValue()) + .value(); + var overrideRerankDef = + rerankDef + .replaceIfMissing(existingCollectionSettings.rerankDefSchemaValue()) + .value(); + + var overrideTableComment = + generateTableComment(overrideLexicalDef, overrideRerankDef); + + if (LOGGER.isDebugEnabled()) { + LOGGER.debug("execute() - overrideTableComment: {}", overrideTableComment); + } + + var newCollectionSettings = + CollectionSchemaObject.createCollectionSettings( + requestContext, + existingTableMetadata, + vectorDesc != null, + getOrDefault( + vectorDesc, + CreateCollectionCommand.Options.VectorSearchDesc::dimension, + 0), + similarityFunction, + embeddingSourceModel, + overrideTableComment, + OBJECT_MAPPER); + + boolean settingsAreEqual = existingCollectionSettings.equals(newCollectionSettings); + if (LOGGER.isDebugEnabled()) { + LOGGER.debug( + "execute() - settingsAreEqual: {}, newCollectionSettings={}", + settingsAreEqual, + newCollectionSettings); + } + + if (settingsAreEqual) { + return executeCollectionCreation( + requestContext, + queryExecutor, + overrideTableComment, + overrideLexicalDef.runningValue(), + true); + } + return Uni.createFrom() + .failure( + SchemaException.Code.EXISTING_COLLECTION_DIFFERENT_SETTINGS.get( + Map.of("collectionName", cqlIdentifierToMessageString(collectionName)))); + }); + } + + @VisibleForTesting + String generateTableComment() { + return generateTableComment(lexicalDef, rerankDef); + } + + @VisibleForTesting + String generateTableComment( + SchemaHolder overrideLexicalDef, + SchemaHolder overrideRerankDef) { + + var optionsNode = OBJECT_MAPPER.createObjectNode(); + + if (indexingDesc != null) { + optionsNode.putPOJO(TableCommentConstants.COLLECTION_INDEXING_KEY, indexingDesc); } - @VisibleForTesting - public CreateCollectionCommand.Options.IndexingDesc indexingDesc() { - return indexingDesc; - } - @VisibleForTesting - public CreateCollectionCommand.Options.VectorSearchDesc vectorDesc() { - return vectorDesc; - } - @VisibleForTesting - public SchemaHolder lexicalDef() { - return lexicalDef; - } - @VisibleForTesting - public SchemaHolder rerankDef() { - return rerankDef; - } - - - @Override - public Uni> execute( - RequestContext requestContext, QueryExecutor queryExecutor) { - - var initialTableComment = generateTableComment(); - LOGGER.info( - "execute()- createCollection for identifier= {}.{}, initialTableComment={}", - commandContext.schemaObject().identifier().keyspace(), - collectionName, - initialTableComment); - - return queryExecutor - .getDriverMetadata(requestContext) - .map(Metadata::getKeyspaces) - .flatMap( - allKeyspaces -> { - - // Step 1 - does the keyspace exist ? - var targetKeyspace = - allKeyspaces.get(commandContext.schemaObject().identifier().keyspace()); - if (targetKeyspace == null) { - return Uni.createFrom() - .failure( - SchemaException.Code.UNKNOWN_KEYSPACE.get( - errVars(commandContext.schemaObject()))); - } - - // Step 2 - is there an existing table and if not is there enough free indexes ? - var existingTableMetadata = - findTableAndValidateLimits(allKeyspaces, targetKeyspace, collectionName); - - // Step 3 - create the collection if no existing table - // use the runningValue() of lexicalDef this will either be the value from user or - // default - if (existingTableMetadata == null) { - return executeCollectionCreation( - requestContext, - queryExecutor, - initialTableComment, - lexicalDef.runningValue(), - false); - } - - // Step 4- Existing collection, check if the schema from the user is the same as the - // existing - // we need to merge in the current schema if the user did not specify anything - var existingCollectionSettings = - CollectionSchemaObject.getCollectionSettings( - requestContext, existingTableMetadata, OBJECT_MAPPER); - if (LOGGER.isDebugEnabled()) { - LOGGER.debug( - "execute() - existingCollectionSettings: {}", existingCollectionSettings); - } - - // Need to resolve the vector settings so we can use them to create a full - // representation - // of what the new collection will look like - var vectorModelName = - getOrDefault( - vectorDesc, - CreateCollectionCommand.Options.VectorSearchDesc::sourceModel, - null); - var embeddingSourceModel = - EmbeddingSourceModel.fromApiNameOrDefault(vectorModelName) - .orElseThrow( - () -> - EmbeddingSourceModel.getUnknownSourceModelException(vectorModelName)); - - var similarityFunctionName = - getOrDefault( - vectorDesc, CreateCollectionCommand.Options.VectorSearchDesc::metric, null); - var similarityFunction = - SimilarityFunction.fromApiNameOrDefault(similarityFunctionName) - .orElseThrow( - () -> - SimilarityFunction.getUnknownFunctionException( - similarityFunctionName)); - - // OK, we know there is an existing collection, and it is different from the one we - // already have. - // So we will replace the lexical and rerank in the new one with the existing if the - // user did not specify new values. - // NOTE: FROM NOW ON WE NEED TO USE THE OVERRIDEN VALUE, (which may or may not be - // actually overidden) - var overrideLexicalDef = - lexicalDef - .replaceIfMissing(existingCollectionSettings.lexicalDefSchemaValue()) - .value(); - var overrideRerankDef = - rerankDef - .replaceIfMissing(existingCollectionSettings.rerankDefSchemaValue()) - .value(); - - var overrideTableComment = - generateTableComment(overrideLexicalDef, overrideRerankDef); - - if (LOGGER.isDebugEnabled()) { - LOGGER.debug("execute() - overrideTableComment: {}", overrideTableComment); - } - - var newCollectionSettings = - CollectionSchemaObject.createCollectionSettings( - requestContext, - existingTableMetadata, - vectorDesc != null, - getOrDefault( - vectorDesc, - CreateCollectionCommand.Options.VectorSearchDesc::dimension, - 0), - similarityFunction, - embeddingSourceModel, - overrideTableComment, - OBJECT_MAPPER); - - boolean settingsAreEqual = existingCollectionSettings.equals(newCollectionSettings); - if (LOGGER.isDebugEnabled()) { - LOGGER.debug( - "execute() - settingsAreEqual: {}, newCollectionSettings={}", - settingsAreEqual, - newCollectionSettings); - } - - if (settingsAreEqual) { - return executeCollectionCreation( - requestContext, - queryExecutor, - overrideTableComment, - overrideLexicalDef.runningValue(), - true); - } - return Uni.createFrom() - .failure( - SchemaException.Code.EXISTING_COLLECTION_DIFFERENT_SETTINGS.get( - Map.of("collectionName", cqlIdentifierToMessageString(collectionName)))); - }); - } - - @VisibleForTesting - String generateTableComment() { - return generateTableComment(lexicalDef, rerankDef); + if (vectorDesc != null) { + optionsNode.putPOJO(TableCommentConstants.COLLECTION_VECTOR_KEY, vectorDesc); } - - @VisibleForTesting - String generateTableComment( - SchemaHolder overrideLexicalDef, - SchemaHolder overrideRerankDef) { - - var optionsNode = OBJECT_MAPPER.createObjectNode(); - - if (indexingDesc != null) { - optionsNode.putPOJO(TableCommentConstants.COLLECTION_INDEXING_KEY, indexingDesc); - } - if (vectorDesc != null) { - optionsNode.putPOJO(TableCommentConstants.COLLECTION_VECTOR_KEY, vectorDesc); - } - // if default_id is not specified during createCollection, resolve type to empty string - if (docIdDesc != null) { - optionsNode.putPOJO(TableCommentConstants.DEFAULT_ID_KEY, docIdDesc); - } else { - optionsNode.putPOJO( - TableCommentConstants.DEFAULT_ID_KEY, - OBJECT_MAPPER.createObjectNode().putPOJO("type", "")); - } - - // Take the running value, this will either be what the user gave us or the appropriate default - optionsNode.putPOJO( - TableCommentConstants.COLLECTION_LEXICAL_CONFIG_KEY, overrideLexicalDef.runningValue()); - optionsNode.putPOJO( - TableCommentConstants.COLLECTION_RERANKING_CONFIG_KEY, overrideRerankDef.runningValue()); - - var collectionNode = OBJECT_MAPPER.createObjectNode(); - collectionNode.put( - TableCommentConstants.COLLECTION_NAME_KEY, cqlIdentifierToJsonKey(collectionName)); - // Use ordinalValue() to get the integer representation of the enum into the JSON - collectionNode.put( - TableCommentConstants.SCHEMA_VERSION_KEY, - CollectionSchemaVersion.CURRENT_VERSION.ordinalValue()); - collectionNode.putPOJO(TableCommentConstants.OPTIONS_KEY, optionsNode); - - var tableCommentNode = OBJECT_MAPPER.createObjectNode(); - tableCommentNode.putPOJO(TableCommentConstants.TOP_LEVEL_KEY, collectionNode); - - return tableCommentNode.toString(); + // if default_id is not specified during createCollection, resolve type to empty string + if (docIdDesc != null) { + optionsNode.putPOJO(TableCommentConstants.DEFAULT_ID_KEY, docIdDesc); + } else { + optionsNode.putPOJO( + TableCommentConstants.DEFAULT_ID_KEY, + OBJECT_MAPPER.createObjectNode().putPOJO("type", "")); } - /** - * execute collection creation and indexes creation - * - * @param requestContext DBRequestContext - * @param queryExecutor QueryExecutor instance - * @param collectionLexicalDef Lexical configuration for the collection - * @param collectionExisted boolean that says if collection existed before - * @return Uni> - */ - private Uni> executeCollectionCreation( - RequestContext requestContext, - QueryExecutor queryExecutor, - String tableComment, - CollectionLexicalDef collectionLexicalDef, - boolean collectionExisted) { - - final Uni execCreateTable = - queryExecutor.executeCreateSchemaChange( - requestContext, getCreateTable(tableComment, collectionLexicalDef)); - - final Uni indexResult = - execCreateTable - .onItem() - .delayIt() - .by(Duration.ofMillis(ddlDelayMillis > 0 ? ddlDelayMillis : 100)) - .onItem() - .transformToUni( - res -> { - if (res.wasApplied()) { - final List indexStatements = - getIndexStatements(collectionLexicalDef, collectionExisted); - Multi indexResultMulti; + // Take the running value, this will either be what the user gave us or the appropriate default + optionsNode.putPOJO( + TableCommentConstants.COLLECTION_LEXICAL_CONFIG_KEY, overrideLexicalDef.runningValue()); + optionsNode.putPOJO( + TableCommentConstants.COLLECTION_RERANKING_CONFIG_KEY, overrideRerankDef.runningValue()); + + var collectionNode = OBJECT_MAPPER.createObjectNode(); + collectionNode.put( + TableCommentConstants.COLLECTION_NAME_KEY, cqlIdentifierToJsonKey(collectionName)); + // Use ordinalValue() to get the integer representation of the enum into the JSON + collectionNode.put( + TableCommentConstants.SCHEMA_VERSION_KEY, + CollectionSchemaVersion.CURRENT_VERSION.ordinalValue()); + collectionNode.putPOJO(TableCommentConstants.OPTIONS_KEY, optionsNode); + + var tableCommentNode = OBJECT_MAPPER.createObjectNode(); + tableCommentNode.putPOJO(TableCommentConstants.TOP_LEVEL_KEY, collectionNode); + + return tableCommentNode.toString(); + } + + /** + * execute collection creation and indexes creation + * + * @param requestContext DBRequestContext + * @param queryExecutor QueryExecutor instance + * @param collectionLexicalDef Lexical configuration for the collection + * @param collectionExisted boolean that says if collection existed before + * @return Uni> + */ + private Uni> executeCollectionCreation( + RequestContext requestContext, + QueryExecutor queryExecutor, + String tableComment, + CollectionLexicalDef collectionLexicalDef, + boolean collectionExisted) { + + final Uni execCreateTable = + queryExecutor.executeCreateSchemaChange( + requestContext, getCreateTable(tableComment, collectionLexicalDef)); + + final Uni indexResult = + execCreateTable + .onItem() + .delayIt() + .by(Duration.ofMillis(ddlDelayMillis > 0 ? ddlDelayMillis : 100)) + .onItem() + .transformToUni( + res -> { + if (res.wasApplied()) { + final List indexStatements = + getIndexStatements(collectionLexicalDef, collectionExisted); + Multi indexResultMulti; /* CI will override ddlDelayMillis to 0 using `-Dstargate.jsonapi.operations.database-config.ddl-delay-millis=0` to speed up the test execution This is ok because CI is run as single cassandra instance and there is no need to wait for the schema changes to propagate */ - if (ddlDelayMillis == 0) { - indexResultMulti = - createIndexParallel(queryExecutor, requestContext, indexStatements); - } else { - indexResultMulti = - createIndexOrdered(queryExecutor, requestContext, indexStatements); - } - - return indexResultMulti - .collect() - .asList() - .onItem() - .transform( - results -> { - final Optional first = - results.stream() - .filter(indexRes -> !(indexRes.wasApplied())) - .findFirst(); - return !first.isPresent(); - }); - } else { - return Uni.createFrom().item(false); - } - }); - - return indexResult - .onItem() - .transform( - res -> { - if (!res) { - // amorton - 13 jan 2026 - this is bad, the old code would swallow the error for - // creating the table and indexes, will need to improve later. - - // table creation failure or index creation failure - // HACK - remove when re-writing this class - return commandResultSupplier( - DatabaseException.Code.CORRUPTED_COLLECTION_SCHEMA.get( - errVars( - commandContext.schemaObject(), - map -> - map.put( - "errorMessage", - "Collection creation failure (unable to create table)")))); - } else { - return new SchemaChangeResult(true); - } - }) - .onFailure( - error -> - // InvalidQueryException(DB index limit violation) - error instanceof InvalidQueryException - && (error - .getMessage() - .matches( - ".*Cannot have more than \\d+ indexes, failed to create index on table.*") - || error.getMessage().matches("Index .* already exists"))) - .recoverWithUni( - // this block only handles the case where the index creation fails because of index - // limit or already exists during create collection - error -> { - // if index creation fails and collection not existed before and rollback is enabled, - // then drop the collection - if (!collectionExisted && tooManyIndexesRollbackEnabled) { - return cleanUpCollectionFailedWithTooManyIndex(requestContext, queryExecutor); - } - - if (error.getMessage().matches("Index .* already exists")) { - // if index creation fails because index already exists - return Uni.createFrom() - .item( - () -> - commandResultSupplier( - SchemaException.Code.EXISTING_INDEX_FOR_COLLECTION.get( - errVars(commandContext.schemaObject())))); - } else { - // if index creation violates DB index limit and collection existed before, - // will not drop the collection - return Uni.createFrom() - .item( - () -> - commandResultSupplier( - SchemaException.Code.TOO_MANY_INDEXES_FOR_COLLECTION.get( - errVars( - commandContext.schemaObject(), - map -> - map.put( - "indexesPerCollection", - String.valueOf( - dbLimitsConfig - .indexesNeededPerCollection())))))); - } - }); + if (ddlDelayMillis == 0) { + indexResultMulti = + createIndexParallel(queryExecutor, requestContext, indexStatements); + } else { + indexResultMulti = + createIndexOrdered(queryExecutor, requestContext, indexStatements); + } + + return indexResultMulti + .collect() + .asList() + .onItem() + .transform( + results -> { + final Optional first = + results.stream() + .filter(indexRes -> !(indexRes.wasApplied())) + .findFirst(); + return !first.isPresent(); + }); + } else { + return Uni.createFrom().item(false); + } + }); + + return indexResult + .onItem() + .transform( + res -> { + if (!res) { + // amorton - 13 jan 2026 - this is bad, the old code would swallow the error for + // creating the table and indexes, will need to improve later. + + // table creation failure or index creation failure + // HACK - remove when re-writing this class + return commandResultSupplier( + DatabaseException.Code.CORRUPTED_COLLECTION_SCHEMA.get( + errVars( + commandContext.schemaObject(), + map -> + map.put( + "errorMessage", + "Collection creation failure (unable to create table)")))); + } else { + return new SchemaChangeResult(true); + } + }) + .onFailure( + error -> + // InvalidQueryException(DB index limit violation) + error instanceof InvalidQueryException + && (error + .getMessage() + .matches( + ".*Cannot have more than \\d+ indexes, failed to create index on table.*") + || error.getMessage().matches("Index .* already exists"))) + .recoverWithUni( + // this block only handles the case where the index creation fails because of index + // limit or already exists during create collection + error -> { + // if index creation fails and collection not existed before and rollback is enabled, + // then drop the collection + if (!collectionExisted && tooManyIndexesRollbackEnabled) { + return cleanUpCollectionFailedWithTooManyIndex(requestContext, queryExecutor); + } + + if (error.getMessage().matches("Index .* already exists")) { + // if index creation fails because index already exists + return Uni.createFrom() + .item( + () -> + commandResultSupplier( + SchemaException.Code.EXISTING_INDEX_FOR_COLLECTION.get( + errVars(commandContext.schemaObject())))); + } else { + // if index creation violates DB index limit and collection existed before, + // will not drop the collection + return Uni.createFrom() + .item( + () -> + commandResultSupplier( + SchemaException.Code.TOO_MANY_INDEXES_FOR_COLLECTION.get( + errVars( + commandContext.schemaObject(), + map -> + map.put( + "indexesPerCollection", + String.valueOf( + dbLimitsConfig + .indexesNeededPerCollection())))))); + } + }); + } + + private Supplier commandResultSupplier(Throwable throwable) { + return () -> + CommandResult.statusOnlyBuilder(RequestTracing.NO_OP).addThrowable(throwable).build(); + } + + /** Create indexes for collections in ordered. This is to avoid schema change conflicts. */ + private Multi createIndexOrdered( + QueryExecutor queryExecutor, + RequestContext requestContext, + List indexStatements) { + + return Multi.createFrom() + .items(indexStatements.stream()) + .onItem() + .transformToUni( + indexStatement -> + queryExecutor.executeCreateSchemaChange(requestContext, indexStatement)) + .concatenate(); + } + + /** Create indexes for collections in parallel. Only used to speed up the CI actions. */ + private Multi createIndexParallel( + QueryExecutor queryExecutor, + RequestContext requestContext, + List indexStatements) { + + return Multi.createFrom() + .items(indexStatements.stream()) + .onItem() + .transformToUni( + indexStatement -> + queryExecutor.executeCreateSchemaChange(requestContext, indexStatement)) + .merge(); + } + + public Uni> cleanUpCollectionFailedWithTooManyIndex( + RequestContext requestContext, QueryExecutor queryExecutor) { + + // turning the name into asInternal() because DeleteCollectionCollectionOperation stil uses + // string + DeleteCollectionCollectionOperation deleteCollectionCollectionOperation = + new DeleteCollectionCollectionOperation(commandContext, collectionName.asInternal()); + + // amorton - 13 jan 2026 - keeping the existing logic here, where the error was returning in + // two situations + // unsure how the second happens + var exception = + SchemaException.Code.TOO_MANY_INDEXES_FOR_COLLECTION.get( + errVars( + commandContext.schemaObject(), + map -> + map.put( + "indexesPerCollection", + String.valueOf(dbLimitsConfig.indexesNeededPerCollection())))); + return deleteCollectionCollectionOperation + .execute(requestContext, queryExecutor) + .onItem() + .transform( + res -> + (Supplier) + () -> + CommandResult.statusOnlyBuilder(RequestTracing.NO_OP) + .addThrowable(exception) + .build()) + .onFailure() + .recoverWithItem( + e -> + // This is unlikely to happen for delete collection though + // Also return with TOO_MANY_INDEXES exception + () -> + CommandResult.statusOnlyBuilder(RequestTracing.NO_OP) + .addThrowable(exception) + .build()); + } + + /** + * Method for finding existing collection with given name: 1. if valid one exists and returning + * that table; 2. if invalid one exists, error out; 3. if no table exists with given name, verify + * maximum table limit and return null; + * + * @return Existing valid collection with given name, if any; {@code null} if not + */ + TableMetadata findTableAndValidateLimits( + Map allKeyspaces, + KeyspaceMetadata currKeyspace, + CqlIdentifier tableName) { + + // First: do we already have a Table with the same name? + for (TableMetadata table : currKeyspace.getTables().values()) { + if (table.getName().equals(tableName)) { + // If that is not a valid Data API collection, error out the createCollectionCommand + if (!COLLECTION_MATCHER.test(table)) { + throw SchemaException.Code.EXISTING_TABLE_NOT_DATA_API_COLLECTION.get( + Map.of("tableName", cqlIdentifierToMessageString(tableName))); + } + // If that is a valid Data API table, we returned it + return table; + } } - private Supplier commandResultSupplier(Throwable throwable) { - return () -> - CommandResult.statusOnlyBuilder(RequestTracing.NO_OP).addThrowable(throwable).build(); + // Otherwise we need to check if we can create a new Collection based on limits; + // limits are calculated across the whole Database, so all Keyspaces need to be checked. + final List allTables = + allKeyspaces.values().stream() + .map(keyspace -> keyspace.getTables().values()) + .flatMap(Collection::stream) + .toList(); + final long collectionCount = allTables.stream().filter(COLLECTION_MATCHER).count(); + final int MAX_COLLECTIONS = dbLimitsConfig.maxCollections(); + if (collectionCount >= MAX_COLLECTIONS) { + throw SchemaException.Code.TOO_MANY_COLLECTIONS.get( + Map.of( + "table", + cqlIdentifierToMessageString(tableName), + "collectionCount", + String.valueOf(collectionCount), + "collectionMaxCount", + String.valueOf(MAX_COLLECTIONS))); } - /** - * Create indexes for collections in ordered. This is to avoid schema change conflicts. - */ - private Multi createIndexOrdered( - QueryExecutor queryExecutor, - RequestContext requestContext, - List indexStatements) { - - return Multi.createFrom() - .items(indexStatements.stream()) - .onItem() - .transformToUni( - indexStatement -> - queryExecutor.executeCreateSchemaChange(requestContext, indexStatement)) - .concatenate(); + // And then see how many Indexes have been created, how many available + int saisUsed = allTables.stream().mapToInt(table -> table.getIndexes().size()).sum(); + if ((saisUsed + dbLimitsConfig.indexesNeededPerCollection()) + > dbLimitsConfig.indexesAvailablePerDatabase()) { + throw SchemaException.Code.TOO_MANY_INDEXES_FOR_COLLECTION.get( + errVars( + commandContext.schemaObject(), + map -> + map.put( + "indexesPerCollection", + String.valueOf(dbLimitsConfig.indexesNeededPerCollection())))); } - /** - * Create indexes for collections in parallel. Only used to speed up the CI actions. - */ - private Multi createIndexParallel( - QueryExecutor queryExecutor, - RequestContext requestContext, - List indexStatements) { - - return Multi.createFrom() - .items(indexStatements.stream()) - .onItem() - .transformToUni( - indexStatement -> - queryExecutor.executeCreateSchemaChange(requestContext, indexStatement)) - .merge(); + return null; + } + + private SimpleStatement getCreateTable(String comment, CollectionLexicalDef overrideLexicalDef) { + + var keyspace = commandContext.schemaObject().identifier().keyspace(); + + CreateTable create = + SchemaBuilder.createTable(keyspace, collectionName) + .ifNotExists() + .withPartitionKey("key", DataTypes.tupleOf(DataTypes.TINYINT, DataTypes.TEXT)) + .withColumn("tx_id", DataTypes.TIMEUUID) + .withColumn("doc_json", DataTypes.TEXT) + .withColumn("exist_keys", DataTypes.setOf(DataTypes.TEXT)) + .withColumn("array_size", DataTypes.mapOf(DataTypes.TEXT, DataTypes.INT)) + .withColumn("array_contains", DataTypes.setOf(DataTypes.TEXT)) + .withColumn("query_bool_values", DataTypes.mapOf(DataTypes.TEXT, DataTypes.TINYINT)) + .withColumn("query_dbl_values", DataTypes.mapOf(DataTypes.TEXT, DataTypes.DECIMAL)) + .withColumn("query_text_values", DataTypes.mapOf(DataTypes.TEXT, DataTypes.TEXT)) + .withColumn( + "query_timestamp_values", DataTypes.mapOf(DataTypes.TEXT, DataTypes.TIMESTAMP)) + .withColumn("query_null_values", DataTypes.setOf(DataTypes.TEXT)); + + if (vectorDesc != null) { + create = + create.withColumn( + "query_vector_value", + new ExtendedVectorType(DataTypes.FLOAT, vectorDesc.dimension())); } - - public Uni> cleanUpCollectionFailedWithTooManyIndex( - RequestContext requestContext, QueryExecutor queryExecutor) { - - // turning the name into asInternal() because DeleteCollectionCollectionOperation stil uses - // string - DeleteCollectionCollectionOperation deleteCollectionCollectionOperation = - new DeleteCollectionCollectionOperation(commandContext, collectionName.asInternal()); - - // amorton - 13 jan 2026 - keeping the existing logic here, where the error was returning in - // two situations - // unsure how the second happens - var exception = - SchemaException.Code.TOO_MANY_INDEXES_FOR_COLLECTION.get( - errVars( - commandContext.schemaObject(), - map -> - map.put( - "indexesPerCollection", - String.valueOf(dbLimitsConfig.indexesNeededPerCollection())))); - return deleteCollectionCollectionOperation - .execute(requestContext, queryExecutor) - .onItem() - .transform( - res -> - (Supplier) - () -> - CommandResult.statusOnlyBuilder(RequestTracing.NO_OP) - .addThrowable(exception) - .build()) - .onFailure() - .recoverWithItem( - e -> - // This is unlikely to happen for delete collection though - // Also return with TOO_MANY_INDEXES exception - () -> - CommandResult.statusOnlyBuilder(RequestTracing.NO_OP) - .addThrowable(exception) - .build()); + if (overrideLexicalDef.enabled()) { + create = create.withColumn("query_lexical_value", DataTypes.TEXT); } - /** - * Method for finding existing collection with given name: 1. if valid one exists and returning - * that table; 2. if invalid one exists, error out; 3. if no table exists with given name, verify - * maximum table limit and return null; - * - * @return Existing valid collection with given name, if any; {@code null} if not - */ - TableMetadata findTableAndValidateLimits( - Map allKeyspaces, - KeyspaceMetadata currKeyspace, - CqlIdentifier tableName) { - - // First: do we already have a Table with the same name? - for (TableMetadata table : currKeyspace.getTables().values()) { - if (table.getName().equals(tableName)) { - // If that is not a valid Data API collection, error out the createCollectionCommand - if (!COLLECTION_MATCHER.test(table)) { - throw SchemaException.Code.EXISTING_TABLE_NOT_DATA_API_COLLECTION.get( - Map.of("tableName", cqlIdentifierToMessageString(tableName))); - } - // If that is a valid Data API table, we returned it - return table; - } - } - - // Otherwise we need to check if we can create a new Collection based on limits; - // limits are calculated across the whole Database, so all Keyspaces need to be checked. - final List allTables = - allKeyspaces.values().stream() - .map(keyspace -> keyspace.getTables().values()) - .flatMap(Collection::stream) - .toList(); - final long collectionCount = allTables.stream().filter(COLLECTION_MATCHER).count(); - final int MAX_COLLECTIONS = dbLimitsConfig.maxCollections(); - if (collectionCount >= MAX_COLLECTIONS) { - throw SchemaException.Code.TOO_MANY_COLLECTIONS.get( - Map.of( - "table", - cqlIdentifierToMessageString(tableName), - "collectionCount", - String.valueOf(collectionCount), - "collectionMaxCount", - String.valueOf(MAX_COLLECTIONS))); - } - - // And then see how many Indexes have been created, how many available - int saisUsed = allTables.stream().mapToInt(table -> table.getIndexes().size()).sum(); - if ((saisUsed + dbLimitsConfig.indexesNeededPerCollection()) - > dbLimitsConfig.indexesAvailablePerDatabase()) { - throw SchemaException.Code.TOO_MANY_INDEXES_FOR_COLLECTION.get( - errVars( - commandContext.schemaObject(), - map -> - map.put( - "indexesPerCollection", - String.valueOf(dbLimitsConfig.indexesNeededPerCollection())))); - } + // adding the comment changes the return into something to deal with options + var statement = comment == null ? create.build() : create.withComment(comment).build(); - return null; + if (LOGGER.isTraceEnabled()) { + LOGGER.trace("getCreateTable() - created table statement: {}", statement.getQuery()); } - - private SimpleStatement getCreateTable(String comment, CollectionLexicalDef overrideLexicalDef) { - - var keyspace = commandContext.schemaObject().identifier().keyspace(); - - CreateTable create = - SchemaBuilder.createTable(keyspace, collectionName) - .ifNotExists() - .withPartitionKey("key", DataTypes.tupleOf(DataTypes.TINYINT, DataTypes.TEXT)) - .withColumn("tx_id", DataTypes.TIMEUUID) - .withColumn("doc_json", DataTypes.TEXT) - .withColumn("exist_keys", DataTypes.setOf(DataTypes.TEXT)) - .withColumn("array_size", DataTypes.mapOf(DataTypes.TEXT, DataTypes.INT)) - .withColumn("array_contains", DataTypes.setOf(DataTypes.TEXT)) - .withColumn("query_bool_values", DataTypes.mapOf(DataTypes.TEXT, DataTypes.TINYINT)) - .withColumn("query_dbl_values", DataTypes.mapOf(DataTypes.TEXT, DataTypes.DECIMAL)) - .withColumn("query_text_values", DataTypes.mapOf(DataTypes.TEXT, DataTypes.TEXT)) - .withColumn( - "query_timestamp_values", DataTypes.mapOf(DataTypes.TEXT, DataTypes.TIMESTAMP)) - .withColumn("query_null_values", DataTypes.setOf(DataTypes.TEXT)); - - if (vectorDesc != null) { - create = - create.withColumn( - "query_vector_value", - new ExtendedVectorType(DataTypes.FLOAT, vectorDesc.dimension())); - } - if (overrideLexicalDef.enabled()) { - create = create.withColumn("query_lexical_value", DataTypes.TEXT); - } - - // adding the comment changes the return into something to deal with options - var statement = comment == null ? create.build() : create.withComment(comment).build(); - - if (LOGGER.isTraceEnabled()) { - LOGGER.trace("getCreateTable() - created table statement: {}", statement.getQuery()); - } - return statement; + return statement; + } + + /* + * When a createCollection is done on a table that already exist the index are run with IF NOT EXISTS. + * For a new table they are run without IF NOT EXISTS. + */ + private List getIndexStatements( + CollectionLexicalDef overrideLexicalDef, boolean collectionExisted) { + + List statements = new ArrayList<>(10); + + var denyAllIndexes = + getOrDefault(indexingDesc, CreateCollectionCommand.Options.IndexingDesc::denyAll, false); + + if (!denyAllIndexes) { + statements.add(saiColumn(collectionExisted, "exists_keys", "exist_keys")); + statements.add(saiEntries(collectionExisted, "array_size", "array_size")); + statements.add(saiColumn(collectionExisted, "array_contains", "array_contains")); + statements.add(saiEntries(collectionExisted, "query_bool_values", "query_bool_values")); + statements.add(saiEntries(collectionExisted, "query_dbl_values", "query_dbl_values")); + statements.add(saiEntries(collectionExisted, "query_text_values", "query_text_values")); + statements.add( + saiEntries(collectionExisted, "query_timestamp_values", "query_timestamp_values")); + statements.add(saiColumn(collectionExisted, "query_null_values", "query_null_values")); } - /* - * When a createCollection is done on a table that already exist the index are run with IF NOT EXISTS. - * For a new table they are run without IF NOT EXISTS. - */ - private List getIndexStatements( - CollectionLexicalDef overrideLexicalDef, boolean collectionExisted) { - - List statements = new ArrayList<>(10); - - var denyAllIndexes = - getOrDefault(indexingDesc, CreateCollectionCommand.Options.IndexingDesc::denyAll, false); - - if (!denyAllIndexes) { - statements.add(saiColumn(collectionExisted, "exists_keys", "exist_keys")); - statements.add(saiEntries(collectionExisted, "array_size", "array_size")); - statements.add(saiColumn(collectionExisted, "array_contains", "array_contains")); - statements.add(saiEntries(collectionExisted, "query_bool_values", "query_bool_values")); - statements.add(saiEntries(collectionExisted, "query_dbl_values", "query_dbl_values")); - statements.add(saiEntries(collectionExisted, "query_text_values", "query_text_values")); - statements.add( - saiEntries(collectionExisted, "query_timestamp_values", "query_timestamp_values")); - statements.add(saiColumn(collectionExisted, "query_null_values", "query_null_values")); - } - - // NOTE: This is a little sloppy, in normal request the CreateCollectionCommandResolver will - // make sure the vectorDesc is valid and has defaults set. So even though they are strings - // they have been validated as the thing we should use. See - // CreateCollectionCommandResolver.validateVectorOptions() - // it gets the proper CQL names from the Enums, replacing what the user sent in. (kind of - // confusing) - // TODO: create a VectorSearchDef that uses the SimilarityFunction and EmbeddingSourceModel - // enums - if (vectorDesc != null) { - // Sanity checking here, if we pass a null value the map go bang, try to stop bang, bang bad - Map vectorOptions = new HashMap<>(); - if (vectorDesc.metric() != null && !vectorDesc.metric().isBlank()) { - vectorOptions.put("similarity_function", vectorDesc.metric()); - } - if (vectorDesc.sourceModel() != null && !vectorDesc.sourceModel().isBlank()) { - vectorOptions.put("source_model", vectorDesc.sourceModel()); - } - statements.add( - buildSaiIndex( - collectionExisted, "query_vector_value", "query_vector_value", false, vectorOptions)); - } - - if (overrideLexicalDef.enabled()) { - var analyzerDef = overrideLexicalDef.analyzerDefinition(); - var analyzerString = analyzerDef.isTextual() ? analyzerDef.asText() : analyzerDef.toString(); - statements.add( - buildSaiIndex( - collectionExisted, - "query_lexical_value", - "query_lexical_value", - false, - Map.of("index_analyzer", analyzerString))); - } - - if (LOGGER.isTraceEnabled()) { - var cqlStrings = - statements.stream().map(SimpleStatement::getQuery).collect(Collectors.joining("; ")); - LOGGER.trace("getIndexStatements() - created index statements: {}", cqlStrings); - } - return statements; + // NOTE: This is a little sloppy, in normal request the CreateCollectionCommandResolver will + // make sure the vectorDesc is valid and has defaults set. So even though they are strings + // they have been validated as the thing we should use. See + // CreateCollectionCommandResolver.validateVectorOptions() + // it gets the proper CQL names from the Enums, replacing what the user sent in. (kind of + // confusing) + // TODO: create a VectorSearchDef that uses the SimilarityFunction and EmbeddingSourceModel + // enums + if (vectorDesc != null) { + // Sanity checking here, if we pass a null value the map go bang, try to stop bang, bang bad + Map vectorOptions = new HashMap<>(); + if (vectorDesc.metric() != null && !vectorDesc.metric().isBlank()) { + vectorOptions.put("similarity_function", vectorDesc.metric()); + } + if (vectorDesc.sourceModel() != null && !vectorDesc.sourceModel().isBlank()) { + vectorOptions.put("source_model", vectorDesc.sourceModel()); + } + statements.add( + buildSaiIndex( + collectionExisted, "query_vector_value", "query_vector_value", false, vectorOptions)); } - private SimpleStatement saiColumn(boolean ifNotExists, String indexSuffix, String column) { - return buildSaiIndex(ifNotExists, indexSuffix, column, false, Map.of()); + if (overrideLexicalDef.enabled()) { + var analyzerDef = overrideLexicalDef.analyzerDefinition(); + var analyzerString = analyzerDef.isTextual() ? analyzerDef.asText() : analyzerDef.toString(); + statements.add( + buildSaiIndex( + collectionExisted, + "query_lexical_value", + "query_lexical_value", + false, + Map.of("index_analyzer", analyzerString))); } - private SimpleStatement saiEntries(boolean ifNotExists, String indexSuffix, String column) { - return buildSaiIndex(ifNotExists, indexSuffix, column, true, Map.of()); + if (LOGGER.isTraceEnabled()) { + var cqlStrings = + statements.stream().map(SimpleStatement::getQuery).collect(Collectors.joining("; ")); + LOGGER.trace("getIndexStatements() - created index statements: {}", cqlStrings); + } + return statements; + } + + private SimpleStatement saiColumn(boolean ifNotExists, String indexSuffix, String column) { + return buildSaiIndex(ifNotExists, indexSuffix, column, false, Map.of()); + } + + private SimpleStatement saiEntries(boolean ifNotExists, String indexSuffix, String column) { + return buildSaiIndex(ifNotExists, indexSuffix, column, true, Map.of()); + } + + private SimpleStatement buildSaiIndex( + boolean ifNotExists, + String indexSuffix, + String columnName, // aaron - next change will make this a CQLIdentifier + boolean isEntries, + Map options) { + + var keyspace = commandContext.schemaObject().identifier().keyspace(); + var index = CqlIdentifier.fromInternal(collectionName.asInternal() + "_" + indexSuffix); + var column = CqlIdentifier.fromInternal(columnName); + + var start = SchemaBuilder.createIndex(index).custom(CQLSAIIndex.SAI_CLASS_NAME); + if (ifNotExists) { + start = start.ifNotExists(); } - private SimpleStatement buildSaiIndex( - boolean ifNotExists, - String indexSuffix, - String columnName, // aaron - next change will make this a CQLIdentifier - boolean isEntries, - Map options) { - - var keyspace = commandContext.schemaObject().identifier().keyspace(); - var index = CqlIdentifier.fromInternal(collectionName.asInternal() + "_" + indexSuffix); - var column = CqlIdentifier.fromInternal(columnName); - - var start = SchemaBuilder.createIndex(index).custom(CQLSAIIndex.SAI_CLASS_NAME); - if (ifNotExists) { - start = start.ifNotExists(); - } - - var onTable = start.onTable(keyspace, collectionName); - var createIndex = isEntries ? onTable.andColumnEntries(column) : onTable.andColumn(column); - - if (!options.isEmpty()) { - // in the CQL statement OPTIONS are the things after WITH, and for the `create index` there is - // an option called OPTIONS calling withSASIOptions deals with this. - createIndex = createIndex.withSASIOptions(options); - } + var onTable = start.onTable(keyspace, collectionName); + var createIndex = isEntries ? onTable.andColumnEntries(column) : onTable.andColumn(column); - return new ExtendedCreateIndex((DefaultCreateIndex) createIndex).build(); + if (!options.isEmpty()) { + // in the CQL statement OPTIONS are the things after WITH, and for the `create index` there is + // an option called OPTIONS calling withSASIOptions deals with this. + createIndex = createIndex.withSASIOptions(options); } + + return new ExtendedCreateIndex((DefaultCreateIndex) createIndex).build(); + } } diff --git a/src/test/java/io/stargate/sgv2/jsonapi/api/model/command/impl/IndexingDescTest.java b/src/test/java/io/stargate/sgv2/jsonapi/api/model/command/impl/IndexingDescTest.java index e326d22b7a..062496e7f2 100644 --- a/src/test/java/io/stargate/sgv2/jsonapi/api/model/command/impl/IndexingDescTest.java +++ b/src/test/java/io/stargate/sgv2/jsonapi/api/model/command/impl/IndexingDescTest.java @@ -18,18 +18,8 @@ public class IndexingDescTest { private final TestConstants TEST_CONSTANTS = new TestConstants(); - public CreateCollectionCommand.Options.IndexingDesc deserialize(String testName, String json) { - LOGGER.info("deserialize() - testName: {}, json: {}", testName, json); - - try { - return TEST_CONSTANTS.OBJECT_MAPPER.readValue( - json, CreateCollectionCommand.Options.IndexingDesc.class); - } catch (JsonProcessingException e) { - throw new RuntimeException(e); - } - } - - public void assertSchemaException( + /** Assert that validating the JSON for the IndexingDesc will result in a SchemaException */ + private void assertSchemaException( String testName, String json, ErrorCode errorCode, String... message) { var indexingDesc = deserialize(testName, json); @@ -46,14 +36,14 @@ public void failAllowAndDeny() { var json = """ - { - "deny": [ - "comment" - ], - "allow": [ - "data" - ] - }"""; + { + "deny": [ + "comment" + ], + "allow": [ + "data" + ] + }"""; assertSchemaException( "failAllowAndDeny()", @@ -67,7 +57,7 @@ public void failNeitherAllowNorDeny() { var json = """ - {}"""; + {}"""; assertSchemaException( "failNeitherAllowNorDeny()", @@ -81,12 +71,12 @@ public void failAllowDuplicates() { var json = """ - { - "allow": [ - "name", - "name" - ] - }"""; + { + "allow": [ + "name", + "name" + ] + }"""; assertSchemaException( "failAllowDuplicates()", @@ -100,12 +90,12 @@ public void failDenyDuplicates() { var json = """ - { - "deny": [ - "name", - "name" - ] - }"""; + { + "deny": [ + "name", + "name" + ] + }"""; assertSchemaException( "failDenyDuplicates()", @@ -119,11 +109,11 @@ public void failAllowEmptyPath() { var json = """ - { - "allow": [ - "" - ] - }"""; + { + "allow": [ + "" + ] + }"""; assertSchemaException( "failAllowEmptyPath()", @@ -137,11 +127,11 @@ public void failDenyEmptyPath() { var json = """ - { - "deny": [ - "" - ] - }"""; + { + "deny": [ + "" + ] + }"""; assertSchemaException( "failDenyEmptyPath()", @@ -155,11 +145,11 @@ public void failAllowDollarPrefix() { var json = """ - { - "allow": [ - "$score" - ] - }"""; + { + "allow": [ + "$score" + ] + }"""; assertSchemaException( "failAllowDollarPrefix()", @@ -173,11 +163,11 @@ public void failDenyDollarPrefix() { var json = """ - { - "deny": [ - "$score" - ] - }"""; + { + "deny": [ + "$score" + ] + }"""; assertSchemaException( "failDenyDollarPrefix()", @@ -191,11 +181,11 @@ public void failAllowInvalidPath() { var json = """ - { - "allow": [ - "bad&path" - ] - }"""; + { + "allow": [ + "bad&path" + ] + }"""; assertSchemaException( "failAllowInvalidPath()", @@ -209,11 +199,11 @@ public void failDenyInvalidPath() { var json = """ - { - "deny": [ - "bad&path" - ] - }"""; + { + "deny": [ + "bad&path" + ] + }"""; assertSchemaException( "failDenyInvalidPath()", @@ -227,11 +217,11 @@ public void successAllowStar() { var json = """ - { - "allow": [ - "*" - ] - }"""; + { + "allow": [ + "*" + ] + }"""; var indexingDesc = deserialize("successAllowStar()", json); indexingDesc.validate(); @@ -244,11 +234,11 @@ public void successDenyStar() { var json = """ - { - "deny": [ - "*" - ] - }"""; + { + "deny": [ + "*" + ] + }"""; var indexingDesc = deserialize("successDenyStar()", json); indexingDesc.validate(); @@ -262,11 +252,11 @@ public void successAllowVector() { var json = """ - { - "allow": [ - "$vector" - ] - }"""; + { + "allow": [ + "$vector" + ] + }"""; var indexingDesc = deserialize("successAllowVector()", json); indexingDesc.validate(); @@ -279,11 +269,11 @@ public void successDenyVector() { var json = """ - { - "deny": [ - "$vector" - ] - }"""; + { + "deny": [ + "$vector" + ] + }"""; var indexingDesc = deserialize("successDenyVector()", json); indexingDesc.validate(); @@ -296,13 +286,13 @@ public void successAllowValidPaths() { var json = """ - { - "allow": [ - "name", - "address.city", - "tags" - ] - }"""; + { + "allow": [ + "name", + "address.city", + "tags" + ] + }"""; var indexingDesc = deserialize("successAllowValidPaths()", json); indexingDesc.validate(); @@ -315,17 +305,28 @@ public void successDenyValidPaths() { var json = """ - { - "deny": [ - "name", - "address.city", - "tags" - ] - }"""; + { + "deny": [ + "name", + "address.city", + "tags" + ] + }"""; var indexingDesc = deserialize("successDenyValidPaths()", json); indexingDesc.validate(); assertThat(indexingDesc.deny()).containsExactly("name", "address.city", "tags"); assertThat(indexingDesc.allow()).isNull(); } + + private CreateCollectionCommand.Options.IndexingDesc deserialize(String testName, String json) { + LOGGER.info("deserialize() - testName: {}, json: {}", testName, json); + + try { + return TEST_CONSTANTS.OBJECT_MAPPER.readValue( + json, CreateCollectionCommand.Options.IndexingDesc.class); + } catch (JsonProcessingException e) { + throw new RuntimeException(e); + } + } } diff --git a/src/test/java/io/stargate/sgv2/jsonapi/service/operation/collections/CreateCollectionOperationTest.java b/src/test/java/io/stargate/sgv2/jsonapi/service/operation/collections/CreateCollectionOperationTest.java index c1e1ae3f74..77aaf9e102 100644 --- a/src/test/java/io/stargate/sgv2/jsonapi/service/operation/collections/CreateCollectionOperationTest.java +++ b/src/test/java/io/stargate/sgv2/jsonapi/service/operation/collections/CreateCollectionOperationTest.java @@ -47,7 +47,6 @@ import java.util.*; import java.util.concurrent.atomic.AtomicInteger; import java.util.regex.Pattern; - import org.junit.jupiter.api.Test; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -97,21 +96,27 @@ public class CreateCollectionOperationTest extends OperationTestBase { // Assume it is delineated by single quotes private static final Pattern TABLE_COMMENT_PATTERN = Pattern.compile("comment='(.*?)'"); - - private SchemaChangeMemento assertOperation(String testName, - CreateCollectionOperation operation, - int expectedCreateTable, - int expectedCreateIndex){ - return assertOperation(testName, operation, expectedCreateTable, expectedCreateIndex, true, false); + private SchemaChangeMemento assertOperation( + String testName, + CreateCollectionOperation operation, + int expectedCreateTable, + int expectedCreateIndex) { + return assertOperation( + testName, operation, expectedCreateTable, expectedCreateIndex, true, false); } - private SchemaChangeMemento assertOperation(String testName, - CreateCollectionOperation operation, - int expectedCreateTable, - int expectedCreateIndex, - boolean mockKeyspaceMetadata, - boolean awaitFailure) { - LOGGER.info("assertOperation() - testName={}, expectedCreateTable={}, expectedCreateIndex={}", testName, expectedCreateTable, expectedCreateIndex); + private SchemaChangeMemento assertOperation( + String testName, + CreateCollectionOperation operation, + int expectedCreateTable, + int expectedCreateIndex, + boolean mockKeyspaceMetadata, + boolean awaitFailure) { + LOGGER.info( + "assertOperation() - testName={}, expectedCreateTable={}, expectedCreateIndex={}", + testName, + expectedCreateTable, + expectedCreateIndex); // track calls to change the schema and mock the keyspace exists for the new collection var mockQueryExecutor = mock(QueryExecutor.class); @@ -120,49 +125,53 @@ private SchemaChangeMemento assertOperation(String testName, // IMPORTANT - we still need to confirm the DB calls even in an error condition // so catch, check db activity, then rethrow - var subscriber = awaitFailure ? - operation - .execute(requestContext, mockQueryExecutor) - .subscribe() - .withSubscriber(UniAssertSubscriber.create()) - .awaitFailure() - : - operation + var subscriber = + awaitFailure + ? operation + .execute(requestContext, mockQueryExecutor) + .subscribe() + .withSubscriber(UniAssertSubscriber.create()) + .awaitFailure() + : operation .execute(requestContext, mockQueryExecutor) - .subscribe() + .subscribe() .withSubscriber(UniAssertSubscriber.create()) - .awaitItem(); + .awaitItem(); - LOGGER.info("assertOperation() - testName={}, schemaChangeMemento={}", testName, schemaChangeMemento); + LOGGER.info( + "assertOperation() - testName={}, schemaChangeMemento={}", testName, schemaChangeMemento); // Validate the change schema calls assertThat(schemaChangeMemento.counter.get()) - .as("%s - total schema calls is expected") - .isEqualTo(expectedCreateTable + expectedCreateIndex); + .as("%s - total schema calls is expected") + .isEqualTo(expectedCreateTable + expectedCreateIndex); - int actualCreateTables = schemaChangeMemento.queries.stream() + int actualCreateTables = + schemaChangeMemento.queries.stream() .filter(query -> query.startsWith("CREATE TABLE")) .toList() .size(); assertThat(actualCreateTables) - .as("%s - expected create table calls", testName) - .isEqualTo(expectedCreateTable); + .as("%s - expected create table calls", testName) + .isEqualTo(expectedCreateTable); - //create table is always first - if ( expectedCreateTable > 0) { + // create table is always first + if (expectedCreateTable > 0) { var collectionComment = schemaChangeMemento.tableComments.getFirst(); assertThat(collectionComment) - .as("%s - collection comment is not blank", testName) - .isNotBlank().as("Collection comment is not blank"); + .as("%s - collection comment is not blank", testName) + .isNotBlank() + .as("Collection comment is not blank"); } - int actualCreateIndexes = schemaChangeMemento.queries.stream() + int actualCreateIndexes = + schemaChangeMemento.queries.stream() .filter(query -> query.startsWith("CREATE CUSTOM INDEX")) .toList() .size(); assertThat(actualCreateIndexes) - .as("%s - expected create index calls", testName) - .isEqualTo(expectedCreateIndex); + .as("%s - expected create index calls", testName) + .isEqualTo(expectedCreateIndex); // if the call failed, then we re-throw if (awaitFailure) { @@ -170,8 +179,8 @@ private SchemaChangeMemento assertOperation(String testName, LOGGER.info("assertOperation() - testName={}, throwable={}", testName, throwable.toString()); assertThat(throwable) - .as("%s - expected failure is APIException", testName) - .isInstanceOf(APIException.class); + .as("%s - expected failure is APIException", testName) + .isInstanceOf(APIException.class); throw (APIException) throwable; } @@ -179,14 +188,13 @@ private SchemaChangeMemento assertOperation(String testName, var commandResult = subscriber.getItem().get(); LOGGER.info("assertOperation() - testName={}, commandResult={}", testName, commandResult); assertThatCommandResult(commandResult) - .as(testName) - .isDDLSuccess() - .hasOnlyStatus(CommandStatus.OK, 1); + .as(testName) + .isDDLSuccess() + .hasOnlyStatus(CommandStatus.OK, 1); return schemaChangeMemento; } - @Test public void successWithLexicalNoVector() { @@ -204,9 +212,10 @@ public void successWithLexicalNoVector() { CollectionRerankDefSchemaFactory.FOR_TESTING_ENABLED.currentVersion(null)); // 1 create Table + 8 super shredder indexes + lexical index - var schemaChangeMemento = assertOperation("successWithLexicalNoVector", operation, 1,9); + var schemaChangeMemento = assertOperation("successWithLexicalNoVector", operation, 1, 9); - var commentNode = collectionNodeFromTableComment("successWithLexicalNoVector", schemaChangeMemento); + var commentNode = + collectionNodeFromTableComment("successWithLexicalNoVector", schemaChangeMemento); var optionsNode = commentNode.get(TableCommentConstants.OPTIONS_KEY); assertThat(optionsNode.get(TableCommentConstants.COLLECTION_VECTOR_KEY)) .as("Collection comment must not have a vector key") @@ -235,15 +244,14 @@ public void successWithLexicalWithVector() { CollectionRerankDefSchemaFactory.FOR_TESTING_ENABLED.currentVersion(null)); // 1 create Table + 8 super shredder indexes + 1 vector index + 1 lexical - var schemaChangeMemento = assertOperation("successWithLexicalWithVector", operation, 1,10); + var schemaChangeMemento = assertOperation("successWithLexicalWithVector", operation, 1, 10); - var commentNode = collectionNodeFromTableComment("successWithLexicalWithVector", schemaChangeMemento); + var commentNode = + collectionNodeFromTableComment("successWithLexicalWithVector", schemaChangeMemento); var optionsNode = commentNode.get(TableCommentConstants.OPTIONS_KEY); var vectorNode = optionsNode.get(TableCommentConstants.COLLECTION_VECTOR_KEY); - assertThat(vectorNode) - .as("Collection comment must have a vector key") - .isNotNull(); + assertThat(vectorNode).as("Collection comment must have a vector key").isNotNull(); // see CollectionSettingsV1Reader var vectorColumnDefinition = VectorColumnDefinition.fromJson(vectorNode, objectMapper); @@ -280,9 +288,10 @@ public void successIndexingDenyAllWithLexical() { CollectionRerankDefSchemaFactory.FOR_TESTING_ENABLED.currentVersion(null)); // 1 create Table + 1 lexical index - // NOTE: because of deny all we do not need any super shredding, but we do need the lexcial still + // NOTE: because of deny all we do not need any super shredding, but we do need the lexcial + // still // for the $lexcial field - var schemaChangeMemento = assertOperation("successIndexingDenyAllWithLexical", operation, 1,1); + var schemaChangeMemento = assertOperation("successIndexingDenyAllWithLexical", operation, 1, 1); // see CollectionSettingsV1Reader var commentNode = @@ -325,41 +334,46 @@ public void successIndexingDenyAllWithLexicalWithVector() { CollectionRerankDefSchemaFactory.FOR_TESTING_ENABLED.currentVersion(null)); // 1 create Table + 1 vector index + 1 lexical - // NOTE: because of deny all we do not need any super shredding, but we do need the lexcial still + // NOTE: because of deny all we do not need any super shredding, but we do need the lexcial + // still // for the $lexcial field - var schemaChangeMemento = assertOperation("successIndexingDenyAllWithLexicalWithVector", operation, 1,2); + var schemaChangeMemento = + assertOperation("successIndexingDenyAllWithLexicalWithVector", operation, 1, 2); // NOTE: no need to test the table comment again, that is covered above } @Test - public void failMissingKeyspace(){ + public void failMissingKeyspace() { var operation = - new CreateCollectionOperation( - KEYSPACE_CONTEXT, - databaseLimitsConfig, - TEST_CONSTANTS.COLLECTION_IDENTIFIER.table(), - 10, - false, - null, - null, - null, - CollectionLexicalDefSchemaFactory.FOR_TESTING_ENABLED.currentVersion(null), - CollectionRerankDefSchemaFactory.FOR_TESTING_ENABLED.currentVersion(null)); - - // this should throw, not return commandresult - var exception = catchThrowable(() -> assertOperation("failMissingKeyspace", operation, 0,0, false, true)); + new CreateCollectionOperation( + KEYSPACE_CONTEXT, + databaseLimitsConfig, + TEST_CONSTANTS.COLLECTION_IDENTIFIER.table(), + 10, + false, + null, + null, + null, + CollectionLexicalDefSchemaFactory.FOR_TESTING_ENABLED.currentVersion(null), + CollectionRerankDefSchemaFactory.FOR_TESTING_ENABLED.currentVersion(null)); + + // this should throw, not return Command + var exception = + catchThrowable(() -> assertOperation("failMissingKeyspace", operation, 0, 0, false, true)); assertThatSchemaException(exception) - .as("failMissingKeyspace()") - .hasCode(SchemaException.Code.UNKNOWN_KEYSPACE) - .hasMessageSnippets("The keyspace used by the command: %s.".formatted(KEYSPACE_CONTEXT.schemaObject().identifier().keyspace())); + .as("failMissingKeyspace()") + .hasCode(SchemaException.Code.UNKNOWN_KEYSPACE) + .hasMessageSnippets( + "The keyspace used by the command: %s." + .formatted(KEYSPACE_CONTEXT.schemaObject().identifier().keyspace())); } /** - * Test: create table works, but there is an index with the same name, the operation should then try - * to drop the table. + * Test: create table works, but there is an index with the same name, the operation should then + * try to drop the table. */ @Test public void failExistingIndexDropTable() { @@ -431,36 +445,32 @@ public void failExistingIndexDropTable() { } private void addMockKeyspaceMetadata(QueryExecutor queryExecutor) { - addMockKeyspaceMetadata(queryExecutor, true); + addMockKeyspaceMetadata(queryExecutor, true); } - /** - * Attaches mock KeyspaceMetadata to the queryExecutor so create tests can find the keyspace - */ + /** Attaches mock KeyspaceMetadata to the queryExecutor so create tests can find the keyspace */ private void addMockKeyspaceMetadata(QueryExecutor queryExecutor, boolean addKeyspaceMetadata) { var allKeyspaces = new HashMap(); if (addKeyspaceMetadata) { var keyspaceMetadata = - new DefaultKeyspaceMetadata( - TEST_CONSTANTS.KEYSPACE_IDENTIFIER.keyspace(), - false, - false, - new HashMap<>(), - new HashMap<>(), - new HashMap<>(), - new HashMap<>(), - new HashMap<>(), - new HashMap<>()); + new DefaultKeyspaceMetadata( + TEST_CONSTANTS.KEYSPACE_IDENTIFIER.keyspace(), + false, + false, + new HashMap<>(), + new HashMap<>(), + new HashMap<>(), + new HashMap<>(), + new HashMap<>(), + new HashMap<>()); allKeyspaces.put(keyspaceMetadata.getName(), keyspaceMetadata); } var driverMetadata = mock(Metadata.class); - when(driverMetadata.getKeyspaces()) - .thenReturn(allKeyspaces); + when(driverMetadata.getKeyspaces()).thenReturn(allKeyspaces); - when(queryExecutor.getDriverMetadata(any())) - .thenReturn(Uni.createFrom().item(driverMetadata)); + when(queryExecutor.getDriverMetadata(any())).thenReturn(Uni.createFrom().item(driverMetadata)); } private AsyncResultSet mockSchemaSuccessResultSet() { @@ -470,7 +480,8 @@ private AsyncResultSet mockSchemaSuccessResultSet() { return new MockAsyncResultSet(schemaColumns, resultRows, null); } - private record SchemaChangeMemento(AtomicInteger counter, List queries, List tableComments) { + private record SchemaChangeMemento( + AtomicInteger counter, List queries, List tableComments) { SchemaChangeMemento { counter = counter == null ? new AtomicInteger() : counter; queries = queries == null ? new ArrayList<>() : queries; @@ -479,18 +490,18 @@ private record SchemaChangeMemento(AtomicInteger counter, List queries, } private SchemaChangeMemento addSchemaChangeMomento(QueryExecutor queryExecutor) { - var memento = new SchemaChangeMemento(null, null,null); + var memento = new SchemaChangeMemento(null, null, null); when(queryExecutor.executeCreateSchemaChange(eq(requestContext), any())) - .then( - invocation -> { - memento.counter.incrementAndGet(); - SimpleStatement statement = invocation.getArgument(1); - memento.queries.add(statement.getQuery()); - var matcher = TABLE_COMMENT_PATTERN.matcher(statement.getQuery()); - memento.tableComments.add(matcher.find() ? matcher.group(1) : null); - return Uni.createFrom().item(mockSchemaSuccessResultSet()); - }); + .then( + invocation -> { + memento.counter.incrementAndGet(); + SimpleStatement statement = invocation.getArgument(1); + memento.queries.add(statement.getQuery()); + var matcher = TABLE_COMMENT_PATTERN.matcher(statement.getQuery()); + memento.tableComments.add(matcher.find() ? matcher.group(1) : null); + return Uni.createFrom().item(mockSchemaSuccessResultSet()); + }); return memento; } @@ -509,8 +520,7 @@ private JsonNode collectionNodeFromTableComment(String testName, String tableCom return root.get(TableCommentConstants.TOP_LEVEL_KEY); } catch (JacksonException e) { throw new RuntimeException( - "Invalid JSON in Table comment for Collection, problem: " + e.getMessage()); + "Invalid JSON in Table comment for Collection, problem: " + e.getMessage()); } } - } diff --git a/src/test/java/io/stargate/sgv2/jsonapi/service/resolver/CreateCollectionCommandResolverTest.java b/src/test/java/io/stargate/sgv2/jsonapi/service/resolver/CreateCollectionCommandResolverTest.java index dff89816f8..ead104eb3b 100644 --- a/src/test/java/io/stargate/sgv2/jsonapi/service/resolver/CreateCollectionCommandResolverTest.java +++ b/src/test/java/io/stargate/sgv2/jsonapi/service/resolver/CreateCollectionCommandResolverTest.java @@ -27,6 +27,10 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; +/** + * Tests how the {@link CreateCollectionCommandResolver} handles inputs and the operation it + * creates. + */ @QuarkusTest @TestProfile(EnabledVectorizeProfile.class) class CreateCollectionCommandResolverTest { @@ -34,23 +38,32 @@ class CreateCollectionCommandResolverTest { private static final Logger LOGGER = LoggerFactory.getLogger(CreateCollectionCommandResolverTest.class); - @Inject private CreateCollectionCommandResolver RESOLVER; + @Inject CreateCollectionCommandResolver RESOLVER; private final TestConstants TEST_CONSTANTS = new TestConstants(); // want a single instance for all calls, keyspaceContext() creates a new each call private final CommandContext COMMAND_CONTEXT = TEST_CONSTANTS.keyspaceContext(); - private Throwable resolveCommandThrows(String testName, String rawJson) { + private Throwable assertCommandThrows(String testName, String rawJson) { return catchThrowable( - () -> resolveCommand(testName, rawJson, TEST_CONSTANTS.COLLECTION_IDENTIFIER.table())); + () -> assertCommand(testName, rawJson, TEST_CONSTANTS.COLLECTION_IDENTIFIER.table())); } - private CreateCollectionOperation resolveCommand(String testName, String rawJson) { - return resolveCommand(testName, rawJson, TEST_CONSTANTS.COLLECTION_IDENTIFIER.table()); + private CreateCollectionOperation assertCommand(String testName, String rawJson) { + return assertCommand(testName, rawJson, TEST_CONSTANTS.COLLECTION_IDENTIFIER.table()); } - private CreateCollectionOperation resolveCommand( + /** + * Run the command through the resolver and do some basic tests on the operation that comes out. + * + * @param testName name for logging etc + * @param rawJson the raw command JSON to parse into a command, {@link + * TestConstants#rawNamesSubstitutor()} is used to replace names + * @param collectionName name of the collection that is in the command, so we can check that. + * @return The operation the resolver created. + */ + private CreateCollectionOperation assertCommand( String testName, String rawJson, CqlIdentifier collectionName) { var json = TEST_CONSTANTS.subsRawNames(rawJson); @@ -82,7 +95,7 @@ private CreateCollectionOperation resolveCommand( @Test public void successWithDefaults() { var operation = - resolveCommand( + assertCommand( "successWithDefaults()", """ { @@ -118,14 +131,14 @@ public void successWithSupportedNames() { } """ .formatted(name); - resolveCommand("successWithSupportedNames()", json, cqlIdentifierFromUserInput(name)); + assertCommand("successWithSupportedNames()", json, cqlIdentifierFromUserInput(name)); } } @Test public void successWithVector() { var operation = - resolveCommand( + assertCommand( "successWithVector()", """ { @@ -153,7 +166,7 @@ public void successWithVector() { public void successWithVectorDefaultMetric() { var operation = - resolveCommand( + assertCommand( "successWithVectorDefaultMetric()", """ { @@ -183,7 +196,7 @@ public void successWithVectorDefaultMetric() { @Test public void successWithVectorize() { var operation = - resolveCommand( + assertCommand( "successWithVector()", """ { @@ -227,7 +240,7 @@ public void successWithVectorize() { @Test public void successWithDenyIndexing() { var operation = - resolveCommand( + assertCommand( "successWithDenyIndexing()", """ { @@ -254,7 +267,7 @@ public void successWithDenyIndexing() { public void failWithUndefinedName() { var throwable = - resolveCommandThrows( + assertCommandThrows( "failWithUndefinedName()", """ { @@ -275,7 +288,7 @@ public void failWithUndefinedName() { public void failWithEmptyName() { var throwable = - resolveCommandThrows( + assertCommandThrows( "failWithUndefinedName()", """ { @@ -299,7 +312,7 @@ public void failWithBlankName() { // Blank is a white space var throwable = - resolveCommandThrows( + assertCommandThrows( "failWithBlankName()", """ { @@ -323,7 +336,7 @@ public void failWithNameTooLong() { var longName = RandomStringUtils.insecure().nextAlphabetic(49); var throwable = - resolveCommandThrows( + assertCommandThrows( "failWithNameTooLong()", """ { @@ -347,7 +360,7 @@ public void failWithNameTooLong() { public void failWithNameSpecialCharacter() { var throwable = - resolveCommandThrows( + assertCommandThrows( "failWithNameSpecialCharacter()", """ { diff --git a/src/test/java/io/stargate/sgv2/jsonapi/util/asserts/APIExceptionAssert.java b/src/test/java/io/stargate/sgv2/jsonapi/util/asserts/APIExceptionAssert.java index 38a3451eaa..464f52ba80 100644 --- a/src/test/java/io/stargate/sgv2/jsonapi/util/asserts/APIExceptionAssert.java +++ b/src/test/java/io/stargate/sgv2/jsonapi/util/asserts/APIExceptionAssert.java @@ -20,8 +20,8 @@ SELF assertThatAPIException( Function assertCtor, Class errorClass, Throwable throwable) { assertThat(throwable) - .as("Throwable is instance of " + errorClass.getSimpleName() + "") - .isInstanceOf(errorClass); + .as("Throwable is instance of " + errorClass.getSimpleName() + "") + .isInstanceOf(errorClass); return assertCtor.apply(errorClass.cast(throwable)); } diff --git a/src/test/java/io/stargate/sgv2/jsonapi/util/asserts/CommandResultAssert.java b/src/test/java/io/stargate/sgv2/jsonapi/util/asserts/CommandResultAssert.java index e125ee35b4..b616e44016 100644 --- a/src/test/java/io/stargate/sgv2/jsonapi/util/asserts/CommandResultAssert.java +++ b/src/test/java/io/stargate/sgv2/jsonapi/util/asserts/CommandResultAssert.java @@ -1,127 +1,84 @@ package io.stargate.sgv2.jsonapi.util.asserts; +import static org.assertj.core.api.Assertions.assertThat; + import io.stargate.sgv2.jsonapi.api.model.command.CommandResult; import io.stargate.sgv2.jsonapi.api.model.command.CommandStatus; import io.stargate.sgv2.jsonapi.api.v1.ResponseAssertions; import io.stargate.sgv2.jsonapi.exception.ErrorCode; -import io.stargate.sgv2.jsonapi.service.shredding.DocRowIdentifer; import org.assertj.core.api.AbstractAssert; -import java.util.List; - -import static org.assertj.core.api.Assertions.assertThat; - /** * Assertions for matching the {@link CommandResult} - *

- * NOTE: there is some logic duplication with {@link ResponseAssertions} which is used for + * + *

NOTE: there is some logic duplication with {@link ResponseAssertions} which is used for * integration tests, it would be good to have some underlying logic but not a big problem - *

*/ public class CommandResultAssert extends AbstractAssert { - private CommandResultAssert(CommandResult commandResult) { - super(commandResult, CommandResultAssert.class); - } + private CommandResultAssert(CommandResult commandResult) { + super(commandResult, CommandResultAssert.class); + } - protected static CommandResultAssert assertThatCommandResult(CommandResult commandResult) { + protected static CommandResultAssert assertThatCommandResult(CommandResult commandResult) { - // must always be non null - assertThat(commandResult) - .as("CommandResult is not null") - .isNotNull(); - return new CommandResultAssert(commandResult); - } + // must always be non null + assertThat(commandResult).as("CommandResult is not null").isNotNull(); + return new CommandResultAssert(commandResult); + } - public CommandResultAssert isDDLSuccess(){ - - assertThat(actual.data()) - .as("isDDLSuccess() - data is null") - .isNull(); - assertThat(actual.status()) - .as("isError() - status is no empty") - .isNotEmpty(); - assertThat(actual.errors()) - .as("isDDLSuccess() - errors is empty") - .isEmpty(); - return this; - } + public CommandResultAssert isDDLSuccess() { - public CommandResultAssert isError(){ - - assertThat(actual.data()) - .as("isError() - data is null") - .isNull(); - assertThat(actual.status()) - .as("isError() - status is empty") - .isEmpty(); - assertThat(actual.errors()) - .as("isError() - errors is not null") - .isNotNull(); - assertThat(actual.errors()) - .as("isError() - errors is not empty") - .isNotEmpty(); - return this; - } + assertThat(actual.data()).as("isDDLSuccess() - data is null").isNull(); + assertThat(actual.status()).as("isError() - status is no empty").isNotEmpty(); + assertThat(actual.errors()).as("isDDLSuccess() - errors is empty").isEmpty(); + return this; + } - public CommandResultAssert hasOnlyStatus(CommandStatus key, Object value) { - assertThat(actual.status()) - .as("status is not null") - .isNotNull(); - assertThat(actual.status()) - .as("status is only one") - .hasSize(1); - assertThat(actual.status().get(key)) - .as("status['%s']", key) - .isEqualTo(value); - - return this; - } + public CommandResultAssert isError() { - public CommandResultAssert hasNoStatus() { - assertThat(actual.status()) - .as("status is empty") - .isEmpty(); - return this; - } + assertThat(actual.data()).as("isError() - data is null").isNull(); + assertThat(actual.status()).as("isError() - status is empty").isEmpty(); + assertThat(actual.errors()).as("isError() - errors is not null").isNotNull(); + assertThat(actual.errors()).as("isError() - errors is not empty").isNotEmpty(); + return this; + } - public CommandResultAssert hasNoErrors() { - assertThat(actual.errors()) - .as("errors is empty") - .isEmpty(); - return this; - } + public CommandResultAssert hasOnlyStatus(CommandStatus key, Object value) { + assertThat(actual.status()).as("status is not null").isNotNull(); + assertThat(actual.status()).as("status is only one").hasSize(1); + assertThat(actual.status().get(key)).as("status['%s']", key).isEqualTo(value); - public CommandResultAssert hasErrorCount(int count) { - assertThat(actual.errors()) - .as("errors is not null") - .isNotNull(); - assertThat(actual.errors()) - .as("errors size") - .hasSize(count); - return this; - } + return this; + } - public CommandResultAssert hasOnlyError(ErrorCode errorCode, String... snippets) { - - assertThat(actual.errors()) - .as("errors is not null") - .isNotNull(); - assertThat(actual.errors()) - .as("errors size") - .hasSize(1); - - var commandError = actual.errors().getFirst(); - assertThat(commandError.errorCode()) - .as("errorCode") - .isEqualTo(errorCode.name()); - - for (var snippet : snippets) { - assertThat(commandError.message()) - .as("message") - .contains(snippet); - } - return this; - } + public CommandResultAssert hasNoStatus() { + assertThat(actual.status()).as("status is empty").isEmpty(); + return this; + } + + public CommandResultAssert hasNoErrors() { + assertThat(actual.errors()).as("errors is empty").isEmpty(); + return this; + } + public CommandResultAssert hasErrorCount(int count) { + assertThat(actual.errors()).as("errors is not null").isNotNull(); + assertThat(actual.errors()).as("errors size").hasSize(count); + return this; + } + + public CommandResultAssert hasOnlyError(ErrorCode errorCode, String... snippets) { + + assertThat(actual.errors()).as("errors is not null").isNotNull(); + assertThat(actual.errors()).as("errors size").hasSize(1); + + var commandError = actual.errors().getFirst(); + assertThat(commandError.errorCode()).as("errorCode").isEqualTo(errorCode.name()); + + for (var snippet : snippets) { + assertThat(commandError.message()).as("message").contains(snippet); + } + return this; + } } diff --git a/src/test/java/io/stargate/sgv2/jsonapi/util/asserts/DataAPIAsserts.java b/src/test/java/io/stargate/sgv2/jsonapi/util/asserts/DataAPIAsserts.java index 211354bb51..ebfde08b4e 100644 --- a/src/test/java/io/stargate/sgv2/jsonapi/util/asserts/DataAPIAsserts.java +++ b/src/test/java/io/stargate/sgv2/jsonapi/util/asserts/DataAPIAsserts.java @@ -5,16 +5,17 @@ public class DataAPIAsserts { - private DataAPIAsserts() {} + private DataAPIAsserts() {} - public static CommandResultAssert assertThatCommandResult(CommandResult commandResult) { - return CommandResultAssert.assertThatCommandResult(commandResult); - } - public static SchemaExceptionAssert assertThatSchemaException(Throwable throwable) { - return SchemaExceptionAssert.assertThatSchemaException(throwable); - } + public static CommandResultAssert assertThatCommandResult(CommandResult commandResult) { + return CommandResultAssert.assertThatCommandResult(commandResult); + } - public static SchemaExceptionAssert assertThatSchemaException(SchemaException schemaException) { - return SchemaExceptionAssert.assertThatSchemaException(schemaException); - } + public static SchemaExceptionAssert assertThatSchemaException(Throwable throwable) { + return SchemaExceptionAssert.assertThatSchemaException(throwable); + } + + public static SchemaExceptionAssert assertThatSchemaException(SchemaException schemaException) { + return SchemaExceptionAssert.assertThatSchemaException(schemaException); + } } diff --git a/src/test/java/io/stargate/sgv2/jsonapi/util/asserts/SchemaExceptionAssert.java b/src/test/java/io/stargate/sgv2/jsonapi/util/asserts/SchemaExceptionAssert.java index b567adea4f..a11e828c83 100644 --- a/src/test/java/io/stargate/sgv2/jsonapi/util/asserts/SchemaExceptionAssert.java +++ b/src/test/java/io/stargate/sgv2/jsonapi/util/asserts/SchemaExceptionAssert.java @@ -9,7 +9,8 @@ private SchemaExceptionAssert(SchemaException actual) { super(actual, SchemaExceptionAssert.class); } - protected static SchemaExceptionAssert assertThatSchemaException(SchemaException schemaException) { + protected static SchemaExceptionAssert assertThatSchemaException( + SchemaException schemaException) { return assertThatAPIException( SchemaExceptionAssert::new, SchemaException.class, schemaException); } From fb0128fc6e5eb81c0da0c151853254cb87149bd7 Mon Sep 17 00:00:00 2001 From: Aaron Morton Date: Wed, 17 Jun 2026 16:17:26 +1200 Subject: [PATCH 4/8] refactor the testing for create collection for the resolver and the operation getting it ready for changes to CreateCollectionOperation --- .../CreateCollectionOperation.java | 29 ++++ .../FindEmbeddingProvidersOperation.java | 5 +- .../CreateCollectionOperationTest.java | 2 +- .../resolver/CommandResolverTestBase.java | 87 +++++++++++ .../CreateCollectionCommandResolverTest.java | 142 +++--------------- ...eateCollectionCommandResolverTestBase.java | 64 ++++++++ ...onCommandResolverVectorizeEnabledTest.java | 74 +++++++++ ...llectionResolverVectorizeDisabledTest.java | 89 ++++------- ...rovidersResolverVectorizeDisabledTest.java | 60 ++++++++ 9 files changed, 363 insertions(+), 189 deletions(-) create mode 100644 src/test/java/io/stargate/sgv2/jsonapi/service/resolver/CommandResolverTestBase.java create mode 100644 src/test/java/io/stargate/sgv2/jsonapi/service/resolver/CreateCollectionCommandResolverTestBase.java create mode 100644 src/test/java/io/stargate/sgv2/jsonapi/service/resolver/CreateCollectionCommandResolverVectorizeEnabledTest.java create mode 100644 src/test/java/io/stargate/sgv2/jsonapi/service/resolver/FindEmbeddingProvidersResolverVectorizeDisabledTest.java diff --git a/src/main/java/io/stargate/sgv2/jsonapi/service/operation/collections/CreateCollectionOperation.java b/src/main/java/io/stargate/sgv2/jsonapi/service/operation/collections/CreateCollectionOperation.java index 51147f5059..7dd3341fba 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/service/operation/collections/CreateCollectionOperation.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/service/operation/collections/CreateCollectionOperation.java @@ -49,6 +49,7 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; +/** Creates a new collection in the target keyspace using the SuperShredding table model */ public class CreateCollectionOperation implements Operation { private static final Logger LOGGER = LoggerFactory.getLogger(CreateCollectionOperation.class); @@ -97,36 +98,64 @@ public CreateCollectionOperation( this.rerankDef = rerankDef; } + /** + * Present and visible because the old testing relied on this being a record, keeping until we + * review the testing in more detail and determine if needed. + */ @VisibleForTesting public CqlIdentifier collectionName() { return collectionName; } + /** + * Present and visible because the old testing relied on this being a record, keeping until we + * review the testing in more detail and determine if needed. + */ @VisibleForTesting public CommandContext commandContext() { return commandContext; } + /** + * Present and visible because the old testing relied on this being a record, keeping until we + * review the testing in more detail and determine if needed. + */ @VisibleForTesting public CreateCollectionCommand.Options.DocIdDesc docIdDesc() { return docIdDesc; } + /** + * Present and visible because the old testing relied on this being a record, keeping until we + * review the testing in more detail and determine if needed. + */ @VisibleForTesting public CreateCollectionCommand.Options.IndexingDesc indexingDesc() { return indexingDesc; } + /** + * Present and visible because the old testing relied on this being a record, keeping until we + * review the testing in more detail and determine if needed. + */ @VisibleForTesting public CreateCollectionCommand.Options.VectorSearchDesc vectorDesc() { return vectorDesc; } + /** + * Present and visible because the old testing relied on this being a record, keeping until we + * review the testing in more detail and determine if needed. + */ @VisibleForTesting public SchemaHolder lexicalDef() { return lexicalDef; } + /** + * Present and visible because the old testing relied on this being a record, keeping until we + * review the testing in more detail and determine if needed. + */ @VisibleForTesting public SchemaHolder rerankDef() { return rerankDef; diff --git a/src/main/java/io/stargate/sgv2/jsonapi/service/operation/embeddings/FindEmbeddingProvidersOperation.java b/src/main/java/io/stargate/sgv2/jsonapi/service/operation/embeddings/FindEmbeddingProvidersOperation.java index d4c54ac0b3..7df7bfaf13 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/service/operation/embeddings/FindEmbeddingProvidersOperation.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/service/operation/embeddings/FindEmbeddingProvidersOperation.java @@ -10,6 +10,7 @@ import io.stargate.sgv2.jsonapi.service.embedding.configuration.EmbeddingProvidersConfig; import io.stargate.sgv2.jsonapi.service.operation.Operation; import io.stargate.sgv2.jsonapi.service.provider.ApiModelSupport; +import io.stargate.sgv2.jsonapi.service.schema.DatabaseSchemaObject; import java.util.*; import java.util.function.Predicate; import java.util.function.Supplier; @@ -20,7 +21,9 @@ * CommandStatus#EXISTING_EMBEDDING_PROVIDERS} command status. */ public record FindEmbeddingProvidersOperation( - FindEmbeddingProvidersCommand command, EmbeddingProvidersConfig config) implements Operation { + FindEmbeddingProvidersCommand command, EmbeddingProvidersConfig config) + implements Operation { + @Override public Uni> execute( RequestContext dataApiRequestInfo, QueryExecutor queryExecutor) { diff --git a/src/test/java/io/stargate/sgv2/jsonapi/service/operation/collections/CreateCollectionOperationTest.java b/src/test/java/io/stargate/sgv2/jsonapi/service/operation/collections/CreateCollectionOperationTest.java index 77aaf9e102..20d99733d4 100644 --- a/src/test/java/io/stargate/sgv2/jsonapi/service/operation/collections/CreateCollectionOperationTest.java +++ b/src/test/java/io/stargate/sgv2/jsonapi/service/operation/collections/CreateCollectionOperationTest.java @@ -373,7 +373,7 @@ public void failMissingKeyspace() { /** * Test: create table works, but there is an index with the same name, the operation should then - * try to drop the table. + * try to drop the table. More complicated than others. */ @Test public void failExistingIndexDropTable() { diff --git a/src/test/java/io/stargate/sgv2/jsonapi/service/resolver/CommandResolverTestBase.java b/src/test/java/io/stargate/sgv2/jsonapi/service/resolver/CommandResolverTestBase.java new file mode 100644 index 0000000000..27856bdd44 --- /dev/null +++ b/src/test/java/io/stargate/sgv2/jsonapi/service/resolver/CommandResolverTestBase.java @@ -0,0 +1,87 @@ +package io.stargate.sgv2.jsonapi.service.resolver; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.catchThrowable; + +import com.fasterxml.jackson.core.JsonProcessingException; +import io.stargate.sgv2.jsonapi.TestConstants; +import io.stargate.sgv2.jsonapi.api.model.command.Command; +import io.stargate.sgv2.jsonapi.api.model.command.CommandContext; +import io.stargate.sgv2.jsonapi.service.operation.Operation; +import io.stargate.sgv2.jsonapi.service.operation.collections.CreateCollectionOperation; +import io.stargate.sgv2.jsonapi.service.schema.SchemaObject; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +/** + * Base class for tests for a {@link CommandResolver} to reduce some boilerplate. + * + *

Notes for users: + * + *

    + *
  • We still need some CDI injection, so there are abstract properties to access objects + * injected after construction on the subclass. + *
+ * + * @param the type of the schema under test, this is the type used by the opertion created + * @param the type of the command under test + * @param the type of the resolver under test + */ +abstract class CommandResolverTestBase< + SCHEMA extends SchemaObject, + COMMAND extends Command, + RESOLVER extends CommandResolver, + OPERATION extends Operation> { + + protected static final Logger LOGGER = LoggerFactory.getLogger(CommandResolverTestBase.class); + + protected final TestConstants TEST_CONSTANTS = new TestConstants(); + + protected abstract RESOLVER resolver(); + + protected abstract CommandContext commandContext(); + + protected abstract Class commandClass(); + + protected Throwable assertResolverThrows(String testName, String rawJson) { + var throwable = catchThrowable(() -> assertResolver(testName, rawJson)); + + assertThat(throwable).as("%s - exception is thrown", testName).isNotNull(); + + return throwable; + } + + /** + * Run the command through the resolver and do some basic tests on the operation that comes out. + * + * @param testName name for logging etc + * @param rawJson the raw command JSON to parse into a command, {@link + * TestConstants#rawNamesSubstitutor()} is used to replace names + * @return The operation the resolver created. + */ + protected OPERATION assertResolver(String testName, String rawJson) { + + var json = TEST_CONSTANTS.subsRawNames(rawJson); + LOGGER.info("assertResolver() - testName: {}, json: {}", testName, json); + + COMMAND command; + try { + command = TEST_CONSTANTS.OBJECT_MAPPER.readValue(json, commandClass()); + } catch (JsonProcessingException e) { + throw new RuntimeException(e); + } + + var localCommandContext = commandContext(); + var operation = resolver().resolveCommand(localCommandContext, command); + + assertThat(operation) + .isInstanceOfSatisfying( + CreateCollectionOperation.class, + op -> { + assertThat(op.commandContext()) + .as("%s - commandContext() is same object", testName) + .isSameAs(localCommandContext); + }); + return (OPERATION) operation; + } +} diff --git a/src/test/java/io/stargate/sgv2/jsonapi/service/resolver/CreateCollectionCommandResolverTest.java b/src/test/java/io/stargate/sgv2/jsonapi/service/resolver/CreateCollectionCommandResolverTest.java index ead104eb3b..2e770323ce 100644 --- a/src/test/java/io/stargate/sgv2/jsonapi/service/resolver/CreateCollectionCommandResolverTest.java +++ b/src/test/java/io/stargate/sgv2/jsonapi/service/resolver/CreateCollectionCommandResolverTest.java @@ -2,26 +2,14 @@ import static io.stargate.sgv2.jsonapi.util.CqlIdentifierUtil.cqlIdentifierFromUserInput; import static io.stargate.sgv2.jsonapi.util.asserts.DataAPIAsserts.assertThatSchemaException; -import static org.assertj.core.api.Assertions.*; -import static org.assertj.core.api.Assertions.catchThrowable; +import static org.assertj.core.api.Assertions.assertThat; -import com.datastax.oss.driver.api.core.CqlIdentifier; -import com.fasterxml.jackson.core.JsonProcessingException; import io.quarkus.test.junit.QuarkusTest; -import io.quarkus.test.junit.TestProfile; -import io.stargate.sgv2.jsonapi.TestConstants; -import io.stargate.sgv2.jsonapi.api.model.command.CommandContext; import io.stargate.sgv2.jsonapi.api.model.command.impl.CreateCollectionCommand; -import io.stargate.sgv2.jsonapi.api.model.command.impl.VectorizeConfig; import io.stargate.sgv2.jsonapi.exception.SchemaException; -import io.stargate.sgv2.jsonapi.service.operation.collections.CreateCollectionOperation; import io.stargate.sgv2.jsonapi.service.schema.EmbeddingSourceModel; -import io.stargate.sgv2.jsonapi.service.schema.KeyspaceSchemaObject; import io.stargate.sgv2.jsonapi.service.schema.SimilarityFunction; -import io.stargate.sgv2.jsonapi.util.profiles.EnabledVectorizeProfile; -import jakarta.inject.Inject; import java.util.List; -import java.util.Map; import org.apache.commons.lang3.RandomStringUtils; import org.junit.jupiter.api.Test; import org.slf4j.Logger; @@ -30,72 +18,20 @@ /** * Tests how the {@link CreateCollectionCommandResolver} handles inputs and the operation it * creates. + * + *

NOTE: subclassed atleast by {@link CreateCollectionResolverVectorizeDisabledTest} to + * change the vectorize enabled setting */ @QuarkusTest -@TestProfile(EnabledVectorizeProfile.class) -class CreateCollectionCommandResolverTest { +class CreateCollectionCommandResolverTest extends CreateCollectionCommandResolverTestBase { - private static final Logger LOGGER = + protected static final Logger LOGGER = LoggerFactory.getLogger(CreateCollectionCommandResolverTest.class); - @Inject CreateCollectionCommandResolver RESOLVER; - private final TestConstants TEST_CONSTANTS = new TestConstants(); - - // want a single instance for all calls, keyspaceContext() creates a new each call - private final CommandContext COMMAND_CONTEXT = - TEST_CONSTANTS.keyspaceContext(); - - private Throwable assertCommandThrows(String testName, String rawJson) { - return catchThrowable( - () -> assertCommand(testName, rawJson, TEST_CONSTANTS.COLLECTION_IDENTIFIER.table())); - } - - private CreateCollectionOperation assertCommand(String testName, String rawJson) { - return assertCommand(testName, rawJson, TEST_CONSTANTS.COLLECTION_IDENTIFIER.table()); - } - - /** - * Run the command through the resolver and do some basic tests on the operation that comes out. - * - * @param testName name for logging etc - * @param rawJson the raw command JSON to parse into a command, {@link - * TestConstants#rawNamesSubstitutor()} is used to replace names - * @param collectionName name of the collection that is in the command, so we can check that. - * @return The operation the resolver created. - */ - private CreateCollectionOperation assertCommand( - String testName, String rawJson, CqlIdentifier collectionName) { - - var json = TEST_CONSTANTS.subsRawNames(rawJson); - LOGGER.info("resolveCommand() - testName: {}, json: {}", testName, json); - - CreateCollectionCommand command; - try { - command = TEST_CONSTANTS.OBJECT_MAPPER.readValue(json, CreateCollectionCommand.class); - } catch (JsonProcessingException e) { - throw new RuntimeException(e); - } - - var operation = RESOLVER.resolveCommand(COMMAND_CONTEXT, command); - - assertThat(operation) - .isInstanceOfSatisfying( - CreateCollectionOperation.class, - op -> { - assertThat(op.collectionName()) - .as("%s - collectionName() matches command", testName) - .isEqualTo(collectionName); - assertThat(op.commandContext()) - .as("%s - commandContext() is same object", testName) - .isSameAs(COMMAND_CONTEXT); - }); - return (CreateCollectionOperation) operation; - } - @Test public void successWithDefaults() { var operation = - assertCommand( + assertResolver( "successWithDefaults()", """ { @@ -131,14 +67,14 @@ public void successWithSupportedNames() { } """ .formatted(name); - assertCommand("successWithSupportedNames()", json, cqlIdentifierFromUserInput(name)); + assertResolver("successWithSupportedNames()", json, cqlIdentifierFromUserInput(name)); } } @Test public void successWithVector() { var operation = - assertCommand( + assertResolver( "successWithVector()", """ { @@ -166,7 +102,7 @@ public void successWithVector() { public void successWithVectorDefaultMetric() { var operation = - assertCommand( + assertResolver( "successWithVectorDefaultMetric()", """ { @@ -193,54 +129,10 @@ public void successWithVectorDefaultMetric() { assertThat(operation.vectorDesc()).isEqualTo(expectedVectorDesc); } - @Test - public void successWithVectorize() { - var operation = - assertCommand( - "successWithVector()", - """ - { - "createCollection": { - "name": "${collection}", - "options": { - "vector": { - "metric": "cosine", - "dimension": 768, - "service": { - "provider": "azureOpenAI", - "modelName": "text-embedding-3-small", - "parameters": { - "resourceName": "testResourceName", - "deploymentId": "testResourceName" - } - } - } - } - } - } - """); - - // NOTE: source model of null turns into DEFAULT - // aaron 15 june 26 - not sure why but we need the name of the source model must be the CQL name - // cql cares about capitals, we dont when processing this normally - var expectedVectorDesc = - new CreateCollectionCommand.Options.VectorSearchDesc( - 768, - "cosine", - EmbeddingSourceModel.DEFAULT.cqlName(), - new VectorizeConfig( - "azureOpenAI", - "text-embedding-3-small", - null, - Map.of("resourceName", "testResourceName", "deploymentId", "testResourceName"))); - - assertThat(operation.vectorDesc()).isEqualTo(expectedVectorDesc); - } - @Test public void successWithDenyIndexing() { var operation = - assertCommand( + assertResolver( "successWithDenyIndexing()", """ { @@ -267,7 +159,7 @@ public void successWithDenyIndexing() { public void failWithUndefinedName() { var throwable = - assertCommandThrows( + assertResolverThrows( "failWithUndefinedName()", """ { @@ -288,8 +180,8 @@ public void failWithUndefinedName() { public void failWithEmptyName() { var throwable = - assertCommandThrows( - "failWithUndefinedName()", + assertResolverThrows( + "failWithEmptyName()", """ { "createCollection": { @@ -312,7 +204,7 @@ public void failWithBlankName() { // Blank is a white space var throwable = - assertCommandThrows( + assertResolverThrows( "failWithBlankName()", """ { @@ -336,7 +228,7 @@ public void failWithNameTooLong() { var longName = RandomStringUtils.insecure().nextAlphabetic(49); var throwable = - assertCommandThrows( + assertResolverThrows( "failWithNameTooLong()", """ { @@ -360,7 +252,7 @@ public void failWithNameTooLong() { public void failWithNameSpecialCharacter() { var throwable = - assertCommandThrows( + assertResolverThrows( "failWithNameSpecialCharacter()", """ { diff --git a/src/test/java/io/stargate/sgv2/jsonapi/service/resolver/CreateCollectionCommandResolverTestBase.java b/src/test/java/io/stargate/sgv2/jsonapi/service/resolver/CreateCollectionCommandResolverTestBase.java new file mode 100644 index 0000000000..7cd3cb6a02 --- /dev/null +++ b/src/test/java/io/stargate/sgv2/jsonapi/service/resolver/CreateCollectionCommandResolverTestBase.java @@ -0,0 +1,64 @@ +package io.stargate.sgv2.jsonapi.service.resolver; + +import static org.assertj.core.api.Assertions.assertThat; + +import com.datastax.oss.driver.api.core.CqlIdentifier; +import io.stargate.sgv2.jsonapi.api.model.command.CommandContext; +import io.stargate.sgv2.jsonapi.api.model.command.impl.CreateCollectionCommand; +import io.stargate.sgv2.jsonapi.service.operation.collections.CreateCollectionOperation; +import io.stargate.sgv2.jsonapi.service.schema.KeyspaceSchemaObject; +import jakarta.inject.Inject; + +/** + * Tests how the {@link CreateCollectionCommandResolver} handles inputs and the operation it + * creates. + * + *

NOTE: subclassed atleast by {@link CreateCollectionResolverVectorizeDisabledTest} to + * change the vectorize enabled setting + */ +class CreateCollectionCommandResolverTestBase + extends CommandResolverTestBase< + KeyspaceSchemaObject, + CreateCollectionCommand, + CreateCollectionCommandResolver, + CreateCollectionOperation> { + + @Inject CreateCollectionCommandResolver RESOLVER; + protected final CommandContext COMMAND_CONTEXT = + TEST_CONSTANTS.keyspaceContext(); + + @Override + protected CreateCollectionCommandResolver resolver() { + return RESOLVER; + } + + @Override + protected CommandContext commandContext() { + return COMMAND_CONTEXT; + } + + @Override + protected Class commandClass() { + return CreateCollectionCommand.class; + } + + /** + * Run the command json through the resolver using the super class, then some extra common + * assertions for the createCollection command. + */ + protected CreateCollectionOperation assertResolver( + String testName, String rawJson, CqlIdentifier collectionName) { + + var operation = super.assertResolver(testName, rawJson); + + assertThat(operation) + .isInstanceOfSatisfying( + CreateCollectionOperation.class, + op -> { + assertThat(op.collectionName()) + .as("%s - collectionName() matches command", testName) + .isEqualTo(collectionName); + }); + return operation; + } +} diff --git a/src/test/java/io/stargate/sgv2/jsonapi/service/resolver/CreateCollectionCommandResolverVectorizeEnabledTest.java b/src/test/java/io/stargate/sgv2/jsonapi/service/resolver/CreateCollectionCommandResolverVectorizeEnabledTest.java new file mode 100644 index 0000000000..2f4ffeca15 --- /dev/null +++ b/src/test/java/io/stargate/sgv2/jsonapi/service/resolver/CreateCollectionCommandResolverVectorizeEnabledTest.java @@ -0,0 +1,74 @@ +package io.stargate.sgv2.jsonapi.service.resolver; + +import static org.assertj.core.api.Assertions.assertThat; + +import io.quarkus.test.junit.QuarkusTest; +import io.quarkus.test.junit.TestProfile; +import io.stargate.sgv2.jsonapi.api.model.command.impl.CreateCollectionCommand; +import io.stargate.sgv2.jsonapi.api.model.command.impl.VectorizeConfig; +import io.stargate.sgv2.jsonapi.service.schema.EmbeddingSourceModel; +import io.stargate.sgv2.jsonapi.util.profiles.EnabledVectorizeProfile; +import java.util.Map; +import org.junit.jupiter.api.Test; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +/** + * Tests how the {@link CreateCollectionCommandResolver} handles inputs and the operation it + * creates. + * + *

NOTE: subclassed atleast by {@link CreateCollectionResolverVectorizeDisabledTest} to + * change the vectorize enabled setting + */ +@QuarkusTest +@TestProfile(EnabledVectorizeProfile.class) +class CreateCollectionCommandResolverVectorizeEnabledTest + extends CreateCollectionCommandResolverTestBase { + + protected static final Logger LOGGER = + LoggerFactory.getLogger(CreateCollectionCommandResolverVectorizeEnabledTest.class); + + @Test + public void successWithVectorize() { + var operation = + assertResolver( + "successWithVector()", + """ + { + "createCollection": { + "name": "${collection}", + "options": { + "vector": { + "metric": "cosine", + "dimension": 768, + "service": { + "provider": "azureOpenAI", + "modelName": "text-embedding-3-small", + "parameters": { + "resourceName": "testResourceName", + "deploymentId": "testResourceName" + } + } + } + } + } + } + """); + + // NOTE: source model of null turns into DEFAULT + // aaron 15 june 26 - not sure why but we need the name of the source model must be the CQL name + // cql cares about capitals, we dont when processing this normally + var expectedVectorDesc = + new CreateCollectionCommand.Options.VectorSearchDesc( + 768, + "cosine", + EmbeddingSourceModel.DEFAULT.cqlName(), + new VectorizeConfig( + "azureOpenAI", + "text-embedding-3-small", + null, + Map.of("resourceName", "testResourceName", "deploymentId", "testResourceName"))); + + assertThat(operation.vectorDesc()).isEqualTo(expectedVectorDesc); + } +} diff --git a/src/test/java/io/stargate/sgv2/jsonapi/service/resolver/CreateCollectionResolverVectorizeDisabledTest.java b/src/test/java/io/stargate/sgv2/jsonapi/service/resolver/CreateCollectionResolverVectorizeDisabledTest.java index 310c462ba3..9cb1b8fb41 100644 --- a/src/test/java/io/stargate/sgv2/jsonapi/service/resolver/CreateCollectionResolverVectorizeDisabledTest.java +++ b/src/test/java/io/stargate/sgv2/jsonapi/service/resolver/CreateCollectionResolverVectorizeDisabledTest.java @@ -1,79 +1,44 @@ package io.stargate.sgv2.jsonapi.service.resolver; -import static org.assertj.core.api.Assertions.assertThat; -import static org.assertj.core.api.Assertions.catchException; +import static io.stargate.sgv2.jsonapi.util.asserts.DataAPIAsserts.assertThatSchemaException; -import com.fasterxml.jackson.databind.ObjectMapper; import io.quarkus.test.junit.QuarkusTest; import io.quarkus.test.junit.TestProfile; -import io.stargate.sgv2.jsonapi.TestConstants; -import io.stargate.sgv2.jsonapi.api.model.command.impl.CreateCollectionCommand; -import io.stargate.sgv2.jsonapi.api.model.command.impl.FindEmbeddingProvidersCommand; import io.stargate.sgv2.jsonapi.exception.SchemaException; import io.stargate.sgv2.jsonapi.util.profiles.DisableVectorizeProfile; -import jakarta.inject.Inject; import org.junit.jupiter.api.Test; @QuarkusTest @TestProfile(DisableVectorizeProfile.class) -public class CreateCollectionResolverVectorizeDisabledTest { - @Inject ObjectMapper objectMapper; - @Inject CreateCollectionCommandResolver createCollectionCommandResolver; - @Inject FindEmbeddingProvidersCommandResolver findEmbeddingProvidersCommandResolver; - private final TestConstants testConstants = new TestConstants(); +public class CreateCollectionResolverVectorizeDisabledTest + extends CreateCollectionCommandResolverTestBase { @Test - public void vectorizeSearchDisabled() throws Exception { - String json = - """ - { - "createCollection": { - "name": "my_collection", - "options": { - "vector": { - "metric": "cosine", - "dimension": 1024, - "service": { - "provider": "nvidia", - "modelName": "NV-Embed-QA" - } - } - } - } - } - """; + public void failVectorizeSearchDisabled() { - CreateCollectionCommand command = objectMapper.readValue(json, CreateCollectionCommand.class); - Exception e = - catchException( - () -> - createCollectionCommandResolver.resolveCommand( - testConstants.keyspaceContext(), command)); - assertThat(e) - .hasFieldOrPropertyWithValue( - "code", SchemaException.Code.VECTORIZE_FEATURE_NOT_AVAILABLE.name()); - } - - @Test - public void findEmbeddingProvidersWithVectorizeSearchDisabled() throws Exception { - // TODO: This test should be moved, these are rests for create collection NOT for - // findEmbeddingProviders. + var throwable = + assertResolverThrows( + "failVectorizeSearchDisabled()", + """ + { + "createCollection": { + "name": "my_collection", + "options": { + "vector": { + "metric": "cosine", + "dimension": 1024, + "service": { + "provider": "nvidia", + "modelName": "NV-Embed-QA" + } + } + } + } + } + """); - String json = - """ - { - "findEmbeddingProviders": {} - } - """; - FindEmbeddingProvidersCommand command = - objectMapper.readValue(json, FindEmbeddingProvidersCommand.class); - Exception e = - catchException( - () -> - findEmbeddingProvidersCommandResolver.resolveCommand( - testConstants.databaseContext(), command)); - assertThat(e) - .hasFieldOrPropertyWithValue( - "code", SchemaException.Code.VECTORIZE_FEATURE_NOT_AVAILABLE.name()); + assertThatSchemaException(throwable) + .as("failVectorizeSearchDisabled()") + .hasCode(SchemaException.Code.VECTORIZE_FEATURE_NOT_AVAILABLE); } } diff --git a/src/test/java/io/stargate/sgv2/jsonapi/service/resolver/FindEmbeddingProvidersResolverVectorizeDisabledTest.java b/src/test/java/io/stargate/sgv2/jsonapi/service/resolver/FindEmbeddingProvidersResolverVectorizeDisabledTest.java new file mode 100644 index 0000000000..6a6d481f01 --- /dev/null +++ b/src/test/java/io/stargate/sgv2/jsonapi/service/resolver/FindEmbeddingProvidersResolverVectorizeDisabledTest.java @@ -0,0 +1,60 @@ +package io.stargate.sgv2.jsonapi.service.resolver; + +import static io.stargate.sgv2.jsonapi.util.asserts.DataAPIAsserts.assertThatSchemaException; + +import io.quarkus.test.junit.QuarkusTest; +import io.quarkus.test.junit.TestProfile; +import io.stargate.sgv2.jsonapi.api.model.command.CommandContext; +import io.stargate.sgv2.jsonapi.api.model.command.impl.FindEmbeddingProvidersCommand; +import io.stargate.sgv2.jsonapi.exception.SchemaException; +import io.stargate.sgv2.jsonapi.service.operation.embeddings.FindEmbeddingProvidersOperation; +import io.stargate.sgv2.jsonapi.service.schema.DatabaseSchemaObject; +import io.stargate.sgv2.jsonapi.util.profiles.DisableVectorizeProfile; +import jakarta.inject.Inject; +import org.junit.jupiter.api.Test; + +@QuarkusTest +@TestProfile(DisableVectorizeProfile.class) +public class FindEmbeddingProvidersResolverVectorizeDisabledTest + extends CommandResolverTestBase< + DatabaseSchemaObject, + FindEmbeddingProvidersCommand, + FindEmbeddingProvidersCommandResolver, + FindEmbeddingProvidersOperation> { + + @Inject FindEmbeddingProvidersCommandResolver RESOLVER; + private final CommandContext COMMAND_CONTEXT = + TEST_CONSTANTS.databaseContext(); + + @Override + protected FindEmbeddingProvidersCommandResolver resolver() { + return RESOLVER; + } + + @Override + protected CommandContext commandContext() { + return COMMAND_CONTEXT; + } + + @Override + protected Class commandClass() { + return FindEmbeddingProvidersCommand.class; + } + + @Test + public void failVectorizeSearchDisabled() { + + var throwable = + assertResolverThrows( + "failVectorizeSearchDisabled()", + """ + { + "findEmbeddingProviders": {} + } + """); + + assertThatSchemaException(throwable) + .as("failVectorizeSearchDisabled()") + .hasCode(SchemaException.Code.VECTORIZE_FEATURE_NOT_AVAILABLE); + } +} From 758bbecdbbe50c1363f93d4c5916865754941b2e Mon Sep 17 00:00:00 2001 From: Aaron Morton Date: Wed, 17 Jun 2026 16:41:26 +1200 Subject: [PATCH 5/8] test fixes --- .../api/request/tenant/SubdomainTenantResolverTests.java | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/test/java/io/stargate/sgv2/jsonapi/api/request/tenant/SubdomainTenantResolverTests.java b/src/test/java/io/stargate/sgv2/jsonapi/api/request/tenant/SubdomainTenantResolverTests.java index c68ccb7cb5..ea99b756ca 100644 --- a/src/test/java/io/stargate/sgv2/jsonapi/api/request/tenant/SubdomainTenantResolverTests.java +++ b/src/test/java/io/stargate/sgv2/jsonapi/api/request/tenant/SubdomainTenantResolverTests.java @@ -31,6 +31,8 @@ public class SubdomainTenantResolverTests { @BeforeEach public void initTenantFactory() { + // when run in the full suite, another test may leave this initialized + TenantFactory.reset(); TenantFactory.initialize(DatabaseType.ASTRA); } From c4ffb07a829a4bd30bbf89eef03dddaacfc87945 Mon Sep 17 00:00:00 2001 From: Aaron Morton Date: Wed, 17 Jun 2026 16:49:31 +1200 Subject: [PATCH 6/8] add missing @TestProfile(NoGlobalResourcesTestProfile.Impl.class) --- .../resolver/CreateCollectionCommandResolverTest.java | 6 +++++- .../resolver/CreateCollectionCommandResolverTestBase.java | 6 +----- ...eateCollectionCommandResolverVectorizeDisabledTest.java} | 3 ++- ...CreateCollectionCommandResolverVectorizeEnabledTest.java | 2 +- 4 files changed, 9 insertions(+), 8 deletions(-) rename src/test/java/io/stargate/sgv2/jsonapi/service/resolver/{CreateCollectionResolverVectorizeDisabledTest.java => CreateCollectionCommandResolverVectorizeDisabledTest.java} (91%) diff --git a/src/test/java/io/stargate/sgv2/jsonapi/service/resolver/CreateCollectionCommandResolverTest.java b/src/test/java/io/stargate/sgv2/jsonapi/service/resolver/CreateCollectionCommandResolverTest.java index 2e770323ce..5e0b3ed688 100644 --- a/src/test/java/io/stargate/sgv2/jsonapi/service/resolver/CreateCollectionCommandResolverTest.java +++ b/src/test/java/io/stargate/sgv2/jsonapi/service/resolver/CreateCollectionCommandResolverTest.java @@ -5,11 +5,14 @@ import static org.assertj.core.api.Assertions.assertThat; import io.quarkus.test.junit.QuarkusTest; +import io.quarkus.test.junit.TestProfile; import io.stargate.sgv2.jsonapi.api.model.command.impl.CreateCollectionCommand; import io.stargate.sgv2.jsonapi.exception.SchemaException; import io.stargate.sgv2.jsonapi.service.schema.EmbeddingSourceModel; import io.stargate.sgv2.jsonapi.service.schema.SimilarityFunction; import java.util.List; + +import io.stargate.sgv2.jsonapi.testresource.NoGlobalResourcesTestProfile; import org.apache.commons.lang3.RandomStringUtils; import org.junit.jupiter.api.Test; import org.slf4j.Logger; @@ -19,10 +22,11 @@ * Tests how the {@link CreateCollectionCommandResolver} handles inputs and the operation it * creates. * - *

NOTE: subclassed atleast by {@link CreateCollectionResolverVectorizeDisabledTest} to + *

NOTE: subclassed atleast by {@link CreateCollectionCommandResolverVectorizeDisabledTest} to * change the vectorize enabled setting */ @QuarkusTest +@TestProfile(NoGlobalResourcesTestProfile.Impl.class) class CreateCollectionCommandResolverTest extends CreateCollectionCommandResolverTestBase { protected static final Logger LOGGER = diff --git a/src/test/java/io/stargate/sgv2/jsonapi/service/resolver/CreateCollectionCommandResolverTestBase.java b/src/test/java/io/stargate/sgv2/jsonapi/service/resolver/CreateCollectionCommandResolverTestBase.java index 7cd3cb6a02..6e7df3b958 100644 --- a/src/test/java/io/stargate/sgv2/jsonapi/service/resolver/CreateCollectionCommandResolverTestBase.java +++ b/src/test/java/io/stargate/sgv2/jsonapi/service/resolver/CreateCollectionCommandResolverTestBase.java @@ -10,11 +10,7 @@ import jakarta.inject.Inject; /** - * Tests how the {@link CreateCollectionCommandResolver} handles inputs and the operation it - * creates. - * - *

NOTE: subclassed atleast by {@link CreateCollectionResolverVectorizeDisabledTest} to - * change the vectorize enabled setting + * re-usable base class for tests for {@link CreateCollectionCommandResolver}. */ class CreateCollectionCommandResolverTestBase extends CommandResolverTestBase< diff --git a/src/test/java/io/stargate/sgv2/jsonapi/service/resolver/CreateCollectionResolverVectorizeDisabledTest.java b/src/test/java/io/stargate/sgv2/jsonapi/service/resolver/CreateCollectionCommandResolverVectorizeDisabledTest.java similarity index 91% rename from src/test/java/io/stargate/sgv2/jsonapi/service/resolver/CreateCollectionResolverVectorizeDisabledTest.java rename to src/test/java/io/stargate/sgv2/jsonapi/service/resolver/CreateCollectionCommandResolverVectorizeDisabledTest.java index 9cb1b8fb41..78730f7aa7 100644 --- a/src/test/java/io/stargate/sgv2/jsonapi/service/resolver/CreateCollectionResolverVectorizeDisabledTest.java +++ b/src/test/java/io/stargate/sgv2/jsonapi/service/resolver/CreateCollectionCommandResolverVectorizeDisabledTest.java @@ -5,12 +5,13 @@ import io.quarkus.test.junit.QuarkusTest; import io.quarkus.test.junit.TestProfile; import io.stargate.sgv2.jsonapi.exception.SchemaException; +import io.stargate.sgv2.jsonapi.testresource.NoGlobalResourcesTestProfile; import io.stargate.sgv2.jsonapi.util.profiles.DisableVectorizeProfile; import org.junit.jupiter.api.Test; @QuarkusTest @TestProfile(DisableVectorizeProfile.class) -public class CreateCollectionResolverVectorizeDisabledTest +public class CreateCollectionCommandResolverVectorizeDisabledTest extends CreateCollectionCommandResolverTestBase { @Test diff --git a/src/test/java/io/stargate/sgv2/jsonapi/service/resolver/CreateCollectionCommandResolverVectorizeEnabledTest.java b/src/test/java/io/stargate/sgv2/jsonapi/service/resolver/CreateCollectionCommandResolverVectorizeEnabledTest.java index 2f4ffeca15..c2ce5c8949 100644 --- a/src/test/java/io/stargate/sgv2/jsonapi/service/resolver/CreateCollectionCommandResolverVectorizeEnabledTest.java +++ b/src/test/java/io/stargate/sgv2/jsonapi/service/resolver/CreateCollectionCommandResolverVectorizeEnabledTest.java @@ -17,7 +17,7 @@ * Tests how the {@link CreateCollectionCommandResolver} handles inputs and the operation it * creates. * - *

NOTE: subclassed atleast by {@link CreateCollectionResolverVectorizeDisabledTest} to + *

NOTE: subclassed atleast by {@link CreateCollectionCommandResolverVectorizeDisabledTest} to * change the vectorize enabled setting */ @QuarkusTest From f6fd4922806678105c01ec900afb61c1c6a7105d Mon Sep 17 00:00:00 2001 From: Aaron Morton Date: Wed, 17 Jun 2026 16:52:38 +1200 Subject: [PATCH 7/8] fmt --- .../resolver/CreateCollectionCommandResolverTest.java | 7 +++---- .../resolver/CreateCollectionCommandResolverTestBase.java | 4 +--- ...eateCollectionCommandResolverVectorizeDisabledTest.java | 1 - ...reateCollectionCommandResolverVectorizeEnabledTest.java | 4 ++-- 4 files changed, 6 insertions(+), 10 deletions(-) diff --git a/src/test/java/io/stargate/sgv2/jsonapi/service/resolver/CreateCollectionCommandResolverTest.java b/src/test/java/io/stargate/sgv2/jsonapi/service/resolver/CreateCollectionCommandResolverTest.java index 5e0b3ed688..0a06912319 100644 --- a/src/test/java/io/stargate/sgv2/jsonapi/service/resolver/CreateCollectionCommandResolverTest.java +++ b/src/test/java/io/stargate/sgv2/jsonapi/service/resolver/CreateCollectionCommandResolverTest.java @@ -10,9 +10,8 @@ import io.stargate.sgv2.jsonapi.exception.SchemaException; import io.stargate.sgv2.jsonapi.service.schema.EmbeddingSourceModel; import io.stargate.sgv2.jsonapi.service.schema.SimilarityFunction; -import java.util.List; - import io.stargate.sgv2.jsonapi.testresource.NoGlobalResourcesTestProfile; +import java.util.List; import org.apache.commons.lang3.RandomStringUtils; import org.junit.jupiter.api.Test; import org.slf4j.Logger; @@ -22,8 +21,8 @@ * Tests how the {@link CreateCollectionCommandResolver} handles inputs and the operation it * creates. * - *

NOTE: subclassed atleast by {@link CreateCollectionCommandResolverVectorizeDisabledTest} to - * change the vectorize enabled setting + *

NOTE: subclassed atleast by {@link + * CreateCollectionCommandResolverVectorizeDisabledTest} to change the vectorize enabled setting */ @QuarkusTest @TestProfile(NoGlobalResourcesTestProfile.Impl.class) diff --git a/src/test/java/io/stargate/sgv2/jsonapi/service/resolver/CreateCollectionCommandResolverTestBase.java b/src/test/java/io/stargate/sgv2/jsonapi/service/resolver/CreateCollectionCommandResolverTestBase.java index 6e7df3b958..1c30f98121 100644 --- a/src/test/java/io/stargate/sgv2/jsonapi/service/resolver/CreateCollectionCommandResolverTestBase.java +++ b/src/test/java/io/stargate/sgv2/jsonapi/service/resolver/CreateCollectionCommandResolverTestBase.java @@ -9,9 +9,7 @@ import io.stargate.sgv2.jsonapi.service.schema.KeyspaceSchemaObject; import jakarta.inject.Inject; -/** - * re-usable base class for tests for {@link CreateCollectionCommandResolver}. - */ +/** re-usable base class for tests for {@link CreateCollectionCommandResolver}. */ class CreateCollectionCommandResolverTestBase extends CommandResolverTestBase< KeyspaceSchemaObject, diff --git a/src/test/java/io/stargate/sgv2/jsonapi/service/resolver/CreateCollectionCommandResolverVectorizeDisabledTest.java b/src/test/java/io/stargate/sgv2/jsonapi/service/resolver/CreateCollectionCommandResolverVectorizeDisabledTest.java index 78730f7aa7..1f71757477 100644 --- a/src/test/java/io/stargate/sgv2/jsonapi/service/resolver/CreateCollectionCommandResolverVectorizeDisabledTest.java +++ b/src/test/java/io/stargate/sgv2/jsonapi/service/resolver/CreateCollectionCommandResolverVectorizeDisabledTest.java @@ -5,7 +5,6 @@ import io.quarkus.test.junit.QuarkusTest; import io.quarkus.test.junit.TestProfile; import io.stargate.sgv2.jsonapi.exception.SchemaException; -import io.stargate.sgv2.jsonapi.testresource.NoGlobalResourcesTestProfile; import io.stargate.sgv2.jsonapi.util.profiles.DisableVectorizeProfile; import org.junit.jupiter.api.Test; diff --git a/src/test/java/io/stargate/sgv2/jsonapi/service/resolver/CreateCollectionCommandResolverVectorizeEnabledTest.java b/src/test/java/io/stargate/sgv2/jsonapi/service/resolver/CreateCollectionCommandResolverVectorizeEnabledTest.java index c2ce5c8949..8a5146030c 100644 --- a/src/test/java/io/stargate/sgv2/jsonapi/service/resolver/CreateCollectionCommandResolverVectorizeEnabledTest.java +++ b/src/test/java/io/stargate/sgv2/jsonapi/service/resolver/CreateCollectionCommandResolverVectorizeEnabledTest.java @@ -17,8 +17,8 @@ * Tests how the {@link CreateCollectionCommandResolver} handles inputs and the operation it * creates. * - *

NOTE: subclassed atleast by {@link CreateCollectionCommandResolverVectorizeDisabledTest} to - * change the vectorize enabled setting + *

NOTE: subclassed atleast by {@link + * CreateCollectionCommandResolverVectorizeDisabledTest} to change the vectorize enabled setting */ @QuarkusTest @TestProfile(EnabledVectorizeProfile.class) From 74fde740611a1fbc346bf67aa56cfc001ead316e Mon Sep 17 00:00:00 2001 From: Aaron Morton Date: Thu, 18 Jun 2026 14:12:54 +1200 Subject: [PATCH 8/8] code tidy, ready for review --- .../model/command/impl/IndexingDescTest.java | 86 +++++++++++-------- .../CreateCollectionOperationTest.java | 38 ++++++-- .../resolver/CommandResolverTestBase.java | 52 ++++++++--- ...eateCollectionCommandResolverTestBase.java | 13 ++- ...onCommandResolverVectorizeEnabledTest.java | 5 -- ...rovidersResolverVectorizeDisabledTest.java | 7 ++ .../util/asserts/APIExceptionAssert.java | 71 ++++++++++++++- .../util/asserts/CommandResultAssert.java | 3 +- .../jsonapi/util/asserts/DataAPIAsserts.java | 7 ++ .../util/asserts/SchemaExceptionAssert.java | 3 + .../profiles/DisableVectorizeProfile.java | 4 + .../profiles/EnabledVectorizeProfile.java | 4 + 12 files changed, 228 insertions(+), 65 deletions(-) diff --git a/src/test/java/io/stargate/sgv2/jsonapi/api/model/command/impl/IndexingDescTest.java b/src/test/java/io/stargate/sgv2/jsonapi/api/model/command/impl/IndexingDescTest.java index 062496e7f2..25063d3750 100644 --- a/src/test/java/io/stargate/sgv2/jsonapi/api/model/command/impl/IndexingDescTest.java +++ b/src/test/java/io/stargate/sgv2/jsonapi/api/model/command/impl/IndexingDescTest.java @@ -7,21 +7,41 @@ import io.stargate.sgv2.jsonapi.TestConstants; import io.stargate.sgv2.jsonapi.exception.ErrorCode; import io.stargate.sgv2.jsonapi.exception.SchemaException; +import java.util.List; import org.junit.jupiter.api.Test; import org.slf4j.Logger; import org.slf4j.LoggerFactory; -/** Unit tests for the {@link CreateCollectionCommand.Options.IndexingDesc} */ +/** + * Unit tests for the {@link CreateCollectionCommand.Options.IndexingDesc} + * + *

We are testing that when created from JSON and validated, exceptions are thrown and functions + * derived from the record state work. Not really testing that deserialisationw works. + */ public class IndexingDescTest { private static final Logger LOGGER = LoggerFactory.getLogger(IndexingDescTest.class); private final TestConstants TEST_CONSTANTS = new TestConstants(); + private CreateCollectionCommand.Options.IndexingDesc assertValidate( + String testName, String json, CreateCollectionCommand.Options.IndexingDesc expected) { + + LOGGER.info("assertValidate() - testName: {}, json:{}", testName, json); + + var actualIndexingDesc = deserialize(testName, json); + actualIndexingDesc.validate(); + + assertThat(actualIndexingDesc).as(testName).isEqualTo(expected); + return actualIndexingDesc; + } + /** Assert that validating the JSON for the IndexingDesc will result in a SchemaException */ - private void assertSchemaException( + private void assertValidateFails( String testName, String json, ErrorCode errorCode, String... message) { + LOGGER.info("assertValidateFails() - testName: {}, json:{}", testName, json); + var indexingDesc = deserialize(testName, json); var throwable = catchThrowable(indexingDesc::validate); @@ -45,7 +65,7 @@ public void failAllowAndDeny() { ] }"""; - assertSchemaException( + assertValidateFails( "failAllowAndDeny()", json, SchemaException.Code.INVALID_INDEXING_DEFINITION, @@ -59,7 +79,7 @@ public void failNeitherAllowNorDeny() { """ {}"""; - assertSchemaException( + assertValidateFails( "failNeitherAllowNorDeny()", json, SchemaException.Code.INVALID_INDEXING_DEFINITION, @@ -78,7 +98,7 @@ public void failAllowDuplicates() { ] }"""; - assertSchemaException( + assertValidateFails( "failAllowDuplicates()", json, SchemaException.Code.INVALID_INDEXING_DEFINITION, @@ -97,7 +117,7 @@ public void failDenyDuplicates() { ] }"""; - assertSchemaException( + assertValidateFails( "failDenyDuplicates()", json, SchemaException.Code.INVALID_INDEXING_DEFINITION, @@ -115,7 +135,7 @@ public void failAllowEmptyPath() { ] }"""; - assertSchemaException( + assertValidateFails( "failAllowEmptyPath()", json, SchemaException.Code.INVALID_INDEXING_DEFINITION, @@ -133,7 +153,7 @@ public void failDenyEmptyPath() { ] }"""; - assertSchemaException( + assertValidateFails( "failDenyEmptyPath()", json, SchemaException.Code.INVALID_INDEXING_DEFINITION, @@ -151,7 +171,7 @@ public void failAllowDollarPrefix() { ] }"""; - assertSchemaException( + assertValidateFails( "failAllowDollarPrefix()", json, SchemaException.Code.INVALID_INDEXING_DEFINITION, @@ -169,7 +189,7 @@ public void failDenyDollarPrefix() { ] }"""; - assertSchemaException( + assertValidateFails( "failDenyDollarPrefix()", json, SchemaException.Code.INVALID_INDEXING_DEFINITION, @@ -187,7 +207,7 @@ public void failAllowInvalidPath() { ] }"""; - assertSchemaException( + assertValidateFails( "failAllowInvalidPath()", json, SchemaException.Code.INVALID_INDEXING_DEFINITION, @@ -205,7 +225,7 @@ public void failDenyInvalidPath() { ] }"""; - assertSchemaException( + assertValidateFails( "failDenyInvalidPath()", json, SchemaException.Code.INVALID_INDEXING_DEFINITION, @@ -223,10 +243,8 @@ public void successAllowStar() { ] }"""; - var indexingDesc = deserialize("successAllowStar()", json); - indexingDesc.validate(); - assertThat(indexingDesc.allow()).containsExactly("*"); - assertThat(indexingDesc.deny()).isNull(); + var expected = new CreateCollectionCommand.Options.IndexingDesc(List.of("*"), null); + assertValidate("successAllowStar()", json, expected); } @Test @@ -240,11 +258,9 @@ public void successDenyStar() { ] }"""; - var indexingDesc = deserialize("successDenyStar()", json); - indexingDesc.validate(); - assertThat(indexingDesc.deny()).containsExactly("*"); - assertThat(indexingDesc.denyAll()).as("delayAll true when deny '*'").isTrue(); - assertThat(indexingDesc.allow()).isNull(); + var expected = new CreateCollectionCommand.Options.IndexingDesc(null, List.of("*")); + var actual = assertValidate("successDenyStar()", json, expected); + assertThat(actual.denyAll()).as("delayAll true when deny '*'").isTrue(); } @Test @@ -258,10 +274,8 @@ public void successAllowVector() { ] }"""; - var indexingDesc = deserialize("successAllowVector()", json); - indexingDesc.validate(); - assertThat(indexingDesc.allow()).containsExactly("$vector"); - assertThat(indexingDesc.deny()).isNull(); + var expected = new CreateCollectionCommand.Options.IndexingDesc(List.of("$vector"), null); + assertValidate("successAllowVector()", json, expected); } @Test @@ -275,10 +289,8 @@ public void successDenyVector() { ] }"""; - var indexingDesc = deserialize("successDenyVector()", json); - indexingDesc.validate(); - assertThat(indexingDesc.deny()).containsExactly("$vector"); - assertThat(indexingDesc.allow()).isNull(); + var expected = new CreateCollectionCommand.Options.IndexingDesc(null, List.of("$vector")); + assertValidate("successDenyVector()", json, expected); } @Test @@ -294,10 +306,10 @@ public void successAllowValidPaths() { ] }"""; - var indexingDesc = deserialize("successAllowValidPaths()", json); - indexingDesc.validate(); - assertThat(indexingDesc.allow()).containsExactly("name", "address.city", "tags"); - assertThat(indexingDesc.deny()).isNull(); + var expected = + new CreateCollectionCommand.Options.IndexingDesc( + List.of("name", "address.city", "tags"), null); + assertValidate("successAllowValidPaths()", json, expected); } @Test @@ -313,10 +325,10 @@ public void successDenyValidPaths() { ] }"""; - var indexingDesc = deserialize("successDenyValidPaths()", json); - indexingDesc.validate(); - assertThat(indexingDesc.deny()).containsExactly("name", "address.city", "tags"); - assertThat(indexingDesc.allow()).isNull(); + var expected = + new CreateCollectionCommand.Options.IndexingDesc( + null, List.of("name", "address.city", "tags")); + assertValidate("successDenyValidPaths()", json, expected); } private CreateCollectionCommand.Options.IndexingDesc deserialize(String testName, String json) { diff --git a/src/test/java/io/stargate/sgv2/jsonapi/service/operation/collections/CreateCollectionOperationTest.java b/src/test/java/io/stargate/sgv2/jsonapi/service/operation/collections/CreateCollectionOperationTest.java index 20d99733d4..5933c86304 100644 --- a/src/test/java/io/stargate/sgv2/jsonapi/service/operation/collections/CreateCollectionOperationTest.java +++ b/src/test/java/io/stargate/sgv2/jsonapi/service/operation/collections/CreateCollectionOperationTest.java @@ -4,9 +4,7 @@ import static io.stargate.sgv2.jsonapi.util.asserts.DataAPIAsserts.assertThatSchemaException; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.catchThrowable; -import static org.mockito.ArgumentMatchers.any; -import static org.mockito.ArgumentMatchers.argThat; -import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.ArgumentMatchers.*; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; @@ -28,6 +26,7 @@ import io.smallrye.mutiny.helpers.test.UniAssertSubscriber; import io.stargate.sgv2.jsonapi.api.model.command.CommandStatus; import io.stargate.sgv2.jsonapi.api.model.command.impl.CreateCollectionCommand; +import io.stargate.sgv2.jsonapi.api.request.RequestContext; import io.stargate.sgv2.jsonapi.config.DatabaseLimitsConfig; import io.stargate.sgv2.jsonapi.config.constants.TableCommentConstants; import io.stargate.sgv2.jsonapi.exception.APIException; @@ -44,7 +43,10 @@ import io.stargate.sgv2.jsonapi.service.testutil.MockRow; import io.stargate.sgv2.jsonapi.testresource.NoGlobalResourcesTestProfile; import jakarta.inject.Inject; -import java.util.*; +import java.util.ArrayList; +import java.util.HashMap; +import java.util.List; +import java.util.Set; import java.util.concurrent.atomic.AtomicInteger; import java.util.regex.Pattern; import org.junit.jupiter.api.Test; @@ -52,7 +54,9 @@ import org.slf4j.LoggerFactory; /** - * NOTE: Example table comment string: + * Tests for how the {@link CreateCollectionOperation} makes calls to the DB. + * + *

NOTE: Example table comment string: * * 

  *  {
@@ -105,6 +109,21 @@ private SchemaChangeMemento assertOperation(
         testName, operation, expectedCreateTable, expectedCreateIndex, true, false);
   }
 
+  /**
+   * Calls {@link CreateCollectionOperation#execute(RequestContext, QueryExecutor)} and makes
+   * assertions on how it works calling the DB. Main re-usable set of assertions.
+   *
+   * @param testName name of the test, used for logging
+   * @param operation Operation to test.
+   * @param expectedCreateTable How many CQL statements should be sent to the DB with CREATE TABLE
+   * @param expectedCreateIndex How many CQL statements should be sent t the DB with CREATE CUSTOM
+   *     INDEX
+   * @param mockKeyspaceMetadata If true, fake metadata for the keyspace is added to the mock. If
+   *     false, we will add mock data but it is empty.
+   * @param awaitFailure if true, we expect an throwable out of the operation and wait for that.
+   * @return {@link SchemaChangeMemento} which describes all the DB schema change calls the
+   *     operation made
+   */
   private SchemaChangeMemento assertOperation(
       String testName,
       CreateCollectionOperation operation,
@@ -112,6 +131,7 @@ private SchemaChangeMemento assertOperation(
       int expectedCreateIndex,
       boolean mockKeyspaceMetadata,
       boolean awaitFailure) {
+
     LOGGER.info(
         "assertOperation() - testName={}, expectedCreateTable={}, expectedCreateIndex={}",
         testName,
@@ -480,6 +500,14 @@ private AsyncResultSet mockSchemaSuccessResultSet() {
     return new MockAsyncResultSet(schemaColumns, resultRows, null);
   }
 
+  /**
+   * Holds the calls the operation made against the DB
+   *
+   * @param counter Total number of calls that were intercepted.
+   * @param queries The CQL from the statements, in order the calls were intercepted.
+   * @param tableComments If the CQL had a command field (for create table) the extracted comment,
+   *     see TABLE_COMMENT_PATTERN
+   */
   private record SchemaChangeMemento(
       AtomicInteger counter, List queries, List tableComments) {
     SchemaChangeMemento {
diff --git a/src/test/java/io/stargate/sgv2/jsonapi/service/resolver/CommandResolverTestBase.java b/src/test/java/io/stargate/sgv2/jsonapi/service/resolver/CommandResolverTestBase.java
index 27856bdd44..a4af075bb6 100644
--- a/src/test/java/io/stargate/sgv2/jsonapi/service/resolver/CommandResolverTestBase.java
+++ b/src/test/java/io/stargate/sgv2/jsonapi/service/resolver/CommandResolverTestBase.java
@@ -19,13 +19,17 @@
  * 
Notes for users:
  *
  * 
    
+ *   
  • Godo exmaple of why this exists, see {@link #assertResolver(String, String)}
  *   
  • We still need some CDI injection, so there are abstract properties to access objects
  *       injected after construction on the subclass.
+ *   
  • Avoid adding actual @Test tests to this class, as they will be run for all subclasses. Add
+ *       features that do asserts etc, and then have the @Test test declared on subclasses
  * 

  *
- * @param  the type of the schema under test, this is the type used by the opertion created
+ * @param  the type of the schema under test, this is the type used by the operation created
  * @param  the type of the command under test
- * @param  the type of the resolver under test
+ * @param  the type of the resolver we will pass the command for testing
+ * @param  the type of the operation that should be created by the resolver.
  */
 abstract class CommandResolverTestBase<
     SCHEMA extends SchemaObject,
@@ -37,32 +41,43 @@ abstract class CommandResolverTestBase<
 
   protected final TestConstants TEST_CONSTANTS = new TestConstants();
 
+  // =================================================================
+  // Required implementation for subclasses
+  // =================================================================
+
+  /**
+   * Implementations should return an instance of the resolver for test, may be a new instance every
+   * call or same instance.
+   */
   protected abstract RESOLVER resolver();
 
+  /**
+   * Implementations should return the CommandContext to call the resolver with, this may be
+   * different for each call but recommend it is the same.
+   */
   protected abstract CommandContext commandContext();
 
+  /** Class of {@link COMMAND}, the JSON of the command is deseralised into this class. */
   protected abstract Class commandClass();
 
-  protected Throwable assertResolverThrows(String testName, String rawJson) {
-    var throwable = catchThrowable(() -> assertResolver(testName, rawJson));
-
-    assertThat(throwable).as("%s - exception is thrown", testName).isNotNull();
-
-    return throwable;
-  }
+  // =================================================================
+  // Useful assertions for subclasses to use.
+  // =================================================================
 
   /**
-   * Run the command through the resolver and do some basic tests on the operation that comes out.
+   * Resolves the command described in the rawJson and does basic assertions on the operaton
+   * returned. Most subclasses testing a resolve need this.
    *
    * @param testName name for logging etc
    * @param rawJson the raw command JSON to parse into a command, {@link
-   *     TestConstants#rawNamesSubstitutor()} is used to replace names
+   *     TestConstants#rawNamesSubstitutor()} is used to replace names such as ${collection}
+   *     
    * @return The operation the resolver created.
    */
   protected OPERATION assertResolver(String testName, String rawJson) {
 
     var json = TEST_CONSTANTS.subsRawNames(rawJson);
-    LOGGER.info("assertResolver() - testName: {}, json: {}", testName, json);
+    LOGGER.info("assertResolver() - testName: {}, (substituted) json: {}", testName, json);
 
     COMMAND command;
     try {
@@ -84,4 +99,17 @@ protected OPERATION assertResolver(String testName, String rawJson) {
             });
     return (OPERATION) operation;
   }
+
+  /**
+   * Runs the command through the resolver, and assumes it will throw an exception.
+   *
+   * 
See {@link #assertResolver(String, String)}
+   */
+  protected Throwable assertResolverThrows(String testName, String rawJson) {
+    var throwable = catchThrowable(() -> assertResolver(testName, rawJson));
+
+    assertThat(throwable).as("%s - exception is thrown", testName).isNotNull();
+
+    return throwable;
+  }
 }
diff --git a/src/test/java/io/stargate/sgv2/jsonapi/service/resolver/CreateCollectionCommandResolverTestBase.java b/src/test/java/io/stargate/sgv2/jsonapi/service/resolver/CreateCollectionCommandResolverTestBase.java
index 1c30f98121..f05259a895 100644
--- a/src/test/java/io/stargate/sgv2/jsonapi/service/resolver/CreateCollectionCommandResolverTestBase.java
+++ b/src/test/java/io/stargate/sgv2/jsonapi/service/resolver/CreateCollectionCommandResolverTestBase.java
@@ -9,8 +9,16 @@
 import io.stargate.sgv2.jsonapi.service.schema.KeyspaceSchemaObject;
 import jakarta.inject.Inject;
 
-/** re-usable base class for tests for {@link CreateCollectionCommandResolver}. */
-class CreateCollectionCommandResolverTestBase
+/**
+ * Re-usable base class for tests for {@link CreateCollectionCommandResolver}.
+ *
+ * 
Use {@link #assertResolver(String, String, CqlIdentifier)} in this class because it has more
+ * checks about creating a collection
+ *
+ * 
Needed because we have tests that run with different profiles, such as with vectorize enabled
+ * or disabled, and this puts re-usable code in one place.
+ */
+abstract class CreateCollectionCommandResolverTestBase
     extends CommandResolverTestBase<
         KeyspaceSchemaObject,
         CreateCollectionCommand,
@@ -18,6 +26,7 @@ class CreateCollectionCommandResolverTestBase
         CreateCollectionOperation> {
 
   @Inject CreateCollectionCommandResolver RESOLVER;
+
   protected final CommandContext COMMAND_CONTEXT =
       TEST_CONSTANTS.keyspaceContext();
 
diff --git a/src/test/java/io/stargate/sgv2/jsonapi/service/resolver/CreateCollectionCommandResolverVectorizeEnabledTest.java b/src/test/java/io/stargate/sgv2/jsonapi/service/resolver/CreateCollectionCommandResolverVectorizeEnabledTest.java
index 8a5146030c..015a88e27d 100644
--- a/src/test/java/io/stargate/sgv2/jsonapi/service/resolver/CreateCollectionCommandResolverVectorizeEnabledTest.java
+++ b/src/test/java/io/stargate/sgv2/jsonapi/service/resolver/CreateCollectionCommandResolverVectorizeEnabledTest.java
@@ -10,8 +10,6 @@
 import io.stargate.sgv2.jsonapi.util.profiles.EnabledVectorizeProfile;
 import java.util.Map;
 import org.junit.jupiter.api.Test;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
 
 /**
  * Tests how the {@link CreateCollectionCommandResolver} handles inputs and the operation it
@@ -25,9 +23,6 @@
 class CreateCollectionCommandResolverVectorizeEnabledTest
     extends CreateCollectionCommandResolverTestBase {
 
-  protected static final Logger LOGGER =
-      LoggerFactory.getLogger(CreateCollectionCommandResolverVectorizeEnabledTest.class);
-
   @Test
   public void successWithVectorize() {
     var operation =
diff --git a/src/test/java/io/stargate/sgv2/jsonapi/service/resolver/FindEmbeddingProvidersResolverVectorizeDisabledTest.java b/src/test/java/io/stargate/sgv2/jsonapi/service/resolver/FindEmbeddingProvidersResolverVectorizeDisabledTest.java
index 6a6d481f01..d785a89937 100644
--- a/src/test/java/io/stargate/sgv2/jsonapi/service/resolver/FindEmbeddingProvidersResolverVectorizeDisabledTest.java
+++ b/src/test/java/io/stargate/sgv2/jsonapi/service/resolver/FindEmbeddingProvidersResolverVectorizeDisabledTest.java
@@ -13,6 +13,13 @@
 import jakarta.inject.Inject;
 import org.junit.jupiter.api.Test;
 
+/**
+ * Integration tests for
+ *
+ * 
findEmbeddingProviders

+ *
+ * when vectorize is disabled.
+ */
 @QuarkusTest
 @TestProfile(DisableVectorizeProfile.class)
 public class FindEmbeddingProvidersResolverVectorizeDisabledTest
diff --git a/src/test/java/io/stargate/sgv2/jsonapi/util/asserts/APIExceptionAssert.java b/src/test/java/io/stargate/sgv2/jsonapi/util/asserts/APIExceptionAssert.java
index 464f52ba80..a8f35f2bd9 100644
--- a/src/test/java/io/stargate/sgv2/jsonapi/util/asserts/APIExceptionAssert.java
+++ b/src/test/java/io/stargate/sgv2/jsonapi/util/asserts/APIExceptionAssert.java
@@ -1,5 +1,6 @@
 package io.stargate.sgv2.jsonapi.util.asserts;
 
+import static io.stargate.sgv2.jsonapi.util.ClassUtils.classSimpleName;
 import static org.assertj.core.api.AssertionsForClassTypes.assertThat;
 
 import io.stargate.sgv2.jsonapi.exception.APIException;
@@ -7,34 +8,98 @@
 import java.util.function.Function;
 import org.assertj.core.api.AbstractAssert;
 
+/**
+ * Base for Assert classes that are designed to assert against {@link APIException} sub-classes.
+ * Most of the work is done in this class, the subclass just brings the type declarations.
+ *
+ * 
For example, from {@link SchemaExceptionAssert}, the subclass sets the types, and declares a
+ * factory:
+ *
+ * 
+ *   public class SchemaExceptionAssert
+ *     extends APIExceptionAssert {
+ *
+ *   private SchemaExceptionAssert(SchemaException actual) {
+ *     super(actual, SchemaExceptionAssert.class);
+ *   }
+ *
+ *   protected static SchemaExceptionAssert assertThatSchemaException(SchemaException schemaException) {
+ *     return assertThatAPIException(
+ *         SchemaExceptionAssert::new, SchemaException.class, schemaException);
+ *   }
+ *   protected static SchemaExceptionAssert assertThatSchemaException(Throwable throwable) {
+ *     return assertThatAPIException(SchemaExceptionAssert::new, SchemaException.class, throwable);
+ *   }
+ *   ...
+ * 

+ *
+ * See {@link #assertThatAPIException(Function, Class, Throwable)}
+ *
+ * @param  type of the subclass, used for fluent API
+ * @param  Type of the API Exception class, e.g. {@link
+ *     io.stargate.sgv2.jsonapi.exception.SchemaException}
+ */
 public abstract class APIExceptionAssert<
         SELF extends APIExceptionAssert, ERROR extends APIException>
     extends AbstractAssert {
 
+  /**
+   * Instantiate with values from the subclass.
+   *
+   * @param actual The actual error instance the assertions will run against.
+   * @param selfClazz Class for {@link SELF}
+   */
   protected APIExceptionAssert(ERROR actual, Class selfClazz) {
     super(actual, selfClazz);
   }
 
+  /**
+   * Factory that subclasses can reuse for all logic to create a new instance, checks that actual
+   * throwable is of the expected class.
+   *
+   * @param assertCtor Constructor for the subclass, called to create an instance after checking
+   * @param errorClass Class for {@link ERROR} that throwable must match.
+   * @param throwable Throwable we want to asset, must be instance of {@link ERROR}
+   * @return New assert sub class.
+   * @param  Type of subclass, same as for the class.
+   * @param  type of the APIException, same as for class.
+   */
   protected static , ERROR extends APIException>
       SELF assertThatAPIException(
           Function assertCtor, Class errorClass, Throwable throwable) {
 
     assertThat(throwable)
-        .as("Throwable is instance of " + errorClass.getSimpleName() + "")
+        .as("Throwable is instance of: " + classSimpleName(errorClass))
         .isInstanceOf(errorClass);
+
     return assertCtor.apply(errorClass.cast(throwable));
   }
 
+  /**
+   * Asserts that {@link APIException#code} matches the supplied {@link ErrorCode}
+   *
+   * @param expected Expected error code.
+   * @return Self
+   */
   public SELF hasCode(ErrorCode expected) {
     isNotNull();
-    assertThat(actual.code).isEqualTo(expected.name());
+    assertThat(actual.code).as("APIException.code() is expected").isEqualTo(expected.name());
     return myself;
   }
 
+  /**
+   * Asserts that the {@link APIException#getMessage()} string contains each of the provided message
+   * snippets.
+   *
+   * @param snippets vararg strings to check for
+   * @return Self
+   */
   public SELF hasMessageSnippets(String... snippets) {
     isNotNull();
     for (String snippet : snippets) {
-      assertThat(actual.getMessage()).contains(snippet);
+      assertThat(actual.getMessage())
+          .as("APIException.message() contains expected message snippet")
+          .contains(snippet);
     }
     return myself;
   }
diff --git a/src/test/java/io/stargate/sgv2/jsonapi/util/asserts/CommandResultAssert.java b/src/test/java/io/stargate/sgv2/jsonapi/util/asserts/CommandResultAssert.java
index b616e44016..e52b1d8507 100644
--- a/src/test/java/io/stargate/sgv2/jsonapi/util/asserts/CommandResultAssert.java
+++ b/src/test/java/io/stargate/sgv2/jsonapi/util/asserts/CommandResultAssert.java
@@ -12,7 +12,8 @@
  * Assertions for matching the {@link CommandResult}
  *
  * 
NOTE: there is some logic duplication with {@link ResponseAssertions} which is used for
- * integration tests, it would be good to have some underlying logic but not a big problem
+ * integration tests which tie in with RestEasy. It would be good to have some underlying logic but
+ * that can be future refactoring
  */
 public class CommandResultAssert extends AbstractAssert {
 
diff --git a/src/test/java/io/stargate/sgv2/jsonapi/util/asserts/DataAPIAsserts.java b/src/test/java/io/stargate/sgv2/jsonapi/util/asserts/DataAPIAsserts.java
index ebfde08b4e..df43ae29f3 100644
--- a/src/test/java/io/stargate/sgv2/jsonapi/util/asserts/DataAPIAsserts.java
+++ b/src/test/java/io/stargate/sgv2/jsonapi/util/asserts/DataAPIAsserts.java
@@ -3,6 +3,13 @@
 import io.stargate.sgv2.jsonapi.api.model.command.CommandResult;
 import io.stargate.sgv2.jsonapi.exception.SchemaException;
 
+/**
+ * Central place to export all new assertions added for the Data API, e.g. same as {@link
+ * org.assertj.core.api.Assertions}.
+ *
+ * 
Assertion helpers in this package should write package-protected factories in their class, and
+ * then add them here as public.
+ */
 public class DataAPIAsserts {
 
   private DataAPIAsserts() {}
diff --git a/src/test/java/io/stargate/sgv2/jsonapi/util/asserts/SchemaExceptionAssert.java b/src/test/java/io/stargate/sgv2/jsonapi/util/asserts/SchemaExceptionAssert.java
index a11e828c83..41882fcdc9 100644
--- a/src/test/java/io/stargate/sgv2/jsonapi/util/asserts/SchemaExceptionAssert.java
+++ b/src/test/java/io/stargate/sgv2/jsonapi/util/asserts/SchemaExceptionAssert.java
@@ -2,6 +2,7 @@
 
 import io.stargate.sgv2.jsonapi.exception.SchemaException;
 
+/** See {@link APIExceptionAssert} and {@link DataAPIAsserts} */
 public class SchemaExceptionAssert
     extends APIExceptionAssert {
 
@@ -11,11 +12,13 @@ private SchemaExceptionAssert(SchemaException actual) {
 
   protected static SchemaExceptionAssert assertThatSchemaException(
       SchemaException schemaException) {
+
     return assertThatAPIException(
         SchemaExceptionAssert::new, SchemaException.class, schemaException);
   }
 
   protected static SchemaExceptionAssert assertThatSchemaException(Throwable throwable) {
+
     return assertThatAPIException(SchemaExceptionAssert::new, SchemaException.class, throwable);
   }
 }
diff --git a/src/test/java/io/stargate/sgv2/jsonapi/util/profiles/DisableVectorizeProfile.java b/src/test/java/io/stargate/sgv2/jsonapi/util/profiles/DisableVectorizeProfile.java
index 089362743b..46318decba 100644
--- a/src/test/java/io/stargate/sgv2/jsonapi/util/profiles/DisableVectorizeProfile.java
+++ b/src/test/java/io/stargate/sgv2/jsonapi/util/profiles/DisableVectorizeProfile.java
@@ -4,7 +4,11 @@
 import io.quarkus.test.junit.QuarkusTestProfile;
 import java.util.Map;
 
+/**
+ * Disables vectorize via operations config, while also disables spinning up DB as a test resource.
+ */
 public class DisableVectorizeProfile implements QuarkusTestProfile {
+
   @Override
   public boolean disableGlobalTestResources() {
     return true;
diff --git a/src/test/java/io/stargate/sgv2/jsonapi/util/profiles/EnabledVectorizeProfile.java b/src/test/java/io/stargate/sgv2/jsonapi/util/profiles/EnabledVectorizeProfile.java
index 5c5b8325bd..6e2808e9c9 100644
--- a/src/test/java/io/stargate/sgv2/jsonapi/util/profiles/EnabledVectorizeProfile.java
+++ b/src/test/java/io/stargate/sgv2/jsonapi/util/profiles/EnabledVectorizeProfile.java
@@ -4,7 +4,11 @@
 import io.quarkus.test.junit.QuarkusTestProfile;
 import java.util.Map;
 
+/**
+ * Enables vectorize via operations config, while also disables spinning up DB as a test resource.
+ */
 public class EnabledVectorizeProfile implements QuarkusTestProfile {
+
   @Override
   public boolean disableGlobalTestResources() {
     return true;