From ef5aef3cd883f1a772aa2261ddc3d82b0fafc669 Mon Sep 17 00:00:00 2001 From: CesarCoelho Date: Fri, 6 Oct 2023 09:51:38 +0200 Subject: [PATCH] Improves INVOKE state machine code --- .../mo/mal/impl/InteractionConsumerMap.java | 7 +- .../impl/patterns/InvokeInteractionImpl.java | 3 +- .../impl/state/InvokeOperationHandler.java | 99 ++++++++----------- .../mo/mal/impl/state/OperationHandler.java | 7 +- .../impl/state/ProgressOperationHandler.java | 25 +++-- .../impl/state/SubmitOperationHandler.java | 13 ++- .../mo/mal/test/patterns/PatternTest.java | 8 +- 7 files changed, 70 insertions(+), 92 deletions(-) diff --git a/mal-impl/src/main/java/esa/mo/mal/impl/InteractionConsumerMap.java b/mal-impl/src/main/java/esa/mo/mal/impl/InteractionConsumerMap.java index cab895df1..8c5012571 100644 --- a/mal-impl/src/main/java/esa/mo/mal/impl/InteractionConsumerMap.java +++ b/mal-impl/src/main/java/esa/mo/mal/impl/InteractionConsumerMap.java @@ -201,7 +201,6 @@ public MALMessage waitForResponse(final Long id) throws MALInteractionException, public void handleStage(final MALMessage msg) throws MALInteractionException, MALException { final Long id = msg.getHeader().getTransactionId(); OperationHandler handler; - StateMachineDetails state = null; synchronized (transactions) { handler = transactions.get(id); @@ -218,7 +217,7 @@ public void handleStage(final MALMessage msg) throws MALInteractionException, MA throw new MALException(txt); } - state = handler.handleStage(msg); + handler.handleStage(msg); // delete entry from trans map if (handler.finished()) { @@ -227,10 +226,6 @@ public void handleStage(final MALMessage msg) throws MALInteractionException, MA transactions.remove(id); } } - - synchronized (handler) { - handler.processStage(state); - } } public void handleError(final MALMessageHeader hdr, final MOErrorException err, final Map qosMap) { diff --git a/mal-impl/src/main/java/esa/mo/mal/impl/patterns/InvokeInteractionImpl.java b/mal-impl/src/main/java/esa/mo/mal/impl/patterns/InvokeInteractionImpl.java index f651c41df..16a963236 100644 --- a/mal-impl/src/main/java/esa/mo/mal/impl/patterns/InvokeInteractionImpl.java +++ b/mal-impl/src/main/java/esa/mo/mal/impl/patterns/InvokeInteractionImpl.java @@ -80,8 +80,7 @@ public MALMessage sendResponse(final MALEncodedBody body) } @Override - public MALMessage sendError(final MOErrorException error) - throws MALException { + public MALMessage sendError(final MOErrorException error) throws MALException { UOctet stage = MALInvokeOperation.INVOKE_ACK_STAGE; if (ackSent) { diff --git a/mal-impl/src/main/java/esa/mo/mal/impl/state/InvokeOperationHandler.java b/mal-impl/src/main/java/esa/mo/mal/impl/state/InvokeOperationHandler.java index b75b48154..b20480985 100644 --- a/mal-impl/src/main/java/esa/mo/mal/impl/state/InvokeOperationHandler.java +++ b/mal-impl/src/main/java/esa/mo/mal/impl/state/InvokeOperationHandler.java @@ -27,7 +27,7 @@ import org.ccsds.moims.mo.mal.MALInteractionException; import org.ccsds.moims.mo.mal.MALInvokeOperation; import org.ccsds.moims.mo.mal.MOErrorException; -import org.ccsds.moims.mo.mal.structures.InteractionType; +import org.ccsds.moims.mo.mal.consumer.MALInteractionListener; import org.ccsds.moims.mo.mal.transport.MALErrorBody; import org.ccsds.moims.mo.mal.transport.MALMessage; import org.ccsds.moims.mo.mal.transport.MALMessageHeader; @@ -39,6 +39,7 @@ public final class InvokeOperationHandler extends OperationHandler { private boolean receivedAck = false; private boolean receivedResponse = false; + private boolean finished = false; /** * Constructor. @@ -52,74 +53,58 @@ public InvokeOperationHandler(final boolean syncOperation, } @Override - public StateMachineDetails handleStage(final MALMessage msg) throws MALInteractionException { - final int interactionType = msg.getHeader().getInteractionType().getOrdinal(); - final int interactionStage = msg.getHeader().getInteractionStage().getValue(); - if (!receivedAck) { - if ((interactionType == InteractionType._INVOKE_INDEX) && (interactionStage == MALInvokeOperation._INVOKE_ACK_STAGE)) { - receivedAck = true; - if (!isSynchronous && msg.getHeader().getIsErrorMessage()) { - receivedResponse = true; - } - return new StateMachineDetails(msg, false); - } else { - receivedResponse = true; - logUnexpectedTransitionError(interactionType, interactionStage); - return new StateMachineDetails(msg, true); - } - } else if ((!receivedResponse) - && (interactionType == InteractionType._INVOKE_INDEX) - && (interactionStage == MALInvokeOperation._INVOKE_RESPONSE_STAGE)) { - receivedResponse = true; - return new StateMachineDetails(msg, false); - } else { - logUnexpectedTransitionError(interactionType, interactionStage); - receivedResponse = true; - return new StateMachineDetails(msg, true); - } - } - - @Override - public void processStage(final StateMachineDetails state) throws MALInteractionException { - MALMessageHeader header = state.getMessage().getHeader(); - final int interactionStage = header.getInteractionStage().getValue(); + public void handleStage(final MALMessage msg) throws MALInteractionException { + MALMessageHeader header = msg.getHeader(); boolean isError = header.getIsErrorMessage(); + int interactionStage = header.getInteractionStage().getValue(); + MALInteractionListener listener = responseHolder.getListener(); + Map qos = msg.getQoSProperties(); try { - if (interactionStage == MALInvokeOperation._INVOKE_ACK_STAGE) { - if (isSynchronous) { - responseHolder.signalResponse(isError, state.getMessage()); - } else { - if (isError) { - responseHolder.getListener().invokeAckErrorReceived(header, - (MALErrorBody) state.getMessage().getBody(), - state.getMessage().getQoSProperties()); + if (!receivedAck) { + if (interactionStage == MALInvokeOperation._INVOKE_ACK_STAGE) { + receivedAck = true; + if (isSynchronous) { + responseHolder.signalResponse(isError, msg); } else { - responseHolder.getListener().invokeAckReceived(header, - state.getMessage().getBody(), - state.getMessage().getQoSProperties()); + if (isError) { + finished = true; + listener.invokeAckErrorReceived(header, (MALErrorBody) msg.getBody(), qos); + } else { + listener.invokeAckReceived(header, msg.getBody(), qos); + } } + } else { + finished = true; + header.setIsErrorMessage(true); + listener.invokeAckErrorReceived(header, ERROR, qos); } + return; } - if (interactionStage == MALInvokeOperation._INVOKE_RESPONSE_STAGE) { - if (isError || !receivedAck) { - responseHolder.getListener().invokeResponseErrorReceived(header, - (MALErrorBody) state.getMessage().getBody(), - state.getMessage().getQoSProperties()); + if (receivedAck && !receivedResponse) { + finished = true; + if (interactionStage == MALInvokeOperation._INVOKE_RESPONSE_STAGE) { + receivedResponse = true; + if (isError) { + listener.invokeResponseErrorReceived(header, (MALErrorBody) msg.getBody(), qos); + } else { + listener.invokeResponseReceived(header, msg.getBody(), msg.getQoSProperties()); + } } else { - responseHolder.getListener().invokeResponseReceived(header, - state.getMessage().getBody(), - state.getMessage().getQoSProperties()); + // The received MAL Header has the interactionStage set at 2 because it is ACK + // However, the response expects a 3 because it comes through the Response Error method! + // To fix, we need to force the field to be 2!!! + // The Correct Solution should be to actually create a "Transition error" on + // the interface, so we can push the correct messages to the top layer + // without modifying them + listener.invokeResponseErrorReceived(header, ERROR, qos); } - } - if (state.isIncorrectState()) { - MALErrorBody errorBody = (MALErrorBody) state.getMessage().getBody(); - throw new MALInteractionException(errorBody.getError()); + return; } } catch (MALException ex) { // nothing we can do with this MALContextFactoryImpl.LOGGER.log(Level.WARNING, - "Exception thrown handling stage {0}", ex); + "Exception thrown handling stage: " + interactionStage, ex); } } @@ -145,6 +130,6 @@ public synchronized void handleError(final MALMessageHeader hdr, @Override public synchronized boolean finished() { - return receivedResponse; + return finished; } } diff --git a/mal-impl/src/main/java/esa/mo/mal/impl/state/OperationHandler.java b/mal-impl/src/main/java/esa/mo/mal/impl/state/OperationHandler.java index 6b9b96e0c..1c78a9195 100644 --- a/mal-impl/src/main/java/esa/mo/mal/impl/state/OperationHandler.java +++ b/mal-impl/src/main/java/esa/mo/mal/impl/state/OperationHandler.java @@ -23,6 +23,7 @@ import esa.mo.mal.impl.MALContextFactoryImpl; import java.util.Map; import java.util.logging.Level; +import org.ccsds.moims.mo.mal.MALHelper; import org.ccsds.moims.mo.mal.MALInteractionException; import org.ccsds.moims.mo.mal.MOErrorException; import org.ccsds.moims.mo.mal.structures.InteractionType; @@ -34,6 +35,8 @@ */ public abstract class OperationHandler { + protected static final DummyErrorBody ERROR + = new DummyErrorBody(new MOErrorException(MALHelper.INCORRECT_STATE_ERROR_NUMBER, null)); protected final boolean isSynchronous; protected final OperationResponseHolder responseHolder; @@ -42,9 +45,7 @@ protected OperationHandler(final boolean isSynchronous, final OperationResponseH this.responseHolder = responseHolder; } - public abstract StateMachineDetails handleStage(final MALMessage msg) throws MALInteractionException; - - public abstract void processStage(final StateMachineDetails details) throws MALInteractionException; + public abstract void handleStage(final MALMessage msg) throws MALInteractionException; public abstract void handleError(final MALMessageHeader hdr, final MOErrorException err, final Map qosMap); diff --git a/mal-impl/src/main/java/esa/mo/mal/impl/state/ProgressOperationHandler.java b/mal-impl/src/main/java/esa/mo/mal/impl/state/ProgressOperationHandler.java index e8ddffdf2..d7099d591 100644 --- a/mal-impl/src/main/java/esa/mo/mal/impl/state/ProgressOperationHandler.java +++ b/mal-impl/src/main/java/esa/mo/mal/impl/state/ProgressOperationHandler.java @@ -52,10 +52,13 @@ public ProgressOperationHandler(final boolean syncOperation, } @Override - public StateMachineDetails handleStage(final MALMessage msg) throws MALInteractionException { - final int interactionType = msg.getHeader().getInteractionType().getOrdinal(); - final int interactionStage = msg.getHeader().getInteractionStage().getValue(); - boolean isError = msg.getHeader().getIsErrorMessage(); + public void handleStage(final MALMessage msg) throws MALInteractionException { + MALMessageHeader header = msg.getHeader(); + final int interactionType = header.getInteractionType().getOrdinal(); + final int interactionStage = header.getInteractionStage().getValue(); + boolean isError = header.getIsErrorMessage(); + StateMachineDetails state; + synchronized (this) { if (!receivedAck) { if ((interactionType == InteractionType._PROGRESS_INDEX) @@ -64,11 +67,11 @@ public StateMachineDetails handleStage(final MALMessage msg) throws MALInteracti if (isError) { receivedResponse = true; } - return new StateMachineDetails(msg, false); + state = new StateMachineDetails(msg, false); } else { receivedResponse = true; logUnexpectedTransitionError(interactionType, interactionStage); - return new StateMachineDetails(msg, true); + state = new StateMachineDetails(msg, true); } } else if ((!receivedResponse) && (interactionType == InteractionType._PROGRESS_INDEX) && ((interactionStage == MALProgressOperation._PROGRESS_UPDATE_STAGE) @@ -80,20 +83,14 @@ public StateMachineDetails handleStage(final MALMessage msg) throws MALInteracti } else { receivedResponse = true; } - return new StateMachineDetails(msg, false); + state = new StateMachineDetails(msg, false); } else { receivedResponse = true; logUnexpectedTransitionError(interactionType, interactionStage); - return new StateMachineDetails(msg, true); + state = new StateMachineDetails(msg, true); } } - } - @Override - public void processStage(final StateMachineDetails state) throws MALInteractionException { - MALMessageHeader header = state.getMessage().getHeader(); - final int interactionStage = header.getInteractionStage().getValue(); - boolean isError = header.getIsErrorMessage(); try { if (interactionStage == MALProgressOperation._PROGRESS_ACK_STAGE) { if (isSynchronous) { diff --git a/mal-impl/src/main/java/esa/mo/mal/impl/state/SubmitOperationHandler.java b/mal-impl/src/main/java/esa/mo/mal/impl/state/SubmitOperationHandler.java index 1e7d0dc0d..fef62212a 100644 --- a/mal-impl/src/main/java/esa/mo/mal/impl/state/SubmitOperationHandler.java +++ b/mal-impl/src/main/java/esa/mo/mal/impl/state/SubmitOperationHandler.java @@ -74,26 +74,25 @@ protected SubmitOperationHandler(final int interactionType, final int interactio } @Override - public synchronized StateMachineDetails handleStage(final MALMessage msg) throws MALInteractionException { + public synchronized void handleStage(final MALMessage msg) throws MALInteractionException { + StateMachineDetails state; + if (!receivedInitialStage) { if ((interactionType == msg.getHeader().getInteractionType().getOrdinal()) && checkStage(msg.getHeader().getInteractionStage().getValue())) { receivedInitialStage = true; - return new StateMachineDetails(msg, false); + state = new StateMachineDetails(msg, false); } else { logUnexpectedTransitionError(msg.getHeader().getInteractionType().getOrdinal(), msg.getHeader().getInteractionStage().getValue()); - return new StateMachineDetails(msg, true); + state = new StateMachineDetails(msg, true); } } else { logUnexpectedTransitionError(interactionType, interactionStage); - return new StateMachineDetails(msg, true); + state = new StateMachineDetails(msg, true); } - } - @Override - public void processStage(final StateMachineDetails state) throws MALInteractionException { if (receivedInitialStage) { try { if (isSynchronous) { diff --git a/testbeds/testbed-mal/src/main/java/org/ccsds/moims/mo/mal/test/patterns/PatternTest.java b/testbeds/testbed-mal/src/main/java/org/ccsds/moims/mo/mal/test/patterns/PatternTest.java index 3c31e53d6..a2c8faa10 100644 --- a/testbeds/testbed-mal/src/main/java/org/ccsds/moims/mo/mal/test/patterns/PatternTest.java +++ b/testbeds/testbed-mal/src/main/java/org/ccsds/moims/mo/mal/test/patterns/PatternTest.java @@ -386,7 +386,8 @@ private boolean addRequestReturnAssertions(ResponseListener monitor, int procedu return (null != msgHeaderFinal); } - private void testInvoke(ResponseListener monitor, IPTestStub ipTest, boolean callMultiVersion, boolean callEmptyVersion, IPTestDefinition testDef) throws Exception { + private void testInvoke(ResponseListener monitor, IPTestStub ipTest, boolean callMultiVersion, + boolean callEmptyVersion, IPTestDefinition testDef) throws Exception { LoggingBase.logMessage("PatternTest.testInvoke(" + callMultiVersion + ")"); if (callMultiVersion) { @@ -448,12 +449,13 @@ private boolean addInvokeReturnAssertions(ResponseListener monitor, int procedur msgHeaderFinal = msgHeaderAck; } - LoggingBase.logMessage("PatternTest.testInvoke(" + msgHeaderAck + "," + msgHeaderFinal + ")"); + LoggingBase.logMessage("PatternTest.addInvokeReturnAssertions(\n" + msgHeaderAck + " ,\n" + msgHeaderFinal + ")"); return (null != msgHeaderFinal); } - private void testProgress(ResponseListener monitor, IPTestStub ipTest, boolean callMultiVersion, boolean callEmptyVersion, IPTestDefinition testDef) throws Exception { + private void testProgress(ResponseListener monitor, IPTestStub ipTest, boolean callMultiVersion, + boolean callEmptyVersion, IPTestDefinition testDef) throws Exception { LoggingBase.logMessage("PatternTest.testProgress(" + callMultiVersion + ")"); synchronized (monitor) {