----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/59881/#review177425 -----------------------------------------------------------
intg/src/main/java/org/apache/atlas/model/typedef/AtlasStructDef.java Line 278 (original), 284 (patched) <https://reviews.apache.org/r/59881/#comment250975> Retain exising constructor and add a new one that takes additional parameter 'defaultValue'; same comment for the next constructor as well. intg/src/main/java/org/apache/atlas/model/typedef/AtlasStructDef.java Lines 463 (patched) <https://reviews.apache.org/r/59881/#comment250956> "defaultValue == that.defaultValue" ==> Object.equals(defaultValue, that.defaultValue) intg/src/main/java/org/apache/atlas/type/AtlasArrayType.java Line 204 (original), 223 (patched) <https://reviews.apache.org/r/59881/#comment250966> To handle obj of type "String" (will be the case when AtlasType.createDefaultValue(Object val) calls this method), consider adding the following: if (obj instanceof String) { obj = AtlasType.fromJson(obj, List.class); } intg/src/main/java/org/apache/atlas/type/AtlasMapType.java Line 150 (original), 163 (patched) <https://reviews.apache.org/r/59881/#comment250967> To handle obj of type "String" (will be the case when AtlasType.createDefaultValue(Object val) calls this method), consider adding the following: if (obj instanceof String) { obj = AtlasType.fromJson(obj, Map.class); } intg/src/main/java/org/apache/atlas/type/AtlasStructType.java Line 269 (original), 280 (patched) <https://reviews.apache.org/r/59881/#comment250971> To handle obj of type "String" (will be the case when AtlasType.createDefaultValue(Object val) calls this method), consider adding the following: if (obj instanceof String) { obj = AtlasType.fromJson(obj, Map.class); } intg/src/main/java/org/apache/atlas/type/AtlasType.java Lines 71 (patched) <https://reviews.apache.org/r/59881/#comment250965> - change parameter type to "Object" - instead of overriding createDefaultValue(Object val) in each implementation, consider the following default implementation here: public Object createDefaultValue(Object val) { return val == null ? createDefaultValue() : getNormalizedValue(val); } intg/src/test/java/org/apache/atlas/TestUtilsV2.java Line 618 (original), 618 (patched) <https://reviews.apache.org/r/59881/#comment250977> Existing constructors of AtlasAttributeDef should be retained - please see earlier comment in AtlasStructDef.java. Once that is taken care of, changes here (addition of null parameter) should be reverted. intg/src/test/java/org/apache/atlas/model/typedef/TestAtlasStructDef.java Line 71 (original), 71 (patched) <https://reviews.apache.org/r/59881/#comment250979> "AtlasBaseTypeDef.ATLAS_TYPE_STRING" doesn't look like a good default value for INT type. Anyway, you should revert this change and add another test for a constructor that take a default value. intg/src/test/java/org/apache/atlas/type/TestAtlasEntityType.java Line 41 (original), 41 (patched) <https://reviews.apache.org/r/59881/#comment250980> This patch has bunch of whitespace changes. Consider avoiding these. These make it difficult to find the real changes to review (and can make merge resolutions difficult during backport). repository/src/main/java/org/apache/atlas/repository/store/graph/AtlasTypeDefGraphStore.java Lines 874 (patched) <https://reviews.apache.org/r/59881/#comment250981> I guess this was added for troubleshooting. Consider adding LOG.debug() or remove this line. repository/src/main/java/org/apache/atlas/repository/store/graph/v1/EntityGraphMapper.java Line 261 (original), 261 (patched) <https://reviews.apache.org/r/59881/#comment250984> Consider replacing this block with: if (attrValue == null) { AtlasAttributeDef attributeDef = attribute.getAttributeDef(); if (attributeDef.getDefaultValue() != null) { attrValue = attrType.createDefaultValue(attributeDef.getdefaultValue()); } else { if (attribute.getAttributeDef().getIsOptional()) { attrValue = attrType.createOptionalDefaultValue(); } else { attrValue = attrType.createDefaultValue(); } } } - Madhan Neethiraj On June 7, 2017, 2:25 p.m., Ruchi Solani wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/59881/ > ----------------------------------------------------------- > > (Updated June 7, 2017, 2:25 p.m.) > > > Review request for atlas, Apoorv Naik, Madhan Neethiraj, Nixon Rodrigues, and > Sarath Subramanian. > > > Bugs: ATLAS-1863 > https://issues.apache.org/jira/browse/ATLAS-1863 > > > Repository: atlas > > > Description > ------- > > While creating entity if attribute value are not set explicitly for primitive > type which are optional, then default value should be set from attributedef. > > > Diffs > ----- > > intg/src/main/java/org/apache/atlas/model/typedef/AtlasStructDef.java > aee4907 > intg/src/main/java/org/apache/atlas/type/AtlasArrayType.java 2d386f1 > intg/src/main/java/org/apache/atlas/type/AtlasBuiltInTypes.java f08a601 > intg/src/main/java/org/apache/atlas/type/AtlasEnumType.java 1cd27b3 > intg/src/main/java/org/apache/atlas/type/AtlasMapType.java 385a9ae > intg/src/main/java/org/apache/atlas/type/AtlasStructType.java 0eeaf9c > intg/src/main/java/org/apache/atlas/type/AtlasType.java 28d0a07 > intg/src/main/java/org/apache/atlas/type/AtlasTypeUtil.java c0135f5 > intg/src/test/java/org/apache/atlas/TestUtilsV2.java 2a6ea92 > intg/src/test/java/org/apache/atlas/model/ModelTestUtil.java 084bcc4 > intg/src/test/java/org/apache/atlas/model/typedef/TestAtlasStructDef.java > 8d2bfe2 > intg/src/test/java/org/apache/atlas/type/TestAtlasEntityType.java 3c53c02 > intg/src/test/java/org/apache/atlas/type/TestAtlasStructType.java a37dd46 > intg/src/test/java/org/apache/atlas/type/TestAtlasTypeRegistry.java accba77 > > repository/src/main/java/org/apache/atlas/repository/store/graph/AtlasTypeDefGraphStore.java > 17b7e17 > > repository/src/main/java/org/apache/atlas/repository/store/graph/v1/AtlasStructDefStoreV1.java > 1c6cfc7 > > repository/src/main/java/org/apache/atlas/repository/store/graph/v1/EntityGraphMapper.java > 80cd1ee > > repository/src/test/java/org/apache/atlas/repository/graph/GraphBackedMetadataRepositoryTest.java > 8120aaa > > repository/src/test/java/org/apache/atlas/repository/store/graph/AtlasTypeDefGraphStoreTest.java > 9f7214c > > repository/src/test/java/org/apache/atlas/repository/store/graph/v1/AtlasDeleteHandlerV1Test.java > 9331e35 > > repository/src/test/java/org/apache/atlas/repository/store/graph/v1/AtlasEntityStoreV1Test.java > 44067b9 > webapp/src/test/java/org/apache/atlas/web/integration/BaseResourceIT.java > b59d3ee > > webapp/src/test/java/org/apache/atlas/web/integration/TypedefsJerseyResourceIT.java > c46689c > > > Diff: https://reviews.apache.org/r/59881/diff/1/ > > > Testing > ------- > > 1. created Type using API with defaultValue attribute, and tested its entity > creation with default value being set in entity attribute. > 2. Tested on different primitive types attributes. > 3. Ran Unit tests using mvn clean install > Two unit test failing: > > HardDeleteHandlerV1Test>AtlasDeleteHandlerV1Test.testDisconnectUnidirectionalArrayReferenceFromStructAndTraitTypes:617 > » AtlasBase > > SoftDeleteHandlerV1Test>AtlasDeleteHandlerV1Test.testDisconnectUnidirectionalArrayReferenceFromStructAndTraitTypes:617 > » AtlasBase > > > Thanks, > > Ruchi Solani > >
