From 4f5805fc2cfa2b637558f1243e4e55ae0361569a Mon Sep 17 00:00:00 2001 From: Tatu Saloranta Date: Wed, 10 Jan 2024 13:15:00 -0800 Subject: [PATCH 01/13] Add no-index support for Shredder --- .../api/model/command/CommandContext.java | 25 ++++++++++---- .../jsonapi/api/v1/CollectionResource.java | 7 +--- .../executor/CollectionSettings.java | 33 ++++++++++++++++++- .../service/projection/DocumentProjector.java | 10 ++++-- .../operation/TestEmbeddingService.java | 10 ++++-- .../model/impl/FindOperationTest.java | 12 ++++++- .../model/impl/InsertOperationTest.java | 12 ++++++- .../impl/ReadAndUpdateOperationTest.java | 12 ++++++- 8 files changed, 101 insertions(+), 20 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 a384cca9f0..3624bfc2bd 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 @@ -7,6 +7,7 @@ import io.stargate.sgv2.jsonapi.service.cqldriver.executor.CollectionSettings; import io.stargate.sgv2.jsonapi.service.embedding.DataVectorizer; import io.stargate.sgv2.jsonapi.service.embedding.operation.EmbeddingService; +import io.stargate.sgv2.jsonapi.service.projection.DocumentProjector; import java.util.List; /** @@ -14,21 +15,21 @@ * * @param namespace The name of the namespace. * @param collection The name of the collection. - * @param isVectorEnabled Whether the vector is enabled for the collection - * @param similarityFunction The similarity function used for indexing the vector + * @param collectionSettings Settings for the collection, if Collection-specific command; if not, + * "empty" Settings {see CollectionSettings#empty()}. */ public record CommandContext( String namespace, String collection, - boolean isVectorEnabled, - CollectionSettings.SimilarityFunction similarityFunction, + CollectionSettings collectionSettings, EmbeddingService embeddingService) { public CommandContext(String namespace, String collection) { - this(namespace, collection, false, null, null); + this(namespace, collection, CollectionSettings.empty(), null); } - private static final CommandContext EMPTY = new CommandContext(null, null, false, null, null); + private static final CommandContext EMPTY = + new CommandContext(null, null, CollectionSettings.empty(), null); /** * @return Returns empty command context, having both {@link #namespace} and {@link #collection} @@ -38,6 +39,18 @@ public static CommandContext empty() { return EMPTY; } + public CollectionSettings.SimilarityFunction similarityFunction() { + return collectionSettings.similarityFunction(); + } + + public boolean isVectorEnabled() { + return collectionSettings.vectorEnabled(); + } + + public DocumentProjector indexingProjector() { + return collectionSettings.indexingProjector(); + } + public void tryVectorize(JsonNodeFactory nodeFactory, List documents) { new DataVectorizer(embeddingService(), nodeFactory).vectorize(documents); } 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 94483526e1..7bdd422c61 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 @@ -179,12 +179,7 @@ else if (error instanceof JsonApiException jsonApiException) { } CommandContext commandContext = - new CommandContext( - namespace, - collection, - collectionProperty.vectorEnabled(), - collectionProperty.similarityFunction(), - embeddingService); + new CommandContext(namespace, collection, collectionProperty, embeddingService); // call processor return meteredCommandProcessor.processCommand(commandContext, command); diff --git a/src/main/java/io/stargate/sgv2/jsonapi/service/cqldriver/executor/CollectionSettings.java b/src/main/java/io/stargate/sgv2/jsonapi/service/cqldriver/executor/CollectionSettings.java index 5ce3af26b5..136cc3eb8e 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/service/cqldriver/executor/CollectionSettings.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/service/cqldriver/executor/CollectionSettings.java @@ -10,13 +10,16 @@ import com.fasterxml.jackson.core.JsonProcessingException; import com.fasterxml.jackson.databind.JsonNode; import com.fasterxml.jackson.databind.ObjectMapper; +import com.google.common.base.Suppliers; import io.stargate.sgv2.jsonapi.config.constants.DocumentConstants; import io.stargate.sgv2.jsonapi.exception.ErrorCode; import io.stargate.sgv2.jsonapi.exception.JsonApiException; +import io.stargate.sgv2.jsonapi.service.projection.DocumentProjector; import java.util.HashSet; import java.util.Map; import java.util.Optional; import java.util.Set; +import java.util.function.Supplier; /** * Refactored as seperate class that represent a collection property. @@ -37,7 +40,35 @@ public record CollectionSettings( String modelName, IndexingConfig indexingConfig) { - public record IndexingConfig(Set allowed, Set denied) { + private static final CollectionSettings EMPTY = + new CollectionSettings("", false, 0, null, null, null, null); + + public static CollectionSettings empty() { + return EMPTY; + } + + public DocumentProjector indexingProjector() { + // IndexingConfig null if no indexing definitions: default, index all: + if (indexingConfig == null) { + return DocumentProjector.identityProjector(); + } + // otherwise get lazily initialized indexing projector from config + return indexingConfig.indexingProjector(); + } + + public record IndexingConfig( + Set allowed, Set denied, Supplier indexedProject) { + public IndexingConfig(Set allowed, Set denied) { + this( + allowed, + denied, + Suppliers.memoize(() -> DocumentProjector.createForIndexing(allowed, denied))); + } + + public DocumentProjector indexingProjector() { + return indexedProject.get(); + } + public static IndexingConfig fromJson(JsonNode jsonNode) { Set allowed = new HashSet<>(); Set denied = new HashSet<>(); diff --git a/src/main/java/io/stargate/sgv2/jsonapi/service/projection/DocumentProjector.java b/src/main/java/io/stargate/sgv2/jsonapi/service/projection/DocumentProjector.java index 886b89c21f..bbf322b08b 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/service/projection/DocumentProjector.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/service/projection/DocumentProjector.java @@ -9,6 +9,7 @@ import java.util.ArrayList; import java.util.List; import java.util.Objects; +import java.util.Set; /** * Helper class that implements functionality needed to support projections on documents fetched via @@ -48,7 +49,7 @@ public static DocumentProjector createFromDefinition( JsonNode projectionDefinition, boolean includeSimilarity) { if (projectionDefinition == null) { if (includeSimilarity) { - return IDENTITY_PROJECTOR_WITH_SIMILARITY; + return identityProjectorWithSimilarity(); } else { return identityProjector(); } @@ -63,11 +64,16 @@ public static DocumentProjector createFromDefinition( return PathCollector.collectPaths(projectionDefinition, includeSimilarity).buildProjector(); } + public static DocumentProjector createForIndexing(Set allowed, Set denied) { + // !!! TO IMPLEMENT + return identityProjector(); + } + public static DocumentProjector identityProjector() { return IDENTITY_PROJECTOR; } - public static DocumentProjector getIdentityProjectorWithSimilarity() { + public static DocumentProjector identityProjectorWithSimilarity() { return IDENTITY_PROJECTOR; } diff --git a/src/test/java/io/stargate/sgv2/jsonapi/service/embedding/operation/TestEmbeddingService.java b/src/test/java/io/stargate/sgv2/jsonapi/service/embedding/operation/TestEmbeddingService.java index 26efafbbb0..edf7ac26d5 100644 --- a/src/test/java/io/stargate/sgv2/jsonapi/service/embedding/operation/TestEmbeddingService.java +++ b/src/test/java/io/stargate/sgv2/jsonapi/service/embedding/operation/TestEmbeddingService.java @@ -11,8 +11,14 @@ public class TestEmbeddingService implements EmbeddingService { new CommandContext( "namespace", "collection", - true, - CollectionSettings.SimilarityFunction.COSINE, + new CollectionSettings( + "collection", + true, + 3, + CollectionSettings.SimilarityFunction.COSINE, + null, + null, + null), new TestEmbeddingService()); @Override diff --git a/src/test/java/io/stargate/sgv2/jsonapi/service/operation/model/impl/FindOperationTest.java b/src/test/java/io/stargate/sgv2/jsonapi/service/operation/model/impl/FindOperationTest.java index 69a2bae266..823f99942d 100644 --- a/src/test/java/io/stargate/sgv2/jsonapi/service/operation/model/impl/FindOperationTest.java +++ b/src/test/java/io/stargate/sgv2/jsonapi/service/operation/model/impl/FindOperationTest.java @@ -54,7 +54,17 @@ public class FindOperationTest extends OperationTestBase { private final CommandContext VECTOR_COMMAND_CONTEXT = new CommandContext( - KEYSPACE_NAME, COLLECTION_NAME, true, CollectionSettings.SimilarityFunction.COSINE, null); + KEYSPACE_NAME, + COLLECTION_NAME, + new CollectionSettings( + COLLECTION_NAME, + true, + -1, + CollectionSettings.SimilarityFunction.COSINE, + null, + null, + null), + null); private final ColumnDefinitions KEY_TXID_JSON_COLUMNS = buildColumnDefs( diff --git a/src/test/java/io/stargate/sgv2/jsonapi/service/operation/model/impl/InsertOperationTest.java b/src/test/java/io/stargate/sgv2/jsonapi/service/operation/model/impl/InsertOperationTest.java index 9b376608e8..a71f73df38 100644 --- a/src/test/java/io/stargate/sgv2/jsonapi/service/operation/model/impl/InsertOperationTest.java +++ b/src/test/java/io/stargate/sgv2/jsonapi/service/operation/model/impl/InsertOperationTest.java @@ -49,7 +49,17 @@ public class InsertOperationTest extends OperationTestBase { private final CommandContext COMMAND_CONTEXT_VECTOR = new CommandContext( - KEYSPACE_NAME, COLLECTION_NAME, true, CollectionSettings.SimilarityFunction.COSINE, null); + KEYSPACE_NAME, + COLLECTION_NAME, + new CollectionSettings( + COLLECTION_NAME, + true, + -1, + CollectionSettings.SimilarityFunction.COSINE, + null, + null, + null), + null); private final ColumnDefinitions COLUMNS_APPLIED = buildColumnDefs(TestColumn.ofBoolean("[applied]")); diff --git a/src/test/java/io/stargate/sgv2/jsonapi/service/operation/model/impl/ReadAndUpdateOperationTest.java b/src/test/java/io/stargate/sgv2/jsonapi/service/operation/model/impl/ReadAndUpdateOperationTest.java index 8793cbd65e..4085443a88 100644 --- a/src/test/java/io/stargate/sgv2/jsonapi/service/operation/model/impl/ReadAndUpdateOperationTest.java +++ b/src/test/java/io/stargate/sgv2/jsonapi/service/operation/model/impl/ReadAndUpdateOperationTest.java @@ -60,7 +60,17 @@ public class ReadAndUpdateOperationTest extends OperationTestBase { private static final CommandContext COMMAND_VECTOR_CONTEXT = new CommandContext( - KEYSPACE_NAME, COLLECTION_NAME, true, CollectionSettings.SimilarityFunction.COSINE, null); + KEYSPACE_NAME, + COLLECTION_NAME, + new CollectionSettings( + COLLECTION_NAME, + true, + -1, + CollectionSettings.SimilarityFunction.COSINE, + null, + null, + null), + null); @Inject Shredder shredder; @Inject ObjectMapper objectMapper; From 25682387cbdd009c4df802da170e2d4be22188bf Mon Sep 17 00:00:00 2001 From: Tatu Saloranta Date: Wed, 10 Jan 2024 13:45:43 -0800 Subject: [PATCH 02/13] minor fix --- .../sgv2/jsonapi/service/projection/DocumentProjector.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/io/stargate/sgv2/jsonapi/service/projection/DocumentProjector.java b/src/main/java/io/stargate/sgv2/jsonapi/service/projection/DocumentProjector.java index bbf322b08b..d989db7450 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/service/projection/DocumentProjector.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/service/projection/DocumentProjector.java @@ -74,7 +74,7 @@ public static DocumentProjector identityProjector() { } public static DocumentProjector identityProjectorWithSimilarity() { - return IDENTITY_PROJECTOR; + return IDENTITY_PROJECTOR_WITH_SIMILARITY; } public boolean isInclusion() { From 466f238fd71dc6c058d9805f6064d34f7b7ac5bc Mon Sep 17 00:00:00 2001 From: Tatu Saloranta Date: Wed, 10 Jan 2024 14:35:45 -0800 Subject: [PATCH 03/13] Fix integration test wrt new defaults for "ordered" insert (false) --- .../io/stargate/sgv2/jsonapi/api/v1/InsertIntegrationTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/java/io/stargate/sgv2/jsonapi/api/v1/InsertIntegrationTest.java b/src/test/java/io/stargate/sgv2/jsonapi/api/v1/InsertIntegrationTest.java index 01b58a619d..30bed90692 100644 --- a/src/test/java/io/stargate/sgv2/jsonapi/api/v1/InsertIntegrationTest.java +++ b/src/test/java/io/stargate/sgv2/jsonapi/api/v1/InsertIntegrationTest.java @@ -1398,7 +1398,7 @@ public void emptyOptionsAllowed() { .post(CollectionResource.BASE_PATH, namespaceName, collectionName) .then() .statusCode(200) - .body("status.insertedIds", contains("doc4", "doc5")) + .body("status.insertedIds", containsInAnyOrder("doc4", "doc5")) .body("data", is(nullValue())) .body("errors", is(nullValue())); } From d65e9b7abb46e2e3ff6a64f7e25dc8d79529b834 Mon Sep 17 00:00:00 2001 From: Tatu Saloranta Date: Wed, 10 Jan 2024 14:44:52 -0800 Subject: [PATCH 04/13] Wire in projector, but always pass "identity projector" that shouldn't change anything --- .../operation/model/impl/ReadAndUpdateOperation.java | 6 +++++- .../resolver/model/impl/InsertManyCommandResolver.java | 4 +++- .../resolver/model/impl/InsertOneCommandResolver.java | 3 ++- .../sgv2/jsonapi/service/shredding/Shredder.java | 10 ++++++++++ 4 files changed, 20 insertions(+), 3 deletions(-) diff --git a/src/main/java/io/stargate/sgv2/jsonapi/service/operation/model/impl/ReadAndUpdateOperation.java b/src/main/java/io/stargate/sgv2/jsonapi/service/operation/model/impl/ReadAndUpdateOperation.java index f173401dc0..b4e09bf91e 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/service/operation/model/impl/ReadAndUpdateOperation.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/service/operation/model/impl/ReadAndUpdateOperation.java @@ -174,7 +174,11 @@ private Uni processUpdate( } final WritableShreddedDocument writableShreddedDocument = - shredder().shred(documentUpdaterResponse.document(), readDocument.txnId()); + shredder() + .shred( + documentUpdaterResponse.document(), + readDocument.txnId(), + commandContext().indexingProjector()); // Have to do this because shredder adds _id field to the document if it doesn't exist JsonNode updatedDocument = writableShreddedDocument.docJsonNode(); diff --git a/src/main/java/io/stargate/sgv2/jsonapi/service/resolver/model/impl/InsertManyCommandResolver.java b/src/main/java/io/stargate/sgv2/jsonapi/service/resolver/model/impl/InsertManyCommandResolver.java index 5d0f204cc0..0a6b7ec778 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/service/resolver/model/impl/InsertManyCommandResolver.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/service/resolver/model/impl/InsertManyCommandResolver.java @@ -5,6 +5,7 @@ import io.stargate.sgv2.jsonapi.api.model.command.impl.InsertManyCommand; import io.stargate.sgv2.jsonapi.service.operation.model.Operation; import io.stargate.sgv2.jsonapi.service.operation.model.impl.InsertOperation; +import io.stargate.sgv2.jsonapi.service.projection.DocumentProjector; import io.stargate.sgv2.jsonapi.service.resolver.model.CommandResolver; import io.stargate.sgv2.jsonapi.service.shredding.Shredder; import io.stargate.sgv2.jsonapi.service.shredding.model.WritableShreddedDocument; @@ -35,8 +36,9 @@ public Operation resolveCommand(CommandContext ctx, InsertManyCommand command) { // Vectorize documents ctx.tryVectorize(objectMapper.getNodeFactory(), command.documents()); + final DocumentProjector projection = ctx.indexingProjector(); final List shreddedDocuments = - command.documents().stream().map(shredder::shred).toList(); + command.documents().stream().map(doc -> shredder.shred(doc, null, projection)).toList(); // resolve ordered InsertManyCommand.Options options = command.options(); diff --git a/src/main/java/io/stargate/sgv2/jsonapi/service/resolver/model/impl/InsertOneCommandResolver.java b/src/main/java/io/stargate/sgv2/jsonapi/service/resolver/model/impl/InsertOneCommandResolver.java index 3747f220e7..0687ecfc33 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/service/resolver/model/impl/InsertOneCommandResolver.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/service/resolver/model/impl/InsertOneCommandResolver.java @@ -34,7 +34,8 @@ public Class getCommandClass() { public Operation resolveCommand(CommandContext ctx, InsertOneCommand command) { // Vectorize document ctx.tryVectorize(objectMapper.getNodeFactory(), List.of(command.document())); - WritableShreddedDocument shreddedDocument = shredder.shred(command.document()); + WritableShreddedDocument shreddedDocument = + shredder.shred(command.document(), null, ctx.indexingProjector()); return new InsertOperation(ctx, shreddedDocument); } } diff --git a/src/main/java/io/stargate/sgv2/jsonapi/service/shredding/Shredder.java b/src/main/java/io/stargate/sgv2/jsonapi/service/shredding/Shredder.java index 398e7fee4c..3ed7760177 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/service/shredding/Shredder.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/service/shredding/Shredder.java @@ -8,6 +8,7 @@ import io.stargate.sgv2.jsonapi.config.constants.DocumentConstants; import io.stargate.sgv2.jsonapi.exception.ErrorCode; import io.stargate.sgv2.jsonapi.exception.JsonApiException; +import io.stargate.sgv2.jsonapi.service.projection.DocumentProjector; import io.stargate.sgv2.jsonapi.service.shredding.model.DocumentId; import io.stargate.sgv2.jsonapi.service.shredding.model.WritableShreddedDocument; import io.stargate.sgv2.jsonapi.util.JsonUtil; @@ -53,6 +54,10 @@ public WritableShreddedDocument shred(JsonNode document) { } public WritableShreddedDocument shred(JsonNode doc, UUID txId) { + return shred(doc, txId, DocumentProjector.identityProjector()); + } + + public WritableShreddedDocument shred(JsonNode doc, UUID txId, DocumentProjector indexProjector) { // Although we could otherwise allow non-Object documents, requirement // to have the _id (or at least place for it) means we cannot allow that. if (!doc.isObject()) { @@ -83,6 +88,11 @@ public WritableShreddedDocument shred(JsonNode doc, UUID txId) { // And then we can validate the document size validateDocumentSize(documentLimits, docJson); + // Before shredding, may need to drop "non-indexed" fields: + if (indexProjector != null) { + indexProjector.applyProjection(docWithId); + } + final WritableShreddedDocument.Builder b = WritableShreddedDocument.builder(docId, txId, docJson, docWithId); From 4d105cd6bde3c521827a946f8172c8e73606c9bb Mon Sep 17 00:00:00 2001 From: Tatu Saloranta Date: Wed, 10 Jan 2024 15:14:02 -0800 Subject: [PATCH 05/13] Fix the other failing test wrt non-ordered inserts --- .../io/stargate/sgv2/jsonapi/api/v1/InsertIntegrationTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/java/io/stargate/sgv2/jsonapi/api/v1/InsertIntegrationTest.java b/src/test/java/io/stargate/sgv2/jsonapi/api/v1/InsertIntegrationTest.java index 30bed90692..e24244cf38 100644 --- a/src/test/java/io/stargate/sgv2/jsonapi/api/v1/InsertIntegrationTest.java +++ b/src/test/java/io/stargate/sgv2/jsonapi/api/v1/InsertIntegrationTest.java @@ -1312,7 +1312,7 @@ public void withDifferentTypes() { .post(CollectionResource.BASE_PATH, namespaceName, collectionName) .then() .statusCode(200) - .body("status.insertedIds", contains("5", 5)) + .body("status.insertedIds", containsInAnyOrder("5", 5)) .body("data", is(nullValue())) .body("errors", is(nullValue())); From 29e3f8850b1274636c8890becfd2d954961b2f5c Mon Sep 17 00:00:00 2001 From: Tatu Saloranta Date: Wed, 10 Jan 2024 15:33:53 -0800 Subject: [PATCH 06/13] MInor additional work --- .../service/projection/DocumentProjector.java | 22 ++++++++++++++++++- .../service/projection/ProjectionLayer.java | 6 +++-- 2 files changed, 25 insertions(+), 3 deletions(-) diff --git a/src/main/java/io/stargate/sgv2/jsonapi/service/projection/DocumentProjector.java b/src/main/java/io/stargate/sgv2/jsonapi/service/projection/DocumentProjector.java index d989db7450..97e34e555b 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/service/projection/DocumentProjector.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/service/projection/DocumentProjector.java @@ -7,6 +7,7 @@ import io.stargate.sgv2.jsonapi.exception.JsonApiException; import java.math.BigDecimal; import java.util.ArrayList; +import java.util.Collections; import java.util.List; import java.util.Objects; import java.util.Set; @@ -65,7 +66,26 @@ public static DocumentProjector createFromDefinition( } public static DocumentProjector createForIndexing(Set allowed, Set denied) { - // !!! TO IMPLEMENT + // Sets are expected to be validated to have one of 3 main cases: + // 1. Non-empty "allowed" (but empty/null "denied") -> build inclusion projection + // 2. Non-empty "denied" (but empty/null "allowed") -> build exclusion projection + // 3. Empty/null "allowed" and "denied" -> return identity projection + // We need not (and should not) do further validation here + if (allowed != null && !allowed.isEmpty()) { + return new DocumentProjector( + ProjectionLayer.buildLayers(allowed, null, false), true, false); + } + if (denied != null && !denied.isEmpty()) { + // One special case: single "*" entry for exclusions means "exclude add" (that is, include nothing). + // This is reverse of identity projector. + if (denied.size() == 1 && denied.contains("*")) { + // Basically inclusion projector with nothing to include + return new DocumentProjector( + ProjectionLayer.buildLayers(Collections.emptySet(), null, false), true, false); + } + return new DocumentProjector( + ProjectionLayer.buildLayers(allowed, null, false), false, false); + } return identityProjector(); } diff --git a/src/main/java/io/stargate/sgv2/jsonapi/service/projection/ProjectionLayer.java b/src/main/java/io/stargate/sgv2/jsonapi/service/projection/ProjectionLayer.java index 7c8fe25cee..ca917e3e1d 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/service/projection/ProjectionLayer.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/service/projection/ProjectionLayer.java @@ -59,8 +59,10 @@ public static ProjectionLayer buildLayers( } // Slices similar to path but processed differently (and while "exclusions" // in a way do not count as ones wrt compatibility) - for (SliceDef slice : slices) { - buildSlicer(slice, root); + if (slices != null) { + for (SliceDef slice : slices) { + buildSlicer(slice, root); + } } // May need to add doc-id inclusion/exclusion as well From 57f3a0596ea7eda8a2a49f1fa41958e0f7c7abac Mon Sep 17 00:00:00 2001 From: Tatu Saloranta Date: Wed, 10 Jan 2024 15:36:57 -0800 Subject: [PATCH 07/13] mvn fmt:format --- .../jsonapi/service/projection/DocumentProjector.java | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/src/main/java/io/stargate/sgv2/jsonapi/service/projection/DocumentProjector.java b/src/main/java/io/stargate/sgv2/jsonapi/service/projection/DocumentProjector.java index 97e34e555b..76c3aa7774 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/service/projection/DocumentProjector.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/service/projection/DocumentProjector.java @@ -72,19 +72,18 @@ public static DocumentProjector createForIndexing(Set allowed, Set return identity projection // We need not (and should not) do further validation here if (allowed != null && !allowed.isEmpty()) { - return new DocumentProjector( - ProjectionLayer.buildLayers(allowed, null, false), true, false); + return new DocumentProjector(ProjectionLayer.buildLayers(allowed, null, false), true, false); } if (denied != null && !denied.isEmpty()) { - // One special case: single "*" entry for exclusions means "exclude add" (that is, include nothing). + // One special case: single "*" entry for exclusions means "exclude add" (that is, include + // nothing). // This is reverse of identity projector. if (denied.size() == 1 && denied.contains("*")) { // Basically inclusion projector with nothing to include return new DocumentProjector( - ProjectionLayer.buildLayers(Collections.emptySet(), null, false), true, false); + ProjectionLayer.buildLayers(Collections.emptySet(), null, false), true, false); } - return new DocumentProjector( - ProjectionLayer.buildLayers(allowed, null, false), false, false); + return new DocumentProjector(ProjectionLayer.buildLayers(allowed, null, false), false, false); } return identityProjector(); } From da61890a7e0a7e9452e2436118e5dd22911b7c40 Mon Sep 17 00:00:00 2001 From: Tatu Saloranta Date: Wed, 10 Jan 2024 15:57:44 -0800 Subject: [PATCH 08/13] First somewhat working version, with first trivial tests --- .../service/projection/DocumentProjector.java | 2 +- .../DocumentProjectorForIndexingTest.java | 74 +++++++++++++++++++ 2 files changed, 75 insertions(+), 1 deletion(-) create mode 100644 src/test/java/io/stargate/sgv2/jsonapi/service/projection/DocumentProjectorForIndexingTest.java diff --git a/src/main/java/io/stargate/sgv2/jsonapi/service/projection/DocumentProjector.java b/src/main/java/io/stargate/sgv2/jsonapi/service/projection/DocumentProjector.java index 76c3aa7774..829c77f089 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/service/projection/DocumentProjector.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/service/projection/DocumentProjector.java @@ -83,7 +83,7 @@ public static DocumentProjector createForIndexing(Set allowed, Set allows, String inputDoc, String expectedDoc) { + DocumentProjector projector = + DocumentProjector.createForIndexing(new LinkedHashSet<>(allows), Collections.emptySet()); + try { + JsonNode input = objectMapper.readTree(inputDoc); + projector.applyProjection(input); + assertThat(input).isEqualTo(objectMapper.readTree(expectedDoc)); + } catch (IOException e) { + throw new RuntimeException(e); + } + } + + private void assertDenyProjection(List denies, String inputDoc, String expectedDoc) { + DocumentProjector projector = + DocumentProjector.createForIndexing(Collections.emptySet(), new LinkedHashSet<>(denies)); + try { + JsonNode input = objectMapper.readTree(inputDoc); + projector.applyProjection(input); + assertThat(input).isEqualTo(objectMapper.readTree(expectedDoc)); + } catch (IOException e) { + throw new RuntimeException(e); + } + } +} From ce8c165a0674ad85b845a8dfa9ccb432902076ba Mon Sep 17 00:00:00 2001 From: Tatu Saloranta Date: Wed, 10 Jan 2024 16:41:53 -0800 Subject: [PATCH 09/13] Add more tests --- .../service/projection/DocumentProjector.java | 18 ++++-- .../DocumentProjectorForIndexingTest.java | 63 +++++++++++++++++++ 2 files changed, 77 insertions(+), 4 deletions(-) diff --git a/src/main/java/io/stargate/sgv2/jsonapi/service/projection/DocumentProjector.java b/src/main/java/io/stargate/sgv2/jsonapi/service/projection/DocumentProjector.java index 829c77f089..5de15dbc42 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/service/projection/DocumentProjector.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/service/projection/DocumentProjector.java @@ -70,21 +70,31 @@ public static DocumentProjector createForIndexing(Set allowed, Set build inclusion projection // 2. Non-empty "denied" (but empty/null "allowed") -> build exclusion projection // 3. Empty/null "allowed" and "denied" -> return identity projection - // We need not (and should not) do further validation here + // as well as 2 special cases: + // 4. Empty "allowed" and single "*" entry for "denied" -> return exclude-all projection + // 5. Empty "deny" and single "*" entry for "allowed" -> return include-all ("identity") + // projection + // We need not (and should not) do further validation here. + // Note that (5) is effectively same as (3) and included for sake of uniformity if (allowed != null && !allowed.isEmpty()) { + // (special) Case 5: + if (allowed.size() == 1 && allowed.contains("*")) { + return identityProjector(); + } + // Case 1: inclusion-based projection return new DocumentProjector(ProjectionLayer.buildLayers(allowed, null, false), true, false); } if (denied != null && !denied.isEmpty()) { - // One special case: single "*" entry for exclusions means "exclude add" (that is, include - // nothing). - // This is reverse of identity projector. + // (special) Case 4: if (denied.size() == 1 && denied.contains("*")) { // Basically inclusion projector with nothing to include return new DocumentProjector( ProjectionLayer.buildLayers(Collections.emptySet(), null, false), true, false); } + // Case 2: exclusion-based projection return new DocumentProjector(ProjectionLayer.buildLayers(denied, null, false), false, false); } + // Case 3: include-all (identity) projection return identityProjector(); } diff --git a/src/test/java/io/stargate/sgv2/jsonapi/service/projection/DocumentProjectorForIndexingTest.java b/src/test/java/io/stargate/sgv2/jsonapi/service/projection/DocumentProjectorForIndexingTest.java index 0c899df126..4280d5952c 100644 --- a/src/test/java/io/stargate/sgv2/jsonapi/service/projection/DocumentProjectorForIndexingTest.java +++ b/src/test/java/io/stargate/sgv2/jsonapi/service/projection/DocumentProjectorForIndexingTest.java @@ -31,6 +31,25 @@ public void testSimpleRootAllow() { { "b": 6, "d": 8 } """); } + + @Test + public void testSimpleNestedAllow() { + assertAllowProjection( + Arrays.asList("a.y", "c"), + """ + { + "a": { "x":1, "y":2 }, + "b": { "x":3, "y":4 }, + "c": { "x":2, "y":3 } + } + """, + """ + { + "a": { "y":2 }, + "c": { "x":2, "y":3 } + } + """); + } } @Nested @@ -46,6 +65,50 @@ public void testSimpleRootDeny() { { "a": 5, "c": 7 } """); } + + @Test + public void testSimpleNestedDeny() { + assertDenyProjection( + Arrays.asList("a.y", "c"), + """ + { + "a": { "x":1, "y":2 }, + "b": { "x":3, "y":4 }, + "c": { "x":2, "y":3 } + } + """, + """ + { + "a": { "x":1 }, + "b": { "x":3, "y":4 } + } + """); + } + } + + @Nested + class SpecialCases { + @Test + public void testAllowAll() { + final String DOC = + """ + { "a": 5, "b": { "enabled": true}, "c": 7, "d": [ { "value": 42} ] } + """; + // First with empty Sets: + assertAllowProjection(Arrays.asList(), DOC, DOC); + // And then "*" notation + assertAllowProjection(Arrays.asList("*"), DOC, DOC); + } + + @Test + public void testDenyAll() { + final String DOC = + """ + { "a": 5, "b": { "enabled": true}, "c": 7, "d": [ { "value": 42} ] } + """; + // Only "*" notation available + assertDenyProjection(Arrays.asList("*"), DOC, "{ }"); + } } private void assertAllowProjection(List allows, String inputDoc, String expectedDoc) { From 1ff103e32d14d1cb02aac93d62caa87e4a0e7a96 Mon Sep 17 00:00:00 2001 From: Tatu Saloranta Date: Wed, 10 Jan 2024 17:40:21 -0800 Subject: [PATCH 10/13] Add support for overlapping paths (allowed for index projections but NOT for query projections) --- .../service/projection/DocumentProjector.java | 10 +- .../service/projection/ProjectionLayer.java | 77 ++++++++++----- .../DocumentProjectorForIndexingTest.java | 94 ++++++++++++++++--- 3 files changed, 140 insertions(+), 41 deletions(-) diff --git a/src/main/java/io/stargate/sgv2/jsonapi/service/projection/DocumentProjector.java b/src/main/java/io/stargate/sgv2/jsonapi/service/projection/DocumentProjector.java index 5de15dbc42..ed6ff57d6e 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/service/projection/DocumentProjector.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/service/projection/DocumentProjector.java @@ -82,17 +82,17 @@ public static DocumentProjector createForIndexing(Set allowed, Set 0) { // inclusion-based projection // doc-id included unless explicitly excluded return new DocumentProjector( - ProjectionLayer.buildLayers(paths, slices, !Boolean.FALSE.equals(idInclusion)), + ProjectionLayer.buildLayersNoOverlap(paths, slices, !Boolean.FALSE.equals(idInclusion)), true, includeSimilarityScore); } else { // exclusion-based // doc-id excluded only if explicitly excluded return new DocumentProjector( - ProjectionLayer.buildLayers(paths, slices, Boolean.FALSE.equals(idInclusion)), + ProjectionLayer.buildLayersNoOverlap(paths, slices, Boolean.FALSE.equals(idInclusion)), false, includeSimilarityScore); } diff --git a/src/main/java/io/stargate/sgv2/jsonapi/service/projection/ProjectionLayer.java b/src/main/java/io/stargate/sgv2/jsonapi/service/projection/ProjectionLayer.java index ca917e3e1d..e8b5f14441 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/service/projection/ProjectionLayer.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/service/projection/ProjectionLayer.java @@ -7,6 +7,7 @@ import io.stargate.sgv2.jsonapi.exception.ErrorCode; import io.stargate.sgv2.jsonapi.exception.JsonApiException; import java.util.Collection; +import java.util.Collections; import java.util.HashMap; import java.util.List; import java.util.Map; @@ -49,55 +50,76 @@ class ProjectionLayer { nextLayers = null; } - public static ProjectionLayer buildLayers( + public static ProjectionLayer buildLayersNoOverlap( Collection dotPaths, List slices, boolean addDocId) { + return buildLayers(dotPaths, slices, addDocId, true); + } + + public static ProjectionLayer buildLayersOverlapOk(Collection dotPaths) { + return buildLayers(dotPaths, Collections.emptyList(), false, false); + } + + private static ProjectionLayer buildLayers( + Collection dotPaths, List slices, boolean addDocId, boolean failOnOverlap) { // Root is always branch (not terminal): ProjectionLayer root = new ProjectionLayer("", false); for (String fullPath : dotPaths) { String[] segments = DOT.split(fullPath); - buildPath(fullPath, root, segments); + buildPath(failOnOverlap, fullPath, root, segments); } // Slices similar to path but processed differently (and while "exclusions" // in a way do not count as ones wrt compatibility) if (slices != null) { for (SliceDef slice : slices) { - buildSlicer(slice, root); + buildSlicer(failOnOverlap, slice, root); } } // May need to add doc-id inclusion/exclusion as well if (addDocId) { buildPath( - DocumentConstants.Fields.DOC_ID, root, new String[] {DocumentConstants.Fields.DOC_ID}); + failOnOverlap, + DocumentConstants.Fields.DOC_ID, + root, + new String[] {DocumentConstants.Fields.DOC_ID}); } return root; } - static void buildPath(String fullPath, ProjectionLayer layer, String[] segments) { + static void buildPath( + boolean failOnOverlap, String fullPath, ProjectionLayer layer, String[] segments) { // First create branches final int last = segments.length - 1; for (int i = 0; i < last; ++i) { // Try to find or create branch - layer = layer.findOrCreateBranch(fullPath, segments[i]); + layer = layer.findOrCreateBranch(failOnOverlap, fullPath, segments[i]); + // May be null if terminal layer found (shorter existing path); if so, we are done + if (layer == null) { + return; + } } // And then attach terminal (leaf) - layer.addTerminal(fullPath, segments[last]); + layer.addTerminal(failOnOverlap, fullPath, segments[last]); } - static void buildSlicer(SliceDef slice, ProjectionLayer layer) { + static void buildSlicer(boolean failOnOverlap, SliceDef slice, ProjectionLayer layer) { final String fullPath = slice.path; String[] segments = DOT.split(fullPath); final int last = segments.length - 1; for (int i = 0; i < last; ++i) { - layer = layer.findOrCreateBranch(fullPath, segments[i]); + layer = layer.findOrCreateBranch(failOnOverlap, fullPath, segments[i]); } - layer.addSlicer(fullPath, segments[last], slice.slicer()); + layer.addSlicer(failOnOverlap, fullPath, segments[last], slice.slicer()); } - ProjectionLayer findOrCreateBranch(String fullPath, String segment) { + ProjectionLayer findOrCreateBranch(boolean failOnOverlap, String fullPath, String segment) { // Cannot proceed past terminal layer (shorter path): if (isTerminal) { - reportPathConflict(this.fullPath, fullPath); + if (failOnOverlap) { + reportPathConflict(this.fullPath, fullPath); + } + // Otherwise leave node as-is, return null to indicate no further traversal + return null; } ProjectionLayer next = nextLayers.get(segment); if (next == null) { @@ -107,27 +129,38 @@ ProjectionLayer findOrCreateBranch(String fullPath, String segment) { return next; } - void addTerminal(String fullPath, String segment) { + void addTerminal(boolean failOnOverlap, String fullPath, String segment) { // Cannot proceed past terminal layer (shorter path): if (isTerminal) { - reportPathConflict(this.fullPath, fullPath); + if (failOnOverlap) { + reportPathConflict(this.fullPath, fullPath); + } + // Otherwise leave node as-is, return null to indicate no further traversal + return; } // But will also not allow existing longer path: - ProjectionLayer next = nextLayers.get(segment); - if (next != null) { - reportPathConflict(fullPath, next.fullPath); + if (failOnOverlap) { + ProjectionLayer next = nextLayers.get(segment); + if (next != null) { + reportPathConflict(fullPath, next.fullPath); + } } nextLayers.put(segment, new ProjectionLayer(fullPath, true)); } - void addSlicer(String fullPath, String segment, Slicer slicer) { + void addSlicer(boolean failOnOverlap, String fullPath, String segment, Slicer slicer) { // Similar checks to "regular" paths if (isTerminal) { - reportPathConflict(this.fullPath, fullPath); + if (failOnOverlap) { + reportPathConflict(this.fullPath, fullPath); + } + return; } - ProjectionLayer next = nextLayers.get(segment); - if (next != null) { - reportPathConflict(fullPath, next.fullPath); + if (failOnOverlap) { + ProjectionLayer next = nextLayers.get(segment); + if (next != null) { + reportPathConflict(fullPath, next.fullPath); + } } nextLayers.put(segment, new ProjectionLayer(fullPath, slicer)); } diff --git a/src/test/java/io/stargate/sgv2/jsonapi/service/projection/DocumentProjectorForIndexingTest.java b/src/test/java/io/stargate/sgv2/jsonapi/service/projection/DocumentProjectorForIndexingTest.java index 4280d5952c..6c88013d72 100644 --- a/src/test/java/io/stargate/sgv2/jsonapi/service/projection/DocumentProjectorForIndexingTest.java +++ b/src/test/java/io/stargate/sgv2/jsonapi/service/projection/DocumentProjectorForIndexingTest.java @@ -21,11 +21,11 @@ public class DocumentProjectorForIndexingTest { @Nested class AllowFiltering { @Test - public void testSimpleRootAllow() { + public void testRootAllow() { assertAllowProjection( - Arrays.asList("d", "b"), + Arrays.asList("d", "b", "y"), // "y" won't match anything """ - { "a": 5, "b": 6, "c": 7, "d": 8 } + { "_id":123, "a": 5, "b": 6, "c": 7, "d": 8 } """, """ { "b": 6, "d": 8 } @@ -33,11 +33,11 @@ public void testSimpleRootAllow() { } @Test - public void testSimpleNestedAllow() { + public void testNestedAllow() { assertAllowProjection( - Arrays.asList("a.y", "c"), + Arrays.asList("a.y", "c", "x"), // "x" won't match anything """ - { + { "_id":123, "a": { "x":1, "y":2 }, "b": { "x":3, "y":4 }, "c": { "x":2, "y":3 } @@ -50,40 +50,106 @@ public void testSimpleNestedAllow() { } """); } + + // Unlike with projection, overlapping paths are accepted for Indexing + @Test + public void testOverlappingAllow() { + final String INPUT_DOC = + """ + { + "_id":123, + "a": { + "b" : { + "z" : true, + "abc": 123 + } + }, + "b" : 123 + } + """; + final String EXP_OUTPUT = + """ + { + "a": { + "b" : { + "z" : true, + "abc": 123 + } + } + } + """; + + // Try with overlapping paths + assertAllowProjection(Arrays.asList("a", "a.b"), INPUT_DOC, EXP_OUTPUT); + assertAllowProjection(Arrays.asList("a.b", "a"), INPUT_DOC, EXP_OUTPUT); + assertAllowProjection(Arrays.asList("a.b.z", "a.b"), INPUT_DOC, EXP_OUTPUT); + assertAllowProjection(Arrays.asList("a.b", "a.b.z"), INPUT_DOC, EXP_OUTPUT); + assertAllowProjection(Arrays.asList("a.b.z", "a"), INPUT_DOC, EXP_OUTPUT); + } } @Nested class DenyFiltering { @Test - public void testSimpleRootDeny() { + public void testRootDeny() { assertDenyProjection( - Arrays.asList("d", "b"), + Arrays.asList("d", "b", "x"), // "x" won't match anything """ - { "a": 5, "b": 6, "c": 7, "d": 8 } + { "_id":123, "a": 5, "b": 6, "c": 7, "d": 8 } """, """ - { "a": 5, "c": 7 } + { "_id":123, "a": 5, "c": 7 } """); } @Test - public void testSimpleNestedDeny() { + public void testNestedDeny() { assertDenyProjection( - Arrays.asList("a.y", "c"), + Arrays.asList("a.y", "c", "z"), // "z" non-matching """ - { + { "_id":123, "a": { "x":1, "y":2 }, "b": { "x":3, "y":4 }, "c": { "x":2, "y":3 } } """, """ - { + { "_id":123, "a": { "x":1 }, "b": { "x":3, "y":4 } } """); } + + @Test + public void testOverlappingDeny() { + final String INPUT_DOC = + """ + { + "_id":123, + "a": { + "b" : { + "z" : true, + "abc": 123 + } + }, + "b" : 123 + } + """; + final String EXP_OUTPUT = + """ + { + "_id":123, + "b" : 123 + } + """; + + // Try with overlapping paths + assertDenyProjection(Arrays.asList("a", "a.b"), INPUT_DOC, EXP_OUTPUT); + assertDenyProjection(Arrays.asList("a.b", "a"), INPUT_DOC, EXP_OUTPUT); + assertDenyProjection(Arrays.asList("a.b.z", "a"), INPUT_DOC, EXP_OUTPUT); + assertDenyProjection(Arrays.asList("a", "a.b.z"), INPUT_DOC, EXP_OUTPUT); + } } @Nested From 7cfffa3187d72e5b1c635c75fd1e0230963e39e0 Mon Sep 17 00:00:00 2001 From: Tatu Saloranta Date: Thu, 11 Jan 2024 09:40:04 -0800 Subject: [PATCH 11/13] Add `DocumentProjector.isPathIncluded()` for filter/sort validation --- .../service/projection/DocumentProjector.java | 26 +++++++ .../service/projection/ProjectionLayer.java | 29 ++++++++ .../DocumentProjectorForIndexingTest.java | 67 ++++++++++++++++--- 3 files changed, 114 insertions(+), 8 deletions(-) diff --git a/src/main/java/io/stargate/sgv2/jsonapi/service/projection/DocumentProjector.java b/src/main/java/io/stargate/sgv2/jsonapi/service/projection/DocumentProjector.java index ed6ff57d6e..9193cdd2b3 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/service/projection/DocumentProjector.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/service/projection/DocumentProjector.java @@ -137,6 +137,32 @@ public void applyProjection(JsonNode document, Float similarityScore) { } } + /** + * Method to call to check if given path (dotted path, that is, dot-separated segments) would be + * included by this Projection. That is, either + * + *
    + *
  • This is inclusion projection, and path is covered by an inclusion path + *
  • This is exclusion projection, and path is NOT covered by any exclusion path + *
+ * + * @param path Dotted path (possibly nested) to check + * @return {@code true} if path is included; {@code false} if not. + */ + public boolean isPathIncluded(String path) { + // First: if we have no layers, we are identity projector and include everything + if (rootLayer == null) { + return true; + } + // Otherwise need to split path, evaluate; but note reversal wrt include/exclude + // projections + if (inclusion) { + return rootLayer.isPathIncluded(path); + } else { + return !rootLayer.isPathIncluded(path); + } + } + // Mostly for deserialization tests @Override public boolean equals(Object o) { diff --git a/src/main/java/io/stargate/sgv2/jsonapi/service/projection/ProjectionLayer.java b/src/main/java/io/stargate/sgv2/jsonapi/service/projection/ProjectionLayer.java index e8b5f14441..a623933bb5 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/service/projection/ProjectionLayer.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/service/projection/ProjectionLayer.java @@ -165,6 +165,35 @@ void addSlicer(boolean failOnOverlap, String fullPath, String segment, Slicer sl nextLayers.put(segment, new ProjectionLayer(fullPath, slicer)); } + /** + * Method called to check if given path is included in the projection for which this is the root + * layer: this is done by traversing layers until determination can be made. + * + * @param path Dot-notation path to check + * @return {@code true} if path is included; {@code false} if not. + */ + public boolean isPathIncluded(String path) { + final String[] segments = DOT.split(path); + return isPathIncluded(segments, 0); + } + + private boolean isPathIncluded(String[] segments, int index) { + // If we are at a terminal layer, we are done + if (isTerminal) { + return true; + } + // Otherwise if we are at the end of path, we are not included + if (index == segments.length) { + return false; + } + // Otherwise we need to traverse further + ProjectionLayer next = nextLayers.get(segments[index]); + if (next == null) { + return false; + } + return next.isPathIncluded(segments, index + 1); + } + /** * Method called to apply Inclusion-based projection, in which everything to be included is * enumerated, and the rest need to be removed (this includes document id as well as regular diff --git a/src/test/java/io/stargate/sgv2/jsonapi/service/projection/DocumentProjectorForIndexingTest.java b/src/test/java/io/stargate/sgv2/jsonapi/service/projection/DocumentProjectorForIndexingTest.java index 6c88013d72..2166c02760 100644 --- a/src/test/java/io/stargate/sgv2/jsonapi/service/projection/DocumentProjectorForIndexingTest.java +++ b/src/test/java/io/stargate/sgv2/jsonapi/service/projection/DocumentProjectorForIndexingTest.java @@ -7,7 +7,6 @@ import io.quarkus.test.junit.QuarkusTest; import io.quarkus.test.junit.TestProfile; import io.stargate.sgv2.common.testprofiles.NoGlobalResourcesTestProfile; -import jakarta.inject.Inject; import java.io.IOException; import java.util.*; import org.junit.jupiter.api.Nested; @@ -16,7 +15,7 @@ @QuarkusTest @TestProfile(NoGlobalResourcesTestProfile.Impl.class) public class DocumentProjectorForIndexingTest { - @Inject ObjectMapper objectMapper; + final ObjectMapper objectMapper = new ObjectMapper(); @Nested class AllowFiltering { @@ -177,9 +176,37 @@ public void testDenyAll() { } } - private void assertAllowProjection(List allows, String inputDoc, String expectedDoc) { - DocumentProjector projector = - DocumentProjector.createForIndexing(new LinkedHashSet<>(allows), Collections.emptySet()); + @Nested + class PathInclusion { + @Test + public void testPathInclusion() { + DocumentProjector rootProj = createAllowProjection(Arrays.asList("a", "c")); + assertIsIncluded(rootProj, "a", "a.x", "a.b.c.d"); + assertIsIncluded(rootProj, "c", "c.a", "c.x.y.z"); + assertIsNotIncluded(rootProj, "b", "d", "b.a", "abc", "cef", "_id"); + + DocumentProjector nestedProj = createAllowProjection(Arrays.asList("a.b")); + assertIsIncluded(nestedProj, "a.b", "a.b.c", "a.b.longer.path.for.sure"); + assertIsNotIncluded(nestedProj, "a", "b", "c", "a.c", "a.x", "a.x.y.z", "_id"); + } + + @Test + public void testPathExclusion() { + DocumentProjector rootProj = createDenyProjection(Arrays.asList("a", "c")); + assertIsNotIncluded(rootProj, "a", "a.x", "a.b.c.d"); + assertIsNotIncluded(rootProj, "c", "c.a", "c.x.y.z"); + assertIsIncluded(rootProj, "b", "d", "b.a", "abc", "cef", "_id"); + + DocumentProjector nestedProj = createDenyProjection(Arrays.asList("a.b", "a.noindex")); + assertIsNotIncluded( + nestedProj, "a.b", "a.b.c", "a.b.longer.path.for.sure", "a.noindex", "a.noindex.x"); + assertIsIncluded(nestedProj, "a", "b", "_id", "a.c", "a.x", "a.x.y.z"); + } + } + + private void assertAllowProjection( + Collection allows, String inputDoc, String expectedDoc) { + DocumentProjector projector = createAllowProjection(allows); try { JsonNode input = objectMapper.readTree(inputDoc); projector.applyProjection(input); @@ -189,9 +216,9 @@ private void assertAllowProjection(List allows, String inputDoc, String } } - private void assertDenyProjection(List denies, String inputDoc, String expectedDoc) { - DocumentProjector projector = - DocumentProjector.createForIndexing(Collections.emptySet(), new LinkedHashSet<>(denies)); + private void assertDenyProjection( + Collection denies, String inputDoc, String expectedDoc) { + DocumentProjector projector = createDenyProjection(denies); try { JsonNode input = objectMapper.readTree(inputDoc); projector.applyProjection(input); @@ -200,4 +227,28 @@ private void assertDenyProjection(List denies, String inputDoc, String e throw new RuntimeException(e); } } + + private DocumentProjector createAllowProjection(Collection allows) { + return DocumentProjector.createForIndexing(new LinkedHashSet<>(allows), null); + } + + private DocumentProjector createDenyProjection(Collection denies) { + return DocumentProjector.createForIndexing(null, new LinkedHashSet<>(denies)); + } + + private void assertIsIncluded(DocumentProjector proj, String... paths) { + for (String path : paths) { + assertThat(proj.isPathIncluded(path)) + .withFailMessage("Path '%s' should be included", path) + .isTrue(); + } + } + + private void assertIsNotIncluded(DocumentProjector proj, String... paths) { + for (String path : paths) { + assertThat(proj.isPathIncluded(path)) + .withFailMessage("Path '%s' should NOT be included", path) + .isFalse(); + } + } } From 028ff0ba9bb7fa8451529f7b16dab554228cea70 Mon Sep 17 00:00:00 2001 From: Tatu Saloranta Date: Thu, 11 Jan 2024 10:23:50 -0800 Subject: [PATCH 12/13] Bit more testing --- .../projection/DocumentProjectorForIndexingTest.java | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/src/test/java/io/stargate/sgv2/jsonapi/service/projection/DocumentProjectorForIndexingTest.java b/src/test/java/io/stargate/sgv2/jsonapi/service/projection/DocumentProjectorForIndexingTest.java index 2166c02760..5e50acdb15 100644 --- a/src/test/java/io/stargate/sgv2/jsonapi/service/projection/DocumentProjectorForIndexingTest.java +++ b/src/test/java/io/stargate/sgv2/jsonapi/service/projection/DocumentProjectorForIndexingTest.java @@ -188,6 +188,12 @@ public void testPathInclusion() { DocumentProjector nestedProj = createAllowProjection(Arrays.asList("a.b")); assertIsIncluded(nestedProj, "a.b", "a.b.c", "a.b.longer.path.for.sure"); assertIsNotIncluded(nestedProj, "a", "b", "c", "a.c", "a.x", "a.x.y.z", "_id"); + + // Let's also check overlapping (redundant) case; most generic used (specific ignored) + // same as just "c": + DocumentProjector overlapProj = createAllowProjection(Arrays.asList("c", "c.x")); + assertIsIncluded(overlapProj, "c", "c.abc", "c.d.e.f"); + assertIsNotIncluded(overlapProj, "a", "b", "d", "a.c", "a.x.y.z", "_id"); } @Test @@ -201,6 +207,12 @@ public void testPathExclusion() { assertIsNotIncluded( nestedProj, "a.b", "a.b.c", "a.b.longer.path.for.sure", "a.noindex", "a.noindex.x"); assertIsIncluded(nestedProj, "a", "b", "_id", "a.c", "a.x", "a.x.y.z"); + + // Let's also check overlapping (redundant) case; most generic used (specific ignored) + // same as just "c": + DocumentProjector overlapProj = createDenyProjection(Arrays.asList("c", "c.x")); + assertIsNotIncluded(overlapProj, "c", "c.abc", "c.d.e.f"); + assertIsIncluded(overlapProj, "a", "b", "d", "a.c", "a.x.y.z", "_id"); } } From a03a31b1df05144afb26fe0ddf09665b01e7c96d Mon Sep 17 00:00:00 2001 From: Tatu Saloranta Date: Thu, 11 Jan 2024 11:05:55 -0800 Subject: [PATCH 13/13] Add a test to ensure that "indexing projector" is created just once --- .../configuration/CollectionSettingsTest.java | 37 +++++++++++++++++++ 1 file changed, 37 insertions(+) create mode 100644 src/test/java/io/stargate/sgv2/jsonapi/api/configuration/CollectionSettingsTest.java diff --git a/src/test/java/io/stargate/sgv2/jsonapi/api/configuration/CollectionSettingsTest.java b/src/test/java/io/stargate/sgv2/jsonapi/api/configuration/CollectionSettingsTest.java new file mode 100644 index 0000000000..29381c4864 --- /dev/null +++ b/src/test/java/io/stargate/sgv2/jsonapi/api/configuration/CollectionSettingsTest.java @@ -0,0 +1,37 @@ +package io.stargate.sgv2.jsonapi.api.configuration; + +import static org.assertj.core.api.Assertions.assertThat; + +import com.fasterxml.jackson.databind.ObjectMapper; +import io.quarkus.test.junit.QuarkusTest; +import io.quarkus.test.junit.TestProfile; +import io.stargate.sgv2.common.testprofiles.NoGlobalResourcesTestProfile; +import io.stargate.sgv2.jsonapi.service.cqldriver.executor.CollectionSettings; +import io.stargate.sgv2.jsonapi.service.projection.DocumentProjector; +import java.util.Collections; +import org.junit.jupiter.api.Test; + +@QuarkusTest +@TestProfile(NoGlobalResourcesTestProfile.Impl.class) +public class CollectionSettingsTest { + final ObjectMapper objectMapper = new ObjectMapper(); + + @Test + public void ensureSingleProjectorCreation() { + CollectionSettings.IndexingConfig indexingConfig = + new CollectionSettings.IndexingConfig(Collections.singleton("abc"), null); + CollectionSettings settings = + new CollectionSettings("collectionName", false, -1, null, null, null, indexingConfig); + DocumentProjector indexingProj = settings.indexingProjector(); + assertThat(indexingProj) + .isNotNull() + // Should get the same instance second time due to memoization + .isSameAs(settings.indexingProjector()) + // and third :) + .isSameAs(settings.indexingProjector()); + + // And should also work as expected: "abc" and below included; the rest, not + assertThat(indexingProj.isPathIncluded("abc.x")).isTrue(); + assertThat(indexingProj.isPathIncluded("_id")).isFalse(); + } +}