From 63d2c4b6072a1a107de66fe435b70c515c8935d6 Mon Sep 17 00:00:00 2001 From: wreulicke <12907474+wreulicke@users.noreply.github.com> Date: Tue, 12 Sep 2023 01:27:22 +0900 Subject: [PATCH] Fix ValidationResult#isSuccess() when result is success (#27) Is this just a bug? expected: violations.isEmpty() => validation is success current: violations.isEmpty() => validation is **not success** I think so according to this example. https://github.com/bufbuild/protovalidate-java#example --------- Co-authored-by: Philip K. Warren Co-authored-by: Philip K. Warren --- .../buf/protovalidate/ValidatorTest.java | 74 +++++++++---------- .../buf/protovalidate/ValidationResult.java | 2 +- 2 files changed, 38 insertions(+), 38 deletions(-) diff --git a/conformance/src/test/java/build/buf/protovalidate/ValidatorTest.java b/conformance/src/test/java/build/buf/protovalidate/ValidatorTest.java index 4c353693..dd6973c3 100644 --- a/conformance/src/test/java/build/buf/protovalidate/ValidatorTest.java +++ b/conformance/src/test/java/build/buf/protovalidate/ValidatorTest.java @@ -21,6 +21,7 @@ import build.buf.validate.conformance.cases.AnEnum; import build.buf.validate.conformance.cases.BoolConstTrue; import build.buf.validate.conformance.cases.BytesContains; +import build.buf.validate.conformance.cases.BytesIn; import build.buf.validate.conformance.cases.DurationGTELTE; import build.buf.validate.conformance.cases.Embed; import build.buf.validate.conformance.cases.EnumDefined; @@ -64,8 +65,8 @@ public void setUp() { public void strprefix() throws Exception { StringPrefix invalid = StringPrefix.newBuilder().setVal("foo").build(); ValidationResult validate = validator.validate(invalid); - assertThat(validate.getViolations().isEmpty()).isTrue(); - assertThat(validate.getViolations()).hasSize(0); + assertThat(validate.getViolations()).isEmpty(); + assertThat(validate.isSuccess()).isTrue(); } @Test @@ -73,16 +74,16 @@ public void bytescontains() throws Exception { BytesContains invalid = BytesContains.newBuilder().setVal(ByteString.copyFromUtf8("candy bars")).build(); ValidationResult validate = validator.validate(invalid); - assertThat(validate.getViolations().isEmpty()).isTrue(); - assertThat(validate.getViolations()).hasSize(0); + assertThat(validate.getViolations()).isEmpty(); + assertThat(validate.isSuccess()).isTrue(); } @Test public void strcontains() throws Exception { StringContains invalid = StringContains.newBuilder().setVal("foobar").build(); ValidationResult validate = validator.validate(invalid); - assertThat(validate.getViolations().isEmpty()).isTrue(); - assertThat(validate.getViolations()).hasSize(0); + assertThat(validate.getViolations()).isEmpty(); + assertThat(validate.isSuccess()).isTrue(); } @Test @@ -90,7 +91,7 @@ public void boolconsttrue() throws Exception { BoolConstTrue invalid = BoolConstTrue.newBuilder().build(); ValidationResult validate = validator.validate(invalid); assertThat(validate.getViolations()).hasSize(1); - assertThat(validate.isSuccess()).isTrue(); + assertThat(validate.isSuccess()).isFalse(); } @Test @@ -99,7 +100,7 @@ public void timestampwithin() throws Exception { TimestampWithin.newBuilder().setVal(Timestamp.newBuilder().build()).build(); ValidationResult validate = validator.validate(invalid); assertThat(validate.getViolations()).hasSize(1); - assertThat(validate.isSuccess()).isTrue(); + assertThat(validate.isSuccess()).isFalse(); } @Test @@ -107,17 +108,17 @@ public void timestampcost() throws Exception { TimestampConst invalid = TimestampConst.newBuilder().setVal(Timestamp.newBuilder().setSeconds(3).build()).build(); ValidationResult validate = validator.validate(invalid); - assertThat(validate.getViolations().isEmpty()).isTrue(); - assertThat(validate.getViolations()).hasSize(0); + assertThat(validate.getViolations()).isEmpty(); + assertThat(validate.isSuccess()).isTrue(); } @Test - public void OneofIgnoreEmpty() throws Exception { + public void oneofIgnoreEmpty() throws Exception { OneofIgnoreEmpty invalid = OneofIgnoreEmpty.newBuilder().setY(ByteString.copyFromUtf8("")).build(); ValidationResult validate = validator.validate(invalid); - assertThat(validate.getViolations().isEmpty()).isTrue(); - assertThat(validate.getViolations()).hasSize(0); + assertThat(validate.getViolations()).isEmpty(); + assertThat(validate.isSuccess()).isTrue(); } @Test @@ -125,7 +126,7 @@ public void enumdefined() throws Exception { EnumDefined invalid = EnumDefined.newBuilder().setValValue(2147483647).build(); ValidationResult validate = validator.validate(invalid); assertThat(validate.getViolations()).hasSize(1); - assertThat(validate.isSuccess()).isTrue(); + assertThat(validate.isSuccess()).isFalse(); } @Test @@ -133,7 +134,7 @@ public void strictFixed32LT() throws Exception { Fixed32LT invalid = Fixed32LT.newBuilder().setVal(5).build(); ValidationResult validate = validator.validate(invalid); assertThat(validate.getViolations()).hasSize(1); - assertThat(validate.isSuccess()).isTrue(); + assertThat(validate.isSuccess()).isFalse(); } @Test @@ -142,7 +143,7 @@ public void strictWrapperDouble() throws Exception { WrapperDouble.newBuilder().setVal(DoubleValue.newBuilder().build()).build(); ValidationResult validate = validator.validate(invalid); assertThat(validate.getViolations()).hasSize(1); - assertThat(validate.isSuccess()).isTrue(); + assertThat(validate.isSuccess()).isFalse(); } @Test @@ -150,7 +151,7 @@ public void strictFieldExpressions() throws Exception { FieldExpressions invalid = FieldExpressions.newBuilder().build(); ValidationResult validate = validator.validate(invalid); assertThat(validate.getViolations()).hasSize(2); - assertThat(validate.isSuccess()).isTrue(); + assertThat(validate.isSuccess()).isFalse(); } @Test @@ -161,14 +162,14 @@ public void strictDurationGTELTE() throws Exception { .build(); ValidationResult validate = validator.validate(invalid); assertThat(validate.getViolations()).hasSize(1); - assertThat(validate.isSuccess()).isTrue(); + assertThat(validate.isSuccess()).isFalse(); } @Test public void strictRepeatedExact() throws Exception { RepeatedExact invalid = RepeatedExact.newBuilder().addAllVal(Arrays.asList(1, 2)).build(); ValidationResult validate = validator.validate(invalid); - assertThat(validate.isSuccess()).isTrue(); + assertThat(validate.isSuccess()).isFalse(); assertThat(validate.getViolations()).hasSize(1); } @@ -176,7 +177,7 @@ public void strictRepeatedExact() throws Exception { public void strictSFixed64In() throws Exception { SFixed64In invalid = SFixed64In.newBuilder().setVal(5).build(); ValidationResult validate = validator.validate(invalid); - assertThat(validate.isSuccess()).isTrue(); + assertThat(validate.isSuccess()).isFalse(); assertThat(validate.getViolations()).hasSize(1); } @@ -188,7 +189,7 @@ public void strictFieldExpressionsNested() throws Exception { .setC(FieldExpressions.Nested.newBuilder().setA(-3).build()) .build(); ValidationResult validate = validator.validate(invalid); - assertThat(validate.isSuccess()).isTrue(); + assertThat(validate.isSuccess()).isFalse(); assertThat(validate.getViolations()).hasSize(4); } @@ -196,15 +197,15 @@ public void strictFieldExpressionsNested() throws Exception { public void strictRepeatedExactIgnore() throws Exception { RepeatedExactIgnore invalid = RepeatedExactIgnore.newBuilder().build(); ValidationResult validate = validator.validate(invalid); - assertThat(validate.getViolations().isEmpty()).isTrue(); - assertThat(validate.getViolations()).hasSize(0); + assertThat(validate.getViolations()).isEmpty(); + assertThat(validate.isSuccess()).isTrue(); } @Test public void strictInt32In() throws Exception { Int32In invalid = Int32In.newBuilder().setVal(4).build(); ValidationResult validate = validator.validate(invalid); - assertThat(validate.isSuccess()).isTrue(); + assertThat(validate.isSuccess()).isFalse(); assertThat(validate.getViolations()).hasSize(1); } @@ -212,7 +213,7 @@ public void strictInt32In() throws Exception { public void strictRepeatedEnumIn() throws Exception { RepeatedEnumIn invalid = RepeatedEnumIn.newBuilder().addVal(AnEnum.AN_ENUM_X).build(); ValidationResult validate = validator.validate(invalid); - assertThat(validate.isSuccess()).isTrue(); + assertThat(validate.isSuccess()).isFalse(); assertThat(validate.getViolations()).hasSize(1); } @@ -224,7 +225,7 @@ public void strictRepeatedMin() throws Exception { .addVal(Embed.newBuilder().setVal(-1).build()) .build(); ValidationResult validate = validator.validate(invalid); - assertThat(validate.isSuccess()).isTrue(); + assertThat(validate.isSuccess()).isFalse(); assertThat(validate.getViolations()).hasSize(1); } @@ -234,20 +235,19 @@ public void testDynRuntimeError() throws Exception { validator.validate(invalid); } - // Needs : https://github.com/projectnessie/cel-java/pull/419 - // @Test - // public void strictBytesIn() throws ValidationException { - // BytesIn invalid = BytesIn.newBuilder().setVal(ByteString.copyFromUtf8("bar")).build(); - // ValidationResult validate = validator.validate(invalid); - // assertThat(validate.isSuccess()).isTrue(); - // } + @Test + public void strictBytesIn() throws ValidationException { + BytesIn invalid = BytesIn.newBuilder().setVal(ByteString.copyFromUtf8("bar")).build(); + ValidationResult validate = validator.validate(invalid); + assertThat(validate.isSuccess()).isTrue(); + } @Test public void strictRepeatedUnique() throws ValidationException { RepeatedUnique invalid = RepeatedUnique.newBuilder().addAllVal(Arrays.asList("foo", "bar", "foo", "baz")).build(); ValidationResult validate = validator.validate(invalid); - assertThat(validate.isSuccess()).isTrue(); + assertThat(validate.isSuccess()).isFalse(); } @Test @@ -264,14 +264,14 @@ public void testRecursiveInvalid() throws ValidationException { MapRecursive.newBuilder().putVal(1, MapRecursive.Msg.newBuilder().build()).build(); ValidationResult validate = validator.validate(test); assertThat(validate.getViolations()).hasSize(1); - assertThat(validate.isSuccess()).isTrue(); + assertThat(validate.isSuccess()).isFalse(); } @Test public void testStringLenEmoji() throws ValidationException { StringLen test = StringLen.newBuilder().setVal("😅😄👾").build(); ValidationResult validate = validator.validate(test); - assertThat(validate.getViolations()).hasSize(0); - assertThat(validate.getViolations().isEmpty()).isTrue(); + assertThat(validate.getViolations()).isEmpty(); + assertThat(validate.isSuccess()).isTrue(); } } diff --git a/src/main/java/build/buf/protovalidate/ValidationResult.java b/src/main/java/build/buf/protovalidate/ValidationResult.java index f143c519..ad411534 100644 --- a/src/main/java/build/buf/protovalidate/ValidationResult.java +++ b/src/main/java/build/buf/protovalidate/ValidationResult.java @@ -48,7 +48,7 @@ public ValidationResult(List violations) { * @return if the validation result was a success. */ public boolean isSuccess() { - return !violations.isEmpty(); + return violations.isEmpty(); } /**