From 73e50ceceb836421a176fe0fa2098a20b3c8a304 Mon Sep 17 00:00:00 2001 From: Christine Poerschke Date: Wed, 28 Dec 2016 10:41:17 +0000 Subject: [PATCH] SOLR-9787, SOLR-9442: Replace json.nl=arrnvp with json.nl=arrntv (array of Name Type Value) style in JSONResponseWriter --- solr/CHANGES.txt | 4 +- .../solr/response/JSONResponseWriter.java | 86 ++++++++++--------- .../apache/solr/response/JSONWriterTest.java | 20 +++-- 3 files changed, 60 insertions(+), 50 deletions(-) diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt index 7cc59ef2d1a..ac6db1afeda 100644 --- a/solr/CHANGES.txt +++ b/solr/CHANGES.txt @@ -74,8 +74,8 @@ New Features Example: { type:terms, field:category, filter:"user:yonik" } (yonik) -* SOLR-9442: Adds Array of NamedValuePair (json.nl=arrnvp) style to JSONResponseWriter. - (Jonny Marks, Christine Poerschke) +* SOLR-9442, SOLR-9787: Adds Array of Name Type Value (json.nl=arrntv) style to JSONResponseWriter. + (Jonny Marks, Christine Poerschke, hossman) * SOLR-9055: Make collection backup/restore extensible. (Hrishikesh Gadre, Varun Thacker, Mark Miller) diff --git a/solr/core/src/java/org/apache/solr/response/JSONResponseWriter.java b/solr/core/src/java/org/apache/solr/response/JSONResponseWriter.java index ae1ea4703ad..513df4eed53 100644 --- a/solr/core/src/java/org/apache/solr/response/JSONResponseWriter.java +++ b/solr/core/src/java/org/apache/solr/response/JSONResponseWriter.java @@ -59,9 +59,9 @@ public class JSONResponseWriter implements QueryResponseWriter { final String namedListStyle = params.get(JSONWriter.JSON_NL_STYLE, JSONWriter.JSON_NL_FLAT).intern(); final JSONWriter w; - if (namedListStyle.equals(JSONWriter.JSON_NL_ARROFNVP)) { - w = new ArrayOfNamedValuePairJSONWriter( - writer, req, rsp, wrapperFunction, namedListStyle); + if (namedListStyle.equals(JSONWriter.JSON_NL_ARROFNTV)) { + w = new ArrayOfNameTypeValueJSONWriter( + writer, req, rsp, wrapperFunction, namedListStyle, true); } else { w = new JSONWriter( writer, req, rsp, wrapperFunction, namedListStyle); @@ -96,7 +96,7 @@ class JSONWriter extends TextResponseWriter { static final String JSON_NL_FLAT="flat"; static final String JSON_NL_ARROFARR="arrarr"; static final String JSON_NL_ARROFMAP="arrmap"; - static final String JSON_NL_ARROFNVP="arrnvp"; + static final String JSON_NL_ARROFNTV="arrntv"; static final String JSON_WRAPPER_FUNCTION="json.wrf"; @@ -331,9 +331,9 @@ class JSONWriter extends TextResponseWriter { writeNamedListAsArrArr(name,val); } else if (namedListStyle==JSON_NL_ARROFMAP) { writeNamedListAsArrMap(name,val); - } else if (namedListStyle==JSON_NL_ARROFNVP) { + } else if (namedListStyle==JSON_NL_ARROFNTV) { throw new UnsupportedOperationException(namedListStyle - + " namedListStyle must only be used with "+ArrayOfNamedValuePairJSONWriter.class.getSimpleName()); + + " namedListStyle must only be used with "+ArrayOfNameTypeValueJSONWriter.class.getSimpleName()); } } @@ -675,20 +675,25 @@ class JSONWriter extends TextResponseWriter { } /** - * Writes NamedLists directly as an array of NamedValuePair JSON objects... - * NamedList("a"=1,"b"=2,null=3,null=null) => [{"name":"a","int":1},{"name":"b","int":2},{"int":3},{"null":null}] - * NamedList("a"=1,"bar"="foo",null=3.4f) => [{"name":"a","int":1},{"name":"bar","str":"foo"},{"float":3.4}] + * Writes NamedLists directly as an array of NameTypeValue JSON objects... + * NamedList("a"=1,"b"=null,null=3,null=null) => + * [{"name":"a","type":"int","value":1}, + * {"name":"b","type":"null","value":null}, + * {"name":null,"type":"int","value":3}, + * {"name":null,"type":"null","value":null}] + * NamedList("a"=1,"bar"="foo",null=3.4f) => + * [{"name":"a","type":"int","value":1}, + * {"name":"bar","type":"str","value":"foo"}, + * {"name":null,"type":"float","value":3.4}] */ -class ArrayOfNamedValuePairJSONWriter extends JSONWriter { - private boolean writeTypeAsKey = false; +class ArrayOfNameTypeValueJSONWriter extends JSONWriter { + protected boolean writeTypeAndValueKey = false; + private final boolean writeNullName; - public ArrayOfNamedValuePairJSONWriter(Writer writer, SolrQueryRequest req, SolrQueryResponse rsp, - String wrapperFunction, String namedListStyle) { + public ArrayOfNameTypeValueJSONWriter(Writer writer, SolrQueryRequest req, SolrQueryResponse rsp, + String wrapperFunction, String namedListStyle, boolean writeNullName) { super(writer, req, rsp, wrapperFunction, namedListStyle); - if (namedListStyle != JSON_NL_ARROFNVP) { - throw new UnsupportedOperationException(ArrayOfNamedValuePairJSONWriter.class.getSimpleName()+" must only be used with " - + JSON_NL_ARROFNVP + " style"); - } + this.writeNullName = writeNullName; } @Override @@ -720,24 +725,24 @@ class ArrayOfNamedValuePairJSONWriter extends JSONWriter { /* * JSONWriter's writeNamedListAsArrMap turns NamedList("bar"="foo") into [{"foo":"bar"}] - * but we here wish to turn it into [ {"name":"bar","str":"foo"} ] instead. + * but we here wish to turn it into [ {"name":"bar","type":"str","value":"foo"} ] instead. * * So first we write the {"name":"bar", portion ... */ writeMapOpener(-1); - if (elementName != null) { + if (elementName != null || writeNullName) { writeKey("name", false); writeVal("name", elementName); writeMapSeparator(); } /* - * ... and then we write the "str":"foo"} portion. + * ... and then we write the "type":"str","value":"foo"} portion. */ - writeTypeAsKey = true; + writeTypeAndValueKey = true; writeVal(null, elementVal); // passing null since writeVal doesn't actually use name (and we already wrote elementName above) - if (writeTypeAsKey) { - throw new RuntimeException("writeTypeAsKey should have been reset to false by writeVal('"+elementName+"','"+elementVal+"')"); + if (writeTypeAndValueKey) { + throw new RuntimeException("writeTypeAndValueKey should have been reset to false by writeVal('"+elementName+"','"+elementVal+"')"); } writeMapCloser(); } @@ -746,82 +751,85 @@ class ArrayOfNamedValuePairJSONWriter extends JSONWriter { writeArrayCloser(); } - private void ifNeededWriteTypeAsKey(String type) throws IOException { - if (writeTypeAsKey) { - writeTypeAsKey = false; - writeKey(type, false); + protected void ifNeededWriteTypeAndValueKey(String type) throws IOException { + if (writeTypeAndValueKey) { + writeTypeAndValueKey = false; + writeKey("type", false); + writeVal("type", type); + writeMapSeparator(); + writeKey("value", false); } } @Override public void writeInt(String name, String val) throws IOException { - ifNeededWriteTypeAsKey("int"); + ifNeededWriteTypeAndValueKey("int"); super.writeInt(name, val); } @Override public void writeLong(String name, String val) throws IOException { - ifNeededWriteTypeAsKey("long"); + ifNeededWriteTypeAndValueKey("long"); super.writeLong(name, val); } @Override public void writeFloat(String name, String val) throws IOException { - ifNeededWriteTypeAsKey("float"); + ifNeededWriteTypeAndValueKey("float"); super.writeFloat(name, val); } @Override public void writeDouble(String name, String val) throws IOException { - ifNeededWriteTypeAsKey("double"); + ifNeededWriteTypeAndValueKey("double"); super.writeDouble(name, val); } @Override public void writeBool(String name, String val) throws IOException { - ifNeededWriteTypeAsKey("bool"); + ifNeededWriteTypeAndValueKey("bool"); super.writeBool(name, val); } @Override public void writeDate(String name, String val) throws IOException { - ifNeededWriteTypeAsKey("date"); + ifNeededWriteTypeAndValueKey("date"); super.writeDate(name, val); } @Override public void writeStr(String name, String val, boolean needsEscaping) throws IOException { - ifNeededWriteTypeAsKey("str"); + ifNeededWriteTypeAndValueKey("str"); super.writeStr(name, val, needsEscaping); } @Override public void writeSolrDocument(String name, SolrDocument doc, ReturnFields returnFields, int idx) throws IOException { - ifNeededWriteTypeAsKey("doc"); + ifNeededWriteTypeAndValueKey("doc"); super.writeSolrDocument(name, doc, returnFields, idx); } @Override public void writeStartDocumentList(String name, long start, int size, long numFound, Float maxScore) throws IOException { - ifNeededWriteTypeAsKey("doclist"); + ifNeededWriteTypeAndValueKey("doclist"); super.writeStartDocumentList(name, start, size, numFound, maxScore); } @Override public void writeMap(String name, Map val, boolean excludeOuter, boolean isFirstVal) throws IOException { - ifNeededWriteTypeAsKey("map"); + ifNeededWriteTypeAndValueKey("map"); super.writeMap(name, val, excludeOuter, isFirstVal); } @Override public void writeArray(String name, Iterator val) throws IOException { - ifNeededWriteTypeAsKey("array"); + ifNeededWriteTypeAndValueKey("array"); super.writeArray(name, val); } @Override public void writeNull(String name) throws IOException { - ifNeededWriteTypeAsKey("null"); + ifNeededWriteTypeAndValueKey("null"); super.writeNull(name); } } diff --git a/solr/core/src/test/org/apache/solr/response/JSONWriterTest.java b/solr/core/src/test/org/apache/solr/response/JSONWriterTest.java index a056016d5b8..45ca7087792 100644 --- a/solr/core/src/test/org/apache/solr/response/JSONWriterTest.java +++ b/solr/core/src/test/org/apache/solr/response/JSONWriterTest.java @@ -81,7 +81,7 @@ public class JSONWriterTest extends SolrTestCaseJ4 { JSONWriter.JSON_NL_MAP, JSONWriter.JSON_NL_ARROFARR, JSONWriter.JSON_NL_ARROFMAP, - JSONWriter.JSON_NL_ARROFNVP, + JSONWriter.JSON_NL_ARROFNTV, }; for (final String namedListStyle : namedListStyles) { implTestJSON(namedListStyle); @@ -116,8 +116,10 @@ public class JSONWriterTest extends SolrTestCaseJ4 { expectedNLjson = "\"nl\":[[\"data1\",\"he\\u2028llo\\u2029!\"],[null,42],[null,null]]"; } else if (namedListStyle == JSONWriter.JSON_NL_ARROFMAP) { expectedNLjson = "\"nl\":[{\"data1\":\"he\\u2028llo\\u2029!\"},42,null]"; - } else if (namedListStyle == JSONWriter.JSON_NL_ARROFNVP) { - expectedNLjson = "\"nl\":[{\"name\":\"data1\",\"str\":\"he\\u2028llo\\u2029!\"},{\"int\":42},{\"null\":null}]"; + } else if (namedListStyle == JSONWriter.JSON_NL_ARROFNTV) { + expectedNLjson = "\"nl\":[{\"name\":\"data1\",\"type\":\"str\",\"value\":\"he\\u2028llo\\u2029!\"}," + + "{\"name\":null,\"type\":\"int\",\"value\":42}," + + "{\"name\":null,\"type\":\"null\",\"value\":null}]"; } else { expectedNLjson = null; fail("unexpected namedListStyle="+namedListStyle); @@ -168,7 +170,7 @@ public class JSONWriterTest extends SolrTestCaseJ4 { } @Test - public void testArrnvpWriterOverridesAllWrites() { + public void testArrntvWriterOverridesAllWrites() { // List rather than Set because two not-overridden methods could share name but not signature final List methodsExpectedNotOverriden = new ArrayList<>(14); methodsExpectedNotOverriden.add("writeResponse"); @@ -189,7 +191,7 @@ public class JSONWriterTest extends SolrTestCaseJ4 { methodsExpectedNotOverriden.add("public void org.apache.solr.response.JSONWriter.writeMap(org.apache.solr.common.MapWriter) throws java.io.IOException"); methodsExpectedNotOverriden.add("public void org.apache.solr.response.JSONWriter.writeIterator(org.apache.solr.common.IteratorWriter) throws java.io.IOException"); - final Class subClass = ArrayOfNamedValuePairJSONWriter.class; + final Class subClass = ArrayOfNameTypeValueJSONWriter.class; final Class superClass = subClass.getSuperclass(); for (final Method superClassMethod : superClass.getDeclaredMethods()) { @@ -231,14 +233,14 @@ public class JSONWriterTest extends SolrTestCaseJ4 { } @Test - public void testArrnvpWriterLacksMethodsOfItsOwn() { - final Class subClass = ArrayOfNamedValuePairJSONWriter.class; + public void testArrntvWriterLacksMethodsOfItsOwn() { + final Class subClass = ArrayOfNameTypeValueJSONWriter.class; final Class superClass = subClass.getSuperclass(); // ArrayOfNamedValuePairJSONWriter is a simple sub-class // which should have (almost) no methods of its own for (final Method subClassMethod : subClass.getDeclaredMethods()) { // only own private method of its own - if (subClassMethod.getName().equals("ifNeededWriteTypeAsKey")) continue; + if (subClassMethod.getName().equals("ifNeededWriteTypeAndValueKey")) continue; try { final Method superClassMethod = superClass.getDeclaredMethod( subClassMethod.getName(), @@ -260,7 +262,7 @@ public class JSONWriterTest extends SolrTestCaseJ4 { assertEquals("flat", JSONWriter.JSON_NL_FLAT); assertEquals("arrarr", JSONWriter.JSON_NL_ARROFARR); assertEquals("arrmap", JSONWriter.JSON_NL_ARROFMAP); - assertEquals("arrnvp", JSONWriter.JSON_NL_ARROFNVP); + assertEquals("arrntv", JSONWriter.JSON_NL_ARROFNTV); assertEquals("json.wrf", JSONWriter.JSON_WRAPPER_FUNCTION); }