Invoking the same servlet with different init parameters

[Gili]

Hi,

I migrated my PermanentRedirect from web.xml to warp-servlet but it
now fails to run because:

1) Previously I had one PermanentRedirect instance per redirect. For
example: I had one instance to redirect from "/dog/*" to "/cat",
another one from "/fish/*" to "/dolphin", etc.

Each servlet init() would get different "source" and "target"
parameters and it would work fine.

2) Under the new mechanism, warp-servlet is using Injector to get an
instance of PermanentRedirect. As far as I understand, this means I no
longer have multiple instances of PermanentRedirect.

So I can either have a @Singleton handle all redirects or Guice will
create a new PermanentRedirect instance per request, but then in that
case the servlet init() is not called and the servlet doesn't know
what to do.

Reworking this servlet into a Singleton isn't trivial and I'm
wondering how one is supposed to solve this kind of problem more
generally since I assume you can't always collapse multiple servlet
instances like this.

Please advise me on what my options are.

Thank you,
Gili

[Gili]

Perhaps the solution is to modify FilterDefinition slightly... since
it doesn't make sense to call init() on one instance then call
doFilter() on another we can safely assume that each FilterDefinition
should bind to exactly one Filter instance.

I would propose the following changes:

1) Add a Filter field
2) FilterDefinition.init() should assign a value to the field instead
of to the local variable
3) doFilter() should use the field instead of invoking
injector.getInstance()

Alternatively, you could check if the filter returned by
Injector.getInstance() has been initialized before, and if not call
init() on it. Essentially this means that init() would be called
lazily. I personally favor the 1st approach instead.

I would like to hear what you think.

Thank you,
Gili

[Gili]

I tried implementing the approach where init() is called anytime a new
Filter is created but the whole thing seems very problematic to me.

1) There is no way to guarantee destroy() is called
2) There is a non-trivial cost associated with tracking which Filter
have already been initialized
3) Filters expect to get initialized at server startup, though this
isn't strictly required by the standard. It just says that init() must
be invoked sometime after the filter is constructed but I don't think
it actually guarantees they will get constructed at startup. Still,
developers assume this.

Anyway, judging by the comments in the code it seems like you already
knew most of this. With a minor change, I think we can give users the
best of both worlds:

1) FilterDefinition assumes the same instance in init() and doFilter()
2) Introduce RequestScopedFilter which is identical to
FilterDefinition with the following differences:

RequestFilterDefinition.init() stores the ServletContext for later
use, then uses it in doFilter() to invoke init() on the filter. Once
the filter's doFilter() completes, we invoke filter.destroy() and exit
the method.

Now, when users invoke FilterBindingBuilder.through(Class) you check
whether the Class is annotated with @RequestScoped or not. Depending
on the annotation you bind it to either FilterDefiniton or
RequestFilterDefinition.

What do you think?

[Dhanji R. Prasanna]

On Fri, Jul 25, 2008 at 6:18 AM, Gili <gili.t...@gmail.com> wrote:

Hi,

I migrated my PermanentRedirect from web.xml to warp-servlet but it
now fails to run because:

1) Previously I had one PermanentRedirect instance per redirect. For
example: I had one instance to redirect from "/dog/*" to "/cat",
another one from "/fish/*" to "/dolphin", etc.

Each servlet init() would get different "source" and "target"
parameters and it would work fine.

2) Under the new mechanism, warp-servlet is using Injector to get an
instance of PermanentRedirect. As far as I understand, this means I no
longer have multiple instances of PermanentRedirect.

No this is not true, unless you have explicitly bound PermanentRedirect as a singleton, it will be instantiated every time it is injected. That's really the source of your problem: the init parameters are being stored during init() and then subsequently lost because new instances are created. 

