Re: [infinispan-dev] GetKeyValueCommand NPE with CR1 in Hibernate 2LC - ISPN-7029
--
Galder Zamarreño
Infinispan, Red Hat
On 15 Feb 2017, at 12:21, Radim Vansa <rvansa(a)redhat.com>
wrote:
On 02/15/2017 11:28 AM, Galder Zamarreño wrote:
> Hey Radim,
>
> Your changes in ISPN-7029 are causing issues with Hibernate 2LC.
>
> In particular [2]. Name.equals() always returns false, so it'll never be found in
the context. So entry is null.
That's obviously a bug in the 2LC testsuite, isn't it?
LOL, is it? You added the class and the javadoc clearly states that this entity defines
equals incorrectly. You must have added it for a reason?
https://github.com/hibernate/hibernate-orm/blob/master/hibernate-infinisp...
In any case, an object with an incorrect equals impl should not result in an NPE within
Infinispan :|
Object used as @EmbeddedId needs to have the equals correctly
defined. How else would you compare ids? I wonder how could that work in the past.
ROFL... it's your baby so you tell me ;)
>
> Moreover, if EntryLookup.lookupEntry javadoc (and some implementations) can and do
return null. Are you expecting that somehow that method will never return null?
With ISPN-7029 in, the entry should be wrapped in the context after
EntryWrappingInterceptor if the key is in readCH, otherwise it should be null. In case
that xDistributionInterceptor finds out that it needs to work on that value despite not
being able to read it (e.g. in case that it's in writeCH during unfinished rebalance),
it should wrap NullCacheEntry.getInstance() using EntryFactory. wrapExternalEntry. More
info about the logic is in o.i.c.EntryFactory javadoc [3].
Not sure I understand what you're trying to imply above... so, is lookupEntry()
allowed to return null or not?
To be more specific, SingleKeyNonTxInvocationContext.lookupEntry() can return null, so
GetKeyValueCommand should be able to handle it? Or should
SingleKeyNonTxInvocationContext.lookupEntry return NullCacheEntry.getInstance instead of
null?
To provide more specifics, SingleKeyNonTxInvocationContext has NullCacheEntry.getInstance
in cacheEntry variable when it's returning null. Should it maybe return
NullCacheEntry.getInstance instead of null from lookupEntry() ?
Cheers,
Radim
[3]
https://github.com/infinispan/infinispan/blob/master/core/src/main/java/o...
>
> I'll create a JIRA to track all issues arising from Hibernate 2LC in a minute,
but wanted to get your thoughts firts.
>
> Cheers,
>
> [1] https://issues.jboss.org/browse/ISPN-7029
> [2]
https://github.com/hibernate/hibernate-orm/blob/master/hibernate-infinisp...
> --
> Galder Zamarreño
> Infinispan, Red Hat
>
--
Radim Vansa <rvansa(a)redhat.com>
JBoss Performance Team