ATLAS-1780: Type deletion should invalidate property keys in Titan to allow…

graphdb/api/src/main/java/org/apache/atlas/repository/graphdb/AtlasGraphManagement.java

@@ -64,6 +64,12 @@ public interface AtlasGraphManagement {
 
     /**
      *  @param propertyKey
+     *
+     */
+    void deletePropertyKey(String propertyKey);
+
+    /**
+     * @param propertyName
      * @return
      */
     AtlasPropertyKey getPropertyKey(String propertyName);
@@ -72,8 +78,8 @@ public interface AtlasGraphManagement {
      * Creates a composite index for the graph.
      *
      * @param propertyName
-     * @param propertyKey
      * @param isUnique
+     * @param propertyKeys
      */
     void createExactMatchIndex(String propertyName, boolean isUnique, List<AtlasPropertyKey> propertyKeys);
 
@@ -81,7 +87,7 @@ public interface AtlasGraphManagement {
      * Looks up the index with the specified name in the graph.  Returns null if
      * there is no index with the given name.
      *
-     * @param edgeIndex
+     * @param indexName
      * @return
      */
     AtlasGraphIndex getGraphIndex(String indexName);
@@ -89,7 +95,7 @@ public interface AtlasGraphManagement {
     /**
      * Creates a mixed Vertex index for the graph.
      *
-     * @param index the name of the index to create
+     * @param name the name of the index to create
      * @param backingIndex the name of the backing index to use
      */
     void createVertexIndex(String name, String backingIndex, List<AtlasPropertyKey> propertyKeys);

graphdb/titan0/src/main/java/org/apache/atlas/repository/graphdb/titan0/Titan0GraphManagement.java

@@ -17,17 +17,6 @@
  */
 package org.apache.atlas.repository.graphdb.titan0;
 
-import java.util.HashSet;
-import java.util.List;
-import java.util.Set;
-
-import org.apache.atlas.repository.graphdb.AtlasCardinality;
-import org.apache.atlas.repository.graphdb.AtlasGraphIndex;
-import org.apache.atlas.repository.graphdb.AtlasGraphManagement;
-import org.apache.atlas.repository.graphdb.AtlasPropertyKey;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
-
 import com.thinkaurelius.titan.core.Cardinality;
 import com.thinkaurelius.titan.core.PropertyKey;
 import com.thinkaurelius.titan.core.schema.Mapping;
@@ -37,6 +26,16 @@ import com.thinkaurelius.titan.core.schema.TitanManagement;
 import com.tinkerpop.blueprints.Edge;
 import com.tinkerpop.blueprints.Element;
 import com.tinkerpop.blueprints.Vertex;
+import org.apache.atlas.repository.graphdb.AtlasCardinality;
+import org.apache.atlas.repository.graphdb.AtlasGraphIndex;
+import org.apache.atlas.repository.graphdb.AtlasGraphManagement;
+import org.apache.atlas.repository.graphdb.AtlasPropertyKey;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.util.HashSet;
+import java.util.List;
+import java.util.Set;
 
 /**
  * Titan 0.5.4 implementation of AtlasGraphManagement.
@@ -112,6 +111,21 @@ public class Titan0GraphManagement implements AtlasGraphManagement {
     }
 
     @Override
+    public void deletePropertyKey(String propertyKey) {
+        PropertyKey titanPropertyKey = management.getPropertyKey(propertyKey);
+
+        if (null == titanPropertyKey) return;
+
+        for (int i = 0;; i++) {
+            String deletedKeyName = titanPropertyKey + "_deleted_" + i;
+            if (null == management.getPropertyKey(deletedKeyName)) {
+                management.changeName(titanPropertyKey, deletedKeyName);
+                break;
+            }
+        }
+    }
+
+    @Override
     public AtlasPropertyKey getPropertyKey(String propertyName) {
 
         return GraphDbObjectFactory.createPropertyKey(management.getPropertyKey(propertyName));

graphdb/titan1/src/main/java/org/apache/atlas/repository/graphdb/titan1/Titan1GraphManagement.java

@@ -17,10 +17,14 @@
  */
 package org.apache.atlas.repository.graphdb.titan1;
 
-import java.util.HashSet;
-import java.util.List;
-import java.util.Set;
-
+import com.google.common.base.Preconditions;
+import com.thinkaurelius.titan.core.Cardinality;
+import com.thinkaurelius.titan.core.PropertyKey;
+import com.thinkaurelius.titan.core.schema.Mapping;
+import com.thinkaurelius.titan.core.schema.PropertyKeyMaker;
+import com.thinkaurelius.titan.core.schema.TitanGraphIndex;
+import com.thinkaurelius.titan.core.schema.TitanManagement;
+import com.thinkaurelius.titan.graphdb.internal.Token;
 import org.apache.atlas.repository.graphdb.AtlasCardinality;
 import org.apache.atlas.repository.graphdb.AtlasGraphIndex;
 import org.apache.atlas.repository.graphdb.AtlasGraphManagement;
@@ -32,14 +36,9 @@ import org.apache.tinkerpop.gremlin.structure.Vertex;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
-import com.google.common.base.Preconditions;
-import com.thinkaurelius.titan.core.Cardinality;
-import com.thinkaurelius.titan.core.PropertyKey;
-import com.thinkaurelius.titan.core.schema.Mapping;
-import com.thinkaurelius.titan.core.schema.PropertyKeyMaker;
-import com.thinkaurelius.titan.core.schema.TitanGraphIndex;
-import com.thinkaurelius.titan.core.schema.TitanManagement;
-import com.thinkaurelius.titan.graphdb.internal.Token;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Set;
 
 /**
  * Titan 1.0.0 implementation of AtlasGraphManagement.
@@ -136,6 +135,21 @@ public class Titan1GraphManagement implements AtlasGraphManagement {
     }
 
     @Override
+    public void deletePropertyKey(String propertyKey) {
+        PropertyKey titanPropertyKey = management.getPropertyKey(propertyKey);
+
+        if (null == titanPropertyKey) return;
+
+        for (int i = 0;; i++) {
+            String deletedKeyName = titanPropertyKey + "_deleted_" + i;
+            if (null == management.getPropertyKey(deletedKeyName)) {
+                management.changeName(titanPropertyKey, deletedKeyName);
+                break;
+            }
+        }
+    }
+
+    @Override
     public AtlasPropertyKey getPropertyKey(String propertyName) {
         checkName(propertyName);
         return GraphDbObjectFactory.createPropertyKey(management.getPropertyKey(propertyName));

intg/src/main/java/org/apache/atlas/listener/TypeDefChangeListener.java

@@ -19,7 +19,6 @@ package org.apache.atlas.listener;
 
 import org.apache.atlas.exception.AtlasBaseException;
 
-@Deprecated
 public interface TypeDefChangeListener {
     void onChange(ChangedTypeDefs changedTypeDefs) throws AtlasBaseException;
 }

intg/src/main/java/org/apache/atlas/type/AtlasTypeRegistry.java

@@ -550,6 +550,7 @@ public class AtlasTypeRegistry {
                     registryData.entityDefs.removeTypeDefByName(typeDef.getName());
                     break;
             }
+            deletedTypes.add(typeDef);
         }
 
         private void removeTypeByGuidWithNoRefResolve(AtlasBaseTypeDef typeDef) {
@@ -567,6 +568,7 @@ public class AtlasTypeRegistry {
                     registryData.entityDefs.removeTypeDefByGuid(typeDef.getGuid());
                     break;
             }
+            deletedTypes.add(typeDef);
         }
 
         public void removeTypeByGuid(String guid) throws AtlasBaseException {

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

@@ -214,7 +214,7 @@ public class GraphBackedSearchIndexer implements SearchIndexer, ActiveStateChang
      * This is upon adding a new type to Store.
      *
      * @param dataTypes data type
-     * @throws org.apache.atlas.AtlasException
+     * @throws AtlasException
      */
     @Override
     public void onAdd(Collection<? extends IDataType> dataTypes) throws AtlasException {
@@ -612,7 +612,9 @@ public class GraphBackedSearchIndexer implements SearchIndexer, ActiveStateChang
 
     @Override
     public void onChange(ChangedTypeDefs changedTypeDefs) throws AtlasBaseException {
-        LOG.info("Adding indexes for changed typedefs");
+        if (LOG.isDebugEnabled()) {
+            LOG.debug("Processing changed typedefs {}", changedTypeDefs);
+        }
         AtlasGraphManagement management = null;
         try {
             management = provider.get().getManagementSystem();
@@ -631,6 +633,13 @@ public class GraphBackedSearchIndexer implements SearchIndexer, ActiveStateChang
                 }
             }
 
+            // Invalidate the property key for deleted types
+            if (CollectionUtils.isNotEmpty(changedTypeDefs.getDeletedTypeDefs())) {
+                for (AtlasBaseTypeDef typeDef : changedTypeDefs.getDeletedTypeDefs()) {
+                    cleanupIndices(management, typeDef);
+                }
+            }
+
             //Commit indexes
             commit(management);
         } catch (RepositoryException | IndexException e) {
@@ -640,6 +649,60 @@ public class GraphBackedSearchIndexer implements SearchIndexer, ActiveStateChang
 
     }
 
+    private void cleanupIndices(AtlasGraphManagement management, AtlasBaseTypeDef typeDef) {
+        Preconditions.checkNotNull(typeDef, "Cannot process null typedef");
+        if (LOG.isDebugEnabled()) {
+            LOG.debug("Cleaning up index for {}", typeDef);
+        }
+
+        if (typeDef instanceof AtlasEnumDef) {
+            // Only handle complex types like Struct, Classification and Entity
+            return;
+        }
+
+        if (typeDef instanceof AtlasStructDef) {
+            AtlasStructDef structDef = (AtlasStructDef) typeDef;
+            List<AtlasAttributeDef> attributeDefs = structDef.getAttributeDefs();
+            if (CollectionUtils.isNotEmpty(attributeDefs)) {
+                for (AtlasAttributeDef attributeDef : attributeDefs) {
+                    cleanupIndexForAttribute(management, typeDef.getName(), attributeDef);
+                }
+            }
+        } else if (!AtlasTypeUtil.isBuiltInType(typeDef.getName())){
+            throw new IllegalArgumentException("bad data type" + typeDef.getName());
+        }
+    }
+
+    private void cleanupIndexForAttribute(AtlasGraphManagement management, String typeName, AtlasAttributeDef attributeDef) {
+        final String propertyName = GraphHelper.encodePropertyKey(typeName + "." + attributeDef.getName());
+        String attribTypeName = attributeDef.getTypeName();
+        boolean isBuiltInType = AtlasTypeUtil.isBuiltInType(attribTypeName);
+        boolean isArrayType = AtlasTypeUtil.isArrayType(attribTypeName);
+        boolean isMapType = AtlasTypeUtil.isMapType(attribTypeName);
+
+        try {
+            AtlasType atlasType = typeRegistry.getType(attribTypeName);
+
+            if (isMapType || isArrayType || isClassificationType(atlasType) || isEntityType(atlasType)) {
+                LOG.warn("Ignoring non-indexable attribute {}", attribTypeName);
+            } else if (isBuiltInType || isEnumType(atlasType)) {
+                cleanupIndex(management, propertyName);
+            } else if (isStructType(atlasType)) {
+                AtlasStructDef structDef = typeRegistry.getStructDefByName(attribTypeName);
+                cleanupIndices(management, structDef);
+            }
+        } catch (AtlasBaseException e) {
+            LOG.error("No type exists for {}", attribTypeName, e);
+        }
+    }
+
+    private void cleanupIndex(AtlasGraphManagement management, String propertyKey) {
+        if (LOG.isDebugEnabled()) {
+            LOG.debug("Invalidating property key = {}", propertyKey);
+        }
+        management.deletePropertyKey(propertyKey);
+    }
+
     private void attemptRollback(ChangedTypeDefs changedTypeDefs, AtlasGraphManagement management)
             throws AtlasBaseException {
         if (null != management) {

repository/src/test/java/org/apache/atlas/repository/impexp/ExportServiceTest.java

@@ -56,8 +56,10 @@ import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
 
-import static org.testng.Assert.*;
 import static org.mockito.Mockito.mock;
+import static org.testng.Assert.assertEquals;
+import static org.testng.Assert.assertNotNull;
+import static org.testng.Assert.assertTrue;
 
 @Guice(modules = TestOnlyModule.class)
 public class ExportServiceTest {

repository/src/test/java/org/apache/atlas/repository/store/graph/AtlasTypeDefGraphStoreTest.java

@@ -26,6 +26,7 @@ import org.apache.atlas.model.typedef.AtlasClassificationDef;
 import org.apache.atlas.model.typedef.AtlasEntityDef;
 import org.apache.atlas.model.typedef.AtlasEnumDef;
 import org.apache.atlas.model.typedef.AtlasStructDef;
+import org.apache.atlas.model.typedef.AtlasStructDef.AtlasAttributeDef;
 import org.apache.atlas.model.typedef.AtlasTypesDef;
 import org.apache.atlas.repository.graph.AtlasGraphProvider;
 import org.apache.atlas.store.AtlasTypeDefStore;
@@ -37,6 +38,7 @@ import org.testng.annotations.DataProvider;
 import org.testng.annotations.Guice;
 import org.testng.annotations.Test;
 
+import java.util.Arrays;
 import java.util.Collections;
 import java.util.List;
 
@@ -398,4 +400,43 @@ public class AtlasTypeDefGraphStoreTest {
         }
     }
 
+    @Test
+    public void testTypeDeletionAndRecreate() {
+        AtlasClassificationDef aTag = new AtlasClassificationDef("testTag");
+        AtlasAttributeDef attributeDef = new AtlasAttributeDef("testAttribute", "string", true,
+                AtlasAttributeDef.Cardinality.SINGLE, 0, 1,
+                false, true,
+                Collections.<AtlasStructDef.AtlasConstraintDef>emptyList());
+        aTag.addAttribute(attributeDef);
+
+        AtlasTypesDef typesDef = new AtlasTypesDef();
+        typesDef.setClassificationDefs(Arrays.asList(aTag));
+
+        try {
+            typeDefStore.createTypesDef(typesDef);
+        } catch (AtlasBaseException e) {
+            fail("Tag creation should've succeeded");
+        }
+
+        try {
+            typeDefStore.deleteTypesDef(typesDef);
+        } catch (AtlasBaseException e) {
+            fail("Tag deletion should've succeeded");
+        }
+
+        aTag = new AtlasClassificationDef("testTag");
+        attributeDef = new AtlasAttributeDef("testAttribute", "int", true,
+                AtlasAttributeDef.Cardinality.SINGLE, 0, 1,
+                false, true,
+                Collections.<AtlasStructDef.AtlasConstraintDef>emptyList());
+        aTag.addAttribute(attributeDef);
+        typesDef.setClassificationDefs(Arrays.asList(aTag));
+
+        try {
+            typeDefStore.createTypesDef(typesDef);
+        } catch (AtlasBaseException e) {
+            fail("Tag re-creation should've succeeded");
+        }
+    }
+
 }
\ No newline at end of file

repository/src/test/java/org/apache/atlas/repository/store/graph/v1/AtlasEntityStoreV1Test.java

@@ -25,6 +25,7 @@ import org.apache.atlas.TestOnlyModule;
 import org.apache.atlas.TestUtils;
 import org.apache.atlas.TestUtilsV2;
 import org.apache.atlas.exception.AtlasBaseException;
+import org.apache.atlas.model.instance.AtlasClassification;
 import org.apache.atlas.model.instance.AtlasEntity;
 import org.apache.atlas.model.instance.AtlasEntity.AtlasEntitiesWithExtInfo;
 import org.apache.atlas.model.instance.AtlasEntity.AtlasEntityExtInfo;
@@ -35,7 +36,10 @@ import org.apache.atlas.model.instance.AtlasStruct;
 import org.apache.atlas.model.instance.EntityMutationResponse;
 import org.apache.atlas.model.instance.EntityMutations;
 import org.apache.atlas.model.instance.EntityMutations.EntityOperation;
+import org.apache.atlas.model.typedef.AtlasClassificationDef;
 import org.apache.atlas.model.typedef.AtlasEntityDef;
+import org.apache.atlas.model.typedef.AtlasStructDef;
+import org.apache.atlas.model.typedef.AtlasStructDef.AtlasAttributeDef;
 import org.apache.atlas.model.typedef.AtlasTypesDef;
 import org.apache.atlas.repository.graph.AtlasGraphProvider;
 import org.apache.atlas.repository.graph.GraphBackedSearchIndexer;
@@ -64,6 +68,7 @@ import org.testng.annotations.Test;
 import javax.inject.Inject;
 import java.util.ArrayList;
 import java.util.Arrays;
+import java.util.Collections;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
@@ -76,6 +81,7 @@ import static org.apache.atlas.TestUtilsV2.TABLE_TYPE;
 import static org.mockito.Mockito.mock;
 import static org.testng.Assert.assertEquals;
 import static org.testng.Assert.assertTrue;
+import static org.testng.Assert.fail;
 
 @Guice(modules = TestOnlyModule.class)
 public class AtlasEntityStoreV1Test {
@@ -839,6 +845,58 @@ public class AtlasEntityStoreV1Test {
         assertEquals(deletedDb2Entity.getStatus(), AtlasEntity.Status.DELETED);
     }
 
+    @Test
+    public void testTagAssociationAfterRedefinition(){
+        AtlasClassificationDef aTag = new AtlasClassificationDef("testTag");
+        AtlasAttributeDef attributeDef = new AtlasAttributeDef("testAttribute", "int", true,
+                AtlasAttributeDef.Cardinality.SINGLE, 0, 1,
+                false, true,
+                Collections.<AtlasStructDef.AtlasConstraintDef>emptyList());
+        aTag.addAttribute(attributeDef);
+
+        AtlasTypesDef typesDef = new AtlasTypesDef();
+        typesDef.setClassificationDefs(Arrays.asList(aTag));
+
+        try {
+            typeDefStore.createTypesDef(typesDef);
+        } catch (AtlasBaseException e) {
+            fail("Tag creation should've succeeded");
+        }
+
+        try {
+            typeDefStore.deleteTypesDef(typesDef);
+        } catch (AtlasBaseException e) {
+            fail("Tag deletion should've succeeded");
+        }
+
+        aTag = new AtlasClassificationDef("testTag");
+        attributeDef = new AtlasAttributeDef("testAttribute", "string", true,
+                AtlasAttributeDef.Cardinality.SINGLE, 0, 1,
+                false, true,
+                Collections.<AtlasStructDef.AtlasConstraintDef>emptyList());
+        aTag.addAttribute(attributeDef);
+        typesDef.setClassificationDefs(Arrays.asList(aTag));
+
+        try {
+            typeDefStore.createTypesDef(typesDef);
+        } catch (AtlasBaseException e) {
+            fail("Tag re-creation should've succeeded");
+        }
+
+        final AtlasEntity dbEntity  = TestUtilsV2.createDBEntity();
+
+        try {
+            EntityMutationResponse response = entityStore.createOrUpdate(new AtlasEntityStream(dbEntity), false);
+            List<AtlasEntityHeader> createdEntity = response.getCreatedEntities();
+            assertTrue(CollectionUtils.isNotEmpty(createdEntity));
+            String guid = createdEntity.get(0).getGuid();
+            entityStore.addClassification(Arrays.asList(guid), new AtlasClassification(aTag.getName(), "testAttribute", "test-string"));
+        } catch (AtlasBaseException e) {
+            fail("DB entity creation should've succeeded");
+        }
+
+    }
+
     private String randomStrWithReservedChars() {
         return randomString() + "\"${}%";
     }