Be aware of the Cyclic Dependency Issue when using Lombok

[GoogleCodeExporter]

Hi Guys,

I came up with an issue regarding the cyclic dependency and 
java.lang.StackOverflowError because of lombok. Here is an explanation of the 
problem and how it was resolved.

Risks of cyclic relationships between classes. 

I had two classes:

@Entity
@Table(name = "INTERESTED_PARTIES")
@Data
public class InterestedParty extends VersionBaseModel {

 @Id
 @Column(name = "INT_PARTY_ID")
 @GeneratedValue(generator = "NAME_ID_SEQ_GEN")
 @SequenceGenerator(name = "NAME_ID_SEQ_GEN", sequenceName = "NAME_ID_SEQ", allocationSize = 1)
 private Long id;

 @OneToOne(mappedBy = "party", fetch = FetchType.LAZY)
 private InterestedPartyAddress address; ....

 ...
}

And,

@Entity
@Table(name = "INTERESTED_PARTY_ADDRESSES")
@Data
public class InterestedPartyAddress extends VersionModel {

 @Id
 @Column(name = "ADDR_ID")
 @GeneratedValue(generator = "ADDR_ID_SEQ_GEN")
 @SequenceGenerator(name = "ADDR_ID_SEQ_GEN", sequenceName = "ADDR_ID_SEQ", allocationSize = 1)
 private Long id;

 @OneToOne
 @JoinColumn(name = "INT_PARTY_ID")
 private InterestedParty party;
 ....
 ...

}

Apparently, there is nothing wrong with these classes. However when using 
lombok's @Data annotation, the above classes will  include additional methods 
(vanilla java code). IE. <<@Data generates all the boilerplate that is normally 
associated with simple POJOs (Plain Old Java Objects) and beans: getters for 
all fields, setters for all non-final fields, and appropriate toString, equals 
and hashCode implementations that involve the fields of the class, and a 
constructor that initializes all final fields, as well as all non-final fields 
with no initializer that have been marked with @NonNull, in order to ensure the 
field is never null.>>

[Source URL: http://projectlombok.org/features/Data.html]

Let's see how the toString() methods will be generated:

For InterestedParty.java, it will produce a toString() method like this:

 @Override
 public String toString() {
  return "InterestedParty [id=" + this.getId() + ", address=" + this.getAddress() + "]";
 }

For InterestedPartyAddress.java, it will produce a toString() method like this:

 @Override
 public String toString() {
  return "InterestedPartyAddress [id=" + this.getId() + ", party=" + this.getParty() + "]";
 }

And here  is the problem. As seen from the toString() methods, the 
InterestedParty class and the InterestedPartyAddress class now have a cyclic 
relationship created because of the toString() methods. A InterestedParty 
instance has a reference to its InterestedPartyAddress in the toString() method 
and a InterestedPartyAddress has a reference to the InterestedParty's 
toString() method. As a result, a StackOverflowError occurs when the respective 
toString() methods of each object try to call one another. 

Error:
Caused by: java.lang.StackOverflowError
        at java.lang.StringBuilder.<init>(StringBuilder.java:85) [rt.jar:1.7.0_60]
        at au.gov.nsw.osr.mars.model.duties.InterestedParty.toString(InterestedParty.java:33) [mars-ejb.jar:]
        at java.lang.String.valueOf(String.java:2847) [rt.jar:1.7.0_60]
        at java.lang.StringBuilder.append(StringBuilder.java:128) [rt.jar:1.7.0_60]

Error detail is attached with this email.

Solution:
Generally this is not an issue if it is a one directional relationship. But in 
case of a bi-directional relationship, it can cause clyclic dependency hence 
causing big problem. We can solve it either of the following two ways:

1. If using lombok's @Data annotation, then resolve by using the 
@ToString(callSuper=true, includeFieldNames=false) on a specific field that we 
do not want to be included in the toString()
Pro: Less code. 
Cons: Debugging pain, need to be cautious about cyclic dependency. Failing to 
do such might cause StackOverflow error. Also sometimes we do not want 
getter-setter for certain fields. In such a case, we must not forget to use 
@Getter(AccessLevel.NONE) or @Setter(AccessLevel.NONE) in a field level

2. By NOT using @Data. Instead, using Eclipse's getter-setter, toString(), 
hasCode() generate feature 
Pros: Debugging made easy. No need to worry about cyclic dependency, we can 
decide which attributes/properties should be included while generating 
getter-setter/toString/hasCode etc
Con: Verbosity

In my case, I resolved the issue by following step 2 i.e. get rid of the 
lombok's @Data annotation and producing getter-setters with Eclipse. I did not 
need toString() at all. 

A further discussion on clyclic relation can be found here: 
http://marxsoftware.blogspot.com.au/2009/07/diagnosing-and-resolving.html

Thanks.
... Chisty

Original issue reported on code.google.com by mchi...@gmail.com on 21 Jan 2015 at 7:56

[GoogleCodeExporter]

> I resolved the issue by ... get rid of the lombok's @Data annotation and 
producing getter-setters with Eclipse.

IMHO that's an outstandingly terrible solution. You could add a trivial 
`toString` calling super and be done. You could also use 
`ToString(exclude="InterestedParty")` in order to break the cycle. You could 
replace `@Data` by `@Getter`, `@Setter`, and whatever else you need.

While Lombok could handle cycles, it's not free. The cost would be much longer 
and slower code, auxiliary methods, thread-local magic, and/or a runtime 
dependency. Note that neither JDK containers, nor Guava's and Apache's 
`ToStringBuilder`, nor any class I've ever seen protects against cycles.

Original comment by Maaarti...@gmail.com on 24 Jan 2015 at 11:40

[GoogleCodeExporter]

As you said <<IMHO that's an outstandingly terrible solution. You could add a 
trivial `toString` calling super and be done. You could also use 
`ToString(exclude="InterestedParty")` in order to break the cycle. You could 
replace `@Data` by `@Getter`, `@Setter`, and whatever else you need.>>

If you look at my comments carefully, I already mentioned it in point #1. 

Terrible solution !!! Well, you could say that because as a Lombok tool 
developer, you will want to promote your tool. I can't blame you.

Also... what is your explanation about debugging? Lombok is just good for 
removing verbosity (or less code); that's all. 

Thanks.

Original comment by mchi...@gmail.com on 25 Jan 2015 at 6:56

[GoogleCodeExporter]

For your information: the java standard libraries are also prone to stack 
overflow problems. Try out the following code:

List<Object> list = new ArrayList<>();
list.add(list);
String content = "" + list;

As one of the core lombok developers, I am biased. I think 
@ToString(exclude={"party"}) is very clear and concise.

Original comment by r.spilker on 27 Jan 2015 at 4:54

• Changed state: WontFix

[GoogleCodeExporter]

[deleted comment]

[GoogleCodeExporter]

I agree. Java standard libraries are also prone to StackOverflow. However in 
the above scenario, the cyclic dependency was my main problem which caused the 
build broken. The StackOverflow error occurred in this case because of the 
cyclic dependency. Like I mentioned in my original post, developers won't even 
have to worry about using ToString at all unless and until there is a 
bi-directional relationship. In any other cases, no issues.

Anyway, you did not answer anything about the debugging problem (debugging of 
getter-setters). Any clue or suggestion?

Thanks.

Original comment by mchi...@gmail.com on 27 Jan 2015 at 8:23

[GoogleCodeExporter]

#2 FYI, I'm not a Lombok developer (not even a contributor apart from some tiny 
things), just a fan.

