Skip to content

Commit af00824

Browse files
l46kokcopybara-github
l46kok
authored and
copybara-github
committed
Add a runtime CEL type resolution for an unknown value
PiperOrigin-RevId: 686325313
1 parent 6dc2f16 commit af00824

File tree

9 files changed

+192
-46
lines changed

9 files changed

+192
-46
lines changed

‎bundle/src/test/java/dev/cel/bundle/CelImplTest.java‎

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@
3535
import dev.cel.expr.Reference;
3636
import dev.cel.expr.Type;
3737
import dev.cel.expr.Type.PrimitiveType;
38+
import dev.cel.expr.Value;
3839
import com.google.api.expr.test.v1.proto2.Proto2ExtensionScopedMessage;
3940
import com.google.api.expr.test.v1.proto3.TestAllTypesProto.TestAllTypes;
4041
import com.google.common.collect.ImmutableList;
@@ -1926,6 +1927,54 @@ public void program_functionParamWithWellKnownType() throws Exception {
19261927
assertThat(result).isTrue();
19271928
}
19281929

1930+
@Test
1931+
public void program_nativeTypeUnknownsEnabled_asIdentifiers() throws Exception {
1932+
Cel cel =
1933+
CelFactory.standardCelBuilder()
1934+
.addVar("x", SimpleType.BOOL)
1935+
.addVar("y", SimpleType.BOOL)
1936+
.setOptions(CelOptions.current().adaptUnknownValueSetToNativeType(true).build())
1937+
.build();
1938+
CelAbstractSyntaxTree ast = cel.compile("x || y").getAst();
1939+
1940+
CelUnknownSet result = (CelUnknownSet) cel.createProgram(ast).eval();
1941+
1942+
assertThat(result.unknownExprIds()).containsExactly(1L, 3L);
1943+
assertThat(result.attributes()).isEmpty();
1944+
}
1945+
1946+
@Test
1947+
public void program_nativeTypeUnknownsEnabled_asCallArguments() throws Exception {
1948+
Cel cel =
1949+
CelFactory.standardCelBuilder()
1950+
.addVar("x", SimpleType.BOOL)
1951+
.addFunctionDeclarations(
1952+
newFunctionDeclaration(
1953+
"foo", newGlobalOverload("foo_bool", SimpleType.BOOL, SimpleType.BOOL)))
1954+
.setOptions(CelOptions.current().adaptUnknownValueSetToNativeType(true).build())
1955+
.build();
1956+
CelAbstractSyntaxTree ast = cel.compile("foo(x)").getAst();
1957+
1958+
CelUnknownSet result = (CelUnknownSet) cel.createProgram(ast).eval();
1959+
1960+
assertThat(result.unknownExprIds()).containsExactly(2L);
1961+
assertThat(result.attributes()).isEmpty();
1962+
}
1963+
1964+
@Test
1965+
public void program_nativeTypeUnknownsEnabled_resolveUnknownType() throws Exception {
1966+
Cel cel =
1967+
CelFactory.standardCelBuilder()
1968+
.addVar("x", SimpleType.BOOL)
1969+
.setOptions(CelOptions.current().adaptUnknownValueSetToNativeType(true).build())
1970+
.build();
1971+
CelAbstractSyntaxTree ast = cel.compile("type(x)").getAst();
1972+
1973+
Value result = (Value) cel.createProgram(ast).eval();
1974+
1975+
assertThat(result.getTypeValue()).isEqualTo("unknown");
1976+
}
1977+
19291978
@Test
19301979
public void toBuilder_isImmutable() {
19311980
CelBuilder celBuilder = CelFactory.standardCelBuilder();

‎common/src/main/java/dev/cel/common/CelOptions.java‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -109,6 +109,8 @@ public enum ProtoUnsetFieldOptions {
109109

110110
public abstract ProtoUnsetFieldOptions fromProtoUnsetFieldOption();
111111

112+
public abstract boolean adaptUnknownValueSetToNativeType();
113+
112114
public abstract Builder toBuilder();
113115

114116
public ImmutableSet<ExprFeatures> toExprFeatures() {
@@ -200,6 +202,7 @@ public static Builder newBuilder() {
200202
.enableCelValue(false)
201203
.comprehensionMaxIterations(-1)
202204
.unwrapWellKnownTypesOnFunctionDispatch(true)
205+
.adaptUnknownValueSetToNativeType(false)
203206
.fromProtoUnsetFieldOption(ProtoUnsetFieldOptions.BIND_DEFAULT);
204207
}
205208

@@ -504,6 +507,16 @@ public abstract static class Builder {
504507
*/
505508
public abstract Builder fromProtoUnsetFieldOption(ProtoUnsetFieldOptions value);
506509

510+
/**
511+
* If enabled, when the result of an evaluation is a set of unknown values, a native-type
512+
* equivalent {@code CelUnknownSet} is returned as a result. Otherwise, ExprValue from {@code
513+
* eval.proto} is returned instead.
514+
*
515+
* <p>This is a temporary flag. It will be removed once the consumers have migrated to the
516+
* native-type representation.
517+
*/
518+
public abstract Builder adaptUnknownValueSetToNativeType(boolean value);
519+
507520
public abstract CelOptions build();
508521
}
509522
}

‎runtime/src/main/java/dev/cel/runtime/BUILD.bazel‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ BASE_SOURCES = [
1919
"TypeResolver.java",
2020
]
2121

22+
# keep sorted
2223
INTERPRETER_SOURCES = [
2324
"Activation.java",
2425
"CallArgumentChecker.java",
@@ -44,6 +45,7 @@ java_library(
4445
],
4546
deps = [
4647
":runtime_helper",
48+
":unknown_attributes",
4749
"//:auto_value",
4850
"//common",
4951
"//common:error_codes",
@@ -249,6 +251,7 @@ java_library(
249251
],
250252
deps = [
251253
":base",
254+
":unknown_attributes",
252255
"//common/annotations",
253256
"@cel_spec//proto/cel/expr:expr_java_proto",
254257
"@maven//:com_google_errorprone_error_prone_annotations",

‎runtime/src/main/java/dev/cel/runtime/CallArgumentChecker.java‎

Lines changed: 18 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -32,13 +32,13 @@
3232
@Internal
3333
class CallArgumentChecker {
3434
private final ArrayList<Long> exprIds;
35-
private Optional<CelUnknownSet> unknowns;
3635
private final RuntimeUnknownResolver resolver;
3736
private final boolean acceptPartial;
37+
private Optional<CelUnknownSet> unknowns;
3838

3939
private CallArgumentChecker(RuntimeUnknownResolver resolver, boolean acceptPartial) {
40-
exprIds = new ArrayList<>();
41-
unknowns = Optional.empty();
40+
this.exprIds = new ArrayList<>();
41+
this.unknowns = Optional.empty();
4242
this.resolver = resolver;
4343
this.acceptPartial = acceptPartial;
4444
}
@@ -76,8 +76,13 @@ void checkArg(DefaultInterpreter.IntermediateResult arg) {
7676

7777
// support for ExprValue unknowns.
7878
if (InterpreterUtil.isUnknown(arg.value())) {
79-
ExprValue exprValue = (ExprValue) arg.value();
80-
exprIds.addAll(exprValue.getUnknown().getExprsList());
79+
if (InterpreterUtil.isExprValueUnknown(arg.value())) {
80+
ExprValue exprValue = (ExprValue) arg.value();
81+
exprIds.addAll(exprValue.getUnknown().getExprsList());
82+
} else if (resolver.getAdaptUnknownValueSetOption()) {
83+
CelUnknownSet unknownSet = (CelUnknownSet) arg.value();
84+
exprIds.addAll(unknownSet.unknownExprIds());
85+
}
8186
}
8287
}
8388

@@ -98,8 +103,14 @@ Optional<Object> maybeUnknowns() {
98103
}
99104

100105
if (!exprIds.isEmpty()) {
101-
return Optional.of(
102-
ExprValue.newBuilder().setUnknown(UnknownSet.newBuilder().addAllExprs(exprIds)).build());
106+
if (resolver.getAdaptUnknownValueSetOption()) {
107+
return Optional.of(CelUnknownSet.create(exprIds));
108+
} else {
109+
return Optional.of(
110+
ExprValue.newBuilder()
111+
.setUnknown(UnknownSet.newBuilder().addAllExprs(exprIds))
112+
.build());
113+
}
103114
}
104115

105116
return Optional.empty();

‎runtime/src/main/java/dev/cel/runtime/CelUnknownSet.java‎

Lines changed: 28 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -28,21 +28,46 @@
2828
*/
2929
@AutoValue
3030
public abstract class CelUnknownSet {
31+
32+
/**
33+
* Set of attributes with a series of selection or index operations marked unknown. This set is
34+
* always empty if enableUnknownTracking is disabled in {@code CelOptions}.
35+
*/
3136
public abstract ImmutableSet<CelAttribute> attributes();
3237

33-
public static CelUnknownSet create(ImmutableSet<CelAttribute> attributes) {
34-
return new AutoValue_CelUnknownSet(attributes);
35-
}
38+
/** Set of subexpression IDs that were decided to be unknown and in the critical path. */
39+
public abstract ImmutableSet<Long> unknownExprIds();
3640

3741
public static CelUnknownSet create(CelAttribute attribute) {
3842
return create(ImmutableSet.of(attribute));
3943
}
4044

45+
public static CelUnknownSet create(ImmutableSet<CelAttribute> attributes) {
46+
return create(attributes, ImmutableSet.of());
47+
}
48+
49+
public static CelUnknownSet create(Long... unknownExprIds) {
50+
return create(ImmutableSet.copyOf(unknownExprIds));
51+
}
52+
53+
static CelUnknownSet create(Iterable<Long> unknownExprIds) {
54+
return create(ImmutableSet.of(), ImmutableSet.copyOf(unknownExprIds));
55+
}
56+
57+
private static CelUnknownSet create(
58+
ImmutableSet<CelAttribute> attributes, ImmutableSet<Long> unknownExprIds) {
59+
return new AutoValue_CelUnknownSet(attributes, unknownExprIds);
60+
}
61+
4162
public static CelUnknownSet union(CelUnknownSet lhs, CelUnknownSet rhs) {
4263
return create(
4364
ImmutableSet.<CelAttribute>builder()
4465
.addAll(lhs.attributes())
4566
.addAll(rhs.attributes())
67+
.build(),
68+
ImmutableSet.<Long>builder()
69+
.addAll(lhs.unknownExprIds())
70+
.addAll(rhs.unknownExprIds())
4671
.build());
4772
}
4873

‎runtime/src/main/java/dev/cel/runtime/DefaultInterpreter.java‎

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -108,10 +108,6 @@ static IntermediateResult create(Object value) {
108108
}
109109
}
110110

111-
public DefaultInterpreter(RuntimeTypeProvider typeProvider, Dispatcher dispatcher) {
112-
this(typeProvider, dispatcher, CelOptions.LEGACY);
113-
}
114-
115111
/**
116112
* Creates a new interpreter
117113
*
@@ -167,7 +163,10 @@ public Object eval(GlobalResolver resolver) throws InterpreterException {
167163
@Override
168164
public Object eval(GlobalResolver resolver, CelEvaluationListener listener)
169165
throws InterpreterException {
170-
return evalTrackingUnknowns(RuntimeUnknownResolver.fromResolver(resolver), listener);
166+
return evalTrackingUnknowns(
167+
RuntimeUnknownResolver.fromResolver(
168+
resolver, celOptions.adaptUnknownValueSetToNativeType()),
169+
listener);
171170
}
172171

173172
@Override
@@ -333,7 +332,9 @@ private IntermediateResult evalFieldSelect(
333332
Object fieldValue = typeProvider.selectField(operand, field);
334333

335334
return IntermediateResult.create(
336-
attribute, InterpreterUtil.valueOrUnknown(fieldValue, expr.id()));
335+
attribute,
336+
InterpreterUtil.valueOrUnknown(
337+
fieldValue, expr.id(), celOptions.adaptUnknownValueSetToNativeType()));
337338
}
338339

339340
private IntermediateResult evalCall(ExecutionFrame frame, CelExpr expr, CelCall callExpr)
@@ -486,7 +487,8 @@ private IntermediateResult mergeBooleanUnknowns(IntermediateResult lhs, Intermed
486487

487488
// Otherwise fallback to normal impl
488489
return IntermediateResult.create(
489-
InterpreterUtil.shortcircuitUnknownOrThrowable(lhs.value(), rhs.value()));
490+
InterpreterUtil.shortcircuitUnknownOrThrowable(
491+
lhs.value(), rhs.value(), celOptions.adaptUnknownValueSetToNativeType()));
490492
}
491493

492494
private enum ShortCircuitableOperators {

‎runtime/src/main/java/dev/cel/runtime/InterpreterUtil.java‎

Lines changed: 43 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -17,9 +17,7 @@
1717
import dev.cel.expr.ExprValue;
1818
import dev.cel.expr.UnknownSet;
1919
import dev.cel.common.annotations.Internal;
20-
import java.util.Arrays;
2120
import java.util.LinkedHashSet;
22-
import java.util.List;
2321
import java.util.Set;
2422
import org.jspecify.annotations.Nullable;
2523

@@ -56,8 +54,19 @@ public static Object strict(Object valueOrThrowable) throws InterpreterException
5654
* @return boolean value if object is unknown.
5755
*/
5856
public static boolean isUnknown(Object obj) {
59-
return obj instanceof ExprValue
60-
&& ((ExprValue) obj).getKindCase() == ExprValue.KindCase.UNKNOWN;
57+
if (isExprValueUnknown(obj)) {
58+
return true;
59+
}
60+
return obj instanceof CelUnknownSet;
61+
}
62+
63+
/** TODO: Remove once clients have been migrated. */
64+
static boolean isExprValueUnknown(Object obj) {
65+
if (obj instanceof ExprValue) {
66+
return ((ExprValue) obj).getKindCase() == ExprValue.KindCase.UNKNOWN;
67+
}
68+
69+
return false;
6170
}
6271

6372
/**
@@ -67,7 +76,7 @@ public static boolean isUnknown(Object obj) {
6776
* @return A new ExprValue object which has all unknown expr ids from input objects, without
6877
* duplication.
6978
*/
70-
public static ExprValue combineUnknownExprValue(Object... objs) {
79+
static ExprValue combineUnknownProtoExprValue(Object... objs) {
7180
UnknownSet.Builder unknownsetBuilder = UnknownSet.newBuilder();
7281
Set<Long> ids = new LinkedHashSet<>();
7382
for (Object object : objs) {
@@ -79,21 +88,25 @@ public static ExprValue combineUnknownExprValue(Object... objs) {
7988
return ExprValue.newBuilder().setUnknown(unknownsetBuilder).build();
8089
}
8190

82-
/** Create a {@code ExprValue} for one or more {@code ids} representing an unknown set. */
83-
public static ExprValue createUnknownExprValue(Long... ids) {
84-
return createUnknownExprValue(Arrays.asList(ids));
91+
static CelUnknownSet combineUnknownExprValue(Object... objs) {
92+
Set<Long> ids = new LinkedHashSet<>();
93+
for (Object object : objs) {
94+
if (isUnknown(object)) {
95+
ids.addAll(((CelUnknownSet) object).unknownExprIds());
96+
}
97+
}
98+
99+
return CelUnknownSet.create(ids);
85100
}
86101

87102
/**
88103
* Create an ExprValue object has UnknownSet, from a list of unknown expr ids
89104
*
90-
* @param ids List of unknown expr ids
105+
* @param id unknown expr id
91106
* @return A new ExprValue object which has all unknown expr ids from input list
92107
*/
93-
public static ExprValue createUnknownExprValue(List<Long> ids) {
94-
ExprValue.Builder exprValueBuilder = ExprValue.newBuilder();
95-
exprValueBuilder.setUnknown(UnknownSet.newBuilder().addAllExprs(ids));
96-
return exprValueBuilder.build();
108+
private static ExprValue createUnknownExprValue(long id) {
109+
return ExprValue.newBuilder().setUnknown(UnknownSet.newBuilder().addExprs(id)).build();
97110
}
98111

99112
/**
@@ -104,11 +117,14 @@ public static ExprValue createUnknownExprValue(List<Long> ids) {
104117
* from any boolean arguments alone. This allows us to consolidate the error/unknown handling for
105118
* both of these operators.
106119
*/
107-
public static Object shortcircuitUnknownOrThrowable(Object left, Object right)
120+
public static Object shortcircuitUnknownOrThrowable(
121+
Object left, Object right, boolean adaptUnknownValueSetToNativeType)
108122
throws InterpreterException {
109123
// unknown <op> unknown ==> unknown combined
110124
if (InterpreterUtil.isUnknown(left) && InterpreterUtil.isUnknown(right)) {
111-
return InterpreterUtil.combineUnknownExprValue(left, right);
125+
return adaptUnknownValueSetToNativeType
126+
? InterpreterUtil.combineUnknownExprValue(left, right)
127+
: InterpreterUtil.combineUnknownProtoExprValue(left, right);
112128
}
113129
// unknown <op> <error> ==> unknown
114130
// unknown <op> t|f ==> unknown
@@ -132,18 +148,24 @@ public static Object shortcircuitUnknownOrThrowable(Object left, Object right)
132148
"Left or/and right object is neither bool, unknown nor error, unexpected behavior.");
133149
}
134150

135-
public static Object valueOrUnknown(@Nullable Object valueOrThrowable, Long id) {
151+
public static Object valueOrUnknown(
152+
@Nullable Object valueOrThrowable, Long id, boolean adaptUnknownValueSet) {
136153
// Handle the unknown value case.
137154
if (isUnknown(valueOrThrowable)) {
138-
ExprValue value = (ExprValue) valueOrThrowable;
139-
if (value.getUnknown().getExprsCount() != 0) {
140-
return valueOrThrowable;
155+
if (adaptUnknownValueSet) {
156+
return CelUnknownSet.create(id);
157+
} else {
158+
// TODO: Remove once clients have been migrated.
159+
ExprValue value = (ExprValue) valueOrThrowable;
160+
if (value.getUnknown().getExprsCount() != 0) {
161+
return valueOrThrowable;
162+
}
163+
return createUnknownExprValue(id);
141164
}
142-
return createUnknownExprValue(id);
143165
}
144166
// Handle the null value case.
145167
if (valueOrThrowable == null) {
146-
return createUnknownExprValue(id);
168+
return adaptUnknownValueSet ? CelUnknownSet.create(id) : createUnknownExprValue(id);
147169
}
148170
return valueOrThrowable;
149171
}

0 commit comments

Comments
 (0)