Second iteration on GIN extensions

This patch includes everything from:
http://codereview.appspot.com/3269042/

Modifications include a mechanism for associating a GinModuleExtension with the GinModule-s that use it. This ensures a GinModuleExtension is only install()-ed with its module, and it removes the need for an isInstance filter in GinModuleExtension.filter (still a good practice to do them for error checking.)

[philippe.beaudoin, 2010-11-25 05:18:24]

http://codereview.appspot.com/3297042/diff/1/src/com/google/gwt/inject/rebind...
File src/com/google/gwt/inject/rebind/ext/FactoryModuleExtension.java (right):

http://codereview.appspot.com/3297042/diff/1/src/com/google/gwt/inject/rebind...
src/com/google/gwt/inject/rebind/ext/FactoryModuleExtension.java:37: if (module
instanceof FactoryModule<?>) {
We probably want to replace this check with:
  if (FactoryModule.class.isAssignableFrom(module.getClass())
to allow user's to subclass FactoryModule (for example, to add state information
and use it in more than one extension).

[t.broyer, 2010-11-25 14:37:49]

I think the overall workflow could be:
1. collect modules (in createModules and BinderAdapter#install)
2. for each module: extract GinProviderExtension-s and GinCreatorExtension-s,
and invoke GinModuleExtension-s
3. now proceed to creating implicit bindings for unresolved keys (making use of
the collected GinProviderExtension-s and GinCreatorExtension-s)

http://codereview.appspot.com/3297042/diff/1/src/com/google/gwt/inject/rebind...
File src/com/google/gwt/inject/rebind/BindingsProcessor.java (right):

http://codereview.appspot.com/3297042/diff/1/src/com/google/gwt/inject/rebind...
src/com/google/gwt/inject/rebind/BindingsProcessor.java:186: private final
Map<GinModule, Set<Class<? extends GinModuleExtension>>>
It means every GinModule has to not break the hashCode/equals contract (unlikely
to happen, but still).
Given that:
 - it's unlikely that the same module instance be installed twice
 - it could be considered an error (in most cases, it would register bindings
twice)
 - it would be harmless wrt GinModuleExtension-s, and if it's not it would
probably be because of duplicate bindings (see above)
Given all that, I propose to use a list of
GinModule-Set<Class<GinModuleExtension>> pairs; but it actually boils done to
simply using the ginModules list and deferring the module-to-extension-classes
mapping to the time we'll use them; which is essentially what we do in
createBindingsForModuleExtensions (i.e. merge populateExtensionsFromModule into
getExtensionsForModule and remove populateExtensionsFromInstalledModules).

http://codereview.appspot.com/3297042/diff/1/src/com/google/gwt/inject/rebind...
src/com/google/gwt/inject/rebind/BindingsProcessor.java:505: Set<Class<? extends
GinModuleExtension>> moduleExtensions =
Move outside the for-loop.

http://codereview.appspot.com/3297042/diff/1/src/com/google/gwt/inject/rebind...
File src/com/google/gwt/inject/rebind/ext/FactoryModuleExtension.java (right):

http://codereview.appspot.com/3297042/diff/1/src/com/google/gwt/inject/rebind...
src/com/google/gwt/inject/rebind/ext/FactoryModuleExtension.java:37: if (module
instanceof FactoryModule<?>) {
On 2010/11/25 05:18:24, philippe.beaudoin wrote:
> We probably want to replace this check with:
>   if (FactoryModule.class.isAssignableFrom(module.getClass())
> to allow user's to subclass FactoryModule (for example, to add state
information
> and use it in more than one extension).

Isn't module instanceof FactoryModule<?> exactly the same as
FactoryModule.class.isAssignableFrom(module.getClass()) ?!
(i.e. it's *not* an .equals())

[t.broyer, 2010-11-26 13:29:00]

http://codereview.appspot.com/3297042/diff/1/src/com/google/gwt/inject/rebind...
File src/com/google/gwt/inject/rebind/BindingsProcessor.java (right):

http://codereview.appspot.com/3297042/diff/1/src/com/google/gwt/inject/rebind...
src/com/google/gwt/inject/rebind/BindingsProcessor.java:186: private final
Map<GinModule, Set<Class<? extends GinModuleExtension>>>
On 2010/11/25 14:37:49, t.broyer wrote:
> It means every GinModule has to not break the hashCode/equals contract
(unlikely
> to happen, but still).

Actually, it seems to be a feature of Guice that modules that compare equals are
installed once only (see Elements.RecordingBinder –the only implementation of
the Binder interface– which uses a Set<Module> and explicitly tests
modules.add() in the install() method, which BTW is also used when creating an
Injector:
http://code.google.com/p/google-guice/source/browse/trunk/core/src/com/google...
)

So not only it is safe to use a Map, but it's the "expected behavior" (even
though I'm unable to find where it's documented).

This feature is used by Multibinder so that multiple modules can contribute to a
single Set with a given Key<?>, without duplicate bindings from the
RealMultibinder#configure().