Skip to content

Modified endpoint generateAndPublish to accept array of events #294

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 13 commits into from
Aug 16, 2024
Merged
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,9 @@
*/
package com.ericsson.eiffel.remrem.publish.controller;

import java.util.EnumSet;
import java.util.Map;
import java.util.*;

import com.google.gson.*;
import org.slf4j.LoggerFactory;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.beans.factory.annotation.Qualifier;
Expand Down Expand Up @@ -48,9 +48,6 @@
import com.ericsson.eiffel.remrem.publish.service.MessageService;
import com.ericsson.eiffel.remrem.publish.service.SendResult;
import com.fasterxml.jackson.databind.JsonNode;
import com.google.gson.JsonElement;
import com.google.gson.JsonObject;
import com.google.gson.JsonParser;

import ch.qos.logback.classic.Logger;
import io.swagger.annotations.Api;
Expand Down Expand Up @@ -87,6 +84,20 @@

private Logger log = (Logger) LoggerFactory.getLogger(ProducerController.class);

public final String JSON_STATUS_RESULT = "result";

public final String JSON_EVENT_MESSAGE_FIELD = "status message";

public final String STATUS_CODE = "Status code";
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah we need this Uppercase 'S' here


public static final String META = "meta";

public static final String DATA = "data";

public static final String LINKS = "links";

public static final String JSON_FATAL_STATUS = "fatal";

public void setMsgServices(MsgService[] msgServices) {
this.msgServices = msgServices;
}
Expand Down Expand Up @@ -149,7 +160,6 @@
/**
* This controller provides single RemRem REST API End Point for both RemRem
* Generate and Publish.
*
* @param msgProtocol
* message protocol (required)
* @param msgType
Expand All @@ -162,6 +172,9 @@
* (not required)
* @param parseData
* (not required, default=false)
* @param body
* (Here json body of string type as input because just to parse
* the string in to JsonElement not using JsonElement directly here.)
* @return A response entity which contains http status and result
*
* @use A typical CURL command: curl -H "Content-Type: application/json" -X POST
Expand Down Expand Up @@ -193,64 +206,184 @@
@ApiParam(value = "okToLeaveOutInvalidOptionalFields true will remove the optional "
+ "event fields from the input event data that does not validate successfully, "
+ "and add those removed field information into customData/remremGenerateFailures") @RequestParam(value = "okToLeaveOutInvalidOptionalFields", required = false, defaultValue = "false") final Boolean okToLeaveOutInvalidOptionalFields,
@ApiParam(value = "JSON message", required = true) @RequestBody final JsonObject bodyJson) {
@ApiParam(value = "JSON message", required = true) @RequestBody final String body){

try {
JsonElement bodyJson = JsonParser.parseString(body);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes that duplicate key check we implement basically in generate

return generateAndPublish(msgProtocol, msgType, userDomain, tag, routingKey, parseData, failIfMultipleFound,
failIfNoneFound, lookupInExternalERs, lookupLimit, okToLeaveOutInvalidOptionalFields, bodyJson);
Comment on lines +257 to +258

Check warning

Code scanning / CodeQL

Cross-site scripting Medium

Cross-site scripting vulnerability due to a
user-provided value
.
} catch (JsonSyntaxException e) {
String exceptionMessage = e.getMessage();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use exceptionMessage at line above.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

JsonObject errorResponse = new JsonObject();
log.error("Unexpected exception caught due to parsed json data", exceptionMessage);
errorResponse.addProperty(STATUS_CODE, HttpStatus.BAD_REQUEST.value());
errorResponse.addProperty(JSON_STATUS_RESULT, JSON_FATAL_STATUS);
errorResponse.addProperty(JSON_EVENT_MESSAGE_FIELD, "Invalid JSON parse data format due to: " + exceptionMessage);
return new ResponseEntity<>(errorResponse, HttpStatus.INTERNAL_SERVER_ERROR);
}
}

/**
* This controller provides single RemRem REST API End Point for both RemRem
* Generate and Publish.
*
* @param msgProtocol
* message protocol (required)
* @param msgType
* message type (required)
* @param userDomain
* user domain (not required)
* @param tag
* (not required)
* @param routingKey
* (not required)
* @param parseData
* (not required, default=false)
* @return A response entity which contains http status and result
*
* @use A typical CURL command: curl -H "Content-Type: application/json" -X POST
* --data "@inputGenerate_activity_finished.txt"
* "http://localhost:8986/generateAndPublish/?mp=eiffelsemantics&msgType=EiffelActivityFinished"
*/

