apply #2220 to 2.6.x branch, issue #2178 (#3519)

* Apply #2925 to branch 2.6.x * apply #2220 to 2.6.x branch, issue #2178 * Revise the naming conventions (#3506) * apply #2220 to 2.6.x branch, issue #2178 * remove java.time package
apache · Feb 21, 2019 · badea86 · badea86
1 parent 40b0ff2
commit badea86
Show file tree

Hide file tree

Showing 7 changed files with 52 additions and 20 deletions.
diff --git a/...i/src/main/java/com/alibaba/dubbo/common/serialize/support/SerializableClassRegistry.java b/...i/src/main/java/com/alibaba/dubbo/common/serialize/support/SerializableClassRegistry.java
@@ -16,21 +16,33 @@
  */
 package com.alibaba.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<Class, Object>();
 
     /**
      * 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/...c/test/java/com/alibaba/dubbo/common/serialize/support/SerializableClassRegistryTest.java b/...c/test/java/com/alibaba/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,13 +29,13 @@ 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 {
     }
 
     private class B {
     }
-}
+}
diff --git a/...bo-serialization-fst/src/main/java/com/alibaba/dubbo/common/serialize/fst/FstFactory.java b/...bo-serialization-fst/src/main/java/com/alibaba/dubbo/common/serialize/fst/FstFactory.java
@@ -37,7 +37,7 @@ public static FstFactory getDefaultFactory() {
     }
 
     public FstFactory() {
-        for (Class clazz : SerializableClassRegistry.getRegisteredClasses()) {
+        for (Class clazz : SerializableClassRegistry.getRegisteredClasses().keySet()) {
             conf.registerClass(clazz);
         }
     }

diff --git a/...ialization-kryo/src/main/java/com/alibaba/dubbo/common/serialize/kryo/CompatibleKryo.java b/...ialization-kryo/src/main/java/com/alibaba/dubbo/common/serialize/kryo/CompatibleKryo.java
@@ -34,12 +34,22 @@ 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");
             }
             return new JavaSerializer();
         }
+
         return super.getDefaultSerializer(type);
     }
 }
diff --git a/...kryo/src/main/java/com/alibaba/dubbo/common/serialize/kryo/utils/AbstractKryoFactory.java b/...kryo/src/main/java/com/alibaba/dubbo/common/serialize/kryo/utils/AbstractKryoFactory.java
@@ -20,6 +20,7 @@
 import com.alibaba.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;
@@ -48,6 +49,7 @@
 import java.util.Hashtable;
 import java.util.LinkedHashSet;
 import java.util.LinkedList;
+import java.util.Map;
 import java.util.Set;
 import java.util.TreeSet;
 import java.util.UUID;
@@ -134,8 +136,12 @@ public Kryo create() {
             kryo.register(clazz);
         }
 
-        for (Class clazz : SerializableClassRegistry.getRegisteredClasses()) {
-            kryo.register(clazz);
+        for (Map.Entry<Class, Object> entry : SerializableClassRegistry.getRegisteredClasses().entrySet()) {
+            if (entry.getValue() == null) {
+                kryo.register(entry.getKey());
+            } else {
+                kryo.register(entry.getKey(), (Serializer) entry.getValue());
+            }
         }
 
         return kryo;

diff --git a/...ion-kryo/src/main/java/com/alibaba/dubbo/common/serialize/kryo/utils/ReflectionUtils.java b/...ion-kryo/src/main/java/com/alibaba/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/com/alibaba/dubbo/common/serialize/serialization/AbstractSerializationTest.java b/...test/java/com/alibaba/dubbo/common/serialize/serialization/AbstractSerializationTest.java
@@ -67,7 +67,7 @@ public abstract class AbstractSerializationTest {
     URL url = new URL("protocol", "1.1.1.1", 1234);
     ByteArrayOutputStream byteArrayOutputStream = new ByteArrayOutputStream();
 
-    // ================ Primitive Type ================ 
+    // ================ Primitive Type ================
     BigPerson bigPerson;
     MediaContent mediaContent;
 
@@ -383,7 +383,7 @@ public void test_BytesRange() throws Exception {
         }
     }
 
-    // ================ Array Type ================ 
+    // ================ Array Type ================
 
     <T> void assertObjectArray(T[] data, Class<T[]> clazz) throws Exception {
         ObjectOutput objectOutput = serialization.serialize(url, byteArrayOutputStream);
@@ -762,7 +762,7 @@ public void test_StringArray_withType() throws Exception {
         assertObjectArrayWithType(new String[]{"1", "b"}, String[].class);
     }
 
-    // ================ Simple Type ================ 
+    // ================ Simple Type ================
 
     @Test
     public void test_IntegerArray() throws Exception {
@@ -979,7 +979,7 @@ public void test_LinkedHashMap() throws Exception {
         }
     }
 
-    // ================ Complex Collection Type ================ 
+    // ================ Complex Collection Type ================
 
     @Test
     public void test_SPersonList() throws Exception {
@@ -1111,7 +1111,7 @@ public void test_MultiObject_WithType() throws Exception {
     }
 
 
-    // abnormal case 
+    // abnormal case
 
     @Test
     public void test_MediaContent_badStream() throws Exception {
@@ -1207,4 +1207,4 @@ public void test_URL_mutable_withType() throws Exception {
         } catch (IOException expected) {
         }
     }
-}
+}