ATLAS-1419: Entity attribute values are overridden to null when their value is…

release-log.txt

@@ -9,6 +9,7 @@ ATLAS-1060 Add composite indexes for exact match performance improvements for al
 ATLAS-1127 Modify creation and modification timestamps to Date instead of Long(sumasai)
 
 ALL CHANGES:
+ATLAS-1419 update entity-update API impl to preserve value of entity attribute when no value is provided
 ATLAS-1346 Search API to return empty list/container object instead of exception (apoorvnaik via mneethiraj)
 ATLAS-1428 Create of entityDef type fails with type already exists exception (sarath.kum4r@gmail.com via mneethiraj)
 ATLAS-1421 Regression : HTML is displayed for deleted entities in search-result and entity-details pages (Kalyanikashikar via mneethiraj)

repository/src/main/java/org/apache/atlas/repository/graph/TypedInstanceToGraphMapper.java

@@ -194,10 +194,12 @@ public final class TypedInstanceToGraphMapper {
 
     void mapAttributeToVertex(ITypedInstance typedInstance, AtlasVertex 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) {
+        if ( typedInstance.isValueSet(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:
@@ -323,10 +325,6 @@ public final class TypedInstanceToGraphMapper {
         List newElements = (List) typedInstance.get(attributeInfo.name);
         boolean newAttributeEmpty = (newElements == null || newElements.isEmpty());
 
-        if (newAttributeEmpty && operation != Operation.UPDATE_FULL) {
-            return;
-        }
-
         IDataType elementType = ((DataTypes.ArrayType) attributeInfo.dataType()).getElemType();
         String propertyName = GraphHelper.getQualifiedFieldName(typedInstance, attributeInfo);
         
@@ -401,9 +399,6 @@ public final class TypedInstanceToGraphMapper {
                 (Map<Object, Object>) typedInstance.get(attributeInfo.name);
 
         boolean newAttributeEmpty = (newAttribute == null || newAttribute.isEmpty());
-        if (newAttributeEmpty && operation != Operation.UPDATE_FULL) {
-            return;
-        }
 
         IDataType elementType = ((DataTypes.MapType) attributeInfo.dataType()).getValueType();
         String propertyName = GraphHelper.getQualifiedFieldName(typedInstance, attributeInfo);
@@ -687,7 +682,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);
@@ -712,12 +707,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;
             }
         }
 

repository/src/main/java/org/apache/atlas/services/DefaultMetadataService.java

@@ -460,30 +460,30 @@ public class DefaultMetadataService implements MetadataService, ActiveStateChang
 
             DataTypes.TypeCategory attrTypeCategory = attributeInfo.dataType().getTypeCategory();
             Object value = updatedEntity.get(attributeName);
-            if (value != null) {
-                switch (attrTypeCategory) {
-                    case CLASS:
+            switch (attrTypeCategory) {
+                case CLASS:
+                    if (value != null) {
                         if (value instanceof Referenceable) {
                             newInstance.set(attributeName, value);
                         } else {
                             Id id = new Id((String) value, 0, attributeInfo.dataType().getName());
                             newInstance.set(attributeName, id);
                         }
-                        break;
-
-                    case ENUM:
-                    case PRIMITIVE:
-                    case ARRAY:
-                    case STRUCT:
-                    case MAP:
-                        newInstance.set(attributeName, value);
-                        break;
-
-                    case TRAIT:
-                        //TODO - handle trait updates as well?
-                    default:
-                        throw new AtlasException("Update of " + attrTypeCategory + " is not supported");
-                }
+                    }
+                    break;
+
+                case ENUM:
+                case PRIMITIVE:
+                case ARRAY:
+                case STRUCT:
+                case MAP:
+                    newInstance.set(attributeName, value);
+                    break;
+
+                case TRAIT:
+                    //TODO - handle trait updates as well?
+                default:
+                    throw new AtlasException("Update of " + attrTypeCategory + " is not supported");
             }
         }
 

repository/src/test/java/org/apache/atlas/TestUtils.java

@@ -333,6 +333,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));
 
 

repository/src/test/java/org/apache/atlas/service/DefaultMetadataServiceTest.java

@@ -81,6 +81,7 @@ import java.util.Collections;
 
 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;
@@ -865,6 +866,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 {
@@ -881,6 +892,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.getEntityDefinitionJson(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));
@@ -889,7 +948,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());

