Skip to content

Commit

Permalink
Fixes #821: apply max-object-properties/max-doc-properties only on in…
Browse files Browse the repository at this point in the history 
…dexed Objects (#844)

Co-authored-by: Mahesh Rajamani <99678631+maheshrajamani@users.noreply.github.com>
  • Loading branch information
@tatu-at-datastax @maheshrajamani
tatu-at-datastax and maheshrajamani authored Jan 31, 2024
1 parent 82da4fa commit 9ae532d
Show file tree
Hide file tree
Showing 6 changed files with 171 additions and 33 deletions.
25 changes: 13 additions & 12 deletions CONFIGURATION.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,18 +23,19 @@ Here are some Stargate-relevant property groups that are necessary for correct s
## Document limits configuration
*Configuration for document limits, defined by [DocumentLimitsConfig.java](src/main/java/io/stargate/sgv2/jsonapi/config/DocumentLimitsConfig.java).*

| Property | Type | Default | Description |
|-----------------------------------------------------------------|-------|-------------|--------------------------------------------------------------------------------------|
| `stargate.jsonapi.document.limits.max-size` | `int` | `4_000_000` | The maximum size of (in characters) a single document. |
| `stargate.jsonapi.document.limits.max-depth` | `int` | `16` | The maximum document depth (nesting). |
| `stargate.jsonapi.document.limits.max-property-name-length` | `int` | `100` | The maximum length of property names in a document for an individual segment. |
| `stargate.jsonapi.document.limits.max-property-path-length` | `int` | `250` | The maximum length of property paths in a document (segments and separating periods) |
| `stargate.jsonapi.document.limits.max-object-properties` | `int` | `1000` | The maximum number of properties any single object in a document can contain. |
| `stargate.jsonapi.document.limits.max-document-properties` | `int` | `2000` | The maximum total number of properties all objects in a document can contain. |
| `stargate.jsonapi.document.limits.max-number-length` | `int` | `100` | The maximum length (in characters) of a single number value in a document. |
| `stargate.jsonapi.document.limits.max-string-length-in-bytes` | `int` | `8000` | The maximum length (in bytes) of a single string value in a document. |
| `stargate.jsonapi.document.limits.max-array-length` | `int` | `1000` | The maximum length (in elements) of a single indexable array in a document. |
| `stargate.jsonapi.document.limits.max-vector-embedding-length` | `int` | `4096` | The maximum length (in floats) of the $vector in a document. |
<<<<<<< HEAD
| Property | Type | Default | Description |
|-----------------------------------------------------------------|-------|-------------|-----------------------------------------------------------------------------------------|
| `stargate.jsonapi.document.limits.max-size` | `int` | `4_000_000` | The maximum size of (in characters) a single document. |
| `stargate.jsonapi.document.limits.max-depth` | `int` | `16` | The maximum document depth (nesting). |
| `stargate.jsonapi.document.limits.max-property-name-length` | `int` | `100` | The maximum length of property names in a document for an individual segment. |
| `stargate.jsonapi.document.limits.max-property-path-length` | `int` | `250` | The maximum length of property paths in a document (segments and separating periods) |
| `stargate.jsonapi.document.limits.max-object-properties` | `int` | `1000` | The maximum number of properties any single indexable object in a document can contain. |
| `stargate.jsonapi.document.limits.max-document-properties` | `int` | `2000` | The maximum total number of properties all objects in a document can contain. |
| `stargate.jsonapi.document.limits.max-number-length` | `int` | `100` | The maximum length (in characters) of a single number value in a document. |
| `stargate.jsonapi.document.limits.max-string-length-in-bytes` | `int` | `8000` | The maximum length (in bytes) of a single indexable string value in a document. |
| `stargate.jsonapi.document.limits.max-array-length` | `int` | `1000` | The maximum length (in elements) of a single indexable array in a document. |
| `stargate.jsonapi.document.limits.max-vector-embedding-length` | `int` | `4096` | The maximum length (in floats) of the $vector in a document. |

## Operations configuration
*Configuration for the operation execution, defined by [OperationsConfig.java](src/main/java/io/stargate/sgv2/jsonapi/config/OperationsConfig.java).*
Expand Down
18 changes: 11 additions & 7 deletions src/main/java/io/stargate/sgv2/jsonapi/config/DocumentLimitsConfig.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,15 @@ public interface DocumentLimitsConfig {
/** Defines the default maximum document (nesting) depth */
int DEFAULT_MAX_DOCUMENT_DEPTH = 16;

/** Defines the default maximum length (in elements) of a single indexable Array value */
/**
* Defines the default maximum length (in elements) of a single indexable Array in JSON document
* can contain.
*/
int DEFAULT_MAX_ARRAY_LENGTH = 1_000;

/**
* Defines the default maximum number of properties any single Object in JSON document can contain
* Defines the default maximum number of properties any single indexable Object in JSON document
* can contain.
*/
int DEFAULT_MAX_OBJECT_PROPERTIES = 1000;

Expand Down Expand Up @@ -88,17 +92,17 @@ public interface DocumentLimitsConfig {
int maxPropertyPathLength();

/**
* @return Defines the maximum number of properties any single Object in JSON document can
* contain, defaults to {@code 1,000} (note: this is not the total number of properties in the
* whole document, only on individual main or sub-document)
* @return Defines the maximum number of properties any single indexable Object in JSON document
* can contain, defaults to {@code 1,000} (note: this is not the total number of properties in
* the whole document but in the main or indexable sub-document)
*/
@Positive
@WithDefault("" + DEFAULT_MAX_OBJECT_PROPERTIES)
int maxObjectProperties();

/**
* @return Defines the maximum number of properties the whole JSON document can contain, defaults
* to {@code 2,000}, including Object- and Array-valued properties.
* @return Defines the maximum number of indexable properties in JSON document can contain,
* defaults to {@code 2,000}, including all indexable Object- and Array-valued properties.
*/
@Positive
@WithDefault("" + DEFAULT_MAX_DOC_PROPERTIES)
Expand Down
12 changes: 7 additions & 5 deletions src/main/java/io/stargate/sgv2/jsonapi/service/shredding/Shredder.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -282,11 +282,6 @@ private void validateObjectValue(
validateDocDepth(limits, depth);

final int propCount = objectValue.size();
if (propCount > limits.maxObjectProperties()) {
throw ErrorCode.SHRED_DOC_LIMIT_VIOLATION.toApiException(
"number of properties an Object has (%d) exceeds maximum allowed (%s)",
objectValue.size(), limits.maxObjectProperties());
}
totalProperties.addAndGet(propCount);

var it = objectValue.fields();
Expand Down Expand Up @@ -387,6 +382,13 @@ private void validateArrayValue(String referringPropertyName, JsonNode arrayValu
}

private void validateObjectValue(String referringPropertyName, JsonNode objectValue) {
final int propCount = objectValue.size();
if (propCount > limits.maxObjectProperties()) {
throw ErrorCode.SHRED_DOC_LIMIT_VIOLATION.toApiException(
"number of properties an indexable Object ('%s') has (%d) exceeds maximum allowed (%s)",
referringPropertyName, objectValue.size(), limits.maxObjectProperties());
}

for (Map.Entry<String, JsonNode> entry : objectValue.properties()) {
validateValue(entry.getKey(), entry.getValue());
}
Expand Down
90 changes: 86 additions & 4 deletions src/test/java/io/stargate/sgv2/jsonapi/api/v1/FindOneAndUpdateNoIndexIntegrationTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@

import com.fasterxml.jackson.databind.ObjectMapper;
import com.fasterxml.jackson.databind.node.ArrayNode;
import com.fasterxml.jackson.databind.node.ObjectNode;
import io.quarkus.test.common.QuarkusTestResource;
import io.quarkus.test.junit.QuarkusIntegrationTest;
import io.restassured.http.ContentType;
Expand Down Expand Up @@ -203,7 +204,7 @@ class ArraySizeLimit {
@Test
@Order(1)
public void allowNonIndexedBigArray() {
insertEmptyDoc("array_size_too_big_doc");
insertEmptyDoc("array_size_big_noindex_doc");
final String arrayJson = bigArray(DocumentLimitsConfig.DEFAULT_MAX_ARRAY_LENGTH + 10);
given()
.header(HttpConstants.AUTHENTICATION_TOKEN_HEADER_NAME, getAuthToken())
Expand All @@ -212,7 +213,7 @@ public void allowNonIndexedBigArray() {
"""
{
"findOneAndUpdate": {
"filter" : {"_id" : "array_size_too_big_doc"},
"filter" : {"_id" : "array_size_big_noindex_doc"},
"update" : {
"$set" : {
"notIndexed.bigArray": %s
Expand All @@ -234,7 +235,7 @@ public void allowNonIndexedBigArray() {
@Test
@Order(2)
public void failOnIndexedTooBigArray() {
insertEmptyDoc("array_size_ok_doc");
insertEmptyDoc("array_size_too_big_doc");
final String arrayJson = bigArray(DocumentLimitsConfig.DEFAULT_MAX_ARRAY_LENGTH + 10);
given()
.header(HttpConstants.AUTHENTICATION_TOKEN_HEADER_NAME, getAuthToken())
Expand All @@ -243,7 +244,7 @@ public void failOnIndexedTooBigArray() {
"""
{
"findOneAndUpdate": {
"filter" : {"_id" : "array_size_ok_doc"},
"filter" : {"_id" : "array_size_too_big_doc"},
"update" : {
"$set" : {
"indexed.bigArray": %s
Expand All @@ -266,6 +267,79 @@ public void failOnIndexedTooBigArray() {
.body("errors[0].message", containsString("exceeds maximum allowed"));
}

@Nested
@Order(4)
@TestClassOrder(ClassOrderer.OrderAnnotation.class)
class ObjectSizeLimit {
@Test
@Order(1)
public void allowNonIndexedBigObject() {
insertEmptyDoc("object_size_big_noindex_doc");
final String objectJson =
bigObject(DocumentLimitsConfig.DEFAULT_MAX_OBJECT_PROPERTIES + 10);
given()
.header(HttpConstants.AUTHENTICATION_TOKEN_HEADER_NAME, getAuthToken())
.contentType(ContentType.JSON)
.body(
"""
{
"findOneAndUpdate": {
"filter" : {"_id" : "object_size_big_noindex_doc"},
"update" : {
"$set" : {
"notIndexed.bigObject": %s
}
}
}
}
"""
.formatted(objectJson))
.when()
.post(CollectionResource.BASE_PATH, namespaceName, collectionName)
.then()
.statusCode(200)
.body("errors", is(nullValue()))
.body("status.matchedCount", is(1))
.body("status.modifiedCount", is(1));
}

@Test
@Order(2)
public void failOnIndexedTooBigObject() {
insertEmptyDoc("object_size_too_big_doc");
final String objectJson =
bigObject(DocumentLimitsConfig.DEFAULT_MAX_OBJECT_PROPERTIES + 10);
given()
.header(HttpConstants.AUTHENTICATION_TOKEN_HEADER_NAME, getAuthToken())
.contentType(ContentType.JSON)
.body(
"""
{
"findOneAndUpdate": {
"filter" : {"_id" : "object_size_too_big_doc"},
"update" : {
"$set" : {
"indexed.bigObject": %s
}
}
}
}
"""
.formatted(objectJson))
.when()
.post(CollectionResource.BASE_PATH, namespaceName, collectionName)
.then()
.statusCode(200)
.body("data", is(nullValue()))
.body("status", is(nullValue()))
.body("errors[0].errorCode", is("SHRED_DOC_LIMIT_VIOLATION"))
.body(
"errors[0].message",
containsString("number of properties an indexable Object ('bigObject')"))
.body("errors[0].message", containsString("exceeds maximum allowed"));
}
}

private void insertEmptyDoc(String docId) {
given()
.header(HttpConstants.AUTHENTICATION_TOKEN_HEADER_NAME, getAuthToken())
Expand Down Expand Up @@ -296,5 +370,13 @@ private final String bigArray(int elementCount) {
}
return array.toString();
}

private final String bigObject(int propertyCount) {
final ObjectNode ob = MAPPER.createObjectNode();
for (int i = 0; i < propertyCount; i++) {
ob.put("prop" + i, i);
}
return ob.toString();
}
}
}
40 changes: 40 additions & 0 deletions src/test/java/io/stargate/sgv2/jsonapi/api/v1/InsertIntegrationTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -824,6 +824,46 @@ public void tryInsertTooLongDoc() throws Exception {
.body("errors[0].message", endsWith(") exceeds maximum allowed (4000000)"));
}

@Test
public void tryInsertDocWithTooBigObject() {
final ObjectNode tooManyPropsDoc = MAPPER.createObjectNode();
tooManyPropsDoc.put("_id", 456);

// Max indexed: 1000, add some more
ObjectNode subdoc = tooManyPropsDoc.putObject("subdoc");
for (int i = 0; i < 1001; ++i) {
subdoc.put("prop" + i, i);
}

String json =
"""
{
"insertOne": {
"document": %s
}
}
"""
.formatted(tooManyPropsDoc);
given()
.header(HttpConstants.AUTHENTICATION_TOKEN_HEADER_NAME, getAuthToken())
.contentType(ContentType.JSON)
.body(json)
.when()
.post(CollectionResource.BASE_PATH, namespaceName, collectionName)
.then()
.statusCode(200)
.body("errors", is(notNullValue()))
.body("errors", hasSize(1))
.body("errors[0].exceptionClass", is("JsonApiException"))
.body("errors[0].errorCode", is("SHRED_DOC_LIMIT_VIOLATION"))
.body(
"errors[0].message",
startsWith("Document size limitation violated: number of properties"))
.body(
"errors[0].message",
endsWith("indexable Object ('subdoc') has (1001) exceeds maximum allowed (1000)"));
}

@Test
public void tryInsertDocWithTooManyProps() {
final ObjectNode tooManyPropsDoc = MAPPER.createObjectNode();
Expand Down
19 changes: 14 additions & 5 deletions src/test/java/io/stargate/sgv2/jsonapi/service/shredding/ShredderDocLimitsTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -120,16 +120,25 @@ class ValidationDocCountViolations {
@Test
public void allowDocWithManyObjectProps() {
// Max allowed is 1,000
final ObjectNode doc = docWithNProps(docLimits.maxObjectProperties());
final ObjectNode doc = docWithNProps("subdoc", docLimits.maxObjectProperties());
assertThat(shredder.shred(doc)).isNotNull();
}

@Test
public void allowDocWithHugeObjectNoIndex() {
// Max allowed 1000 normally, but if Object not-indexed, not limited
final ObjectNode doc = docWithNProps("no_index", docLimits.maxObjectProperties() + 100);
DocumentProjector indexProjector =
DocumentProjector.createForIndexing(null, Collections.singleton("no_index"));
assertThat(shredder.shred(doc, null, indexProjector)).isNotNull();
}

@Test
public void catchTooManyObjectProps() {
// Max allowed 100, so fail with just one above
final int maxObProps = docLimits.maxObjectProperties();
final int tooManyProps = maxObProps + 1;
final ObjectNode doc = docWithNProps(tooManyProps);
final ObjectNode doc = docWithNProps("subdoc", tooManyProps);

Exception e = catchException(() -> shredder.shred(doc));
assertThat(e)
Expand All @@ -138,17 +147,17 @@ public void catchTooManyObjectProps() {
.hasFieldOrPropertyWithValue("errorCode", ErrorCode.SHRED_DOC_LIMIT_VIOLATION)
.hasMessageStartingWith(ErrorCode.SHRED_DOC_LIMIT_VIOLATION.getMessage())
.hasMessageEndingWith(
" number of properties an Object has ("
" number of properties an indexable Object ('subdoc') has ("
+ tooManyProps
+ ") exceeds maximum allowed ("
+ maxObProps
+ ")");
}

private ObjectNode docWithNProps(int count) {
private ObjectNode docWithNProps(String propName, int count) {
final ObjectNode doc = objectMapper.createObjectNode();
doc.put("_id", 123);
ObjectNode obNode = doc.putObject("subdoc");
ObjectNode obNode = doc.putObject(propName);
for (int i = 0; i < count; ++i) {
obNode.put("prop" + i, i);
}
Expand Down

0 comments on commit 9ae532d

Please sign in to comment.