Modified endpoint generateAndPublish to accept array of events #294

shudhansu-shekhar · 2024-06-10T12:02:48Z

Applicable Issues

Description of the Change

In this after these changes, publish service generateAndPublish endpoint accept array of events

Alternate Designs

Possible Drawbacks

Sign-off

Developer's Certificate of Origin 1.1

By making a contribution to this project, I certify that:

(a) The contribution was created in whole or in part by me and I
have the right to submit it under the open source license
indicated in the file; or

(b) The contribution is based upon previous work that, to the best
of my knowledge, is covered under an appropriate open source
license and I have the right under that license to submit that
work with modifications, whether created in whole or in part
by me, under the same open source license (unless I am
permitted to submit under a different license), as indicated
in the file; or

(c) The contribution was provided directly to me by some other
person who certified (a), (b) or (c) and I have not modified
it.

(d) I understand and agree that this project and the contribution
are public and that a record of the contribution (including all
personal information I submit with it, including my sign-off) is
maintained indefinitely and may be redistributed consistent with
this project or the open source license(s) involved.

Signed-off-by: shudhansu.shekhar.ext@ericsson.com

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

z-sztrom · 2024-06-10T15:32:13Z

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

+                }
+            }
+        } else {
+            return new ResponseEntity<>("Invalid event content", HttpStatus.BAD_REQUEST);


Can user understand what is wrong?

z-sztrom · 2024-06-10T15:35:47Z

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

+            HttpHeaders headers = new HttpHeaders();
+            headers.setContentType(MediaType.APPLICATION_JSON_UTF8);
+            HttpEntity<String> entity = new HttpEntity<>(bodyJsonOut, headers);
+            EnumSet<HttpStatus> getStatus = EnumSet.of(HttpStatus.SERVICE_UNAVAILABLE, HttpStatus.UNAUTHORIZED, HttpStatus.NOT_ACCEPTABLE, HttpStatus.EXPECTATION_FAILED, HttpStatus.INTERNAL_SERVER_ERROR, HttpStatus.UNPROCESSABLE_ENTITY);


Why you create a const object over and over?

z-sztrom · 2024-06-10T15:37:38Z

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

+                String generateUrl = generateURLTemplate.getUrl() + "&failIfMultipleFound=" + failIfMultipleFound
+                        + "&failIfNoneFound=" + failIfNoneFound + "&lookupInExternalERs=" + lookupInExternalERs
+                        + "&lookupLimit=" + lookupLimit + "&okToLeaveOutInvalidOptionalFields=" + okToLeaveOutInvalidOptionalFields;
+                ResponseEntity<String> response = restTemplate.postForEntity(generateUrl,


Why send just one vent to generate service? There's generate endpoint accepting array of events. This restores the former behaviour: a network connection per event.

z-sztrom · 2024-06-19T11:50:05Z

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

+        } catch (JsonSyntaxException e) {
+            JsonObject errorResponse = new JsonObject();
+            log.error("Unexpected exception caught due to parse json data", e.getMessage());
+            String exceptionMessage = e.getMessage();


Use exceptionMessage at line above.

z-sztrom · 2024-06-19T11:51:47Z

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

+            log.error("Unexpected exception caught due to parse json data", e.getMessage());
+            String exceptionMessage = e.getMessage();
+            errorResponse.addProperty("Status code", HttpStatus.BAD_REQUEST.value());
+            errorResponse.addProperty("result", "Fatal");


Upper-case vs. lower-case property names. Remember, JSON is case-sensitive. If the case is mismatched, user's handlers may not work properly. Define constants for property names.

z-sztrom · 2024-06-19T11:53:32Z

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

+                if (element.isJsonObject()) {
+                    events.add(element.getAsJsonObject());
+                } else {
+                    return new ResponseEntity<>("Invalid events content or format", HttpStatus.BAD_REQUEST);


Why not to say directly: "Array item must be a JSON object"?

yes we can do like that

z-sztrom · 2024-06-19T11:54:20Z

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

+                }
+            }
+        } else {
+            return new ResponseEntity<>("Invalid, event is not in the correct format", HttpStatus.BAD_REQUEST);


Again, very confusing message. Inform directly what went wrong.

z-sztrom · 2024-06-19T11:55:19Z

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

-        headers.setContentType(MediaType.APPLICATION_JSON_UTF8);
-        HttpEntity<String> entity = new HttpEntity<>(bodyJsonOut, headers);
-        EnumSet<HttpStatus> getStatus = EnumSet.of(HttpStatus.SERVICE_UNAVAILABLE, HttpStatus.UNAUTHORIZED, HttpStatus.NOT_ACCEPTABLE, HttpStatus.EXPECTATION_FAILED, HttpStatus.INTERNAL_SERVER_ERROR, HttpStatus.UNPROCESSABLE_ENTITY);
+                    // -- parse params in incoming request -> body -------------


Is the comment still valid?

z-sztrom · 2024-06-19T11:56:51Z

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

-                // publishing requires an array if you want status code
-                String responseBody = "[" + response.getBody() + "]";
-                MsgService msgService = PublishUtils.getMessageService(msgProtocol, msgServices);
+                JsonArray responseArray = JsonParser.parseString(response.getBody()).getAsJsonArray();


What if getJsonArray() fails?

Yeah from his documentation, it seems it will throw exception with message like Not a JSON Array

z-sztrom · 2024-06-19T11:58:59Z

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

+                        }
+
+                            synchronized (this) {
+                                JsonObject jsonObject = jsonResponse.getAsJsonObject();


What if getJsonObject() fails?

By his method documentation, it seems it will throw exception with message like not a jsonObject

z-sztrom · 2024-06-19T12:02:56Z

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

                }
            } else {
-                return response;
+                return new ResponseEntity<>("Invalid Json event content", HttpStatus.BAD_REQUEST);


Why not to forward the message obtained from generate service?

yes we can do but in the else part not sure ,for some reason event processing failed so we should notify user with detailed or more failed info right?

z-sztrom · 2024-06-19T12:03:39Z

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

+        try {
+            return jsonObject.has("meta") && jsonObject.has("data") &&
+                    jsonObject.has("links") && !jsonObject.has("Status code");
+        } catch (Exception e) {


When this exception can be thrown?

This exception throw for various reason like if there is any incorrect json structure, unexpected data types etc

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

+            return generateAndPublish(msgProtocol, msgType, userDomain, tag, routingKey, parseData, failIfMultipleFound,
+                    failIfNoneFound, lookupInExternalERs, lookupLimit, okToLeaveOutInvalidOptionalFields, bodyJson);


z-sztrom · 2024-07-15T11:20:50Z

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

+                    eventResponse = new HashMap<>();
+                    JsonElement jsonResponseEvent = element.getAsJsonArray().get(i);
+                    if (isValid(jsonResponseEvent)) {
+                        synchronized (this) {


Why the same code starts at line 314?

Yeah put it one helper method

z-sztrom · 2024-07-15T11:21:32Z

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

+
+    private  Boolean isValid(JsonElement jsonObject) {
+        try {
+            return jsonObject.getAsJsonObject().has("meta") && jsonObject.getAsJsonObject().has("data") &&


getAsJsonObject() used over and over.

Can make one simple method for this

z-sztrom · 2024-07-15T11:21:50Z

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

+
+    private  Boolean isValid(JsonElement jsonObject) {
+        try {
+            return jsonObject.getAsJsonObject().has("meta") && jsonObject.getAsJsonObject().has("data") &&


Replace "meta", ... by constants.

z-sztrom · 2024-07-15T11:23:02Z

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

+        try {
+            return jsonObject.getAsJsonObject().has("meta") && jsonObject.getAsJsonObject().has("data") &&
+                    jsonObject.getAsJsonObject().has("links") && !jsonObject.getAsJsonObject().has("Status code");
+        } catch (Exception e) {


Don't handle general exception.

z-sztrom · 2024-07-17T09:59:19Z

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

+                                             @ApiParam(value = "JSON message", required = true) @RequestBody final String body){
+
+        try {
+            JsonElement bodyJson = JsonParser.parseString(body);


Should JSON parser be configured as in case of Generate service, i.e. not allowing duplicate keys, etc.?

Yeah can do this but why we need this here? Is it necessary if already there is check for correct event publishing means like incorrect event is not published on mb basically

OK, so we have the check in generator, right?

Yes that duplicate key check we implement basically in generate

z-sztrom

When generate service is down, the following error message is displayed
{
"timestamp": "Jul 29, 2024, 1:05:51 PM",
"status": 500,
"error": "Internal Server Error",
"path": "/generateAndPublish"
}
It should say clearly what's wrong
When an error is reported
{
"status message": {
"status code": 400,
"result": "{"status_code": 400, "result": "FAIL", "message":"Template is not correct format or something is missing in the template, please check"}",
"message": "fail"
}
Value of result shouldn't be escaped. Value of a property can be JSON object.
The error message above: failure is should be unified, i.e. "FAIL" vs. "fail".

z-sztrom

Please, add comment like "Done" or "Resolved" to resolved comments.

z-sztrom · 2024-07-31T09:23:17Z

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

@@ -88,7 +88,9 @@ public class ProducerController {

    public final String JSON_EVENT_MESSAGE_FIELD = "status message";

-    public final String JSON_EVENT_STATUS_VALUE = "status";
+    public final String STATUS_CODE = "Status code";


Why upper-case 'S'? Other properties are lower-case.

Yeah we need this Uppercase 'S' here

z-sztrom · 2024-07-31T09:25:06Z

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

        } else if (bodyJson.isJsonArray()) {
            for (JsonElement element : bodyJson.getAsJsonArray()) {
                if (element.isJsonObject()) {
-                    events.add(element.getAsJsonObject());
+                    events.add(getAsJsonObject(element));
                } else {
                    return new ResponseEntity<>("Invalid, array events must be a JSON object", HttpStatus.BAD_REQUEST);


I think there should be createResponesEntity method available.

z-sztrom · 2024-07-31T09:25:45Z

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

@@ -223,11 +225,11 @@ public ResponseEntity generateAndPublish(final String msgProtocol, final String
        MsgService msgService = PublishUtils.getMessageService(msgProtocol, msgServices);


Can we continue if msgService == null?

no we can do it here just like we do in generate ok?

if (msgService == null) {
return createResponseEntity(HttpStatus.SERVICE_UNAVAILABLE,
"No protocol service has been found registered", JSON_ERROR_STATUS).getBody();
}

Yes, that's how it should work.

z-sztrom · 2024-07-31T09:26:30Z

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

@@ -284,25 +286,8 @@ public ResponseEntity generateAndPublish(final String msgProtocol, final String
                if (msgService != null && msgProtocol != null) {


Why is msgServcie set to rmqHelper repeatedly?

Yeah but it seems set only once not repeatedly, please clear if i am wrong here

z-sztrom · 2024-07-31T09:30:02Z

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

+        List<Map<String, Object>> responseEvent = new ArrayList<>();
+        Map<String, Object> eventResponse;
+        JsonElement element = JsonParser.parseString(eventMsg);
+        for (int i = 0; i < element.getAsJsonArray().size(); i++) {


What is element isn't an array?

Basically here eventMsg is a response body of string type

z-sztrom · 2024-07-31T09:30:50Z

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

+        JsonElement element = JsonParser.parseString(eventMsg);
+        for (int i = 0; i < element.getAsJsonArray().size(); i++) {
+            eventResponse = new HashMap<>();
+            JsonElement jsonResponseEvent = element.getAsJsonArray().get(i);


Why to call getAsJsonArray() in each iteration?

yeah i put in one variable, use that one in each iteration right?

z-sztrom · 2024-07-31T09:31:29Z

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

+            JsonElement jsonResponseEvent = element.getAsJsonArray().get(i);
+            if (isValid(jsonResponseEvent)) {
+                synchronized (this) {
+                    JsonObject validResponse = jsonResponseEvent.getAsJsonObject();


Is it guaranteed that validResponse is non-null?

Yeah we can add check before doing this process like that ?

if (eventResponseMessage == null){
eventResponse.put("Parameter 'eventResponseMessage' must not be null", HttpStatus.BAD_REQUEST);
}

I don't mean eventResponseMessage, I mean validResponse.

yeah but here 'validResponse' is extracting from 'eventResponseMessage' So if this 'eventResponseMessage' is not null then validResponse will also not null because there is not much changes between that

z-sztrom · 2024-07-31T09:35:10Z

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

-            return jsonObject.getAsJsonObject().has("meta") && jsonObject.getAsJsonObject().has("data") &&
-                    jsonObject.getAsJsonObject().has("links") && !jsonObject.getAsJsonObject().has("Status code");
-        } catch (Exception e) {
+            return getAsJsonObject(jsonElement).has(META) && getAsJsonObject(jsonElement).has("data") &&


Use constants instead of hard-coded values, i.e. "data" and "links". There're constants defined in Event class or somewhere...

yeah i do that but i need defined that with in this class...There is no separate class for constant as in generate

z-sztrom · 2024-07-31T09:58:52Z

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

    private  Boolean isValid(JsonElement jsonElement) {
        try {
-            return getAsJsonObject(jsonElement).has(META) && getAsJsonObject(jsonElement).has("data") &&
-                    getAsJsonObject(jsonElement).has("links") && !getAsJsonObject(jsonElement).has(STATUS_CODE);
+            return getAsJsonObject(jsonElement).has(META) && getAsJsonObject(jsonElement).has(DATA) &&


As I wrote, use existing constants defined for event properties instead of defining new ones.

no there is not any existing constant, i have to define new one

z-sztrom · 2024-08-01T08:41:59Z

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

+        Map<String, Object> eventResponse = null;
+
+        if (eventResponseMessage == null) {
+            eventResponse.put("Parameter 'eventResponseMessage' must not be null", HttpStatus.BAD_REQUEST);


This is response to end user. How the user knows what 'eventResponseMessage' is?

yeah so we can put it like that?

eventResponse.put("Parameter 'eventResponseMessage' that is response generated by generate must not be null",
HttpStatus.BAD_REQUEST);

I'd write something like 'Missing response from 'generate' service.'

z-sztrom · 2024-08-01T08:43:13Z

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

+            return responseEvent;
+        }
+        JsonElement eventElement = JsonParser.parseString(eventResponseMessage);
+        JsonArray eventArray = eventElement.getAsJsonArray();


Is it guaranteed that eventElement is an array?

yeah i check it and it seems eventElement is an array, if you want i can show

I can't see that...

z-sztrom · 2024-08-01T08:45:24Z

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

        try {
            return getAsJsonObject(jsonElement).has(META) && getAsJsonObject(jsonElement).has(DATA) &&
-                    getAsJsonObject(jsonElement).has(LINKS) && !getAsJsonObject(jsonElement).has(STATUS_CODE);
+                    getAsJsonObject(jsonElement).has(LINKS) && !getAsJsonObject(jsonElement).has(JSON_STATUS_CODE);


Why the same code 'getAsJsonObject(jsonElement)' repeats 4 times?

z-sztrom · 2024-08-01T08:46:56Z

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

        try {
            return getAsJsonObject(jsonElement).has(META) && getAsJsonObject(jsonElement).has(DATA) &&
-                    getAsJsonObject(jsonElement).has(LINKS) && !getAsJsonObject(jsonElement).has(STATUS_CODE);
+                    getAsJsonObject(jsonElement).has(LINKS) && !getAsJsonObject(jsonElement).has(JSON_STATUS_CODE);
        } catch (IllegalStateException e) {
            log.error("Error while validating event: ", e.getMessage());


Event ID should be published whenever a message related to event is produced.

yeah right but i am not getting any scope or need might be here, might be i am wrong?

Something like ...get(META).get(ID), but I haven't checked...

Yeah it can do like this but not sure about the place you suggest to put not sure it will benefit anyway or not but yeah we can discuss once

shudhansu-shekhar

please check my comment

shudhansu-shekhar · 2024-06-20T08:03:21Z

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

+                if (element.isJsonObject()) {
+                    events.add(element.getAsJsonObject());
+                } else {
+                    return new ResponseEntity<>("Invalid events content or format", HttpStatus.BAD_REQUEST);


yes we can do like that

shudhansu-shekhar · 2024-06-20T08:07:09Z

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

+                }
+            }
+        } else {
+            return new ResponseEntity<>("Invalid, event is not in the correct format", HttpStatus.BAD_REQUEST);


shudhansu-shekhar · 2024-06-20T08:09:42Z

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

-                // publishing requires an array if you want status code
-                String responseBody = "[" + response.getBody() + "]";
-                MsgService msgService = PublishUtils.getMessageService(msgProtocol, msgServices);
+                JsonArray responseArray = JsonParser.parseString(response.getBody()).getAsJsonArray();


Yeah from his documentation, it seems it will throw exception with message like Not a JSON Array

shudhansu-shekhar · 2024-06-20T08:28:32Z

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

                }
            } else {
-                return response;
+                return new ResponseEntity<>("Invalid Json event content", HttpStatus.BAD_REQUEST);


yes we can do but in the else part not sure ,for some reason event processing failed so we should notify user with detailed or more failed info right?

shudhansu-shekhar · 2024-06-20T08:54:56Z

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

+        try {
+            return jsonObject.has("meta") && jsonObject.has("data") &&
+                    jsonObject.has("links") && !jsonObject.has("Status code");
+        } catch (Exception e) {


This exception throw for various reason like if there is any incorrect json structure, unexpected data types etc

shudhansu-shekhar · 2024-08-01T09:49:39Z

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

+            return responseEvent;
+        }
+        JsonElement eventElement = JsonParser.parseString(eventResponseMessage);
+        JsonArray eventArray = eventElement.getAsJsonArray();


yeah i check it and it seems eventElement is an array, if you want i can show

shudhansu-shekhar · 2024-08-01T09:55:50Z

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

        try {
            return getAsJsonObject(jsonElement).has(META) && getAsJsonObject(jsonElement).has(DATA) &&
-                    getAsJsonObject(jsonElement).has(LINKS) && !getAsJsonObject(jsonElement).has(STATUS_CODE);
+                    getAsJsonObject(jsonElement).has(LINKS) && !getAsJsonObject(jsonElement).has(JSON_STATUS_CODE);
        } catch (IllegalStateException e) {
            log.error("Error while validating event: ", e.getMessage());


yeah right but i am not getting any scope or need might be here, might be i am wrong?

shudhansu-shekhar · 2024-08-01T10:23:22Z

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

        try {
            return getAsJsonObject(jsonElement).has(META) && getAsJsonObject(jsonElement).has(DATA) &&
-                    getAsJsonObject(jsonElement).has(LINKS) && !getAsJsonObject(jsonElement).has(STATUS_CODE);
+                    getAsJsonObject(jsonElement).has(LINKS) && !getAsJsonObject(jsonElement).has(JSON_STATUS_CODE);
        } catch (IllegalStateException e) {
            log.error("Error while validating event: ", e.getMessage());


yeah right but i am not getting any scope or need might be here, might be i am wrong?

shudhansu-shekhar · 2024-08-01T10:24:13Z

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

            eventResponse = new HashMap<>();
-            JsonElement jsonResponseEvent = eventElement.getAsJsonArray().get(i);
+            JsonElement jsonResponseEvent = eventArray.get(i);
            if (isValid(jsonResponseEvent)) {
                synchronized (this) {
                    JsonObject validResponse = jsonResponseEvent.getAsJsonObject();


Yeah we can add check before doing this process like that ?

if (eventResponseMessage == null){
eventResponse.put("Parameter 'eventResponseMessage' must not be null", HttpStatus.BAD_REQUEST);
}

shudhansu-shekhar · 2024-08-01T10:24:35Z

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

+            return responseEvent;
+        }
+        JsonElement eventElement = JsonParser.parseString(eventResponseMessage);
+        JsonArray eventArray = eventElement.getAsJsonArray();


yeah i check it and it seems eventElement is an array, if you want i can show

z-sztrom · 2024-08-01T12:19:18Z

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

@@ -223,11 +225,11 @@ public ResponseEntity generateAndPublish(final String msgProtocol, final String
        MsgService msgService = PublishUtils.getMessageService(msgProtocol, msgServices);


Yes, that's how it should work.

z-sztrom · 2024-08-01T12:31:06Z

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

+        Map<String, Object> eventResponse = null;
+
+        if (eventResponseMessage == null) {
+            eventResponse.put("Parameter 'eventResponseMessage' must not be null", HttpStatus.BAD_REQUEST);


I'd write something like 'Missing response from 'generate' service.'

z-sztrom · 2024-08-01T12:31:45Z

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

+            return responseEvent;
+        }
+        JsonElement eventElement = JsonParser.parseString(eventResponseMessage);
+        JsonArray eventArray = eventElement.getAsJsonArray();


I can't see that...

z-sztrom · 2024-08-01T12:34:32Z

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

+            JsonElement jsonResponseEvent = element.getAsJsonArray().get(i);
+            if (isValid(jsonResponseEvent)) {
+                synchronized (this) {
+                    JsonObject validResponse = jsonResponseEvent.getAsJsonObject();


I don't mean eventResponseMessage, I mean validResponse.

z-sztrom · 2024-08-01T12:36:39Z

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

        try {
            return getAsJsonObject(jsonElement).has(META) && getAsJsonObject(jsonElement).has(DATA) &&
-                    getAsJsonObject(jsonElement).has(LINKS) && !getAsJsonObject(jsonElement).has(STATUS_CODE);
+                    getAsJsonObject(jsonElement).has(LINKS) && !getAsJsonObject(jsonElement).has(JSON_STATUS_CODE);
        } catch (IllegalStateException e) {
            log.error("Error while validating event: ", e.getMessage());


Something like ...get(META).get(ID), but I haven't checked...

shudhansu-shekhar

Please check my comment and if need we can discuss

shudhansu-shekhar · 2024-08-01T13:33:42Z

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

+        Map<String, Object> eventResponse = null;
+
+        if (eventResponseMessage == null) {
+            eventResponse.put("Parameter 'eventResponseMessage' must not be null", HttpStatus.BAD_REQUEST);


shudhansu-shekhar · 2024-08-01T14:48:03Z

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

+            JsonElement jsonResponseEvent = element.getAsJsonArray().get(i);
+            if (isValid(jsonResponseEvent)) {
+                synchronized (this) {
+                    JsonObject validResponse = jsonResponseEvent.getAsJsonObject();


yeah but here 'validResponse' is extracting from 'eventResponseMessage' So if this 'eventResponseMessage' is not null then validResponse will also not null because there is not much changes between that

shudhansu-shekhar · 2024-08-02T04:33:45Z

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

        try {
            return getAsJsonObject(jsonElement).has(META) && getAsJsonObject(jsonElement).has(DATA) &&
-                    getAsJsonObject(jsonElement).has(LINKS) && !getAsJsonObject(jsonElement).has(STATUS_CODE);
+                    getAsJsonObject(jsonElement).has(LINKS) && !getAsJsonObject(jsonElement).has(JSON_STATUS_CODE);
        } catch (IllegalStateException e) {
            log.error("Error while validating event: ", e.getMessage());


Yeah it can do like this but not sure about the place you suggest to put not sure it will benefit anyway or not but yeah we can discuss once

z-sztrom · 2024-08-05T09:25:17Z

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

+                                             @ApiParam(value = "JSON message", required = true) @RequestBody final String body){
+
+        try {
+            JsonElement bodyJson = JsonParser.parseString(body);


OK, so we have the check in generator, right?

z-sztrom · 2024-08-05T09:35:20Z

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

        for (int i = 0; i < eventArray.size(); i++) {
            eventResponse = new HashMap<>();
            JsonElement jsonResponseEvent = eventArray.get(i);
            if (isValid(jsonResponseEvent)) {
                synchronized (this) {
                    JsonObject validResponse = jsonResponseEvent.getAsJsonObject();
+                    if (validResponse == null) {
+                        String errorMessage = "Invalid, response json event is not in the form of JSON object";


Misleading error message, use something like "Invalid, array item expected to be in the form of JSON object".

z-sztrom · 2024-08-05T09:36:39Z

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

        } catch (IllegalStateException e) {
-            log.error("Error while validating event: ", e.getMessage());
+            log.error("Error while validating json event: ", e.getMessage());


json -> JSON

z-sztrom · 2024-08-05T09:59:45Z

When submitting a bad request, something like that is reported

{
"status message": {
"status code": 400,
"result": "{"status_code": 400, "result": "FAIL", "message":"Template is not correct format or something is missing in the template, please check"}",
"message": "FAIL"
}
}

while result of parsing (viewing in debugger) is

{"message":"Cannot validate given JSON string","cause":"com.ericsson.eiffel.remrem.semantics.validator.EiffelValidationException: Multiple trace links are not allowed for link type COMPOSITION"}

Where this error message disappears? Why is it replaced by a "Template is not correct format or something is missing in the template, please check"?

shudhansu-shekhar · 2024-08-05T11:53:15Z

When submitting a bad request, something like that is reported

{ "status message": { "status code": 400, "result": "{"status_code": 400, "result": "FAIL", "message":"Template is not correct format or something is missing in the template, please check"}", "message": "FAIL" } }

while result of parsing (viewing in debugger) is

{"message":"Cannot validate given JSON string","cause":"com.ericsson.eiffel.remrem.semantics.validator.EiffelValidationException: Multiple trace links are not allowed for link type COMPOSITION"}

Where this error message disappears? Why is it replaced by a "Template is not correct format or something is missing in the template, please check"?

Done, now the responses look as same in debugger

shudhansu-shekhar

please check my comment

shudhansu-shekhar · 2024-08-05T11:54:53Z

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

+                                             @ApiParam(value = "JSON message", required = true) @RequestBody final String body){
+
+        try {
+            JsonElement bodyJson = JsonParser.parseString(body);


Yes that duplicate key check we implement basically in generate

shudhansu-shekhar · 2024-08-05T11:57:03Z

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

        for (int i = 0; i < eventArray.size(); i++) {
            eventResponse = new HashMap<>();
            JsonElement jsonResponseEvent = eventArray.get(i);
            if (isValid(jsonResponseEvent)) {
                synchronized (this) {
                    JsonObject validResponse = jsonResponseEvent.getAsJsonObject();
+                    if (validResponse == null) {
+                        String errorMessage = "Invalid, response json event is not in the form of JSON object";


shudhansu-shekhar · 2024-08-05T11:57:41Z

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

        } catch (IllegalStateException e) {
-            log.error("Error while validating event: ", e.getMessage());
+            log.error("Error while validating json event: ", e.getMessage());


z-sztrom

Please, add documentation, as in case of generate service.

shudhansu-shekhar · 2024-08-12T13:44:21Z

Please, add documentation, as in case of generate service.

No need there is already in publish

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

z-sztrom

Works, fine. Just change the error messages.

z-sztrom · 2024-08-15T08:42:39Z

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

+            return send(msgProtocol, userDomain, tag, routingKey, inputBody);
+        } catch (JsonSyntaxException e) {
+            String exceptionMessage = e.getMessage();
+            log.error("Unexpected exception caught due to parsed json data", exceptionMessage);


Use log.error("Cannot parse the following JSON data:\n" + inputBody, exceptionMessage);

If we use inputBodyhere in the catch section we need initiailize this and it seems null in the catch, so it is not better to use body instead of inputBody?

z-sztrom · 2024-08-15T08:43:10Z

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

+            String exceptionMessage = e.getMessage();
+            log.error("Unexpected exception caught due to parsed json data", exceptionMessage);
+            return createResponseEntity(HttpStatus.BAD_REQUEST, JSON_FATAL_STATUS,
+                    "Invalid JSON parse data format due to: " + exceptionMessage);


"Invalid JSON data: " + exceptionMessage

shudhansu-shekhar

Fix the error messages, please check the comment

shudhansu-shekhar

please check my comment, just need to check one thing

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

+            @ApiParam(value = "eiffel event", required = true) @RequestBody final String body) {
+        try {
+            JsonElement inputBody = JsonParser.parseString(body);
+            return send(msgProtocol, userDomain, tag, routingKey, inputBody);


z-sztrom · 2024-08-16T08:42:22Z

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

            return createResponseEntity(HttpStatus.BAD_REQUEST, JSON_FATAL_STATUS,
-                    "Invalid JSON parse data format due to: " + exceptionMessage);
+                    "Invalid JSON data:" + exceptionMessage);


A space missing after ':'.

z-sztrom · 2024-08-16T08:43:57Z

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

@@ -195,9 +195,9 @@ public ResponseEntity send(
            return send(msgProtocol, userDomain, tag, routingKey, inputBody);
        } catch (JsonSyntaxException e) {
            String exceptionMessage = e.getMessage();
-            log.error("Unexpected exception caught due to parsed json data", exceptionMessage);
+            log.error("Cannot parse the following JSON data: " + body, exceptionMessage);


Separate the JSON data from the rest of logs:

... data:\n" + body + '\n'

shudhansu-shekhar

Fixed now, please check

z-sztrom · 2024-08-16T11:25:48Z

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

@@ -195,9 +195,9 @@ public ResponseEntity send(
            return send(msgProtocol, userDomain, tag, routingKey, inputBody);
        } catch (JsonSyntaxException e) {
            String exceptionMessage = e.getMessage();
-            log.error("Cannot parse the following JSON data: " + body, exceptionMessage);
+            log.error("Cannot parse the following JSON data:\n" + body + '\n', exceptionMessage);


I can't see the error message in log file.

Modified endpoint generateAndPublish to accept array of events

65fc749

github-advanced-security bot found potential problems Jun 10, 2024

View reviewed changes

...-service/src/main/java/com/ericsson/eiffel/remrem/publish/controller/ProducerController.java Fixed Show fixed Hide fixed

z-sztrom requested changes Jun 10, 2024

View reviewed changes

Modified endpoint generate to accept array of events

0136d8b

z-sztrom requested changes Jun 19, 2024

View reviewed changes

Modified endpoint generateAndPublish to accept array of events

14a5f7f

github-advanced-security bot found potential problems Jul 11, 2024

View reviewed changes

z-sztrom requested changes Jul 15, 2024

View reviewed changes

Modified endpoint generateAndPublish to accept array of events

2d55610

z-sztrom requested changes Jul 17, 2024

View reviewed changes

z-sztrom requested changes Jul 29, 2024

View reviewed changes

z-sztrom requested changes Jul 31, 2024

View reviewed changes

Modified endpoint generateAndPublish to accept array of events

04c79e6

z-sztrom requested changes Jul 31, 2024

View reviewed changes

Modified endpoint generateAndPublish to accept array of events

5566a4d

z-sztrom requested changes Aug 1, 2024

View reviewed changes

shudhansu-shekhar commented Aug 1, 2024

View reviewed changes

z-sztrom requested changes Aug 1, 2024

View reviewed changes

shudhansu-shekhar commented Aug 2, 2024

View reviewed changes

Modified endpoint generateAndPublish to accept array of events

7870507

z-sztrom requested changes Aug 5, 2024

View reviewed changes

shudhansu-shekhar commented Aug 5, 2024

View reviewed changes

shudhansu-shekhar added 2 commits August 5, 2024 17:48

Modified endpoint generateAndPublish to accept array of events

be8ff35

Modified endpoint generateAndPublish to accept array of events

6d9aca4

z-sztrom requested changes Aug 8, 2024

View reviewed changes

z-sztrom approved these changes Aug 14, 2024

View reviewed changes

Modified endpoint generateAndPublish to accept array of events

1fdf99d

github-advanced-security bot found potential problems Aug 14, 2024

View reviewed changes

...-service/src/main/java/com/ericsson/eiffel/remrem/publish/controller/ProducerController.java Fixed Show fixed Hide fixed

z-sztrom requested changes Aug 15, 2024

View reviewed changes

shudhansu-shekhar commented Aug 16, 2024

View reviewed changes

Modified endpoint generateAndPublish to accept array of events

4a596e3

github-advanced-security bot found potential problems Aug 16, 2024

View reviewed changes

z-sztrom requested changes Aug 16, 2024

View reviewed changes

shudhansu-shekhar commented Aug 16, 2024

View reviewed changes

Modified endpoint generateAndPublish to accept array of events

4c624e5

z-sztrom requested changes Aug 16, 2024

View reviewed changes

Modified endpoint generateAndPublish to accept array of events

acb0c70

z-sztrom approved these changes Aug 16, 2024

View reviewed changes

shudhansu-shekhar merged commit 4aa72b9 into eiffel-community:master Aug 16, 2024
3 checks passed

		return generateAndPublish(msgProtocol, msgType, userDomain, tag, routingKey, parseData, failIfMultipleFound,
		failIfNoneFound, lookupInExternalERs, lookupLimit, okToLeaveOutInvalidOptionalFields, bodyJson);

		@@ -223,11 +225,11 @@ public ResponseEntity generateAndPublish(final String msgProtocol, final String
		MsgService msgService = PublishUtils.getMessageService(msgProtocol, msgServices);

		@@ -284,25 +286,8 @@ public ResponseEntity generateAndPublish(final String msgProtocol, final String
		if (msgService != null && msgProtocol != null) {

Modified endpoint generateAndPublish to accept array of events #294

Modified endpoint generateAndPublish to accept array of events #294

Uh oh!

Conversation

shudhansu-shekhar commented Jun 10, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Applicable Issues

Description of the Change

Alternate Designs

Possible Drawbacks

Sign-off

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Check warning

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

shudhansu-shekhar commented Jun 10, 2024 •

edited

Loading