You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@atlas.apache.org by David Kantor <dk...@us.ibm.com> on 2016/04/12 15:41:02 UTC

Re: Review Request 45948: Atlas-645: avoid infinite recursion in FieldMapping.output()

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45948/
-----------------------------------------------------------

(Updated April 12, 2016, 1:41 p.m.)


Review request for atlas.


Bugs: ATLAS-645
    https://issues.apache.org/jira/browse/ATLAS-645


Repository: atlas


Description
-------

ATLAS-645: In FieldMapping.output(), avoid infinite recursion when IReferenceableInstance and IStruct instances reference each other.


Diffs
-----

  typesystem/src/main/java/org/apache/atlas/typesystem/types/FieldMapping.java 36149bafff80b68ce176e82dcacac87035459362 
  typesystem/src/test/java/org/apache/atlas/typesystem/types/FieldMappingTest.java PRE-CREATION 

Diff: https://reviews.apache.org/r/45948/diff/


Testing
-------

Ran all unit and integration tests with no regressions.  Added test cases


Thanks,

David Kantor


Re: Review Request 45948: Atlas-645: avoid infinite recursion in FieldMapping.output()

Posted by Shwetha GS <ss...@hortonworks.com>.

> On April 13, 2016, 12:21 a.m., Hemanth Yamijala wrote:
> > typesystem/src/main/java/org/apache/atlas/typesystem/types/FieldMapping.java, line 51
> > <https://reviews.apache.org/r/45948/diff/1/?file=1337582#file1337582line51>
> >
> >     This may not be a valid case - maybe Shwetha would know better... but if a FieldMapping instance's output is called from different threads, wouldn't this mutable state cause issues? Can't think of an easy way to fix that very quickly though. So please take a call based on whether it is a valid use case or not.

Yes, this will not work as there is single instance of type in the system. Can you create the list and pass it in the output() method instead?


- Shwetha


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45948/#review128589
-----------------------------------------------------------


On April 12, 2016, 1:41 p.m., David Kantor wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45948/
> -----------------------------------------------------------
> 
> (Updated April 12, 2016, 1:41 p.m.)
> 
> 
> Review request for atlas.
> 
> 
> Bugs: ATLAS-645
>     https://issues.apache.org/jira/browse/ATLAS-645
> 
> 
> Repository: atlas
> 
> 
> Description
> -------
> 
> ATLAS-645: In FieldMapping.output(), avoid infinite recursion when IReferenceableInstance and IStruct instances reference each other.
> 
> 
> Diffs
> -----
> 
>   typesystem/src/main/java/org/apache/atlas/typesystem/types/FieldMapping.java 36149bafff80b68ce176e82dcacac87035459362 
>   typesystem/src/test/java/org/apache/atlas/typesystem/types/FieldMappingTest.java PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/45948/diff/
> 
> 
> Testing
> -------
> 
> Ran all unit and integration tests with no regressions.  Added test cases
> 
> 
> Thanks,
> 
> David Kantor
> 
>


Re: Review Request 45948: Atlas-645: avoid infinite recursion in FieldMapping.output()

Posted by David Kantor <dk...@us.ibm.com>.

> On April 13, 2016, 12:21 a.m., Hemanth Yamijala wrote:
> > typesystem/src/main/java/org/apache/atlas/typesystem/types/FieldMapping.java, line 51
> > <https://reviews.apache.org/r/45948/diff/1/?file=1337582#file1337582line51>
> >
> >     This may not be a valid case - maybe Shwetha would know better... but if a FieldMapping instance's output is called from different threads, wouldn't this mutable state cause issues? Can't think of an easy way to fix that very quickly though. So please take a call based on whether it is a valid use case or not.
> 
> Shwetha GS wrote:
>     Yes, this will not work as there is single instance of type in the system. Can you create the list and pass it in the output() method instead?
> 
> David Kantor wrote:
>     Rather than passing a list around, I moved the output status tracking to the structs themselves, which are not singletons like FieldMapping.
> 
> Shwetha GS wrote:
>     outputInProgress shouldn't be a class level field and it has no significance outside of output(). I think it makes more sense to maintain it as argument to output()

Agreed, I have changed the implementation to pass an "in process" Set as an argument to FieldMapping.output() and IDataType.output().  Thanks very much for reviewing the code.


- David


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45948/#review128589
-----------------------------------------------------------


On April 18, 2016, 5:13 p.m., David Kantor wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45948/
> -----------------------------------------------------------
> 
> (Updated April 18, 2016, 5:13 p.m.)
> 
> 
> Review request for atlas.
> 
> 
> Bugs: ATLAS-645
>     https://issues.apache.org/jira/browse/ATLAS-645
> 
> 
> Repository: atlas
> 
> 
> Description
> -------
> 
> ATLAS-645: In FieldMapping.output(), avoid infinite recursion when IReferenceableInstance and IStruct instances reference each other.
> 
> 
> Diffs
> -----
> 
>   typesystem/src/main/java/org/apache/atlas/typesystem/persistence/ReferenceableInstance.java 31ef49d5e6accd8b4621385cce1e3a6b7a6936ff 
>   typesystem/src/main/java/org/apache/atlas/typesystem/persistence/StructInstance.java af62442bfe5daa221079207acf361e1316cab3ad 
>   typesystem/src/main/java/org/apache/atlas/typesystem/types/AbstractDataType.java 92be3c794077bd3e437aa6f3426ed8b7892b945e 
>   typesystem/src/main/java/org/apache/atlas/typesystem/types/ClassType.java 90cf3ccfe8e9d0e20901452c0b83796ba3994998 
>   typesystem/src/main/java/org/apache/atlas/typesystem/types/DataTypes.java 55ec91f5c3cba65f1bf7c524c84c075cc1c96dff 
>   typesystem/src/main/java/org/apache/atlas/typesystem/types/FieldMapping.java 36149bafff80b68ce176e82dcacac87035459362 
>   typesystem/src/main/java/org/apache/atlas/typesystem/types/IDataType.java 373ad2c93efdecc292b68fab1ea2403afce84dac 
>   typesystem/src/main/java/org/apache/atlas/typesystem/types/StructType.java 54e344f5d6322a00ac7825ee8964f43a1552dcbe 
>   typesystem/src/main/java/org/apache/atlas/typesystem/types/TraitType.java 84c22bf96a059a78b065a73a8feefae87deb9975 
>   typesystem/src/main/java/org/apache/atlas/typesystem/types/TypedStructHandler.java da246d66d94bcd3e221b70b859de2850fa6cf7a8 
>   typesystem/src/test/java/org/apache/atlas/typesystem/types/FieldMappingTest.java PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/45948/diff/
> 
> 
> Testing
> -------
> 
> Ran all unit and integration tests with no regressions.  Added test cases
> 
> 
> Thanks,
> 
> David Kantor
> 
>