public ResponseEntity generateAndPublish(final String msgProtocol, final String msgType, final String userDomain, final String tag, final String routingKey,
final Boolean parseData, final Boolean failIfMultipleFound, final Boolean failIfNoneFound, final Boolean lookupInExternalERs,
final int lookupLimit, final Boolean okToLeaveOutInvalidOptionalFields, final JsonElement bodyJson) {
if (isAuthenticationEnabled) {
logUserName();
}

String bodyJsonOut = null;
if(parseData) {
// -- parse params in incoming request -> body -------------
EventTemplateHandler eventTemplateHandler = new EventTemplateHandler();
JsonNode parsedTemplate = eventTemplateHandler.eventTemplateParser(bodyJson.toString(), msgType);
bodyJsonOut = parsedTemplate.toString();
log.info("Parsed template: " + bodyJsonOut);
}else{
bodyJsonOut = bodyJson.toString();
MsgService msgService = PublishUtils.getMessageService(msgProtocol, msgServices);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we continue if msgService == null?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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();
}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, that's how it should work.

List<JsonObject> events = new ArrayList<>();
if (bodyJson.isJsonObject()) {
events.add(getAsJsonObject(bodyJson));
} else if (bodyJson.isJsonArray()) {
for (JsonElement element : bodyJson.getAsJsonArray()) {
if (element.isJsonObject()) {
events.add(getAsJsonObject(element));
} else {
return new ResponseEntity<>("Invalid, array events must be a JSON object", HttpStatus.BAD_REQUEST);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think there should be createResponesEntity method available.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

}
}
} else {
return new ResponseEntity<>("Invalid, event is neither in the form of JSON object nor in the JSON array", HttpStatus.BAD_REQUEST);
}

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);

