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

Scoped bindings not reusing instances? #149

Closed
philipbjorge opened this issue Sep 5, 2016 · 2 comments
Closed

Scoped bindings not reusing instances? #149

philipbjorge opened this issue Sep 5, 2016 · 2 comments

Comments

@philipbjorge
Copy link

Docs on Scoped/Unscoped Bindings

A binding is scoped when we call its method scope(). bind(IFoo.class).to(Foo.class).scope() express more than the simple association IFoo --> Foo. A scoped bindings means :

there is an association IFoo --> Foo, in the same way as an unscoped binding does;
AND the same instance of Foo is recycled/reused for each injection of IFoo
AND the instance of Foo will be created inside the scope. All the dependencies of Foo will have to be found at runtime in the scope where the binding is scoped or in its parent scopes.

bind(OkHttpClient.class).toProvider(OkHttpClientProvider.class).scope();
This creates a new instance every injection, which makes me think AND the same instance of Foo is recycled/reused for each injection of IFoo is not accurate any more? Is that true?

It looks like I will want to use a combination of @Scope and @ScopeInstances to accomplish the same effect now.

@stephanenicolas
Copy link
Owner

stephanenicolas commented Sep 19, 2016

Hi @philipbjorge, first, thx for submitting the issues and the PR. We have been on vacations and were pretty busy when coming back, we had no time to dedicate to TP.

We have been talking with @dlemures about the issue you raised. For this one, you are right, there is a problem in the docs.

Actually the issue is not so easy. We could feel a point of tension in TP and the guice API for bindings as a binding holds 3 different informations:

  1. where is the binding defined and available
  2. whether or not the binding is scoped = instances will be created not in the current scope that is used to perform the getInstance call but in the scope where the binding is defined
  3. reusing instances or not

We introduced the method scope() to allow 2 and 3 together, but it was not a good idea and we finally relied on a different mechanism for 3. So, for now, scope() only means 2, not 3 : it doesn't reuse instances = it does not create singletons.

We want to fix that and get an API that allows programmatically to do all possible operations on bindings, independently of annotations. Annotations are more a syntactic sugar in our idea of TP.

So, we want to introduce the following changes:

  • change the doc to reflect that scope() doesn't create singletons
  • make it clear that scope() triggers the scope verification mechanism to enforce instances are created within the scope where the binding is defined.
  • check the wiki for scope annotations, for renamed annotation
  • check the wiki for scoping a binding == using a scope annotation. This is not true any more but using the singletonInScope() method is now equivalent to using a scope annotation.
  • define whether or not the @ProvidesSingletonInScope is compatible with a scope annotation for providers. --> we need the scope annotation to get the right target scope for the provider factory.
  • check if scopingTest defines enough tests, especially for new methods on bindings.
  • change our checks so that scope() can be called when the target of the binding is annotated with a scope annotation that is supported by the scope where the binding is defined. This will be checked at binding time, by the current check configurations.
  • create a scopeSingleton() method on bindings to create (lazy) singletons, they will be scoped and instances will be recycled in the scope where the binding is called.
  • create a instancesScopeSingleton() method on bindings for providers to create scoped singletons bindings.

What do you think of our plan ?

@stephanenicolas
Copy link
Owner

will be shipped in RC 10

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

No branches or pull requests

2 participants