Skip to content

Commit fabdc93

Browse files
author
jchadwick-buf
authored
Implement structured field and rule paths for violations (#213)
Implements the changes needed to pass conformance after bufbuild/protovalidate#265. This is basically a straight port of what is done in bufbuild/protovalidate-go#154, owing to the similarities between protovalidate-go and protovalidate-java. Unfortunately this means it inherits some of the trickier maneuvers needed to weave the right data into the right places. This version is also not as efficient as a result of differences between Go and Java protobufs, but effort was made to try to prevent big regressions in the case of successful validation. Some of this is probably not as pretty as it could be. I'm open to improvements if there are any obvious things (I am certainly not a Java expert after all) but if possible I'd like to defer anything that would require major rethinking to down the road. (Happy to add TODOs for those, though.) DO NOT MERGE until the following is done: - [x] bufbuild/protovalidate#265 is merged - [x] Dependencies updated to stable version release (waiting on manage module push)
1 parent 54ba1f7 commit fabdc93

29 files changed

+5328
-295
lines changed

conformance/src/main/java/build/buf/validate/conformance/cases/custom_constraints/CustomConstraintsProto.java

Lines changed: 30 additions & 25 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

conformance/src/main/java/build/buf/validate/conformance/cases/custom_constraints/FieldExpressions.java

Lines changed: 66 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

conformance/src/main/java/build/buf/validate/conformance/cases/custom_constraints/FieldExpressionsOrBuilder.java

Lines changed: 6 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

gradle.properties

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
# Version of buf.build/bufbuild/protovalidate to use.
2-
protovalidate.version = v0.8.2
2+
protovalidate.version = v0.9.0
33

44
# Arguments to the protovalidate-conformance CLI
55
protovalidate.conformance.args = --strict --strict_message --strict_error

src/main/java/build/buf/protovalidate/Validator.java

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
import build.buf.protovalidate.exceptions.CompilationException;
1818
import build.buf.protovalidate.exceptions.ValidationException;
1919
import build.buf.protovalidate.internal.celext.ValidateLibrary;
20+
import build.buf.protovalidate.internal.errors.FieldPathUtils;
2021
import build.buf.protovalidate.internal.evaluator.Evaluator;
2122
import build.buf.protovalidate.internal.evaluator.EvaluatorBuilder;
2223
import build.buf.protovalidate.internal.evaluator.MessageValue;
@@ -74,7 +75,11 @@ public ValidationResult validate(Message msg) throws ValidationException {
7475
}
7576
Descriptor descriptor = msg.getDescriptorForType();
7677
Evaluator evaluator = evaluatorBuilder.load(descriptor);
77-
return evaluator.evaluate(new MessageValue(msg), failFast);
78+
ValidationResult result = evaluator.evaluate(new MessageValue(msg), failFast);
79+
if (result.isSuccess()) {
80+
return result;
81+
}
82+
return new ValidationResult(FieldPathUtils.calculateFieldPathStrings(result.getViolations()));
7883
}
7984

