[Catalina] Discussion - Interceptor Architecture

["Craig R. McClanahan" <cm...@mytownnet.com>]

The Interceptor architecture in the Catalina proposal is loosely based
on work I did earlier in Apache JServ, and also on the existing
"ServiceInterceptor" and "LifecycleInterceptor" interfaces in Tomcat
3.0.  Although the proposed architecture appears to meet the goals
outlined in the proposal, there are also (at least) a couple of problems
with it:

* Because interceptors are executed in a multithread
  environment just like servlets are, there is no
  convenient way to maintain per-request state
  between the preService() and postService() calls
  that occur for the same request.

* Exception handling is really fragile -- the current
  Javadocs say that if (say) a Container throws an
  exception, then the postService() method of all
  interceptors -- when you already called preService
  on them -- will be skipped.  This makes it much more
  difficult to release resources that were allocated
  in the preService methods.

In the Apache JServ component-based architecture, a different approach
called "Valves" was taken.  Instead of adding the interceptors into a
stack, they are added to a linked list instead.  A given Valve would
either process the request and return, or pass the request on to the
next Valve in the pipeline (after possibly modifying the request
parameters).  I have checked in the following files to illustrate the
Valve interface, and a convenience base class that implements the
appropriate linking.

    proposals/catalina/src/share/org/apache/tomcat/Valve.java

    proposals/catalina/src/share/org/apache/tomcat/core/ValveBase.java

This approach seems to meet all of the design objectives of
Interceptors, and adds some additional features:

* Per-request state is easy to keep, using local variables
  in the invoke() method.

* Exceptions thrown by subsequent Valves or Containers
  can be caught, so you can gracefully release anything
  you've allocated.

* You can even replace the request and/or response
  objects before calling the next valve in the pipeline,
  which would give you the ability to implement post-servlet
  filtering if you really wanted to (I don't like that approach,
  but it is enabled by this architecture).

If we like it better than Interceptors, I would propose that
Interceptors (in Catalina) be replaced by Valves (although I'm pretty
neutral about which name we keep).  What do we all think?

Craig McClanahan

Re: [Catalina] Discussion - Interceptor Architecture

[co...@costin.dnt.ro]

