frjaeger220 / google-guice

Automatically exported from code.google.com/p/google-guice
Apache License 2.0
0 stars 0 forks source link

@Singleton not respected if type is parameterized #317

Open GoogleCodeExporter opened 9 years ago

GoogleCodeExporter commented 9 years ago
In the unit test below I have tagged the Injected class with @Singleton yet 
Guice creates 2 instance of it. This only breaks because of the generic 
parameter. If you remove <T> from the code below the unit test passes. 
----------------------------------------------
public class TestSingleton {
    @ImplementedBy(Injected.class) public interface In<T> {}

    @Singleton public static class Injected<T>  implements In<T>{}

    public static class InjectSameThingTwice{
        @Inject public InjectSameThingTwice(In<String> in, 
Injected<String> in2){
            if (in != in2){
                throw new RuntimeException("not the same 
instance");
            }
        }
    }

    @Test public void testGuice() throws Exception {

Guice.createInjector().getInstance(InjectSameThingTwice.class);
    }
}

Original issue reported on code.google.com by NikolayM...@gmail.com on 25 Jan 2009 at 12:43

GoogleCodeExporter commented 9 years ago
Turns out @ImplementedBy isn't really doing what you want. In this case, the 
equivalent bindings is to the raw 
type, not to the parameterized one you expect. It's equivalent to:
  bind(new TypeLiteral<In<String>>() {}).to(Injected.class);

I checked in this test to demonstrate our behaviour:
  public void testSingletonAnnotationOnParameterizedType() {
    Injector injector = Guice.createInjector();
    assertSame(injector.getInstance(new Key<Injected<String>>() {}),
        injector.getInstance(new Key<Injected<String>>() {}));
    assertSame(injector.getInstance(new Key<In<Integer>>() {}),
        injector.getInstance(new Key<In<Short>>() {}));
  }

So as a lame result, you get the same singleton instance for an In<Short> and 
an In<Long>. I admit that it's 
lame, but unfortunately I think doing the canonical solution is tricky in 
practice. Your example is pretty 
straightforward, but in general the relation between the type parameters in the 
interface and implementation 
is not always as clear cut. Consider
  @ImplementedBy(MyEntrySet.class) public interface MySet<E> {}
  @Singleton public static class MyEntrySet<V, K> implements MySet<Map.Entry<K, V>> {}
In this case, if the user asks to inject a MySet<Map.Entry<Short, Long>>, we 
need to figure out that to get 
that type we need to build a MyEntrySet<Long, Short>. I imagine it can be done 
but we haven't invested the 
effort yet.

As a workaround I recommend you use an explicit binding. Sorry I can't be much 
more helpful.

Original comment by limpbizkit on 25 Jan 2009 at 9:03

GoogleCodeExporter commented 9 years ago
Unfortunately I do not understand the Guice implementation to be able to help 
too 
much here. However as an end user I have to admit that the behaviour is 
definitely counter intuitive. With regards to your example: If MyEntrySet has 
been tagged as a 
Singleton then I an not quite sure why Guice would need to bother figuring out 
its parametrized type (erasure means that it doesn't matter). As long as Guice 
is able to 
instantiate one instance of MyEntrySet than that is the instance to be used 
everywhere. If code breaks than its the application writers fault and not 
Guice. 
I fully understand if fixing this bug is going to be very tricky but it will 
definitely cause very subtle bugs if the end user is not aware of it. 

Original comment by NikolayM...@gmail.com on 26 Jan 2009 at 12:24

GoogleCodeExporter commented 9 years ago
We should be discouraging use of @ImplementedBy anyway. Perhaps this can be 
used as an additional reason in 
the docs not to use it unless you know what you're doing.

Original comment by dha...@gmail.com on 26 Jan 2009 at 1:11

GoogleCodeExporter commented 9 years ago
What about the ability to disable recognition of @ImplementedBy 
programmatically using a simple setter - 
Binder.ignoreImplementedByAnnotations().

Original comment by miroslav...@gmail.com on 20 Jun 2010 at 3:05