how to find the scope of an injectee in InjectionListener - and how to find the scope of a binding?

88 views
Skip to first unread message

James Strachan

unread,
Apr 3, 2009, 8:20:16 AM4/3/09
to google...@googlegroups.com
So after Dhanji's great suggestion...
http://code.google.com/p/google-guice/issues/detail?id=62

I started trying to implement a pre destroy hook using the injectable
type hooks. The idea being that if a type has a method annotated with
@PreDestroy (or whatever the close hook is - e.g. its
DisposableBean.destroy() in Spring), we register a handler into some
'close handler' object in the same scope (assuming
application/request/session scopes etc).

The problem is, there's no way to really know for sure what scope the
type/injectee is in. Well we could filter all objects which are
annotated with @Singleton and add the hook for those and repeat for
each scope annotation. The problem is that would not fire for objects
bound to a scope using the module code. e.g.

bind(Foo.class).in(Singleton.class);

As Foo might not be annotated with Singleton.class in this case

I wonder if the InjectionListener should also expose the Binding as
well (which has its Scoping?). Scoping is currently not on the Binding
interface but is on BindingImpl (and I don't mind a hacky cast now and
again :). Maybe we should add getScopeAnnotation() to Binding?

Another problem I have is that given the injectee I've got to look up
the 'close handler' object for the same scope as the injectee so I can
register the handler into the right scope. A scope annotation isn't a
binding annotation, so I can't do
getProvider(Key.get(CloseHandler.class, someScopeAnnotation)).

However if I can find the scope annotation for the injectee, I can
then try to get all the bindings for the "close handler" type - then
find the one with the same scope annotation - then use that one.

You'd hope scopes are fairly small in number; so I could create a sub
class "close handler" for each scope and annotate it with the scope
annotation, then look that up; then I just need to keep a map of scope
annotations to the sub type of the close handler. It'd work around
some limitations; though its a tad icky in that its now hard coded to
the number of scope annotations out there.

I just wondered if folks had any other cunning ideas to implement this
in a less sucky way?

Exposing the binding to the injectee / InjectionListener seems
necessary really; to avoid only implementing types with a scope
annotation.


Or another approach is to just apply some kind of patch like I
submitted before - so there's basically some kinda way to iterate
through all the objects in a scope; afterall each scope stores the
objects in its scope - being able to iterate through those does seem
way simpler and more extensible.

Thoughts?

--
James
-------
http://macstrac.blogspot.com/

Open Source Integration
http://fusesource.com/

Bob Lee

unread,
Apr 3, 2009, 1:45:13 PM4/3/09
to google...@googlegroups.com
I actually have a plan for this! :-)

We can't expose a binding to the injection listener. The listener only knows about injectable types which are orthogonal to bindings.

My plan is to make singleton scope overridable in Guice 2 (using Modules.override() internally). Then, you can bind your own singleton scope implementation that has a destroy() method and looks for @PreDestroy methods. This will work for all singletons, not just those annotated with @Singleton.

Ideally, we'd have a callback on the Scope interface so we could tell an impl at initialization time about all of the bindings in that scope (and you could validate the types, etc.), but this will have to wait for Guice 3.

Bob

James Strachan

unread,
Apr 3, 2009, 7:07:26 PM4/3/09
to google...@googlegroups.com
On 03/04/2009, Bob Lee <craz...@crazybob.org> wrote:
> I actually have a plan for this! :-)
>
> We can't expose a binding to the injection listener. The listener only knows
> about injectable types which are orthogonal to bindings.

OK - I thought that creating an instance to pass into the
InjectionListener would have used the binding to know if it should
even bother creating an instance. I'll take your word for it though
:-)

>
> My plan is to make singleton scope overridable in Guice 2 (using
> Modules.override() internally). Then, you can bind your own singleton scope
> implementation that has a destroy() method and looks for @PreDestroy
> methods. This will work for all singletons, not just those annotated with
> @Singleton.

Would this approach only work for singleton? What about other scopes
like request/session/conversation/test or other wacky scopes folks
might use?


