ATLAS-822 Type updates - don't allow updating supertypes ( shwethags via sumasai )

addons/falcon-bridge/pom.xml

@@ -87,7 +87,6 @@
         <dependency>
             <groupId>org.apache.atlas</groupId>
             <artifactId>atlas-typesystem</artifactId>
-            <version>${parent.version}</version>
             <classifier>tests</classifier>
             <scope>test</scope>
         </dependency>

addons/hdfs-model/pom.xml

@@ -88,7 +88,6 @@
         <dependency>
             <groupId>org.apache.atlas</groupId>
             <artifactId>atlas-typesystem</artifactId>
-            <version>${parent.version}</version>
             <classifier>tests</classifier>
             <scope>test</scope>
         </dependency>

addons/hive-bridge/pom.xml

@@ -150,7 +150,6 @@
         <dependency>
             <groupId>org.apache.atlas</groupId>
             <artifactId>atlas-typesystem</artifactId>
-            <version>${parent.version}</version>
             <classifier>tests</classifier>
             <scope>test</scope>
         </dependency>

addons/sqoop-bridge/pom.xml

@@ -152,7 +152,6 @@
         <dependency>
             <groupId>org.apache.atlas</groupId>
             <artifactId>atlas-typesystem</artifactId>
-            <version>${parent.version}</version>
             <classifier>tests</classifier>
             <scope>test</scope>
         </dependency>

addons/storm-bridge/pom.xml

@@ -127,7 +127,6 @@
         <dependency>
             <groupId>org.apache.atlas</groupId>
             <artifactId>atlas-typesystem</artifactId>
-            <version>${parent.version}</version>
             <classifier>tests</classifier>
             <scope>test</scope>
         </dependency>

catalog/pom.xml

@@ -103,7 +103,6 @@
         <dependency>
             <groupId>org.apache.atlas</groupId>
             <artifactId>atlas-typesystem</artifactId>
-            <version>${parent.version}</version>
             <classifier>tests</classifier>
             <scope>test</scope>
         </dependency>

client/pom.xml

@@ -69,7 +69,6 @@
         <dependency>
             <groupId>org.apache.atlas</groupId>
             <artifactId>atlas-typesystem</artifactId>
-            <version>${parent.version}</version>
             <classifier>tests</classifier>
             <scope>test</scope>
         </dependency>

notification/pom.xml

@@ -98,7 +98,6 @@
         <dependency>
             <groupId>org.apache.atlas</groupId>
             <artifactId>atlas-typesystem</artifactId>
-            <version>${parent.version}</version>
             <classifier>tests</classifier>
             <scope>test</scope>
         </dependency>

pom.xml

@@ -1020,6 +1020,14 @@
 
             <dependency>
                 <groupId>org.apache.atlas</groupId>
+                <artifactId>atlas-typesystem</artifactId>
+                <version>${project.version}</version>
+                <classifier>tests</classifier>
+                <scope>test</scope>
+            </dependency>
+
+            <dependency>
+                <groupId>org.apache.atlas</groupId>
                 <artifactId>atlas-titan</artifactId>
                 <version>${project.version}</version>
             </dependency>

release-log.txt

@@ -12,6 +12,7 @@ ATLAS-925 Change master version to 0.8-incubating (shwethags)
 --Release 0.7-incubating
 
 INCOMPATIBLE CHANGES:
+ATLAS-822 Type updates - don't allow updating supertypes ( shwethags via sumasai )
 ATLAS-698 Remove Rexster Graph API (svimal2106 via shwethags)
 ATLAS-844 Remove titan berkeley and elastic search jars if hbase/solr based profiles are chosen (yhemanth via shwethags)
 ATLAS-819 All user defined types should have a set of common attributes (shwethags)

repository/pom.xml

@@ -158,7 +158,6 @@
         <dependency>
             <groupId>org.apache.atlas</groupId>
             <artifactId>atlas-typesystem</artifactId>
-            <version>${parent.version}</version>
             <classifier>tests</classifier>
             <scope>test</scope>
         </dependency>

