From 8c468636fb77ee273db0d2f80e7416ce119fdc5a Mon Sep 17 00:00:00 2001 From: Blaz Snuderl Date: Thu, 23 Apr 2026 09:30:00 +0200 Subject: [PATCH 1/2] Fix cache key mismatch in RuleCache.compileRule The descriptorMap lookup at the top of compileRule() used fieldDescriptor (the user's field), but the corresponding store at the bottom uses ruleFieldDesc (the rule field, e.g. StringRules.pattern). The read therefore always missed, and cached CelRules were never reused across different user fields that share the same predefined rule. Use ruleFieldDesc for the lookup so it matches the store. --- src/main/java/build/buf/protovalidate/RuleCache.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/build/buf/protovalidate/RuleCache.java b/src/main/java/build/buf/protovalidate/RuleCache.java index 44497dae..8cec0c0d 100644 --- a/src/main/java/build/buf/protovalidate/RuleCache.java +++ b/src/main/java/build/buf/protovalidate/RuleCache.java @@ -144,7 +144,7 @@ List compile( FieldDescriptor ruleFieldDesc, Message message) throws CompilationException { - List celRules = descriptorMap.get(fieldDescriptor); + List celRules = descriptorMap.get(ruleFieldDesc); if (celRules != null) { return celRules; } From dc0f5c8220567bea5aeefafa55abadc1bf22ab97 Mon Sep 17 00:00:00 2001 From: Blaz Snuderl Date: Thu, 23 Apr 2026 09:40:33 +0200 Subject: [PATCH 2/2] Cache compiled CEL programs alongside ASTs in RuleCache Match the protovalidate-go design where the rule cache stores the compiled program (not just the AST) per rule field descriptor, and per-field rule values are bound at call time. Before: compile() rebuilt the rule Cel environment and called createProgram(ast) on every invocation, even when the AST came from the cache. The only per-call input that actually differs between two fields sharing the same ruleFieldDesc is the rule value (e.g. min_len = 5 vs. min_len = 10), which is already bound as a CEL variable and not baked into the program. After: compileRule() builds the program once, alongside the AST, and stores both in descriptorMap keyed by ruleFieldDesc. compile() just wraps the cached program with per-call ObjectValue + rule variable. This mirrors loadOrCompileStandardRule in protovalidate-go (internal/constraints/cache.go), which caches per protoreflect.FieldDescriptor and binds rule values via WithRuleValues at evaluation time. No new cache was introduced. --- .../build/buf/protovalidate/RuleCache.java | 40 ++++++++++--------- 1 file changed, 22 insertions(+), 18 deletions(-) diff --git a/src/main/java/build/buf/protovalidate/RuleCache.java b/src/main/java/build/buf/protovalidate/RuleCache.java index 8cec0c0d..58dff7a1 100644 --- a/src/main/java/build/buf/protovalidate/RuleCache.java +++ b/src/main/java/build/buf/protovalidate/RuleCache.java @@ -30,6 +30,7 @@ import dev.cel.bundle.Cel; import dev.cel.common.types.StructTypeReference; import dev.cel.runtime.CelEvaluationException; +import dev.cel.runtime.CelRuntime.Program; import java.util.ArrayList; import java.util.Collections; import java.util.List; @@ -41,11 +42,14 @@ final class RuleCache { private static class CelRule { final AstExpression astExpression; + final Program program; final FieldDescriptor field; final FieldPath rulePath; - private CelRule(AstExpression astExpression, FieldDescriptor field, FieldPath rulePath) { + private CelRule( + AstExpression astExpression, Program program, FieldDescriptor field, FieldPath rulePath) { this.astExpression = astExpression; + this.program = program; this.field = field; this.rulePath = rulePath; } @@ -119,20 +123,14 @@ List compile( } List programs = new ArrayList<>(); for (CelRule rule : completeProgramList) { - Cel ruleCel = getRuleCel(fieldDescriptor, message, rule.field, forItems); - try { - programs.add( - new CompiledProgram( - ruleCel.createProgram(rule.astExpression.ast), - rule.astExpression.source, - rule.rulePath, - new ObjectValue(rule.field, message.getField(rule.field)), - Variable.newRuleVariable( - message, ProtoAdapter.toCel(rule.field, message.getField(rule.field))))); - } catch (CelEvaluationException e) { - throw new CompilationException( - "failed to evaluate rule " + rule.astExpression.source.id, e); - } + programs.add( + new CompiledProgram( + rule.program, + rule.astExpression.source, + rule.rulePath, + new ObjectValue(rule.field, message.getField(rule.field)), + Variable.newRuleVariable( + message, ProtoAdapter.toCel(rule.field, message.getField(rule.field))))); } return Collections.unmodifiableList(programs); } @@ -159,9 +157,15 @@ List compile( .addElements(FieldPathUtils.fieldPathElement(setOneof)) .addElements(FieldPathUtils.fieldPathElement(ruleFieldDesc)) .build(); - celRules.add( - new CelRule( - AstExpression.newAstExpression(ruleCel, expression), ruleFieldDesc, rulePath)); + AstExpression astExpression = AstExpression.newAstExpression(ruleCel, expression); + Program program; + try { + program = ruleCel.createProgram(astExpression.ast); + } catch (CelEvaluationException e) { + throw new CompilationException( + "failed to create program for rule " + astExpression.source.id, e); + } + celRules.add(new CelRule(astExpression, program, ruleFieldDesc, rulePath)); } descriptorMap.put(ruleFieldDesc, celRules); return celRules;