diff --git a/src/main/java/org/zalando/problem/spring/web/advice/validation/BaseBindingResultAdviceTrait.java b/src/main/java/org/zalando/problem/spring/web/advice/validation/BaseBindingResultAdviceTrait.java index 54e46412..288c9a35 100644 --- a/src/main/java/org/zalando/problem/spring/web/advice/validation/BaseBindingResultAdviceTrait.java +++ b/src/main/java/org/zalando/problem/spring/web/advice/validation/BaseBindingResultAdviceTrait.java @@ -21,10 +21,10 @@ default Violation createViolation(final ObjectError error) { return new Violation(fieldName, error.getDefaultMessage()); } - default List createViolations(BindingResult bindingResult) { - return Stream.concat( - bindingResult.getFieldErrors().stream().map(this::createViolation), - bindingResult.getGlobalErrors().stream().map(this::createViolation)).collect(toList()); + default List createViolations(final BindingResult result) { + final Stream fieldErrors = result.getFieldErrors().stream().map(this::createViolation); + final Stream globalErrors = result.getGlobalErrors().stream().map(this::createViolation); + return Stream.concat(fieldErrors, globalErrors).collect(toList()); } } diff --git a/src/main/java/org/zalando/problem/spring/web/advice/validation/BaseValidationAdviceTrait.java b/src/main/java/org/zalando/problem/spring/web/advice/validation/BaseValidationAdviceTrait.java index e2047de8..5b70c1ab 100644 --- a/src/main/java/org/zalando/problem/spring/web/advice/validation/BaseValidationAdviceTrait.java +++ b/src/main/java/org/zalando/problem/spring/web/advice/validation/BaseValidationAdviceTrait.java @@ -6,6 +6,7 @@ import org.zalando.problem.spring.web.advice.AdviceTrait; import javax.ws.rs.core.Response.StatusType; +import java.net.URI; import java.util.Collection; import java.util.List; @@ -15,6 +16,10 @@ interface BaseValidationAdviceTrait extends AdviceTrait { + default URI defaultConstraintViolationType() { + return ConstraintViolationProblem.TYPE; + } + default StatusType defaultConstraintViolationStatus() { return BAD_REQUEST; } @@ -32,13 +37,17 @@ default String formatFieldName(final String fieldName) { default ResponseEntity newConstraintViolationProblem(final Throwable throwable, final Collection stream, final NativeWebRequest request) { + final URI type = defaultConstraintViolationType(); final StatusType status = defaultConstraintViolationStatus(); + final List violations = stream.stream() // sorting to make tests deterministic .sorted(comparing(Violation::getField).thenComparing(Violation::getMessage)) .collect(toList()); - return create(throwable, new ConstraintViolationProblem(status, violations), request); + final Problem problem = new ConstraintViolationProblem(type, status, violations); + + return create(throwable, problem, request); } } diff --git a/src/main/java/org/zalando/problem/spring/web/advice/validation/ConstraintViolationProblem.java b/src/main/java/org/zalando/problem/spring/web/advice/validation/ConstraintViolationProblem.java index 84685662..d25d52be 100644 --- a/src/main/java/org/zalando/problem/spring/web/advice/validation/ConstraintViolationProblem.java +++ b/src/main/java/org/zalando/problem/spring/web/advice/validation/ConstraintViolationProblem.java @@ -1,10 +1,7 @@ package org.zalando.problem.spring.web.advice.validation; -import com.fasterxml.jackson.annotation.JsonCreator; -import com.fasterxml.jackson.annotation.JsonTypeName; import org.zalando.problem.ThrowableProblem; -import javax.annotation.Nullable; import javax.annotation.concurrent.Immutable; import javax.ws.rs.core.Response.StatusType; import java.net.URI; @@ -13,30 +10,28 @@ import java.util.List; @Immutable -@JsonTypeName(ConstraintViolationProblem.TYPE_VALUE) public final class ConstraintViolationProblem extends ThrowableProblem { public static final String TYPE_VALUE = "https://zalando.github.io/problem/constraint-violation"; public static final URI TYPE = URI.create(TYPE_VALUE); + private final URI type; private final StatusType status; - private final String detail; private final List violations; public ConstraintViolationProblem(final StatusType status, final List violations) { - this(status, null, new ArrayList<>(violations)); + this(TYPE, status, new ArrayList<>(violations)); } - @JsonCreator - ConstraintViolationProblem(final StatusType status, @Nullable final String detail, final List violations) { + ConstraintViolationProblem(final URI type, final StatusType status, final List violations) { + this.type = type; this.status = status; - this.detail = detail; this.violations = Collections.unmodifiableList(violations); } @Override public URI getType() { - return TYPE; + return type; } @Override @@ -49,11 +44,6 @@ public StatusType getStatus() { return status; } - @Override - public String getDetail() { - return detail; - } - public List getViolations() { return violations; } diff --git a/src/main/java/org/zalando/problem/validation/ConstraintViolationProblemMixin.java b/src/main/java/org/zalando/problem/validation/ConstraintViolationProblemMixin.java index 4bc74ae8..6754782c 100644 --- a/src/main/java/org/zalando/problem/validation/ConstraintViolationProblemMixin.java +++ b/src/main/java/org/zalando/problem/validation/ConstraintViolationProblemMixin.java @@ -5,6 +5,8 @@ import java.util.List; +// TODO package private +// TODO rename to MixIn public interface ConstraintViolationProblemMixin { @JsonProperty("violations") diff --git a/src/main/java/org/zalando/problem/validation/ConstraintViolationProblemModule.java b/src/main/java/org/zalando/problem/validation/ConstraintViolationProblemModule.java index d3ac0ca6..f9f72ba9 100644 --- a/src/main/java/org/zalando/problem/validation/ConstraintViolationProblemModule.java +++ b/src/main/java/org/zalando/problem/validation/ConstraintViolationProblemModule.java @@ -9,15 +9,15 @@ /** * A companion to {@link org.zalando.problem.ProblemModule} to enable serialization - * of {@link ConstraintViolationProblem} and {@link Violation} without relying on autodetection. + * of {@link ConstraintViolationProblem} and {@link Violation} without relying on auto detection. */ -public class ConstraintViolationProblemModule extends Module { - @Override - public void setupModule(SetupContext context) { - final SimpleModule module = new SimpleModule(); +public final class ConstraintViolationProblemModule extends Module { - module.setMixInAnnotation(Violation.class, ViolationMixIn.class); - module.setMixInAnnotation(ConstraintViolationProblem.class, ConstraintViolationProblemMixin.class); + @Override + public void setupModule(final SetupContext context) { + final SimpleModule module = new SimpleModule() + .setMixInAnnotation(ConstraintViolationProblem.class, ConstraintViolationProblemMixin.class) + .setMixInAnnotation(Violation.class, ViolationMixIn.class); module.setupModule(context); } @@ -27,10 +27,11 @@ public String getModuleName() { return ConstraintViolationProblemModule.class.getSimpleName(); } - @SuppressWarnings("deprecation") @Override + @SuppressWarnings("deprecation") public Version version() { return VersionUtil.mavenVersionFor(ConstraintViolationProblemModule.class.getClassLoader(), "org.zalando", "problem-spring-web"); } + } diff --git a/src/main/java/org/zalando/problem/validation/ViolationMixIn.java b/src/main/java/org/zalando/problem/validation/ViolationMixIn.java index 71457653..db2addbd 100644 --- a/src/main/java/org/zalando/problem/validation/ViolationMixIn.java +++ b/src/main/java/org/zalando/problem/validation/ViolationMixIn.java @@ -2,6 +2,7 @@ import com.fasterxml.jackson.annotation.JsonProperty; +// TODO package private public interface ViolationMixIn { @JsonProperty("field") diff --git a/src/test/java/org/zalando/problem/spring/web/advice/validation/ConstraintViolationProblemModuleTest.java b/src/test/java/org/zalando/problem/spring/web/advice/validation/ConstraintViolationProblemModuleTest.java new file mode 100644 index 00000000..3ebd342d --- /dev/null +++ b/src/test/java/org/zalando/problem/spring/web/advice/validation/ConstraintViolationProblemModuleTest.java @@ -0,0 +1,58 @@ +package org.zalando.problem.spring.web.advice.validation; + +import com.fasterxml.jackson.core.JsonProcessingException; +import com.fasterxml.jackson.databind.MapperFeature; +import com.fasterxml.jackson.databind.ObjectMapper; +import lombok.extern.slf4j.Slf4j; +import org.junit.Test; +import org.zalando.problem.ProblemModule; +import org.zalando.problem.validation.ConstraintViolationProblemModule; + +import java.net.URI; + +import static com.jayway.jsonassert.JsonAssert.with; +import static java.util.Collections.emptyList; +import static java.util.Collections.singletonList; +import static javax.ws.rs.core.Response.Status.BAD_REQUEST; +import static org.hamcrest.Matchers.contains; +import static org.hamcrest.Matchers.hasSize; +import static org.hamcrest.Matchers.is; + +@Slf4j +public class ConstraintViolationProblemModuleTest { + + @Test + public void shouldSerializeWithoutAutoDetect() throws JsonProcessingException { + final ObjectMapper mapper = new ObjectMapper() + .disable(MapperFeature.AUTO_DETECT_FIELDS) + .disable(MapperFeature.AUTO_DETECT_GETTERS) + .disable(MapperFeature.AUTO_DETECT_IS_GETTERS) + .registerModule(new ProblemModule()) + .registerModule(new ConstraintViolationProblemModule()); + + final Violation violation = new Violation("bob", "was missing"); + final ConstraintViolationProblem unit = new ConstraintViolationProblem(BAD_REQUEST, singletonList(violation)); + + with(mapper.writeValueAsString(unit)) + .assertThat("status", is(400)) + .assertThat("type", is(ConstraintViolationProblem.TYPE_VALUE)) + .assertThat("title", is("Constraint Violation")) + .assertThat("violations", hasSize(1)) + .assertThat("violations.*.field", contains("bob")) + .assertThat("violations.*.message", contains("was missing")); + } + + @Test + public void shouldSerializeCustomType() throws JsonProcessingException { + final ObjectMapper mapper = new ObjectMapper() + .registerModule(new ProblemModule()) + .registerModule(new ConstraintViolationProblemModule()); + + final URI type = URI.create("foo"); + final ConstraintViolationProblem unit = new ConstraintViolationProblem(type, BAD_REQUEST, emptyList()); + + with(mapper.writeValueAsString(unit)) + .assertThat("type", is("foo")); + } + +} \ No newline at end of file diff --git a/src/test/java/org/zalando/problem/spring/web/advice/validation/ConstraintViolationProblemTest.java b/src/test/java/org/zalando/problem/spring/web/advice/validation/ConstraintViolationProblemTest.java index 971d6596..a999cac1 100644 --- a/src/test/java/org/zalando/problem/spring/web/advice/validation/ConstraintViolationProblemTest.java +++ b/src/test/java/org/zalando/problem/spring/web/advice/validation/ConstraintViolationProblemTest.java @@ -8,7 +8,6 @@ import static javax.ws.rs.core.Response.Status.BAD_REQUEST; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.hasSize; -import static org.hamcrest.Matchers.is; public class ConstraintViolationProblemTest { @@ -22,7 +21,6 @@ public void shouldCreateCopyOfViolations(){ violations.clear(); assertThat(problem.getViolations(), hasSize(1)); - assertThat(problem.getViolations().get(0).getField(), is("x")); - assertThat(problem.getViolations().get(0).getMessage(), is("y")); } + } diff --git a/src/test/java/org/zalando/problem/validation/ConstraintViolationProblemModuleTest.java b/src/test/java/org/zalando/problem/validation/ConstraintViolationProblemModuleTest.java deleted file mode 100644 index 2597d82e..00000000 --- a/src/test/java/org/zalando/problem/validation/ConstraintViolationProblemModuleTest.java +++ /dev/null @@ -1,48 +0,0 @@ -package org.zalando.problem.validation; - -import com.fasterxml.jackson.core.JsonProcessingException; -import com.fasterxml.jackson.databind.MapperFeature; -import com.fasterxml.jackson.databind.ObjectMapper; -import com.jayway.jsonassert.JsonAssert; -import lombok.extern.slf4j.Slf4j; -import org.junit.Test; -import org.zalando.problem.ProblemModule; -import org.zalando.problem.spring.web.advice.validation.ConstraintViolationProblem; -import org.zalando.problem.spring.web.advice.validation.Violation; - -import javax.ws.rs.core.Response; - -import static java.util.Arrays.asList; -import static org.hamcrest.Matchers.contains; -import static org.hamcrest.Matchers.hasSize; -import static org.hamcrest.Matchers.is; - -@Slf4j -public class ConstraintViolationProblemModuleTest { - - @Test - public void serializeConstraintViolationUsingMixinAnnotations() throws JsonProcessingException { - ObjectMapper mapperWithDisabledAutoDetection = new ObjectMapper() - .disable(MapperFeature.AUTO_DETECT_FIELDS) - .disable(MapperFeature.AUTO_DETECT_GETTERS) - .disable(MapperFeature.AUTO_DETECT_IS_GETTERS); - - mapperWithDisabledAutoDetection.registerModule(new ProblemModule()); - mapperWithDisabledAutoDetection.registerModule(new ConstraintViolationProblemModule()); - - Violation violation = new Violation("bob", "was missing"); - ConstraintViolationProblem constraintViolationProblem = new ConstraintViolationProblem(Response.Status.BAD_REQUEST, asList(violation)); - - - String json = mapperWithDisabledAutoDetection.writeValueAsString(constraintViolationProblem); - - JsonAssert.with(json) - .assertThat("status", is(400)) - .assertThat("type", is(ConstraintViolationProblem.TYPE_VALUE)) - .assertThat("title", is("Constraint Violation")) - .assertThat("violations", hasSize(1)) - .assertThat("violations.*.field", contains("bob")) - .assertThat("violations.*.message", contains("was missing")); - } - -} \ No newline at end of file