Guice servlet module crashed when used with Elements.getElements()

[GoogleCodeExporter]

To reproduice:

1. Create a web.xml with a Guice servlet config containing at least one filter 
mapping.

2. Before creating the injector use Elements.elements() to insepect all the 
modules. I.e.

In my case i need to search if a binding has been defined for a specific type:

for (Element element : Elements.getElements(stage, modules)) {
    Boolean res = element.acceptVisitor(new DefaultElementVisitor<Boolean>() {
        @Override
        public <T> Boolean visit(Binding<T> binding) {
            return key.equals(binding.getKey());
        }
    });
    if (res != null && res)
        return true;
}

3. Then create the injector with the modules you have introspected.

ServletsModuleBuilder crashed there:

Set<String> servletUris = Sets.newHashSet();
for (ServletDefinition servletDefinition : servletDefinitions) {
  if (servletUris.contains(servletDefinition.getPattern())) {
    addError("More than one servlet was mapped to the same URI pattern: "
        + servletDefinition.getPattern());
  }
  else {
    bind(Key.get(ServletDefinition.class, UniqueAnnotations.create())).toProvider(servletDefinition);
    servletUris.add(servletDefinition.getPattern());
  }
}

When introspecting, servletDefinitions have been added to the module's instance 
variable. So when the injector goes through the module again, it launches an 
error saying that the servletDefinitions already exist.

This is quite critical for use... Would it be possible to fix or is there at 
this time a workaround for this ?

NB: we don't have the possibllity to instanciate the modules again.

Original issue reported on code.google.com by mathieu....@gmail.com on 24 Sep 2010 at 2:09

[GoogleCodeExporter]

Perhaps a clear of the two lists in ServletsModuleBuilder and 
FiltersModuleBuilder should be done before calling  configureServlets(); in 
ServletModule

Original comment by mathieu....@gmail.com on 24 Sep 2010 at 2:26

[GoogleCodeExporter]

Here is a quick fix: cretae this class:

import com.google.inject.Module;
import com.google.inject.servlet.ServletModule;

import java.lang.reflect.Field;
import java.util.List;

/**
 * FIX BUG http://code.google.com/p/google-guice/issues/detail?id=544
 * TODO: REMOVE THIS WHEN FIXED
 */
public final class ServletModuleFix {
    public static void fix(ServletModule me) {
        try {
            Field servletsModuleBuilder = ServletModule.class.getDeclaredField("servletsModuleBuilder");
            servletsModuleBuilder.setAccessible(true);

            Field filtersModuleBuilder = ServletModule.class.getDeclaredField("filtersModuleBuilder");
            filtersModuleBuilder.setAccessible(true);

            Module servletsModule = (Module) servletsModuleBuilder.get(me);
            Module filtersModule = (Module) filtersModuleBuilder.get(me);

            for (Module module : new Module[]{servletsModule, filtersModule}) {
                for (Field field : module.getClass().getDeclaredFields()) {
                    if (List.class.isAssignableFrom(field.getType())) {
                        field.setAccessible(true);
                        ((List) field.get(module)).clear();
                    }
                }
            }
        } catch (Exception e) {
            throw new RuntimeException(e.getMessage(), e);
        }
    }
}

And use it in your servlets modules like this:

public class RestModule extends ServletModule {
    @Override
    protected void configureServlets() {
        ServletModuleFix.fix(this);
        [...]
    }
}

Original comment by mathieu....@gmail.com on 24 Sep 2010 at 2:36

[GoogleCodeExporter]

Is this a problem with Guice 2.0, or Guice SVN head?  If 2.0, can you confirm 
if it's still a problen with SVN?  If SVN, can you confirm if it's a regression 
since 2.0?

Thanks!

Original comment by sberlin on 24 Sep 2010 at 2:54

[GoogleCodeExporter]

Oups sorry!

We are using the trunk we've built on the 7th september 2010

Original comment by mathieu....@gmail.com on 24 Sep 2010 at 3:00

[GoogleCodeExporter]

Note that for the regression, the issue is not specific to guice-servlets. All 
Guice modules using states like ServletsModuleBuilder have this potential crash 
issue because the introspection through Elemnts.getElements() could modify 
these states.

This is the case for:

http://code.google.com/p/googl-guice/source/browse/trunk/extensions/servlet/src/
com/google/inject/servlet/ServletsModuleBuilder.java

If i check the revisions of this class, the bug has been introduced by this 
commit:

r802        
Error checking for duplicate URI pattern mapping (servlets only).
Jan 06, 2009    
dhanji

This is when the check has been added. 

But to my mind, this is not a bad commit: I think this check MUST occur. 

My point is that before calling the configureServlets() method, you first 
should ensure that the two lists are empty.

Original comment by mathieu....@gmail.com on 24 Sep 2010 at 3:09

[GoogleCodeExporter]

Got it, thanks for the awesome details.

Original comment by sberlin on 24 Sep 2010 at 3:40

[GoogleCodeExporter]

Also, a workaround you can use for now is to use Elements.getModule(elements) 
to recreate a module out of the Elements instead of reusing the existing 
modules.

Original comment by sberlin on 24 Sep 2010 at 4:32

[GoogleCodeExporter]

I've tried, but it does not work. A lot of exceptions are thrown within the 
injector because of bindings not found.

Original comment by mathieu....@gmail.com on 24 Sep 2010 at 5:02

[GoogleCodeExporter]

fixed in r1273 -- thanks for pointing out the problem!

Original comment by sberlin on 26 Sep 2010 at 10:06

• Changed state: Fixed

[GoogleCodeExporter]

Hi,

I've built guice from trunk a and its working fine for me in our project.

Thank you !

Original comment by mathieu....@gmail.com on 27 Sep 2010 at 4:08