You are viewing a plain text version of this content. The canonical link for it is here.
Posted to users@groovy.apache.org by "Nikolai (gmail)" <ni...@googlemail.com> on 2019/02/05 21:46:18 UTC

AST Transformation Issues

Hello,

I experienced some Issues using ToString and AutoClone on Entities 
(Spring JPA).

Both can lead to endless loops / a stackoverflow, when classes reference 
each other.

While it's fine for AutoClone, I think the generated toString-Method 
should recognize it was already called once and return a dummy value or 
just the object reference.


Anyway, I got bigger issues using AutoClone:

1. I use an abstract class to add auditing columns to all my tables. All 
my Entities inherit from that class (AbstractEvent in this example). 
Since these fields should be empty/null on cloning, I'd like to omit the 
AutoClone annotation on that class. But this leads to an error: No 
signature of method: Event.cloneOrCopyMembers() is applicable for 
argument types: (Event) values: [Event(null, null, [], Event@54d9d12d)].

2. SubEvent objects within Event.subEvents were not cloned.

import groovy.transform.AutoClone
import groovy.transform.Canonical
import groovy.transform.ToString
import static groovy.transform.AutoCloneStyle.SIMPLE

@AutoClone(style=SIMPLE) //1. error when you remove this
abstract class AbstractEvent {

     Date created
     String createdBy

     Date modified
     String modifiedBy
}

@AutoClone(style=SIMPLE)
@ToString(includeSuper = true)
class Event extends AbstractEvent {
     Long id
     String someText

     ArrayList<SubEvent> subEvents = new ArrayList();
}

@AutoClone(style=SIMPLE)
@ToString(includeSuper = true, excludes = ['event'])
class SubEvent extends AbstractEvent {
     Long id
     String someText

     Event event;
}

public static void main(String[] args){
     Event event = new Event(
         id: 1,
         someText: "Event 1",
         created: new Date(),
         createdBy: "me");

     SubEvent subEvent1 = new SubEvent(
         id: 1,
         someText: "SubEvent 1",
         event: event);

     event.subEvents += subEvent1


     Event clonedEvent = event.clone()
     assert !event.is(clonedEvent)
     assert !event.subEvents.is(clonedEvent.subEvents)
     assert !event.subEvents.first().is(clonedEvent.subEvents.first()) 
// 2. fails
}


Hope, you can help me here.



Kind Regards

Nikolai


Re: AST Transformation Issues

Posted by "Nikolai (gmail)" <ni...@googlemail.com>.
Thank you.

I was wondering why the List members were not cloned and just noticed 
the clone method was called on the List, not the members.
Also I added one more test, but I don't understand why it succeeds:
assert clonedEvent.subEvents.first().event.is(clonedEvent)
Obviously the nested event is a shallow copy of the clonedEvent. That's 
really nice, although I expected a copy of the original event here.


With regard to the ToString-Annotation and mutual recursion, I think a 
representation similar to those by the InvokerHelper methods seem to fit 
for data structures only (trees, lists, maps).
In the Event/SubEvent example I'd like to know/see that SubEvents have 
an event which reference the parent Event. However, the former is a good 
start to avoid stackoverflows in the first place.



Regards

Nikolai


