Skip to content

Commit ff28d55

Browse files
l46kokcopybara-github
authored andcommitted
Fix optional or/orValue to propagate errors and unknowns
PiperOrigin-RevId: 768180303
1 parent 78c744b commit ff28d55

File tree

2 files changed

+57
-20
lines changed

2 files changed

+57
-20
lines changed

extensions/src/test/java/dev/cel/extensions/CelOptionalLibraryTest.java

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -913,6 +913,45 @@ public void traditionalIndex_onOptionalList_returnsOptionalEmpty() throws Except
913913
assertThat(result).isEqualTo(Optional.empty());
914914
}
915915

916+
@Test
917+
// LHS
918+
@TestParameters("{expression: 'optx.or(optional.of(1))'}")
919+
@TestParameters("{expression: 'optx.orValue(1)'}")
920+
// RHS
921+
@TestParameters("{expression: 'optional.none().or(optx)'}")
922+
@TestParameters("{expression: 'optional.none().orValue(optx)'}")
923+
public void optionalChainedFunctions_lhsIsUnknown_returnsUnknown(String expression)
924+
throws Exception {
925+
Cel cel =
926+
newCelBuilder()
927+
.addVar("optx", OptionalType.create(SimpleType.INT))
928+
.addVar("x", SimpleType.INT)
929+
.build();
930+
CelAbstractSyntaxTree ast = cel.compile(expression).getAst();
931+
932+
Object result = cel.createProgram(ast).eval();
933+
934+
assertThat(InterpreterUtil.isUnknown(result)).isTrue();
935+
}
936+
937+
@Test
938+
// LHS
939+
@TestParameters("{expression: 'optional.of(1/0).or(optional.of(1))'}")
940+
@TestParameters("{expression: 'optional.of(1/0).orValue(1)'}")
941+
// RHS
942+
@TestParameters("{expression: 'optional.none().or(optional.of(1/0))'}")
943+
@TestParameters("{expression: 'optional.none().orValue(1/0)'}")
944+
public void optionalChainedFunctions_lhsIsError_returnsError(String expression) throws Exception {
945+
Cel cel =
946+
newCelBuilder()
947+
.addVar("optx", OptionalType.create(SimpleType.INT))
948+
.addVar("x", SimpleType.INT)
949+
.build();
950+
CelAbstractSyntaxTree ast = cel.compile(expression).getAst();
951+
952+
assertThrows(CelEvaluationException.class, () -> cel.createProgram(ast).eval());
953+
}
954+
916955
@Test
917956
public void traditionalIndex_onOptionalList_returnsOptionalValue() throws Exception {
918957
Cel cel =

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

Lines changed: 18 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -680,30 +680,28 @@ private IntermediateResult evalType(ExecutionFrame frame, CelCall callExpr)
680680

681681
private IntermediateResult evalOptionalOr(ExecutionFrame frame, CelCall callExpr)
682682
throws CelEvaluationException {
683-
CelExpr lhsExpr = callExpr.target().get();
684-
IntermediateResult lhsResult = evalInternal(frame, lhsExpr);
685-
if (!(lhsResult.value() instanceof Optional)) {
686-
throw CelEvaluationExceptionBuilder.newBuilder(
687-
"expected optional value, found: %s", lhsResult.value())
688-
.setErrorCode(CelErrorCode.INVALID_ARGUMENT)
689-
.setMetadata(metadata, lhsExpr.id())
690-
.build();
691-
}
692-
693-
Optional<?> lhsOptionalValue = (Optional<?>) lhsResult.value();
694-
695-
if (lhsOptionalValue.isPresent()) {
696-
// Short-circuit lhs if a value exists
697-
return lhsResult;
698-
}
699-
700-
return evalInternal(frame, callExpr.args().get(0));
683+
return evalOptionalOrInternal(frame, callExpr, /* unwrapOptional= */ false);
701684
}
702685

703686
private IntermediateResult evalOptionalOrValue(ExecutionFrame frame, CelCall callExpr)
704687
throws CelEvaluationException {
705-
CelExpr lhsExpr = callExpr.target().get();
688+
return evalOptionalOrInternal(frame, callExpr, /* unwrapOptional= */ true);
689+
}
690+
691+
private IntermediateResult evalOptionalOrInternal(
692+
ExecutionFrame frame, CelCall callExpr, boolean unwrapOptional)
693+
throws CelEvaluationException {
694+
CelExpr lhsExpr =
695+
callExpr
696+
.target()
697+
.orElseThrow(
698+
() -> new IllegalStateException("Missing target for chained optional function"));
706699
IntermediateResult lhsResult = evalInternal(frame, lhsExpr);
700+
701+
if (isUnknownValue(lhsResult.value())) {
702+
return lhsResult;
703+
}
704+
707705
if (!(lhsResult.value() instanceof Optional)) {
708706
throw CelEvaluationExceptionBuilder.newBuilder(
709707
"expected optional value, found: %s", lhsResult.value())
@@ -716,7 +714,7 @@ private IntermediateResult evalOptionalOrValue(ExecutionFrame frame, CelCall cal
716714

717715
if (lhsOptionalValue.isPresent()) {
718716
// Short-circuit lhs if a value exists
719-
return IntermediateResult.create(lhsOptionalValue.get());
717+
return unwrapOptional ? IntermediateResult.create(lhsOptionalValue.get()) : lhsResult;
720718
}
721719

722720
return evalInternal(frame, callExpr.args().get(0));

0 commit comments

Comments
 (0)