From 975bdc0ef1e79edc2ab0a6732805c57e4eb6f2dc Mon Sep 17 00:00:00 2001 From: Tatu Saloranta Date: Wed, 4 Sep 2024 10:20:11 -0700 Subject: [PATCH 01/19] Fix #1355: add general-purpose Feature flag system --- .../config/feature/DataApiFeatureConfig.java | 10 ++++++++++ .../config/feature/DataApiFeatureFlag.java | 15 +++++++++++++++ src/main/resources/application.yaml | 4 ++++ 3 files changed, 29 insertions(+) create mode 100644 src/main/java/io/stargate/sgv2/jsonapi/config/feature/DataApiFeatureConfig.java create mode 100644 src/main/java/io/stargate/sgv2/jsonapi/config/feature/DataApiFeatureFlag.java diff --git a/src/main/java/io/stargate/sgv2/jsonapi/config/feature/DataApiFeatureConfig.java b/src/main/java/io/stargate/sgv2/jsonapi/config/feature/DataApiFeatureConfig.java new file mode 100644 index 0000000000..8d60dd0812 --- /dev/null +++ b/src/main/java/io/stargate/sgv2/jsonapi/config/feature/DataApiFeatureConfig.java @@ -0,0 +1,10 @@ +package io.stargate.sgv2.jsonapi.config.feature; + +import io.smallrye.config.ConfigMapping; +import java.util.Map; + +/** Configuration mapping for Data API Feature flags. */ +@ConfigMapping(prefix = "stargate.feature") +public interface DataApiFeatureConfig { + Map flags(); +} diff --git a/src/main/java/io/stargate/sgv2/jsonapi/config/feature/DataApiFeatureFlag.java b/src/main/java/io/stargate/sgv2/jsonapi/config/feature/DataApiFeatureFlag.java new file mode 100644 index 0000000000..357d5d4469 --- /dev/null +++ b/src/main/java/io/stargate/sgv2/jsonapi/config/feature/DataApiFeatureFlag.java @@ -0,0 +1,15 @@ +package io.stargate.sgv2.jsonapi.config.feature; + +public enum DataApiFeatureFlag { + TABLES("tables"); + + private final String featureName; + + DataApiFeatureFlag(String featureName) { + this.featureName = featureName; + } + + public String featureName() { + return featureName; + } +} diff --git a/src/main/resources/application.yaml b/src/main/resources/application.yaml index 11f37783b7..d24835d7cc 100644 --- a/src/main/resources/application.yaml +++ b/src/main/resources/application.yaml @@ -25,6 +25,10 @@ stargate: exception-mappers: enabled: false + feature: + flags: + TABLES: null + # custom grpc settings grpc: From 15d73d573a996a43c486ba2d3d765f96f86f1ee2 Mon Sep 17 00:00:00 2001 From: Tatu Saloranta Date: Wed, 4 Sep 2024 11:13:46 -0700 Subject: [PATCH 02/19] Remove Tables-enabled check from NamespaceCache, cannot override on per-call basis --- .../config/feature/DataApiFeatureConfig.java | 13 +++++++++++++ .../cqldriver/executor/NamespaceCache.java | 18 +++--------------- .../cqldriver/executor/SchemaCache.java | 6 +----- .../cqldriver/executor/NamespaceCacheTest.java | 2 +- 4 files changed, 18 insertions(+), 21 deletions(-) diff --git a/src/main/java/io/stargate/sgv2/jsonapi/config/feature/DataApiFeatureConfig.java b/src/main/java/io/stargate/sgv2/jsonapi/config/feature/DataApiFeatureConfig.java index 8d60dd0812..5c19d922ba 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/config/feature/DataApiFeatureConfig.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/config/feature/DataApiFeatureConfig.java @@ -2,9 +2,22 @@ import io.smallrye.config.ConfigMapping; import java.util.Map; +import java.util.Optional; /** Configuration mapping for Data API Feature flags. */ @ConfigMapping(prefix = "stargate.feature") public interface DataApiFeatureConfig { Map flags(); + + default boolean isFeatureEnabled(DataApiFeatureFlag flag) { + return isFeatureEnabled(flag, false); + } + + default boolean isFeatureEnabled(DataApiFeatureFlag flag, boolean defaultValue) { + return Optional.ofNullable(safeFlags().get(flag)).orElse(defaultValue); + } + + default Map safeFlags() { + return (flags() != null) ? flags() : Map.of(); + } } diff --git a/src/main/java/io/stargate/sgv2/jsonapi/service/cqldriver/executor/NamespaceCache.java b/src/main/java/io/stargate/sgv2/jsonapi/service/cqldriver/executor/NamespaceCache.java index d1e3b00478..ca5cfca6d5 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/service/cqldriver/executor/NamespaceCache.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/service/cqldriver/executor/NamespaceCache.java @@ -22,8 +22,6 @@ public class NamespaceCache { private final ObjectMapper objectMapper; - private final boolean apiTablesEnabled; - // TODO: move the settings to config // TODO: set the cache loader when creating the cache private static final long CACHE_TTL_SECONDS = 300; @@ -34,15 +32,10 @@ public class NamespaceCache { .maximumSize(CACHE_MAX_SIZE) .build(); - public NamespaceCache( - String namespace, - boolean apiTablesEnabled, - QueryExecutor queryExecutor, - ObjectMapper objectMapper) { + public NamespaceCache(String namespace, QueryExecutor queryExecutor, ObjectMapper objectMapper) { this.namespace = namespace; this.queryExecutor = queryExecutor; this.objectMapper = objectMapper; - this.apiTablesEnabled = apiTablesEnabled; } protected Uni getSchemaObject( @@ -108,13 +101,8 @@ private Uni loadSchemaObject( optionalTable.get(), objectMapper); } - if (apiTablesEnabled) { - return new TableSchemaObject(table); - } - - // Target is not a collection and we are not supporting tables - throw ErrorCode.INVALID_JSONAPI_COLLECTION_SCHEMA.toApiException( - "%s", collectionName); + // 04-Sep-2024, tatu: Used to check that API Tables enabled; no longer checked here + return new TableSchemaObject(table); }); } diff --git a/src/main/java/io/stargate/sgv2/jsonapi/service/cqldriver/executor/SchemaCache.java b/src/main/java/io/stargate/sgv2/jsonapi/service/cqldriver/executor/SchemaCache.java index b4f602833d..864087a9a8 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/service/cqldriver/executor/SchemaCache.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/service/cqldriver/executor/SchemaCache.java @@ -9,7 +9,6 @@ import com.github.benmanes.caffeine.cache.Caffeine; import io.smallrye.mutiny.Uni; import io.stargate.sgv2.jsonapi.api.request.DataApiRequestInfo; -import io.stargate.sgv2.jsonapi.config.ApiTablesConfig; import io.stargate.sgv2.jsonapi.config.OperationsConfig; import jakarta.enterprise.context.ApplicationScoped; import jakarta.inject.Inject; @@ -26,8 +25,6 @@ public class SchemaCache { @Inject OperationsConfig operationsConfig; - @Inject ApiTablesConfig apiTablesConfig; - // TODO: The size of the cache should be in configuration. // TODO: set the cache loader when creating the cache private final Cache schemaCache = @@ -68,8 +65,7 @@ public void evictCollectionSettingCacheEntry( } private NamespaceCache addNamespaceCache(CacheKey cacheKey) { - return new NamespaceCache( - cacheKey.namespace(), apiTablesConfig.enabled(), queryExecutor, objectMapper); + return new NamespaceCache(cacheKey.namespace(), queryExecutor, objectMapper); } /** diff --git a/src/test/java/io/stargate/sgv2/jsonapi/service/cqldriver/executor/NamespaceCacheTest.java b/src/test/java/io/stargate/sgv2/jsonapi/service/cqldriver/executor/NamespaceCacheTest.java index ad93cfed04..d2ef1f1062 100644 --- a/src/test/java/io/stargate/sgv2/jsonapi/service/cqldriver/executor/NamespaceCacheTest.java +++ b/src/test/java/io/stargate/sgv2/jsonapi/service/cqldriver/executor/NamespaceCacheTest.java @@ -371,6 +371,6 @@ public void checkInvalidJsonApiTable() { } private NamespaceCache createNamespaceCache(QueryExecutor qe) { - return new NamespaceCache("ks", false, qe, objectMapper); + return new NamespaceCache("ks", qe, objectMapper); } } From 267449136a6015b3f6e73706487102efafbda4bc Mon Sep 17 00:00:00 2001 From: Tatu Saloranta Date: Wed, 4 Sep 2024 11:32:30 -0700 Subject: [PATCH 03/19] Fix unit test wrt now passing access to table schema info --- .../executor/NamespaceCacheTest.java | 22 +++++-------------- 1 file changed, 6 insertions(+), 16 deletions(-) diff --git a/src/test/java/io/stargate/sgv2/jsonapi/service/cqldriver/executor/NamespaceCacheTest.java b/src/test/java/io/stargate/sgv2/jsonapi/service/cqldriver/executor/NamespaceCacheTest.java index d2ef1f1062..d7891cb379 100644 --- a/src/test/java/io/stargate/sgv2/jsonapi/service/cqldriver/executor/NamespaceCacheTest.java +++ b/src/test/java/io/stargate/sgv2/jsonapi/service/cqldriver/executor/NamespaceCacheTest.java @@ -18,8 +18,6 @@ import io.smallrye.mutiny.Uni; import io.smallrye.mutiny.helpers.test.UniAssertSubscriber; import io.stargate.sgv2.jsonapi.api.request.DataApiRequestInfo; -import io.stargate.sgv2.jsonapi.exception.ErrorCode; -import io.stargate.sgv2.jsonapi.exception.JsonApiException; import io.stargate.sgv2.jsonapi.testresource.NoGlobalResourcesTestProfile; import jakarta.inject.Inject; import java.util.HashMap; @@ -292,7 +290,7 @@ public void checkValidJsonApiTableWithIndexing() { .awaitItem() .getItem(); - assertThat(schemaObject instanceof CollectionSchemaObject); + assertThat(schemaObject).isInstanceOf(CollectionSchemaObject.class); var collectionSchemaObject = (CollectionSchemaObject) schemaObject; assertThat(collectionSchemaObject) .satisfies( @@ -304,7 +302,7 @@ public void checkValidJsonApiTableWithIndexing() { } @Test - public void checkInvalidJsonApiTable() { + public void checkNonCollectionJsonApiTable() { QueryExecutor queryExecutor = mock(QueryExecutor.class); when(queryExecutor.getSchema(any(), any(), any())) .then( @@ -350,23 +348,15 @@ public void checkInvalidJsonApiTable() { new HashMap<>()))); }); NamespaceCache namespaceCache = createNamespaceCache(queryExecutor); - Throwable error = + var schemaObject = namespaceCache .getSchemaObject(dataApiRequestInfo, "table") .subscribe() .withSubscriber(UniAssertSubscriber.create()) - .awaitFailure() - .getFailure(); + .awaitItem() + .getItem(); - assertThat(error) - .isInstanceOfSatisfying( - JsonApiException.class, - s -> { - assertThat(s.getErrorCode()).isEqualTo(ErrorCode.INVALID_JSONAPI_COLLECTION_SCHEMA); - assertThat(s.getMessage()) - .isEqualTo( - ErrorCode.INVALID_JSONAPI_COLLECTION_SCHEMA.getMessage() + ": table"); - }); + assertThat(schemaObject).isInstanceOf(TableSchemaObject.class); } } From ba9798785cf533d4adc32c66b777b5fdc855035a Mon Sep 17 00:00:00 2001 From: Tatu Saloranta Date: Wed, 4 Sep 2024 13:54:31 -0700 Subject: [PATCH 04/19] Re-wire enable-tables, temporarily --- .../stargate/sgv2/jsonapi/api/v1/NamespaceResource.java | 8 +++++--- .../service/operation/tables/CreateTableOperation.java | 3 ++- src/main/resources/application.yaml | 1 + 3 files changed, 8 insertions(+), 4 deletions(-) diff --git a/src/main/java/io/stargate/sgv2/jsonapi/api/v1/NamespaceResource.java b/src/main/java/io/stargate/sgv2/jsonapi/api/v1/NamespaceResource.java index 28c39a5fae..add710d335 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/api/v1/NamespaceResource.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/api/v1/NamespaceResource.java @@ -9,8 +9,9 @@ import io.stargate.sgv2.jsonapi.api.model.command.impl.DeleteCollectionCommand; import io.stargate.sgv2.jsonapi.api.model.command.impl.FindCollectionsCommand; import io.stargate.sgv2.jsonapi.api.request.DataApiRequestInfo; -import io.stargate.sgv2.jsonapi.config.ApiTablesConfig; import io.stargate.sgv2.jsonapi.config.constants.OpenApiConstants; +import io.stargate.sgv2.jsonapi.config.feature.DataApiFeatureConfig; +import io.stargate.sgv2.jsonapi.config.feature.DataApiFeatureFlag; import io.stargate.sgv2.jsonapi.exception.ErrorCode; import io.stargate.sgv2.jsonapi.exception.mappers.ThrowableCommandResultSupplier; import io.stargate.sgv2.jsonapi.service.cqldriver.executor.KeyspaceSchemaObject; @@ -51,7 +52,7 @@ public class NamespaceResource { @Inject private DataApiRequestInfo dataApiRequestInfo; - @Inject ApiTablesConfig apiTablesConfig; + @Inject DataApiFeatureConfig apiFeatureConfig; @Inject public NamespaceResource(MeteredCommandProcessor meteredCommandProcessor) { @@ -106,7 +107,8 @@ public Uni> postCommand( @Size(min = 1, max = 48) String namespace) { - if (command instanceof TableOnlyCommand && !apiTablesConfig.enabled()) { + if (command instanceof TableOnlyCommand + && !apiFeatureConfig.isFeatureEnabled(DataApiFeatureFlag.TABLES)) { return Uni.createFrom() .item( new ThrowableCommandResultSupplier( diff --git a/src/main/java/io/stargate/sgv2/jsonapi/service/operation/tables/CreateTableOperation.java b/src/main/java/io/stargate/sgv2/jsonapi/service/operation/tables/CreateTableOperation.java index 2136d6ed9c..3d7af004fc 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/service/operation/tables/CreateTableOperation.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/service/operation/tables/CreateTableOperation.java @@ -22,6 +22,7 @@ import java.util.HashSet; import java.util.List; import java.util.Map; +import java.util.Objects; import java.util.Set; import java.util.function.Supplier; import org.slf4j.Logger; @@ -47,7 +48,7 @@ public CreateTableOperation( String comment) { this.commandContext = commandContext; this.tableName = tableName; - this.columnTypes = columnTypes; + this.columnTypes = Objects.requireNonNull(columnTypes, "columnTypes must not be null"); this.partitionKeys = partitionKeys; this.clusteringKeys = clusteringKeys; this.comment = comment; diff --git a/src/main/resources/application.yaml b/src/main/resources/application.yaml index d24835d7cc..1ae0b4258f 100644 --- a/src/main/resources/application.yaml +++ b/src/main/resources/application.yaml @@ -27,6 +27,7 @@ stargate: feature: flags: + # null value means "disabled but may be enabled by request override" TABLES: null # custom grpc settings From 4cd752615d4f7e2667362b852dadd623c13bc972 Mon Sep 17 00:00:00 2001 From: Tatu Saloranta Date: Wed, 4 Sep 2024 14:10:24 -0700 Subject: [PATCH 05/19] Add sysprop for ITs to enable Tables feature --- .../sgv2/jsonapi/api/v1/NamespaceResource.java | 17 +++++++++-------- .../jsonapi/testresource/DseTestResource.java | 6 +++++- 2 files changed, 14 insertions(+), 9 deletions(-) diff --git a/src/main/java/io/stargate/sgv2/jsonapi/api/v1/NamespaceResource.java b/src/main/java/io/stargate/sgv2/jsonapi/api/v1/NamespaceResource.java index add710d335..8e10c4fe24 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/api/v1/NamespaceResource.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/api/v1/NamespaceResource.java @@ -107,6 +107,14 @@ public Uni> postCommand( @Size(min = 1, max = 48) String namespace) { + // create context + // 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 commandContext = + new CommandContext<>(new KeyspaceSchemaObject(namespace), null, "", null); + + // Need context first to check if feature is enabled if (command instanceof TableOnlyCommand && !apiFeatureConfig.isFeatureEnabled(DataApiFeatureFlag.TABLES)) { return Uni.createFrom() @@ -116,14 +124,7 @@ public Uni> postCommand( .map(commandResult -> commandResult.map()); } - // create context - // 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 commandContext = - new CommandContext<>(new KeyspaceSchemaObject(namespace), null, "", null); - - // call processor + // call processor return meteredCommandProcessor .processCommand(dataApiRequestInfo, commandContext, command) // map to 2xx unless overridden by error diff --git a/src/test/java/io/stargate/sgv2/jsonapi/testresource/DseTestResource.java b/src/test/java/io/stargate/sgv2/jsonapi/testresource/DseTestResource.java index 9799f2f24b..f574594556 100644 --- a/src/test/java/io/stargate/sgv2/jsonapi/testresource/DseTestResource.java +++ b/src/test/java/io/stargate/sgv2/jsonapi/testresource/DseTestResource.java @@ -71,7 +71,11 @@ public Map start() { ImmutableMap.Builder propsBuilder = ImmutableMap.builder(); propsBuilder.putAll(env); propsBuilder.put("stargate.jsonapi.custom.embedding.enabled", "true"); - propsBuilder.put("stargate.tables.enabled", "true"); + + // 04-Sep-2024, tatu: [data-api#1335] Enable Tables using new Feature Flag: + // propsBuilder.put("stargate.tables.enabled", "true"); + propsBuilder.put("stargate.feature.flags.TABLES", "true"); + propsBuilder.put( "stargate.jsonapi.custom.embedding.clazz", "io.stargate.sgv2.jsonapi.service.embedding.operation.test.CustomITEmbeddingProvider"); From 68a3cfb977a4397db576ce2697a6ad09cfb48f1b Mon Sep 17 00:00:00 2001 From: Tatu Saloranta Date: Wed, 4 Sep 2024 15:10:36 -0700 Subject: [PATCH 06/19] Incremental progress... --- .../jsonapi/api/v1/CollectionResource.java | 13 ++++++++ .../jsonapi/api/v1/NamespaceResource.java | 5 ++- .../config/feature/DataApiFeatureConfig.java | 5 ++- .../config/feature/DataApiFeatureFlag.java | 4 +++ .../config/feature/DataApiFeatures.java | 33 +++++++++++++++++++ 5 files changed, 58 insertions(+), 2 deletions(-) create mode 100644 src/main/java/io/stargate/sgv2/jsonapi/config/feature/DataApiFeatures.java diff --git a/src/main/java/io/stargate/sgv2/jsonapi/api/v1/CollectionResource.java b/src/main/java/io/stargate/sgv2/jsonapi/api/v1/CollectionResource.java index d87d2baa7a..53b341a2e4 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/api/v1/CollectionResource.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/api/v1/CollectionResource.java @@ -20,9 +20,14 @@ import io.stargate.sgv2.jsonapi.api.request.DataApiRequestInfo; import io.stargate.sgv2.jsonapi.api.v1.metrics.JsonProcessingMetricsReporter; import io.stargate.sgv2.jsonapi.config.constants.OpenApiConstants; +import io.stargate.sgv2.jsonapi.config.feature.DataApiFeatureConfig; +import io.stargate.sgv2.jsonapi.config.feature.DataApiFeatureFlag; +import io.stargate.sgv2.jsonapi.config.feature.DataApiFeatures; +import io.stargate.sgv2.jsonapi.exception.ErrorCode; import io.stargate.sgv2.jsonapi.exception.JsonApiException; import io.stargate.sgv2.jsonapi.exception.mappers.ThrowableCommandResultSupplier; import io.stargate.sgv2.jsonapi.service.cqldriver.executor.SchemaCache; +import io.stargate.sgv2.jsonapi.service.cqldriver.executor.SchemaObject; import io.stargate.sgv2.jsonapi.service.cqldriver.executor.VectorConfig; import io.stargate.sgv2.jsonapi.service.embedding.operation.EmbeddingProvider; import io.stargate.sgv2.jsonapi.service.embedding.operation.EmbeddingProviderFactory; @@ -68,6 +73,8 @@ public class CollectionResource { @Inject private DataApiRequestInfo dataApiRequestInfo; + @Inject DataApiFeatureConfig apiFeatureConfig; + @Inject private JsonProcessingMetricsReporter jsonProcessingMetricsReporter; @Inject @@ -181,6 +188,12 @@ public Uni> postCommand( return Uni.createFrom().item(new ThrowableCommandResultSupplier(error)); } else { // TODO No need for the else clause here, simplify + final DataApiFeatures features = DataApiFeatures.fromConfigOnly(apiFeatureConfig); + if ((schemaObject.type == SchemaObject.SchemaObjectType.TABLE) + && !features.isFeatureEnabled(DataApiFeatureFlag.TABLES)) { + return Uni.createFrom() + .failure(ErrorCode.TABLE_FEATURE_NOT_ENABLED.toApiException()); + } // TODO: refactor this code to be cleaner so it assigns on one line EmbeddingProvider embeddingProvider = null; final VectorConfig.VectorizeConfig vectorizeConfig = diff --git a/src/main/java/io/stargate/sgv2/jsonapi/api/v1/NamespaceResource.java b/src/main/java/io/stargate/sgv2/jsonapi/api/v1/NamespaceResource.java index 8e10c4fe24..372d7b23e4 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/api/v1/NamespaceResource.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/api/v1/NamespaceResource.java @@ -12,6 +12,7 @@ import io.stargate.sgv2.jsonapi.config.constants.OpenApiConstants; import io.stargate.sgv2.jsonapi.config.feature.DataApiFeatureConfig; import io.stargate.sgv2.jsonapi.config.feature.DataApiFeatureFlag; +import io.stargate.sgv2.jsonapi.config.feature.DataApiFeatures; import io.stargate.sgv2.jsonapi.exception.ErrorCode; import io.stargate.sgv2.jsonapi.exception.mappers.ThrowableCommandResultSupplier; import io.stargate.sgv2.jsonapi.service.cqldriver.executor.KeyspaceSchemaObject; @@ -107,6 +108,8 @@ public Uni> postCommand( @Size(min = 1, max = 48) String namespace) { + final DataApiFeatures features = DataApiFeatures.fromConfigOnly(apiFeatureConfig); + // create context // TODO: Aaron , left here to see what CTOR was used, there was a lot of different ones. // CommandContext commandContext = new CommandContext(namespace, null); @@ -116,7 +119,7 @@ public Uni> postCommand( // Need context first to check if feature is enabled if (command instanceof TableOnlyCommand - && !apiFeatureConfig.isFeatureEnabled(DataApiFeatureFlag.TABLES)) { + && !features.isFeatureEnabled(DataApiFeatureFlag.TABLES)) { return Uni.createFrom() .item( new ThrowableCommandResultSupplier( diff --git a/src/main/java/io/stargate/sgv2/jsonapi/config/feature/DataApiFeatureConfig.java b/src/main/java/io/stargate/sgv2/jsonapi/config/feature/DataApiFeatureConfig.java index 5c19d922ba..ff74d2a1d0 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/config/feature/DataApiFeatureConfig.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/config/feature/DataApiFeatureConfig.java @@ -4,7 +4,10 @@ import java.util.Map; import java.util.Optional; -/** Configuration mapping for Data API Feature flags. */ +/** + * Configuration mapping for Data API Feature flags as read from main application configuration + * (with possible properety / sysenv overrides). + */ @ConfigMapping(prefix = "stargate.feature") public interface DataApiFeatureConfig { Map flags(); diff --git a/src/main/java/io/stargate/sgv2/jsonapi/config/feature/DataApiFeatureFlag.java b/src/main/java/io/stargate/sgv2/jsonapi/config/feature/DataApiFeatureFlag.java index 357d5d4469..fb907e3d01 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/config/feature/DataApiFeatureFlag.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/config/feature/DataApiFeatureFlag.java @@ -1,5 +1,9 @@ package io.stargate.sgv2.jsonapi.config.feature; +/** + * Set of "Feature Flags" that can be used to enable/disable certain features in the Data API. + * Enumeration defines the key used to introspect state of feature. + */ public enum DataApiFeatureFlag { TABLES("tables"); diff --git a/src/main/java/io/stargate/sgv2/jsonapi/config/feature/DataApiFeatures.java b/src/main/java/io/stargate/sgv2/jsonapi/config/feature/DataApiFeatures.java new file mode 100644 index 0000000000..343d4cb8fe --- /dev/null +++ b/src/main/java/io/stargate/sgv2/jsonapi/config/feature/DataApiFeatures.java @@ -0,0 +1,33 @@ +package io.stargate.sgv2.jsonapi.config.feature; + +import java.util.Collections; +import java.util.Map; +import java.util.Optional; + +/** + * Accessor for combined state of feature flags; typically based on static configuration (with its + * overrides) and possible per-request settings. + */ +public class DataApiFeatures { + private final Map fromConfig; + + DataApiFeatures(Map fromConfig) { + this.fromConfig = (fromConfig == null) ? Collections.emptyMap() : fromConfig; + } + + public static DataApiFeatures empty() { + return new DataApiFeatures(Collections.emptyMap()); + } + + public static DataApiFeatures fromConfigOnly(DataApiFeatureConfig config) { + return new DataApiFeatures(config.flags()); + } + + public boolean isFeatureEnabled(DataApiFeatureFlag flag) { + return isFeatureEnabled(flag, false); + } + + public boolean isFeatureEnabled(DataApiFeatureFlag flag, boolean defaultValue) { + return Optional.ofNullable(fromConfig.get(flag)).orElse(defaultValue); + } +} From a4b10e11e1dd1d65748ad3ccbd931851a2ad9b4e Mon Sep 17 00:00:00 2001 From: Tatu Saloranta Date: Wed, 4 Sep 2024 15:57:38 -0700 Subject: [PATCH 07/19] Wire DataApiFeatures in CommandContext for better access --- .../api/model/command/CommandContext.java | 24 ++++++++++++------- .../jsonapi/api/v1/NamespaceResource.java | 6 ++--- .../stargate/sgv2/jsonapi/TestConstants.java | 15 ++++++++---- .../operation/TestEmbeddingProvider.java | 3 ++- .../CreateCollectionOperationTest.java | 7 +++++- .../FindCollectionOperationTest.java | 3 ++- .../InsertCollectionOperationTest.java | 3 ++- .../collections/OperationTestBase.java | 20 +++++++++++++--- .../ReadAndUpdateCollectionOperationTest.java | 3 ++- .../CommandResolverWithVectorizerTest.java | 6 +++-- 10 files changed, 64 insertions(+), 26 deletions(-) diff --git a/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/CommandContext.java b/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/CommandContext.java index c410c517e1..2a7bd1db0b 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/CommandContext.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/CommandContext.java @@ -2,6 +2,7 @@ import com.google.common.base.Preconditions; import io.stargate.sgv2.jsonapi.api.v1.metrics.JsonProcessingMetricsReporter; +import io.stargate.sgv2.jsonapi.config.feature.DataApiFeatures; import io.stargate.sgv2.jsonapi.exception.ErrorCode; import io.stargate.sgv2.jsonapi.service.cqldriver.executor.*; import io.stargate.sgv2.jsonapi.service.embedding.operation.EmbeddingProvider; @@ -16,7 +17,8 @@ public record CommandContext( T schemaObject, EmbeddingProvider embeddingProvider, String commandName, - JsonProcessingMetricsReporter jsonProcessingMetricsReporter) { + JsonProcessingMetricsReporter jsonProcessingMetricsReporter, + DataApiFeatures apiFeatures) { // TODO: this is what the original EMPTY had, no idea why the name of the command is missing // this is used by the GeneralResource @@ -79,9 +81,10 @@ public static CommandContext forSchemaObject( CollectionSchemaObject schemaObject, EmbeddingProvider embeddingProvider, String commandName, - JsonProcessingMetricsReporter jsonProcessingMetricsReporter) { + JsonProcessingMetricsReporter jsonProcessingMetricsReporter, + DataApiFeatures apiFeatures) { return new CommandContext<>( - schemaObject, embeddingProvider, commandName, jsonProcessingMetricsReporter); + schemaObject, embeddingProvider, commandName, jsonProcessingMetricsReporter, apiFeatures); } /** @@ -98,9 +101,10 @@ public static CommandContext forSchemaObject( TableSchemaObject schemaObject, EmbeddingProvider embeddingProvider, String commandName, - JsonProcessingMetricsReporter jsonProcessingMetricsReporter) { + JsonProcessingMetricsReporter jsonProcessingMetricsReporter, + DataApiFeatures apiFeatures) { return new CommandContext<>( - schemaObject, embeddingProvider, commandName, jsonProcessingMetricsReporter); + schemaObject, embeddingProvider, commandName, jsonProcessingMetricsReporter, apiFeatures); } /** @@ -117,9 +121,10 @@ public static CommandContext forSchemaObject( KeyspaceSchemaObject schemaObject, EmbeddingProvider embeddingProvider, String commandName, - JsonProcessingMetricsReporter jsonProcessingMetricsReporter) { + JsonProcessingMetricsReporter jsonProcessingMetricsReporter, + DataApiFeatures apiFeatures) { return new CommandContext<>( - schemaObject, embeddingProvider, commandName, jsonProcessingMetricsReporter); + schemaObject, embeddingProvider, commandName, jsonProcessingMetricsReporter, apiFeatures); } /** @@ -136,9 +141,10 @@ public static CommandContext forSchemaObject( DatabaseSchemaObject schemaObject, EmbeddingProvider embeddingProvider, String commandName, - JsonProcessingMetricsReporter jsonProcessingMetricsReporter) { + JsonProcessingMetricsReporter jsonProcessingMetricsReporter, + DataApiFeatures apiFeatures) { return new CommandContext<>( - schemaObject, embeddingProvider, commandName, jsonProcessingMetricsReporter); + schemaObject, embeddingProvider, commandName, jsonProcessingMetricsReporter, apiFeatures); } @SuppressWarnings("unchecked") diff --git a/src/main/java/io/stargate/sgv2/jsonapi/api/v1/NamespaceResource.java b/src/main/java/io/stargate/sgv2/jsonapi/api/v1/NamespaceResource.java index 372d7b23e4..43bf53ce28 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/api/v1/NamespaceResource.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/api/v1/NamespaceResource.java @@ -108,18 +108,18 @@ public Uni> postCommand( @Size(min = 1, max = 48) String namespace) { - final DataApiFeatures features = DataApiFeatures.fromConfigOnly(apiFeatureConfig); + final DataApiFeatures apiFeatures = DataApiFeatures.fromConfigOnly(apiFeatureConfig); // create context // 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 commandContext = - new CommandContext<>(new KeyspaceSchemaObject(namespace), null, "", null); + new CommandContext<>(new KeyspaceSchemaObject(namespace), null, "", null, apiFeatures); // Need context first to check if feature is enabled if (command instanceof TableOnlyCommand - && !features.isFeatureEnabled(DataApiFeatureFlag.TABLES)) { + && !apiFeatures.isFeatureEnabled(DataApiFeatureFlag.TABLES)) { return Uni.createFrom() .item( new ThrowableCommandResultSupplier( diff --git a/src/test/java/io/stargate/sgv2/jsonapi/TestConstants.java b/src/test/java/io/stargate/sgv2/jsonapi/TestConstants.java index 31a5c4f62e..60353e940c 100644 --- a/src/test/java/io/stargate/sgv2/jsonapi/TestConstants.java +++ b/src/test/java/io/stargate/sgv2/jsonapi/TestConstants.java @@ -1,6 +1,7 @@ package io.stargate.sgv2.jsonapi; import io.stargate.sgv2.jsonapi.api.model.command.CommandContext; +import io.stargate.sgv2.jsonapi.config.feature.DataApiFeatures; import io.stargate.sgv2.jsonapi.service.cqldriver.executor.*; import org.apache.commons.lang3.RandomStringUtils; @@ -40,15 +41,21 @@ public final class TestConstants { // CommandContext for working on the schema objects above + public static final DataApiFeatures DEFAULT_API_FEATURES_FOR_TESTS = DataApiFeatures.empty(); + public static final CommandContext COLLECTION_CONTEXT = - new CommandContext<>(COLLECTION_SCHEMA_OBJECT, null, TEST_COMMAND_NAME, null); + new CommandContext<>( + COLLECTION_SCHEMA_OBJECT, null, TEST_COMMAND_NAME, null, DEFAULT_API_FEATURES_FOR_TESTS); public static final CommandContext VECTOR_COLLECTION_CONTEXT = - new CommandContext<>(VECTOR_COLLECTION_SCHEMA_OBJECT, null, null, null); + new CommandContext<>( + VECTOR_COLLECTION_SCHEMA_OBJECT, null, null, null, DEFAULT_API_FEATURES_FOR_TESTS); public static final CommandContext KEYSPACE_CONTEXT = - new CommandContext<>(KEYSPACE_SCHEMA_OBJECT, null, TEST_COMMAND_NAME, null); + new CommandContext<>( + KEYSPACE_SCHEMA_OBJECT, null, TEST_COMMAND_NAME, null, DEFAULT_API_FEATURES_FOR_TESTS); public static final CommandContext DATABASE_CONTEXT = - new CommandContext<>(DATABASE_SCHEMA_OBJECT, null, TEST_COMMAND_NAME, null); + new CommandContext<>( + DATABASE_SCHEMA_OBJECT, null, TEST_COMMAND_NAME, null, DEFAULT_API_FEATURES_FOR_TESTS); } diff --git a/src/test/java/io/stargate/sgv2/jsonapi/service/embedding/operation/TestEmbeddingProvider.java b/src/test/java/io/stargate/sgv2/jsonapi/service/embedding/operation/TestEmbeddingProvider.java index bfa8077236..eb7b1f78a7 100644 --- a/src/test/java/io/stargate/sgv2/jsonapi/service/embedding/operation/TestEmbeddingProvider.java +++ b/src/test/java/io/stargate/sgv2/jsonapi/service/embedding/operation/TestEmbeddingProvider.java @@ -24,7 +24,8 @@ public class TestEmbeddingProvider extends EmbeddingProvider { null), new TestEmbeddingProvider(), "testCommand", - null); + null, + TestConstants.DEFAULT_API_FEATURES_FOR_TESTS); @Override public Uni vectorize( 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 1934e33b2c..c3d58a3b9b 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 @@ -53,7 +53,12 @@ public class CreateCollectionOperationTest extends OperationTestBase { private CommandContext COMMAND_CONTEXT = - new CommandContext<>(COLLECTION_SCHEMA_OBJECT, null, "CreateCollectionCommand", null); + new CommandContext<>( + COLLECTION_SCHEMA_OBJECT, + null, + "CreateCollectionCommand", + null, + DEFAULT_API_FEATURES_FOR_TESTS); @Inject DatabaseLimitsConfig databaseLimitsConfig; diff --git a/src/test/java/io/stargate/sgv2/jsonapi/service/operation/collections/FindCollectionOperationTest.java b/src/test/java/io/stargate/sgv2/jsonapi/service/operation/collections/FindCollectionOperationTest.java index 4c5c0da691..0517c220d0 100644 --- a/src/test/java/io/stargate/sgv2/jsonapi/service/operation/collections/FindCollectionOperationTest.java +++ b/src/test/java/io/stargate/sgv2/jsonapi/service/operation/collections/FindCollectionOperationTest.java @@ -85,7 +85,8 @@ public void init() { null), null, "testCommand", - jsonProcessingMetricsReporter); + jsonProcessingMetricsReporter, + DEFAULT_API_FEATURES_FOR_TESTS); } @Nested diff --git a/src/test/java/io/stargate/sgv2/jsonapi/service/operation/collections/InsertCollectionOperationTest.java b/src/test/java/io/stargate/sgv2/jsonapi/service/operation/collections/InsertCollectionOperationTest.java index 941f297541..033b70387f 100644 --- a/src/test/java/io/stargate/sgv2/jsonapi/service/operation/collections/InsertCollectionOperationTest.java +++ b/src/test/java/io/stargate/sgv2/jsonapi/service/operation/collections/InsertCollectionOperationTest.java @@ -86,7 +86,8 @@ public void init() { null), null, "testCommand", - jsonProcessingMetricsReporter); + jsonProcessingMetricsReporter, + DEFAULT_API_FEATURES_FOR_TESTS); } @Nested diff --git a/src/test/java/io/stargate/sgv2/jsonapi/service/operation/collections/OperationTestBase.java b/src/test/java/io/stargate/sgv2/jsonapi/service/operation/collections/OperationTestBase.java index 3564a1cef0..f3fe7989b6 100644 --- a/src/test/java/io/stargate/sgv2/jsonapi/service/operation/collections/OperationTestBase.java +++ b/src/test/java/io/stargate/sgv2/jsonapi/service/operation/collections/OperationTestBase.java @@ -19,6 +19,7 @@ import io.stargate.sgv2.jsonapi.api.request.DataApiRequestInfo; import io.stargate.sgv2.jsonapi.api.v1.metrics.JsonProcessingMetricsReporter; import io.stargate.sgv2.jsonapi.config.constants.DocumentConstants; +import io.stargate.sgv2.jsonapi.config.feature.DataApiFeatures; 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.SchemaObjectName; @@ -37,6 +38,7 @@ public class OperationTestBase { protected final String COLLECTION_NAME = RandomStringUtils.randomAlphanumeric(16); protected final SchemaObjectName SCHEMA_OBJECT_NAME = new SchemaObjectName(KEYSPACE_NAME, COLLECTION_NAME); + protected final DataApiFeatures DEFAULT_API_FEATURES_FOR_TESTS = DataApiFeatures.empty(); protected final CollectionSchemaObject COLLECTION_SCHEMA_OBJECT = new CollectionSchemaObject( @@ -51,10 +53,18 @@ public class OperationTestBase { protected final CommandContext COLLECTION_CONTEXT = new CommandContext<>( - COLLECTION_SCHEMA_OBJECT, null, COMMAND_NAME, jsonProcessingMetricsReporter); + COLLECTION_SCHEMA_OBJECT, + null, + COMMAND_NAME, + jsonProcessingMetricsReporter, + DEFAULT_API_FEATURES_FOR_TESTS); protected final CommandContext KEYSPACE_CONTEXT = new CommandContext<>( - KEYSPACE_SCHEMA_OBJECT, null, COMMAND_NAME, jsonProcessingMetricsReporter); + KEYSPACE_SCHEMA_OBJECT, + null, + COMMAND_NAME, + jsonProcessingMetricsReporter, + DEFAULT_API_FEATURES_FOR_TESTS); @InjectMock protected DataApiRequestInfo dataApiRequestInfo; @@ -64,7 +74,11 @@ public class OperationTestBase { protected CommandContext createCommandContextWithCommandName( String commandName) { return new CommandContext<>( - COLLECTION_SCHEMA_OBJECT, null, commandName, jsonProcessingMetricsReporter); + COLLECTION_SCHEMA_OBJECT, + null, + commandName, + jsonProcessingMetricsReporter, + DEFAULT_API_FEATURES_FOR_TESTS); } protected ColumnDefinitions buildColumnDefs(TestColumn... columns) { diff --git a/src/test/java/io/stargate/sgv2/jsonapi/service/operation/collections/ReadAndUpdateCollectionOperationTest.java b/src/test/java/io/stargate/sgv2/jsonapi/service/operation/collections/ReadAndUpdateCollectionOperationTest.java index 552001f2bf..b10a676b48 100644 --- a/src/test/java/io/stargate/sgv2/jsonapi/service/operation/collections/ReadAndUpdateCollectionOperationTest.java +++ b/src/test/java/io/stargate/sgv2/jsonapi/service/operation/collections/ReadAndUpdateCollectionOperationTest.java @@ -120,7 +120,8 @@ public void init() { null), null, "testCommand", - jsonProcessingMetricsReporter); + jsonProcessingMetricsReporter, + DEFAULT_API_FEATURES_FOR_TESTS); } private MockRow resultRow(ColumnDefinitions columnDefs, int index, Object... values) { diff --git a/src/test/java/io/stargate/sgv2/jsonapi/service/resolver/CommandResolverWithVectorizerTest.java b/src/test/java/io/stargate/sgv2/jsonapi/service/resolver/CommandResolverWithVectorizerTest.java index 0342eb54c8..5fae7e4442 100644 --- a/src/test/java/io/stargate/sgv2/jsonapi/service/resolver/CommandResolverWithVectorizerTest.java +++ b/src/test/java/io/stargate/sgv2/jsonapi/service/resolver/CommandResolverWithVectorizerTest.java @@ -19,6 +19,7 @@ import io.stargate.sgv2.jsonapi.api.model.command.impl.UpdateOneCommand; import io.stargate.sgv2.jsonapi.api.request.DataApiRequestInfo; import io.stargate.sgv2.jsonapi.config.OperationsConfig; +import io.stargate.sgv2.jsonapi.config.feature.DataApiFeatures; import io.stargate.sgv2.jsonapi.exception.ErrorCode; import io.stargate.sgv2.jsonapi.exception.JsonApiException; import io.stargate.sgv2.jsonapi.service.cqldriver.executor.CollectionSchemaObject; @@ -71,7 +72,7 @@ public class CommandResolverWithVectorizerTest { @Nested class Resolve { - // TODO: do these need to be uniqe to this test ? Can we use TestConstants ? + // TODO: do these need to be unique to this test ? Can we use TestConstants ? protected final String KEYSPACE_NAME = RandomStringUtils.randomAlphanumeric(16); protected final String COLLECTION_NAME = RandomStringUtils.randomAlphanumeric(16); private final CommandContext VECTOR_COMMAND_CONTEXT = @@ -83,7 +84,8 @@ class Resolve { null), null, null, - null); + null, + DataApiFeatures.empty()); @Test public void find() throws Exception { From 7a5278c8a8e82b6d0eb0f8eebe720b82210bdac5 Mon Sep 17 00:00:00 2001 From: Tatu Saloranta Date: Wed, 4 Sep 2024 17:26:21 -0700 Subject: [PATCH 08/19] Complete initial implementation: things now seem to work as designed --- .../api/model/command/CommandContext.java | 15 +++++--- .../api/request/DataApiRequestInfo.java | 38 +++++++++++++++++++ .../jsonapi/api/v1/CollectionResource.java | 9 +++-- .../sgv2/jsonapi/api/v1/GeneralResource.java | 12 +++++- .../jsonapi/api/v1/NamespaceResource.java | 3 +- .../sgv2/jsonapi/config/ApiTablesConfig.java | 12 ------ .../config/feature/DataApiFeatureConfig.java | 13 +------ .../config/feature/DataApiFeatureFlag.java | 18 ++++++++- .../config/feature/DataApiFeatures.java | 28 +++++++++----- src/main/resources/application.yaml | 11 ++---- 10 files changed, 107 insertions(+), 52 deletions(-) delete mode 100644 src/main/java/io/stargate/sgv2/jsonapi/config/ApiTablesConfig.java diff --git a/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/CommandContext.java b/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/CommandContext.java index 2a7bd1db0b..1c2e556864 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/CommandContext.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/CommandContext.java @@ -42,26 +42,31 @@ public static CommandContext forSchemaObject( T schemaObject, EmbeddingProvider embeddingProvider, String commandName, - JsonProcessingMetricsReporter jsonProcessingMetricsReporter) { + JsonProcessingMetricsReporter jsonProcessingMetricsReporter, + DataApiFeatures apiFeatures) { // TODO: upgrade to use the modern switch statements // TODO: how to remove the unchecked cast ? Had to use unchecked cast to get back to the // CommandContext if (schemaObject instanceof CollectionSchemaObject cso) { return (CommandContext) - forSchemaObject(cso, embeddingProvider, commandName, jsonProcessingMetricsReporter); + forSchemaObject( + cso, embeddingProvider, commandName, jsonProcessingMetricsReporter, apiFeatures); } if (schemaObject instanceof TableSchemaObject tso) { return (CommandContext) - forSchemaObject(tso, embeddingProvider, commandName, jsonProcessingMetricsReporter); + forSchemaObject( + tso, embeddingProvider, commandName, jsonProcessingMetricsReporter, apiFeatures); } if (schemaObject instanceof KeyspaceSchemaObject kso) { return (CommandContext) - forSchemaObject(kso, embeddingProvider, commandName, jsonProcessingMetricsReporter); + forSchemaObject( + kso, embeddingProvider, commandName, jsonProcessingMetricsReporter, apiFeatures); } if (schemaObject instanceof DatabaseSchemaObject dso) { return (CommandContext) - forSchemaObject(dso, embeddingProvider, commandName, jsonProcessingMetricsReporter); + forSchemaObject( + dso, embeddingProvider, commandName, jsonProcessingMetricsReporter, apiFeatures); } throw ErrorCode.SERVER_INTERNAL_ERROR.toApiException( "Unknown schema object type: %s", schemaObject.getClass().getName()); diff --git a/src/main/java/io/stargate/sgv2/jsonapi/api/request/DataApiRequestInfo.java b/src/main/java/io/stargate/sgv2/jsonapi/api/request/DataApiRequestInfo.java index 65cbd8d63d..76ff4484e3 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/api/request/DataApiRequestInfo.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/api/request/DataApiRequestInfo.java @@ -18,6 +18,7 @@ public class DataApiRequestInfo { private final Optional tenantId; private final Optional cassandraToken; private final EmbeddingCredentials embeddingCredentials; + private final HttpHeaderAccess httpHeaders; /** * Constructor that will be useful in the offline library mode, where only the tenant will be set @@ -29,6 +30,7 @@ public DataApiRequestInfo(Optional tenantId) { this.tenantId = tenantId; this.cassandraToken = Optional.empty(); this.embeddingCredentials = null; + httpHeaders = null; } @Inject @@ -41,6 +43,7 @@ public DataApiRequestInfo( this.embeddingCredentials = apiKeysResolver.get().resolveEmbeddingCredentials(routingContext); this.tenantId = (tenantResolver.get()).resolve(routingContext, securityContext); this.cassandraToken = (tokenResolver.get()).resolve(routingContext, securityContext); + httpHeaders = new HttpHeaderAccess(routingContext.request().headers()); } public Optional getTenantId() { @@ -54,4 +57,39 @@ public Optional getCassandraToken() { public EmbeddingCredentials getEmbeddingCredentials() { return this.embeddingCredentials; } + + public HttpHeaderAccess getHttpHeaders() { + return this.httpHeaders; + } + + /** + * Simple wrapper around internal HTTP header container, providing safe(r) access to typed header + * values. Minimal API, currently mainly used for feature flags. + */ + public static class HttpHeaderAccess { + private final io.vertx.core.MultiMap headers; + + public HttpHeaderAccess(io.vertx.core.MultiMap headers) { + this.headers = headers; + } + + /** + * Accessor for getting value of given header (case-insensitive), as {@code Boolean} if (and + * only if!) value is one of "true" or "false". + * + * @param headerName Name of header to check + * @return Boolean.TRUE if header value is "true", Boolean.FALSE if "false", or null if not + */ + public Boolean getHeaderAsBoolean(String headerName) { + String str = headers.get(headerName); + // Only consider strict "true" and "false"; ignore other values + if ("true".equals(str)) { + return Boolean.TRUE; + } + if ("false".equals(str)) { + return Boolean.FALSE; + } + return null; + } + } } diff --git a/src/main/java/io/stargate/sgv2/jsonapi/api/v1/CollectionResource.java b/src/main/java/io/stargate/sgv2/jsonapi/api/v1/CollectionResource.java index 53b341a2e4..8227595be4 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/api/v1/CollectionResource.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/api/v1/CollectionResource.java @@ -188,9 +188,11 @@ public Uni> postCommand( return Uni.createFrom().item(new ThrowableCommandResultSupplier(error)); } else { // TODO No need for the else clause here, simplify - final DataApiFeatures features = DataApiFeatures.fromConfigOnly(apiFeatureConfig); + final DataApiFeatures apiFeatures = + DataApiFeatures.fromConfigAndRequest( + apiFeatureConfig, dataApiRequestInfo.getHttpHeaders()); if ((schemaObject.type == SchemaObject.SchemaObjectType.TABLE) - && !features.isFeatureEnabled(DataApiFeatureFlag.TABLES)) { + && !apiFeatures.isFeatureEnabled(DataApiFeatureFlag.TABLES)) { return Uni.createFrom() .failure(ErrorCode.TABLE_FEATURE_NOT_ENABLED.toApiException()); } @@ -216,7 +218,8 @@ public Uni> postCommand( schemaObject, embeddingProvider, command.getClass().getSimpleName(), - jsonProcessingMetricsReporter); + jsonProcessingMetricsReporter, + apiFeatures); return meteredCommandProcessor.processCommand( dataApiRequestInfo, commandContext, command); diff --git a/src/main/java/io/stargate/sgv2/jsonapi/api/v1/GeneralResource.java b/src/main/java/io/stargate/sgv2/jsonapi/api/v1/GeneralResource.java index 6bec37ca36..f7760c4174 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/api/v1/GeneralResource.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/api/v1/GeneralResource.java @@ -7,6 +7,8 @@ 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.config.feature.DataApiFeatureConfig; +import io.stargate.sgv2.jsonapi.config.feature.DataApiFeatures; import io.stargate.sgv2.jsonapi.service.cqldriver.executor.DatabaseSchemaObject; import io.stargate.sgv2.jsonapi.service.processor.MeteredCommandProcessor; import jakarta.inject.Inject; @@ -37,6 +39,8 @@ public class GeneralResource { @Inject private DataApiRequestInfo dataApiRequestInfo; + @Inject DataApiFeatureConfig apiFeatureConfig; + public static final String BASE_PATH = "/v1"; private final MeteredCommandProcessor meteredCommandProcessor; @@ -75,10 +79,16 @@ public GeneralResource(MeteredCommandProcessor meteredCommandProcessor) { }))) @POST public Uni> postCommand(@NotNull @Valid GeneralCommand command) { + final DataApiFeatures apiFeatures = + DataApiFeatures.fromConfigAndRequest(apiFeatureConfig, dataApiRequestInfo.getHttpHeaders()); var commandContext = CommandContext.forSchemaObject( - new DatabaseSchemaObject(), null, command.getClass().getSimpleName(), null); + new DatabaseSchemaObject(), + null, + command.getClass().getSimpleName(), + null, + apiFeatures); return meteredCommandProcessor .processCommand(dataApiRequestInfo, commandContext, command) diff --git a/src/main/java/io/stargate/sgv2/jsonapi/api/v1/NamespaceResource.java b/src/main/java/io/stargate/sgv2/jsonapi/api/v1/NamespaceResource.java index 43bf53ce28..a60cb5c253 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/api/v1/NamespaceResource.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/api/v1/NamespaceResource.java @@ -108,7 +108,8 @@ public Uni> postCommand( @Size(min = 1, max = 48) String namespace) { - final DataApiFeatures apiFeatures = DataApiFeatures.fromConfigOnly(apiFeatureConfig); + final DataApiFeatures apiFeatures = + DataApiFeatures.fromConfigAndRequest(apiFeatureConfig, dataApiRequestInfo.getHttpHeaders()); // create context // TODO: Aaron , left here to see what CTOR was used, there was a lot of different ones. diff --git a/src/main/java/io/stargate/sgv2/jsonapi/config/ApiTablesConfig.java b/src/main/java/io/stargate/sgv2/jsonapi/config/ApiTablesConfig.java deleted file mode 100644 index d4a423540e..0000000000 --- a/src/main/java/io/stargate/sgv2/jsonapi/config/ApiTablesConfig.java +++ /dev/null @@ -1,12 +0,0 @@ -package io.stargate.sgv2.jsonapi.config; - -import io.smallrye.config.ConfigMapping; -import io.smallrye.config.WithDefault; - -/** Configuration mapping for API Tables feature. */ -@ConfigMapping(prefix = "stargate.tables") -public interface ApiTablesConfig { - /** Setting that determines if the API Tables feature is enabled. */ - @WithDefault("false") - boolean enabled(); -} diff --git a/src/main/java/io/stargate/sgv2/jsonapi/config/feature/DataApiFeatureConfig.java b/src/main/java/io/stargate/sgv2/jsonapi/config/feature/DataApiFeatureConfig.java index ff74d2a1d0..66c58d85b3 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/config/feature/DataApiFeatureConfig.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/config/feature/DataApiFeatureConfig.java @@ -2,7 +2,6 @@ import io.smallrye.config.ConfigMapping; import java.util.Map; -import java.util.Optional; /** * Configuration mapping for Data API Feature flags as read from main application configuration @@ -12,15 +11,7 @@ public interface DataApiFeatureConfig { Map flags(); - default boolean isFeatureEnabled(DataApiFeatureFlag flag) { - return isFeatureEnabled(flag, false); - } - - default boolean isFeatureEnabled(DataApiFeatureFlag flag, boolean defaultValue) { - return Optional.ofNullable(safeFlags().get(flag)).orElse(defaultValue); - } - - default Map safeFlags() { - return (flags() != null) ? flags() : Map.of(); + default Boolean isFeatureEnabled(DataApiFeatureFlag ff) { + return (flags() == null) ? null : flags().get(ff); } } diff --git a/src/main/java/io/stargate/sgv2/jsonapi/config/feature/DataApiFeatureFlag.java b/src/main/java/io/stargate/sgv2/jsonapi/config/feature/DataApiFeatureFlag.java index fb907e3d01..640a14080e 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/config/feature/DataApiFeatureFlag.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/config/feature/DataApiFeatureFlag.java @@ -5,15 +5,29 @@ * Enumeration defines the key used to introspect state of feature. */ public enum DataApiFeatureFlag { - TABLES("tables"); + TABLES("tables", false); private final String featureName; - DataApiFeatureFlag(String featureName) { + private final boolean enabledByDefault; + + private final String featureNameAsHeader; + + DataApiFeatureFlag(String featureName, boolean enabledByDefault) { this.featureName = featureName; + featureNameAsHeader = "x-stargate-feature-" + featureName; + this.enabledByDefault = enabledByDefault; } public String featureName() { return featureName; } + + public String httpHeaderName() { + return featureNameAsHeader; + } + + public boolean enabledByDefault() { + return enabledByDefault; + } } diff --git a/src/main/java/io/stargate/sgv2/jsonapi/config/feature/DataApiFeatures.java b/src/main/java/io/stargate/sgv2/jsonapi/config/feature/DataApiFeatures.java index 343d4cb8fe..c7bc527a5a 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/config/feature/DataApiFeatures.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/config/feature/DataApiFeatures.java @@ -1,8 +1,8 @@ package io.stargate.sgv2.jsonapi.config.feature; +import io.stargate.sgv2.jsonapi.api.request.DataApiRequestInfo; import java.util.Collections; import java.util.Map; -import java.util.Optional; /** * Accessor for combined state of feature flags; typically based on static configuration (with its @@ -10,24 +10,34 @@ */ public class DataApiFeatures { private final Map fromConfig; + private final DataApiRequestInfo.HttpHeaderAccess httpHeaders; - DataApiFeatures(Map fromConfig) { + DataApiFeatures( + Map fromConfig, + DataApiRequestInfo.HttpHeaderAccess httpHeaders) { this.fromConfig = (fromConfig == null) ? Collections.emptyMap() : fromConfig; + this.httpHeaders = httpHeaders; } public static DataApiFeatures empty() { - return new DataApiFeatures(Collections.emptyMap()); + return new DataApiFeatures(Collections.emptyMap(), null); } - public static DataApiFeatures fromConfigOnly(DataApiFeatureConfig config) { - return new DataApiFeatures(config.flags()); + public static DataApiFeatures fromConfigAndRequest( + DataApiFeatureConfig config, DataApiRequestInfo.HttpHeaderAccess httpHeaders) { + return new DataApiFeatures(config.flags(), httpHeaders); } public boolean isFeatureEnabled(DataApiFeatureFlag flag) { - return isFeatureEnabled(flag, false); - } + // First check if there is definition from configuration + Boolean b = fromConfig.get(flag); - public boolean isFeatureEnabled(DataApiFeatureFlag flag, boolean defaultValue) { - return Optional.ofNullable(fromConfig.get(flag)).orElse(defaultValue); + if (b == null) { + // and only if not, allow per-request specification + if (httpHeaders != null) { + b = httpHeaders.getHeaderAsBoolean(flag.httpHeaderName()); + } + } + return (b == null) ? flag.enabledByDefault() : b.booleanValue(); } } diff --git a/src/main/resources/application.yaml b/src/main/resources/application.yaml index 1ae0b4258f..76b1757c3a 100644 --- a/src/main/resources/application.yaml +++ b/src/main/resources/application.yaml @@ -25,10 +25,9 @@ stargate: exception-mappers: enabled: false - feature: - flags: - # null value means "disabled but may be enabled by request override" - TABLES: null + feature: # See io.stargate.sgv2.jsonapi.config.feature.DataApiFeatureConfig + flags: # Ok to leave out features that have no default value (enabled or disabled) + TABLES: # custom grpc settings grpc: @@ -53,10 +52,6 @@ stargate: multi-tenancy: enabled: false - # Is support for API Tables enabled? - tables: - enabled: false - quarkus: # general app properties application: From 594ebb497c768ae6b97bb5b0ee90d8198d36ad9c Mon Sep 17 00:00:00 2001 From: Tatu Saloranta Date: Wed, 4 Sep 2024 17:41:09 -0700 Subject: [PATCH 09/19] Comment fixes --- .../config/feature/DataApiFeatureConfig.java | 6 +----- .../jsonapi/config/feature/DataApiFeatureFlag.java | 13 +++++++++++++ 2 files changed, 14 insertions(+), 5 deletions(-) diff --git a/src/main/java/io/stargate/sgv2/jsonapi/config/feature/DataApiFeatureConfig.java b/src/main/java/io/stargate/sgv2/jsonapi/config/feature/DataApiFeatureConfig.java index 66c58d85b3..a1534d9546 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/config/feature/DataApiFeatureConfig.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/config/feature/DataApiFeatureConfig.java @@ -5,13 +5,9 @@ /** * Configuration mapping for Data API Feature flags as read from main application configuration - * (with possible properety / sysenv overrides). + * (with possible property / sysenv overrides). */ @ConfigMapping(prefix = "stargate.feature") public interface DataApiFeatureConfig { Map flags(); - - default Boolean isFeatureEnabled(DataApiFeatureFlag ff) { - return (flags() == null) ? null : flags().get(ff); - } } diff --git a/src/main/java/io/stargate/sgv2/jsonapi/config/feature/DataApiFeatureFlag.java b/src/main/java/io/stargate/sgv2/jsonapi/config/feature/DataApiFeatureFlag.java index 640a14080e..88cfb83283 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/config/feature/DataApiFeatureFlag.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/config/feature/DataApiFeatureFlag.java @@ -1,16 +1,29 @@ package io.stargate.sgv2.jsonapi.config.feature; +import io.stargate.sgv2.jsonapi.exception.ErrorCode; + /** * Set of "Feature Flags" that can be used to enable/disable certain features in the Data API. * Enumeration defines the key used to introspect state of feature. */ public enum DataApiFeatureFlag { + /** + * API Tables feature flag: if enabled, the API will expose table-specific Namespace resource + * commands, and support Collection commands on Tables. If disabled, those operations will fail + * with {@link ErrorCode#TABLE_FEATURE_NOT_ENABLED}. + * + *

