-
Notifications
You must be signed in to change notification settings - Fork 802
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
[SCB-406] Chassis must support standard parameter validation handler #607
Conversation
public void handle(Invocation invocation, AsyncResponse asyncResp) throws Exception { | ||
Object[] args = invocation.getArgs(); | ||
boolean invalid = false; | ||
List<Object> errList = new ArrayList<>(); |
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.
both solution and code is too complex
i remember it's very simple to IMPL validator, i'll give a example later
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.
have you showed the example to sukesh? @wujimin
List<Object> errList = new ArrayList<>(); | ||
if (null != args) { | ||
for (Object arg : args) { | ||
Set<ConstraintViolation<Object>> violations = validator.validate(arg); |
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.
validation annotation addressed not only model field, but also method parameter
so just validate arg is useless.
validate method invoke should use this way:
javax.validation.Validator.forExecutables()
javax.validation.executable.ExecutableValidator.validateParameters(T, Method, Object[], Class<?>...)
and i'm not sure if we need to validate return value:
javax.validation.executable.ExecutableValidator.validateReturnValue(T, Method, Object, Class<?>...)
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.
invocation arguments in handler, is about contract arguments, not always equals producer arguments.
so validation feature should be inject to SwaggerProducerOperation.
maybe need to create a extension point before producerMethod.invoke, do not named it to be validateXXX, this mechanism just like AOP.
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.
do not hard code response for validate failed, just throw some validation exception(maybe you can reference spring mechanism)
by org.apache.servicecomb.swagger.invocation.exception.ExceptionToResponseConverters,we provide a default validation exception converter
add a method: int getOrder() in ExceptionToResponseConverter, so that customers can override our default converter.
We can refer to spring validation here, https://docs.spring.io/spring/docs/4.1.x/spring-framework-reference/html/validation.html, Spring MVC 3 Validation |
validation will be added in already existing producer invoke handler |
Follow this checklist to help us incorporate your contribution quickly and easily:
[SCB-XXX] Fixes bug in ApproximateQuantiles
, where you replaceSCB-XXX
with the appropriate JIRA issue.mvn clean install
to make sure basic checks pass. A more thorough check will be performed on your pull request automatically.