Am 06.02.19 um 02:38 schrieb Paul King:
>
> Hi,
>
> With regard to stack overflow when printing. This is a known 
> limitation. ToString has been made smart enough to handle self 
> references, e.g.
>
>     import groovy.transform.*
>
>     @ToString
>     class Tree {
>         Tree left, right
>     }
>
>     def t1 = new Tree()
>     t1.left = t1
>     println t1 // => Tree((this), null)
>
> but isn't smart enough to handle mutual recursion, e.g.:
>
>     def t2 = new Tree()
>     t1.left = t2
>     t2.left = t1
>     println t1 // => StackOverflowError
>
> The plan has always been to make it smarter but we haven't done it 
> yet. PRs welcome.
>
> If anyone is interested, I'd recommend something simple like what we 
> have done for lists and maps in the respective InvokerHelper methods.
> E.g. for maps, self reference is already handled:
>
>     def t3 = [:]
>     t3.with{ left = t3; right = t3 }
>     println t3 // => [left:(this Map), right:(this Map)]
>
> And mutual reference is handled by setting a maximum size (30 in the 
> example below and three dots is used once the toString becomes greater 
> than 30 in size):
>
>     import org.codehaus.groovy.runtime.InvokerHelper
>     def t4 = [left: t3]
>     t3.right = t4
>     println InvokerHelper.toMapString(t3, 30) // => [left:(this Map), 
> right:[left:[...]]]
>
> It works so long as the Map contents themselves don't have stack 
> overflow scenarios that aren't catered for (some scenarios are handled).
>
> Similarly for lists, self reference is handled:
>
>     def items = []
>     3.times{ items << items }
>     println items // [(this Collection), (this Collection), (this 
> Collection)]
>
> but you can limit the size (e.g. for the case above) or to handle 
> mutual reference:
>
>     println InvokerHelper.toListString(items, 30) // => [(this 
> Collection), (this Collection), ...]
>
>     def list1 = []
>     def list2 = []
>     list1 << list2
>     list2 << list1
>     //println list1 // StackOverflowError
>     println InvokerHelper.toListString(list1, 10) // => 
> [[[[[[[[[[[...]]]]]]]]]]]
>
> So getting back to @ToString, I'd imagine an enhancement could involve 
> either generating a toString(int maxSize) method or supporting a 
> maxSize annotation attribute that was automatically supported in the 
> generated code for the normal toString method.
>
> With regard to the AutoClone issues, as per the documentation, the 
> supported cloning styles have various assumptions, e.g. SIMPLE style 
> assumes child classes are of a similar style (or have equivalent 
> methods), and the basic styles support only shallow cloning. For your 
> case you want deep cloning, so SERIALIZATION would be the way to go.
>
>     abstract class AbstractEvent {
>         Date created
>         String createdBy
>         Date modified
>         String modifiedBy
>     }
>
>     @AutoClone(style=SERIALIZATION)
>     @ToString(includeSuper = true)
>     class Event extends AbstractEvent implements Serializable {
>         Long id
>         String someText
>         ArrayList<SubEvent> subEvents = new ArrayList()
>     }
>
>     @AutoClone(style=CLONE)
>     @ToString(includeSuper = true, excludes = ['event'])
>     class SubEvent extends AbstractEvent implements Serializable {
>         Long id
>         String someText
>         Event event
>     }
>
> Cheers, Paul.
>
>
>
>
>
> <https://www.avast.com/sig-email?utm_medium=email&utm_source=link&utm_campaign=sig-email&utm_content=webmail> 
> 	Virus-free. www.avast.com 
> <https://www.avast.com/sig-email?utm_medium=email&utm_source=link&utm_campaign=sig-email&utm_content=webmail> 
>
>
>
> On Wed, Feb 6, 2019 at 7:46 AM Nikolai (gmail) 
> <nikolai.dueck@googlemail.com <ma...@googlemail.com>> 
> wrote:
>
>     Hello,
>
>     I experienced some Issues using ToString and AutoClone on Entities
>     (Spring JPA).
>
>     Both can lead to endless loops / a stackoverflow, when classes
>     reference each other.
>
>     While it's fine for AutoClone, I think the generated
>     toString-Method should recognize it was already called once and
>     return a dummy value or just the object reference.
>
>
>     Anyway, I got bigger issues using AutoClone:
>
>     1. I use an abstract class to add auditing columns to all my
>     tables. All my Entities inherit from that class (AbstractEvent in
>     this example). Since these fields should be empty/null on cloning,
>     I'd like to omit the AutoClone annotation on that class. But this
>     leads to an error: No signature of method:
>     Event.cloneOrCopyMembers() is applicable for argument types:
>     (Event) values: [Event(null, null, [], Event@54d9d12d)].
>
>     2. SubEvent objects within Event.subEvents were not cloned.
>
>     import groovy.transform.AutoClone
>     import groovy.transform.Canonical
>     import groovy.transform.ToString
>     import static groovy.transform.AutoCloneStyle.SIMPLE
>
>     @AutoClone(style=SIMPLE) //1. error when you remove this
>     abstract class AbstractEvent {
>
>         Date created
>         String createdBy
>
>         Date modified
>         String modifiedBy
>     }
>
>     @AutoClone(style=SIMPLE)
>     @ToString(includeSuper = true)
>     class Event extends AbstractEvent {
>         Long id
>         String someText
>
>         ArrayList<SubEvent> subEvents = new ArrayList();
>     }
>
>     @AutoClone(style=SIMPLE)
>     @ToString(includeSuper = true, excludes = ['event'])
>     class SubEvent extends AbstractEvent {
>         Long id
>         String someText
>
>         Event event;
>     }
>
>     public static void main(String[] args){
>         Event event = new Event(
>             id: 1,
>             someText: "Event 1",
>             created: new Date(),
>             createdBy: "me");
>
>         SubEvent subEvent1 = new SubEvent(
>             id: 1,
>             someText: "SubEvent 1",
>             event: event);
>
>         event.subEvents += subEvent1
>
>
>         Event clonedEvent = event.clone()
>         assert !event.is <http://event.is>(clonedEvent)
>         assert !event.subEvents.is
>     <http://event.subEvents.is>(clonedEvent.subEvents)
>         assert
>     !event.subEvents.first().is(clonedEvent.subEvents.first()) // 2. fails
>     }
>
>
>     Hope, you can help me here.
>
>
>
>     Kind Regards
>
>     Nikolai
>