> > And again ( I'm tired of repeating that ), the Interceptor pattern is used
> > by Apache and most other servers ( an Apache module is a set of
> > interceptors), with a lot of experience and knowledge behind it.
> 
> Can we just settle on having existing Interceptor with the proper fixes
> to make the work as advertised, plus Interceptors that support Valves
> for those than prefer to work with Valves.

We certainly need to fix Interceptors, and we will certainly provide
bridges between models, whatever architecture we choose. 

Costin

Re: [Catalina] Discussion - Interceptor Architecture

[Assaf Arkin <ar...@exoffice.com>]

costin@costin.dnt.ro wrote:

> And again ( I'm tired of repeating that ), the Interceptor pattern is used
> by Apache and most other servers ( an Apache module is a set of
> interceptors), with a lot of experience and knowledge behind it.

Can we just settle on having existing Interceptor with the proper fixes
to make the work as advertised, plus Interceptors that support Valves
for those than prefer to work with Valves.

arkin

> 
> ( and of course, IMVHO, Valves are the wrong pattern for request
> processing, and will work only in the current super-restricted mapping
> mechanisms allowed by the servlet API. But it's your revolution and I will
> wait till it's done, maybe I'm all wrong).
> 
> Costin
> 
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: tomcat-dev-unsubscribe@jakarta.apache.org
> For additional commands, e-mail: tomcat-dev-help@jakarta.apache.org

-- 
----------------------------------------------------------------------
Assaf Arkin                                           www.exoffice.com
CTO, Exoffice Technologies, Inc.                        www.exolab.org

Re: [Catalina] Discussion - Interceptor Architecture

[co...@costin.dnt.ro]

> > If we like Valves, I would propose that it should replace Interceptors --
> > there's no need for both.  Thus, the methods like
> > Container.addInterceptor() would be changed to Container.addValve().
> > Configuration could be pretty much the same other than that -- adding a new
> > Valve adds it to the front of the pipeline for a particular Container.
> 
> Switching patterns from Interceptor to Valve is a bit of a stretch. I
> like Valves, but I also like Interceptors, I don't want to see a pattern
> change to solve a very easy to solve problem. If you want to support
> Valves simply add an Interceptor that supports the Valve model.

And again ( I'm tired of repeating that ), the Interceptor pattern is used
by Apache and most other servers ( an Apache module is a set of
interceptors), with a lot of experience and knowledge behind it.

( and of course, IMVHO, Valves are the wrong pattern for request
processing, and will work only in the current super-restricted mapping
mechanisms allowed by the servlet API. But it's your revolution and I will
wait till it's done, maybe I'm all wrong).

Costin 

Re: [Catalina] Discussion - Interceptor Architecture

[Assaf Arkin <ar...@exoffice.com>]

> The current Catalina design implies a single Interceptor instance (of a
> particular type) attached to a given Container.  That is why dealing with
> per-request state is an issue -- your interceptor instance can be invoked
> multiple times simultaneously (just like a servlet).

No problem. Put something between the Interceptor and the actual code.


> In addition, assume you've allocated a resource (say, a JDBC connection
> from a connection pool) in the preService() method for this particular
> request.  You need a way to release that resource when the postService()
> method is called for that same request, so you need a way to save some sort
> of state (which resources were allocated to which request).

Wrong answer.

In order to do that in Tyrex I have a very complex mechanism which
releases the connection not when the Servlet terminates, but when the
transaction actually commits. But, the Connection-Thread-Transaction
association is released when the Servlet termiantes.

Tyrex deals with that in a very particular manner. The current
architecture works for me because you don't force me to deal with it in
a specific manner.

Other interceptors will have different ways of dealing with that, and
will simply provide different implementations.


> An exception thrown by a servlet is thrown back to the lowest level
> container that invoked the servlet (a Wrapper in the Catalina design).  The
> problem is, what do I do now?  Do I call all the postService() methods
> anyway, ignoring the fact that the exception was thrown (which would mean,
> for example, that I could not use an Interceptor to log servlet-thrown
> exceptions).  Shoiuld the Interceptor care that an exception occurred?
> (The current Interceptor API provides no way to do this).

The Interceptor should be notified that an exception has occured,
allowed to process it.

An Interceptor might be able to throw an exception in preService to
prevent the service from executing, but never from postService (once it
has executed).

I agree that the current Interceptor API is broken because it does not
deal with exceptions, but adding them should be easy, and in fact will
get you the same API as the EJB server.


> If we like Valves, I would propose that it should replace Interceptors --
> there's no need for both.  Thus, the methods like
> Container.addInterceptor() would be changed to Container.addValve().
> Configuration could be pretty much the same other than that -- adding a new
> Valve adds it to the front of the pipeline for a particular Container.

Switching patterns from Interceptor to Valve is a bit of a stretch. I
like Valves, but I also like Interceptors, I don't want to see a pattern
change to solve a very easy to solve problem. If you want to support
Valves simply add an Interceptor that supports the Valve model.

arkin

> 
> >
> > So, if you have a plug-in that doesn't like the way Interceptors are
> > working today, you provide an Interceptor that provides the right
> > semantics to that plug-in and use it. An adaptor interceptor.
> >
> 
> As long as your gadget can gain control every place it wants to, this is
> feasible.  The current Interceptor design doesn't quite do that IMHO.
> 
> >
> > arkin
> >
> 
> Craig
> 
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: tomcat-dev-unsubscribe@jakarta.apache.org
> For additional commands, e-mail: tomcat-dev-help@jakarta.apache.org

-- 
----------------------------------------------------------------------
Assaf Arkin                                           www.exoffice.com
CTO, Exoffice Technologies, Inc.                        www.exolab.org

Re: [Catalina] Discussion - Interceptor Architecture

[co...@costin.dnt.ro]

> > So, if you have a plug-in that doesn't like the way Interceptors are
> > working today, you provide an Interceptor that provides the right
> > semantics to that plug-in and use it. An adaptor interceptor.
> >
> 
> As long as your gadget can gain control every place it wants to, this is
> feasible.  The current Interceptor design doesn't quite do that IMHO.

Well, it is a feature, not a bug :-)

The model works well enough for Apache, and NSAPI and ISAPI seem to
follow the same model. 

( the main reasons I like the interceptors model more than
 Containers in Catalina are derived from that, it's a proven model and
will allow better integration into existing servers. )


Costin

Re: [Catalina] Discussion - Interceptor Architecture

["Craig R. McClanahan" <cm...@mytownnet.com>]

Assaf Arkin wrote:

> "Craig R. McClanahan" wrote:
> > * Because interceptors are executed in a multithread
> >   environment just like servlets are, there is no
> >   convenient way to maintain per-request state
> >   between the preService() and postService() calls
> >   that occur for the same request.
>
> That's a problem of whoever implements the Interceptor. I would like to
> see some handle you can use to identify the request, but I don't want to
> see an interceptor instance per request model.
>

The current Catalina design implies a single Interceptor instance (of a
particular type) attached to a given Container.  That is why dealing with
per-request state is an issue -- your interceptor instance can be invoked
multiple times simultaneously (just like a servlet).

In addition, assume you've allocated a resource (say, a JDBC connection
from a connection pool) in the preService() method for this particular
request.  You need a way to release that resource when the postService()
method is called for that same request, so you need a way to save some sort
of state (which resources were allocated to which request).

