vipx / google-guice

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

Integrate Warp-Servlet as com.google.inject.servlet #239

Open GoogleCodeExporter opened 9 years ago

GoogleCodeExporter commented 9 years ago
Dhanji's contributing Warp::Servlet as v2 of the Guice Servlet extension. This 
will replace (and build 
on) our current InjectedHttpServlet code.
   http://www.wideplay.com/warp::servlet

Original issue reported on code.google.com by limpbizkit on 26 Aug 2008 at 12:05

GoogleCodeExporter commented 9 years ago
Great news! Let me mention two things that are on my wish list:
  * Support for multiple WS modules per injector: this is crucial for third party
extensions, for example my HttpInvoker port:
http://code.google.com/p/garbagecollected/wiki/GuiceHttpInvoker
  * A revised DSL (optional). I don't like the filters() in:
Servlets.configure().filters().servlets().serve("*.html").with(MyServlet.class)

We might want to consider these changes before we release. Also, will WS follow 
its
own release schedule?

Original comment by robbie.v...@gmail.com on 26 Aug 2008 at 10:40

GoogleCodeExporter commented 9 years ago

Original comment by limpbizkit on 31 Aug 2008 at 2:23

GoogleCodeExporter commented 9 years ago
Dhanji and I have discussed this at length. Our plan is:

1. Axe InjectedHttpServlet and GuiceServletContextListener. These weren't in 
the 1.0 release, so they won't be 
missed. But it leaves a hole for getting servlets injected.

2. Add warp-servlet's dispatcher builder class to our servlet extension. This 
is an alternative to web.xml that 
allows servlets to be instantiated by the Injector rather than by web.xml's 
parser.

3. The dispatcher module will request static injection of GuiceFilter. That's 
the hook to delegate to either the 
warp-configured-servlets or the standard servlets. If both match, it'll explode 
rather than arbitrating. The 
static injection is admittedly lame, but no less lame than web.xml (which is 
also static).

Original comment by limpbizkit on 27 Nov 2008 at 10:14

GoogleCodeExporter commented 9 years ago
4. It will also explode if multiple injectors attempt to statically inject the 
same GuiceFilter (obviously).

As for Robbie's comment, Im disinclined to have multiple modules support 
because then you don't know where 
all your servlets are from one place. You can easily support an RPC mechanism 
by asking the user to map one 
blanket url to your RPC listener:

serve("/rpc/*").with(RpcServlet.class)

Original comment by dha...@gmail.com on 27 Nov 2008 at 10:24

GoogleCodeExporter commented 9 years ago
I'd like to support registering servlets from multiple modules. That allows 
applications as plugin-style 
components, for example to allow the billing plugin and campaign management 
plugins to be deployed 
independently or as a unit.

I've got a secret technique so that you can do this without having to 
coordinate in advance:
http://publicobject.com/2008/05/elite-guice-2-binding-de-duplication.html

Original comment by limpbizkit on 28 Nov 2008 at 1:45

GoogleCodeExporter commented 9 years ago
Unfortunately binding deduping will not work in this case, since we have to 
resolve partial path matches based 
on both regex and servlet-style pattern grammars.

Here's what I mean:
serve("*.html").with (..)

and elsewhere
serve("/xx").with(..)

if my URI is /xx.html, it will match both. To detect whether these bindings 
collide is a problem I suspect is not 
even computable in polynomial time. Leave alone regex grammars! It's especially 
for this reason that I'd like to 
have bindings in one place, where human eyes are still the best arbiter of 
duplication.

Dhanji.

Original comment by dha...@gmail.com on 15 Dec 2008 at 11:16

GoogleCodeExporter commented 9 years ago
I don't think that should be a showstopper. In servlet-style patterns the 
ambiguity 
can be resolved by following the servlet spec, and if people throw in regexes 
they 
have to realize they're on their own anyway. As long as there is some kind of 
log we 
can turn on that shows which binding was taken to resolve a specific request, 
having 
support for multiple modules will not make matters worse, I think. :)

Some random impl ideas
- in Stage.DEVELOPMENT combined with regexes, maybe warn if more than one regex 
matches.
- mandatory URL prefixes, like Servlets.install("/rpc", module)
- make it work with Modules.override() == no need for multiple modules?

Original comment by robbie.v...@gmail.com on 16 Dec 2008 at 12:01

GoogleCodeExporter commented 9 years ago
This issue is mostly done. A few items still on my checklist:

- blow up if web.xml maps servlets to the same paths as Guice Servlet
- bind keys as a singleton; if already (explicitly) bound as non-singleton, 
explode
- find a way to allow contributions from multiple modules that is safe and sane

There's simply no reasonable way for us to detect if two patterns collide. We 
just dispatch based on ordering, 
like servlet containers do.

Original comment by dha...@gmail.com on 24 Dec 2008 at 11:55

GoogleCodeExporter commented 9 years ago
If would be cool to be able to specify the scope directly in the servlet module:

filter("*.html").through(StrutsPrepareAndExecuteFilter.class).in(Scopes.SINGLETO
N);