Re: AST Transformation Issues

Posted by Paul King <pa...@asert.com.au>.
Hi,

With regard to stack overflow when printing. This is a known limitation.
ToString has been made smart enough to handle self references, e.g.

    import groovy.transform.*

    @ToString
    class Tree {
        Tree left, right
    }

    def t1 = new Tree()
    t1.left = t1
    println t1 // => Tree((this), null)

but isn't smart enough to handle mutual recursion, e.g.:

    def t2 = new Tree()
    t1.left = t2
    t2.left = t1
    println t1 // => StackOverflowError

The plan has always been to make it smarter but we haven't done it yet. PRs
welcome.

If anyone is interested, I'd recommend something simple like what we have
done for lists and maps in the respective InvokerHelper methods.
E.g. for maps, self reference is already handled:

    def t3 = [:]
    t3.with{ left = t3; right = t3 }
    println t3 // => [left:(this Map), right:(this Map)]

And mutual reference is handled by setting a maximum size (30 in the
example below and three dots is used once the toString becomes greater than
30 in size):

    import org.codehaus.groovy.runtime.InvokerHelper
    def t4 = [left: t3]
    t3.right = t4
    println InvokerHelper.toMapString(t3, 30) // => [left:(this Map),
right:[left:[...]]]

It works so long as the Map contents themselves don't have stack overflow
scenarios that aren't catered for (some scenarios are handled).

Similarly for lists, self reference is handled:

    def items = []
    3.times{ items << items }
    println items // [(this Collection), (this Collection), (this
Collection)]

but you can limit the size (e.g. for the case above) or to handle mutual
reference:

    println InvokerHelper.toListString(items, 30) // => [(this Collection),
(this Collection), ...]

    def list1 = []
    def list2 = []
    list1 << list2
    list2 << list1
    //println list1 // StackOverflowError
    println InvokerHelper.toListString(list1, 10) // =>
[[[[[[[[[[[...]]]]]]]]]]]

So getting back to @ToString, I'd imagine an enhancement could involve
either generating a toString(int maxSize) method or supporting a maxSize
annotation attribute that was automatically supported in the generated code
for the normal toString method.