typesystem/src/main/java/org/apache/atlas/typesystem/ITypedInstance.java

@@ -83,4 +83,6 @@ public interface ITypedInstance extends IInstance {
     void setString(String attrName, String val) throws AtlasException;
 
     String getSignatureHash(MessageDigest digester) throws AtlasException;
+    
+    boolean isValueSet(String attrName) throws AtlasException;
 }

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

typesystem/src/main/java/org/apache/atlas/typesystem/persistence/Id.java

@@ -281,6 +281,10 @@ public class Id implements ITypedReferenceableInstance {
         throw new AtlasException("Get/Set not supported on an Id object");
     }
 
+    public boolean isValueSet(String attrName) throws AtlasException {
+        throw new AtlasException("Attributes not set on an Id object");
+    }
+
     @Override
     public String getSignatureHash(MessageDigest digester) throws AtlasException {
         digester.update(id.getBytes(Charset.forName("UTF-8")));

typesystem/src/main/java/org/apache/atlas/typesystem/persistence/ReferenceableInstance.java

@@ -49,11 +49,11 @@ public class ReferenceableInstance extends StructInstance implements ITypedRefer
 
 
     public ReferenceableInstance(Id id, String dataTypeName, AtlasSystemAttributes systemAttributes, FieldMapping fieldMapping, boolean[] nullFlags,
-            boolean[] bools, byte[] bytes, short[] shorts, int[] ints, long[] longs, float[] floats, double[] doubles,
+            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,
             ReferenceableInstance[] referenceableInstances, Id[] ids, ImmutableMap<String, ITypedStruct> traits) {
-        super(dataTypeName, fieldMapping, nullFlags, bools, bytes, shorts, ints, longs, floats, doubles, bigDecimals,
+        super(dataTypeName, fieldMapping, nullFlags, explicitSets, bools, bytes, shorts, ints, longs, floats, doubles, bigDecimals,
                 bigIntegers, dates, strings, arrays, maps, structs, referenceableInstances, ids);
         this.id = id;
         this.traits = traits;
@@ -62,10 +62,10 @@ public class ReferenceableInstance extends StructInstance implements ITypedRefer
             b.add(t);
         }
         this.traitNames = b.build();
-        if(systemAttributes == null){
+        if (systemAttributes == null){
             this.systemAttributes = new AtlasSystemAttributes();
         }
-        else{
+        else {
             this.systemAttributes = systemAttributes;
         }
     }

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
@@ -746,6 +774,12 @@ public class StructInstance implements ITypedStruct {
     }
 
     @Override
+    public boolean isValueSet(final String attrName) throws AtlasException {
+        int nullPos = fieldMapping.fieldNullPos.get(attrName);
+        return explicitSets[nullPos];
+    }
+
+    @Override
     public String toShortString() {
         return String.format("struct[type=%s]", dataTypeName);
     }

typesystem/src/main/java/org/apache/atlas/typesystem/types/ClassType.java

@@ -35,15 +35,13 @@ import org.apache.atlas.typesystem.persistence.AtlasSystemAttributes;
 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> {
@@ -150,10 +148,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);
+                        }
                     }
                 }
 
@@ -203,7 +208,7 @@ public class ClassType extends HierarchicalType<ClassType, IReferenceableInstanc
         }
 
         return new ReferenceableInstance(id == null ? new Id(getName()) : id, getName(), systemAttributes, 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],

typesystem/src/main/java/org/apache/atlas/typesystem/types/TypedStructHandler.java

@@ -89,6 +89,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],

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