> Ideally, we'd have a callback on the Scope interface so we could tell an
> impl at initialization time about all of the bindings in that scope (and you
> could validate the types, etc.), but this will have to wait for Guice 3.

Would it be so bad for a Scope to have an iterate() method so tools &
frameworks could iterate through all the objects that have been
created in a scope?

Like the Collections framework an implementation could throw an
unsupported exception if the developer found it too hard to implement?
(or implementing Iterable could be optional - or a scope could return
an empty iterator).

I could imagine tools might find it handy being able to easily
enumerate the objects that have been created in a scope. Eg for
tuning, knowing the request scoped objects created might be handy?

As an added bonus anyone can then write any kind of close mechanism on
any kind of scope easily with minimal changes to guice?

It's a while since I did my original lifecycle patch so my memory is
hazy (and am on my cell now so can't easily check the code :-) but I
seem to remember enumerating request/session/test style scopes was
easy - though singleton was quite hard - purely due to very deep
nesting of implementation classes.

Dhanji R. Prasanna

unread,
Apr 3, 2009, 8:16:52 PM4/3/09
to google...@googlegroups.com
On Sat, Apr 4, 2009 at 10:07 AM, James Strachan <james.s...@gmail.com> wrote:

On 03/04/2009, Bob Lee <craz...@crazybob.org> wrote:
> I actually have a plan for this! :-)
>
> We can't expose a binding to the injection listener. The listener only knows
> about injectable types which are orthogonal to bindings.

OK - I thought that creating an instance to pass into the
InjectionListener would have used the binding to know if it should
even bother creating an instance. I'll take your word for it though
:-)

>
> My plan is to make singleton scope overridable in Guice 2 (using
> Modules.override() internally). Then, you can bind your own singleton scope
> implementation that has a destroy() method and looks for @PreDestroy
> methods. This will work for all singletons, not just those annotated with
> @Singleton.

Would this approach only work for singleton? What about other scopes
like request/session/conversation/test or other wacky scopes folks
might use?


> Ideally, we'd have a callback on the Scope interface so we could tell an
> impl at initialization time about all of the bindings in that scope (and you
> could validate the types, etc.), but this will have to wait for Guice 3.

Would it be so bad for a Scope to have an iterate() method so tools &
frameworks could iterate through all the objects that have been
created in a scope?

