ATS-250 : ATS: T-Engines: Improve the convert message error handling (T-Base) (#51)

- code refactoring:
    if successful -> return the object ( Optional.ofNullable(request) )
    if not successful -> throw an exception (instead of sending an error message on the queue)
- when calling 'convert()' method, catch this exception and send a T-Reply with error message on the queue
- add correlationID to error logs

alfresco-transformer-base/src/main/java/org/alfresco/transformer/QueueTransformService.java

@@ -27,6 +27,7 @@ import javax.jms.Message;
 
 import org.alfresco.transform.client.model.TransformReply;
 import org.alfresco.transform.client.model.TransformRequest;
+import org.alfresco.transform.exceptions.TransformException;
 import org.alfresco.transformer.messaging.TransformMessageConverter;
 import org.alfresco.transformer.messaging.TransformReplySender;
 import org.slf4j.Logger;
@@ -37,6 +38,8 @@ import org.springframework.jms.annotation.JmsListener;
 import org.springframework.jms.support.converter.MessageConversionException;
 import org.springframework.stereotype.Component;
 
+import java.util.Optional;
+
 /**
  * Queue Transformer service.
  * This service reads all the requests for the particular engine, forwards them to the worker
@@ -92,77 +95,73 @@ public class QueueTransformService
             return;
         }
 
-        logger.info("New T-Request from queue with correlationId: {0}", correlationId);
+        logger.info("New T-Request from queue with correlationId: {}", correlationId);
 
-
+        Optional<TransformRequest> transformRequest;
-        TransformRequest transformRequest = convert(msg, correlationId, replyToDestinationQueue);
+        try
-        if (transformRequest == null)
         {
-            logger.error("Exception during T-Request deserialization! T-Reply with error has been "
+            transformRequest = convert(msg, correlationId);
-                + "sent to T-Router!");
+        }
+        catch (TransformException e)
+        {
+            logger.error(e.getMessage(), e);
+            replyWithError(replyToDestinationQueue, HttpStatus.valueOf(e.getStatusCode()),
+                e.getMessage(), correlationId);
             return;
         }
 
-        // Tries to convert and return the object. If it fails to convert, the method sends an error message and returns null.
+        if (!transformRequest.isPresent())
-        TransformReply reply = transformController.transform(
+        {
-            transformRequest, null).getBody();
+            logger.error("T-Request from message with correlationID {} is null!", correlationId);
+            replyWithInternalSvErr(replyToDestinationQueue,
+                "JMS exception during T-Request deserialization: ", correlationId);
+            return;
+        }
+
+        TransformReply reply = transformController.transform(transformRequest.get(), null)
+            .getBody();
 
         transformReplySender.send(replyToDestinationQueue, reply);
     }
 
     /**
      * Tries to convert the JMS {@link Message} to a {@link TransformRequest}
-     * If any errors occur standard error {@link TransformReply} are sent back to T-Router
+     * If any error occurs, a {@link TransformException} is thrown
      *
      * @param msg Message to be deserialized
-     * @param correlationId CorrelationId of the message
+     * @return The converted {@link TransformRequest} instance
-     * @param destination Needed in case deserialization fails. Passed here so we don't retrieve it again.
-     * @return The converted {@link TransformRequest} instance or null in case of errors
      */
-    private TransformRequest convert(final Message msg, final String correlationId, Destination destination)
+    private Optional<TransformRequest> convert(final Message msg, String correlationId)
     {
         try
         {
             TransformRequest request = (TransformRequest) transformMessageConverter
                 .fromMessage(msg);
-            if (request == null)
+            return Optional.ofNullable(request);
-            {
-                logger.error("T-Request is null deserialization!");
-                replyWithInternalSvErr(destination,
-                    "JMS exception during T-Request deserialization: ", correlationId);
-            }
-            return request;
         }
         catch (MessageConversionException e)
         {
-            String message = "Message conversion exception during T-Request deserialization: ";
+            String message =
-            logger.error(message + e.getMessage(), e);
+                "MessageConversionException during T-Request deserialization of message with correlationID "
-            replyWithBadRequest(destination, message + e.getMessage(), correlationId);
+                    + correlationId + ": ";
+            throw new TransformException(HttpStatus.BAD_REQUEST.value(), message + e.getMessage());
         }
         catch (JMSException e)
         {
-            String message = "JMS exception during T-Request deserialization: ";
+            String message =
-            logger.error(message + e.getMessage(), e);
+                "JMSException during T-Request deserialization of message with correlationID "
-            replyWithInternalSvErr(destination, message + e.getMessage(), correlationId);
+                    + correlationId + ": ";
+            throw new TransformException(HttpStatus.INTERNAL_SERVER_ERROR.value(),
+                message + e.getMessage());
         }
         catch (Exception e)
         {
-            String message = "Exception during T-Request deserialization: ";
+            String message =
-            logger.error(message + e.getMessage(), e);
+                "Exception during T-Request deserialization of message with correlationID "
-            replyWithInternalSvErr(destination, message + e.getMessage(), correlationId);
+                    + correlationId + ": ";
+            throw new TransformException(HttpStatus.INTERNAL_SERVER_ERROR.value(),
+                message + e.getMessage());
         }
-        catch (Throwable t)
-        {
-            logger.error("Error during T-Request deserialization" + t.getMessage(), t);
-            throw t;
-        }
-        return null;
-    }
-
-
-    private void replyWithBadRequest(final Destination destination, final String msg, final String correlationId)
-    {
-        replyWithError(destination, HttpStatus.BAD_REQUEST, msg, correlationId);
     }
 
     private void replyWithInternalSvErr(final Destination destination, final String msg, final String correlationId)

alfresco-transformer-base/src/test/java/org/alfresco/transformer/QueueTransformServiceTest.java

@@ -89,19 +89,21 @@ public class QueueTransformServiceTest
     public void testConvertMessageReturnsNullThenReplyWithInternalServerError() throws JMSException
     {
         ActiveMQObjectMessage msg = new ActiveMQObjectMessage();
+        msg.setCorrelationId("1234");
         ActiveMQQueue destination = new ActiveMQQueue();
         msg.setJMSReplyTo(destination);
 
         TransformReply reply = TransformReply.builder()
-            .withStatus(HttpStatus.INTERNAL_SERVER_ERROR.value())
+            .withStatus(HttpStatus.INTERNAL_SERVER_ERROR.value()).withErrorDetails(
-            .withErrorDetails("JMS exception during T-Request deserialization: ").build();
+                "JMS exception during T-Request deserialization of message with correlationID "
+                    + msg.getCorrelationId() + ": null").build();
 
         doReturn(null).when(transformMessageConverter).fromMessage(msg);
 
         queueTransformService.receive(msg);
 
         verify(transformMessageConverter).fromMessage(msg);
-        verify(transformReplySender).send(destination, reply, null);
+        verify(transformReplySender).send(destination, reply, msg.getCorrelationId());
 
         verifyNoMoreInteractions(transformController);
     }
@@ -111,19 +113,21 @@ public class QueueTransformServiceTest
         throws JMSException
     {
         ActiveMQObjectMessage msg = new ActiveMQObjectMessage();
+        msg.setCorrelationId("1234");
         ActiveMQQueue destination = new ActiveMQQueue();
         msg.setJMSReplyTo(destination);
 
         TransformReply reply = TransformReply.builder().withStatus(HttpStatus.BAD_REQUEST.value())
-            .withErrorDetails("Message conversion exception during T-Request deserialization: ")
+            .withErrorDetails(
-            .build();
+                "Message conversion exception during T-Request deserialization of message with correlationID"
+                    + msg.getCorrelationId() + ": null").build();
 
         doThrow(MessageConversionException.class).when(transformMessageConverter).fromMessage(msg);
 
         queueTransformService.receive(msg);
 
         verify(transformMessageConverter).fromMessage(msg);
-        verify(transformReplySender).send(destination, reply, reply.getRequestId());
+        verify(transformReplySender).send(destination, reply, msg.getCorrelationId());
 
         verifyNoMoreInteractions(transformController);
     }
@@ -133,18 +137,21 @@ public class QueueTransformServiceTest
         throws JMSException
     {
         ActiveMQObjectMessage msg = new ActiveMQObjectMessage();
+        msg.setCorrelationId("1234");
         ActiveMQQueue destination = new ActiveMQQueue();
         msg.setJMSReplyTo(destination);
 
-        TransformReply reply = TransformReply.builder().withStatus(HttpStatus.BAD_REQUEST.value())
+        TransformReply reply = TransformReply.builder()
-            .withErrorDetails("JMS exception during T-Request deserialization: ").build();
+            .withStatus(HttpStatus.INTERNAL_SERVER_ERROR.value()).withErrorDetails(
+                "JMSException during T-Request deserialization of message with correlationID " + msg
+                    .getCorrelationId() + ": null").build();
 
         doThrow(JMSException.class).when(transformMessageConverter).fromMessage(msg);
 
         queueTransformService.receive(msg);
 
         verify(transformMessageConverter).fromMessage(msg);
-        verify(transformReplySender).send(destination, reply, reply.getRequestId());
+        verify(transformReplySender).send(destination, reply, msg.getCorrelationId());
 
         verifyNoMoreInteractions(transformController);
     }