Skip to content

Commit

Permalink
Fix issue of missing shard when a members injection binding exists in…
Browse files Browse the repository at this point in the history 
… a parent component.

Fixes #3143

This issue occurs in MembersInjectionMethods, where we use the key to look-up the binding. The problem is that we were using BindingGraph.membersInjectionBinding(Key) and BindingGraph.contributionBinding(Key) which can return bindings in parent components. We should have been using BindingGraph.localMembersInjectionBinding(Key) and BindingGraph.localContributionBinding(Key) instead to ensure we get a binding from the current component rather than a parent.

See #3143

RELNOTES=Fixes #3143: Fix issue with missing shard when a members injection binding exists in a parent component.
PiperOrigin-RevId: 419927744
  • Loading branch information
@bcorso
bcorso authored and Dagger Team committed Jan 7, 2022
1 parent d69478c commit 2f1c120
Show file tree
Hide file tree
Showing 2 changed files with 222 additions and 3 deletions.
6 changes: 3 additions & 3 deletions java/dagger/internal/codegen/writing/MembersInjectionMethods.java
View file
Expand Up @@ -81,9 +81,9 @@ final class MembersInjectionMethods {
*/
Expression getInjectExpression(Key key, CodeBlock instance, ClassName requestingClass) {
Binding binding =
graph.membersInjectionBinding(key).isPresent()
? graph.membersInjectionBinding(key).get()
: graph.contributionBinding(key);
graph.localMembersInjectionBinding(key).isPresent()
? graph.localMembersInjectionBinding(key).get()
: graph.localContributionBinding(key).get();
Expression expression =
reentrantComputeIfAbsent(
injectMethodExpressions, key, k -> injectMethodExpression(binding));
Expand Down
219 changes: 219 additions & 0 deletions javatests/dagger/internal/codegen/MembersInjectionTest.java
View file
Expand Up @@ -1908,4 +1908,223 @@ public void testConstructorInjectedFieldInjection() {
.generatedSourceFile("test.B_MembersInjector")
.hasSourceEquivalentTo(expectedBMembersInjector);
}

// Regression test for https://github.com/google/dagger/issues/3143
@Test
public void testMembersInjectionBindingExistsInParentComponent() {
JavaFileObject component =
JavaFileObjects.forSourceLines(
"test.MyComponent",
"package test;",
"",
"import dagger.Component;",
"",
"@Component(modules = MyComponentModule.class)",
"public interface MyComponent {",
" void inject(Bar bar);",
"",
" MySubcomponent subcomponent();",
"}");

JavaFileObject subcomponent =
JavaFileObjects.forSourceLines(
"test.MySubcomponent",
"package test;",
"",
"import dagger.Subcomponent;",
"",
"@Subcomponent(modules = MySubcomponentModule.class)",
"interface MySubcomponent {",
" Foo foo();",
"}");

JavaFileObject foo =
JavaFileObjects.forSourceLines(
"test.Foo",
"package test;",
"",
"import javax.inject.Inject;",
"",
"class Foo {",
" @Inject Foo(Bar bar) {}",
"}");

JavaFileObject bar =
JavaFileObjects.forSourceLines(
"test.Bar",
"package test;",
"",
"import java.util.Set;",
"import javax.inject.Inject;",
"",
"class Bar {",
" @Inject Set<String> multibindingStrings;",
" @Inject Bar() {}",
"}");

JavaFileObject componentModule =
JavaFileObjects.forSourceLines(
"test.MyComponentModule",
"package test;",
"",
"import dagger.Module;",
"import dagger.Provides;",
"import dagger.multibindings.IntoSet;",
"",
"@Module",
"interface MyComponentModule {",
" @Provides",
" @IntoSet",
" static String provideString() {",
" return \"\";",
" }",
"}");

JavaFileObject subcomponentModule =
JavaFileObjects.forSourceLines(
"test.MySubcomponentModule",
"package test;",
"",
"import dagger.Module;",
"import dagger.Provides;",
"import dagger.multibindings.IntoSet;",
"",
"@Module",
"interface MySubcomponentModule {",
" @Provides",
" @IntoSet",
" static String provideString() {",
" return \"\";",
" }",
"}");

Compilation compilation =
compilerWithOptions(compilerMode.javacopts())
.compile(component, subcomponent, foo, bar, componentModule, subcomponentModule);
assertThat(compilation).succeeded();

// Check that the injectBar() method is not shared across components.
// We avoid sharing them in general because they may be different (e.g. in this case we inject
// multibindings that are different across components).
assertThat(compilation)
.generatedSourceFile("test.DaggerMyComponent")
.containsElementsIn(
JavaFileObjects.forSourceLines(
"test.DaggerMyComponent",
"package test;",
"",
GeneratedLines.generatedAnnotations(),
"public final class DaggerMyComponent implements MyComponent {",
" private Set<String> setOfString() {",
" return ImmutableSet.<String>of(",
" MyComponentModule_ProvideStringFactory.provideString());",
" }",
"",
" @Override",
" public void inject(Bar bar) {",
" injectBar(bar);",
" }",
"",
" @CanIgnoreReturnValue",
" private Bar injectBar(Bar instance) {",
" Bar_MembersInjector.injectMultibindingStrings(instance, setOfString());",
" return instance;",
" }",
"",
" private static final class MySubcomponentImpl implements MySubcomponent {",
" private Set<String> setOfString() {",
" return ImmutableSet.<String>of(",
" MyComponentModule_ProvideStringFactory.provideString(),",
" MySubcomponentModule_ProvideStringFactory.provideString());",
" }",
"",
" private Bar bar() {",
" return injectBar(Bar_Factory.newInstance());",
" }",
"",
" @Override",
" public Foo foo() {",
" return new Foo(bar());",
" }",
"",
" @CanIgnoreReturnValue",
" private Bar injectBar(Bar instance) {",
" Bar_MembersInjector.injectMultibindingStrings(instance, setOfString());",
" return instance;",
" }",
" }",
"}"));
}

// Test that if both a MembersInjectionBinding and ProvisionBinding both exist in the same
// component they share the same inject methods rather than generating their own.
@Test
public void testMembersInjectionBindingSharesInjectMethodsWithProvisionBinding() {
JavaFileObject component =
JavaFileObjects.forSourceLines(
"test.MyComponent",
"package test;",
"",
"import dagger.Component;",
"",
"@Component",
"public interface MyComponent {",
" Foo foo();",
"",
" void inject(Foo foo);",
"}");

JavaFileObject foo =
JavaFileObjects.forSourceLines(
"test.Foo",
"package test;",
"",
"import javax.inject.Inject;",
"",
"class Foo {",
" @Inject Bar bar;",
" @Inject Foo() {}",
"}");

JavaFileObject bar =
JavaFileObjects.forSourceLines(
"test.Bar",
"package test;",
"",
"import javax.inject.Inject;",
"",
"class Bar {",
" @Inject Bar() {}",
"}");

Compilation compilation =
compilerWithOptions(compilerMode.javacopts()).compile(component, foo, bar);
assertThat(compilation).succeeded();

assertThat(compilation)
.generatedSourceFile("test.DaggerMyComponent")
.containsElementsIn(
JavaFileObjects.forSourceLines(
"test.DaggerMyComponent",
"package test;",
"",
GeneratedLines.generatedAnnotations(),
"public final class DaggerMyComponent implements MyComponent {",
" @Override",
" public Foo foo() {",
" return injectFoo(Foo_Factory.newInstance());",
" }",
"",
" @Override",
" public void inject(Foo foo) {",
" injectFoo(foo);",
" }",
"",
" @CanIgnoreReturnValue",
" private Foo injectFoo(Foo instance) {",
" Foo_MembersInjector.injectBar(instance, new Bar());",
" return instance;",
" }",
"}"));
}
}

0 comments on commit 2f1c120

Please sign in to comment.