[hibernate-dev] HHH-12353: have getReference stick to the Java API it promises?
Sanne Grinovero
sanne at hibernate.org
Mon Mar 5 10:32:30 EST 2018
On 5 March 2018 at 15:24, Steve Ebersole <steve at hibernate.org> wrote:
> Fun trip down memory lane :)
>
> No one really like my proposed solution, but also no one really seemed to
> dislike it enough to implement something themselves ;)
>
> Anyway, those older discussions are a piece of the same overall discussion
> that this one belongs to. This discussion is just a specific part of it.
> Sanne and I both thought that the API should just accept the interface - we
> just had different opinions about how that should work in mapping and in the
> impl of the call. Neither of our suggestions is spec compliant though. And
> in this specific case I think (a) is the only real option
Right, the JPA API doesn't leave much room to explore the alternatives
we discussed in the past.
What do you suggest we do now?
In a nudge towards backwards compatibility it would be nice to start
logging a bootstrap warning when a final class is being declared with
such a proxy: we could warn about the inconsistent return that will
happen at runtime, and would also be a good opportunity to warn we
might remove support for such a tricky mapping altogether in a future
release.
And thanks Gunnar for finding the previous discussions, I had a vague
idea we discussed this before :)
Sanne
>
> On Mon, Mar 5, 2018 at 8:57 AM Gunnar Morling <gunnar at hibernate.org> wrote:
>>
>> Hey,
>>
>> It's just three and six years, respectively, since we discussed this
>> matter
>> last [1], [2] (wow, I'm feeling old somehow now ;)
>>
>> Detecting the situation at boot time and raising an exception early on
>> would be the best.
>>
>> --Gunnar
>>
>> [1] http://lists.jboss.org/pipermail/hibernate-dev/2015-April/012520.html
>> [2]
>> http://lists.jboss.org/pipermail/hibernate-dev/2012-January/007629.html
>>
>>
>> 2018-03-05 14:53 GMT+01:00 Sanne Grinovero <sanne at hibernate.org>:
>>
>> > Issue HHH-12353 was just reported and I have to agree it looks dodgy.
>> >
>> > Essentially invoking:
>> >
>> > entityManager.getReference( Person.class )
>> >
>> > might not return a type compatible with Person, when Person is final
>> > and annotated with @Proxy(proxyClass = AnotherTypeX.class), then
>> > AnotherTypeX might be returned and this is not necessarily a subtype
>> > of Person.
>> >
>> > That's ok as I understand we hardly have alternatives, but the
>> > EntityManager API for getReference mandates this signature:
>> >
>> > <T> T getReference(Class<T> entityClass, Object primaryKey)
>> >
>> > so we're not actually returning the expected type T in all
>> > circumstances, confusing compilers and tooling into giving the wrong
>> > suggestions.
>> >
>> > I see some options:
>> > a# disallow using @Proxy on a final entity - seems reasonable since
>> > the docs warn against pitfalls.
>> > b# throw an exception on Person.class being used as parameter, as the
>> > user should actually use the proxytype?
>> > c# somehow have the proxy still extend the final entity?
>> >
>> > Option b# seems problematic as it's not obvious to users either, and
>> > we would need to return a proxy reference even when there actually is
>> > a managed Person instance in the current session.
>> >
>> > Option c# is probably unrealistic and problematic too, as it would
>> > require enhancing of the final class and I guess we can't rely on that
>> > consistently.
>> >
>> > In summary, I believe we should throw a bootstrap exception requiring
>> > to remove the `final` modifier on the parent entity. Or challenge the
>> > specification :(
>> >
>> > Thanks,
>> > Sanne
>> > _______________________________________________
>> > hibernate-dev mailing list
>> > hibernate-dev at lists.jboss.org
>> > https://lists.jboss.org/mailman/listinfo/hibernate-dev
>> >
>> _______________________________________________
>> hibernate-dev mailing list
>> hibernate-dev at lists.jboss.org
>> https://lists.jboss.org/mailman/listinfo/hibernate-dev
More information about the hibernate-dev
mailing list