diff options
| author | 2023-12-08 19:15:08 +0000 | |
|---|---|---|
| committer | 2023-12-12 14:42:45 -0500 | |
| commit | 98562fded9a6378d505bad19584373f748ac252f (patch) | |
| tree | a19e3b97c192d2e7a8b9526a75561e312423a3f9 | |
| parent | 13af7616bf77ee53d59accbb1d2c923f305c07bd (diff) | |
aconfig: fix template bug in exported mode.
FakeFeatureFlagsImpl still generated all flags in its mFlagMap field, but it should generate only exported flags in exported mode.
Test: atest aconfig.test.java
Bug: 311152507
Change-Id: I61843cd87b3bb5035772791a5869a91b07d574d8
| -rw-r--r-- | tools/aconfig/Android.bp | 13 | ||||
| -rw-r--r-- | tools/aconfig/src/codegen/java.rs | 26 | ||||
| -rw-r--r-- | tools/aconfig/templates/FakeFeatureFlagsImpl.java.template | 9 | ||||
| -rw-r--r-- | tools/aconfig/templates/FeatureFlagsImpl.java.template | 4 | ||||
| -rw-r--r-- | tools/aconfig/tests/AconfigTest.java | 8 | ||||
| -rw-r--r-- | tools/aconfig/tests/test_exported.aconfig | 17 |
6 files changed, 64 insertions, 13 deletions
diff --git a/tools/aconfig/Android.bp b/tools/aconfig/Android.bp index 2d40365ca6..85202bd8aa 100644 --- a/tools/aconfig/Android.bp +++ b/tools/aconfig/Android.bp @@ -90,6 +90,12 @@ aconfig_declarations { srcs: ["tests/test.aconfig"], } +aconfig_declarations { + name: "aconfig.test.exported.flags", + package: "com.android.aconfig.test.exported", + srcs: ["tests/test_exported.aconfig"], +} + aconfig_values { name: "aconfig.test.flag.values", package: "com.android.aconfig.test", @@ -113,6 +119,12 @@ java_aconfig_library { aconfig_declarations: "aconfig.test.flags", } +java_aconfig_library { + name: "aconfig_test_java_library_exported", + aconfig_declarations: "aconfig.test.exported.flags", + mode: "exported", +} + android_test { name: "aconfig.test.java", srcs: [ @@ -122,6 +134,7 @@ android_test { certificate: "platform", static_libs: [ "aconfig_test_java_library", + "aconfig_test_java_library_exported", "androidx.test.rules", "testng", ], diff --git a/tools/aconfig/src/codegen/java.rs b/tools/aconfig/src/codegen/java.rs index f958c21bac..ae3f274648 100644 --- a/tools/aconfig/src/codegen/java.rs +++ b/tools/aconfig/src/codegen/java.rs @@ -14,7 +14,7 @@ * limitations under the License. */ -use anyhow::Result; +use anyhow::{anyhow, Result}; use serde::Serialize; use std::collections::{BTreeMap, BTreeSet}; use std::path::PathBuf; @@ -34,17 +34,26 @@ where { let flag_elements: Vec<FlagElement> = parsed_flags_iter.map(|pf| create_flag_element(package, pf)).collect(); + let exported_flag_elements: Vec<FlagElement> = + flag_elements.iter().filter(|elem| elem.exported).cloned().collect(); let namespace_flags = gen_flags_by_namespace(&flag_elements); let properties_set: BTreeSet<String> = flag_elements.iter().map(|fe| format_property_name(&fe.device_config_namespace)).collect(); - let is_read_write = flag_elements.iter().any(|elem| elem.is_read_write); let is_test_mode = codegen_mode == CodegenMode::Test; let library_exported = codegen_mode == CodegenMode::Exported; + let runtime_lookup_required = + flag_elements.iter().any(|elem| elem.is_read_write) || library_exported; + + if library_exported && exported_flag_elements.is_empty() { + return Err(anyhow!("exported library contains no exported flags")); + } + let context = Context { flag_elements, + exported_flag_elements, namespace_flags, is_test_mode, - is_read_write, + runtime_lookup_required, properties_set, package_name: package.to_string(), library_exported, @@ -100,9 +109,10 @@ fn gen_flags_by_namespace(flags: &[FlagElement]) -> Vec<NamespaceFlags> { #[derive(Serialize)] struct Context { pub flag_elements: Vec<FlagElement>, + pub exported_flag_elements: Vec<FlagElement>, pub namespace_flags: Vec<NamespaceFlags>, pub is_test_mode: bool, - pub is_read_write: bool, + pub runtime_lookup_required: bool, pub properties_set: BTreeSet<String>, pub package_name: String, pub library_exported: bool, @@ -655,14 +665,8 @@ mod tests { } private Map<String, Boolean> mFlagMap = new HashMap<>( Map.ofEntries( - Map.entry(Flags.FLAG_DISABLED_RO, false), - Map.entry(Flags.FLAG_DISABLED_RW, false), Map.entry(Flags.FLAG_DISABLED_RW_EXPORTED, false), - Map.entry(Flags.FLAG_DISABLED_RW_IN_OTHER_NAMESPACE, false), - Map.entry(Flags.FLAG_ENABLED_FIXED_RO, false), - Map.entry(Flags.FLAG_ENABLED_RO, false), - Map.entry(Flags.FLAG_ENABLED_RO_EXPORTED, false), - Map.entry(Flags.FLAG_ENABLED_RW, false) + Map.entry(Flags.FLAG_ENABLED_RO_EXPORTED, false) ) ); } diff --git a/tools/aconfig/templates/FakeFeatureFlagsImpl.java.template b/tools/aconfig/templates/FakeFeatureFlagsImpl.java.template index fd2e26a934..8010b884c4 100644 --- a/tools/aconfig/templates/FakeFeatureFlagsImpl.java.template +++ b/tools/aconfig/templates/FakeFeatureFlagsImpl.java.template @@ -52,11 +52,20 @@ public class FakeFeatureFlagsImpl implements FeatureFlags \{ } private Map<String, Boolean> mFlagMap = new HashMap<>( + {{ if library_exported }} + Map.ofEntries( + {{-for item in exported_flag_elements}} + Map.entry(Flags.FLAG_{item.flag_name_constant_suffix}, false) + {{ -if not @last }},{{ endif }} + {{ -endfor }} + ) + {{ else }} Map.ofEntries( {{-for item in flag_elements}} Map.entry(Flags.FLAG_{item.flag_name_constant_suffix}, false) {{ -if not @last }},{{ endif }} {{ -endfor }} ) + {{ endif }} ); } diff --git a/tools/aconfig/templates/FeatureFlagsImpl.java.template b/tools/aconfig/templates/FeatureFlagsImpl.java.template index 33aa605a77..7a52ceb4ee 100644 --- a/tools/aconfig/templates/FeatureFlagsImpl.java.template +++ b/tools/aconfig/templates/FeatureFlagsImpl.java.template @@ -2,13 +2,13 @@ package {package_name}; // TODO(b/303773055): Remove the annotation after access issue is resolved. import android.compat.annotation.UnsupportedAppUsage; {{ if not is_test_mode }} -{{ if is_read_write- }} +{{ if runtime_lookup_required- }} import android.provider.DeviceConfig; import android.provider.DeviceConfig.Properties; {{ endif }} /** @hide */ public final class FeatureFlagsImpl implements FeatureFlags \{ -{{- if is_read_write }} +{{- if runtime_lookup_required }} {{- for namespace_with_flags in namespace_flags }} private static boolean {namespace_with_flags.namespace}_is_cached = false; {{- endfor- }} diff --git a/tools/aconfig/tests/AconfigTest.java b/tools/aconfig/tests/AconfigTest.java index 958b02e29b..bb993c4453 100644 --- a/tools/aconfig/tests/AconfigTest.java +++ b/tools/aconfig/tests/AconfigTest.java @@ -8,6 +8,8 @@ import static com.android.aconfig.test.Flags.disabledRw; import static com.android.aconfig.test.Flags.enabledFixedRo; import static com.android.aconfig.test.Flags.enabledRo; import static com.android.aconfig.test.Flags.enabledRw; +import static com.android.aconfig.test.exported.Flags.exportedFlag; +import static com.android.aconfig.test.exported.Flags.FLAG_EXPORTED_FLAG; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertThrows; @@ -64,4 +66,10 @@ public final class AconfigTest { fakeFeatureFlags.setFlag(FLAG_ENABLED_RW, false); assertFalse(fakeFeatureFlags.enabledRw()); } + + @Test + public void testExportedFlag() { + assertEquals("com.android.aconfig.test.exported.exported_flag", FLAG_EXPORTED_FLAG); + assertFalse(exportedFlag()); + } } diff --git a/tools/aconfig/tests/test_exported.aconfig b/tools/aconfig/tests/test_exported.aconfig new file mode 100644 index 0000000000..20f23a3182 --- /dev/null +++ b/tools/aconfig/tests/test_exported.aconfig @@ -0,0 +1,17 @@ +package: "com.android.aconfig.test.exported" +container: "system" + +flag { + name: "exported_flag" + namespace: "aconfig_test" + description: "This is an exported flag" + is_exported: true + bug: "888" +} + +flag { + name: "not_exported_flag" + namespace: "aconfig_test" + description: "This flag is not exported" + bug: "777" +} |