Skip to content
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

Report all violation for collection #12

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
package org.unbrokendome.jackson.beanvalidation;

import com.fasterxml.jackson.core.JsonStreamContext;

import javax.validation.ConstraintViolation;
import javax.validation.ConstraintViolationException;
import java.util.Collections;
import java.util.LinkedHashSet;
import java.util.Set;
Expand All @@ -12,25 +15,28 @@
*/
final class InvalidObject {

private final Class<?> type;
private Set<ConstraintViolation<?>> violations;
private int indexInArray;


InvalidObject(Class<?> type, Set<ConstraintViolation<?>> violations) {
this.type = type;
this.violations = new LinkedHashSet<>(violations);
InvalidObject(ConstraintViolationException ex, JsonStreamContext parsingContext) {
this.violations = new LinkedHashSet<>(ex.getConstraintViolations());
this.indexInArray = PropertyPathUtils.getIndexInArray(parsingContext);
}


Class<?> getType() {
return type;
return violations.iterator().next().getRootBeanClass();
}


Set<ConstraintViolation<?>> getViolations() {
return Collections.unmodifiableSet(violations);
}

int getIndexInArray() {
return indexInArray;
}

void addAdditionalViolations(Set<? extends ConstraintViolation<?>> violations) {
this.violations.addAll(violations);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,11 @@ static Path constructPropertyPath(SettableBeanProperty prop, BeanValidationFeatu
@Nonnull
static Path constructPropertyPath(SettableBeanProperty prop, BeanValidationFeatureSet features,
@Nullable JsonStreamContext parsingContext) {
return constructPropertyPath(prop, features, getIndexInArray(parsingContext));
}

@Nonnull
static Path constructPropertyPath(SettableBeanProperty prop, BeanValidationFeatureSet features, int indexInArray) {

PathBuilder propertyPathBuilder = PathBuilder.create()
.appendBeanNode();
Expand All @@ -39,13 +44,8 @@ static Path constructPropertyPath(SettableBeanProperty prop, BeanValidationFeatu
propertyName = prop.getName();
}

if (parsingContext != null && parsingContext.inArray()) {
if (parsingContext.hasCurrentIndex()) {
propertyName += "[" + parsingContext.getCurrentIndex() + "]";
} else if (parsingContext.getParent().getCurrentValue() instanceof Collection) {
Collection currentValue = (Collection) parsingContext.getParent().getCurrentValue();
propertyName += "[" + currentValue.size() + "]";
}
if (indexInArray >= 0) {
propertyName += "[" + indexInArray + "]";
}

if (prop instanceof CreatorProperty &&
Expand All @@ -62,4 +62,17 @@ static Path constructPropertyPath(SettableBeanProperty prop, BeanValidationFeatu

return propertyPathBuilder.build();
}

static int getIndexInArray(@Nullable JsonStreamContext parsingContext) {
int index = -1;
if (parsingContext != null && parsingContext.inArray()) {
if (parsingContext.hasCurrentIndex()) {
index = parsingContext.getCurrentIndex();
} else if (parsingContext.getParent().getCurrentValue() instanceof Collection) {
Collection currentValue = (Collection) parsingContext.getParent().getCurrentValue();
index = currentValue.size();
}
}
return index;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
import com.fasterxml.jackson.databind.deser.impl.NullsConstantProvider;
import com.fasterxml.jackson.databind.deser.impl.PropertyBasedCreator;
import com.fasterxml.jackson.databind.exc.MismatchedInputException;
import com.fasterxml.jackson.databind.exc.UnrecognizedPropertyException;
import com.fasterxml.jackson.databind.util.TokenBuffer;
import org.unbrokendome.jackson.beanvalidation.path.PathBuilder;
import org.unbrokendome.jackson.beanvalidation.violation.ConstraintViolationUtils;
Expand All @@ -21,6 +22,7 @@
import javax.validation.constraints.NotNull;
import javax.validation.metadata.ConstraintDescriptor;
import java.io.IOException;
import java.util.Collection;
import java.util.Collections;
import java.util.HashSet;
import java.util.LinkedHashSet;
Expand Down Expand Up @@ -483,8 +485,7 @@ protected Object _deserializeUsingPropertyBased(JsonParser p, DeserializationCon
} catch (ConstraintViolationException ex) {
// Retrieve any violation from the set, to find out which concrete type was
// being instantiated
ConstraintViolation<?> violation = ex.getConstraintViolations().iterator().next();
bean = new InvalidObject(violation.getRootBeanClass(), ex.getConstraintViolations());
bean = new InvalidObject(ex, p.getParsingContext());

} catch (Exception e) {
bean = wrapInstantiationProblem(e, ctxt);
Expand All @@ -506,6 +507,10 @@ protected Object _deserializeUsingPropertyBased(JsonParser p, DeserializationCon
}
// or just clean?
bean = deserialize(p, ctxt, bean);

if (p.getParsingContext().inArray()) {
return bean;
}
return unwrapInvalidObject(bean);
}
continue;
Expand Down Expand Up @@ -541,6 +546,12 @@ protected Object _deserializeUsingPropertyBased(JsonParser p, DeserializationCon
Object bean = null;
try {
bean = creator.build(ctxt, buffer);
} catch (ConstraintViolationException e) {
if (p.getParsingContext().inArray()) {
bean = new InvalidObject(e, p.getParsingContext());
} else {
wrapInstantiationProblem(e, ctxt);
}
} catch (Exception e) {
wrapInstantiationProblem(e, ctxt);
assert false; // never gets here
Expand Down Expand Up @@ -581,8 +592,23 @@ protected Object _deserializeProperty(
String beanPropertyName = PropertyUtils.getPropertyNameFromMember(prop.getMember());
propertyViolations =
(Set) validator.validateValue(handledType(), beanPropertyName, value);
} else if (value instanceof Collection) {
propertyViolations = (Set<ConstraintViolation<?>>) ((Collection) value).stream()
.filter(InvalidObject.class::isInstance)
.flatMap(item -> {
InvalidObject invalidObject = (InvalidObject) item;
Path basePath = PropertyPathUtils.constructPropertyPath(
prop, features, invalidObject.getIndexInArray());
return invalidObject.getViolations().stream()
.map(violation -> ConstraintViolationUtils.withBasePath(
violation, bean, (Class) handledType(), basePath));
})
.collect(Collectors.toSet());
}

} catch (UnrecognizedPropertyException ex) {
wrapAndThrow(ex, handledType(), prop.getName(), ctxt);

} catch (MismatchedInputException ex) {
propertyViolations = Collections.singleton(handleMismatchedInput(p, bean, prop));

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,12 @@ package org.unbrokendome.jackson.beanvalidation
import assertk.assertThat
import assertk.assertions.hasSize
import com.fasterxml.jackson.databind.DeserializationFeature
import com.fasterxml.jackson.databind.exc.UnrecognizedPropertyException
import com.fasterxml.jackson.module.kotlin.KotlinModule
import org.junit.jupiter.api.BeforeEach
import org.junit.jupiter.api.Nested
import org.junit.jupiter.api.Test
import org.junit.jupiter.api.assertThrows
import org.junit.jupiter.api.extension.ExtensionContext
import org.junit.jupiter.params.ParameterizedTest
import org.junit.jupiter.params.provider.Arguments
Expand Down Expand Up @@ -136,6 +138,35 @@ class KotlinValidationTest : AbstractValidationTest() {
assertNoViolationsOnDeserialization<BeanWithLateinitVarProp>(json)
}


@Test
fun `should throw UnrecognizedPropertyException for object`() {

val json = """{ "left": "abc", "right": 42, "other": "o" }"""

assertThrows<UnrecognizedPropertyException> {
objectMapper.readValue(json, BeanWithNotNullCreatorProps::class.java)
}
}


@JsonValidated
data class Container(val list: List<Item>)

@JsonValidated
data class Item(val value: String)

@Test
fun `should throw UnrecognizedPropertyException for nested list`() {

val json = """{ "list": [{"value": "test", "other": "o"}, {"value": "abc"}] }"""

assertThrows<UnrecognizedPropertyException> {
objectMapper.readValue(json, Container::class.java)
}
}


@JsonValidated
data class ValidatedDataWithValidNestedList(val nested: List<SimpleData>)

Expand Down Expand Up @@ -176,5 +207,17 @@ class KotlinValidationTest : AbstractValidationTest() {
assertThat(violations).hasSize(1)
assertThat(violations).hasViolation<JsonRequired>(violationPath)
}

@Test
fun `should report violation on all records in @Valid-annotated nested list`() {

val json = """{ "nested": [{}, {"value":"2"}, {"value":null}, {"value":"4"}] }"""

val violations = assertViolationsOnDeserialization<ValidatedDataWithValidNestedList>(json)

assertThat(violations).hasSize(2)
assertThat(violations).hasViolation<JsonRequired>("nested[0].value")
assertThat(violations).hasViolation<NotNull>("nested[2].value")
}
}
}