Author: sannegrinovero Date: 2009-04-22 02:52:05 -0400 (Wed, 22 Apr 2009) New Revision: 16392 Added: search/branches/Branch_3_1/src/test/org/hibernate/search/test/engine/EventListenerSerializationTest.java Removed: search/branches/Branch_3_1/src/java/org/hibernate/search/event/IndexWorkFlushEventListener.java Modified: search/branches/Branch_3_1/doc/reference/en/modules/configuration.xml search/branches/Branch_3_1/src/java/org/hibernate/search/backend/impl/EventSourceTransactionContext.java search/branches/Branch_3_1/src/java/org/hibernate/search/backend/impl/PostTransactionWorkQueueSynchronization.java search/branches/Branch_3_1/src/java/org/hibernate/search/backend/impl/TransactionalWorker.java search/branches/Branch_3_1/src/java/org/hibernate/search/event/EventListenerRegister.java search/branches/Branch_3_1/src/java/org/hibernate/search/event/FullTextIndexEventListener.java search/branches/Branch_3_1/src/test/org/hibernate/search/test/TestCase.java Log: merging changes for HSEARCH-178 from trunk to branch 3.1 Modified: search/branches/Branch_3_1/doc/reference/en/modules/configuration.xml =================================================================== --- search/branches/Branch_3_1/doc/reference/en/modules/configuration.xml 2009-04-21 20:24:58 UTC (rev 16391) +++ search/branches/Branch_3_1/doc/reference/en/modules/configuration.xml 2009-04-22 06:52:05 UTC (rev 16392) @@ -600,8 +600,8 @@ <para>To enable Hibernate Search in Hibernate Core (ie. if you don't use Hibernate Annotations), add the <literal>FullTextIndexEventListener</literal> for the following six - Hibernate events and add the <literal>IndexWorkFlushEventListener</literal> after - the default <literal>DefaultFlushEventListener</literal>, as in the following example.</para> + Hibernate events and also add it after the default + <literal>DefaultFlushEventListener</literal>, as in the following example.</para> <example> <title>Explicitly enabling Hibernate Search by configuring the @@ -630,7 +630,7 @@ &lt;/event&gt; &lt;event type="flush"&gt; &lt;listener class="org.hibernate.event.def.DefaultFlushEventListener"/&gt; - &lt;listener class="org.hibernate.search.event.IndexWorkFlushEventListener"/&gt; + &lt;listener class="org.hibernate.search.event.FullTextIndexEventListener"/&gt; &lt;/event&gt; &lt;/session-factory&gt; &lt;/hibernate-configuration&gt;</programlisting> Modified: search/branches/Branch_3_1/src/java/org/hibernate/search/backend/impl/EventSourceTransactionContext.java =================================================================== --- search/branches/Branch_3_1/src/java/org/hibernate/search/backend/impl/EventSourceTransactionContext.java 2009-04-21 20:24:58 UTC (rev 16391) +++ search/branches/Branch_3_1/src/java/org/hibernate/search/backend/impl/EventSourceTransactionContext.java 2009-04-22 06:52:05 UTC (rev 16392) @@ -5,12 +5,12 @@ import javax.transaction.Synchronization; -import org.hibernate.AssertionFailure; import org.hibernate.Transaction; import org.hibernate.event.EventSource; import org.hibernate.event.FlushEventListener; +import org.hibernate.search.SearchException; import org.hibernate.search.backend.TransactionContext; -import org.hibernate.search.event.IndexWorkFlushEventListener; +import org.hibernate.search.event.FullTextIndexEventListener; import org.hibernate.search.util.LoggerFactory; import org.slf4j.Logger; @@ -26,7 +26,9 @@ private static final Logger log = LoggerFactory.make(); private final EventSource eventSource; - private final IndexWorkFlushEventListener flushListener; + + //this transient is required to break recursive serialization + private transient FullTextIndexEventListener flushListener; //constructor time is too early to define the value of realTxInProgress, //postpone it, otherwise doing @@ -36,7 +38,7 @@ public EventSourceTransactionContext(EventSource eventSource) { this.eventSource = eventSource; - this.flushListener = findIndexWorkFlushEventListener(); + this.flushListener = getIndexWorkFlushEventListener(); } public Object getTransactionIdentifier() { @@ -54,25 +56,32 @@ transaction.registerSynchronization( synchronization ); } else { + //registerSynchronization is only called if isRealTransactionInProgress or if + // a flushListener was found; still we might need to find the listener again + // as it might have been cleared by serialization (is transient). + flushListener = getIndexWorkFlushEventListener(); if ( flushListener != null ) { flushListener.addSynchronization( eventSource, synchronization ); } else { - //It appears we are flushing out of transaction and have no way to perform the index update - //Not expected: see check in isTransactionInProgress() - throw new AssertionFailure( "On flush out of transaction: IndexWorkFlushEventListener not registered" ); + //shouldn't happen if the code about serialization is fine: + throw new SearchException( "AssertionFailure: flushListener not registered any more."); } } } - private IndexWorkFlushEventListener findIndexWorkFlushEventListener() { + private FullTextIndexEventListener getIndexWorkFlushEventListener() { + if ( this.flushListener != null) { + //for the "transient" case: might have been nullified. + return flushListener; + } FlushEventListener[] flushEventListeners = eventSource.getListeners().getFlushEventListeners(); for (FlushEventListener listener : flushEventListeners) { - if ( listener.getClass().equals( IndexWorkFlushEventListener.class ) ) { - return (IndexWorkFlushEventListener) listener; + if ( listener.getClass().equals( FullTextIndexEventListener.class ) ) { + return (FullTextIndexEventListener) listener; } } - log.debug( "No IndexWorkFlushEventListener was registered" ); + log.debug( "No FullTextIndexEventListener was registered" ); return null; } @@ -81,7 +90,7 @@ //This is because we want to behave as "inTransaction" if the flushListener is registered. public boolean isTransactionInProgress() { // either it is a real transaction, or if we are capable to manage this in the IndexWorkFlushEventListener - return isRealTransactionInProgress() || flushListener != null; + return getIndexWorkFlushEventListener() != null || isRealTransactionInProgress(); } private boolean isRealTransactionInProgress() { Modified: search/branches/Branch_3_1/src/java/org/hibernate/search/backend/impl/PostTransactionWorkQueueSynchronization.java =================================================================== --- search/branches/Branch_3_1/src/java/org/hibernate/search/backend/impl/PostTransactionWorkQueueSynchronization.java 2009-04-21 20:24:58 UTC (rev 16391) +++ search/branches/Branch_3_1/src/java/org/hibernate/search/backend/impl/PostTransactionWorkQueueSynchronization.java 2009-04-22 06:52:05 UTC (rev 16392) @@ -15,6 +15,13 @@ * @author Emmanuel Bernard */ public class PostTransactionWorkQueueSynchronization implements Synchronization { + + /** + * FullTextIndexEventListener is using a WeakIdentityHashMap<Session,Synchronization> + * So make sure all Synchronization implementations don't have any + * (direct or indirect) reference to the Session. + */ + private final QueueingProcessor queueingProcessor; private boolean consumed; private final WeakIdentityHashMap queuePerTransaction; Modified: search/branches/Branch_3_1/src/java/org/hibernate/search/backend/impl/TransactionalWorker.java =================================================================== --- search/branches/Branch_3_1/src/java/org/hibernate/search/backend/impl/TransactionalWorker.java 2009-04-21 20:24:58 UTC (rev 16391) +++ search/branches/Branch_3_1/src/java/org/hibernate/search/backend/impl/TransactionalWorker.java 2009-04-22 06:52:05 UTC (rev 16392) @@ -30,8 +30,8 @@ private static final Logger log = LoggerFactory.make(); - //FIXME: discuss the next line! it looks like there actually is concurrent access - //not a synchronized map since for a given transaction, we have not concurrent access + //this is being used from different threads, but doesn't need a + //synchronized map since for a given transaction, we have not concurrent access protected final WeakIdentityHashMap<Object, Synchronization> synchronizationPerTransaction = new WeakIdentityHashMap<Object, Synchronization>(); private QueueingProcessor queueingProcessor; Modified: search/branches/Branch_3_1/src/java/org/hibernate/search/event/EventListenerRegister.java =================================================================== --- search/branches/Branch_3_1/src/java/org/hibernate/search/event/EventListenerRegister.java 2009-04-21 20:24:58 UTC (rev 16391) +++ search/branches/Branch_3_1/src/java/org/hibernate/search/event/EventListenerRegister.java 2009-04-22 06:52:05 UTC (rev 16392) @@ -6,6 +6,7 @@ import org.slf4j.Logger; import org.hibernate.event.EventListeners; +import org.hibernate.event.FlushEventListener; import org.hibernate.event.PostCollectionRecreateEventListener; import org.hibernate.event.PostCollectionRemoveEventListener; import org.hibernate.event.PostCollectionUpdateEventListener; @@ -15,7 +16,6 @@ import org.hibernate.search.Environment; import org.hibernate.search.util.LoggerFactory; - /** * Helper methods initializing Hibernate Search event listeners. * @@ -94,13 +94,14 @@ new PostCollectionUpdateEventListener[] { searchListener } ) ); - // Adding IndexWorkFlushEventListener to manage events out-of-transaction - if ( ! isFlushEventListenerRegistered( listeners.getFlushEventListeners() ) ) { - listeners.setFlushEventListeners( appendToArray( - listeners.getFlushEventListeners(), - new IndexWorkFlushEventListener() - ) ); - } + // Adding also as FlushEventListener to manage events out-of-transaction + listeners.setFlushEventListeners( + addIfNeeded( + listeners.getFlushEventListeners(), + searchListener, + new FlushEventListener[] { searchListener } + ) + ); } /** @@ -166,18 +167,4 @@ return false; } - /** - * Verifies if an IndexWorkFlushEventListener is contained in the array of listeners. - * @param listeners - * @return true if it found in the listeners, false otherwise. - */ - private static boolean isFlushEventListenerRegistered(Object[] listeners) { - for ( Object eventListener : listeners ) { - if ( IndexWorkFlushEventListener.class == eventListener.getClass() ) { - return true; - } - } - return false; - } - } Modified: search/branches/Branch_3_1/src/java/org/hibernate/search/event/FullTextIndexEventListener.java =================================================================== --- search/branches/Branch_3_1/src/java/org/hibernate/search/event/FullTextIndexEventListener.java 2009-04-21 20:24:58 UTC (rev 16391) +++ search/branches/Branch_3_1/src/java/org/hibernate/search/event/FullTextIndexEventListener.java 2009-04-22 06:52:05 UTC (rev 16392) @@ -1,15 +1,27 @@ //$Id$ package org.hibernate.search.event; +import java.io.IOException; +import java.io.ObjectInputStream; +import java.io.ObjectOutputStream; import java.io.Serializable; +import java.lang.reflect.Field; +import java.util.Map; +import javax.transaction.Status; +import javax.transaction.Synchronization; + import org.slf4j.Logger; +import org.hibernate.Session; import org.hibernate.cfg.Configuration; import org.hibernate.engine.EntityEntry; import org.hibernate.event.AbstractCollectionEvent; import org.hibernate.event.AbstractEvent; import org.hibernate.event.Destructible; +import org.hibernate.event.EventSource; +import org.hibernate.event.FlushEvent; +import org.hibernate.event.FlushEventListener; import org.hibernate.event.Initializable; import org.hibernate.event.PostCollectionRecreateEvent; import org.hibernate.event.PostCollectionRecreateEventListener; @@ -28,6 +40,7 @@ import org.hibernate.search.backend.impl.EventSourceTransactionContext; import org.hibernate.search.engine.SearchFactoryImplementor; import org.hibernate.search.util.LoggerFactory; +import org.hibernate.search.util.WeakIdentityHashMap; /** * This listener supports setting a parent directory for all generated index files. @@ -36,20 +49,27 @@ * @author Gavin King * @author Emmanuel Bernard * @author Mattias Arbin + * @author Sanne Grinovero */ -//TODO work on sharing the same indexWriters and readers across a single post operation... //TODO implement and use a LockableDirectoryProvider that wraps a DP to handle the lock inside the LDP //TODO make this class final as soon as FullTextIndexCollectionEventListener is removed. @SuppressWarnings( "serial" ) public class FullTextIndexEventListener implements PostDeleteEventListener, PostInsertEventListener, PostUpdateEventListener, PostCollectionRecreateEventListener, PostCollectionRemoveEventListener, - PostCollectionUpdateEventListener, Initializable, Destructible { + PostCollectionUpdateEventListener, FlushEventListener, Initializable, Destructible { private static final Logger log = LoggerFactory.make(); protected boolean used; protected SearchFactoryImplementor searchFactoryImplementor; + + //only used by the FullTextIndexEventListener instance playing in the FlushEventListener role. + // transient because it's not serializable (and state doesn't need to live longer than a flush). + // final because it's initialization should be published to other threads. + // ! update the readObject() method in case of name changes ! + // make sure the Synchronization doesn't contain references to Session, otherwise we'll leak memory. + private transient final Map<Session,Synchronization> flushSynch = new WeakIdentityHashMap<Session,Synchronization>(0); /** * Initialize method called by Hibernate Core when the SessionFactory starts @@ -157,4 +177,59 @@ } return id; } + + /** + * Make sure the indexes are updated right after the hibernate flush, + * avoiding object loading during a flush. Not needed during transactions. + */ + public void onFlush(FlushEvent event) { + if ( used ) { + Session session = event.getSession(); + Synchronization synchronization = flushSynch.get( session ); + if ( synchronization != null ) { + //first cleanup + flushSynch.remove( session ); + log.debug( "flush event causing index update out of transaction" ); + synchronization.beforeCompletion(); + synchronization.afterCompletion( Status.STATUS_COMMITTED ); + } + } + } + + /** + * Adds a synchronization to be performed in the onFlush method; + * should only be used as workaround for the case a flush is happening + * out of transaction. + * Warning: if the synchronization contains a hard reference + * to the Session proper cleanup is not guaranteed and memory leaks + * will happen. + * @param eventSource should be the Session doing the flush + * @param synchronization + */ + public void addSynchronization(EventSource eventSource, Synchronization synchronization) { + this.flushSynch.put( eventSource, synchronization ); + } + + /* Might want to implement AutoFlushEventListener in future? + public void onAutoFlush(AutoFlushEvent event) throws HibernateException { + // Currently not needed as auto-flush is not happening + // when out of transaction. + } + */ + + private void writeObject(ObjectOutputStream os) throws IOException { + os.defaultWriteObject(); + } + + //needs to implement custom readObject to restore the transient fields + private void readObject(ObjectInputStream is) throws IOException, ClassNotFoundException, SecurityException, NoSuchFieldException, IllegalArgumentException, IllegalAccessException { + is.defaultReadObject(); + Class<FullTextIndexEventListener> cl = FullTextIndexEventListener.class; + Field f = cl.getDeclaredField("flushSynch"); + f.setAccessible( true ); + Map<Session,Synchronization> flushSynch = new WeakIdentityHashMap<Session,Synchronization>(0); + // setting a final field by reflection during a readObject is considered as safe as in a constructor: + f.set( this, flushSynch ); + } + } Deleted: search/branches/Branch_3_1/src/java/org/hibernate/search/event/IndexWorkFlushEventListener.java =================================================================== --- search/branches/Branch_3_1/src/java/org/hibernate/search/event/IndexWorkFlushEventListener.java 2009-04-21 20:24:58 UTC (rev 16391) +++ search/branches/Branch_3_1/src/java/org/hibernate/search/event/IndexWorkFlushEventListener.java 2009-04-22 06:52:05 UTC (rev 16392) @@ -1,61 +0,0 @@ -// $Id$ -package org.hibernate.search.event; - -import java.io.Serializable; -import java.util.concurrent.ConcurrentHashMap; - -import javax.transaction.Status; -import javax.transaction.Synchronization; - -import org.hibernate.AssertionFailure; -import org.hibernate.HibernateException; -import org.hibernate.Session; -import org.hibernate.event.EventSource; -import org.hibernate.event.FlushEvent; -import org.hibernate.event.FlushEventListener; -import org.hibernate.search.util.LoggerFactory; -import org.slf4j.Logger; - -/** - * FlushEventListener to make sure the indexes are updated right after the hibernate flush, - * avoiding object loading during a flush. Not needed during transactions. - * - * @author Sanne Grinovero - */ -public final class IndexWorkFlushEventListener implements FlushEventListener, Serializable { - - private static final Logger log = LoggerFactory.make(); - - private final ConcurrentHashMap<Session, Synchronization> synchronizationPerTransaction - = new ConcurrentHashMap<Session, Synchronization>(); - - public IndexWorkFlushEventListener() { - } - - public void onFlush(FlushEvent event) throws HibernateException { - Session session = event.getSession(); - Synchronization synchronization = synchronizationPerTransaction.get( session ); - if ( synchronization != null ) { - log.debug( "flush event causing index update out of transaction" ); - synchronizationPerTransaction.remove( session ); - synchronization.beforeCompletion(); - synchronization.afterCompletion( Status.STATUS_COMMITTED ); - } - } - - public void addSynchronization(EventSource eventSource, Synchronization synchronization) { - Synchronization previousSync = synchronizationPerTransaction.put( eventSource, synchronization ); - if ( previousSync != null ) { - throw new AssertionFailure( "previous registered sync not discarded in IndexWorkFlushEventListener" ); - } - } - - /* - * Might want to implement AutoFlushEventListener in future? - public void onAutoFlush(AutoFlushEvent event) throws HibernateException { - // Currently not needed as auto-flush is not happening - // when out of transaction. - } - */ - -} Modified: search/branches/Branch_3_1/src/test/org/hibernate/search/test/TestCase.java =================================================================== --- search/branches/Branch_3_1/src/test/org/hibernate/search/test/TestCase.java 2009-04-21 20:24:58 UTC (rev 16391) +++ search/branches/Branch_3_1/src/test/org/hibernate/search/test/TestCase.java 2009-04-22 06:52:05 UTC (rev 16392) @@ -15,7 +15,7 @@ import org.hibernate.dialect.Dialect; import org.hibernate.event.FlushEventListener; import org.hibernate.event.def.DefaultFlushEventListener; -import org.hibernate.search.event.IndexWorkFlushEventListener; +import org.hibernate.search.event.FullTextIndexEventListener; /** * A modified base class for tests without annotations. @@ -154,7 +154,7 @@ cfg.setListener( "post-collection-remove", "org.hibernate.search.event.FullTextIndexEventListener" ); cfg.setListener( "post-collection-update", "org.hibernate.search.event.FullTextIndexEventListener" ); - cfg.setListeners( "flush", new FlushEventListener[]{new DefaultFlushEventListener(), new IndexWorkFlushEventListener()} ); + cfg.setListeners( "flush", new FlushEventListener[]{new DefaultFlushEventListener(), new FullTextIndexEventListener()} ); cfg.setProperty( "hibernate.search.default.directory_provider", RAMDirectoryProvider.class.getName() ); cfg.setProperty( org.hibernate.search.Environment.ANALYZER_CLASS, StopAnalyzer.class.getName() ); Copied: search/branches/Branch_3_1/src/test/org/hibernate/search/test/engine/EventListenerSerializationTest.java (from rev 16306, search/trunk/src/test/java/org/hibernate/search/test/engine/EventListenerSerializationTest.java) =================================================================== --- search/branches/Branch_3_1/src/test/org/hibernate/search/test/engine/EventListenerSerializationTest.java (rev 0) +++ search/branches/Branch_3_1/src/test/org/hibernate/search/test/engine/EventListenerSerializationTest.java 2009-04-22 06:52:05 UTC (rev 16392) @@ -0,0 +1,26 @@ +package org.hibernate.search.test.engine; + +import java.io.IOException; + +import junit.framework.TestCase; + +import org.hibernate.search.event.FullTextIndexEventListener; +import org.hibernate.search.test.SerializationTestHelper; + +/** + * Tests that the FullTextIndexEventListener is Serializable + * + * @author Sanne Grinovero + */ +public class EventListenerSerializationTest extends TestCase { + + public void testEventListenerSerializable() throws IOException, ClassNotFoundException { + FullTextIndexEventListener eventListener = new FullTextIndexEventListener(); + eventListener.addSynchronization( null, null ); + Object secondListener = SerializationTestHelper + .duplicateBySerialization(eventListener); + assertNotNull(secondListener); + assertFalse(secondListener == eventListener); + } + +}