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); + } } }