Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for $each modifier of $push update operator #99

Merged
merged 5 commits into from
Feb 13, 2023
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -30,33 +30,78 @@ public static PushOperation construct(ObjectNode args) {
List<PushAction> updates = new ArrayList<>();
while (fieldIter.hasNext()) {
Map.Entry<String, JsonNode> entry = fieldIter.next();
// 06-Feb-2023, tatu: Until "$each" supported, verify that no modifiers included
// At main level we must have field name (no modifiers)
final String name = entry.getKey();
if (looksLikeModifier(name)) {
throw new JsonApiException(
ErrorCode.UNSUPPORTED_UPDATE_OPERATION_PARAM,
ErrorCode.UNSUPPORTED_UPDATE_OPERATION_PARAM.getMessage()
+ ": $push requires field names at main level, found modifier: "
+ name);
}
// But within field value modifiers are allowed: if there's one, all must be modifiers
JsonNode value = entry.getValue();
String firstModifier = findModifier(value);
if (firstModifier != null) {
PushAction action;
if (value.isObject() && hasModifier((ObjectNode) value)) {
action = buildActionWithModifiers(name, (ObjectNode) value);
} else {
action = new PushAction(name, entry.getValue(), false);
}
updates.add(action);
}
return new PushOperation(updates);
}

private static PushAction buildActionWithModifiers(String propName, ObjectNode actionDef) {
// We know there is at least one modifier; and if so, all must be modifiers.
// For now we only support "$each"
JsonNode eachArg = null;

Iterator<Map.Entry<String, JsonNode>> it = actionDef.fields();
while (it.hasNext()) {
Map.Entry<String, JsonNode> entry = it.next();
final String modifier = entry.getKey();

// Since we only support "$each" for now
if (!"$each".equals(modifier)) {
throw new JsonApiException(
ErrorCode.UNSUPPORTED_UPDATE_OPERATION_MODIFIER,
ErrorCode.UNSUPPORTED_UPDATE_OPERATION_MODIFIER.getMessage()
+ ": $push does not yet support modifiers; trying to use "
+ firstModifier);
+ ": $push only supports $each currently; trying to use '"
+ modifier
+ "'");
}
// And argument for $each must be an Array
eachArg = entry.getValue();
if (!eachArg.isArray()) {
throw new JsonApiException(
ErrorCode.UNSUPPORTED_UPDATE_OPERATION_PARAM,
ErrorCode.UNSUPPORTED_UPDATE_OPERATION_PARAM.getMessage()
+ ": $push modifier $each requires ARRAY argument, found: "
+ eachArg.getNodeType());
}
updates.add(new PushAction(entry.getKey(), entry.getValue()));
}
return new PushOperation(updates);

// For now should not be possible to occur but once we add other modifiers could:
if (eachArg == null) {
throw new JsonApiException(
ErrorCode.UNSUPPORTED_UPDATE_OPERATION_PARAM,
ErrorCode.UNSUPPORTED_UPDATE_OPERATION_PARAM.getMessage()
+ ": $push modifiers can only be used with $each modifier; none included");
}

return new PushAction(propName, eachArg, true);
}