repository/src/test/java/org/apache/atlas/repository/typestore/GraphBackedTypeStoreTest.java

@@ -25,7 +25,6 @@ import com.thinkaurelius.titan.core.util.TitanCleanup;
 import com.tinkerpop.blueprints.Direction;
 import com.tinkerpop.blueprints.Edge;
 import com.tinkerpop.blueprints.Vertex;
-
 import org.apache.atlas.AtlasException;
 import org.apache.atlas.RepositoryMetadataModule;
 import org.apache.atlas.TestUtils;
@@ -54,8 +53,6 @@ import org.testng.annotations.Guice;
 import org.testng.annotations.Test;
 
 import javax.inject.Inject;
-
-import java.util.Arrays;
 import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
@@ -172,30 +169,18 @@ public class GraphBackedTypeStoreTest {
                         createRequiredAttrDef("city", DataTypes.STRING_TYPE),
                         createOptionalAttrDef("state", DataTypes.STRING_TYPE));
 
-        //Add supertype
-        HierarchicalTypeDefinition<ClassType> superTypeDef = createClassTypeDef("Division", ImmutableSet.<String>of(),
-                createOptionalAttrDef("dname", DataTypes.STRING_TYPE));
-
         HierarchicalTypeDefinition<ClassType> deptTypeDef = createClassTypeDef("Department", "Department"+_description,
-            ImmutableSet.of(superTypeDef.typeName), createRequiredAttrDef("name", DataTypes.STRING_TYPE),
+            ImmutableSet.<String>of(), createRequiredAttrDef("name", DataTypes.STRING_TYPE),
                 new AttributeDefinition("employees", String.format("array<%s>", "Person"), Multiplicity.OPTIONAL,
                         true, "department"));
         TypesDef typesDef = TypesUtil.getTypesDef(ImmutableList.of(orgLevelEnum), ImmutableList.of(addressDetails),
                 ImmutableList.<HierarchicalTypeDefinition<TraitType>>of(),
-                ImmutableList.of(deptTypeDef, superTypeDef));
+                ImmutableList.of(deptTypeDef));
 
         Map<String, IDataType> typesAdded = ts.updateTypes(typesDef);
         typeStore.store(ts, ImmutableList.copyOf(typesAdded.keySet()));
 
-        // ATLAS-474: verify that type update did not write duplicate edges to the type store.
-        if (typeStore instanceof GraphBackedTypeStore) {
-            GraphBackedTypeStore gbTypeStore = (GraphBackedTypeStore) typeStore;
-            Vertex typeVertex = gbTypeStore.findVertex(TypeCategory.CLASS, "Department");
-            int edgeCount = countOutgoingEdges(typeVertex, GraphBackedTypeStore.SUPERTYPE_EDGE_LABEL);
-            Assert.assertEquals(edgeCount, 1);
-            edgeCount = countOutgoingEdges(typeVertex, gbTypeStore.getEdgeLabel("Department", "employees"));
-            Assert.assertEquals(edgeCount, 1, "Should only be 1 edge for employees attribute on Department type vertex");
-        }
+        verifyEdges();
         
         //Validate the updated types
         TypesDef types = typeStore.restore();
