You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@atlas.apache.org by Ruchi Solani <ru...@freestoneinfotech.com> on 2017/06/07 14:25:40 UTC
Review Request 59881: ATLAS-1863 :- Set default value for primitive
types attributes in entity based on attributeDef in Typedef
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59881/
-----------------------------------------------------------
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
Re: Review Request 59881: ATLAS-1863 :- Set default value for
primitive types attributes in entity based on attributeDef in Typedef
Posted by Madhan Neethiraj <ma...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59881/#review177698
-----------------------------------------------------------
intg/src/main/java/org/apache/atlas/type/AtlasArrayType.java
Lines 209 (patched)
<https://reviews.apache.org/r/59881/#comment251422>
here 'obj' will either be a List or a Set. This 'if' block should be moved just prior to line #204.
intg/src/main/java/org/apache/atlas/type/AtlasEnumType.java
Lines 69 (patched)
<https://reviews.apache.org/r/59881/#comment251424>
This override seems unnecessary. Base class implementation in AtlasType.createDefaultValue(defaultValue) should be enough.
intg/src/main/java/org/apache/atlas/type/AtlasStructType.java
Lines 279 (patched)
<https://reviews.apache.org/r/59881/#comment251426>
this doesn't seem necessary.
intg/src/main/java/org/apache/atlas/type/AtlasStructType.java
Line 483 (original), 496 (patched)
<https://reviews.apache.org/r/59881/#comment251425>
dataType.createDefaultValue() ==> dataType.createDefaultValue(attributeDef.getDefaultValue());
- Madhan Neethiraj
On June 12, 2017, 2:14 p.m., Ruchi Solani wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59881/
> -----------------------------------------------------------
>
> (Updated June 12, 2017, 2:14 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/AtlasEntityType.java 6516d48
> 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
> 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
>
>
> Diff: https://reviews.apache.org/r/59881/diff/2/
>
>
> 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
>
>
Re: Review Request 59881: ATLAS-1863 :- Set default value for
primitive types attributes in entity based on attributeDef in Typedef
Posted by Madhan Neethiraj <ma...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59881/#review178109
-----------------------------------------------------------
Ship it!
Ship It!
- Madhan Neethiraj
On June 16, 2017, 5:34 a.m., Ruchi Solani wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59881/
> -----------------------------------------------------------
>
> (Updated June 16, 2017, 5:34 a.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/AtlasEntityType.java 6516d48
> 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/test/java/org/apache/atlas/TestUtilsV2.java 2a6ea92
> intg/src/test/java/org/apache/atlas/type/TestAtlasArrayType.java e882473
> 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/store/graph/v1/AtlasEntityStoreV1Test.java 44067b9
>
>
> Diff: https://reviews.apache.org/r/59881/diff/3/
>
>
> 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
>
>
Re: Review Request 59881: ATLAS-1863 :- Set default value for
primitive types attributes in entity based on attributeDef in Typedef
Posted by Ruchi Solani <ru...@freestoneinfotech.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59881/
-----------------------------------------------------------
(Updated June 16, 2017, 5:34 a.m.)
Review request for atlas, Apoorv Naik, Madhan Neethiraj, Nixon Rodrigues, and Sarath Subramanian.
Changes
-------
This patch includes changes to address review comments given by Madhan and Unit test to validate whether defaultValue is set to entity attribute when attributedef has default Value property.
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 (updated)
-----
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/AtlasEntityType.java 6516d48
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/test/java/org/apache/atlas/TestUtilsV2.java 2a6ea92
intg/src/test/java/org/apache/atlas/type/TestAtlasArrayType.java e882473
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/store/graph/v1/AtlasEntityStoreV1Test.java 44067b9
Diff: https://reviews.apache.org/r/59881/diff/3/
Changes: https://reviews.apache.org/r/59881/diff/2-3/
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
Re: Review Request 59881: ATLAS-1863 :- Set default value for
primitive types attributes in entity based on attributeDef in Typedef
Posted by Ruchi Solani <ru...@freestoneinfotech.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59881/
-----------------------------------------------------------
(Updated June 12, 2017, 2:14 p.m.)
Review request for atlas, Apoorv Naik, Madhan Neethiraj, Nixon Rodrigues, and Sarath Subramanian.
Changes
-------
This patch includes changes to address review comments given by Madhan Neethiraj
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 (updated)
-----
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/AtlasEntityType.java 6516d48
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
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
Diff: https://reviews.apache.org/r/59881/diff/2/
Changes: https://reviews.apache.org/r/59881/diff/1-2/
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
Re: Review Request 59881: ATLAS-1863 :- Set default value for
primitive types attributes in entity based on attributeDef in Typedef
Posted by Madhan Neethiraj <ma...@apache.org>.
-----------------------------------------------------------
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
>
>