Implement predefined field constraints

[jchadwick-buf]

This fails in CI for now, it depends on bufbuild/protovalidate#246.

Assuming you have protovalidate cloned in the parent folder of protovalidate-go, and that you've ran make conformance after checking out bufbuild/protovalidate#246, the following command can be used to test against the local protovalidate. This will not currently pass all the way; it will have two failures that are fixed by bufbuild/protovalidate#245.

(rm -fr .tmp/protovalidate; buf generate ../protovalidate/proto/protovalidate -o .tmp/protovalidate; mv .tmp/protovalidate/internal/gen/buf .tmp/protovalidate; rm -fr .tmp/protovalidate/internal; cd .tmp/protovalidate; go mod init buf.build/gen/go/bufbuild/protovalidate/protocolbuffers/go; go mod tidy; cd ..; go mod edit -replace=buf.build/gen/go/bufbuild/protovalidate/protocolbuffers/go=./.tmp/protovalidate && go mod tidy) && go build -o protovalidate-conformance-go ./internal/cmd/protovalidate-conformance-go && ../protovalidate/.tmp/bin/protovalidate-conformance --strict --strict_message --strict_error ./protovalidate-conformance-go

[jchadwick-buf]

FYI: Some changes are being worked on with regards to compatibility. For posterity sake I want to write an explanation. In particular, I noticed some rather serious problems with how our approach to resolving unknown extensions would interplay with users that need to use dynamic descriptor sets; we were relying on the GlobalTypes registry to resolve extensions, but this would not work for users that many have multiple sets of file descriptor sets that are distinct, as the user will not be able to register them. To rectify this I unified the code that resolves extensions for all constraint options.

Now there are tests that should ensure any proto.Message with field constraints will always resolve, no matter if they're using the concrete gencode types, somehow an incompatible gencode type (e.g. if the protovalidate package path changes), a dynamicpb.Message, or if the field constraints are hidden away in unknown fields. In addition, to ensure predefined field constraints are resolved correctly, the user can pass in an extension resolver when creating a validator. Otherwise, GlobalTypes will be used, which works in all of the obvious cases. Similar adaptations will probably be needed for other versions (I already added it to the Java runtime.)

There's actually quite a few bits of nuance I ran into that made this tricky. For example, we cannot mutate the return value of Options() - it is illegal to do so according to protoreflect and would not be goroutine safe anyway, which would probably surprise people since it would only ever come up in edge cases.

Despite this, we do have at least one win: it is possible to avoid RangeExtensions while still not panicing, because we can use ProtoReflect().Get() instead, which is exactly what proto.GetExtension does internally anyways. Then later on, we can use the same logic we normally would to determine if a reparse is necessary.

The code winds up having to do up to four field presence checks:

• On the concrete fields, using the normal extension tag number
• On the concrete fields, using the legacy extension tag number
• On the reparsed unknown fields, using the normal extension tag number
• On the reparsed unknown fields, using the legacy extension tag number

The reparsed checks have to be done separately because again, we can't actually mutate the field options, so we have to reparse into a new message and check that. However this should not be a major concern, since this step is cached.

A net effect of this is that the conformance test suite runner is even more agnostic to the input data, and previous versions of the conformance test suite should work correctly with the new runner.

That said, at the moment this is only backward compatibility and not forward compatibility. If you use a standard constraint that is too new, it will not work, just as it currently does not work. I am going forward with the assumption that this is OK but it may warrant some thought so I'll ask a couple folks their thoughts anyways.

[jchadwick-buf]

Update: Added a WithAllowUnknownFields option which defaults to false. That way, the default behavior doesn't allow missing definitions to result in silently-broken validation. This is another breaking change, although in theory there is no way it can break anyone, because the version that adds it will also be the version that adds custom predefined rules, and by definition has a superset of all standard constraints ever defined prior.

I have also been investigating to ensure that our changes do not tank performance. Performance is mostly within margin of error although the switch to proto2 has increased the number of allocations in the invalid path (because the violation structure with proto2 explicit field presence is full of pointer-chasing.) I think it isn't worth investing any time into. Not sure what you could really do to improve that; string interning would probably help but I think it would be extremely unlikely to matter in real world use cases.

[jchadwick-buf]

Unfortunately I am currently blocked on getting the upstream protovalidate changes released for the moment so I have made some temporary adaptations in 040b31c that will make the diff as close to possible as the actual final diff. I'm hoping that we can get the protovalidate changes released before end-of-week but getting some eyes on the changes here would be great either way.

[jchadwick-buf]

Protovalidate 0.8.1 is out now so I've reverted the temporary changes and updated to it. We should be good to go.

[oliversun9]

For example, we cannot mutate the return value of Options() - it is illegal to do so according to protoreflect and would not be goroutine safe anyway, which would probably surprise people since it would only ever come up in edge cases.

It doesn't sound like this PR mutates any return value of Options() but I am curious for my own knowledge why protovalidate go needs to mutates this value ever 👀

[jchadwick-buf]

For example, we cannot mutate the return value of Options() - it is illegal to do so according to protoreflect and would not be goroutine safe anyway, which would probably surprise people since it would only ever come up in edge cases.

It doesn't sound like this PR mutates any return value of Options() but I am curious for my own knowledge why protovalidate go needs to mutates this value ever 👀

The reason why it might need to mutate the Options() data is because we need to reparse the options. The normal reparse logic we'd use in Go just does a merge-parse directly into the protoreflect.Message, but doing so here would be illegal, so we do it into an empty FieldOptions instead and then loop over those fields separately.

edit: Also, just to make it clear, this PR actually removes all mutation of Options() values, rather than adding more. Before this PR, the resolution of the old protovalidate extension (pre 0.2) was both 1. Mutating Options() in an illegal and goroutine unsafe way and 2. relying on protobuf internals to register the synthetic legacy extension. This PR works a bit of overtime to avoid both caveats without degrading performance (especially in the happy path.)

(As mentioned previously, this PR does degrade performance slightly in the violation case, but AFAICT it is because of the migration to proto2/explicit field presence requiring more allocs.)

[rodaine]

Suggested change

	// fields as needed to get it into the right concrete message as needed. Resolve
	// fields as needed to get it into the right concrete message. Resolve

[jchadwick-buf]

Fixed in 9d5c469

[rodaine]

Suggested change

	// the provided extension (or an equivalent legacy extension.) If there was no
	// the provided extension (or an equivalent legacy extension). If there was no

[jchadwick-buf]

Fixed in 9d5c469