DWR

Tim can you take a look at this?

I was not able to get the DWR-Guice integration module to work with Guice 2.0. I don't think this is the only problem I encountered.

I'm not sure what the issue creator means by "I think that they don't make sense there, because you already bind them in the beginning of the method." There are different bindings in the beginning of the method.

The changes proposed here could break existing code.

Sorry about that, I was sleepy and I did a mistake, actually there is no problem with the bindPotentiallyConflictingTypes, the problem is with the DwrGuiceServletModule class, line 112. This line makes dwr incompatible with google-guice 2.0. It generates the following error:

1) A binding to javax.servlet.ServletContext was already configured at org.directwebremoting.guice.DwrGuiceServletModule.configure(DwrGuiceServletModule.java:112).
at com.google.inject.servlet.InternalServletModule.configure(InternalServletModule.java:71)

I don't know exactly the structure of DWR, but I would suggest to add a annotatedWith version and to put this line also inside the IF of line 76. I did this change and now it's everything work perfect. I've also attached a patch, please take a look on it.

Thanks

I think this patch only goes halfway. The point of bindPotentiallyConflictingTypes and related logic is to allow users not to worry about whether they are including the guice-servlet jar in addition to DWR. This patch would break any existing code that was expecting to have ServletContext injected (without an annotation).

How about adding the (un-annotated) ServletContext binding to the if-clause of bindPotentiallyConflictingTypes? I don't have time to make a patch, though.

Note that bindPotentiallyConflictingTypes, unless overridden by the user, will be false if LocalUtil.classForName("com.google.inject.servlet.ServletModule") returns true. See AbstractDwrModule.java for details.

Yes, that's why my patch work and will not break any source code. If the user has guice-servlet in the classpath, DWR will set the context just with the Dwr annotation, otherwiser DWR will set the context with and without annotation, so it will not break existing code.

Ah, sorry – I didn't read your patch carefully enough. I don't have time to test it, but it looks good to me.

Can we get this fixed and committed? We are having to override the dwr code to get this to work with Guice 2. Its very messy and makes starting new projects based on our tech stack cumbersome.

Has anyone besides Eduardo tried this patch? Six months on, I can't dredge up enough context to evaluate this quickly, but it looks as though back then I thought the patch made sense.

No, but if Jason agrees I can put the patch in and he can test it in one of our development builds.

I can test it if it makes it into the dev snapshot.

Thank you Jason. Sorry this took so long. I just applied the patch and checked it in. It is in the latest dev build (79):
http://ci.directwebremoting.org/bamboo/browse/DWR-TRUNK-79/artifact

Anyone?