[hibernate-dev] AbstractStandardBasicType#getReplacement() prototype is incorrect since HHH-12054

[Guillaume Smet]

Hi,

When HHH-12054 was fixed (see
https://github.com/hibernate/hibernate-orm/pull/2042/files), we took into
account the fact that original and target could be set to
LazyPropertyInitializer.UNFETCHED_PROPERTY in the
AbstractStandardBasicType#getReplacement() method whereas we haven't
changed the prototype of the method.

Meaning we expect for instance LazyPropertyInitializer.UNFETCHED_PROPERTY
to be a T, which is not the case.

This led to ClassCastException such as in
https://hibernate.atlassian.net/browse/HHH-12555 . To be honest, I'm
wondering why we don't have more of them.

We have 2 ways of fixing this issue:
1- change the getReplacement() prototype to take Object parameters and
return an Object as the originating replace() method
2- move the LazyPropertyInitializer.UNFETCHED_PROPERTY logic up to
replace() and make getReplacement() a "I'm sure everything is OK, now get
me a clone of my original object" thing.

1- will obviously break compatibility with any user type extending
AbstractStandardBasicType#getReplacement(). 2- will slightly change the
meaning of replace() and getReplacement() and might lead to subtle behavior
changes in user applications.

2- also means that we would have to override both replace() and
getReplacement() in BlobType and allegates.

I pushed a commit for 1- at
https://github.com/gsmet/hibernate-orm/commit/73e66e40f3438794375451145af45214d7d1dc00
so that you can fully grasp the issue.

Not saying it's the path we should take.

Thoughts?

-- 
Guillaume