>
> If someone needs that sort of implementation, an interceptor can be used
> to provide that support to another interceptor.
>
> > * Exception handling is really fragile -- the current
> >   Javadocs say that if (say) a Container throws an
> >   exception, then the postService() method of all
> >   interceptors -- when you already called preService
> >   on them -- will be skipped.  This makes it much more
> >   difficult to release resources that were allocated
> >   in the preService methods.
>
> Exceptions thrown by a Servlet must be held, then interceptors called in
> sequence, then the exception should be dealt with. An exception thrown
> by a Servlet should not necessarily be thrown back to the container.

An exception thrown by a servlet is thrown back to the lowest level
container that invoked the servlet (a Wrapper in the Catalina design).  The
problem is, what do I do now?  Do I call all the postService() methods
anyway, ignoring the fact that the exception was thrown (which would mean,
for example, that I could not use an Interceptor to log servlet-thrown
exceptions).  Shoiuld the Interceptor care that an exception occurred?
(The current Interceptor API provides no way to do this).

This is one of the key reasons I am liking the Valve design pattern better
than Interceptors -- a Valve can trap the exception if it wants to (and
re-throw it after cleaning up its own state), or ignore it.

>
> If you look at how EJB works, an exception thrown by the EJB bean is not
> throw back to the client. The container first does the cleanup
> (invalidating the bean, rolling back the transaction), the figures out
> which type of exception to throw to the client. Application exceptions
> are thrown as is, runtime exceptions are converted to RMIException.
>

Because Interceptors and Valves are system level things, the same logic
applies -- they should clean up after themselves.  With the current design,
Interceptors cannot do so, but Valves can.

At the highest level, the thing returned to the client when a servlet
throws a ServletException is usually a canned error message of some sort,
provided by the servlet container.  I'd like the option of inserting an
Interceptor/Valve gadget that can display this message the way I'd like to
(and perhaps log it in the process).

>
> > In the Apache JServ component-based architecture, a different approach
> > called "Valves" was taken.  Instead of adding the interceptors into a
> > stack, they are added to a linked list instead.  A given Valve would
> > either process the request and return, or pass the request on to the
> > next Valve in the pipeline (after possibly modifying the request
> > parameters).  I have checked in the following files to illustrate the
> > Valve interface, and a convenience base class that implements the
> > appropriate linking.
>
> I like the Valves design pattern and I use it a lot, but I don't see how
> it fits Interceptors. Tomcat should have a stupid and generic design to
> hook into interceptor. Internally the interceptor could use Valves and
> other design patterns to offer the required services.
>

