Skip to content

Commit

Permalink
Fix failing tests.
Browse files Browse the repository at this point in the history
This commit fixes failing unit and integration tests.

It cherry picks multiple commits from the ajm/tables branch below.

-> SKIPPED - did not cherry pick the original see below <-
commit 0b53fba
Author: Tatu Saloranta <tatu.saloranta@datastax.com>
Date:   Tue Jul 9 17:34:14 2024 -0700

    Revert UT->IT cherry-picked merge

commit 951a092
Author: Tatu Saloranta <tatu.saloranta@datastax.com>
Date:   Tue Jul 9 17:32:20 2024 -0700

    Minor clean up of test contexts, javadocs

commit 28c9ebb
Author: Tatu Saloranta <tatu.saloranta@datastax.com>
Date:   Tue Jul 9 17:21:20 2024 -0700

    Minor clean up

commit 9801566
Author: Tatu Saloranta <tatu.saloranta@datastax.com>
Date:   Tue Jul 9 17:09:18 2024 -0700

    Fix import stmt

-> merge commit - not cherry picked <-
commit b14dfa2
Merge: 13678a5 a9cb436
Author: Tatu Saloranta <tatu.saloranta@datastax.com>
Date:   Tue Jul 9 16:49:10 2024 -0700

    Merge branch 'main' into ajm/tables

