Author: sannegrinovero Date: 2009-04-13 11:26:20 -0400 (Mon, 13 Apr 2009) New Revision: 16306 Added: search/trunk/src/test/java/org/hibernate/search/test/engine/EventListenerSerializationTest.java Removed: search/trunk/src/main/java/org/hibernate/search/event/IndexWorkFlushEventListener.java Modified: search/trunk/src/main/docbook/en-US/modules/configuration.xml search/trunk/src/main/java/ search/trunk/src/main/java/org/hibernate/search/backend/impl/EventSourceTransactionContext.java search/trunk/src/main/java/org/hibernate/search/backend/impl/TransactionalWorker.java search/trunk/src/main/java/org/hibernate/search/event/EventListenerRegister.java search/trunk/src/main/java/org/hibernate/search/event/FullTextIndexEventListener.java search/trunk/src/test/java/org/hibernate/search/test/TestCase.java Log: HSEARCH-178 for trunk only Modified: search/trunk/src/main/docbook/en-US/modules/configuration.xml =================================================================== --- search/trunk/src/main/docbook/en-US/modules/configuration.xml 2009-04-11 04:54:37 UTC (rev 16305) +++ search/trunk/src/main/docbook/en-US/modules/configuration.xml 2009-04-13 15:26:20 UTC (rev 16306) @@ -605,8 +605,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 @@ -635,7 +635,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> Property changes on: search/trunk/src/main/java ___________________________________________________________________ Name: svn:mergeinfo - Modified: search/trunk/src/main/java/org/hibernate/search/backend/impl/EventSourceTransactionContext.java =================================================================== --- search/trunk/src/main/java/org/hibernate/search/backend/impl/EventSourceTransactionContext.java 2009-04-11 04:54:37 UTC (rev 16305) +++ search/trunk/src/main/java/org/hibernate/search/backend/impl/EventSourceTransactionContext.java 2009-04-13 15:26:20 UTC (rev 16306) @@ -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 { - if ( flushListener != null ) { + //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). + FullTextIndexEventListener flushList = getIndexWorkFlushEventListener(); + if ( flushList != 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/trunk/src/main/java/org/hibernate/search/backend/impl/TransactionalWorker.java =================================================================== --- search/trunk/src/main/java/org/hibernate/search/backend/impl/TransactionalWorker.java 2009-04-11 04:54:37 UTC (rev 16305) +++ search/trunk/src/main/java/org/hibernate/search/backend/impl/TransactionalWorker.java 2009-04-13 15:26:20 UTC (rev 16306) @@ -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/trunk/src/main/java/org/hibernate/search/event/EventListenerRegister.java =================================================================== --- search/trunk/src/main/java/org/hibernate/search/event/EventListenerRegister.java 2009-04-11 04:54:37 UTC (rev 16305) +++ search/trunk/src/main/java/org/hibernate/search/event/EventListenerRegister.java 2009-04-13 15:26:20 UTC (rev 16306) @@ -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/trunk/src/main/java/org/hibernate/search/event/FullTextIndexEventListener.java =================================================================== --- search/trunk/src/main/java/org/hibernate/search/event/FullTextIndexEventListener.java 2009-04-11 04:54:37 UTC (rev 16305) +++ search/trunk/src/main/java/org/hibernate/search/event/FullTextIndexEventListener.java 2009-04-13 15:26:20 UTC (rev 16306) @@ -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.ref.WeakReference; +import java.lang.reflect.Field; +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; @@ -36,20 +48,29 @@ * @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. + // T.Local because different threads could be flushing on this listener, still the reference + // to session should be weak so that sessions which had errors on flush can be discarded. + // ! update the readObject() method in case of name changes ! + // It's not static as we couldn't properly cleanup otherwise. + private transient final ThreadLocal<FlushContextContainer> flushSynch = new ThreadLocal<FlushContextContainer>(); /** * Initialize method called by Hibernate Core when the SessionFactory starts @@ -157,4 +178,70 @@ } 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(); + FlushContextContainer flushContextContainer = this.flushSynch.get(); + if ( flushContextContainer != null ) { + //first cleanup the ThreadLocal + this.flushSynch.set( null ); + EventSource registeringEventSource = flushContextContainer.eventSource.get(); + //check that we are still in the same session which registered the flushSync: + if ( registeringEventSource != null && registeringEventSource == session ) { + log.debug( "flush event causing index update out of transaction" ); + Synchronization synchronization = flushContextContainer.synchronization; + synchronization.beforeCompletion(); + synchronization.afterCompletion( Status.STATUS_COMMITTED ); + } + } + } + } + + public void addSynchronization(EventSource eventSource, Synchronization synchronization) { + //no need to check for "unused" state, as this method is used by Search itself only. + FlushContextContainer flushContext = new FlushContextContainer(eventSource, synchronization); + //ignoring previously set data: if there was something, it's coming from a previous thread + //which had some error when flushing and couldn't cleanup. + this.flushSynch.set( flushContext ); + } + + /* 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 static class FlushContextContainer { + + private final WeakReference<EventSource> eventSource; + private final Synchronization synchronization; + + public FlushContextContainer(EventSource eventSource, Synchronization synchronization) { + this.eventSource = new WeakReference<EventSource>( eventSource ); + this.synchronization = synchronization; + } + + } + + 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 ); + ThreadLocal<FlushContextContainer> flushSynch = new ThreadLocal<FlushContextContainer>(); + // setting a final field by reflection during a readObject is considered as safe as in a constructor: + f.set( this, flushSynch ); + } + } Deleted: search/trunk/src/main/java/org/hibernate/search/event/IndexWorkFlushEventListener.java =================================================================== --- search/trunk/src/main/java/org/hibernate/search/event/IndexWorkFlushEventListener.java 2009-04-11 04:54:37 UTC (rev 16305) +++ search/trunk/src/main/java/org/hibernate/search/event/IndexWorkFlushEventListener.java 2009-04-13 15:26:20 UTC (rev 16306) @@ -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/trunk/src/test/java/org/hibernate/search/test/TestCase.java =================================================================== --- search/trunk/src/test/java/org/hibernate/search/test/TestCase.java 2009-04-11 04:54:37 UTC (rev 16305) +++ search/trunk/src/test/java/org/hibernate/search/test/TestCase.java 2009-04-13 15:26:20 UTC (rev 16306) @@ -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() ); Added: search/trunk/src/test/java/org/hibernate/search/test/engine/EventListenerSerializationTest.java =================================================================== --- search/trunk/src/test/java/org/hibernate/search/test/engine/EventListenerSerializationTest.java (rev 0) +++ search/trunk/src/test/java/org/hibernate/search/test/engine/EventListenerSerializationTest.java 2009-04-13 15:26:20 UTC (rev 16306) @@ -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); + } + +}