ATLAS-667 Entity delete should check for required reverse references ( dkantor via sumasai )

release-log.txt

@@ -20,6 +20,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-667 Entity delete should check for required reverse references ( dkantor via sumasai )
 ATLAS-738 Add query ability on system properties like guid, state, createdtime etc (shwethags)
 ATLAS-692 Create abstraction layer for graph databases (jnhagelb via yhemanth)
 ATLAS-689 Migrate Atlas-Storm integration to use Storm 1.0 dependencies. (svimal2106 via yhemanth)

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

@@ -21,9 +21,11 @@ package org.apache.atlas.repository.graph;
 import com.tinkerpop.blueprints.Direction;
 import com.tinkerpop.blueprints.Edge;
 import com.tinkerpop.blueprints.Vertex;
+
 import org.apache.atlas.AtlasException;
 import org.apache.atlas.RequestContext;
 import org.apache.atlas.repository.Constants;
+import org.apache.atlas.typesystem.exception.NullRequiredAttributeException;
 import org.apache.atlas.typesystem.persistence.Id;
 import org.apache.atlas.typesystem.types.AttributeInfo;
 import org.apache.atlas.typesystem.types.DataTypes;
@@ -243,7 +245,7 @@ public abstract class DeleteHandler {
                 attributeName);
         String typeName = GraphHelper.getTypeName(outVertex);
         String outId = GraphHelper.getIdFromVertex(outVertex);
