[incubator-dubbo] branch master updated: Merge pull request #2220, leave jdk standard classes to kryo, use the default registered serializer by kryo.

[li...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

liujun pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/incubator-dubbo.git


The following commit(s) were added to refs/heads/master by this push:
     new 8beeb64  Merge pull request #2220, leave jdk standard classes to kryo, use the default registered serializer by kryo.
8beeb64 is described below

commit 8beeb649f2941b6b5941895ebc3ef8ab260c1f9e
Author: ken.lj <ke...@gmail.com>
AuthorDate: Mon Aug 13 10:48:40 2018 +0800

    Merge pull request #2220, leave jdk standard classes to kryo, use the default registered serializer by kryo.
    
    Fixes #2178, support java8 time types.
---
 .../support/SerializableClassRegistry.java         | 23 +++++++++++++++++-----
 .../support/SerializableClassRegistryTest.java     |  8 ++++----
 .../dubbo/common/serialize/fst/FstFactory.java     |  5 +----
 .../common/serialize/kryo/CompatibleKryo.java      | 18 ++++++++++++-----
 .../serialize/kryo/utils/AbstractKryoFactory.java  | 16 +++++++++------
 .../serialize/kryo/utils/ReflectionUtils.java      |  4 ++++
 .../serialization/AbstractSerializationTest.java   |  7 ++++++-
 7 files changed, 56 insertions(+), 25 deletions(-)

diff --git a/dubbo-serialization/dubbo-serialization-api/src/main/java/org/apache/dubbo/common/serialize/support/SerializableClassRegistry.java b/dubbo-serialization/dubbo-serialization-api/src/main/java/org/apache/dubbo/common/serialize/support/SerializableClassRegistry.java
index dd71859..2ebb3a3 100644
--- a/dubbo-serialization/dubbo-serialization-api/src/main/java/org/apache/dubbo/common/serialize/support/SerializableClassRegistry.java
+++ b/dubbo-serialization/dubbo-serialization-api/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/dubbo-serialization/dubbo-serialization-api/src/test/java/org/apache/dubbo/common/serialize/support/SerializableClassRegistryTest.java b/dubbo-serialization/dubbo-serialization-api/src/test/java/org/apache/dubbo/common/serialize/support/SerializableClassRegistryTest.java
index f471bc4..efde85c 100644
--- a/dubbo-serialization/dubbo-serialization-api/src/test/java/org/apache/dubbo/common/serialize/support/SerializableClassRegistryTest.java
+++ b/dubbo-serialization/dubbo-serialization-api/src/test/java/org/apache/dubbo/common/serialize/support/SerializableClassRegistryTest.java
@@ -18,9 +18,9 @@ package org.apache.dubbo.common.serialize.support;
 
 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 class SerializableClassRegistryTest {
         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/dubbo-serialization/dubbo-serialization-fst/src/main/java/org/apache/dubbo/common/serialize/fst/FstFactory.java b/dubbo-serialization/dubbo-serialization-fst/src/main/java/org/apache/dubbo/common/serialize/fst/FstFactory.java
index feb9ce4..6329d1e 100644
--- a/dubbo-serialization/dubbo-serialization-fst/src/main/java/org/apache/dubbo/common/serialize/fst/FstFactory.java
+++ b/dubbo-serialization/dubbo-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 class FstFactory {
     }
 
     public FstFactory() {
-        for (Class clazz : SerializableClassRegistry.getRegisteredClasses()) {
-            conf.registerClass(clazz);
-        }
+        SerializableClassRegistry.getRegisteredClasses().keySet().forEach(conf::registerClass);
     }
 
     public FSTObjectOutput getObjectOutput(OutputStream outputStream) {
diff --git a/dubbo-serialization/dubbo-serialization-kryo/src/main/java/org/apache/dubbo/common/serialize/kryo/CompatibleKryo.java b/dubbo-serialization/dubbo-serialization-kryo/src/main/java/org/apache/dubbo/common/serialize/kryo/CompatibleKryo.java
index dcd0007..ba854e5 100644
--- a/dubbo-serialization/dubbo-serialization-kryo/src/main/java/org/apache/dubbo/common/serialize/kryo/CompatibleKryo.java
+++ b/dubbo-serialization/dubbo-serialization-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 class CompatibleKryo extends Kryo {
             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/dubbo-serialization/dubbo-serialization-kryo/src/main/java/org/apache/dubbo/common/serialize/kryo/utils/AbstractKryoFactory.java b/dubbo-serialization/dubbo-serialization-kryo/src/main/java/org/apache/dubbo/common/serialize/kryo/utils/AbstractKryoFactory.java
index ca0abad..523b59f 100644
--- a/dubbo-serialization/dubbo-serialization-kryo/src/main/java/org/apache/dubbo/common/serialize/kryo/utils/AbstractKryoFactory.java
+++ b/dubbo-serialization/dubbo-serialization-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.SynchronizedCollectionsSerializer;
 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 abstract class AbstractKryoFactory implements KryoFactory {
             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/dubbo-serialization/dubbo-serialization-kryo/src/main/java/org/apache/dubbo/common/serialize/kryo/utils/ReflectionUtils.java b/dubbo-serialization/dubbo-serialization-kryo/src/main/java/org/apache/dubbo/common/serialize/kryo/utils/ReflectionUtils.java
index 473685e..7c3b45a 100644
--- a/dubbo-serialization/dubbo-serialization-kryo/src/main/java/org/apache/dubbo/common/serialize/kryo/utils/ReflectionUtils.java
+++ b/dubbo-serialization/dubbo-serialization-kryo/src/main/java/org/apache/dubbo/common/serialize/kryo/utils/ReflectionUtils.java
@@ -26,4 +26,8 @@ public abstract class ReflectionUtils {
             return false;
         }
     }
+
+    public static boolean isJdk(Class clazz) {
+        return clazz.getName().startsWith("java.") || clazz.getName().startsWith("javax.");
+    }
 }
diff --git a/dubbo-serialization/dubbo-serialization-kryo/src/test/java/org/apache/dubbo/common/serialize/serialization/AbstractSerializationTest.java b/dubbo-serialization/dubbo-serialization-kryo/src/test/java/org/apache/dubbo/common/serialize/serialization/AbstractSerializationTest.java
index a40e378..411627f 100644
--- a/dubbo-serialization/dubbo-serialization-kryo/src/test/java/org/apache/dubbo/common/serialize/serialization/AbstractSerializationTest.java
+++ b/dubbo-serialization/dubbo-serialization-kryo/src/test/java/org/apache/dubbo/common/serialize/serialization/AbstractSerializationTest.java
@@ -34,7 +34,6 @@ import org.apache.dubbo.common.model.person.Phone;
 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.io.IOException;
 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;
@@ -859,6 +859,11 @@ public abstract class AbstractSerializationTest {
     }
 
     @Test
+    public void test_LocalDateTime() throws Exception {
+        assertObject(LocalDateTime.now());
+    }
+
+    @Test
     public void test_enum() throws Exception {
         assertObject(AnimalEnum.dog);
     }