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: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
