From ea94ba1b104fc8e00d12ead79e6e9cc9b9380ea0 Mon Sep 17 00:00:00 2001 From: John Chadwick Date: Fri, 6 Sep 2024 08:08:58 -0400 Subject: [PATCH 1/4] Implement shared field rules --- .../conformance/FileDescriptorUtil.java | 27 ++- .../buf/protovalidate/conformance/Main.java | 18 +- .../java/build/buf/protovalidate/Config.java | 68 ++++++- .../build/buf/protovalidate/Validator.java | 8 +- .../internal/constraints/ConstraintCache.java | 172 ++++++++++++++---- .../evaluator/ConstraintResolver.java | 12 +- .../internal/evaluator/EvaluatorBuilder.java | 48 ++--- .../internal/expression/Variable.java | 13 ++ 8 files changed, 289 insertions(+), 77 deletions(-) diff --git a/conformance/src/main/java/build/buf/protovalidate/conformance/FileDescriptorUtil.java b/conformance/src/main/java/build/buf/protovalidate/conformance/FileDescriptorUtil.java index a310c262..295e119b 100644 --- a/conformance/src/main/java/build/buf/protovalidate/conformance/FileDescriptorUtil.java +++ b/conformance/src/main/java/build/buf/protovalidate/conformance/FileDescriptorUtil.java @@ -14,8 +14,6 @@ package build.buf.protovalidate.conformance; -import com.google.protobuf.DescriptorProtos; -import com.google.protobuf.Descriptors; import java.util.ArrayList; import java.util.HashMap; import java.util.List; @@ -70,4 +68,29 @@ static Map parseFileDescriptors( } return fileDescriptorMap; } + + static TypeRegistry createTypeRegistry( + Iterable fileDescriptors) { + TypeRegistry.Builder registryBuilder = TypeRegistry.newBuilder(); + for (Descriptors.FileDescriptor fileDescriptor : fileDescriptors) { + registryBuilder.add(fileDescriptor.getMessageTypes()); + } + return registryBuilder.build(); + } + + static ExtensionRegistry createExtensionRegistry( + Iterable fileDescriptors) { + ExtensionRegistry registry = ExtensionRegistry.newInstance(); + for (Descriptors.FileDescriptor fileDescriptor : fileDescriptors) { + for (Descriptors.FieldDescriptor fieldDescriptor : fileDescriptor.getExtensions()) { + if (fieldDescriptor.getJavaType() == Descriptors.FieldDescriptor.JavaType.MESSAGE) { + registry.add( + fieldDescriptor, DynamicMessage.getDefaultInstance(fieldDescriptor.getMessageType())); + } else { + registry.add(fieldDescriptor); + } + } + } + return registry; + } } diff --git a/conformance/src/main/java/build/buf/protovalidate/conformance/Main.java b/conformance/src/main/java/build/buf/protovalidate/conformance/Main.java index cc06ebe6..e369f94f 100644 --- a/conformance/src/main/java/build/buf/protovalidate/conformance/Main.java +++ b/conformance/src/main/java/build/buf/protovalidate/conformance/Main.java @@ -27,12 +27,6 @@ import build.buf.validate.conformance.harness.TestResult; import com.google.common.base.Splitter; import com.google.errorprone.annotations.FormatMethod; -import com.google.protobuf.Any; -import com.google.protobuf.ByteString; -import com.google.protobuf.Descriptors; -import com.google.protobuf.DynamicMessage; -import com.google.protobuf.ExtensionRegistry; -import com.google.protobuf.InvalidProtocolBufferException; import java.util.HashMap; import java.util.List; import java.util.Map; @@ -57,7 +51,17 @@ static TestConformanceResponse testConformance(TestConformanceRequest request) { try { Map descriptorMap = FileDescriptorUtil.parse(request.getFdset()); - Validator validator = new Validator(Config.newBuilder().build()); + Map fileDescriptorMap = + FileDescriptorUtil.parseFileDescriptors(request.getFdset()); + TypeRegistry typeRegistry = FileDescriptorUtil.createTypeRegistry(fileDescriptorMap.values()); + ExtensionRegistry extensionRegistry = + FileDescriptorUtil.createExtensionRegistry(fileDescriptorMap.values()); + Validator validator = + new Validator( + Config.newBuilder() + .setTypeRegistry(typeRegistry) + .setExtensionRegistry(extensionRegistry) + .build()); TestConformanceResponse.Builder responseBuilder = TestConformanceResponse.newBuilder(); Map resultsMap = new HashMap<>(); for (Map.Entry entry : request.getCasesMap().entrySet()) { diff --git a/src/main/java/build/buf/protovalidate/Config.java b/src/main/java/build/buf/protovalidate/Config.java index b4fa1aeb..0f0e2440 100644 --- a/src/main/java/build/buf/protovalidate/Config.java +++ b/src/main/java/build/buf/protovalidate/Config.java @@ -14,14 +14,36 @@ package build.buf.protovalidate; +import build.buf.validate.ValidateProto; +import com.google.protobuf.ExtensionRegistry; +import com.google.protobuf.TypeRegistry; + /** Config is the configuration for a Validator. */ public final class Config { + private static final TypeRegistry DEFAULT_TYPE_REGISTRY = TypeRegistry.getEmptyTypeRegistry(); + private static final ExtensionRegistry DEFAULT_EXTENSION_REGISTRY = + ExtensionRegistry.newInstance(); + + static { + DEFAULT_EXTENSION_REGISTRY.add(ValidateProto.message); + DEFAULT_EXTENSION_REGISTRY.add(ValidateProto.field); + DEFAULT_EXTENSION_REGISTRY.add(ValidateProto.oneof); + } + private final boolean failFast; private final boolean disableLazy; + private final TypeRegistry typeRegistry; + private final ExtensionRegistry extensionRegistry; - private Config(boolean failFast, boolean disableLazy) { + private Config( + boolean failFast, + boolean disableLazy, + TypeRegistry typeRegistry, + ExtensionRegistry extensionRegistry) { this.failFast = failFast; this.disableLazy = disableLazy; + this.typeRegistry = typeRegistry; + this.extensionRegistry = extensionRegistry; } /** @@ -51,10 +73,30 @@ public boolean isDisableLazy() { return disableLazy; } + /** + * Gets the registry used for resolving unknown protobuf fields and messages. + * + * @return a type registry + */ + public TypeRegistry getTypeRegistry() { + return typeRegistry; + } + + /** + * Gets the registry used for resolving unknown protobuf extensions. + * + * @return an extension registry + */ + public ExtensionRegistry getExtensionRegistry() { + return extensionRegistry; + } + /** Builder for configuration. Provides a forward compatible API for users. */ public static final class Builder { private boolean failFast; private boolean disableLazy; + private TypeRegistry typeRegistry = DEFAULT_TYPE_REGISTRY; + private ExtensionRegistry extensionRegistry = DEFAULT_EXTENSION_REGISTRY; private Builder() {} @@ -80,13 +122,35 @@ public Builder setDisableLazy(boolean disableLazy) { return this; } + /** + * Set the type registry for resolving unknown messages. + * + * @param typeRegistry the type registry to use + * @return this builder + */ + public Builder setTypeRegistry(TypeRegistry typeRegistry) { + this.typeRegistry = typeRegistry; + return this; + } + + /** + * Set the extension registry for resolving unknown extensions. + * + * @param extensionRegistry the extension registry to use + * @return this builder + */ + public Builder setExtensionRegistry(ExtensionRegistry extensionRegistry) { + this.extensionRegistry = extensionRegistry; + return this; + } + /** * Build the corresponding {@link Config}. * * @return the configuration. */ public Config build() { - return new Config(failFast, disableLazy); + return new Config(failFast, disableLazy, typeRegistry, extensionRegistry); } } } diff --git a/src/main/java/build/buf/protovalidate/Validator.java b/src/main/java/build/buf/protovalidate/Validator.java index 979d686f..1c197ac3 100644 --- a/src/main/java/build/buf/protovalidate/Validator.java +++ b/src/main/java/build/buf/protovalidate/Validator.java @@ -43,7 +43,9 @@ public class Validator { */ public Validator(Config config) { Env env = Env.newEnv(Library.Lib(new ValidateLibrary())); - this.evaluatorBuilder = new EvaluatorBuilder(env, config.isDisableLazy()); + this.evaluatorBuilder = + new EvaluatorBuilder( + env, config.isDisableLazy(), config.getTypeRegistry(), config.getExtensionRegistry()); this.failFast = config.isFailFast(); } @@ -51,7 +53,9 @@ public Validator(Config config) { public Validator() { Config config = Config.newBuilder().build(); Env env = Env.newEnv(Library.Lib(new ValidateLibrary())); - this.evaluatorBuilder = new EvaluatorBuilder(env, config.isDisableLazy()); + this.evaluatorBuilder = + new EvaluatorBuilder( + env, config.isDisableLazy(), config.getTypeRegistry(), config.getExtensionRegistry()); this.failFast = config.isFailFast(); } diff --git a/src/main/java/build/buf/protovalidate/internal/constraints/ConstraintCache.java b/src/main/java/build/buf/protovalidate/internal/constraints/ConstraintCache.java index 40fa96bd..e4806229 100644 --- a/src/main/java/build/buf/protovalidate/internal/constraints/ConstraintCache.java +++ b/src/main/java/build/buf/protovalidate/internal/constraints/ConstraintCache.java @@ -22,7 +22,6 @@ import build.buf.validate.FieldConstraints; import build.buf.validate.priv.PrivateProto; import com.google.protobuf.Descriptors.FieldDescriptor; -import com.google.protobuf.Message; import java.util.ArrayList; import java.util.Collections; import java.util.List; @@ -41,6 +40,16 @@ /** A build-through cache for computed standard constraints. */ public class ConstraintCache { + private static class CelRule { + public final AstExpression astExpression; + public final FieldDescriptor field; + + public CelRule(AstExpression astExpression, FieldDescriptor field) { + this.astExpression = astExpression; + this.field = field; + } + } + /** Partial eval options for evaluating the constraint's expression. */ private static final ProgramOption PARTIAL_EVAL_OPTIONS = ProgramOption.evalOptions( @@ -53,19 +62,30 @@ public class ConstraintCache { * Concurrent map for caching {@link FieldDescriptor} and their associated List of {@link * AstExpression}. */ - private static final Map> descriptorMap = + private static final Map> descriptorMap = new ConcurrentHashMap<>(); /** The environment to use for evaluation. */ private final Env env; + /** Registry used to resolve dynamic messages. */ + private final TypeRegistry typeRegistry; + + /** Registry used to resolve dynamic extensions. */ + private final ExtensionRegistry extensionRegistry; + /** - * Constructs a new build-through cache for the standard constraints. + * Constructs a new build-through cache for the standard constraints, with a provided registry to + * resolve dynamic extensions. * * @param env The CEL environment for evaluation. + * @param typeRegistry A type registry to resolve messages. + * @param extensionRegistry An extension registry to resolve extensions. */ - public ConstraintCache(Env env) { + public ConstraintCache(Env env, TypeRegistry typeRegistry, ExtensionRegistry extensionRegistry) { this.env = env; + this.typeRegistry = typeRegistry; + this.extensionRegistry = extensionRegistry; } /** @@ -86,36 +106,21 @@ public List compile( // Message null means there were no constraints resolved. return Collections.emptyList(); } - Env finalEnv = - env.extend( - EnvOption.types(message.getDefaultInstanceForType()), - EnvOption.declarations( - Decls.newVar( - Variable.THIS_NAME, DescriptorMappings.getCELType(fieldDescriptor, forItems)), - Decls.newVar( - Variable.RULES_NAME, - Decls.newObjectType(message.getDescriptorForType().getFullName())))); - ProgramOption rulesOption = ProgramOption.globals(Variable.newRulesVariable(message)); - List completeProgramList = new ArrayList<>(); + List completeProgramList = new ArrayList<>(); for (Map.Entry entry : message.getAllFields().entrySet()) { FieldDescriptor constraintFieldDesc = entry.getKey(); - if (!descriptorMap.containsKey(constraintFieldDesc)) { - build.buf.validate.priv.FieldConstraints constraints = - constraintFieldDesc.getOptions().getExtension(PrivateProto.field); - List expressions = Expression.fromPrivConstraints(constraints.getCelList()); - List astExpressions = new ArrayList<>(); - for (Expression expression : expressions) { - astExpressions.add(AstExpression.newAstExpression(finalEnv, expression)); - } - descriptorMap.put(constraintFieldDesc, astExpressions); - } - List programList = descriptorMap.get(constraintFieldDesc); + List programList = + compileRule(fieldDescriptor, forItems, constraintFieldDesc, message); + if (programList == null) continue; completeProgramList.addAll(programList); } List programs = new ArrayList<>(); - for (AstExpression astExpression : completeProgramList) { + for (CelRule rule : completeProgramList) { + Env ruleEnv = getRuleEnv(fieldDescriptor, message, rule.field, forItems); + Variable ruleVar = Variable.newRuleVariable(message, message.getField(rule.field)); + ProgramOption globals = ProgramOption.globals(ruleVar); try { - Program program = finalEnv.program(astExpression.ast, rulesOption, PARTIAL_EVAL_OPTIONS); + Program program = ruleEnv.program(rule.astExpression.ast, globals, PARTIAL_EVAL_OPTIONS); Program.EvalResult evalResult = program.eval(Activation.emptyActivation()); Val value = evalResult.getVal(); if (value != null) { @@ -127,18 +132,102 @@ public List compile( continue; } } - Ast residual = finalEnv.residualAst(astExpression.ast, evalResult.getEvalDetails()); + Ast residual = ruleEnv.residualAst(rule.astExpression.ast, evalResult.getEvalDetails()); programs.add( - new CompiledProgram(finalEnv.program(residual, rulesOption), astExpression.source)); + new CompiledProgram(ruleEnv.program(residual, globals), rule.astExpression.source)); } catch (Exception e) { programs.add( new CompiledProgram( - finalEnv.program(astExpression.ast, rulesOption), astExpression.source)); + ruleEnv.program(rule.astExpression.ast, globals), rule.astExpression.source)); } } return Collections.unmodifiableList(programs); } + private @Nullable List compileRule( + FieldDescriptor fieldDescriptor, + boolean forItems, + FieldDescriptor constraintFieldDesc, + Message message) + throws CompilationException { + if (descriptorMap.containsKey(constraintFieldDesc)) { + return descriptorMap.get(constraintFieldDesc); + } + build.buf.validate.priv.FieldConstraints constraints = getFieldConstraints(constraintFieldDesc); + if (constraints == null) return null; + List expressions = Expression.fromPrivConstraints(constraints.getCelList()); + List celRules = new ArrayList<>(); + Env ruleEnv = getRuleEnv(fieldDescriptor, message, constraintFieldDesc, forItems); + for (Expression expression : expressions) { + celRules.add( + new CelRule(AstExpression.newAstExpression(ruleEnv, expression), constraintFieldDesc)); + } + descriptorMap.put(constraintFieldDesc, celRules); + return celRules; + } + + private @Nullable build.buf.validate.priv.FieldConstraints getFieldConstraints( + FieldDescriptor constraintFieldDesc) throws CompilationException { + DescriptorProtos.FieldOptions options = constraintFieldDesc.getOptions(); + // If the protovalidate field option is unknown, reparse using extension registry. + if (options.getUnknownFields().hasField(PrivateProto.field.getNumber())) { + try { + ExtensionRegistry registry = ExtensionRegistry.newInstance(); + registry.add(PrivateProto.field); + options = DescriptorProtos.FieldOptions.parseFrom(options.toByteString(), registry); + } catch (InvalidProtocolBufferException e) { + throw new CompilationException("Failed to parse field options", e); + } + } + if (!options.hasExtension(PrivateProto.field)) { + return null; + } + Object extensionValue = options.getField(PrivateProto.field.getDescriptor()); + build.buf.validate.priv.FieldConstraints constraints; + if (extensionValue instanceof build.buf.validate.priv.FieldConstraints) { + constraints = (build.buf.validate.priv.FieldConstraints) extensionValue; + } else if (extensionValue instanceof MessageLite) { + // Extension is parsed but with different gencode. We need to reparse it. + try { + constraints = + build.buf.validate.priv.FieldConstraints.parseFrom( + ((MessageLite) extensionValue).toByteString(), extensionRegistry); + } catch (InvalidProtocolBufferException e) { + throw new CompilationException("Failed to parse field constraints", e); + } + } else { + // Extension was not a message, just discard it. + return null; + } + return constraints; + } + + /** + * Calculates the environment for a specific rule invocation. + * + * @param fieldDescriptor The field descriptor of the field with the constraint. + * @param constraintMessage The message of the standard constraints. + * @param constraintFieldDesc The field descriptor of the constraint. + * @param forItems Whether the field is a list type or not. + * @return An environment with requisite declarations and types added. + */ + private Env getRuleEnv( + FieldDescriptor fieldDescriptor, + Message constraintMessage, + FieldDescriptor constraintFieldDesc, + boolean forItems) { + return env.extend( + EnvOption.types(constraintMessage.getDefaultInstanceForType()), + EnvOption.declarations( + Decls.newVar( + Variable.THIS_NAME, DescriptorMappings.getCELType(fieldDescriptor, forItems)), + Decls.newVar( + Variable.RULES_NAME, + Decls.newObjectType(constraintMessage.getDescriptorForType().getFullName())), + Decls.newVar( + Variable.RULE_NAME, DescriptorMappings.getCELType(constraintFieldDesc, forItems)))); + } + /** * Extracts the standard constraints for the specified field. An exception is thrown if the wrong * constraints are applied to a field (typically if there is a type-mismatch). Null is returned if @@ -179,8 +268,25 @@ private Message resolveConstraints( return null; } - // Return the field from the field constraints identified by the oneof field descriptor, casted + // Get the field from the field constraints identified by the oneof field descriptor, casted // as a Message. - return (Message) fieldConstraints.getField(oneofFieldDescriptor); + Message typeConstraints = (Message) fieldConstraints.getField(oneofFieldDescriptor); + if (!typeConstraints.getUnknownFields().isEmpty()) { + // If there are unknown fields, try to resolve them using the provided TypeRegistry. + Descriptors.Descriptor expectedConstraintDynamicDescriptor = + typeRegistry.find(expectedConstraintDescriptor.getMessageType().getFullName()); + if (expectedConstraintDynamicDescriptor != null) { + try { + typeConstraints = + DynamicMessage.parseFrom( + expectedConstraintDynamicDescriptor, + typeConstraints.toByteString(), + extensionRegistry); + } catch (InvalidProtocolBufferException e) { + throw new RuntimeException(e); + } + } + } + return typeConstraints; } } diff --git a/src/main/java/build/buf/protovalidate/internal/evaluator/ConstraintResolver.java b/src/main/java/build/buf/protovalidate/internal/evaluator/ConstraintResolver.java index 282e065a..f37e7e1a 100644 --- a/src/main/java/build/buf/protovalidate/internal/evaluator/ConstraintResolver.java +++ b/src/main/java/build/buf/protovalidate/internal/evaluator/ConstraintResolver.java @@ -36,11 +36,13 @@ class ConstraintResolver { * @param desc the message descriptor. * @return the resolved {@link MessageConstraints}. */ - MessageConstraints resolveMessageConstraints(Descriptor desc, ExtensionRegistry registry) + MessageConstraints resolveMessageConstraints(Descriptor desc) throws InvalidProtocolBufferException, CompilationException { DescriptorProtos.MessageOptions options = desc.getOptions(); // If the protovalidate message extension is unknown, reparse using extension registry. if (options.getUnknownFields().hasField(ValidateProto.message.getNumber())) { + ExtensionRegistry registry = ExtensionRegistry.newInstance(); + registry.add(ValidateProto.message); options = DescriptorProtos.MessageOptions.parseFrom(options.toByteString(), registry); } if (!options.hasExtension(ValidateProto.message)) { @@ -66,11 +68,13 @@ MessageConstraints resolveMessageConstraints(Descriptor desc, ExtensionRegistry * @param desc the oneof descriptor. * @return the resolved {@link OneofConstraints}. */ - OneofConstraints resolveOneofConstraints(OneofDescriptor desc, ExtensionRegistry registry) + OneofConstraints resolveOneofConstraints(OneofDescriptor desc) throws InvalidProtocolBufferException, CompilationException { DescriptorProtos.OneofOptions options = desc.getOptions(); // If the protovalidate oneof extension is unknown, reparse using extension registry. if (options.getUnknownFields().hasField(ValidateProto.oneof.getNumber())) { + ExtensionRegistry registry = ExtensionRegistry.newInstance(); + registry.add(ValidateProto.oneof); options = DescriptorProtos.OneofOptions.parseFrom(options.toByteString(), registry); } if (!options.hasExtension(ValidateProto.oneof)) { @@ -96,11 +100,13 @@ OneofConstraints resolveOneofConstraints(OneofDescriptor desc, ExtensionRegistry * @param desc the field descriptor. * @return the resolved {@link FieldConstraints}. */ - FieldConstraints resolveFieldConstraints(FieldDescriptor desc, ExtensionRegistry registry) + FieldConstraints resolveFieldConstraints(FieldDescriptor desc) throws InvalidProtocolBufferException, CompilationException { DescriptorProtos.FieldOptions options = desc.getOptions(); // If the protovalidate field option is unknown, reparse using extension registry. if (options.getUnknownFields().hasField(ValidateProto.field.getNumber())) { + ExtensionRegistry registry = ExtensionRegistry.newInstance(); + registry.add(ValidateProto.field); options = DescriptorProtos.FieldOptions.parseFrom(options.toByteString(), registry); } if (!options.hasExtension(ValidateProto.field)) { diff --git a/src/main/java/build/buf/protovalidate/internal/evaluator/EvaluatorBuilder.java b/src/main/java/build/buf/protovalidate/internal/evaluator/EvaluatorBuilder.java index 30da6527..81b4b046 100644 --- a/src/main/java/build/buf/protovalidate/internal/evaluator/EvaluatorBuilder.java +++ b/src/main/java/build/buf/protovalidate/internal/evaluator/EvaluatorBuilder.java @@ -27,16 +27,9 @@ import build.buf.validate.Ignore; import build.buf.validate.MessageConstraints; import build.buf.validate.OneofConstraints; -import build.buf.validate.ValidateProto; import com.google.common.collect.ImmutableMap; -import com.google.protobuf.ByteString; -import com.google.protobuf.Descriptors; import com.google.protobuf.Descriptors.Descriptor; import com.google.protobuf.Descriptors.FieldDescriptor; -import com.google.protobuf.DynamicMessage; -import com.google.protobuf.ExtensionRegistry; -import com.google.protobuf.InvalidProtocolBufferException; -import com.google.protobuf.Message; import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; @@ -49,19 +42,12 @@ /** A build-through cache of message evaluators keyed off the provided descriptor. */ public class EvaluatorBuilder { - private static final ExtensionRegistry EXTENSION_REGISTRY = ExtensionRegistry.newInstance(); - - static { - EXTENSION_REGISTRY.add(ValidateProto.message); - EXTENSION_REGISTRY.add(ValidateProto.field); - EXTENSION_REGISTRY.add(ValidateProto.oneof); - } - private volatile ImmutableMap evaluatorCache = ImmutableMap.of(); private final Env env; private final boolean disableLazy; private final ConstraintCache constraints; + private final ExtensionRegistry extensionRegistry; /** * Constructs a new {@link EvaluatorBuilder}. @@ -69,10 +55,15 @@ public class EvaluatorBuilder { * @param env The CEL environment for evaluation. * @param disableLazy Determines whether lazy loading of evaluators is disabled. */ - public EvaluatorBuilder(Env env, boolean disableLazy) { + public EvaluatorBuilder( + Env env, + boolean disableLazy, + TypeRegistry typeRegistry, + ExtensionRegistry extensionRegistry) { this.env = env; this.disableLazy = disableLazy; - this.constraints = new ConstraintCache(env); + this.constraints = new ConstraintCache(env, typeRegistry, extensionRegistry); + this.extensionRegistry = extensionRegistry; } /** @@ -109,7 +100,8 @@ private Evaluator build(Descriptor desc) throws CompilationException { } // Rebuild cache with this descriptor (and any of its dependencies). ImmutableMap updatedCache = - new DescriptorCacheBuilder(env, constraints, evaluatorCache).build(desc); + new DescriptorCacheBuilder(env, constraints, evaluatorCache, extensionRegistry) + .build(desc); evaluatorCache = updatedCache; eval = updatedCache.get(desc); if (eval == null) { @@ -125,14 +117,17 @@ private static class DescriptorCacheBuilder { private final Env env; private final ConstraintCache constraintCache; private final HashMap cache; + private final ExtensionRegistry extensionRegistry; private DescriptorCacheBuilder( Env env, ConstraintCache constraintCache, - ImmutableMap previousCache) { + ImmutableMap previousCache, + ExtensionRegistry extensionRegistry) { this.env = Objects.requireNonNull(env, "env"); this.constraintCache = Objects.requireNonNull(constraintCache, "constraintCache"); this.cache = new HashMap<>(previousCache); + this.extensionRegistry = Objects.requireNonNull(extensionRegistry, "extensionRegistry"); } /** @@ -165,11 +160,10 @@ private void buildMessage(Descriptor desc, MessageEvaluator msgEval) try { DynamicMessage defaultInstance = DynamicMessage.newBuilder(desc) - .mergeFrom(new byte[0], EXTENSION_REGISTRY) + .mergeFrom(new byte[0], extensionRegistry) .buildPartial(); Descriptor descriptor = defaultInstance.getDescriptorForType(); - MessageConstraints msgConstraints = - resolver.resolveMessageConstraints(descriptor, EXTENSION_REGISTRY); + MessageConstraints msgConstraints = resolver.resolveMessageConstraints(descriptor); if (msgConstraints.getDisabled()) { return; } @@ -208,8 +202,7 @@ private void processOneofConstraints(Descriptor desc, MessageEvaluator msgEval) throws InvalidProtocolBufferException, CompilationException { List oneofs = desc.getOneofs(); for (Descriptors.OneofDescriptor oneofDesc : oneofs) { - OneofConstraints oneofConstraints = - resolver.resolveOneofConstraints(oneofDesc, EXTENSION_REGISTRY); + OneofConstraints oneofConstraints = resolver.resolveOneofConstraints(oneofDesc); OneofEvaluator oneofEvaluatorEval = new OneofEvaluator(oneofDesc, oneofConstraints.getRequired()); msgEval.append(oneofEvaluatorEval); @@ -221,8 +214,7 @@ private void processFields(Descriptor desc, MessageEvaluator msgEval) List fields = desc.getFields(); for (FieldDescriptor fieldDescriptor : fields) { FieldDescriptor descriptor = desc.findFieldByName(fieldDescriptor.getName()); - FieldConstraints fieldConstraints = - resolver.resolveFieldConstraints(descriptor, EXTENSION_REGISTRY); + FieldConstraints fieldConstraints = resolver.resolveFieldConstraints(descriptor); FieldEvaluator fldEval = buildField(descriptor, fieldConstraints); msgEval.append(fldEval); } @@ -341,7 +333,7 @@ private Object zeroValue(FieldDescriptor fieldDescriptor, boolean forItems) private Message createMessageForType(Descriptor messageType) throws CompilationException { try { - return DynamicMessage.parseFrom(messageType, new byte[0], EXTENSION_REGISTRY); + return DynamicMessage.parseFrom(messageType, new byte[0], extensionRegistry); } catch (InvalidProtocolBufferException e) { throw new CompilationException("field descriptor type is invalid " + e.getMessage(), e); } @@ -361,7 +353,7 @@ private void processFieldExpressions( try { DynamicMessage defaultInstance = DynamicMessage.parseFrom( - fieldDescriptor.getMessageType(), new byte[0], EXTENSION_REGISTRY); + fieldDescriptor.getMessageType(), new byte[0], extensionRegistry); opts = Arrays.asList( EnvOption.types(defaultInstance), diff --git a/src/main/java/build/buf/protovalidate/internal/expression/Variable.java b/src/main/java/build/buf/protovalidate/internal/expression/Variable.java index bb743199..3da33ae8 100644 --- a/src/main/java/build/buf/protovalidate/internal/expression/Variable.java +++ b/src/main/java/build/buf/protovalidate/internal/expression/Variable.java @@ -29,6 +29,9 @@ public class Variable implements Activation { /** The {@value} variable in CEL. */ public static final String RULES_NAME = "rules"; + /** The {@value} variable in CEL. */ + public static final String RULE_NAME = "rule"; + /** The parent activation */ private final Activation next; @@ -65,6 +68,16 @@ public static Variable newRulesVariable(Object val) { return new Variable(new NowVariable(), RULES_NAME, val); } + /** + * Creates a "rule" variable. + * + * @param val the value. + * @return {@link Variable}. + */ + public static Variable newRuleVariable(Object rules, Object val) { + return new Variable(newRulesVariable(rules), RULE_NAME, val); + } + @Override public ResolvedValue resolveName(String name) { if (this.name.equals(name)) { From 793ae90f988e055b4578430977d8064c027c3fd2 Mon Sep 17 00:00:00 2001 From: John Chadwick Date: Fri, 6 Sep 2024 08:22:01 -0400 Subject: [PATCH 2/4] Fix things spotlessApply borked --- .../buf/protovalidate/conformance/FileDescriptorUtil.java | 5 +++++ .../java/build/buf/protovalidate/conformance/Main.java | 7 +++++++ .../internal/constraints/ConstraintCache.java | 8 ++++++++ .../internal/evaluator/EvaluatorBuilder.java | 7 +++++++ 4 files changed, 27 insertions(+) diff --git a/conformance/src/main/java/build/buf/protovalidate/conformance/FileDescriptorUtil.java b/conformance/src/main/java/build/buf/protovalidate/conformance/FileDescriptorUtil.java index 295e119b..5af9af8c 100644 --- a/conformance/src/main/java/build/buf/protovalidate/conformance/FileDescriptorUtil.java +++ b/conformance/src/main/java/build/buf/protovalidate/conformance/FileDescriptorUtil.java @@ -14,6 +14,11 @@ package build.buf.protovalidate.conformance; +import com.google.protobuf.DescriptorProtos; +import com.google.protobuf.Descriptors; +import com.google.protobuf.DynamicMessage; +import com.google.protobuf.ExtensionRegistry; +import com.google.protobuf.TypeRegistry; import java.util.ArrayList; import java.util.HashMap; import java.util.List; diff --git a/conformance/src/main/java/build/buf/protovalidate/conformance/Main.java b/conformance/src/main/java/build/buf/protovalidate/conformance/Main.java index e369f94f..474a701a 100644 --- a/conformance/src/main/java/build/buf/protovalidate/conformance/Main.java +++ b/conformance/src/main/java/build/buf/protovalidate/conformance/Main.java @@ -27,6 +27,13 @@ import build.buf.validate.conformance.harness.TestResult; import com.google.common.base.Splitter; import com.google.errorprone.annotations.FormatMethod; +import com.google.protobuf.Any; +import com.google.protobuf.ByteString; +import com.google.protobuf.Descriptors; +import com.google.protobuf.DynamicMessage; +import com.google.protobuf.ExtensionRegistry; +import com.google.protobuf.InvalidProtocolBufferException; +import com.google.protobuf.TypeRegistry; import java.util.HashMap; import java.util.List; import java.util.Map; diff --git a/src/main/java/build/buf/protovalidate/internal/constraints/ConstraintCache.java b/src/main/java/build/buf/protovalidate/internal/constraints/ConstraintCache.java index e4806229..6e2eb4dd 100644 --- a/src/main/java/build/buf/protovalidate/internal/constraints/ConstraintCache.java +++ b/src/main/java/build/buf/protovalidate/internal/constraints/ConstraintCache.java @@ -21,7 +21,15 @@ import build.buf.protovalidate.internal.expression.Variable; import build.buf.validate.FieldConstraints; import build.buf.validate.priv.PrivateProto; +import com.google.protobuf.DescriptorProtos; +import com.google.protobuf.Descriptors; import com.google.protobuf.Descriptors.FieldDescriptor; +import com.google.protobuf.DynamicMessage; +import com.google.protobuf.ExtensionRegistry; +import com.google.protobuf.InvalidProtocolBufferException; +import com.google.protobuf.Message; +import com.google.protobuf.MessageLite; +import com.google.protobuf.TypeRegistry; import java.util.ArrayList; import java.util.Collections; import java.util.List; diff --git a/src/main/java/build/buf/protovalidate/internal/evaluator/EvaluatorBuilder.java b/src/main/java/build/buf/protovalidate/internal/evaluator/EvaluatorBuilder.java index 81b4b046..4dfada52 100644 --- a/src/main/java/build/buf/protovalidate/internal/evaluator/EvaluatorBuilder.java +++ b/src/main/java/build/buf/protovalidate/internal/evaluator/EvaluatorBuilder.java @@ -28,8 +28,15 @@ import build.buf.validate.MessageConstraints; import build.buf.validate.OneofConstraints; import com.google.common.collect.ImmutableMap; +import com.google.protobuf.ByteString; +import com.google.protobuf.Descriptors; import com.google.protobuf.Descriptors.Descriptor; import com.google.protobuf.Descriptors.FieldDescriptor; +import com.google.protobuf.DynamicMessage; +import com.google.protobuf.ExtensionRegistry; +import com.google.protobuf.InvalidProtocolBufferException; +import com.google.protobuf.Message; +import com.google.protobuf.TypeRegistry; import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; From 0348196c53abe41a8b41972f9025a1675e4494ec Mon Sep 17 00:00:00 2001 From: John Chadwick Date: Fri, 6 Sep 2024 08:27:46 -0400 Subject: [PATCH 3/4] Clean up extension registry bits --- .../java/build/buf/protovalidate/Config.java | 7 ------- .../internal/constraints/ConstraintCache.java | 13 ++++++++---- .../evaluator/ConstraintResolver.java | 20 ++++++++++--------- 3 files changed, 20 insertions(+), 20 deletions(-) diff --git a/src/main/java/build/buf/protovalidate/Config.java b/src/main/java/build/buf/protovalidate/Config.java index 0f0e2440..c99b4f10 100644 --- a/src/main/java/build/buf/protovalidate/Config.java +++ b/src/main/java/build/buf/protovalidate/Config.java @@ -14,7 +14,6 @@ package build.buf.protovalidate; -import build.buf.validate.ValidateProto; import com.google.protobuf.ExtensionRegistry; import com.google.protobuf.TypeRegistry; @@ -24,12 +23,6 @@ public final class Config { private static final ExtensionRegistry DEFAULT_EXTENSION_REGISTRY = ExtensionRegistry.newInstance(); - static { - DEFAULT_EXTENSION_REGISTRY.add(ValidateProto.message); - DEFAULT_EXTENSION_REGISTRY.add(ValidateProto.field); - DEFAULT_EXTENSION_REGISTRY.add(ValidateProto.oneof); - } - private final boolean failFast; private final boolean disableLazy; private final TypeRegistry typeRegistry; diff --git a/src/main/java/build/buf/protovalidate/internal/constraints/ConstraintCache.java b/src/main/java/build/buf/protovalidate/internal/constraints/ConstraintCache.java index 6e2eb4dd..5c39df24 100644 --- a/src/main/java/build/buf/protovalidate/internal/constraints/ConstraintCache.java +++ b/src/main/java/build/buf/protovalidate/internal/constraints/ConstraintCache.java @@ -58,6 +58,12 @@ public CelRule(AstExpression astExpression, FieldDescriptor field) { } } + private static final ExtensionRegistry EXTENSION_REGISTRY = ExtensionRegistry.newInstance(); + + static { + EXTENSION_REGISTRY.add(PrivateProto.field); + } + /** Partial eval options for evaluating the constraint's expression. */ private static final ProgramOption PARTIAL_EVAL_OPTIONS = ProgramOption.evalOptions( @@ -177,12 +183,11 @@ public List compile( private @Nullable build.buf.validate.priv.FieldConstraints getFieldConstraints( FieldDescriptor constraintFieldDesc) throws CompilationException { DescriptorProtos.FieldOptions options = constraintFieldDesc.getOptions(); - // If the protovalidate field option is unknown, reparse using extension registry. + // If the protovalidate field option is unknown, reparse options using our extension registry. if (options.getUnknownFields().hasField(PrivateProto.field.getNumber())) { try { - ExtensionRegistry registry = ExtensionRegistry.newInstance(); - registry.add(PrivateProto.field); - options = DescriptorProtos.FieldOptions.parseFrom(options.toByteString(), registry); + options = + DescriptorProtos.FieldOptions.parseFrom(options.toByteString(), EXTENSION_REGISTRY); } catch (InvalidProtocolBufferException e) { throw new CompilationException("Failed to parse field options", e); } diff --git a/src/main/java/build/buf/protovalidate/internal/evaluator/ConstraintResolver.java b/src/main/java/build/buf/protovalidate/internal/evaluator/ConstraintResolver.java index f37e7e1a..b94cfb8e 100644 --- a/src/main/java/build/buf/protovalidate/internal/evaluator/ConstraintResolver.java +++ b/src/main/java/build/buf/protovalidate/internal/evaluator/ConstraintResolver.java @@ -29,6 +29,13 @@ /** Manages the resolution of protovalidate constraints. */ class ConstraintResolver { + private static final ExtensionRegistry EXTENSION_REGISTRY = ExtensionRegistry.newInstance(); + + static { + EXTENSION_REGISTRY.add(ValidateProto.message); + EXTENSION_REGISTRY.add(ValidateProto.oneof); + EXTENSION_REGISTRY.add(ValidateProto.field); + } /** * Resolves the constraints for a message descriptor. @@ -41,9 +48,8 @@ MessageConstraints resolveMessageConstraints(Descriptor desc) DescriptorProtos.MessageOptions options = desc.getOptions(); // If the protovalidate message extension is unknown, reparse using extension registry. if (options.getUnknownFields().hasField(ValidateProto.message.getNumber())) { - ExtensionRegistry registry = ExtensionRegistry.newInstance(); - registry.add(ValidateProto.message); - options = DescriptorProtos.MessageOptions.parseFrom(options.toByteString(), registry); + options = + DescriptorProtos.MessageOptions.parseFrom(options.toByteString(), EXTENSION_REGISTRY); } if (!options.hasExtension(ValidateProto.message)) { return MessageConstraints.getDefaultInstance(); @@ -73,9 +79,7 @@ OneofConstraints resolveOneofConstraints(OneofDescriptor desc) DescriptorProtos.OneofOptions options = desc.getOptions(); // If the protovalidate oneof extension is unknown, reparse using extension registry. if (options.getUnknownFields().hasField(ValidateProto.oneof.getNumber())) { - ExtensionRegistry registry = ExtensionRegistry.newInstance(); - registry.add(ValidateProto.oneof); - options = DescriptorProtos.OneofOptions.parseFrom(options.toByteString(), registry); + options = DescriptorProtos.OneofOptions.parseFrom(options.toByteString(), EXTENSION_REGISTRY); } if (!options.hasExtension(ValidateProto.oneof)) { return OneofConstraints.getDefaultInstance(); @@ -105,9 +109,7 @@ FieldConstraints resolveFieldConstraints(FieldDescriptor desc) DescriptorProtos.FieldOptions options = desc.getOptions(); // If the protovalidate field option is unknown, reparse using extension registry. if (options.getUnknownFields().hasField(ValidateProto.field.getNumber())) { - ExtensionRegistry registry = ExtensionRegistry.newInstance(); - registry.add(ValidateProto.field); - options = DescriptorProtos.FieldOptions.parseFrom(options.toByteString(), registry); + options = DescriptorProtos.FieldOptions.parseFrom(options.toByteString(), EXTENSION_REGISTRY); } if (!options.hasExtension(ValidateProto.field)) { return FieldConstraints.getDefaultInstance(); From 55b6de6d46f93a5190091fb9ba3004f787c4dc6d Mon Sep 17 00:00:00 2001 From: John Chadwick Date: Fri, 6 Sep 2024 08:38:26 -0400 Subject: [PATCH 4/4] Fix javadoc errors --- .../buf/protovalidate/internal/evaluator/EvaluatorBuilder.java | 2 ++ .../build/buf/protovalidate/internal/expression/Variable.java | 3 ++- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/src/main/java/build/buf/protovalidate/internal/evaluator/EvaluatorBuilder.java b/src/main/java/build/buf/protovalidate/internal/evaluator/EvaluatorBuilder.java index 4dfada52..fd641ecd 100644 --- a/src/main/java/build/buf/protovalidate/internal/evaluator/EvaluatorBuilder.java +++ b/src/main/java/build/buf/protovalidate/internal/evaluator/EvaluatorBuilder.java @@ -61,6 +61,8 @@ public class EvaluatorBuilder { * * @param env The CEL environment for evaluation. * @param disableLazy Determines whether lazy loading of evaluators is disabled. + * @param typeRegistry Type registry used for resolving unknown messages. + * @param extensionRegistry Extension registry used for resolving unknown extensions. */ public EvaluatorBuilder( Env env, diff --git a/src/main/java/build/buf/protovalidate/internal/expression/Variable.java b/src/main/java/build/buf/protovalidate/internal/expression/Variable.java index 3da33ae8..53ca6051 100644 --- a/src/main/java/build/buf/protovalidate/internal/expression/Variable.java +++ b/src/main/java/build/buf/protovalidate/internal/expression/Variable.java @@ -71,7 +71,8 @@ public static Variable newRulesVariable(Object val) { /** * Creates a "rule" variable. * - * @param val the value. + * @param rules the value of the "rules" variable. + * @param val the value of the "rule" variable. * @return {@link Variable}. */ public static Variable newRuleVariable(Object rules, Object val) {