Improve the consistency of the various JsonParser implementations

- Consistent error handling applied to BasicJsonParser,
  GsonJsonParser, JsonSimpleJsonParser and YamlJsonParser
- Add tests in AbstractJsonParserTests to verify consistency
- Rename tests for JsonSimpleJsonParser to match the name of the
  class under test.

Closes gh-4690

spring-boot/src/main/java/org/springframework/boot/json/BasicJsonParser.java

@@ -17,7 +17,6 @@
 package org.springframework.boot.json;
 
 import java.util.ArrayList;
-import java.util.HashMap;
 import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Map;
@@ -31,6 +30,7 @@ import org.springframework.util.StringUtils;
  * YAML are supported).
  *
  * @author Dave Syer
+ * @author Jean de Klerk
  * @since 1.2.0
  * @see JsonParserFactory
  */
@@ -43,11 +43,8 @@ public class BasicJsonParser implements JsonParser {
 			if (json.startsWith("{")) {
 				return parseMapInternal(json);
 			}
-			else if (json.equals("")) {
-				return new HashMap<String, Object>();
-			}
 		}
-		return null;
+		throw new IllegalArgumentException("Cannot parse JSON");
 	}
 
 	@Override
@@ -57,11 +54,8 @@ public class BasicJsonParser implements JsonParser {
 			if (json.startsWith("[")) {
 				return parseListInternal(json);
 			}
-			else if (json.trim().equals("")) {
-				return new ArrayList<Object>();
-			}
 		}
