You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@ignite.apache.org by dm...@apache.org on 2016/03/16 11:54:16 UTC

ignite git commit: IGNITE-2849: BinaryObjectBuilder doesn't properly check metadata

Repository: ignite
Updated Branches:
  refs/heads/ignite-2849 69d1f4b77 -> 08d4a76f0


IGNITE-2849: BinaryObjectBuilder doesn't properly check metadata


Project: http://git-wip-us.apache.org/repos/asf/ignite/repo
Commit: http://git-wip-us.apache.org/repos/asf/ignite/commit/08d4a76f
Tree: http://git-wip-us.apache.org/repos/asf/ignite/tree/08d4a76f
Diff: http://git-wip-us.apache.org/repos/asf/ignite/diff/08d4a76f

Branch: refs/heads/ignite-2849
Commit: 08d4a76f01e24106258ddbea9f36e53cb280e396
Parents: 69d1f4b
Author: Denis Magda <dm...@gridgain.com>
Authored: Wed Mar 16 13:54:07 2016 +0300
Committer: Denis Magda <dm...@gridgain.com>
Committed: Wed Mar 16 13:54:07 2016 +0300

----------------------------------------------------------------------
 .../binary/builder/BinaryObjectBuilderImpl.java | 107 +++++++++------
 .../BinaryObjectBuilderAdditionalSelfTest.java  | 137 ++++++++++++++++++-
 2 files changed, 195 insertions(+), 49 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/ignite/blob/08d4a76f/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 9043a8b..ced3826 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
