Skip to content

Commit

Permalink
Fix output fields order inconsistent with the input (#81)
Browse files Browse the repository at this point in the history
  • Loading branch information
Shawyeok committed Jul 27, 2021
1 parent 733f39c commit 0696b36
Show file tree
Hide file tree
Showing 6 changed files with 50 additions and 36 deletions.
Original file line number Diff line number Diff line change
@@ -1,10 +1,9 @@
package net.thisptr.jackson.jq.internal.misc;

import java.io.Serializable;
import java.util.Collections;
import java.util.Comparator;
import java.util.HashMap;
import java.util.List;
import java.util.Iterator;
import java.util.Map;

import com.fasterxml.jackson.databind.JsonNode;
Expand Down Expand Up @@ -92,30 +91,18 @@ public int compare(final JsonNode o1, final JsonNode o2) {
}

if (type == JsonNodeType.OBJECT) {
final List<String> names1 = Lists.newArrayList(o1.fieldNames());
final List<String> names2 = Lists.newArrayList(o2.fieldNames());

// compare by keys
Collections.sort(names1);
Collections.sort(names2);
final int s = Math.min(names1.size(), names2.size());
for (int i = 0; i < s; ++i) {
final int rr = names1.get(i).compareTo(names2.get(i));
if (rr != 0)
return rr;
if (o1.size() != o2.size()) {
return o1.size() - o2.size();
}
final int rr = Integer.compare(names1.size(), names2.size());
if (rr != 0)
return rr;

// compare by values (keys are sorted alphabetically)
for (final String name : names1) {
final int rrr = compare(o1.get(name), o2.get(name));
if (rrr != 0)
return rrr;
Iterator<Map.Entry<String, JsonNode>> it1 = o1.fields();
Iterator<Map.Entry<String, JsonNode>> it2 = o2.fields();
int rr = 0;
while (rr == 0 && it1.hasNext()) {
Map.Entry<String, JsonNode> entry1 = it1.next();
Map.Entry<String, JsonNode> entry2 = it2.next();
rr = entry1.getKey().compareTo(entry2.getKey()) | compare(entry1.getValue(), entry2.getValue());
}

return 0;
return rr;
}

throw new IllegalArgumentException("Unknown JsonNodeType: " + type);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
package net.thisptr.jackson.jq.internal.tree;

import java.util.ArrayList;
import java.util.HashMap;
import java.util.LinkedHashMap;
import java.util.List;
import java.util.Map;
import java.util.Map.Entry;
Expand All @@ -26,7 +26,7 @@ public void add(final FieldConstruction field) {

@Override
public void apply(final Scope scope, final JsonNode in, final Path ipath, final PathOutput output, final boolean requirePath) throws JsonQueryException {
final Map<String, JsonNode> tmp = new HashMap<>();
final Map<String, JsonNode> tmp = new LinkedHashMap<>(fields.size());
applyRecursive(scope, in, output, fields, tmp);
}

Expand Down
27 changes: 20 additions & 7 deletions jackson-jq/src/test/java/net/thisptr/jackson/jq/JsonQueryTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@
import java.util.concurrent.ConcurrentHashMap;
import java.util.stream.Stream;

import com.fasterxml.jackson.core.JsonProcessingException;
import net.thisptr.jackson.jq.internal.misc.JsonNodeComparator;
import org.junit.jupiter.api.AfterAll;
import org.junit.jupiter.api.BeforeAll;
import org.junit.jupiter.params.ParameterizedTest;
Expand Down Expand Up @@ -132,10 +134,6 @@ static Stream<String> defaultTestCases() throws Throwable {
});
}

private static List<ComparableJsonNode> wrap(final List<JsonNode> values) {
return ComparableJsonNode.wrap(values);
}

private static Map<Version, Boolean> hasJqCache = new ConcurrentHashMap<>();
private static Evaluator cachedJqEvaluator;

Expand All @@ -162,15 +160,15 @@ private void test(final TestCase tc, final Version version) throws Throwable {
if (!tc.ignoreTrueJqBehavior && hasJqCache.computeIfAbsent(version, v -> TrueJqEvaluator.hasJq(v))) {
final Result result = cachedJqEvaluator.evaluate(tc.q, tc.in, version, 2000L);
assumeThat(result.error).as("%s", command).isNull();
assumeThat(wrap(tc.out)).as("%s", command).isEqualTo(wrap(result.values));
assertEquals(tc.out, result.values, "%s", command);
}

boolean failed = false;
try {
final JsonQuery q = JsonQuery.compile(tc.q, version);
final List<JsonNode> out = new ArrayList<>();
q.apply(scope, tc.in, out::add);
assertThat(wrap(out)).as("%s", command).isEqualTo(wrap(tc.out));
assertEquals(tc.out, out, "%s", command);

// JsonQuery.compile($.toString()).toString() === $.toString()
final String s1 = q.toString();
Expand All @@ -181,7 +179,7 @@ private void test(final TestCase tc, final Version version) throws Throwable {
final JsonQuery q1 = JsonQuery.compile(s1, version);
final List<JsonNode> out1 = new ArrayList<>();
q1.apply(scope, tc.in, out1::add);
assertThat(wrap(out1)).as("bad tostring: %s", command).isEqualTo(wrap(tc.out));
assertEquals(tc.out, out1, "bad tostring: %s", command);
} catch (final Throwable e) {
failed = true;
if (!tc.failing) {
Expand All @@ -196,6 +194,21 @@ private void test(final TestCase tc, final Version version) throws Throwable {
assertThat(failed).describedAs("unexpectedly succeeded").isTrue();
}

private void assertEquals(List<JsonNode> expected, List<JsonNode> actual, String description, Object... args) {
assertThat(actual).as(description, args)
.usingComparator((o1List, o2List) -> {
int s1 = o1List != null ? o1List.size() : -1;
int s2 = o2List != null ? o2List.size() : -1;
if (s1 != s2 || s1 == -1) return s1 - s2;
int r = 0;
for (int i = 0; i < s1 && r == 0; i++) {
r = JsonNodeComparator.getInstance().compare(o1List.get(i), o2List.get(i));
}
return r;
})
.isEqualTo(expected);
}

@ParameterizedTest
@MethodSource("defaultTestCases")
public void test(final String tcText) throws Throwable {
Expand Down
16 changes: 15 additions & 1 deletion jackson-jq/src/test/resources/jq-test-extra-ok.json
Original file line number Diff line number Diff line change
Expand Up @@ -484,7 +484,7 @@
"q": ".num_entries[\"1\"] = 10",
"in": {"num_entries": {"2": 20}},
"out": [
{"num_entries": {"1": 10, "2": 20}}
{"num_entries": {"2": 20, "1": 10}}
]
},
{
Expand Down Expand Up @@ -1036,5 +1036,19 @@
[20, 30],
[20, 40]
]
},
{
"q": "{a: {depth: 1, b: {depth: 2}}}",
"in": null,
"out": [
{
"a": {
"depth": 1,
"b": {
"depth": 2
}
}
}
]
}
]
2 changes: 1 addition & 1 deletion jackson-jq/src/test/resources/tests/jq-1.5.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -844,7 +844,7 @@
- q: '.foo = .bar'
in: {"bar": 42}
out:
- {"foo": 42, "bar": 42}
- {"bar": 42, "foo": 42}
v: '[1.5, 1.5]'
- q: '.foo |= .+1'
in: {"foo": 42}
Expand Down
2 changes: 1 addition & 1 deletion jackson-jq/src/test/resources/tests/jq-1.6.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -1047,7 +1047,7 @@
- q: '.foo = .bar'
in: {"bar": 42}
out:
- {"foo": 42, "bar": 42}
- {"bar": 42, "foo": 42}
v: '[1.6, 1.6]'
- q: '.foo |= .+1'
in: {"foo": 42}
Expand Down

0 comments on commit 0696b36

Please sign in to comment.