Fixing a critical bug! The following code was broken (although we did have a TODO dating back to sometime last year)
@ImplementedBy(RealFoo.class)
interface Foo {}
Module fooModule = new AbstractModule() {
public void configure() {
bind(Foo.class).in(Scopes.SINGLETON);
}
}
The problem was we had a TODO to support scopes on @ProvidedBy and @ImplementedBy that wasn't actually done. This is now fixed. I'm surprised we didn't catch this earlier.
git-svn-id: https://google-guice.googlecode.com/svn/trunk@594 d779f126-a31b-0410-b53b-1d3aecad763e
diff --git a/src/com/google/inject/BindingProcessor.java b/src/com/google/inject/BindingProcessor.java
index 92b071d..9ed2f37 100644
--- a/src/com/google/inject/BindingProcessor.java
+++ b/src/com/google/inject/BindingProcessor.java
@@ -18,10 +18,8 @@
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Lists;
-import com.google.inject.internal.Classes;
import com.google.inject.internal.Errors;
import com.google.inject.internal.ErrorsException;
-import com.google.inject.internal.StackTraceElements;
import com.google.inject.spi.BindingScopingVisitor;
import com.google.inject.spi.BindingTargetVisitor;
import java.lang.annotation.Annotation;
@@ -229,14 +227,7 @@
}
private <T> void validateKey(Object source, Key<T> key) {
- Class<? super T> rawType = key.getRawType();
- if (!Classes.isConcrete(rawType)) {
- Class<? extends Annotation> scopeAnnotation = Scopes.findScopeAnnotation(errors, rawType);
- if (scopeAnnotation != null) {
- errors.withSource(StackTraceElements.forType(rawType))
- .scopeAnnotationOnAbstractType(scopeAnnotation, rawType, source);
- }
- }
+ Scopes.checkForMisplacedScopeAnnotations(key.getRawType(), source, errors);
}
<T> InvalidBindingImpl<T> invalidBinding(InjectorImpl injector, Key<T> key, Object source) {
diff --git a/src/com/google/inject/InjectorImpl.java b/src/com/google/inject/InjectorImpl.java
index 918c6d2..5db555d 100644
--- a/src/com/google/inject/InjectorImpl.java
+++ b/src/com/google/inject/InjectorImpl.java
@@ -442,15 +442,15 @@
// Handle @ImplementedBy
ImplementedBy implementedBy = type.getAnnotation(ImplementedBy.class);
if (implementedBy != null) {
- // TODO: Scope internal factory.
- return createImplementedByBinding(type, implementedBy, loadStrategy, errors);
+ Scopes.checkForMisplacedScopeAnnotations(type, source, errors);
+ return createImplementedByBinding(type, scope, implementedBy, loadStrategy, errors);
}
// Handle @ProvidedBy.
ProvidedBy providedBy = type.getAnnotation(ProvidedBy.class);
if (providedBy != null) {
- // TODO: Scope internal factory.
- return createProvidedByBinding(type, providedBy, loadStrategy, errors);
+ Scopes.checkForMisplacedScopeAnnotations(type, source, errors);
+ return createProvidedByBinding(type, scope, providedBy, loadStrategy, errors);
}
// We can't inject abstract classes.
@@ -509,8 +509,8 @@
}
/** Creates a binding for a type annotated with @ProvidedBy. */
- <T> BindingImpl<T> createProvidedByBinding(final Class<T> type, ProvidedBy providedBy,
- LoadStrategy loadStrategy, Errors errors) throws ErrorsException {
+ <T> BindingImpl<T> createProvidedByBinding(final Class<T> type, Scope scope,
+ ProvidedBy providedBy, LoadStrategy loadStrategy, Errors errors) throws ErrorsException {
final Class<? extends Provider<?>> providerType = providedBy.value();
// Make sure it's not the same type. TODO: Can we check for deeper loops?
@@ -542,19 +542,20 @@
}
};
+ Key<T> key = Key.get(type);
return new LinkedProviderBindingImpl<T>(
this,
- Key.get(type),
+ key,
StackTraceElements.forType(type),
- internalFactory,
- Scopes.NO_SCOPE,
+ Scopes.<T>scope(key, this, internalFactory, scope),
+ scope,
providerKey,
loadStrategy);
}
/** Creates a binding for a type annotated with @ImplementedBy. */
- <T> BindingImpl<T> createImplementedByBinding(
- Class<T> type, ImplementedBy implementedBy, LoadStrategy loadStrategy, Errors errors)
+ <T> BindingImpl<T> createImplementedByBinding(Class<T> type, Scope scope,
+ ImplementedBy implementedBy, LoadStrategy loadStrategy, Errors errors)
throws ErrorsException {
// TODO: Use scope annotation on type if present. Right now, we always use NO_SCOPE.
Class<?> implementationType = implementedBy.value();
@@ -583,12 +584,13 @@
}
};
+ Key<T> key = Key.get(type);
return new LinkedBindingImpl<T>(
this,
- Key.get(type),
+ key,
StackTraceElements.forType(type),
- internalFactory,
- Scopes.NO_SCOPE,
+ Scopes.<T>scope(key, this, internalFactory, scope),
+ scope,
Key.get(subclass),
loadStrategy);
}
diff --git a/src/com/google/inject/Scopes.java b/src/com/google/inject/Scopes.java
index 52e36ec..faf99bc 100644
--- a/src/com/google/inject/Scopes.java
+++ b/src/com/google/inject/Scopes.java
@@ -16,7 +16,9 @@
package com.google.inject;
+import com.google.inject.internal.Classes;
import com.google.inject.internal.Errors;
+import com.google.inject.internal.StackTraceElements;
import java.lang.annotation.Annotation;
/**
@@ -124,6 +126,22 @@
}
/**
+ * Adds an error if there is a misplaced annotations on {@code type}. Scoping
+ * annotations are not allowed on abstract classes or interfaces.
+ */
+ static void checkForMisplacedScopeAnnotations(Class<?> type, Object source, Errors errors) {
+ if (Classes.isConcrete(type)) {
+ return;
+ }
+
+ Class<? extends Annotation> scopeAnnotation = findScopeAnnotation(errors, type);
+ if (scopeAnnotation != null) {
+ errors.withSource(StackTraceElements.forType(type))
+ .scopeAnnotationOnAbstractType(scopeAnnotation, type, source);
+ }
+ }
+
+ /**
* Scopes an internal factory.
*/
static <T> InternalFactory<? extends T> scope(Key<T> key,
diff --git a/test/com/google/inject/ScopesTest.java b/test/com/google/inject/ScopesTest.java
index 1a9b7a1..1de9daf 100644
--- a/test/com/google/inject/ScopesTest.java
+++ b/test/com/google/inject/ScopesTest.java
@@ -41,6 +41,8 @@
bind(LinkedSingleton.class).to(RealLinkedSingleton.class);
bind(DependsOnJustInTimeSingleton.class);
bind(NotASingleton.class);
+ bind(ImplementedBySingleton.class).in(Scopes.SINGLETON);
+ bind(ProvidedBySingleton.class).in(Scopes.SINGLETON);
}
};
@@ -51,6 +53,8 @@
RealLinkedSingleton.nextInstanceId = 0;
JustInTimeSingleton.nextInstanceId = 0;
NotASingleton.nextInstanceId = 0;
+ Implementation.nextInstanceId = 0;
+ ProvidedBySingleton.nextInstanceId = 0;
}
public void testSingletons() {
@@ -79,6 +83,14 @@
assertNotSame(
injector.getInstance(NotASingleton.class),
injector.getInstance(NotASingleton.class));
+
+ assertSame(
+ injector.getInstance(ImplementedBySingleton.class),
+ injector.getInstance(ImplementedBySingleton.class));
+
+ assertSame(
+ injector.getInstance(ProvidedBySingleton.class),
+ injector.getInstance(ProvidedBySingleton.class));
}
public void testJustInTimeAnnotatedSingleton() {
@@ -350,4 +362,24 @@
@Singleton @CustomScoped
static class SingletonAndCustomScoped {}
+
+ @ImplementedBy(Implementation.class)
+ static interface ImplementedBySingleton {}
+
+ @ProvidedBy(ImplementationProvider.class)
+ static class ProvidedBySingleton {
+ static int nextInstanceId;
+ final int instanceId = nextInstanceId++;
+ }
+
+ static class Implementation implements ImplementedBySingleton {
+ static int nextInstanceId;
+ final int instanceId = nextInstanceId++;
+ }
+
+ static class ImplementationProvider implements Provider<ProvidedBySingleton> {
+ public ProvidedBySingleton get() {
+ return new ProvidedBySingleton();
+ }
+ }
}
