Skip to content

Commit 03ce6ca

Browse files
rbygraveSentryMan
rbygrave
and
SentryMan
authored
Fix "continue validation" (list of Pattern validation etc) (#270)
* Add Tests for "continue validation" existing behavior * No behavior change - Update existing tests (and fix some of those) * Change AbstractConstraintAdapter to continue validation, Adjust NullableAdapter and NotEmptyAdapter to support existing behavior NullableAdapter and NotEmptyAdapter support returning NOT "continue validation" * Add PatternListTest Which passes with the prior fixes in place wrt "continue validation" * Change NotBlank(max=..) and Size of String to "continue validation" when they are invalid * Refactor remove warning for boxing and isEmpty() --------- Co-authored-by: Josiah Noel <32279667+SentryMan@users.noreply.github.com>
1 parent 5cb6a2f commit 03ce6ca

18 files changed

+502
-322
lines changed

‎blackbox-test/src/test/java/example/avaje/repeat/PatternListTest.java

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,54 @@
1+
package example.avaje.repeat;
2+
3+
import static org.junit.jupiter.api.Assertions.assertEquals;
4+
import static org.junit.jupiter.api.Assertions.assertThrows;
5+
6+
import org.junit.jupiter.api.Test;
7+
8+
import io.avaje.validation.ConstraintViolation;
9+
import io.avaje.validation.ConstraintViolationException;
10+
import io.avaje.validation.Validator;
11+
import jakarta.validation.Valid;
12+
import jakarta.validation.constraints.Pattern;
13+
14+
public class PatternListTest {
15+
16+
@Valid
17+
public static class Bean {
18+
19+
@Pattern(regexp = "^[ABCD]{4}$", message = "Missing ABCD")
20+
@Pattern(regexp = ".*[A].*", message = "Missing A")
21+
@Pattern(regexp = ".*[B].*", message = "Missing B")
22+
@Pattern(regexp = ".*[C].*", message = "Missing C")
23+
@Pattern(regexp = ".*[D].*", message = "Missing D")
24+
public String field;
25+
}
26+
27+
private final Validator validator =
28+
Validator.builder()
29+
.build();
30+
31+
@Test
32+
void testNoViolations() {
33+
final Bean bean = new Bean();
34+
bean.field = "ABCD";
35+
36+
validator.validate(bean);
37+
// no exception
38+
}
39+
40+
@Test
41+
void testMultipleViolations() {
42+
final Bean bean = new Bean();
43+
bean.field = "f";
44+
45+
final ConstraintViolationException exceptions =
46+
assertThrows(ConstraintViolationException.class, () -> validator.validate(bean));
47+
48+
for (final ConstraintViolation violation : exceptions.violations()) {
49+
System.out.println("Violation: " + violation.toString());
50+
}
51+
52+
assertEquals(5, exceptions.violations().size());
53+
}
54+
}

‎blackbox-test/src/test/java/example/avaje/repeat/SignupRequestTest.java

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,10 +23,11 @@ void lowercaseNoSpecial() {
2323
SignupRequest req = new SignupRequest("foo");
2424

2525
var violations = all(req, Locale.ENGLISH);
26-
assertThat(violations).hasSize(3);
26+
assertThat(violations).hasSize(4);
2727
assertThat(violations.get(0).message()).isEqualTo("Signup password size error");
2828
assertThat(violations.get(1).message()).isEqualTo("Signup must have at least 1 upper case");
29-
assertThat(violations.get(2).message()).isEqualTo("Signup special character");
29+
assertThat(violations.get(2).message()).isEqualTo("Signup digit");
30+
assertThat(violations.get(3).message()).isEqualTo("Signup special character");
3031
}
3132

3233
@Test

‎validator/src/main/java/io/avaje/validation/adapter/AbstractConstraintAdapter.java

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,6 @@ public final boolean validate(T value, ValidationRequest req, String propertyNam
3232
}
3333
if (!isValid(value)) {
3434
req.addViolation(message, propertyName);
35-
return false;
3635
}
3736
return true;
3837
}

‎validator/src/main/java/io/avaje/validation/core/adapters/BasicAdapters.java

