-
Notifications
You must be signed in to change notification settings - Fork 77
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
Changes from 5 commits
65fc749
0136d8b
14a5f7f
2d55610
04c79e6
5566a4d
7870507
be8ff35
6d9aca4
1fdf99d
4a596e3
4c624e5
acb0c70
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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; | ||
|
@@ -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; | ||
|
@@ -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"; | ||
|
||
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; | ||
} | ||
|
@@ -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 | ||
|
@@ -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 | ||
|
@@ -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); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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.? There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. OK, so we have the check in generator, right? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 warningCode scanning / CodeQL Cross-site scripting Medium
Cross-site scripting vulnerability due to a
user-provided value Error loading related location Loading |
||
} catch (JsonSyntaxException e) { | ||
String exceptionMessage = e.getMessage(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Use exceptionMessage at line above. There was a problem hiding this comment. Choose a reason for hiding this commentThe 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); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can we continue if msgService == null? There was a problem hiding this comment. Choose a reason for hiding this commentThe 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) { There was a problem hiding this comment. Choose a reason for hiding this commentThe 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); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think there should be createResponesEntity method available. There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 ------------- | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is the comment still valid? There was a problem hiding this comment. Choose a reason for hiding this commentThe 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) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why is msgServcie set to rmqHelper repeatedly? There was a problem hiding this comment. Choose a reason for hiding this commentThe 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(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is it guaranteed that validResponse is non-null? There was a problem hiding this comment. Choose a reason for hiding this commentThe 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){ There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't mean eventResponseMessage, I mean validResponse. There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 There was a problem hiding this comment. Choose a reason for hiding this commentThe 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){ |
||
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) && | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe 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()); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe 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? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Something like ...get(META).get(ID), but I haven't checked... There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 There was a problem hiding this comment. Choose a reason for hiding this commentThe 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; | ||
} | ||
} | ||
|
||
|
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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