ATLAS-1514 Remove duplicates from class array attribute when target is deleted (dkantor)

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-1514 Remove duplicates from class array attribute when target is deleted (dkantor)
 ATLAS-1369 Optimize Gremlin queries generated by DSL translator (jnhagelb) 
 ATLAS-1513 updated AtlasEntityType with methods to get foreign-key references; added helper methods in AtlasAttribute (mneethiraj via kevalbhatt)
 ATLAS-1502 added configuration to restrict entity-types editable via UI (Kalyanikashikar via mneethiraj)

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

@@ -354,7 +354,10 @@ public abstract class DeleteHandler {
                                         attributeName);
                             }
 
-                            elements.remove(elementEdge.getId().toString());
+                            // Remove all occurrences of the edge ID from the list.
+                            // This prevents dangling edge IDs (i.e. edge IDs for deleted edges)
+                            // from the remaining in the list if there are duplicates.
+                            elements.removeAll(Collections.singletonList(elementEdge.getId().toString()));
                             GraphHelper.setProperty(outVertex, propertyName, elements);
                             break;
 

repository/src/main/java/org/apache/atlas/repository/store/graph/v1/DeleteHandlerV1.java

@@ -447,7 +447,10 @@ public abstract class DeleteHandlerV1 {
                             //but when column is deleted, table will not reference the deleted column
                             LOG.debug("Removing edge {} from the array attribute {}", string(elementEdge),
                                 attributeName);
-                            elements.remove(elementEdge.getId().toString());
+                            // Remove all occurrences of the edge ID from the list.
+                            // This prevents dangling edge IDs (i.e. edge IDs for deleted edges)
+                            // from the remaining in the list if there are duplicates.
+                            elements.removeAll(Collections.singletonList(elementEdge.getId().toString()));
                             GraphHelper.setProperty(outVertex, propertyName, elements);
                             break;
 

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

@@ -1048,6 +1048,97 @@ public abstract class GraphBackedMetadataRepositoryDeleteTestBase {
             Arrays.asList(mapOwnerGuid, mapValueGuid, mapValueReferencerContainerGuid, mapValueReferencerGuid)));
     }
 
