ATLAS-1419: fix entity-update to retain value of attributes when not provided
Signed-off-by: Madhan Neethiraj <[email protected]> Project: http://git-wip-us.apache.org/repos/asf/incubator-atlas/repo Commit: http://git-wip-us.apache.org/repos/asf/incubator-atlas/commit/dc0b2944 Tree: http://git-wip-us.apache.org/repos/asf/incubator-atlas/tree/dc0b2944 Diff: http://git-wip-us.apache.org/repos/asf/incubator-atlas/diff/dc0b2944 Branch: refs/heads/0.7-incubating Commit: dc0b29446304e7de71e2f6ded9c233e46ff5f8c8 Parents: 9325dd6 Author: Suma Shivaprasad <[email protected]> Authored: Thu Dec 22 14:23:23 2016 -0800 Committer: Madhan Neethiraj <[email protected]> Committed: Tue Dec 27 15:52:31 2016 -0800 ---------------------------------------------------------------------- release-log.txt | 1 + .../graph/TypedInstanceToGraphMapper.java | 15 +++-- .../test/java/org/apache/atlas/TestUtils.java | 2 + .../service/DefaultMetadataServiceTest.java | 61 +++++++++++++++++++- .../org/apache/atlas/typesystem/Struct.java | 2 +- .../persistence/ReferenceableInstance.java | 4 +- .../typesystem/persistence/StructInstance.java | 36 ++++++++++-- .../atlas/typesystem/types/ClassType.java | 23 +++++--- .../typesystem/types/TypedStructHandler.java | 1 + .../types/ValueConversionException.java | 4 ++ 10 files changed, 126 insertions(+), 23 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/incubator-atlas/blob/dc0b2944/release-log.txt ---------------------------------------------------------------------- diff --git a/release-log.txt b/release-log.txt index 7fcbb16..73867e8 100644 --- a/release-log.txt +++ b/release-log.txt @@ -32,6 +32,7 @@ ATLAS-409 Atlas will not import avro tables with schema read from a file (dosset ATLAS-379 Create sqoop and falcon metadata addons (venkatnrangan,bvellanki,sowmyaramesh via shwethags) ALL CHANGES: +ATLAS-1419 fix entity-update to retain value of attributes when not provided (sumasai) ATLAS-1364 HiveHook : Fix Auth issue with doAs (sumasai) ATLAS-1403 Performance fixes for search, lineage ATLAS-1342 Titan Solrclient - Add timeouts for zookeeper connect and session (sumasai) http://git-wip-us.apache.org/repos/asf/incubator-atlas/blob/dc0b2944/repository/src/main/java/org/apache/atlas/repository/graph/TypedInstanceToGraphMapper.java ---------------------------------------------------------------------- diff --git a/repository/src/main/java/org/apache/atlas/repository/graph/TypedInstanceToGraphMapper.java b/repository/src/main/java/org/apache/atlas/repository/graph/TypedInstanceToGraphMapper.java index 2e0414e..aff21af 100644 --- a/repository/src/main/java/org/apache/atlas/repository/graph/TypedInstanceToGraphMapper.java +++ b/repository/src/main/java/org/apache/atlas/repository/graph/TypedInstanceToGraphMapper.java @@ -191,10 +191,13 @@ public final class TypedInstanceToGraphMapper { void mapAttributeToVertex(ITypedInstance typedInstance, Vertex instanceVertex, AttributeInfo attributeInfo, Operation operation) throws AtlasException { - Object attrValue = typedInstance.get(attributeInfo.name); - LOG.debug("Mapping attribute {} = {}", attributeInfo.name, attrValue); - if (attrValue != null || operation == Operation.UPDATE_FULL) { + final Map<String, Object> valuesMap = typedInstance.getValuesMap(); + if ( valuesMap.containsKey(attributeInfo.name) || operation == Operation.CREATE ) { + + Object attrValue = typedInstance.get(attributeInfo.name); + LOG.debug("Mapping attribute {} = {}", attributeInfo.name, attrValue); + switch (attributeInfo.dataType().getTypeCategory()) { case PRIMITIVE: case ENUM: @@ -681,7 +684,7 @@ public final class TypedInstanceToGraphMapper { final String vertexPropertyName = GraphHelper.getQualifiedFieldName(typedInstance, attributeInfo); Object propertyValue = null; - if (attrValue == null ) { + if (attrValue == null) { propertyValue = null; } else if (attributeInfo.dataType() == DataTypes.STRING_TYPE) { propertyValue = typedInstance.getString(attributeInfo.name); @@ -706,12 +709,12 @@ public final class TypedInstanceToGraphMapper { } else if (attributeInfo.dataType() == DataTypes.DATE_TYPE) { final Date dateVal = typedInstance.getDate(attributeInfo.name); //Convert Property value to Long while persisting - if(dateVal != null) { + if (dateVal != null) { propertyValue = dateVal.getTime(); } } else if (attributeInfo.dataType().getTypeCategory() == DataTypes.TypeCategory.ENUM) { if (attrValue != null) { - propertyValue = ((EnumValue)attrValue).value; + propertyValue = ((EnumValue) attrValue).value; } } http://git-wip-us.apache.org/repos/asf/incubator-atlas/blob/dc0b2944/repository/src/test/java/org/apache/atlas/TestUtils.java ---------------------------------------------------------------------- diff --git a/repository/src/test/java/org/apache/atlas/TestUtils.java b/repository/src/test/java/org/apache/atlas/TestUtils.java index bd9df62..f07f2cf 100755 --- a/repository/src/test/java/org/apache/atlas/TestUtils.java +++ b/repository/src/test/java/org/apache/atlas/TestUtils.java @@ -250,6 +250,8 @@ public final class TestUtils { createClassTypeDef(DATABASE_TYPE, DATABASE_TYPE + _description,ImmutableSet.of(SUPER_TYPE_NAME), TypesUtil.createUniqueRequiredAttrDef(NAME, DataTypes.STRING_TYPE), createOptionalAttrDef("created", DataTypes.DATE_TYPE), + createOptionalAttrDef("isReplicated", DataTypes.BOOLEAN_TYPE), + new AttributeDefinition("parameters", new DataTypes.MapType(DataTypes.STRING_TYPE, DataTypes.STRING_TYPE).getName(), Multiplicity.OPTIONAL, false, null), createRequiredAttrDef("description", DataTypes.STRING_TYPE)); http://git-wip-us.apache.org/repos/asf/incubator-atlas/blob/dc0b2944/repository/src/test/java/org/apache/atlas/service/DefaultMetadataServiceTest.java ---------------------------------------------------------------------- diff --git a/repository/src/test/java/org/apache/atlas/service/DefaultMetadataServiceTest.java b/repository/src/test/java/org/apache/atlas/service/DefaultMetadataServiceTest.java index 6782970..76e08b1 100644 --- a/repository/src/test/java/org/apache/atlas/service/DefaultMetadataServiceTest.java +++ b/repository/src/test/java/org/apache/atlas/service/DefaultMetadataServiceTest.java @@ -77,6 +77,7 @@ import java.util.Map; import static org.apache.atlas.TestUtils.COLUMNS_ATTR_NAME; import static org.apache.atlas.TestUtils.COLUMN_TYPE; +import static org.apache.atlas.TestUtils.DATABASE_TYPE; import static org.apache.atlas.TestUtils.PII; import static org.apache.atlas.TestUtils.TABLE_TYPE; import static org.apache.atlas.TestUtils.createColumnEntity; @@ -871,6 +872,16 @@ public class DefaultMetadataServiceTest { } } + @Test(expectedExceptions = ValueConversionException.class) + public void testCreateRequiredAttrNull() throws Exception { + //Update required attribute + + Referenceable tableEntity = new Referenceable(TABLE_TYPE); + tableEntity.set(NAME, "table_" + TestUtils.randomString()); + + TestUtils.createInstance(metadataService, tableEntity); + Assert.fail("Expected exception while creating with required attribute null"); + } @Test(expectedExceptions = ValueConversionException.class) public void testUpdateRequiredAttrToNull() throws Exception { @@ -887,6 +898,54 @@ public class DefaultMetadataServiceTest { } @Test + public void testCheckOptionalAttrValueRetention() throws Exception { + + Referenceable entity = createDBEntity(); + + String dbId = TestUtils.createInstance(metadataService, entity); + + entity = getEntity(dbId); + + //The optional boolean attribute should have a non-null value + final String isReplicatedAttr = "isReplicated"; + final String paramsAttr = "parameters"; + Assert.assertNotNull(entity.get(isReplicatedAttr)); + Assert.assertEquals(entity.get(isReplicatedAttr), Boolean.FALSE); + Assert.assertNull(entity.get(paramsAttr)); + + //Update to true + entity.set(isReplicatedAttr, Boolean.TRUE); + //Update array + final HashMap<String, String> params = new HashMap<String, String>() {{ put("param1", "val1"); put("param2", "val2"); }}; + entity.set(paramsAttr, params); + //Complete update + updateInstance(entity); + + entity = getEntity(dbId); + + Assert.assertNotNull(entity.get(isReplicatedAttr)); + Assert.assertEquals(entity.get(isReplicatedAttr), Boolean.TRUE); + Assert.assertEquals(entity.get(paramsAttr), params); + + //Complete update without setting the attribute + Referenceable newEntity = createDBEntity(); + //Reset name to the current DB name + newEntity.set(NAME, entity.get(NAME)); + updateInstance(newEntity); + + entity = getEntity(dbId); + Assert.assertNotNull(entity.get(isReplicatedAttr)); + Assert.assertEquals(entity.get(isReplicatedAttr), Boolean.TRUE); + Assert.assertEquals(entity.get(paramsAttr), params); + } + + private Referenceable getEntity(String guid) throws AtlasException { + String entityJson = metadataService.getEntityDefinition(guid); + Assert.assertNotNull(entityJson); + return InstanceSerialization.fromJsonReferenceable(entityJson, true); + } + + @Test public void testUpdateOptionalAttrToNull() throws Exception { String tableDefinitionJson = metadataService.getEntityDefinition(TestUtils.TABLE_TYPE, NAME, (String) table.get(NAME)); @@ -895,7 +954,7 @@ public class DefaultMetadataServiceTest { //Update optional Attribute Assert.assertNotNull(tableDefinition.get("created")); //Update optional attribute - table.setNull("created"); + table.setNull("created"); String newtableId = updateInstance(table).getUpdateEntities().get(0); assertEquals(newtableId, tableId._getId()); http://git-wip-us.apache.org/repos/asf/incubator-atlas/blob/dc0b2944/typesystem/src/main/java/org/apache/atlas/typesystem/Struct.java ---------------------------------------------------------------------- diff --git a/typesystem/src/main/java/org/apache/atlas/typesystem/Struct.java b/typesystem/src/main/java/org/apache/atlas/typesystem/Struct.java index 8f32b1a..f8d2e42 100755 --- a/typesystem/src/main/java/org/apache/atlas/typesystem/Struct.java +++ b/typesystem/src/main/java/org/apache/atlas/typesystem/Struct.java @@ -69,7 +69,7 @@ public class Struct implements IStruct { @Override public void setNull(String attrName) throws AtlasException { - values.remove(attrName); + values.put(attrName, null); } @Override http://git-wip-us.apache.org/repos/asf/incubator-atlas/blob/dc0b2944/typesystem/src/main/java/org/apache/atlas/typesystem/persistence/ReferenceableInstance.java ---------------------------------------------------------------------- diff --git a/typesystem/src/main/java/org/apache/atlas/typesystem/persistence/ReferenceableInstance.java b/typesystem/src/main/java/org/apache/atlas/typesystem/persistence/ReferenceableInstance.java index 561cb62..4e21410 100755 --- a/typesystem/src/main/java/org/apache/atlas/typesystem/persistence/ReferenceableInstance.java +++ b/typesystem/src/main/java/org/apache/atlas/typesystem/persistence/ReferenceableInstance.java @@ -48,11 +48,11 @@ public class ReferenceableInstance extends StructInstance implements ITypedRefer public ReferenceableInstance(Id id, String dataTypeName, FieldMapping fieldMapping, boolean[] nullFlags, - boolean[] bools, byte[] bytes, short[] shorts, int[] ints, long[] longs, float[] floats, double[] doubles, + boolean[] explicitNullFlags, boolean[] bools, byte[] bytes, short[] shorts, int[] ints, long[] longs, float[] floats, double[] doubles, BigDecimal[] bigDecimals, BigInteger[] bigIntegers, Date[] dates, String[] strings, ImmutableList<Object>[] arrays, ImmutableMap<Object, Object>[] maps, StructInstance[] structs, ReferenceableInstance[] referenceableInstances, Id[] ids, ImmutableMap<String, ITypedStruct> traits) { - super(dataTypeName, fieldMapping, nullFlags, bools, bytes, shorts, ints, longs, floats, doubles, bigDecimals, + super(dataTypeName, fieldMapping, nullFlags, explicitNullFlags, bools, bytes, shorts, ints, longs, floats, doubles, bigDecimals, bigIntegers, dates, strings, arrays, maps, structs, referenceableInstances, ids); this.id = id; this.traits = traits; http://git-wip-us.apache.org/repos/asf/incubator-atlas/blob/dc0b2944/typesystem/src/main/java/org/apache/atlas/typesystem/persistence/StructInstance.java ---------------------------------------------------------------------- diff --git a/typesystem/src/main/java/org/apache/atlas/typesystem/persistence/StructInstance.java b/typesystem/src/main/java/org/apache/atlas/typesystem/persistence/StructInstance.java index 6fb2087..cdfaad0 100755 --- a/typesystem/src/main/java/org/apache/atlas/typesystem/persistence/StructInstance.java +++ b/typesystem/src/main/java/org/apache/atlas/typesystem/persistence/StructInstance.java @@ -45,6 +45,7 @@ public class StructInstance implements ITypedStruct { public final String dataTypeName; public final FieldMapping fieldMapping; public final boolean nullFlags[]; + public final boolean explicitSets[]; public final boolean[] bools; public final byte[] bytes; public final short[] shorts; @@ -62,7 +63,7 @@ public class StructInstance implements ITypedStruct { public final ReferenceableInstance[] referenceables; public final Id[] ids; - public StructInstance(String dataTypeName, FieldMapping fieldMapping, boolean[] nullFlags, boolean[] bools, + public StructInstance(String dataTypeName, FieldMapping fieldMapping, boolean[] nullFlags, boolean[] explicitSets, boolean[] bools, byte[] bytes, short[] shorts, int[] ints, long[] longs, float[] floats, double[] doubles, BigDecimal[] bigDecimals, BigInteger[] bigIntegers, Date[] dates, String[] strings, ImmutableList<Object>[] arrays, ImmutableMap<Object, Object>[] maps, StructInstance[] structs, @@ -71,6 +72,7 @@ public class StructInstance implements ITypedStruct { this.dataTypeName = dataTypeName; this.fieldMapping = fieldMapping; this.nullFlags = nullFlags; + this.explicitSets = explicitSets; this.bools = bools; this.bytes = bytes; this.shorts = shorts; @@ -91,6 +93,10 @@ public class StructInstance implements ITypedStruct { for (int i = 0; i < nullFlags.length; i++) { nullFlags[i] = true; } + + for (int i = 0; i < explicitSets.length; i++) { + explicitSets[i] = false; + } } @Override @@ -108,10 +114,13 @@ public class StructInstance implements ITypedStruct { if (i == null) { throw new ValueConversionException(getTypeName(), val, "Unknown field " + attrName); } + int pos = fieldMapping.fieldPos.get(attrName); int nullPos = fieldMapping.fieldNullPos.get(attrName); Object cVal = null; + explicitSets[nullPos] = true; + if (val != null && val instanceof Id) { ClassType clsType = TypeSystem.getInstance().getDataType(ClassType.class, i.dataType().getName()); clsType.validateId((Id) val); @@ -179,7 +188,11 @@ public class StructInstance implements ITypedStruct { int nullPos = fieldMapping.fieldNullPos.get(attrName); if (nullFlags[nullPos]) { - return null; + if ( i.dataType().getTypeCategory() == DataTypes.TypeCategory.PRIMITIVE) { + return ((DataTypes.PrimitiveType) i.dataType()).nullValue(); + } else { + return null; + } } if (i.dataType() == DataTypes.BOOLEAN_TYPE) { @@ -231,6 +244,7 @@ public class StructInstance implements ITypedStruct { } int nullPos = fieldMapping.fieldNullPos.get(attrName); nullFlags[nullPos] = true; + explicitSets[nullPos] = true; int pos = fieldMapping.fieldPos.get(attrName); @@ -263,10 +277,12 @@ public class StructInstance implements ITypedStruct { */ @Override public Map<String, Object> getValuesMap() throws AtlasException { - Map<String, Object> m = new HashMap<>(); for (String attr : fieldMapping.fields.keySet()) { - m.put(attr, get(attr)); + int pos = fieldMapping.fieldNullPos.get(attr); + if ( explicitSets[pos] ) { + m.put(attr, get(attr)); + } } return m; } @@ -531,6 +547,7 @@ public class StructInstance implements ITypedStruct { nullFlags[nullPos] = false; bools[pos] = val; + explicitSets[nullPos] = true; } public void setByte(String attrName, byte val) throws AtlasException { @@ -550,6 +567,7 @@ public class StructInstance implements ITypedStruct { nullFlags[nullPos] = false; bytes[pos] = val; + explicitSets[nullPos] = true; } public void setShort(String attrName, short val) throws AtlasException { @@ -569,6 +587,7 @@ public class StructInstance implements ITypedStruct { nullFlags[nullPos] = false; shorts[pos] = val; + explicitSets[nullPos] = true; } public void setInt(String attrName, int val) throws AtlasException { @@ -588,6 +607,7 @@ public class StructInstance implements ITypedStruct { nullFlags[nullPos] = false; ints[pos] = val; + explicitSets[nullPos] = true; } public void setLong(String attrName, long val) throws AtlasException { @@ -607,6 +627,7 @@ public class StructInstance implements ITypedStruct { nullFlags[nullPos] = false; longs[pos] = val; + explicitSets[nullPos] = true; } public void setFloat(String attrName, float val) throws AtlasException { @@ -626,6 +647,7 @@ public class StructInstance implements ITypedStruct { nullFlags[nullPos] = false; floats[pos] = val; + explicitSets[nullPos] = true; } public void setDouble(String attrName, double val) throws AtlasException { @@ -645,6 +667,7 @@ public class StructInstance implements ITypedStruct { nullFlags[nullPos] = false; doubles[pos] = val; + explicitSets[nullPos] = true; } public void setBigInt(String attrName, BigInteger val) throws AtlasException { @@ -664,6 +687,7 @@ public class StructInstance implements ITypedStruct { nullFlags[nullPos] = val == null; bigIntegers[pos] = val; + explicitSets[nullPos] = true; } public void setBigDecimal(String attrName, BigDecimal val) throws AtlasException { @@ -683,6 +707,7 @@ public class StructInstance implements ITypedStruct { nullFlags[nullPos] = val == null; bigDecimals[pos] = val; + explicitSets[nullPos] = true; } public void setDate(String attrName, Date val) throws AtlasException { @@ -702,9 +727,11 @@ public class StructInstance implements ITypedStruct { nullFlags[nullPos] = val == null; dates[pos] = val; + explicitSets[nullPos] = true; } public void setString(String attrName, String val) throws AtlasException { + AttributeInfo i = fieldMapping.fields.get(attrName); if (i == null) { throw new AtlasException(String.format("Unknown field %s for Struct %s", attrName, getTypeName())); @@ -721,6 +748,7 @@ public class StructInstance implements ITypedStruct { nullFlags[nullPos] = val == null; strings[pos] = val; + explicitSets[nullPos] = true; } @Override http://git-wip-us.apache.org/repos/asf/incubator-atlas/blob/dc0b2944/typesystem/src/main/java/org/apache/atlas/typesystem/types/ClassType.java ---------------------------------------------------------------------- diff --git a/typesystem/src/main/java/org/apache/atlas/typesystem/types/ClassType.java b/typesystem/src/main/java/org/apache/atlas/typesystem/types/ClassType.java index c56987a..24d2750 100755 --- a/typesystem/src/main/java/org/apache/atlas/typesystem/types/ClassType.java +++ b/typesystem/src/main/java/org/apache/atlas/typesystem/types/ClassType.java @@ -33,15 +33,13 @@ import org.apache.atlas.typesystem.Struct; import org.apache.atlas.typesystem.persistence.Id; import org.apache.atlas.typesystem.persistence.ReferenceableInstance; import org.apache.atlas.typesystem.persistence.StructInstance; +import scala.tools.cmd.gen.AnyVals; import java.math.BigDecimal; import java.math.BigInteger; import java.nio.charset.Charset; import java.security.MessageDigest; -import java.util.Date; -import java.util.List; -import java.util.Map; -import java.util.Set; +import java.util.*; public class ClassType extends HierarchicalType<ClassType, IReferenceableInstance> implements IConstructableType<IReferenceableInstance, ITypedReferenceableInstance> { @@ -139,10 +137,17 @@ public class ClassType extends HierarchicalType<ClassType, IReferenceableInstanc } } - try { - tr.set(attrKey, aVal); - } catch (ValueConversionException ve) { - throw new ValueConversionException(this, val, ve); + if(!i.multiplicity.nullAllowed() && !s.getValuesMap().containsKey(attrKey)){ + throw new ValueConversionException.NullConversionException(i.multiplicity, + String.format(" Value expected for required attribute %s", i.name)); + } else { + try { + if (s.getValuesMap().containsKey(attrKey)) { + tr.set(attrKey, aVal); + } + } catch (ValueConversionException ve) { + throw new ValueConversionException(this, val, ve); + } } } @@ -188,7 +193,7 @@ public class ClassType extends HierarchicalType<ClassType, IReferenceableInstanc } return new ReferenceableInstance(id == null ? new Id(getName()) : id, getName(), fieldMapping, - new boolean[fieldMapping.fields.size()], + new boolean[fieldMapping.fields.size()], new boolean[fieldMapping.fields.size()], fieldMapping.numBools == 0 ? null : new boolean[fieldMapping.numBools], fieldMapping.numBytes == 0 ? null : new byte[fieldMapping.numBytes], fieldMapping.numShorts == 0 ? null : new short[fieldMapping.numShorts], http://git-wip-us.apache.org/repos/asf/incubator-atlas/blob/dc0b2944/typesystem/src/main/java/org/apache/atlas/typesystem/types/TypedStructHandler.java ---------------------------------------------------------------------- diff --git a/typesystem/src/main/java/org/apache/atlas/typesystem/types/TypedStructHandler.java b/typesystem/src/main/java/org/apache/atlas/typesystem/types/TypedStructHandler.java index b97669a..eca7d2e 100755 --- a/typesystem/src/main/java/org/apache/atlas/typesystem/types/TypedStructHandler.java +++ b/typesystem/src/main/java/org/apache/atlas/typesystem/types/TypedStructHandler.java @@ -88,6 +88,7 @@ public class TypedStructHandler { public ITypedStruct createInstance() { return new StructInstance(structType.getName(), fieldMapping, new boolean[fieldMapping.fields.size()], + new boolean[fieldMapping.fields.size()], fieldMapping.numBools == 0 ? null : new boolean[fieldMapping.numBools], fieldMapping.numBytes == 0 ? null : new byte[fieldMapping.numBytes], fieldMapping.numShorts == 0 ? null : new short[fieldMapping.numShorts], http://git-wip-us.apache.org/repos/asf/incubator-atlas/blob/dc0b2944/typesystem/src/main/java/org/apache/atlas/typesystem/types/ValueConversionException.java ---------------------------------------------------------------------- diff --git a/typesystem/src/main/java/org/apache/atlas/typesystem/types/ValueConversionException.java b/typesystem/src/main/java/org/apache/atlas/typesystem/types/ValueConversionException.java index 7fe667a..f756135 100755 --- a/typesystem/src/main/java/org/apache/atlas/typesystem/types/ValueConversionException.java +++ b/typesystem/src/main/java/org/apache/atlas/typesystem/types/ValueConversionException.java @@ -52,6 +52,10 @@ public class ValueConversionException extends AtlasException { super(String.format("Null value not allowed for multiplicty %s", m)); } + public NullConversionException(Multiplicity m, String msg){ + super(String.format("Null value not allowed for multiplicty %s . Message %s", m, msg)); + } + public NullConversionException(String msg, Exception e) { super(msg, e); }
