SOLR-17176: Fix log history V2 API serialization (#2293)
diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt
index e485de5..a9c2054 100644
--- a/solr/CHANGES.txt
+++ b/solr/CHANGES.txt
@@ -174,6 +174,8 @@
* SOLR-17213: Fix spurious warnings about solr url format in Solr CLI when users aren't providing a deprecated solr url. (Eric Pugh)
+* SOLR-17176: Fix log history V2 API serialization (Michael Gibney)
+
Dependency Upgrades
---------------------
diff --git a/solr/core/src/java/org/apache/solr/logging/LogWatcher.java b/solr/core/src/java/org/apache/solr/logging/LogWatcher.java
index 86ca1b8..30f4f6e 100644
--- a/solr/core/src/java/org/apache/solr/logging/LogWatcher.java
+++ b/solr/core/src/java/org/apache/solr/logging/LogWatcher.java
@@ -21,10 +21,13 @@
import java.util.Collections;
import java.util.Date;
import java.util.Iterator;
+import java.util.LinkedHashMap;
import java.util.List;
+import java.util.Map;
import java.util.concurrent.atomic.AtomicBoolean;
import org.apache.solr.common.SolrDocument;
import org.apache.solr.common.SolrDocumentList;
+import org.apache.solr.common.util.CollectionUtil;
import org.apache.solr.core.SolrResourceLoader;
import org.apache.solr.logging.jul.JulWatcher;
import org.apache.solr.logging.log4j2.Log4j2Watcher;
@@ -75,7 +78,16 @@
* instance to multiple callers, we should guard against modification.
*/
private static SolrDocument unmodifiable(SolrDocument doc) {
- return new SolrDocument(Collections.unmodifiableMap(doc.getFieldValueMap()));
+ LinkedHashMap<String, Object> map = CollectionUtil.newLinkedHashMap(doc.size());
+ for (Map.Entry<String, Object> e : doc) {
+ Object v = e.getValue();
+ if (v instanceof Collection) {
+ map.put(e.getKey(), Collections.unmodifiableCollection((Collection<?>) v));
+ } else {
+ map.put(e.getKey(), v);
+ }
+ }
+ return new SolrDocument(Collections.unmodifiableMap(map));
}
public long getLastEvent() {
diff --git a/solr/core/src/test/org/apache/solr/logging/TestLogWatcher.java b/solr/core/src/test/org/apache/solr/logging/TestLogWatcher.java
index 11cdf8e..34cd05e 100644
--- a/solr/core/src/test/org/apache/solr/logging/TestLogWatcher.java
+++ b/solr/core/src/test/org/apache/solr/logging/TestLogWatcher.java
@@ -16,17 +16,31 @@
*/
package org.apache.solr.logging;
+import java.io.ByteArrayOutputStream;
+import java.io.IOException;
+import java.io.StringWriter;
import java.lang.invoke.MethodHandles;
+import java.nio.charset.StandardCharsets;
import java.util.ArrayList;
import java.util.List;
import java.util.concurrent.TimeUnit;
import org.apache.solr.SolrTestCaseJ4;
+import org.apache.solr.common.MapWriter;
import org.apache.solr.common.SolrDocument;
import org.apache.solr.common.SolrDocumentList;
+import org.apache.solr.common.params.ModifiableSolrParams;
import org.apache.solr.common.util.TimeSource;
+import org.apache.solr.request.SolrQueryRequest;
+import org.apache.solr.request.SolrQueryRequestBase;
+import org.apache.solr.response.BinaryQueryResponseWriter;
+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.util.TimeOut;
import org.junit.Before;
import org.junit.Test;
+import org.noggit.CharArr;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@@ -45,7 +59,7 @@
// All we really care about is that new watchers get the new messages, so test for that
// explicitly. See SOLR-12732.
@Test
- public void testLog4jWatcher() throws InterruptedException {
+ public void testLog4jWatcher() throws InterruptedException, IOException {
LogWatcher<?> watcher = null;
int lim = random().nextInt(3) + 2;
// Every time through this loop, ensure that, of all the test messages that have been logged,
@@ -65,9 +79,12 @@
boolean foundNewMsg = false;
boolean foundOldMessage = false;
// In local testing this loop usually succeeds 1-2 tries, so it's not very expensive to loop.
+ QueryResponseWriter responseWriter =
+ random().nextBoolean() ? new JacksonJsonWriter() : new JSONResponseWriter();
do {
// Returns an empty (but non-null) list even if there are no messages yet.
SolrDocumentList events = watcher.getHistory(-1, null);
+ validateWrite(responseWriter, events, msg);
for (SolrDocument doc : events) {
String oneMsg = (String) doc.get("message");
if (oneMsg.equals(msg)) {
@@ -104,4 +121,56 @@
oldMessages.add(msg);
}
}
+
+ /**
+ * Here we validate that serialization works as expected for several different methods. Ideally we
+ * would use actual serialization from Jersey/Jackson, since this is what really happens in V2
+ * APIs. But this is simpler, and should give us roughly equivalent assurances.
+ */
+ private static void validateWrite(
+ QueryResponseWriter responseWriter, SolrDocumentList docs, String expectMsg)
+ throws IOException {
+ SolrQueryRequest req = new SolrQueryRequestBase(null, new ModifiableSolrParams()) {};
+ SolrQueryResponse rsp = new SolrQueryResponse();
+ rsp.addResponse(docs);
+ String output;
+ if (responseWriter instanceof BinaryQueryResponseWriter) {
+ ByteArrayOutputStream baos = new ByteArrayOutputStream();
+ ((BinaryQueryResponseWriter) responseWriter).write(baos, req, rsp);
+ baos.close();
+ output = baos.toString(StandardCharsets.UTF_8);
+ } else {
+ StringWriter writer = new StringWriter();
+ responseWriter.write(writer, req, rsp);
+ writer.close();
+ output = writer.toString();
+ }
+ assertTrue("found: " + output, output.contains(expectMsg));
+ validateWrite(docs, expectMsg);
+ }
+
+ private static void validateWrite(SolrDocumentList docs, String expectMsg) throws IOException {
+ CharArr arr = new CharArr();
+ org.noggit.JSONWriter w = new org.noggit.JSONWriter(arr, 2);
+ docs.writeMap(
+ new MapWriter.EntryWriter() {
+ boolean first = true;
+
+ @Override
+ public MapWriter.EntryWriter put(CharSequence k, Object v) {
+ if (first) {
+ first = false;
+ } else {
+ w.writeValueSeparator();
+ }
+ w.indent();
+ w.writeString(k.toString());
+ w.writeNameSeparator();
+ w.write(v);
+ return this;
+ }
+ });
+ String output = arr.toString();
+ assertTrue("found: " + output, output.contains(expectMsg));
+ }
}
