11:59 a.m.
Hey guys to warm up in 2011, I've reviewed HSEARCH-361.
I have two remarks
Can you add a unit test that checks mutable properties. I think it's covered but
let's plan for a later bug :)
FullTextIndexEventListener compute the list of dirty properties by calling
EntityPersister.findDirty(). I don't think that's entirely accurate. If you look
when findDirty is called in Core, you'll see that we need to handle interceptors. Also
in some cases we call findModified.
The good news is that core computes and caches the list of dirty properties (in
FlushEntityEvent.getDirtyProperties()). Now the even we receive in Hibernate Search is
PostUpdateEvent. I think we can enhance Core to pass along the list of dirty properties.
EntityUpdateAction.postUpdate / postCommitUpdate do create PostUpdateEvents and has the
info (dirtyFields).
Basically we could keep the existing half working impl in HSearch until Hibernate Core
3.6.1 and use the new exposed property of PostUpdateEvent if available (reflection call).
To minimize the reflection call, we should ahve a strategy (ie pre 3.6.1 and post 3.6.1)
and at startup time instantiate the correct one.
WDYT?
5:42 p.m.
Hi Emmanuel,
to adress the missing test I've prepared HSEARCH-658 (
https://github.com/hibernate/hibernate-search/pull/30 ).
Would be very nice to have Core report what changed directly.
BTW, anybody could elaborate on the differences between findModified
and findDirty ?
I just found this code:
if ( event.hasDatabaseSnapshot() ) {
dirtyProperties = persister.findModified(
event.getDatabaseSnapshot(), values, entity, session );
}
else {
dirtyProperties = persister.findDirty( values, entry.getLoadedState(),
entity, session );
}
And as I'm handling update only, it would seem that I should have used
findModified instead.
findModified and findDirty method implementations seem to be very similar, up to
org.hibernate.type.Type.isDirty(Object, Object, boolean[], SessionImplementor)
vs
org.hibernate.type.Type.isModified(Object, Object, boolean[],
SessionImplementor).
Looking into some implementations, I couldn't really understand which
one to use;
"isModified" seems safer as is *usually* performs some extra checks
before delegating
to "isDirty", but there are weird exceptions such as
CollectionType.isModified which returns false for all input.
Bottomline, it seems a bit too much to handle in the FTIEL and having
Core care for it would be great.
Regarding avoiding reflection, why do we usually read the values from
the entity anyway as we have all values
already read out in the array :D ? Good to open a far-future improvement issue?
Sanne
2011/1/3 Emmanuel Bernard <emmanuel(a)hibernate.org>:
Hey guys to warm up in 2011, I've reviewed HSEARCH-361.
I have two remarks
Can you add a unit test that checks mutable properties. I think it's covered but
let's plan for a later bug :)
FullTextIndexEventListener compute the list of dirty properties by calling
EntityPersister.findDirty(). I don't think that's entirely accurate. If you look
when findDirty is called in Core, you'll see that we need to handle interceptors. Also
in some cases we call findModified.
The good news is that core computes and caches the list of dirty properties (in
FlushEntityEvent.getDirtyProperties()). Now the even we receive in Hibernate Search is
PostUpdateEvent. I think we can enhance Core to pass along the list of dirty properties.
EntityUpdateAction.postUpdate / postCommitUpdate do create PostUpdateEvents and has the
info (dirtyFields).
Basically we could keep the existing half working impl in HSearch until Hibernate Core
3.6.1 and use the new exposed property of PostUpdateEvent if available (reflection call).
To minimize the reflection call, we should ahve a strategy (ie pre 3.6.1 and post 3.6.1)
and at startup time instantiate the correct one.
WDYT?
_______________________________________________
hibernate-dev mailing list
hibernate-dev(a)lists.jboss.org
https://lists.jboss.org/mailman/listinfo/hibernate-dev
6:41 p.m.
On 3 janv. 2011, at 17:42, Sanne Grinovero wrote:
Regarding avoiding reflection, why do we usually read the values from
the entity anyway as we have all values
already read out in the array :D ? Good to open a far-future improvement issue?
I'm talking about supporting Core 3.6.0 and 3.6.1 to avoid matrix compatibility
headaches to people.
4862
days inactive
4862
days old
2 comments
2 participants
participants (2)
-
Emmanuel Bernard -
Sanne Grinovero