Lines changed: 39 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -110,7 +110,6 @@ public boolean validate(Object value, ValidationRequest req, String propertyName
110110
final var len = sequence.length();
111111
if (len > max || len < min) {
112112
req.addViolation(message, propertyName);
113-
return false;
114113
}
115114
} else if (value instanceof final Collection<?> col) {
116115
final var len = col.size();
@@ -173,7 +172,6 @@ public boolean validate(CharSequence value, ValidationRequest req, String proper
173172
}
174173
if (maxLength > 0 && value.length() > maxLength) {
175174
req.addViolation(maxLengthMessage != null ? maxLengthMessage : message, propertyName);
176-
return false;
177175
}
178176
return true;
179177
}
@@ -192,26 +190,41 @@ static boolean isBlank(final CharSequence cs) {
192190
}
193191
}
194192

195-
private static final class NotEmptyAdapter extends AbstractConstraintAdapter<Object> {
193+
private static final class NotEmptyAdapter implements ValidationAdapter<Object> {
194+
195+
private final ValidationContext.Message message;
196+
private final Set<Class<?>> groups;
196197

197198
NotEmptyAdapter(AdapterCreateRequest request) {
198-
super(request);
199+
this.groups = request.groups();
200+
this.message = request.message();
199201
}
200202

201203
@Override
202-
public boolean isValid(Object value) {
203-
if (value == null) {
204+
public boolean validate(Object value, ValidationRequest req, String propertyName) {
205+
if (!checkGroups(groups, req)) {
206+
return true;
207+
}
208+
if (invalid(value)) {
209+
req.addViolation(message, propertyName);
204210
return false;
211+
}
212+
return true;
213+
}
214+
215+
private boolean invalid(Object value) {
216+
if (value == null) {
217+
return true;
205218
} else if (value instanceof final Collection<?> col) {
206-
return !col.isEmpty();
219+
return col.isEmpty();
207220
} else if (value instanceof final Map<?, ?> map) {
208-
return !map.isEmpty();
221+
return map.isEmpty();
209222
} else if (value instanceof final CharSequence sequence) {
210-
return sequence.length() != 0;
223+
return sequence.isEmpty();
211224
} else if (value.getClass().isArray()) {
212-
return arrayLength(value) != 0;
225+
return arrayLength(value) == 0;
213226
}
214-
return true;
227+
return false;
215228
}
216229
}
217230

@@ -226,7 +239,7 @@ private static final class AssertBooleanAdapter extends PrimitiveAdapter<Boolean
226239

227240
@Override
228241
public boolean isValid(Boolean value) {
229-
return value == null || assertBool == value.booleanValue();
242+
return value == null || assertBool == value;
230243
}
231244

232245
@Override
@@ -235,18 +248,28 @@ public boolean isValid(boolean value) {
235248
}
236249
}
237250

238-
private static final class NullableAdapter extends AbstractConstraintAdapter<Object> {
251+
private static final class NullableAdapter implements ValidationAdapter<Object> {
239252

240253
private final boolean shouldBeNull;
254+
private final ValidationContext.Message message;
255+
private final Set<Class<?>> groups;
241256

242257
NullableAdapter(AdapterCreateRequest request, boolean shouldBeNull) {
243-
super(request);
244258
this.shouldBeNull = shouldBeNull;
259+
this.groups = request.groups();
260+
this.message = request.message();
245261
}
246262

247263
@Override
248-
public boolean isValid(Object value) {
249-
return (value == null) == shouldBeNull;
264+
public boolean validate(Object value, ValidationRequest req, String propertyName) {
265+
if (!checkGroups(groups, req)) {
266+
return true;
267+
}
268+
if ((value == null) != shouldBeNull) {
269+
req.addViolation(message, propertyName);
270+
return false;
271+
}
272+
return true;
250273
}
251274
}
252275

‎validator/src/test/java/io/avaje/validation/core/BasicTest.java

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
import io.avaje.validation.ConstraintViolation;
44
import io.avaje.validation.ConstraintViolationException;
55
import io.avaje.validation.Validator;
6+
import io.avaje.validation.adapter.ValidationAdapter;
67
import io.avaje.validation.adapter.ValidationContext;
78

89
import java.time.Duration;
@@ -40,4 +41,10 @@ protected ConstraintViolation one(Object pojo, Locale locale, Class<?>... groups
4041
return violations.get(0);
4142
}
4243
}
44+
45+
protected boolean isValid(ValidationAdapter<Object> adapter, Object obj) {
46+
var req = new DRequest((DValidator) validator, false, null, List.of());
47+
adapter.validate(obj, req);
48+
return !req.hasViolations();
49+
}
4350
}

‎validator/src/test/java/io/avaje/validation/core/adapters/AssertBooleanTest.java

Lines changed: 13 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -23,21 +23,28 @@ class AssertBooleanTest extends BasicTest {
2323
ValidationAdapter<Object> falseAdapter =
2424
ctx.adapter(AssertFalse.class, Map.of("message", "is false"));
2525

26+
@Test
27+
void continueOnInvalid_expect_false() {
28+
// does not matter if it continues or not really
29+
assertThat(trueAdapter.validate(false, request, "foo")).isTrue();
30+
assertThat(falseAdapter.validate(true, request, "foo")).isTrue();
31+
}
32+
2633
@Test
2734
void testNull() {
28-
assertThat(trueAdapter.validate(null, request)).isTrue();
29-
assertThat(falseAdapter.validate(null, request)).isTrue();
35+
assertThat(isValid(trueAdapter, null)).isTrue();
36+
assertThat(isValid(falseAdapter, null)).isTrue();
3037
}
3138

3239
@Test
3340
void testTrue() {
34-
assertThat(trueAdapter.validate(true, request)).isTrue();
35-
assertThat(falseAdapter.validate(true, request)).isFalse();
41+
assertThat(isValid(trueAdapter, true)).isTrue();
42+
assertThat(isValid(falseAdapter, true)).isFalse();
3643
}
3744

3845
@Test
3946
void testFalse() {
40-
assertThat(trueAdapter.validate(false, request)).isFalse();
41-
assertThat(falseAdapter.validate(false, request)).isTrue();
47+
assertThat(isValid(trueAdapter, false)).isFalse();
48+
assertThat(isValid(falseAdapter, false)).isTrue();
4249
}
4350
}

‎validator/src/test/java/io/avaje/validation/core/adapters/DecimalMinMaxTest.java

Lines changed: 40 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -23,58 +23,64 @@ class DecimalMinMaxTest extends BasicTest {
2323
ValidationAdapter<Object> maxAdapter =
2424
ctx.adapter(DecimalMax.class, Map.of("message", "maxwell", "value", "69", "_type", "Number"));
2525

26+
@Test
27+
void continueOnInvalid_expect_false() {
28+
assertThat(minAdapter.validate(BigDecimal.valueOf(-100), request, "foo")).isTrue();
29+
assertThat(maxAdapter.validate(BigDecimal.valueOf(100), request, "foo")).isTrue();
30+
}
31+
2632
@Test
2733
void testNull() {
28-
assertThat(minAdapter.validate(null, request)).isTrue();
29-
assertThat(maxAdapter.validate(null, request)).isTrue();
34+
assertThat(isValid(minAdapter, null)).isTrue();
35+
assertThat(isValid(maxAdapter, null)).isTrue();
3036
}
3137

3238
@Test
3339
void testMax() {
3440

35-
assertThat(maxAdapter.validate(0, request)).isTrue();
36-
assertThat(maxAdapter.validate(0f, request)).isTrue();
37-
assertThat(maxAdapter.validate(0D, request)).isTrue();
38-
assertThat(maxAdapter.validate(0L, request)).isTrue();
39-
assertThat(maxAdapter.validate((short) 0, request)).isTrue();
40-
assertThat(maxAdapter.validate((byte) 0, request)).isTrue();
41-
assertThat(maxAdapter.validate(BigInteger.valueOf(0), request)).isTrue();
42-
assertThat(maxAdapter.validate(BigDecimal.valueOf(0), request)).isTrue();
41+
assertThat(isValid(maxAdapter, 0)).isTrue();
42+
assertThat(isValid(maxAdapter, 0f)).isTrue();
43+
assertThat(isValid(maxAdapter, 0D)).isTrue();
44+
assertThat(isValid(maxAdapter, 0L)).isTrue();
45+
assertThat(isValid(maxAdapter, (short) 0)).isTrue();
46+
assertThat(isValid(maxAdapter, (byte) 0)).isTrue();
47+
assertThat(isValid(maxAdapter, BigInteger.valueOf(0))).isTrue();
48+
assertThat(isValid(maxAdapter, BigDecimal.valueOf(0))).isTrue();
4349
}
4450

4551
@Test
4652
void testMin() {
47-
assertThat(minAdapter.validate(-0, request)).isTrue();
48-
assertThat(minAdapter.validate(-0f, request)).isTrue();
49-
assertThat(minAdapter.validate(-0D, request)).isTrue();
50-
assertThat(minAdapter.validate(-0L, request)).isTrue();
51-
assertThat(minAdapter.validate((short) -0, request)).isTrue();
52-
assertThat(minAdapter.validate((byte) -0, request)).isTrue();
53-
assertThat(minAdapter.validate(BigInteger.valueOf(-0), request)).isTrue();
54-
assertThat(minAdapter.validate(BigDecimal.valueOf(-0), request)).isTrue();
53+
assertThat(isValid(minAdapter, -0)).isTrue();
54+
assertThat(isValid(minAdapter, -0f)).isTrue();
55+
assertThat(isValid(minAdapter, -0D)).isTrue();
56+
assertThat(isValid(minAdapter, -0L)).isTrue();
57+
assertThat(isValid(minAdapter, (short) -0)).isTrue();
58+
assertThat(isValid(minAdapter, (byte) -0)).isTrue();
59+
assertThat(isValid(minAdapter, BigInteger.valueOf(-0))).isTrue();
60+
assertThat(isValid(minAdapter, BigDecimal.valueOf(-0))).isTrue();
5561
}
5662

5763
@Test
5864
void testMaxInValid() {
59-
assertThat(maxAdapter.validate(01234, request)).isFalse();
60-
assertThat(maxAdapter.validate(01234f, request)).isFalse();
61-
assertThat(maxAdapter.validate(01234D, request)).isFalse();
62-
assertThat(maxAdapter.validate(01234L, request)).isFalse();
63-
assertThat(maxAdapter.validate((short) 01234, request)).isFalse();
64-
assertThat(maxAdapter.validate((byte) 01234567, request)).isFalse();
65-
assertThat(maxAdapter.validate(BigInteger.valueOf(01234), request)).isFalse();
66-
assertThat(maxAdapter.validate(BigDecimal.valueOf(01234), request)).isFalse();
65+
assertThat(isValid(maxAdapter, 01234)).isFalse();
66+
assertThat(isValid(maxAdapter, 01234f)).isFalse();
67+
assertThat(isValid(maxAdapter, 01234D)).isFalse();
68+
assertThat(isValid(maxAdapter, 01234L)).isFalse();
69+
assertThat(isValid(maxAdapter, (short) 01234)).isFalse();
70+
assertThat(isValid(maxAdapter, (byte) 01234567)).isFalse();
71+
assertThat(isValid(maxAdapter, BigInteger.valueOf(01234))).isFalse();
72+
assertThat(isValid(maxAdapter, BigDecimal.valueOf(01234))).isFalse();
6773
}
6874

6975
@Test
7076
void testMinInValid() {
71-
assertThat(minAdapter.validate(-01234, request)).isFalse();
72-
assertThat(minAdapter.validate(-01234f, request)).isFalse();
73-
assertThat(minAdapter.validate(-01234D, request)).isFalse();
74-
assertThat(minAdapter.validate(-01234L, request)).isFalse();
75-
assertThat(minAdapter.validate((short) -01234, request)).isFalse();
76-
assertThat(minAdapter.validate((byte) -01234567, request)).isFalse();
77-
assertThat(minAdapter.validate(BigInteger.valueOf(-01234), request)).isFalse();
78-
assertThat(minAdapter.validate(BigDecimal.valueOf(-01234), request)).isFalse();
77+
assertThat(isValid(minAdapter, -01234)).isFalse();
78+
assertThat(isValid(minAdapter, -01234f)).isFalse();
79+
assertThat(isValid(minAdapter, -01234D)).isFalse();
80+
assertThat(isValid(minAdapter, -01234L)).isFalse();
81+
assertThat(isValid(minAdapter, (short) -01234)).isFalse();
82+
assertThat(isValid(minAdapter, (byte) -01234567)).isFalse();
83+
assertThat(isValid(minAdapter, BigInteger.valueOf(-01234))).isFalse();
84+
assertThat(isValid(minAdapter, BigDecimal.valueOf(-01234))).isFalse();
7985
}
8086
}

0 commit comments

Comments
 (0)