square / dagger

A fast dependency injector for Android and Java.
https://square.github.io/dagger/
Apache License 2.0
7.31k stars 3.07k forks source link

Different instances being returned for @Inject vs .get() #323

Open amangel opened 11 years ago

amangel commented 11 years ago

While working on an Android project, I was experimenting with having an Activity provide a child module that would provide some convenience methods. The idea was that while testing, if I could overwrite the module, I could mock what was being returned and verify the interactions. I noticed some strange behavior however and I've boiled the interactions down to a simple class that illustrates what is happening.

import javax.inject.Inject;

import dagger.Module;
import dagger.ObjectGraph;
import dagger.Provides;

public class ClassA {

    @Inject String string;

    public ClassA() {
        ObjectGraph graph = ObjectGraph.create(new MainModule()).plus(new ChildModule());
        graph.inject(this);
        System.out.println("@Injected string: " + string);
        System.out.println("String from .get(String.class): " + graph.get(String.class));
    }

    @Module(injects = {ClassA.class,
            String.class})
    public class MainModule {
        @Provides String provideString() {
            return "String from the main module";
        }
    }

    @Module(injects = {ClassA.class,
            String.class})
    public class ChildModule {
        @Provides String provideString() {
            return "String from the child module";
        }
    }

    public static void main(String[] args) {
        new ClassA();
    }
}

Running this gives the output:

@Injected string: String from the main module
String from .get(String.class): String from the child module

This is using Dagger 1.1.0 Is there something that I'm missing in using child graphs or is this an actual issue?

JakeWharton commented 11 years ago

Woah. This should fail at runtime! Two things cannot provide the same type. Does it fail at compile time if you put addsTo=MainModule.class on ChildModule?

The .plus'd graph is a superset of the parent graph which does not allow overrides (which you aren't even actaully specifying).

amangel commented 11 years ago

If I add the addsTo=MainModule.class then it gives a compile time warning for the duplicate binding.

JakeWharton commented 11 years ago

I'll try to write a test case tonight. And maybe even seek out the root cause. Will report back.

JakeWharton commented 11 years ago

Easy to reproduce. Wasn't able to find an obvious place that it should be failing, though. Checks for duplicate bindings is only covered by the full-graph analysis. I'll have to defer to @cgruber for more insight into what should be done here. Dagger's internals have changed a bit since I last dug in.

cgruber commented 11 years ago

Can you post the test-case on this thread? I'm not sure I'm following the concrete specifics of when it goes awry.

cgruber commented 11 years ago

Crap - I'm brain-dead. The code is above. I'll try to repro it locally using that.

JakeWharton commented 10 years ago

@cgruber any chance you'd be able to look into this soon? If not, I can try to squeeze it in one of these nights or weekends.