> If you look at my comments carefully, I already mentioned it in point #1.

You did, but you chose the worse solution. YMMV. For me, keeping the source 
compact and clean is very important. And yes, that's all Lombok can do.

Concerning debugging, I guess you got no answer because there's no such 
question? At least, I can't see it. When I run into a problem with a ToString 
cycle, I just fix it. I don't try debug into trivial generated methods, just 
like I don't try debug into bridge methods. It wouldn't work, but I couldn't 
care less as there's nothing interesting there.

Original comment by Maaarti...@gmail.com on 28 Jan 2015 at 6:12

[GoogleCodeExporter]

[deleted comment]

[GoogleCodeExporter]

[deleted comment]

[GoogleCodeExporter]

[deleted comment]

[GoogleCodeExporter]

[deleted comment]

[GoogleCodeExporter]

As you mentioned <<When I run into a problem with a ToString cycle, I just fix 
it. I don't try debug into trivial generated methods>>. 

Answer: Neither do I. Let me explain: say you have a class with lomboks' @Data 
annotated. Now read carefully: I an NOT talking about cyclic dependency and 
ToString annotation. It is a different topic. (Cyclic dependency happens only 
when there is bi-directional relationship and developer do not use ToString as 
you mentioned). Now I will explain the debugging issue.

Let's say, at some point, your application is throwing an exception from UI 
layer. E.G. you tick a checkbox in your page, you expect a boolean property 
(someProperty) in your class to be true but it seems like the value is is still 
false. What could be wrong? What is causing the value not to be updated? Is 
ajax refresh working? Is the page re-rendered after a onclick event..... etc .. 
etc.. lot of question. So you might want to debug whether the property values  
are being set/updated correctly; if so, where or at what point; you might want 
to set the debug point on isSomeProperty(), setSomeProperty() methods [don't 
say you do not do debugging and you fix code without debugging). Can you do 
that? You can't. Because there is NO is/get/set methods. Got it?

<<Concerning debugging, I guess you got no answer because there's no such 
question?>>

Answer: I do not get it. Do you really have the answer? There is no such 
question just because nobody asked it before? What's your point?

Thanks.

P.S. I am not expecting more answers like "I don't try debugging...  I just fix 
it...  there is no such questions " ..... :). If you do not have the answer, 
admit it.

Original comment by mchi...@gmail.com on 28 Jan 2015 at 10:08

[GoogleCodeExporter]

For debugging purposes you can always use delombok to (temporarily) convert 
your files to plain java and use those files to compile and debug your project. 

That said, there should really be no need to step into, say, a getter. Getters 
and setters are simple. They do what they are supposed to do. If you get an 
unexpected value, it's not the getter or setter that failed, but the one 
calling the setter or constructor. 

The Eclipse Outline View is also very helpful. If you right click on the lombok 
generated method there are several useful options. 

Toggle Method Breakpoint will break when the method is called. Although you 
cannot see the actual source of the method, the Variables view will display the 
values of the parameters. And Step Into, Over or Return will step to the 
calling code. The Breakpoint View allows you to set more properties, allowing 
for conditions, for instance to only break when a certain parameter is set, or 
to break on exit, allowing inspection of state of the object.

The References option allows you to quickly find the callers of the lombok 
generated code.

All in all, I think there are enough options to debug your lombok laced code.

Original comment by r.spilker on 29 Jan 2015 at 5:16