Repository: incubator-atlas Updated Branches: refs/heads/0.8-incubating ef6b1c1ed -> f85b1b4c8
ATLAS-1730,1736,1747: fixes in type validations and special character handling in attribute names Signed-off-by: Madhan Neethiraj <mad...@apache.org> (cherry picked from commit 3a9594e1811e7c860f265fea0ef79713612ed0f5) Project: http://git-wip-us.apache.org/repos/asf/incubator-atlas/repo Commit: http://git-wip-us.apache.org/repos/asf/incubator-atlas/commit/f85b1b4c Tree: http://git-wip-us.apache.org/repos/asf/incubator-atlas/tree/f85b1b4c Diff: http://git-wip-us.apache.org/repos/asf/incubator-atlas/diff/f85b1b4c Branch: refs/heads/0.8-incubating Commit: f85b1b4c89ded3452bfbc1a33a69cba73880ea51 Parents: ef6b1c1 Author: apoorvnaik <an...@hortonworks.com> Authored: Mon May 1 16:51:43 2017 -0700 Committer: Madhan Neethiraj <mad...@apache.org> Committed: Mon May 1 17:34:56 2017 -0700 ---------------------------------------------------------------------- .../store/graph/v1/AtlasStructDefStoreV1.java | 13 ++++ .../typestore/GraphBackedTypeStore.java | 55 ++++++--------- .../typestore/GraphBackedTypeStoreTest.java | 73 ++++++++++++-------- 3 files changed, 77 insertions(+), 64 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/incubator-atlas/blob/f85b1b4c/repository/src/main/java/org/apache/atlas/repository/store/graph/v1/AtlasStructDefStoreV1.java ---------------------------------------------------------------------- diff --git a/repository/src/main/java/org/apache/atlas/repository/store/graph/v1/AtlasStructDefStoreV1.java b/repository/src/main/java/org/apache/atlas/repository/store/graph/v1/AtlasStructDefStoreV1.java index 6f1b80c..1c6cfc7 100644 --- a/repository/src/main/java/org/apache/atlas/repository/store/graph/v1/AtlasStructDefStoreV1.java +++ b/repository/src/main/java/org/apache/atlas/repository/store/graph/v1/AtlasStructDefStoreV1.java @@ -103,6 +103,10 @@ public class AtlasStructDefStoreV1 extends AtlasAbstractDefStoreV1 implements At vertex = (AtlasVertex)preCreateResult; } + if (CollectionUtils.isEmpty(structDef.getAttributeDefs())) { + throw new AtlasBaseException(AtlasErrorCode.BAD_REQUEST, "Missing attributes for structdef"); + } + AtlasStructDefStoreV1.updateVertexAddReferences(structDef, vertex, typeDefStore); AtlasStructDef ret = toStructDef(vertex); @@ -410,6 +414,9 @@ public class AtlasStructDefStoreV1 extends AtlasAbstractDefStoreV1 implements At typeDefStore.updateTypeVertex(structDef, vertex); + // Load up current struct definition for matching attributes + AtlasStructDef currentStructDef = toStructDef(vertex, new AtlasStructDef(), typeDefStore); + // add/update attributes that are present in updated structDef if (CollectionUtils.isNotEmpty(structDef.getAttributeDefs())) { for (AtlasAttributeDef attributeDef : structDef.getAttributeDefs()) { @@ -419,6 +426,7 @@ public class AtlasStructDefStoreV1 extends AtlasAbstractDefStoreV1 implements At throw new AtlasBaseException(AtlasErrorCode.CANNOT_ADD_MANDATORY_ATTRIBUTE, structDef.getName(), attributeDef.getName()); } } + // Validate the mandatory features of an attribute (compatibility with legacy type system) if (StringUtils.isEmpty(attributeDef.getName())) { throw new AtlasBaseException(AtlasErrorCode.MISSING_MANDATORY_ATTRIBUTE, structDef.getName(), "name"); @@ -427,6 +435,11 @@ public class AtlasStructDefStoreV1 extends AtlasAbstractDefStoreV1 implements At throw new AtlasBaseException(AtlasErrorCode.MISSING_MANDATORY_ATTRIBUTE, structDef.getName(), "typeName"); } + AtlasAttributeDef existingAttribute = currentStructDef.getAttribute(attributeDef.getName()); + if (null != existingAttribute && !attributeDef.getTypeName().equals(existingAttribute.getTypeName())) { + throw new AtlasBaseException(AtlasErrorCode.BAD_REQUEST, "Data type update for attribute is not supported"); + } + String propertyKey = AtlasGraphUtilsV1.getTypeDefPropertyKey(structDef, attributeDef.getName()); AtlasGraphUtilsV1.setProperty(vertex, propertyKey, toJsonFromAttribute(structType.getAttribute(attributeDef.getName()))); http://git-wip-us.apache.org/repos/asf/incubator-atlas/blob/f85b1b4c/repository/src/main/java/org/apache/atlas/repository/typestore/GraphBackedTypeStore.java ---------------------------------------------------------------------- diff --git a/repository/src/main/java/org/apache/atlas/repository/typestore/GraphBackedTypeStore.java b/repository/src/main/java/org/apache/atlas/repository/typestore/GraphBackedTypeStore.java index 7037d1e..ac13586 100644 --- a/repository/src/main/java/org/apache/atlas/repository/typestore/GraphBackedTypeStore.java +++ b/repository/src/main/java/org/apache/atlas/repository/typestore/GraphBackedTypeStore.java @@ -18,16 +18,12 @@ package org.apache.atlas.repository.typestore; -import static org.apache.atlas.repository.graph.GraphHelper.setProperty; - -import java.util.ArrayList; -import java.util.HashMap; -import java.util.HashSet; -import java.util.Iterator; -import java.util.List; -import java.util.Map; -import java.util.Set; - +import com.google.common.base.Function; +import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableSet; +import com.google.common.collect.Lists; +import com.google.inject.Inject; +import com.google.inject.Singleton; import org.apache.atlas.AtlasException; import org.apache.atlas.GraphTransaction; import org.apache.atlas.repository.Constants; @@ -39,33 +35,22 @@ import org.apache.atlas.repository.graphdb.AtlasEdgeDirection; import org.apache.atlas.repository.graphdb.AtlasGraph; import org.apache.atlas.repository.graphdb.AtlasVertex; import org.apache.atlas.typesystem.TypesDef; -import org.apache.atlas.typesystem.types.AttributeDefinition; -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.*; import org.apache.atlas.typesystem.types.DataTypes.TypeCategory; -import org.apache.atlas.typesystem.types.EnumType; -import org.apache.atlas.typesystem.types.EnumTypeDefinition; -import org.apache.atlas.typesystem.types.EnumValue; -import org.apache.atlas.typesystem.types.HierarchicalType; -import org.apache.atlas.typesystem.types.HierarchicalTypeDefinition; -import org.apache.atlas.typesystem.types.IDataType; -import org.apache.atlas.typesystem.types.StructType; -import org.apache.atlas.typesystem.types.StructTypeDefinition; -import org.apache.atlas.typesystem.types.TraitType; -import org.apache.atlas.typesystem.types.TypeSystem; -import org.apache.atlas.typesystem.types.TypeUtils; import org.apache.atlas.typesystem.types.utils.TypesUtil; import org.codehaus.jettison.json.JSONException; import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import com.google.common.base.Function; -import com.google.common.collect.ImmutableList; -import com.google.common.collect.ImmutableSet; -import com.google.common.collect.Lists; -import com.google.inject.Inject; -import com.google.inject.Singleton; +import java.util.ArrayList; +import java.util.HashMap; +import java.util.HashSet; +import java.util.Iterator; +import java.util.List; +import java.util.Map; +import java.util.Set; + +import static org.apache.atlas.repository.graph.GraphHelper.setProperty; @Singleton @Deprecated @@ -298,7 +283,7 @@ public class GraphBackedTypeStore implements ITypeStore { String typeName = GraphHelper.getSingleValuedProperty(vertex, Constants.TYPENAME_PROPERTY_KEY, String.class); String typeDescription = GraphHelper.getSingleValuedProperty(vertex, Constants.TYPEDESCRIPTION_PROPERTY_KEY, String.class); List<EnumValue> enumValues = new ArrayList<>(); - List<String> values = vertex.getListProperty(getPropertyKey(typeName)); + List<String> values = GraphHelper.getListProperty(vertex, getPropertyKey(typeName)); for (String value : values) { String valueProperty = getPropertyKey(typeName, value); enumValues.add(new EnumValue(value, GraphHelper.getSingleValuedProperty(vertex, valueProperty, Integer.class))); @@ -316,12 +301,12 @@ public class GraphBackedTypeStore implements ITypeStore { private AttributeDefinition[] getAttributes(AtlasVertex vertex, String typeName) throws AtlasException { List<AttributeDefinition> attributes = new ArrayList<>(); - List<String> attrNames = vertex.getListProperty(getPropertyKey(typeName)); + List<String> attrNames = GraphHelper.getListProperty(vertex, getPropertyKey(typeName)); if (attrNames != null) { for (String attrName : attrNames) { try { - String propertyKey = getPropertyKey(typeName, attrName); - AttributeDefinition attrValue = AttributeInfo.fromJson((String) vertex.getJsonProperty(propertyKey)); + String encodedPropertyKey = GraphHelper.encodePropertyKey(getPropertyKey(typeName, attrName)); + AttributeDefinition attrValue = AttributeInfo.fromJson((String) vertex.getJsonProperty(encodedPropertyKey)); if (attrValue != null) { attributes.add(attrValue); http://git-wip-us.apache.org/repos/asf/incubator-atlas/blob/f85b1b4c/repository/src/test/java/org/apache/atlas/repository/typestore/GraphBackedTypeStoreTest.java ---------------------------------------------------------------------- diff --git a/repository/src/test/java/org/apache/atlas/repository/typestore/GraphBackedTypeStoreTest.java b/repository/src/test/java/org/apache/atlas/repository/typestore/GraphBackedTypeStoreTest.java index c08bb88..732a382 100755 --- a/repository/src/test/java/org/apache/atlas/repository/typestore/GraphBackedTypeStoreTest.java +++ b/repository/src/test/java/org/apache/atlas/repository/typestore/GraphBackedTypeStoreTest.java @@ -18,18 +18,8 @@ package org.apache.atlas.repository.typestore; -import static org.apache.atlas.typesystem.types.utils.TypesUtil.createClassTypeDef; -import static org.apache.atlas.typesystem.types.utils.TypesUtil.createOptionalAttrDef; -import static org.apache.atlas.typesystem.types.utils.TypesUtil.createRequiredAttrDef; -import static org.apache.atlas.typesystem.types.utils.TypesUtil.createStructTypeDef; - -import java.util.Collections; -import java.util.Iterator; -import java.util.List; -import java.util.Map; - -import javax.inject.Inject; - +import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableSet; import org.apache.atlas.AtlasException; import org.apache.atlas.RepositoryMetadataModule; import org.apache.atlas.TestUtils; @@ -41,20 +31,7 @@ import org.apache.atlas.repository.graphdb.AtlasEdgeDirection; import org.apache.atlas.repository.graphdb.AtlasGraph; import org.apache.atlas.repository.graphdb.AtlasVertex; import org.apache.atlas.typesystem.TypesDef; -import org.apache.atlas.typesystem.types.AttributeDefinition; -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.EnumType; -import org.apache.atlas.typesystem.types.EnumTypeDefinition; -import org.apache.atlas.typesystem.types.EnumValue; -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.StructType; -import org.apache.atlas.typesystem.types.StructTypeDefinition; -import org.apache.atlas.typesystem.types.TraitType; -import org.apache.atlas.typesystem.types.TypeSystem; +import org.apache.atlas.typesystem.types.*; import org.apache.atlas.typesystem.types.utils.TypesUtil; import org.testng.Assert; import org.testng.annotations.AfterClass; @@ -62,8 +39,16 @@ import org.testng.annotations.BeforeClass; import org.testng.annotations.Guice; import org.testng.annotations.Test; -import com.google.common.collect.ImmutableList; -import com.google.common.collect.ImmutableSet; +import javax.inject.Inject; +import java.util.Collections; +import java.util.Iterator; +import java.util.List; +import java.util.Map; + +import static org.apache.atlas.typesystem.types.utils.TypesUtil.createClassTypeDef; +import static org.apache.atlas.typesystem.types.utils.TypesUtil.createOptionalAttrDef; +import static org.apache.atlas.typesystem.types.utils.TypesUtil.createRequiredAttrDef; +import static org.apache.atlas.typesystem.types.utils.TypesUtil.createStructTypeDef; @Guice(modules = RepositoryMetadataModule.class) public class GraphBackedTypeStoreTest { @@ -149,6 +134,37 @@ public class GraphBackedTypeStoreTest { ts.defineTypes(types); } + @Test + public void testTypeWithSpecialChars() throws AtlasException { + HierarchicalTypeDefinition<ClassType> specialTypeDef1 = createClassTypeDef("SpecialTypeDef1", "Typedef with special character", + ImmutableSet.<String>of(), createRequiredAttrDef("attribute$", DataTypes.STRING_TYPE)); + + HierarchicalTypeDefinition<ClassType> specialTypeDef2 = createClassTypeDef("SpecialTypeDef2", "Typedef with special character", + ImmutableSet.<String>of(), createRequiredAttrDef("attribute%", DataTypes.STRING_TYPE)); + + HierarchicalTypeDefinition<ClassType> specialTypeDef3 = createClassTypeDef("SpecialTypeDef3", "Typedef with special character", + ImmutableSet.<String>of(), createRequiredAttrDef("attribute{", DataTypes.STRING_TYPE)); + + HierarchicalTypeDefinition<ClassType> specialTypeDef4 = createClassTypeDef("SpecialTypeDef4", "Typedef with special character", + ImmutableSet.<String>of(), createRequiredAttrDef("attribute}", DataTypes.STRING_TYPE)); + + HierarchicalTypeDefinition<ClassType> specialTypeDef5 = createClassTypeDef("SpecialTypeDef5", "Typedef with special character", + ImmutableSet.<String>of(), createRequiredAttrDef("attribute$%{}", DataTypes.STRING_TYPE)); + + TypesDef typesDef = TypesUtil.getTypesDef(ImmutableList.<EnumTypeDefinition>of(), + ImmutableList.<StructTypeDefinition>of(), + ImmutableList.<HierarchicalTypeDefinition<TraitType>>of(), + ImmutableList.of(specialTypeDef1, specialTypeDef2, specialTypeDef3, specialTypeDef4, specialTypeDef5)); + + Map<String, IDataType> createdTypes = ts.defineTypes(typesDef); + typeStore.store(ts, ImmutableList.copyOf(createdTypes.keySet())); + + //Validate the updated types + TypesDef types = typeStore.restore(); + ts.reset(); + ts.defineTypes(types); + } + @Test(dependsOnMethods = "testStore") public void testTypeUpdate() throws Exception { //Add enum value @@ -238,5 +254,4 @@ public class GraphBackedTypeStoreTest { } Assert.assertTrue(clsTypeFound, typeName + " type not restored"); } - }