Endpoint /generateAndPublish doesn't validate all events correctly (#318)

Endpoint /generateAndPublish uses MsgService.validateMsg() method to
validate generated event.

Author: z-sztrom

CHANGELOG.md

@@ -1,4 +1,7 @@
 
+## 2.1.10
+  Message validation generalized for /generateAndPublish
+
 ## 2.1.9
   com.fasterxml.jackson.core:jackson-core uplifted
 

pom.xml

@@ -9,7 +9,7 @@
         <version>2.0.15</version>
     </parent>
     <properties>
-        <eiffel-remrem-publish.version>2.1.9</eiffel-remrem-publish.version>
+        <eiffel-remrem-publish.version>2.1.10</eiffel-remrem-publish.version>
         <eiffel-remrem-semantics.version>2.4.2</eiffel-remrem-semantics.version>
     </properties>
     <artifactId>eiffel-remrem-publish</artifactId>

publish-service/src/main/java/com/ericsson/eiffel/remrem/publish/controller/ProducerController.java

@@ -16,6 +16,7 @@
 
 import java.util.*;
 
+import com.ericsson.eiffel.remrem.protocol.ValidationResult;
 import com.ericsson.eiffel.remrem.publish.service.*;
 import com.google.gson.*;
 import org.slf4j.LoggerFactory;
@@ -392,6 +393,8 @@ public ResponseEntity generateAndPublish(final String msgProtocol, final String
 
             responseStatus = response.getStatusCode();
             String responseBody = null;
+            // TODO We should not rely on bodyJson (an input string), but rather on given
+            //      result, i.e. response.getBody() and check this for type (object or array).
             if (bodyJson.isJsonObject()) {
                 responseBody = "[" + response.getBody() + "]";
             } else if (bodyJson.isJsonArray()) {
@@ -407,8 +410,8 @@ public ResponseEntity generateAndPublish(final String msgProtocol, final String
                 if (msgService != null && msgProtocol != null) {
                     rmqHelper.rabbitMqPropertiesInit(msgProtocol);
                 }
-                responseEvents = processingValidEvent(responseBody, msgProtocol, userDomain,
-                        tag, routingKey);
+                responseEvents = processingValidEvent(responseBody, msgProtocol, msgType, userDomain,
+                        tag, routingKey, okToLeaveOutInvalidOptionalFields);
             } else {
                 return response;
             }
@@ -423,7 +426,8 @@ public ResponseEntity generateAndPublish(final String msgProtocol, final String
             } else if (bodyJson.isJsonArray()) {
                 responseBody = responseMessage;
             }
-            responseEvents = processingValidEvent(responseBody, msgProtocol, userDomain, tag, routingKey);
+            responseEvents = processingValidEvent(responseBody, msgProtocol, msgType, userDomain,
+                    tag, routingKey, okToLeaveOutInvalidOptionalFields);
             return new ResponseEntity<>(responseEvents, HttpStatus.BAD_REQUEST);
         }
         //Status here is the status returned from generate service, except BAD_REQUEST which already handled above
@@ -477,8 +481,8 @@ public void initializeResponse(HttpStatus status, String errorMessage, String re
      * @return list of responses
      */
     // TODO Why public?
-    public List<Map<String, Object>> processingValidEvent(String eventResponseMessage, final String msgProtocol, final String userDomain,
-                                                          final String tag, final String routingKey) {
+    public List<Map<String, Object>> processingValidEvent(String eventResponseMessage, final String msgProtocol, final String msgType,
+                                                          final String userDomain, final String tag, final String routingKey, final Boolean okToLeaveOutInvalidOptionalFields) {
         MsgService msgService = PublishUtils.getMessageService(msgProtocol, msgServices);
         List<Map<String, Object>> responseEvent = new ArrayList<>();
         Map<String, Object> eventResponse = null;
@@ -510,16 +514,15 @@ public List<Map<String, Object>> processingValidEvent(String eventResponseMessag
         }
 
         for (int i = 0; i < eventArray.size(); i++) {
+            // TODO Having hash map here is very chaotic: key are sometimes predefined values,
+            //      e.g. JSON_STATUS_RESULT, JSON_EVENT_MESSAGE_FIELD, but also a general string
+            //      as in case of reporting a bad request.
             eventResponse = new HashMap<>();
-            JsonElement jsonResponseEvent = eventArray.get(i);
-            if (isValid(jsonResponseEvent)) {
-                JsonObject validResponse = jsonResponseEvent.getAsJsonObject();
-                if (validResponse == null) {
-                    String errorMessage = "Invalid, array item expected to be in the form of JSON object";
-                    log.error(errorMessage);
-                    eventResponse.put(errorMessage, HttpStatus.BAD_REQUEST);
-                }
-                String validResponseBody = validResponse.toString();
+            // TODO Could it be null?
+            JsonObject jsonResponseEvent = eventArray.get(i).getAsJsonObject();
+            ValidationResult validationResult = msgService.validateMsg(msgType, jsonResponseEvent, okToLeaveOutInvalidOptionalFields);
+            if (validationResult.isValid()) {
+                String validResponseBody = jsonResponseEvent.toString();
                 SendResult result = messageService.send(validResponseBody, msgService, userDomain, tag, routingKey);
                 eventResponse.put(JSON_STATUS_RESULT, result);
             } else {
@@ -540,18 +543,6 @@ public JsonObject getAsJsonObject(JsonElement jsonElement) {
         return jsonObject;
     }
 
-    /**
-     * This helper method check the json event is valid or not
-     * @param jsonElement An event which need to check
-     * @return boolean type value like it is valid or not
-     */
-    private boolean isValid(JsonElement jsonElement) {
-        JsonObject jsonObject = getAsJsonObject(jsonElement);
-        return jsonObject != null &&
-            jsonObject.has(META) && jsonObject.has(DATA) && jsonObject.has(LINKS) &&
-            !jsonObject.has(JSON_STATUS_CODE);
-    }
-
     /**
      * @return this method returns the current version of publish and all loaded
      *         protocols.
@@ -563,5 +554,4 @@ public JsonElement getVersions() {
         Map<String, Map<String, String>> versions = new VersionService().getMessagingVersions();
         return parser.parse(versions.toString());
     }
-
 }

publish-service/src/test/java/com/ericsson/eiffel/remrem/publish/service/EiffelRemremCommonControllerUnitTest.java

@@ -23,6 +23,7 @@
 import java.util.HashMap;
 import java.util.Map;
 
+import com.ericsson.eiffel.remrem.protocol.ValidationResult;
 import org.junit.Before;
 import org.junit.Test;
 import org.junit.runner.RunWith;
@@ -111,6 +112,7 @@ public void setUp() throws Exception {
 
         when(service.getServiceName()).thenReturn("eiffelsemantics");
         when(service2.getServiceName()).thenReturn("eiffelsemantics");
+        when(service.validateMsg(ArgumentMatchers.anyString(), ArgumentMatchers.any(JsonObject.class), ArgumentMatchers.anyBoolean())).thenReturn(new ValidationResult(true, "OK"));
         // TODO remove in a future
         // 'getHttpStatus' is no more available in MessageService as it required a status value
         // preventing parallel correct publishing of events.