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

Allow user-provided "autobinders" #49

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

Allow user-provided "autobinders" #49

gissuebot opened this issue Jul 7, 2014 · 27 comments

Comments

@gissuebot
Copy link

From kevinb9n on February 28, 2007 17:09:35

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: http://code.google.com/p/google-guice/issues/detail?id=49

@gissuebot
Copy link
Author

From kevinb9n on March 05, 2007 14:49:19

I just had a realization: we could allow specification of a "MissingBindingHandler"
or somesuch.

@gissuebot
Copy link
Author

From kevinb9n on March 05, 2007 15:09:38

(No comment was entered for this change.)

Summary: MissingBindingHandler

@gissuebot
Copy link
Author

From kevinb9n on March 08, 2007 14:14:42

(No comment was entered for this change.)

Labels: 1.1

@gissuebot
Copy link
Author

From kevinb9n on March 13, 2007 17:30:41

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.

@gissuebot
Copy link
Author

From kevinb9n on March 19, 2007 10:58:32

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

Labels: -1.1

@gissuebot
Copy link
Author

From ajoo.email on March 19, 2007 11:43:00

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.

@gissuebot
Copy link
Author

From kevinb9n on March 19, 2007 12:56:19

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. :)

Summary: MissingBindingHandler / a Provider-Provider

@gissuebot
Copy link
Author

From kevinb9n on March 19, 2007 14:47:30

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.

@gissuebot
Copy link
Author

From kevinb9n on June 03, 2007 11:25:03

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")!

Summary: Allow user-provided "autobinders"
Labels: -Type-Defect Type-Enhancement

@gissuebot
Copy link
Author

From crazyboblee on June 04, 2007 16:42:46

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?

@gissuebot
Copy link
Author

From crazyboblee on June 04, 2007 16:45:27

Oh, you pass it a LinkedBindingBuilder, not a full Binder. Interesting.

@gissuebot
Copy link
Author

From gili.tzabari on June 06, 2007 06:02:28

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?

@gissuebot
Copy link
Author

From kevinb9n on June 08, 2007 12:35:45

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

@gissuebot
Copy link
Author

From mcculls on July 23, 2007 01:28:26

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.

@gissuebot
Copy link
Author

From edward.costello@orionhealth.com on October 18, 2007 21:36:08

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);
      }
    }
  }

Attachment: gist
   autobinder.patch

@gissuebot
Copy link
Author

From edward.costello@orionhealth.com on October 23, 2007 13:40:38

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.

@gissuebot
Copy link
Author

From edward.costello@orionhealth.com on October 23, 2007 19:02:18

>> 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.

Attachment: gist
   autobinder.patch

@gissuebot
Copy link
Author

From mcculls on November 05, 2007 23:48:52

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.

Attachment: gist
   GUICE_ISSUE_49_PATCH_SUGGESTION_20071106.txt

@gissuebot
Copy link
Author

From mcculls on November 06, 2007 19:00:03

Updated patch: exceptions thrown from BindingFactories are now logged in the Binder
using addError.

Attachment: gist
   GUICE_ISSUE_49_PATCH_SUGGESTION_20071107.txt

@gissuebot
Copy link
Author

From mcculls on November 28, 2007 23:35:03

My latest prototype patch for this issue is available from: http://peaberry.googlecode.com/svn/trunk/patch/BindingFactory.txt

@gissuebot
Copy link
Author

From mcculls on March 31, 2008 00:49:07

some design doc: https://code.google.com/p/peaberry/wiki/Patch_BindingFactory

@gissuebot
Copy link
Author

From rcouto on July 28, 2008 07:45:38

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.

@gissuebot
Copy link
Author

From erik.putrycz on November 19, 2008 14:15:56

What is the status of this? The binderfactory would be a great feature.

@gissuebot
Copy link
Author

From crazyboblee on November 19, 2008 14:18:58

No plans to support this at the moment.

Summary: Allow user-provided "autobinders"

@gissuebot
Copy link
Author

From sberlin on February 19, 2011 12:25:02

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).

@gissuebot
Copy link
Author

From sberlin on February 19, 2011 12:28:57

(No comment was entered for this change.)

Status: WontFix

@elandau
Copy link

elandau commented Aug 19, 2016

Any chance this issue could be re-opened for discussion. I've prototyped an auto-binding implementation using the ElementVisitor SPI but it requires custom processing of the elements prior to creating the injector and does not work with just in time binding. It would be nice to be able to register a MissingBindingHandler with a matcher (similar to the bindListener API) that can be added via the normal path of installing modules.

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