@@ -195,6 +195,10 @@ public class BinaryObjectBuilderImpl implements BinaryObjectBuilder {
 
             Set<Integer> remainsFlds = null;
 
+            BinaryType meta = ctx.metadata(typeId);
+
+            Map<String, Integer> fieldsMeta = null;
+
             if (reader != null) {
                 BinarySchema schema = reader.schema();
 
@@ -204,9 +208,15 @@ public class BinaryObjectBuilderImpl implements BinaryObjectBuilder {
                     assignedFldsById = U.newHashMap(assignedVals.size());
 
                     for (Map.Entry<String, Object> entry : assignedVals.entrySet()) {
-                        int fieldId = ctx.fieldId(typeId, entry.getKey());
+                        String name = entry.getKey();
+                        Object val = entry.getValue();
+
+                        int fieldId = ctx.fieldId(typeId, name);
 
-                        assignedFldsById.put(fieldId, entry.getValue());
+                        assignedFldsById.put(fieldId, val);
+
+                        if (val != REMOVED_FIELD_MARKER)
+                            fieldsMeta = checkMetadata(meta, fieldsMeta, val, name);
                     }
 
                     remainsFlds = assignedFldsById.keySet();
@@ -280,10 +290,6 @@ public class BinaryObjectBuilderImpl implements BinaryObjectBuilder {
                 }
             }
 
-            BinaryType meta = ctx.metadata(typeId);
-
-            Map<String, Integer> fieldsMeta = null;
-
             if (assignedVals != null && (remainsFlds == null || !remainsFlds.isEmpty())) {
                 for (Map.Entry<String, Object> entry : assignedVals.entrySet()) {
                     Object val = entry.getValue();
@@ -302,43 +308,9 @@ public class BinaryObjectBuilderImpl implements BinaryObjectBuilder {
 
                     serializer.writeValue(writer, val);
 
-                    String oldFldTypeName = meta == null ? null : meta.fieldTypeName(name);
-
-                    boolean nullObjField = false;
-
-                    int newFldTypeId;
-
-                    if (val instanceof BinaryValueWithType) {
-                        newFldTypeId = ((BinaryValueWithType)val).typeId();
-
-                        if (newFldTypeId == GridBinaryMarshaller.OBJ && ((BinaryValueWithType)val).value() == null)
-                            nullObjField = true;
-                    }
-                    else
-                        newFldTypeId = BinaryUtils.typeByClass(val.getClass());
-
-                    String newFldTypeName = BinaryUtils.fieldTypeName(newFldTypeId);
-
-                    if (oldFldTypeName == null) {
-                        // It's a new field, we have to add it to metadata.
-                        if (fieldsMeta == null)
-                            fieldsMeta = new HashMap<>();
-
-                        fieldsMeta.put(name, BinaryUtils.fieldTypeId(newFldTypeName));
-                    }
-                    else if (!nullObjField) {
-                        String objTypeName = BinaryUtils.fieldTypeName(GridBinaryMarshaller.OBJ);
-
-                        if (!objTypeName.equals(oldFldTypeName) && !oldFldTypeName.equals(newFldTypeName)) {
-                            throw new BinaryObjectException(
-                                "Wrong value has been set [" +
-                                    "typeName=" + (typeName == null ? meta.typeName() : typeName) +
-                                    ", fieldName=" + name +
-                                    ", fieldType=" + oldFldTypeName +
-                                    ", assignedValueType=" + newFldTypeName + ']'
-                            );
-                        }
-                    }
+                    if (reader == null)
+                        // Metadata has already been checked.
+                        fieldsMeta = checkMetadata(meta, fieldsMeta, val, name);
                 }
             }
 
@@ -386,6 +358,55 @@ public class BinaryObjectBuilderImpl implements BinaryObjectBuilder {
         }
     }
 
+    /**
+     * Checks metadata when a BinaryObject is being serialized.
+     *
+     * @param meta Current metadata.
+     * @param fieldsMeta Map holding metadata information that has to be updated.
+     * @param newVal Field value being serialized.
+     * @param name Field name.
+     */
+    private Map<String, Integer> checkMetadata(BinaryType meta, Map<String, Integer> fieldsMeta, Object newVal,
+        String name) {
+        String oldFldTypeName = meta == null ? null : meta.fieldTypeName(name);
+
+        boolean nullFieldVal = false;
+
+        int newFldTypeId;
+
+        if (newVal instanceof BinaryValueWithType) {
+            newFldTypeId = ((BinaryValueWithType)newVal).typeId();
+
+            if (((BinaryValueWithType)newVal).value() == null)
+                nullFieldVal = true;
+        }
+        else
+            newFldTypeId = BinaryUtils.typeByClass(newVal.getClass());
+
+        String newFldTypeName = BinaryUtils.fieldTypeName(newFldTypeId);
+
+        if (oldFldTypeName == null) {
+            // It's a new field, we have to add it to metadata.
+            if (fieldsMeta == null)
+                fieldsMeta = new HashMap<>();
+
+            fieldsMeta.put(name, BinaryUtils.fieldTypeId(newFldTypeName));
+        }
+        else if (!nullFieldVal) {
+            if (!oldFldTypeName.equals(newFldTypeName)) {
+                throw new BinaryObjectException(
+                    "Wrong value has been set [" +
+                        "typeName=" + (typeName == null ? meta.typeName() : typeName) +
+                        ", fieldName=" + name +
+                        ", fieldType=" + oldFldTypeName +
+                        ", assignedValueType=" + newFldTypeName + ']'
+                );
+            }
+        }
+
+        return fieldsMeta;
+    }
+
     /** {@inheritDoc} */
     @Override public BinaryObjectBuilderImpl hashCode(int hashCode) {
         this.hashCode = hashCode;

http://git-wip-us.apache.org/repos/asf/ignite/blob/08d4a76f/modules/core/src/test/java/org/apache/ignite/internal/binary/BinaryObjectBuilderAdditionalSelfTest.java
----------------------------------------------------------------------
diff --git a/modules/core/src/test/java/org/apache/ignite/internal/binary/BinaryObjectBuilderAdditionalSelfTest.java b/modules/core/src/test/java/org/apache/ignite/internal/binary/BinaryObjectBuilderAdditionalSelfTest.java
index 804c060..074649b 100644
--- a/modules/core/src/test/java/org/apache/ignite/internal/binary/BinaryObjectBuilderAdditionalSelfTest.java
+++ b/modules/core/src/test/java/org/apache/ignite/internal/binary/BinaryObjectBuilderAdditionalSelfTest.java
@@ -26,6 +26,7 @@ import org.apache.ignite.IgniteBinary;
 import org.apache.ignite.IgniteCheckedException;
 import org.apache.ignite.binary.BinaryObject;
 import org.apache.ignite.binary.BinaryObjectBuilder;
+import org.apache.ignite.binary.BinaryObjectException;
 import org.apache.ignite.binary.BinaryType;
 import org.apache.ignite.configuration.BinaryConfiguration;
 import org.apache.ignite.configuration.CacheConfiguration;
@@ -234,7 +235,7 @@ public class BinaryObjectBuilderAdditionalSelfTest extends GridCommonAbstractTes
     public void testDateArrayModification() {
         GridBinaryTestClasses.TestObjectAllTypes obj = new GridBinaryTestClasses.TestObjectAllTypes();
 
-        obj.dateArr =  new Date[] {new Date(11111), new Date(11111), new Date(11111)};
+        obj.dateArr = new Date[] {new Date(11111), new Date(11111), new Date(11111)};
 
         BinaryObjectBuilderImpl mutObj = wrap(obj);
 
@@ -995,6 +996,130 @@ public class BinaryObjectBuilderAdditionalSelfTest extends GridCommonAbstractTes
     /**
      *
      */
+    public void testWrongMetadataNullField() {
+        BinaryObjectBuilder builder = binaries().builder("SomeType");
+
+        builder.setField("dateField", null);
+
+        builder.setField("objectField", null, Integer.class);
+
+        builder.build();
+
+        builder = binaries().builder("SomeType");
+
+        try {
+            builder.setField("dateField", new Date());
+
+            builder.build();
+
+            fail("BinaryObjectBuilder accepted wrong metadata");
+        }
+        catch (BinaryObjectException ex) {
+            assertTrue(ex.getMessage().startsWith("Wrong value has been set"));
+        }
+
+        builder = binaries().builder("SomeType");
+
+        try {
+            builder.setField("objectField", new GridBinaryTestClasses.Company());
+
+            builder.build();
+
+            fail("BinaryObjectBuilder accepted wrong metadata");
+        }
+        catch (BinaryObjectException ex) {
+            assertTrue(ex.getMessage().startsWith("Wrong value has been set"));
+        }
+    }
+
+    /**
+     *
+     */
+    public void testWrongMetadataNullField2() {
+        BinaryObjectBuilder builder = binaries().builder("SomeType1");
+
+        builder.setField("dateField", null);
+
+        builder.setField("objectField", null, Integer.class);
+
+        BinaryObject obj = builder.build();
+
+
+        builder = binaries().builder(obj);
+
+        try {
+            builder.setField("dateField", new Date());
+
+            builder.build();
+
+            fail("BinaryObjectBuilder accepted wrong metadata");
+        }
+        catch (BinaryObjectException ex) {
+            assertTrue(ex.getMessage().startsWith("Wrong value has been set"));
+        }
+
+        builder = binaries().builder(obj);
+
+        try {
+            builder.setField("objectField", new GridBinaryTestClasses.Company());
+
+            builder.build();
+
+            fail("BinaryObjectBuilder accepted wrong metadata");
+        }
+        catch (BinaryObjectException ex) {
+            assertTrue(ex.getMessage().startsWith("Wrong value has been set"));
+        }
+    }
+
+    /**
+     *
+     */
+    public void testCorrectMetadataNullField() {
+        BinaryObjectBuilder builder = binaries().builder("SomeType2");
+
+        builder.setField("dateField", null, Date.class);
+
+        builder.setField("objectField", null, GridBinaryTestClasses.Company.class);
+
+        builder.build();
+
+
+        builder = binaries().builder("SomeType2");
+
+        builder.setField("dateField", new Date());
+
+        builder.setField("objectField", new GridBinaryTestClasses.Company());
+
+        builder.build();
+
+    }
+
+    /**
+     *
+     */
+    public void testCorrectMetadataNullField2() {
+        BinaryObjectBuilder builder = binaries().builder("SomeType3");
+
+        builder.setField("dateField", null, Date.class);
+
+        builder.setField("objectField", null, GridBinaryTestClasses.Company.class);
+
+        BinaryObject obj = builder.build();
+
+
+        builder = binaries().builder(obj);
+
+        builder.setField("dateField", new Date());
+
+        builder.setField("objectField", new GridBinaryTestClasses.Company());
+
+        builder.build();
+    }
+
+    /**
+     *
+     */
     public void testDateInObjectField() {
         GridBinaryTestClasses.TestObjectContainer obj = new GridBinaryTestClasses.TestObjectContainer();
 
@@ -1053,7 +1178,7 @@ public class BinaryObjectBuilderAdditionalSelfTest extends GridCommonAbstractTes
 
         BinaryObjectBuilderImpl mutableObj = wrap(obj);
 
-        Date[] arr = { new Date() };
+        Date[] arr = {new Date()};
 
         mutableObj.setField("foo", arr);
 
@@ -1072,7 +1197,7 @@ public class BinaryObjectBuilderAdditionalSelfTest extends GridCommonAbstractTes
 
         BinaryObjectBuilderImpl mutableObj = wrap(obj);
 
-        Timestamp[] arr = { new Timestamp(100020003) };
+        Timestamp[] arr = {new Timestamp(100020003)};
 
         mutableObj.setField("foo", arr);
 
@@ -1264,7 +1389,7 @@ public class BinaryObjectBuilderAdditionalSelfTest extends GridCommonAbstractTes
         CacheObjectBinaryProcessorImpl processor = (CacheObjectBinaryProcessorImpl)(
             (IgniteBinaryImpl)binaries()).processor();
 
-        return new BinaryObjectBuilderImpl(processor.binaryContext(), processor.typeId(aCls.getName()), 
+        return new BinaryObjectBuilderImpl(processor.binaryContext(), processor.typeId(aCls.getName()),
             processor.binaryContext().userTypeName(aCls.getName()));
     }
 
@@ -1305,7 +1430,7 @@ public class BinaryObjectBuilderAdditionalSelfTest extends GridCommonAbstractTes
         root.setField("linkedHashSet", linkedHashSet);
 
         root.setField("singletonList", Collections.singletonList(Integer.MAX_VALUE), Collection.class);
-        root.setField("singletonSet",  Collections.singleton(Integer.MAX_VALUE), Collection.class);
+        root.setField("singletonSet", Collections.singleton(Integer.MAX_VALUE), Collection.class);
 
         // maps
         root.setField("hashMap", hashMap);
@@ -1319,7 +1444,7 @@ public class BinaryObjectBuilderAdditionalSelfTest extends GridCommonAbstractTes
         root.setField("asMap", Collections.singletonMap("key", "val"));
         root.setField("asListHint", Collections.singletonList(Integer.MAX_VALUE), List.class);
         root.setField("asSetHint", Collections.singleton(Integer.MAX_VALUE), Set.class);
-        root.setField("asMapHint", (AbstractMap) Collections.singletonMap("key", "val"), AbstractMap.class);
+        root.setField("asMapHint", (AbstractMap)Collections.singletonMap("key", "val"), AbstractMap.class);
 
         BinaryObject binaryObj = root.build();