pombreda / google-guice

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

Allow user-provided "autobinders" #49

Open GoogleCodeExporter opened 9 years ago

GoogleCodeExporter commented 9 years ago
It would be helpful if we could configure an injector in a special mode
where whenever it encounters a request for a type it doesn't have bound, it
will just create a dummy for that type.  It could do this with Proxy or
cglib or easy mock or whatever.

Original issue reported on code.google.com by kevin...@gmail.com on 28 Feb 2007 at 10:09

GoogleCodeExporter commented 9 years ago
I just had a realization: we could allow specification of a 
"MissingBindingHandler"
or somesuch.  

Original comment by kevin...@gmail.com on 5 Mar 2007 at 10:49

GoogleCodeExporter commented 9 years ago

Original comment by kevin...@gmail.com on 5 Mar 2007 at 11:09

GoogleCodeExporter commented 9 years ago

Original comment by kevin...@gmail.com on 8 Mar 2007 at 10:14

GoogleCodeExporter commented 9 years ago
TODO: have to cycle through 9 different possible names for this thing before 
finally
settling on one.  It's kind of a "provider provider". :-)

This seems limited by the fact that you could have only one of these per 
injector. 
Alternatively, we could register them by package or something.  I don't know.

Original comment by kevin...@gmail.com on 14 Mar 2007 at 12:30

GoogleCodeExporter commented 9 years ago
needs further thought; may be a better way to solve this like an alternate 
Binder
implementation; this has potential for abuse

Original comment by kevin...@gmail.com on 19 Mar 2007 at 5:58

GoogleCodeExporter commented 9 years ago
Is this the same as the "default impl resolver" proposal?

I really think it is very valuable. And not having it is a real pain to our 
project.

btw, I have an implementation of ImplementationResolver on my pc. I can share 
that
for reference if needed.

Original comment by ajoo.em...@gmail.com on 19 Mar 2007 at 6:43

GoogleCodeExporter commented 9 years ago
I think it may be the same.  It's a provider of providers.  e.g., if we don't 
find a
provider/implementation for a Key, and the Key matches some particular Matcher, 
then
we'd dynamically generate a provider using your user-supplied class.

Bob and I are concerned about this one.  It carries some risk of opening the
floodgates to a wide variety of customizations to guice that result in 
applications
being decidedly un-guicy, by which I mean even someone who already understands 
guice
could switch projects to another guice-based project and look at the code and 
think,
"what the bollocks is going on here?"

That's a negative.  And of course, all other things being equal, more power in 
the
framework is positive.  I wish there were a simple objective way to evaluate
decisions like this, but I don't think there's anything of the sort.  Though 
once
again, what works better than anything is to barrage us with compelling, valid,
real-world use cases of code using the feature for good, not evil.

My use case: when using a one-off Injector to wire a small number of objects 
together
for a test case, I'd like the missing bindings to be automatically provided by 
simple
dead-stupid Proxies.  Bob's response to this use case is that he has a better 
idea
for how to support it.  Bob, your turn. :)

Original comment by kevin...@gmail.com on 19 Mar 2007 at 7:56

GoogleCodeExporter commented 9 years ago
Ben <ajoo.email@gmail.com> to google-guice
------------------------------------------

I wrote up my use case before. But maybe this is a better place for
it. So here it goes:

We have a company-wide naming convention that a implementation class
of an interface XYZ is created in the same package and is named
XYZImpl. (We have other variants of the convention too, like using a
separate package for impl classes, but I don't like that anyway)

We'd like to be able to avoid writing up hundreds of lines of
"bind(XYZ.class).to(XYZImpl.class)"  in the module. That's no fun and
tedious.

Of course we could create some kind of loop like:
for(Class itf: interfacesWithDeaultImplClassFollowingTheNamingConvention) {
 Class defaultImpl = ...;
 bind(itf).to(defaultImpl);
}

But this is not as convenient as not having to write anything at all.
We will have to do more maintainance work to make sure all XYZ
interfaces that have an XYZImpl class are bound. It also suffers from
the "binding not overridable" restriction.

As for the risk, I don't think I understand the risk yet. Can you name
a use case where a default resolver can be reasonably abused to create
confusion?

To me,

public interface ImplementationResolver {
 <T> Provider<T> resolve(Class<T> itf);
}

Injector injector = Guice.createInjector(new MyDeafultImplResolver(),
module1, module2);

looks type-safe and not confusing at all. It is quite possible that a
few well-built common default resolver classes are provided out-of-box
and very rarely one would ever need to create his own resolver.

Original comment by kevin...@gmail.com on 19 Mar 2007 at 9:47

GoogleCodeExporter commented 9 years ago
I'm tentatively naming this feature "autobinders".

An autobinder receives a Key and a Binder, and can provide a binding for the 
key to
the binder.  The built-in support for "implicit bindings", @ImplementedBy and
@ProvidedBy can all be considered as simple bundled autobinders (and could
potentially be implemented that way).  However, for user-provided autobinders, 
we
must consider whether they should be permitted to add new bindings to an 
existing
injector (as the three core autobinders mentioned above are currently able to 
do). 
The choices are: disallow, allow, make an option on the autobinder or its
registration, an option in the injector; and then there is a choice for whether 
to
modify the existing support for the "core autobinders" to follow the same logic 
or not.

My preference would be to make the injector configuration immutable as much as
possible, with a simple way to make an exception for places that need it in as 
local
a way as possible (e.g. for a particular autobinder?).

Here's a sketch.

  interface Autobinder<B> {
    <T extends B> boolean autobind(Key<T> key, LinkedBindingBuilder<T> bindThisKey);
  }

In this way a particular autobinder can be restricted for use only with 
subtypes of a
given base type (B), which can always be Object if you want.  The method 
returns true
if it was successfully able to make the binding; false if the injector should
continue to try the next autobinder in precedence order (questionable).

Then we tell Guice about this autobinder thus:

  public class MyModule implements Module {
    public void configure(Binder binder) {
      autobind(Action.class)
          .annotatedWith(Foo.class)
          .using(MyActionAutobinder.class);
      }
    }
  }

I believe that this feature as currently described is a generalization of what 
I was
describing already in this issue.  As well, I'm hopeful that this would obviate 
the
need for issue 27 (giving providers access to the "Injectee")!

Original comment by kevin...@gmail.com on 3 Jun 2007 at 6:25

GoogleCodeExporter commented 9 years ago
Could the autobinder interface take a Key<T> and return a Provider<T> instead? 
Why
does your example bind to "@Foo Action"? I got the impression that autobinders 
would
be able to return providers for a variety of keys. Is your intention to match
subclasses? Perhaps you should bind an autobinder using Matcher<Key> instead. 
Also,
how do you plan to handle ordering of autobinders?

Original comment by crazybob...@gmail.com on 4 Jun 2007 at 11:42

GoogleCodeExporter commented 9 years ago
Oh, you pass it a LinkedBindingBuilder, not a full Binder. Interesting.

Original comment by crazybob...@gmail.com on 4 Jun 2007 at 11:45

GoogleCodeExporter commented 9 years ago
Sorry, I'm not sure I fully understand this feature and its usefulness yet. Can
someone please give a clearer example (hopefully with a code sniplet) of where 
you
would use this and why the existing mechanism isn't good enough?

Original comment by gili.tza...@gmail.com on 6 Jun 2007 at 1:02

GoogleCodeExporter commented 9 years ago
todo: I will add links to all the mailing list threads containing problems that 
would
have been solved by this.

Original comment by kevin...@gmail.com on 8 Jun 2007 at 7:35

GoogleCodeExporter commented 9 years ago
what's the outlook for this issue?  I'm willing to pitch in and help 
prototype/test this, as I'd find it very useful.
at the moment I've patched my local copy of google-guice with a very basic 
implementation of autobinding:

  1) BindingAnnotations can be marked as @Autobinding

  2) if the injector cannot find an InternalFactory for a member+key, it checks the key's BindingAnnotation

  3) if the BindingAnnotation has @Autobinding, it creates a new InternalFactory to do the autobinding

  4) the autobinding factory looks for an implementation of Autobinder with the same binding annotation

  5) when the factory is invoked, it calls the Autobinder impl with the original key and returns the result

this does enough for my needs, and only requires a minor patch to the trunk - 
but isn't as flexible as Kevin's 
design. However, I don't mind putting in some cycles to get the official design 
implemented and into trunk.

Original comment by mccu...@gmail.com on 23 Jul 2007 at 8:28

GoogleCodeExporter commented 9 years ago
I'm attaching a patch that implements this feature (as I need and understant 
it).
This patch is against the 1.0 tag.

This is slightly different from what was initially suggested. Autobinders are 
tied to
a binding annotation rather than a superclass. E.g. to lookup OSGI services 
you'd
define an @OSGIService binding annotation. You then associate an autobinder 
with this
in order to automatically creating these bindings.

Explict bindings take precedence over those from an autobinder.