@@ -214,57 +199,24 @@ public class GraphBackedTypeStoreTest {
         Assert.assertEquals(addressType.numFields, 3);
         Assert.assertEquals(addressType.fieldMapping.fields.get("state").dataType(), DataTypes.STRING_TYPE);
 
-        //Assert new super type
-        ClassType deptType = ts.getDataType(ClassType.class, deptTypeDef.typeName);
-        Assert.assertTrue(deptType.superTypes.contains(superTypeDef.typeName));
-        Assert.assertNotNull(ts.getDataType(ClassType.class, superTypeDef.typeName));
-    }
-
-    @Test(dependsOnMethods = "testTypeUpdate")
-    public void testAddSecondSuperType() throws Exception {
-        // Add a second supertype to Department class
-        HierarchicalTypeDefinition<ClassType> superTypeDef2 = createClassTypeDef("SuperClass2", ImmutableSet.<String>of(),
-                createOptionalAttrDef("name", DataTypes.STRING_TYPE));
-        HierarchicalTypeDefinition<ClassType> deptTypeDef = createClassTypeDef("Department",
-            ImmutableSet.of("Division", superTypeDef2.typeName), createRequiredAttrDef("name", DataTypes.STRING_TYPE),
-            new AttributeDefinition("employees", String.format("array<%s>", "Person"), Multiplicity.OPTIONAL,
-                    true, "department"));
-        TypesDef typesDef = TypesUtil.getTypesDef(ImmutableList.<EnumTypeDefinition>of(), ImmutableList.<StructTypeDefinition>of(),
+        //Updating the definition again shouldn't add another edge
+        typesDef = TypesUtil.getTypesDef(ImmutableList.<EnumTypeDefinition>of(),
+                ImmutableList.<StructTypeDefinition>of(),
                 ImmutableList.<HierarchicalTypeDefinition<TraitType>>of(),
-            ImmutableList.of(deptTypeDef, superTypeDef2));
-        ts.updateTypes(typesDef);
-        typeStore.store(ts, ImmutableList.of(superTypeDef2.typeName, deptTypeDef.typeName));
+                ImmutableList.of(deptTypeDef));
+        typesAdded = ts.updateTypes(typesDef);
+        typeStore.store(ts, ImmutableList.copyOf(typesAdded.keySet()));
+        verifyEdges();
+    }
 
