Add support for Bean Validation's constraint groups

[nosan]

gh-887

• Groups can be obtained from configuration (key: groups)
• As an alternative solution, ConstraintDescriptions can be extended with additional methods with groups as an argument.

P.S. docs also need to be updated.

[nosan]

@wilkinsona
Is this something you'd be interested in supporting? I just want to understand whether I should invest time in this or not.

[wilkinsona]

I have very little time for REST Docs at the moment but I shouldn't have let this sit for so long without at least acknowledging it. Sorry. I'll try to make some time to look at it properly in the next few days.

[nosan]

No pressure - take your time. I just wanted to check if it makes sense to proceed, since there's a merge conflict, and understand what to do with the pull request.

[wilkinsona]

I've finally found some time to review this. Thanks for the PR, @nosan, and for your patience.

I consider groups to be a concept that's specific to bean validation. As such, I'd prefer to minimise how many classes know about that concept. If I were to do that building on what REST Docs offers today, I'd use a custom sub-class of ValidatorConstraintResolver. Something like this:

public class ValidatorGroupConstraintResolver extends ValidatorConstraintResolver {

	private final Class<?> group;

	public ValidatorGroupConstraintResolver(Class<?> group) {
		this.group = group;
	}

	@Override
	public List<Constraint> resolveForProperty(String property, Class<?> clazz) {
		return super.resolveForProperty(property, clazz).stream()
			.filter((constraint) -> isMember(constraint, this.group))
			.toList();
	}

	private boolean isMember(Constraint constraint, Class<?> group) {
		for (Class<?> candidate : (Class<?>[]) constraint.getConfiguration().get("groups")) {
			if (group.isAssignableFrom(candidate)) {
				return true;
			}
		}
		return false;
	}

}

I think it might be worth REST Docs making this a bit easier as, for example, dealing with the inheritance of the groups is somewhat subtle (I think I've got that right by using isAssignableFrom above). I think adding this functionality directly to ValidatorConstraintResolver makes sense. It could have a Set<Class<?>> of groups that it looks to match. When that set's null or empty it matches all constraints (current behavior). When it's not empty it matches constraints that belong to one of the specified groups.

[nosan]

Thanks, @wilkinsona

I think the group logic is not as simple as we thought. Fortunately for us, Bean Validation provides an API to retrieve constraints for the given groups.

I'm not sure that passing groups via the constructor is a good option here. While it works, it means that I need to create a new ValidatorConstraintResolver every time I want to use different groups, which, to be honest, is quite inconvenient.

As an alternative to using the constructor with less pain, perhaps the following method could be introduced:

public ValidatorConstraintResolver withGroups(Class<?>... groups) {
    return new ValidatorConstraintResolver(this.validator, groups);
}

The new instance would then take the groups into account. This would allow people to reuse the same Validator instance while creating a ValidatorConstraintResolver configured with different groups.

The second observation is an inconsistency between how Bean Validation performs validation and the constraints that REST Docs exposes. When Validator.validate(...) is invoked, Bean Validation implicitly applies the Default group if no groups are specified. As a result, only constraints that belong to the Default group (i.e., those without an explicit groups attribute) are validated. Conversely, when specific groups are provided, Bean Validation ignores constraints that do not belong to those groups (also ignores constraints that don't have groups at all) .

REST Docs, however, currently returns all constraints. It's unclear whether this behavior is intentional.

Here is a small test case to reproduce the issue:

@Test
void constraintsWithDefaultGroup() {
    Validator validator = Validation.buildDefaultValidatorFactory().getValidator();
    assertThat(validator.validate(new ConstrainedFields())).isEmpty();
    List<Constraint> constraints = new ValidatorConstraintResolver(validator).resolveForProperty("doorCode", ConstrainedFields.class);
    // Expecting empty but was: ...
    assertThat(constraints).isEmpty();
}

private static final class ConstrainedFields {

    @NotNull(groups = Serializable.class)
    String doorCode;

}

---

As I understand it, the main entry point for the end user is ConstraintDescriptions. However, with the current changes, there won't be an easy way to use it with a groups attribute.

It would probably end up looking like this:

new ConstraintDescriptions(UserInput.class, new ValidatorConstraintResolver(MyGroup.class)) .descriptionsForProperty("foo");

If withGroups(...) is used instead of the constructor, it would look something like this:

ValidatorConstraintResolver validatorConstraintResolver = new ValidatorConstraintResolver();
new ConstraintDescriptions( UserInput.class, validatorConstraintResolver.withGroups(MyGroup.class))
    .descriptionsForProperty("foo");

This also doesn't look very clean, as I would need to create a new ConstraintDescriptions each time with a different resolver.

I think introducing a ValidatorGroupConstraintDescriptions, or simply a ValidatorConstraintDescriptions, could help address both issues:

• It could provide dedicated methods for specifying groups.
• It could automatically apply the Default group to align the behavior with Bean Validation.

What do you think?

---

I've updated the PR with the requested changes.

[wilkinsona]

Thanks, @nosan. Plenty of food for thought there.

REST Docs, however, currently returns all constraints. It's unclear whether this behavior is intentional.

It's too long ago need for me to remember the intent with any degree of certainty. Looking at the code and tests, I suspect that I just didn't consider groups at all and the current behaviour wasn't intended.

I'm not sure that passing groups via the constructor is a good option here. While it works, it means that I need to create a new ValidatorConstraintResolver every time I want to use different groups, which, to be honest, is quite inconvenient.

I wonder if we should be making some more assumptions about how validation is triggered, certainly in the golden-path case. If we assume the use of Framework's @Validated, we could look for that on the class that's passed to resolveForProperty to determine the groups. This could also work for method parameter constraints, support for which may be coming in the future.

[nosan]

I wonder if we should be making some more assumptions about how validation is triggered, certainly in the golden-path case. If we assume the use of Framework's @Validated, we could look for that on the class that's passed to resolveForProperty to determine the groups. This could also work for method parameter constraints, support for which #1026.

I think the @Validated annotation on the type itself is a very rare case. Usually, people just define classes with Bean Validation annotations and then use @Validated(...) on controller methods, for example:

@PatchMapping
Person update(@Validated(UpdateGroup.class) Person p) {}

@PostMapping
Person create(@Validated(CreateGroup.class) Person p) {}

In this case, the Person class itself does not have a @Validated annotation. So I think we cannot determine the groups for the class that is passed to resolveForProperty.

However, for #1026, it could be possible. It is interesting to understand how this works when I have @Validated(...) on the controller class and also @Validated on method arguments. Which one wins? Are they merged?

[wilkinsona]

I think the @Validated annotation on the type itself is a very rare case. Usually, people just define classes with Bean Validation annotations and then use @Validated(...) on controller methods

Wishful thinking on my part. Your point is particularly true with groups where different usages will require different validation and @Validated on the class removes that flexibility.

It is interesting to understand how this works when I have @Validated(...) on the controller class and also @Validated on method arguments. Which one wins? Are they merged?

The method-level annotation wins. From the @Validated javadoc:

Applying this annotation at the method level allows for overriding the validation groups for a specific method