Skip to content

Commit

Permalink
Make requestInjection with an explicit type actually use the type. Fixes
Browse files Browse the repository at this point in the history
 #1071.

PiperOrigin-RevId: 525789577
  • Loading branch information
sameb authored and Guice Team committed Apr 21, 2023
1 parent 0c43c8d commit 6e6da79
Show file tree
Hide file tree
Showing 9 changed files with 225 additions and 28 deletions.
8 changes: 8 additions & 0 deletions core/src/com/google/inject/AbstractModule.java
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,14 @@ protected void requestInjection(Object instance) {
binder().requestInjection(instance);
}

/**
* @see Binder#requestInjection(TypeLiteral, Object)
* @since vNext
*/
protected <T> void requestInjection(TypeLiteral<T> type, T instance) {
binder().requestInjection(type, instance);
}

/** @see Binder#requestStaticInjection(Class[]) */
protected void requestStaticInjection(Class<?>... types) {
binder().requestStaticInjection(types);
Expand Down
21 changes: 17 additions & 4 deletions core/src/com/google/inject/internal/BindingProcessor.java
Original file line number Diff line number Diff line change
Expand Up @@ -98,11 +98,24 @@ public Boolean visit(InstanceBinding<? extends T> binding) {
prepareBinding();
Set<InjectionPoint> injectionPoints = binding.getInjectionPoints();
T instance = binding.getInstance();
@SuppressWarnings("unchecked") // safe to cast to binding<T> because
// the processor was constructed w/ it
// Note: We cannot use type=`binding.getKey().getTypeLiteral()`, even though it would
// be more accurate, because it's very likely that people do:
// bind(new TypeLiteral<Foo>() {}).toInstance(someFooBarInstance);
// bind(new TypeLiteral<Bar>() {}).toInstance(someFooBarInstance);
// ... and if we captured the exact type when passing to requestInjection, then we'd
// fail because the same instance requested injection from two different types.

@SuppressWarnings("unchecked") // safe because processor was constructed w/ it
Binding<T> bindingT = (Binding<T>) binding;
Initializable<T> ref =
initializer.requestInjection(
injector, instance, (Binding<T>) binding, source, injectionPoints);
injector,
/* type= */ null,
instance,
bindingT,
source,
injectionPoints,
errors);
ConstantFactory<? extends T> factory = new ConstantFactory<>(ref);
InternalFactory<? extends T> scopedFactory =
Scoping.scope(key, injector, factory, source, scoping);
Expand All @@ -125,7 +138,7 @@ public Boolean visit(ProviderInstanceBinding<? extends T> binding) {
Set<InjectionPoint> injectionPoints = binding.getInjectionPoints();
Initializable<? extends javax.inject.Provider<? extends T>> initializable =
initializer.<javax.inject.Provider<? extends T>>requestInjection(
injector, provider, null, source, injectionPoints);
injector, /* type= */ null, provider, null, source, injectionPoints, errors);
// always visited with Binding<T>
@SuppressWarnings("unchecked")
InternalFactory<T> factory =
Expand Down
1 change: 1 addition & 0 deletions core/src/com/google/inject/internal/ErrorId.java
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ public enum ErrorId {
RECURSIVE_BINDING,
RECURSIVE_IMPLEMENTATION_TYPE,
RECURSIVE_PROVIDER_TYPE,
REQUEST_INJECTION_WITH_DIFFERENT_TYPES,
SCOPE_ANNOTATION_ON_ABSTRACT_TYPE,
SCOPE_NOT_FOUND,
STATIC_INJECTION_ON_INTERFACE,
Expand Down
13 changes: 13 additions & 0 deletions core/src/com/google/inject/internal/Errors.java
Original file line number Diff line number Diff line change
Expand Up @@ -499,6 +499,19 @@ public Errors errorCheckingDuplicateBinding(Key<?> key, Object source, Throwable
t);
}

public Errors requestInjectionWithDifferentTypes(
Object instance, TypeLiteral<?> type1, Object type1Source, TypeLiteral<?> type2) {
return addMessage(
ErrorId.REQUEST_INJECTION_WITH_DIFFERENT_TYPES,
"Cannot request injection on one instance with two different types. requestInjection was"
+ " already called for instance %s at %s (with type %s), which is different than type"
+ " %s.",
instance.getClass().getName() + "@" + System.identityHashCode(instance),
type1Source,
type1,
type2);
}

public Errors errorNotifyingTypeListener(
TypeListenerBinding listener, TypeLiteral<?> type, Throwable cause) {
return errorInUserCode(
Expand Down
27 changes: 19 additions & 8 deletions core/src/com/google/inject/internal/Initializer.java
Original file line number Diff line number Diff line change
Expand Up @@ -75,10 +75,12 @@ final class Initializer {
*/
<T> Initializable<T> requestInjection(
InjectorImpl injector,
TypeLiteral<T> type,
T instance,
Binding<T> binding,
Object source,
Set<InjectionPoint> injectionPoints) {
Set<InjectionPoint> injectionPoints,
Errors errors) {
checkNotNull(source);
Preconditions.checkState(
!validationStarted, "Member injection could not be requested after validation is started");
Expand All @@ -93,16 +95,27 @@ <T> Initializable<T> requestInjection(
return Initializables.of(instance);
}

if (type == null) {
@SuppressWarnings("unchecked") // the type of 'T' is a TypeLiteral<T>
TypeLiteral<T> instanceType = TypeLiteral.get((Class<T>) instance.getClass());
type = instanceType;
}

if (initializablesCache.containsKey(instance)) {
@SuppressWarnings("unchecked") // Map from T to InjectableReference<T>
Initializable<T> cached = (Initializable<T>) initializablesCache.get(instance);
InjectableReference<T> cached = (InjectableReference<T>) initializablesCache.get(instance);
if (!cached.type.equals(type)) {
// Record the error, but proceed to capture as many errors as possible.
errors.requestInjectionWithDifferentTypes(instance, cached.type, cached.source, type);
}
return cached;
}

InjectableReference<T> injectableReference =
new InjectableReference<T>(
injector,
instance,
type,
binding == null ? null : binding.getKey(),
provisionCallback,
source,
Expand Down Expand Up @@ -158,6 +171,7 @@ private static class InjectableReference<T> implements Initializable<T> {

private final InjectorImpl injector;
private final T instance;
private final TypeLiteral<T> type;
private final Object source;
private final Key<T> key;
private final ProvisionListenerStackCallback<T> provisionCallback;
Expand All @@ -166,6 +180,7 @@ private static class InjectableReference<T> implements Initializable<T> {
public InjectableReference(
InjectorImpl injector,
T instance,
TypeLiteral<T> type,
Key<T> key,
ProvisionListenerStackCallback<T> provisionCallback,
Object source,
Expand All @@ -174,19 +189,15 @@ public InjectableReference(
this.key = key; // possibly null!
this.provisionCallback = provisionCallback; // possibly null!
this.instance = checkNotNull(instance, "instance");
this.type = checkNotNull(type, "type");
this.source = checkNotNull(source, "source");
this.lock = checkNotNull(lock, "lock");
}

public void validate(Errors errors) throws ErrorsException {
@SuppressWarnings("unchecked") // the type of 'T' is a TypeLiteral<T>
TypeLiteral<T> type = TypeLiteral.get((Class<T>) instance.getClass());
membersInjector = injector.membersInjectorStore.get(type, errors.withSource(source));
Preconditions.checkNotNull(
membersInjector,
"No membersInjector available for instance: %s, from key: %s",
instance,
key);
membersInjector, "No membersInjector available for type: %s, from key: %s", type, key);
state = InjectableReferenceState.VALIDATED;
}

Expand Down
33 changes: 21 additions & 12 deletions core/src/com/google/inject/internal/InjectionRequestProcessor.java
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@
import com.google.common.collect.Lists;
import com.google.inject.ConfigurationException;
import com.google.inject.Stage;
import com.google.inject.TypeLiteral;
import com.google.inject.spi.InjectionPoint;
import com.google.inject.spi.InjectionRequest;
import com.google.inject.spi.StaticInjectionRequest;
Expand Down Expand Up @@ -61,23 +60,33 @@ public Boolean visit(InjectionRequest<?> request) {
injectionPoints = e.getPartialValue();
}

initializer.requestInjection(
injector, request.getInstance(), null, request.getSource(), injectionPoints);
// When recreating the injection request, we revise the TypeLiteral to be the type
// of the instance. This is because currently Guice ignores the user's TypeLiteral
// when determining the types for members injection.
// If/when this is fixed, we can report the exact type back to the user.
// (Otherwise the injection points exposed from the request may be wrong.)
requestInjection(request, injectionPoints, errors);

// Drop the actual instance from the binding data we store for the SPI.
// TODO(sameb): Why?
injector
.getBindingData()
.putInjectionRequest(
new InjectionRequest<>(
request.getSource(),
TypeLiteral.get(request.getInstance().getClass()),
/* instance= */ null));
new InjectionRequest<>(request.getSource(), request.getType(), /* instance= */ null));
return true;
}

// Note: This is extracted to a separate method just to help Java infer the generics correctly.
private <T> void requestInjection(
InjectionRequest<T> request, Set<InjectionPoint> injectionPoints, Errors errors) {
// We don't need to keep the return value, because we're not _using_ the injected value
// anyway... we're just injecting it.
Initializable<T> unused =
initializer.requestInjection(
injector,
request.getType(),
request.getInstance(),
null,
request.getSource(),
injectionPoints,
errors);
}

void validate() {
for (StaticInjection staticInjection : staticInjections) {
staticInjection.validate();
Expand Down
3 changes: 1 addition & 2 deletions core/src/com/google/inject/spi/InjectionRequest.java
Original file line number Diff line number Diff line change
Expand Up @@ -77,8 +77,7 @@ public TypeLiteral<T> getType() {
* the valid injection points.
*/
public Set<InjectionPoint> getInjectionPoints() throws ConfigurationException {
return InjectionPoint.forInstanceMethodsAndFields(
instance != null ? TypeLiteral.get(instance.getClass()) : type);
return InjectionPoint.forInstanceMethodsAndFields(type);
}

@Override
Expand Down
50 changes: 49 additions & 1 deletion core/test/com/google/inject/BinderTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@

package com.google.inject;

import static com.google.common.truth.Truth.assertThat;
import static com.google.inject.Asserts.assertContains;
import static com.google.inject.Asserts.assertNotSerializable;
import static com.google.inject.Asserts.getDeclaringSourcePart;
Expand All @@ -37,7 +38,9 @@
import java.util.logging.Logger;
import junit.framework.TestCase;

/** @author crazybob@google.com (Bob Lee) */
/**
* @author crazybob@google.com (Bob Lee)
*/
public class BinderTest extends TestCase {

private final Logger loggerToWatch = Logger.getLogger(Guice.class.getName());
Expand All @@ -63,6 +66,7 @@ public void close() throws SecurityException {}
protected void setUp() throws Exception {
super.setUp();
loggerToWatch.addHandler(fakeHandler);
TwoParts.injectedCount = 0;
}

@Override
Expand Down Expand Up @@ -689,4 +693,48 @@ public void testInjectRawProvider() {
"while locating Provider");
}
}

private static interface Part1 {
String getStr();
}

private static interface Part2 {}

private static class TwoParts implements Part1, Part2 {
String str;
static int injectedCount = 0;

@Inject
void inject(String str) {
injectedCount++;
this.str = str;
}

@Override
public String getStr() {
return str;
}
}

public void testToInstanceWithDifferentTypesWorksAndDoesntInjectInstanceTwice() {
Injector injector =
Guice.createInjector(
new AbstractModule() {
@Override
protected void configure() {
TwoParts parts = new TwoParts();
bind(Part1.class).toInstance(parts);
bind(Part2.class).toInstance(parts);
}

@Provides
String provideStr() {
return "foo";
}
});
Part1 parts = injector.getInstance(Part1.class);
assertThat(parts).isSameInstanceAs(injector.getInstance(Part2.class));
assertThat(parts.getStr()).isEqualTo("foo");
assertThat(TwoParts.injectedCount).isEqualTo(1);
}
}
Loading

0 comments on commit 6e6da79

Please sign in to comment.