Auto validate jsonconverter objects #3890
Conversation
|
|
There was a problem hiding this comment.
Hi @KYash03,
Well done and thank you very much for your hard work on this pull request. ⭐
We truly appreciate the effort you've put into it! However, I found some smaller issues that need to be addressed before we can proceed.
First of all, we follow a specific branch naming pattern. Could you please rename your branch to:
feature-3885-auto-validate-jsonconverter-objects
For the other issues, please see my comments!
Once these issues are addressed, we can move forward with the merge. Please let me know if you have any questions or need further clarification.
(Small hint: to avoid a red pipeline because of formatting issues run ./gradlew spotlessApply before a commit.)
Thank you again for your valuable contributions!
Best regards,
Laura
| ((JSONValidatable) result).validate(); | ||
| } catch (JSONValidationException e) { | ||
| throw new JSONConverterException("Validation failed for " + clazz.getSimpleName(), e); | ||
| } |
There was a problem hiding this comment.
I would add a generic catch block to catch any unsuspected exceptions that might occur during the validation process (e.g. an unsuspected null pointer).
e.g. Something like:
| } | |
| } | |
| catch (Exception e){ | |
| throw new JSONConverterException("Could not validate result for " + clazz.getSimpleName(), e); | |
| } |
| /** | ||
| * Exception thrown when JSON validation fails | ||
| */ | ||
| public class JSONValidationException extends SecHubRuntimeException { |
There was a problem hiding this comment.
We don't need to extend the SecHubRuntimeException here, RuntimeException is fine
| public class JSONValidationException extends SecHubRuntimeException { | |
| public class JSONValidationException extends RuntimeException { |
|
New PR: #3938 |
Closes #3885