If no configuration specified (config or request), the feature will be Disabled. + */ TABLES("tables", false); private final String featureName; private final boolean enabledByDefault; + /** + * HTTP header name to be used to override the feature flag for a specific request: lower-case, + * prefixed with "x-stargate-feature-"; lookup case-insensitive. + */ private final String featureNameAsHeader; DataApiFeatureFlag(String featureName, boolean enabledByDefault) { From 4e0fad704a4af1bab491523459c1db338e1a90f1 Mon Sep 17 00:00:00 2001 From: Tatu Saloranta Date: Thu, 5 Sep 2024 09:36:38 -0700 Subject: [PATCH 10/19] Change API Feature flags to use lower-case names in all config (but UPPER CASE in Enum name) --- CONFIGURATION.md | 9 ++++++++- .../jsonapi/config/feature/DataApiFeatureFlag.java | 10 ++++++++++ src/main/resources/application.yaml | 2 +- .../sgv2/jsonapi/testresource/DseTestResource.java | 2 +- 4 files changed, 20 insertions(+), 3 deletions(-) diff --git a/CONFIGURATION.md b/CONFIGURATION.md index bb0763d268..ee756d523e 100644 --- a/CONFIGURATION.md +++ b/CONFIGURATION.md @@ -71,10 +71,17 @@ Other Quarkus properties that are specifically relevant for the service: ## Command level logging configuration -*Configuration for command level logging, defined by [CommandLoggingConfig.java](src/main/java/io/stargate/sgv2/jsonapi/config/CommandLoggingConfig.java).* +*Configuration for command level logging, defined by [CommandLevelLoggingConfig.java](src/main/java/io/stargate/sgv2/jsonapi/config/CommandLoggingConfig.java).* | Property | Type | Default | Description | |-----------------------------------------------------|-----------|---------|--------------------------------------------------------------------------------------------------------------------------------------------------------------------| | `stargate.jsonapi.logging.enabled` | `boolean` | `false` | Setting it to `true` enables command level logging. | | `stargate.jsonapi.logging.only-results-with-errors` | `boolean` | `true` | Setting it to `true` prints the command level info only for the commands where the command result has errors. | | `stargate.jsonapi.logging.enabled-tenants` | `string` | `ALL` | Comma separated list of tenants for which command level logging should be enabled. Default is a special keyword called `ALL` which prints this log for all tenants | + +## API Feature enabling configuration +*Configuration for enabling Features, defined by [DataApiFeatureConfig.java](src/main/java/io/stargate/sgv2/jsonapi/config/CommandLoggingConfig.java).* + +| Property | Type | Default | Description | +|---------------------------------|-----------|---------|----------------------------------------------------------------------------------------------------------------------| +| `stargate.feature.flags.tables` | `boolean` | `null` | Setting it to `true` enables Tables functionality; `false` disables; leaving as `null` allowes per-request override. | diff --git a/src/main/java/io/stargate/sgv2/jsonapi/config/feature/DataApiFeatureFlag.java b/src/main/java/io/stargate/sgv2/jsonapi/config/feature/DataApiFeatureFlag.java index 88cfb83283..0304ced5b3 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/config/feature/DataApiFeatureFlag.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/config/feature/DataApiFeatureFlag.java @@ -1,10 +1,14 @@ package io.stargate.sgv2.jsonapi.config.feature; +import com.fasterxml.jackson.annotation.JsonValue; import io.stargate.sgv2.jsonapi.exception.ErrorCode; /** * Set of "Feature Flags" that can be used to enable/disable certain features in the Data API. * Enumeration defines the key used to introspect state of feature. + * + *

NOTE: although flag names are in upper case (like {@code TABLES}), the actual configuration + * uses lower-case names (like {@code tables}) (with proper prefix). */ public enum DataApiFeatureFlag { /** @@ -32,6 +36,7 @@ public enum DataApiFeatureFlag { this.enabledByDefault = enabledByDefault; } + @JsonValue // for Jackson to serialize as lower-case public String featureName() { return featureName; } @@ -40,6 +45,11 @@ public String httpHeaderName() { return featureNameAsHeader; } + /** + * Default state of the feature flag, if not explicitly configured (either by config or request). + * + * @return {@code true} if the feature is enabled by default, {@code false} otherwise. + */ public boolean enabledByDefault() { return enabledByDefault; } diff --git a/src/main/resources/application.yaml b/src/main/resources/application.yaml index 76b1757c3a..f44e54c6df 100644 --- a/src/main/resources/application.yaml +++ b/src/main/resources/application.yaml @@ -27,7 +27,7 @@ stargate: feature: # See io.stargate.sgv2.jsonapi.config.feature.DataApiFeatureConfig flags: # Ok to leave out features that have no default value (enabled or disabled) - TABLES: + tables: # custom grpc settings grpc: diff --git a/src/test/java/io/stargate/sgv2/jsonapi/testresource/DseTestResource.java b/src/test/java/io/stargate/sgv2/jsonapi/testresource/DseTestResource.java index f574594556..6237118c1a 100644 --- a/src/test/java/io/stargate/sgv2/jsonapi/testresource/DseTestResource.java +++ b/src/test/java/io/stargate/sgv2/jsonapi/testresource/DseTestResource.java @@ -74,7 +74,7 @@ public Map start() { // 04-Sep-2024, tatu: [data-api#1335] Enable Tables using new Feature Flag: // propsBuilder.put("stargate.tables.enabled", "true"); - propsBuilder.put("stargate.feature.flags.TABLES", "true"); + propsBuilder.put("stargate.feature.flags.tables", "true"); propsBuilder.put( "stargate.jsonapi.custom.embedding.clazz", From f7920f381a6c4b8b471622339623abb22fc2e805 Mon Sep 17 00:00:00 2001 From: Tatu Saloranta Date: Thu, 5 Sep 2024 15:25:05 -0700 Subject: [PATCH 11/19] Renaming from code review --- CONFIGURATION.md | 2 +- .../api/model/command/CommandContext.java | 14 ++++++------- .../jsonapi/api/v1/CollectionResource.java | 15 +++++++------- .../sgv2/jsonapi/api/v1/GeneralResource.java | 11 +++++----- .../jsonapi/api/v1/NamespaceResource.java | 15 +++++++------- ...ataApiFeatureFlag.java => ApiFeature.java} | 4 ++-- ...{DataApiFeatures.java => ApiFeatures.java} | 20 +++++++++---------- ...FeatureConfig.java => FeaturesConfig.java} | 4 ++-- .../stargate/sgv2/jsonapi/TestConstants.java | 4 ++-- .../collections/OperationTestBase.java | 4 ++-- .../CommandResolverWithVectorizerTest.java | 4 ++-- 11 files changed, 50 insertions(+), 47 deletions(-) rename src/main/java/io/stargate/sgv2/jsonapi/config/feature/{DataApiFeatureFlag.java => ApiFeature.java} (94%) rename src/main/java/io/stargate/sgv2/jsonapi/config/feature/{DataApiFeatures.java => ApiFeatures.java} (64%) rename src/main/java/io/stargate/sgv2/jsonapi/config/feature/{DataApiFeatureConfig.java => FeaturesConfig.java} (78%) diff --git a/CONFIGURATION.md b/CONFIGURATION.md index ee756d523e..42ef5ee6ee 100644 --- a/CONFIGURATION.md +++ b/CONFIGURATION.md @@ -80,7 +80,7 @@ Other Quarkus properties that are specifically relevant for the service: | `stargate.jsonapi.logging.enabled-tenants` | `string` | `ALL` | Comma separated list of tenants for which command level logging should be enabled. Default is a special keyword called `ALL` which prints this log for all tenants | ## API Feature enabling configuration -*Configuration for enabling Features, defined by [DataApiFeatureConfig.java](src/main/java/io/stargate/sgv2/jsonapi/config/CommandLoggingConfig.java).* +*Configuration for enabling Features, defined by [FeaturesConfig.java](src/main/java/io/stargate/sgv2/jsonapi/config/CommandLoggingConfig.java).* | Property | Type | Default | Description | |---------------------------------|-----------|---------|----------------------------------------------------------------------------------------------------------------------| diff --git a/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/CommandContext.java b/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/CommandContext.java index 1c2e556864..78f6fba0e5 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/CommandContext.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/CommandContext.java @@ -2,7 +2,7 @@ import com.google.common.base.Preconditions; import io.stargate.sgv2.jsonapi.api.v1.metrics.JsonProcessingMetricsReporter; -import io.stargate.sgv2.jsonapi.config.feature.DataApiFeatures; +import io.stargate.sgv2.jsonapi.config.feature.ApiFeatures; import io.stargate.sgv2.jsonapi.exception.ErrorCode; import io.stargate.sgv2.jsonapi.service.cqldriver.executor.*; import io.stargate.sgv2.jsonapi.service.embedding.operation.EmbeddingProvider; @@ -18,7 +18,7 @@ public record CommandContext( EmbeddingProvider embeddingProvider, String commandName, JsonProcessingMetricsReporter jsonProcessingMetricsReporter, - DataApiFeatures apiFeatures) { + ApiFeatures apiFeatures) { // TODO: this is what the original EMPTY had, no idea why the name of the command is missing // this is used by the GeneralResource @@ -43,7 +43,7 @@ public static CommandContext forSchemaObject( EmbeddingProvider embeddingProvider, String commandName, JsonProcessingMetricsReporter jsonProcessingMetricsReporter, - DataApiFeatures apiFeatures) { + ApiFeatures apiFeatures) { // TODO: upgrade to use the modern switch statements // TODO: how to remove the unchecked cast ? Had to use unchecked cast to get back to the @@ -87,7 +87,7 @@ public static CommandContext forSchemaObject( EmbeddingProvider embeddingProvider, String commandName, JsonProcessingMetricsReporter jsonProcessingMetricsReporter, - DataApiFeatures apiFeatures) { + ApiFeatures apiFeatures) { return new CommandContext<>( schemaObject, embeddingProvider, commandName, jsonProcessingMetricsReporter, apiFeatures); } @@ -107,7 +107,7 @@ public static CommandContext forSchemaObject( EmbeddingProvider embeddingProvider, String commandName, JsonProcessingMetricsReporter jsonProcessingMetricsReporter, - DataApiFeatures apiFeatures) { + ApiFeatures apiFeatures) { return new CommandContext<>( schemaObject, embeddingProvider, commandName, jsonProcessingMetricsReporter, apiFeatures); } @@ -127,7 +127,7 @@ public static CommandContext forSchemaObject( EmbeddingProvider embeddingProvider, String commandName, JsonProcessingMetricsReporter jsonProcessingMetricsReporter, - DataApiFeatures apiFeatures) { + ApiFeatures apiFeatures) { return new CommandContext<>( schemaObject, embeddingProvider, commandName, jsonProcessingMetricsReporter, apiFeatures); } @@ -147,7 +147,7 @@ public static CommandContext forSchemaObject( EmbeddingProvider embeddingProvider, String commandName, JsonProcessingMetricsReporter jsonProcessingMetricsReporter, - DataApiFeatures apiFeatures) { + ApiFeatures apiFeatures) { return new CommandContext<>( schemaObject, embeddingProvider, commandName, jsonProcessingMetricsReporter, apiFeatures); } diff --git a/src/main/java/io/stargate/sgv2/jsonapi/api/v1/CollectionResource.java b/src/main/java/io/stargate/sgv2/jsonapi/api/v1/CollectionResource.java index 8227595be4..05577c5a35 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/api/v1/CollectionResource.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/api/v1/CollectionResource.java @@ -20,9 +20,9 @@ import io.stargate.sgv2.jsonapi.api.request.DataApiRequestInfo; import io.stargate.sgv2.jsonapi.api.v1.metrics.JsonProcessingMetricsReporter; import io.stargate.sgv2.jsonapi.config.constants.OpenApiConstants; -import io.stargate.sgv2.jsonapi.config.feature.DataApiFeatureConfig; -import io.stargate.sgv2.jsonapi.config.feature.DataApiFeatureFlag; -import io.stargate.sgv2.jsonapi.config.feature.DataApiFeatures; +import io.stargate.sgv2.jsonapi.config.feature.FeaturesConfig; +import io.stargate.sgv2.jsonapi.config.feature.ApiFeature; +import io.stargate.sgv2.jsonapi.config.feature.ApiFeatures; import io.stargate.sgv2.jsonapi.exception.ErrorCode; import io.stargate.sgv2.jsonapi.exception.JsonApiException; import io.stargate.sgv2.jsonapi.exception.mappers.ThrowableCommandResultSupplier; @@ -73,7 +73,8 @@ public class CollectionResource { @Inject private DataApiRequestInfo dataApiRequestInfo; - @Inject DataApiFeatureConfig apiFeatureConfig; + @Inject + FeaturesConfig apiFeatureConfig; @Inject private JsonProcessingMetricsReporter jsonProcessingMetricsReporter; @@ -188,11 +189,11 @@ public Uni> postCommand( return Uni.createFrom().item(new ThrowableCommandResultSupplier(error)); } else { // TODO No need for the else clause here, simplify - final DataApiFeatures apiFeatures = - DataApiFeatures.fromConfigAndRequest( + final ApiFeatures apiFeatures = + ApiFeatures.fromConfigAndRequest( apiFeatureConfig, dataApiRequestInfo.getHttpHeaders()); if ((schemaObject.type == SchemaObject.SchemaObjectType.TABLE) - && !apiFeatures.isFeatureEnabled(DataApiFeatureFlag.TABLES)) { + && !apiFeatures.isFeatureEnabled(ApiFeature.TABLES)) { return Uni.createFrom() .failure(ErrorCode.TABLE_FEATURE_NOT_ENABLED.toApiException()); } diff --git a/src/main/java/io/stargate/sgv2/jsonapi/api/v1/GeneralResource.java b/src/main/java/io/stargate/sgv2/jsonapi/api/v1/GeneralResource.java index f7760c4174..60f614df71 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/api/v1/GeneralResource.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/api/v1/GeneralResource.java @@ -7,8 +7,8 @@ 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.config.feature.DataApiFeatureConfig; -import io.stargate.sgv2.jsonapi.config.feature.DataApiFeatures; +import io.stargate.sgv2.jsonapi.config.feature.FeaturesConfig; +import io.stargate.sgv2.jsonapi.config.feature.ApiFeatures; import io.stargate.sgv2.jsonapi.service.cqldriver.executor.DatabaseSchemaObject; import io.stargate.sgv2.jsonapi.service.processor.MeteredCommandProcessor; import jakarta.inject.Inject; @@ -39,7 +39,8 @@ public class GeneralResource { @Inject private DataApiRequestInfo dataApiRequestInfo; - @Inject DataApiFeatureConfig apiFeatureConfig; + @Inject + FeaturesConfig apiFeatureConfig; public static final String BASE_PATH = "/v1"; @@ -79,8 +80,8 @@ public GeneralResource(MeteredCommandProcessor meteredCommandProcessor) { }))) @POST public Uni> postCommand(@NotNull @Valid GeneralCommand command) { - final DataApiFeatures apiFeatures = - DataApiFeatures.fromConfigAndRequest(apiFeatureConfig, dataApiRequestInfo.getHttpHeaders()); + final ApiFeatures apiFeatures = + ApiFeatures.fromConfigAndRequest(apiFeatureConfig, dataApiRequestInfo.getHttpHeaders()); var commandContext = CommandContext.forSchemaObject( diff --git a/src/main/java/io/stargate/sgv2/jsonapi/api/v1/NamespaceResource.java b/src/main/java/io/stargate/sgv2/jsonapi/api/v1/NamespaceResource.java index a60cb5c253..c78b01a902 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/api/v1/NamespaceResource.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/api/v1/NamespaceResource.java @@ -10,9 +10,9 @@ import io.stargate.sgv2.jsonapi.api.model.command.impl.FindCollectionsCommand; import io.stargate.sgv2.jsonapi.api.request.DataApiRequestInfo; import io.stargate.sgv2.jsonapi.config.constants.OpenApiConstants; -import io.stargate.sgv2.jsonapi.config.feature.DataApiFeatureConfig; -import io.stargate.sgv2.jsonapi.config.feature.DataApiFeatureFlag; -import io.stargate.sgv2.jsonapi.config.feature.DataApiFeatures; +import io.stargate.sgv2.jsonapi.config.feature.FeaturesConfig; +import io.stargate.sgv2.jsonapi.config.feature.ApiFeature; +import io.stargate.sgv2.jsonapi.config.feature.ApiFeatures; import io.stargate.sgv2.jsonapi.exception.ErrorCode; import io.stargate.sgv2.jsonapi.exception.mappers.ThrowableCommandResultSupplier; import io.stargate.sgv2.jsonapi.service.cqldriver.executor.KeyspaceSchemaObject; @@ -53,7 +53,8 @@ public class NamespaceResource { @Inject private DataApiRequestInfo dataApiRequestInfo; - @Inject DataApiFeatureConfig apiFeatureConfig; + @Inject + FeaturesConfig apiFeatureConfig; @Inject public NamespaceResource(MeteredCommandProcessor meteredCommandProcessor) { @@ -108,8 +109,8 @@ public Uni> postCommand( @Size(min = 1, max = 48) String namespace) { - final DataApiFeatures apiFeatures = - DataApiFeatures.fromConfigAndRequest(apiFeatureConfig, dataApiRequestInfo.getHttpHeaders()); + final ApiFeatures apiFeatures = + ApiFeatures.fromConfigAndRequest(apiFeatureConfig, dataApiRequestInfo.getHttpHeaders()); // create context // TODO: Aaron , left here to see what CTOR was used, there was a lot of different ones. @@ -120,7 +121,7 @@ public Uni> postCommand( // Need context first to check if feature is enabled if (command instanceof TableOnlyCommand - && !apiFeatures.isFeatureEnabled(DataApiFeatureFlag.TABLES)) { + && !apiFeatures.isFeatureEnabled(ApiFeature.TABLES)) { return Uni.createFrom() .item( new ThrowableCommandResultSupplier( diff --git a/src/main/java/io/stargate/sgv2/jsonapi/config/feature/DataApiFeatureFlag.java b/src/main/java/io/stargate/sgv2/jsonapi/config/feature/ApiFeature.java similarity index 94% rename from src/main/java/io/stargate/sgv2/jsonapi/config/feature/DataApiFeatureFlag.java rename to src/main/java/io/stargate/sgv2/jsonapi/config/feature/ApiFeature.java index 0304ced5b3..70683b08de 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/config/feature/DataApiFeatureFlag.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/config/feature/ApiFeature.java @@ -10,7 +10,7 @@ *

NOTE: although flag names are in upper case (like {@code TABLES}), the actual configuration * uses lower-case names (like {@code tables}) (with proper prefix). */ -public enum DataApiFeatureFlag { +public enum ApiFeature { /** * API Tables feature flag: if enabled, the API will expose table-specific Namespace resource * commands, and support Collection commands on Tables. If disabled, those operations will fail @@ -30,7 +30,7 @@ public enum DataApiFeatureFlag { */ private final String featureNameAsHeader; - DataApiFeatureFlag(String featureName, boolean enabledByDefault) { + ApiFeature(String featureName, boolean enabledByDefault) { this.featureName = featureName; featureNameAsHeader = "x-stargate-feature-" + featureName; this.enabledByDefault = enabledByDefault; diff --git a/src/main/java/io/stargate/sgv2/jsonapi/config/feature/DataApiFeatures.java b/src/main/java/io/stargate/sgv2/jsonapi/config/feature/ApiFeatures.java similarity index 64% rename from src/main/java/io/stargate/sgv2/jsonapi/config/feature/DataApiFeatures.java rename to src/main/java/io/stargate/sgv2/jsonapi/config/feature/ApiFeatures.java index c7bc527a5a..5051690746 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/config/feature/DataApiFeatures.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/config/feature/ApiFeatures.java @@ -8,27 +8,27 @@ * Accessor for combined state of feature flags; typically based on static configuration (with its * overrides) and possible per-request settings. */ -public class DataApiFeatures { - private final Map fromConfig; +public class ApiFeatures { + private final Map fromConfig; private final DataApiRequestInfo.HttpHeaderAccess httpHeaders; - DataApiFeatures( - Map fromConfig, + ApiFeatures( + Map fromConfig, DataApiRequestInfo.HttpHeaderAccess httpHeaders) { this.fromConfig = (fromConfig == null) ? Collections.emptyMap() : fromConfig; this.httpHeaders = httpHeaders; } - public static DataApiFeatures empty() { - return new DataApiFeatures(Collections.emptyMap(), null); + public static ApiFeatures empty() { + return new ApiFeatures(Collections.emptyMap(), null); } - public static DataApiFeatures fromConfigAndRequest( - DataApiFeatureConfig config, DataApiRequestInfo.HttpHeaderAccess httpHeaders) { - return new DataApiFeatures(config.flags(), httpHeaders); + public static ApiFeatures fromConfigAndRequest( + FeaturesConfig config, DataApiRequestInfo.HttpHeaderAccess httpHeaders) { + return new ApiFeatures(config.flags(), httpHeaders); } - public boolean isFeatureEnabled(DataApiFeatureFlag flag) { + public boolean isFeatureEnabled(ApiFeature flag) { // First check if there is definition from configuration Boolean b = fromConfig.get(flag); diff --git a/src/main/java/io/stargate/sgv2/jsonapi/config/feature/DataApiFeatureConfig.java b/src/main/java/io/stargate/sgv2/jsonapi/config/feature/FeaturesConfig.java similarity index 78% rename from src/main/java/io/stargate/sgv2/jsonapi/config/feature/DataApiFeatureConfig.java rename to src/main/java/io/stargate/sgv2/jsonapi/config/feature/FeaturesConfig.java index a1534d9546..04b349cfbe 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/config/feature/DataApiFeatureConfig.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/config/feature/FeaturesConfig.java @@ -8,6 +8,6 @@ * (with possible property / sysenv overrides). */ @ConfigMapping(prefix = "stargate.feature") -public interface DataApiFeatureConfig { - Map flags(); +public interface FeaturesConfig { + Map flags(); } diff --git a/src/test/java/io/stargate/sgv2/jsonapi/TestConstants.java b/src/test/java/io/stargate/sgv2/jsonapi/TestConstants.java index 60353e940c..e50b656de2 100644 --- a/src/test/java/io/stargate/sgv2/jsonapi/TestConstants.java +++ b/src/test/java/io/stargate/sgv2/jsonapi/TestConstants.java @@ -1,7 +1,7 @@ package io.stargate.sgv2.jsonapi; import io.stargate.sgv2.jsonapi.api.model.command.CommandContext; -import io.stargate.sgv2.jsonapi.config.feature.DataApiFeatures; +import io.stargate.sgv2.jsonapi.config.feature.ApiFeatures; import io.stargate.sgv2.jsonapi.service.cqldriver.executor.*; import org.apache.commons.lang3.RandomStringUtils; @@ -41,7 +41,7 @@ public final class TestConstants { // CommandContext for working on the schema objects above - public static final DataApiFeatures DEFAULT_API_FEATURES_FOR_TESTS = DataApiFeatures.empty(); + public static final ApiFeatures DEFAULT_API_FEATURES_FOR_TESTS = ApiFeatures.empty(); public static final CommandContext COLLECTION_CONTEXT = new CommandContext<>( diff --git a/src/test/java/io/stargate/sgv2/jsonapi/service/operation/collections/OperationTestBase.java b/src/test/java/io/stargate/sgv2/jsonapi/service/operation/collections/OperationTestBase.java index f3fe7989b6..30736c8d74 100644 --- a/src/test/java/io/stargate/sgv2/jsonapi/service/operation/collections/OperationTestBase.java +++ b/src/test/java/io/stargate/sgv2/jsonapi/service/operation/collections/OperationTestBase.java @@ -19,7 +19,7 @@ import io.stargate.sgv2.jsonapi.api.request.DataApiRequestInfo; import io.stargate.sgv2.jsonapi.api.v1.metrics.JsonProcessingMetricsReporter; import io.stargate.sgv2.jsonapi.config.constants.DocumentConstants; -import io.stargate.sgv2.jsonapi.config.feature.DataApiFeatures; +import io.stargate.sgv2.jsonapi.config.feature.ApiFeatures; 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.SchemaObjectName; @@ -38,7 +38,7 @@ public class OperationTestBase { protected final String COLLECTION_NAME = RandomStringUtils.randomAlphanumeric(16); protected final SchemaObjectName SCHEMA_OBJECT_NAME = new SchemaObjectName(KEYSPACE_NAME, COLLECTION_NAME); - protected final DataApiFeatures DEFAULT_API_FEATURES_FOR_TESTS = DataApiFeatures.empty(); + protected final ApiFeatures DEFAULT_API_FEATURES_FOR_TESTS = ApiFeatures.empty(); protected final CollectionSchemaObject COLLECTION_SCHEMA_OBJECT = new CollectionSchemaObject( diff --git a/src/test/java/io/stargate/sgv2/jsonapi/service/resolver/CommandResolverWithVectorizerTest.java b/src/test/java/io/stargate/sgv2/jsonapi/service/resolver/CommandResolverWithVectorizerTest.java index 5fae7e4442..4a8a185e43 100644 --- a/src/test/java/io/stargate/sgv2/jsonapi/service/resolver/CommandResolverWithVectorizerTest.java +++ b/src/test/java/io/stargate/sgv2/jsonapi/service/resolver/CommandResolverWithVectorizerTest.java @@ -19,7 +19,7 @@ import io.stargate.sgv2.jsonapi.api.model.command.impl.UpdateOneCommand; import io.stargate.sgv2.jsonapi.api.request.DataApiRequestInfo; import io.stargate.sgv2.jsonapi.config.OperationsConfig; -import io.stargate.sgv2.jsonapi.config.feature.DataApiFeatures; +import io.stargate.sgv2.jsonapi.config.feature.ApiFeatures; import io.stargate.sgv2.jsonapi.exception.ErrorCode; import io.stargate.sgv2.jsonapi.exception.JsonApiException; import io.stargate.sgv2.jsonapi.service.cqldriver.executor.CollectionSchemaObject; @@ -85,7 +85,7 @@ class Resolve { null, null, null, - DataApiFeatures.empty()); + ApiFeatures.empty()); @Test public void find() throws Exception { From b68176f586d6f7bdb853cad96892063323645b6b Mon Sep 17 00:00:00 2001 From: Tatu Saloranta Date: Thu, 5 Sep 2024 15:41:40 -0700 Subject: [PATCH 12/19] More changes from code review --- .../jsonapi/api/request/DataApiRequestInfo.java | 5 +++-- .../sgv2/jsonapi/api/v1/CollectionResource.java | 5 ++--- .../sgv2/jsonapi/api/v1/GeneralResource.java | 5 ++--- .../sgv2/jsonapi/api/v1/NamespaceResource.java | 8 +++----- .../sgv2/jsonapi/config/feature/ApiFeature.java | 4 ++++ .../sgv2/jsonapi/config/feature/ApiFeatures.java | 15 +++++++++------ 6 files changed, 23 insertions(+), 19 deletions(-) diff --git a/src/main/java/io/stargate/sgv2/jsonapi/api/request/DataApiRequestInfo.java b/src/main/java/io/stargate/sgv2/jsonapi/api/request/DataApiRequestInfo.java index 76ff4484e3..4a17fba7fc 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/api/request/DataApiRequestInfo.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/api/request/DataApiRequestInfo.java @@ -74,8 +74,9 @@ public HttpHeaderAccess(io.vertx.core.MultiMap headers) { } /** - * Accessor for getting value of given header (case-insensitive), as {@code Boolean} if (and - * only if!) value is one of "true" or "false". + * Accessor for getting value of given header, as {@code Boolean} if (and only if!) value is one + * of "true" or "false". Access by name is (and has to be) case-insensitive as per HTTP + * standard. * * @param headerName Name of header to check * @return Boolean.TRUE if header value is "true", Boolean.FALSE if "false", or null if not diff --git a/src/main/java/io/stargate/sgv2/jsonapi/api/v1/CollectionResource.java b/src/main/java/io/stargate/sgv2/jsonapi/api/v1/CollectionResource.java index 05577c5a35..a4f8e2d844 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/api/v1/CollectionResource.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/api/v1/CollectionResource.java @@ -20,9 +20,9 @@ import io.stargate.sgv2.jsonapi.api.request.DataApiRequestInfo; import io.stargate.sgv2.jsonapi.api.v1.metrics.JsonProcessingMetricsReporter; import io.stargate.sgv2.jsonapi.config.constants.OpenApiConstants; -import io.stargate.sgv2.jsonapi.config.feature.FeaturesConfig; import io.stargate.sgv2.jsonapi.config.feature.ApiFeature; import io.stargate.sgv2.jsonapi.config.feature.ApiFeatures; +import io.stargate.sgv2.jsonapi.config.feature.FeaturesConfig; import io.stargate.sgv2.jsonapi.exception.ErrorCode; import io.stargate.sgv2.jsonapi.exception.JsonApiException; import io.stargate.sgv2.jsonapi.exception.mappers.ThrowableCommandResultSupplier; @@ -73,8 +73,7 @@ public class CollectionResource { @Inject private DataApiRequestInfo dataApiRequestInfo; - @Inject - FeaturesConfig apiFeatureConfig; + @Inject FeaturesConfig apiFeatureConfig; @Inject private JsonProcessingMetricsReporter jsonProcessingMetricsReporter; diff --git a/src/main/java/io/stargate/sgv2/jsonapi/api/v1/GeneralResource.java b/src/main/java/io/stargate/sgv2/jsonapi/api/v1/GeneralResource.java index 60f614df71..6363ae5786 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/api/v1/GeneralResource.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/api/v1/GeneralResource.java @@ -7,8 +7,8 @@ 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.config.feature.FeaturesConfig; import io.stargate.sgv2.jsonapi.config.feature.ApiFeatures; +import io.stargate.sgv2.jsonapi.config.feature.FeaturesConfig; import io.stargate.sgv2.jsonapi.service.cqldriver.executor.DatabaseSchemaObject; import io.stargate.sgv2.jsonapi.service.processor.MeteredCommandProcessor; import jakarta.inject.Inject; @@ -39,8 +39,7 @@ public class GeneralResource { @Inject private DataApiRequestInfo dataApiRequestInfo; - @Inject - FeaturesConfig apiFeatureConfig; + @Inject FeaturesConfig apiFeatureConfig; public static final String BASE_PATH = "/v1"; diff --git a/src/main/java/io/stargate/sgv2/jsonapi/api/v1/NamespaceResource.java b/src/main/java/io/stargate/sgv2/jsonapi/api/v1/NamespaceResource.java index c78b01a902..69700b5699 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/api/v1/NamespaceResource.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/api/v1/NamespaceResource.java @@ -10,9 +10,9 @@ import io.stargate.sgv2.jsonapi.api.model.command.impl.FindCollectionsCommand; import io.stargate.sgv2.jsonapi.api.request.DataApiRequestInfo; import io.stargate.sgv2.jsonapi.config.constants.OpenApiConstants; -import io.stargate.sgv2.jsonapi.config.feature.FeaturesConfig; import io.stargate.sgv2.jsonapi.config.feature.ApiFeature; import io.stargate.sgv2.jsonapi.config.feature.ApiFeatures; +import io.stargate.sgv2.jsonapi.config.feature.FeaturesConfig; import io.stargate.sgv2.jsonapi.exception.ErrorCode; import io.stargate.sgv2.jsonapi.exception.mappers.ThrowableCommandResultSupplier; import io.stargate.sgv2.jsonapi.service.cqldriver.executor.KeyspaceSchemaObject; @@ -53,8 +53,7 @@ public class NamespaceResource { @Inject private DataApiRequestInfo dataApiRequestInfo; - @Inject - FeaturesConfig apiFeatureConfig; + @Inject FeaturesConfig apiFeatureConfig; @Inject public NamespaceResource(MeteredCommandProcessor meteredCommandProcessor) { @@ -120,8 +119,7 @@ public Uni> postCommand( new CommandContext<>(new KeyspaceSchemaObject(namespace), null, "", null, apiFeatures); // Need context first to check if feature is enabled - if (command instanceof TableOnlyCommand - && !apiFeatures.isFeatureEnabled(ApiFeature.TABLES)) { + if (command instanceof TableOnlyCommand && !apiFeatures.isFeatureEnabled(ApiFeature.TABLES)) { return Uni.createFrom() .item( new ThrowableCommandResultSupplier( diff --git a/src/main/java/io/stargate/sgv2/jsonapi/config/feature/ApiFeature.java b/src/main/java/io/stargate/sgv2/jsonapi/config/feature/ApiFeature.java index 70683b08de..3f0ae12460 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/config/feature/ApiFeature.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/config/feature/ApiFeature.java @@ -31,6 +31,10 @@ public enum ApiFeature { private final String featureNameAsHeader; ApiFeature(String featureName, boolean enabledByDefault) { + if (!featureName.equals(featureName.toLowerCase())) { + throw new IllegalStateException( + "Internal error: 'featureName' must be lower-case, was: \"" + featureName + "\""); + } this.featureName = featureName; featureNameAsHeader = "x-stargate-feature-" + featureName; this.enabledByDefault = enabledByDefault; diff --git a/src/main/java/io/stargate/sgv2/jsonapi/config/feature/ApiFeatures.java b/src/main/java/io/stargate/sgv2/jsonapi/config/feature/ApiFeatures.java index 5051690746..edee5fb802 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/config/feature/ApiFeatures.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/config/feature/ApiFeatures.java @@ -12,10 +12,9 @@ public class ApiFeatures { private final Map fromConfig; private final DataApiRequestInfo.HttpHeaderAccess httpHeaders; - ApiFeatures( - Map fromConfig, - DataApiRequestInfo.HttpHeaderAccess httpHeaders) { - this.fromConfig = (fromConfig == null) ? Collections.emptyMap() : fromConfig; + private ApiFeatures( + Map fromConfig, DataApiRequestInfo.HttpHeaderAccess httpHeaders) { + this.fromConfig = fromConfig; this.httpHeaders = httpHeaders; } @@ -24,8 +23,12 @@ public static ApiFeatures empty() { } public static ApiFeatures fromConfigAndRequest( - FeaturesConfig config, DataApiRequestInfo.HttpHeaderAccess httpHeaders) { - return new ApiFeatures(config.flags(), httpHeaders); + FeaturesConfig config, DataApiRequestInfo.HttpHeaderAccess httpHeaders) { + Map fromConfig = config.flags(); + if (fromConfig == null) { + fromConfig = Collections.emptyMap(); + } + return new ApiFeatures(fromConfig, httpHeaders); } public boolean isFeatureEnabled(ApiFeature flag) { From 26ceefa0cc748a6080f4496261e342b670145abe Mon Sep 17 00:00:00 2001 From: Tatu Saloranta Date: Fri, 6 Sep 2024 15:07:12 -0700 Subject: [PATCH 13/19] ... --- .../api/v1/tables/AbstractTableIntegrationTestBase.java | 9 +++++---- .../jsonapi/api/v1/util/DataApiCommandSenderBase.java | 4 ++-- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/src/test/java/io/stargate/sgv2/jsonapi/api/v1/tables/AbstractTableIntegrationTestBase.java b/src/test/java/io/stargate/sgv2/jsonapi/api/v1/tables/AbstractTableIntegrationTestBase.java index 39b6c01b30..f898f41092 100644 --- a/src/test/java/io/stargate/sgv2/jsonapi/api/v1/tables/AbstractTableIntegrationTestBase.java +++ b/src/test/java/io/stargate/sgv2/jsonapi/api/v1/tables/AbstractTableIntegrationTestBase.java @@ -10,6 +10,7 @@ import io.stargate.sgv2.jsonapi.api.v1.AbstractNamespaceIntegrationTestBase; import io.stargate.sgv2.jsonapi.api.v1.CollectionResource; import io.stargate.sgv2.jsonapi.api.v1.util.DataApiCommandSenders; +import io.stargate.sgv2.jsonapi.api.v1.util.DataApiResponseValidator; import java.io.IOException; import java.util.Map; @@ -17,9 +18,9 @@ public class AbstractTableIntegrationTestBase extends AbstractNamespaceIntegrationTestBase { private static final ObjectMapper MAPPER = new ObjectMapper(); - protected void createTableWithColumns( + protected DataApiResponseValidator createTableWithColumns( String tableName, Map columns, Object primaryKeyDef) { - createTable( + return createTable( """ { "name": "%s", @@ -32,8 +33,8 @@ protected void createTableWithColumns( .formatted(tableName, asJSON(columns), asJSON(primaryKeyDef))); } - protected void createTable(String tableDefAsJSON) { - DataApiCommandSenders.assertNamespaceCommand(namespaceName) + protected DataApiResponseValidator createTable(String tableDefAsJSON) { + return DataApiCommandSenders.assertNamespaceCommand(namespaceName) .postCreateTable(tableDefAsJSON) .hasNoErrors() .body("status.ok", is(1)); diff --git a/src/test/java/io/stargate/sgv2/jsonapi/api/v1/util/DataApiCommandSenderBase.java b/src/test/java/io/stargate/sgv2/jsonapi/api/v1/util/DataApiCommandSenderBase.java index 5450a93d5a..a0b461cb14 100644 --- a/src/test/java/io/stargate/sgv2/jsonapi/api/v1/util/DataApiCommandSenderBase.java +++ b/src/test/java/io/stargate/sgv2/jsonapi/api/v1/util/DataApiCommandSenderBase.java @@ -50,7 +50,7 @@ protected T _this() { * @param jsonBody JSON body to POST * @return Response validator for further assertions */ - public final DataApiResponseValidator postRaw(String jsonBody) { + public final DataApiResponseValidator postJSON(String jsonBody) { RequestSpecification request = given() .port(getTestPort()) @@ -64,7 +64,7 @@ public final DataApiResponseValidator postRaw(String jsonBody) { } public DataApiResponseValidator postCommand(String command, String commandClause) { - return postRaw("{ \"%s\": %s }".formatted(command, commandClause)); + return postJSON("{ \"%s\": %s }".formatted(command, commandClause)); } protected abstract io.restassured.response.Response postInternal(RequestSpecification request); From fa09a869fe26572fd564fced932473848eb55aee Mon Sep 17 00:00:00 2001 From: Tatu Saloranta Date: Fri, 6 Sep 2024 16:09:00 -0700 Subject: [PATCH 14/19] Fix CreateTableIntegrationTest --- pom.xml | 2 +- .../jsonapi/api/v1/tables/CreateTableIntegrationTest.java | 5 ++--- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/pom.xml b/pom.xml index ab57200d78..51f3b6f5b6 100644 --- a/pom.xml +++ b/pom.xml @@ -238,7 +238,7 @@ org.junit-pioneer junit-pioneer - 2.0.0 + 2.2.0 test diff --git a/src/test/java/io/stargate/sgv2/jsonapi/api/v1/tables/CreateTableIntegrationTest.java b/src/test/java/io/stargate/sgv2/jsonapi/api/v1/tables/CreateTableIntegrationTest.java index 9b882e815e..c978f7847d 100644 --- a/src/test/java/io/stargate/sgv2/jsonapi/api/v1/tables/CreateTableIntegrationTest.java +++ b/src/test/java/io/stargate/sgv2/jsonapi/api/v1/tables/CreateTableIntegrationTest.java @@ -9,17 +9,16 @@ import io.stargate.sgv2.jsonapi.api.v1.AbstractNamespaceIntegrationTestBase; import io.stargate.sgv2.jsonapi.api.v1.NamespaceResource; import io.stargate.sgv2.jsonapi.testresource.DseTestResource; -import org.junit.Test; import org.junit.jupiter.api.ClassOrderer; import org.junit.jupiter.api.Nested; import org.junit.jupiter.api.Order; +import org.junit.jupiter.api.Test; import org.junit.jupiter.api.TestClassOrder; @QuarkusIntegrationTest @WithTestResource(value = DseTestResource.class, restrictToAnnotatedClass = false) @TestClassOrder(ClassOrderer.OrderAnnotation.class) class CreateTableIntegrationTest extends AbstractNamespaceIntegrationTestBase { - @Nested @Order(1) class CreateTable { @@ -111,7 +110,7 @@ private void deleteTable(String tableName) { .body( """ { - "deleteTable": { + "deleteCollection": { "name": "%s" } } From f378b9a5faccb07d1c8bfb9d22fe15e66d123762 Mon Sep 17 00:00:00 2001 From: Tatu Saloranta Date: Fri, 6 Sep 2024 16:46:44 -0700 Subject: [PATCH 15/19] Add an IT to verify blocking of CreateTable if feature flag not enabled --- .../TableFeatureDisabledIntegrationTest.java | 60 +++++++++++++++++++ .../jsonapi/testresource/DseTestResource.java | 11 +++- 2 files changed, 69 insertions(+), 2 deletions(-) create mode 100644 src/test/java/io/stargate/sgv2/jsonapi/api/v1/tables/TableFeatureDisabledIntegrationTest.java diff --git a/src/test/java/io/stargate/sgv2/jsonapi/api/v1/tables/TableFeatureDisabledIntegrationTest.java b/src/test/java/io/stargate/sgv2/jsonapi/api/v1/tables/TableFeatureDisabledIntegrationTest.java new file mode 100644 index 0000000000..df3d4d2949 --- /dev/null +++ b/src/test/java/io/stargate/sgv2/jsonapi/api/v1/tables/TableFeatureDisabledIntegrationTest.java @@ -0,0 +1,60 @@ +package io.stargate.sgv2.jsonapi.api.v1.tables; + +import io.quarkus.test.common.WithTestResource; +import io.quarkus.test.junit.QuarkusIntegrationTest; +import io.stargate.sgv2.jsonapi.api.v1.util.DataApiCommandSenders; +import io.stargate.sgv2.jsonapi.exception.ErrorCodeV1; +import io.stargate.sgv2.jsonapi.testresource.DseTestResource; +import org.junit.jupiter.api.ClassOrderer; +import org.junit.jupiter.api.Order; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.TestClassOrder; + +/** + * Basic testing to see what happens when "Tables" feature is disabled (other tests will have + * feature enabled) + */ +@QuarkusIntegrationTest +@WithTestResource( + value = TableFeatureDisabledIntegrationTest.TestResource.class, + restrictToAnnotatedClass = true) +@TestClassOrder(ClassOrderer.OrderAnnotation.class) +public class TableFeatureDisabledIntegrationTest extends AbstractTableIntegrationTestBase { + // Need to be able to enable/disable the TABLES feature + public static class TestResource extends DseTestResource { + public TestResource() {} + + @Override + public String getFeatureFlagTables() { + // return empty to leave feature "undefined" (disabled unless per-request header override) + // ("false" would be "disabled" for all tests, regardless of headers) + return ""; + } + } + + // By default, table creation should fail + @Order(1) + @Test + public void failCreateWithoutFeatureEnabled() { + DataApiCommandSenders.assertNamespaceCommand(namespaceName) + .postCreateTable(simpleTableDef("failCreateWithoutFeatureEnabled")) + .hasSingleApiError(ErrorCodeV1.TABLE_FEATURE_NOT_ENABLED); + } + + private static String simpleTableDef(String tableName) { + return + """ + { + "name": "%s", + "definition": { + "columns": { + "id": { "type": "text" }, + "name": { "type": "text" } + }, + "primaryKey": "id" + } + } + """ + .formatted(tableName); + } +} diff --git a/src/test/java/io/stargate/sgv2/jsonapi/testresource/DseTestResource.java b/src/test/java/io/stargate/sgv2/jsonapi/testresource/DseTestResource.java index 6237118c1a..be691958ad 100644 --- a/src/test/java/io/stargate/sgv2/jsonapi/testresource/DseTestResource.java +++ b/src/test/java/io/stargate/sgv2/jsonapi/testresource/DseTestResource.java @@ -65,6 +65,11 @@ public Long getMaxDocumentSortCount() { return 100L; } + // By default, we enable the feature flag for tables + public String getFeatureFlagTables() { + return "true"; + } + @Override public Map start() { Map env = super.start(); @@ -73,8 +78,10 @@ public Map start() { propsBuilder.put("stargate.jsonapi.custom.embedding.enabled", "true"); // 04-Sep-2024, tatu: [data-api#1335] Enable Tables using new Feature Flag: - // propsBuilder.put("stargate.tables.enabled", "true"); - propsBuilder.put("stargate.feature.flags.tables", "true"); + String tableFeatureSetting = getFeatureFlagTables(); + if (tableFeatureSetting != null) { + propsBuilder.put("stargate.feature.flags.tables", tableFeatureSetting); + } propsBuilder.put( "stargate.jsonapi.custom.embedding.clazz", From bb355c72645a4647dc9db804cf1bf13f74b9c713 Mon Sep 17 00:00:00 2001 From: Tatu Saloranta Date: Fri, 6 Sep 2024 17:14:31 -0700 Subject: [PATCH 16/19] Complete IT to verify `ApiFeature.TABLES` functioning, header override --- .../TableFeatureDisabledIntegrationTest.java | 37 ++++++++- .../api/v1/util/DataApiCommandSenderBase.java | 79 ++++++++++++------- 2 files changed, 88 insertions(+), 28 deletions(-) diff --git a/src/test/java/io/stargate/sgv2/jsonapi/api/v1/tables/TableFeatureDisabledIntegrationTest.java b/src/test/java/io/stargate/sgv2/jsonapi/api/v1/tables/TableFeatureDisabledIntegrationTest.java index df3d4d2949..4337c832a3 100644 --- a/src/test/java/io/stargate/sgv2/jsonapi/api/v1/tables/TableFeatureDisabledIntegrationTest.java +++ b/src/test/java/io/stargate/sgv2/jsonapi/api/v1/tables/TableFeatureDisabledIntegrationTest.java @@ -1,8 +1,11 @@ package io.stargate.sgv2.jsonapi.api.v1.tables; +import static org.hamcrest.Matchers.is; + import io.quarkus.test.common.WithTestResource; import io.quarkus.test.junit.QuarkusIntegrationTest; import io.stargate.sgv2.jsonapi.api.v1.util.DataApiCommandSenders; +import io.stargate.sgv2.jsonapi.config.feature.ApiFeature; import io.stargate.sgv2.jsonapi.exception.ErrorCodeV1; import io.stargate.sgv2.jsonapi.testresource.DseTestResource; import org.junit.jupiter.api.ClassOrderer; @@ -32,15 +35,47 @@ public String getFeatureFlagTables() { } } + private static final String TABLE_TO_CREATE = "table_with_table_feature"; + // By default, table creation should fail @Order(1) @Test public void failCreateWithoutFeatureEnabled() { DataApiCommandSenders.assertNamespaceCommand(namespaceName) - .postCreateTable(simpleTableDef("failCreateWithoutFeatureEnabled")) + .postCreateTable(simpleTableDef(TABLE_TO_CREATE)) .hasSingleApiError(ErrorCodeV1.TABLE_FEATURE_NOT_ENABLED); } + // But with header override, should succeed + @Order(2) + @Test + public void okCreateWithFeatureEnabledViaHeader() { + DataApiCommandSenders.assertNamespaceCommand(namespaceName) + .header(ApiFeature.TABLES.httpHeaderName(), "true") + .postCreateTable(simpleTableDef(TABLE_TO_CREATE)) + .hasNoErrors() + .body("status.ok", is(1)); + } + + // But even with table, find() should fail without Feature enabled + @Order(3) + @Test + public void failFindWithoutFeature() { + DataApiCommandSenders.assertTableCommand(namespaceName, TABLE_TO_CREATE) + .postFindOne("{}") + .hasSingleApiError(ErrorCodeV1.TABLE_FEATURE_NOT_ENABLED); + } + + // And finally, with header override, should succeed in findOne() + @Order(4) + @Test + public void okFindWithFeatureEnabledViaHeader() { + DataApiCommandSenders.assertTableCommand(namespaceName, TABLE_TO_CREATE) + .header(ApiFeature.TABLES.httpHeaderName(), "true") + .postFindOne("{}") + .hasNoErrors(); + } + private static String simpleTableDef(String tableName) { return """ diff --git a/src/test/java/io/stargate/sgv2/jsonapi/api/v1/util/DataApiCommandSenderBase.java b/src/test/java/io/stargate/sgv2/jsonapi/api/v1/util/DataApiCommandSenderBase.java index a0b461cb14..5bbab6a6b0 100644 --- a/src/test/java/io/stargate/sgv2/jsonapi/api/v1/util/DataApiCommandSenderBase.java +++ b/src/test/java/io/stargate/sgv2/jsonapi/api/v1/util/DataApiCommandSenderBase.java @@ -12,6 +12,7 @@ import io.stargate.sgv2.jsonapi.service.embedding.operation.test.CustomITEmbeddingProvider; import jakarta.ws.rs.core.Response; import java.util.Base64; +import java.util.LinkedHashMap; import java.util.Map; import org.eclipse.microprofile.config.ConfigProvider; @@ -24,8 +25,11 @@ public abstract class DataApiCommandSenderBase headers; + protected DataApiCommandSenderBase(String namespace) { this.namespace = namespace; + headers = DefaultHeaders.mutableCopy(); } /** @@ -39,6 +43,22 @@ public T expectHttpStatus(Response.Status expectedHttpStatus) { return _this(); } + /** + * Fluent method for adding/overriding/removing a header in the request. + * + * @param name Name of header to set + * @param value Value of header to set; if null, header is removed, otherwise added or overridden + * @return Type-safe "this" sender for call chaining + */ + public T header(String name, String value) { + if (value == null) { + headers.remove(name); + } else { + headers.put(name, value); + } + return _this(); + } + protected T _this() { return (T) this; } @@ -54,7 +74,7 @@ public final DataApiResponseValidator postJSON(String jsonBody) { RequestSpecification request = given() .port(getTestPort()) - .headers(getHeaders()) + .headers(headers) .contentType(ContentType.JSON) .body(jsonBody) .when(); @@ -69,36 +89,41 @@ public DataApiResponseValidator postCommand(String command, String commandClause protected abstract io.restassured.response.Response postInternal(RequestSpecification request); - protected Map getHeaders() { - if (useCoordinator()) { - return Map.of( - HttpConstants.AUTHENTICATION_TOKEN_HEADER_NAME, - getAuthToken(), - HttpConstants.EMBEDDING_AUTHENTICATION_TOKEN_HEADER_NAME, - CustomITEmbeddingProvider.TEST_API_KEY); - } else { - String credential = - "Cassandra:" - + Base64.getEncoder().encodeToString(getCassandraUsername().getBytes()) - + ":" - + Base64.getEncoder().encodeToString(getCassandraPassword().getBytes()); - return Map.of( - HttpConstants.AUTHENTICATION_TOKEN_HEADER_NAME, - credential, - HttpConstants.EMBEDDING_AUTHENTICATION_TOKEN_HEADER_NAME, - CustomITEmbeddingProvider.TEST_API_KEY); - } - } - - private boolean useCoordinator() { - return Boolean.getBoolean("testing.containers.use-coordinator"); - } - - protected int getTestPort() { + protected static int getTestPort() { try { return ConfigProvider.getConfig().getValue("quarkus.http.test-port", Integer.class); } catch (Exception e) { return Integer.parseInt(System.getProperty("quarkus.http.test-port")); } } + + private static class DefaultHeaders { + private static final Map HEADERS = collectDefaultHeaders(); + + public static Map mutableCopy() { + return new LinkedHashMap<>(HEADERS); + } + + private static Map collectDefaultHeaders() { + final boolean useCoordinator = Boolean.getBoolean("testing.containers.use-coordinator"); + if (useCoordinator) { + return Map.of( + HttpConstants.AUTHENTICATION_TOKEN_HEADER_NAME, + getAuthToken(), + HttpConstants.EMBEDDING_AUTHENTICATION_TOKEN_HEADER_NAME, + CustomITEmbeddingProvider.TEST_API_KEY); + } else { + String credential = + "Cassandra:" + + Base64.getEncoder().encodeToString(getCassandraUsername().getBytes()) + + ":" + + Base64.getEncoder().encodeToString(getCassandraPassword().getBytes()); + return Map.of( + HttpConstants.AUTHENTICATION_TOKEN_HEADER_NAME, + credential, + HttpConstants.EMBEDDING_AUTHENTICATION_TOKEN_HEADER_NAME, + CustomITEmbeddingProvider.TEST_API_KEY); + } + } + } } From e318d5a8f7d796d9409693c4d2c0a5c11f70bfe8 Mon Sep 17 00:00:00 2001 From: Tatu Saloranta Date: Fri, 6 Sep 2024 17:26:46 -0700 Subject: [PATCH 17/19] Final touches wrt naming --- .../jsonapi/config/feature/ApiFeature.java | 19 ++++++++++++++++--- .../config/feature/FeaturesConfig.java | 5 +++++ 2 files changed, 21 insertions(+), 3 deletions(-) diff --git a/src/main/java/io/stargate/sgv2/jsonapi/config/feature/ApiFeature.java b/src/main/java/io/stargate/sgv2/jsonapi/config/feature/ApiFeature.java index 3f0ae12460..340847ec01 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/config/feature/ApiFeature.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/config/feature/ApiFeature.java @@ -1,7 +1,7 @@ package io.stargate.sgv2.jsonapi.config.feature; import com.fasterxml.jackson.annotation.JsonValue; -import io.stargate.sgv2.jsonapi.exception.ErrorCode; +import io.stargate.sgv2.jsonapi.exception.ErrorCodeV1; /** * Set of "Feature Flags" that can be used to enable/disable certain features in the Data API. @@ -9,17 +9,30 @@ * *

NOTE: although flag names are in upper case (like {@code TABLES}), the actual configuration * uses lower-case names (like {@code tables}) (with proper prefix). + * + *

Usage: Features may be enabled via configuration: see {@link FeaturesConfig}; if defined at + * that level, they are either enabled or disabled for all requests. If not defined (left as empty + * or {@code null}}, HTTP Request headers can be used to enable/disable features on per-request + * basis. Finally, if neither configuration nor request headers are used, {@link + * ApiFeature#enabledByDefault()} value is used. */ public enum ApiFeature { /** * API Tables feature flag: if enabled, the API will expose table-specific Namespace resource * commands, and support Collection commands on Tables. If disabled, those operations will fail - * with {@link ErrorCode#TABLE_FEATURE_NOT_ENABLED}. + * with {@link ErrorCodeV1#TABLE_FEATURE_NOT_ENABLED}. * *

If no configuration specified (config or request), the feature will be Disabled. */ TABLES("tables", false); + /** + * Prefix for HTTP headers used to override feature flags for specific requests: prepended before + * {@link #featureName}, so f.ex for {@link #TABLES} flag, the header name would be {@code + * Feature-Flag-tables}. + */ + public static final String HTTP_HEADER_PREFIX = "Feature-Flag-"; + private final String featureName; private final boolean enabledByDefault; @@ -36,7 +49,7 @@ public enum ApiFeature { "Internal error: 'featureName' must be lower-case, was: \"" + featureName + "\""); } this.featureName = featureName; - featureNameAsHeader = "x-stargate-feature-" + featureName; + featureNameAsHeader = HTTP_HEADER_PREFIX + featureName; this.enabledByDefault = enabledByDefault; } diff --git a/src/main/java/io/stargate/sgv2/jsonapi/config/feature/FeaturesConfig.java b/src/main/java/io/stargate/sgv2/jsonapi/config/feature/FeaturesConfig.java index 04b349cfbe..765f1d00c0 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/config/feature/FeaturesConfig.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/config/feature/FeaturesConfig.java @@ -6,6 +6,11 @@ /** * Configuration mapping for Data API Feature flags as read from main application configuration * (with possible property / sysenv overrides). + * + *

NOTE: actual keys in YAML or similar configuration file would be {@code + * stargate.feature.}, where {@code } comes from {@link + * ApiFeature#featureName()} (which is always lower-case). So, for example, to enable {@link + * ApiFeature#TABLES} feature, one would use: {@code stargate.feature.flags.tables} as the key. */ @ConfigMapping(prefix = "stargate.feature") public interface FeaturesConfig { From be9acd7d4d9316d7f682b17004b789fadc6e0e19 Mon Sep 17 00:00:00 2001 From: Tatu Saloranta Date: Fri, 6 Sep 2024 17:30:41 -0700 Subject: [PATCH 18/19] Further improve docs (comment) --- .../io/stargate/sgv2/jsonapi/config/feature/ApiFeatures.java | 5 ++++- src/main/resources/application.yaml | 2 +- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/src/main/java/io/stargate/sgv2/jsonapi/config/feature/ApiFeatures.java b/src/main/java/io/stargate/sgv2/jsonapi/config/feature/ApiFeatures.java index edee5fb802..5327ad6969 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/config/feature/ApiFeatures.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/config/feature/ApiFeatures.java @@ -6,7 +6,10 @@ /** * Accessor for combined state of feature flags; typically based on static configuration (with its - * overrides) and possible per-request settings. + * overrides) and possible per-request settings. For code that wants to check whether given feature + * is enabled or not, method to use is {@link ApiFeatures#isFeatureEnabled(ApiFeature)}. For details + * on how configuration settings and request headers are combined, see {@link ApiFeature} and {@link + * FeaturesConfig} */ public class ApiFeatures { private final Map fromConfig; diff --git a/src/main/resources/application.yaml b/src/main/resources/application.yaml index f44e54c6df..c67e7196cc 100644 --- a/src/main/resources/application.yaml +++ b/src/main/resources/application.yaml @@ -25,7 +25,7 @@ stargate: exception-mappers: enabled: false - feature: # See io.stargate.sgv2.jsonapi.config.feature.DataApiFeatureConfig + feature: # See io.stargate.sgv2.jsonapi.config.feature.FeaturesConfig flags: # Ok to leave out features that have no default value (enabled or disabled) tables: From 7ca1f3e540e15059d212dc20b65c82e599dfccf1 Mon Sep 17 00:00:00 2001 From: Tatu Saloranta Date: Mon, 9 Sep 2024 13:12:34 -0700 Subject: [PATCH 19/19] Last changes from code review --- CONFIGURATION.md | 6 ++-- .../jsonapi/config/feature/ApiFeature.java | 33 +++++++------------ .../jsonapi/config/feature/ApiFeatures.java | 2 +- 3 files changed, 15 insertions(+), 26 deletions(-) diff --git a/CONFIGURATION.md b/CONFIGURATION.md index 42ef5ee6ee..735351d0d0 100644 --- a/CONFIGURATION.md +++ b/CONFIGURATION.md @@ -82,6 +82,6 @@ Other Quarkus properties that are specifically relevant for the service: ## API Feature enabling configuration *Configuration for enabling Features, defined by [FeaturesConfig.java](src/main/java/io/stargate/sgv2/jsonapi/config/CommandLoggingConfig.java).* -| Property | Type | Default | Description | -|---------------------------------|-----------|---------|----------------------------------------------------------------------------------------------------------------------| -| `stargate.feature.flags.tables` | `boolean` | `null` | Setting it to `true` enables Tables functionality; `false` disables; leaving as `null` allowes per-request override. | +| Property | Type | Default | Description | +|---------------------------------|-----------|---------|---------------------------------------------------------------------------------------------------------------------| +| `stargate.feature.flags.tables` | `boolean` | `null` | Setting it to `true` enables Tables functionality; `false` disables; leaving as `null` allows per-request override.| diff --git a/src/main/java/io/stargate/sgv2/jsonapi/config/feature/ApiFeature.java b/src/main/java/io/stargate/sgv2/jsonapi/config/feature/ApiFeature.java index 340847ec01..8d98f05091 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/config/feature/ApiFeature.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/config/feature/ApiFeature.java @@ -12,45 +12,43 @@ * *

Usage: Features may be enabled via configuration: see {@link FeaturesConfig}; if defined at * that level, they are either enabled or disabled for all requests. If not defined (left as empty - * or {@code null}}, HTTP Request headers can be used to enable/disable features on per-request - * basis. Finally, if neither configuration nor request headers are used, {@link - * ApiFeature#enabledByDefault()} value is used. + * or {@code null}), HTTP Request headers can be used to enable/disable features on per-request + * basis. Finally, if neither configuration nor request headers are used, feature is disabled. */ public enum ApiFeature { /** * API Tables feature flag: if enabled, the API will expose table-specific Namespace resource - * commands, and support Collection commands on Tables. If disabled, those operations will fail - * with {@link ErrorCodeV1#TABLE_FEATURE_NOT_ENABLED}. - * - *

If no configuration specified (config or request), the feature will be Disabled. + * commands, and support commands on Tables. If disabled, those operations will fail with {@link + * ErrorCodeV1#TABLE_FEATURE_NOT_ENABLED}. */ - TABLES("tables", false); + TABLES("tables"); /** * Prefix for HTTP headers used to override feature flags for specific requests: prepended before - * {@link #featureName}, so f.ex for {@link #TABLES} flag, the header name would be {@code + * {@link #featureName()}, so f.ex for {@link #TABLES} flag, the header name would be {@code * Feature-Flag-tables}. */ public static final String HTTP_HEADER_PREFIX = "Feature-Flag-"; + /** + * Feature flag name, in lower-case with hyphens (aka "kebab case"), used for as serialization + * (JSON and YAML config files) as well as for constructing HTTP header names. + */ private final String featureName; - private final boolean enabledByDefault; - /** * HTTP header name to be used to override the feature flag for a specific request: lower-case, * prefixed with "x-stargate-feature-"; lookup case-insensitive. */ private final String featureNameAsHeader; - ApiFeature(String featureName, boolean enabledByDefault) { + ApiFeature(String featureName) { if (!featureName.equals(featureName.toLowerCase())) { throw new IllegalStateException( "Internal error: 'featureName' must be lower-case, was: \"" + featureName + "\""); } this.featureName = featureName; featureNameAsHeader = HTTP_HEADER_PREFIX + featureName; - this.enabledByDefault = enabledByDefault; } @JsonValue // for Jackson to serialize as lower-case @@ -61,13 +59,4 @@ public String featureName() { public String httpHeaderName() { return featureNameAsHeader; } - - /** - * Default state of the feature flag, if not explicitly configured (either by config or request). - * - * @return {@code true} if the feature is enabled by default, {@code false} otherwise. - */ - public boolean enabledByDefault() { - return enabledByDefault; - } } diff --git a/src/main/java/io/stargate/sgv2/jsonapi/config/feature/ApiFeatures.java b/src/main/java/io/stargate/sgv2/jsonapi/config/feature/ApiFeatures.java index 5327ad6969..f31bafdccf 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/config/feature/ApiFeatures.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/config/feature/ApiFeatures.java @@ -44,6 +44,6 @@ public boolean isFeatureEnabled(ApiFeature flag) { b = httpHeaders.getHeaderAsBoolean(flag.httpHeaderName()); } } - return (b == null) ? flag.enabledByDefault() : b.booleanValue(); + return (b != null) && b.booleanValue(); } }