[BUG] Non-deterministic error values when using string.uuid

[oxisto]

Description

After upgrading from 0.5 to 0.6 I get a non-deterministic behaviour when validating strings with the UUID validator when the string is empty. Half of the time I get value is empty, which is not a valid UUID, and half of the time I get value must be a valid UUID. I guess the first one might be better, but it would be good (especially for testing) if either one or the other is used.

Steps to Reproduce

1. Use 0.6.0 of this library
2. Use the constraint [(buf.validate.field).string.uuid = true];
3. Set the field to an empty string

Expected Behavior

I would probably expect the new value is empty, which is not a valid UUID error string

Actual Behavior

Half of the time I get value is empty, which is not a valid UUID, and half of the time I get value must be a valid UUID

Environment

• Operating System: maOS
• Protobuf Compiler & Version: buf 1.29.0
• Protovalidate Version: v0.6.0

[nicksnyder]

For a given message, you should see a deterministic result based on this logic:

• The error value is empty, which is not a valid UUID should happen if the value is the empty string.
• The error value must be a valid UUID should happen if the value is a non-empty string that doesn't match the UUID regex.

I assume you are seeing both errors in your tests because you have test cases that exercise both kinds of validation failures. You should be able to just update your test cases to expect the more precise error messages for each case.

If you are actually seeing non-determinism, or if the behavior doesn't match what I described above, then please share a reproducible example.

[mfridman]

Hey @oxisto, great to hear from you and hope protovalidate is working out in your project.

Since you're bumping the bufbuild/protovalidate-go dependency from v0.5.0 -> v0.6.0, there were some changes to the output, namely the snippet below (added in bufbuild/protovalidate v0.5.5)

https://github.com/bufbuild/protovalidate/pull/133/files#diff-e165063a9360981b1ee21bb31246352782bfef10a431c4ee8c001ce9630d0962R2897-R2901

I suggest the following:

1. search/replace value must be a valid UUID with value is empty, which is not a valid UUID (19 occurrences)
2. run the tests, and fix the failing tests where you do actually have an invalid uuid with value must be a valid UUID (3 occurrences)

Happy to help out if you'd like an upstream contribution.

[oxisto]

For a given message, you should see a deterministic result based on this logic:

• The error value is empty, which is not a valid UUID should happen if the value is the empty string.
• The error value must be a valid UUID should happen if the value is a non-empty string that doesn't match the UUID regex.

I assume you are seeing both errors in your tests because you have test cases that exercise both kinds of validation failures. You should be able to just update your test cases to expect the more precise error messages for each case.

If you are actually seeing non-determinism, or if the behavior doesn't match what I described above, then please share a reproducible example.

You are completely right. It seems that we had two validation rules one in the "outer" and one in the "inner" message and both were triggered.

[oxisto]

Hey @oxisto, great to hear from you and hope protovalidate is working out in your project.

Since you're bumping the bufbuild/protovalidate-go dependency from v0.5.0 -> v0.6.0, there were some changes to the output, namely the snippet below (added in bufbuild/protovalidate v0.5.5)

https://github.com/bufbuild/protovalidate/pull/133/files#diff-e165063a9360981b1ee21bb31246352782bfef10a431c4ee8c001ce9630d0962R2897-R2901

I suggest the following:

1. search/replace value must be a valid UUID with value is empty, which is not a valid UUID (19 occurrences)
2. run the tests, and fix the failing tests where you do actually have an invalid uuid with value must be a valid UUID (3 occurrences)

Happy to help out if you'd like an upstream contribution.

Thanks for the details on this! I was wondering where the exact messages were coming from. And thanks for the support, I managed to fix all the failing tests now.