private static String findModifier(JsonNode node) {
if (node.isObject()) {
// Sigh. Wish things were not returned as iterators by JsonNode...
Iterator<String> it = node.fieldNames();
while (it.hasNext()) {
String name = it.next();
if (name.startsWith("$")) {
return name;
}
private static boolean hasModifier(ObjectNode node) {
// Sigh. Wish things were not returned as iterators by JsonNode...
Iterator<String> it = node.fieldNames();
while (it.hasNext()) {
if (looksLikeModifier(it.next())) {
return true;
}
}
return null;
return false;
}

@Override
Expand All @@ -65,10 +110,12 @@ public boolean updateDocument(ObjectNode doc) {
final String path = update.path;
final JsonNode toAdd = update.value;
JsonNode node = doc.get(path);
ArrayNode array;

if (node == null) { // No such property? Add new 1-element array
doc.putArray(path).add(toAdd);
array = doc.putArray(path);
} else if (node.isArray()) { // Already array? Append
((ArrayNode) node).add(toAdd);
array = (ArrayNode) node;
} else { // Something else? fail
throw new JsonApiException(
ErrorCode.UNSUPPORTED_UPDATE_OPERATION_TARGET,
Expand All @@ -78,6 +125,14 @@ public boolean updateDocument(ObjectNode doc) {
+ " of type "
+ node.getNodeType());
}
// Regular add or $each?
if (update.each) {
for (JsonNode element : toAdd) {
array.add(element);
}
} else {
array.add(toAdd);
}
}

// Every valid update operation modifies document so need just one:
Expand All @@ -95,5 +150,5 @@ public boolean equals(Object o) {
* Value class for per-field update operations: initially simple replacement but will need
* different value type soon to allow {@code $each modifier}.
*/
private record PushAction(String path, JsonNode value) {}
private record PushAction(String path, JsonNode value, boolean each) {}
}
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ protected static String validateSetPath(UpdateOperator oper, String path) {
* @return Path passed in if valid
*/
protected static String validateNonModifierPath(UpdateOperator oper, String path) {
if (path.startsWith("$")) {
if (looksLikeModifier(path)) {
throw new JsonApiException(
ErrorCode.UNSUPPORTED_UPDATE_OPERATION_MODIFIER,
ErrorCode.UNSUPPORTED_UPDATE_OPERATION_MODIFIER.getMessage()
Expand All @@ -52,4 +52,8 @@ protected static String validateNonModifierPath(UpdateOperator oper, String path
}
return path;
}

protected static boolean looksLikeModifier(String path) {
return path.startsWith("$");
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -540,36 +540,81 @@ public void findByColumnAndSetSubDoc() {
.statusCode(200)
.body("data.docs[0]", jsonEquals(expected));
}
}

@Nested
@TestMethodOrder(MethodOrderer.OrderAnnotation.class)
class UpdateOneWithPush {
@Test
@Order(2)
public void findByColumnAndPushToArray() {
public void findByColumnAndPush() {
insertDoc(
"""
{
"_id": "update_doc_push",
"array": [ 2 ]
}
""");
String json =
"""
{
"insertOne": {
"document": {
"_id": "update_doc_push",
"array": [ 2 ]
}
"updateOne": {
"filter" : {"_id" : "update_doc_push"},
"update" : {"$push" : {"array": 13 }}
}
}
""";

given()
.header(HttpConstants.AUTHENTICATION_TOKEN_HEADER_NAME, getAuthToken())
.contentType(ContentType.JSON)
.body(json)
.when()
.post(CollectionResource.BASE_PATH, keyspaceId.asInternal(), collectionName)
.then()
.statusCode(200);
.statusCode(200)
.body("status.updatedIds[0]", is("update_doc_push"));

String expected = "{\"_id\":\"update_doc_push\", \"array\": [2, 13]}";
json =
"""
{
"find": {
"filter" : {"_id" : "update_doc_push"}
}
}
""";
given()
.header(HttpConstants.AUTHENTICATION_TOKEN_HEADER_NAME, getAuthToken())
.contentType(ContentType.JSON)
.body(json)
.when()
.post(CollectionResource.BASE_PATH, keyspaceId.asInternal(), collectionName)
.then()
.statusCode(200)
.body("data.docs[0]", jsonEquals(expected));
}

@Test
@Order(2)
public void findByColumnAndPushWithEach() {
insertDoc(
"""
{
"_id": "update_doc_push_each",
"array": [ 1 ]
}
""");
String json =
"""
{
"updateOne": {
"filter" : {"_id" : "update_doc_push"},
"update" : {"$push" : {"array": 13 }}
"filter" : {"_id" : "update_doc_push_each"},
"update" : {
"$push" : {
"array": { "$each" : [ 2, 3 ] },
"newArray": { "$each" : [ true ] }
}
}
}
}
""";
Expand All @@ -581,17 +626,22 @@ public void findByColumnAndPushToArray() {
.post(CollectionResource.BASE_PATH, keyspaceId.asInternal(), collectionName)
.then()
.statusCode(200)
.body("status.updatedIds[0]", is("update_doc_push"));
.body("status.updatedIds[0]", is("update_doc_push_each"));

String expected = "{\"_id\":\"update_doc_push\", \"array\": [2, 13]}";
String expected =
"""
{ "_id":"update_doc_push_each",
"array": [1, 2, 3],
"newArray": [true] }
""";
json =
"""
{
"find": {
"filter" : {"_id" : "update_doc_push"}
}
}
""";
{
"find": {
"filter" : {"_id" : "update_doc_push_each"}
}
}
""";
given()
.header(HttpConstants.AUTHENTICATION_TOKEN_HEADER_NAME, getAuthToken())
.contentType(ContentType.JSON)
Expand All @@ -602,30 +652,21 @@ public void findByColumnAndPushToArray() {
.statusCode(200)
.body("data.docs[0]", jsonEquals(expected));
}
}

@Nested
@TestMethodOrder(MethodOrderer.OrderAnnotation.class)
class UpdateOneWithInc {
@Test
@Order(2)
public void findByColumnAndInc() {
String doc =
insertDoc(
"""
{
"insertOne": {
"document": {
"_id": "update_doc_inc",
"number": 123
}
}
{
"_id": "update_doc_inc",
"number": 123
}
""";

given()
.header(HttpConstants.AUTHENTICATION_TOKEN_HEADER_NAME, getAuthToken())
.contentType(ContentType.JSON)
.body(doc)
.when()
.post(CollectionResource.BASE_PATH, keyspaceId.asInternal(), collectionName)
.then()
.statusCode(200);
""");
String updateJson =
"""
{
Expand All @@ -648,12 +689,12 @@ public void findByColumnAndInc() {
String expectedDoc = "{\"_id\":\"update_doc_inc\", \"number\": 119, \"newProp\": 0.25 }";
String findJson =
"""
{
"find": {
"filter" : {"_id" : "update_doc_inc"}
{
"find": {
"filter" : {"_id" : "update_doc_inc"}
}
}
}
""";
""";
given()
.header(HttpConstants.AUTHENTICATION_TOKEN_HEADER_NAME, getAuthToken())
.contentType(ContentType.JSON)
Expand All @@ -665,4 +706,25 @@ public void findByColumnAndInc() {
.body("data.docs[0]", jsonEquals(expectedDoc));
}
}

private void insertDoc(String docJson) {
String doc =
"""
{
"insertOne": {
"document": %s
}
}
"""
.formatted(docJson);

given()
.header(HttpConstants.AUTHENTICATION_TOKEN_HEADER_NAME, getAuthToken())
.contentType(ContentType.JSON)
.body(doc)
.when()
.post(CollectionResource.BASE_PATH, keyspaceId.asInternal(), collectionName)
.then()
.statusCode(200);
}
}
Loading