Re: Review Request 45948: Atlas-645: avoid infinite recursion in FieldMapping.output()

Posted by Shwetha GS <ss...@hortonworks.com>.

> On April 13, 2016, 12:21 a.m., Hemanth Yamijala wrote:
> > typesystem/src/main/java/org/apache/atlas/typesystem/types/FieldMapping.java, line 51
> > <https://reviews.apache.org/r/45948/diff/1/?file=1337582#file1337582line51>
> >
> >     This may not be a valid case - maybe Shwetha would know better... but if a FieldMapping instance's output is called from different threads, wouldn't this mutable state cause issues? Can't think of an easy way to fix that very quickly though. So please take a call based on whether it is a valid use case or not.
> 
> Shwetha GS wrote:
>     Yes, this will not work as there is single instance of type in the system. Can you create the list and pass it in the output() method instead?
> 
> David Kantor wrote:
>     Rather than passing a list around, I moved the output status tracking to the structs themselves, which are not singletons like FieldMapping.

outputInProgress shouldn't be a class level field and it has no significance outside of output(). I think it makes more sense to maintain it as argument to output()


- Shwetha


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45948/#review128589
-----------------------------------------------------------


On April 15, 2016, 6:36 p.m., David Kantor wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45948/
> -----------------------------------------------------------
> 
> (Updated April 15, 2016, 6:36 p.m.)
> 
> 
> Review request for atlas.
> 
> 
> Bugs: ATLAS-645
>     https://issues.apache.org/jira/browse/ATLAS-645
> 
> 
> Repository: atlas
> 
> 
> Description
> -------
> 
> ATLAS-645: In FieldMapping.output(), avoid infinite recursion when IReferenceableInstance and IStruct instances reference each other.
> 
> 
> Diffs
> -----
> 
>   typesystem/src/main/java/org/apache/atlas/typesystem/AbstractStruct.java PRE-CREATION 
>   typesystem/src/main/java/org/apache/atlas/typesystem/IStruct.java e0f85761a0f436cd4b4cd355d6c8ab65dca7fcc9 
>   typesystem/src/main/java/org/apache/atlas/typesystem/Struct.java 70deab29df36e7e18f517c4880d7088f8bec06a5 
>   typesystem/src/main/java/org/apache/atlas/typesystem/persistence/DownCastStructInstance.java d3b9a3376ebce92ff8b50708fa841ab5868b366a 
>   typesystem/src/main/java/org/apache/atlas/typesystem/persistence/Id.java d742bb73e89772afcf8e0bd25f4fda9b7c94758b 
>   typesystem/src/main/java/org/apache/atlas/typesystem/persistence/StructInstance.java 16c3a24e51f0389946e0e6094a6a3e4e1b9dff0f 
>   typesystem/src/main/java/org/apache/atlas/typesystem/types/FieldMapping.java 36149bafff80b68ce176e82dcacac87035459362 
>   typesystem/src/test/java/org/apache/atlas/typesystem/types/FieldMappingTest.java PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/45948/diff/
> 
> 
> Testing
> -------
> 
> Ran all unit and integration tests with no regressions.  Added test cases
> 
> 
> Thanks,
> 
> David Kantor
> 
>


Re: Review Request 45948: Atlas-645: avoid infinite recursion in FieldMapping.output()

Posted by David Kantor <dk...@us.ibm.com>.

> On April 13, 2016, 12:21 a.m., Hemanth Yamijala wrote:
> > typesystem/src/main/java/org/apache/atlas/typesystem/types/FieldMapping.java, line 51
> > <https://reviews.apache.org/r/45948/diff/1/?file=1337582#file1337582line51>
> >
> >     This may not be a valid case - maybe Shwetha would know better... but if a FieldMapping instance's output is called from different threads, wouldn't this mutable state cause issues? Can't think of an easy way to fix that very quickly though. So please take a call based on whether it is a valid use case or not.
> 
> Shwetha GS wrote:
>     Yes, this will not work as there is single instance of type in the system. Can you create the list and pass it in the output() method instead?

Rather than passing a list around, I moved the output status tracking to the structs themselves, which are not singletons like FieldMapping.


- David


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45948/#review128589
-----------------------------------------------------------


On April 15, 2016, 6:36 p.m., David Kantor wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45948/
> -----------------------------------------------------------
> 
> (Updated April 15, 2016, 6:36 p.m.)
> 
> 
> Review request for atlas.
> 
> 
> Bugs: ATLAS-645
>     https://issues.apache.org/jira/browse/ATLAS-645
> 
> 
> Repository: atlas
> 
> 
> Description
> -------
> 
> ATLAS-645: In FieldMapping.output(), avoid infinite recursion when IReferenceableInstance and IStruct instances reference each other.
> 
> 
> Diffs
> -----
> 
>   typesystem/src/main/java/org/apache/atlas/typesystem/AbstractStruct.java PRE-CREATION 
>   typesystem/src/main/java/org/apache/atlas/typesystem/IStruct.java e0f85761a0f436cd4b4cd355d6c8ab65dca7fcc9 
>   typesystem/src/main/java/org/apache/atlas/typesystem/Struct.java 70deab29df36e7e18f517c4880d7088f8bec06a5 
>   typesystem/src/main/java/org/apache/atlas/typesystem/persistence/DownCastStructInstance.java d3b9a3376ebce92ff8b50708fa841ab5868b366a 
>   typesystem/src/main/java/org/apache/atlas/typesystem/persistence/Id.java d742bb73e89772afcf8e0bd25f4fda9b7c94758b 
>   typesystem/src/main/java/org/apache/atlas/typesystem/persistence/StructInstance.java 16c3a24e51f0389946e0e6094a6a3e4e1b9dff0f 
>   typesystem/src/main/java/org/apache/atlas/typesystem/types/FieldMapping.java 36149bafff80b68ce176e82dcacac87035459362 
>   typesystem/src/test/java/org/apache/atlas/typesystem/types/FieldMappingTest.java PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/45948/diff/
> 
> 
> Testing
> -------
> 
> Ran all unit and integration tests with no regressions.  Added test cases
> 
> 
> Thanks,
> 
> David Kantor
> 
>


Re: Review Request 45948: Atlas-645: avoid infinite recursion in FieldMapping.output()

Posted by Hemanth Yamijala <yh...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45948/#review128589
-----------------------------------------------------------




