You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@solr.apache.org by ho...@apache.org on 2023/08/29 15:23:18 UTC
[solr] branch main updated: SOLR-16825: Fix response serialization bug (#1867)
This is an automated email from the ASF dual-hosted git repository.
houston pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/solr.git
The following commit(s) were added to refs/heads/main by this push:
new 9a453854a56 SOLR-16825: Fix response serialization bug (#1867)
9a453854a56 is described below
commit 9a453854a56125fef6740ce8e4a1f69fb444eabe
Author: Jason Gerlowski <ge...@apache.org>
AuthorDate: Tue Aug 29 11:23:10 2023 -0400
SOLR-16825: Fix response serialization bug (#1867)
Unknown types will try to serialize using Reflection,
but will use a "toString()" serialization if no JSONProperty annotations
are found. This is backwards compatible.
Co-authored-by: Houston Putman <ho...@apache.org>
---
.../api/model/AddReplicaPropertyRequestBody.java | 3 +-
.../org/apache/solr/common/DelegateMapWriter.java | 46 ------------
.../org/apache/solr/common/util/JavaBinCodec.java | 8 +--
.../org/apache/solr/common/util/TextWriter.java | 8 +--
.../java/org/apache/solr/common/util/Utils.java | 84 +++++++++++++++++++---
5 files changed, 85 insertions(+), 64 deletions(-)
diff --git a/solr/api/src/java/org/apache/solr/client/api/model/AddReplicaPropertyRequestBody.java b/solr/api/src/java/org/apache/solr/client/api/model/AddReplicaPropertyRequestBody.java
index f78c7bebd9c..603524e4ad1 100644
--- a/solr/api/src/java/org/apache/solr/client/api/model/AddReplicaPropertyRequestBody.java
+++ b/solr/api/src/java/org/apache/solr/client/api/model/AddReplicaPropertyRequestBody.java
@@ -19,9 +19,8 @@ package org.apache.solr.client.api.model;
import com.fasterxml.jackson.annotation.JsonProperty;
import io.swagger.v3.oas.annotations.media.Schema;
-import org.apache.solr.client.api.util.ReflectWritable;
-public class AddReplicaPropertyRequestBody implements ReflectWritable {
+public class AddReplicaPropertyRequestBody {
public AddReplicaPropertyRequestBody() {}
public AddReplicaPropertyRequestBody(String value) {
diff --git a/solr/solrj/src/java/org/apache/solr/common/DelegateMapWriter.java b/solr/solrj/src/java/org/apache/solr/common/DelegateMapWriter.java
deleted file mode 100644
index 9fe01b09b14..00000000000
--- a/solr/solrj/src/java/org/apache/solr/common/DelegateMapWriter.java
+++ /dev/null
@@ -1,46 +0,0 @@
-/*
- * 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.common;
-
-import com.fasterxml.jackson.annotation.JsonAnyGetter;
-import com.fasterxml.jackson.annotation.JsonProperty;
-import java.io.IOException;
-import org.apache.solr.common.util.Utils;
-
-public class DelegateMapWriter implements MapWriter {
-
- private final Object delegate;
-
- public DelegateMapWriter(Object delegate) {
- this.delegate = delegate;
- }
-
- @Override
- public void writeMap(EntryWriter ew) throws IOException {
- Utils.reflectWrite(
- ew,
- delegate,
- // TODO Should we be lenient here and accept both the Jackson and our homegrown annotation?
- field -> field.getAnnotation(JsonProperty.class) != null,
- JsonAnyGetter.class,
- field -> {
- final JsonProperty prop = field.getAnnotation(JsonProperty.class);
- return prop.value().isEmpty() ? field.getName() : prop.value();
- });
- }
-}
diff --git a/solr/solrj/src/java/org/apache/solr/common/util/JavaBinCodec.java b/solr/solrj/src/java/org/apache/solr/common/util/JavaBinCodec.java
index 488aa8e8427..341edfb864f 100644
--- a/solr/solrj/src/java/org/apache/solr/common/util/JavaBinCodec.java
+++ b/solr/solrj/src/java/org/apache/solr/common/util/JavaBinCodec.java
@@ -44,7 +44,6 @@ import java.util.function.Function;
import java.util.function.Predicate;
import org.apache.solr.client.api.util.ReflectWritable;
import org.apache.solr.common.ConditionalKeyMapWriter;
-import org.apache.solr.common.DelegateMapWriter;
import org.apache.solr.common.EnumFieldValue;
import org.apache.solr.common.IteratorWriter;
import org.apache.solr.common.IteratorWriter.ItemWriter;
@@ -270,10 +269,11 @@ public class JavaBinCodec implements PushWriter {
if (writeKnownType(tmpVal)) return;
}
}
- // Fallback to do *something*.
+ // Fallback to do *something*, either use a reflection writer or write as a string
+ // representation.
// note: if the user of this codec doesn't want this (e.g. UpdateLog) it can supply an
// ObjectResolver that does something else like throw an exception.
- writeVal(val.getClass().getName() + ':' + val.toString());
+ writeVal(Utils.getReflectWriter(val));
}
protected static final Object END_OBJ = new Object();
@@ -394,7 +394,7 @@ public class JavaBinCodec implements PushWriter {
return true;
}
if (val instanceof ReflectWritable) {
- writeMap(new DelegateMapWriter(val));
+ writeVal(Utils.getReflectWriter(val));
return true;
}
if (val instanceof Map) {
diff --git a/solr/solrj/src/java/org/apache/solr/common/util/TextWriter.java b/solr/solrj/src/java/org/apache/solr/common/util/TextWriter.java
index c42240aeed5..7c64954b31f 100644
--- a/solr/solrj/src/java/org/apache/solr/common/util/TextWriter.java
+++ b/solr/solrj/src/java/org/apache/solr/common/util/TextWriter.java
@@ -35,7 +35,6 @@ import java.util.concurrent.atomic.AtomicLong;
import java.util.concurrent.atomic.LongAccumulator;
import java.util.concurrent.atomic.LongAdder;
import org.apache.solr.client.api.util.ReflectWritable;
-import org.apache.solr.common.DelegateMapWriter;
import org.apache.solr.common.EnumFieldValue;
import org.apache.solr.common.IteratorWriter;
import org.apache.solr.common.MapSerializable;
@@ -88,7 +87,7 @@ public interface TextWriter extends PushWriter {
} else if (val instanceof MapWriter) {
writeMap(name, (MapWriter) val);
} else if (val instanceof ReflectWritable) {
- writeMap(name, new DelegateMapWriter(val));
+ writeVal(name, Utils.getReflectWriter(val));
} else if (val instanceof MapSerializable) {
// todo find a better way to reuse the map more efficiently
writeMap(name, ((MapSerializable) val).toMap(new LinkedHashMap<>()), false, true);
@@ -110,8 +109,9 @@ public interface TextWriter extends PushWriter {
writeStr(name, val.toString(), true);
}
} else {
- // default... for debugging only. Would be nice to "assert false" ?
- writeStr(name, val.getClass().getName() + ':' + val.toString(), true);
+ // Fallback to do *something*, either use a reflection writer or write as a string
+ // representation.
+ writeVal(name, Utils.getReflectWriter(val));
}
}
diff --git a/solr/solrj/src/java/org/apache/solr/common/util/Utils.java b/solr/solrj/src/java/org/apache/solr/common/util/Utils.java
index 31d6e736a2e..0ae4886d670 100644
--- a/solr/solrj/src/java/org/apache/solr/common/util/Utils.java
+++ b/solr/solrj/src/java/org/apache/solr/common/util/Utils.java
@@ -20,6 +20,7 @@ import static java.nio.charset.StandardCharsets.UTF_8;
import static java.util.Collections.singletonList;
import static java.util.concurrent.TimeUnit.NANOSECONDS;
+import com.fasterxml.jackson.annotation.JsonAnyGetter;
import java.io.ByteArrayInputStream;
import java.io.ByteArrayOutputStream;
import java.io.IOException;
@@ -827,6 +828,45 @@ public class Utils {
public static final String CATCH_ALL_PROPERTIES_METHOD_NAME = "unknownProperties";
+ /**
+ * Return a writable object that will be serialized using the reflection-friendly properties of
+ * the class, notably the fields that have the {@link JsonProperty} annotation.
+ *
+ * <p>If the class has no reflection-friendly fields, then it will be serialized as a string,
+ * using the class's name and {@code toString()} method.
+ *
+ * @param o the object to get a serializable version of
+ * @return a serializable version of the object
+ */
+ public static Object getReflectWriter(Object o) {
+ List<FieldWriter> fieldWriters = null;
+ try {
+ fieldWriters =
+ Utils.getReflectData(
+ o.getClass(),
+ // TODO Should we be lenient here and accept both the Jackson and our homegrown
+ // annotation?
+ field ->
+ field.getAnnotation(com.fasterxml.jackson.annotation.JsonProperty.class) != null,
+ JsonAnyGetter.class,
+ field -> {
+ final com.fasterxml.jackson.annotation.JsonProperty prop =
+ field.getAnnotation(com.fasterxml.jackson.annotation.JsonProperty.class);
+ return prop.value().isEmpty() ? field.getName() : prop.value();
+ });
+ } catch (IllegalAccessException e) {
+ throw new RuntimeException(e);
+ }
+ if (!fieldWriters.isEmpty()) {
+ return new DelegateReflectWriter(o, fieldWriters);
+ } else {
+ // Do not serialize an empty class, use a string representation instead.
+ // This is because the class is likely not using the serialization annotations
+ // and still expects to provide some information.
+ return o.getClass().getName() + ':' + o;
+ }
+ }
+
/**
* Convert an input object to a map, writing only those fields that match a provided {@link
* Predicate}
@@ -851,14 +891,7 @@ public class Utils {
} catch (IllegalAccessException e) {
throw new RuntimeException(e);
}
- for (FieldWriter fieldWriter : fieldWriters) {
- try {
- fieldWriter.write(ew, o);
- } catch (Throwable e) {
- throw new RuntimeException(e);
- // should not happen
- }
- }
+ reflectWrite(ew, o, fieldWriters);
}
private static List<FieldWriter> getReflectData(
@@ -887,6 +920,26 @@ public class Utils {
return Collections.unmodifiableList(mutableFieldWriters);
}
+ /**
+ * Convert an input object to a map, using the provided {@link FieldWriter}s.
+ *
+ * @param ew an {@link org.apache.solr.common.MapWriter.EntryWriter} to do the actual map
+ * insertion/writing
+ * @param o the object to be converted
+ * @param fieldWriters a list of fields to write and how to write them
+ */
+ private static void reflectWrite(
+ MapWriter.EntryWriter ew, Object o, List<FieldWriter> fieldWriters) {
+ for (FieldWriter fieldWriter : fieldWriters) {
+ try {
+ fieldWriter.write(ew, o);
+ } catch (Throwable e) {
+ throw new RuntimeException(e);
+ // should not happen
+ }
+ }
+ }
+
private static List<FieldWriter> addTraditionalFieldWriters(
Class<?> c,
MethodHandles.Lookup lookup,
@@ -1010,6 +1063,21 @@ public class Utils {
private static final Map<Class<?>, List<FieldWriter>> storedReflectData =
new ConcurrentHashMap<>();
+ public static class DelegateReflectWriter implements MapWriter {
+ private final Object object;
+ private final List<Utils.FieldWriter> fieldWriters;
+
+ DelegateReflectWriter(Object object, List<Utils.FieldWriter> fieldWriters) {
+ this.object = object;
+ this.fieldWriters = fieldWriters;
+ }
+
+ @Override
+ public void writeMap(EntryWriter ew) throws IOException {
+ Utils.reflectWrite(ew, object, fieldWriters);
+ }
+ }
+
interface FieldWriter {
void write(MapWriter.EntryWriter ew, Object inst) throws Throwable;
}