-		return null;
+		throw new IllegalArgumentException("Cannot parse JSON");
 	}
 
 	private List<Object> parseListInternal(String json) {

spring-boot/src/main/java/org/springframework/boot/json/GsonJsonParser.java

@@ -26,6 +26,7 @@ import com.google.gson.GsonBuilder;
  * Thin wrapper to adapt {@link Gson} to a {@link JsonParser}.
  *
  * @author Dave Syer
+ * @author Jean de Klerk
  * @since 1.2.0
  * @see JsonParserFactory
  */
@@ -34,17 +35,27 @@ public class GsonJsonParser implements JsonParser {
 	private Gson gson = new GsonBuilder().create();
 
 	@Override
+	@SuppressWarnings("unchecked")
 	public Map<String, Object> parseMap(String json) {
-		@SuppressWarnings("unchecked")
+		if (json != null) {
-		Map<String, Object> value = this.gson.fromJson(json, Map.class);
+			json = json.trim();
-		return value;
+			if (json.startsWith("{")) {
+				return this.gson.fromJson(json, Map.class);
+			}
+		}
+		throw new IllegalArgumentException("Cannot parse JSON");
 	}
 
 	@Override
+	@SuppressWarnings("unchecked")
 	public List<Object> parseList(String json) {
-		@SuppressWarnings("unchecked")
+		if (json != null) {
-		List<Object> value = this.gson.fromJson(json, List.class);
+			json = json.trim();
-		return value;
+			if (json.startsWith("[")) {
+				return this.gson.fromJson(json, List.class);
+			}
+		}
+		throw new IllegalArgumentException("Cannot parse JSON");
 	}
 
 }

spring-boot/src/main/java/org/springframework/boot/json/JsonSimpleJsonParser.java

@@ -16,8 +16,6 @@
 
 package org.springframework.boot.json;
 
-import java.util.ArrayList;
-import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Map;
 
@@ -28,38 +26,32 @@ import org.json.simple.parser.ParseException;
  * Thin wrapper to adapt {@link org.json.simple.JSONObject} to a {@link JsonParser}.
  *
  * @author Dave Syer
+ * @author Jean de Klerk
  * @since 1.2.0
  * @see JsonParserFactory
  */
 public class JsonSimpleJsonParser implements JsonParser {
 
 	@Override
+	@SuppressWarnings("unchecked")
 	public Map<String, Object> parseMap(String json) {
-		Map<String, Object> map = new LinkedHashMap<String, Object>();
 		try {
-			@SuppressWarnings("unchecked")
+			return (Map<String, Object>) new JSONParser().parse(json);
-			Map<String, Object> value = (Map<String, Object>) new JSONParser()
-					.parse(json);
-			map.putAll(value);
 		}
 		catch (ParseException ex) {
 			throw new IllegalArgumentException("Cannot parse JSON", ex);
 		}
-		return map;
 	}
 
 	@Override
+	@SuppressWarnings("unchecked")
 	public List<Object> parseList(String json) {
-		List<Object> nested = new ArrayList<Object>();
 		try {
-			@SuppressWarnings("unchecked")
+			return (List<Object>) new JSONParser().parse(json);
-			List<Object> value = (List<Object>) new JSONParser().parse(json);
-			nested.addAll(value);
 		}
 		catch (ParseException ex) {
 			throw new IllegalArgumentException("Cannot parse JSON", ex);
 		}
-		return nested;
 	}
 
 }

spring-boot/src/main/java/org/springframework/boot/json/YamlJsonParser.java

@@ -25,6 +25,7 @@ import org.yaml.snakeyaml.Yaml;
  * Thin wrapper to adapt Snake {@link Yaml} to {@link JsonParser}.
  *
  * @author Dave Syer
+ * @author Jean de Klerk
  * @see JsonParserFactory
  */
 public class YamlJsonParser implements JsonParser {
@@ -32,13 +33,25 @@ public class YamlJsonParser implements JsonParser {
 	@Override
 	@SuppressWarnings("unchecked")
 	public Map<String, Object> parseMap(String json) {
-		return new Yaml().loadAs(json, Map.class);
+		if (json != null) {
+			json = json.trim();
+			if (json.startsWith("{")) {
+				return new Yaml().loadAs(json, Map.class);
+			}
+		}
+		throw new IllegalArgumentException("Cannot parse JSON");
 	}
 
 	@Override
 	@SuppressWarnings("unchecked")
 	public List<Object> parseList(String json) {
-		return new Yaml().loadAs(json, List.class);
+		if (json != null) {
+			json = json.trim();
+			if (json.startsWith("[")) {
+				return new Yaml().loadAs(json, List.class);
+			}
+		}
+		throw new IllegalArgumentException("Cannot parse JSON");
 	}
 
 }

spring-boot/src/test/java/org/springframework/boot/json/AbstractJsonParserTests.java

@@ -19,7 +19,9 @@ package org.springframework.boot.json;
 import java.util.List;
 import java.util.Map;
 
+import org.junit.Rule;
 import org.junit.Test;
+import org.junit.rules.ExpectedException;
 
 import static org.junit.Assert.assertEquals;
 
@@ -27,15 +29,19 @@ import static org.junit.Assert.assertEquals;
  * Base for {@link JsonParser} tests.
  *
  * @author Dave Syer
+ * @author Jean de Klerk
  */
 public abstract class AbstractJsonParserTests {
 
+	@Rule
+	public ExpectedException thrown = ExpectedException.none();
+
 	private final JsonParser parser = getParser();
 
 	protected abstract JsonParser getParser();
 
 	@Test
-	public void testSimpleMap() {
+	public void simpleMap() {
 		Map<String, Object> map = this.parser.parseMap("{\"foo\":\"bar\",\"spam\":1}");
 		assertEquals(2, map.size());
 		assertEquals("bar", map.get("foo"));
@@ -43,7 +49,7 @@ public abstract class AbstractJsonParserTests {
 	}
 
 	@Test
-	public void testDoubleValue() {
+	public void doubleValue() {
 		Map<String, Object> map = this.parser.parseMap("{\"foo\":\"bar\",\"spam\":1.23}");
 		assertEquals(2, map.size());
 		assertEquals("bar", map.get("foo"));
@@ -51,27 +57,27 @@ public abstract class AbstractJsonParserTests {
 	}
 
 	@Test
-	public void testEmptyMap() {
+	public void emptyMap() {
 		Map<String, Object> map = this.parser.parseMap("{}");
 		assertEquals(0, map.size());
 	}
 
 	@Test
-	public void testSimpleList() {
+	public void simpleList() {
 		List<Object> list = this.parser.parseList("[\"foo\",\"bar\",1]");
 		assertEquals(3, list.size());
 		assertEquals("bar", list.get(1));
 	}
 
 	@Test
-	public void testEmptyList() {
+	public void emptyList() {
 		List<Object> list = this.parser.parseList("[]");
 		assertEquals(0, list.size());
 	}
 
 	@SuppressWarnings("unchecked")
 	@Test
-	public void testListOfMaps() {
+	public void listOfMaps() {
 		List<Object> list = this.parser
 				.parseList("[{\"foo\":\"bar\",\"spam\":1},{\"foo\":\"baz\",\"spam\":2}]");
 		assertEquals(2, list.size());
@@ -80,11 +86,73 @@ public abstract class AbstractJsonParserTests {
 
 	@SuppressWarnings("unchecked")
 	@Test
-	public void testMapOfLists() {
+	public void mapOfLists() {
 		Map<String, Object> map = this.parser.parseMap(
 				"{\"foo\":[{\"foo\":\"bar\",\"spam\":1},{\"foo\":\"baz\",\"spam\":2}]}");
 		assertEquals(1, map.size());
 		assertEquals(2, ((List<Object>) map.get("foo")).size());
 	}
 
+	@Test
+	public void mapWithNullThrowsARuntimeException() {
+		this.thrown.expect(RuntimeException.class);
+		this.parser.parseMap(null);
+	}
+
+	@Test
+	public void listWithNullThrowsARuntimeException() {
+		this.thrown.expect(RuntimeException.class);
+		this.parser.parseList(null);
+	}
+
+	@Test
+	public void mapWithEmptyStringThrowsARuntimeException() {
+		this.thrown.expect(RuntimeException.class);
+		this.parser.parseMap("");
+	}
+
+	@Test
+	public void listWithEmptyStringThrowsARuntimeException() {
+		this.thrown.expect(RuntimeException.class);
+		this.parser.parseList("");
+	}
+
+	@Test
+	public void mapWithListThrowsARuntimeException() {
+		this.thrown.expect(RuntimeException.class);
+		this.parser.parseMap("[]");
+	}
+
+	@Test
+	public void listWithMapThrowsARuntimeException() {
+		this.thrown.expect(RuntimeException.class);
+		this.parser.parseList("{}");
+	}
+
+	@Test
+	public void listWithLeadingWhitespace() {
+		List<Object> list = this.parser.parseList("\n\t[\"foo\"]");
+		assertEquals(1, list.size());
+		assertEquals("foo", list.get(0));
+	}
+
+	@Test
+	public void mapWithLeadingWhitespace() {
+		Map<String, Object> map = this.parser.parseMap("\n\t{\"foo\":\"bar\"}");
+		assertEquals(1, map.size());
+		assertEquals("bar", map.get("foo"));
+	}
+
+	@Test
+	public void mapWithLeadingWhitespaceListThrowsARuntimeException() {
+		this.thrown.expect(RuntimeException.class);
+		this.parser.parseMap("\n\t[]");
+	}
+
+	@Test
+	public void listWithLeadingWhitespaceMapThrowsARuntimeException() {
+		this.thrown.expect(RuntimeException.class);
+		this.parser.parseList("\n\t{}");
+	}
+
 }

spring-boot/src/test/java/org/springframework/boot/json/JsonJsonParserTests.java

@@ -17,7 +17,7 @@
 package org.springframework.boot.json;
 
 /**
- * Tests for {@link JsonSimpleJsonParser}.
+ * Tests for {@link JsonJsonParser}.
  *
  * @author Dave Syer
  */
@@ -25,7 +25,7 @@ public class JsonJsonParserTests extends AbstractJsonParserTests {
 
 	@Override
 	protected JsonParser getParser() {
-		return new JsonSimpleJsonParser();
+		return new JsonJsonParser();
 	}
 
 }

spring-boot/src/test/java/org/springframework/boot/json/JsonSimpleJsonParserTests.java

@@ -17,15 +17,15 @@
 package org.springframework.boot.json;
 
 /**
- * Tests for {@link JsonJsonParser}.
+ * Tests for {@link JsonSimpleJsonParser}.
  *
  * @author Dave Syer
  */
-public class SimpleJsonJsonParserTests extends AbstractJsonParserTests {
+public class JsonSimpleJsonParserTests extends AbstractJsonParserTests {
 
 	@Override
 	protected JsonParser getParser() {
-		return new JsonJsonParser();
+		return new JsonSimpleJsonParser();
 	}
 
 }