typesystem/src/main/java/org/apache/atlas/typesystem/types/FieldMapping.java (line 51)
<https://reviews.apache.org/r/45948/#comment192038>

    This may not be a valid case - maybe Shwetha would know better... but if a FieldMapping instance's output is called from different threads, wouldn't this mutable state cause issues? Can't think of an easy way to fix that very quickly though. So please take a call based on whether it is a valid use case or not.


- Hemanth Yamijala


On April 12, 2016, 1:41 p.m., David Kantor wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45948/
> -----------------------------------------------------------
> 
> (Updated April 12, 2016, 1:41 p.m.)
> 
> 
> Review request for atlas.
> 
> 
> Bugs: ATLAS-645
>     https://issues.apache.org/jira/browse/ATLAS-645
> 
> 
> Repository: atlas
> 
> 
> Description
> -------
> 
> ATLAS-645: In FieldMapping.output(), avoid infinite recursion when IReferenceableInstance and IStruct instances reference each other.
> 
> 
> Diffs
> -----
> 
>   typesystem/src/main/java/org/apache/atlas/typesystem/types/FieldMapping.java 36149bafff80b68ce176e82dcacac87035459362 
>   typesystem/src/test/java/org/apache/atlas/typesystem/types/FieldMappingTest.java PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/45948/diff/
> 
> 
> Testing
> -------
> 
> Ran all unit and integration tests with no regressions.  Added test cases
> 
> 
> Thanks,
> 
> David Kantor
> 
>


Re: Review Request 45948: Atlas-645: avoid infinite recursion in FieldMapping.output()

Posted by David Kantor <dk...@us.ibm.com>.

> On April 22, 2016, 6:41 a.m., Shwetha GS wrote:
> > typesystem/src/main/java/org/apache/atlas/typesystem/persistence/StructInstance.java, line 728
> > <https://reviews.apache.org/r/45948/diff/3/?file=1348562#file1348562line728>
> >
> >     Why is this removed?

I removed it because it was not being called anywhere and duplicated code/functionality available elsewhere.  If there is usage of this method that I missed, please advise.


> On April 22, 2016, 6:41 a.m., Shwetha GS wrote:
> > typesystem/src/main/java/org/apache/atlas/typesystem/types/AbstractDataType.java, line 48
> > <https://reviews.apache.org/r/45948/diff/3/?file=1348563#file1348563line48>
> >
> >     rename to outputInProgress everywhere?

I've removed the inProcess method argument.


- David


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45948/#review130045
-----------------------------------------------------------


On April 29, 2016, 7:24 p.m., David Kantor wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45948/
> -----------------------------------------------------------
> 
> (Updated April 29, 2016, 7:24 p.m.)
> 
> 
> Review request for atlas.
> 
> 
> Bugs: ATLAS-645
>     https://issues.apache.org/jira/browse/ATLAS-645
> 
> 
> Repository: atlas
> 
> 
> Description
> -------
> 
> ATLAS-645: In FieldMapping.output(), avoid infinite recursion when IReferenceableInstance and IStruct instances reference each other.
> 
> 
> Diffs
> -----
> 
>   typesystem/src/main/java/org/apache/atlas/typesystem/persistence/StructInstance.java af62442bfe5daa221079207acf361e1316cab3ad 
>   typesystem/src/main/java/org/apache/atlas/typesystem/types/FieldMapping.java 36149bafff80b68ce176e82dcacac87035459362 
>   typesystem/src/main/java/org/apache/atlas/typesystem/types/HierarchicalType.java 89fcea6828c9e23c28a60952c3e4ca27c0667494 
>   typesystem/src/main/java/org/apache/atlas/typesystem/types/OutputStatus.java PRE-CREATION 
>   typesystem/src/main/java/org/apache/atlas/typesystem/types/StructType.java 54e344f5d6322a00ac7825ee8964f43a1552dcbe 
>   typesystem/src/test/java/org/apache/atlas/typesystem/types/OutputStatusTest.java PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/45948/diff/
> 
> 
> Testing
> -------
> 
> Ran all unit and integration tests with no regressions.  Added test cases
> 
> 
> Thanks,
> 
> David Kantor
> 
>


Re: Review Request 45948: Atlas-645: avoid infinite recursion in FieldMapping.output()

Posted by Shwetha GS <ss...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45948/#review130045
-----------------------------------------------------------




typesystem/src/main/java/org/apache/atlas/typesystem/persistence/StructInstance.java 
<https://reviews.apache.org/r/45948/#comment193664>

    Why is this removed?



typesystem/src/main/java/org/apache/atlas/typesystem/types/AbstractDataType.java (line 48)
<https://reviews.apache.org/r/45948/#comment193663>

    rename to outputInProgress everywhere?


- Shwetha GS


On April 18, 2016, 5:13 p.m., David Kantor wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45948/
> -----------------------------------------------------------
> 
> (Updated April 18, 2016, 5:13 p.m.)
> 
> 
> Review request for atlas.
> 
> 
> Bugs: ATLAS-645
>     https://issues.apache.org/jira/browse/ATLAS-645
> 
> 
> Repository: atlas
> 
> 
> Description
> -------
> 
> ATLAS-645: In FieldMapping.output(), avoid infinite recursion when IReferenceableInstance and IStruct instances reference each other.
> 
> 
> Diffs
> -----
> 
>   typesystem/src/main/java/org/apache/atlas/typesystem/persistence/ReferenceableInstance.java 31ef49d5e6accd8b4621385cce1e3a6b7a6936ff 
>   typesystem/src/main/java/org/apache/atlas/typesystem/persistence/StructInstance.java af62442bfe5daa221079207acf361e1316cab3ad 
>   typesystem/src/main/java/org/apache/atlas/typesystem/types/AbstractDataType.java 92be3c794077bd3e437aa6f3426ed8b7892b945e 
>   typesystem/src/main/java/org/apache/atlas/typesystem/types/ClassType.java 90cf3ccfe8e9d0e20901452c0b83796ba3994998 
>   typesystem/src/main/java/org/apache/atlas/typesystem/types/DataTypes.java 55ec91f5c3cba65f1bf7c524c84c075cc1c96dff 
>   typesystem/src/main/java/org/apache/atlas/typesystem/types/FieldMapping.java 36149bafff80b68ce176e82dcacac87035459362 
>   typesystem/src/main/java/org/apache/atlas/typesystem/types/IDataType.java 373ad2c93efdecc292b68fab1ea2403afce84dac 
>   typesystem/src/main/java/org/apache/atlas/typesystem/types/StructType.java 54e344f5d6322a00ac7825ee8964f43a1552dcbe 
>   typesystem/src/main/java/org/apache/atlas/typesystem/types/TraitType.java 84c22bf96a059a78b065a73a8feefae87deb9975 
>   typesystem/src/main/java/org/apache/atlas/typesystem/types/TypedStructHandler.java da246d66d94bcd3e221b70b859de2850fa6cf7a8 
>   typesystem/src/test/java/org/apache/atlas/typesystem/types/FieldMappingTest.java PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/45948/diff/
> 
> 
> Testing
> -------
> 
> Ran all unit and integration tests with no regressions.  Added test cases
> 
> 
> Thanks,
> 
> David Kantor
> 
>


