Skip to content

Commit

Permalink
Issue 4306 - Bean validation should fail if @Valid field is not intro…
Browse files Browse the repository at this point in the history
…spected (#4411)

* Issue 4306 - Bean validation should fail if @Valid field is not introspected
- emit validation failure when @Valid alone is applied to a container of elements where the element type is not @introspected.
- this does not address cases with additional constraint(s), since that might still be legal if the element type is supported by those constraints directly (e.g. @Valid @SiZe(min=1, max=2) List<String> strs), even though something like @Valid @SiZe(min=1, max=2) List<NotIntrospected> oops might be bogus.

* Issue 4306 - Bean validation should fail if @Valid field is not introspected
- emit validation failure when @Valid alone is applied to a container of elements where the element type is not @introspected.
- this does not address cases with additional constraint(s), since that might still be legal if the element type is supported by those constraints directly (e.g. @Valid @SiZe(min=1, max=2) List<String> strs), even though something like @Valid @SiZe(min=1, max=2) List<NotIntrospected> oops might be bogus.

* Issue 4306 - Bean validation should fail if @Valid field is not introspected
- emit validation failure when @Valid alone is applied to a container of elements where the element type is not @introspected.
- this does not address cases with additional constraint(s), since that might still be legal if the element type is supported by those constraints directly (e.g. @Valid @SiZe(min=1, max=2) List<String> strs), even though something like @Valid @SiZe(min=1, max=2) List<NotIntrospected> oops might be bogus.
  • Loading branch information
wetted committed Oct 30, 2020
1 parent b4ba433 commit 212bd71
Show file tree
Hide file tree
Showing 2 changed files with 99 additions and 13 deletions.
Expand Up @@ -854,24 +854,13 @@ public void keyedValue(String nodeName, Object key, Object keyedValue) {
}
} else {
context.addParameterNode(argument.getName(), i);
String messageTemplate = "{" + Introspected.class.getName() + ".message}";
overallViolations.add(new DefaultConstraintViolation(
object,
rootClass,
object,
parameterValue,
messageSource.interpolate(messageTemplate, MessageSource.MessageContext.of(Collections.singletonMap("type", parameterType.getName()))),
messageTemplate,
new PathImpl(context.currentPath),
null,
parameters));
overallViolations.add(createIntrospectionConstraintViolation(rootClass, object, context,
parameterType, parameterValue, parameters));
context.removeLast();
}
}
}
}


}

}
Expand Down Expand Up @@ -1331,6 +1320,13 @@ private <T> void cascadeToOne(
@Nullable DefaultPropertyNode container) {

final BeanIntrospection<Object> beanIntrospection = getBeanIntrospection(propertyValue);
AnnotationMetadata annotationMetadata = cascadeProperty.getAnnotationMetadata();
if (beanIntrospection == null && !annotationMetadata.hasStereotype(Constraint.class)) {
// error: only has @Valid but the propertyValue class is not @Introspected
overallViolations.add(createIntrospectionConstraintViolation(
rootClass, rootBean, context, propertyValue.getClass(), propertyValue));
return;
}

if (beanIntrospection != null) {
if (container != null) {
Expand Down Expand Up @@ -1757,6 +1753,21 @@ private <T> void failOnError(@NonNull BeanResolutionContext resolutionContext, S
}
}

@NonNull
private <T> DefaultConstraintViolation<T> createIntrospectionConstraintViolation(
@NonNull Class<T> rootClass,
T object,
DefaultConstraintValidatorContext context,
Class<?> parameterType,
Object parameterValue,
Object... parameters
) {
String messageTemplate = "{" + Introspected.class.getName() + ".message}";
return new DefaultConstraintViolation<>(object, rootClass, object, parameterValue,
messageSource.interpolate(messageTemplate, MessageSource.MessageContext.of(Collections.singletonMap("type", parameterType.getName()))),
messageTemplate, new PathImpl(context.currentPath), null, parameters);
}

/**
* The context object.
*/
Expand Down
Expand Up @@ -6,6 +6,7 @@ import io.micronaut.context.annotation.Prototype
import io.micronaut.core.annotation.Introspected
import io.micronaut.validation.validator.resolver.CompositeTraversableResolver
import spock.lang.AutoCleanup
import spock.lang.Ignore
import spock.lang.Shared
import spock.lang.Specification

Expand Down Expand Up @@ -377,6 +378,80 @@ class ValidatorSpec extends Specification {
!beanDescriptor.isBeanConstrained()
beanDescriptor.getConstrainedProperties().size() == 0
}

void "test cascade to container of non-introspected class" () {
when:
Bee notIntrospected = new Bee(name: "")
HiveOfBeeList beeHive = new HiveOfBeeList(bees: [notIntrospected])
def violations = validator.validate(beeHive)

then:
violations.size() == 1
!violations[0].constraintDescriptor
violations[0].message == "Cannot validate io.micronaut.validation.validator.Bee. No bean introspection present. " +
"Please add @Introspected to the class and ensure Micronaut annotation processing is enabled"
}

void "test cascade to map of non-introspected value class" () {
when:
Bee notIntrospected = new Bee(name: "")
HiveOfBeeMap beeHive = new HiveOfBeeMap(bees: ["blank" : notIntrospected])
def violations = validator.validate(beeHive)

then:
violations.size() == 1
!violations[0].constraintDescriptor
violations[0].message == "Cannot validate io.micronaut.validation.validator.Bee. No bean introspection present. " +
"Please add @Introspected to the class and ensure Micronaut annotation processing is enabled"
}

@Ignore("FIXME: https://github.com/micronaut-projects/micronaut-core/issues/4410")
void "test element validation in String collection" () {
when:
ListOfStrings strings = new ListOfStrings(strings: ["", null, "a string that's too long"])
def violations = validator.validate(strings)

then:
// should be: two for violating element size, and one null string violation
violations.size() == 3
violations[0].constraintDescriptor
violations[0].constraintDescriptor.annotation instanceof Size
violations[1].constraintDescriptor
violations[1].constraintDescriptor.annotation instanceof NotNull
violations[2].constraintDescriptor
violations[2].constraintDescriptor.annotation instanceof Size
}
}

@Introspected
class HiveOfBeeMap {
@Valid
Map<String, Bee> bees
}

@Introspected
class HiveOfBeeList {
@Valid
List<Bee> bees
}

// not introspected, expect validation failure
class Bee {
@NotBlank
String name
}

// FIXME see https://github.com/micronaut-projects/micronaut-core/issues/4410
// demonstrated by "test fail elements in String list"
// List, Set and array all work same
// 1) With Valid and constraints, validation is broken because it also applies @Size constraint to the container itself
// 2) Without @Valid only the container itself is validated for given constraints (makes sense)
@Introspected
class ListOfStrings {
@Valid
@Size(min=1, max=2)
@NotNull
List<String> strings
}

@Introspected
Expand Down

0 comments on commit 212bd71

Please sign in to comment.