Original comment by mathias....@gmail.com on 20 Jan 2009 at 2:18

GoogleCodeExporter commented 9 years ago
@9 All filters and servlets are bound as singletons anyway (and can't have any 
other scope).

Original comment by dha...@gmail.com on 20 Jan 2009 at 10:12

GoogleCodeExporter commented 9 years ago
Then why do I still see this error?

2009-01-21 16:28:44.142::WARN:  Failed startup of context
org.mortbay.jetty.plugin.Jetty6PluginWebAppContext@594560cf{/,jar:file:/C:/Java/
projects/pia
zza/trunk/java/server/target/piazza-server-1-SNAPSHOT.war!/}
javax.servlet.ServletException: Servlets must be bound as singletons.
Key[type=org.apache.shindig.gadgets.servlet.JsServlet, annotation=[none]] was no
t bound in singleton scope.
        at com.google.inject.servlet.ServletDefinition.init(ServletDefinition.java:67)
        at
com.google.inject.servlet.ManagedServletPipeline.init(ManagedServletPipeline.jav
a:75)
        at
com.google.inject.servlet.ManagedFilterPipeline.initPipeline(ManagedFilterPipeli
ne.java:102)
        at com.google.inject.servlet.GuiceFilter.init(GuiceFilter.java:168)
        at org.mortbay.jetty.servlet.FilterHolder.doStart(FilterHolder.java:97)
        at org.mortbay.component.AbstractLifeCycle.start(AbstractLifeCycle.java:50)
        at org.mortbay.jetty.servlet.ServletHandler.initialize(ServletHandler.java:620)
        at org.mortbay.jetty.servlet.Context.startContext(Context.java:

Binding it manually to the singleton scope avoids this error, but isn't too nice
either (bind(JsServlet.class).in(Scopes.SINGLETON);).

Original comment by mathias....@gmail.com on 21 Jan 2009 at 3:30

GoogleCodeExporter commented 9 years ago
Also, this causes a nullpointer at shutdown:

2009-01-21 16:38:03.649::INFO:  Shutdown hook executing
2009-01-21 16:38:04.165::WARN:  EXCEPTION
java.lang.NullPointerException
        at com.google.inject.servlet.ServletDefinition.destroy(ServletDefinition.java:95)
        at
com.google.inject.servlet.ManagedServletPipeline.destroy(ManagedServletPipeline.
java:95)
        at
com.google.inject.servlet.ManagedFilterPipeline.destroyPipeline(ManagedFilterPip
eline.java:154)
        at com.google.inject.servlet.GuiceFilter.destroy(GuiceFilter.java:175)
        at org.mortbay.jetty.servlet.FilterHolder.destroyInstance(FilterHolder.java:127)
        at org.mortbay.jetty.servlet.FilterHolder.doStop(FilterHolder.java:107)
        at org.mortbay.component.AbstractLifeCycle.stop(AbstractLifeCycle.java:78)
        at org.mortbay.jetty.servlet.ServletHandler.doStop(ServletHandler.java:162)
        at org.mortbay.component.AbstractLifeCycle.stop(AbstractLifeCycle.java:78)
        at org.mortbay.jetty.handler.HandlerWrapper.doStop(HandlerWrapper.java:142)
        at org.mortbay.component.AbstractLifeCycle.stop(AbstractLifeCycle.java:78)
        at org.mortbay.jetty.handler.HandlerWrapper.doStop(HandlerWrapper.java:142)
        at org.mortbay.jetty.servlet.SessionHandler.doStop(SessionHandler.java:124)
        at org.mortbay.component.AbstractLifeCycle.stop(AbstractLifeCycle.java:78)
        at org.mortbay.jetty.handler.HandlerWrapper.doStop(HandlerWrapper.java:142)
        at org.mortbay.jetty.handler.ContextHandler.doStop(ContextHandler.java:591)
        at org.mortbay.jetty.webapp.WebAppContext.doStop(WebAppContext.java:498)
        at
org.mortbay.jetty.plugin.Jetty6PluginWebAppContext.doStop(Jetty6PluginWebAppCont
ext.java:132)
        at org.mortbay.component.AbstractLifeCycle.stop(AbstractLifeCycle.java:78)
        at org.mortbay.jetty.handler.HandlerCollection.doStop(HandlerCollection.java:169)
        at org.mortbay.component.AbstractLifeCycle.stop(AbstractLifeCycle.java:78)
        at org.mortbay.jetty.handler.HandlerCollection.doStop(HandlerCollection.java:169)

Since the exception is thrown, the httpServlet reference is null, hence causing 
a nptr.

Original comment by mathias....@gmail.com on 21 Jan 2009 at 3:39

GoogleCodeExporter commented 9 years ago
@11 You see the error because the binding is wrong. It requires at least an 
@Singleton to be present on the 
Filter or servlet, failing which you need the explicit binding. Unfortunately 
there's no clear way for us to tell if a 
binding is present.

@12 Thanks for reporting that error. This has been fixed in HEAD.

This is pretty much the finished Guice Servlet 2.0, I think.

Original comment by dha...@gmail.com on 31 Jan 2009 at 5:34