axis-java-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Glen Daniels <g...@thoughtcraft.com>
Subject Re: [axis2] Recent changes
Date Thu, 26 Apr 2007 20:52:30 GMT
Hi Deepal!

Deepal Jayasinghe wrote:
>> 2) I removed the InstanceDispatcher, since it really wasn't a
>> Dispatcher at all,
> 
> Why not it dispatch contexts , so it is a Dispatcher.

See below.

>> but rather a place for functionality (setting up contexts) that should
>> always happen at the end of dispatching - sometimes it was in the
>> Dispatch phase, and sometimes in the PostDispatch phase.  I moved that
>> functionality into DispatchPhase.checkPostConditions(), so now it's
>> built in to the end of the phase.
>>
> I am big -1 on this change , doing such a major changes after stables
> releases is not a good idea at all, as well as that  will break backward
> compatibility among the releases. Btw was there a particular reason for
> removing InstanceDispatcher ?.

The reason I removed it was because sometimes it was configured in the 
"PostDispatch" phase and sometimes it was configured in the "Dispatch" 
phase - if it ran PostDispatch, as it was in some stuff I was testing 
with, the checks in the DispatchPhase post-conditions didn't work in 
some situations because the contexts were not set up correctly.  So 
rather than having this be something a configuration screwup can break, 
I decided to relocate the same functionality - now the context setup 
occurs someplace which will always be called, just like the check for 
valid operation/service.

Before I roll the changes back, let me see if I can convince you to 
rescind your -1....

First of all, there is no backward compatibility breakage.  All the 
functionality of the InstanceDispatcher still exists - the loadContext() 
method has simply moved to DispatchPhase so that it happens 
automatically at the end of EVERY dispatch phase regardless of how 
people want to configure their dispatchers.  And as David mentioned, 
I've left the class there so even old axis2.xml's with references to 
InstanceDispatcher will continue to work without any problem.  If we 
want to remove the class for good at some point, that's the only time 
breakage will occur, and we can warn people well in advance.  I 
certainly don't think anyone is relying on the InstanceDispatcher being 
a configurable Handler, that's just the way we had it working.

Regarding Dispatchers, they exist so that we can get the correct 
AxisService and AxisOperation in a /variety/ of ways.  The reason this 
functionality isn't built in to the system at a lower level (i.e. why we 
use Handlers) is so you can tweak your axis2.xml and decide only to have 
the URL based dispatch, for instance, or only have one custom 
dispatcher.  The context setup seems to me to be a different kind of 
thing - once you've got the AxisService/AxisOperation.

I would also really like to complete a review on the design of the way 
ServiceContext/SessionContext/ServiceGroupContext are related (we 
discussed this at ApacheCon last year), and that connects to this code 
as well....

So what I'd like to do now is this - leave the change there (since it 
won't break anyone), and finish reviewing the context relationships. 
Once that's done I'll email the list with a proposal for how I'd like to 
clean things up, and how that will affect this code.  Then we can have a 
discussion to finalize how we want it to work, and I'll change it to 
match whatever we resolve at that point.

Let me know what you think of this line of reasoning, and if you still 
-1 it, I can move the logic back before I make the other proposal.

Thanks,
--Glen

---------------------------------------------------------------------
To unsubscribe, e-mail: axis-dev-unsubscribe@ws.apache.org
For additional commands, e-mail: axis-dev-help@ws.apache.org


Mime
View raw message