Re: Review Request 45948: Atlas-645: avoid infinite recursion in FieldMapping.output()

Posted by Shwetha GS <ss...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45948/#review133063
-----------------------------------------------------------


Ship it!




Ship It!

- Shwetha GS


On May 13, 2016, 2:09 a.m., David Kantor wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45948/
> -----------------------------------------------------------
> 
> (Updated May 13, 2016, 2:09 a.m.)
> 
> 
> Review request for atlas.
> 
> 
> Bugs: ATLAS-645
>     https://issues.apache.org/jira/browse/ATLAS-645
> 
> 
> Repository: atlas
> 
> 
> Description
> -------
> 
> ATLAS-645: In FieldMapping.output(), avoid infinite recursion when IReferenceableInstance and IStruct instances reference each other.
> 
> 
> Diffs
> -----
> 
>   typesystem/src/main/java/org/apache/atlas/typesystem/persistence/ReferenceableInstance.java 31ef49d5e6accd8b4621385cce1e3a6b7a6936ff 
>   typesystem/src/main/java/org/apache/atlas/typesystem/persistence/StructInstance.java af62442bfe5daa221079207acf361e1316cab3ad 
>   typesystem/src/main/java/org/apache/atlas/typesystem/types/AbstractDataType.java 92be3c794077bd3e437aa6f3426ed8b7892b945e 
>   typesystem/src/main/java/org/apache/atlas/typesystem/types/AttributeInfo.java e748579df8056994899624fac8e8a86958a27a6f 
>   typesystem/src/main/java/org/apache/atlas/typesystem/types/ClassType.java 90cf3ccfe8e9d0e20901452c0b83796ba3994998 
>   typesystem/src/main/java/org/apache/atlas/typesystem/types/DataTypes.java 55ec91f5c3cba65f1bf7c524c84c075cc1c96dff 
>   typesystem/src/main/java/org/apache/atlas/typesystem/types/FieldMapping.java 36149bafff80b68ce176e82dcacac87035459362 
>   typesystem/src/main/java/org/apache/atlas/typesystem/types/HierarchicalType.java 89fcea6828c9e23c28a60952c3e4ca27c0667494 
>   typesystem/src/main/java/org/apache/atlas/typesystem/types/IDataType.java 373ad2c93efdecc292b68fab1ea2403afce84dac 
>   typesystem/src/main/java/org/apache/atlas/typesystem/types/Multiplicity.java a54dabc2f8366dbecc8372d81a48c741b2d65258 
>   typesystem/src/main/java/org/apache/atlas/typesystem/types/StructType.java 54e344f5d6322a00ac7825ee8964f43a1552dcbe 
>   typesystem/src/main/java/org/apache/atlas/typesystem/types/TraitType.java 84c22bf96a059a78b065a73a8feefae87deb9975 
>   typesystem/src/main/java/org/apache/atlas/typesystem/types/TypedStructHandler.java da246d66d94bcd3e221b70b859de2850fa6cf7a8 
>   typesystem/src/test/java/org/apache/atlas/typesystem/types/FieldMappingTest.java PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/45948/diff/
> 
> 
> Testing
> -------
> 
> Ran all unit and integration tests with no regressions.  Added test cases
> 
> 
> Thanks,
> 
> David Kantor
> 
>


Re: Review Request 45948: Atlas-645: avoid infinite recursion in FieldMapping.output()

Posted by David Kantor <dk...@us.ibm.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45948/
-----------------------------------------------------------

(Updated May 13, 2016, 2:09 a.m.)


Review request for atlas.


Changes
-------

Addressed Shwetha's comments - inProcess set passed as argument rather than using thread local storage.


Bugs: ATLAS-645
    https://issues.apache.org/jira/browse/ATLAS-645


Repository: atlas


Description
-------

ATLAS-645: In FieldMapping.output(), avoid infinite recursion when IReferenceableInstance and IStruct instances reference each other.


Diffs (updated)
-----

  typesystem/src/main/java/org/apache/atlas/typesystem/persistence/ReferenceableInstance.java 31ef49d5e6accd8b4621385cce1e3a6b7a6936ff 
  typesystem/src/main/java/org/apache/atlas/typesystem/persistence/StructInstance.java af62442bfe5daa221079207acf361e1316cab3ad 
  typesystem/src/main/java/org/apache/atlas/typesystem/types/AbstractDataType.java 92be3c794077bd3e437aa6f3426ed8b7892b945e 
  typesystem/src/main/java/org/apache/atlas/typesystem/types/AttributeInfo.java e748579df8056994899624fac8e8a86958a27a6f 
  typesystem/src/main/java/org/apache/atlas/typesystem/types/ClassType.java 90cf3ccfe8e9d0e20901452c0b83796ba3994998 
  typesystem/src/main/java/org/apache/atlas/typesystem/types/DataTypes.java 55ec91f5c3cba65f1bf7c524c84c075cc1c96dff 
  typesystem/src/main/java/org/apache/atlas/typesystem/types/FieldMapping.java 36149bafff80b68ce176e82dcacac87035459362 
  typesystem/src/main/java/org/apache/atlas/typesystem/types/HierarchicalType.java 89fcea6828c9e23c28a60952c3e4ca27c0667494 
  typesystem/src/main/java/org/apache/atlas/typesystem/types/IDataType.java 373ad2c93efdecc292b68fab1ea2403afce84dac 
  typesystem/src/main/java/org/apache/atlas/typesystem/types/Multiplicity.java a54dabc2f8366dbecc8372d81a48c741b2d65258 
  typesystem/src/main/java/org/apache/atlas/typesystem/types/StructType.java 54e344f5d6322a00ac7825ee8964f43a1552dcbe 
  typesystem/src/main/java/org/apache/atlas/typesystem/types/TraitType.java 84c22bf96a059a78b065a73a8feefae87deb9975 
  typesystem/src/main/java/org/apache/atlas/typesystem/types/TypedStructHandler.java da246d66d94bcd3e221b70b859de2850fa6cf7a8 
  typesystem/src/test/java/org/apache/atlas/typesystem/types/FieldMappingTest.java PRE-CREATION 

Diff: https://reviews.apache.org/r/45948/diff/


Testing
-------

