Commit d06b8229 by Dave Kantor

ATLAS-991 Lower bound checking not always disabled for entities being deleted. (dkantor)

parent 127b378d
......@@ -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)
......
......@@ -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) {
......
......@@ -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());
......
......@@ -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
}
......@@ -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
}
}
}
......@@ -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);
}
}
......@@ -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);
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment