Bugfix/1430 do not get unused value

[aleksandy]

Some cleanup and fix #1430

[asolntsev]

@xael-fry LTGT. Recommended to merge.

[straycat264]

I notice you've tidied up the section of code which started:
} else if (value instanceof PersistentCollection) { PersistentCollection col = (PersistentCollection) value; if (((PersistentCollection) value).wasInitialized()) {

Then proceeded to cascade the changes whether or not the collection was initialised.
Your tidy-up consists of dispensing with the wasInitialized() check - presumably because it was seemingly redundant.

My question: we've observed that this code causes saveOrCascade to load collections that cannot have had any modifications (as they're not currently loaded) before saving - which has led to performance issues. Is there a good reason for this behaviour? I assume so, but it would be good to know what that reason is.

[aleksandy]

@straycat264, my changes didn't change code behaviour. It wasn't the goal of this PR.

The code in the if and else branches was the same, except that the PersistenceCollection in the if branch was passed through a cast of value, and the else used a separate variable col obtained by the same casting of the value.

In my opinion, your question should be moved to separate ticket. Perhaps here should use the approach used to processing PersistenceMap and do nothing if the collection is not initialized.

[xael-fry]

Merge in master
Thanks @aleksandy

[JohannesBeranek]

@aleksandy @xael-fry

                            if (((PersistentCollection) value).wasInitialized()) {

This line was removed in this commit - why?

[asolntsev]

@JohannesBeranek I don't remember the details, but I believe the resulting code is effectively the same. I think it was a refactoring suggested by IDEA: it removed duplicated/unneeded line, thus leading to a shorter code that does the same job.