From 820ef2beb1ae0a16ec78a3859631d47d09f13310 Mon Sep 17 00:00:00 2001 From: Lee Shombert Date: Tue, 1 Mar 2022 13:55:03 -0800 Subject: [PATCH] Correct the PropertyInvalidatedCache module definition Bug: 219609105 Change the meaning of "module" in PropertyInvalidatedCache to be a string rather than an integer. One piece of the unit test has been removed because of this. Lint uncovered several lock errors. These are corrected as well. The corresponding CTS test is updated in the next review. Test: * atest FrameworksCoreTests:PropertyInvalidatedCacheTests Change-Id: Iaaa55974df2d22fc052d5b4b4744384c012f070a --- core/api/module-lib-current.txt | 7 +-- core/api/test-current.txt | 13 ++--- .../android/app/PropertyInvalidatedCache.java | 47 +++++++------------ .../app/PropertyInvalidatedCacheTests.java | 15 ++---- 4 files changed, 32 insertions(+), 50 deletions(-) diff --git a/core/api/module-lib-current.txt b/core/api/module-lib-current.txt index 7ec239d681d0a..179df016cc407 100644 --- a/core/api/module-lib-current.txt +++ b/core/api/module-lib-current.txt @@ -55,12 +55,13 @@ package android.app { } public class PropertyInvalidatedCache { - ctor public PropertyInvalidatedCache(int, int, @NonNull String, @NonNull String, @NonNull android.app.PropertyInvalidatedCache.QueryHandler); + ctor public PropertyInvalidatedCache(int, @NonNull String, @NonNull String, @NonNull String, @NonNull android.app.PropertyInvalidatedCache.QueryHandler); method public final void disableForCurrentProcess(); method public final void invalidateCache(); - method public static void invalidateCache(int, @NonNull String); + method public static void invalidateCache(@NonNull String, @NonNull String); method @Nullable public final Result query(@NonNull Query); - field public static final int MODULE_BLUETOOTH = 2; // 0x2 + field public static final String MODULE_BLUETOOTH = "bluetooth"; + field public static final String MODULE_TELEPHONY = "telephony"; } public abstract static class PropertyInvalidatedCache.QueryHandler { diff --git a/core/api/test-current.txt b/core/api/test-current.txt index 22637ca95b9cf..b6f2451279ccf 100644 --- a/core/api/test-current.txt +++ b/core/api/test-current.txt @@ -370,8 +370,8 @@ package android.app { } public class PropertyInvalidatedCache { - ctor public PropertyInvalidatedCache(int, int, @NonNull String, @NonNull String, @NonNull android.app.PropertyInvalidatedCache.QueryHandler); - method @NonNull public static String createPropertyName(int, @NonNull String); + ctor public PropertyInvalidatedCache(int, @NonNull String, @NonNull String, @NonNull String, @NonNull android.app.PropertyInvalidatedCache.QueryHandler); + method @NonNull public static String createPropertyName(@NonNull String, @NonNull String); method public final void disableForCurrentProcess(); method public static void disableForTestMode(); method public final void disableInstance(); @@ -379,14 +379,15 @@ package android.app { method public final void forgetDisableLocal(); method public boolean getDisabledState(); method public final void invalidateCache(); - method public static void invalidateCache(int, @NonNull String); + method public static void invalidateCache(@NonNull String, @NonNull String); method public final boolean isDisabled(); method @Nullable public final Result query(@NonNull Query); method public static void setTestMode(boolean); method public void testPropertyName(); - field public static final int MODULE_BLUETOOTH = 2; // 0x2 - field public static final int MODULE_SYSTEM = 1; // 0x1 - field public static final int MODULE_TEST = 0; // 0x0 + field public static final String MODULE_BLUETOOTH = "bluetooth"; + field public static final String MODULE_SYSTEM = "system_server"; + field public static final String MODULE_TELEPHONY = "telephony"; + field public static final String MODULE_TEST = "test"; } public abstract static class PropertyInvalidatedCache.QueryHandler { diff --git a/core/java/android/app/PropertyInvalidatedCache.java b/core/java/android/app/PropertyInvalidatedCache.java index 715de14345f77..2f202d95e0e36 100644 --- a/core/java/android/app/PropertyInvalidatedCache.java +++ b/core/java/android/app/PropertyInvalidatedCache.java @@ -16,7 +16,6 @@ package android.app; -import android.annotation.IntDef; import android.annotation.NonNull; import android.annotation.Nullable; import android.annotation.SystemApi; @@ -265,13 +264,6 @@ public class PropertyInvalidatedCache { * the permissions granted to the processes that contain the corresponding caches. * @hide */ - @IntDef(prefix = { "MODULE_" }, value = { - MODULE_TEST, - MODULE_SYSTEM, - MODULE_BLUETOOTH - }) - @Retention(RetentionPolicy.SOURCE) - public @interface Module {} /** * The module used for unit tests and cts tests. It is expected that no process in @@ -279,7 +271,7 @@ public class PropertyInvalidatedCache { * @hide */ @TestApi - public static final int MODULE_TEST = 0; + public static final String MODULE_TEST = "test"; /** * The module used for system server/framework caches. This is not visible outside @@ -287,7 +279,7 @@ public class PropertyInvalidatedCache { * @hide */ @TestApi - public static final int MODULE_SYSTEM = 1; + public static final String MODULE_SYSTEM = "system_server"; /** * The module used for bluetooth caches. @@ -295,11 +287,12 @@ public class PropertyInvalidatedCache { */ @SystemApi(client=SystemApi.Client.MODULE_LIBRARIES) @TestApi - public static final int MODULE_BLUETOOTH = 2; + public static final String MODULE_BLUETOOTH = "bluetooth"; - // A static array mapping module constants to strings. - private final static String[] sModuleNames = - { "test", "system_server", "bluetooth" }; + /** + * The module used for telephony caches. + */ + public static final String MODULE_TELEPHONY = "telephony"; /** * Construct a system property that matches the rules described above. The module is @@ -315,7 +308,8 @@ public class PropertyInvalidatedCache { * @hide */ @TestApi - public static @NonNull String createPropertyName(@Module int module, @NonNull String apiName) { + public static @NonNull String createPropertyName(@NonNull String module, + @NonNull String apiName) { char[] api = apiName.toCharArray(); int upper = 0; for (int i = 0; i < api.length; i++) { @@ -338,14 +332,7 @@ public class PropertyInvalidatedCache { } } - String moduleName = null; - try { - moduleName = sModuleNames[module]; - } catch (ArrayIndexOutOfBoundsException e) { - throw new IllegalArgumentException("invalid module " + module); - } - - return "cache_key." + moduleName + "." + new String(suffix); + return "cache_key." + module + "." + new String(suffix); } /** @@ -548,7 +535,7 @@ public class PropertyInvalidatedCache { */ @SystemApi(client=SystemApi.Client.MODULE_LIBRARIES) @TestApi - public PropertyInvalidatedCache(int maxEntries, @Module int module, @NonNull String api, + public PropertyInvalidatedCache(int maxEntries, @NonNull String module, @NonNull String api, @NonNull String cacheName, @NonNull QueryHandler computer) { mPropertyName = createPropertyName(module, api); mCacheName = cacheName; @@ -989,7 +976,7 @@ public class PropertyInvalidatedCache { */ @SystemApi(client=SystemApi.Client.MODULE_LIBRARIES) @TestApi - public static void invalidateCache(@Module int module, @NonNull String api) { + public static void invalidateCache(@NonNull String module, @NonNull String api) { invalidateCache(createPropertyName(module, api)); } @@ -1211,8 +1198,10 @@ public class PropertyInvalidatedCache { getHandlerLocked().sendEmptyMessageAtTime(0, mUncorkDeadlineMs); PropertyInvalidatedCache.corkInvalidations(mPropertyName); } else { - final long count = sCorkedInvalidates.getOrDefault(mPropertyName, (long) 0); - sCorkedInvalidates.put(mPropertyName, count + 1); + synchronized (sCorkLock) { + final long count = sCorkedInvalidates.getOrDefault(mPropertyName, (long) 0); + sCorkedInvalidates.put(mPropertyName, count + 1); + } } } } @@ -1341,7 +1330,7 @@ public class PropertyInvalidatedCache { } } - private void dumpContents(PrintWriter pw, String[] args) { + private void dumpContents(PrintWriter pw) { long invalidateCount; long corkedInvalidates; synchronized (sCorkLock) { @@ -1418,7 +1407,7 @@ public class PropertyInvalidatedCache { for (int i = 0; i < activeCaches.size(); i++) { PropertyInvalidatedCache currentCache = activeCaches.get(i); - currentCache.dumpContents(pw, args); + currentCache.dumpContents(pw); pw.flush(); } } catch (IOException e) { diff --git a/core/tests/coretests/src/android/app/PropertyInvalidatedCacheTests.java b/core/tests/coretests/src/android/app/PropertyInvalidatedCacheTests.java index 5338d046596ac..ed2b10117308b 100644 --- a/core/tests/coretests/src/android/app/PropertyInvalidatedCacheTests.java +++ b/core/tests/coretests/src/android/app/PropertyInvalidatedCacheTests.java @@ -38,7 +38,7 @@ import org.junit.Test; public class PropertyInvalidatedCacheTests { // Configuration for creating caches - private static final int MODULE = PropertyInvalidatedCache.MODULE_TEST; + private static final String MODULE = PropertyInvalidatedCache.MODULE_TEST; private static final String API = "testApi"; // This class is a proxy for binder calls. It contains a counter that increments @@ -214,13 +214,13 @@ public class PropertyInvalidatedCacheTests { this(MODULE, API); } - TestCache(int module, String api) { + TestCache(String module, String api) { this(module, api, new TestQuery()); setTestMode(true); testPropertyName(); } - TestCache(int module, String api, TestQuery query) { + TestCache(String module, String api, TestQuery query) { super(4, module, api, api, query); mQuery = query; setTestMode(true); @@ -364,15 +364,6 @@ public class PropertyInvalidatedCacheTests { n1 = PropertyInvalidatedCache.createPropertyName( PropertyInvalidatedCache.MODULE_SYSTEM, "get_package_info"); assertEquals(n1, "cache_key.system_server.get_package_info"); - try { - n1 = PropertyInvalidatedCache.createPropertyName( - PropertyInvalidatedCache.MODULE_SYSTEM - 1, "get package_info"); - // n1 is an invalid api name. - assertEquals(false, true); - } catch (IllegalArgumentException e) { - // An exception is expected here. - } - n1 = PropertyInvalidatedCache.createPropertyName( PropertyInvalidatedCache.MODULE_BLUETOOTH, "getState"); assertEquals(n1, "cache_key.bluetooth.get_state");