List<Map<String, Object>> responseEvents;
EnumSet<HttpStatus> getStatus = EnumSet.of(HttpStatus.SERVICE_UNAVAILABLE, HttpStatus.UNAUTHORIZED,
HttpStatus.NOT_ACCEPTABLE, HttpStatus.EXPECTATION_FAILED, HttpStatus.INTERNAL_SERVER_ERROR, HttpStatus.UNPROCESSABLE_ENTITY);
Map<String, Object> eventResponse;
try {
String bodyJsonOut;
if (parseData) {
EventTemplateHandler eventTemplateHandler = new EventTemplateHandler();
StringBuffer parsedTemplates = new StringBuffer();
parsedTemplates.append("[");
for (JsonElement eventJson : events) {
// -- parse params in incoming request -> body -------------
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is the comment still valid?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes

JsonNode parsedTemplate = eventTemplateHandler.eventTemplateParser(eventJson.toString(), msgType);
if (parsedTemplates.length() > 1) {
parsedTemplates.append(",");
}
parsedTemplates.append(parsedTemplate.toString());
}
parsedTemplates.append("]");
bodyJsonOut = parsedTemplates.toString();
log.info("Parsed template: " + bodyJsonOut);
} else {
bodyJsonOut = bodyJson.toString();
}
HttpHeaders headers = new HttpHeaders();
headers.setContentType(MediaType.APPLICATION_JSON_UTF8);
HttpEntity<String> entity = new HttpEntity<>(bodyJsonOut, headers);
String generateUrl = generateURLTemplate.getUrl() + "&failIfMultipleFound=" + failIfMultipleFound
+ "&failIfNoneFound=" + failIfNoneFound + "&lookupInExternalERs=" + lookupInExternalERs
+ "&lookupLimit=" + lookupLimit + "&okToLeaveOutInvalidOptionalFields=" + okToLeaveOutInvalidOptionalFields;

ResponseEntity<String> response = restTemplate.postForEntity(generateUrl,
entity, String.class, generateURLTemplate.getMap(msgProtocol, msgType));

String responseBody = null;
if (bodyJson.isJsonObject()) {
responseBody = "[" + response.getBody() + "]";
} else if (bodyJson.isJsonArray()) {
responseBody = response.getBody();
}
if (response.getStatusCode() == HttpStatus.OK) {
log.info("The result from REMReM Generate is: " + response.getStatusCodeValue());

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

log.debug("mp: " + msgProtocol);
log.debug("body: " + responseBody);
log.debug("user domain suffix: " + userDomain + " tag: " + tag + " routing key: " + routingKey);

if (msgService != null && msgProtocol != null) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is msgServcie set to rmqHelper repeatedly?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

rmqHelper.rabbitMqPropertiesInit(msgProtocol);
}
synchronized(this) {
SendResult result = messageService.send(responseBody, msgService, userDomain, tag, routingKey);
return new ResponseEntity(result, messageService.getHttpStatus());
}
responseEvents = processingValidEvent(responseBody, msgProtocol, userDomain,
tag, routingKey);
} else {
return response;
}
} catch (RemRemPublishException e) {
eventResponse = new HashMap<>();
eventResponse.put(JSON_EVENT_MESSAGE_FIELD, e.getMessage());
return new ResponseEntity<>(eventResponse, HttpStatus.NOT_FOUND);
} catch (HttpStatusCodeException e) {
responseEvents = processingValidEvent(e.getResponseBodyAsString(), msgProtocol, userDomain,
tag, routingKey);
return new ResponseEntity<>(responseEvents, HttpStatus.BAD_REQUEST);
}
catch (RemRemPublishException e) {
return new ResponseEntity(e.getMessage(), HttpStatus.NOT_FOUND);
}
catch (HttpStatusCodeException e) {
HttpStatus result = HttpStatus.BAD_REQUEST;
if (getStatus.contains(e.getStatusCode())) {
result = e.getStatusCode();
return new ResponseEntity<>(responseEvents, HttpStatus.OK);
}

/**
* This one is helper method publish messages on mb
* @param eventMsg
* @param msgProtocol
* @param userDomain
* @param tag
* @param routingKey
* @return list of responses
*/
public List<Map<String, Object>> processingValidEvent(String eventMsg, final String msgProtocol, final String userDomain,
final String tag, final String routingKey) {
MsgService msgService = PublishUtils.getMessageService(msgProtocol, msgServices);
List<Map<String, Object>> responseEvent = new ArrayList<>();
Map<String, Object> eventResponse;
JsonElement eventElement = JsonParser.parseString(eventMsg);
for (int i = 0; i < eventElement.getAsJsonArray().size(); i++) {
eventResponse = new HashMap<>();
JsonElement jsonResponseEvent = eventElement.getAsJsonArray().get(i);
if (isValid(jsonResponseEvent)) {
synchronized (this) {
JsonObject validResponse = jsonResponseEvent.getAsJsonObject();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it guaranteed that validResponse is non-null?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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);
}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't mean eventResponseMessage, I mean validResponse.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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);
}

String validResponseBody = validResponse.toString();
SendResult result = messageService.send(validResponseBody, msgService, userDomain, tag, routingKey);
eventResponse.put(JSON_STATUS_RESULT, result);
}
} else {
eventResponse.put(JSON_EVENT_MESSAGE_FIELD, jsonResponseEvent);
}
return new ResponseEntity(parser.parse(e.getResponseBodyAsString()), result);
responseEvent.add(eventResponse);
}
return responseEvent;
}

public JsonObject getAsJsonObject(JsonElement jsonElement) {
JsonObject jsonObject = jsonElement.getAsJsonObject();
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) {
try {
return getAsJsonObject(jsonElement).has(META) && getAsJsonObject(jsonElement).has(DATA) &&
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

getAsJsonObject(jsonElement).has(LINKS) && !getAsJsonObject(jsonElement).has(STATUS_CODE);
} catch (IllegalStateException e) {
log.error("Error while validating event: ", e.getMessage());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor Author

@shudhansu-shekhar shudhansu-shekhar Aug 2, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

return false;
}
}

Expand Down