You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@unomi.apache.org by "jsinovassin (via GitHub)" <gi...@apache.org> on 2023/04/28 16:42:13 UTC
[GitHub] [unomi] jsinovassin opened a new pull request, #612: Unomi 775 add validation endpoint
jsinovassin opened a new pull request, #612:
URL: https://github.com/apache/unomi/pull/612
https://issues.apache.org/jira/browse/UNOMI-775
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@unomi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [unomi] jsinovassin merged pull request #612: Unomi 775 add validation endpoint
Posted by "jsinovassin (via GitHub)" <gi...@apache.org>.
jsinovassin merged PR #612:
URL: https://github.com/apache/unomi/pull/612
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@unomi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [unomi] jkevan commented on a diff in pull request #612: Unomi 775 add validation endpoint
Posted by "jkevan (via GitHub)" <gi...@apache.org>.
jkevan commented on code in PR #612:
URL: https://github.com/apache/unomi/pull/612#discussion_r1182249233
##########
extensions/json-schema/services/src/main/java/org/apache/unomi/schema/impl/SchemaServiceImpl.java:
##########
@@ -111,12 +112,52 @@ public boolean isEventValid(String event) {
@Override
public Set<ValidationError> validateEvent(String event) throws ValidationException {
- JsonNode jsonEvent = parseData(event);
- String eventType = extractEventType(jsonEvent);
+ return validateNodeEvent(parseData(event));
+ }
+
+ @Override
+ public Map<String, Set<ValidationError>> validateEvents(String events) throws ValidationException {
+ Map<String, Set<ValidationError>> errorsPerEventType = new HashMap<>();
+ JsonNode eventsNodes = parseData(events);
+ eventsNodes.forEach(event -> {
+ String eventType = event.get("eventType").asText();
+ try {
+ Set<ValidationError> errors = validateNodeEvent(event);
+ if (errorsPerEventType.containsKey(eventType)) {
+ errorsPerEventType.get(eventType).addAll(errors);
+ } else {
+ errorsPerEventType.put(eventType, errors);
+ }
+ } catch (ValidationException e) {
+ Set<ValidationError> errors = buildCustomErrorMessage();
+ if (errorsPerEventType.containsKey(eventType)) {
+ errorsPerEventType.get(eventType).addAll(errors);
+ } else {
+ errorsPerEventType.put(eventType, errors);
+ }
+ errorsPerEventType.put(eventType, errors);
+
+ logger.debug(e.getMessage());
Review Comment:
I think this two lines need to be removed ?
##########
extensions/json-schema/services/src/main/java/org/apache/unomi/schema/api/ValidationError.java:
##########
@@ -42,7 +42,8 @@ public String toString() {
}
public boolean equals(Object o) {
- return validationMessage.equals(o);
+ ValidationError other = (ValidationError) o;
+ return validationMessage.equals(other.validationMessage);
Review Comment:
Could be simpler to replace directly the inner ValidationMessage object by the String error message at this step.
I would also simplify the building of custom error messages.
##########
extensions/json-schema/services/src/main/java/org/apache/unomi/schema/impl/SchemaServiceImpl.java:
##########
@@ -111,12 +112,52 @@ public boolean isEventValid(String event) {
@Override
public Set<ValidationError> validateEvent(String event) throws ValidationException {
- JsonNode jsonEvent = parseData(event);
- String eventType = extractEventType(jsonEvent);
+ return validateNodeEvent(parseData(event));
+ }
+
+ @Override
+ public Map<String, Set<ValidationError>> validateEvents(String events) throws ValidationException {
+ Map<String, Set<ValidationError>> errorsPerEventType = new HashMap<>();
+ JsonNode eventsNodes = parseData(events);
+ eventsNodes.forEach(event -> {
+ String eventType = event.get("eventType").asText();
Review Comment:
Could be interesting to make the eventType part of the ValidationException to make it a bit more generic.
This would allow to avoid parsing the event type here, as it is supposed to be done in the **extractEventType** function.
##########
extensions/json-schema/services/src/main/java/org/apache/unomi/schema/impl/SchemaServiceImpl.java:
##########
@@ -111,12 +112,52 @@ public boolean isEventValid(String event) {
@Override
public Set<ValidationError> validateEvent(String event) throws ValidationException {
- JsonNode jsonEvent = parseData(event);
- String eventType = extractEventType(jsonEvent);
+ return validateNodeEvent(parseData(event));
+ }
+
+ @Override
+ public Map<String, Set<ValidationError>> validateEvents(String events) throws ValidationException {
+ Map<String, Set<ValidationError>> errorsPerEventType = new HashMap<>();
+ JsonNode eventsNodes = parseData(events);
+ eventsNodes.forEach(event -> {
+ String eventType = event.get("eventType").asText();
+ try {
+ Set<ValidationError> errors = validateNodeEvent(event);
+ if (errorsPerEventType.containsKey(eventType)) {
+ errorsPerEventType.get(eventType).addAll(errors);
+ } else {
+ errorsPerEventType.put(eventType, errors);
+ }
+ } catch (ValidationException e) {
+ Set<ValidationError> errors = buildCustomErrorMessage();
Review Comment:
Not against building custom error messages, but we should reuse the error message from the ValidationException then.
##########
extensions/json-schema/services/src/main/java/org/apache/unomi/schema/impl/SchemaServiceImpl.java:
##########
@@ -111,12 +112,52 @@ public boolean isEventValid(String event) {
@Override
public Set<ValidationError> validateEvent(String event) throws ValidationException {
- JsonNode jsonEvent = parseData(event);
- String eventType = extractEventType(jsonEvent);
+ return validateNodeEvent(parseData(event));
+ }
+
+ @Override
+ public Map<String, Set<ValidationError>> validateEvents(String events) throws ValidationException {
+ Map<String, Set<ValidationError>> errorsPerEventType = new HashMap<>();
+ JsonNode eventsNodes = parseData(events);
+ eventsNodes.forEach(event -> {
+ String eventType = event.get("eventType").asText();
+ try {
+ Set<ValidationError> errors = validateNodeEvent(event);
+ if (errorsPerEventType.containsKey(eventType)) {
+ errorsPerEventType.get(eventType).addAll(errors);
+ } else {
+ errorsPerEventType.put(eventType, errors);
+ }
+ } catch (ValidationException e) {
+ Set<ValidationError> errors = buildCustomErrorMessage();
+ if (errorsPerEventType.containsKey(eventType)) {
+ errorsPerEventType.get(eventType).addAll(errors);
+ } else {
+ errorsPerEventType.put(eventType, errors);
+ }
+ errorsPerEventType.put(eventType, errors);
+
+ logger.debug(e.getMessage());
+ }
+ });
+ return errorsPerEventType;
+ }
+
+ private Set<ValidationError> buildCustomErrorMessage() {
+ ValidationMessage.Builder builder = new ValidationMessage.Builder();
+ builder.customMessage("No Schema found for this event type").format(new MessageFormat("Not used pattern. Message format is required"));
+ ValidationError error = new ValidationError(builder.build());
+ Set<ValidationError> errors = new HashSet<>();
+ errors.add(error);
+ return errors;
Review Comment:
This part would be simpler if we directly return the String validation error in the response instead of a thirdparty object from the external lib.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@unomi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org