From b3bd292db3eebcc87938674636cf267e11136168 Mon Sep 17 00:00:00 2001 From: Rick Jensen Date: Thu, 6 Oct 2016 16:22:13 -0500 Subject: [PATCH] Add ability to sort mixed-type lists to JsonSlurper --- CHANGELOG.md | 2 ++ .../bard/webservice/util/JsonSlurper.java | 28 +++++++++++-------- 2 files changed, 19 insertions(+), 11 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 10d15e99df..bbf02766ac 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -66,6 +66,8 @@ Current - [#45, removing sorting from weight check queries](https://github.com/yahoo/fili/pull/46) +- `JsonSlurper` can now handle sorting lists with mixed-type entries, even if the list starts with a string, number, or + boolean ### Known Issues: diff --git a/fili-core/src/test/java/com/yahoo/bard/webservice/util/JsonSlurper.java b/fili-core/src/test/java/com/yahoo/bard/webservice/util/JsonSlurper.java index 840790317b..0f73d5cf9c 100644 --- a/fili-core/src/test/java/com/yahoo/bard/webservice/util/JsonSlurper.java +++ b/fili-core/src/test/java/com/yahoo/bard/webservice/util/JsonSlurper.java @@ -5,6 +5,7 @@ /* * Changed HashMap to LinkedHashMap for consistency. Added ability to optionally sort the lists and maps. */ + import static groovy.json.JsonTokenType.CLOSE_BRACKET; import static groovy.json.JsonTokenType.CLOSE_CURLY; import static groovy.json.JsonTokenType.COLON; @@ -50,7 +51,7 @@ @SuppressWarnings({ "rawtypes", "javadoc", "unchecked" }) public class JsonSlurper { - private final Comparator mapComparator; + private final Comparator variableTypedComparator; private final JsonSortStrategy sortStrategy; @@ -71,17 +72,17 @@ public JsonSlurper() { * @param sortStrategy Strategy to use when sorting the parsed JSON object */ public JsonSlurper(JsonSortStrategy sortStrategy) { - this(sortStrategy, new ToStringComparator()); + this(sortStrategy, new ToStringComparator<>()); } /** * Create a new JsonSlurper with the given sort strategy and map comparator. * * @param sortStrategy Strategy to use when sorting the parsed JSON object - * @param mapComparator Comparator to use when sorting a list of maps + * @param variableTypedComparator Comparator to use when sorting a list with variable types */ - public JsonSlurper(JsonSortStrategy sortStrategy, Comparator mapComparator) { - this.mapComparator = mapComparator; + public JsonSlurper(JsonSortStrategy sortStrategy, Comparator variableTypedComparator) { + this.variableTypedComparator = variableTypedComparator; this.sortStrategy = sortStrategy; switch (sortStrategy) { case SORT_BOTH: @@ -103,8 +104,8 @@ public JsonSlurper(JsonSortStrategy sortStrategy, Comparator mapComparator) } } - public Comparator getMapComparator() { - return mapComparator; + public Comparator getVariableTypedComparator() { + return variableTypedComparator; } public JsonSortStrategy getSortStrategy() { @@ -126,11 +127,16 @@ public boolean getSortMaps() { */ private void sortIfNeeded(List listToSort) { if (sortLists) { - // Sort by the native compareTo if available, otherwise just sort by hash code to ensure consistent sorting - if (listToSort.size() > 0 && listToSort.get(0) instanceof Comparable) { - Collections.sort(listToSort); + // Sort by the native compareTo, otherwise just sort by the object comparator to ensure consistent sorting + if (listToSort.stream().anyMatch(it -> it instanceof Comparable)) { + try { + Collections.sort(listToSort); + } catch (ClassCastException ignored) { + // Failed to sort a consistently typed list of Comparables. Fall back to the collection comparator + Collections.sort(listToSort, variableTypedComparator); + } } else { - Collections.sort(listToSort, mapComparator); + Collections.sort(listToSort, variableTypedComparator); } } }