If we like Valves, I would propose that it should replace Interceptors --
there's no need for both.  Thus, the methods like
Container.addInterceptor() would be changed to Container.addValve().
Configuration could be pretty much the same other than that -- adding a new
Valve adds it to the front of the pipeline for a particular Container.

>
> So, if you have a plug-in that doesn't like the way Interceptors are
> working today, you provide an Interceptor that provides the right
> semantics to that plug-in and use it. An adaptor interceptor.
>

As long as your gadget can gain control every place it wants to, this is
feasible.  The current Interceptor design doesn't quite do that IMHO.

>
> arkin
>

Craig

Re: [Catalina] Discussion - Interceptor Architecture

[Assaf Arkin <ar...@exoffice.com>]

"Craig R. McClanahan" wrote:
> By that (and your later) reasoning, there is no reason for the extra argument -- so
> it can be removed as well.  Makes sense that the only entity that cares about
> exceptions should really be Container.invoke().

The Interceptor is entitled to know whether the Servlet completed or
failed because the Interceptor is servicing the Servlet. (E.g. Tyrex
will rollback the transaction, a different Interceptor might show the
500 error code page, etc)

But the Interceptor is not serving the one in front of it or behind it,
so it has no reason to know about their failure. Just like an
Interceptor never finds out about exceptions that were thrown in the
Servlet but also caught in the Servlet.

> I'll clean this up later today (I've got some "day job" tasks to complete), as part
> of playing with valves vs. interceptors.  It turns out that you cannot fully emulate
> a Valve by implementing it as an Interceptor (because you cannot replace the request
> or response objects for the rest of the pipeline), but you can do the reverse --
> implement an Interceptor stack inside a Valve.  That will be part of what I check in
> later, so either design pattern remains possible.

I don't follow. An Interceptor should not attempt to emulate a Valve, it
should attempt to offer the Valve API to Values. So a ValveInterceptor
would be an Interceptor, but it would contain and control any number of
Valves.

Just make sure you don't turn Interceptor into another API for writing
Servlets. We all agreed that's not the way it should go.

arkin

> 
> Craig
> 
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: tomcat-dev-unsubscribe@jakarta.apache.org
> For additional commands, e-mail: tomcat-dev-help@jakarta.apache.org

Re: [Catalina] Discussion - Interceptor Architecture

["Craig R. McClanahan" <cm...@mytownnet.com>]

Assaf Arkin wrote:

> Swallow the exception. If the logger interceptor in front of me throw an
> exception, I don't care about it. I didn't know it existed to begin
> with, neither should I start caring now. The fact that it comes before
> (or after) me is purely conicidental.
>

By that (and your later) reasoning, there is no reason for the extra argument -- so
it can be removed as well.  Makes sense that the only entity that cares about
exceptions should really be Container.invoke().

I'll clean this up later today (I've got some "day job" tasks to complete), as part
of playing with valves vs. interceptors.  It turns out that you cannot fully emulate
a Valve by implementing it as an Interceptor (because you cannot replace the request
or response objects for the rest of the pipeline), but you can do the reverse --
implement an Interceptor stack inside a Valve.  That will be part of what I check in
later, so either design pattern remains possible.

Craig

Re: [Catalina] Discussion - Interceptor Architecture

[Assaf Arkin <ar...@exoffice.com>]

> It is true that there is no "lower level" Interceptor in terms of direct calling
> from one to the other -- the layers protect you from that.  However, there is
> the possibility of a "later invoked" Interceptor throwing an exception (along
> with possible exceptions from a Container) that you may want to know about.
> Your choices (in postService()) are to ignore the fact that an exception was
> thrown, take note of that fact if it affects the processing you are doing, or to
> rethrow something else (in the same spirit that EJB and RMI servers re-throw
> application level exceptions inside an RMIException).

Swallow the exception. If the logger interceptor in front of me throw an
exception, I don't care about it. I didn't know it existed to begin
with, neither should I start caring now. The fact that it comes before
(or after) me is purely conicidental.

Not only is it's exception not of any value to me, I cannot expect to
communicate with it through any form of exception.

Anything else, you're just rebuilding Servlet chaining back into
Interceptors.


