From aa696210789b88ab30a6100ead42d77c3f2ccf4c Mon Sep 17 00:00:00 2001 From: Anton Telechev Date: Sat, 10 Mar 2018 23:20:07 +0100 Subject: [PATCH 1/2] Refactor spring-boot JSON parsers Refactor JSON parser wrappers to remove duplicate code portions in the parseMap() and parseList() methods by adding an AbstractJsonParser. See gh-12428 --- .../boot/json/AbstractJsonParser.java | 91 +++++++++++++++++++ .../boot/json/BasicJsonParser.java | 18 +--- .../boot/json/GsonJsonParser.java | 20 +--- .../boot/json/JacksonJsonParser.java | 4 +- .../boot/json/JsonSimpleJsonParser.java | 4 +- .../boot/json/YamlJsonParser.java | 18 +--- 6 files changed, 106 insertions(+), 49 deletions(-) create mode 100644 spring-boot-project/spring-boot/src/main/java/org/springframework/boot/json/AbstractJsonParser.java diff --git a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/json/AbstractJsonParser.java b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/json/AbstractJsonParser.java new file mode 100644 index 0000000000..28e44ad8df --- /dev/null +++ b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/json/AbstractJsonParser.java @@ -0,0 +1,91 @@ +/* + * Copyright 2012-2017 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.boot.json; + +import java.util.List; +import java.util.Map; +import java.util.Optional; +import java.util.function.Function; + +/** + * Base class for parsers wrapped or implemented in this package. + * + * @author Anton Telechev + */ +abstract class AbstractJsonParser implements JsonParser { + + /** Start symbol of a JSON map. **/ + private static final String START_MAP = "{"; + + /** Start symbol of a JSON list. **/ + private static final String START_LIST = "["; + + /** + * Parses the specified JSON string and returns the extracted contents as a Map of + * String to Object. + * + * @param json the JSON string to parse. + * @param parser the parser function. + * @return Map<String, Object> parsed contents + * @throws IllegalArgumentException if the json String cannot be parsed as a + * Map<String, Object> + */ + Map parseMap(String json, + Function> parser) { + assert parser != null; + + return trimIfStartsWith(json, START_MAP).map(parser::apply) + .orElseThrow(AbstractJsonParser::cannotParseJson); + } + + /** + * Parses the specified JSON string and returns the extracted contents as a List of Objects. + * + * @param json the JSON string to parse. + * @param parser the parser function. + * @return List<Object> parsed contents + * @throws IllegalArgumentException if the json String cannot be parsed as a + * List<Object> + */ + List parseList(String json, Function> parser) { + assert parser != null; + + return trimIfStartsWith(json, START_LIST).map(parser::apply) + .orElseThrow(AbstractJsonParser::cannotParseJson); + } + + private static IllegalArgumentException cannotParseJson() { + return cannotParseJson(null); + } + + static IllegalArgumentException cannotParseJson(Exception cause) { + return new IllegalArgumentException("Cannot parse JSON", cause); + } + + private static Optional trimIfStartsWith(String json, String expectedPrefix) { + assert expectedPrefix != null; + + if (json != null) { + final String trimmed = json.trim(); + if (trimmed.startsWith(expectedPrefix)) { + return Optional.of(trimmed); + } + } + return Optional.empty(); + } + +} diff --git a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/json/BasicJsonParser.java b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/json/BasicJsonParser.java index ab66f62508..b74f8a6500 100644 --- a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/json/BasicJsonParser.java +++ b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/json/BasicJsonParser.java @@ -35,28 +35,16 @@ import org.springframework.util.StringUtils; * @since 1.2.0 * @see JsonParserFactory */ -public class BasicJsonParser implements JsonParser { +public class BasicJsonParser extends AbstractJsonParser { @Override public Map parseMap(String json) { - if (json != null) { - json = json.trim(); - if (json.startsWith("{")) { - return parseMapInternal(json); - } - } - throw new IllegalArgumentException("Cannot parse JSON"); + return parseMap(json, this::parseMapInternal); } @Override public List parseList(String json) { - if (json != null) { - json = json.trim(); - if (json.startsWith("[")) { - return parseListInternal(json); - } - } - throw new IllegalArgumentException("Cannot parse JSON"); + return parseList(json, this::parseListInternal); } private List parseListInternal(String json) { diff --git a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/json/GsonJsonParser.java b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/json/GsonJsonParser.java index 127404421e..5fd6a123e5 100644 --- a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/json/GsonJsonParser.java +++ b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/json/GsonJsonParser.java @@ -31,7 +31,7 @@ import com.google.gson.reflect.TypeToken; * @since 1.2.0 * @see JsonParserFactory */ -public class GsonJsonParser implements JsonParser { +public class GsonJsonParser extends AbstractJsonParser { private static final TypeToken MAP_TYPE = new MapTypeToken(); @@ -41,24 +41,14 @@ public class GsonJsonParser implements JsonParser { @Override public Map parseMap(String json) { - if (json != null) { - json = json.trim(); - if (json.startsWith("{")) { - return this.gson.fromJson(json, MAP_TYPE.getType()); - } - } - throw new IllegalArgumentException("Cannot parse JSON"); + return parseMap(json, + (trimmed) -> this.gson.fromJson(trimmed, MAP_TYPE.getType())); } @Override public List parseList(String json) { - if (json != null) { - json = json.trim(); - if (json.startsWith("[")) { - return this.gson.fromJson(json, LIST_TYPE.getType()); - } - } - throw new IllegalArgumentException("Cannot parse JSON"); + return parseList(json, + (trimmed) -> this.gson.fromJson(trimmed, LIST_TYPE.getType())); } private static final class MapTypeToken extends TypeToken> { diff --git a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/json/JacksonJsonParser.java b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/json/JacksonJsonParser.java index 1d830c834d..e3c653b87f 100644 --- a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/json/JacksonJsonParser.java +++ b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/json/JacksonJsonParser.java @@ -42,7 +42,7 @@ public class JacksonJsonParser implements JsonParser { return getObjectMapper().readValue(json, MAP_TYPE); } catch (Exception ex) { - throw new IllegalArgumentException("Cannot parse JSON", ex); + throw AbstractJsonParser.cannotParseJson(ex); } } @@ -52,7 +52,7 @@ public class JacksonJsonParser implements JsonParser { return getObjectMapper().readValue(json, LIST_TYPE); } catch (Exception ex) { - throw new IllegalArgumentException("Cannot parse JSON", ex); + throw AbstractJsonParser.cannotParseJson(ex); } } diff --git a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/json/JsonSimpleJsonParser.java b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/json/JsonSimpleJsonParser.java index 80b9bd5447..1e9e9ce581 100644 --- a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/json/JsonSimpleJsonParser.java +++ b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/json/JsonSimpleJsonParser.java @@ -39,7 +39,7 @@ public class JsonSimpleJsonParser implements JsonParser { return (Map) new JSONParser().parse(json); } catch (ParseException ex) { - throw new IllegalArgumentException("Cannot parse JSON", ex); + throw AbstractJsonParser.cannotParseJson(ex); } } @@ -50,7 +50,7 @@ public class JsonSimpleJsonParser implements JsonParser { return (List) new JSONParser().parse(json); } catch (ParseException ex) { - throw new IllegalArgumentException("Cannot parse JSON", ex); + throw AbstractJsonParser.cannotParseJson(ex); } } diff --git a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/json/YamlJsonParser.java b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/json/YamlJsonParser.java index d2d46536ac..eb3f13b6b3 100644 --- a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/json/YamlJsonParser.java +++ b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/json/YamlJsonParser.java @@ -28,30 +28,18 @@ import org.yaml.snakeyaml.Yaml; * @author Jean de Klerk * @see JsonParserFactory */ -public class YamlJsonParser implements JsonParser { +public class YamlJsonParser extends AbstractJsonParser { @Override @SuppressWarnings("unchecked") public Map parseMap(String json) { - if (json != null) { - json = json.trim(); - if (json.startsWith("{")) { - return new Yaml().loadAs(json, Map.class); - } - } - throw new IllegalArgumentException("Cannot parse JSON"); + return parseMap(json, (trimmed) -> new Yaml().loadAs(trimmed, Map.class)); } @Override @SuppressWarnings("unchecked") public List parseList(String json) { - if (json != null) { - json = json.trim(); - if (json.startsWith("[")) { - return new Yaml().loadAs(json, List.class); - } - } - throw new IllegalArgumentException("Cannot parse JSON"); + return parseList(json, (trimmed) -> new Yaml().loadAs(trimmed, List.class)); } } From 90949669be923b3b9b7692b663b49e9dee6702ff Mon Sep 17 00:00:00 2001 From: Phillip Webb Date: Wed, 14 Mar 2018 14:50:29 -0700 Subject: [PATCH 2/2] Polish "refactor spring-boot JSON parser" Polish JSON parser refactoring and remove some more duplication. Closes gh-12428 --- .../boot/json/AbstractJsonParser.java | 83 +++++++------------ .../boot/json/GsonJsonParser.java | 2 +- .../boot/json/JacksonJsonParser.java | 20 ++--- .../boot/json/JsonParseException.java | 36 ++++++++ .../springframework/boot/json/JsonParser.java | 8 +- .../boot/json/JsonSimpleJsonParser.java | 20 ++--- .../boot/json/YamlJsonParser.java | 2 +- 7 files changed, 84 insertions(+), 87 deletions(-) create mode 100644 spring-boot-project/spring-boot/src/main/java/org/springframework/boot/json/JsonParseException.java diff --git a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/json/AbstractJsonParser.java b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/json/AbstractJsonParser.java index 28e44ad8df..c7541292c3 100644 --- a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/json/AbstractJsonParser.java +++ b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/json/AbstractJsonParser.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2017 the original author or authors. + * Copyright 2012-2018 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -18,74 +18,49 @@ package org.springframework.boot.json; import java.util.List; import java.util.Map; -import java.util.Optional; +import java.util.concurrent.Callable; import java.util.function.Function; +import org.springframework.util.ReflectionUtils; + /** * Base class for parsers wrapped or implemented in this package. * * @author Anton Telechev + * @author Phillip Webb */ abstract class AbstractJsonParser implements JsonParser { - /** Start symbol of a JSON map. **/ - private static final String START_MAP = "{"; - - /** Start symbol of a JSON list. **/ - private static final String START_LIST = "["; - - /** - * Parses the specified JSON string and returns the extracted contents as a Map of - * String to Object. - * - * @param json the JSON string to parse. - * @param parser the parser function. - * @return Map<String, Object> parsed contents - * @throws IllegalArgumentException if the json String cannot be parsed as a - * Map<String, Object> - */ - Map parseMap(String json, + protected final Map parseMap(String json, Function> parser) { - assert parser != null; - - return trimIfStartsWith(json, START_MAP).map(parser::apply) - .orElseThrow(AbstractJsonParser::cannotParseJson); + return trimParse(json, "{", parser); } - /** - * Parses the specified JSON string and returns the extracted contents as a List of Objects. - * - * @param json the JSON string to parse. - * @param parser the parser function. - * @return List<Object> parsed contents - * @throws IllegalArgumentException if the json String cannot be parsed as a - * List<Object> - */ - List parseList(String json, Function> parser) { - assert parser != null; - - return trimIfStartsWith(json, START_LIST).map(parser::apply) - .orElseThrow(AbstractJsonParser::cannotParseJson); + protected final List parseList(String json, + Function> parser) { + return trimParse(json, "[", parser); } - private static IllegalArgumentException cannotParseJson() { - return cannotParseJson(null); - } - - static IllegalArgumentException cannotParseJson(Exception cause) { - return new IllegalArgumentException("Cannot parse JSON", cause); - } - - private static Optional trimIfStartsWith(String json, String expectedPrefix) { - assert expectedPrefix != null; - - if (json != null) { - final String trimmed = json.trim(); - if (trimmed.startsWith(expectedPrefix)) { - return Optional.of(trimmed); - } + protected final T trimParse(String json, String prefix, + Function parser) { + String trimmed = (json == null ? "" : json.trim()); + if (trimmed.startsWith(prefix)) { + return parser.apply(trimmed); + } + throw new JsonParseException(); + } + + protected final T tryParse(Callable parser, Class check) { + try { + return parser.call(); + } + catch (Exception ex) { + if (check.isAssignableFrom(ex.getClass())) { + throw new JsonParseException(ex); + } + ReflectionUtils.rethrowRuntimeException(ex); + throw new IllegalStateException(ex); } - return Optional.empty(); } } diff --git a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/json/GsonJsonParser.java b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/json/GsonJsonParser.java index 5fd6a123e5..428645406b 100644 --- a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/json/GsonJsonParser.java +++ b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/json/GsonJsonParser.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2017 the original author or authors. + * Copyright 2012-2018 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. diff --git a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/json/JacksonJsonParser.java b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/json/JacksonJsonParser.java index e3c653b87f..41651fa1df 100644 --- a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/json/JacksonJsonParser.java +++ b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/json/JacksonJsonParser.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2017 the original author or authors. + * Copyright 2012-2018 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -28,7 +28,7 @@ import com.fasterxml.jackson.databind.ObjectMapper; * @author Dave Syer * @see JsonParserFactory */ -public class JacksonJsonParser implements JsonParser { +public class JacksonJsonParser extends AbstractJsonParser { private static final TypeReference MAP_TYPE = new MapTypeReference(); @@ -38,22 +38,14 @@ public class JacksonJsonParser implements JsonParser { @Override public Map parseMap(String json) { - try { - return getObjectMapper().readValue(json, MAP_TYPE); - } - catch (Exception ex) { - throw AbstractJsonParser.cannotParseJson(ex); - } + return tryParse(() -> getObjectMapper().readValue(json, MAP_TYPE), + Exception.class); } @Override public List parseList(String json) { - try { - return getObjectMapper().readValue(json, LIST_TYPE); - } - catch (Exception ex) { - throw AbstractJsonParser.cannotParseJson(ex); - } + return tryParse(() -> getObjectMapper().readValue(json, LIST_TYPE), + Exception.class); } private ObjectMapper getObjectMapper() { diff --git a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/json/JsonParseException.java b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/json/JsonParseException.java new file mode 100644 index 0000000000..10d618fdf9 --- /dev/null +++ b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/json/JsonParseException.java @@ -0,0 +1,36 @@ +/* + * Copyright 2012-2018 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.boot.json; + +/** + * {@link IllegalArgumentException} thrown when source JSON is invalid. + * + * @author Anton Telechev + * @author Phillip Webb + * @since 2.0.1 + */ +public class JsonParseException extends IllegalArgumentException { + + public JsonParseException() { + this(null); + } + + public JsonParseException(Throwable cause) { + super("Cannot parse JSON", cause); + } + +} diff --git a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/json/JsonParser.java b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/json/JsonParser.java index 40af726562..c2cdabb158 100644 --- a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/json/JsonParser.java +++ b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/json/JsonParser.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2017 the original author or authors. + * Copyright 2012-2018 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -35,14 +35,16 @@ public interface JsonParser { * Parse the specified JSON string into a Map. * @param json the JSON to parse * @return the parsed JSON as a map + * @throws JsonParseException if the JSON cannot be parsed */ - Map parseMap(String json); + Map parseMap(String json) throws JsonParseException; /** * Parse the specified JSON string into a List. * @param json the JSON to parse * @return the parsed JSON as a list + * @throws JsonParseException if the JSON cannot be parsed */ - List parseList(String json); + List parseList(String json) throws JsonParseException; } diff --git a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/json/JsonSimpleJsonParser.java b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/json/JsonSimpleJsonParser.java index 1e9e9ce581..bdede7e059 100644 --- a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/json/JsonSimpleJsonParser.java +++ b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/json/JsonSimpleJsonParser.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2017 the original author or authors. + * Copyright 2012-2018 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -30,28 +30,20 @@ import org.json.simple.parser.ParseException; * @since 1.2.0 * @see JsonParserFactory */ -public class JsonSimpleJsonParser implements JsonParser { +public class JsonSimpleJsonParser extends AbstractJsonParser { @Override @SuppressWarnings("unchecked") public Map parseMap(String json) { - try { - return (Map) new JSONParser().parse(json); - } - catch (ParseException ex) { - throw AbstractJsonParser.cannotParseJson(ex); - } + return (Map) tryParse(() -> new JSONParser().parse(json), + ParseException.class); } @Override @SuppressWarnings("unchecked") public List parseList(String json) { - try { - return (List) new JSONParser().parse(json); - } - catch (ParseException ex) { - throw AbstractJsonParser.cannotParseJson(ex); - } + return (List) tryParse(() -> new JSONParser().parse(json), + ParseException.class); } } diff --git a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/json/YamlJsonParser.java b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/json/YamlJsonParser.java index eb3f13b6b3..14c6f68c87 100644 --- a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/json/YamlJsonParser.java +++ b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/json/YamlJsonParser.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2017 the original author or authors. + * Copyright 2012-2018 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License.