[axis2] Phases

[Glen Daniels <gl...@thoughtcraft.com>]

After looking at the current code, I'd like to rework Phases a bit.

I think the whole "pre-dispatch / post-dispatch / checker" architecture 
is very confusing (and potentially error-prone) as it stands.  Rather 
than having an explicit "post-dispatch" phase in order to do the checks 
necessary after dispatching, I'd like to implement my original idea for 
Phases, which is that each Phase *itself* has pre and post conditions. 
Since this really means that there is some work that might need to be 
done at the beginning/end of a Phase (more than just checking 
conditions), I'm calling the new methods startPhase/endPhase:

class Phase {
   // Default impls do nothing.  Overrides will check
   // conditions and potentially throw Exceptions
   public void startPhase(MessageContext ctx)
       throws AxisFault {
   }
   public void endPhase(MessageContext ctx)
       throws AxisFault {
   }

   public final void invoke(MessageContext ctx) {
      startPhase();
      ... run through included handlers ...
      endPhase();
   }
}

Then rather than doing all the configuration to get the post-dispatch 
stuff ordered correctly, we'd define:

class DispatchPhase extends Phase {
   public void endPhase(MessageContext ctx) throws AxisFault {
       ...
       if (dispatchNotComplete) {
           throw new AxisFault("Dispatching not complete!");
       }
       // Dispatch OK, locate contexts...
       ...
   }
}

When defining phases in axis2.xml, we'd add a class attribute:

<phase name="Dispatch" class="org.apache.axis2.phases.DispatchPhase"/>

I think this ends up being much cleaner, and it's also clearer what's 
going on when a user wants to add a custom Phase with pre- and 
post-conditions - this way the semantics of what makes the Phase special 
live in the Phase implementation class, not in lots of different pieces 
which all must be deployed in a certain way to work correctly.

Once this is done we can do away with the "PostDispatch" Phase, a 
further simplification.

I'd like to make this change soon after ServiceGroup work is checked in. 
  Thoughts?

--Glen

Re: [axis2] Phases

[Sanjiva Weerawarana <sa...@opensource.lk>]

On Tue, 2005-09-13 at 11:28 -0400, Glen Daniels wrote:
> After looking at the current code, I'd like to rework Phases a bit.

Overall I'm not in favor of the proposal .. details below.

> I think the whole "pre-dispatch / post-dispatch / checker" architecture 
> is very confusing (and potentially error-prone) as it stands. 

Why? Phases are just symbolic names .. and the module author type user
can see what phases exist and how they are ordered.

>  Rather 
> than having an explicit "post-dispatch" phase in order to do the checks 
> necessary after dispatching, I'd like to implement my original idea for 
> Phases, which is that each Phase *itself* has pre and post conditions. 
> Since this really means that there is some work that might need to be 
> done at the beginning/end of a Phase (more than just checking 
> conditions), I'm calling the new methods startPhase/endPhase:
> 
> class Phase {
>    // Default impls do nothing.  Overrides will check
>    // conditions and potentially throw Exceptions
>    public void startPhase(MessageContext ctx)
>        throws AxisFault {
>    }
>    public void endPhase(MessageContext ctx)
>        throws AxisFault {
>    }
> 
>    public final void invoke(MessageContext ctx) {
>       startPhase();
>       ... run through included handlers ...
>       endPhase();
>    }
> }

Currently phases are just labels- if you don't like the name
"pre-dispatch", "post-dispatch" etc. you can of course rename them A, B,
C etc. (or whatever you want). The order of the phases to be executed is
given to the engine using axis.xml.

So currently, phases are not part of the programming model of Axis2.
They are the way in which modules schedule their handlers for execution
by the engine.

You're proposing to make phases a programming model concept. I'm not
convinced there's much value in that and in fact see harm: I doubt that
its the definer of a Phase that knows what the pre and post conditions
are for that phase .. its the modules themselves. So right now, the
post-dispatch phase can be empty for example and life would be fine. If
a specific module wants to make sure some statement is true after
dispatch then it can put a handler into the postdispatch phase. With
your approach they'd have to change the Phase class for that phase; how
would they even be able to do it if there already was a Phase class for
that phase? 

Clearly we can't have specific modules changing the Phase classes; those
are engine-wide global. To me, the pre- and post- conditions have to be
asserted by the modules and not by a Phase class. 

Sanjiva.