It seems sensible that the HandlerMethodValidator should directly handle Cross-Parameter constraints or at a minimum not throw an IllegalArgumentException.
Similarly to @Valid annotations on @RequestBody arguments, Cross-Parameter constraints are applied only if another argument of the method has an @Constriant declared.
Unfortunately, unless the ConstraintValidatorContext of the ConstraintValidator.isValid method is manipulated with a new ConstraintViolation that has a node kind in its path that MethodValidationAdapter.adaptViolations handles (PROPERTY, RETURN_VALUE, PARAMETER) an exception is thrown:
```Caused by: java.lang.IllegalArgumentException: 'results' is required and must not be empty
at org.springframework.util.Assert.notEmpty(Assert.java:381)
at org.springframework.validation.method.DefaultMethodValidationResult.
Here is a spring-boot-based example (apologies for not having time to do a 'pure' spring-framework one without security)
```java
package org.springframework.gh33271;
import static java.lang.annotation.ElementType.*;
import static java.lang.annotation.RetentionPolicy.*;
import static org.springframework.http.MediaType.*;
import static org.springframework.security.test.web.servlet.request.SecurityMockMvcRequestPostProcessors.*;
import static org.springframework.test.context.TestConstructor.AutowireMode.ALL;
import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.*;
import static org.springframework.test.web.servlet.result.MockMvcResultHandlers.*;
import java.lang.annotation.Documented;
import java.lang.annotation.Retention;
import java.lang.annotation.Target;
import jakarta.validation.Constraint;
import jakarta.validation.ConstraintValidator;
import jakarta.validation.ConstraintValidatorContext;
import jakarta.validation.Payload;
import jakarta.validation.constraints.NotBlank;
import jakarta.validation.constraintvalidation.SupportedValidationTarget;
import jakarta.validation.constraintvalidation.ValidationTarget;
import lombok.AllArgsConstructor;
import lombok.Value;
import lombok.experimental.Accessors;
import lombok.experimental.NonFinal;
import org.junit.jupiter.api.Test;
import org.springframework.boot.autoconfigure.EnableAutoConfiguration;
import org.springframework.boot.autoconfigure.ImportAutoConfiguration;
import org.springframework.boot.test.autoconfigure.web.servlet.MockMvcAutoConfiguration;
import org.springframework.boot.test.autoconfigure.web.servlet.WebMvcTest;
import org.springframework.context.annotation.Configuration;
import org.springframework.gh33271.Gh33271Test.CrossParameterSimpleExample.CrossParameterSimpleExampleValidator;
import org.springframework.security.test.context.support.WithMockUser;
import org.springframework.test.context.TestConstructor;
import org.springframework.test.web.servlet.MockMvc;
import org.springframework.web.bind.annotation.GetMapping;
import org.springframework.web.bind.annotation.PathVariable;
import org.springframework.web.bind.annotation.RequestParam;
import org.springframework.web.bind.annotation.RestController;
@WebMvcTest(controllers = { org.springframework.gh33271.Gh33271Test.Config.SimpleController.class })
@TestConstructor(autowireMode = ALL)
@AllArgsConstructor
class Gh33271Test {
MockMvc mockMvc;
@WithMockUser(value = "spring")
@Test
void testCrossParam() throws Exception {
this.mockMvc
.perform(get("/has/cross/param/simple/{pathTestString}", "path-test-string")
.queryParam("queryParmTestString", "query-param-test-string")
.with(jwt()))
.andDo(print())
.andReturn();
}
@Configuration
@EnableAutoConfiguration
@Value
@NonFinal
@Accessors(fluent = true)
@ImportAutoConfiguration({ MockMvcAutoConfiguration.class })
static class Config {
@RestController
@Value
@NonFinal
public static class SimpleController {
@SuppressWarnings("static-method")
@GetMapping(path = { "/has/cross/param/simple/{pathTestString}" }, produces = TEXT_PLAIN_VALUE)
@CrossParameterSimpleExample
String hasCrossParamSimple(@SuppressWarnings("unused") @PathVariable @NotBlank final String pathTestString,
@SuppressWarnings("unused") @RequestParam @NotBlank final String queryParmTestString) {
return "can't get here";
}
}
}
@Documented
@Constraint(validatedBy = CrossParameterSimpleExampleValidator.class)
@Target({ CONSTRUCTOR, METHOD })
@Retention(RUNTIME)
public @interface CrossParameterSimpleExample {
String message()
default "Cross ${validatedValue}";
Class<?>[] groups() default {};
Class<? extends Payload>[] payload() default {};
@SupportedValidationTarget(ValidationTarget.PARAMETERS)
@Value
@Accessors(fluent = true)
class CrossParameterSimpleExampleValidator
implements ConstraintValidator<CrossParameterSimpleExample, Object[]> {
@Override
public boolean isValid(final Object[] parameters, final ConstraintValidatorContext context) {
return false;
}
}
}
}
(Hacky) Workaround
This workaround will create two violations, one for each parameter with the same message.
```java @Override public boolean isValid(final Object[] parameters, final ConstraintValidatorContext context) { //validation of the parameters array omitted var hibernateCrossParameterConstraintValidatorContext = context.unwrap(HibernateCrossParameterConstraintValidatorContext.class);
hibernateCrossParameterConstraintValidatorContext.disableDefaultConstraintViolation();
var methodParameterNames = hibernateCrossParameterConstraintValidatorContext.getMethodParameterNames();
var hibernateConstraintViolationBuilder = hibernateCrossParameterConstraintValidatorContext
.addMessageParameter("param0Name", methodParameterNames
.get(0))
.addMessageParameter("param1Name", methodParameterNames
.get(1))
.addMessageParameter("param0Value", parameters[0])
.addMessageParameter("param1Value", parameters[1])
.buildConstraintViolationWithTemplate(
"A {param0Name} with a value of {param0Value} and a {param1Name} with a value of {param1Value} are not compatible.");
hibernateConstraintViolationBuilder.addParameterNode(0);
hibernateConstraintViolationBuilder.addConstraintViolation();
hibernateConstraintViolationBuilder.addParameterNode(1);
hibernateConstraintViolationBuilder.addConstraintViolation();
return false;
}
Comment From: rstoyanchev
Thanks for the report. It would help a lot if you could provide snippets, or a small sample to experiment with.
Comment From: jmax01
Thanks for the report. It would help a lot if you could provide snippets, or a small sample to experiment with.
I will post one later today.
Note that it will simply be a GetMapping method with 2 arguments that have constraints and a method level cross-parameter constraint whose isValid method always returns false.
Comment From: jmax01
Example added.
Comment From: jmax01
Workaround added.
Comment From: rstoyanchev
Thanks for the sample.
For cross-parameter validation, the property path has the method node and then a cross-parameter node, but no other useful info on which parameters were validated or what the argument values were. Is that because the ConstraintValidator implementation is empty and just returns false? In other words is the validator able to and expected to update the context in a way that would expose more information in the resulting ConstraintViolation?
At any rate, a cross-parameter violation does not fit into the current model where a MethodValidationResult holds value results (violations of parameter constraints) or bean results (violations of nested object field constraints) both of which aggregate violations for a specific method parameter.
We could create a CrossParameterValidationResult that holds basic MessageSourceResolvable's prepared from the annotation name and the target class/method name, but I'm not sure if we can get anything more useful than that.
Comment From: jmax01
I 100% agree that the current model doesn't fit.
The challenge in my mind is, at the end of the day, we need a single exception type that holds all method argument ConstraintViolations.
This exception type should contain violations of cross-parameter constraints and violations of individual arguments annotated with @Constraints or annotated with @Valid.
This would provide a complete solution for handling pre-method execution ConstraintViolations.
The MethodValidationResult and MethodValidationException names imply this functionality but currently require a lot of finagling to achieve it (ConstraintViolation manipulation in the case of cross-parameter constraints and the requirement to have additional @Constraints on the method for '@Valid` to be evaluated ).
Thanks for the sample.
For cross-parameter validation, the property path has the method node and then a cross-parameter node, but no other useful info on which parameters were validated or what the argument values were. Is that because the
ConstraintValidatorimplementation is empty and just returns false? In other words, is the validator able to and expected to update the context in a way that would expose more information in the resultingConstraintViolation?At any rate, a cross-parameter violation does not fit into the current model where a
MethodValidationResultholds value results (violations of parameter constraints) or bean results (violations of nested object field constraints) both of which aggregate violations for a specific method parameter.We could create a
CrossParameterValidationResultthat holds basicMessageSourceResolvable's prepared from the annotation name and the target class/method name, but I'm not sure if we can get anything more useful than that.
Comment From: rstoyanchev
the requirement to have additional
@Constraintson the method for '@Valid` to be evaluated
I'm not sure what you mean. @Valid does not need anything else to trigger validation. It is true that if there are no @Constraints at all on method parameter, then it is validated on its own, raising MethodArgumentNotValidException, which is designed to be very similar to MethodValidationException and one can be adapted to the other. This is covered in the docs.
You do raise a point that with method level @Constraints, currently we assume they are for the return value, but it may be for cross-parameter validation. If this is what you are pointing out, then yes, we do need to update that.
ConstraintViolation manipulation in the case of cross-parameter constraints
For the rest, I proposed above a CrossParameterValidationResult held in MethodValidationResult in addition to ParameterValidationResults. However, I don't see a way to tell which specific parameters were involved, which means we can't aggregate cross-parameter violations in any way. So we might just expose a list of MessageSourceResolvables from MethodValidationResult that represent cross-param violations.
Comment From: breun
I see that getAllValidationResults() in MethodValidationResult has been deprecated and now delegates to getParameterValidationResults().
Cross-parameter validation results are not included in the result of getParameterValidationResults(), right?
I noticed that getAllErrors() and hasErrors() only use getParameterValidationResults(), so that would then also not include cross-parameter validation results? If so, is this intentional?
Comment From: breun
I've been trying to create a test that results in a HandlerMethodValidationException with cross-parameter validation results, but in order to get the cross-parameter validation executed I need to annotate the @RestController class with @Validated, and once I do that I no longer get a HandlerValidationException, but a ConstraintViolationException, so maybe a HandlerMethodValidationException with cross-parameter validation results is just not something that can ever really occur in the real world?