Merge pull request #49 from gravity9-tech/feature/48_sonar_issues

#48 Fixed issues reported by Sonar

build.gradle

@@ -29,6 +29,7 @@ buildscript {
 
 plugins {
     id("net.ltgt.errorprone") version "3.0.1" apply false
+    id "org.sonarqube" version "4.2.1.3168"
 }
 
 apply(plugin: "java");

src/main/java/com/gravity9/jsonpatch/DualPathOperation.java

@@ -20,11 +20,11 @@
 package com.gravity9.jsonpatch;
 
 import com.fasterxml.jackson.core.JsonGenerator;
-import com.fasterxml.jackson.core.JsonProcessingException;
 import com.fasterxml.jackson.databind.SerializerProvider;
 import com.fasterxml.jackson.databind.annotation.JsonSerialize;
 import com.fasterxml.jackson.databind.jsontype.TypeSerializer;
 import com.fasterxml.jackson.databind.ser.std.ToStringSerializer;
+
 import java.io.IOException;
 
 /**
@@ -48,7 +48,7 @@ protected DualPathOperation(final String op, final String from, final String pat
 	}
 
 	@Override
-	public final void serialize(final JsonGenerator jgen, final SerializerProvider provider) throws IOException, JsonProcessingException {
+	public final void serialize(final JsonGenerator jgen, final SerializerProvider provider) throws IOException {
 		jgen.writeStartObject();
 		jgen.writeStringField("op", op);
 		jgen.writeStringField("path", path);
@@ -58,7 +58,7 @@ public final void serialize(final JsonGenerator jgen, final SerializerProvider p
 
 	@Override
 	public final void serializeWithType(final JsonGenerator jgen, final SerializerProvider provider,
-										final TypeSerializer typeSer) throws IOException, JsonProcessingException {
+										final TypeSerializer typeSer) throws IOException {
 		serialize(jgen, provider);
 	}
 

src/main/java/com/gravity9/jsonpatch/JsonPathParser.java

@@ -2,6 +2,8 @@
 
 class JsonPathParser {
 
+	private JsonPathParser() {}
+
 	private static final String ARRAY_ELEMENT_REGEX = "(?<=\\.)(\\d+)";
 
 	/**

src/main/java/com/gravity9/jsonpatch/PathParser.java

@@ -7,6 +7,8 @@
 
 public class PathParser {
 
+	private PathParser() {}
+
 	private static final String FILTER_PLACEHOLDER = "[?]";
 
 	/**
@@ -55,7 +57,7 @@ private static boolean isMultiIndexNotation(String path) {
 		String pathWithoutBracket = path
 				.replace("[", "")
 				.replace("]", "");
-		return !pathWithoutBracket.startsWith("'") && !pathWithoutBracket.matches("[0-9]+");
+		return !pathWithoutBracket.startsWith("'") && !pathWithoutBracket.matches("\\d+");
 	}
 
 	private static String getNewNodeName(String[] splitJsonPath) {

src/main/java/com/gravity9/jsonpatch/PathValueOperation.java

@@ -20,11 +20,11 @@
 package com.gravity9.jsonpatch;
 
 import com.fasterxml.jackson.core.JsonGenerator;
-import com.fasterxml.jackson.core.JsonProcessingException;
 import com.fasterxml.jackson.databind.JsonNode;
 import com.fasterxml.jackson.databind.SerializerProvider;
 import com.fasterxml.jackson.databind.annotation.JsonSerialize;
 import com.fasterxml.jackson.databind.jsontype.TypeSerializer;
+
 import java.io.IOException;
 
 /**
@@ -48,17 +48,17 @@ protected PathValueOperation(final String op, final String path, final JsonNode
 	}
 
 	@Override
-	public final void serialize(final JsonGenerator jgen, final SerializerProvider provider) throws IOException, JsonProcessingException {
+	public final void serialize(final JsonGenerator jgen, final SerializerProvider provider) throws IOException {
 		jgen.writeStartObject();
 		jgen.writeStringField("op", op);
-		jgen.writeStringField("path", path.toString());
+		jgen.writeStringField("path", path);
 		jgen.writeFieldName("value");
 		jgen.writeTree(value);
 		jgen.writeEndObject();
 	}
 
 	@Override
-	public final void serializeWithType(final JsonGenerator jgen, final SerializerProvider provider, final TypeSerializer typeSer) throws IOException, JsonProcessingException {
+	public final void serializeWithType(final JsonGenerator jgen, final SerializerProvider provider, final TypeSerializer typeSer) throws IOException {
 		serialize(jgen, provider);
 	}
 

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

@@ -25,25 +25,25 @@
 import com.gravity9.jsonpatch.JsonPatchOperation;
 import com.gravity9.jsonpatch.jackson.JsonNumEquals;
 
+
+import javax.annotation.Nullable;
 import java.util.ArrayList;
 import java.util.Collections;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
-import javax.annotation.Nullable;
 
-// TODO: cleanup
 final class DiffProcessor {
 
 	private static final JsonNumEquals EQUIVALENCE
 		= JsonNumEquals.getInstance();
 
 	private final Map<JsonPointer, JsonNode> unchanged;
 
-	private final List<DiffOperation> diffs = new ArrayList<DiffOperation>();
+	private final List<DiffOperation> diffs = new ArrayList<>();
 
 	DiffProcessor(final Map<JsonPointer, JsonNode> unchanged) {
-		this.unchanged = Collections.unmodifiableMap(new HashMap<JsonPointer, JsonNode>(unchanged));
+		this.unchanged = Collections.unmodifiableMap(new HashMap<>(unchanged));
 	}
 
 	void valueReplaced(final JsonPointer pointer, final JsonNode oldValue,
@@ -73,7 +73,7 @@ void valueAdded(final JsonPointer pointer, final JsonNode value) {
 	}
 
 	JsonPatch getPatch() {
-		final List<JsonPatchOperation> list = new ArrayList<JsonPatchOperation>();
+		final List<JsonPatchOperation> list = new ArrayList<>();
 
 		for (final DiffOperation op : diffs)
 			list.add(op.asJsonPatchOperation());

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

@@ -79,6 +79,8 @@ public final class JsonDiff {
 	private static final JsonNumEquals EQUIVALENCE
 		= JsonNumEquals.getInstance();
 
+	private static final String NULL_ARGUMENT_KEY = "common.nullArgument";
+
 	private JsonDiff() {
 	}
 
@@ -93,8 +95,8 @@ private JsonDiff() {
 	 */
 	public static JsonPatch asJsonPatch(final JsonNode source,
 										final JsonNode target) {
-		BUNDLE.checkNotNull(source, "common.nullArgument");
-		BUNDLE.checkNotNull(target, "common.nullArgument");
+		BUNDLE.checkNotNull(source, NULL_ARGUMENT_KEY);
+		BUNDLE.checkNotNull(target, NULL_ARGUMENT_KEY);
 		final Map<JsonPointer, JsonNode> unchanged
 			= getUnchangedValues(source, target);
 		final DiffProcessor processor = new DiffProcessor(unchanged);
@@ -116,8 +118,8 @@ public static JsonPatch asJsonPatch(final JsonNode source,
 	 */
 	public static JsonPatch asJsonPatchIgnoringFields(final JsonNode source,
 										final JsonNode target, final List<String> fieldsToIgnore) throws JsonPatchException {
-		BUNDLE.checkNotNull(source, "common.nullArgument");
-		BUNDLE.checkNotNull(target, "common.nullArgument");
+		BUNDLE.checkNotNull(source, NULL_ARGUMENT_KEY);
+		BUNDLE.checkNotNull(target, NULL_ARGUMENT_KEY);
 		final List<JsonPatchOperation> ignoredFieldsRemoveOperations = getJsonPatchRemoveOperationsForIgnoredFields(fieldsToIgnore);
 
 		JsonNode sourceWithoutIgnoredFields = removeIgnoredFields(source, ignoredFieldsRemoveOperations);
@@ -161,14 +163,15 @@ private static JsonNode removeIgnoredFieldOrIgnore(JsonNode nodeWithoutIgnoredFi
 	 * @param source the node to be patched
 	 * @param target the expected result after applying the patch
 	 * @return the patch as a {@link JsonNode}
+	 * @throws JsonPatchException when cannot generate JSON diff
 	 */
-	public static JsonNode asJson(final JsonNode source, final JsonNode target) {
+	public static JsonNode asJson(final JsonNode source, final JsonNode target) throws JsonPatchException {
 		final String s;
 		try {
 			s = MAPPER.writeValueAsString(asJsonPatch(source, target));
 			return MAPPER.readTree(s);
 		} catch (IOException e) {
-			throw new RuntimeException("cannot generate JSON diff", e);
+			throw new JsonPatchException("cannot generate JSON diff", e);
 		}
 	}
 
@@ -189,7 +192,7 @@ public static JsonNode asJsonIgnoringFields(final JsonNode source, final JsonNod
 			s = MAPPER.writeValueAsString(asJsonPatchIgnoringFields(source, target, fieldsToIgnore));
 			return MAPPER.readTree(s);
 		} catch (IOException e) {
-			throw new RuntimeException("cannot generate JSON diff", e);
+			throw new JsonPatchException("cannot generate JSON diff", e);
 		}
 	}
 
@@ -221,7 +224,7 @@ private static void generateDiffs(final DiffProcessor processor,
 		}
 
 		/*
-		 * If we reach this point, both nodes are either objects or arrays;
+		 * If we reach this point, both nodes are either objects or arrays
 		 * delegate.
 		 */
 		if (firstType == NodeType.OBJECT)
@@ -262,12 +265,6 @@ private static void generateObjectDiffs(final DiffProcessor processor,
 	}
 
 	private static <T> Set<T> collect(Iterator<T> from, Set<T> to) {
-		if (from == null) {
-			throw new NullPointerException();
-		}
-		if (to == null) {
-			throw new NullPointerException();
-		}
 		while (from.hasNext()) {
 			to.add(from.next());
 		}

src/main/java/com/gravity9/jsonpatch/mergepatch/NonObjectMergePatch.java

@@ -32,9 +32,6 @@ final class NonObjectMergePatch extends JsonMergePatch {
 	private final JsonNode node;
 
 	NonObjectMergePatch(final JsonNode node) {
-		if (node == null) {
-			throw new NullPointerException();
-		}
 		this.node = node;
 	}
 

src/main/java/com/gravity9/jsonpatch/mergepatch/ObjectMergePatch.java

@@ -72,7 +72,7 @@ public JsonNode apply(final JsonNode input) throws JsonPatchException {
 			 * the modifiedMembers map, values are JsonMergePatch instances:
 			 *
 			 * * if it is a NonObjectMergePatch, the value is replaced
-			 *   unconditionally;
+			 *   unconditionally
 			 * * if it is an ObjectMergePatch, we get back here; the value will
 			 *   be replaced with a JSON Object anyway before being processed.
 			 */

src/test/java/com/gravity9/jsonpatch/JsonPatchTestSuite.java → src/test/java/com/gravity9/jsonpatch/JsonPatchTests.java

@@ -31,11 +31,11 @@
 import static org.testng.Assert.assertTrue;
 import static org.testng.Assert.fail;
 
-public final class JsonPatchTestSuite {
+public final class JsonPatchTests {
 
 	private final JsonNode testNode;
 
-	public JsonPatchTestSuite()
+	public JsonPatchTests()
 		throws IOException {
 		testNode = JsonLoader.fromResource("/jsonpatch/testsuite.json");
 	}

src/test/java/com/gravity9/jsonpatch/JsonPathParserTest.java

@@ -1,41 +1,26 @@
 package com.gravity9.jsonpatch;
 
+import org.testng.annotations.DataProvider;
 import org.testng.annotations.Test;
 
 import static org.testng.Assert.assertEquals;
 
 public class JsonPathParserTest {
 
-	@Test
-	public void shouldConvertPointerToJsonPath() throws JsonPatchException {
-		String jsonPointerWithQuery = "/productPrice/prodPriceAlteration";
-		String expected = "$.productPrice.prodPriceAlteration";
-		String result = JsonPathParser.parsePathToJsonPath(jsonPointerWithQuery);
-		assertEquals(result, expected);
-	}
-
-	@Test
-	public void shouldConvertPointerWithArrayToJsonPath() throws JsonPatchException {
-		String jsonPointerWithQuery = "/productPrice/1/prodPriceAlteration";
-		String expected = "$.productPrice.[1].prodPriceAlteration";
-		String result = JsonPathParser.parsePathToJsonPath(jsonPointerWithQuery);
-		assertEquals(result, expected);
+	@DataProvider
+	public static Object[][] jsonPointerToJsonPathTestCases() {
+		return new Object[][] {
+				{"/productPrice/prodPriceAlteration", "$.productPrice.prodPriceAlteration"},
+				{"/productPrice/1/prodPriceAlteration", "$.productPrice.[1].prodPriceAlteration"},
+				{"/productPrice/prodPriceAlteration/1", "$.productPrice.prodPriceAlteration.[1]"},
+				{"/2/1/-", "$.[2].[1].-"}
+		};
 	}
 
-	@Test
-	public void shouldConvertPointerWithArrayAtTheEndToJsonPath() throws JsonPatchException {
-		String jsonPointerWithQuery = "/productPrice/prodPriceAlteration/1";
-		String expected = "$.productPrice.prodPriceAlteration.[1]";
-		String result = JsonPathParser.parsePathToJsonPath(jsonPointerWithQuery);
-		assertEquals(result, expected);
-	}
-
-	@Test
-	public void shouldConvertArrayPathToJsonPath() throws JsonPatchException {
-		String jsonPointer = "/2/1/-";
-		String expected = "$.[2].[1].-";
-		String result = JsonPathParser.parsePathToJsonPath(jsonPointer);
-		assertEquals(result, expected);
+	@Test(dataProvider = "jsonPointerToJsonPathTestCases")
+	public void shouldConvertPointerToJsonPath(String jsonPointerExpression, String expectedJsonPath) throws JsonPatchException {
+		String result = JsonPathParser.parsePathToJsonPath(jsonPointerExpression);
+		assertEquals(result, expectedJsonPath);
 	}
 
 	@Test