diff --git a/env/BUILD b/env/BUILD index 55297b190..41ffc1723 100644 --- a/env/BUILD +++ b/env/BUILD @@ -52,6 +52,7 @@ cc_library( ":config", "//checker:type_checker_builder", "//common:constant", + "//common:container", "//common:decl", "//common:type", "//compiler", diff --git a/env/config.h b/env/config.h index 10b23d030..e427832ff 100644 --- a/env/config.h +++ b/env/config.h @@ -34,9 +34,16 @@ class Config { struct ContainerConfig { std::string name; - // TODO(uncreated-issue/87): add support for aliases and abbreviations. + std::vector abbreviations; + struct Alias { + std::string alias; + std::string qualified_name; + }; + std::vector aliases; - bool IsEmpty() const { return name.empty(); } + bool IsEmpty() const { + return name.empty() && abbreviations.empty() && aliases.empty(); + } }; void SetContainerConfig(ContainerConfig container_config) { diff --git a/env/env.cc b/env/env.cc index 5a4198497..42652ce59 100644 --- a/env/env.cc +++ b/env/env.cc @@ -24,6 +24,7 @@ #include "absl/strings/string_view.h" #include "checker/type_checker_builder.h" #include "common/constant.h" +#include "common/container.h" #include "common/decl.h" #include "common/type.h" #include "compiler/compiler.h" @@ -130,7 +131,16 @@ absl::StatusOr> Env::NewCompilerBuilder() { cel::TypeCheckerBuilder& checker_builder = compiler_builder->GetCheckerBuilder(); - checker_builder.set_container(config_.GetContainerConfig().name); + ExpressionContainer container; + CEL_RETURN_IF_ERROR( + container.SetContainer(config_.GetContainerConfig().name)); + for (const auto& abbr : config_.GetContainerConfig().abbreviations) { + CEL_RETURN_IF_ERROR(container.AddAbbreviation(abbr)); + } + for (const auto& alias : config_.GetContainerConfig().aliases) { + CEL_RETURN_IF_ERROR(container.AddAlias(alias.alias, alias.qualified_name)); + } + checker_builder.SetExpressionContainer(std::move(container)); if (!config_.GetStandardLibraryConfig().disable) { CEL_RETURN_IF_ERROR( diff --git a/env/env_test.cc b/env/env_test.cc index 076eb57bc..b599aa569 100644 --- a/env/env_test.cc +++ b/env/env_test.cc @@ -314,6 +314,36 @@ TEST(ContainerConfigTest, ContainerConfig) { EXPECT_THAT(result.GetIssues(), IsEmpty()) << result.FormatError(); } +TEST(ContainerConfigTest, ContainerConfigWithAbbreviations) { + Env env; + env.SetDescriptorPool(internal::GetSharedTestingDescriptorPool()); + Config config; + config.SetContainerConfig( + {.name = "cel.expr.conformance", + .abbreviations = {"cel.expr.conformance.proto2.TestAllTypes"}}); + env.SetConfig(config); + ASSERT_OK_AND_ASSIGN(std::unique_ptr compiler, env.NewCompiler()); + ASSERT_OK_AND_ASSIGN(auto result, compiler->Compile("TestAllTypes{}")); + + EXPECT_THAT(result.GetIssues(), IsEmpty()) << result.FormatError(); +} + +TEST(ContainerConfigTest, ContainerConfigWithAliases) { + Env env; + env.SetDescriptorPool(internal::GetSharedTestingDescriptorPool()); + Config config; + config.SetContainerConfig( + {.name = "cel.expr.conformance", + .aliases = { + {.alias = "MyTestType", + .qualified_name = "cel.expr.conformance.proto2.TestAllTypes"}}}); + env.SetConfig(config); + ASSERT_OK_AND_ASSIGN(std::unique_ptr compiler, env.NewCompiler()); + ASSERT_OK_AND_ASSIGN(auto result, compiler->Compile("MyTestType{}")); + + EXPECT_THAT(result.GetIssues(), IsEmpty()) << result.FormatError(); +} + struct VariableConfigWithValueTestCase { Config::VariableConfig variable_config; std::string validate_type_expr; diff --git a/env/env_yaml.cc b/env/env_yaml.cc index 4ba16ea84..159786598 100644 --- a/env/env_yaml.cc +++ b/env/env_yaml.cc @@ -150,12 +150,72 @@ absl::Status ParseName(Config& config, absl::string_view yaml, absl::Status ParseContainerConfig(Config& config, absl::string_view yaml, const YAML::Node& root) { const YAML::Node container = root["container"]; - if (container.IsDefined()) { - if (!container.IsScalar()) { - return YamlError(yaml, container, "Node 'container' is not a string"); - } + if (!container.IsDefined()) { + return absl::OkStatus(); + } + + if (container.IsScalar()) { config.SetContainerConfig({.name = GetString(yaml, container)}); + return absl::OkStatus(); } + + if (!container.IsMap()) { + return YamlError(yaml, container, + "Node 'container' is neither a string nor a map"); + } + + Config::ContainerConfig container_config; + + const YAML::Node name = container["name"]; + if (name.IsDefined()) { + if (!name.IsScalar()) { + return YamlError(yaml, name, "Node 'name' in container is not a string"); + } + container_config.name = GetString(yaml, name); + } + + const YAML::Node abbreviations = container["abbreviations"]; + if (abbreviations.IsDefined()) { + if (!abbreviations.IsSequence()) { + return YamlError(yaml, abbreviations, + "Node 'abbreviations' is not a sequence"); + } + for (const YAML::Node& abbr : abbreviations) { + if (!abbr.IsScalar()) { + return YamlError(yaml, abbr, "Abbreviation is not a string"); + } + container_config.abbreviations.push_back(GetString(yaml, abbr)); + } + } + + const YAML::Node aliases = container["aliases"]; + if (aliases.IsDefined()) { + if (!aliases.IsSequence()) { + return YamlError(yaml, aliases, "Node 'aliases' is not a sequence"); + } + for (const YAML::Node& alias_node : aliases) { + if (!alias_node.IsMap()) { + return YamlError(yaml, alias_node, "Alias entry is not a map"); + } + const YAML::Node alias_key = alias_node["alias"]; + const YAML::Node qualified_name_key = alias_node["qualified_name"]; + + if (!alias_key.IsDefined() || !alias_key.IsScalar()) { + return YamlError(yaml, alias_node, + "Alias entry missing 'alias' string"); + } + if (!qualified_name_key.IsDefined() || !qualified_name_key.IsScalar()) { + return YamlError(yaml, alias_node, + "Alias entry missing 'qualified_name' string"); + } + + container_config.aliases.push_back( + {.alias = GetString(yaml, alias_key), + .qualified_name = GetString(yaml, qualified_name_key)}); + } + } + + config.SetContainerConfig(std::move(container_config)); return absl::OkStatus(); } @@ -686,7 +746,44 @@ void EmitContainerConfig(const Config& env_config, YAML::Emitter& out) { } out << YAML::Key << "container"; - out << YAML::Value << YAML::DoubleQuoted << container_config.name; + if (container_config.abbreviations.empty() && + container_config.aliases.empty()) { + out << YAML::Value << YAML::DoubleQuoted << container_config.name; + } else { + out << YAML::Value << YAML::BeginMap; + if (!container_config.name.empty()) { + out << YAML::Key << "name" << YAML::Value << YAML::DoubleQuoted + << container_config.name; + } + if (!container_config.abbreviations.empty()) { + std::vector sorted_abbrs = container_config.abbreviations; + absl::c_sort(sorted_abbrs); + out << YAML::Key << "abbreviations" << YAML::Value << YAML::BeginSeq; + for (const auto& abbr : sorted_abbrs) { + out << YAML::Value << YAML::DoubleQuoted << abbr; + } + out << YAML::EndSeq; + } + if (!container_config.aliases.empty()) { + std::vector sorted_aliases = + container_config.aliases; + absl::c_sort(sorted_aliases, [](const Config::ContainerConfig::Alias& a, + const Config::ContainerConfig::Alias& b) { + return a.alias < b.alias; + }); + out << YAML::Key << "aliases" << YAML::Value << YAML::BeginSeq; + for (const auto& alias : sorted_aliases) { + out << YAML::BeginMap; + out << YAML::Key << "alias" << YAML::Value << YAML::DoubleQuoted + << alias.alias; + out << YAML::Key << "qualified_name" << YAML::Value + << YAML::DoubleQuoted << alias.qualified_name; + out << YAML::EndMap; + } + out << YAML::EndSeq; + } + out << YAML::EndMap; + } } void EmitExtensionConfigs(const Config& env_config, YAML::Emitter& out) { diff --git a/env/env_yaml_test.cc b/env/env_yaml_test.cc index 25cc63206..d19c0dbfb 100644 --- a/env/env_yaml_test.cc +++ b/env/env_yaml_test.cc @@ -55,6 +55,31 @@ TEST(EnvYamlTest, ParseContainerConfig) { Field(&Config::ContainerConfig::name, "test.container")); } +TEST(EnvYamlTest, ParseContainerConfig_AlternativeSyntax) { + ASSERT_OK_AND_ASSIGN(Config config, EnvConfigFromYaml(R"yaml( + container: + name: test.container + abbreviations: + - abbr1.Abbr1 + - abbr2.Abbr2 + aliases: + - alias: alias1 + qualified_name: qual.name1 + - alias: alias2 + qualified_name: qual.name2 + )yaml")); + + const auto& container_config = config.GetContainerConfig(); + EXPECT_EQ(container_config.name, "test.container"); + EXPECT_THAT(container_config.abbreviations, + UnorderedElementsAre("abbr1.Abbr1", "abbr2.Abbr2")); + ASSERT_THAT(container_config.aliases, SizeIs(2)); + EXPECT_EQ(container_config.aliases[0].alias, "alias1"); + EXPECT_EQ(container_config.aliases[0].qualified_name, "qual.name1"); + EXPECT_EQ(container_config.aliases[1].alias, "alias2"); + EXPECT_EQ(container_config.aliases[1].qualified_name, "qual.name2"); +} + TEST(EnvYamlTest, ParseExtensionConfigs) { ASSERT_OK_AND_ASSIGN(Config config, EnvConfigFromYaml(R"yaml( extensions: @@ -550,9 +575,78 @@ INSTANTIATE_TEST_SUITE_P( container: - error: "error" )yaml", - .expected_error = "3:19: Node 'container' is not a string\n" - "| - error: \"error\"\n" - "| ^", + .expected_error = + "3:19: Node 'container' is neither a string nor a map\n" + "| - error: \"error\"\n" + "| ^", + }, + ParseTestCase{ + .yaml = R"yaml( + container: + name: [] + )yaml", + .expected_error = "3:25: Node 'name' in container is not a string\n" + "| name: []\n" + "| ^", + }, + ParseTestCase{ + .yaml = R"yaml( + container: + abbreviations: "abbr" + )yaml", + .expected_error = "3:34: Node 'abbreviations' is not a sequence\n" + "| abbreviations: \"abbr\"\n" + "| ^", + }, + ParseTestCase{ + .yaml = R"yaml( + container: + abbreviations: + - [] + )yaml", + .expected_error = "4:21: Abbreviation is not a string\n" + "| - []\n" + "| ^", + }, + ParseTestCase{ + .yaml = R"yaml( + container: + aliases: "not a sequence" + )yaml", + .expected_error = "3:28: Node 'aliases' is not a sequence\n" + "| aliases: \"not a sequence\"\n" + "| ^", + }, + ParseTestCase{ + .yaml = R"yaml( + container: + aliases: + - "not a map" + )yaml", + .expected_error = "4:21: Alias entry is not a map\n" + "| - \"not a map\"\n" + "| ^", + }, + ParseTestCase{ + .yaml = R"yaml( + container: + aliases: + - qualified_name: "qual" + )yaml", + .expected_error = "4:21: Alias entry missing 'alias' string\n" + "| - qualified_name: \"qual\"\n" + "| ^", + }, + ParseTestCase{ + .yaml = R"yaml( + container: + aliases: + - alias: "my_alias" + )yaml", + .expected_error = "4:21: Alias entry missing" + " 'qualified_name' string\n" + "| - alias: \"my_alias\"\n" + "| ^", }, ParseTestCase{ .yaml = R"yaml( @@ -946,6 +1040,33 @@ std::vector GetExportTestCases() { container: "test.container" )yaml", }, + ExportTestCase{ + .config = []() -> absl::StatusOr { + Config config; + config.SetName("test.env"); + config.SetContainerConfig( + {.name = "test.container", + .abbreviations = {"foo", "bar"}, + .aliases = { + {.alias = "foo", .qualified_name = "test.foo"}, + {.alias = "bar", .qualified_name = "test.bar"}, + }}); + return config; + }(), + .expected_yaml = R"yaml( + name: "test.env" + container: + name: "test.container" + abbreviations: + - "bar" + - "foo" + aliases: + - alias: "bar" + qualified_name: "test.bar" + - alias: "foo" + qualified_name: "test.foo" + )yaml", + }, ExportTestCase{ .config = []() -> absl::StatusOr { Config config; @@ -1385,6 +1506,18 @@ std::vector GetRoundTripTestCases() { overloads: - id: "string_to_timestamp" )yaml", + R"yaml( + container: + name: "test.container" + abbreviations: + - "abbr1.Abbr1" + - "abbr2.Abbr2" + aliases: + - alias: "alias1" + qualified_name: "qual.name1" + - alias: "alias2" + qualified_name: "qual.name2" + )yaml", R"yaml( extensions: - name: "bindings"