-        if (outId != null && RequestContext.get().getDeletedEntityIds().contains(outId)) {
+        if (outId != null && RequestContext.get().isDeletedEntity(outId)) {
             //If the reference vertex is marked for deletion, skip updating the reference
             return;
         }
@@ -257,11 +259,14 @@ public abstract class DeleteHandler {
         switch (attributeInfo.dataType().getTypeCategory()) {
         case CLASS:
             //If its class attribute, its the only edge between two vertices
-            //TODO need to enable this
-            //            if (refAttributeInfo.multiplicity == Multiplicity.REQUIRED) {
-            //                throw new AtlasException("Can't set attribute " + refAttributeName + " to null as its required attribute");
-            //            }
-            edge = GraphHelper.getEdgeForLabel(outVertex, edgeLabel);
+            if (attributeInfo.multiplicity.nullAllowed()) {
+                edge = GraphHelper.getEdgeForLabel(outVertex, edgeLabel);
+            }
+            else {
+                // Cannot unset a required attribute.
+                throw new NullRequiredAttributeException("Cannot unset required attribute " + GraphHelper.getQualifiedFieldName(type, attributeName) +
+                    " on " + string(outVertex) + " edge = " + edgeLabel);
+            }
             break;
 
         case ARRAY:
@@ -277,8 +282,15 @@ public abstract class DeleteHandler {
 
                     Vertex elementVertex = elementEdge.getVertex(Direction.IN);
                     if (elementVertex.getId().toString().equals(inVertex.getId().toString())) {
-                        edge = elementEdge;
-
+                        if (attributeInfo.multiplicity.nullAllowed() || elements.size() > attributeInfo.multiplicity.lower) {
+                            edge = elementEdge;
+                        }
+                        else {
+                            // Deleting this edge would violate the attribute's lower bound.
+                            throw new NullRequiredAttributeException(
+                                "Cannot remove array element from required attribute " +
+                                    GraphHelper.getQualifiedFieldName(type, attributeName) + " on " + string(outVertex) + " " + string(elementEdge));
+                        }
                         if (shouldUpdateReverseAttribute || attributeInfo.isComposite) {
                             //if composite attribute, remove the reference as well. else, just remove the edge
                             //for example, when table is deleted, process still references the table
@@ -305,7 +317,15 @@ public abstract class DeleteHandler {
                     Edge mapEdge = graphHelper.getEdgeById(mapEdgeId);
                     Vertex mapVertex = mapEdge.getVertex(Direction.IN);
                     if (mapVertex.getId().toString().equals(inVertex.getId().toString())) {
-                        edge = mapEdge;
+                        if (attributeInfo.multiplicity.nullAllowed() || keys.size() > attributeInfo.multiplicity.lower) {
+                            edge = mapEdge;
+                        }
+                        else {
+                            // Deleting this entry would violate the attribute's lower bound.
+                            throw new NullRequiredAttributeException(
+                                "Cannot remove map entry " + keyPropertyName + " from required attribute " +
+                                    GraphHelper.getQualifiedFieldName(type, attributeName) + " on " + string(outVertex) + " " + string(mapEdge));
+                        }
 
                         if (shouldUpdateReverseAttribute || attributeInfo.isComposite) {
                             //remove this key

repository/src/test/java/org/apache/atlas/repository/graph/GraphBackedRepositoryHardDeleteTest.java

@@ -19,6 +19,7 @@
 package org.apache.atlas.repository.graph;
 
 import com.tinkerpop.blueprints.Vertex;
+
 import org.apache.atlas.AtlasClient;
 import org.apache.atlas.TestUtils;
 import org.apache.atlas.repository.Constants;
@@ -26,6 +27,7 @@ import org.apache.atlas.typesystem.IStruct;
 import org.apache.atlas.typesystem.ITypedReferenceableInstance;
 import org.apache.atlas.typesystem.ITypedStruct;
 import org.apache.atlas.typesystem.exception.EntityNotFoundException;
+import org.apache.atlas.typesystem.exception.NullRequiredAttributeException;
 import org.apache.atlas.typesystem.types.TypeSystem;
 import org.testng.Assert;
 
@@ -75,18 +77,18 @@ public class GraphBackedRepositoryHardDeleteTest extends GraphBackedMetadataRepo
     }
 
     @Override
-    protected void assertTestUpdateEntity_MultiplicityOneNonCompositeReference() throws Exception {
+    protected void assertTestUpdateEntity_MultiplicityOneNonCompositeReference(String janeGuid) throws Exception {
         // Verify that max is no longer a subordinate of jane.
-        ITypedReferenceableInstance jane = repositoryService.getEntityDefinition("Manager", "name", "Jane");
+        ITypedReferenceableInstance jane = repositoryService.getEntityDefinition(janeGuid);
         List<ITypedReferenceableInstance> subordinates = (List<ITypedReferenceableInstance>) jane.get("subordinates");
         Assert.assertEquals(subordinates.size(), 1);
     }
 
     @Override
-    protected void assertTestDisconnectBidirectionalReferences() throws Exception {
+    protected void assertTestDisconnectBidirectionalReferences(String janeGuid) throws Exception {
         // Verify that the Manager.subordinates reference to the deleted employee
         // Max was disconnected.
-        ITypedReferenceableInstance jane = repositoryService.getEntityDefinition("Manager", "name", "Jane");
+        ITypedReferenceableInstance jane = repositoryService.getEntityDefinition(janeGuid);
         List<ITypedReferenceableInstance> subordinates = (List<ITypedReferenceableInstance>) jane.get("subordinates");
         assertEquals(subordinates.size(), 1);
     }
@@ -118,4 +120,11 @@ public class GraphBackedRepositoryHardDeleteTest extends GraphBackedMetadataRepo
         object = mapOwnerVertex.getProperty("MapOwner.biMap.value1");
         assertNull(object);
     }
+
+    @Override
+    protected void assertTestDeleteTargetOfMultiplicityRequiredReference() throws Exception {
+
+        Assert.fail("Lower bound on attribute Manager.subordinates was not enforced - " +
+            NullRequiredAttributeException.class.getSimpleName() + " was expected but none thrown");
+    }
 }

repository/src/test/java/org/apache/atlas/repository/graph/GraphBackedRepositorySoftDeleteTest.java

@@ -19,6 +19,7 @@
 package org.apache.atlas.repository.graph;
 
 import com.tinkerpop.blueprints.Vertex;
+
 import org.apache.atlas.AtlasClient;
 import org.apache.atlas.TestUtils;
 import org.apache.atlas.repository.Constants;
@@ -76,17 +77,17 @@ public class GraphBackedRepositorySoftDeleteTest extends GraphBackedMetadataRepo
     }
 
     @Override
-    protected void assertTestUpdateEntity_MultiplicityOneNonCompositeReference() throws Exception {
-        // Verify that max is no longer a subordinate of jane.
-        ITypedReferenceableInstance jane = repositoryService.getEntityDefinition("Manager", "name", "Jane");
+    protected void assertTestUpdateEntity_MultiplicityOneNonCompositeReference(String janeGuid) throws Exception {
+        // Verify Jane's subordinates reference cardinality is still 2.
+        ITypedReferenceableInstance jane = repositoryService.getEntityDefinition(janeGuid);
         List<ITypedReferenceableInstance> subordinates = (List<ITypedReferenceableInstance>) jane.get("subordinates");
         Assert.assertEquals(subordinates.size(), 2);
     }
 
     @Override
-    protected void assertTestDisconnectBidirectionalReferences() throws Exception {
+    protected void assertTestDisconnectBidirectionalReferences(String janeGuid) throws Exception {
         // Verify that the Manager.subordinates still references deleted employee
-        ITypedReferenceableInstance jane = repositoryService.getEntityDefinition("Manager", "name", "Jane");
+        ITypedReferenceableInstance jane = repositoryService.getEntityDefinition(janeGuid);
         List<ITypedReferenceableInstance> subordinates = (List<ITypedReferenceableInstance>) jane.get("subordinates");
         assertEquals(subordinates.size(), 2);
     }
@@ -95,7 +96,7 @@ public class GraphBackedRepositorySoftDeleteTest extends GraphBackedMetadataRepo
     protected void assertTestDisconnectUnidirectionalArrayReferenceFromStructAndTraitTypes(String structContainerGuid)
             throws Exception {
         // Verify that the unidirectional references from the struct and trait instances
-        // to the deleted entities were disconnected.
+        // to the deleted entities were not disconnected.
         ITypedReferenceableInstance structContainerConvertedEntity =
                 repositoryService.getEntityDefinition(structContainerGuid);
         ITypedStruct struct = (ITypedStruct) structContainerConvertedEntity.get("struct");
@@ -118,4 +119,10 @@ public class GraphBackedRepositorySoftDeleteTest extends GraphBackedMetadataRepo
         assertNotNull(biMap);
         assertEquals(biMap.size(), 1);
     }
+
+    @Override
+    protected void assertTestDeleteTargetOfMultiplicityRequiredReference() throws Exception {
+
+        // No-op - it's ok that no exception was thrown if soft deletes are enabled.
+    }
 }

server-api/src/main/java/org/apache/atlas/RequestContext.java

@@ -107,4 +107,8 @@ public class RequestContext {
     public long getRequestTime() {
         return requestTime;
     }
+    
+    public boolean isDeletedEntity(String entityGuid) {
+        return deletedEntityIds.contains(entityGuid);
+    }
 }

server-api/src/main/java/org/apache/atlas/typesystem/exception/NullRequiredAttributeException.java

+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.atlas.typesystem.exception;
+
+import org.apache.atlas.AtlasException;
+import org.apache.atlas.typesystem.types.Multiplicity;
+
+
+/**
+ * Thrown when a repository operation attempts to
+ * unset an attribute that is defined as required in the
+ * type system.  A required attribute has a non-zero
+ * lower bound in its multiplicity.
+ *
+ * @see Multiplicity#REQUIRED
+ * @see Multiplicity#COLLECTION
+ * @see Multiplicity#SET
+ *
+ */
+public class NullRequiredAttributeException extends AtlasException {
+
+    private static final long serialVersionUID = 4023597038462910948L;
+
+    public NullRequiredAttributeException() {
+        super();
+    }
+
+    public NullRequiredAttributeException(String message, Throwable cause, boolean enableSuppression, boolean writableStackTrace) {
+        super(message, cause, enableSuppression, writableStackTrace);
+    }
+
+    public NullRequiredAttributeException(String message, Throwable cause) {
+        super(message, cause);
+    }
+
+    public NullRequiredAttributeException(String message) {
+        super(message);
+    }
+
+    public NullRequiredAttributeException(Throwable cause) {
+        super(cause);
+    }
+
+}

webapp/src/test/java/org/apache/atlas/notification/EntityNotificationIT.java

@@ -107,7 +107,8 @@ public class EntityNotificationIT extends BaseResourceIT {
     @Test
     public void testDeleteEntity() throws Exception {
         final String tableName = "table-" + randomString();
-        Referenceable tableInstance = createHiveTableInstance(DATABASE_NAME, tableName);
+        final String dbName = "db-" + randomString();
+        Referenceable tableInstance = createHiveTableInstance(dbName, tableName);
         final Id tableId = createInstance(tableInstance);
         final String guid = tableId._getId();