From d06b8229ab0a08b52c9bac87056c30a5b9ad02ac Mon Sep 17 00:00:00 2001 From: Dave Kantor <dkantor@us.ibm.com> Date: Tue, 9 Aug 2016 22:21:25 +0100 Subject: [PATCH] ATLAS-991 Lower bound checking not always disabled for entities being deleted. (dkantor) --- release-log.txt | 1 + repository/src/main/java/org/apache/atlas/repository/graph/DeleteHandler.java | 62 ++++++++++++++++++++++++++++++++++++++++++-------------------- repository/src/main/java/org/apache/atlas/repository/graph/GraphBackedMetadataRepository.java | 15 +++++++++++---- repository/src/main/java/org/apache/atlas/repository/graph/GraphHelper.java | 157 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++----- repository/src/test/java/org/apache/atlas/repository/graph/GraphBackedMetadataRepositoryDeleteTestBase.java | 201 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++---------------------------- repository/src/test/java/org/apache/atlas/repository/graph/GraphBackedRepositoryHardDeleteTest.java | 18 ++++++++++++++++++ repository/src/test/java/org/apache/atlas/repository/graph/GraphBackedRepositorySoftDeleteTest.java | 15 +++++++++++++++ repository/src/test/java/org/apache/atlas/repository/graph/GraphHelperTest.java | 97 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++------------ 8 files changed, 497 insertions(+), 69 deletions(-) diff --git a/release-log.txt b/release-log.txt index 08daa8e..e458343 100644 --- a/release-log.txt +++ b/release-log.txt @@ -6,6 +6,7 @@ INCOMPATIBLE CHANGES: ATLAS-1060 Add composite indexes for exact match performance improvements for all attributes (sumasai via shwethags) ALL CHANGES: +ATLAS-991 Lower bound checking not always disabled for entities being deleted (dkantor) ATLAS-1104 Get outgoing edges by label doesn't work in some cases (shwethags) ATLAS-1106 Fix Build failure due to wrong version in graphdb/common pom (sumasai) ATLAS-1105 Disable HiveLiteralRewriterTest since its not used currently (sumasai) diff --git a/repository/src/main/java/org/apache/atlas/repository/graph/DeleteHandler.java b/repository/src/main/java/org/apache/atlas/repository/graph/DeleteHandler.java index 8d31c1b..957750d 100644 --- a/repository/src/main/java/org/apache/atlas/repository/graph/DeleteHandler.java +++ b/repository/src/main/java/org/apache/atlas/repository/graph/DeleteHandler.java @@ -25,6 +25,7 @@ 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.repository.graph.GraphHelper.VertexInfo; import org.apache.atlas.typesystem.exception.NullRequiredAttributeException; import org.apache.atlas.typesystem.persistence.Id; import org.apache.atlas.typesystem.types.AttributeInfo; @@ -38,8 +39,11 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; import java.util.ArrayList; +import java.util.Collections; +import java.util.HashSet; import java.util.Iterator; import java.util.List; +import java.util.Set; import static org.apache.atlas.repository.graph.GraphHelper.EDGE_LABEL_PREFIX; import static org.apache.atlas.repository.graph.GraphHelper.string; @@ -60,24 +64,42 @@ public abstract class DeleteHandler { } /** - * Deletes the entity vertex - deletes the traits and all the references - * @param instanceVertex + * Deletes the specified entity vertices. + * Deletes any traits, composite entities, and structs owned by each entity. + * Also deletes all the references from/to the entity. + * + * @param instanceVertices * @throws AtlasException */ - public void deleteEntity(Vertex instanceVertex) throws AtlasException { - RequestContext requestContext = RequestContext.get(); - String guid = GraphHelper.getIdFromVertex(instanceVertex); - Id.EntityState state = GraphHelper.getState(instanceVertex); - if (requestContext.getDeletedEntityIds().contains(guid) || state == Id.EntityState.DELETED) { - LOG.debug("Skipping deleting {} as its already deleted", guid); - return; - } - String typeName = GraphHelper.getTypeName(instanceVertex); - requestContext.recordEntityDelete(guid, typeName); + public void deleteEntities(List<Vertex> instanceVertices) throws AtlasException { + RequestContext requestContext = RequestContext.get(); + + Set<Vertex> deletionCandidateVertices = new HashSet<>(); - deleteAllTraits(instanceVertex); + for (Vertex instanceVertex : instanceVertices) { + String guid = GraphHelper.getIdFromVertex(instanceVertex); + Id.EntityState state = GraphHelper.getState(instanceVertex); + if (requestContext.getDeletedEntityIds().contains(guid) || state == Id.EntityState.DELETED) { + LOG.debug("Skipping deletion of {} as it is already deleted", guid); + continue; + } - deleteTypeVertex(instanceVertex, false); + // Get GUIDs and vertices for all deletion candidates. + Set<VertexInfo> compositeVertices = GraphHelper.getCompositeVertices(instanceVertex); + + // Record all deletion candidate GUIDs in RequestContext + // and gather deletion candidate vertices. + for (VertexInfo vertexInfo : compositeVertices) { + requestContext.recordEntityDelete(vertexInfo.getGuid(), vertexInfo.getTypeName()); + deletionCandidateVertices.add(vertexInfo.getVertex()); + } + } + + // Delete traits and vertices. + for (Vertex deletionCandidateVertex : deletionCandidateVertices) { + deleteAllTraits(deletionCandidateVertex); + deleteTypeVertex(deletionCandidateVertex, false); + } } protected abstract void deleteEdge(Edge edge, boolean force) throws AtlasException; @@ -96,7 +118,7 @@ public abstract class DeleteHandler { break; case CLASS: - deleteEntity(instanceVertex); + deleteEntities(Collections.singletonList(instanceVertex)); break; default: @@ -280,7 +302,7 @@ public abstract class DeleteHandler { } else { // Cannot unset a required attribute. throw new NullRequiredAttributeException("Cannot unset required attribute " + GraphHelper.getQualifiedFieldName(type, attributeName) + - " on " + string(outVertex) + " edge = " + edgeLabel); + " on " + GraphHelper.getVertexDetails(outVertex) + " edge = " + edgeLabel); } break; @@ -306,7 +328,7 @@ public abstract class DeleteHandler { throw new NullRequiredAttributeException( "Cannot remove array element from required attribute " + GraphHelper.getQualifiedFieldName(type, attributeName) + " on " - + string(outVertex) + " " + string(elementEdge)); + + GraphHelper.getVertexDetails(outVertex) + " " + GraphHelper.getEdgeDetails(elementEdge)); } if (shouldUpdateReverseAttribute) { @@ -344,7 +366,7 @@ public abstract class DeleteHandler { // 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)); + GraphHelper.getQualifiedFieldName(type, attributeName) + " on " + GraphHelper.getVertexDetails(outVertex) + " " + GraphHelper.getEdgeDetails(mapEdge)); } if (shouldUpdateReverseAttribute) { @@ -367,8 +389,8 @@ public abstract class DeleteHandler { break; default: - throw new IllegalStateException("There can't be an edge from " + string(outVertex) + " to " - + string(inVertex) + " with attribute name " + attributeName + " which is not class/array/map attribute"); + throw new IllegalStateException("There can't be an edge from " + GraphHelper.getVertexDetails(outVertex) + " to " + + GraphHelper.getVertexDetails(inVertex) + " with attribute name " + attributeName + " which is not class/array/map attribute"); } if (edge != null) { diff --git a/repository/src/main/java/org/apache/atlas/repository/graph/GraphBackedMetadataRepository.java b/repository/src/main/java/org/apache/atlas/repository/graph/GraphBackedMetadataRepository.java index e301a00..569949d 100755 --- a/repository/src/main/java/org/apache/atlas/repository/graph/GraphBackedMetadataRepository.java +++ b/repository/src/main/java/org/apache/atlas/repository/graph/GraphBackedMetadataRepository.java @@ -331,7 +331,8 @@ public class GraphBackedMetadataRepository implements MetadataRepository { if (guids == null || guids.size() == 0) { throw new IllegalArgumentException("guids must be non-null and non-empty"); } - + + List<Vertex> vertices = new ArrayList<>(guids.size()); for (String guid : guids) { if (guid == null) { LOG.warn("deleteEntities: Ignoring null guid"); @@ -339,16 +340,22 @@ public class GraphBackedMetadataRepository implements MetadataRepository { } try { Vertex instanceVertex = graphHelper.getVertexForGUID(guid); - deleteHandler.deleteEntity(instanceVertex); + vertices.add(instanceVertex); } catch (EntityNotFoundException e) { // Entity does not exist - treat as non-error, since the caller // wanted to delete the entity and it's already gone. LOG.info("Deletion request ignored for non-existent entity with guid " + guid); continue; - } catch (AtlasException e) { - throw new RepositoryException(e); } } + + try { + deleteHandler.deleteEntities(vertices); + } + catch (AtlasException e) { + throw new RepositoryException(e); + } + RequestContext requestContext = RequestContext.get(); return new AtlasClient.EntityResult(requestContext.getCreatedEntityIds(), requestContext.getUpdatedEntityIds(), requestContext.getDeletedEntityIds()); diff --git a/repository/src/main/java/org/apache/atlas/repository/graph/GraphHelper.java b/repository/src/main/java/org/apache/atlas/repository/graph/GraphHelper.java index 26ebbb1..dd77ba8 100755 --- a/repository/src/main/java/org/apache/atlas/repository/graph/GraphHelper.java +++ b/repository/src/main/java/org/apache/atlas/repository/graph/GraphHelper.java @@ -21,9 +21,11 @@ package org.apache.atlas.repository.graph; import java.util.ArrayList; import java.util.Collection; import java.util.HashMap; +import java.util.HashSet; import java.util.Iterator; import java.util.List; import java.util.Set; +import java.util.Stack; import java.util.UUID; import org.apache.atlas.AtlasException; @@ -37,6 +39,7 @@ import org.apache.atlas.typesystem.persistence.Id; import org.apache.atlas.typesystem.types.AttributeInfo; import org.apache.atlas.typesystem.types.ClassType; import org.apache.atlas.typesystem.types.DataTypes; +import org.apache.atlas.typesystem.types.DataTypes.TypeCategory; import org.apache.atlas.typesystem.types.HierarchicalType; import org.apache.atlas.typesystem.types.IDataType; import org.apache.atlas.typesystem.types.TypeSystem; @@ -449,6 +452,139 @@ public final class GraphHelper { return result; } + /** + * Guid and Vertex combo + */ + public static class VertexInfo { + private String guid; + private Vertex vertex; + private String typeName; + + public VertexInfo(String guid, Vertex vertex, String typeName) { + this.guid = guid; + this.vertex = vertex; + this.typeName = typeName; + } + + public String getGuid() { + return guid; + } + public Vertex getVertex() { + return vertex; + } + public String getTypeName() { + return typeName; + } + + @Override + public int hashCode() { + + final int prime = 31; + int result = 1; + result = prime * result + ((guid == null) ? 0 : guid.hashCode()); + result = prime * result + ((vertex == null) ? 0 : vertex.hashCode()); + return result; + } + + @Override + public boolean equals(Object obj) { + + if (this == obj) + return true; + if (obj == null) + return false; + if (!(obj instanceof VertexInfo)) + return false; + VertexInfo other = (VertexInfo)obj; + if (guid == null) { + if (other.guid != null) + return false; + } else if (!guid.equals(other.guid)) + return false; + return true; + } + } + + /** + * Get the GUIDs and vertices for all composite entities owned/contained by the specified root entity vertex. + * The graph is traversed from the root entity through to the leaf nodes of the containment graph. + * + * @param entityVertex the root entity vertex + * @return set of VertexInfo for all composite entities + * @throws AtlasException + */ + public static Set<VertexInfo> getCompositeVertices(Vertex entityVertex) throws AtlasException { + Set<VertexInfo> result = new HashSet<>(); + Stack<Vertex> vertices = new Stack<>(); + vertices.push(entityVertex); + while (vertices.size() > 0) { + Vertex vertex = vertices.pop(); + String typeName = GraphHelper.getTypeName(vertex); + String guid = GraphHelper.getIdFromVertex(vertex); + Id.EntityState state = GraphHelper.getState(vertex); + if (state == Id.EntityState.DELETED) { + //If the reference vertex is marked for deletion, skip it + continue; + } + result.add(new VertexInfo(guid, vertex, typeName)); + ClassType classType = typeSystem.getDataType(ClassType.class, typeName); + for (AttributeInfo attributeInfo : classType.fieldMapping().fields.values()) { + if (!attributeInfo.isComposite) { + continue; + } + String edgeLabel = GraphHelper.getEdgeLabel(classType, attributeInfo); + switch (attributeInfo.dataType().getTypeCategory()) { + case CLASS: + Edge edge = GraphHelper.getEdgeForLabel(vertex, edgeLabel); + if (edge != null && GraphHelper.getState(edge) == Id.EntityState.ACTIVE) { + Vertex compositeVertex = edge.getVertex(Direction.IN); + vertices.push(compositeVertex); + } + break; + case ARRAY: + IDataType elementType = ((DataTypes.ArrayType) attributeInfo.dataType()).getElemType(); + DataTypes.TypeCategory elementTypeCategory = elementType.getTypeCategory(); + if (elementTypeCategory != TypeCategory.CLASS) { + continue; + } + Iterator<Edge> edges = GraphHelper.getOutGoingEdgesByLabel(vertex, edgeLabel); + if (edges != null) { + while (edges.hasNext()) { + edge = edges.next(); + if (edge != null && GraphHelper.getState(edge) == Id.EntityState.ACTIVE) { + Vertex compositeVertex = edge.getVertex(Direction.IN); + vertices.push(compositeVertex); + } + } + } + break; + case MAP: + DataTypes.MapType mapType = (DataTypes.MapType) attributeInfo.dataType(); + DataTypes.TypeCategory valueTypeCategory = mapType.getValueType().getTypeCategory(); + if (valueTypeCategory != TypeCategory.CLASS) { + continue; + } + String propertyName = GraphHelper.getQualifiedFieldName(classType, attributeInfo.name); + List<String> keys = vertex.getProperty(propertyName); + if (keys != null) { + for (String key : keys) { + String mapEdgeLabel = GraphHelper.getQualifiedNameForMapKey(edgeLabel, key); + edge = GraphHelper.getEdgeForLabel(vertex, mapEdgeLabel); + if (edge != null && GraphHelper.getState(edge) == Id.EntityState.ACTIVE) { + Vertex compositeVertex = edge.getVertex(Direction.IN); + vertices.push(compositeVertex); + } + } + } + break; + default: + continue; + } + } + } + return result; + } + public static void dumpToLog(final Graph graph) { LOG.debug("*******************Graph Dump****************************"); LOG.debug("Vertices of {}", graph); @@ -472,27 +608,38 @@ public final class GraphHelper { return "vertex[null]"; } else { if (LOG.isDebugEnabled()) { - return String.format("vertex[id=%s type=%s guid=%s]", vertex.getId().toString(), getTypeName(vertex), - getIdFromVertex(vertex)); + return getVertexDetails(vertex); } else { return String.format("vertex[id=%s]", vertex.getId().toString()); } } } + public static String getVertexDetails(Vertex vertex) { + + return String.format("vertex[id=%s type=%s guid=%s]", vertex.getId().toString(), getTypeName(vertex), + getIdFromVertex(vertex)); + } + + public static String string(Edge edge) { if(edge == null) { return "edge[null]"; } else { if (LOG.isDebugEnabled()) { - return String.format("edge[id=%s label=%s from %s -> to %s]", edge.getId().toString(), edge.getLabel(), - string(edge.getVertex(Direction.OUT)), string(edge.getVertex(Direction.IN))); + return getEdgeDetails(edge); } else { return String.format("edge[id=%s]", edge.getId().toString()); } } } + public static String getEdgeDetails(Edge edge) { + + return String.format("edge[id=%s label=%s from %s -> to %s]", edge.getId().toString(), edge.getLabel(), + string(edge.getVertex(Direction.OUT)), string(edge.getVertex(Direction.IN))); + } + @VisibleForTesting //Keys copied from com.thinkaurelius.titan.graphdb.types.StandardRelationTypeMaker //Titan checks that these chars are not part of any keys. So, encoding... @@ -540,4 +687,4 @@ public final class GraphHelper { return null; } -} \ No newline at end of file +} diff --git a/repository/src/test/java/org/apache/atlas/repository/graph/GraphBackedMetadataRepositoryDeleteTestBase.java b/repository/src/test/java/org/apache/atlas/repository/graph/GraphBackedMetadataRepositoryDeleteTestBase.java index 34842c3..550a98e 100644 --- a/repository/src/test/java/org/apache/atlas/repository/graph/GraphBackedMetadataRepositoryDeleteTestBase.java +++ b/repository/src/test/java/org/apache/atlas/repository/graph/GraphBackedMetadataRepositoryDeleteTestBase.java @@ -23,7 +23,9 @@ import com.google.common.collect.ImmutableSet; import com.thinkaurelius.titan.core.TitanGraph; import com.thinkaurelius.titan.core.util.TitanCleanup; import com.tinkerpop.blueprints.Vertex; + import org.apache.atlas.AtlasClient; +import org.apache.atlas.AtlasClient.EntityResult; import org.apache.atlas.AtlasException; import org.apache.atlas.RepositoryMetadataModule; import org.apache.atlas.RequestContext; @@ -37,6 +39,7 @@ import org.apache.atlas.typesystem.ITypedStruct; import org.apache.atlas.typesystem.Referenceable; import org.apache.atlas.typesystem.Struct; import org.apache.atlas.typesystem.TypesDef; +import org.apache.atlas.typesystem.exception.EntityExistsException; import org.apache.atlas.typesystem.exception.EntityNotFoundException; import org.apache.atlas.typesystem.exception.NullRequiredAttributeException; import org.apache.atlas.typesystem.persistence.Id; @@ -45,6 +48,7 @@ import org.apache.atlas.typesystem.types.ClassType; import org.apache.atlas.typesystem.types.DataTypes; import org.apache.atlas.typesystem.types.EnumTypeDefinition; import org.apache.atlas.typesystem.types.HierarchicalTypeDefinition; +import org.apache.atlas.typesystem.types.IDataType; import org.apache.atlas.typesystem.types.Multiplicity; import org.apache.atlas.typesystem.types.StructTypeDefinition; import org.apache.atlas.typesystem.types.TraitType; @@ -58,6 +62,7 @@ import org.testng.annotations.Guice; import org.testng.annotations.Test; import javax.inject.Inject; + import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; @@ -95,6 +100,10 @@ public abstract class GraphBackedMetadataRepositoryDeleteTestBase { private TypeSystem typeSystem; + private ClassType compositeMapOwnerType; + + private ClassType compositeMapValueType; + @BeforeClass public void setUp() throws Exception { typeSystem = TypeSystem.getInstance(); @@ -106,6 +115,24 @@ public abstract class GraphBackedMetadataRepositoryDeleteTestBase { TestUtils.defineDeptEmployeeTypes(typeSystem); TestUtils.createHiveTypes(typeSystem); + + // Define type for map value. + HierarchicalTypeDefinition<ClassType> mapValueDef = TypesUtil.createClassTypeDef("CompositeMapValue", + ImmutableSet.<String>of(), + TypesUtil.createUniqueRequiredAttrDef(NAME, DataTypes.STRING_TYPE)); + + // Define type with map where the value is a composite class reference to MapValue. + HierarchicalTypeDefinition<ClassType> mapOwnerDef = TypesUtil.createClassTypeDef("CompositeMapOwner", + ImmutableSet.<String>of(), + TypesUtil.createUniqueRequiredAttrDef(NAME, DataTypes.STRING_TYPE), + new AttributeDefinition("map", DataTypes.mapTypeName(DataTypes.STRING_TYPE.getName(), + "CompositeMapValue"), Multiplicity.OPTIONAL, true, null)); + TypesDef typesDef = TypesUtil.getTypesDef(ImmutableList.<EnumTypeDefinition>of(), + ImmutableList.<StructTypeDefinition>of(), ImmutableList.<HierarchicalTypeDefinition<TraitType>>of(), + ImmutableList.of(mapOwnerDef, mapValueDef)); + typeSystem.defineTypes(typesDef); + compositeMapOwnerType = typeSystem.getDataType(ClassType.class, "CompositeMapOwner"); + compositeMapValueType = typeSystem.getDataType(ClassType.class, "CompositeMapValue"); } abstract DeleteHandler getDeleteHandler(TypeSystem typeSystem); @@ -343,45 +370,22 @@ public abstract class GraphBackedMetadataRepositoryDeleteTestBase { @Test public void testDeleteEntitiesWithCompositeMapReference() throws Exception { - // Define type for map value. - HierarchicalTypeDefinition<ClassType> mapValueDef = TypesUtil.createClassTypeDef("CompositeMapValue", - ImmutableSet.<String>of(), - TypesUtil.createOptionalAttrDef("attr1", DataTypes.STRING_TYPE)); - - // Define type with map where the value is a composite class reference to MapValue. - HierarchicalTypeDefinition<ClassType> mapOwnerDef = TypesUtil.createClassTypeDef("CompositeMapOwner", - ImmutableSet.<String>of(), - new AttributeDefinition("map", DataTypes.mapTypeName(DataTypes.STRING_TYPE.getName(), - "CompositeMapValue"), Multiplicity.OPTIONAL, true, null)); - TypesDef typesDef = TypesUtil.getTypesDef(ImmutableList.<EnumTypeDefinition>of(), - ImmutableList.<StructTypeDefinition>of(), ImmutableList.<HierarchicalTypeDefinition<TraitType>>of(), - ImmutableList.of(mapOwnerDef, mapValueDef)); - typeSystem.defineTypes(typesDef); - ClassType mapOwnerType = typeSystem.getDataType(ClassType.class, "CompositeMapOwner"); - ClassType mapValueType = typeSystem.getDataType(ClassType.class, "CompositeMapValue"); - // Create instances of MapOwner and MapValue. // Set MapOwner.map with one entry that references MapValue instance. - ITypedReferenceableInstance mapOwnerInstance = mapOwnerType.createInstance(); - ITypedReferenceableInstance mapValueInstance = mapValueType.createInstance(); - mapOwnerInstance.set("map", Collections.singletonMap("value1", mapValueInstance)); - List<String> createEntitiesResult = repositoryService.createEntities(mapOwnerInstance, mapValueInstance); - Assert.assertEquals(createEntitiesResult.size(), 2); - List<String> guids = repositoryService.getEntityList("CompositeMapOwner"); - Assert.assertEquals(guids.size(), 1); - String mapOwnerGuid = guids.get(0); + ITypedReferenceableInstance entityDefinition = createMapOwnerAndValueEntities(); + String mapOwnerGuid = entityDefinition.getId()._getId(); // Verify MapOwner.map attribute has expected value. - mapOwnerInstance = repositoryService.getEntityDefinition(mapOwnerGuid); + ITypedReferenceableInstance mapOwnerInstance = repositoryService.getEntityDefinition(mapOwnerGuid); Object object = mapOwnerInstance.get("map"); Assert.assertNotNull(object); Assert.assertTrue(object instanceof Map); Map<String, ITypedReferenceableInstance> map = (Map<String, ITypedReferenceableInstance>)object; Assert.assertEquals(map.size(), 1); - mapValueInstance = map.get("value1"); + ITypedReferenceableInstance mapValueInstance = map.get("value1"); Assert.assertNotNull(mapValueInstance); String mapValueGuid = mapValueInstance.getId()._getId(); - String edgeLabel = GraphHelper.getEdgeLabel(mapOwnerType, mapOwnerType.fieldMapping.fields.get("map")); + String edgeLabel = GraphHelper.getEdgeLabel(compositeMapOwnerType, compositeMapOwnerType.fieldMapping.fields.get("map")); String mapEntryLabel = edgeLabel + "." + "value1"; AtlasEdgeLabel atlasEdgeLabel = new AtlasEdgeLabel(mapEntryLabel); Vertex mapOwnerVertex = GraphHelper.getInstance().getVertexForGUID(mapOwnerGuid); @@ -397,6 +401,21 @@ public abstract class GraphBackedMetadataRepositoryDeleteTestBase { assertEntityDeleted(mapValueGuid); } + private ITypedReferenceableInstance createMapOwnerAndValueEntities() + throws AtlasException, RepositoryException, EntityExistsException { + + ITypedReferenceableInstance mapOwnerInstance = compositeMapOwnerType.createInstance(); + mapOwnerInstance.set(NAME, TestUtils.randomString()); + ITypedReferenceableInstance mapValueInstance = compositeMapValueType.createInstance(); + mapValueInstance.set(NAME, TestUtils.randomString()); + mapOwnerInstance.set("map", Collections.singletonMap("value1", mapValueInstance)); + List<String> createEntitiesResult = repositoryService.createEntities(mapOwnerInstance, mapValueInstance); + Assert.assertEquals(createEntitiesResult.size(), 2); + ITypedReferenceableInstance entityDefinition = repositoryService.getEntityDefinition("CompositeMapOwner", + NAME, mapOwnerInstance.get(NAME)); + return entityDefinition; + } + private AtlasClient.EntityResult updatePartial(ITypedReferenceableInstance entity) throws RepositoryException { RequestContext.createContext(); return repositoryService.updatePartial(entity); @@ -879,6 +898,126 @@ public abstract class GraphBackedMetadataRepositoryDeleteTestBase { } } + @Test + public void testLowerBoundsIgnoredOnDeletedEntities() throws Exception { + + String hrDeptGuid = createHrDeptGraph(); + ITypedReferenceableInstance hrDept = repositoryService.getEntityDefinition(hrDeptGuid); + Map<String, String> nameGuidMap = getEmployeeNameGuidMap(hrDept); + + ITypedReferenceableInstance john = repositoryService.getEntityDefinition(nameGuidMap.get("John")); + String johnGuid = john.getId()._getId(); + + ITypedReferenceableInstance max = repositoryService.getEntityDefinition(nameGuidMap.get("Max")); + String maxGuid = max.getId()._getId(); + + ITypedReferenceableInstance jane = repositoryService.getEntityDefinition(nameGuidMap.get("Jane")); + String janeGuid = jane.getId()._getId(); + + // The lower bound constraint on Manager.subordinates should not be enforced on Jane since that entity is being deleted. + // Prior to the fix for ATLAS-991, this call would fail with a NullRequiredAttributeException. + EntityResult deleteResult = deleteEntities(johnGuid, maxGuid, janeGuid); + Assert.assertEquals(deleteResult.getDeletedEntities().size(), 3); + Assert.assertTrue(deleteResult.getDeletedEntities().containsAll(Arrays.asList(johnGuid, maxGuid, janeGuid))); + Assert.assertEquals(deleteResult.getUpdateEntities().size(), 1); + + // Verify that Department entity was updated to disconnect its references to the deleted employees. + Assert.assertEquals(deleteResult.getUpdateEntities().get(0), hrDeptGuid); + hrDept = repositoryService.getEntityDefinition(hrDeptGuid); + Object object = hrDept.get("employees"); + Assert.assertTrue(object instanceof List); + List<ITypedReferenceableInstance> employees = (List<ITypedReferenceableInstance>) object; + assertTestLowerBoundsIgnoredOnDeletedEntities(employees); + } + + protected abstract void assertTestLowerBoundsIgnoredOnDeletedEntities(List<ITypedReferenceableInstance> employees); + + @Test + public void testLowerBoundsIgnoredOnCompositeDeletedEntities() throws Exception { + String hrDeptGuid = createHrDeptGraph(); + ITypedReferenceableInstance hrDept = repositoryService.getEntityDefinition(hrDeptGuid); + Map<String, String> nameGuidMap = getEmployeeNameGuidMap(hrDept); + ITypedReferenceableInstance john = repositoryService.getEntityDefinition(nameGuidMap.get("John")); + String johnGuid = john.getId()._getId(); + ITypedReferenceableInstance max = repositoryService.getEntityDefinition(nameGuidMap.get("Max")); + String maxGuid = max.getId()._getId(); + + // The lower bound constraint on Manager.subordinates should not be enforced on the composite entity + // for Jane owned by the Department entity, since that entity is being deleted. + // Prior to the fix for ATLAS-991, this call would fail with a NullRequiredAttributeException. + EntityResult deleteResult = deleteEntities(johnGuid, maxGuid, hrDeptGuid); + Assert.assertEquals(deleteResult.getDeletedEntities().size(), 5); + Assert.assertTrue(deleteResult.getDeletedEntities().containsAll(nameGuidMap.values())); + Assert.assertTrue(deleteResult.getDeletedEntities().contains(hrDeptGuid)); + assertTestLowerBoundsIgnoredOnCompositeDeletedEntities(hrDeptGuid); + } + + + protected abstract void assertTestLowerBoundsIgnoredOnCompositeDeletedEntities(String hrDeptGuid) throws Exception; + + @Test + public void testLowerBoundsIgnoredWhenDeletingCompositeEntitesOwnedByMap() throws Exception { + // Define MapValueReferencer type with required reference to CompositeMapValue. + HierarchicalTypeDefinition<ClassType> mapValueReferencerTypeDef = TypesUtil.createClassTypeDef("MapValueReferencer", + ImmutableSet.<String>of(), + new AttributeDefinition("refToMapValue", "CompositeMapValue", Multiplicity.REQUIRED, false, null)); + + // Define MapValueReferencerContainer type with required composite map reference to MapValueReferencer. + HierarchicalTypeDefinition<ClassType> mapValueReferencerContainerTypeDef = + TypesUtil.createClassTypeDef("MapValueReferencerContainer", + ImmutableSet.<String>of(), + new AttributeDefinition("requiredMap", DataTypes.mapTypeName(DataTypes.STRING_TYPE.getName(), "MapValueReferencer"), Multiplicity.REQUIRED, true, null)); + + Map<String, IDataType> definedClassTypes = typeSystem.defineClassTypes(mapValueReferencerTypeDef, mapValueReferencerContainerTypeDef); + ClassType mapValueReferencerClassType = (ClassType) definedClassTypes.get("MapValueReferencer"); + ClassType mapValueReferencerContainerType = (ClassType) definedClassTypes.get("MapValueReferencerContainer"); + + // Create instances of CompositeMapOwner and CompositeMapValue. + // Set MapOwner.map with one entry that references MapValue instance. + ITypedReferenceableInstance entityDefinition = createMapOwnerAndValueEntities(); + String mapOwnerGuid = entityDefinition.getId()._getId(); + + // Verify MapOwner.map attribute has expected value. + ITypedReferenceableInstance mapOwnerInstance = repositoryService.getEntityDefinition(mapOwnerGuid); + Object object = mapOwnerInstance.get("map"); + Assert.assertNotNull(object); + Assert.assertTrue(object instanceof Map); + Map<String, ITypedReferenceableInstance> map = (Map<String, ITypedReferenceableInstance>)object; + Assert.assertEquals(map.size(), 1); + ITypedReferenceableInstance mapValueInstance = map.get("value1"); + Assert.assertNotNull(mapValueInstance); + String mapValueGuid = mapValueInstance.getId()._getId(); + + // Create instance of MapValueReferencerContainer + RequestContext.createContext(); + ITypedReferenceableInstance mapValueReferencerContainer = mapValueReferencerContainerType.createInstance(); + List<String> createdEntities = repositoryService.createEntities(mapValueReferencerContainer); + Assert.assertEquals(createdEntities.size(), 1); + String mapValueReferencerContainerGuid = createdEntities.get(0); + mapValueReferencerContainer = repositoryService.getEntityDefinition(createdEntities.get(0)); + + // Create instance of MapValueReferencer, and update mapValueReferencerContainer + // to reference it. + ITypedReferenceableInstance mapValueReferencer = mapValueReferencerClassType.createInstance(); + mapValueReferencerContainer.set("requiredMap", Collections.singletonMap("value1", mapValueReferencer)); + mapValueReferencer.set("refToMapValue", mapValueInstance.getId()); + + RequestContext.createContext(); + EntityResult updateEntitiesResult = repositoryService.updateEntities(mapValueReferencerContainer); + Assert.assertEquals(updateEntitiesResult.getCreatedEntities().size(), 1); + Assert.assertEquals(updateEntitiesResult.getUpdateEntities().size(), 1); + Assert.assertEquals(updateEntitiesResult.getUpdateEntities().get(0), mapValueReferencerContainerGuid); + String mapValueReferencerGuid = updateEntitiesResult.getCreatedEntities().get(0); + + // Delete map owner and map referencer container. A total of 4 entities should be deleted, + // including the composite entities. The lower bound constraint on MapValueReferencer.refToMapValue + // should not be enforced on the composite MapValueReferencer since it is being deleted. + EntityResult deleteEntitiesResult = repositoryService.deleteEntities(Arrays.asList(mapOwnerGuid, mapValueReferencerContainerGuid)); + Assert.assertEquals(deleteEntitiesResult.getDeletedEntities().size(), 4); + Assert.assertTrue(deleteEntitiesResult.getDeletedEntities().containsAll( + Arrays.asList(mapOwnerGuid, mapValueGuid, mapValueReferencerContainerGuid, mapValueReferencerGuid))); + } + private String createHrDeptGraph() throws Exception { ITypedReferenceableInstance hrDept = TestUtils.createDeptEg1(typeSystem); @@ -886,6 +1025,12 @@ public abstract class GraphBackedMetadataRepositoryDeleteTestBase { Assert.assertNotNull(guids); Assert.assertEquals(guids.size(), 5); + return getDepartmentGuid(guids); + } + + private String getDepartmentGuid(List<String> guids) + throws RepositoryException, EntityNotFoundException { + String hrDeptGuid = null; for (String guid : guids) { ITypedReferenceableInstance entityDefinition = repositoryService.getEntityDefinition(guid); diff --git a/repository/src/test/java/org/apache/atlas/repository/graph/GraphBackedRepositoryHardDeleteTest.java b/repository/src/test/java/org/apache/atlas/repository/graph/GraphBackedRepositoryHardDeleteTest.java index cc60264..79b48b5 100644 --- a/repository/src/test/java/org/apache/atlas/repository/graph/GraphBackedRepositoryHardDeleteTest.java +++ b/repository/src/test/java/org/apache/atlas/repository/graph/GraphBackedRepositoryHardDeleteTest.java @@ -184,4 +184,22 @@ public class GraphBackedRepositoryHardDeleteTest extends GraphBackedMetadataRepo Assert.fail("Lower bound on attribute Manager.subordinates was not enforced - " + NullRequiredAttributeException.class.getSimpleName() + " was expected but none thrown"); } + + @Override + protected void assertTestLowerBoundsIgnoredOnDeletedEntities(List<ITypedReferenceableInstance> employees) { + + Assert.assertEquals(employees.size(), 1, "References to deleted employees were not disconnected"); + } + + @Override + protected void assertTestLowerBoundsIgnoredOnCompositeDeletedEntities(String hrDeptGuid) throws Exception { + + try { + repositoryService.getEntityDefinition(hrDeptGuid); + Assert.fail(EntityNotFoundException.class.getSimpleName() + " was expected but none thrown"); + } + catch (EntityNotFoundException e) { + // good + } + } } diff --git a/repository/src/test/java/org/apache/atlas/repository/graph/GraphBackedRepositorySoftDeleteTest.java b/repository/src/test/java/org/apache/atlas/repository/graph/GraphBackedRepositorySoftDeleteTest.java index 90bb635..a0af487 100644 --- a/repository/src/test/java/org/apache/atlas/repository/graph/GraphBackedRepositorySoftDeleteTest.java +++ b/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.AtlasException; import org.apache.atlas.TestUtils; @@ -28,6 +29,7 @@ import org.apache.atlas.typesystem.IStruct; import org.apache.atlas.typesystem.ITypedReferenceableInstance; import org.apache.atlas.typesystem.ITypedStruct; import org.apache.atlas.typesystem.persistence.Id; +import org.apache.atlas.typesystem.persistence.Id.EntityState; import org.apache.atlas.typesystem.types.TypeSystem; import org.testng.Assert; @@ -214,4 +216,17 @@ public class GraphBackedRepositorySoftDeleteTest extends GraphBackedMetadataRepo protected void assertTestDeleteTargetOfMultiplicityRequiredReference() throws Exception { // No-op - it's ok that no exception was thrown if soft deletes are enabled. } + + @Override + protected void assertTestLowerBoundsIgnoredOnDeletedEntities(List<ITypedReferenceableInstance> employees) { + + Assert.assertEquals(employees.size(), 4, "References to deleted employees should not have been disconnected with soft deletes enabled"); + } + + @Override + protected void assertTestLowerBoundsIgnoredOnCompositeDeletedEntities(String hrDeptGuid) throws Exception { + + ITypedReferenceableInstance hrDept = repositoryService.getEntityDefinition(hrDeptGuid); + Assert.assertEquals(hrDept.getId().getState(), EntityState.DELETED); + } } diff --git a/repository/src/test/java/org/apache/atlas/repository/graph/GraphHelperTest.java b/repository/src/test/java/org/apache/atlas/repository/graph/GraphHelperTest.java index 428846f..683a40b 100644 --- a/repository/src/test/java/org/apache/atlas/repository/graph/GraphHelperTest.java +++ b/repository/src/test/java/org/apache/atlas/repository/graph/GraphHelperTest.java @@ -18,24 +18,38 @@ package org.apache.atlas.repository.graph; -import com.thinkaurelius.titan.core.TitanGraph; -import com.thinkaurelius.titan.core.TitanVertex; -import com.tinkerpop.blueprints.Edge; -import org.apache.atlas.RepositoryMetadataModule; -import org.testng.annotations.DataProvider; -import org.testng.annotations.Guice; -import org.testng.annotations.Test; - -import javax.inject.Inject; - -import java.util.Iterator; - import static org.testng.Assert.assertEquals; import static org.testng.Assert.assertFalse; import static org.testng.Assert.assertNotNull; import static org.testng.Assert.assertNull; import static org.testng.Assert.assertTrue; +import java.util.HashMap; +import java.util.Iterator; +import java.util.HashSet; +import java.util.List; +import java.util.Set; + +import javax.inject.Inject; + +import org.apache.atlas.RepositoryMetadataModule; +import org.apache.atlas.TestUtils; +import org.apache.atlas.repository.graph.GraphHelper.VertexInfo; +import org.apache.atlas.typesystem.ITypedReferenceableInstance; +import org.apache.atlas.typesystem.types.TypeSystem; +import org.testng.Assert; +import org.testng.annotations.AfterClass; +import org.testng.annotations.BeforeClass; +import org.testng.annotations.DataProvider; +import org.testng.annotations.Guice; +import org.testng.annotations.Test; + +import com.thinkaurelius.titan.core.TitanGraph; +import com.thinkaurelius.titan.core.TitanVertex; +import com.thinkaurelius.titan.core.util.TitanCleanup; +import com.tinkerpop.blueprints.Edge; +import com.tinkerpop.blueprints.Vertex; + @Guice(modules = RepositoryMetadataModule.class) public class GraphHelperTest { @Inject @@ -57,6 +71,65 @@ public class GraphHelperTest { }; } + @Inject + private GraphBackedMetadataRepository repositoryService; + + private TypeSystem typeSystem; + + @BeforeClass + public void setUp() throws Exception { + typeSystem = TypeSystem.getInstance(); + typeSystem.reset(); + + new GraphBackedSearchIndexer(graphProvider); + + TestUtils.defineDeptEmployeeTypes(typeSystem); + } + + @AfterClass + public void tearDown() throws Exception { + TypeSystem.getInstance().reset(); + try { + //TODO - Fix failure during shutdown while using BDB + graphProvider.get().shutdown(); + } catch (Exception e) { + e.printStackTrace(); + } + try { + TitanCleanup.clear(graphProvider.get()); + } catch (Exception e) { + e.printStackTrace(); + } + } + + @Test + public void testGetCompositeGuidsAndVertices() throws Exception { + ITypedReferenceableInstance hrDept = TestUtils.createDeptEg1(typeSystem); + List<String> createdGuids = repositoryService.createEntities(hrDept); + String deptGuid = null; + Set<String> expectedGuids = new HashSet<>(); + + for (String guid : createdGuids) { + ITypedReferenceableInstance entityDefinition = repositoryService.getEntityDefinition(guid); + expectedGuids.add(guid); + if (entityDefinition.getId().getTypeName().equals(TestUtils.DEPARTMENT_TYPE)) { + deptGuid = guid; + } + } + Vertex deptVertex = GraphHelper.getInstance().getVertexForGUID(deptGuid); + Set<VertexInfo> compositeVertices = GraphHelper.getCompositeVertices(deptVertex); + HashMap<String, VertexInfo> verticesByGuid = new HashMap<>(); + for (VertexInfo vertexInfo: compositeVertices) { + verticesByGuid.put(vertexInfo.getGuid(), vertexInfo); + } + + // Verify compositeVertices has entries for all expected guids. + Assert.assertEquals(compositeVertices.size(), expectedGuids.size()); + for (String expectedGuid : expectedGuids) { + Assert.assertTrue(verticesByGuid.containsKey(expectedGuid)); + } + } + @Test(dataProvider = "encodeDecodeTestData") public void testEncodeDecode(String str, String expectedEncodedStr) throws Exception { String encodedStr = GraphHelper.encodePropertyKey(str); -- libgit2 0.27.1