> The Interceptor contract doesn't need to say much, but the Container contract
> does -- it has to define the layered calling sequence,  promise an Interceptor
> that it will call postService() if it called preService(), and it needs to
> notify postService() -- by passing a Throwable -- that an exception of some sort
> did occur along the way.  Perhaps most of the comments about preService() and
> postService() in Interceptor.java really ought to live in Container.java
> instead.

The sequence of order is important in the sense that you will be called
in your place in the pipe, but you (the Interceptor) have no clue about
your place in the pipe. You could be first, last, or stuck in the
middle.

If a Servlet throws an exception the Interceptor has to know about it
and deal with it. The Interceptor is there to service the Servlet.

If another Interceptor throws an exception, no other Interceptor needs
to know about it, or can properly deal with it to begin with. It's the
container's decision whether to swallow it, or terminate the
Interceptor.

There's just nothing in being an Interceptor that tells you what
exception was thrown by whon, what it signifies and what you should (or
can) do about it.

arkin


> 
> >
> > arkin
> >
> 
> Craig
> 
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: tomcat-dev-unsubscribe@jakarta.apache.org
> For additional commands, e-mail: tomcat-dev-help@jakarta.apache.org

Re: [Catalina] Discussion - Interceptor Architecture

["Craig R. McClanahan" <cm...@mytownnet.com>]

Assaf Arkin wrote:

> Craig, you missed a point there. There is no such thing as a lower-level
> interceptor.
>

It is true that there is no "lower level" Interceptor in terms of direct calling
from one to the other -- the layers protect you from that.  However, there is
the possibility of a "later invoked" Interceptor throwing an exception (along
with possible exceptions from a Container) that you may want to know about.
Your choices (in postService()) are to ignore the fact that an exception was
thrown, take note of that fact if it affects the processing you are doing, or to
rethrow something else (in the same spirit that EJB and RMI servers re-throw
application level exceptions inside an RMIException).

I was thinking about the Valve pattern when I wrote that particular Javadoc
comment, and I'm going to clarify this to "subsequently invoked" instead.

>
> The fact that there is order in the way interceptors are called is only
> to assure proper calling sequence, but they are not layered in any way.
> The interceptor contract is very very simple and says nothing about
> layering.
>

The Interceptor contract doesn't need to say much, but the Container contract
does -- it has to define the layered calling sequence,  promise an Interceptor
that it will call postService() if it called preService(), and it needs to
notify postService() -- by passing a Throwable -- that an exception of some sort
did occur along the way.  Perhaps most of the comments about preService() and
postService() in Interceptor.java really ought to live in Container.java
instead.

>
> arkin
>

Craig

Re: [Catalina] Discussion - Interceptor Architecture

[Assaf Arkin <ar...@exoffice.com>]

Craig, you missed a point there. There is no such thing as a lower-level
interceptor.

The fact that there is order in the way interceptors are called is only
to assure proper calling sequence, but they are not layered in any way.
The interceptor contract is very very simple and says nothing about
layering.

arkin


"Craig R. McClanahan" wrote:
> 
> Assaf Arkin wrote:
> 
> > Exceptions thrown by a Servlet must be held, then interceptors called in
> > sequence, then the exception should be dealt with. An exception thrown
> > by a Servlet should not necessarily be thrown back to the container.
> >
> 
> The Interceptor design pattern in Catalina has been adjusted so that
> postService() now receives a reference to any exception that was thrown by
> a lower-level Interceptor or Container (or the servlet that was ultimately
> called), and the implementation now guarantees that postService() will be
> reliably called if preService() was called, even in the face of exceptions.
> 
> Issues of Interceptor-versus-Valve, as well as component configuration,
> will be addressed later today.  I'm still exploring some alternatives to
> see what looks like the most reasonable approaches to suggest.
> 
> Craig
> 
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: tomcat-dev-unsubscribe@jakarta.apache.org
> For additional commands, e-mail: tomcat-dev-help@jakarta.apache.org

Re: [Catalina] Discussion - Interceptor Architecture