+    @Test
+    public void testDeleteMixOfExistentAndNonExistentEntities() throws Exception {
+        ITypedReferenceableInstance entity1 = compositeMapValueType.createInstance();
+        ITypedReferenceableInstance entity2 = compositeMapValueType.createInstance();
+        List<String> createEntitiesResult = repositoryService.createEntities(entity1, entity2);
+        Assert.assertEquals(createEntitiesResult.size(), 2);
+        List<String> guids = Arrays.asList(createEntitiesResult.get(0), "non-existent-guid1", "non-existent-guid2", createEntitiesResult.get(1));
+        EntityResult deleteEntitiesResult = repositoryService.deleteEntities(guids);
+        Assert.assertEquals(deleteEntitiesResult.getDeletedEntities().size(), 2);
+        Assert.assertTrue(deleteEntitiesResult.getDeletedEntities().containsAll(createEntitiesResult));
+    }
+
+    @Test
+    public void testDeleteMixOfNullAndNonNullGuids() throws Exception {
+        ITypedReferenceableInstance entity1 = compositeMapValueType.createInstance();
+        ITypedReferenceableInstance entity2 = compositeMapValueType.createInstance();
+        List<String> createEntitiesResult = repositoryService.createEntities(entity1, entity2);
+        Assert.assertEquals(createEntitiesResult.size(), 2);
+        List<String> guids = Arrays.asList(createEntitiesResult.get(0), null, null, createEntitiesResult.get(1));
+        EntityResult deleteEntitiesResult = repositoryService.deleteEntities(guids);
+        Assert.assertEquals(deleteEntitiesResult.getDeletedEntities().size(), 2);
+        Assert.assertTrue(deleteEntitiesResult.getDeletedEntities().containsAll(createEntitiesResult));
+    }
+
+    @Test
+    public void testDeleteCompositeEntityAndContainer() throws Exception {
+        Referenceable db = createDBEntity();
+        String dbId = createInstance(db);
+
+        Referenceable column = createColumnEntity();
+        String colId = createInstance(column);
+
+        Referenceable table1 = createTableEntity(dbId);
+        table1.set(COLUMNS_ATTR_NAME, Arrays.asList(new Id(colId, 0, COLUMN_TYPE)));
+        String table1Id = createInstance(table1);
+        Referenceable table2 = createTableEntity(dbId);
+        String table2Id = createInstance(table2);
+
+        // Delete the tables and column
+        AtlasClient.EntityResult entityResult = deleteEntities(table1Id, colId, table2Id);
+        Assert.assertEquals(entityResult.getDeletedEntities().size(), 3);
+        Assert.assertTrue(entityResult.getDeletedEntities().containsAll(Arrays.asList(colId, table1Id, table2Id)));
+        assertEntityDeleted(table1Id);
+        assertEntityDeleted(colId);
+        assertEntityDeleted(table2Id);
+    }
+
+    @Test
+    public void testDeleteEntityWithDuplicateReferenceListElements() throws Exception {
+        // Create a table entity, with 2 composite column entities
+        Referenceable dbEntity = createDBEntity();
+        String dbGuid = createInstance(dbEntity);
+        Referenceable table1Entity = createTableEntity(dbGuid);
+        String tableName = TestUtils.randomString();
+        table1Entity.set(NAME, tableName);
+        Referenceable col1 = createColumnEntity();
+        col1.set(NAME, TestUtils.randomString());
+        Referenceable col2 = createColumnEntity();
+        col2.set(NAME, TestUtils.randomString());
+        // Populate columns reference list with duplicates.
+        table1Entity.set(COLUMNS_ATTR_NAME, ImmutableList.of(col1, col2, col1, col2));
+        ClassType dataType = typeSystem.getDataType(ClassType.class, table1Entity.getTypeName());
+        ITypedReferenceableInstance instance = dataType.convert(table1Entity, Multiplicity.REQUIRED);
+        TestUtils.resetRequestContext();
+        List<String> result = repositoryService.createEntities(instance);
+        Assert.assertEquals(result.size(), 3);
+        ITypedReferenceableInstance entityDefinition = repositoryService.getEntityDefinition(TABLE_TYPE, NAME, tableName);
+        String tableGuid = entityDefinition.getId()._getId();
+        Object attrValue = entityDefinition.get(COLUMNS_ATTR_NAME);
+        assertTrue(attrValue instanceof List);
+        List<ITypedReferenceableInstance> columns = (List<ITypedReferenceableInstance>) attrValue;
+        Assert.assertEquals(columns.size(), 4);
+        TestUtils.resetRequestContext();
+        String columnGuid = columns.get(0).getId()._getId();
+
+        // Delete one of the columns.
+        EntityResult deleteResult = repositoryService.deleteEntities(Collections.singletonList(columnGuid));
+        Assert.assertEquals(deleteResult.getDeletedEntities().size(), 1);
+        Assert.assertTrue(deleteResult.getDeletedEntities().contains(columnGuid));
+        Assert.assertEquals(deleteResult.getUpdateEntities().size(), 1);
+        Assert.assertTrue(deleteResult.getUpdateEntities().contains(tableGuid));
+
+        // Verify the duplicate edge IDs were all removed from reference property list.
+        AtlasVertex tableVertex = GraphHelper.getInstance().getVertexForGUID(tableGuid);
+        String columnsPropertyName = GraphHelper.getQualifiedFieldName(dataType, COLUMNS_ATTR_NAME);
+        List columnsPropertyValue = tableVertex.getProperty(columnsPropertyName, List.class);
+        verifyTestDeleteEntityWithDuplicateReferenceListElements(columnsPropertyValue);
+    }
+
+    protected abstract void verifyTestDeleteEntityWithDuplicateReferenceListElements(List columnsPropertyValue);
+
     private String createHrDeptGraph() throws Exception {
         ITypedReferenceableInstance hrDept = TestUtils.createDeptEg1(typeSystem);
 

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

@@ -202,4 +202,12 @@ public class GraphBackedRepositoryHardDeleteTest extends GraphBackedMetadataRepo
             // good
         }
     }
+
+    @Override
+    protected void verifyTestDeleteEntityWithDuplicateReferenceListElements(List columnsPropertyValue) {
+
+        // With hard deletes enabled, verify that duplicate edge IDs for deleted edges
+        // were removed from the array property list.
+        Assert.assertEquals(columnsPropertyValue.size(), 2);
+    }
 }

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

@@ -229,4 +229,12 @@ public class GraphBackedRepositorySoftDeleteTest extends GraphBackedMetadataRepo
         ITypedReferenceableInstance hrDept = repositoryService.getEntityDefinition(hrDeptGuid);
         Assert.assertEquals(hrDept.getId().getState(), EntityState.DELETED);
     }
+
+    @Override
+    protected void verifyTestDeleteEntityWithDuplicateReferenceListElements(List columnsPropertyValue) {
+
+        // With soft deletes enabled, verify that edge IDs for deleted edges
+        // were not removed from the array property list.
+        Assert.assertEquals(columnsPropertyValue.size(), 4);
+    }
 }