Skip to content

Commit

Permalink
HHH-13831 Refresh listeners when one is replaced
Browse files Browse the repository at this point in the history
  • Loading branch information
@sebersole
sebersole committed Jan 30, 2020
1 parent 739ca86 commit 2f86c49
Show file tree
Hide file tree
Showing 2 changed files with 118 additions and 95 deletions.
211 changes: 117 additions & 94 deletions ...rnate-core/src/main/java/org/hibernate/event/service/internal/EventListenerGroupImpl.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,14 +7,12 @@
package org.hibernate.event.service.internal;

import java.lang.reflect.Array;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
import java.util.LinkedHashSet;
import java.util.List;
import java.util.ListIterator;
import java.util.Map;
import java.util.Set;
import java.util.function.BiConsumer;
import java.util.function.Consumer;
import java.util.function.Supplier;

import org.hibernate.event.service.spi.DuplicationStrategy;
Expand All @@ -25,25 +23,22 @@
import org.hibernate.event.spi.EventType;
import org.hibernate.jpa.event.spi.CallbackRegistryConsumer;

import org.jboss.logging.Logger;

/**
* @author Steve Ebersole
* @author Sanne Grinovero
*/
class EventListenerGroupImpl<T> implements EventListenerGroup<T> {
private EventType<T> eventType;
private static final Logger log = Logger.getLogger( EventListenerGroupImpl.class );

private final EventType<T> eventType;
private final EventListenerRegistryImpl listenerRegistry;

private final Set<DuplicationStrategy> duplicationStrategies = new LinkedHashSet<>();

// Performance: make sure iteration on this type is efficient; in particular we do not want to allocate iterators,
// not having to capture state in lambdas.
// So we keep the listeners in both a List (for convenience) and in an array (for iteration). Make sure
// their content stays in synch!
private T[] listeners = null;

//Update both fields when making changes!
private List<T> listenersAsList;

public EventListenerGroupImpl(EventType<T> eventType, EventListenerRegistryImpl listenerRegistry) {
this.eventType = eventType;
this.listenerRegistry = listenerRegistry;
Expand Down Expand Up @@ -82,20 +77,18 @@ public int count() {

@Override
public void clear() {
if ( duplicationStrategies != null ) {
duplicationStrategies.clear();
}
duplicationStrategies.clear();
listeners = null;
listenersAsList = null;
}

@Override
public final <U> void fireLazyEventOnEachListener(final Supplier<U> eventSupplier, final BiConsumer<T,U> actionOnEvent) {
final T[] ls = listeners;
if ( ls != null && ls.length != 0 ) {
final U event = eventSupplier.get();
for ( T listener : ls ) {
actionOnEvent.accept( listener, event );
//noinspection ForLoopReplaceableByForEach
for ( int i = 0; i < ls.length; i++ ) {
actionOnEvent.accept( ls[i], event );
}
}
}
Expand All @@ -104,8 +97,9 @@ public final <U> void fireLazyEventOnEachListener(final Supplier<U> eventSupplie
public final <U> void fireEventOnEachListener(final U event, final BiConsumer<T,U> actionOnEvent) {
final T[] ls = listeners;
if ( ls != null ) {
for ( T listener : ls ) {
actionOnEvent.accept( listener, event );
//noinspection ForLoopReplaceableByForEach
for ( int i = 0; i < ls.length; i++ ) {
actionOnEvent.accept( ls[i], event );
}
}
}
Expand All @@ -114,8 +108,9 @@ public final <U> void fireEventOnEachListener(final U event, final BiConsumer<T,
public <U,X> void fireEventOnEachListener(final U event, final X parameter, final EventActionWithParameter<T, U, X> actionOnEvent) {
final T[] ls = listeners;
if ( ls != null ) {
for ( T listener : ls ) {
actionOnEvent.applyEventToListener( listener, event, parameter );
//noinspection ForLoopReplaceableByForEach
for ( int i = 0; i < ls.length; i++ ) {
actionOnEvent.applyEventToListener( ls[i], event, parameter );
}
}
}
Expand All @@ -125,119 +120,137 @@ public void addDuplicationStrategy(DuplicationStrategy strategy) {
duplicationStrategies.add( strategy );
}

/**
* Implementation note: should be final for performance reasons.
* @deprecated this is not the most efficient way for iterating the event listeners.
* See {@link #fireEventOnEachListener(Object, BiConsumer)} and co. for better alternatives.
*/
@Override
@Deprecated
public final Iterable<T> listeners() {
final List<T> ls = listenersAsList;
return ls == null ? Collections.EMPTY_LIST : ls;
public void appendListener(T listener) {
handleListenerAddition( listener, this::internalAppend );
}

@Override
@SafeVarargs
public final void appendListeners(T... listeners) {
internalAppendListeners( listeners );
checkForArrayRefresh();
//noinspection ForLoopReplaceableByForEach
for ( int i = 0; i < listeners.length; i++ ) {
handleListenerAddition( listeners[i], this::internalAppend );
}
}

private void checkForArrayRefresh() {
final List<T> list = listenersAsList;
if ( this.listeners == null ) {
T[] a = (T[]) Array.newInstance( eventType.baseListenerInterface(), list.size() );
listeners = list.<T>toArray( a );
private void internalAppend(T listener) {
prepareListener( listener );

if ( listeners == null ) {
//noinspection unchecked
listeners = (T[]) Array.newInstance( eventType.baseListenerInterface(), 1 );
listeners[0] = listener;
}
}
else {
final int size = listeners.length;

//noinspection unchecked
final T[] newCopy = (T[]) Array.newInstance( eventType.baseListenerInterface(), size+1 );

private void internalAppendListeners(T[] listeners) {
for ( T listener : listeners ) {
internalAppendListener( listener );
// put the new one first
newCopy[0] = listener;

// and copy the rest after it
System.arraycopy( listeners, 0, newCopy, 1, size );
}
}

@Override
public void appendListener(T listener) {
internalAppendListener( listener );
checkForArrayRefresh();
}

private void internalAppendListener(T listener) {
if ( listenerShouldGetAdded( listener ) ) {
internalAppend( listener );
}
@Override
public void prependListener(T listener) {
handleListenerAddition( listener, this::internalPrepend );
}

@Override
@SafeVarargs
public final void prependListeners(T... listeners) {
internalPrependListeners( listeners );
checkForArrayRefresh();
//noinspection ForLoopReplaceableByForEach
for ( int i = 0; i < listeners.length; i++ ) {
handleListenerAddition( listeners[i], this::internalPrepend );
}
}

private void internalPrependListeners(T[] listeners) {
for ( T listener : listeners ) {
internalPreprendListener( listener );
private void internalPrepend(T listener) {
prepareListener( listener );

if ( listeners == null ) {
//noinspection unchecked
listeners = (T[]) Array.newInstance( eventType.baseListenerInterface(), 1 );
listeners[0] = listener;
}
}
else {
final int size = listeners.length;

@Override
public void prependListener(T listener) {
internalPreprendListener( listener );
checkForArrayRefresh();
}
//noinspection unchecked
final T[] newCopy = (T[]) Array.newInstance( eventType.baseListenerInterface(), size+1 );

private void internalPreprendListener(T listener) {
if ( listenerShouldGetAdded( listener ) ) {
internalPrepend( listener );
// first copy the existing listeners
System.arraycopy( listeners, 0, newCopy, 0, size );

// and then put the new one after them
newCopy[size] = listener;
}
}

private boolean listenerShouldGetAdded(T listener) {
final List<T> ts = listenersAsList;
if ( ts == null ) {
listenersAsList = new ArrayList<>();
return true;
// no need to do de-dup checks
private void handleListenerAddition(T listener, Consumer<T> additionHandler) {
if ( listeners == null ) {
additionHandler.accept( listener );
return;
}

boolean doAdd = true;
strategy_loop: for ( DuplicationStrategy strategy : duplicationStrategies ) {
final ListIterator<T> itr = ts.listIterator();
while ( itr.hasNext() ) {
final T existingListener = itr.next();
final T[] localListenersRef = this.listeners;

for ( DuplicationStrategy strategy : duplicationStrategies ) {

// for each strategy, see if the strategy indicates that any of the existing
// listeners match the listener being added. If so, we want to apply that
// strategy's action. Control it returned immediately after applying the action
// on match - meaning no further strategies are checked...

for ( int i = 0; i < localListenersRef.length; i++ ) {
final T existingListener = localListenersRef[i];
log.debugf(
"Checking incoming listener [`%s`] for match against existing listener [`%s`]",
listener,
existingListener
);

if ( strategy.areMatch( listener, existingListener ) ) {
log.debugf( "Found listener match between `%s` and `%s`", listener, existingListener );

switch ( strategy.getAction() ) {
// todo : add debug logging of what happens here...
case ERROR: {
throw new EventListenerRegistrationException( "Duplicate event listener found" );
}
case KEEP_ORIGINAL: {
doAdd = false;
break strategy_loop;
log.debugf( "Skipping listener registration (%s) : `%s`", strategy.getAction(), listener );
return;
}
case REPLACE_ORIGINAL: {
checkAgainstBaseInterface( listener );
performInjections( listener );
itr.set( listener );
listeners = null; //Marks it for refreshing
doAdd = false;
break strategy_loop;
log.debugf( "Replacing listener registration (%s) : `%s` -> ", strategy.getAction(), existingListener, listener );
prepareListener( listener );

listeners[i] = listener;
}
}

// we've found a match - we should return
// - the match action has already been applied at this point
return;
}
}
}
return doAdd;

// we did not find any match.. add it
checkAgainstBaseInterface( listener );
performInjections( listener );
additionHandler.accept( listener );
}

private void internalPrepend(T listener) {
private void prepareListener(T listener) {
checkAgainstBaseInterface( listener );
performInjections( listener );
listenersAsList.add( 0, listener );
listeners = null; //Marks it for refreshing
}

private void performInjections(T listener) {
Expand All @@ -260,10 +273,20 @@ private void checkAgainstBaseInterface(T listener) {
}
}

private void internalAppend(T listener) {
checkAgainstBaseInterface( listener );
performInjections( listener );
listenersAsList.add( listener );
listeners = null; //Marks it for refreshing

/**
* Implementation note: should be final for performance reasons.
* @deprecated this is not the most efficient way for iterating the event listeners.
* See {@link #fireEventOnEachListener(Object, BiConsumer)} and co. for better alternatives.
*/
@Override
@Deprecated
public final Iterable<T> listeners() {
if ( listeners == null ) {
//noinspection unchecked
return Collections.EMPTY_LIST;
}

return Arrays.asList( listeners );
}
}
2 changes: 1 addition & 1 deletion .../test/java/org/hibernate/event/service/internal/EventListenerReplacementStrategyTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,7 @@ private static class ReplaceOriginalStrategy implements DuplicationStrategy {

@Override
public boolean areMatch(Object listener, Object original) {
return original instanceof ClearEvent && listener instanceof ClearEvent;
return original instanceof ClearEventListener && listener instanceof ClearEventListener;
}

@Override
Expand Down

0 comments on commit 2f86c49

Please sign in to comment.