You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@solr.apache.org by no...@apache.org on 2023/05/11 03:13:11 UTC

[solr] branch branch_9x updated: SOLR-16691 :Use Jackson for JSON serialization (2) (#1633)

This is an automated email from the ASF dual-hosted git repository.

noble pushed a commit to branch branch_9x
in repository https://gitbox.apache.org/repos/asf/solr.git


The following commit(s) were added to refs/heads/branch_9x by this push:
     new e5ca599f51f SOLR-16691 :Use Jackson for JSON serialization (2) (#1633)
e5ca599f51f is described below

commit e5ca599f51f859834efde2e190ba1e3f17cbb4b1
Author: Noble Paul <no...@users.noreply.github.com>
AuthorDate: Thu May 11 13:12:10 2023 +1000

    SOLR-16691 :Use Jackson for JSON serialization (2) (#1633)
---
 .../src/java/org/apache/solr/core/SolrCore.java    |   4 +-
 .../apache/solr/handler/export/ExportWriter.java   |   5 +-
 .../solr/response/BinaryQueryResponseWriter.java   |  14 +-
 .../apache/solr/response/JacksonJsonWriter.java    | 235 +++++++++++++++++++++
 .../solr/response/QueryResponseWriterUtil.java     |  45 ++--
 .../response/transform/GeoTransformerFactory.java  |   4 +-
 .../apache/solr/servlet/DirectSolrConnection.java  |  11 +-
 .../solr/response/TestRawResponseWriter.java       |   7 +-
 .../src/java/org/apache/solr/JSONTestUtil.java     |   1 +
 .../src/java/org/apache/solr/SolrTestCase.java     |  10 +
 10 files changed, 306 insertions(+), 30 deletions(-)

diff --git a/solr/core/src/java/org/apache/solr/core/SolrCore.java b/solr/core/src/java/org/apache/solr/core/SolrCore.java
index ed3b00f97a9..c77b885beb4 100644
--- a/solr/core/src/java/org/apache/solr/core/SolrCore.java
+++ b/solr/core/src/java/org/apache/solr/core/SolrCore.java
@@ -126,7 +126,7 @@ import org.apache.solr.response.BinaryResponseWriter;
 import org.apache.solr.response.CSVResponseWriter;
 import org.apache.solr.response.GeoJSONResponseWriter;
 import org.apache.solr.response.GraphMLResponseWriter;
-import org.apache.solr.response.JSONResponseWriter;
+import org.apache.solr.response.JacksonJsonWriter;
 import org.apache.solr.response.PHPResponseWriter;
 import org.apache.solr.response.PHPSerializedResponseWriter;
 import org.apache.solr.response.PythonResponseWriter;
@@ -3024,7 +3024,7 @@ public class SolrCore implements SolrInfoBean, Closeable {
   static {
     HashMap<String, QueryResponseWriter> m = new HashMap<>(15, 1);
     m.put("xml", new XMLResponseWriter());
-    m.put(CommonParams.JSON, new JSONResponseWriter());
+    m.put(CommonParams.JSON, new JacksonJsonWriter());
     m.put("standard", m.get(CommonParams.JSON));
     m.put("geojson", new GeoJSONResponseWriter());
     m.put("graphml", new GraphMLResponseWriter());
diff --git a/solr/core/src/java/org/apache/solr/handler/export/ExportWriter.java b/solr/core/src/java/org/apache/solr/handler/export/ExportWriter.java
index 6b40fc69175..8f38c94942e 100644
--- a/solr/core/src/java/org/apache/solr/handler/export/ExportWriter.java
+++ b/solr/core/src/java/org/apache/solr/handler/export/ExportWriter.java
@@ -60,6 +60,7 @@ import org.apache.solr.request.SolrQueryRequest;
 import org.apache.solr.request.SolrRequestInfo;
 import org.apache.solr.response.BinaryResponseWriter;
 import org.apache.solr.response.JSONResponseWriter;
+import org.apache.solr.response.JacksonJsonWriter;
 import org.apache.solr.response.QueryResponseWriter;
 import org.apache.solr.response.SolrQueryResponse;
 import org.apache.solr.schema.BoolField;
@@ -185,7 +186,9 @@ public class ExportWriter implements SolrCore.RawWriter, Closeable {
 
   private void _write(OutputStream os) throws IOException {
     QueryResponseWriter rw = req.getCore().getResponseWriters().get(wt);
-    if (rw instanceof BinaryResponseWriter) {
+    if (rw instanceof JacksonJsonWriter) {
+      writer = ((JacksonJsonWriter) rw).getWriter(os, req, res);
+    } else if (rw instanceof BinaryResponseWriter) {
       // todo add support for other writers after testing
       writer = new JavaBinCodec(os, null);
     } else {
diff --git a/solr/core/src/java/org/apache/solr/response/BinaryQueryResponseWriter.java b/solr/core/src/java/org/apache/solr/response/BinaryQueryResponseWriter.java
index cb38190069f..c9ecfb8c55d 100644
--- a/solr/core/src/java/org/apache/solr/response/BinaryQueryResponseWriter.java
+++ b/solr/core/src/java/org/apache/solr/response/BinaryQueryResponseWriter.java
@@ -18,6 +18,7 @@ package org.apache.solr.response;
 
 import java.io.IOException;
 import java.io.OutputStream;
+import java.nio.charset.StandardCharsets;
 import org.apache.solr.request.SolrQueryRequest;
 
 /**
@@ -31,6 +32,17 @@ import org.apache.solr.request.SolrQueryRequest;
 public interface BinaryQueryResponseWriter extends QueryResponseWriter {
 
   /** Use it to write the response in a binary format */
-  public void write(OutputStream out, SolrQueryRequest request, SolrQueryResponse response)
+  void write(OutputStream out, SolrQueryRequest request, SolrQueryResponse response)
       throws IOException;
+
+  default String serializeResponse(SolrQueryRequest req, SolrQueryResponse rsp) {
+    java.io.ByteArrayOutputStream baos = new java.io.ByteArrayOutputStream();
+    try {
+      write(baos, req, rsp);
+      return baos.toString(StandardCharsets.UTF_8);
+    } catch (IOException e) {
+      // unlikely
+      throw new RuntimeException(e);
+    }
+  }
 }
diff --git a/solr/core/src/java/org/apache/solr/response/JacksonJsonWriter.java b/solr/core/src/java/org/apache/solr/response/JacksonJsonWriter.java
new file mode 100644
index 00000000000..7a1ff5276a8
--- /dev/null
+++ b/solr/core/src/java/org/apache/solr/response/JacksonJsonWriter.java
@@ -0,0 +1,235 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You 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.apache.solr.response;
+
+import com.fasterxml.jackson.core.JsonEncoding;
+import com.fasterxml.jackson.core.JsonFactory;
+import com.fasterxml.jackson.core.JsonGenerator;
+import com.fasterxml.jackson.core.PrettyPrinter;
+import com.fasterxml.jackson.core.util.DefaultPrettyPrinter;
+import java.io.IOException;
+import java.io.OutputStream;
+import java.math.BigDecimal;
+import java.math.BigInteger;
+import org.apache.solr.common.PushWriter;
+import org.apache.solr.request.SolrQueryRequest;
+
+/** A JSON ResponseWriter that uses jackson. */
+public class JacksonJsonWriter extends BinaryResponseWriter {
+
+  protected final JsonFactory jsonfactory;
+  protected static final PrettyPrinter pretty =
+      new DefaultPrettyPrinter()
+          .withoutSpacesInObjectEntries()
+          .withArrayIndenter(DefaultPrettyPrinter.NopIndenter.instance);
+
+  public JacksonJsonWriter() {
+    super();
+    jsonfactory = new JsonFactory();
+  }
+
+  @Override
+  public void write(OutputStream out, SolrQueryRequest request, SolrQueryResponse response)
+      throws IOException {
+    WriterImpl sw = new WriterImpl(jsonfactory, out, request, response);
+    sw.writeResponse();
+    sw.close();
+  }
+
+  public PushWriter getWriter(
+      OutputStream out, SolrQueryRequest request, SolrQueryResponse response) {
+    return new WriterImpl(jsonfactory, out, request, response);
+  }
+
+  @Override
+  public String getContentType(SolrQueryRequest request, SolrQueryResponse response) {
+    return JSONResponseWriter.CONTENT_TYPE_JSON_UTF8;
+  }
+  // So we extend JSONWriter and override the relevant methods
+
+  public static class WriterImpl extends JSONWriter {
+
+    protected JsonGenerator gen;
+
+    public WriterImpl(
+        JsonFactory j, OutputStream out, SolrQueryRequest req, SolrQueryResponse rsp) {
+      super(null, req, rsp);
+      try {
+        gen = j.createGenerator(out, JsonEncoding.UTF8);
+        if (doIndent) {
+          gen.setPrettyPrinter(pretty);
+        }
+      } catch (IOException e) {
+        throw new RuntimeException(e);
+      }
+    }
+
+    @Override
+    public void writeResponse() throws IOException {
+      super.writeNamedList(null, rsp.getValues());
+      gen.close();
+    }
+
+    @Override
+    public void writeNumber(String name, Number val) throws IOException {
+      if (val instanceof Integer) {
+        gen.writeNumber(val.intValue());
+      } else if (val instanceof Long) {
+        gen.writeNumber(val.longValue());
+      } else if (val instanceof Float) {
+        gen.writeNumber(val.floatValue());
+      } else if (val instanceof Double) {
+        gen.writeNumber(val.doubleValue());
+      } else if (val instanceof Short) {
+        gen.writeNumber(val.shortValue());
+      } else if (val instanceof Byte) {
+        gen.writeNumber(val.byteValue());
+      } else if (val instanceof BigInteger) {
+        gen.writeNumber((BigInteger) val);
+      } else if (val instanceof BigDecimal) {
+        gen.writeNumber((BigDecimal) val);
+      } else {
+        gen.writeString(val.getClass().getName() + ':' + val.toString());
+        // default... for debugging only
+      }
+    }
+
+    @Override
+    public void writeBool(String name, Boolean val) throws IOException {
+      gen.writeBoolean(val);
+    }
+
+    @Override
+    public void writeNull(String name) throws IOException {
+      gen.writeNull();
+    }
+
+    @Override
+    public void writeStr(String name, String val, boolean needsEscaping) throws IOException {
+      if (needsEscaping) {
+        gen.writeString(val);
+      } else {
+        gen.writeRawValue(val);
+      }
+    }
+
+    @Override
+    public void writeLong(String name, long val) throws IOException {
+      gen.writeNumber(val);
+    }
+
+    @Override
+    public void writeInt(String name, int val) throws IOException {
+      gen.writeNumber(val);
+    }
+
+    @Override
+    public void writeBool(String name, boolean val) throws IOException {
+      gen.writeBoolean(val);
+    }
+
+    @Override
+    public void writeFloat(String name, float val) throws IOException {
+      gen.writeNumber(val);
+    }
+
+    @Override
+    public void writeArrayCloser() throws IOException {
+      gen.writeEndArray();
+    }
+
+    @Override
+    public void writeArraySeparator() {
+      // do nothing
+    }
+
+    @Override
+    public void writeArrayOpener(int size) throws IOException, IllegalArgumentException {
+      gen.writeStartArray();
+    }
+
+    @Override
+    public void writeMapCloser() throws IOException {
+      gen.writeEndObject();
+    }
+
+    @Override
+    public void writeMapSeparator() {
+      // do nothing
+    }
+
+    @Override
+    public void writeMapOpener(int size) throws IOException, IllegalArgumentException {
+      gen.writeStartObject();
+    }
+
+    @Override
+    public void writeKey(String fname, boolean needsEscaping) throws IOException {
+      gen.writeFieldName(fname);
+    }
+
+    @Override
+    public void writeByteArr(String name, byte[] buf, int offset, int len) throws IOException {
+      gen.writeBinary(buf, offset, len);
+    }
+
+    @Override
+    public void setLevel(int level) {
+      // do nothing
+    }
+
+    @Override
+    public int level() {
+      return 0;
+    }
+
+    @Override
+    public void indent() throws IOException {
+      // do nothing
+    }
+
+    @Override
+    public void indent(int lev) throws IOException {
+      // do nothing
+    }
+
+    @Override
+    public int incLevel() {
+      return 0;
+    }
+
+    @Override
+    public int decLevel() {
+      return 0;
+    }
+
+    @Override
+    public void close() throws IOException {
+      gen.close();
+    }
+
+    @Override
+    public void writeStrRaw(String name, String val) throws IOException {
+      gen.writeRawValue(val);
+    }
+
+    @Override
+    public void writeDate(String name, String val) throws IOException {
+      gen.writeString(val);
+    }
+  }
+}
diff --git a/solr/core/src/java/org/apache/solr/response/QueryResponseWriterUtil.java b/solr/core/src/java/org/apache/solr/response/QueryResponseWriterUtil.java
index f2738333ed5..05f79bf9113 100644
--- a/solr/core/src/java/org/apache/solr/response/QueryResponseWriterUtil.java
+++ b/solr/core/src/java/org/apache/solr/response/QueryResponseWriterUtil.java
@@ -16,6 +16,7 @@
  */
 package org.apache.solr.response;
 
+import java.io.BufferedOutputStream;
 import java.io.IOException;
 import java.io.OutputStream;
 import java.io.OutputStreamWriter;
@@ -48,25 +49,16 @@ public final class QueryResponseWriterUtil {
       String contentType)
       throws IOException {
 
-    if (responseWriter instanceof BinaryQueryResponseWriter) {
+    if (responseWriter instanceof JacksonJsonWriter) {
+      JacksonJsonWriter binWriter = (JacksonJsonWriter) responseWriter;
+      BufferedOutputStream bos = new BufferedOutputStream(new NonFlushingStream(outputStream));
+      binWriter.write(bos, solrRequest, solrResponse);
+      bos.flush();
+    } else if (responseWriter instanceof BinaryQueryResponseWriter) {
       BinaryQueryResponseWriter binWriter = (BinaryQueryResponseWriter) responseWriter;
       binWriter.write(outputStream, solrRequest, solrResponse);
     } else {
-      OutputStream out =
-          new OutputStream() {
-            @Override
-            public void write(int b) throws IOException {
-              outputStream.write(b);
-            }
-
-            @Override
-            public void flush() throws IOException {
-              // We don't flush here, which allows us to flush below
-              // and only flush internal buffers, not the response.
-              // If we flush the response early, we trigger chunked encoding.
-              // See SOLR-8669.
-            }
-          };
+      OutputStream out = new NonFlushingStream(outputStream);
       Writer writer = buildWriter(out, ContentStreamBase.getCharsetFromContentType(contentType));
       responseWriter.write(writer, solrRequest, solrResponse);
       writer.flush();
@@ -82,4 +74,25 @@ public final class QueryResponseWriterUtil {
 
     return new FastWriter(writer);
   }
+
+  private static class NonFlushingStream extends OutputStream {
+    private final OutputStream outputStream;
+
+    public NonFlushingStream(OutputStream outputStream) {
+      this.outputStream = outputStream;
+    }
+
+    @Override
+    public void write(int b) throws IOException {
+      outputStream.write(b);
+    }
+
+    @Override
+    public void flush() throws IOException {
+      // We don't flush here, which allows us to flush below
+      // and only flush internal buffers, not the response.
+      // If we flush the response early, we trigger chunked encoding.
+      // See SOLR-8669.
+    }
+  }
 }
diff --git a/solr/core/src/java/org/apache/solr/response/transform/GeoTransformerFactory.java b/solr/core/src/java/org/apache/solr/response/transform/GeoTransformerFactory.java
index 1e971021b69..443426a1d68 100644
--- a/solr/core/src/java/org/apache/solr/response/transform/GeoTransformerFactory.java
+++ b/solr/core/src/java/org/apache/solr/response/transform/GeoTransformerFactory.java
@@ -37,6 +37,7 @@ import org.apache.solr.common.SolrException.ErrorCode;
 import org.apache.solr.common.params.SolrParams;
 import org.apache.solr.request.SolrQueryRequest;
 import org.apache.solr.response.JSONResponseWriter;
+import org.apache.solr.response.JacksonJsonWriter;
 import org.apache.solr.response.QueryResponseWriter;
 import org.apache.solr.schema.AbstractSpatialFieldType;
 import org.apache.solr.schema.SchemaField;
@@ -134,7 +135,8 @@ public class GeoTransformerFactory extends TransformerFactory
 
     QueryResponseWriter qw = req.getCore().getQueryResponseWriter(req);
     updater.isJSON =
-        (qw.getClass() == JSONResponseWriter.class) && (updater.writer instanceof GeoJSONWriter);
+        (qw.getClass() == JSONResponseWriter.class || qw.getClass() == JacksonJsonWriter.class)
+            && (updater.writer instanceof GeoJSONWriter);
 
     // Using ValueSource
     if (shapes != null) {
diff --git a/solr/core/src/java/org/apache/solr/servlet/DirectSolrConnection.java b/solr/core/src/java/org/apache/solr/servlet/DirectSolrConnection.java
index 7cc7f798d57..f7b5f59b48a 100644
--- a/solr/core/src/java/org/apache/solr/servlet/DirectSolrConnection.java
+++ b/solr/core/src/java/org/apache/solr/servlet/DirectSolrConnection.java
@@ -30,6 +30,7 @@ import org.apache.solr.core.SolrCore;
 import org.apache.solr.request.SolrQueryRequest;
 import org.apache.solr.request.SolrRequestHandler;
 import org.apache.solr.request.SolrRequestInfo;
+import org.apache.solr.response.BinaryResponseWriter;
 import org.apache.solr.response.QueryResponseWriter;
 import org.apache.solr.response.SolrQueryResponse;
 
@@ -116,9 +117,13 @@ public class DirectSolrConnection {
 
       // Now write it out
       QueryResponseWriter responseWriter = core.getQueryResponseWriter(req);
-      StringWriter out = new StringWriter();
-      responseWriter.write(out, req, rsp);
-      return out.toString();
+      if (responseWriter instanceof BinaryResponseWriter) {
+        return ((BinaryResponseWriter) responseWriter).serializeResponse(req, rsp);
+      } else {
+        StringWriter out = new StringWriter();
+        responseWriter.write(out, req, rsp);
+        return out.toString();
+      }
     } finally {
       if (req != null) {
         req.close();
diff --git a/solr/core/src/test/org/apache/solr/response/TestRawResponseWriter.java b/solr/core/src/test/org/apache/solr/response/TestRawResponseWriter.java
index 07560024089..adf15bdb706 100644
--- a/solr/core/src/test/org/apache/solr/response/TestRawResponseWriter.java
+++ b/solr/core/src/test/org/apache/solr/response/TestRawResponseWriter.java
@@ -155,12 +155,7 @@ public class TestRawResponseWriter extends SolrTestCaseJ4 {
 
     // json
     String json = "{\n" + "  \"content\":\"test\",\n" + "  \"foo\":\"bar\"}\n";
-    StringWriter jsonSout = new StringWriter();
-    writerJsonBase.write(jsonSout, req(), rsp);
-    assertEquals(json, jsonSout.toString());
-    ByteArrayOutputStream jsonBout = new ByteArrayOutputStream();
-    writerJsonBase.write(jsonBout, req(), rsp);
-    assertEquals(json, jsonBout.toString(StandardCharsets.UTF_8.toString()));
+    assertJSONEquals(json, writerJsonBase.serializeResponse(req(), rsp));
 
     // javabin
     ByteArrayOutputStream bytes = new ByteArrayOutputStream();
diff --git a/solr/test-framework/src/java/org/apache/solr/JSONTestUtil.java b/solr/test-framework/src/java/org/apache/solr/JSONTestUtil.java
index 029ba3bfbb8..70cdcc27fd7 100644
--- a/solr/test-framework/src/java/org/apache/solr/JSONTestUtil.java
+++ b/solr/test-framework/src/java/org/apache/solr/JSONTestUtil.java
@@ -77,6 +77,7 @@ public class JSONTestUtil {
     int pos = pathAndExpected.indexOf("==");
     String path = pos >= 0 ? pathAndExpected.substring(0, pos) : null;
     String expected = pos >= 0 ? pathAndExpected.substring(pos + 2) : pathAndExpected;
+    System.out.println("IN-PUT: " + input);
     return match(path, input, expected, delta);
   }
 
diff --git a/solr/test-framework/src/java/org/apache/solr/SolrTestCase.java b/solr/test-framework/src/java/org/apache/solr/SolrTestCase.java
index 3858f8cb959..d9566648bf8 100644
--- a/solr/test-framework/src/java/org/apache/solr/SolrTestCase.java
+++ b/solr/test-framework/src/java/org/apache/solr/SolrTestCase.java
@@ -18,6 +18,7 @@
 package org.apache.solr;
 
 import static com.carrotsearch.randomizedtesting.RandomizedTest.systemPropertyAsBoolean;
+import static org.apache.solr.common.util.Utils.fromJSONString;
 
 import com.carrotsearch.randomizedtesting.annotations.ThreadLeakFilters;
 import com.carrotsearch.randomizedtesting.annotations.ThreadLeakLingering;
@@ -26,6 +27,7 @@ import com.carrotsearch.randomizedtesting.rules.SystemPropertiesRestoreRule;
 import java.io.File;
 import java.lang.invoke.MethodHandles;
 import java.util.List;
+import java.util.Objects;
 import java.util.regex.Pattern;
 import org.apache.lucene.tests.util.LuceneTestCase;
 import org.apache.lucene.tests.util.LuceneTestCase.SuppressSysoutChecks;
@@ -39,6 +41,7 @@ import org.apache.solr.util.StartupLoggingUtils;
 import org.junit.Before;
 import org.junit.BeforeClass;
 import org.junit.ClassRule;
+import org.junit.ComparisonFailure;
 import org.junit.rules.RuleChain;
 import org.junit.rules.TestRule;
 import org.slf4j.Logger;
@@ -183,4 +186,11 @@ public class SolrTestCase extends LuceneTestCase {
     final String PROP = "tests.force.assumption.failure.before";
     assumeFalse(PROP + " == true", systemPropertyAsBoolean(PROP, false));
   }
+
+  public static void assertJSONEquals(String expected, String actual) {
+    Object json1 = fromJSONString(expected);
+    Object json2 = fromJSONString(actual);
+    if (Objects.equals(json2, json1)) return;
+    throw new ComparisonFailure("", expected, actual);
+  }
 }