frjaeger220 / google-guice

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

Assisted-injection does not validate that factory methods will work in TOOL stage #445

Open GoogleCodeExporter opened 9 years ago

GoogleCodeExporter commented 9 years ago
Normally, creating an injector in TOOL stage fails if not all the explicitly 
bound keys can be instantiated, which is great for testing that a given set 
of modules composes a "complete" injector.

But FactoryProvider2 validates that its dependencies are bound using an 
@Inject method, which isn't called in Stage.TOOL.

This means that tests cannot rely on creating an injector in TOOL stage to 
validate that the injector is "complete".

Original issue reported on code.google.com by net...@gmail.com on 11 Nov 2009 at 4:25

GoogleCodeExporter commented 9 years ago
Yea =(

Are you going to work on this?

Original comment by dha...@gmail.com on 11 Nov 2009 at 11:36

GoogleCodeExporter commented 9 years ago
I looked through it a bit; I'll take a shot.

Original comment by net...@gmail.com on 12 Nov 2009 at 12:55

GoogleCodeExporter commented 9 years ago
I'm not sure what the fix is. The same problem exists in Multibindings. The 
only 
reasonable solution would be for TOOL stage to perform known-safe injections. 
Perhaps we use a method naming convention like "@Inject private void 
toolStageInject(...) {...}" and make it so such injections are performed in 
tool stage...

Original comment by limpbizkit on 13 Nov 2009 at 5:18

GoogleCodeExporter commented 9 years ago
But even in TOOL stage, some validation is performed. The injector creation 
fails when 
a type is bound that has a dependency to an unbound type. How does that work? I 
had 
assumed it was something with HasDependency, but apparently not.

Original comment by net...@gmail.com on 13 Nov 2009 at 5:20

GoogleCodeExporter commented 9 years ago
What about an additional annotation @Toolable that Tool stage looks for on
@Injectable methods.

Original comment by sberlin on 13 Nov 2009 at 5:23

GoogleCodeExporter commented 9 years ago
Or how about 

public interface ToolStageValidatable {
  void toolStageValidate(Injector injector, Errors errors) throws ErrorException;
}

That's better than a naming convention?

Original comment by net...@gmail.com on 13 Nov 2009 at 8:31

GoogleCodeExporter commented 9 years ago
In tool stage the only user-code Guice runs is Module.configure(). It doesn't 
call 
constructors, build singletons, or perform any member injections.

My preferred fix is to find a way for certain injections to mark themselves as 
tool-
friendly. I like Sam's idea of just using @Toolable on a regular @Inject 
method. I also 
like a naming convention because it doesn't pollute the public API with an 
annotation 
that almost zero people should ever use. Especially since if they do use it, 
they can 
break tool stage.

Whomever wants to do the fix: take your pick (annotation vs. naming 
convention), 
add isToolable() to InjectionPoint, and wire in tool stage to perform those 
injections.

Original comment by jessewil...@google.com on 5 Feb 2010 at 4:01

GoogleCodeExporter commented 9 years ago
I'll take a stab on it.  Have a lot of time on my hands for a month or so. :-)

Original comment by sberlin on 5 Feb 2010 at 4:15

GoogleCodeExporter commented 9 years ago
committed changes in r1140 (
http://code.google.com/p/google-guice/source/detail?r=1140 ).  went with the
@Toolable route, but feel free to change it to a name-pattern -- i felt 
@Toolable was
the least intrusive and easiest to scan projects for.  (hard to do regex 
matching on
names to find everything that would have toolable.)  pretty straightforward 
change,
with possible the scariest bit being inside MembersInjectorImpl.

Original comment by sberlin on 5 Feb 2010 at 9:14