Please see my comments below.
On 12/15/05, Glen Daniels <[EMAIL PROTECTED]> wrote:
Actually what I merged was the AxisEngine.send() method (not receive() ). Shouldn't both chains be available by the time this get called by the MessageReceiver ?
If so merging in the begining should not be a problem.
For the receive() method, agreed :) . (actually this seems to be the way it currently work. Dynamic chain replacing has been implemented in the 'checkPostConditions' method of the 'DispatchPhase' class).
+1. The reason for going for two resume() methods was the act of TransportSender and MessageReceiver being called seperately in the send() and receive() methods. If they are a part of the execution chain one resume() method would do it.
+1
Thanks,
Chamikara
Hi Chamikara:
+1 and -1 to parts of this patch. :)
First, I agree about merging the chains (op-specifc and global) into one
- that was the intent (a single EC) from the first Axis2 meetings. The
problem with doing it here is that I think this method might get called
before operation dispatching has occurred (so getAxisOperation() will
probably give you a null, resulting in a NPE). The way this should
probably work is that the act of dispatching to the AxisOperation itself
(i.e. MessageContext.setAxisOperation()) should cause the chains to be
merged/replaced. Then this portion of the code only needs to worry
about pausing/resuming a single chain.
Actually what I merged was the AxisEngine.send() method (not receive() ). Shouldn't both chains be available by the time this get called by the MessageReceiver ?
If so merging in the begining should not be a problem.
For the receive() method, agreed :) . (actually this seems to be the way it currently work. Dynamic chain replacing has been implemented in the 'checkPostConditions' method of the 'DispatchPhase' class).
Along the lines of making this simpler, I don't like having resumeSend()
and resumeReceive() - why should the core execution code care which
direction things are going? I believe that the correct thing to do here
is to slightly refactor the chains, and simply have the transport sender
/ message receiver as Handlers on the ends of the execution chain. That
way we could remove all the extra logic of calling senders/receivers
manually, and just have them invoked automatically by executing the
handler chain (that's how Axis 1 did it too).
+1. The reason for going for two resume() methods was the act of TransportSender and MessageReceiver being called seperately in the send() and receive() methods. If they are a part of the execution chain one resume() method would do it.
So the end result is a single execution chain which pauses/resumes
simply by indexes, updates itself at operation dispatch, and no extra
logic to split receiving/sending.
Sound reasonable?
+1
Thanks,
Chamikara
Chamikara Jayalath wrote:
> Hi All,
>
> There seems to be a bug in the AxisEngine.resume() method (which was
> recently added)
>
> Wihtin this, only the currently attached execution chain is resumed.
> But AxisEngine.send() method attachs two execution chains (service
> specific and global) and invoke both. Because of this if pausing happen
> in a service specific out handler, the global out handlers are ommitted
> when resuming. Also resume mothod does not seem to be calling the
> MessageReceiver or TransportSender.
>
> I have attached a fix to this. First doing two msgContext.invoke() calls
> in the send() method will not work because when resuming we don't know
> weather the first or second chain we were in when pausing. So I combined
> both chains to a one ArrayList and invoked at once.
>
> Also since we have to call the MessageReceiver or TransportSender after
> the invocation of handlers (depending on weather we paused in the send()
> method or receive() method) it seems better to have two resume methods
> named resumeSend() and resumeReceive() which will call TransportSender
> and MessageReceiver respectively (after invoking the execution chain).
>
> Please see the attached patch.
>
> Thank you,
> Chamikara
>
>
> ------------------------------------------------------------------------
>
> Index: engine/AxisEngine.java
> ===================================================================
> --- engine/AxisEngine.java (revision 356796)
> +++ engine/AxisEngine.java (working copy)
> @@ -66,12 +66,13 @@
> public void send(MessageContext msgContext) throws AxisFault {
> //find and invoke the Phases
> OperationContext operationContext = msgContext.getOperationContext();
> - ArrayList executionChain = operationContext.getAxisOperation().getPhasesOutFlow();
> - msgContext.setExecutionChain((ArrayList) executionChain.clone ());
> + ArrayList outFlow = (ArrayList) operationContext.getAxisOperation().getPhasesOutFlow().clone();
> + ArrayList globalOutFlow = (ArrayList) msgContext.getConfigurationContext().getAxisConfiguration().getGlobalOutPhases().clone();
> +
> + outFlow.addAll(globalOutFlow);
> +
> + msgContext.setExecutionChain(outFlow);
> msgContext.invoke();
> - msgContext.setExecutionChain((ArrayList)msgContext.getConfigurationContext().
> - getAxisConfiguration().getGlobalOutPhases().clone());
> - msgContext.invoke();
>
> if (!msgContext.isPaused()) {
> //write the Message to the Wire
> @@ -385,10 +386,29 @@
> }
>
>
> - public void resume(MessageContext msgctx)
> + public void resumeSend(MessageContext msgctx)
> throws AxisFault {
> msgctx.resume();
> +
> + if (!msgctx.isPaused()) {
> + //write the Message to the Wire
> + TransportOutDescription transportOut = msgctx.getTransportOut();
> + TransportSender sender = transportOut.getSender();
> + sender.invoke(msgctx);
> + }
> }
> +
> + public void resumeReceive(MessageContext msgctx)
> + throws AxisFault {
> + msgctx.resume();
> +
> + if (msgctx.isServerSide() && !msgctx.isPaused()) {
> + // invoke the Message Receivers
> + checkMustUnderstand(msgctx);
> + MessageReceiver receiver = msgctx.getAxisOperation().getMessageReceiver();
> + receiver.receive(msgctx);
> + }
> + }
>
> private String getSenderFaultCode(String soapNamespace) {
> return SOAP12Constants.SOAP_ENVELOPE_NAMESPACE_URI.equals(
>
>
>