["Craig R. McClanahan" <cm...@mytownnet.com>]

Assaf Arkin wrote:

> Exceptions thrown by a Servlet must be held, then interceptors called in
> sequence, then the exception should be dealt with. An exception thrown
> by a Servlet should not necessarily be thrown back to the container.
>

The Interceptor design pattern in Catalina has been adjusted so that
postService() now receives a reference to any exception that was thrown by
a lower-level Interceptor or Container (or the servlet that was ultimately
called), and the implementation now guarantees that postService() will be
reliably called if preService() was called, even in the face of exceptions.

Issues of Interceptor-versus-Valve, as well as component configuration,
will be addressed later today.  I'm still exploring some alternatives to
see what looks like the most reasonable approaches to suggest.

Craig

Re: [Catalina] Discussion - Interceptor Architecture

[Assaf Arkin <ar...@exoffice.com>]

"Craig R. McClanahan" wrote:
> * Because interceptors are executed in a multithread
>   environment just like servlets are, there is no
>   convenient way to maintain per-request state
>   between the preService() and postService() calls
>   that occur for the same request.

That's a problem of whoever implements the Interceptor. I would like to
see some handle you can use to identify the request, but I don't want to
see an interceptor instance per request model.

If someone needs that sort of implementation, an interceptor can be used
to provide that support to another interceptor.


> * Exception handling is really fragile -- the current
>   Javadocs say that if (say) a Container throws an
>   exception, then the postService() method of all
>   interceptors -- when you already called preService
>   on them -- will be skipped.  This makes it much more
>   difficult to release resources that were allocated
>   in the preService methods.

Exceptions thrown by a Servlet must be held, then interceptors called in
sequence, then the exception should be dealt with. An exception thrown
by a Servlet should not necessarily be thrown back to the container.

If you look at how EJB works, an exception thrown by the EJB bean is not
throw back to the client. The container first does the cleanup
(invalidating the bean, rolling back the transaction), the figures out
which type of exception to throw to the client. Application exceptions
are thrown as is, runtime exceptions are converted to RMIException.


> In the Apache JServ component-based architecture, a different approach
> called "Valves" was taken.  Instead of adding the interceptors into a
> stack, they are added to a linked list instead.  A given Valve would
> either process the request and return, or pass the request on to the
> next Valve in the pipeline (after possibly modifying the request
> parameters).  I have checked in the following files to illustrate the
> Valve interface, and a convenience base class that implements the
> appropriate linking.

I like the Valves design pattern and I use it a lot, but I don't see how
it fits Interceptors. Tomcat should have a stupid and generic design to
hook into interceptor. Internally the interceptor could use Valves and
other design patterns to offer the required services.

So, if you have a plug-in that doesn't like the way Interceptors are
working today, you provide an Interceptor that provides the right
semantics to that plug-in and use it. An adaptor interceptor.

arkin


> 
>     proposals/catalina/src/share/org/apache/tomcat/Valve.java
> 
>     proposals/catalina/src/share/org/apache/tomcat/core/ValveBase.java
> 
> This approach seems to meet all of the design objectives of
> Interceptors, and adds some additional features:
> 
> * Per-request state is easy to keep, using local variables
>   in the invoke() method.
> 
> * Exceptions thrown by subsequent Valves or Containers
>   can be caught, so you can gracefully release anything
>   you've allocated.
> 
> * You can even replace the request and/or response
>   objects before calling the next valve in the pipeline,
>   which would give you the ability to implement post-servlet
>   filtering if you really wanted to (I don't like that approach,
>   but it is enabled by this architecture).
> 
> If we like it better than Interceptors, I would propose that
> Interceptors (in Catalina) be replaced by Valves (although I'm pretty
> neutral about which name we keep).  What do we all think?
> 
> Craig McClanahan
> 
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: tomcat-dev-unsubscribe@jakarta.apache.org
> For additional commands, e-mail: tomcat-dev-help@jakarta.apache.org

-- 
----------------------------------------------------------------------
Assaf Arkin                                           www.exoffice.com
CTO, Exoffice Technologies, Inc.                        www.exolab.org