This would make Scope source incompatible with all Guice 1.0 and transitional code. =(

Also, the way we have designed most scopes now, scope closure is internal to the scope (or perhaps completely orthogonal as in the case of GuiceFilter, where the Filter is totally agnostic to the scope). I think there's a happy compromise with registration of scoped elements using an injection listener or a (re)bound scope.

Dhanji.

James Strachan

unread,
Apr 4, 2009, 2:41:51 AM4/4/09
to google...@googlegroups.com
How would a Scope optionally implementing Iterable cause a problem?


>
> Also, the way we have designed most scopes now, scope closure is internal to
> the scope

Understood - it would be up to the scope to figure out how to iterate
through whatever data structure it's using to cache instances. But
there always is some structure right (apart from NO_SCOPE) - it could
be Http request/session stuff or walking the injectors bindings for
application scope - but a scope must know how to find an object for a
key right?

Like collections, it would be fine having this an optional feature -
over time I hope we could figure out how to implement it for most
implementations?


> (or perhaps completely orthogonal as in the case of GuiceFilter,
> where the Filter is totally agnostic to the scope). I think there's a happy
> compromise with registration of scoped elements using an injection listener
> or a (re)bound scope.
>
> Dhanji.
>
> >
>


Dhanji R. Prasanna

unread,
Apr 4, 2009, 2:57:22 AM4/4/09
to google...@googlegroups.com
On Sat, Apr 4, 2009 at 5:41 PM, James Strachan <james.s...@gmail.com> wrote:

On 04/04/2009, Dhanji R. Prasanna <dha...@gmail.com> wrote:
> On Sat, Apr 4, 2009 at 10:07 AM, James Strachan
> <james.s...@gmail.com>wrote:

>> Would it be so bad for a Scope to have an iterate() method so tools &
>> frameworks could iterate through all the objects that have been
>> created in a scope?
>
>
> This would make Scope source incompatible with all Guice 1.0 and
> transitional code. =(

How would a Scope optionally implementing Iterable cause a problem?

If we add a method to Scope it will break a lot of existing code out there that doesn't implement that method. Not to mention binaries that are compiled against the current version of Scope.

j.u.Collection doesn't allow you to optionally implement a method, it just allows you to throw exceptions in methods that are not supported in a particular implementation.

Dhanji.

Bob Lee

unread,
Apr 4, 2009, 3:04:57 AM4/4/09
to google...@googlegroups.com
On Fri, Apr 3, 2009 at 11:57 PM, Dhanji R. Prasanna <dha...@gmail.com> wrote:
If we add a method to Scope it will break a lot of existing code out there that doesn't implement that method. Not to mention binaries that are compiled against the current version of Scope.

I think James means that a scope may or may not implement Iterable, i.e. he'd do an "instanceof Iterable" at run time.

I'd just assume keep scopes simple though. I'm not a fan of PostConstruct/PreDestroy in general.

Bob

James Strachan

unread,
Apr 4, 2009, 3:57:04 AM4/4/09
to google...@googlegroups.com
Agreed - that's a very elegant solution with no api changes or impact
on scope implementations, if someone can figure out a clever way of
finding the binding/scope for an injectee instance inside the
InjectionListener.

> or a (re)bound scope.
>
> Dhanji.
>
> >
>


James Strachan

unread,
Apr 6, 2009, 6:07:40 AM4/6/09
to google...@googlegroups.com
2009/4/4 Bob Lee <craz...@crazybob.org>:

OK how's this for another idea - one that keeps the scopes nice and
simple, doesn't require hacking the InjectionListener API - or indeed
the public guice API at all, but just requires an optional minor
refactor in each Scope to implement (and a few trivial delegation
methods here and there - more on that later...).

So a Scope by definition of its interface is just a proxy Provider
which takes the Key and the real provider, looks up some cached value
and if its not set, lazily creates it from the real provider. e.g.
something like this..

public <T> Provider<T> scope(Key<T> key, final Provider<T> creator) {
return new Provider<T>() {
public T get() {
T t = getCachedValue(key);
if (t == null) {
t = creator.get();
setCachedValue(key, t);
}
return t;
}
}
}


This is how singleton/request/session scopes all work and I'm sure
pretty much all scopes are like this.

Now from an Injector we can get the Bindings which expose the
Provider. So we already have an easy way to iterate through all the
bindings/scopes - it'd be trivial to filter out all the bindings for a
certain scope or whatever etc.

The only thing that is missing is that from a provider is the ability
to expose the getScopeValue(). i.e. if we introduced an internal
interface (none of this has to be part of the public Guice API/SPI as
its only a couple of tooling methods & frameworks that are gonna use
it) called CachingProvider

/** A scope may implement this interface so it can expose its cached value */
interface CachingProvider<T> extends Provider<T> {
/** Returns the cached value for the given key or null if its not
cached or supported */
T getCachedValue(Key<T> key);
}

then a very minor and optional refactor in scopes would look like this...

public <T> Provider<T> scope(Key<T> key, final Provider<T> creator) {
return new CachingProvider<T>() {
public T get() {
T t = getCachedValue(key);
if (t == null) {
t = creator.get();
setCachedValue(key, t);
}
return t;
}
}
public T getCachedValue(Key<T> key) {
...
}
}

So no new iteration required on the scopes; we purely have an optional
interface on the providers the scopes create.

The only issue remaining now is that this provider gets wrapped a few
times before its put into the binding; so we'd need to add a few
delegates of CachingProvider on the proxies & facades that are invoked
before the scope's provider is. Then anyone who knows this trick could
iterate through all the objects in a scope if they want - or folks
could implement their own close/pre-destroy hooks above Guice - while
keeping the core of Guice nice and clean and Bob doesn't have to ever
see a pre-destroy hook :).

I've created a patch for this to show how trivial it is which works
for singleton/request/session with very minimal changes to those
scopes...
http://code.google.com/p/google-guice/issues/detail?id=354

Thoughts?

Reply all
Reply to author
Forward
0 new messages