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

jorgebg pushed a commit to branch TINKERPOP-1942
in repository https://gitbox.apache.org/repos/asf/tinkerpop.git


The following commit(s) were added to refs/heads/TINKERPOP-1942 by this push:
     new 2068c17  Improve type resolution for interfaces and subclasses
2068c17 is described below

commit 2068c179373a9814bbb505088f99f3b468d27fd3
Author: Jorge Bay Gondra <jorgebaygon...@gmail.com>
AuthorDate: Tue Dec 4 15:17:27 2018 +0100

    Improve type resolution for interfaces and subclasses
---
 .../driver/ser/binary/TypeSerializerRegistry.java  | 34 +++++++++++++++++-----
 .../GraphBinaryReaderWriterRoundTripTest.java      | 16 +++++-----
 2 files changed, 36 insertions(+), 14 deletions(-)

diff --git 
a/gremlin-driver/src/main/java/org/apache/tinkerpop/gremlin/driver/ser/binary/TypeSerializerRegistry.java
 
b/gremlin-driver/src/main/java/org/apache/tinkerpop/gremlin/driver/ser/binary/TypeSerializerRegistry.java
index 831d44c..6a795ce 100644
--- 
a/gremlin-driver/src/main/java/org/apache/tinkerpop/gremlin/driver/ser/binary/TypeSerializerRegistry.java
+++ 
b/gremlin-driver/src/main/java/org/apache/tinkerpop/gremlin/driver/ser/binary/TypeSerializerRegistry.java
@@ -61,6 +61,7 @@ import java.util.List;
 import java.util.Map;
 import java.util.Set;
 import java.util.UUID;
+import java.util.concurrent.ConcurrentHashMap;
 
 public class TypeSerializerRegistry {
     public static final TypeSerializerRegistry INSTANCE = build().create();
@@ -188,6 +189,12 @@ public class TypeSerializerRegistry {
     private final Map<DataType, TypeSerializer<?>> serializersByDataType = new 
HashMap<>();
     private final Map<String, CustomTypeSerializer> 
serializersByCustomTypeName = new HashMap<>();
 
+    /**
+     * Stores serializers by class, where the class resolution involved a 
lookup or special conditions.
+     * This is used for interface implementations and enums.
+     */
+    private final ConcurrentHashMap<Class<?>, TypeSerializer<?>> 
serializersByImplementation = new ConcurrentHashMap<>();
+
     private TypeSerializerRegistry(final Collection<RegistryEntry> entries) {
         for (RegistryEntry entry : entries) {
             put(entry);
@@ -229,21 +236,34 @@ public class TypeSerializerRegistry {
         TypeSerializer<?> serializer = serializers.get(type);
 
         if (null == serializer) {
-            // Find by interface
+            // Try to obtain the serializer by the type interface 
implementation or superclass,
+            // when previously accessed.
+            serializer = serializersByImplementation.get(type);
+        }
+
+        if (null == serializer && Enum.class.isAssignableFrom(type)) {
+            // maybe it's a enum - enums with bodies are weird in java, they 
are subclasses of themselves, so
+            // Columns.values will be of type Column$2.
+            serializer = serializers.get(type.getSuperclass());
+
+            // In case it matched, store the match
+            if (serializer != null) {
+                serializersByImplementation.put(type, serializer);
+            }
+        }
+
+        if (null == serializer) {
+            // Lookup by interface
             for (Map.Entry<Class<?>, TypeSerializer<?>> entry : 
serializersByInterface.entrySet()) {
                 if (entry.getKey().isAssignableFrom(type)) {
                     serializer = entry.getValue();
+                    // Store the type-to-interface match, to avoid looking it 
up in the future
+                    serializersByImplementation.put(type, serializer);
                     break;
                 }
             }
         }
 
-        if (null == serializer && Enum.class.isAssignableFrom(type)) {
-            // maybe it's a enum - enums with bodies are weird in java, they 
are subclasses of themselves, so
-            // Columns.values will be of type Column$2. could be a 
better/safer way to do this.
-            serializer = serializers.get(type.getSuperclass());
-        }         
-
         return validateInstance(serializer, type.getTypeName());
     }
 
diff --git 
a/gremlin-driver/src/test/java/org/apache/tinkerpop/gremlin/driver/ser/binary/GraphBinaryReaderWriterRoundTripTest.java
 
b/gremlin-driver/src/test/java/org/apache/tinkerpop/gremlin/driver/ser/binary/GraphBinaryReaderWriterRoundTripTest.java
index 9501980..055df5d 100644
--- 
a/gremlin-driver/src/test/java/org/apache/tinkerpop/gremlin/driver/ser/binary/GraphBinaryReaderWriterRoundTripTest.java
+++ 
b/gremlin-driver/src/test/java/org/apache/tinkerpop/gremlin/driver/ser/binary/GraphBinaryReaderWriterRoundTripTest.java
@@ -38,14 +38,12 @@ import 
org.apache.tinkerpop.gremlin.structure.util.reference.ReferenceVertex;
 import 
org.apache.tinkerpop.gremlin.structure.util.reference.ReferenceVertexProperty;
 import org.apache.tinkerpop.gremlin.tinkergraph.structure.TinkerFactory;
 import org.apache.tinkerpop.gremlin.util.function.Lambda;
-import org.junit.Assert;
 import org.junit.Test;
 import org.junit.runner.RunWith;
 import org.junit.runners.Parameterized;
 
 import java.util.*;
 import java.util.function.Consumer;
-import java.util.function.Predicate;
 
 import static org.junit.Assert.assertEquals;
 
@@ -101,6 +99,7 @@ public class GraphBinaryReaderWriterRoundTripTest {
                 new Object[] {"Columns", Column.values, null},
                 new Object[] {"Direction", Direction.BOTH, null},
                 new Object[] {"Operator", Operator.sum, null},
+                new Object[] {"Operator", Operator.div, null},
                 new Object[] {"Order", Order.desc, null},
                 new Object[] {"Pick", TraversalOptionParent.Pick.any, null},
                 new Object[] {"Pop", Pop.mixed, null},
@@ -139,10 +138,13 @@ public class GraphBinaryReaderWriterRoundTripTest {
 
     @Test
     public void shouldWriteAndRead() throws Exception {
-        final ByteBuf buffer = writer.write(value, allocator);
-        buffer.readerIndex(0);
-        final Object result = reader.read(buffer);
-
-        Optional.ofNullable(assertion).orElse((Consumer) r -> 
assertEquals(value, r)).accept(result);
+        // Test it multiple times as the type registry might change its 
internal state
+        for (int i = 0; i < 5; i++) {
+            final ByteBuf buffer = writer.write(value, allocator);
+            buffer.readerIndex(0);
+            final Object result = reader.read(buffer);
+
+            Optional.ofNullable(assertion).orElse((Consumer) r -> 
assertEquals(value, r)).accept(result);
+        }
     }
 }

Reply via email to