Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Multibinder Bindings Don't Always Respect Singleton Scope #791

Closed
gissuebot opened this issue Jul 7, 2014 · 5 comments
Closed

Multibinder Bindings Don't Always Respect Singleton Scope #791

gissuebot opened this issue Jul 7, 2014 · 5 comments
Labels

Comments

@gissuebot
Copy link

From nachtrabe on December 11, 2013 13:10:31

Description of the issue: When using the multibinder, if you use binder.addBinding().to(B.class).in(Singleton.class); it will not enforce the singleton nature on that object, but using the @`Singleton` annotation directly will. Steps to reproduce: 1. Create a Multibinder.newSetBinder instance with an interface. 2. Bind an implementation using `.in(Singleton.class)`. 3. Bind another implementation annotated with @Singleton.
4. Inspect that the instances retrieved are not singletons in the cases where .in(Singleton.class) was used, but are in the cases where ``@Singleton was used.

Expected behavior:

- I'd expect for .in(Singleton.class) to work, to not be exposed, or to throw an exception.

The attached tests illustrate the problem: getInstance_ofSet_ContainsB and getInstance_ofB_IsIdentical both fail under both Guice 3.0 and Guice 4.0-beta.

Attachment: gist
   GuiceTest.java

Original issue: http://code.google.com/p/google-guice/issues/detail?id=791

@gissuebot
Copy link
Author

From sberlin on December 11, 2013 10:15:19

Use a separate bind(B.class).in(Singleton.class) to make this work.  What is happening is that you're making linked binding a singleton, but not the underlying binding a singleton.

The same thing would happen if you did:
  bind(Foo.class).to(FooImpl.class).in(Singleton.class);
and then tried to compare injecting Foo vs injecting FooImpl.  Injecting Foo multiple times would be the same, but it'd be a different instance than FooImpl, because "Foo" is the singleton, not "FooImpl".

Status: WorkingAsIntended

@gjoseph
Copy link

gjoseph commented Feb 25, 2015

Is there no way around this ? (e.g is it unreasonable to not want to explicitly bind using FooImpl as a key, or at least not make it injectable? If Foo implements Bar want to be able to inject Foo as well as Set<Bar>, but not FooImpl)

@sameb
Copy link
Member

sameb commented Feb 25, 2015

The best way to make FooImpl not injectable is to make it package-private, so others can't refer to it.

@gjoseph
Copy link

gjoseph commented Feb 26, 2015

Sure; that works if I have a Module in that same package. I'm dealing with a ball of legacy code, and can't really do that (or ensure that everyone does it, anyway). That said, the extra binding is also probably no big deal, but I have no way of checking (in plugin-based environment, my only option now is to let the bindings fail at runtime, and it might not fail for everyone)

@gjoseph
Copy link

gjoseph commented Mar 2, 2015

Sam and OP, I got this to work this way: (with help from @tmattsson)
Op's original binding:

            binder.addBinding().to(B.class).in(Singleton.class);

replaced by this:

            binder().bind(B.class).in(Singleton.class);
            binder.addBinding().to(Key.get(B.class));

makes it work as we expect. WDYT ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants