You are viewing a plain text version of this content. The canonical link for it is here.
Posted to java-dev@axis.apache.org by Glyn Normington <gl...@uk.ibm.com> on 2001/12/14 17:34:10 UTC
[Architecture Improvement] Handler lifecycle events and undo()
The motivation behind the following changes is to simplify the Handler
architecture and to remove certain speculative features on the basis that
we can put them back in if and when they are needed (extreme programming
refers to this as YAGNI - "You ain't gonna need it").
I've been exporing the Handler interface and especially the undo() method.
Currently, no handlers implement undo with other than a no-op. I propose to
change this by implementing undo() in the BasicHandler abstract class and
then overriding this only in the handlers which are responsible for driving
chains of "subordinate" handlers in order to propagate undo events to them
and give them the opportunity to override undo.
In general, I think most (but not all) handlers will be stateless
transformers acting on a MessageContext and that these are unlikely to need
to modify the MessageContext after a fault. That's why I want to put the
default behaviour in BasicHandler so that most handlers can just ignore
this. (I'd prefer to get rid of undo altogether, but this would appear to
be an up-hill struggle since some developers anticipate it being useful. If
anyone can give me a *real*, non-fictional example of where it will be
necessary, I'd love to hear from them.)
Furthermore, I propose to rename undo to onFault to give a better
indication of its intended use.
Also, I propose to delete the Handler init method. Any handler which
requires this can do the work either at construction time or right at the
beginning of invoke. Non-fictional counter-examples are again welcome.
Finally, I propose to delete the Handler cleanup method. If a real
requirement for this surfaces, I think we should explore an "event-driven"
approach to avoid the overhead of driving multiple no-ops in the mainline
path. Again, real examples please.
The first step towards these changes is included below and contains the
following changes:
* Rename undo to onFault in the Handler interface and BasicHandler abstract
class and implement it with a no-op in BasicHandler
* Rename undo to onFault in SimpleChain and SimpleTargetedChain
* Delete init and cleanup from Handler interface
* Delete FaultableHandler, SimpleChain, SimpleTargetedChain, and
JWSProcessor init and cleanup logic
* (Improvements to a few comments in Handler and BasicHandler - unrelated
to this change and I'll remove them if anyone objects)
Next step (which I'll implement as soon as this patch has been committed):
* Drive onFault through "subordinate" handlers
* Remove dead code (inits, cleanups, and all or most remaining undos)
Glyn
Index: xml-axis/java/src/org/apache/axis/FaultableHandler.java
===================================================================
RCS file: /home/cvspublic/xml-axis/java/src/org/apache/axis/FaultableHandler.java,v
retrieving revision 1.29
diff -u -r1.29 FaultableHandler.java
--- xml-axis/java/src/org/apache/axis/FaultableHandler.java 2001/12/03 22:49:22 1.29
+++ xml-axis/java/src/org/apache/axis/FaultableHandler.java 2001/12/14 16:07:36
@@ -86,14 +86,6 @@
this.workHandler = workHandler;
}
- public void init() {
- workHandler.init();
- }
-
- public void cleanup() {
- workHandler.cleanup();
- }
-
/**
* Invokes the specified handler. If there's a fault the appropriate
* key will be calculated and used to find the fault chain to be
@@ -150,10 +142,10 @@
/**
* Some handler later on has faulted so we need to undo our work.
*/
- public void undo(MessageContext msgContext) {
- category.debug(JavaUtils.getMessage("enter00", "FaultableHandler::undo"));
- workHandler.undo( msgContext );
- category.debug(JavaUtils.getMessage("exit00", "FaultableHandler::undo"));
+ public void onFault(MessageContext msgContext) {
+ category.debug(JavaUtils.getMessage("enter00", "FaultableHandler::onFault"));
+ workHandler.onFault( msgContext );
+ category.debug(JavaUtils.getMessage("exit00", "FaultableHandler::onFault"));
};
public boolean canHandleBlock(QName qname) {
Index: xml-axis/java/src/org/apache/axis/Handler.java
===================================================================
RCS file: /home/cvspublic/xml-axis/java/src/org/apache/axis/Handler.java,v
retrieving revision 1.23
diff -u -r1.23 Handler.java
--- xml-axis/java/src/org/apache/axis/Handler.java 2001/11/09 23:17:47 1.23
+++ xml-axis/java/src/org/apache/axis/Handler.java 2001/12/14 16:07:36
@@ -67,17 +67,6 @@
* @author Doug Davis (dug@us.ibm.com)
*/
public interface Handler extends Serializable {
- /**
- * Init is called when the chain containing this Handler object
- * is instantiated.
- */
- public void init();
-
- /**
- * Cleanup is called when the chain containing this Handler object
- * is done processing the chain.
- */
- public void cleanup();
/**
* Invoke is called to do the actual work of the Handler object.
@@ -91,9 +80,9 @@
public void invoke(MessageContext msgContext) throws AxisFault ;
/**
- * Called when a fault occurs to 'undo' whatever 'invoke' did.
+ * Called when a subsequent handler throws a fault.
*/
- public void undo(MessageContext msgContext);
+ public void onFault(MessageContext msgContext);
/**
* Can this Handler process this QName?
@@ -102,6 +91,7 @@
/**
* Add the given option (name/value) to this handler's bag of options
+ * or replace the option if it is already present.
*/
public void setOption(String name, Object value);
@@ -133,7 +123,7 @@
public Hashtable getOptions();
/**
- * Sets a whole list of options
+ * Set all the options at once after removing the previous list.
*/
public void setOptions(Hashtable opts);
Index: xml-axis/java/src/org/apache/axis/SimpleChain.java
===================================================================
RCS file: /home/cvspublic/xml-axis/java/src/org/apache/axis/SimpleChain.java,v
retrieving revision 1.35
diff -u -r1.35 SimpleChain.java
--- xml-axis/java/src/org/apache/axis/SimpleChain.java 2001/12/03 22:49:22 1.35
+++ xml-axis/java/src/org/apache/axis/SimpleChain.java 2001/12/14 16:07:36
@@ -79,16 +79,6 @@
protected Vector handlers ;
protected Hashtable options ;
- public void init() {
- for ( int i = 0 ; i < handlers.size() ; i++ )
- ((Handler) handlers.elementAt( i )).init();
- }
-
- public void cleanup() {
- for ( int i = 0 ; i < handlers.size() ; i++ )
- ((Handler) handlers.elementAt( i )).cleanup();
- }
-
static InvocationStrategy iVisitor = new InvocationStrategy();
static WSDLGenStrategy wsdlVisitor = new WSDLGenStrategy();
@@ -131,7 +121,7 @@
// undo in reverse order - rethrow
category.error( e );
while( --i >= 0 )
- ((Handler) handlers.elementAt( i )).undo( msgContext );
+ ((Handler) handlers.elementAt( i )).onFault( msgContext );
throw AxisFault.makeFault(e);
}
@@ -145,17 +135,17 @@
* Undo all of the work this chain completed because some handler
* later on has faulted - in reverse order.
*/
- public void undo(MessageContext msgContext) {
+ public void onFault(MessageContext msgContext) {
if (category.isDebugEnabled()) {
category.debug(JavaUtils.getMessage("enter00",
- "SimpleChain::undo"));
+ "SimpleChain::onFault"));
}
for ( int i = handlers.size()-1 ; i >= 0 ; i-- )
- ((Handler) handlers.elementAt( i )).undo( msgContext );
+ ((Handler) handlers.elementAt( i )).onFault( msgContext );
if (category.isDebugEnabled()) {
- category.debug(JavaUtils.getMessage("exit00", "SimpleChain::undo"));
+ category.debug(JavaUtils.getMessage("exit00", "SimpleChain::onFault"));
}
}
Index: xml-axis/java/src/org/apache/axis/SimpleTargetedChain.java
===================================================================
RCS file: /home/cvspublic/xml-axis/java/src/org/apache/axis/SimpleTargetedChain.java,v
retrieving revision 1.33
diff -u -r1.33 SimpleTargetedChain.java
--- xml-axis/java/src/org/apache/axis/SimpleTargetedChain.java 2001/12/07 18:30:27 1.33
+++ xml-axis/java/src/org/apache/axis/SimpleTargetedChain.java 2001/12/14 16:07:36
@@ -77,18 +77,6 @@
protected Handler pivotHandler ;
protected Handler responseHandler ;
- public void init() {
- if ( requestHandler != null ) requestHandler.init();
- if ( pivotHandler != null ) pivotHandler.init();
- if ( responseHandler != null ) responseHandler.init();
- }
-
- public void cleanup() {
- if ( requestHandler != null ) requestHandler.cleanup();
- if ( pivotHandler != null ) pivotHandler.cleanup();
- if ( responseHandler != null ) responseHandler.cleanup();
- }
-
/**
* Invoke the request chain, pivot handler and response chain. If there's
* a fault we need to make sure that we undo any completed handler
@@ -106,7 +94,7 @@
catch( Exception e ) {
category.error( "SimpleTargetedChain caught exception", e );
if ( requestHandler != null )
- requestHandler.undo( msgContext );
+ requestHandler.onFault( msgContext );
throw AxisFault.makeFault(e);
}
msgContext.setPastPivot(true);
@@ -116,9 +104,9 @@
}
catch( Exception e ) {
category.error( e );
- if ( pivotHandler != null ) pivotHandler.undo( msgContext );
+ if ( pivotHandler != null ) pivotHandler.onFault( msgContext );
if ( requestHandler != null )
- requestHandler.undo( msgContext );
+ requestHandler.onFault( msgContext );
throw AxisFault.makeFault(e);
}
@@ -139,7 +127,7 @@
catch( Exception e ) {
category.error( e );
if ( requestHandler != null )
- requestHandler.undo( msgContext );
+ requestHandler.onFault( msgContext );
throw AxisFault.makeFault(e);
}
msgContext.setPastPivot(true);
@@ -160,17 +148,17 @@
/**
* Undo all of the work - in reverse order.
*/
- public void undo(MessageContext msgContext) {
+ public void onFault(MessageContext msgContext) {
if (category.isDebugEnabled()) {
- category.debug(JavaUtils.getMessage("enter00", "SimpleTargetedChain::undo") );
+ category.debug(JavaUtils.getMessage("enter00", "SimpleTargetedChain::onFault") );
}
- if ( responseHandler != null ) responseHandler.undo( msgContext );
- if ( pivotHandler != null ) pivotHandler.undo( msgContext );
- if ( requestHandler != null ) requestHandler.undo( msgContext );
+ if ( responseHandler != null ) responseHandler.onFault( msgContext );
+ if ( pivotHandler != null ) pivotHandler.onFault( msgContext );
+ if ( requestHandler != null ) requestHandler.onFault( msgContext );
if (category.isDebugEnabled()) {
- category.debug(JavaUtils.getMessage("exit00", "SimpleTargetedChain::undo") );
+ category.debug(JavaUtils.getMessage("exit00", "SimpleTargetedChain::onFault") );
}
}
Index: xml-axis/java/src/org/apache/axis/handlers/BasicHandler.java
===================================================================
RCS file: /home/cvspublic/xml-axis/java/src/org/apache/axis/handlers/BasicHandler.java,v
retrieving revision 1.22
diff -u -r1.22 BasicHandler.java
--- xml-axis/java/src/org/apache/axis/handlers/BasicHandler.java 2001/11/09 23:17:47 1.22
+++ xml-axis/java/src/org/apache/axis/handlers/BasicHandler.java 2001/12/14 16:07:37
@@ -98,9 +98,9 @@
return false;
}
- /** Must implement this in subclasses.
- */
- public abstract void undo(MessageContext msgContext);
+ public void onFault(MessageContext msgContext)
+ {
+ }
/** Must implement this in subclasses.
*/
@@ -138,8 +138,11 @@
return( options );
}
+ /**
+ * Set all the options at once after removing the previous list.
+ */
public void setOptions(Hashtable opts) {
- options = opts ;
+ options = opts;
}
/**
Index: xml-axis/java/src/org/apache/axis/handlers/JWSProcessor.java
===================================================================
RCS file: /home/cvspublic/xml-axis/java/src/org/apache/axis/handlers/JWSProcessor.java,v
retrieving revision 1.31
diff -u -r1.31 JWSProcessor.java
--- xml-axis/java/src/org/apache/axis/handlers/JWSProcessor.java 2001/12/10 22:25:53 1.31
+++ xml-axis/java/src/org/apache/axis/handlers/JWSProcessor.java 2001/12/14 16:07:37
@@ -239,12 +239,10 @@
*/
rpc.setOption( "methodName", "*");
- rpc.init(); // ??
if (doWsdl)
rpc.generateWSDL(msgContext);
else
rpc.invoke( msgContext );
- rpc.cleanup(); // ??
}
catch( Exception e ) {
category.error( "JWSProcessor Exception", e );
Re: [Architecture Improvement] Handler lifecycle events and undo()
Posted by Berin Loritsch <bl...@apache.org>.
Sanjiva Weerawarana wrote:
> Berin Loritsch writes:
>
>>So here are some LAWS of development life:
>>
> I come from the design school which says only design for the problem
> you have in hand; not for the one that some user may have in their
> hands at some point in their life. I also believe strongly in
> iteration and throwing away stuff and doing it again.
>
> Clearly all architectures are iterative. Neither your approach nor
> mine will give the "right" architecture right away; mine starts small
> and yours start big. Well, as they say, there's more than one way
> to skin a cat, eh?
Possibly, but my approach also has the benefit of the experience of
_several_ projects to arrive at the architecture it has now. The short
list is:
* Jakarta JAMES
* Jesktop
* Cocoon
* Avalon DB (an experimental work in progress)
* FOP
* Avalon Proxy (another work in progress)
There are more--although many are proprietary. Those are just the ones
that are in some way affiliated with Apache.
So, if you leverage the experience of 6 projects and all their developers,
you'd think that the core architecture is pretty well tested, and needs
for the lifecycle interfaces have already been discussed and needed. Not
to mention the documentation support for Avalon vs. whatever the proprietary
AxKit comes up with.
Since we are all wanting something easy, in the long run, it would be easier
simply using a well documented architecture than reinventing the wheel--at
first poorly, until you arrive at something comparable but incompatible.
There are other pros to using Avalon, in that solutions originally invented
for Cocoon would be able to easily be used in Axis. If you want DB pooling,
Avalon Excalibur has that. If you want something that automagically manages
your lifecycle so that the AxisEngine doesn't have to add the extra complexity,
Avalon Excalibur has that. If you want a framework to use whatever Logging
toolkit you want, Avalon Framework has that. If you want to have Thread
managing objects, Avalon Excalibur has that. If you want to monitor resources
in the background Avalon Excalibur has that. If you want a test framework
that will set up the environment to test all the Components individually,
Avalon Excalibur has that. If you want a consistent API for an XPath processor,
Avalon Excalibur has that.
You see, using Avalon gains the experience of the Avalon team plus those of
all the other projects that use it. If you use the Component Based approach,
it allows an easier development, because there is that decoupling of classes
at the Component level. Components that need other Components can gain access
through the ComponentManager. The ComponentManager will map the Component role
to an implementation. That means that your Component might be running with a
different implementation than it was originally created with--but can be assured
that the new component does not violate the contracts defined by its role.
Not all Handlers will need the functionalities expressed above, but some will,
and it will be an invaluable tool. As some projects incorporate their Components
that are general in Avalon Excalibur (as Cocoon has done on a few occasions),
the list of functionalities available to Avalon based applications only grows.
Compare that to a completely proprietary approach, where you may come up with
implementations for all of the above features as they are needed, but the
implementations won't be as well tested due to the larger user base of Avalon,
nor will it be compatible.
That is something to think about.
--
"They that give up essential liberty to obtain a little temporary safety
deserve neither liberty nor safety."
- Benjamin Franklin
Re: [Architecture Improvement] Handler lifecycle events and undo()
Posted by Sanjiva Weerawarana <sa...@watson.ibm.com>.
Berin Loritsch writes:
> So here are some LAWS of development life:
>
> "You will never have all the requirements stated up front."
> "Requirements will never remain static over the life of a project."
> "Somebody, Somewhere will need the 'feature' you are trying to remove."
>
> Keep this in mind.
>
> Also, why spend too much mental sinapses on deciding whether it is
fundamentally
> wrong for a Handler to have an init() function when there is a framework
that
> makes it easier to make that optional--so you can have it when you need
it.
>
> BTW, it is not fundamentally wrong, you just haven't run accross a
requirement
> for it yet.
I come from the design school which says only design for the problem
you have in hand; not for the one that some user may have in their
hands at some point in their life. I also believe strongly in
iteration and throwing away stuff and doing it again.
Clearly all architectures are iterative. Neither your approach nor
mine will give the "right" architecture right away; mine starts small
and yours start big. Well, as they say, there's more than one way
to skin a cat, eh?
Sanjiva.
Re: [Architecture Improvement] Handler lifecycle events and undo()
Posted by Berin Loritsch <bl...@apache.org>.
Sanjiva Weerawarana wrote:
> Berin Loritsch writes:
>
>>Here is what the Avalon team has learned in respect to Lifecycle:
>>
>>1) Not every component will need every lifecycle interface
>>2) A collection of components will more than likely need all lifecycle
>> interfaces. While it is rare for one component to use all of them,
>> when you have a number of Components with different needs, each will
>> use a different set--and eventually all lifecycle methods are needed.
>>3) Event based systems scale better than monolithic ones (i.e. SAX vs.
>> DOM, non-blocking vs. blocking IO, event based architecture vs.
>> thread per client architecture).
>>
>>In order to address this, the Avalon team developed a set of interfaces
>>that handle different aspects of the lifecycle. The lifecycle follows
>>a well defined path so as not to confuse people unnecessarily.
>>
>
> While this framework certainly could be used to indicate the
> nature of the chosen lifecycle, it doesn't address real issue we
> were discussing: what is the lifecycle of the handlers? Once that's
> decided I have no objection with using Avalon to actually indicate
> the lifecycle.
You merely have to concider what Avalon doesn't provide that you need.
Avalon takes care of general LifeCycle contracts for you. As Axis
finds that Avalon does not address certain needs, such as the Transactional
Handler interface, you can easily add new interfaces as they are needed.
Thus you enjoy an iterative development cycle, with full backwards
compatibility (once you make the plunge).
>
>
>>Getting back to the init()/destroy()/delete() functionality issues, it
>>is incorrect to force _all_ Handlers to define them. A Handler's
>>
> interface
>
>>should only support the methods required to *Handle* a request. All other
>>methods that are sometimes required should be placed in other interfaces.
>>
>
> The real question is what things the Axis engine should check for
> in the handler: init-able, destroyable, undo-able or ..? All
> you've done is separated these into separate interfaces. That's good
> because its proper separation of concerns (as you noted), but it
> doesn't help decide the question of whether something should be
> init-able at all.
Avalon Excalibur has an incredibly useful tool called the ExcaliburComponentManager.
It handle's all the tough work of lifecycle management (inluding pooling
Handlers that are not threadsafe), and when the AxisEngine asks for a specific
Handler, it gets the reference to a fully instantiated and ready Handler
that is in the Active portion of it's lifecycle.
As to deciding whether something should be initializbale or not, you cannot
say for certain whether it needs to be done until you come accross an instance
where you need to have it done. By separating the lifecycle into separate
interfaces, you can allow the Handler implementer to identify *their* needs,
as opposed to the needs that the omniscient Axis developers deigned to include
in the interfaces :). Seriously, eventually you will find a need, but until
then, Handlers that don't require any preparation beyond simple instantiation
don't have to implement methods that they don't need.
So here are some LAWS of development life:
"You will never have all the requirements stated up front."
"Requirements will never remain static over the life of a project."
"Somebody, Somewhere will need the 'feature' you are trying to remove."
Keep this in mind.
Also, why spend too much mental sinapses on deciding whether it is fundamentally
wrong for a Handler to have an init() function when there is a framework that
makes it easier to make that optional--so you can have it when you need it.
BTW, it is not fundamentally wrong, you just haven't run accross a requirement
for it yet.
--
"They that give up essential liberty to obtain a little temporary safety
deserve neither liberty nor safety."
- Benjamin Franklin
Re: [Architecture Improvement] Handler lifecycle events and undo()
Posted by Sanjiva Weerawarana <sa...@watson.ibm.com>.
Berin Loritsch writes:
> Here is what the Avalon team has learned in respect to Lifecycle:
>
> 1) Not every component will need every lifecycle interface
> 2) A collection of components will more than likely need all lifecycle
> interfaces. While it is rare for one component to use all of them,
> when you have a number of Components with different needs, each will
> use a different set--and eventually all lifecycle methods are needed.
> 3) Event based systems scale better than monolithic ones (i.e. SAX vs.
> DOM, non-blocking vs. blocking IO, event based architecture vs.
> thread per client architecture).
>
> In order to address this, the Avalon team developed a set of interfaces
> that handle different aspects of the lifecycle. The lifecycle follows
> a well defined path so as not to confuse people unnecessarily.
While this framework certainly could be used to indicate the
nature of the chosen lifecycle, it doesn't address real issue we
were discussing: what is the lifecycle of the handlers? Once that's
decided I have no objection with using Avalon to actually indicate
the lifecycle.
> Getting back to the init()/destroy()/delete() functionality issues, it
> is incorrect to force _all_ Handlers to define them. A Handler's
interface
> should only support the methods required to *Handle* a request. All other
> methods that are sometimes required should be placed in other interfaces.
The real question is what things the Axis engine should check for
in the handler: init-able, destroyable, undo-able or ..? All
you've done is separated these into separate interfaces. That's good
because its proper separation of concerns (as you noted), but it
doesn't help decide the question of whether something should be
init-able at all.
Sanjiva.
Re: [Architecture Improvement] Handler lifecycle events and undo()
Posted by giacomo <gi...@apache.org>.
On Fri, 14 Dec 2001, Berin Loritsch wrote:
> Sanjiva Weerawarana wrote:
>
> > I'd like to see this taken further. Using your own argument Glynn,
> > has anyone given a "*real*, non-fictional example" or where any
> > of init()/destroy()/undo() is useful? If so keep them all (rename
> > or do whatever; I don't care).
> >
> > If not, YAGNI.
>
>
> Since we are talking about Lifecycle and Architecture, I have to pipe up ;P
>
> It is very important that architecture be consistent, and that the
> method names be accurately named. This reduces the learning curve.
>
> Here is what the Avalon team has learned in respect to Lifecycle:
>
> 1) Not every component will need every lifecycle interface
> 2) A collection of components will more than likely need all lifecycle
> interfaces. While it is rare for one component to use all of them,
> when you have a number of Components with different needs, each will
> use a different set--and eventually all lifecycle methods are needed.
> 3) Event based systems scale better than monolithic ones (i.e. SAX vs.
> DOM, non-blocking vs. blocking IO, event based architecture vs.
> thread per client architecture).
>
> In order to address this, the Avalon team developed a set of interfaces
> that handle different aspects of the lifecycle. The lifecycle follows
> a well defined path so as not to confuse people unnecessarily.
>
> Many components have *real* needs to prepare itself before it can be
> used. Again, many components have real needs to release resources
> when the component is no longer needed. The Avalon interfaces for
> these are Initializable and Disposable, and each have one method:
>
> Initializable.initialize() throws Exception;
> Disposable.dispose();
>
> For instance, a Handler may have lookup data that it needs to obtain
> outside of itself, and cannot get a reference to the required component
> during construction. This is quite common in environments that enforce
> Inversion of Control. Again, that lookup data will have to be released
> properly when the Handler is no longer needed.
>
> Regarding undo(), it is improperly named--but the concept is quite
> necessary. It is for transactional services. If the service failed,
> the handler needs to clean up resources from the transaction. This does
> not mean that the component/handler needs to release all resources--just
> the ones from this transaction.
>
> However, transactional services do require a more traditional commit/abort
> cycle.
>
> Getting back to the init()/destroy()/delete() functionality issues, it
> is incorrect to force _all_ Handlers to define them. A Handler's interface
> should only support the methods required to *Handle* a request. All other
> methods that are sometimes required should be placed in other interfaces.
Above paragraph states the main point!
> This approach allows the developer to separate the concern areas of his
> specific handler. The simplest Handlers will merely process information
> on the fly with no lookup information required for operation. More complex
> handlers can add functionality as needed. While this does add a little more
> complexity to the Container for the Handlers, the complexity is easily
> managed. For example, with the three point "lifecycle" that Handlers have
> hardcoded, during initialization, the Container tests all the Handlers if
> they implement the Initializable interface and call the appropriate
> method as needed. During run time, all the Handlers are treated identically.
> During tear down, the same idea happens with the Disposable interface
> as what happened with the Initializable one.
>
> This separation of concerns simplifies the system for the Handler writer.
>
>
>
>
> It is something to consider....
Couldn't have said better :)
Giacomo
Re: [Architecture Improvement] Handler lifecycle events and undo()
Posted by Berin Loritsch <bl...@apache.org>.
Sanjiva Weerawarana wrote:
> I'd like to see this taken further. Using your own argument Glynn,
> has anyone given a "*real*, non-fictional example" or where any
> of init()/destroy()/undo() is useful? If so keep them all (rename
> or do whatever; I don't care).
>
> If not, YAGNI.
Since we are talking about Lifecycle and Architecture, I have to pipe up ;P
It is very important that architecture be consistent, and that the
method names be accurately named. This reduces the learning curve.
Here is what the Avalon team has learned in respect to Lifecycle:
1) Not every component will need every lifecycle interface
2) A collection of components will more than likely need all lifecycle
interfaces. While it is rare for one component to use all of them,
when you have a number of Components with different needs, each will
use a different set--and eventually all lifecycle methods are needed.
3) Event based systems scale better than monolithic ones (i.e. SAX vs.
DOM, non-blocking vs. blocking IO, event based architecture vs.
thread per client architecture).
In order to address this, the Avalon team developed a set of interfaces
that handle different aspects of the lifecycle. The lifecycle follows
a well defined path so as not to confuse people unnecessarily.
Many components have *real* needs to prepare itself before it can be
used. Again, many components have real needs to release resources
when the component is no longer needed. The Avalon interfaces for
these are Initializable and Disposable, and each have one method:
Initializable.initialize() throws Exception;
Disposable.dispose();
For instance, a Handler may have lookup data that it needs to obtain
outside of itself, and cannot get a reference to the required component
during construction. This is quite common in environments that enforce
Inversion of Control. Again, that lookup data will have to be released
properly when the Handler is no longer needed.
Regarding undo(), it is improperly named--but the concept is quite
necessary. It is for transactional services. If the service failed,
the handler needs to clean up resources from the transaction. This does
not mean that the component/handler needs to release all resources--just
the ones from this transaction.
However, transactional services do require a more traditional commit/abort
cycle.
Getting back to the init()/destroy()/delete() functionality issues, it
is incorrect to force _all_ Handlers to define them. A Handler's interface
should only support the methods required to *Handle* a request. All other
methods that are sometimes required should be placed in other interfaces.
This approach allows the developer to separate the concern areas of his
specific handler. The simplest Handlers will merely process information
on the fly with no lookup information required for operation. More complex
handlers can add functionality as needed. While this does add a little more
complexity to the Container for the Handlers, the complexity is easily
managed. For example, with the three point "lifecycle" that Handlers have
hardcoded, during initialization, the Container tests all the Handlers if
they implement the Initializable interface and call the appropriate
method as needed. During run time, all the Handlers are treated identically.
During tear down, the same idea happens with the Disposable interface
as what happened with the Initializable one.
This separation of concerns simplifies the system for the Handler writer.
It is something to consider....
--
"They that give up essential liberty to obtain a little temporary safety
deserve neither liberty nor safety."
- Benjamin Franklin
Re: [Architecture Improvement] Handler lifecycle events and undo()
Posted by Sanjiva Weerawarana <sa...@watson.ibm.com>.
I'd like to see this taken further. Using your own argument Glynn,
has anyone given a "*real*, non-fictional example" or where any
of init()/destroy()/undo() is useful? If so keep them all (rename
or do whatever; I don't care).
If not, YAGNI.
Sanjiva.
----- Original Message -----
From: "Glyn Normington" <gl...@uk.ibm.com>
To: <ax...@xml.apache.org>
Sent: Friday, December 14, 2001 10:34 PM
Subject: [Architecture Improvement] Handler lifecycle events and undo()
> The motivation behind the following changes is to simplify the Handler
> architecture and to remove certain speculative features on the basis that
> we can put them back in if and when they are needed (extreme programming
> refers to this as YAGNI - "You ain't gonna need it").
>
> I've been exporing the Handler interface and especially the undo() method.
> Currently, no handlers implement undo with other than a no-op. I propose
to
> change this by implementing undo() in the BasicHandler abstract class and
> then overriding this only in the handlers which are responsible for
driving
> chains of "subordinate" handlers in order to propagate undo events to them
> and give them the opportunity to override undo.
>
> In general, I think most (but not all) handlers will be stateless
> transformers acting on a MessageContext and that these are unlikely to
need
> to modify the MessageContext after a fault. That's why I want to put the
> default behaviour in BasicHandler so that most handlers can just ignore
> this. (I'd prefer to get rid of undo altogether, but this would appear to
> be an up-hill struggle since some developers anticipate it being useful.
If
> anyone can give me a *real*, non-fictional example of where it will be
> necessary, I'd love to hear from them.)
>
> Furthermore, I propose to rename undo to onFault to give a better
> indication of its intended use.
>
> Also, I propose to delete the Handler init method. Any handler which
> requires this can do the work either at construction time or right at the
> beginning of invoke. Non-fictional counter-examples are again welcome.
>
> Finally, I propose to delete the Handler cleanup method. If a real
> requirement for this surfaces, I think we should explore an "event-driven"
> approach to avoid the overhead of driving multiple no-ops in the mainline
> path. Again, real examples please.
>
> The first step towards these changes is included below and contains the
> following changes:
>
> * Rename undo to onFault in the Handler interface and BasicHandler
abstract
> class and implement it with a no-op in BasicHandler
> * Rename undo to onFault in SimpleChain and SimpleTargetedChain
> * Delete init and cleanup from Handler interface
> * Delete FaultableHandler, SimpleChain, SimpleTargetedChain, and
> JWSProcessor init and cleanup logic
> * (Improvements to a few comments in Handler and BasicHandler - unrelated
> to this change and I'll remove them if anyone objects)
>
> Next step (which I'll implement as soon as this patch has been committed):
> * Drive onFault through "subordinate" handlers
> * Remove dead code (inits, cleanups, and all or most remaining undos)
>
> Glyn
> Index: xml-axis/java/src/org/apache/axis/FaultableHandler.java
> ===================================================================
> RCS file:
/home/cvspublic/xml-axis/java/src/org/apache/axis/FaultableHandler.java,v
> retrieving revision 1.29
> diff -u -r1.29 FaultableHandler.java
> --- xml-axis/java/src/org/apache/axis/FaultableHandler.java 2001/12/03
22:49:22 1.29
> +++ xml-axis/java/src/org/apache/axis/FaultableHandler.java 2001/12/14
16:07:36
> @@ -86,14 +86,6 @@
> this.workHandler = workHandler;
> }
>
> - public void init() {
> - workHandler.init();
> - }
> -
> - public void cleanup() {
> - workHandler.cleanup();
> - }
> -
> /**
> * Invokes the specified handler. If there's a fault the appropriate
> * key will be calculated and used to find the fault chain to be
> @@ -150,10 +142,10 @@
> /**
> * Some handler later on has faulted so we need to undo our work.
> */
> - public void undo(MessageContext msgContext) {
> - category.debug(JavaUtils.getMessage("enter00",
"FaultableHandler::undo"));
> - workHandler.undo( msgContext );
> - category.debug(JavaUtils.getMessage("exit00",
"FaultableHandler::undo"));
> + public void onFault(MessageContext msgContext) {
> + category.debug(JavaUtils.getMessage("enter00",
"FaultableHandler::onFault"));
> + workHandler.onFault( msgContext );
> + category.debug(JavaUtils.getMessage("exit00",
"FaultableHandler::onFault"));
> };
>
> public boolean canHandleBlock(QName qname) {
> Index: xml-axis/java/src/org/apache/axis/Handler.java
> ===================================================================
> RCS file: /home/cvspublic/xml-axis/java/src/org/apache/axis/Handler.java,v
> retrieving revision 1.23
> diff -u -r1.23 Handler.java
> --- xml-axis/java/src/org/apache/axis/Handler.java 2001/11/09
23:17:47 1.23
> +++ xml-axis/java/src/org/apache/axis/Handler.java 2001/12/14
16:07:36
> @@ -67,17 +67,6 @@
> * @author Doug Davis (dug@us.ibm.com)
> */
> public interface Handler extends Serializable {
> - /**
> - * Init is called when the chain containing this Handler object
> - * is instantiated.
> - */
> - public void init();
> -
> - /**
> - * Cleanup is called when the chain containing this Handler object
> - * is done processing the chain.
> - */
> - public void cleanup();
>
> /**
> * Invoke is called to do the actual work of the Handler object.
> @@ -91,9 +80,9 @@
> public void invoke(MessageContext msgContext) throws AxisFault ;
>
> /**
> - * Called when a fault occurs to 'undo' whatever 'invoke' did.
> + * Called when a subsequent handler throws a fault.
> */
> - public void undo(MessageContext msgContext);
> + public void onFault(MessageContext msgContext);
>
> /**
> * Can this Handler process this QName?
> @@ -102,6 +91,7 @@
>
> /**
> * Add the given option (name/value) to this handler's bag of options
> + * or replace the option if it is already present.
> */
> public void setOption(String name, Object value);
>
> @@ -133,7 +123,7 @@
> public Hashtable getOptions();
>
> /**
> - * Sets a whole list of options
> + * Set all the options at once after removing the previous list.
> */
> public void setOptions(Hashtable opts);
>
> Index: xml-axis/java/src/org/apache/axis/SimpleChain.java
> ===================================================================
> RCS file:
/home/cvspublic/xml-axis/java/src/org/apache/axis/SimpleChain.java,v
> retrieving revision 1.35
> diff -u -r1.35 SimpleChain.java
> --- xml-axis/java/src/org/apache/axis/SimpleChain.java 2001/12/03
22:49:22 1.35
> +++ xml-axis/java/src/org/apache/axis/SimpleChain.java 2001/12/14
16:07:36
> @@ -79,16 +79,6 @@
> protected Vector handlers ;
> protected Hashtable options ;
>
> - public void init() {
> - for ( int i = 0 ; i < handlers.size() ; i++ )
> - ((Handler) handlers.elementAt( i )).init();
> - }
> -
> - public void cleanup() {
> - for ( int i = 0 ; i < handlers.size() ; i++ )
> - ((Handler) handlers.elementAt( i )).cleanup();
> - }
> -
> static InvocationStrategy iVisitor = new InvocationStrategy();
> static WSDLGenStrategy wsdlVisitor = new WSDLGenStrategy();
>
> @@ -131,7 +121,7 @@
> // undo in reverse order - rethrow
> category.error( e );
> while( --i >= 0 )
> - ((Handler) handlers.elementAt( i )).undo( msgContext );
> + ((Handler) handlers.elementAt( i )).onFault(
msgContext );
> throw AxisFault.makeFault(e);
> }
>
> @@ -145,17 +135,17 @@
> * Undo all of the work this chain completed because some handler
> * later on has faulted - in reverse order.
> */
> - public void undo(MessageContext msgContext) {
> + public void onFault(MessageContext msgContext) {
> if (category.isDebugEnabled()) {
> category.debug(JavaUtils.getMessage("enter00",
> - "SimpleChain::undo"));
> + "SimpleChain::onFault"));
> }
>
> for ( int i = handlers.size()-1 ; i >= 0 ; i-- )
> - ((Handler) handlers.elementAt( i )).undo( msgContext );
> + ((Handler) handlers.elementAt( i )).onFault( msgContext );
>
> if (category.isDebugEnabled()) {
> - category.debug(JavaUtils.getMessage("exit00",
"SimpleChain::undo"));
> + category.debug(JavaUtils.getMessage("exit00",
"SimpleChain::onFault"));
> }
> }
>
> Index: xml-axis/java/src/org/apache/axis/SimpleTargetedChain.java
> ===================================================================
> RCS file:
/home/cvspublic/xml-axis/java/src/org/apache/axis/SimpleTargetedChain.java,v
> retrieving revision 1.33
> diff -u -r1.33 SimpleTargetedChain.java
> --- xml-axis/java/src/org/apache/axis/SimpleTargetedChain.java
2001/12/07 18:30:27 1.33
> +++ xml-axis/java/src/org/apache/axis/SimpleTargetedChain.java
2001/12/14 16:07:36
> @@ -77,18 +77,6 @@
> protected Handler pivotHandler ;
> protected Handler responseHandler ;
>
> - public void init() {
> - if ( requestHandler != null ) requestHandler.init();
> - if ( pivotHandler != null ) pivotHandler.init();
> - if ( responseHandler != null ) responseHandler.init();
> - }
> -
> - public void cleanup() {
> - if ( requestHandler != null ) requestHandler.cleanup();
> - if ( pivotHandler != null ) pivotHandler.cleanup();
> - if ( responseHandler != null ) responseHandler.cleanup();
> - }
> -
> /**
> * Invoke the request chain, pivot handler and response chain. If
there's
> * a fault we need to make sure that we undo any completed handler
> @@ -106,7 +94,7 @@
> catch( Exception e ) {
> category.error( "SimpleTargetedChain caught exception", e );
> if ( requestHandler != null )
> - requestHandler.undo( msgContext );
> + requestHandler.onFault( msgContext );
> throw AxisFault.makeFault(e);
> }
> msgContext.setPastPivot(true);
> @@ -116,9 +104,9 @@
> }
> catch( Exception e ) {
> category.error( e );
> - if ( pivotHandler != null ) pivotHandler.undo( msgContext );
> + if ( pivotHandler != null ) pivotHandler.onFault(
msgContext );
> if ( requestHandler != null )
> - requestHandler.undo( msgContext );
> + requestHandler.onFault( msgContext );
> throw AxisFault.makeFault(e);
> }
>
> @@ -139,7 +127,7 @@
> catch( Exception e ) {
> category.error( e );
> if ( requestHandler != null )
> - requestHandler.undo( msgContext );
> + requestHandler.onFault( msgContext );
> throw AxisFault.makeFault(e);
> }
> msgContext.setPastPivot(true);
> @@ -160,17 +148,17 @@
> /**
> * Undo all of the work - in reverse order.
> */
> - public void undo(MessageContext msgContext) {
> + public void onFault(MessageContext msgContext) {
> if (category.isDebugEnabled()) {
> - category.debug(JavaUtils.getMessage("enter00",
"SimpleTargetedChain::undo") );
> + category.debug(JavaUtils.getMessage("enter00",
"SimpleTargetedChain::onFault") );
> }
>
> - if ( responseHandler != null ) responseHandler.undo(
msgContext );
> - if ( pivotHandler != null ) pivotHandler.undo( msgContext );
> - if ( requestHandler != null ) requestHandler.undo(
msgContext );
> + if ( responseHandler != null ) responseHandler.onFault(
msgContext );
> + if ( pivotHandler != null ) pivotHandler.onFault( msgContext );
> + if ( requestHandler != null ) requestHandler.onFault(
msgContext );
>
> if (category.isDebugEnabled()) {
> - category.debug(JavaUtils.getMessage("exit00",
"SimpleTargetedChain::undo") );
> + category.debug(JavaUtils.getMessage("exit00",
"SimpleTargetedChain::onFault") );
> }
> }
>
> Index: xml-axis/java/src/org/apache/axis/handlers/BasicHandler.java
> ===================================================================
> RCS file:
/home/cvspublic/xml-axis/java/src/org/apache/axis/handlers/BasicHandler.java
,v
> retrieving revision 1.22
> diff -u -r1.22 BasicHandler.java
> --- xml-axis/java/src/org/apache/axis/handlers/BasicHandler.java
2001/11/09 23:17:47 1.22
> +++ xml-axis/java/src/org/apache/axis/handlers/BasicHandler.java
2001/12/14 16:07:37
> @@ -98,9 +98,9 @@
> return false;
> }
>
> - /** Must implement this in subclasses.
> - */
> - public abstract void undo(MessageContext msgContext);
> + public void onFault(MessageContext msgContext)
> + {
> + }
>
> /** Must implement this in subclasses.
> */
> @@ -138,8 +138,11 @@
> return( options );
> }
>
> + /**
> + * Set all the options at once after removing the previous list.
> + */
> public void setOptions(Hashtable opts) {
> - options = opts ;
> + options = opts;
> }
>
> /**
> Index: xml-axis/java/src/org/apache/axis/handlers/JWSProcessor.java
> ===================================================================
> RCS file:
/home/cvspublic/xml-axis/java/src/org/apache/axis/handlers/JWSProcessor.java
,v
> retrieving revision 1.31
> diff -u -r1.31 JWSProcessor.java
> --- xml-axis/java/src/org/apache/axis/handlers/JWSProcessor.java
2001/12/10 22:25:53 1.31
> +++ xml-axis/java/src/org/apache/axis/handlers/JWSProcessor.java
2001/12/14 16:07:37
> @@ -239,12 +239,10 @@
> */
> rpc.setOption( "methodName", "*");
>
> - rpc.init(); // ??
> if (doWsdl)
> rpc.generateWSDL(msgContext);
> else
> rpc.invoke( msgContext );
> - rpc.cleanup(); // ??
> }
> catch( Exception e ) {
> category.error( "JWSProcessor Exception", e );
>