-
Notifications
You must be signed in to change notification settings - Fork 70
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
Modified generate's endpoint generate to accept array of events #226
Changes from 4 commits
6eb83b3
d33c54c
1476f2d
e58d75b
55d720d
037c273
89b52ef
145f2b8
dfa1799
7ea6195
3a3fd1c
962cabb
dbb2ed3
ddca4e6
b5ed6b4
cd2a736
6456f81
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 |
---|---|---|
|
@@ -18,12 +18,12 @@ | |
import com.ericsson.eiffel.remrem.generate.constants.RemremGenerateServiceConstants; | ||
import com.ericsson.eiffel.remrem.generate.exception.REMGenerateException; | ||
import com.ericsson.eiffel.remrem.protocol.MsgService; | ||
import com.google.gson.Gson; | ||
import com.google.gson.GsonBuilder; | ||
import com.google.gson.JsonArray; | ||
import com.google.gson.JsonElement; | ||
import com.google.gson.JsonObject; | ||
import com.google.gson.JsonParser; | ||
import com.fasterxml.jackson.core.JsonFactory; | ||
import com.fasterxml.jackson.core.JsonProcessingException; | ||
import com.fasterxml.jackson.databind.JsonMappingException; | ||
import com.fasterxml.jackson.databind.JsonNode; | ||
import com.fasterxml.jackson.databind.ObjectMapper; | ||
import com.google.gson.*; | ||
|
||
import ch.qos.logback.classic.Logger; | ||
import io.swagger.annotations.*; | ||
|
@@ -46,10 +46,7 @@ | |
|
||
import springfox.documentation.annotations.ApiIgnore; | ||
|
||
import java.io.BufferedReader; | ||
import java.io.FileInputStream; | ||
import java.io.IOException; | ||
import java.io.InputStreamReader; | ||
import java.io.*; | ||
import java.util.List; | ||
import java.util.Map; | ||
import java.util.Map.Entry; | ||
|
@@ -99,64 +96,166 @@ public void setRestTemplate(RestTemplate restTemplate) { | |
* <p> | ||
* Returns: The event information as a json element | ||
*/ | ||
|
||
@ApiOperation(value = "To generate eiffel event based on the message protocol", response = String.class) | ||
@ApiResponses(value = { @ApiResponse(code = 200, message = "Event sent successfully"), | ||
@ApiResponse(code = 400, message = "Malformed JSON"), | ||
@ApiResponse(code = 500, message = "Internal server error"), | ||
@ApiResponse(code = 503, message = "Message protocol is invalid") }) | ||
@RequestMapping(value = "/{mp" + REGEX + "}", method = RequestMethod.POST) | ||
public ResponseEntity<?> generate( | ||
@ApiParam(value = "message protocol", required = true) @PathVariable("mp") final String msgProtocol, | ||
@ApiParam(value = "message type", required = true) @RequestParam("msgType") final String msgType, | ||
@ApiParam(value = "ER lookup result multiple found, Generate will fail") @RequestParam(value = "failIfMultipleFound", required = false, defaultValue = "false") final Boolean failIfMultipleFound, | ||
@ApiParam(value = "ER lookup result none found, Generate will fail") @RequestParam(value = "failIfNoneFound", required = false, defaultValue = "false") final Boolean failIfNoneFound, | ||
@ApiParam(value = RemremGenerateServiceConstants.LOOKUP_IN_EXTERNAL_ERS) @RequestParam(value = "lookupInExternalERs", required = false, defaultValue = "false") final Boolean lookupInExternalERs, | ||
@ApiParam(value = RemremGenerateServiceConstants.LOOKUP_LIMIT) @RequestParam(value = "lookupLimit", required = false, defaultValue = "1") final int lookupLimit, | ||
@ApiParam(value = RemremGenerateServiceConstants.LenientValidation) @RequestParam(value = "okToLeaveOutInvalidOptionalFields", required = false, defaultValue = "false") final Boolean okToLeaveOutInvalidOptionalFields, | ||
@ApiParam(value = "JSON message", required = true) @RequestBody JsonObject bodyJson) { | ||
public ResponseEntity<?> generate(@ApiParam(value = "message protocol", required = true) @PathVariable("mp") final String msgProtocol, | ||
@ApiParam(value = "message type", required = true) @RequestParam("msgType") final String msgType, | ||
@ApiParam(value = "ER lookup result multiple found, Generate will fail") @RequestParam(value = "failIfMultipleFound", required = false, defaultValue = "false") final Boolean failIfMultipleFound, | ||
@ApiParam(value = "ER lookup result none found, Generate will fail") @RequestParam(value = "failIfNoneFound", required = false, defaultValue = "false") final Boolean failIfNoneFound, | ||
@ApiParam(value = RemremGenerateServiceConstants.LOOKUP_IN_EXTERNAL_ERS) @RequestParam(value = "lookupInExternalERs", required = false, defaultValue = "false") final Boolean lookupInExternalERs, | ||
@ApiParam(value = RemremGenerateServiceConstants.LOOKUP_LIMIT) @RequestParam(value = "lookupLimit", required = false, defaultValue = "1") final int lookupLimit, | ||
@ApiParam(value = RemremGenerateServiceConstants.LenientValidation) @RequestParam(value = "okToLeaveOutInvalidOptionalFields", required = false, defaultValue = "false") final Boolean okToLeaveOutInvalidOptionalFields, | ||
@ApiParam(value = "JSON message", required = true) @RequestBody String body) { | ||
JsonObject errorResponse = null; | ||
try { | ||
errorResponse = new JsonObject(); | ||
JsonFactory jsonFactory = JsonFactory.builder().build().enable(com.fasterxml.jackson.core.JsonParser.Feature.STRICT_DUPLICATE_DETECTION); | ||
ObjectMapper mapper = new ObjectMapper(jsonFactory); | ||
JsonNode node = mapper.readTree(body); | ||
Gson gson = new Gson(); | ||
JsonElement userInputJson = gson.fromJson(node.toString(), JsonElement.class); | ||
return generate(msgProtocol, msgType, failIfMultipleFound, failIfNoneFound, lookupInExternalERs, | ||
lookupLimit, okToLeaveOutInvalidOptionalFields, userInputJson); | ||
} catch (JsonSyntaxException e) { | ||
String exceptionMessage = e.getMessage(); | ||
log.error("Invalid JSON parse data format due to:", 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. Remove ':'. |
||
errorResponse.addProperty(RemremGenerateServiceConstants.JSON_STATUS_CODE, HttpStatus.BAD_REQUEST.value()); | ||
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. createResponseEntity() should be used here. |
||
errorResponse.addProperty(RemremGenerateServiceConstants.JSON_STATUS_RESULT, "fatal"); | ||
errorResponse.addProperty(RemremGenerateServiceConstants.JSON_ERROR_MESSAGE_FIELD, "Invalid JSON parse data format due to: " + exceptionMessage); | ||
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 status code provided? |
||
return new ResponseEntity<>(errorResponse, HttpStatus.INTERNAL_SERVER_ERROR); | ||
} catch (JsonProcessingException e) { | ||
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 JsonSyntaxException and JsonProcessingException can be handled together. No need to distinguish between them. |
||
String exceptionMessage = e.getMessage(); | ||
log.info("duplicate key detected", exceptionMessage); | ||
errorResponse.addProperty(RemremGenerateServiceConstants.JSON_STATUS_RESULT, "fatal"); | ||
errorResponse.addProperty(RemremGenerateServiceConstants.JSON_ERROR_MESSAGE_FIELD, "duplicate key detected, please check " + exceptionMessage); | ||
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. How do you know that this is a duplicate key issue? |
||
return new ResponseEntity<>(errorResponse, HttpStatus.BAD_REQUEST); | ||
} | ||
} | ||
|
||
public ResponseEntity<?> generate(final String msgProtocol, final String msgType, final Boolean failIfMultipleFound, final Boolean failIfNoneFound, final Boolean lookupInExternalERs, final int lookupLimit, final Boolean okToLeaveOutInvalidOptionalFields, JsonElement userInputData) { | ||
|
||
JsonArray generatedEventResults = new JsonArray(); | ||
JsonObject errorResponse = new JsonObject(); | ||
try { | ||
bodyJson = erLookup(bodyJson, failIfMultipleFound, failIfNoneFound, lookupInExternalERs, lookupLimit); | ||
MsgService msgService = getMessageService(msgProtocol); | ||
String response; | ||
if (msgService != null) { | ||
response = msgService.generateMsg(msgType, bodyJson, isLenientEnabled(okToLeaveOutInvalidOptionalFields)); | ||
JsonElement parsedResponse = parser.parse(response); | ||
if(lookupLimit <= 0) { | ||
return new ResponseEntity<>("LookupLimit must be greater than or equals to 1", HttpStatus.BAD_REQUEST); | ||
if (lookupLimit <= 0) { | ||
return new ResponseEntity<>("LookupLimit must be greater than or equals to 1", 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. Should createResponseEntity be used? 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 think here we need, here we just need to return one response not as many like in other scenerio |
||
} | ||
if (userInputData == null) { | ||
log.error("Json event must not be null"); | ||
errorResponse.addProperty(RemremGenerateServiceConstants.JSON_ERROR_MESSAGE_FIELD, "userInputData must not be 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. User doesn't know what 'userInputData' is. Use 'Input data' or something like that instead. |
||
return new ResponseEntity<>(errorResponse, 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. Why to continue when inputData is 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. done |
||
if (userInputData.isJsonArray()) { | ||
JsonArray inputEventJsonArray = userInputData.getAsJsonArray(); | ||
for (JsonElement element : inputEventJsonArray) { | ||
JsonObject generatedEvent = (processEvent(msgProtocol, msgType, | ||
failIfMultipleFound, failIfNoneFound, lookupInExternalERs, lookupLimit, | ||
okToLeaveOutInvalidOptionalFields, element.getAsJsonObject())); | ||
generatedEventResults.add(generatedEvent); | ||
} | ||
boolean success = true; | ||
for (JsonElement result : generatedEventResults) { | ||
JsonObject jsonObject = result.getAsJsonObject(); | ||
success &= jsonObject.has(RemremGenerateServiceConstants.META); | ||
} | ||
if (!parsedResponse.getAsJsonObject().has(RemremGenerateServiceConstants.JSON_ERROR_MESSAGE_FIELD)) { | ||
return new ResponseEntity<>(parsedResponse, HttpStatus.OK); | ||
return new ResponseEntity<>(generatedEventResults, success ? HttpStatus.OK : HttpStatus.BAD_REQUEST); | ||
|
||
} else if (userInputData.isJsonObject()) { | ||
JsonObject inputJsonObject = userInputData.getAsJsonObject(); | ||
JsonObject processedJson = processEvent(msgProtocol, msgType, failIfMultipleFound, failIfNoneFound, | ||
lookupInExternalERs, lookupLimit, okToLeaveOutInvalidOptionalFields, inputJsonObject); | ||
|
||
if (processedJson.has(RemremGenerateServiceConstants.META)) { | ||
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. Simplify result handling as in line 164. 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 i checked the flow of code is like that we need the code as it is means need that service_unavailable part here that's why i not do any change regarding this in my latest push |
||
return new ResponseEntity<>(processedJson, HttpStatus.OK); | ||
} | ||
if (processedJson.has(RemremGenerateServiceConstants.JSON_STATUS_CODE) && "400".equals(processedJson.get(RemremGenerateServiceConstants.JSON_STATUS_CODE).toString())) { | ||
return new ResponseEntity<>(processedJson, HttpStatus.BAD_REQUEST); | ||
} else { | ||
return new ResponseEntity<>(parsedResponse, HttpStatus.BAD_REQUEST); | ||
return new ResponseEntity<>(processedJson, HttpStatus.SERVICE_UNAVAILABLE); | ||
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. This strange state should be logged. 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 logged like this? log.error("Unexpected error caught, due to service property is missing or wrong"); |
||
} | ||
} else { | ||
return new ResponseEntity<>(parser.parse(RemremGenerateServiceConstants.NO_SERVICE_ERROR), | ||
HttpStatus.SERVICE_UNAVAILABLE); | ||
} | ||
} catch (REMGenerateException e1) { | ||
if (e1.getMessage().contains(Integer.toString(HttpStatus.NOT_ACCEPTABLE.value()))) { | ||
return new ResponseEntity<>(parser.parse(e1.getMessage()), HttpStatus.NOT_ACCEPTABLE); | ||
} | ||
else if (e1.getMessage().contains(Integer.toString(HttpStatus.EXPECTATION_FAILED.value()))) { | ||
return new ResponseEntity<>(parser.parse(e1.getMessage()), HttpStatus.EXPECTATION_FAILED); | ||
return createResponseEntity(RemremGenerateServiceConstants.JSON_STATUS_CODE, RemremGenerateServiceConstants.JSON_STATUS_RESULT, RemremGenerateServiceConstants.JSON_ERROR_MESSAGE_FIELD, | ||
HttpStatus.BAD_REQUEST, "Invalid JSON format,expected either single template or array of templates", "fail", errorResponse, HttpStatus.BAD_REQUEST.value()); | ||
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. A space is missing after comma (,). |
||
} | ||
else if (e1.getMessage().contains(Integer.toString(HttpStatus.EXPECTATION_FAILED.value()))) { | ||
return new ResponseEntity<>(parser.parse(e1.getMessage()), HttpStatus.EXPECTATION_FAILED); | ||
} | ||
else if (e1.getMessage() | ||
.contains(Integer.toString(HttpStatus.SERVICE_UNAVAILABLE.value()))) { | ||
return new ResponseEntity<>(parser.parse(RemremGenerateServiceConstants.NO_ER), | ||
HttpStatus.SERVICE_UNAVAILABLE); | ||
} catch (REMGenerateException | JsonSyntaxException e) { | ||
return handleException(e); | ||
} | ||
} | ||
|
||
public ResponseEntity<JsonObject> createResponseEntity(String statusCode, String statusResult, String statusMessage, | ||
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 to pass the first three values. They're always the same. 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 |
||
HttpStatus status, String errorMessage, String resultMessage, JsonObject errorResponse, | ||
int statusCodeValue) { | ||
errorResponse.addProperty(statusCode, statusCodeValue); | ||
errorResponse.addProperty(statusResult, resultMessage); | ||
errorResponse.addProperty(statusMessage, errorMessage); | ||
return new ResponseEntity<>(errorResponse, status); | ||
|
||
} | ||
|
||
private ResponseEntity<JsonObject> handleException(Exception e) { | ||
JsonObject errorResponse = new JsonObject(); | ||
String exceptionMessage = e.getMessage(); | ||
if (e instanceof REMGenerateException) { | ||
List<HttpStatus> statuses = List.of( | ||
HttpStatus.NOT_ACCEPTABLE, HttpStatus.EXPECTATION_FAILED, HttpStatus.SERVICE_UNAVAILABLE, HttpStatus.UNPROCESSABLE_ENTITY | ||
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. Split the line. 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 |
||
); | ||
for (HttpStatus status : statuses) { | ||
if (exceptionMessage.contains(Integer.toString(status.value()))) { | ||
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 guess there should be more strict test, not just test for occurrence in message. The message is a JSON 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. Yeah but whatever the test is handled by these exception error, it is same like as earlier.. |
||
return createResponseEntity(RemremGenerateServiceConstants.JSON_STATUS_CODE, RemremGenerateServiceConstants.JSON_STATUS_RESULT, | ||
RemremGenerateServiceConstants.JSON_ERROR_MESSAGE_FIELD, | ||
status, e.getMessage(), "fail", errorResponse, status.value()); | ||
} | ||
} | ||
else { | ||
return new ResponseEntity<>(parser.parse(e1.getMessage()), HttpStatus.UNPROCESSABLE_ENTITY); | ||
return createResponseEntity(RemremGenerateServiceConstants.JSON_STATUS_CODE, RemremGenerateServiceConstants.JSON_STATUS_RESULT, | ||
RemremGenerateServiceConstants.JSON_ERROR_MESSAGE_FIELD, | ||
HttpStatus.BAD_REQUEST, e.getMessage(), "fail", errorResponse, HttpStatus.BAD_REQUEST.value()); | ||
} else if (e instanceof JsonSyntaxException) { | ||
log.error("Failed to parse JSON: ", exceptionMessage); | ||
return createResponseEntity(RemremGenerateServiceConstants.JSON_STATUS_CODE, RemremGenerateServiceConstants.JSON_STATUS_RESULT, RemremGenerateServiceConstants.JSON_ERROR_MESSAGE_FIELD, | ||
HttpStatus.BAD_REQUEST, e.getMessage(), "fail", errorResponse, HttpStatus.BAD_REQUEST.value()); | ||
} else { | ||
log.error("Unexpected exception caught", exceptionMessage); | ||
return createResponseEntity(RemremGenerateServiceConstants.JSON_STATUS_CODE, RemremGenerateServiceConstants.JSON_STATUS_RESULT, | ||
RemremGenerateServiceConstants.JSON_ERROR_MESSAGE_FIELD, | ||
HttpStatus.INTERNAL_SERVER_ERROR, exceptionMessage, "fail", errorResponse, HttpStatus.INTERNAL_SERVER_ERROR.value()); | ||
} | ||
} | ||
|
||
/** | ||
* This helper method basically generate or process one event | ||
* @return JsonObject generated event | ||
*/ | ||
|
||
public JsonObject processEvent(String msgProtocol, String msgType, Boolean failIfMultipleFound, | ||
Boolean failIfNoneFound, Boolean lookupInExternalERs, int lookupLimit, | ||
Boolean okToLeaveOutInvalidOptionalFields, JsonObject jsonObject) throws REMGenerateException, JsonSyntaxException { | ||
JsonObject eventResponse = new JsonObject(); | ||
JsonElement parsedResponse; | ||
|
||
JsonObject event = erLookup(jsonObject, failIfMultipleFound, failIfNoneFound, lookupInExternalERs, lookupLimit); | ||
MsgService msgService = getMessageService(msgProtocol); | ||
|
||
if (msgService != null) { | ||
String response = msgService.generateMsg(msgType, event, isLenientEnabled(okToLeaveOutInvalidOptionalFields)); | ||
parsedResponse = parser.parse(response); | ||
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. parse() is marked as deprecated. 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 use JsonParser() instead of this |
||
|
||
JsonObject parsedJson = parsedResponse.getAsJsonObject(); | ||
|
||
if (!parsedJson.has(RemremGenerateServiceConstants.JSON_ERROR_MESSAGE_FIELD)) { | ||
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. Reverse condition. 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 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. Import constants in order to skip class name when referencing a constant. |
||
return parsedJson; | ||
} else { | ||
eventResponse.addProperty(RemremGenerateServiceConstants.JSON_STATUS_CODE, HttpStatus.BAD_REQUEST.value()); | ||
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 createResponse() method. Please, check if the method can be used at other places. |
||
eventResponse.addProperty(RemremGenerateServiceConstants.JSON_STATUS_RESULT, "fail"); | ||
eventResponse.addProperty(RemremGenerateServiceConstants.JSON_ERROR_MESSAGE_FIELD, RemremGenerateServiceConstants.TEMPLATE_ERROR); | ||
return eventResponse; | ||
} | ||
} catch (Exception e) { | ||
log.error("Unexpected exception caught", e); | ||
return new ResponseEntity<>(parser.parse(RemremGenerateServiceConstants.INTERNAL_SERVER_ERROR), | ||
HttpStatus.INTERNAL_SERVER_ERROR); | ||
} | ||
return eventResponse; | ||
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. We cannot return an empty JSON object. Situation when msgService == null must be handled. 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 we can handle this in same as original one means the how it handle earlier right? else { |
||
} | ||
|
||
private JsonObject erLookup(final JsonObject bodyJson, Boolean failIfMultipleFound, Boolean failIfNoneFound, | ||
|
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.
Use exceptionMessage at line 122, too.
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.
done