Merge pull request #2220, leave jdk standard classes to kryo, use the…

… default registered serializer by kryo. Fixes #2178, support java8 time types.
apache · Aug 13, 2018 · 8beeb64 · 8beeb64
1 parent e350840
commit 8beeb64
Show file tree

Hide file tree

Showing 7 changed files with 56 additions and 25 deletions.
diff --git a/...pi/src/main/java/org/apache/dubbo/common/serialize/support/SerializableClassRegistry.java b/...pi/src/main/java/org/apache/dubbo/common/serialize/support/SerializableClassRegistry.java
@@ -16,21 +16,34 @@
  */
 package org.apache.dubbo.common.serialize.support;
 
-import java.util.LinkedHashSet;
-import java.util.Set;
+import com.esotericsoftware.kryo.Serializer;
+
+import java.util.LinkedHashMap;
+import java.util.Map;
 
 public abstract class SerializableClassRegistry {
 
-    private static final Set<Class> registrations = new LinkedHashSet<Class>();
+
+    private static final Map<Class, Object> registrations = new LinkedHashMap<>();
 
     /**
      * only supposed to be called at startup time
      */
     public static void registerClass(Class clazz) {
-        registrations.add(clazz);
+        registerClass(clazz, null);
+    }
+
+    /**
+     * only supposed to be called at startup time
+     */
+    public static void registerClass(Class clazz, Serializer serializer) {
+        if (clazz == null) {
+            throw new IllegalArgumentException("Class registered to kryo cannot be null!");
+        }
+        registrations.put(clazz, serializer);
     }
 
-    public static Set<Class> getRegisteredClasses() {
+    public static Map<Class, Object> getRegisteredClasses() {
         return registrations;
     }
 }
diff --git a/...rc/test/java/org/apache/dubbo/common/serialize/support/SerializableClassRegistryTest.java b/...rc/test/java/org/apache/dubbo/common/serialize/support/SerializableClassRegistryTest.java
@@ -18,9 +18,9 @@
 
 import org.junit.Test;
 
-import java.util.Set;
+import java.util.Map;
 
-import static org.hamcrest.Matchers.hasSize;
+import static org.hamcrest.Matchers.equalTo;
 import static org.junit.Assert.assertThat;
 
 public class SerializableClassRegistryTest {
@@ -29,8 +29,8 @@ public void testAddClasses() {
         SerializableClassRegistry.registerClass(A.class);
         SerializableClassRegistry.registerClass(B.class);
 
-        Set<Class> registeredClasses = SerializableClassRegistry.getRegisteredClasses();
-        assertThat(registeredClasses, hasSize(2));
+        Map<Class, Object> registeredClasses = SerializableClassRegistry.getRegisteredClasses();
+        assertThat(registeredClasses.size(), equalTo(2));
     }
 
     private class A {

diff --git a/...bbo-serialization-fst/src/main/java/org/apache/dubbo/common/serialize/fst/FstFactory.java b/...bbo-serialization-fst/src/main/java/org/apache/dubbo/common/serialize/fst/FstFactory.java
@@ -17,7 +17,6 @@
 package org.apache.dubbo.common.serialize.fst;
 
 import org.apache.dubbo.common.serialize.support.SerializableClassRegistry;
-
 import org.nustaq.serialization.FSTConfiguration;
 import org.nustaq.serialization.FSTObjectInput;
 import org.nustaq.serialization.FSTObjectOutput;
@@ -37,9 +36,7 @@ public static FstFactory getDefaultFactory() {
     }
 
     public FstFactory() {
-        for (Class clazz : SerializableClassRegistry.getRegisteredClasses()) {
-            conf.registerClass(clazz);
-        }
+        SerializableClassRegistry.getRegisteredClasses().keySet().forEach(conf::registerClass);
     }
 
     public FSTObjectOutput getObjectOutput(OutputStream outputStream) {

diff --git a/...rialization-kryo/src/main/java/org/apache/dubbo/common/serialize/kryo/CompatibleKryo.java b/...rialization-kryo/src/main/java/org/apache/dubbo/common/serialize/kryo/CompatibleKryo.java
@@ -16,13 +16,12 @@
  */
 package org.apache.dubbo.common.serialize.kryo;
 
-import org.apache.dubbo.common.logger.Logger;
-import org.apache.dubbo.common.logger.LoggerFactory;
-import org.apache.dubbo.common.serialize.kryo.utils.ReflectionUtils;
-
 import com.esotericsoftware.kryo.Kryo;
 import com.esotericsoftware.kryo.Serializer;
 import com.esotericsoftware.kryo.serializers.JavaSerializer;
+import org.apache.dubbo.common.logger.Logger;
+import org.apache.dubbo.common.logger.LoggerFactory;
+import org.apache.dubbo.common.serialize.kryo.utils.ReflectionUtils;
 
 public class CompatibleKryo extends Kryo {
 
@@ -34,7 +33,16 @@ public Serializer getDefaultSerializer(Class type) {
             throw new IllegalArgumentException("type cannot be null.");
         }
 
-        if (!type.isArray() && !type.isEnum() && !ReflectionUtils.checkZeroArgConstructor(type)) {
+        /**
+         * Kryo requires every class to provide a zero argument constructor. For any class does not match this condition, kryo have two ways:
+         * 1. Use JavaSerializer,
+         * 2. Set 'kryo.setInstantiatorStrategy(new DefaultInstantiatorStrategy(new StdInstantiatorStrategy()));', StdInstantiatorStrategy can generate an instance bypassing the constructor.
+         *
+         * In practice, it's not possible for Dubbo users to register kryo Serializer for every customized class. So in most cases, customized classes with/without zero argument constructor will
+         * default to the default serializer.
+         * It is the responsibility of kryo to handle with every standard jdk classes, so we will just escape these classes.
+         */
+        if (!ReflectionUtils.isJdk(type) && !type.isArray() && !type.isEnum() && !ReflectionUtils.checkZeroArgConstructor(type)) {
             if (logger.isWarnEnabled()) {
                 logger.warn(type + " has no zero-arg constructor and this will affect the serialization performance");
             }

diff --git a/...-kryo/src/main/java/org/apache/dubbo/common/serialize/kryo/utils/AbstractKryoFactory.java b/...-kryo/src/main/java/org/apache/dubbo/common/serialize/kryo/utils/AbstractKryoFactory.java
@@ -16,10 +16,8 @@
  */
 package org.apache.dubbo.common.serialize.kryo.utils;
 
-import org.apache.dubbo.common.serialize.kryo.CompatibleKryo;
-import org.apache.dubbo.common.serialize.support.SerializableClassRegistry;
-
 import com.esotericsoftware.kryo.Kryo;
+import com.esotericsoftware.kryo.Serializer;
 import com.esotericsoftware.kryo.pool.KryoFactory;
 import com.esotericsoftware.kryo.serializers.DefaultSerializers;
 import de.javakaffee.kryoserializers.ArraysAsListSerializer;
@@ -31,6 +29,8 @@
 import de.javakaffee.kryoserializers.URISerializer;
 import de.javakaffee.kryoserializers.UUIDSerializer;
 import de.javakaffee.kryoserializers.UnmodifiableCollectionsSerializer;
+import org.apache.dubbo.common.serialize.kryo.CompatibleKryo;
+import org.apache.dubbo.common.serialize.support.SerializableClassRegistry;
 
 import java.lang.reflect.InvocationHandler;
 import java.math.BigDecimal;
@@ -134,9 +134,13 @@ public Kryo create() {
             kryo.register(clazz);
         }
 
-        for (Class clazz : SerializableClassRegistry.getRegisteredClasses()) {
-            kryo.register(clazz);
-        }
+        SerializableClassRegistry.getRegisteredClasses().forEach((clazz, ser) -> {
+            if (ser == null) {
+                kryo.register(clazz);
+            } else {
+                kryo.register(clazz, (Serializer) ser);
+            }
+        });
 
         return kryo;
     }

diff --git a/...tion-kryo/src/main/java/org/apache/dubbo/common/serialize/kryo/utils/ReflectionUtils.java b/...tion-kryo/src/main/java/org/apache/dubbo/common/serialize/kryo/utils/ReflectionUtils.java
@@ -26,4 +26,8 @@ public static boolean checkZeroArgConstructor(Class clazz) {
             return false;
         }
     }
+
+    public static boolean isJdk(Class clazz) {
+        return clazz.getName().startsWith("java.") || clazz.getName().startsWith("javax.");
+    }
 }
diff --git a/.../test/java/org/apache/dubbo/common/serialize/serialization/AbstractSerializationTest.java b/.../test/java/org/apache/dubbo/common/serialize/serialization/AbstractSerializationTest.java
@@ -34,7 +34,6 @@
 import org.apache.dubbo.common.serialize.ObjectInput;
 import org.apache.dubbo.common.serialize.ObjectOutput;
 import org.apache.dubbo.common.serialize.Serialization;
-
 import org.junit.Test;
 
 import java.io.ByteArrayInputStream;
@@ -43,6 +42,7 @@
 import java.math.BigDecimal;
 import java.math.BigInteger;
 import java.sql.Time;
+import java.time.LocalDateTime;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Date;
@@ -858,6 +858,11 @@ public void test_BizExceptionNoDefaultConstructor_WithType() throws Exception {
         assertEquals("Hello", ((BizExceptionNoDefaultConstructor) read).getMessage());
     }
 
+    @Test
+    public void test_LocalDateTime() throws Exception {
+        assertObject(LocalDateTime.now());
+    }
+
     @Test
     public void test_enum() throws Exception {
         assertObject(AnimalEnum.dog);