The new interfaces are basically:

  interface Autobinder {
    <T> boolean autobind(Key<T> key, LinkedBindingBuilder<T> bindThisKey);
  }

  public class MyModule implements Module {
    public void configure(Binder binder) {
      binder.autobind(Foo.class)
          .using(MyActionAutobinder.class);
      }
    }
  }

Original comment by edward.c...@orionhealth.com on 19 Oct 2007 at 4:36

Attachments:

GoogleCodeExporter commented 9 years ago
Over the weekend I found a couple of bugs with the patch
* When an autobinder returns false (indicating no binding) the linked binding 
builder
stays associated with the binder. This means if the bindings are updated at some
future point (e.g. another autobinder call finds a binding) a half completed 
binding
will be constructed. I believe the fix to this is to only add the linked binding
buider to the binder when the autobinder returns true.
* This patch isn't thread safe. When the injector is updated by an autobinder it
could still be being accessed by other threads. The simplest fix to this is to 
block
access to the injector while it's being updated. This isn't as bad as it sounds 
since
the result of an autobinding is cached, so the injector is only updated when new
bindings are discovered.

Original comment by edward.c...@orionhealth.com on 23 Oct 2007 at 8:40

GoogleCodeExporter commented 9 years ago
[deleted comment]
GoogleCodeExporter commented 9 years ago
>> Updated previous patch was broken <<

Updated Version of the Patch Fixing the two above issues.

Basically now the bindings added by an autobinder will block on the getProvider 
or
getInternalFactory methods until injection of the underlying object is 
complete. This
fixes the thread saftey issue.

The other issue is fixed by only adding the binding builder if the autobinder 
returns
true.

Original comment by edward.c...@orionhealth.com on 24 Oct 2007 at 2:02

Attachments:

GoogleCodeExporter commented 9 years ago
New patch suggestion based on discussions with Bob, Kevin, Ed, and others. 
Missing
bindings that match dependency matchers are looked up during Injector creation 
and
cannot be changed once the injector is created.

  Binder :
  void bind(Matcher<? super Dependency<?>> dependencyMatcher,
      BindingFactory<?> bindingFactory);

  BindingFactory<B> :
  <T extends B> boolean bind(Dependency<T> dependency,
      LinkedBindingBuilder<T> linkedBindingBuilder);

Patch needs bit more javadoc and additional unit tests, but should work as 
designed.

Original comment by mccu...@gmail.com on 6 Nov 2007 at 7:48

Attachments:

GoogleCodeExporter commented 9 years ago
Updated patch: exceptions thrown from BindingFactories are now logged in the 
Binder
using addError.

Original comment by mccu...@gmail.com on 7 Nov 2007 at 3:00

Attachments:

GoogleCodeExporter commented 9 years ago
My latest prototype patch for this issue is available from:

   http://peaberry.googlecode.com/svn/trunk/patch/BindingFactory.txt

Original comment by mccu...@gmail.com on 29 Nov 2007 at 7:35

GoogleCodeExporter commented 9 years ago
[deleted comment]
GoogleCodeExporter commented 9 years ago
some design doc: http://code.google.com/p/peaberry/wiki/Patch_BindingFactory

Original comment by mccu...@gmail.com on 31 Mar 2008 at 7:49

GoogleCodeExporter commented 9 years ago
Would this be a solution to generic injection issue (id: 52)? Will the 
Dependency<T>
class have the information about generic types of members that are being 
injected?

Trying to be a bit more clear, if my class is Foo<T> and I have a field Bar<T>, 
could
I implement an BindingFactory and corresponding Matcher such that if I have a 
class
Concrete that has Foo<String> as a field, I can dynamically create a binding for
Bar<String> as well?

Currently, nasty things happen because TypeLiteral does not support 
TypeVariable nor
WildcardType.

Original comment by rco...@gmail.com on 28 Jul 2008 at 2:45

GoogleCodeExporter commented 9 years ago
What is the status of this? The binderfactory would be a great feature.

Original comment by erik.put...@gmail.com on 19 Nov 2008 at 10:15

GoogleCodeExporter commented 9 years ago
No plans to support this at the moment.

Original comment by crazybob...@gmail.com on 19 Nov 2008 at 10:18

GoogleCodeExporter commented 9 years ago
Same resolution as issue 27 - It's been 4 years since the issue was opened, 
Guice has seen wide use, and the general consensus thus far has been to 
purposely not expose this feature.  It makes debugging even harder than it 
already is (which is the #1 pain point with Guice).

Original comment by sberlin on 19 Feb 2011 at 8:25

GoogleCodeExporter commented 9 years ago

Original comment by sberlin on 19 Feb 2011 at 8:28