+    private void verifyEdges() {
         // ATLAS-474: verify that type update did not write duplicate edges to the type store.
         if (typeStore instanceof GraphBackedTypeStore) {
             GraphBackedTypeStore gbTypeStore = (GraphBackedTypeStore) typeStore;
             Vertex typeVertex = gbTypeStore.findVertex(TypeCategory.CLASS, "Department");
-            // There should now be 2 super type outgoing edges on the Department type vertex.
-            int edgeCount = countOutgoingEdges(typeVertex, GraphBackedTypeStore.SUPERTYPE_EDGE_LABEL);
-            Assert.assertEquals(edgeCount, 2);
-            // There should still be 1 outgoing edge for the employees attribute.
-            edgeCount = countOutgoingEdges(typeVertex, gbTypeStore.getEdgeLabel("Department", "employees"));
-            Assert.assertEquals(edgeCount, 1);
-        }
-        
-        // Verify Department now has 2 super types.
-        TypesDef types = typeStore.restore();
-        for (HierarchicalTypeDefinition<ClassType> classTypeDef : types.classTypesAsJavaList()) {
-            if (classTypeDef.typeName.equals("Department")) {
-                Assert.assertEquals(classTypeDef.superTypes.size(), 2);
-                Assert.assertTrue(classTypeDef.superTypes.containsAll(
-                    Arrays.asList("Division", superTypeDef2.typeName)));
-                break;
-            }
+            int edgeCount = countOutgoingEdges(typeVertex, gbTypeStore.getEdgeLabel("Department", "employees"));
+            Assert.assertEquals(edgeCount, 1, "Should only be 1 edge for employees attribute on Department type vertex");
         }
-        ts.reset();
-        Map<String, IDataType> typesMap = ts.defineTypes(types);
-        IDataType dataType = typesMap.get(deptTypeDef.typeName);
-        Assert.assertTrue(dataType instanceof ClassType);
-        ClassType deptType = (ClassType) dataType;
-        Assert.assertEquals(deptType.superTypes.size(), 2);
-        Assert.assertTrue(deptType.superTypes.containsAll(
-            Arrays.asList("Division", superTypeDef2.typeName)));
     }
 
     private int countOutgoingEdges(Vertex typeVertex, String edgeLabel) {

typesystem/pom.xml

@@ -124,16 +124,6 @@
                 <groupId>org.apache.maven.plugins</groupId>
                 <artifactId>maven-jar-plugin</artifactId>
                 <version>2.4</version>
-                <configuration>
-                    <excludes>
-                        <exclude>**/log4j.xml</exclude>
-                    </excludes>
-                </configuration>
-            </plugin>
-            <plugin>
-                <groupId>org.apache.maven.plugins</groupId>
-                <artifactId>maven-jar-plugin</artifactId>
-                <version>2.4</version>
                 <executions>
                     <execution>
                         <goals>

typesystem/src/main/java/org/apache/atlas/typesystem/types/HierarchicalType.java

@@ -123,8 +123,10 @@ public abstract class HierarchicalType<ST extends HierarchicalType, T> extends A
         HierarchicalType newHierarchicalType = (HierarchicalType) newType;
 
         //validate on supertypes
-        if (!newHierarchicalType.superTypes.containsAll(superTypes)) {
-            throw new TypeUpdateException(newType, "New type doesn't contain all super types of old type");
+
+        if ((newHierarchicalType.superTypes.size() != superTypes.size())
+                || !newHierarchicalType.superTypes.containsAll(superTypes)) {
+            throw new TypeUpdateException(newType, "New type cannot modify superTypes");
         }
 
         //validate on fields

typesystem/src/test/java/org/apache/atlas/typesystem/types/HierarchicalTypeTest.java

@@ -26,29 +26,36 @@ import org.testng.Assert;
 import org.testng.annotations.Test;
 
 public abstract class HierarchicalTypeTest<T extends HierarchicalType> extends TypeUpdateBaseTest {
-    @Test(enabled = false)
+    @Test
     public void testTypeUpdate() throws Exception {
         testTypeUpdateForAttributes();
 
         //Test super types
-        HierarchicalTypeDefinition classType =
-                getTypeDefinition(newName(), TypesUtil.createRequiredAttrDef("a", DataTypes.INT_TYPE));
+        HierarchicalTypeDefinition baseSuperType =
+                getTypeDefinition(newName(), TypesUtil.createOptionalAttrDef("s", DataTypes.INT_TYPE));
+
+        HierarchicalTypeDefinition classType = getTypeDefinition(newName(), ImmutableSet.of(baseSuperType.typeName),
+                        TypesUtil.createRequiredAttrDef("a", DataTypes.INT_TYPE));
         TypeSystem ts = getTypeSystem();
-        ts.defineTypes(getTypesDef(classType));
+        ts.defineTypes(getTypesDef(baseSuperType, classType));
 
         //Add super type with optional attribute
         HierarchicalTypeDefinition superType =
                 getTypeDefinition(newName(), TypesUtil.createOptionalAttrDef("s", DataTypes.INT_TYPE));
-        classType = getTypeDefinition(classType.typeName, ImmutableSet.of(superType.typeName),
+        classType = getTypeDefinition(classType.typeName, ImmutableSet.of(superType.typeName, baseSuperType.typeName),
                 TypesUtil.createRequiredAttrDef("a", DataTypes.INT_TYPE));
+        try {
             ts.updateTypes(getTypesDef(superType, classType));
-
+            Assert.fail("Expected TypeUpdateException");
+        } catch (TypeUpdateException e) {
+            //expected
+        }
 
         //Add super type with required attribute should fail
         HierarchicalTypeDefinition superTypeRequired =
                 getTypeDefinition(newName(), TypesUtil.createRequiredAttrDef("s", DataTypes.INT_TYPE));
         classType = getTypeDefinition(classType.typeName,
-                ImmutableSet.of(superTypeRequired.typeName, superType.typeName),
+                ImmutableSet.of(superTypeRequired.typeName, baseSuperType.typeName),
                 TypesUtil.createRequiredAttrDef("a", DataTypes.INT_TYPE));
         try {
             ts.updateTypes(getTypesDef(superTypeRequired, classType));
@@ -60,7 +67,7 @@ public abstract class HierarchicalTypeTest<T extends HierarchicalType> extends T
         //Deleting super type should fail
         classType = getTypeDefinition(classType.typeName, TypesUtil.createRequiredAttrDef("a", DataTypes.INT_TYPE));
         try {
-            ts.updateTypes(getTypesDef(superType, classType));
+            ts.updateTypes(getTypesDef(classType));
             Assert.fail("Expected TypeUpdateException");
         } catch (TypeUpdateException e) {
             //expected

webapp/pom.xml

@@ -331,7 +331,6 @@
         <dependency>
             <groupId>org.apache.atlas</groupId>
             <artifactId>atlas-typesystem</artifactId>
-            <version>${parent.version}</version>
             <classifier>tests</classifier>
             <scope>test</scope>
         </dependency>

webapp/src/test/java/org/apache/atlas/web/resources/TypesJerseyResourceIT.java

@@ -50,6 +50,8 @@ import javax.ws.rs.core.Response;
 import java.util.ArrayList;
 import java.util.List;
 
+import static org.testng.Assert.assertEquals;
+
 /**
  * Integration test for types jersey resource.
  */
@@ -79,15 +81,15 @@ public class TypesJerseyResourceIT extends BaseResourceIT {
 
             ClientResponse clientResponse = resource.accept(Servlets.JSON_MEDIA_TYPE).type(Servlets.JSON_MEDIA_TYPE)
                     .method(HttpMethod.POST, ClientResponse.class, typesAsJSON);
-            Assert.assertEquals(clientResponse.getStatus(), Response.Status.CREATED.getStatusCode());
+            assertEquals(clientResponse.getStatus(), Response.Status.CREATED.getStatusCode());
 
             String responseAsString = clientResponse.getEntity(String.class);
             Assert.assertNotNull(responseAsString);
 
             JSONObject response = new JSONObject(responseAsString);
             JSONArray typesAdded = response.getJSONArray(AtlasClient.TYPES);
-            Assert.assertEquals(typesAdded.length(), 1);
-            Assert.assertEquals(typesAdded.getJSONObject(0).getString("name"), typeDefinition.typeName);
+            assertEquals(typesAdded.length(), 1);
+            assertEquals(typesAdded.getJSONObject(0).getString("name"), typeDefinition.typeName);
             Assert.assertNotNull(response.get(AtlasClient.REQUEST_ID));
         }
     }
@@ -98,25 +100,24 @@ public class TypesJerseyResourceIT extends BaseResourceIT {
                 .createClassTypeDef(randomString(), ImmutableSet.<String>of(),
                         TypesUtil.createUniqueRequiredAttrDef("name", DataTypes.STRING_TYPE));
         List<String> typesCreated = serviceClient.createType(TypesSerialization.toJson(typeDefinition, false));
-        Assert.assertEquals(typesCreated.size(), 1);
-        Assert.assertEquals(typesCreated.get(0), typeDefinition.typeName);
-
-        //Add super type
-        HierarchicalTypeDefinition<ClassType> superTypeDefinition = TypesUtil
-                .createClassTypeDef(randomString(), ImmutableSet.<String>of(),
-                        TypesUtil.createOptionalAttrDef("sname", DataTypes.STRING_TYPE));
+        assertEquals(typesCreated.size(), 1);
+        assertEquals(typesCreated.get(0), typeDefinition.typeName);
 
+        //Add attribute description
         typeDefinition = TypesUtil.createClassTypeDef(typeDefinition.typeName,
-            ImmutableSet.of(superTypeDefinition.typeName),
+            ImmutableSet.<String>of(),
                 TypesUtil.createUniqueRequiredAttrDef("name", DataTypes.STRING_TYPE),
                 TypesUtil.createOptionalAttrDef("description", DataTypes.STRING_TYPE));
         TypesDef typeDef = TypesUtil.getTypesDef(ImmutableList.<EnumTypeDefinition>of(),
                 ImmutableList.<StructTypeDefinition>of(), ImmutableList.<HierarchicalTypeDefinition<TraitType>>of(),
-                ImmutableList.of(superTypeDefinition, typeDefinition));
+                ImmutableList.of(typeDefinition));
         List<String> typesUpdated = serviceClient.updateType(typeDef);
-        Assert.assertEquals(typesUpdated.size(), 2);
-        Assert.assertTrue(typesUpdated.contains(superTypeDefinition.typeName));
+        assertEquals(typesUpdated.size(), 1);
         Assert.assertTrue(typesUpdated.contains(typeDefinition.typeName));
+
+        HierarchicalTypeDefinition<ClassType>
+                updatedType = serviceClient.getType(typeDefinition.typeName).classTypesAsJavaList().get(0);
+        assertEquals(updatedType.attributeDefinitions.length, 2);
     }
 
     @Test(dependsOnMethods = "testSubmit")
@@ -128,7 +129,7 @@ public class TypesJerseyResourceIT extends BaseResourceIT {
 
             ClientResponse clientResponse = resource.accept(Servlets.JSON_MEDIA_TYPE).type(Servlets.JSON_MEDIA_TYPE)
                     .method(HttpMethod.GET, ClientResponse.class);
-            Assert.assertEquals(clientResponse.getStatus(), Response.Status.OK.getStatusCode());
+            assertEquals(clientResponse.getStatus(), Response.Status.OK.getStatusCode());
 
             String responseAsString = clientResponse.getEntity(String.class);
             Assert.assertNotNull(responseAsString);
@@ -142,8 +143,8 @@ public class TypesJerseyResourceIT extends BaseResourceIT {
             for (HierarchicalTypeDefinition<ClassType> classType : hierarchicalTypeDefinitions) {
                 for (AttributeDefinition attrDef : classType.attributeDefinitions) {
                     if ("name".equals(attrDef.name)) {
-                        Assert.assertEquals(attrDef.isIndexable, true);
-                        Assert.assertEquals(attrDef.isUnique, true);
+                        assertEquals(attrDef.isIndexable, true);
+                        assertEquals(attrDef.isUnique, true);
                     }
                 }
             }
@@ -156,7 +157,7 @@ public class TypesJerseyResourceIT extends BaseResourceIT {
 
         ClientResponse clientResponse = resource.accept(Servlets.JSON_MEDIA_TYPE).type(Servlets.JSON_MEDIA_TYPE)
                 .method(HttpMethod.GET, ClientResponse.class);
-        Assert.assertEquals(clientResponse.getStatus(), Response.Status.NOT_FOUND.getStatusCode());
+        assertEquals(clientResponse.getStatus(), Response.Status.NOT_FOUND.getStatusCode());
     }
 
     @Test(dependsOnMethods = "testSubmit")
@@ -165,7 +166,7 @@ public class TypesJerseyResourceIT extends BaseResourceIT {
 
         ClientResponse clientResponse = resource.accept(Servlets.JSON_MEDIA_TYPE).type(Servlets.JSON_MEDIA_TYPE)
                 .method(HttpMethod.GET, ClientResponse.class);
-        Assert.assertEquals(clientResponse.getStatus(), Response.Status.OK.getStatusCode());
+        assertEquals(clientResponse.getStatus(), Response.Status.OK.getStatusCode());
 
         String responseAsString = clientResponse.getEntity(String.class);
         Assert.assertNotNull(responseAsString);
@@ -191,7 +192,7 @@ public class TypesJerseyResourceIT extends BaseResourceIT {
         ClientResponse clientResponse =
                 resource.queryParam("type", DataTypes.TypeCategory.TRAIT.name()).accept(Servlets.JSON_MEDIA_TYPE)
                         .type(Servlets.JSON_MEDIA_TYPE).method(HttpMethod.GET, ClientResponse.class);
-        Assert.assertEquals(clientResponse.getStatus(), Response.Status.OK.getStatusCode());
+        assertEquals(clientResponse.getStatus(), Response.Status.OK.getStatusCode());
 
         String responseAsString = clientResponse.getEntity(String.class);
         Assert.assertNotNull(responseAsString);