There are many solutions, the simplest being use binding annotations and create 3 different singletons (which is essentially what you're doing in web.xml, but instead of binding annotations you use <servlet-name>). Check out the docs under "Setting Scopes on servlets and filters" for more details:

http://www.wideplay.com/managedservletsandfilters

Dhanji.

[Gili]

I don't understand how one can use binding annotations in this case. I
mean, I think I know how binding annotations work but it isn't clear
how this helps me in this case. Can you please give a short example?

Thank you,
Gili

On Jul 24, 5:38 pm, "Dhanji R. Prasanna" <dha...@gmail.com> wrote:

[Gili]

Sorry Dhanji, I noticed you replied to the other discussion thread a
couple of hours ago but not to this one. I'm not sure if you missed it
by mistake...

Is it possible for you to please provide me with a short example of
what you meant?

Thank you,
Gili

[Gili]

The more I think about this problem, the more I dislike the existing
implementation because it doesn't respect init() and destroy() for
request-scoped servlets, and it violates the principal of least-
surprise for non-guice servlets.

If I implement the design I mentioned above, with FilterDefinition and
RequestFilterDefinition, would you consider folding it back into trunk?

[Dhanji R. Prasanna]

On Sat, Jul 26, 2008 at 1:56 AM, Gili <gili.t...@gmail.com> wrote:

The more I think about this problem, the more I dislike the existing
implementation because it doesn't respect init() and destroy() for
request-scoped servlets, and it violates the principal of least-
surprise for non-guice servlets.

Actually it does respect init() and destroy() according to the *servlet* lifecycle. And it is clearly documented as such.

I deliberately did not make a per-servlet init and destroy because this violates the servlet lifecycle. Per-object init and destroy is a totally different lifecycle to servlets (see @PostConstruct and @PreDestroy in Java EE). This decision was precisely to prevent violation of least surprise.

Dhanji.

[Dhanji R. Prasanna]

On Fri, Jul 25, 2008 at 10:57 AM, Gili <gili.t...@gmail.com> wrote:

I don't understand how one can use binding annotations in this case. I
mean, I think I know how binding annotations work but it isn't clear
how this helps me in this case. Can you please give a short example?

You want to bind 2 different servlets of the same class as singletons so use a binding annotation to distinguish them:

  .servlets()

       .serve("/dog").with(Key.get(MyServlet.class, Dog.class))

       .serve("/cat").with(Key.get(MyServlet.class, Cat.class))

And in your module:

bind(MyServlet.class).annotatedWith(Dog.class).in(Singleton.class);

bind(MyServlet.class).annotatedWith(Class.class).in(Singleton.class);

This gives you 2 singleton servlets of the same class, bound at different URIs.

Dhanji.

[Gili]

On Jul 25, 8:00 pm, "Dhanji R. Prasanna" <dha...@gmail.com> wrote:
> You want to bind 2 different servlets of the same class as singletons so use
> a binding annotation to distinguish them:
>
>   .servlets()
>        .serve("/dog").with(Key.get(MyServlet.class, Dog.class))
>        .serve("/cat").with(Key.get(MyServlet.class, Cat.class))
>
> And in your module:
>
> bind(MyServlet.class).annotatedWith(Dog.class).in(Singleton.class);
> bind(MyServlet.class).annotatedWith(Class.class).in(Singleton.class);
> This gives you 2 singleton servlets of the same class, bound at different
> URIs.

I guess my confusion arises from Key.get(MyServlet.class, Dog.class).

Wouldn't this require me to go into MyServlet.java and annotate the
class with @Dog and @Cat? Even then, where does it say that Guice will
create two difference instances for these two bindings? Wouldn't Guice
be smart enough to notice that both bindings point to the same class
and collapse both statements into one? I actually remember reading
somewhere that if two different bindings match the same type Guice
will only pick up the first one or something. Am I wrong?

Sorry for the confusion, I've never used Key.get() before...

Thank you,
Gili

[Gili]

On Jul 25, 7:57 pm, "Dhanji R. Prasanna" <dha...@gmail.com> wrote:
> I deliberately did not make a per-servlet init and destroy because this
> violates the servlet lifecycle.

How so? The documents I've read simply said that init() must be called
after the servlet is constructed and before it is used and destroy()
must be called before it is destroyed. The javadoc I am referring to
can be found here: http://java.sun.com/javaee/5/docs/api/javax/servlet/Servlet.html#init(javax.servlet.ServletConfig)

You might be referring to the fact that init() and destroy() are
supposed to be invoked once per servlet lifetime, across all threads,
but that's only because architecturally-speaking the container
instantiates one servlet and shares it between all requests. Nothing
prevents us from introducing an architecture where you have multiple
servlet instances. In that case you should be able to call init() and
destroy() once per instance over its lifetime without violating the
specification.

> Per-object init and destroy is a totally
> different lifecycle to servlets (see @PostConstruct and @PreDestroy in Java
> EE). This decision was precisely to prevent violation of least surprise.

Reading http://java.sun.com/javaee/5/docs/api/javax/annotation/PostConstruct.html
where does it indicate you can't apply @PostConstruct, @PreDestroy on
a per-instance basis?

I am probably missing something here... please let me know.

Thanks,
Gili

[Dhanji R. Prasanna]

On Sat, Jul 26, 2008 at 10:45 AM, Gili <gili.t...@gmail.com> wrote:

On Jul 25, 7:57 pm, "Dhanji R. Prasanna" <dha...@gmail.com> wrote:
> I deliberately did not make a per-servlet init and destroy because this
> violates the servlet lifecycle.

You might be referring to the fact that init() and destroy() are
supposed to be invoked once per servlet lifetime, across all threads,
but that's only because architecturally-speaking the container
instantiates one servlet and shares it between all requests. Nothing
prevents us from introducing an architecture where you have multiple
servlet instances. In that case you should be able to call init() and
destroy() once per instance over its lifetime without violating the
specification.

Not true, if you read the servlet specification, only one instance is allowed per servlet declaration. This architecture is not just a coincidence, it is a deliberate design choice. That's is why you use the wrapping workaround if you want to make non-singleton servlets.

Dhanji.

[Gili]

You're right, section SRV.2.2 of the Servlet 2.4 specification
confirms this. I guess my only remaining unanswered question is post
#8 above.

Thank you,
Gili

On Jul 26, 3:02 am, "Dhanji R. Prasanna" <dha...@gmail.com> wrote:

[Dhanji R. Prasanna]

This is not how binding annotations work. Check out the example source
code for warp-servlet to see an example:

http://code.google.com/p/warp-datagrid/source/browse/trunk/warp-servlet/src/com/wideplay/example/servlets/MyGuiceCreator.java

Line #31 - Usage of Key.get()
Line #53 - binding to annotation (in this case, @Named)

You can repeat the pair of lines 31 & 53 for as many different
annotations as you like with the same implementation class. Which
produces *different* singletons.

If the Guice user guide doesn't help you with binding annotations I
recommend Robbie's book.

Dhanji.

[Gili]

I think I get it now.

1) You tell Guice to bind a certain Key to a Provider.
2) You ask Guice for an instance with the same Key
3) If Guice is asked to Inject "@Blue Service service" it will
actually look up Key.get(Service.class, Blue.class)
4) The key point (and this is the part I missed) is that although
Guice uses Key for object injection you don't actually *need* to
inject anything to look up the binding. You can bind to
Key.get(Service.class, Blue.class) and look it up somewhere else,
without using "@Blue Service" anywhere.

