SOLR-16825: Fix response serialization bug

solr/api/src/java/org/apache/solr/client/api/model/AddReplicaPropertyRequestBody.java

@@ -19,9 +19,8 @@
 
 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) {

solr/solrj/src/java/org/apache/solr/common/util/JavaBinCodec.java

@@ -44,7 +44,6 @@
 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 void writeVal(Object val) throws IOException {
         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.getReflectWritable(val));
   }
 
   protected static final Object END_OBJ = new Object();
@@ -394,7 +394,7 @@ public boolean writeKnownType(Object val) throws IOException {
       return true;
     }
     if (val instanceof ReflectWritable) {
-      writeMap(new DelegateMapWriter(val));
+      writeVal(Utils.getReflectWritable(val));
       return true;
     }
     if (val instanceof Map) {

solr/solrj/src/java/org/apache/solr/common/util/TextWriter.java

@@ -35,7 +35,6 @@
 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 @@ default void writeVal(String name, Object val, boolean raw) throws IOException {
     } else if (val instanceof MapWriter) {
       writeMap(name, (MapWriter) val);
     } else if (val instanceof ReflectWritable) {
-      writeMap(name, new DelegateMapWriter(val));
+      writeVal(name, Utils.getReflectWritable(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 @@ default void writeVal(String name, Object val, boolean raw) throws IOException {
         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.getReflectWritable(val));
     }
   }
 

solr/solrj/src/java/org/apache/solr/common/util/Utils.java

@@ -20,6 +20,7 @@
 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,35 @@ public static void reflectWrite(MapWriter.EntryWriter ew, Object o) {
 
   public static final String CATCH_ALL_PROPERTIES_METHOD_NAME = "unknownProperties";
 
+  public static Object getReflectWritable(Object o) throws IOException {
+    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 +881,7 @@ public static void reflectWrite(
     } 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 +910,26 @@ private static List<FieldWriter> getReflectData(
     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 +1053,21 @@ private MapWriter.EntryWriter writeEntry(CharSequence k, Object v) {
   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;
   }