Skip to content

Commit

Permalink
Merge pull request lukas-krecan#92 from lukas-krecan/arrays
Browse files Browse the repository at this point in the history 
Better array comparison
  • Loading branch information
@lukas-krecan
lukas-krecan authored Dec 10, 2017
2 parents 2d6df00 + 575da45 commit c72f44e
Show file tree
Hide file tree
Showing 5 changed files with 226 additions and 48 deletions.
123 changes: 90 additions & 33 deletions json-unit-core/src/main/java/net/javacrumbs/jsonunit/core/internal/Diff.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@
import static net.javacrumbs.jsonunit.core.Option.IGNORING_EXTRA_FIELDS;
import static net.javacrumbs.jsonunit.core.Option.IGNORING_VALUES;
import static net.javacrumbs.jsonunit.core.internal.ClassUtils.isClassPresent;
import static net.javacrumbs.jsonunit.core.internal.JsonUnitLogger.NULL_LOGGER;
import static net.javacrumbs.jsonunit.core.internal.JsonUtils.convertToJson;
import static net.javacrumbs.jsonunit.core.internal.JsonUtils.getNode;
import static net.javacrumbs.jsonunit.core.internal.JsonUtils.quoteIfNeeded;
Expand All @@ -57,26 +58,31 @@
public class Diff {
private static final String REGEX_PLACEHOLDER = "${json-unit.regex}";
private static final Pattern MATCHER_PLACEHOLDER_PATTERN = Pattern.compile("\\$\\{json-unit.matches:(.+)\\}(.*)");

private static final JsonUnitLogger DEFAULT_DIFF_LOGGER = createLogger("net.javacrumbs.jsonunit.difference.diff");
private static final JsonUnitLogger DEFAULT_VALUE_LOGGER = createLogger("net.javacrumbs.jsonunit.difference.values");

private final Node expectedRoot;
private final Node actualRoot;
private final Differences differences = new Differences();
private final String startPath;
private boolean compared = false;
private final Configuration configuration;

private final JsonUnitLogger diffLogger;
private final JsonUnitLogger valuesLogger;

private static final JsonUnitLogger diffLogger = createLogger("net.javacrumbs.jsonunit.difference.diff");
private static final JsonUnitLogger valuesLogger = createLogger("net.javacrumbs.jsonunit.difference.values");

private Diff(Node expected, Node actual, String startPath, Configuration configuration) {
private Diff(Node expected, Node actual, String startPath, Configuration configuration, JsonUnitLogger diffLogger, JsonUnitLogger valuesLogger) {
this.expectedRoot = expected;
this.actualRoot = actual;
this.startPath = startPath;
this.configuration = configuration;
this.diffLogger = diffLogger;
this.valuesLogger = valuesLogger;
}

public static Diff create(Object expected, Object actual, String actualName, String startPath, Configuration configuration) {
return new Diff(convertToJson(quoteIfNeeded(expected), "expected", true), convertToJson(actual, actualName, false), startPath, configuration);
return new Diff(convertToJson(quoteIfNeeded(expected), "expected", true), convertToJson(actual, actualName, false), startPath, configuration, DEFAULT_DIFF_LOGGER, DEFAULT_VALUE_LOGGER);
}

private void compare() {
Expand Down Expand Up @@ -381,34 +387,24 @@ private void compareArrayNodes(Node expectedNode, Node actualNode, String path)
structureDifferenceFound("Array \"%s\" has invalid length, expected: <at least %d> but was: <%d>.", path, expectedElements.size(), actualElements.size());
}
}
List<Node> extraValues = new ArrayList<Node>();
List<Node> missingValues = new ArrayList<Node>(expectedElements);

if (hasOption(IGNORING_ARRAY_ORDER)) {
for (Node actual : actualElements) {
int index = indexOf(missingValues, actual);
if (index != -1) {
missingValues.remove(index);
} else {
extraValues.add(actual);
}
}
ArrayComparison arrayComparison = compareArraysIgnoringOrder(expectedElements, actualElements);
List<Node> missingValues = arrayComparison.missingValues;
List<Node> extraValues = arrayComparison.extraValues;
if (expectedElements.size() == actualElements.size() && missingValues.size() == 1 && extraValues.size() == 1) {
// handling special case where only one difference is found.
Node missing = missingValues.get(0);
Node extra = extraValues.get(0);
int missingIndex = indexOf(expectedElements, missing);
int extraIndex = indexOf(actualElements, extra);

valueDifferenceFound("Different value found when comparing expected array element %s to actual element %s.", getArrayPath(path, missingIndex), getArrayPath(path, extraIndex));
compareNodes(missing, extra, getArrayPath(path, extraIndex));
} else if (failOnExtraArrayItems()) {
if (!missingValues.isEmpty() || !extraValues.isEmpty()) {
valueDifferenceFound("Array \"%s\" has different content. Missing values %s, extra values %s", path, missingValues, extraValues);
}
} else {
if (!missingValues.isEmpty()) {
valueDifferenceFound("Array \"%s\" has different content. Missing values %s", path, missingValues);
}
List<Integer> missingIndex = indexOf(expectedElements, missing);
List<Integer> extraIndex = indexOf(actualElements, extra);

valueDifferenceFound("Different value found when comparing expected array element %s to actual element %s.", getArrayPath(path, missingIndex.get(0)), getArrayPath(path, extraIndex.get(0)));
compareNodes(missing, extra, getArrayPath(path, extraIndex.get(0)));
} else if (failOnExtraArrayItems() && (!missingValues.isEmpty() || !extraValues.isEmpty())) {
valueDifferenceFound("Array \"%s\" has different content, expected: <%s> but was: <%s>. Missing values %s, extra values %s", path, expectedNode, actualNode, missingValues, extraValues);
} else if (!missingValues.isEmpty()) {
valueDifferenceFound("Array \"%s\" has different content, expected: <%s> but was: <%s>. Missing values %s", path, expectedNode, actualNode, missingValues);
}
} else {
for (int i = 0; i < Math.min(expectedElements.size(), actualElements.size()); i++) {
Expand All @@ -417,6 +413,66 @@ private void compareArrayNodes(Node expectedNode, Node actualNode, String path)
}
}

private ArrayComparison compareArraysIgnoringOrder(List<Node> expectedElements, List<Node> actualElements) {
return new ArrayComparison(expectedElements, actualElements).compareArraysIgnoringOrder();
}

private class ArrayComparison {
private final int compareFrom;
private final List<Node> actualElements;
private final List<Node> extraValues;
private final List<Node> missingValues;

private ArrayComparison(int compareFrom, List<Node> actualElements, List<Node> extraValues, List<Node> missingValues) {
this.compareFrom = compareFrom;
this.actualElements = actualElements;
this.extraValues = extraValues;
this.missingValues = missingValues;
}

ArrayComparison(List<Node> expectedElements, List<Node> actualElements) {
this(0, actualElements, new ArrayList<Node>(), new ArrayList<Node>(expectedElements));
}

ArrayComparison copy(int compareFrom) {
return new ArrayComparison(compareFrom, actualElements, new ArrayList<Node>(extraValues), new ArrayList<Node>(missingValues));
}

private ArrayComparison compareArraysIgnoringOrder() {
for (int i = compareFrom; i < actualElements.size(); i++) {
Node actual = actualElements.get(i);
List<Integer> matches = indexOf(missingValues, actual);
if (matches.size() == 1) {
removeMissing(matches.get(0));
} else if (matches.size() > 0) {
// we have more matches, since comparison does not have to be transitive ([1, 2] == [2] == [2, 3]), we have to check all the possibilities
for (int match : matches) {
ArrayComparison copy = copy(i + 1);
copy.removeMissing(match);
copy.compareArraysIgnoringOrder();
if (copy.isMatching()) {
return copy;
}
}
// no combination matching, let's report the first difference
removeMissing(matches.get(0));
} else {
extraValues.add(actual);
}
}
return this;
}

private boolean isMatching() {
return missingValues.isEmpty() && (extraValues.isEmpty() || !failOnExtraArrayItems());
}

private void removeMissing(int index) {
missingValues.remove(index);
}

}

private boolean failOnExtraArrayItems() {
return !hasOption(IGNORING_EXTRA_ARRAY_ITEMS);
}
Expand All @@ -428,16 +484,17 @@ private boolean failOnExtraArrayItems() {
* @param actual
* @return
*/
private int indexOf(List<Node> expectedElements, Node actual) {
private List<Integer> indexOf(List<Node> expectedElements, Node actual) {
List<Integer> result = new ArrayList<Integer>();
int i = 0;
for (Node expected : expectedElements) {
Diff diff = new Diff(expected, actual, "", configuration);
Diff diff = new Diff(expected, actual, "", configuration, NULL_LOGGER, NULL_LOGGER);
if (diff.similar()) {
return i;
result.add(i);
}
i++;
}
return -1;
return result;
}


Expand Down Expand Up @@ -559,7 +616,7 @@ private static JsonUnitLogger createLogger(String name) {
if (isClassPresent("org.slf4j.Logger")) {
return new JsonUnitLogger.SLF4JLogger(name);
} else {
return new JsonUnitLogger.NullLogger();
return NULL_LOGGER;
}
}
}
5 changes: 5 additions & 0 deletions json-unit-core/src/main/java/net/javacrumbs/jsonunit/core/internal/JsonOrgNodeFactory.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -205,6 +205,11 @@ public void remove() {
public int size() {
return value.length();
}

@Override
public String toString() {
return value.toString();
}
}

private static final class JSONObjectNode extends NodeSkeleton {
Expand Down
20 changes: 10 additions & 10 deletions json-unit-core/src/main/java/net/javacrumbs/jsonunit/core/internal/JsonUnitLogger.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,11 +22,20 @@
* There are some people that do not like dependency on SLF4J. Let's not force them to use it.
*/
interface JsonUnitLogger {

boolean isEnabled();

void log(String message, Object... params);

JsonUnitLogger NULL_LOGGER = new JsonUnitLogger() {
@Override
public boolean isEnabled() {
return false;
}

@Override
public void log(String message, Object... params) {}
};

final class SLF4JLogger implements JsonUnitLogger {
private final Logger logger;

Expand All @@ -42,13 +51,4 @@ public void log(String message, Object... params) {
logger.debug(message, params);
}
}

final class NullLogger implements JsonUnitLogger {
public boolean isEnabled() {
return false;
}

public void log(String message, Object... params) {}
}

}
60 changes: 55 additions & 5 deletions tests/test-base/src/main/java/net/javacrumbs/jsonunit/test/base/AbstractJsonAssertTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -698,6 +698,55 @@ public void shouldFailIfArrayContentIsDifferent() {
}
}

@Test
public void arraysShouldMatchEvenWhenIgnoringExtraFields() {
assertJsonEquals("[[2],[1]]", "[[1,2],[2]]", when(IGNORING_ARRAY_ORDER, IGNORING_EXTRA_ARRAY_ITEMS));
}

@Test
public void arraysShouldMatchEvenWhenIgnoringExtraFieldsComplex() {
assertJsonEquals("[[3],[2],[1]]", "[[1,2],[2,3],[3,1]]", when(IGNORING_ARRAY_ORDER, IGNORING_EXTRA_ARRAY_ITEMS));
}

@Test
public void arraysShouldMatchEvenWhenIgnoringExtraFieldsInEmbeddedObjects() {
assertJsonEquals("[{\"a\":[\"b\"]},{\"a\":[\"a\"]}]", "[{\"a\":[\"b\",\"a\"]},{\"a\":[\"b\",\"c\"]}]", when(IGNORING_ARRAY_ORDER, IGNORING_EXTRA_ARRAY_ITEMS));
}

@Test
public void arraysMatchShouldReportErrorCorrectlyWhenIgnoringExtraFields() {
try {
assertJsonEquals("[[2],[1]]", "[[1,2],[3]]", when(IGNORING_ARRAY_ORDER, IGNORING_EXTRA_ARRAY_ITEMS));
failIfNoException();
} catch (AssertionError e) {
assertEquals("JSON documents are different:\n" +
"Different value found when comparing expected array element [1] to actual element [1].\n" +
"Different value found when comparing expected array element [1][0] to actual element [1][0].\n" +
"Different value found in node \"[1][0]\", expected: <1> but was: <3>.\n", e.getMessage());
}
}

@Test
public void arraysMatchShouldReportErrorCorrectlyWhenIgnoringExtraFieldsComplex() {
try {
assertJsonEquals("[[3],[2],[1]]", "[[1,2],[2,3],[2,4]]", when(IGNORING_ARRAY_ORDER, IGNORING_EXTRA_ARRAY_ITEMS));
} catch (AssertionError e) {
assertEquals("JSON documents are different:\n" +
"Different value found when comparing expected array element [2] to actual element [2].\n" +
"Array \"[2]\" has different content, expected: <[1]> but was: <[2,4]>. Missing values [1]\n", e.getMessage());
}
}

@Test
public void arraysMatchShouldReportErrorCorrectlyWhenIgnoringExtraFieldsInEmbeddedObjects() {
try {
assertJsonEquals("[{\"a\":[\"b\"]},{\"a\":[\"a\"]}]", "[{\"a\":[\"b\",\"a\"]},{\"a\":[\"d\",\"c\"]}]", when(IGNORING_ARRAY_ORDER, IGNORING_EXTRA_ARRAY_ITEMS));
} catch (AssertionError e) {
assertEquals("JSON documents are different:\n" +
"Different value found when comparing expected array element [1] to actual element [1].\n" +
"Array \"[1].a\" has different content, expected: <[\"a\"]> but was: <[\"d\",\"c\"]>. Missing values [\"a\"]\n", e.getMessage());
}
}

@Test
public void shouldFailIfArrayContentIsDifferentMoreThaOneDifference() {
Expand All @@ -707,7 +756,7 @@ public void shouldFailIfArrayContentIsDifferentMoreThaOneDifference() {
failIfNoException();
} catch (AssertionError e) {
assertEquals("JSON documents are different:\n" +
"Array \"test\" has different content. Missing values [1, 2], extra values [4, 5]\n", e.getMessage());
"Array \"test\" has different content, expected: <[1,2,3]> but was: <[3,4,5]>. Missing values [1, 2], extra values [4, 5]\n", e.getMessage());
}
}

Expand All @@ -718,7 +767,8 @@ public void shouldFailIfArrayContentIsDifferentOnObjectArrays() {
assertJsonEquals("{\"test\":[{\"key\":1},{\"key\":2},{\"key\":3}]}", "{\"test\":[{\"key\":3},{\"key\":2},{\"key\":4}]}");
failIfNoException();
} catch (AssertionError e) {
assertEquals("JSON documents are different:\nDifferent value found when comparing expected array element test[0] to actual element test[2].\n" +
assertEquals("JSON documents are different:\n" +
"Different value found when comparing expected array element test[0] to actual element test[2].\n" +
"Different value found in node \"test[2].key\", expected: <1> but was: <4>.\n", e.getMessage());
}
}
Expand Down Expand Up @@ -891,7 +941,7 @@ public void shouldFailIfOneValueIsMissing() {
failIfNoException();
} catch (AssertionError e) {
assertEquals("JSON documents are different:\n" +
"Array \"test\" has different content. Missing values [9]\n", e.getMessage());
"Array \"test\" has different content, expected: <[1,2,3,9]> but was: <[5,5,4,4,3,3,2,2,1,1]>. Missing values [9]\n", e.getMessage());
}
}

Expand All @@ -903,7 +953,7 @@ public void shouldFailIfIfExpectedIsLongerAndOrderIsIgnored() {
} catch (AssertionError e) {
assertEquals("JSON documents are different:\n" +
"Array \"test\" has invalid length, expected: <at least 4> but was: <3>.\n" +
"Array \"test\" has different content. Missing values [9]\n", e.getMessage());
"Array \"test\" has different content, expected: <[1,2,3,9]> but was: <[3,2,1]>. Missing values [9]\n", e.getMessage());
}
}

Expand All @@ -915,7 +965,7 @@ public void shouldFailIfIfExpectedIsLongerAndOrderIsIgnoredAndTheItemsDoNotMatch
} catch (AssertionError e) {
assertEquals("JSON documents are different:\n" +
"Array \"test\" has invalid length, expected: <at least 4> but was: <3>.\n" +
"Array \"test\" has different content. Missing values [5, 6, 7, 8]\n", e.getMessage());
"Array \"test\" has different content, expected: <[5,6,7,8]> but was: <[3,2,1]>. Missing values [5, 6, 7, 8]\n", e.getMessage());
}
}

Expand Down
Loading

0 comments on commit c72f44e

Please sign in to comment.