Is that correct?

Thanks,
Gili

On Jul 27, 2:57 am, "Dhanji R. Prasanna" <dha...@gmail.com> wrote:

> On Sat, Jul 26, 2008 at 10:34 AM, Gili <gili.tzab...@gmail.com> wrote:
>
> > On Jul 25, 8:00 pm, "Dhanji R. Prasanna" <dha...@gmail.com> wrote:
> >> You want to bind 2 different servlets of the same class as singletons so use
> >> a binding annotation to distinguish them:
>
> >>   .servlets()
> >>        .serve("/dog").with(Key.get(MyServlet.class, Dog.class))
> >>        .serve("/cat").with(Key.get(MyServlet.class, Cat.class))
>
> >> And in your module:
>
> >> bind(MyServlet.class).annotatedWith(Dog.class).in(Singleton.class);
> >> bind(MyServlet.class).annotatedWith(Class.class).in(Singleton.class);
> >> This gives you 2 singleton servlets of the same class, bound at different
> >> URIs.
>
> > I guess my confusion arises from Key.get(MyServlet.class, Dog.class).
>
> > Wouldn't this require me to go into MyServlet.java and annotate the
> > class with @Dog and @Cat? Even then, where does it say that Guice will
> > create two difference instances for these two bindings? Wouldn't Guice
> > be smart enough to notice that both bindings point to the same class
> > and collapse both statements into one? I actually remember reading
> > somewhere that if two different bindings match the same type Guice
> > will only pick up the first one or something. Am I wrong?
>
> This is not how binding annotations work. Check out the example source
> code for warp-servlet to see an example:
>

> http://code.google.com/p/warp-datagrid/source/browse/trunk/warp-servl...

[Dhanji R. Prasanna]

Yes, you can use service location (via injector.getInstance) to do exactly the same thing as dependency injection (via @Inject...).

