From 60ee8a633f822f3ba60f4a7fa04f4f2245f84df4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?M=C3=A5rten=20Kongstad?= Date: Thu, 29 Jun 2023 10:01:52 +0200 Subject: [PATCH 1/3] aconfig: improve package identifier test readability Rearrange the tests in test_is_valid_package_ident to make it more apparent what constitutes a valid package pattern, and what doesn't. Bug: 283910447 Test: atest aconfig.test Change-Id: I3d2b87aed008d0ed3a0aa6e483e655178dda7277 --- tools/aconfig/src/codegen.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/aconfig/src/codegen.rs b/tools/aconfig/src/codegen.rs index d96d4f9391..e50be0be40 100644 --- a/tools/aconfig/src/codegen.rs +++ b/tools/aconfig/src/codegen.rs @@ -65,8 +65,8 @@ mod tests { assert!(is_valid_package_ident("foo_bar_123")); assert!(is_valid_package_ident("foo.bar")); assert!(is_valid_package_ident("foo.bar.a123")); - assert!(!is_valid_package_ident("foo._bar")); + assert!(!is_valid_package_ident("foo._bar")); assert!(!is_valid_package_ident("")); assert!(!is_valid_package_ident("123_foo")); assert!(!is_valid_package_ident("foo-bar")); From c0d618c3a4f528055567ef99f62cf741f55e61ad Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?M=C3=A5rten=20Kongstad?= Date: Thu, 29 Jun 2023 10:18:20 +0200 Subject: [PATCH 2/3] aconfig: fix incorrect check in create_device_config_ident Use is_valid_name_ident to check the flag name, not is_valid_package_ident. Bug: 283910447 Test: atest aconfig.test Change-Id: I3718e7de565b975a1696190e8effbcb077e5fde2 --- tools/aconfig/src/codegen.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/aconfig/src/codegen.rs b/tools/aconfig/src/codegen.rs index e50be0be40..fe52c8df81 100644 --- a/tools/aconfig/src/codegen.rs +++ b/tools/aconfig/src/codegen.rs @@ -37,7 +37,7 @@ pub fn is_valid_package_ident(s: &str) -> bool { pub fn create_device_config_ident(package: &str, flag_name: &str) -> Result { ensure!(is_valid_package_ident(package), "bad package"); - ensure!(is_valid_package_ident(flag_name), "bad flag name"); + ensure!(is_valid_name_ident(flag_name), "bad flag name"); Ok(format!("{}.{}", package, flag_name)) } From 19776d19e6379e50994a2a5c0175917698c175d8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?M=C3=A5rten=20Kongstad?= Date: Thu, 29 Jun 2023 10:38:02 +0200 Subject: [PATCH 3/3] aconfig: package fields must contain at least one dot char Introduce a new requirement on package fields: a package must contain at least one dot character. Bug: 289336036 Test: atest aconfig.test Change-Id: Idadcd2a76783a484cc5c6d6e94778c0248fa475f --- tools/aconfig/src/codegen.rs | 9 +++++++-- tools/aconfig/src/protos.rs | 6 +++--- 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/tools/aconfig/src/codegen.rs b/tools/aconfig/src/codegen.rs index fe52c8df81..b7fb08f08c 100644 --- a/tools/aconfig/src/codegen.rs +++ b/tools/aconfig/src/codegen.rs @@ -32,6 +32,9 @@ pub fn is_valid_name_ident(s: &str) -> bool { } pub fn is_valid_package_ident(s: &str) -> bool { + if !s.contains('.') { + return false; + } s.split('.').all(is_valid_name_ident) } @@ -61,11 +64,12 @@ mod tests { #[test] fn test_is_valid_package_ident() { - assert!(is_valid_package_ident("foo")); - assert!(is_valid_package_ident("foo_bar_123")); assert!(is_valid_package_ident("foo.bar")); + assert!(is_valid_package_ident("foo.bar_baz")); assert!(is_valid_package_ident("foo.bar.a123")); + assert!(!is_valid_package_ident("foo_bar_123")); + assert!(!is_valid_package_ident("foo")); assert!(!is_valid_package_ident("foo._bar")); assert!(!is_valid_package_ident("")); assert!(!is_valid_package_ident("123_foo")); @@ -75,6 +79,7 @@ mod tests { assert!(!is_valid_package_ident(".foo.bar")); assert!(!is_valid_package_ident("foo.bar.")); assert!(!is_valid_package_ident(".")); + assert!(!is_valid_package_ident("..")); assert!(!is_valid_package_ident("foo..bar")); assert!(!is_valid_package_ident("foo.__bar")); } diff --git a/tools/aconfig/src/protos.rs b/tools/aconfig/src/protos.rs index 4d824f25f6..17019becac 100644 --- a/tools/aconfig/src/protos.rs +++ b/tools/aconfig/src/protos.rs @@ -549,7 +549,7 @@ parsed_flag { // bad input: parsed_flag not sorted by package let text_proto = r#" parsed_flag { - package: "bbb" + package: "bbb.bbb" name: "first" namespace: "first_ns" description: "This is the description of the first flag." @@ -562,7 +562,7 @@ parsed_flag { } } parsed_flag { - package: "aaa" + package: "aaa.aaa" name: "second" namespace: "second_ns" description: "This is the description of the second flag." @@ -578,7 +578,7 @@ parsed_flag { let error = try_from_binary_proto_from_text_proto(text_proto).unwrap_err(); assert_eq!( format!("{:?}", error), - "bad parsed flags: not sorted: bbb.first comes before aaa.second" + "bad parsed flags: not sorted: bbb.bbb.first comes before aaa.aaa.second" ); // bad input: parsed_flag not sorted by name