[Java] support writeReplace/readResolve for Externalizable (#1048)

support writeReplace/readResolve for Externalizable
apache · Oct 31, 2023 · cb332d0 · cb332d0
1 parent 6b63b2d
commit cb332d0
Show file tree

Hide file tree

Showing 6 changed files with 171 additions and 23 deletions.
diff --git a/java/fury-core/src/main/java/io/fury/resolver/ClassResolver.java b/java/fury-core/src/main/java/io/fury/resolver/ClassResolver.java
@@ -840,8 +840,6 @@ public Class<? extends Serializer> getSerializerClass(Class<?> cls, boolean code
         return TimeSerializers.ZoneIdSerializer.class;
       } else if (TimeZone.class.isAssignableFrom(cls)) {
         return TimeSerializers.TimeZoneSerializer.class;
-      } else if (Externalizable.class.isAssignableFrom(cls)) {
-        return ExternalizableSerializer.class;
       } else if (ByteBuffer.class.isAssignableFrom(cls)) {
         return BufferSerializers.ByteBufferSerializer.class;
       }
@@ -889,6 +887,9 @@ public Class<? extends Serializer> getSerializerClass(Class<?> cls, boolean code
       if (useReplaceResolveSerializer(cls)) {
         return ReplaceResolveSerializer.class;
       }
+      if (Externalizable.class.isAssignableFrom(cls)) {
+        return ExternalizableSerializer.class;
+      }
       if (requireJavaSerialization(cls)) {
         return getJavaSerializer(cls);
       }

diff --git a/java/fury-core/src/main/java/io/fury/serializer/JavaSerializer.java b/java/fury-core/src/main/java/io/fury/serializer/JavaSerializer.java
@@ -101,8 +101,16 @@ public Object read(MemoryBuffer buffer) {
     throw new IllegalStateException("unreachable code");
   }
 
+  private static final ClassValue<Method> writeObjectMethodCache =
+      new ClassValue<Method>() {
+        @Override
+        protected Method computeValue(Class<?> type) {
+          return getWriteObjectMethod(type, true);
+        }
+      };
+
   public static Method getWriteObjectMethod(Class<?> clz) {
-    return getWriteObjectMethod(clz, true);
+    return writeObjectMethodCache.get(clz);
   }
 
   public static Method getWriteObjectMethod(Class<?> clz, boolean searchParent) {
@@ -122,8 +130,16 @@ public static boolean isWriteObjectMethod(Method method) {
         && Modifier.isPrivate(method.getModifiers());
   }
 
+  private static final ClassValue<Method> readObjectMethodCache =
+      new ClassValue<Method>() {
+        @Override
+        protected Method computeValue(Class<?> type) {
+          return getReadObjectMethod(type, true);
+        }
+      };
+
   public static Method getReadObjectMethod(Class<?> clz) {
-    return getReadObjectMethod(clz, true);
+    return readObjectMethodCache.get(clz);
   }
 
   public static Method getReadObjectMethod(Class<?> clz, boolean searchParent) {
@@ -152,26 +168,48 @@ public static Method getReadObjectNoData(Class<?> clz, boolean searchParent) {
     return null;
   }
 
+  private static final ClassValue<Method> readResolveCache =
+      new ClassValue<Method>() {
+        @Override
+        protected Method computeValue(Class<?> type) {
+          Method readResolve = getMethod(type, "readResolve", true);
+          if (readResolve != null) {
+            if (readResolve.getParameterTypes().length == 0
+                && readResolve.getReturnType() == Object.class) {
+              return readResolve;
+            } else {
+              LOG.warn(
+                  "`readResolve` method doesn't match signature: `ANY-ACCESS-MODIFIER Object readResolve()`");
+            }
+          }
+          return null;
+        }
+      };
+
   public static Method getReadResolveMethod(Class<?> clz) {
-    Method readResolve = getMethod(clz, "readResolve", true);
-    if (readResolve != null) {
-      if (readResolve.getParameterTypes().length == 0
-          && readResolve.getReturnType() == Object.class) {
-        return readResolve;
-      }
-    }
-    return null;
+    return readResolveCache.get(clz);
   }
 
+  private static final ClassValue<Method> writeReplaceCache =
+      new ClassValue<Method>() {
+        @Override
+        protected Method computeValue(Class<?> type) {
+          Method writeReplace = getMethod(type, "writeReplace", true);
+          if (writeReplace != null) {
+            if (writeReplace.getParameterTypes().length == 0
+                && writeReplace.getReturnType() == Object.class) {
+              return writeReplace;
+            } else {
+              LOG.warn(
+                  "`writeReplace` method doesn't match signature: `ANY-ACCESS-MODIFIER Object writeReplace()");
+            }
+          }
+          return null;
+        }
+      };
+
   public static Method getWriteReplaceMethod(Class<?> clz) {
-    Method writeReplace = getMethod(clz, "writeReplace", true);
-    if (writeReplace != null) {
-      if (writeReplace.getParameterTypes().length == 0
-          && writeReplace.getReturnType() == Object.class) {
-        return writeReplace;
-      }
-    }
-    return null;
+    return writeReplaceCache.get(clz);
   }
 
   private static Method getMethod(Class<?> clz, String methodName, boolean searchParent) {

diff --git a/java/fury-core/src/main/java/io/fury/serializer/ReplaceResolveSerializer.java b/java/fury-core/src/main/java/io/fury/serializer/ReplaceResolveSerializer.java
@@ -26,6 +26,7 @@
 import io.fury.util.Platform;
 import io.fury.util.ReflectionUtils;
 import io.fury.util.unsafe._JDKAccess;
+import java.io.Externalizable;
 import java.io.ObjectStreamClass;
 import java.io.Serializable;
 import java.lang.invoke.MethodHandles;
@@ -148,7 +149,9 @@ static JDKReplaceResolveMethodInfoCache newJDKMethodInfoCache(Class<?> cls, Fury
     JDKReplaceResolveMethodInfoCache methodInfoCache =
         new JDKReplaceResolveMethodInfoCache(writeReplaceMethod, readResolveMethod, null);
     Class<? extends Serializer> serializerClass;
-    if (JavaSerializer.getReadObjectMethod(cls, true) == null
+    if (Externalizable.class.isAssignableFrom(cls)) {
+      serializerClass = ExternalizableSerializer.class;
+    } else if (JavaSerializer.getReadObjectMethod(cls, true) == null
         && JavaSerializer.getWriteObjectMethod(cls, true) == null) {
       serializerClass =
           fury.getClassResolver()

diff --git a/java/fury-core/src/test/java/io/fury/FuryTestBase.java b/java/fury-core/src/test/java/io/fury/FuryTestBase.java
@@ -173,12 +173,12 @@ public static void serDeCheckSerializerAndEqual(Fury fury, Object obj, String cl
     Assert.assertEquals(serDeCheckSerializer(fury, obj, classRegex), obj);
   }
 
-  public static Object serDeCheckSerializer(Fury fury, Object obj, String classRegex) {
+  public static <T> T serDeCheckSerializer(Fury fury, Object obj, String classRegex) {
     byte[] bytes = fury.serialize(obj);
     String serializerName = fury.getClassResolver().getSerializerClass(obj.getClass()).getName();
     Matcher matcher = Pattern.compile(classRegex).matcher(serializerName);
     Assert.assertTrue(matcher.find());
-    return fury.deserialize(bytes);
+    return (T) fury.deserialize(bytes);
   }
 
   public static Object serDe(Fury fury1, Fury fury2, Object obj) {

diff --git a/java/fury-core/src/test/java/io/fury/TestUtils.java b/java/fury-core/src/test/java/io/fury/TestUtils.java
@@ -20,6 +20,9 @@
 import com.google.common.collect.ImmutableMap;
 import io.fury.util.FieldAccessor;
 import io.fury.util.ReflectionUtils;
+import java.io.ByteArrayOutputStream;
+import java.io.IOException;
+import java.io.ObjectOutputStream;
 import java.util.ArrayList;
 import java.util.function.Supplier;
 
@@ -73,4 +76,20 @@ private static void triggerOOM() {
       }
     }
   }
+
+  public static byte[] jdkSerialize(Object data) {
+    ByteArrayOutputStream bas = new ByteArrayOutputStream();
+    jdkSerialize(bas, data);
+    return bas.toByteArray();
+  }
+
+  public static void jdkSerialize(ByteArrayOutputStream bas, Object data) {
+    bas.reset();
+    try (ObjectOutputStream objectOutputStream = new ObjectOutputStream(bas)) {
+      objectOutputStream.writeObject(data);
+      objectOutputStream.flush();
+    } catch (IOException e) {
+      throw new RuntimeException(e);
+    }
+  }
 }
diff --git a/java/fury-core/src/test/java/io/fury/serializer/ReplaceResolveSerializerTest.java b/java/fury-core/src/test/java/io/fury/serializer/ReplaceResolveSerializerTest.java
@@ -27,8 +27,11 @@
 import io.fury.Fury;
 import io.fury.FuryTestBase;
 import io.fury.config.Language;
+import java.io.Externalizable;
 import java.io.IOException;
+import java.io.ObjectInput;
 import java.io.ObjectInputStream;
+import java.io.ObjectOutput;
 import java.io.ObjectOutputStream;
 import java.io.Serializable;
 import java.util.List;
@@ -484,4 +487,88 @@ public void testInheritance() {
     InheritanceTestClass o = (InheritanceTestClass) fury.deserialize(bytes);
     assertEquals(o.f1, 10);
   }
+
+  static class WriteReplaceExternalizable implements Externalizable {
+    private transient int f1;
+
+    public WriteReplaceExternalizable(int f1) {
+      this.f1 = f1;
+    }
+
+    @Override
+    public void writeExternal(ObjectOutput out) throws IOException {
+      throw new RuntimeException();
+    }
+
+    @Override
+    public void readExternal(ObjectInput in) throws IOException, ClassNotFoundException {
+      throw new RuntimeException();
+    }
+
+    private Object writeReplace() {
+      return new ReplaceExternalizableProxy(f1);
+    }
+  }
+
+  static class ReplaceExternalizableProxy implements Serializable {
+    private int f1;
+
+    public ReplaceExternalizableProxy(int f1) {
+      this.f1 = f1;
+    }
+
+    private Object readResolve() {
+      return new WriteReplaceExternalizable(f1);
+    }
+  }
+
+  @Test
+  public void testWriteReplaceExternalizable() {
+    WriteReplaceExternalizable o =
+        serDeCheckSerializer(
+            getJavaFury(),
+            new WriteReplaceExternalizable(10),
+            ReplaceResolveSerializer.class.getName());
+    assertEquals(o.f1, 10);
+  }
+
+  static class ReplaceSelfExternalizable implements Externalizable {
+    private transient int f1;
+    private transient boolean newInstance;
+
+    public ReplaceSelfExternalizable(int f1, boolean newInstance) {
+      this.f1 = f1;
+      this.newInstance = newInstance;
+    }
+
+    @Override
+    public void writeExternal(ObjectOutput out) throws IOException {
+      out.writeInt(f1);
+    }
+
+    @Override
+    public void readExternal(ObjectInput in) throws IOException, ClassNotFoundException {
+      f1 = in.readInt();
+    }
+
+    private Object writeReplace() {
+      return newInstance ? new ReplaceSelfExternalizable(f1, false) : this;
+    }
+  }
+
+  @Test
+  public void testWriteReplaceSelfExternalizable() {
+    ReplaceSelfExternalizable o =
+        serDeCheckSerializer(
+            getJavaFury(),
+            new ReplaceSelfExternalizable(10, false),
+            ReplaceResolveSerializer.class.getName());
+    assertEquals(o.f1, 10);
+    ReplaceSelfExternalizable o1 =
+        serDeCheckSerializer(
+            getJavaFury(),
+            new ReplaceSelfExternalizable(10, true),
+            ReplaceResolveSerializer.class.getName());
+    assertEquals(o1.f1, 10);
+  }
 }