----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/71649/#review218869 -----------------------------------------------------------
intg/src/main/java/org/apache/atlas/model/typedef/AtlasNamespaceDef.java Lines 75 (patched) <https://reviews.apache.org/r/71649/#comment306778> this check is incorrect - adding null/empty 'attributeDefs' will reset the attributeDefs to emplty list. (old entries are removed). Please review if (CollectionUtils.isNotEmpty(attributeDefs)) { this.attributeDefs.addAll(attributeDefs); } intg/src/main/java/org/apache/atlas/model/typedef/AtlasNamespaceDef.java Lines 162 (patched) <https://reviews.apache.org/r/71649/#comment306779> 'maxStrLength' and 'validPattern' are string attribute specific constraints, consider adding this constraints in AtlasAttributeDef.options map (define allowed string constraint enums). This will be extensible to add any condition in the future. intg/src/main/java/org/apache/atlas/type/AtlasEntityType.java Lines 49 (patched) <https://reviews.apache.org/r/71649/#comment306776> nit: remove new line intg/src/main/java/org/apache/atlas/type/AtlasEntityType.java Lines 1000 (patched) <https://reviews.apache.org/r/71649/#comment306777> consider refactoring this method: private void addNamespaceAttribute(AtlasNamespaceAttribute nsAttribute) { String nsName = nsAttribute.getDefinedInType().getTypeName(); if (!namespaceAttributes.containsKey(nsName)) { namespaceAttributes.put(nsName, new ArrayList<>()); } namespaceAttributes.get(nsName).add(nsAttribute); } intg/src/main/java/org/apache/atlas/type/AtlasNamespaceType.java Lines 31 (patched) <https://reviews.apache.org/r/71649/#comment306782> nit: remove unused import. repository/src/main/java/org/apache/atlas/repository/graph/GraphBackedSearchIndexer.java Lines 586 (patched) <https://reviews.apache.org/r/71649/#comment306783> this method is similar to method in line 505. consider refactoring to a single method and change behavior for AtlasNamespaceDef and AtlasStructDef repository/src/main/java/org/apache/atlas/repository/store/graph/v2/AtlasAbstractDefStoreV2.java Lines 67 (patched) <https://reviews.apache.org/r/71649/#comment306775> consider refactoring line 67-81 to avoid duplicates: if (typeDef instanceof AtlasStructDef) { for (AtlasStructDef.AtlasAttributeDef attrDef : ((AtlasStructDef) typeDef).getAttributeDefs()) { validateAttributeName(typeDef, attrDef.getName()); } } else if (typeDef instanceof AtlasNamespaceDef) { for (AtlasNamespaceDef.AtlasNamespaceAttributeDef attrDef : ((AtlasNamespaceDef) typeDef).getAttributeDefs()) { validateAttributeName(typeDef, attrDef.getName()); } } private void validateAttributeName(AtlasBaseTypeDef typeDef, String attributeName) throws AtlasBaseException { if (AtlasDSL.Parser.isKeyword(attributeName)) { throw new AtlasBaseException(AtlasErrorCode.ATTRIBUTE_NAME_INVALID, attributeName, typeDef.getCategory().name()); } } repository/src/main/java/org/apache/atlas/repository/store/graph/v2/AtlasAbstractDefStoreV2.java Lines 93 (patched) <https://reviews.apache.org/r/71649/#comment306774> nit: remove new line repository/src/main/java/org/apache/atlas/repository/store/graph/v2/AtlasNamespaceDefStoreV2.java Lines 370 (patched) <https://reviews.apache.org/r/71649/#comment306780> encodedtypeNamePropertyKey => encodedTypeNamePropertyKey repository/src/test/java/org/apache/atlas/repository/store/graph/v2/AtlasNamespaceDefStoreV2Test.java Lines 32 (patched) <https://reviews.apache.org/r/71649/#comment306781> nit: remove unused imports in line 32-33 - Sarath Subramanian On Nov. 21, 2019, 6:03 p.m., Aadarsh Jajodia wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/71649/ > ----------------------------------------------------------- > > (Updated Nov. 21, 2019, 6:03 p.m.) > > > Review request for atlas, Ashutosh Mestry, Sridhar K, Le Ma, Madhan > Neethiraj, and Sarath Subramanian. > > > Bugs: ATLAS-3486 > https://issues.apache.org/jira/browse/ATLAS-3486 > > > Repository: atlas > > > Description > ------- > > This change is the first part of the bigger task defined as part of > ATLAS-3485. > This adds the data model needed for supporting namespaces and namespace > attributes and also updates the type registry to include the applicable > namespace attributes for every entity type > > > Diffs > ----- > > intg/src/main/java/org/apache/atlas/AtlasErrorCode.java > 7a2aae2e9ae8174c5309164f3e41c940cbf3ddf8 > intg/src/main/java/org/apache/atlas/model/TypeCategory.java > f06f64f450f407e3f9a0e742726ff4dd12ccc695 > intg/src/main/java/org/apache/atlas/model/typedef/AtlasNamespaceDef.java > PRE-CREATION > intg/src/main/java/org/apache/atlas/model/typedef/AtlasStructDef.java > bb7ead0f9f8bab3094eb82e9e286dd58e8a6e3de > intg/src/main/java/org/apache/atlas/model/typedef/AtlasTypesDef.java > 3634fdfd313639eb97b3c4698e091487b0e44a80 > intg/src/main/java/org/apache/atlas/store/AtlasTypeDefStore.java > 4ee68a936f99bb4c819b5335da2cc8bf7d539397 > intg/src/main/java/org/apache/atlas/type/AtlasEntityType.java > 557ef74a95c2a939b4b89cd1db8fa4c73d52dd51 > intg/src/main/java/org/apache/atlas/type/AtlasNamespaceType.java > PRE-CREATION > intg/src/main/java/org/apache/atlas/type/AtlasTypeRegistry.java > b071dc9d664cee9e1ffc54726ffbf15f4f602d30 > intg/src/main/java/org/apache/atlas/type/AtlasTypeUtil.java > 0883d54f490e22c6510e6fc0cb804b87713a7ecb > intg/src/main/java/org/apache/atlas/typesystem/types/DataTypes.java > dba2d88146eff314191ae6bb24ad7337b0ea10ae > intg/src/test/java/org/apache/atlas/TestRelationshipUtilsV2.java > 02613b5f7250b14324ed294c22de079b74d55b08 > intg/src/test/java/org/apache/atlas/TestUtilsV2.java > ff79994c519702e90b2e478d00cae0008889f956 > > intg/src/test/java/org/apache/atlas/model/typedef/TestAtlasNamespaceDef.java > PRE-CREATION > > repository/src/main/java/org/apache/atlas/repository/graph/GraphBackedSearchIndexer.java > 7c551304b2b65b90302f6e5fa9fc5b9f1b8e2c12 > > repository/src/main/java/org/apache/atlas/repository/store/graph/AtlasTypeDefGraphStore.java > 2e2ab1a664171555c57560e1c0b4cbdbc20c0f6f > > repository/src/main/java/org/apache/atlas/repository/store/graph/v2/AtlasAbstractDefStoreV2.java > 2cb2b47898ded4d6c5c84800ff93fa58b2c480da > > repository/src/main/java/org/apache/atlas/repository/store/graph/v2/AtlasNamespaceDefStoreV2.java > PRE-CREATION > > repository/src/main/java/org/apache/atlas/repository/store/graph/v2/AtlasTypeDefGraphStoreV2.java > a5ccfb5b2055c88f596312f4033bc0034d3d165c > > repository/src/test/java/org/apache/atlas/repository/store/graph/v2/AtlasNamespaceDefStoreV2Test.java > PRE-CREATION > webapp/src/main/java/org/apache/atlas/examples/QuickStartV2.java > 6cd0ee331b7ae24757b58e76ec47bf556106846a > webapp/src/main/java/org/apache/atlas/web/rest/TypesREST.java > fb56fad6412079e20dd3e345b81a08d9e5ace657 > > > Diff: https://reviews.apache.org/r/71649/diff/8/ > > > Testing > ------- > > Added unit tests > > > Thanks, > > Aadarsh Jajodia > >
