#30 Ignore PathNotFoundException when removing fields which should be ignored, but they don't exist. Breaking diff process, when ignored fields contains invalid JsonPath or JsonPointer expression

Author: Piotr Bugara

src/main/java/com/gravity9/jsonpatch/diff/JsonDiff.java

@@ -34,6 +34,7 @@
 import com.gravity9.jsonpatch.JsonPatchMessages;
 import com.gravity9.jsonpatch.JsonPatchOperation;
 import com.gravity9.jsonpatch.RemoveOperation;
+import com.jayway.jsonpath.PathNotFoundException;
 
 import javax.annotation.ParametersAreNonnullByDefault;
 import java.io.IOException;
@@ -110,10 +111,11 @@ public static JsonPatch asJsonPatch(final JsonNode source,
 	 * @param target the expected result after applying the patch
 	 * @param fieldsToIgnore list of JsonPath or JsonPointer paths which should be ignored when generating diff. Non-existing fields are ignored.
 	 * @return the patch as a {@link JsonPatch}
-	 * @since 1.9
+	 * @throws JsonPatchException if fieldsToIgnored not in valid JsonPath or JsonPointer format
+	 * @since 2.0.0
 	 */
 	public static JsonPatch asJsonPatchIgnoringFields(final JsonNode source,
-										final JsonNode target, final List<String> fieldsToIgnore) {
+										final JsonNode target, final List<String> fieldsToIgnore) throws JsonPatchException {
 		BUNDLE.checkNotNull(source, "common.nullArgument");
 		BUNDLE.checkNotNull(target, "common.nullArgument");
 		final List<JsonPatchOperation> ignoredFieldsRemoveOperations = getJsonPatchRemoveOperationsForIgnoredFields(fieldsToIgnore);
@@ -129,23 +131,26 @@ public static JsonPatch asJsonPatchIgnoringFields(final JsonNode source,
 		return processor.getPatch();
 	}
 
-	private static JsonNode removeIgnoredFields(JsonNode node, List<JsonPatchOperation> ignoredFieldsRemoveOperations) {
+	private static JsonNode removeIgnoredFields(JsonNode node, List<JsonPatchOperation> ignoredFieldsRemoveOperations) throws JsonPatchException {
 		JsonNode nodeWithoutIgnoredFields = node;
 		for (JsonPatchOperation operation : ignoredFieldsRemoveOperations) {
 			nodeWithoutIgnoredFields = removeIgnoredFieldOrIgnore(nodeWithoutIgnoredFields, operation);
 		}
 		return nodeWithoutIgnoredFields;
 	}
 
-	private static JsonNode removeIgnoredFieldOrIgnore(JsonNode nodeWithoutIgnoredFields, JsonPatchOperation operation) {
+	private static JsonNode removeIgnoredFieldOrIgnore(JsonNode nodeWithoutIgnoredFields, JsonPatchOperation operation) throws JsonPatchException {
 		try {
 			List<JsonPatchOperation> operationsList = new ArrayList<>();
 			operationsList.add(operation);
 			return new JsonPatch(operationsList).apply(nodeWithoutIgnoredFields);
 		} catch (JsonPatchException e) {
-			// If remove for specific path failed, it means that node does not contain specific field which should be ignored.
+			// If remove for specific path throws PathNotFound, it means that node does not contain specific field which should be ignored.
 			// See more `empty patch if object does not contain ignored field` in diff.json file.
-			return nodeWithoutIgnoredFields;
+			if (e.getCause() instanceof PathNotFoundException) {
+				return nodeWithoutIgnoredFields;
+			}
+			throw e;
 		}
 	}
 
@@ -175,8 +180,10 @@ public static JsonNode asJson(final JsonNode source, final JsonNode target) {
 	 * @param target the expected result after applying the patch
 	 * @param fieldsToIgnore list of JsonPath or JsonPointer paths which should be ignored when generating diff. Non-existing fields are ignored.
 	 * @return the patch as a {@link JsonNode}
+	 * @throws JsonPatchException if fieldsToIgnored not in valid JsonPath or JsonPointer format
+	 * @since 2.0.0
 	 */
-	public static JsonNode asJsonIgnoringFields(final JsonNode source, final JsonNode target, List<String> fieldsToIgnore) {
+	public static JsonNode asJsonIgnoringFields(final JsonNode source, final JsonNode target, List<String> fieldsToIgnore) throws JsonPatchException {
 		final String s;
 		try {
 			s = MAPPER.writeValueAsString(asJsonPatchIgnoringFields(source, target, fieldsToIgnore));
@@ -229,9 +236,9 @@ private static void generateObjectDiffs(final DiffProcessor processor,
 											final JsonPointer pointer, final ObjectNode source,
 											final ObjectNode target) {
 		final Set<String> firstFields
-			= collect(source.fieldNames(), new TreeSet<String>());
+			= collect(source.fieldNames(), new TreeSet<>());
 		final Set<String> secondFields
-			= collect(target.fieldNames(), new TreeSet<String>());
+			= collect(target.fieldNames(), new TreeSet<>());
 
 		final Set<String> copy1 = new HashSet<>(firstFields);
 		copy1.removeAll(secondFields);

src/test/java/com/gravity9/jsonpatch/diff/JsonDiffTest.java

@@ -19,7 +19,9 @@
 
 package com.gravity9.jsonpatch.diff;
 
+import com.fasterxml.jackson.core.JsonProcessingException;
 import com.fasterxml.jackson.databind.JsonNode;
+import com.fasterxml.jackson.databind.ObjectMapper;
 import com.github.fge.jackson.JsonLoader;
 import com.github.fge.jackson.JsonNumEquals;
 import com.google.common.collect.Lists;
@@ -33,6 +35,7 @@
 import org.testng.annotations.Test;
 
 import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
 
 public final class JsonDiffTest {
 
@@ -122,7 +125,7 @@ public Iterator<Object[]> getDiffsWithIgnoredFields() {
 	)
 	public void generatedPatchesIgnoreFields(final String message,
 											 final JsonNode first, final JsonNode second, final JsonNode expected,
-											 final JsonNode ignoreFields) {
+											 final JsonNode ignoreFields) throws JsonPatchException {
 
 		final List<String> ignoreFieldsList = new ArrayList<>();
 		final Iterator<JsonNode> ignoreFieldsIterator = ignoreFields.elements();
@@ -137,4 +140,36 @@ public void generatedPatchesIgnoreFields(final String message,
 						+ "expected: %s\nactual: %s\n", message, expected, actual
 		).isTrue();
 	}
+
+	@DataProvider
+	public Iterator<Object[]> getInvalidIgnoreFieldsExpressions() {
+		final List<Object[]> list = Lists.newArrayList();
+		list.add(new Object[]{
+				"$.a[(@.length-1)]", "Could not parse token starting at position 3. Expected ?, ', 0-9, * "
+		});
+		list.add(new Object[]{
+				"/a/?", "Invalid path, `?` are not allowed in JsonPointer expressions."
+		});
+		return list.iterator();
+	}
+
+	@Test(
+			dataProvider = "getInvalidIgnoreFieldsExpressions"
+	)
+	public void shouldNotPerformDiffWhenIgnoreFieldsContainsInvalidExpression(String ignoreFieldsExpression, String expectedExceptionMessage) throws JsonProcessingException {
+		// given
+		JsonNode source = new ObjectMapper().readTree("{\"a\": \"1\"}");
+		JsonNode target = new ObjectMapper().readTree("{\"a\": \"1\"}");
+		List<String> ignoreFields = new ArrayList<>();
+		ignoreFields.add(ignoreFieldsExpression);
+
+		// when
+		assertThatThrownBy(() -> JsonDiff.asJsonIgnoringFields(source, target, ignoreFields))
+				.isExactlyInstanceOf(JsonPatchException.class)
+				.hasMessageStartingWith(expectedExceptionMessage);
+
+		assertThatThrownBy(() -> JsonDiff.asJsonPatchIgnoringFields(source, target, ignoreFields))
+				.isExactlyInstanceOf(JsonPatchException.class)
+				.hasMessageStartingWith(expectedExceptionMessage);
+	}
 }