Ran all unit and integration tests with no regressions.  Added test cases


Thanks,

David Kantor


Re: Review Request 45948: Atlas-645: avoid infinite recursion in FieldMapping.output()

Posted by David Kantor <dk...@us.ibm.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45948/
-----------------------------------------------------------

(Updated April 29, 2016, 7:24 p.m.)


Review request for atlas.


Changes
-------

Addressed Shwetha's comments.  Reimplemented to use thread local storage for the inProcess set rather than passing as an method argument.


Bugs: ATLAS-645
    https://issues.apache.org/jira/browse/ATLAS-645


Repository: atlas


Description
-------

ATLAS-645: In FieldMapping.output(), avoid infinite recursion when IReferenceableInstance and IStruct instances reference each other.


Diffs (updated)
-----

  typesystem/src/main/java/org/apache/atlas/typesystem/persistence/StructInstance.java af62442bfe5daa221079207acf361e1316cab3ad 
  typesystem/src/main/java/org/apache/atlas/typesystem/types/FieldMapping.java 36149bafff80b68ce176e82dcacac87035459362 
  typesystem/src/main/java/org/apache/atlas/typesystem/types/HierarchicalType.java 89fcea6828c9e23c28a60952c3e4ca27c0667494 
  typesystem/src/main/java/org/apache/atlas/typesystem/types/OutputStatus.java PRE-CREATION 
  typesystem/src/main/java/org/apache/atlas/typesystem/types/StructType.java 54e344f5d6322a00ac7825ee8964f43a1552dcbe 
  typesystem/src/test/java/org/apache/atlas/typesystem/types/OutputStatusTest.java PRE-CREATION 

Diff: https://reviews.apache.org/r/45948/diff/


Testing
-------

Ran all unit and integration tests with no regressions.  Added test cases


Thanks,

David Kantor


Re: Review Request 45948: Atlas-645: avoid infinite recursion in FieldMapping.output()

Posted by Shwetha GS <ss...@hortonworks.com>.

> On April 19, 2016, 11 a.m., Shwetha GS wrote:
> > typesystem/src/main/java/org/apache/atlas/typesystem/types/FieldMapping.java, line 99
> > <https://reviews.apache.org/r/45948/diff/3/?file=1348566#file1348566line99>
> >
> >     Can we maintain this as Set<String> and check on just typeName?
> 
> David Kantor wrote:
>     I don't understand how checking typeName would avoid the infinite recursion.  The problem use case is when structs reference each other.  So let's say there is a bi-directional reference between A and B.  When A is output, its attribute values are output.  So then B is output, and when its attributes are output, FieldMapping.output() is called recursively on A.  We need to detect that A is already in the process of being output to avoid the infinite recursion.  How does checking typeName help with this?  Please advise.
> 
> Shwetha GS wrote:
>     In FieldMappingTest.testOutputReferenceableInstance(), even ownerType.toString() throws stack overflow. So, thought you were addressing type.tostring()
>     
>     For type.toString(), we can use typename. For instance.tostring(), can we use id string, instead of full instance in the set
> 
> David Kantor wrote:
>     Thanks for pointing out the stack overflow on ownerType.toString().  This involves type system objects and does not go through the FieldMapping.output() and IDataType.output() code path for outputting instance data - it takes a completely different code path. So changing the "in process" arg on these methods to Set<String> would not address this bug.  Here is the relevant call stack:
>       
>       ClassType(HierarchicalType<ST,T>).toString() line: 371	
>       String.valueOf(Object) line: 1657	
>       StringBuilder.append(Object) line: 194	
>       AttributeInfo.toString() line: 65	
>       SingletonImmutableList<E>.toString() line: 97	
>       String.valueOf(Object) line: 1657	
>       StringBuilder.append(Object) line: 194	
>       ClassType(HierarchicalType<ST,T>).toString() line: 372	
>       String.valueOf(Object) line: 1657	
>       StringBuilder.append(Object) line: 194	
>       AttributeInfo.toString() line: 65	
>       SingletonImmutableList<E>.toString() line: 97	
>       String.valueOf(Object) line: 1657	
>       StringBuilder.append(Object) line: 194	
>       ClassType(HierarchicalType<ST,T>).toString() line: 372	
>     
>     Also, for instance data, using the id string is not sufficient because structs that are not implementations of IReferenceableInstance won't have an id string.  And for those structs that are IReferenceableInstance objects, if the structs are newly created in memory and have not been persisted, they won't have a meaningful id string.  Such objects would all have the same id string, and result in false positives on the inProcess check.
>     
>     I have re-implemented the fix to maintain the inProcess set in thread local storage rather than passing it as an argument.  This fix addresses the recursion issue for both types and instance data.

Sorry, it took long to get to this.

About thread local variables:
	I think passing inProgress as argument is a better approach \u2013 it makes the interfaces clear (as arguments are explicit) and avoids global state. Thread local variables are bad idea as its global state and there is no control on who can modify it. With the application server like atlas where server threads are re-used after request completes, there should be cleanup of thread local variables for every request which is an extra overhead.
      Earlier, I added RequestContext as a thread local variable because I couldn\u2019t find a cleaner way to set this for every request and pass the user info from filters to the APIs. We reinitisalise RequestContext for every request in AuditFilter. Also, if its called from other threads outside of APIs, it needs to be handled separately. Its a pain, and I need to clean that up as well.
       I guess the reason why you added thread local variable is for toString() methods as it doesn't accept any arguments. Can toString() methods use output() methods in turn(like in instance classes) and pass the inProgress to output()?

About inProgress set:
       For types, typesInProgress can be set of type names, so set<string> should work.


- Shwetha


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45948/#review129505
-----------------------------------------------------------


On April 29, 2016, 7:24 p.m., David Kantor wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45948/
> -----------------------------------------------------------
> 
> (Updated April 29, 2016, 7:24 p.m.)
> 
> 
> Review request for atlas.
> 
> 
> Bugs: ATLAS-645
>     https://issues.apache.org/jira/browse/ATLAS-645
> 
> 
> Repository: atlas
> 
> 
> Description
> -------
> 
> ATLAS-645: In FieldMapping.output(), avoid infinite recursion when IReferenceableInstance and IStruct instances reference each other.
> 
> 
> Diffs
> -----
> 
>   typesystem/src/main/java/org/apache/atlas/typesystem/persistence/StructInstance.java af62442bfe5daa221079207acf361e1316cab3ad 
>   typesystem/src/main/java/org/apache/atlas/typesystem/types/FieldMapping.java 36149bafff80b68ce176e82dcacac87035459362 
>   typesystem/src/main/java/org/apache/atlas/typesystem/types/HierarchicalType.java 89fcea6828c9e23c28a60952c3e4ca27c0667494 
>   typesystem/src/main/java/org/apache/atlas/typesystem/types/OutputStatus.java PRE-CREATION 
>   typesystem/src/main/java/org/apache/atlas/typesystem/types/StructType.java 54e344f5d6322a00ac7825ee8964f43a1552dcbe 
>   typesystem/src/test/java/org/apache/atlas/typesystem/types/OutputStatusTest.java PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/45948/diff/
> 
> 
> Testing
> -------
> 
> Ran all unit and integration tests with no regressions.  Added test cases
> 
> 
> Thanks,
> 
> David Kantor
> 
>


