IGNITE-2213: Fixed serialization of duplicate fields.
Project: http://git-wip-us.apache.org/repos/asf/ignite/repo Commit: http://git-wip-us.apache.org/repos/asf/ignite/commit/627134b2 Tree: http://git-wip-us.apache.org/repos/asf/ignite/tree/627134b2 Diff: http://git-wip-us.apache.org/repos/asf/ignite/diff/627134b2 Branch: refs/heads/master Commit: 627134b25ca730d7a7dbcec9facde158c5d15dc3 Parents: d8576b8 Author: vozerov-gridgain <voze...@gridgain.com> Authored: Tue Dec 22 11:28:44 2015 +0300 Committer: vozerov-gridgain <voze...@gridgain.com> Committed: Tue Dec 22 11:28:44 2015 +0300 ---------------------------------------------------------------------- .../internal/binary/BinaryClassDescriptor.java | 56 ++++++++-- .../ignite/internal/binary/BinaryUtils.java | 11 ++ .../binary/BinaryMarshallerSelfTest.java | 110 +++++++++++++++++++ .../IgniteBinaryCacheQueryTestSuite.java | 3 +- 4 files changed, 169 insertions(+), 11 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/ignite/blob/627134b2/modules/core/src/main/java/org/apache/ignite/internal/binary/BinaryClassDescriptor.java ---------------------------------------------------------------------- diff --git a/modules/core/src/main/java/org/apache/ignite/internal/binary/BinaryClassDescriptor.java b/modules/core/src/main/java/org/apache/ignite/internal/binary/BinaryClassDescriptor.java index 1eb3882..1105809 100644 --- a/modules/core/src/main/java/org/apache/ignite/internal/binary/BinaryClassDescriptor.java +++ b/modules/core/src/main/java/org/apache/ignite/internal/binary/BinaryClassDescriptor.java @@ -35,6 +35,7 @@ import java.lang.reflect.Constructor; import java.lang.reflect.Field; import java.lang.reflect.InvocationTargetException; import java.lang.reflect.Method; +import java.lang.reflect.Modifier; import java.math.BigDecimal; import java.sql.Timestamp; import java.util.ArrayList; @@ -44,11 +45,9 @@ import java.util.Date; import java.util.HashMap; import java.util.HashSet; import java.util.Map; +import java.util.Set; import java.util.UUID; -import static java.lang.reflect.Modifier.isStatic; -import static java.lang.reflect.Modifier.isTransient; - /** * Binary class descriptor. */ @@ -250,21 +249,24 @@ public class BinaryClassDescriptor { BinarySchema.Builder schemaBuilder = BinarySchema.Builder.newBuilder(); + Set<String> duplicates = duplicateFields(cls); + Collection<String> names = new HashSet<>(); Collection<Integer> ids = new HashSet<>(); for (Class<?> c = cls; c != null && !c.equals(Object.class); c = c.getSuperclass()) { for (Field f : c.getDeclaredFields()) { - int mod = f.getModifiers(); - - if (!isStatic(mod) && !isTransient(mod)) { + if (serializeField(f)) { f.setAccessible(true); String name = f.getName(); - if (!names.add(name)) - throw new BinaryObjectException("Duplicate field name [fieldName=" + name + - ", cls=" + cls.getName() + ']'); + if (duplicates.contains(name)) + name = BinaryUtils.qualifiedFieldName(c, name); + + boolean added = names.add(name); + + assert added : name; int fieldId = idMapper.fieldId(typeId, name); @@ -308,6 +310,42 @@ public class BinaryClassDescriptor { } /** + * Find all fields with duplicate names in the class. + * + * @param cls Class. + * @return Fields with duplicate names. + */ + private static Set<String> duplicateFields(Class cls) { + Set<String> all = new HashSet<>(); + Set<String> duplicates = new HashSet<>(); + + for (Class<?> c = cls; c != null && !c.equals(Object.class); c = c.getSuperclass()) { + for (Field f : c.getDeclaredFields()) { + if (serializeField(f)) { + String name = f.getName(); + + if (!all.add(name)) + duplicates.add(name); + } + } + } + + return duplicates; + } + + /** + * Whether the field must be serialized. + * + * @param f Field. + * @return {@code True} if must be serialized. + */ + private static boolean serializeField(Field f) { + int mod = f.getModifiers(); + + return !Modifier.isStatic(mod) && !Modifier.isTransient(mod); + } + + /** * @return {@code True} if enum. */ boolean isEnum() { http://git-wip-us.apache.org/repos/asf/ignite/blob/627134b2/modules/core/src/main/java/org/apache/ignite/internal/binary/BinaryUtils.java ---------------------------------------------------------------------- diff --git a/modules/core/src/main/java/org/apache/ignite/internal/binary/BinaryUtils.java b/modules/core/src/main/java/org/apache/ignite/internal/binary/BinaryUtils.java index 8cb4b38..62a9d26 100644 --- a/modules/core/src/main/java/org/apache/ignite/internal/binary/BinaryUtils.java +++ b/modules/core/src/main/java/org/apache/ignite/internal/binary/BinaryUtils.java @@ -1883,6 +1883,17 @@ public class BinaryUtils { } /** + * Create qualified field name. + * + * @param cls Class. + * @param fieldName Field name. + * @return Qualified field name. + */ + public static String qualifiedFieldName(Class cls, String fieldName) { + return cls.getName() + "." + fieldName; + } + + /** * Enum type. */ private static class EnumType { http://git-wip-us.apache.org/repos/asf/ignite/blob/627134b2/modules/core/src/test/java/org/apache/ignite/internal/binary/BinaryMarshallerSelfTest.java ---------------------------------------------------------------------- diff --git a/modules/core/src/test/java/org/apache/ignite/internal/binary/BinaryMarshallerSelfTest.java b/modules/core/src/test/java/org/apache/ignite/internal/binary/BinaryMarshallerSelfTest.java index fcd511b..ac9771f 100644 --- a/modules/core/src/test/java/org/apache/ignite/internal/binary/BinaryMarshallerSelfTest.java +++ b/modules/core/src/test/java/org/apache/ignite/internal/binary/BinaryMarshallerSelfTest.java @@ -54,6 +54,7 @@ import java.util.concurrent.ConcurrentSkipListSet; import junit.framework.Assert; import org.apache.ignite.IgniteCheckedException; import org.apache.ignite.binary.BinaryCollectionFactory; +import org.apache.ignite.binary.BinaryField; import org.apache.ignite.binary.BinaryIdMapper; import org.apache.ignite.binary.BinaryMapFactory; import org.apache.ignite.binary.BinaryObject; @@ -63,6 +64,7 @@ import org.apache.ignite.binary.BinaryRawReader; import org.apache.ignite.binary.BinaryRawWriter; import org.apache.ignite.binary.BinaryReader; import org.apache.ignite.binary.BinarySerializer; +import org.apache.ignite.binary.BinaryType; import org.apache.ignite.binary.BinaryTypeConfiguration; import org.apache.ignite.binary.BinaryWriter; import org.apache.ignite.binary.Binarylizable; @@ -2315,6 +2317,63 @@ public class BinaryMarshallerSelfTest extends GridCommonAbstractTest { } /** + * Test duplicate fields. + * + * @throws Exception If failed. + */ + public void testDuplicateFields() throws Exception { + BinaryMarshaller marsh = binaryMarshaller(); + + DuplicateFieldsB obj = new DuplicateFieldsB(1, 2); + + BinaryObjectImpl objBin = marshal(obj, marsh); + + String fieldName = "x"; + String fieldNameA = DuplicateFieldsA.class.getName() + "." + fieldName; + String fieldNameB = DuplicateFieldsB.class.getName() + "." + fieldName; + + // Check "hasField". + assert !objBin.hasField(fieldName); + assert objBin.hasField(fieldNameA); + assert objBin.hasField(fieldNameB); + + // Check direct field access. + assertNull(objBin.field(fieldName)); + assertEquals(1, objBin.field(fieldNameA)); + assertEquals(2, objBin.field(fieldNameB)); + + // Check metadata. + BinaryType type = objBin.type(); + + Collection<String> fieldNames = type.fieldNames(); + + assertEquals(2, fieldNames.size()); + + assert !fieldNames.contains(fieldName); + assert fieldNames.contains(fieldNameA); + assert fieldNames.contains(fieldNameB); + + // Check field access through type. + BinaryField field = type.field(fieldName); + BinaryField fieldA = type.field(fieldNameA); + BinaryField fieldB = type.field(fieldNameB); + + assert !field.exists(objBin); + assert fieldA.exists(objBin); + assert fieldB.exists(objBin); + + assertNull(field.value(objBin)); + assertEquals(1, fieldA.value(objBin)); + assertEquals(2, fieldB.value(objBin)); + + // Check object deserialization. + DuplicateFieldsB deserialized = objBin.deserialize(); + + assertEquals(obj.xA(), deserialized.xA()); + assertEquals(obj.xB(), deserialized.xB()); + } + + /** * */ private static interface SomeItf { @@ -4120,6 +4179,57 @@ public class BinaryMarshallerSelfTest extends GridCommonAbstractTest { } /** + * Class B for duplicate fields test. + */ + private static class DuplicateFieldsA { + /** Field. */ + int x; + + /** + * Constructor. + * + * @param x Field. + */ + protected DuplicateFieldsA(int x) { + this.x = x; + } + + /** + * @return A's field. + */ + public int xA() { + return x; + } + } + + /** + * Class B for duplicate fields test. + */ + private static class DuplicateFieldsB extends DuplicateFieldsA { + /** Field. */ + int x; + + /** + * Constructor. + * + * @param xA Field for parent class. + * @param xB Field for current class. + */ + public DuplicateFieldsB(int xA, int xB) { + super(xA); + + this.x = xB; + } + + /** + * @return B's field. + */ + public int xB() { + return x; + } + } + + /** * */ private static class DecimalReflective { http://git-wip-us.apache.org/repos/asf/ignite/blob/627134b2/modules/indexing/src/test/java/org/apache/ignite/testsuites/IgniteBinaryCacheQueryTestSuite.java ---------------------------------------------------------------------- diff --git a/modules/indexing/src/test/java/org/apache/ignite/testsuites/IgniteBinaryCacheQueryTestSuite.java b/modules/indexing/src/test/java/org/apache/ignite/testsuites/IgniteBinaryCacheQueryTestSuite.java index 1f5d6d1..b145a90 100644 --- a/modules/indexing/src/test/java/org/apache/ignite/testsuites/IgniteBinaryCacheQueryTestSuite.java +++ b/modules/indexing/src/test/java/org/apache/ignite/testsuites/IgniteBinaryCacheQueryTestSuite.java @@ -18,6 +18,7 @@ package org.apache.ignite.testsuites; import junit.framework.TestSuite; +import org.apache.ignite.internal.binary.BinaryMarshaller; import org.apache.ignite.internal.processors.cache.BinarySerializationQuerySelfTest; import org.apache.ignite.internal.processors.cache.BinarySerializationQueryWithReflectiveSerializerSelfTest; import org.apache.ignite.internal.processors.cache.CacheLocalQueryMetricsSelfTest; @@ -32,7 +33,6 @@ import org.apache.ignite.internal.processors.cache.GridCacheQueryIndexingDisable import org.apache.ignite.internal.processors.cache.GridCacheQueryInternalKeysSelfTest; import org.apache.ignite.internal.processors.cache.GridCacheQuerySerializationSelfTest; import org.apache.ignite.internal.processors.cache.GridCacheReduceQueryMultithreadedSelfTest; -import org.apache.ignite.internal.processors.cache.IgniteBinaryObjectFieldsQuerySelfTest; import org.apache.ignite.internal.processors.cache.IgniteCacheBinaryObjectsScanSelfTest; import org.apache.ignite.internal.processors.cache.IgniteCacheCollocatedQuerySelfTest; import org.apache.ignite.internal.processors.cache.IgniteCacheDuplicateEntityConfigurationSelfTest; @@ -101,7 +101,6 @@ import org.apache.ignite.internal.processors.query.IgniteSqlSplitterSelfTest; import org.apache.ignite.internal.processors.query.h2.sql.BaseH2CompareQueryTest; import org.apache.ignite.internal.processors.query.h2.sql.GridQueryParsingTest; import org.apache.ignite.internal.processors.query.h2.sql.H2CompareBigQueryTest; -import org.apache.ignite.internal.binary.BinaryMarshaller; import org.apache.ignite.spi.communication.tcp.GridOrderedMessageCancelSelfTest; import org.apache.ignite.testframework.config.GridTestProperties;
