Refactor validator creation to support various configurations

[smaye81]

Fixes #51

This makes a series of changes to the way that a validator is created. Doing so enables more flexible ways to configure a validator and also provides a bit more safety when attempting to create a validator with invalid state. The impetus behind this change was to allow for the ability to specify a list of seed descriptors when building a validator and for the optional ability to disable lazy-loading of any descriptors not provided.

Previously, a validator could be created either with a default constructor (new Validator()) or via a Config object (new Validator(config)). However, the Config object had no way to provide a list of descriptors, it only had a disableLazy field. Simply adding a list of descriptors to the Config presented a few problems though:

• Since these were two unrelated fields in the config, users had a serious footgun to play with because it was possible to set disableLazy to true and not specify any descriptors. Doing so would return an error at validation time with a validation message about an unknown evaluator.
• Even if the above was tolerable, building validation rules from descriptors necessitates catching a CompilationException. This meant that if the seed descriptors were added to the config, all users creating a validator would now have to catch a checked exception even if they weren't using the seed descriptors.

The solution in this PR converts validator creation from using constructors into a builder pattern with the following changes:

• disableLazy and the new descriptors list have been moved from the config and are now parameters at instantiation time.
• There are two options for building once the builder is created:
  • build()
  • buildWithDescriptors(List<Descriptor>, boolean disableLazy) throws CompilationException, InvalidStateException

The overload that accepts seed descriptors is the only one that throws a CompilationException. If users choose to use build with no arguments, no checked exception needs to be handled.

Note that the above also allows us to provide a helpful check at instantiation time if a user sets disableLazy to true but doesn't provide any descriptors. In this case, an IllegalStateException is thrown. It is still possible to provide an incorrect list of descriptors, but that is impossible to catch at instantiation time (but hopefully is now a bit harder to do).

The new API now adds a ValidatorFactory class with static methods for creating a validator. In addition, the type returned is now an interface (still named Validator) which provides us with a bit more flexibility both for testing and for making non-breaking changes in the future.

Examples of creating a validator with no seed descriptors:

// Create a Validator with a default config
Validator validator = ValidatorFactory.newBuilder().build();

// Create a Validator with a custom config
Config cfg = Config.newBuilder().setFailFast(true).build();
Validator validator = ValidatoryFactory.newBuilder().withConfig(cfg).build();

Creating a validator with seed descriptors is similar. You would still call newBuilder, but at build time, you call buildWithDescriptors and pass the descriptors and set disableLazy accordingly. Using this approach requires catching exceptions at build time:

// Create a validator with seed descriptors
MyMessage msg = MyMessage.newBuilder().build();
List<Descriptor> seedDescriptors = new ArrayList<Descriptor>();
seedDescriptors.add(msg.getDescriptorForType());

try {
    Validator validator = ValidatorFactory.newBuilder().buildWithDescriptors(seedDescriptors, false);

    // Note you can also create a validator with a config
    // Validator validator = ValidatorFactory.newBuilder().withConfig(cfg).buildWithDescriptors(seedDescriptors, false);

    // Also note that the buildWithDescriptors function is what throws the exception so the validator 
    // builder can be created ahead of time with no need for catching an exception.
} catch (CompilationException ce) {
    // Exception pre-warming the cache with the seed descriptors
} catch (IllegalStateException ise) {
    // This is thrown if seedDescriptors is empty and disableLazy is true
}

Note: THIS IS A BREAKING CHANGE, but the hope is that this makes it easier and more intuitive for creating a validator while providing future-proof flexibility. To migrate, users should make the following changes:

With no config

import build.buf.protovalidate.Validator;
+ import build.buf.protovalidate.ValidatorFactory;

- Validator validator = new Validator();
+ Validator validator = ValidatorFactory.newBuilder().build();

With config

import build.buf.protovalidate.Validator;
+ import build.buf.protovalidate.ValidatorFactory;

Config cfg = Config.newBuilder().setFailFast(true).build();
- Validator validator = new Validator(cfg);
+ Validator validator = ValidatorFactory.newBuilder().withConfig(cfg).build();

In addition, the loadMessages and loadDescriptors methods from the validator have been removed. Users should instead use the newBuilder(List<Descriptor> descriptors, boolean disableLazy) approach to load descriptors ahead of time.

[pkwarren]

This only looks to be referenced in the constructor. We should be able to remove this field and just iterate over the constructor arg below.

[pkwarren]

Suggested change

	public class ValidatorFactory {
	public final class ValidatorFactory {

[pkwarren]

Suggested change

;

[pkwarren]

Suggested change

	if (disableLazy && this.descriptors.size() == 0) {
	if (disableLazy && this.descriptors.isEmpty()) {

[pkwarren]

This should be made static and lifted into the ValidatorFactory class or moved to a separate .java file.

[pkwarren]

Perhaps a simpler design would be to have a single builder class, with two build methods:

public Validator build() {}

public Validator buildWithDescriptors(List<Descriptor> descriptors, boolean disableLazy) throws CompilationException, IllegalStateException;

[smaye81]

That's fair. To do that, we'd have to create the EvaluatorBuilder in the builder and keep it in state so that it can be passed to the ValidatorImpl constructor. Not sure if that's desirable or not. It definitely simplifies the design but at the cost of having more logic in the actual builder itself. Wdyt?

[pkwarren]

I think we should leave this out - it isn't that much more to type to have someone just do ValidatorFactory.newBuilder().build(). We could always add it back if desired.

This method as written feels like it is returning the same instance each time (similar to the global validator in pv-go: https://pkg.go.dev/buf.build/go/protovalidate#Validator), not a new instance each time w/ the default config.

[pkwarren]

The nil check isn't needed here - if anything we could add an Objects.requireNonNull(descriptors) above if we wanted with an error message. Doesn't hurt but typically you never pass around null collections.

[smaye81]

Without the check, it throws an NPE though: java.lang.NullPointerException: Cannot invoke "java.util.List.iterator()". I think we should do something here to catch that. Seems like an easy thing to prevent.

[pkwarren]

This would only happen though if someone passes null to the buildDescriptors method, right?

Typically if you wanted a nicer error you'd do Objects.requireNonNull(descriptors, "descriptors must not be null") as the first line in the constructor (or the equivalent Preconditions.checkNotNull from Guava).

[smaye81]

OK, removed the null check and added the requireNonNull check instead.