Re: Review Request 45948: Atlas-645: avoid infinite recursion in FieldMapping.output()

Posted by David Kantor <dk...@us.ibm.com>.

> On April 19, 2016, 11 a.m., Shwetha GS wrote:
> > typesystem/src/main/java/org/apache/atlas/typesystem/types/FieldMapping.java, line 99
> > <https://reviews.apache.org/r/45948/diff/3/?file=1348566#file1348566line99>
> >
> >     Can we maintain this as Set<String> and check on just typeName?

I don't understand how checking typeName would avoid the infinite recursion.  The problem use case is when structs reference each other.  So let's say there is a bi-directional reference between A and B.  When A is output, its attribute values are output.  So then B is output, and when its attributes are output, FieldMapping.output() is called recursively on A.  We need to detect that A is already in the process of being output to avoid the infinite recursion.  How does checking typeName help with this?  Please advise.


- David


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45948/#review129505
-----------------------------------------------------------


On April 18, 2016, 5:13 p.m., David Kantor wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45948/
> -----------------------------------------------------------
> 
> (Updated April 18, 2016, 5:13 p.m.)
> 
> 
> Review request for atlas.
> 
> 
> Bugs: ATLAS-645
>     https://issues.apache.org/jira/browse/ATLAS-645
> 
> 
> Repository: atlas
> 
> 
> Description
> -------
> 
> ATLAS-645: In FieldMapping.output(), avoid infinite recursion when IReferenceableInstance and IStruct instances reference each other.
> 
> 
> Diffs
> -----
> 
>   typesystem/src/main/java/org/apache/atlas/typesystem/persistence/ReferenceableInstance.java 31ef49d5e6accd8b4621385cce1e3a6b7a6936ff 
>   typesystem/src/main/java/org/apache/atlas/typesystem/persistence/StructInstance.java af62442bfe5daa221079207acf361e1316cab3ad 
>   typesystem/src/main/java/org/apache/atlas/typesystem/types/AbstractDataType.java 92be3c794077bd3e437aa6f3426ed8b7892b945e 
>   typesystem/src/main/java/org/apache/atlas/typesystem/types/ClassType.java 90cf3ccfe8e9d0e20901452c0b83796ba3994998 
>   typesystem/src/main/java/org/apache/atlas/typesystem/types/DataTypes.java 55ec91f5c3cba65f1bf7c524c84c075cc1c96dff 
>   typesystem/src/main/java/org/apache/atlas/typesystem/types/FieldMapping.java 36149bafff80b68ce176e82dcacac87035459362 
>   typesystem/src/main/java/org/apache/atlas/typesystem/types/IDataType.java 373ad2c93efdecc292b68fab1ea2403afce84dac 
>   typesystem/src/main/java/org/apache/atlas/typesystem/types/StructType.java 54e344f5d6322a00ac7825ee8964f43a1552dcbe 
>   typesystem/src/main/java/org/apache/atlas/typesystem/types/TraitType.java 84c22bf96a059a78b065a73a8feefae87deb9975 
>   typesystem/src/main/java/org/apache/atlas/typesystem/types/TypedStructHandler.java da246d66d94bcd3e221b70b859de2850fa6cf7a8 
>   typesystem/src/test/java/org/apache/atlas/typesystem/types/FieldMappingTest.java PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/45948/diff/
> 
> 
> Testing
> -------
> 
> Ran all unit and integration tests with no regressions.  Added test cases
> 
> 
> Thanks,
> 
> David Kantor
> 
>


Re: Review Request 45948: Atlas-645: avoid infinite recursion in FieldMapping.output()

Posted by Shwetha GS <ss...@hortonworks.com>.

> On April 19, 2016, 11 a.m., Shwetha GS wrote:
> > typesystem/src/main/java/org/apache/atlas/typesystem/types/FieldMapping.java, line 99
> > <https://reviews.apache.org/r/45948/diff/3/?file=1348566#file1348566line99>
> >
> >     Can we maintain this as Set<String> and check on just typeName?
> 
> David Kantor wrote:
>     I don't understand how checking typeName would avoid the infinite recursion.  The problem use case is when structs reference each other.  So let's say there is a bi-directional reference between A and B.  When A is output, its attribute values are output.  So then B is output, and when its attributes are output, FieldMapping.output() is called recursively on A.  We need to detect that A is already in the process of being output to avoid the infinite recursion.  How does checking typeName help with this?  Please advise.

In FieldMappingTest.testOutputReferenceableInstance(), even ownerType.toString() throws stack overflow. So, thought you were addressing type.tostring()

For type.toString(), we can use typename. For instance.tostring(), can we use id string, instead of full instance in the set


- Shwetha


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45948/#review129505
-----------------------------------------------------------


On April 18, 2016, 5:13 p.m., David Kantor wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45948/
> -----------------------------------------------------------
> 
> (Updated April 18, 2016, 5:13 p.m.)
> 
> 
> Review request for atlas.
> 
> 
> Bugs: ATLAS-645
>     https://issues.apache.org/jira/browse/ATLAS-645
> 
> 
> Repository: atlas
> 
> 
> Description
> -------
> 
> ATLAS-645: In FieldMapping.output(), avoid infinite recursion when IReferenceableInstance and IStruct instances reference each other.
> 
> 
> Diffs
> -----
> 
>   typesystem/src/main/java/org/apache/atlas/typesystem/persistence/ReferenceableInstance.java 31ef49d5e6accd8b4621385cce1e3a6b7a6936ff 
>   typesystem/src/main/java/org/apache/atlas/typesystem/persistence/StructInstance.java af62442bfe5daa221079207acf361e1316cab3ad 
>   typesystem/src/main/java/org/apache/atlas/typesystem/types/AbstractDataType.java 92be3c794077bd3e437aa6f3426ed8b7892b945e 
>   typesystem/src/main/java/org/apache/atlas/typesystem/types/ClassType.java 90cf3ccfe8e9d0e20901452c0b83796ba3994998 
>   typesystem/src/main/java/org/apache/atlas/typesystem/types/DataTypes.java 55ec91f5c3cba65f1bf7c524c84c075cc1c96dff 
>   typesystem/src/main/java/org/apache/atlas/typesystem/types/FieldMapping.java 36149bafff80b68ce176e82dcacac87035459362 
>   typesystem/src/main/java/org/apache/atlas/typesystem/types/IDataType.java 373ad2c93efdecc292b68fab1ea2403afce84dac 
>   typesystem/src/main/java/org/apache/atlas/typesystem/types/StructType.java 54e344f5d6322a00ac7825ee8964f43a1552dcbe 
>   typesystem/src/main/java/org/apache/atlas/typesystem/types/TraitType.java 84c22bf96a059a78b065a73a8feefae87deb9975 
>   typesystem/src/main/java/org/apache/atlas/typesystem/types/TypedStructHandler.java da246d66d94bcd3e221b70b859de2850fa6cf7a8 
>   typesystem/src/test/java/org/apache/atlas/typesystem/types/FieldMappingTest.java PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/45948/diff/
> 
> 
> Testing
> -------
> 
> Ran all unit and integration tests with no regressions.  Added test cases
> 
> 
> Thanks,
> 
> David Kantor
> 
>