8085
/**

src/main/java/build/buf/protovalidate/internal/constraints/ConstraintCache.java

Lines changed: 34 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -16,11 +16,13 @@
1616

1717
import build.buf.protovalidate.Config;
1818
import build.buf.protovalidate.exceptions.CompilationException;
19+
import build.buf.protovalidate.internal.errors.FieldPathUtils;
1920
import build.buf.protovalidate.internal.expression.AstExpression;
2021
import build.buf.protovalidate.internal.expression.CompiledProgram;
2122
import build.buf.protovalidate.internal.expression.Expression;
2223
import build.buf.protovalidate.internal.expression.Variable;
2324
import build.buf.validate.FieldConstraints;
25+
import build.buf.validate.FieldPath;
2426
import build.buf.validate.ValidateProto;
2527
import com.google.protobuf.DescriptorProtos;
2628
import com.google.protobuf.Descriptors;
@@ -52,10 +54,12 @@ public class ConstraintCache {
5254
private static class CelRule {
5355
public final AstExpression astExpression;
5456
public final FieldDescriptor field;
57+
public final FieldPath rulePath;
5558

56-
public CelRule(AstExpression astExpression, FieldDescriptor field) {
59+
public CelRule(AstExpression astExpression, FieldDescriptor field, FieldPath rulePath) {
5760
this.astExpression = astExpression;
5861
this.field = field;
62+
this.rulePath = rulePath;
5963
}
6064
}
6165

@@ -119,16 +123,17 @@ public ConstraintCache(Env env, Config config) {
119123
public List<CompiledProgram> compile(
120124
FieldDescriptor fieldDescriptor, FieldConstraints fieldConstraints, boolean forItems)
121125
throws CompilationException {
122-
Message message = resolveConstraints(fieldDescriptor, fieldConstraints, forItems);
123-
if (message == null) {
126+
ResolvedConstraint resolved = resolveConstraints(fieldDescriptor, fieldConstraints, forItems);
127+
if (resolved == null) {
124128
// Message null means there were no constraints resolved.
125129
return Collections.emptyList();
126130
}
131+
Message message = resolved.message;
127132
List<CelRule> completeProgramList = new ArrayList<>();
128133
for (Map.Entry<FieldDescriptor, Object> entry : message.getAllFields().entrySet()) {
129134
FieldDescriptor constraintFieldDesc = entry.getKey();
130135
List<CelRule> programList =
131-
compileRule(fieldDescriptor, forItems, constraintFieldDesc, message);
136+
compileRule(fieldDescriptor, forItems, resolved.setOneof, constraintFieldDesc, message);
132137
if (programList == null) continue;
133138
completeProgramList.addAll(programList);
134139
}
@@ -152,11 +157,14 @@ public List<CompiledProgram> compile(
152157
}
153158
Ast residual = ruleEnv.residualAst(rule.astExpression.ast, evalResult.getEvalDetails());
154159
programs.add(
155-
new CompiledProgram(ruleEnv.program(residual, globals), rule.astExpression.source));
160+
new CompiledProgram(
161+
ruleEnv.program(residual, globals), rule.astExpression.source, rule.rulePath));
156162
} catch (Exception e) {
157163
programs.add(
158164
new CompiledProgram(
159-
ruleEnv.program(rule.astExpression.ast, globals), rule.astExpression.source));
165+
ruleEnv.program(rule.astExpression.ast, globals),
166+
rule.astExpression.source,
167+
rule.rulePath));
160168
}
161169
}
162170
return Collections.unmodifiableList(programs);
@@ -165,6 +173,7 @@ public List<CompiledProgram> compile(
165173
private @Nullable List<CelRule> compileRule(
166174
FieldDescriptor fieldDescriptor,
167175
boolean forItems,
176+
FieldDescriptor setOneof,
168177
FieldDescriptor constraintFieldDesc,
169178
Message message)
170179
throws CompilationException {
@@ -178,8 +187,14 @@ public List<CompiledProgram> compile(
178187
celRules = new ArrayList<>(expressions.size());
179188
Env ruleEnv = getRuleEnv(fieldDescriptor, message, constraintFieldDesc, forItems);
180189
for (Expression expression : expressions) {
190+
FieldPath rulePath =
191+
FieldPath.newBuilder()
192+
.addElements(FieldPathUtils.fieldPathElement(setOneof))
193+
.addElements(FieldPathUtils.fieldPathElement(constraintFieldDesc))
194+
.build();
181195
celRules.add(
182-
new CelRule(AstExpression.newAstExpression(ruleEnv, expression), constraintFieldDesc));
196+
new CelRule(
197+
AstExpression.newAstExpression(ruleEnv, expression), constraintFieldDesc, rulePath));
183198
}
184199
descriptorMap.put(constraintFieldDesc, celRules);
185200
return celRules;
@@ -246,13 +261,23 @@ private Env getRuleEnv(
246261
Variable.RULE_NAME, DescriptorMappings.getCELType(constraintFieldDesc, false))));
247262
}
248263

264+
private static class ResolvedConstraint {
265+
final Message message;
266+
final FieldDescriptor setOneof;
267+
268+
ResolvedConstraint(Message message, FieldDescriptor setOneof) {
269+
this.message = message;
270+
this.setOneof = setOneof;
271+
}
272+
}
273+
249274
/**
250275
* Extracts the standard constraints for the specified field. An exception is thrown if the wrong
251276
* constraints are applied to a field (typically if there is a type-mismatch). Null is returned if
252277
* there are no standard constraints to apply to this field.
253278
*/
254279
@Nullable
255-
private Message resolveConstraints(
280+
private ResolvedConstraint resolveConstraints(
256281
FieldDescriptor fieldDescriptor, FieldConstraints fieldConstraints, boolean forItems)
257282
throws CompilationException {
258283
// Get the oneof field descriptor from the field constraints.
@@ -313,6 +338,6 @@ private Message resolveConstraints(
313338
if (!allowUnknownFields && !typeConstraints.getUnknownFields().isEmpty()) {
314339
throw new CompilationException("unrecognized field constraints");
315340
}
316-
return typeConstraints;
341+
return new ResolvedConstraint(typeConstraints, oneofFieldDescriptor);
317342
}
318343
}

0 commit comments

Comments
 (0)