From cd8c65a5cde49c194dfa03d33d62f112b602618a Mon Sep 17 00:00:00 2001 From: Tatu Saloranta Date: Wed, 8 Mar 2023 11:41:19 -0800 Subject: [PATCH 1/5] Add skeletal $min/$max handlers --- .../clause/update/MinMaxOperation.java | 78 +++++++++++++++++++ .../command/clause/update/UpdateOperator.java | 14 ++++ 2 files changed, 92 insertions(+) create mode 100644 src/main/java/io/stargate/sgv2/jsonapi/api/model/command/clause/update/MinMaxOperation.java diff --git a/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/clause/update/MinMaxOperation.java b/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/clause/update/MinMaxOperation.java new file mode 100644 index 0000000000..a07590ac5e --- /dev/null +++ b/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/clause/update/MinMaxOperation.java @@ -0,0 +1,78 @@ +package io.stargate.sgv2.jsonapi.api.model.command.clause.update; + +import com.fasterxml.jackson.databind.JsonNode; +import com.fasterxml.jackson.databind.node.ObjectNode; +import java.util.ArrayList; +import java.util.Iterator; +import java.util.List; +import java.util.Map; + +/** + * Implementation of {@code $inc} update operation used to modify numeric field values in documents. + * See {@href https://www.mongodb.com/docs/manual/reference/operator/update/inc/} for full + * explanation. + */ +public class MinMaxOperation extends UpdateOperation { + private final List actions; + + private final boolean isMaxAction; + + private MinMaxOperation(boolean isMaxAction, List actions) { + this.isMaxAction = isMaxAction; + this.actions = sortByPath(actions); + } + + public static MinMaxOperation constructMax(ObjectNode args) { + return construct(args, UpdateOperator.MAX, true); + } + + public static MinMaxOperation constructMin(ObjectNode args) { + return construct(args, UpdateOperator.MIN, false); + } + + private static MinMaxOperation construct(ObjectNode args, UpdateOperator oper, boolean isMax) { + Iterator> fieldIter = args.fields(); + + List actions = new ArrayList<>(); + while (fieldIter.hasNext()) { + Map.Entry entry = fieldIter.next(); + // Verify we do not try to change doc id + String path = validateUpdatePath(oper, entry.getKey()); + actions.add(new MinMaxAction(path, entry.getValue())); + } + return new MinMaxOperation(isMax, actions); + } + + @Override + public boolean updateDocument(ObjectNode doc, UpdateTargetLocator targetLocator) { + // Almost always changes, except if adding zero; need to track + boolean modified = false; + for (MinMaxAction action : actions) { + final String path = action.path; + final JsonNode value = action.value; + + UpdateTarget target = targetLocator.findOrCreate(doc, path); + JsonNode oldValue = target.valueNode(); + + if (oldValue == null) { // No such property? Add value + target.replaceValue(value); + modified = true; + } else { // Otherwise, need to see if less-than (min) or greater-than (max) + if (shouldReplace(oldValue, value)) { + target.replaceValue(value); + modified = true; + } + } + } + + return modified; + } + + private boolean shouldReplace(JsonNode oldValue, JsonNode newValue) { + // !!! TODO + return true; + } + + /** Value class for per-field update operations. */ + private record MinMaxAction(String path, JsonNode value) implements ActionWithPath {} +} diff --git a/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/clause/update/UpdateOperator.java b/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/clause/update/UpdateOperator.java index 5491203c46..8998307cdf 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/clause/update/UpdateOperator.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/clause/update/UpdateOperator.java @@ -22,6 +22,20 @@ public UpdateOperation resolveOperation(ObjectNode arguments) { } }, + MAX("$max") { + @Override + public UpdateOperation resolveOperation(ObjectNode arguments) { + return MinMaxOperation.constructMax(arguments); + } + }, + + MIN("$min") { + @Override + public UpdateOperation resolveOperation(ObjectNode arguments) { + return MinMaxOperation.constructMin(arguments); + } + }, + POP("$pop") { @Override public UpdateOperation resolveOperation(ObjectNode arguments) { From a65af960c7adccc563be2615b7593325f3dc7707 Mon Sep 17 00:00:00 2001 From: Tatu Saloranta Date: Wed, 8 Mar 2023 17:05:05 -0800 Subject: [PATCH 2/5] Add JsonNodeComparator, tests for it --- .../sgv2/jsonapi/util/JsonNodeComparator.java | 126 +++++++++++++++ .../clause/update/MinMaxOperationTest.java | 48 ++++++ .../jsonapi/util/JsonNodeComparatorTest.java | 153 ++++++++++++++++++ 3 files changed, 327 insertions(+) create mode 100644 src/main/java/io/stargate/sgv2/jsonapi/util/JsonNodeComparator.java create mode 100644 src/test/java/io/stargate/sgv2/jsonapi/service/operation/model/command/clause/update/MinMaxOperationTest.java create mode 100644 src/test/java/io/stargate/sgv2/jsonapi/util/JsonNodeComparatorTest.java diff --git a/src/main/java/io/stargate/sgv2/jsonapi/util/JsonNodeComparator.java b/src/main/java/io/stargate/sgv2/jsonapi/util/JsonNodeComparator.java new file mode 100644 index 0000000000..f305528f62 --- /dev/null +++ b/src/main/java/io/stargate/sgv2/jsonapi/util/JsonNodeComparator.java @@ -0,0 +1,126 @@ +package io.stargate.sgv2.jsonapi.util; + +import com.fasterxml.jackson.databind.JsonNode; +import com.fasterxml.jackson.databind.node.ArrayNode; +import com.fasterxml.jackson.databind.node.JsonNodeType; +import com.fasterxml.jackson.databind.node.ObjectNode; + +import java.math.BigDecimal; +import java.util.Comparator; +import java.util.Iterator; +import java.util.Map; + +public class JsonNodeComparator implements Comparator +{ + private final static Comparator ASC = new JsonNodeComparator(); + + private final static Comparator DESC = ASC.reversed(); + + public static Comparator ascending() { return ASC; } + public static Comparator descending() { return DESC; } + + @Override + public int compare(JsonNode o1, JsonNode o2) { + JsonNodeType type1 = o1.getNodeType(); + JsonNodeType type2 = o2.getNodeType(); + + // If value types differ, base on type precedence as per Mongo specs: + if (type1 != type2) { + return typePriority(type1) - typePriority(type2); + } + + switch (type1) { + case NULL: + return 0; // nulls are same so... + case NUMBER: + return compareNumbers(o1.decimalValue(), o2.decimalValue()); + case STRING: + return compareStrings(o1.textValue(), o2.textValue()); + case OBJECT: + return compareObjects((ObjectNode) o1, (ObjectNode) o2); + case ARRAY: + return compareArrays((ArrayNode) o1, (ArrayNode) o2); + case BOOLEAN: + return compareBooleans(o1.booleanValue(), o2.booleanValue()); + default: + // Should never happen: + throw new IllegalStateException("Unsupported JsonNodeType for comparison: "+type1); + } + } + + private int compareBooleans(boolean b1, boolean b2) { + if (b1 == b2) { + return 0; + } + return b1 ? 1 : -1; + } + + private int compareNumbers(BigDecimal n1, BigDecimal n2) { + return n1.compareTo(n2); + } + + private int compareStrings(String n1, String n2) { + return n1.compareTo(n2); + } + + private int compareArrays(ArrayNode n1, ArrayNode n2) { + final int len1 = n1.size(); + final int len2 = n2.size(); + + // First: compare first N entries that are common + for (int i = 0, end = Math.min(len1, len2); i < end; ++i) { + int diff = compare(n1.get(i), n2.get(i)); + if (diff != 0) { + return diff; + } + } + + // and if no difference, longer Array has higher precedence + return len1 - len2; + } + + private int compareObjects(ObjectNode n1, ObjectNode n2) { + // Object comparison is interesting: compares entries in order, + // first by property name, then by value. If all else equal, "longer one wins" + Iterator> it1 = n1.fields(); + Iterator> it2 = n2.fields(); + + while (it1.hasNext() && it2.hasNext()) { + Map.Entry entry1 = it1.next(); + Map.Entry entry2 = it2.next(); + + // First, key: + int diff = entry1.getKey().compareTo(entry2.getKey()); + if (diff == 0) { + // If key same, then value + diff = compare(entry1.getValue(), entry2.getValue()); + if (diff == 0) { + continue; + } + } + return diff; + } + + // Longer one wins, otherwise + return n1.size() - n2.size(); + } + + private int typePriority(JsonNodeType type) { + switch (type) { + case NULL: + return 0; + case NUMBER: + return 1; + case STRING: + return 2; + case OBJECT: + return 3; + case ARRAY: + return 4; + case BOOLEAN: + return 5; + default: + return 6; + } + } +} diff --git a/src/test/java/io/stargate/sgv2/jsonapi/service/operation/model/command/clause/update/MinMaxOperationTest.java b/src/test/java/io/stargate/sgv2/jsonapi/service/operation/model/command/clause/update/MinMaxOperationTest.java new file mode 100644 index 0000000000..9646e2e222 --- /dev/null +++ b/src/test/java/io/stargate/sgv2/jsonapi/service/operation/model/command/clause/update/MinMaxOperationTest.java @@ -0,0 +1,48 @@ +package io.stargate.sgv2.jsonapi.service.operation.model.command.clause.update; + +import com.fasterxml.jackson.databind.node.ObjectNode; +import io.quarkus.test.junit.QuarkusTest; +import io.quarkus.test.junit.TestProfile; +import io.stargate.sgv2.common.testprofiles.NoGlobalResourcesTestProfile; +import io.stargate.sgv2.jsonapi.api.model.command.clause.update.IncOperation; +import io.stargate.sgv2.jsonapi.api.model.command.clause.update.UpdateOperation; +import io.stargate.sgv2.jsonapi.api.model.command.clause.update.UpdateOperator; +import org.junit.jupiter.api.MethodOrderer; +import org.junit.jupiter.api.Nested; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.TestMethodOrder; + +import static org.assertj.core.api.Assertions.assertThat; + +@QuarkusTest +@TestProfile(NoGlobalResourcesTestProfile.Impl.class) +public class MinMaxOperationTest extends UpdateOperationTestBase { + @Nested + @TestMethodOrder(MethodOrderer.OrderAnnotation.class) + class HappyPathMin { + @Test + public void testSimpleMinRoot() { + ObjectNode doc = objectFromJson("{ \"x\": 1, \"y\":2}"); + // 3 updates: 2 for existing property, one for not + UpdateOperation oper = + UpdateOperator.MIN.resolveOperation( + objectFromJson("{ \"x\": -1, \"y\":99, \"z\":0}")); + assertThat(oper.updateDocument(doc, targetLocator)).isTrue(); + ObjectNode expected = objectFromJson("{ \"x\": -1, \"y\":2, \"z\":0}"); + assertThat(doc).isEqualTo(expected); + } + + @Test + public void testSimpleMaxRoot() { + ObjectNode doc = objectFromJson("{ \"x\": 1, \"y\":2}"); + // 3 updates: 2 for existing property, one for not + UpdateOperation oper = + UpdateOperator.MAX.resolveOperation( + objectFromJson("{ \"x\": -1, \"y\":99, \"z\":0}")); + assertThat(oper.updateDocument(doc, targetLocator)).isTrue(); + ObjectNode expected = objectFromJson("{ \"x\": 1, \"y\":99, \"z\":0}"); + assertThat(doc).isEqualTo(expected); + } + } + +} diff --git a/src/test/java/io/stargate/sgv2/jsonapi/util/JsonNodeComparatorTest.java b/src/test/java/io/stargate/sgv2/jsonapi/util/JsonNodeComparatorTest.java new file mode 100644 index 0000000000..81e83576a9 --- /dev/null +++ b/src/test/java/io/stargate/sgv2/jsonapi/util/JsonNodeComparatorTest.java @@ -0,0 +1,153 @@ +package io.stargate.sgv2.jsonapi.util; + +import com.fasterxml.jackson.databind.JsonNode; +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 org.junit.jupiter.api.Test; + +import javax.inject.Inject; +import java.io.IOException; +import java.util.Comparator; + +import static org.assertj.core.api.Assertions.assertThat; + +// No need for injection +public class JsonNodeComparatorTest { + private final ObjectMapper mapper = new ObjectMapper(); + + private final Comparator COMP = JsonNodeComparator.ascending(); + + @Test + public void testOrderingBoolean() { + _verifyIdentityEquals("true"); + _verifyIdentityEquals("false"); + + _verifyAscending("false", "true"); + } + + @Test + public void testOrderingNumbers() { + _verifyIdentityEquals("0"); + _verifyIdentityEquals("-9"); + _verifyIdentityEquals("0.25"); + + _verifyAscending("0", "1"); + _verifyAscending("1", "2"); + _verifyAscending("-1", "0"); + _verifyAscending("0.0", "0.05"); + _verifyAscending("0.101", "0.11"); + _verifyAscending("1", "1.25"); + _verifyAscending("-10", "1"); + } + + @Test + public void testOrderingStrings() { + _verifyIdentityEquals("\"\""); + _verifyIdentityEquals("\"abc\""); + _verifyIdentityEquals("\"xyz 125\""); + + _verifyAscending("\"a\"", "\"b\""); + _verifyAscending("\"a100\"", "\"a99\""); + _verifyAscending("\"abc\"", "\"abca\""); + } + + @Test + public void testOrderingArrays() { + _verifyIdentityEquals("[]"); + _verifyIdentityEquals("[1, false, 3]"); + _verifyIdentityEquals("[1, \"foo\", { \"z\": true }]"); + + // Ordered comparison by element, recursively. If one subset of the other, + // longer one sorted last + _verifyAscending("[0]", "[1]"); + _verifyAscending("[1,0]", "[1,1]"); + _verifyAscending("[1,2]", "[1,2,0]"); + _verifyAscending("[1,[0]]", "[1,[1]]"); + } + + @Test + public void testOrderingObjects() { + _verifyIdentityEquals("{}"); + _verifyIdentityEquals("{\"x\":1, \"y\":2, \"arr\": [ 1, 2, 3]}"); + + // Ordered comparison by field: first field name, then value; if same fields + // starting, longer one sorted last + _verifyAscending("{\"a\":1}", "{\"b\":0}"); + _verifyAscending("{\"a\":1}", "{\"a\":2}"); + _verifyAscending("{\"a\":1}", "{\"a\":1,\"b\":0}"); + } + + @Test + public void testOrderingMixedScalars() { + // Ordering by type: NULL, NUMBER, STRING, OBJECT, ARRAY, BOOLEAN + _verifyAscending("null", "1"); + _verifyAscending("null", "\"0\""); + _verifyAscending("null", "{}"); + _verifyAscending("null", "[]"); + _verifyAscending("null", "false"); + + _verifyAscending("1", "\"0\""); + _verifyAscending("1", "{}"); + _verifyAscending("1", "[]"); + _verifyAscending("1", "true"); + + _verifyAscending("\"abc\"", "{}"); + _verifyAscending("\"abc\"", "[]"); + _verifyAscending("\"abc\"", "false"); + + _verifyAscending("{}", "[]"); + _verifyAscending("{}", "false"); + + _verifyAscending("[]", "false"); + } + + @Test + public void testOrderingMixedArrays() { + // Ordering by type: NULL, NUMBER, STRING, OBJECT, ARRAY, BOOLEAN + _verifyAscending("[null]", "[1]"); + _verifyAscending("[null]", "[\"0\"]"); + _verifyAscending("[null]", "[{}]"); + _verifyAscending("[null]", "[[]]"); + _verifyAscending("[null]", "[false]"); + + _verifyAscending("[1,5]", "[1,\"abc\"]"); + _verifyAscending("[1,5]", "[1,{}]"); + _verifyAscending("[1,5]", "[1,[14]]"); + _verifyAscending("[1,5]", "[1,true]"); + } + + @Test + public void testOrderingMixedObject() { + // Ordering first by field name, then by type + _verifyAscending("{}", "{\"a\":1}"); + _verifyAscending("{\"x\":2}", "{\"y\":1}"); + _verifyAscending("{\"x\":1}", "{\"x\":\"value\"}"); + _verifyAscending("{\"x\":1}", "{\"x\":1,\"a\":null}"); + } + + private void _verifyAscending(String json1, String json2) { + // verify symmetry by comparing both as given and the reverse + assertThat(COMP.compare(jsonNode(json1), jsonNode(json2))).isLessThan(0); + assertThat(COMP.compare(jsonNode(json2), jsonNode(json1))).isGreaterThan(0); + } + + private void _verifyIdentityEquals(String jsonValue) { + _verifyEquals(jsonValue, jsonValue); + } + + private void _verifyEquals(String json1, String json2) { + // verify both ways to ensure comparison is symmetric + assertThat(COMP.compare(jsonNode(json1), jsonNode(json2))).isEqualTo(0); + assertThat(COMP.compare(jsonNode(json2), jsonNode(json1))).isEqualTo(0); + } + + private JsonNode jsonNode(String json) { + try { + return mapper.readTree(json); + } catch (IOException e) { + throw new RuntimeException(e); + } + } +} From 9ec656e0aa0680cab7a9fd6644e0d31929498cff Mon Sep 17 00:00:00 2001 From: Tatu Saloranta Date: Wed, 8 Mar 2023 17:05:31 -0800 Subject: [PATCH 3/5] mvn fmt:format --- .../sgv2/jsonapi/util/JsonNodeComparator.java | 199 +++++++------ .../clause/update/MinMaxOperationTest.java | 54 ++-- .../jsonapi/util/JsonNodeComparatorTest.java | 279 +++++++++--------- 3 files changed, 263 insertions(+), 269 deletions(-) diff --git a/src/main/java/io/stargate/sgv2/jsonapi/util/JsonNodeComparator.java b/src/main/java/io/stargate/sgv2/jsonapi/util/JsonNodeComparator.java index f305528f62..0ee9ec4dbf 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/util/JsonNodeComparator.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/util/JsonNodeComparator.java @@ -4,123 +4,126 @@ import com.fasterxml.jackson.databind.node.ArrayNode; import com.fasterxml.jackson.databind.node.JsonNodeType; import com.fasterxml.jackson.databind.node.ObjectNode; - import java.math.BigDecimal; import java.util.Comparator; import java.util.Iterator; import java.util.Map; -public class JsonNodeComparator implements Comparator -{ - private final static Comparator ASC = new JsonNodeComparator(); - - private final static Comparator DESC = ASC.reversed(); +public class JsonNodeComparator implements Comparator { + private static final Comparator ASC = new JsonNodeComparator(); - public static Comparator ascending() { return ASC; } - public static Comparator descending() { return DESC; } + private static final Comparator DESC = ASC.reversed(); - @Override - public int compare(JsonNode o1, JsonNode o2) { - JsonNodeType type1 = o1.getNodeType(); - JsonNodeType type2 = o2.getNodeType(); + public static Comparator ascending() { + return ASC; + } - // If value types differ, base on type precedence as per Mongo specs: - if (type1 != type2) { - return typePriority(type1) - typePriority(type2); - } + public static Comparator descending() { + return DESC; + } - switch (type1) { - case NULL: - return 0; // nulls are same so... - case NUMBER: - return compareNumbers(o1.decimalValue(), o2.decimalValue()); - case STRING: - return compareStrings(o1.textValue(), o2.textValue()); - case OBJECT: - return compareObjects((ObjectNode) o1, (ObjectNode) o2); - case ARRAY: - return compareArrays((ArrayNode) o1, (ArrayNode) o2); - case BOOLEAN: - return compareBooleans(o1.booleanValue(), o2.booleanValue()); - default: - // Should never happen: - throw new IllegalStateException("Unsupported JsonNodeType for comparison: "+type1); - } - } + @Override + public int compare(JsonNode o1, JsonNode o2) { + JsonNodeType type1 = o1.getNodeType(); + JsonNodeType type2 = o2.getNodeType(); - private int compareBooleans(boolean b1, boolean b2) { - if (b1 == b2) { - return 0; - } - return b1 ? 1 : -1; + // If value types differ, base on type precedence as per Mongo specs: + if (type1 != type2) { + return typePriority(type1) - typePriority(type2); } - private int compareNumbers(BigDecimal n1, BigDecimal n2) { - return n1.compareTo(n2); + switch (type1) { + case NULL: + return 0; // nulls are same so... + case NUMBER: + return compareNumbers(o1.decimalValue(), o2.decimalValue()); + case STRING: + return compareStrings(o1.textValue(), o2.textValue()); + case OBJECT: + return compareObjects((ObjectNode) o1, (ObjectNode) o2); + case ARRAY: + return compareArrays((ArrayNode) o1, (ArrayNode) o2); + case BOOLEAN: + return compareBooleans(o1.booleanValue(), o2.booleanValue()); + default: + // Should never happen: + throw new IllegalStateException("Unsupported JsonNodeType for comparison: " + type1); } + } - private int compareStrings(String n1, String n2) { - return n1.compareTo(n2); + private int compareBooleans(boolean b1, boolean b2) { + if (b1 == b2) { + return 0; } - - private int compareArrays(ArrayNode n1, ArrayNode n2) { - final int len1 = n1.size(); - final int len2 = n2.size(); - - // First: compare first N entries that are common - for (int i = 0, end = Math.min(len1, len2); i < end; ++i) { - int diff = compare(n1.get(i), n2.get(i)); - if (diff != 0) { - return diff; - } - } - - // and if no difference, longer Array has higher precedence - return len1 - len2; + return b1 ? 1 : -1; + } + + private int compareNumbers(BigDecimal n1, BigDecimal n2) { + return n1.compareTo(n2); + } + + private int compareStrings(String n1, String n2) { + return n1.compareTo(n2); + } + + private int compareArrays(ArrayNode n1, ArrayNode n2) { + final int len1 = n1.size(); + final int len2 = n2.size(); + + // First: compare first N entries that are common + for (int i = 0, end = Math.min(len1, len2); i < end; ++i) { + int diff = compare(n1.get(i), n2.get(i)); + if (diff != 0) { + return diff; + } } - private int compareObjects(ObjectNode n1, ObjectNode n2) { - // Object comparison is interesting: compares entries in order, - // first by property name, then by value. If all else equal, "longer one wins" - Iterator> it1 = n1.fields(); - Iterator> it2 = n2.fields(); - - while (it1.hasNext() && it2.hasNext()) { - Map.Entry entry1 = it1.next(); - Map.Entry entry2 = it2.next(); - - // First, key: - int diff = entry1.getKey().compareTo(entry2.getKey()); - if (diff == 0) { - // If key same, then value - diff = compare(entry1.getValue(), entry2.getValue()); - if (diff == 0) { - continue; - } - } - return diff; + // and if no difference, longer Array has higher precedence + return len1 - len2; + } + + private int compareObjects(ObjectNode n1, ObjectNode n2) { + // Object comparison is interesting: compares entries in order, + // first by property name, then by value. If all else equal, "longer one wins" + Iterator> it1 = n1.fields(); + Iterator> it2 = n2.fields(); + + while (it1.hasNext() && it2.hasNext()) { + Map.Entry entry1 = it1.next(); + Map.Entry entry2 = it2.next(); + + // First, key: + int diff = entry1.getKey().compareTo(entry2.getKey()); + if (diff == 0) { + // If key same, then value + diff = compare(entry1.getValue(), entry2.getValue()); + if (diff == 0) { + continue; } - - // Longer one wins, otherwise - return n1.size() - n2.size(); + } + return diff; } - private int typePriority(JsonNodeType type) { - switch (type) { - case NULL: - return 0; - case NUMBER: - return 1; - case STRING: - return 2; - case OBJECT: - return 3; - case ARRAY: - return 4; - case BOOLEAN: - return 5; - default: - return 6; - } + // Longer one wins, otherwise + return n1.size() - n2.size(); + } + + private int typePriority(JsonNodeType type) { + switch (type) { + case NULL: + return 0; + case NUMBER: + return 1; + case STRING: + return 2; + case OBJECT: + return 3; + case ARRAY: + return 4; + case BOOLEAN: + return 5; + default: + return 6; } + } } diff --git a/src/test/java/io/stargate/sgv2/jsonapi/service/operation/model/command/clause/update/MinMaxOperationTest.java b/src/test/java/io/stargate/sgv2/jsonapi/service/operation/model/command/clause/update/MinMaxOperationTest.java index 9646e2e222..7417785bad 100644 --- a/src/test/java/io/stargate/sgv2/jsonapi/service/operation/model/command/clause/update/MinMaxOperationTest.java +++ b/src/test/java/io/stargate/sgv2/jsonapi/service/operation/model/command/clause/update/MinMaxOperationTest.java @@ -1,10 +1,11 @@ package io.stargate.sgv2.jsonapi.service.operation.model.command.clause.update; +import static org.assertj.core.api.Assertions.assertThat; + import com.fasterxml.jackson.databind.node.ObjectNode; import io.quarkus.test.junit.QuarkusTest; import io.quarkus.test.junit.TestProfile; import io.stargate.sgv2.common.testprofiles.NoGlobalResourcesTestProfile; -import io.stargate.sgv2.jsonapi.api.model.command.clause.update.IncOperation; import io.stargate.sgv2.jsonapi.api.model.command.clause.update.UpdateOperation; import io.stargate.sgv2.jsonapi.api.model.command.clause.update.UpdateOperator; import org.junit.jupiter.api.MethodOrderer; @@ -12,37 +13,32 @@ import org.junit.jupiter.api.Test; import org.junit.jupiter.api.TestMethodOrder; -import static org.assertj.core.api.Assertions.assertThat; - @QuarkusTest @TestProfile(NoGlobalResourcesTestProfile.Impl.class) public class MinMaxOperationTest extends UpdateOperationTestBase { - @Nested - @TestMethodOrder(MethodOrderer.OrderAnnotation.class) - class HappyPathMin { - @Test - public void testSimpleMinRoot() { - ObjectNode doc = objectFromJson("{ \"x\": 1, \"y\":2}"); - // 3 updates: 2 for existing property, one for not - UpdateOperation oper = - UpdateOperator.MIN.resolveOperation( - objectFromJson("{ \"x\": -1, \"y\":99, \"z\":0}")); - assertThat(oper.updateDocument(doc, targetLocator)).isTrue(); - ObjectNode expected = objectFromJson("{ \"x\": -1, \"y\":2, \"z\":0}"); - assertThat(doc).isEqualTo(expected); - } - - @Test - public void testSimpleMaxRoot() { - ObjectNode doc = objectFromJson("{ \"x\": 1, \"y\":2}"); - // 3 updates: 2 for existing property, one for not - UpdateOperation oper = - UpdateOperator.MAX.resolveOperation( - objectFromJson("{ \"x\": -1, \"y\":99, \"z\":0}")); - assertThat(oper.updateDocument(doc, targetLocator)).isTrue(); - ObjectNode expected = objectFromJson("{ \"x\": 1, \"y\":99, \"z\":0}"); - assertThat(doc).isEqualTo(expected); - } + @Nested + @TestMethodOrder(MethodOrderer.OrderAnnotation.class) + class HappyPathMin { + @Test + public void testSimpleMinRoot() { + ObjectNode doc = objectFromJson("{ \"x\": 1, \"y\":2}"); + // 3 updates: 2 for existing property, one for not + UpdateOperation oper = + UpdateOperator.MIN.resolveOperation(objectFromJson("{ \"x\": -1, \"y\":99, \"z\":0}")); + assertThat(oper.updateDocument(doc, targetLocator)).isTrue(); + ObjectNode expected = objectFromJson("{ \"x\": -1, \"y\":2, \"z\":0}"); + assertThat(doc).isEqualTo(expected); } + @Test + public void testSimpleMaxRoot() { + ObjectNode doc = objectFromJson("{ \"x\": 1, \"y\":2}"); + // 3 updates: 2 for existing property, one for not + UpdateOperation oper = + UpdateOperator.MAX.resolveOperation(objectFromJson("{ \"x\": -1, \"y\":99, \"z\":0}")); + assertThat(oper.updateDocument(doc, targetLocator)).isTrue(); + ObjectNode expected = objectFromJson("{ \"x\": 1, \"y\":99, \"z\":0}"); + assertThat(doc).isEqualTo(expected); + } + } } diff --git a/src/test/java/io/stargate/sgv2/jsonapi/util/JsonNodeComparatorTest.java b/src/test/java/io/stargate/sgv2/jsonapi/util/JsonNodeComparatorTest.java index 81e83576a9..ecae5ff2b4 100644 --- a/src/test/java/io/stargate/sgv2/jsonapi/util/JsonNodeComparatorTest.java +++ b/src/test/java/io/stargate/sgv2/jsonapi/util/JsonNodeComparatorTest.java @@ -1,153 +1,148 @@ package io.stargate.sgv2.jsonapi.util; +import static org.assertj.core.api.Assertions.assertThat; + import com.fasterxml.jackson.databind.JsonNode; 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 org.junit.jupiter.api.Test; - -import javax.inject.Inject; import java.io.IOException; import java.util.Comparator; - -import static org.assertj.core.api.Assertions.assertThat; +import org.junit.jupiter.api.Test; // No need for injection public class JsonNodeComparatorTest { - private final ObjectMapper mapper = new ObjectMapper(); - - private final Comparator COMP = JsonNodeComparator.ascending(); - - @Test - public void testOrderingBoolean() { - _verifyIdentityEquals("true"); - _verifyIdentityEquals("false"); - - _verifyAscending("false", "true"); - } - - @Test - public void testOrderingNumbers() { - _verifyIdentityEquals("0"); - _verifyIdentityEquals("-9"); - _verifyIdentityEquals("0.25"); - - _verifyAscending("0", "1"); - _verifyAscending("1", "2"); - _verifyAscending("-1", "0"); - _verifyAscending("0.0", "0.05"); - _verifyAscending("0.101", "0.11"); - _verifyAscending("1", "1.25"); - _verifyAscending("-10", "1"); - } - - @Test - public void testOrderingStrings() { - _verifyIdentityEquals("\"\""); - _verifyIdentityEquals("\"abc\""); - _verifyIdentityEquals("\"xyz 125\""); - - _verifyAscending("\"a\"", "\"b\""); - _verifyAscending("\"a100\"", "\"a99\""); - _verifyAscending("\"abc\"", "\"abca\""); - } - - @Test - public void testOrderingArrays() { - _verifyIdentityEquals("[]"); - _verifyIdentityEquals("[1, false, 3]"); - _verifyIdentityEquals("[1, \"foo\", { \"z\": true }]"); - - // Ordered comparison by element, recursively. If one subset of the other, - // longer one sorted last - _verifyAscending("[0]", "[1]"); - _verifyAscending("[1,0]", "[1,1]"); - _verifyAscending("[1,2]", "[1,2,0]"); - _verifyAscending("[1,[0]]", "[1,[1]]"); - } - - @Test - public void testOrderingObjects() { - _verifyIdentityEquals("{}"); - _verifyIdentityEquals("{\"x\":1, \"y\":2, \"arr\": [ 1, 2, 3]}"); - - // Ordered comparison by field: first field name, then value; if same fields - // starting, longer one sorted last - _verifyAscending("{\"a\":1}", "{\"b\":0}"); - _verifyAscending("{\"a\":1}", "{\"a\":2}"); - _verifyAscending("{\"a\":1}", "{\"a\":1,\"b\":0}"); - } - - @Test - public void testOrderingMixedScalars() { - // Ordering by type: NULL, NUMBER, STRING, OBJECT, ARRAY, BOOLEAN - _verifyAscending("null", "1"); - _verifyAscending("null", "\"0\""); - _verifyAscending("null", "{}"); - _verifyAscending("null", "[]"); - _verifyAscending("null", "false"); - - _verifyAscending("1", "\"0\""); - _verifyAscending("1", "{}"); - _verifyAscending("1", "[]"); - _verifyAscending("1", "true"); - - _verifyAscending("\"abc\"", "{}"); - _verifyAscending("\"abc\"", "[]"); - _verifyAscending("\"abc\"", "false"); - - _verifyAscending("{}", "[]"); - _verifyAscending("{}", "false"); - - _verifyAscending("[]", "false"); - } - - @Test - public void testOrderingMixedArrays() { - // Ordering by type: NULL, NUMBER, STRING, OBJECT, ARRAY, BOOLEAN - _verifyAscending("[null]", "[1]"); - _verifyAscending("[null]", "[\"0\"]"); - _verifyAscending("[null]", "[{}]"); - _verifyAscending("[null]", "[[]]"); - _verifyAscending("[null]", "[false]"); - - _verifyAscending("[1,5]", "[1,\"abc\"]"); - _verifyAscending("[1,5]", "[1,{}]"); - _verifyAscending("[1,5]", "[1,[14]]"); - _verifyAscending("[1,5]", "[1,true]"); - } - - @Test - public void testOrderingMixedObject() { - // Ordering first by field name, then by type - _verifyAscending("{}", "{\"a\":1}"); - _verifyAscending("{\"x\":2}", "{\"y\":1}"); - _verifyAscending("{\"x\":1}", "{\"x\":\"value\"}"); - _verifyAscending("{\"x\":1}", "{\"x\":1,\"a\":null}"); - } - - private void _verifyAscending(String json1, String json2) { - // verify symmetry by comparing both as given and the reverse - assertThat(COMP.compare(jsonNode(json1), jsonNode(json2))).isLessThan(0); - assertThat(COMP.compare(jsonNode(json2), jsonNode(json1))).isGreaterThan(0); - } - - private void _verifyIdentityEquals(String jsonValue) { - _verifyEquals(jsonValue, jsonValue); - } - - private void _verifyEquals(String json1, String json2) { - // verify both ways to ensure comparison is symmetric - assertThat(COMP.compare(jsonNode(json1), jsonNode(json2))).isEqualTo(0); - assertThat(COMP.compare(jsonNode(json2), jsonNode(json1))).isEqualTo(0); - } - - private JsonNode jsonNode(String json) { - try { - return mapper.readTree(json); - } catch (IOException e) { - throw new RuntimeException(e); - } + private final ObjectMapper mapper = new ObjectMapper(); + + private final Comparator COMP = JsonNodeComparator.ascending(); + + @Test + public void testOrderingBoolean() { + _verifyIdentityEquals("true"); + _verifyIdentityEquals("false"); + + _verifyAscending("false", "true"); + } + + @Test + public void testOrderingNumbers() { + _verifyIdentityEquals("0"); + _verifyIdentityEquals("-9"); + _verifyIdentityEquals("0.25"); + + _verifyAscending("0", "1"); + _verifyAscending("1", "2"); + _verifyAscending("-1", "0"); + _verifyAscending("0.0", "0.05"); + _verifyAscending("0.101", "0.11"); + _verifyAscending("1", "1.25"); + _verifyAscending("-10", "1"); + } + + @Test + public void testOrderingStrings() { + _verifyIdentityEquals("\"\""); + _verifyIdentityEquals("\"abc\""); + _verifyIdentityEquals("\"xyz 125\""); + + _verifyAscending("\"a\"", "\"b\""); + _verifyAscending("\"a100\"", "\"a99\""); + _verifyAscending("\"abc\"", "\"abca\""); + } + + @Test + public void testOrderingArrays() { + _verifyIdentityEquals("[]"); + _verifyIdentityEquals("[1, false, 3]"); + _verifyIdentityEquals("[1, \"foo\", { \"z\": true }]"); + + // Ordered comparison by element, recursively. If one subset of the other, + // longer one sorted last + _verifyAscending("[0]", "[1]"); + _verifyAscending("[1,0]", "[1,1]"); + _verifyAscending("[1,2]", "[1,2,0]"); + _verifyAscending("[1,[0]]", "[1,[1]]"); + } + + @Test + public void testOrderingObjects() { + _verifyIdentityEquals("{}"); + _verifyIdentityEquals("{\"x\":1, \"y\":2, \"arr\": [ 1, 2, 3]}"); + + // Ordered comparison by field: first field name, then value; if same fields + // starting, longer one sorted last + _verifyAscending("{\"a\":1}", "{\"b\":0}"); + _verifyAscending("{\"a\":1}", "{\"a\":2}"); + _verifyAscending("{\"a\":1}", "{\"a\":1,\"b\":0}"); + } + + @Test + public void testOrderingMixedScalars() { + // Ordering by type: NULL, NUMBER, STRING, OBJECT, ARRAY, BOOLEAN + _verifyAscending("null", "1"); + _verifyAscending("null", "\"0\""); + _verifyAscending("null", "{}"); + _verifyAscending("null", "[]"); + _verifyAscending("null", "false"); + + _verifyAscending("1", "\"0\""); + _verifyAscending("1", "{}"); + _verifyAscending("1", "[]"); + _verifyAscending("1", "true"); + + _verifyAscending("\"abc\"", "{}"); + _verifyAscending("\"abc\"", "[]"); + _verifyAscending("\"abc\"", "false"); + + _verifyAscending("{}", "[]"); + _verifyAscending("{}", "false"); + + _verifyAscending("[]", "false"); + } + + @Test + public void testOrderingMixedArrays() { + // Ordering by type: NULL, NUMBER, STRING, OBJECT, ARRAY, BOOLEAN + _verifyAscending("[null]", "[1]"); + _verifyAscending("[null]", "[\"0\"]"); + _verifyAscending("[null]", "[{}]"); + _verifyAscending("[null]", "[[]]"); + _verifyAscending("[null]", "[false]"); + + _verifyAscending("[1,5]", "[1,\"abc\"]"); + _verifyAscending("[1,5]", "[1,{}]"); + _verifyAscending("[1,5]", "[1,[14]]"); + _verifyAscending("[1,5]", "[1,true]"); + } + + @Test + public void testOrderingMixedObject() { + // Ordering first by field name, then by type + _verifyAscending("{}", "{\"a\":1}"); + _verifyAscending("{\"x\":2}", "{\"y\":1}"); + _verifyAscending("{\"x\":1}", "{\"x\":\"value\"}"); + _verifyAscending("{\"x\":1}", "{\"x\":1,\"a\":null}"); + } + + private void _verifyAscending(String json1, String json2) { + // verify symmetry by comparing both as given and the reverse + assertThat(COMP.compare(jsonNode(json1), jsonNode(json2))).isLessThan(0); + assertThat(COMP.compare(jsonNode(json2), jsonNode(json1))).isGreaterThan(0); + } + + private void _verifyIdentityEquals(String jsonValue) { + _verifyEquals(jsonValue, jsonValue); + } + + private void _verifyEquals(String json1, String json2) { + // verify both ways to ensure comparison is symmetric + assertThat(COMP.compare(jsonNode(json1), jsonNode(json2))).isEqualTo(0); + assertThat(COMP.compare(jsonNode(json2), jsonNode(json1))).isEqualTo(0); + } + + private JsonNode jsonNode(String json) { + try { + return mapper.readTree(json); + } catch (IOException e) { + throw new RuntimeException(e); } + } } From c85b3f2b60aa5cd18b3a4a4a244fd05406da6122 Mon Sep 17 00:00:00 2001 From: Tatu Saloranta Date: Wed, 8 Mar 2023 17:43:05 -0800 Subject: [PATCH 4/5] Complete $min, $max implementations, add integration tests --- .../clause/update/MinMaxOperation.java | 16 +- .../api/v1/FindAndUpdateIntegrationTest.java | 143 +++++++++++++++++- .../clause/update/MinMaxOperationTest.java | 94 +++++++++++- 3 files changed, 244 insertions(+), 9 deletions(-) diff --git a/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/clause/update/MinMaxOperation.java b/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/clause/update/MinMaxOperation.java index a07590ac5e..d95b040380 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/clause/update/MinMaxOperation.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/clause/update/MinMaxOperation.java @@ -2,15 +2,17 @@ import com.fasterxml.jackson.databind.JsonNode; import com.fasterxml.jackson.databind.node.ObjectNode; +import io.stargate.sgv2.jsonapi.util.JsonNodeComparator; import java.util.ArrayList; import java.util.Iterator; import java.util.List; import java.util.Map; /** - * Implementation of {@code $inc} update operation used to modify numeric field values in documents. - * See {@href https://www.mongodb.com/docs/manual/reference/operator/update/inc/} for full - * explanation. + * Implementation of {@code $min} and {@code $max} update operation used to modify numeric field + * values in documents. See {@href + * https://www.mongodb.com/docs/manual/reference/operator/update/min/} and {@href + * https://www.mongodb.com/docs/manual/reference/operator/update/max/} for full explanations. */ public class MinMaxOperation extends UpdateOperation { private final List actions; @@ -69,8 +71,12 @@ public boolean updateDocument(ObjectNode doc, UpdateTargetLocator targetLocator) } private boolean shouldReplace(JsonNode oldValue, JsonNode newValue) { - // !!! TODO - return true; + if (isMaxAction) { + // For $max, replace if newValue sorts later + return JsonNodeComparator.ascending().compare(oldValue, newValue) < 0; + } + // For $min, replace if newValue sorts earlier + return JsonNodeComparator.ascending().compare(oldValue, newValue) > 0; } /** Value class for per-field update operations. */ diff --git a/src/test/java/io/stargate/sgv2/jsonapi/api/v1/FindAndUpdateIntegrationTest.java b/src/test/java/io/stargate/sgv2/jsonapi/api/v1/FindAndUpdateIntegrationTest.java index 9afab74211..780a76f625 100644 --- a/src/test/java/io/stargate/sgv2/jsonapi/api/v1/FindAndUpdateIntegrationTest.java +++ b/src/test/java/io/stargate/sgv2/jsonapi/api/v1/FindAndUpdateIntegrationTest.java @@ -1687,7 +1687,6 @@ public void findByColumnAndAddToSetWithEach() { @Nested class UpdateOneWithSetOnInsert { @Test - @Order(2) public void findByIdUpsertAndAddOnInsert() { String json = """ @@ -1805,4 +1804,146 @@ private void insertDoc(String docJson) { .body("status.insertedIds[0]", not(emptyString())) .statusCode(200); } + + @Nested + class UpdateOneWithMin { + @Test + public void findByColumnAndMin() { + insertDoc( + """ + { + "_id": "update_doc_min", + "min": 1, + "max": 99, + "numbers": { + "values": [ 1 ] + } + } + """); + String updateJson = + """ + { + "updateOne": { + "filter" : {"_id" : "update_doc_min"}, + "update" : {"$min" : { + "min": 2, + "max" : 25, + "numbers.values" : [ -9 ] + } + } + } + } + """; + given() + .header(HttpConstants.AUTHENTICATION_TOKEN_HEADER_NAME, getAuthToken()) + .contentType(ContentType.JSON) + .body(updateJson) + .when() + .post(CollectionResource.BASE_PATH, keyspaceId.asInternal(), collectionName) + .then() + .statusCode(200) + .body("status.matchedCount", is(1)) + .body("status.modifiedCount", is(1)); + + String expectedDoc = + """ + { + "_id": "update_doc_min", + "min": 1, + "max": 25, + "numbers": { + "values": [ -9 ] + } + } + """; + String findJson = + """ + { + "find": { + "filter" : {"_id" : "update_doc_min"} + } + } + """; + given() + .header(HttpConstants.AUTHENTICATION_TOKEN_HEADER_NAME, getAuthToken()) + .contentType(ContentType.JSON) + .body(findJson) + .when() + .post(CollectionResource.BASE_PATH, keyspaceId.asInternal(), collectionName) + .then() + .statusCode(200) + .body("data.docs[0]", jsonEquals(expectedDoc)); + } + } + + @Nested + class UpdateOneWithMax { + @Test + public void findByColumnAndMax() { + insertDoc( + """ + { + "_id": "update_doc_max", + "min": 1, + "max": 99, + "numbers": { + "values": { "x":1, "y":2 } + } + } + """); + String updateJson = + """ + { + "updateOne": { + "filter" : {"_id" : "update_doc_max"}, + "update" : {"$max" : { + "min": 2, + "max" : 25, + "numbers.values": { "x":1, "y":3 } + } + } + } + } + """; + given() + .header(HttpConstants.AUTHENTICATION_TOKEN_HEADER_NAME, getAuthToken()) + .contentType(ContentType.JSON) + .body(updateJson) + .when() + .post(CollectionResource.BASE_PATH, keyspaceId.asInternal(), collectionName) + .then() + .statusCode(200) + .body("status.matchedCount", is(1)) + .body("status.modifiedCount", is(1)); + + String expectedDoc = + """ + { + "_id": "update_doc_max", + "min": 2, + "max": 99, + "numbers": { + "values": { "x":1, "y":3 } + } + } + """; + String findJson = + """ + { + "find": { + "filter" : {"_id" : "update_doc_max"} + } + } + """; + given() + .header(HttpConstants.AUTHENTICATION_TOKEN_HEADER_NAME, getAuthToken()) + .contentType(ContentType.JSON) + .body(findJson) + .when() + .post(CollectionResource.BASE_PATH, keyspaceId.asInternal(), collectionName) + .then() + .statusCode(200) + .body("data.docs[0]", jsonEquals(expectedDoc)); + } + } } diff --git a/src/test/java/io/stargate/sgv2/jsonapi/service/operation/model/command/clause/update/MinMaxOperationTest.java b/src/test/java/io/stargate/sgv2/jsonapi/service/operation/model/command/clause/update/MinMaxOperationTest.java index 7417785bad..df3bbc2e12 100644 --- a/src/test/java/io/stargate/sgv2/jsonapi/service/operation/model/command/clause/update/MinMaxOperationTest.java +++ b/src/test/java/io/stargate/sgv2/jsonapi/service/operation/model/command/clause/update/MinMaxOperationTest.java @@ -8,16 +8,13 @@ import io.stargate.sgv2.common.testprofiles.NoGlobalResourcesTestProfile; import io.stargate.sgv2.jsonapi.api.model.command.clause.update.UpdateOperation; import io.stargate.sgv2.jsonapi.api.model.command.clause.update.UpdateOperator; -import org.junit.jupiter.api.MethodOrderer; import org.junit.jupiter.api.Nested; import org.junit.jupiter.api.Test; -import org.junit.jupiter.api.TestMethodOrder; @QuarkusTest @TestProfile(NoGlobalResourcesTestProfile.Impl.class) public class MinMaxOperationTest extends UpdateOperationTestBase { @Nested - @TestMethodOrder(MethodOrderer.OrderAnnotation.class) class HappyPathMin { @Test public void testSimpleMinRoot() { @@ -30,6 +27,53 @@ public void testSimpleMinRoot() { assertThat(doc).isEqualTo(expected); } + @Test + public void testSimpleMinNested() { + ObjectNode doc = objectFromJson("{ \"subdoc\":{\"x\": \"abc\", \"y\":\"def\"}}"); + // 3 updates: 2 for existing, 1 for non-existing + UpdateOperation oper = + UpdateOperator.MIN.resolveOperation( + objectFromJson( + "{ \"subdoc.x\": \"afx\", \"subdoc.y\":\"\", \"subdoc.z\":\"value\"}")); + assertThat(oper.updateDocument(doc, targetLocator)).isTrue(); + ObjectNode expected = + objectFromJson("{\"subdoc\":{\"x\": \"abc\", \"y\":\"\", \"z\":\"value\"}}"); + assertThat(doc).isEqualTo(expected); + } + + @Test + public void testMinNoChanges() { + ObjectNode orig = objectFromJson("{ \"a\":1, \"b\":true}"); + ObjectNode doc = orig.deepCopy(); + UpdateOperation oper = + UpdateOperator.MIN.resolveOperation(objectFromJson("{\"a\":2, \"b\":true }")); + assertThat(oper.updateDocument(doc, targetLocator)).isFalse(); + assertThat(doc).isEqualTo(orig); + } + + @Test + public void testMinMixedTypes() { + ObjectNode doc = objectFromJson("{ \"a\":1, \"b\":true}"); + UpdateOperation oper = + UpdateOperator.MIN.resolveOperation(objectFromJson("{\"a\":\"value\", \"b\":123 }")); + assertThat(oper.updateDocument(doc, targetLocator)).isTrue(); + ObjectNode expected = objectFromJson("{ \"a\":1, \"b\":123}"); + assertThat(doc).isEqualTo(expected); + } + + @Test + public void testMinWithArray() { + ObjectNode doc = objectFromJson("{ \"a\":[1, true]}"); + UpdateOperation oper = + UpdateOperator.MIN.resolveOperation(objectFromJson("{\"a\":[1, false] }")); + assertThat(oper.updateDocument(doc, targetLocator)).isTrue(); + ObjectNode expected = objectFromJson("{\"a\":[1, false] }"); + assertThat(doc).isEqualTo(expected); + } + } + + @Nested + class HappyPathMax { @Test public void testSimpleMaxRoot() { ObjectNode doc = objectFromJson("{ \"x\": 1, \"y\":2}"); @@ -40,5 +84,49 @@ public void testSimpleMaxRoot() { ObjectNode expected = objectFromJson("{ \"x\": 1, \"y\":99, \"z\":0}"); assertThat(doc).isEqualTo(expected); } + + @Test + public void testSimpleMaxNested() { + ObjectNode doc = objectFromJson("{ \"subdoc\":{\"x\": \"abc\", \"y\":\"def\"}}"); + // 3 updates: 2 for existing, 1 for non-existing + UpdateOperation oper = + UpdateOperator.MAX.resolveOperation( + objectFromJson( + "{ \"subdoc.x\": \"afx\", \"subdoc.y\":\"\", \"subdoc.z\":\"value\"}")); + assertThat(oper.updateDocument(doc, targetLocator)).isTrue(); + ObjectNode expected = + objectFromJson("{\"subdoc\":{\"x\": \"afx\", \"y\":\"def\", \"z\":\"value\"}}"); + assertThat(doc).isEqualTo(expected); + } + + @Test + public void testMaxNoChanges() { + ObjectNode orig = objectFromJson("{ \"a\":1, \"b\":true}"); + ObjectNode doc = orig.deepCopy(); + UpdateOperation oper = + UpdateOperator.MAX.resolveOperation(objectFromJson("{\"a\":0, \"b\":true }")); + assertThat(oper.updateDocument(doc, targetLocator)).isFalse(); + assertThat(doc).isEqualTo(orig); + } + + @Test + public void testMaxMixedTypes() { + ObjectNode doc = objectFromJson("{ \"a\":1, \"b\":true}"); + UpdateOperation oper = + UpdateOperator.MAX.resolveOperation(objectFromJson("{\"a\":\"value\", \"b\":123 }")); + assertThat(oper.updateDocument(doc, targetLocator)).isTrue(); + ObjectNode expected = objectFromJson("{ \"a\":\"value\", \"b\":true}"); + assertThat(doc).isEqualTo(expected); + } + + @Test + public void testMaxWithArray() { + ObjectNode doc = objectFromJson("{ \"arr\":[1, 2]}"); + UpdateOperation oper = + UpdateOperator.MAX.resolveOperation(objectFromJson("{\"arr\":[1, 2, 3] }")); + assertThat(oper.updateDocument(doc, targetLocator)).isTrue(); + ObjectNode expected = objectFromJson("{\"arr\":[1, 2, 3] }"); + assertThat(doc).isEqualTo(expected); + } } } From 6d3fc429d5d3e321794aa8c62f6e36d273b23787 Mon Sep 17 00:00:00 2001 From: Tatu Saloranta Date: Wed, 8 Mar 2023 17:49:22 -0800 Subject: [PATCH 5/5] Javadocs additions --- .../sgv2/jsonapi/util/JsonNodeComparator.java | 23 +++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/src/main/java/io/stargate/sgv2/jsonapi/util/JsonNodeComparator.java b/src/main/java/io/stargate/sgv2/jsonapi/util/JsonNodeComparator.java index 0ee9ec4dbf..0ddf734699 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/util/JsonNodeComparator.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/util/JsonNodeComparator.java @@ -9,6 +9,29 @@ import java.util.Iterator; import java.util.Map; +/** + * {@link Comparator} for sorting {@link JsonNode} values as needed for operations like {@code $min} + * and {@code $max}. Uses definitions of BSON type-based sorting, where order of types is (from + * lowest to highest precedence): + * + *
    + *
  1. Null + *
  2. Number + *
  3. String + *
  4. Object + *
  5. Array + *
  6. Boolean + *
+ * + * (NOTE: these are types we have -- MongoDB has more native types so this is a subset of BSON + * sorting definitions). + * + *

Within each type sorting is as usual for most types (Numbers, Strings, Booleans). Arrays use + * straight-forward element-by-element sorting (similar to Strings). The only more esoteric case are + * Objects, where sorting is by ordered fields, first comparing field name (String sort), if same, + * then recursively by value; and if first N fields the same, Object with more properties is sorted + * last. + */ public class JsonNodeComparator implements Comparator { private static final Comparator ASC = new JsonNodeComparator();