Re: Review Request 45948: Atlas-645: avoid infinite recursion in FieldMapping.output()

Posted by David Kantor <dk...@us.ibm.com>.

> On April 19, 2016, 11 a.m., Shwetha GS wrote:
> > typesystem/src/main/java/org/apache/atlas/typesystem/types/FieldMapping.java, line 99
> > <https://reviews.apache.org/r/45948/diff/3/?file=1348566#file1348566line99>
> >
> >     Can we maintain this as Set<String> and check on just typeName?
> 
> David Kantor wrote:
>     I don't understand how checking typeName would avoid the infinite recursion.  The problem use case is when structs reference each other.  So let's say there is a bi-directional reference between A and B.  When A is output, its attribute values are output.  So then B is output, and when its attributes are output, FieldMapping.output() is called recursively on A.  We need to detect that A is already in the process of being output to avoid the infinite recursion.  How does checking typeName help with this?  Please advise.
> 
> Shwetha GS wrote:
>     In FieldMappingTest.testOutputReferenceableInstance(), even ownerType.toString() throws stack overflow. So, thought you were addressing type.tostring()
>     
>     For type.toString(), we can use typename. For instance.tostring(), can we use id string, instead of full instance in the set

Thanks for pointing out the stack overflow on ownerType.toString().  This involves type system objects and does not go through the FieldMapping.output() and IDataType.output() code path for outputting instance data - it takes a completely different code path. So changing the "in process" arg on these methods to Set<String> would not address this bug.  Here is the relevant call stack:
  
  ClassType(HierarchicalType<ST,T>).toString() line: 371	
  String.valueOf(Object) line: 1657	
  StringBuilder.append(Object) line: 194	
  AttributeInfo.toString() line: 65	
  SingletonImmutableList<E>.toString() line: 97	
  String.valueOf(Object) line: 1657	
  StringBuilder.append(Object) line: 194	
  ClassType(HierarchicalType<ST,T>).toString() line: 372	
  String.valueOf(Object) line: 1657	
  StringBuilder.append(Object) line: 194	
  AttributeInfo.toString() line: 65	
  SingletonImmutableList<E>.toString() line: 97	
  String.valueOf(Object) line: 1657	
  StringBuilder.append(Object) line: 194	
  ClassType(HierarchicalType<ST,T>).toString() line: 372	

Also, for instance data, using the id string is not sufficient because structs that are not implementations of IReferenceableInstance won't have an id string.  And for those structs that are IReferenceableInstance objects, if the structs are newly created in memory and have not been persisted, they won't have a meaningful id string.  Such objects would all have the same id string, and result in false positives on the inProcess check.

I have re-implemented the fix to maintain the inProcess set in thread local storage rather than passing it as an argument.  This fix addresses the recursion issue for both types and instance data.


- David


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45948/#review129505
-----------------------------------------------------------


On April 29, 2016, 7:24 p.m., David Kantor wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45948/
> -----------------------------------------------------------
> 
> (Updated April 29, 2016, 7:24 p.m.)
> 
> 
> Review request for atlas.
> 
> 
> Bugs: ATLAS-645
>     https://issues.apache.org/jira/browse/ATLAS-645
> 
> 
> Repository: atlas
> 
> 
> Description
> -------
> 
> ATLAS-645: In FieldMapping.output(), avoid infinite recursion when IReferenceableInstance and IStruct instances reference each other.
> 
> 
> Diffs
> -----
> 
>   typesystem/src/main/java/org/apache/atlas/typesystem/persistence/StructInstance.java af62442bfe5daa221079207acf361e1316cab3ad 
>   typesystem/src/main/java/org/apache/atlas/typesystem/types/FieldMapping.java 36149bafff80b68ce176e82dcacac87035459362 
>   typesystem/src/main/java/org/apache/atlas/typesystem/types/HierarchicalType.java 89fcea6828c9e23c28a60952c3e4ca27c0667494 
>   typesystem/src/main/java/org/apache/atlas/typesystem/types/OutputStatus.java PRE-CREATION 
>   typesystem/src/main/java/org/apache/atlas/typesystem/types/StructType.java 54e344f5d6322a00ac7825ee8964f43a1552dcbe 
>   typesystem/src/test/java/org/apache/atlas/typesystem/types/OutputStatusTest.java PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/45948/diff/
> 
> 
> Testing
> -------
> 
> Ran all unit and integration tests with no regressions.  Added test cases
> 
> 
> Thanks,
> 
> David Kantor
> 
>


Re: Review Request 45948: Atlas-645: avoid infinite recursion in FieldMapping.output()

Posted by Shwetha GS <ss...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45948/#review129505
-----------------------------------------------------------




typesystem/src/main/java/org/apache/atlas/typesystem/types/FieldMapping.java (line 99)
<https://reviews.apache.org/r/45948/#comment192941>

    Can we maintain this as Set<String> and check on just typeName?


- Shwetha GS


