Fix issue 779 -- deduplicate listeners & interceptors. Using a slightly modified patch from Tavian Barnes, thanks Tavian!
-------------
Created by MOE: http://code.google.com/p/moe-java
MOE_MIGRATED_REVID=57798796
diff --git a/core/src/com/google/inject/internal/EncounterImpl.java b/core/src/com/google/inject/internal/EncounterImpl.java
index 957743b..8aa3e94 100644
--- a/core/src/com/google/inject/internal/EncounterImpl.java
+++ b/core/src/com/google/inject/internal/EncounterImpl.java
@@ -19,6 +19,7 @@
import static com.google.common.base.Preconditions.checkState;
import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Lists;
import com.google.inject.Key;
import com.google.inject.MembersInjector;
@@ -76,16 +77,16 @@
}
/*end[AOP]*/
- ImmutableList<MembersInjector<? super T>> getMembersInjectors() {
+ ImmutableSet<MembersInjector<? super T>> getMembersInjectors() {
return membersInjectors == null
- ? ImmutableList.<MembersInjector<? super T>>of()
- : ImmutableList.copyOf(membersInjectors);
+ ? ImmutableSet.<MembersInjector<? super T>>of()
+ : ImmutableSet.copyOf(membersInjectors);
}
- ImmutableList<InjectionListener<? super T>> getInjectionListeners() {
+ ImmutableSet<InjectionListener<? super T>> getInjectionListeners() {
return injectionListeners == null
- ? ImmutableList.<InjectionListener<? super T>>of()
- : ImmutableList.copyOf(injectionListeners);
+ ? ImmutableSet.<InjectionListener<? super T>>of()
+ : ImmutableSet.copyOf(injectionListeners);
}
public void register(MembersInjector<? super T> membersInjector) {
diff --git a/core/src/com/google/inject/internal/MembersInjectorImpl.java b/core/src/com/google/inject/internal/MembersInjectorImpl.java
index ba32d28..498b441 100644
--- a/core/src/com/google/inject/internal/MembersInjectorImpl.java
+++ b/core/src/com/google/inject/internal/MembersInjectorImpl.java
@@ -34,8 +34,8 @@
private final TypeLiteral<T> typeLiteral;
private final InjectorImpl injector;
private final ImmutableList<SingleMemberInjector> memberInjectors;
- private final ImmutableList<MembersInjector<? super T>> userMembersInjectors;
- private final ImmutableList<InjectionListener<? super T>> injectionListeners;
+ private final ImmutableSet<MembersInjector<? super T>> userMembersInjectors;
+ private final ImmutableSet<InjectionListener<? super T>> injectionListeners;
/*if[AOP]*/
private final ImmutableList<MethodAspect> addedAspects;
/*end[AOP]*/
@@ -135,9 +135,7 @@
// TODO: There's no way to know if a user's MembersInjector wants toolable injections.
if(!toolableOnly) {
- // optimization: use manual for/each to save allocating an iterator here
- for (int i = 0, size = userMembersInjectors.size(); i < size; i++) {
- MembersInjector<? super T> userMembersInjector = userMembersInjectors.get(i);
+ for (MembersInjector<? super T> userMembersInjector : userMembersInjectors) {
try {
userMembersInjector.injectMembers(t);
} catch (RuntimeException e) {
diff --git a/core/src/com/google/inject/internal/MembersInjectorStore.java b/core/src/com/google/inject/internal/MembersInjectorStore.java
index 7604d4c..8e2acdd 100644
--- a/core/src/com/google/inject/internal/MembersInjectorStore.java
+++ b/core/src/com/google/inject/internal/MembersInjectorStore.java
@@ -18,9 +18,11 @@
import com.google.common.collect.ImmutableList;
import com.google.common.collect.Lists;
+import com.google.common.collect.Sets;
import com.google.inject.ConfigurationException;
import com.google.inject.TypeLiteral;
import com.google.inject.spi.InjectionPoint;
+import com.google.inject.spi.TypeListener;
import com.google.inject.spi.TypeListenerBinding;
import java.lang.reflect.Field;
@@ -97,12 +99,15 @@
errors.throwIfNewErrors(numErrorsBefore);
EncounterImpl<T> encounter = new EncounterImpl<T>(errors, injector.lookups);
- for (TypeListenerBinding typeListener : typeListenerBindings) {
- if (typeListener.getTypeMatcher().matches(type)) {
+ Set<TypeListener> alreadySeenListeners = Sets.newHashSet();
+ for (TypeListenerBinding binding : typeListenerBindings) {
+ TypeListener typeListener = binding.getListener();
+ if (!alreadySeenListeners.contains(typeListener) && binding.getTypeMatcher().matches(type)) {
+ alreadySeenListeners.add(typeListener);
try {
- typeListener.getListener().hear(type, encounter);
+ typeListener.hear(type, encounter);
} catch (RuntimeException e) {
- errors.errorNotifyingTypeListener(typeListener, type, e);
+ errors.errorNotifyingTypeListener(binding, type, e);
}
}
}
diff --git a/core/src/com/google/inject/internal/ProvisionListenerStackCallback.java b/core/src/com/google/inject/internal/ProvisionListenerStackCallback.java
index a99c513..ad292a6 100644
--- a/core/src/com/google/inject/internal/ProvisionListenerStackCallback.java
+++ b/core/src/com/google/inject/internal/ProvisionListenerStackCallback.java
@@ -17,12 +17,14 @@
package com.google.inject.internal;
import com.google.common.collect.ImmutableList;
+import com.google.common.collect.Sets;
import com.google.inject.Binding;
import com.google.inject.ProvisionException;
import com.google.inject.spi.DependencyAndSource;
import com.google.inject.spi.ProvisionListener;
import java.util.List;
+import java.util.Set;
/**
* Intercepts provisions with a stack of listeners.
@@ -49,7 +51,8 @@
if (listeners.isEmpty()) {
this.listeners = EMPTY_LISTENER;
} else {
- this.listeners = listeners.toArray(new ProvisionListener[listeners.size()]);
+ Set<ProvisionListener> deDuplicated = Sets.newLinkedHashSet(listeners);
+ this.listeners = deDuplicated.toArray(new ProvisionListener[deDuplicated.size()]);
}
}
diff --git a/core/src/com/google/inject/internal/ProxyFactory.java b/core/src/com/google/inject/internal/ProxyFactory.java
index 0b39f76..13aff81 100644
--- a/core/src/com/google/inject/internal/ProxyFactory.java
+++ b/core/src/com/google/inject/internal/ProxyFactory.java
@@ -20,6 +20,7 @@
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Lists;
import com.google.common.collect.Maps;
import com.google.inject.spi.InjectionPoint;
@@ -135,8 +136,10 @@
interceptorsMapBuilder = ImmutableMap.builder();
}
- interceptorsMapBuilder.put(pair.method, ImmutableList.copyOf(pair.interceptors));
- callbacks[i] = new InterceptorStackCallback(pair.method, pair.interceptors);
+ ImmutableList<MethodInterceptor> deDuplicated =
+ ImmutableSet.copyOf(pair.interceptors).asList();
+ interceptorsMapBuilder.put(pair.method, deDuplicated);
+ callbacks[i] = new InterceptorStackCallback(pair.method, deDuplicated);
}
interceptors = interceptorsMapBuilder != null
diff --git a/core/test/com/google/inject/MethodInterceptionTest.java b/core/test/com/google/inject/MethodInterceptionTest.java
index 6fe8b4e..3a0eef5 100644
--- a/core/test/com/google/inject/MethodInterceptionTest.java
+++ b/core/test/com/google/inject/MethodInterceptionTest.java
@@ -313,4 +313,17 @@
}
}
+ public void testDeDuplicateInterceptors() throws Exception {
+ Injector injector = Guice.createInjector(new AbstractModule() {
+ @Override protected void configure() {
+ CountingInterceptor interceptor = new CountingInterceptor();
+ bindInterceptor(Matchers.any(), Matchers.any(), interceptor);
+ bindInterceptor(Matchers.any(), Matchers.any(), interceptor);
+ }
+ });
+
+ Interceptable interceptable = injector.getInstance(Interceptable.class);
+ interceptable.foo();
+ assertEquals(1, count.get());
+ }
}
diff --git a/core/test/com/google/inject/ProvisionListenerTest.java b/core/test/com/google/inject/ProvisionListenerTest.java
index 7cf317d..a66d346 100644
--- a/core/test/com/google/inject/ProvisionListenerTest.java
+++ b/core/test/com/google/inject/ProvisionListenerTest.java
@@ -695,4 +695,17 @@
this.x = xProvider.get();
}
}
+
+ public void testDeDuplicateProvisionListeners() {
+ final Counter counter = new Counter();
+ Injector injector = Guice.createInjector(new AbstractModule() {
+ @Override
+ protected void configure() {
+ bindListener(Matchers.any(), counter);
+ bindListener(Matchers.any(), counter);
+ }
+ });
+ injector.getInstance(Many.class);
+ assertEquals("ProvisionListener not de-duplicated", 1, counter.count);
+ }
}
diff --git a/core/test/com/google/inject/TypeListenerTest.java b/core/test/com/google/inject/TypeListenerTest.java
index e4b5f63..ba7520d 100644
--- a/core/test/com/google/inject/TypeListenerTest.java
+++ b/core/test/com/google/inject/TypeListenerTest.java
@@ -630,6 +630,49 @@
}
}
+ private static class CountingMembersInjector implements MembersInjector<D> {
+ public void injectMembers(D instance) {
+ ++instance.userInjected;
+ }
+ }
+
+ private static class CountingInjectionListener implements InjectionListener<D> {
+ public void afterInjection(D injectee) {
+ ++injectee.listenersNotified;
+ }
+ }
+
+ private static class DuplicatingTypeListener implements TypeListener {
+ int count = 0;
+
+ @SuppressWarnings({"rawtypes", "unchecked"})
+ public <I> void hear(TypeLiteral<I> type, TypeEncounter<I> encounter) {
+ ++count;
+
+ MembersInjector membersInjector = new CountingMembersInjector();
+ encounter.register(membersInjector);
+ encounter.register(membersInjector);
+
+ InjectionListener injectionListener = new CountingInjectionListener();
+ encounter.register(injectionListener);
+ encounter.register(injectionListener);
+ }
+ }
+
+ public void testDeDuplicateTypeListeners() {
+ final DuplicatingTypeListener typeListener = new DuplicatingTypeListener();
+ Injector injector = Guice.createInjector(new AbstractModule() {
+ @Override
+ protected void configure() {
+ bindListener(any(), typeListener);
+ bindListener(only(new TypeLiteral<D>() {}), typeListener);
+ }
+ });
+ D d = injector.getInstance(D.class);
+ d.assertAllCounts(1);
+ assertEquals(1, typeListener.count);
+ }
+
// TODO: recursively accessing a lookup should fail
static class A {