Dhanji.

[Gili]

Dhanji,

I would appreciate your help with the following Guice bug:
http://code.google.com/p/google-guice/issues/detail?id=222

Here is what I am doing...

My module contains:

binder.bind(Key.get(PermanentRedirect.class,
Names.named("RedirectMain"))).in(Scopes.SINGLETON);

My WarpServletContextListener contains:
Map<String, String> redirectMain = new HashMap<String, String>();
redirectMain.put("source", "/Main");
redirectMain.put("target", "/main");
Servlets.configure().filters().filterRegex("/Main/
*").through(Key.get(PermanentRedirect.class,
Names.named("RedirectMain")), redirectMain);

Any ideas on how to work around this bug?

Gili

[Gili]

Small correction. I am actually getting this stack-trace (different
from the issue I linked to):

java.lang.NullPointerException
at com.google.inject.ProviderToInternalFactoryAdapter
$1.call(ProviderToInternalFactoryAdapter.java:37)
at
com.google.inject.InjectorImpl.callInContext(InjectorImpl.java:756)
at
com.google.inject.ProviderToInternalFactoryAdapter.get(ProviderToInternalFactoryAdapter.java:
35)
at com.google.inject.Scopes$1$1.get(Scopes.java:53)
at
com.google.inject.InternalFactoryToProviderAdapter.get(InternalFactoryToProviderAdapter.java:
41)
at com.google.inject.InjectorImpl$9$1.call(InjectorImpl.java:
708)
at
com.google.inject.InjectorImpl.callInContext(InjectorImpl.java:747)
at com.google.inject.InjectorImpl$9.get(InjectorImpl.java:702)
at
com.google.inject.InjectorImpl.getInstance(InjectorImpl.java:724)
at
com.wideplay.warp.servlet.FilterDefinition.init(FilterDefinition.java:
53)
at
com.wideplay.warp.servlet.ManagedFilterPipeline.initPipeline(ManagedFilterPipeline.java:
39)
at com.wideplay.warp.servlet.WebFilter.init(WebFilter.java:55)
at
org.apache.catalina.core.ApplicationFilterConfig.getFilter(ApplicationFilterConfig.java:
257)
at
org.apache.catalina.core.ApplicationFilterConfig.setFilterDef(ApplicationFilterConfig.java:
369)
at
org.apache.catalina.core.ApplicationFilterConfig.<init>(ApplicationFilterConfig.java:
103)
at
org.apache.catalina.core.StandardContext.filterStart(StandardContext.java:
4389)
at
org.apache.catalina.core.StandardContext.start(StandardContext.java:
5189)
at com.sun.enterprise.web.WebModule.start(WebModule.java:326)
at
com.sun.enterprise.web.LifecycleStarter.doRun(LifecycleStarter.java:
58)
at
com.sun.appserv.management.util.misc.RunnableBase.runSync(RunnableBase.java:
304)
at
com.sun.appserv.management.util.misc.RunnableBase.run(RunnableBase.java:
341)
at java.util.concurrent.Executors
$RunnableAdapter.call(Executors.java:441)
at java.util.concurrent.FutureTask
$Sync.innerRun(FutureTask.java:303)
at java.util.concurrent.FutureTask.run(FutureTask.java:138)
at java.util.concurrent.ThreadPoolExecutor
$Worker.runTask(ThreadPoolExecutor.java:886)
at java.util.concurrent.ThreadPoolExecutor
$Worker.run(ThreadPoolExecutor.java:908)
at java.lang.Thread.run(Thread.java:619)

Gili

[Dhanji R. Prasanna]

Have you bound the PermanentRedirect class elsewhere to the given (mapped) key?

We should put in a getBinding() test and a friendly error. 

Dhanji.

[Gili Tzabari]

I never bound PermanentRedirect to() anything, but now that I think
about it, why is that a problem? I've seen Guice code floating around
where someone bound a scope to a concrete type (which is what I'm doing
too) without specifying a to() because it expected Guice to default to
the same class. If this isn't the case then yes I'd expect Guice to give
a friendly error message.

Gili

[Dhanji R. Prasanna]

That's because you arent referring to a type directly, you're referring to a tuple key which is unbound (PermRedir.class + named("RedirectMain")).

Yea the error ought to be caught on Guice's end. But we can (will) do some validation too.

Thanks,

Dhanji.