On April 18, 2016, 5:13 p.m., David Kantor wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45948/
> -----------------------------------------------------------
> 
> (Updated April 18, 2016, 5:13 p.m.)
> 
> 
> Review request for atlas.
> 
> 
> Bugs: ATLAS-645
>     https://issues.apache.org/jira/browse/ATLAS-645
> 
> 
> Repository: atlas
> 
> 
> Description
> -------
> 
> ATLAS-645: In FieldMapping.output(), avoid infinite recursion when IReferenceableInstance and IStruct instances reference each other.
> 
> 
> Diffs
> -----
> 
>   typesystem/src/main/java/org/apache/atlas/typesystem/persistence/ReferenceableInstance.java 31ef49d5e6accd8b4621385cce1e3a6b7a6936ff 
>   typesystem/src/main/java/org/apache/atlas/typesystem/persistence/StructInstance.java af62442bfe5daa221079207acf361e1316cab3ad 
>   typesystem/src/main/java/org/apache/atlas/typesystem/types/AbstractDataType.java 92be3c794077bd3e437aa6f3426ed8b7892b945e 
>   typesystem/src/main/java/org/apache/atlas/typesystem/types/ClassType.java 90cf3ccfe8e9d0e20901452c0b83796ba3994998 
>   typesystem/src/main/java/org/apache/atlas/typesystem/types/DataTypes.java 55ec91f5c3cba65f1bf7c524c84c075cc1c96dff 
>   typesystem/src/main/java/org/apache/atlas/typesystem/types/FieldMapping.java 36149bafff80b68ce176e82dcacac87035459362 
>   typesystem/src/main/java/org/apache/atlas/typesystem/types/IDataType.java 373ad2c93efdecc292b68fab1ea2403afce84dac 
>   typesystem/src/main/java/org/apache/atlas/typesystem/types/StructType.java 54e344f5d6322a00ac7825ee8964f43a1552dcbe 
>   typesystem/src/main/java/org/apache/atlas/typesystem/types/TraitType.java 84c22bf96a059a78b065a73a8feefae87deb9975 
>   typesystem/src/main/java/org/apache/atlas/typesystem/types/TypedStructHandler.java da246d66d94bcd3e221b70b859de2850fa6cf7a8 
>   typesystem/src/test/java/org/apache/atlas/typesystem/types/FieldMappingTest.java PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/45948/diff/
> 
> 
> Testing
> -------
> 
> Ran all unit and integration tests with no regressions.  Added test cases
> 
> 
> Thanks,
> 
> David Kantor
> 
>


Re: Review Request 45948: Atlas-645: avoid infinite recursion in FieldMapping.output()

Posted by David Kantor <dk...@us.ibm.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45948/
-----------------------------------------------------------

(Updated April 18, 2016, 5:13 p.m.)


Review request for atlas.


Changes
-------

Addressed Shwetha's comment - "in process" set now passed as an argument to FieldMapping.output() and IDataType.output()


Bugs: ATLAS-645
    https://issues.apache.org/jira/browse/ATLAS-645


Repository: atlas


Description
-------

ATLAS-645: In FieldMapping.output(), avoid infinite recursion when IReferenceableInstance and IStruct instances reference each other.


Diffs (updated)
-----

  typesystem/src/main/java/org/apache/atlas/typesystem/persistence/ReferenceableInstance.java 31ef49d5e6accd8b4621385cce1e3a6b7a6936ff 
  typesystem/src/main/java/org/apache/atlas/typesystem/persistence/StructInstance.java af62442bfe5daa221079207acf361e1316cab3ad 
  typesystem/src/main/java/org/apache/atlas/typesystem/types/AbstractDataType.java 92be3c794077bd3e437aa6f3426ed8b7892b945e 
  typesystem/src/main/java/org/apache/atlas/typesystem/types/ClassType.java 90cf3ccfe8e9d0e20901452c0b83796ba3994998 
  typesystem/src/main/java/org/apache/atlas/typesystem/types/DataTypes.java 55ec91f5c3cba65f1bf7c524c84c075cc1c96dff 
  typesystem/src/main/java/org/apache/atlas/typesystem/types/FieldMapping.java 36149bafff80b68ce176e82dcacac87035459362 
  typesystem/src/main/java/org/apache/atlas/typesystem/types/IDataType.java 373ad2c93efdecc292b68fab1ea2403afce84dac 
  typesystem/src/main/java/org/apache/atlas/typesystem/types/StructType.java 54e344f5d6322a00ac7825ee8964f43a1552dcbe 
  typesystem/src/main/java/org/apache/atlas/typesystem/types/TraitType.java 84c22bf96a059a78b065a73a8feefae87deb9975 
  typesystem/src/main/java/org/apache/atlas/typesystem/types/TypedStructHandler.java da246d66d94bcd3e221b70b859de2850fa6cf7a8 
  typesystem/src/test/java/org/apache/atlas/typesystem/types/FieldMappingTest.java PRE-CREATION 

Diff: https://reviews.apache.org/r/45948/diff/


Testing
-------

Ran all unit and integration tests with no regressions.  Added test cases


Thanks,

David Kantor


Re: Review Request 45948: Atlas-645: avoid infinite recursion in FieldMapping.output()

Posted by David Kantor <dk...@us.ibm.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45948/
-----------------------------------------------------------

(Updated April 15, 2016, 6:36 p.m.)


Review request for atlas.


Changes
-------

Addressed Hemanth and Shwetha's comment - removed mutable state from FieldMapping, added IStruct methods to track output status that are implemented in new base class AbstractStruct.


Bugs: ATLAS-645
    https://issues.apache.org/jira/browse/ATLAS-645


Repository: atlas


Description
-------

ATLAS-645: In FieldMapping.output(), avoid infinite recursion when IReferenceableInstance and IStruct instances reference each other.


Diffs (updated)
-----

  typesystem/src/main/java/org/apache/atlas/typesystem/AbstractStruct.java PRE-CREATION 
  typesystem/src/main/java/org/apache/atlas/typesystem/IStruct.java e0f85761a0f436cd4b4cd355d6c8ab65dca7fcc9 
  typesystem/src/main/java/org/apache/atlas/typesystem/Struct.java 70deab29df36e7e18f517c4880d7088f8bec06a5 
  typesystem/src/main/java/org/apache/atlas/typesystem/persistence/DownCastStructInstance.java d3b9a3376ebce92ff8b50708fa841ab5868b366a 
  typesystem/src/main/java/org/apache/atlas/typesystem/persistence/Id.java d742bb73e89772afcf8e0bd25f4fda9b7c94758b 
  typesystem/src/main/java/org/apache/atlas/typesystem/persistence/StructInstance.java 16c3a24e51f0389946e0e6094a6a3e4e1b9dff0f 
  typesystem/src/main/java/org/apache/atlas/typesystem/types/FieldMapping.java 36149bafff80b68ce176e82dcacac87035459362 
  typesystem/src/test/java/org/apache/atlas/typesystem/types/FieldMappingTest.java PRE-CREATION 

Diff: https://reviews.apache.org/r/45948/diff/


Testing
-------

Ran all unit and integration tests with no regressions.  Added test cases


Thanks,

David Kantor