-> came from merge, not cherry picked <-
commit a9cb436
Author: Tatu Saloranta <tatu.saloranta@datastax.com>
Date:   Tue Jul 9 16:47:00 2024 -0700

    Update Quarkus Data API uses to 3.9.5 (from parent 3.6.x) (#1246)

-> merge commit - not cherry picked <-
commit 13678a5
Merge: c7eccc0 dd42485
Author: Tatu Saloranta <tatu.saloranta@datastax.com>
Date:   Tue Jul 9 12:29:49 2024 -0700

    Merge branch 'main' into ajm/tables

-> came from merge, not cherry picked <-
commit dd42485
Author: Tatu Saloranta <tatu.saloranta@datastax.com>
Date:   Tue Jul 9 11:40:21 2024 -0700

    Fix a problem that would prevent Quarkus 3.12 upgrade (due to change in Smallrye lib) (#1248)

-> came from merge, not cherry picked <-
commit 15cd3f6
Author: YuqiDu <istimdu@gmail.com>
Date:   Tue Jul 9 10:31:03 2024 -0700

    fix http port to 8181, refactor application.yaml as alphabetic order (#1247)

-> came from merge, not cherry picked <-
commit ce2b337
Author: YuqiDu <istimdu@gmail.com>
Date:   Tue Jul 9 09:19:11 2024 -0700

    Bridge-Removal - Detach the quarkus-common-module dependency (OFF WE GO!) (#1191)

    Co-authored-by: Tatu Saloranta <tatu.saloranta@datastax.com>

commit c7eccc0
Author: Aaron Morton <aaron.morton@datastax.com>
Date:   Tue Jul 9 19:31:31 2024 +1200

    fmt fix

commit 64930f5
Author: Aaron Morton <aaron.morton@datastax.com>
Date:   Tue Jul 9 19:28:42 2024 +1200

    Unit test fixes for Namespace commands

-> merge commit - not cherry picked <-
commit e7a6d15
Merge: baa6ea5 5796eca
Author: Aaron Morton <aaron.morton@datastax.com>
Date:   Tue Jul 9 19:18:55 2024 +1200

    Merge branch 'main' into ajm/tables

commit baa6ea5
Author: Aaron Morton <aaron.morton@datastax.com>
Date:   Tue Jul 9 19:14:03 2024 +1200

    Fixes for Namespace Commands

    so they work on DatabaseSchemaObject

commit cfbbc8c
Author: Aaron Morton <aaron.morton@datastax.com>
Date:   Tue Jul 9 14:57:20 2024 +1200

    fix CreateCollectionResolverVectorizeDisabledTest

-> SKIPPED - later reverted , broke IT's <-
commit c50d7fd
Author: Tatu Saloranta <tatu.saloranta@datastax.com>
Date:   Mon Jul 8 19:30:57 2024 -0700

    Fixes #1245: change 3 unit tests to integration tests to speed up unit test runtime

    git cherry-pick 1996697

-> came from merge, not cherry picked <-
commit 5796eca
Author: Tatu Saloranta <tatu.saloranta@datastax.com>
Date:   Mon Jul 8 19:08:23 2024 -0700

    Refactor unit test profiles, try to speed things up (#1244)

-> came from merge, not cherry picked <-
commit 30c7ae0
Author: Tatu Saloranta <tatu.saloranta@datastax.com>
Date:   Mon Jul 8 18:36:41 2024 -0700

    Fix accidental dependencies to shaded packages (#1243)

-> merge commit, not cherry picked <-
commit 18a804b
Merge: 7ee94cd 35ecb00
Author: Tatu Saloranta <tatu.saloranta@datastax.com>
Date:   Mon Jul 8 18:33:51 2024 -0700

    Merge branch 'main' into ajm/tables

commit 7ee94cd
Author: Aaron Morton <aaron.morton@datastax.com>
Date:   Tue Jul 9 13:13:47 2024 +1200

    fix CreateCollectionResolverVectorDisabledTest

commit b12c484
Author: Aaron Morton <aaron.morton@datastax.com>
Date:   Tue Jul 9 12:45:23 2024 +1200

    Fix for failing Collection based tests

-> came from merge, not cherry picked <-
commit 35ecb00
Author: YuqiDu <istimdu@gmail.com>
Date:   Mon Jul 8 17:32:05 2024 -0700

    Bridge-Removal - miscellaneous cleanups (#1149)

    Co-authored-by: Tatu Saloranta <tatu.saloranta@datastax.com>

commit 4681ccd
Author: Aaron Morton <aaron.morton@datastax.com>
Date:   Tue Jul 9 11:52:31 2024 +1200

    fix for createCollection

    and other commands that run against the namespace

    FindIntegrationTest works, waiting to find out about others
  • Loading branch information
amorton committed Jul 16, 2024
1 parent cab2c63 commit 680a795
Show file tree
Hide file tree
Showing 53 changed files with 283 additions and 146 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,7 @@

import com.google.common.base.Preconditions;
import io.stargate.sgv2.jsonapi.api.v1.metrics.JsonProcessingMetricsReporter;
import io.stargate.sgv2.jsonapi.service.cqldriver.executor.CollectionSchemaObject;
import io.stargate.sgv2.jsonapi.service.cqldriver.executor.KeyspaceSchemaObject;
import io.stargate.sgv2.jsonapi.service.cqldriver.executor.SchemaObject;
import io.stargate.sgv2.jsonapi.service.cqldriver.executor.TableSchemaObject;
import io.stargate.sgv2.jsonapi.service.cqldriver.executor.*;
import io.stargate.sgv2.jsonapi.service.embedding.operation.EmbeddingProvider;

/**
Expand All @@ -25,12 +22,6 @@ public record CommandContext<T extends SchemaObject>(
// private static final CommandContext EMPTY =
// new CommandContext(null, null, CollectionSettings.empty(), null, "testCommand", null);

public static final CommandContext<CollectionSchemaObject> EMPTY_COLLECTION =
new CommandContext<>(CollectionSchemaObject.MISSING, null, "testCommand", null);

public static final CommandContext<TableSchemaObject> EMPTY_TABLE =
new CommandContext<>(TableSchemaObject.MISSING, null, "testCommand", null);

/**
* Factory method to create a new instance of {@link CommandContext} based on the schema object we
* are working with.
Expand Down Expand Up @@ -65,6 +56,10 @@ public static <T extends SchemaObject> CommandContext<T> forSchemaObject(
return (CommandContext<T>)
forSchemaObject(kso, embeddingProvider, commandName, jsonProcessingMetricsReporter);
}
if (schemaObject instanceof DatabaseSchemaObject dso) {
return (CommandContext<T>)
forSchemaObject(dso, embeddingProvider, commandName, jsonProcessingMetricsReporter);
}
throw new IllegalArgumentException("Unknown schema object type: " + schemaObject.getClass());
}

Expand Down Expand Up @@ -125,34 +120,55 @@ public static CommandContext<KeyspaceSchemaObject> forSchemaObject(
schemaObject, embeddingProvider, commandName, jsonProcessingMetricsReporter);
}

/**
* Factory method to create a new instance of {@link CommandContext} based on the schema object we
* are working with
*
* @param schemaObject
* @param embeddingProvider
* @param commandName
* @param jsonProcessingMetricsReporter
* @return
*/
public static CommandContext<DatabaseSchemaObject> forSchemaObject(
DatabaseSchemaObject schemaObject,
EmbeddingProvider embeddingProvider,
String commandName,
JsonProcessingMetricsReporter jsonProcessingMetricsReporter) {
return new CommandContext<>(
schemaObject, embeddingProvider, commandName, jsonProcessingMetricsReporter);
}

@SuppressWarnings("unchecked")
public CommandContext<CollectionSchemaObject> asCollectionContext() {
Preconditions.checkArgument(
schemaObject().type == CollectionSchemaObject.TYPE,
"SchemaObject type is actual was %s expected was %s ",
schemaObject().type,
CollectionSchemaObject.TYPE);
checkSchemaObjectType(CollectionSchemaObject.TYPE);
return (CommandContext<CollectionSchemaObject>) this;
}

@SuppressWarnings("unchecked")
public CommandContext<TableSchemaObject> asTableContext() {
Preconditions.checkArgument(
schemaObject().type == TableSchemaObject.TYPE,
"SchemaObject type is actual was %s expected was %s ",
schemaObject().type,
TableSchemaObject.TYPE);
checkSchemaObjectType(TableSchemaObject.TYPE);
return (CommandContext<TableSchemaObject>) this;
}

@SuppressWarnings("unchecked")
public CommandContext<KeyspaceSchemaObject> asKeyspaceContext() {
checkSchemaObjectType(KeyspaceSchemaObject.TYPE);
return (CommandContext<KeyspaceSchemaObject>) this;
}

@SuppressWarnings("unchecked")
public CommandContext<DatabaseSchemaObject> asDatabaseContext() {
checkSchemaObjectType(DatabaseSchemaObject.TYPE);
return (CommandContext<DatabaseSchemaObject>) this;
}

private void checkSchemaObjectType(SchemaObject.SchemaObjectType expectedType) {
Preconditions.checkArgument(
schemaObject().type == KeyspaceSchemaObject.TYPE,
"SchemaObject type is actual was %s expected was %s ",
schemaObject().type == expectedType,
"SchemaObject type actual was %s expected was %s ",
schemaObject().type,
KeyspaceSchemaObject.TYPE);
return (CommandContext<KeyspaceSchemaObject>) this;
expectedType);
}

// TODO: why do we have these public ctors, and a static factor, and this is a record ??
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
import io.stargate.sgv2.jsonapi.api.model.command.impl.CreateNamespaceCommand;
import io.stargate.sgv2.jsonapi.api.request.DataApiRequestInfo;
import io.stargate.sgv2.jsonapi.config.constants.OpenApiConstants;
import io.stargate.sgv2.jsonapi.service.cqldriver.executor.DatabaseSchemaObject;
import io.stargate.sgv2.jsonapi.service.processor.MeteredCommandProcessor;
import jakarta.inject.Inject;
import jakarta.validation.Valid;
Expand Down Expand Up @@ -45,6 +46,7 @@ public GeneralResource(MeteredCommandProcessor meteredCommandProcessor) {
this.meteredCommandProcessor = meteredCommandProcessor;
}

// TODO: add example for findEmbeddingProviders
@Operation(summary = "Execute command", description = "Executes a single general command.")
@RequestBody(
content =
Expand Down Expand Up @@ -73,9 +75,13 @@ public GeneralResource(MeteredCommandProcessor meteredCommandProcessor) {
})))
@POST
public Uni<RestResponse<CommandResult>> postCommand(@NotNull @Valid GeneralCommand command) {
// call processor

var commandContext =
CommandContext.forSchemaObject(
new DatabaseSchemaObject(), null, command.getClass().getSimpleName(), null);

return meteredCommandProcessor
.processCommand(dataApiRequestInfo, CommandContext.EMPTY_COLLECTION, command)
.processCommand(dataApiRequestInfo, commandContext, command)
// map to 2xx unless overridden by error
.map(commandResult -> commandResult.map());
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ public Uni<RestResponse<CommandResult>> postCommand(
String namespace) {

// create context
// TODO: Aaron , left here to see what CTOR was used, there was a lot of different onces.
// TODO: Aaron , left here to see what CTOR was used, there was a lot of different ones.
// CommandContext commandContext = new CommandContext(namespace, null);
// HACK TODO: The above did not set a command name on the command context, how did that work ?
CommandContext<KeyspaceSchemaObject> commandContext =
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
package io.stargate.sgv2.jsonapi.service.cqldriver.executor;

public class DatabaseSchemaObject extends SchemaObject {

public static final SchemaObjectType TYPE = SchemaObjectType.DATABASE;

public DatabaseSchemaObject() {
super(TYPE, SchemaObjectName.MISSING);
}

@Override
public VectorConfig vectorConfig() {
return VectorConfig.notEnabledVectorConfig();
}

@Override
public IndexUsage newIndexUsage() {
return IndexUsage.NO_OP;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,10 @@ public void merge(IndexUsage indexUsage) {
Tags getTags();

/**
* This method is used to merge the index usage of two different types for filters used in a query
* Method used to merge the index usage of two different types for filters used in a query; tags
* from {@code otherIndexUsage} are merged into {@code this} instance.
*
* @param indexUsage
* @param otherIndexUsage the other index usage to merge into this instance
*/
void merge(IndexUsage indexUsage);
void merge(IndexUsage otherIndexUsage);
}
Original file line number Diff line number Diff line change
Expand Up @@ -9,13 +9,33 @@ public class KeyspaceSchemaObject extends SchemaObject {
new KeyspaceSchemaObject(SchemaObjectName.MISSING);

public KeyspaceSchemaObject(String keyspace) {
this(new SchemaObjectName(keyspace, SchemaObjectName.MISSING_NAME));
this(newObjectName(keyspace));
}

public KeyspaceSchemaObject(SchemaObjectName name) {
super(TYPE, name);
}

/**
* Construct a {@link KeyspaceSchemaObject} that represents the keyspace the collection is in.
*
* @param collection
* @return
*/
public static KeyspaceSchemaObject fromSchemaObject(CollectionSchemaObject collection) {
return new KeyspaceSchemaObject(newObjectName(collection.name.keyspace()));
}

/**
* Construct a {@link KeyspaceSchemaObject} that represents the keyspace the collection is in.
*
* @param table
* @return
*/
public static KeyspaceSchemaObject fromSchemaObject(TableSchemaObject table) {
return new KeyspaceSchemaObject(newObjectName(table.name.keyspace()));
}

@Override
public VectorConfig vectorConfig() {
return VectorConfig.notEnabledVectorConfig();
Expand All @@ -25,4 +45,15 @@ public VectorConfig vectorConfig() {
public IndexUsage newIndexUsage() {
return IndexUsage.NO_OP;
}

/**
* Centralised creation of the name for a Keyspace so we always use the correct marker object for
* collection name
*
* @param keyspaceName
* @return
*/
private static SchemaObjectName newObjectName(String keyspaceName) {
return new SchemaObjectName(keyspaceName, SchemaObjectName.MISSING_NAME);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ public enum SchemaObjectType {
TABLE,
COLLECTION,
KEYSPACE,
DATABASE
}

public final SchemaObjectType type;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,8 @@ public record SchemaObjectName(String keyspace, String table) {

@SuppressWarnings("StringEquality")
public SchemaObjectName(String keyspace, String table) {
// Check using reference equality for the missing value marker object, so we only allow emptry
// string if that
// object is used.
// Check using reference equality for the missing value marker object, so we only allow empty
// string if that object is used.
Preconditions.checkArgument(
(MISSING_NAME == keyspace) || !Strings.isNullOrEmpty(keyspace), "keyspace cannot be null");
Preconditions.checkArgument(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,7 @@ default Uni<FindResponse> findDocument(
Row row = rowIterator.next();
ReadDocument document = null;
try {
// TODO: Use the field name, not the ordinal for the field this is too brittle
JsonNode root = readDocument ? objectMapper.readTree(row.getString(2)) : null;
if (root != null) {
// create metrics
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
import io.stargate.sgv2.jsonapi.exception.JsonApiException;
import io.stargate.sgv2.jsonapi.service.cqldriver.CQLSessionCache;
import io.stargate.sgv2.jsonapi.service.cqldriver.executor.CollectionSchemaObject;
import io.stargate.sgv2.jsonapi.service.cqldriver.executor.KeyspaceSchemaObject;
import io.stargate.sgv2.jsonapi.service.cqldriver.executor.QueryExecutor;
import io.stargate.sgv2.jsonapi.service.operation.model.Operation;
import io.stargate.sgv2.jsonapi.service.schema.model.JsonapiTableMatcher;
Expand All @@ -31,7 +32,7 @@
import org.slf4j.LoggerFactory;

public record CreateCollectionOperation(
CommandContext<CollectionSchemaObject> commandContext,
CommandContext<KeyspaceSchemaObject> commandContext,
DatabaseLimitsConfig dbLimitsConfig,
ObjectMapper objectMapper,
CQLSessionCache cqlSessionCache,
Expand All @@ -51,7 +52,7 @@ public record CreateCollectionOperation(
private static final JsonapiTableMatcher COLLECTION_MATCHER = new JsonapiTableMatcher();

public static CreateCollectionOperation withVectorSearch(
CommandContext<CollectionSchemaObject> commandContext,
CommandContext<KeyspaceSchemaObject> commandContext,
DatabaseLimitsConfig dbLimitsConfig,
ObjectMapper objectMapper,
CQLSessionCache cqlSessionCache,
Expand All @@ -78,7 +79,7 @@ public static CreateCollectionOperation withVectorSearch(
}

public static CreateCollectionOperation withoutVectorSearch(
CommandContext<CollectionSchemaObject> commandContext,
CommandContext<KeyspaceSchemaObject> commandContext,
DatabaseLimitsConfig dbLimitsConfig,
ObjectMapper objectMapper,
CQLSessionCache cqlSessionCache,
Expand Down Expand Up @@ -308,6 +309,7 @@ private Multi<AsyncResultSet> createIndexParallel(

public Uni<JsonApiException> cleanUpCollectionFailedWithTooManyIndex(
DataApiRequestInfo dataApiRequestInfo, QueryExecutor queryExecutor) {

DeleteCollectionOperation deleteCollectionOperation =
new DeleteCollectionOperation(commandContext, name);
return deleteCollectionOperation
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
import io.stargate.sgv2.jsonapi.api.model.command.CommandContext;
import io.stargate.sgv2.jsonapi.api.model.command.CommandResult;
import io.stargate.sgv2.jsonapi.api.request.DataApiRequestInfo;
import io.stargate.sgv2.jsonapi.service.cqldriver.executor.CollectionSchemaObject;
import io.stargate.sgv2.jsonapi.service.cqldriver.executor.KeyspaceSchemaObject;
import io.stargate.sgv2.jsonapi.service.cqldriver.executor.QueryExecutor;
import io.stargate.sgv2.jsonapi.service.operation.model.Operation;
import java.util.function.Supplier;
Expand All @@ -18,7 +18,7 @@
* @param context Command context, carries namespace of the collection.
* @param name Collection name.
*/
public record DeleteCollectionOperation(CommandContext<CollectionSchemaObject> context, String name)
public record DeleteCollectionOperation(CommandContext<KeyspaceSchemaObject> context, String name)
implements Operation {
private static final Logger logger = LoggerFactory.getLogger(DeleteCollectionOperation.class);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
import io.stargate.sgv2.jsonapi.exception.JsonApiException;
import io.stargate.sgv2.jsonapi.service.cqldriver.CQLSessionCache;
import io.stargate.sgv2.jsonapi.service.cqldriver.executor.CollectionSchemaObject;
import io.stargate.sgv2.jsonapi.service.cqldriver.executor.KeyspaceSchemaObject;
import io.stargate.sgv2.jsonapi.service.cqldriver.executor.QueryExecutor;
import io.stargate.sgv2.jsonapi.service.operation.model.Operation;
import io.stargate.sgv2.jsonapi.service.schema.model.JsonapiTableMatcher;
Expand All @@ -35,17 +36,19 @@ public record FindCollectionsOperation(
ObjectMapper objectMapper,
CQLSessionCache cqlSessionCache,
JsonapiTableMatcher tableMatcher,
CommandContext<CollectionSchemaObject> commandContext)
CommandContext<KeyspaceSchemaObject> commandContext)
implements Operation {

// shared table matcher instance
// TODO: if this is static why does the record that have an instance variable passed by the ctor
// below ?
private static final JsonapiTableMatcher TABLE_MATCHER = new JsonapiTableMatcher();

public FindCollectionsOperation(
boolean explain,
ObjectMapper objectMapper,
CQLSessionCache cqlSessionCache,
CommandContext<CollectionSchemaObject> commandContext) {
CommandContext<KeyspaceSchemaObject> commandContext) {
this(explain, objectMapper, cqlSessionCache, TABLE_MATCHER, commandContext);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -316,6 +316,8 @@ public Uni<Supplier<CommandResult>> execute(
// map the response to result
.map(
docs -> {
// TODO: why is this here and not higher up where it can happen for any command result
// ?
commandContext
.jsonProcessingMetricsReporter()
.reportJsonReadDocsMetrics(commandContext().commandName(), docs.docs().size());
Expand Down
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
package io.stargate.sgv2.jsonapi.service.operation.model.tables;

import com.google.common.base.Preconditions;
import io.stargate.sgv2.jsonapi.api.model.command.CommandContext;
import io.stargate.sgv2.jsonapi.api.model.command.clause.filter.LogicalExpression;
import io.stargate.sgv2.jsonapi.service.cqldriver.executor.CollectionSchemaObject;
import java.util.Objects;

/** For now, a marker class / interface for operations that read data in a table. */
abstract class TableReadOperation extends TableOperation {
Expand All @@ -13,9 +13,8 @@ abstract class TableReadOperation extends TableOperation {

public TableReadOperation(
CommandContext<CollectionSchemaObject> commandContext, LogicalExpression logicalExpression) {
Preconditions.checkNotNull(commandContext, "commandContext cannot be null");
Preconditions.checkNotNull(logicalExpression, "logicalExpression cannot be null");
this.commandContext = commandContext;
this.logicalExpression = logicalExpression;
this.commandContext = Objects.requireNonNull(commandContext, "commandContext cannot be null");
this.logicalExpression =
Objects.requireNonNull(logicalExpression, "logicalExpression cannot be null");
}
}
Loading

0 comments on commit 680a795

Please sign in to comment.