With regard to the AutoClone issues, as per the documentation, the
supported cloning styles have various assumptions, e.g. SIMPLE style
assumes child classes are of a similar style (or have equivalent methods),
and the basic styles support only shallow cloning. For your case you want
deep cloning, so SERIALIZATION would be the way to go.

    abstract class AbstractEvent {
        Date created
        String createdBy
        Date modified
        String modifiedBy
    }

    @AutoClone(style=SERIALIZATION)
    @ToString(includeSuper = true)
    class Event extends AbstractEvent implements Serializable {
        Long id
        String someText
        ArrayList<SubEvent> subEvents = new ArrayList()
    }

    @AutoClone(style=CLONE)
    @ToString(includeSuper = true, excludes = ['event'])
    class SubEvent extends AbstractEvent implements Serializable {
        Long id
        String someText
        Event event
    }

Cheers, Paul.





<https://www.avast.com/sig-email?utm_medium=email&utm_source=link&utm_campaign=sig-email&utm_content=webmail>
Virus-free.
www.avast.com
<https://www.avast.com/sig-email?utm_medium=email&utm_source=link&utm_campaign=sig-email&utm_content=webmail>
<#DAB4FAD8-2DD7-40BB-A1B8-4E2AA1F9FDF2>

On Wed, Feb 6, 2019 at 7:46 AM Nikolai (gmail) <ni...@googlemail.com>
wrote:

> Hello,
>
> I experienced some Issues using ToString and AutoClone on Entities (Spring
> JPA).
>
> Both can lead to endless loops / a stackoverflow, when classes reference
> each other.
>
> While it's fine for AutoClone, I think the generated toString-Method
> should recognize it was already called once and return a dummy value or
> just the object reference.
>
>
> Anyway, I got bigger issues using AutoClone:
>
> 1. I use an abstract class to add auditing columns to all my tables. All
> my Entities inherit from that class (AbstractEvent in this example). Since
> these fields should be empty/null on cloning, I'd like to omit the
> AutoClone annotation on that class. But this leads to an error: No
> signature of method: Event.cloneOrCopyMembers() is applicable for argument
> types: (Event) values: [Event(null, null, [], Event@54d9d12d)].
>
> 2. SubEvent objects within Event.subEvents were not cloned.
>
> import groovy.transform.AutoClone
> import groovy.transform.Canonical
> import groovy.transform.ToString
> import static groovy.transform.AutoCloneStyle.SIMPLE
>
> @AutoClone(style=SIMPLE) //1. error when you remove this
> abstract class AbstractEvent {
>
>     Date created
>     String createdBy
>
>     Date modified
>     String modifiedBy
> }
>
> @AutoClone(style=SIMPLE)
> @ToString(includeSuper = true)
> class Event extends AbstractEvent {
>     Long id
>     String someText
>
>     ArrayList<SubEvent> subEvents = new ArrayList();
> }
>
> @AutoClone(style=SIMPLE)
> @ToString(includeSuper = true, excludes = ['event'])
> class SubEvent extends AbstractEvent {
>     Long id
>     String someText
>
>     Event event;
> }
>
> public static void main(String[] args){
>     Event event = new Event(
>         id: 1,
>         someText: "Event 1",
>         created: new Date(),
>         createdBy: "me");
>
>     SubEvent subEvent1 = new SubEvent(
>         id: 1,
>         someText: "SubEvent 1",
>         event: event);
>
>     event.subEvents += subEvent1
>
>
>     Event clonedEvent = event.clone()
>     assert !event.is(clonedEvent)
>     assert !event.subEvents.is(clonedEvent.subEvents)
>     assert !event.subEvents.first().is(clonedEvent.subEvents.first()) //
> 2. fails
> }
>
>
> Hope, you can help me here.
>
>
>
> Kind Regards
>
> Nikolai
>