IGNITE-4063: Preserved order of fields in the metadata according with schema. This closes #1270.
Project: http://git-wip-us.apache.org/repos/asf/ignite/repo Commit: http://git-wip-us.apache.org/repos/asf/ignite/commit/c143bc1a Tree: http://git-wip-us.apache.org/repos/asf/ignite/tree/c143bc1a Diff: http://git-wip-us.apache.org/repos/asf/ignite/diff/c143bc1a Branch: refs/heads/master Commit: c143bc1a77baa13f61d6ba00509fa1fcb33757b1 Parents: 6e48563 Author: tledkov-gridgain <[email protected]> Authored: Fri Dec 9 16:05:03 2016 +0300 Committer: devozerov <[email protected]> Committed: Fri Dec 9 16:05:03 2016 +0300 ---------------------------------------------------------------------- .../internal/binary/BinaryClassDescriptor.java | 12 ++-- .../ignite/internal/binary/BinaryUtils.java | 10 ++- .../binary/builder/BinaryObjectBuilderImpl.java | 11 +++- .../platform/PlatformContextImpl.java | 2 +- .../platform/utils/PlatformUtils.java | 28 +++++++++ .../binary/BinaryMarshallerSelfTest.java | 66 ++++++++++++++++++++ 6 files changed, 119 insertions(+), 10 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/ignite/blob/c143bc1a/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 b121337..5ec519a 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 @@ -28,7 +28,6 @@ import java.sql.Timestamp; import java.util.Collection; import java.util.Collections; import java.util.Date; -import java.util.HashMap; import java.util.HashSet; import java.util.LinkedHashMap; import java.util.Map; @@ -275,15 +274,20 @@ public class BinaryClassDescriptor { case OBJECT: // Must not use constructor to honor transient fields semantics. ctor = null; - stableFieldsMeta = metaDataEnabled ? new HashMap<String, Integer>() : null; Map<Object, BinaryFieldAccessor> fields0; - if (BinaryUtils.FIELDS_SORTED_ORDER) + if (BinaryUtils.FIELDS_SORTED_ORDER) { fields0 = new TreeMap<>(); - else + + stableFieldsMeta = metaDataEnabled ? new TreeMap<String, Integer>() : null; + } + else { fields0 = new LinkedHashMap<>(); + stableFieldsMeta = metaDataEnabled ? new LinkedHashMap<String, Integer>() : null; + } + Set<String> duplicates = duplicateFields(cls); Collection<String> names = new HashSet<>(); http://git-wip-us.apache.org/repos/asf/ignite/blob/c143bc1a/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 b304082..bbf5021 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 @@ -947,11 +947,17 @@ public class BinaryUtils { } // Check and merge fields. - boolean changed = false; + Map<String, Integer> mergedFields; + + if (FIELDS_SORTED_ORDER) + mergedFields = new TreeMap<>(oldMeta.fieldsMap()); + else + mergedFields = new LinkedHashMap<>(oldMeta.fieldsMap()); - Map<String, Integer> mergedFields = new HashMap<>(oldMeta.fieldsMap()); Map<String, Integer> newFields = newMeta.fieldsMap(); + boolean changed = false; + for (Map.Entry<String, Integer> newField : newFields.entrySet()) { Integer oldFieldType = mergedFields.put(newField.getKey(), newField.getValue()); http://git-wip-us.apache.org/repos/asf/ignite/blob/c143bc1a/modules/core/src/main/java/org/apache/ignite/internal/binary/builder/BinaryObjectBuilderImpl.java ---------------------------------------------------------------------- diff --git a/modules/core/src/main/java/org/apache/ignite/internal/binary/builder/BinaryObjectBuilderImpl.java b/modules/core/src/main/java/org/apache/ignite/internal/binary/builder/BinaryObjectBuilderImpl.java index 6ea9e7e..68a0ff3 100644 --- a/modules/core/src/main/java/org/apache/ignite/internal/binary/builder/BinaryObjectBuilderImpl.java +++ b/modules/core/src/main/java/org/apache/ignite/internal/binary/builder/BinaryObjectBuilderImpl.java @@ -400,8 +400,12 @@ public class BinaryObjectBuilderImpl implements BinaryObjectBuilder { if (oldFldTypeName == null) { // It's a new field, we have to add it to metadata. - if (fieldsMeta == null) - fieldsMeta = new HashMap<>(); + if (fieldsMeta == null) { + if (BinaryUtils.FIELDS_SORTED_ORDER) + fieldsMeta = new TreeMap<>(); + else + fieldsMeta = new LinkedHashMap<>(); + } fieldsMeta.put(name, newFldTypeId); } @@ -532,11 +536,12 @@ public class BinaryObjectBuilderImpl implements BinaryObjectBuilder { @Override public BinaryObjectBuilder setField(String name, Object val0) { Object val = val0 == null ? new BinaryValueWithType(BinaryUtils.typeByClass(Object.class), null) : val0; - if (assignedVals == null) + if (assignedVals == null) { if (BinaryUtils.FIELDS_SORTED_ORDER) assignedVals = new TreeMap<>(); else assignedVals = new LinkedHashMap<>(); + } Object oldVal = assignedVals.put(name, val); http://git-wip-us.apache.org/repos/asf/ignite/blob/c143bc1a/modules/core/src/main/java/org/apache/ignite/internal/processors/platform/PlatformContextImpl.java ---------------------------------------------------------------------- diff --git a/modules/core/src/main/java/org/apache/ignite/internal/processors/platform/PlatformContextImpl.java b/modules/core/src/main/java/org/apache/ignite/internal/processors/platform/PlatformContextImpl.java index e7fdb0a..6cec7a1 100644 --- a/modules/core/src/main/java/org/apache/ignite/internal/processors/platform/PlatformContextImpl.java +++ b/modules/core/src/main/java/org/apache/ignite/internal/processors/platform/PlatformContextImpl.java @@ -361,7 +361,7 @@ public class PlatformContextImpl implements PlatformContext { String typeName = reader.readString(); String affKey = reader.readString(); - Map<String, Integer> fields = PlatformUtils.readMap(reader, + Map<String, Integer> fields = PlatformUtils.readLinkedMap(reader, new PlatformReaderBiClosure<String, Integer>() { @Override public IgniteBiTuple<String, Integer> read(BinaryRawReaderEx reader) { return F.t(reader.readString(), reader.readInt()); http://git-wip-us.apache.org/repos/asf/ignite/blob/c143bc1a/modules/core/src/main/java/org/apache/ignite/internal/processors/platform/utils/PlatformUtils.java ---------------------------------------------------------------------- diff --git a/modules/core/src/main/java/org/apache/ignite/internal/processors/platform/utils/PlatformUtils.java b/modules/core/src/main/java/org/apache/ignite/internal/processors/platform/utils/PlatformUtils.java index 0d30ad9..959ff68 100644 --- a/modules/core/src/main/java/org/apache/ignite/internal/processors/platform/utils/PlatformUtils.java +++ b/modules/core/src/main/java/org/apache/ignite/internal/processors/platform/utils/PlatformUtils.java @@ -388,6 +388,34 @@ public class PlatformUtils { } /** + * Read linked map. + * + * @param reader Reader. + * @param readClo Reader closure. + * @return Map. + */ + public static <K, V> Map<K, V> readLinkedMap(BinaryRawReaderEx reader, + @Nullable PlatformReaderBiClosure<K, V> readClo) { + int cnt = reader.readInt(); + + Map<K, V> map = U.newLinkedHashMap(cnt); + + if (readClo == null) { + for (int i = 0; i < cnt; i++) + map.put((K)reader.readObjectDetached(), (V)reader.readObjectDetached()); + } + else { + for (int i = 0; i < cnt; i++) { + IgniteBiTuple<K, V> entry = readClo.read(reader); + + map.put(entry.getKey(), entry.getValue()); + } + } + + return map; + } + + /** * Read nullable map. * * @param reader Reader. http://git-wip-us.apache.org/repos/asf/ignite/blob/c143bc1a/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 39a4d32..6d07c9b 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 @@ -53,6 +53,7 @@ import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ConcurrentSkipListSet; import junit.framework.Assert; import org.apache.ignite.IgniteCheckedException; +import org.apache.ignite.IgniteSystemProperties; import org.apache.ignite.binary.BinaryBasicIdMapper; import org.apache.ignite.binary.BinaryBasicNameMapper; import org.apache.ignite.binary.BinaryCollectionFactory; @@ -3108,6 +3109,71 @@ public class BinaryMarshallerSelfTest extends GridCommonAbstractTest { assertNotEquals(binObj02, binObj11); } + + /** + * The test must be refactored after {@link IgniteSystemProperties#IGNITE_BINARY_SORT_OBJECT_FIELDS} + * is removed. + * + * @throws Exception If failed. + */ + public void testFieldOrder() throws Exception { + if (BinaryUtils.FIELDS_SORTED_ORDER) + return; + + BinaryMarshaller m = binaryMarshaller(); + + BinaryObjectImpl binObj = marshal(simpleObject(), m); + + Collection<String> fieldsBin = binObj.type().fieldNames(); + + Field[] fields = SimpleObject.class.getDeclaredFields(); + + assertEquals(fields.length, fieldsBin.size()); + + int i = 0; + + for (String fieldName : fieldsBin) { + assertEquals(fields[i].getName(), fieldName); + + ++i; + } + } + + /** + * The test must be refactored after {@link IgniteSystemProperties#IGNITE_BINARY_SORT_OBJECT_FIELDS} + * is removed. + * + * @throws Exception If failed. + */ + public void testFieldOrderByBuilder() throws Exception { + if (BinaryUtils.FIELDS_SORTED_ORDER) + return; + + BinaryMarshaller m = binaryMarshaller(); + + BinaryObjectBuilder builder = new BinaryObjectBuilderImpl(binaryContext(m), "MyFakeClass"); + + String[] fieldNames = {"field9", "field8", "field0", "field1", "field2"}; + + for (String fieldName : fieldNames) + builder.setField(fieldName, 0); + + BinaryObject binObj = builder.build(); + + + Collection<String> fieldsBin = binObj.type().fieldNames(); + + assertEquals(fieldNames.length, fieldsBin.size()); + + int i = 0; + + for (String fieldName : fieldsBin) { + assertEquals(fieldNames[i], fieldName); + + ++i; + } + } + /** * @param obj Instance of the BinaryObjectImpl to offheap marshalling. * @param marsh Binary marshaller.
