Skip to content

Commit c140c3c

Browse files
dmitriplotnikovcopybara-github
authored andcommitted
Introduce versioning to the CEL lists extension library.
Tests are added to ensure that each function/macro is only available in the versions it's expected to be in. PiperOrigin-RevId: 876342976
1 parent a69d6b1 commit c140c3c

4 files changed

Lines changed: 169 additions & 39 deletions

File tree

extensions/BUILD

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -456,6 +456,7 @@ cc_test(
456456
srcs = ["lists_functions_test.cc"],
457457
deps = [
458458
":lists_functions",
459+
"//checker:type_check_issue",
459460
"//checker:validation_result",
460461
"//common:source",
461462
"//common:value",
@@ -476,6 +477,7 @@ cc_test(
476477
"//runtime:runtime_builder",
477478
"//runtime:runtime_options",
478479
"//runtime:standard_runtime_builder_factory",
480+
"@com_google_absl//absl/algorithm:container",
479481
"@com_google_absl//absl/status",
480482
"@com_google_absl//absl/status:status_matchers",
481483
"@com_google_absl//absl/strings:string_view",

extensions/lists_functions.cc

Lines changed: 52 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -557,7 +557,8 @@ const Type& ListTypeParamType() {
557557
return *kInstance;
558558
}
559559

560-
absl::Status RegisterListsCheckerDecls(TypeCheckerBuilder& builder) {
560+
absl::Status RegisterListsCheckerDecls(TypeCheckerBuilder& builder,
561+
int version) {
561562
CEL_ASSIGN_OR_RETURN(
562563
FunctionDecl distinct_decl,
563564
MakeFunctionDecl("distinct", MakeMemberOverloadDecl(
@@ -615,22 +616,40 @@ absl::Status RegisterListsCheckerDecls(TypeCheckerBuilder& builder) {
615616
ListTypeParamType(), ListTypeParamType(), list_type)));
616617
}
617618

619+
CEL_RETURN_IF_ERROR(builder.AddFunction(std::move(slice_decl)));
620+
if (version == 0) {
621+
return absl::OkStatus();
622+
}
623+
624+
CEL_RETURN_IF_ERROR(builder.AddFunction(std::move(flatten_decl)));
625+
if (version == 1) {
626+
return absl::OkStatus();
627+
}
628+
618629
CEL_RETURN_IF_ERROR(builder.AddFunction(std::move(sort_decl)));
619630
CEL_RETURN_IF_ERROR(builder.AddFunction(std::move(sort_by_key_decl)));
620631
CEL_RETURN_IF_ERROR(builder.AddFunction(std::move(distinct_decl)));
621-
CEL_RETURN_IF_ERROR(builder.AddFunction(std::move(flatten_decl)));
622632
CEL_RETURN_IF_ERROR(builder.AddFunction(std::move(range_decl)));
623633
// MergeFunction is used to combine with the reverse function
624634
// defined in strings extension.
625635
CEL_RETURN_IF_ERROR(builder.MergeFunction(std::move(reverse_decl)));
626-
CEL_RETURN_IF_ERROR(builder.AddFunction(std::move(slice_decl)));
627636
return absl::OkStatus();
628637
}
629638

630-
std::vector<Macro> lists_macros() { return {ListSortByMacro()}; }
639+
std::vector<Macro> lists_macros(int version) {
640+
switch (version) {
641+
case 0:
642+
return {};
643+
case 1:
644+
return {};
645+
case 2:
646+
default:
647+
return {ListSortByMacro()};
648+
};
649+
}
631650

632-
absl::Status ConfigureParser(ParserBuilder& builder) {
633-
for (const Macro& macro : lists_macros()) {
651+
absl::Status ConfigureParser(ParserBuilder& builder, int version) {
652+
for (const Macro& macro : lists_macros(version)) {
634653
CEL_RETURN_IF_ERROR(builder.AddMacro(macro));
635654
}
636655
return absl::OkStatus();
@@ -639,28 +658,44 @@ absl::Status ConfigureParser(ParserBuilder& builder) {
639658
} // namespace
640659

641660
absl::Status RegisterListsFunctions(FunctionRegistry& registry,
642-
const RuntimeOptions& options) {
643-
CEL_RETURN_IF_ERROR(RegisterListDistinctFunction(registry));
661+
const RuntimeOptions& options,
662+
int version) {
663+
CEL_RETURN_IF_ERROR(RegisterListSliceFunction(registry));
664+
if (version == 0) {
665+
return absl::OkStatus();
666+
}
667+
668+
// Since version 1
644669
CEL_RETURN_IF_ERROR(RegisterListFlattenFunction(registry));
670+
if (version == 1) {
671+
return absl::OkStatus();
672+
}
673+
674+
// Since version 2
675+
CEL_RETURN_IF_ERROR(RegisterListDistinctFunction(registry));
645676
CEL_RETURN_IF_ERROR(RegisterListRangeFunction(registry));
646677
CEL_RETURN_IF_ERROR(RegisterListReverseFunction(registry));
647-
CEL_RETURN_IF_ERROR(RegisterListSliceFunction(registry));
648678
CEL_RETURN_IF_ERROR(RegisterListSortFunction(registry));
649679
return absl::OkStatus();
650680
}
651681

652-
absl::Status RegisterListsMacros(MacroRegistry& registry,
653-
const ParserOptions&) {
654-
return registry.RegisterMacros(lists_macros());
682+
absl::Status RegisterListsMacros(MacroRegistry& registry, const ParserOptions&,
683+
int version) {
684+
return registry.RegisterMacros(lists_macros(version));
655685
}
656686

657-
CheckerLibrary ListsCheckerLibrary() {
658-
return {.id = "cel.lib.ext.lists", .configure = RegisterListsCheckerDecls};
687+
CheckerLibrary ListsCheckerLibrary(int version) {
688+
return {.id = "cel.lib.ext.lists",
689+
.configure = [version](TypeCheckerBuilder& builder) {
690+
return RegisterListsCheckerDecls(builder, version);
691+
}};
659692
}
660693

661-
CompilerLibrary ListsCompilerLibrary() {
662-
auto lib = CompilerLibrary::FromCheckerLibrary(ListsCheckerLibrary());
663-
lib.configure_parser = ConfigureParser;
694+
CompilerLibrary ListsCompilerLibrary(int version) {
695+
auto lib = CompilerLibrary::FromCheckerLibrary(ListsCheckerLibrary(version));
696+
lib.configure_parser = [version](ParserBuilder& builder) {
697+
return ConfigureParser(builder, version);
698+
};
664699
return lib;
665700
}
666701

extensions/lists_functions.h

Lines changed: 35 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -25,65 +25,78 @@
2525

2626
namespace cel::extensions {
2727

28+
constexpr int kListsExtensionLatestVersion = 2;
29+
2830
// Register implementations for list extension functions.
2931
//
30-
// lists.range(n: int) -> list(int)
31-
//
32-
// <list(T)>.distinct() -> list(T)
32+
// === Since version 0 ===
33+
// <list(T)>.slice(start: int, end: int) -> list(T)
3334
//
35+
// === Since version 1 ===
3436
// <list(dyn)>.flatten() -> list(dyn)
3537
// <list(dyn)>.flatten(limit: int) -> list(dyn)
3638
//
39+
// === Since version 2 ===
40+
// lists.range(n: int) -> list(int)
41+
//
42+
// <list(T)>.distinct() -> list(T)
43+
//
3744
// <list(T)>.reverse() -> list(T)
3845
//
3946
// <list(T)>.sort() -> list(T)
4047
//
41-
// <list(T)>.slice(start: int, end: int) -> list(T)
4248
absl::Status RegisterListsFunctions(FunctionRegistry& registry,
43-
const RuntimeOptions& options);
49+
const RuntimeOptions& options,
50+
int version = kListsExtensionLatestVersion);
4451

4552
// Register list macros.
4653
//
54+
// === Since version 2 ===
55+
//
4756
// <list(T)>.sortBy(<element name>, <element key expression>)
4857
absl::Status RegisterListsMacros(MacroRegistry& registry,
49-
const ParserOptions& options);
58+
const ParserOptions& options,
59+
int version = kListsExtensionLatestVersion);
5060

5161
// Type check declarations for the lists extension library.
5262
// Provides decls for the following functions:
5363
//
54-
// lists.range(n: int) -> list(int)
55-
//
56-
// <list(T)>.distinct() -> list(T)
64+
// === Since version 0 ===
65+
// <list(T)>.slice(start: int, end: int) -> list(T)
5766
//
67+
// === Since version 1 ===
5868
// <list(dyn)>.flatten() -> list(dyn)
5969
// <list(dyn)>.flatten(limit: int) -> list(dyn)
6070
//
71+
// === Since version 2 ===
72+
// lists.range(n: int) -> list(int)
73+
//
74+
// <list(T)>.distinct() -> list(T)
75+
//
6176
// <list(T)>.reverse() -> list(T)
6277
//
6378
// <list(T_)>.sort() -> list(T_) where T_ is partially orderable
64-
//
65-
// <list(T)>.slice(start: int, end: int) -> list(T)
66-
CheckerLibrary ListsCheckerLibrary();
79+
CheckerLibrary ListsCheckerLibrary(int version = kListsExtensionLatestVersion);
6780

6881
// Provides decls for the following functions:
6982
//
70-
// lists.range(n: int) -> list(int)
71-
//
72-
// <list(T)>.distinct() -> list(T)
83+
// === Since version 0 ===
84+
// <list(T)>.slice(start: int, end: int) -> list(T)
7385
//
86+
// === Since version 1 ===
7487
// <list(dyn)>.flatten() -> list(dyn)
7588
// <list(dyn)>.flatten(limit: int) -> list(dyn)
7689
//
77-
// <list(T)>.reverse() -> list(T)
78-
//
79-
// <list(T_)>.sort() -> list(T_) where T_ is partially orderable
90+
// === Since version 2 ===
91+
// lists.range(n: int) -> list(int)
8092
//
81-
// <list(T)>.slice(start: int, end: int) -> list(T)
93+
// <list(T)>.distinct() -> list(T)
8294
//
83-
// and the following macros:
95+
// <list(T)>.reverse() -> list(T)
8496
//
85-
// <list(T)>.sortBy(<element name>, <element key expression>)
86-
CompilerLibrary ListsCompilerLibrary();
97+
// <list(T_)>.sort() -> list(T_) where T_ is partially orderable
98+
CompilerLibrary ListsCompilerLibrary(
99+
int version = kListsExtensionLatestVersion);
87100

88101
} // namespace cel::extensions
89102

extensions/lists_functions_test.cc

Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,9 +20,11 @@
2020
#include <vector>
2121

2222
#include "cel/expr/syntax.pb.h"
23+
#include "absl/algorithm/container.h"
2324
#include "absl/status/status.h"
2425
#include "absl/status/status_matchers.h"
2526
#include "absl/strings/string_view.h"
27+
#include "checker/type_check_issue.h"
2628
#include "checker/validation_result.h"
2729
#include "common/source.h"
2830
#include "common/value.h"
@@ -54,7 +56,9 @@ using ::cel::test::ErrorValueIs;
5456
using ::cel::expr::Expr;
5557
using ::cel::expr::ParsedExpr;
5658
using ::cel::expr::SourceInfo;
59+
using ::testing::Contains;
5760
using ::testing::HasSubstr;
61+
using ::testing::IsEmpty;
5862
using ::testing::ValuesIn;
5963

6064
struct TestInfo {
@@ -377,5 +381,81 @@ std::vector<ListCheckerTestCase> createListsCheckerParams() {
377381
INSTANTIATE_TEST_SUITE_P(ListsCheckerLibraryTest, ListsCheckerLibraryTest,
378382
ValuesIn(createListsCheckerParams()));
379383

384+
struct ListsExtensionVersionTestCase {
385+
std::string expr;
386+
std::vector<int> expected_supported_versions;
387+
};
388+
389+
class ListsExtensionVersionTest
390+
: public ::testing::TestWithParam<ListsExtensionVersionTestCase> {};
391+
392+
TEST_P(ListsExtensionVersionTest, ListsExtensionVersions) {
393+
const ListsExtensionVersionTestCase& test_case = GetParam();
394+
for (int version = 0;
395+
version <= cel::extensions::kListsExtensionLatestVersion; ++version) {
396+
CompilerLibrary compiler_library = ListsCompilerLibrary(version);
397+
398+
ASSERT_OK_AND_ASSIGN(
399+
std::unique_ptr<CompilerBuilder> builder,
400+
cel::NewCompilerBuilder(internal::GetTestingDescriptorPool(),
401+
CompilerOptions()));
402+
ASSERT_THAT(builder->AddLibrary(StandardCompilerLibrary()), IsOk());
403+
ASSERT_THAT(builder->AddLibrary(std::move(compiler_library)), IsOk());
404+
405+
ASSERT_OK_AND_ASSIGN(std::unique_ptr<Compiler> compiler, builder->Build());
406+
ASSERT_OK_AND_ASSIGN(ValidationResult result,
407+
compiler->Compile(test_case.expr));
408+
if (absl::c_contains(test_case.expected_supported_versions, version)) {
409+
EXPECT_THAT(result.GetIssues(), IsEmpty())
410+
<< "Expected no issues for expr: " << test_case.expr
411+
<< " at version: " << version << " but got: " << result.FormatError();
412+
} else {
413+
EXPECT_THAT(result.GetIssues(),
414+
Contains(Property(&TypeCheckIssue::message,
415+
HasSubstr("undeclared reference"))));
416+
}
417+
}
418+
};
419+
420+
std::vector<ListsExtensionVersionTestCase> CreateListsExtensionVersionParams() {
421+
return {
422+
ListsExtensionVersionTestCase{
423+
.expr = "[0,1,2,3].slice(0, 2)",
424+
.expected_supported_versions = {0, 1, 2},
425+
},
426+
ListsExtensionVersionTestCase{
427+
.expr = "[[0]].flatten()",
428+
.expected_supported_versions = {1, 2},
429+
},
430+
ListsExtensionVersionTestCase{
431+
.expr = "[[0]].flatten(1)",
432+
.expected_supported_versions = {1, 2},
433+
},
434+
ListsExtensionVersionTestCase{
435+
.expr = "[1,2,3,4].sort()",
436+
.expected_supported_versions = {2},
437+
},
438+
ListsExtensionVersionTestCase{
439+
.expr = "[1,2,3,4].sortBy(x, x)",
440+
.expected_supported_versions = {2},
441+
},
442+
ListsExtensionVersionTestCase{
443+
.expr = "[1,2,3,4].distinct()",
444+
.expected_supported_versions = {2},
445+
},
446+
ListsExtensionVersionTestCase{
447+
.expr = "lists.range(4)",
448+
.expected_supported_versions = {2},
449+
},
450+
ListsExtensionVersionTestCase{
451+
.expr = "[1,2,3,4].reverse()",
452+
.expected_supported_versions = {2},
453+
},
454+
};
455+
}
456+
457+
INSTANTIATE_TEST_SUITE_P(ListsExtensionVersionTest, ListsExtensionVersionTest,
458+
ValuesIn(CreateListsExtensionVersionParams()));
459+
380460
} // namespace
381461
} // namespace cel::extensions

0 commit comments

Comments
 (0)