From 8ea234f52e70a1dac44f596daef06768e6485728 Mon Sep 17 00:00:00 2001 From: tomnatan Date: Mon, 20 Dec 2021 15:55:13 +0000 Subject: [PATCH] Use put/removeAllPackageOverrides in AppCompatOverridesService Fix: 199730202 Test: atest FrameworksMockingServicesTests:AppCompatOverridesServiceTest Change-Id: Id49573adb0fef25bbae1aee12f9fb85757635142 --- .../overrides/AppCompatOverridesService.java | 116 ++++--- .../AppCompatOverridesServiceTest.java | 286 ++++++++++-------- 2 files changed, 230 insertions(+), 172 deletions(-) diff --git a/services/core/java/com/android/server/compat/overrides/AppCompatOverridesService.java b/services/core/java/com/android/server/compat/overrides/AppCompatOverridesService.java index 7e58b6cddf081..880dbf6421ddd 100644 --- a/services/core/java/com/android/server/compat/overrides/AppCompatOverridesService.java +++ b/services/core/java/com/android/server/compat/overrides/AppCompatOverridesService.java @@ -41,11 +41,14 @@ import android.os.RemoteException; import android.os.ServiceManager; import android.provider.DeviceConfig; import android.provider.DeviceConfig.Properties; +import android.util.ArrayMap; import android.util.ArraySet; import android.util.Slog; import com.android.internal.annotations.VisibleForTesting; import com.android.internal.compat.CompatibilityOverrideConfig; +import com.android.internal.compat.CompatibilityOverridesByPackageConfig; +import com.android.internal.compat.CompatibilityOverridesToRemoveByPackageConfig; import com.android.internal.compat.CompatibilityOverridesToRemoveConfig; import com.android.internal.compat.IPlatformCompat; import com.android.server.SystemService; @@ -150,6 +153,9 @@ public final class AppCompatOverridesService { Set packageNames = new ArraySet<>(properties.getKeyset()); packageNames.remove(FLAG_OWNED_CHANGE_IDS); packageNames.remove(FLAG_REMOVE_OVERRIDES); + Map packageNameToOverridesToAdd = new ArrayMap<>(); + Map packageNameToOverridesToRemove = + new ArrayMap<>(); for (String packageName : packageNames) { Long versionCode = getVersionCodeOrNull(packageName); if (versionCode == null) { @@ -157,11 +163,30 @@ public final class AppCompatOverridesService { continue; } - applyPackageOverrides(properties.getString(packageName, /* defaultValue= */ ""), - packageName, versionCode, ownedChangeIds, - packageToChangeIdsToSkip.getOrDefault(packageName, emptySet()), - /* removeOtherOwnedOverrides= */ true); + Set changeIdsToSkip = packageToChangeIdsToSkip.getOrDefault(packageName, + emptySet()); + Map overridesToAdd = mOverridesParser.parsePackageOverrides( + properties.getString(packageName, /* defaultValue= */ ""), packageName, + versionCode, changeIdsToSkip); + if (!overridesToAdd.isEmpty()) { + packageNameToOverridesToAdd.put(packageName, + new CompatibilityOverrideConfig(overridesToAdd)); + } + + Set overridesToRemove = new ArraySet<>(); + for (Long changeId : ownedChangeIds) { + if (!overridesToAdd.containsKey(changeId) && !changeIdsToSkip.contains(changeId)) { + overridesToRemove.add(changeId); + } + } + if (!overridesToRemove.isEmpty()) { + packageNameToOverridesToRemove.put(packageName, + new CompatibilityOverridesToRemoveConfig(overridesToRemove)); + } } + + putAllPackageOverrides(packageNameToOverridesToAdd); + removeAllPackageOverrides(packageNameToOverridesToRemove); } /** @@ -177,41 +202,14 @@ public final class AppCompatOverridesService { // We apply overrides for each namespace separately so that if there is a failure for // one namespace, the other namespaces won't be affected. Set ownedChangeIds = getOwnedChangeIds(namespace); - applyPackageOverrides( - DeviceConfig.getString(namespace, packageName, /* defaultValue= */ ""), - packageName, versionCode, ownedChangeIds, + putPackageOverrides(packageName, mOverridesParser.parsePackageOverrides( + DeviceConfig.getString(namespace, packageName, /* defaultValue= */""), + packageName, versionCode, getOverridesToRemove(namespace, ownedChangeIds).getOrDefault(packageName, - emptySet()), /* removeOtherOwnedOverrides */ false); + emptySet()))); } } - /** - * Calls {@link AppCompatOverridesParser#parsePackageOverrides} on the given arguments and adds - * the resulting overrides via {@link IPlatformCompat#putOverridesOnReleaseBuilds}. - * - *

In addition, if {@code removeOtherOwnedOverrides} is true, removes any override that - * wasn't just added, whose change ID is in {@code ownedChangeIds} but not in {@code - * changeIdsToSkip}, via {@link IPlatformCompat#removeOverridesOnReleaseBuilds}. - */ - private void applyPackageOverrides(String configStr, String packageName, long versionCode, - Set ownedChangeIds, Set changeIdsToSkip, - boolean removeOtherOwnedOverrides) { - Map overridesToAdd = mOverridesParser.parsePackageOverrides( - configStr, packageName, versionCode, changeIdsToSkip); - putPackageOverrides(packageName, overridesToAdd); - - if (!removeOtherOwnedOverrides) { - return; - } - Set overridesToRemove = new ArraySet<>(); - for (Long changeId : ownedChangeIds) { - if (!overridesToAdd.containsKey(changeId) && !changeIdsToSkip.contains(changeId)) { - overridesToRemove.add(changeId); - } - } - removePackageOverrides(packageName, overridesToRemove); - } - /** * Removes all owned overrides in all supported namespaces for the given {@code packageName}. * @@ -231,14 +229,18 @@ public final class AppCompatOverridesService { } /** - * Calls {@link IPlatformCompat#removeOverridesOnReleaseBuilds} on each package name and - * respective change IDs in {@code overridesToRemove}. + * Calls {@link IPlatformCompat#removeAllOverridesOnReleaseBuilds} on {@code + * packageNameToOverridesToRemove}. */ - private void removeOverrides(Map> overridesToRemove) { - for (Map.Entry> packageNameAndOverrides : overridesToRemove.entrySet()) { - removePackageOverrides(packageNameAndOverrides.getKey(), - packageNameAndOverrides.getValue()); + private void removeOverrides(Map> packageNameToOverridesToRemove) { + Map packageNameToConfig = + new ArrayMap<>(); + for (Map.Entry> packageNameAndChangeIds : + packageNameToOverridesToRemove.entrySet()) { + packageNameToConfig.put(packageNameAndChangeIds.getKey(), + new CompatibilityOverridesToRemoveConfig(packageNameAndChangeIds.getValue())); } + removeAllPackageOverrides(packageNameToConfig); } /** @@ -262,6 +264,20 @@ public final class AppCompatOverridesService { DeviceConfig.getString(namespace, FLAG_OWNED_CHANGE_IDS, /* defaultValue= */ "")); } + private void putAllPackageOverrides( + Map packageNameToOverrides) { + if (packageNameToOverrides.isEmpty()) { + return; + } + CompatibilityOverridesByPackageConfig config = new CompatibilityOverridesByPackageConfig( + packageNameToOverrides); + try { + mPlatformCompat.putAllOverridesOnReleaseBuilds(config); + } catch (RemoteException e) { + Slog.e(TAG, "Failed to call IPlatformCompat#putAllOverridesOnReleaseBuilds", e); + } + } + private void putPackageOverrides(String packageName, Map overridesToAdd) { if (overridesToAdd.isEmpty()) { @@ -271,7 +287,21 @@ public final class AppCompatOverridesService { try { mPlatformCompat.putOverridesOnReleaseBuilds(config, packageName); } catch (RemoteException e) { - Slog.w(TAG, "Failed to call IPlatformCompat#putOverridesOnReleaseBuilds", e); + Slog.e(TAG, "Failed to call IPlatformCompat#putOverridesOnReleaseBuilds", e); + } + } + + private void removeAllPackageOverrides( + Map packageNameToOverridesToRemove) { + if (packageNameToOverridesToRemove.isEmpty()) { + return; + } + CompatibilityOverridesToRemoveByPackageConfig config = + new CompatibilityOverridesToRemoveByPackageConfig(packageNameToOverridesToRemove); + try { + mPlatformCompat.removeAllOverridesOnReleaseBuilds(config); + } catch (RemoteException e) { + Slog.e(TAG, "Failed to call IPlatformCompat#removeAllOverridesOnReleaseBuilds", e); } } @@ -284,7 +314,7 @@ public final class AppCompatOverridesService { try { mPlatformCompat.removeOverridesOnReleaseBuilds(config, packageName); } catch (RemoteException e) { - Slog.w(TAG, "Failed to call IPlatformCompat#removeOverridesOnReleaseBuilds", e); + Slog.e(TAG, "Failed to call IPlatformCompat#removeOverridesOnReleaseBuilds", e); } } diff --git a/services/tests/mockingservicestests/src/com/android/server/compat/overrides/AppCompatOverridesServiceTest.java b/services/tests/mockingservicestests/src/com/android/server/compat/overrides/AppCompatOverridesServiceTest.java index 349da03e3f4e2..edf68166bb7d1 100644 --- a/services/tests/mockingservicestests/src/com/android/server/compat/overrides/AppCompatOverridesServiceTest.java +++ b/services/tests/mockingservicestests/src/com/android/server/compat/overrides/AppCompatOverridesServiceTest.java @@ -57,6 +57,8 @@ import androidx.test.InstrumentationRegistry; import androidx.test.filters.SmallTest; import com.android.internal.compat.CompatibilityOverrideConfig; +import com.android.internal.compat.CompatibilityOverridesByPackageConfig; +import com.android.internal.compat.CompatibilityOverridesToRemoveByPackageConfig; import com.android.internal.compat.CompatibilityOverridesToRemoveConfig; import com.android.internal.compat.IPlatformCompat; import com.android.modules.utils.testing.TestableDeviceConfig.TestableDeviceConfigRule; @@ -108,7 +110,13 @@ public class AppCompatOverridesServiceTest { @Captor private ArgumentCaptor mOverridesToAddConfigCaptor; @Captor + private ArgumentCaptor + mOverridesToAddByPackageConfigCaptor; + @Captor private ArgumentCaptor mOverridesToRemoveConfigCaptor; + @Captor + private ArgumentCaptor + mOverridesToRemoveByPackageConfigCaptor; @Rule public TestableDeviceConfigRule mDeviceConfigRule = new TestableDeviceConfigRule(); @@ -165,13 +173,19 @@ public class AppCompatOverridesServiceTest { .setString(PACKAGE_3, "123:1:9:true,123:10:11:false,123:11::true") .setString(PACKAGE_4, "").build()); + verify(mPlatformCompat).putAllOverridesOnReleaseBuilds( + mOverridesToAddByPackageConfigCaptor.capture()); + verify(mPlatformCompat).removeAllOverridesOnReleaseBuilds( + mOverridesToRemoveByPackageConfigCaptor.capture()); + Map packageNameToAddedOverrides = + mOverridesToAddByPackageConfigCaptor.getValue().packageNameToOverrides; + Map packageNameToRemovedOverrides = + mOverridesToRemoveByPackageConfigCaptor.getValue().packageNameToOverridesToRemove; Map addedOverrides; + assertThat(packageNameToAddedOverrides.keySet()).containsExactly(PACKAGE_1, PACKAGE_3); + assertThat(packageNameToRemovedOverrides.keySet()).containsExactly(PACKAGE_3, PACKAGE_4); // Package 1 - verify(mPlatformCompat).putOverridesOnReleaseBuilds(mOverridesToAddConfigCaptor.capture(), - eq(PACKAGE_1)); - verify(mPlatformCompat, never()).removeOverridesOnReleaseBuilds( - any(CompatibilityOverridesToRemoveConfig.class), eq(PACKAGE_1)); - addedOverrides = mOverridesToAddConfigCaptor.getValue().overrides; + addedOverrides = packageNameToAddedOverrides.get(PACKAGE_1).overrides; assertThat(addedOverrides).hasSize(3); assertThat(addedOverrides.get(123L)).isEqualTo( new PackageOverride.Builder().setEnabled(true).build()); @@ -179,29 +193,17 @@ public class AppCompatOverridesServiceTest { new PackageOverride.Builder().setMinVersionCode(2).setEnabled(true).build()); assertThat(addedOverrides.get(789L)).isEqualTo( new PackageOverride.Builder().setEnabled(false).build()); - // Package 2 - verify(mPlatformCompat, never()).putOverridesOnReleaseBuilds( - any(CompatibilityOverrideConfig.class), eq(PACKAGE_2)); - verify(mPlatformCompat, never()).removeOverridesOnReleaseBuilds( - any(CompatibilityOverridesToRemoveConfig.class), eq(PACKAGE_2)); // Package 3 - verify(mPlatformCompat).putOverridesOnReleaseBuilds(mOverridesToAddConfigCaptor.capture(), - eq(PACKAGE_3)); - verify(mPlatformCompat).removeOverridesOnReleaseBuilds( - mOverridesToRemoveConfigCaptor.capture(), eq(PACKAGE_3)); - addedOverrides = mOverridesToAddConfigCaptor.getValue().overrides; + addedOverrides = packageNameToAddedOverrides.get(PACKAGE_3).overrides; assertThat(addedOverrides).hasSize(1); assertThat(addedOverrides.get(123L)).isEqualTo( new PackageOverride.Builder().setMinVersionCode(10).setMaxVersionCode( 11).setEnabled(false).build()); - assertThat(mOverridesToRemoveConfigCaptor.getValue().changeIds).containsExactly(456L, 789L); - // Package 4 - verify(mPlatformCompat, never()).putOverridesOnReleaseBuilds( - any(CompatibilityOverrideConfig.class), eq(PACKAGE_4)); - verify(mPlatformCompat).removeOverridesOnReleaseBuilds( - mOverridesToRemoveConfigCaptor.capture(), eq(PACKAGE_4)); - assertThat(mOverridesToRemoveConfigCaptor.getValue().changeIds).containsExactly(123L, 456L, + assertThat(packageNameToRemovedOverrides.get(PACKAGE_3).changeIds).containsExactly(456L, 789L); + // Package 4 + assertThat(packageNameToRemovedOverrides.get(PACKAGE_4).changeIds).containsExactly(123L, + 456L, 789L); } @Test @@ -213,11 +215,15 @@ public class AppCompatOverridesServiceTest { DeviceConfig.setProperties(new Properties.Builder(NAMESPACE_1) .setString(PACKAGE_1, "123:::true").build()); - verify(mPlatformCompat).putOverridesOnReleaseBuilds(mOverridesToAddConfigCaptor.capture(), - eq(PACKAGE_1)); - verify(mPlatformCompat, never()).removeOverridesOnReleaseBuilds( - any(CompatibilityOverridesToRemoveConfig.class), eq(PACKAGE_1)); - assertThat(mOverridesToAddConfigCaptor.getValue().overrides.keySet()).containsExactly(123L); + verify(mPlatformCompat).putAllOverridesOnReleaseBuilds( + mOverridesToAddByPackageConfigCaptor.capture()); + verify(mPlatformCompat, never()).removeAllOverridesOnReleaseBuilds( + any(CompatibilityOverridesToRemoveByPackageConfig.class)); + Map packageNameToAddedOverrides = + mOverridesToAddByPackageConfigCaptor.getValue().packageNameToOverrides; + assertThat(packageNameToAddedOverrides.keySet()).containsExactly(PACKAGE_1); + assertThat(packageNameToAddedOverrides.get(PACKAGE_1).overrides.keySet()).containsExactly( + 123L); } @Test @@ -238,30 +244,28 @@ public class AppCompatOverridesServiceTest { .setString(PACKAGE_2, "123:::true") .setString(PACKAGE_3, "456:::true").build()); + verify(mPlatformCompat).putAllOverridesOnReleaseBuilds( + mOverridesToAddByPackageConfigCaptor.capture()); + verify(mPlatformCompat).removeAllOverridesOnReleaseBuilds( + mOverridesToRemoveByPackageConfigCaptor.capture()); + Map packageNameToAddedOverrides = + mOverridesToAddByPackageConfigCaptor.getValue().packageNameToOverrides; + Map packageNameToRemovedOverrides = + mOverridesToRemoveByPackageConfigCaptor.getValue().packageNameToOverridesToRemove; + assertThat(packageNameToAddedOverrides.keySet()).containsExactly(PACKAGE_1, PACKAGE_3); + assertThat(packageNameToRemovedOverrides.keySet()).containsExactly(PACKAGE_2, PACKAGE_3); // Package 1 - verify(mPlatformCompat).putOverridesOnReleaseBuilds(mOverridesToAddConfigCaptor.capture(), - eq(PACKAGE_1)); - verify(mPlatformCompat, never()).removeOverridesOnReleaseBuilds( - any(CompatibilityOverridesToRemoveConfig.class), eq(PACKAGE_1)); - assertThat(mOverridesToAddConfigCaptor.getValue().overrides.keySet()).containsExactly(789L); + assertThat(packageNameToAddedOverrides.get(PACKAGE_1).overrides.keySet()).containsExactly( + 789L); // Package 2 - verify(mPlatformCompat, never()).putOverridesOnReleaseBuilds( - any(CompatibilityOverrideConfig.class), eq(PACKAGE_2)); - verify(mPlatformCompat).removeOverridesOnReleaseBuilds( - mOverridesToRemoveConfigCaptor.capture(), eq(PACKAGE_2)); - assertThat(mOverridesToRemoveConfigCaptor.getValue().changeIds).containsExactly(456L, 789L); + assertThat(packageNameToRemovedOverrides.get(PACKAGE_2).changeIds).containsExactly(456L, + 789L); // Package 3 - verify(mPlatformCompat).putOverridesOnReleaseBuilds(mOverridesToAddConfigCaptor.capture(), - eq(PACKAGE_3)); - verify(mPlatformCompat).removeOverridesOnReleaseBuilds( - mOverridesToRemoveConfigCaptor.capture(), eq(PACKAGE_3)); - assertThat(mOverridesToAddConfigCaptor.getValue().overrides.keySet()).containsExactly(456L); - assertThat(mOverridesToRemoveConfigCaptor.getValue().changeIds).containsExactly(123L, 789L); + assertThat(packageNameToAddedOverrides.get(PACKAGE_3).overrides.keySet()).containsExactly( + 456L); + assertThat(packageNameToRemovedOverrides.get(PACKAGE_3).changeIds).containsExactly(123L, + 789L); // Package 4 (not applied because it hasn't changed after the listener was added) - verify(mPlatformCompat, never()).putOverridesOnReleaseBuilds( - any(CompatibilityOverrideConfig.class), eq(PACKAGE_4)); - verify(mPlatformCompat, never()).removeOverridesOnReleaseBuilds( - any(CompatibilityOverridesToRemoveConfig.class), eq(PACKAGE_4)); } @Test @@ -279,23 +283,28 @@ public class AppCompatOverridesServiceTest { .setString(FLAG_REMOVE_OVERRIDES, PACKAGE_1 + "=123:456," + PACKAGE_2 + "=*").build()); - // Package 1 - verify(mPlatformCompat, never()).putOverridesOnReleaseBuilds( - any(CompatibilityOverrideConfig.class), eq(PACKAGE_1)); - verify(mPlatformCompat, times(2)).removeOverridesOnReleaseBuilds( - mOverridesToRemoveConfigCaptor.capture(), eq(PACKAGE_1)); - List configs = - mOverridesToRemoveConfigCaptor.getAllValues(); + verify(mPlatformCompat, never()).putAllOverridesOnReleaseBuilds( + any(CompatibilityOverridesByPackageConfig.class)); + verify(mPlatformCompat, times(2)).removeAllOverridesOnReleaseBuilds( + mOverridesToRemoveByPackageConfigCaptor.capture()); + List configs = + mOverridesToRemoveByPackageConfigCaptor.getAllValues(); assertThat(configs.size()).isAtLeast(2); - assertThat(configs.get(configs.size() - 2).changeIds).containsExactly(123L, 456L); - assertThat(configs.get(configs.size() - 1).changeIds).containsExactly(789L); - // Package 2 - verify(mPlatformCompat, never()).putOverridesOnReleaseBuilds( - any(CompatibilityOverrideConfig.class), eq(PACKAGE_2)); - verify(mPlatformCompat).removeOverridesOnReleaseBuilds( - mOverridesToRemoveConfigCaptor.capture(), eq(PACKAGE_2)); - assertThat(mOverridesToRemoveConfigCaptor.getValue().changeIds).containsExactly(123L, 456L, + Map firstPackageNameToRemovedOverrides = + configs.get(configs.size() - 2).packageNameToOverridesToRemove; + Map secondPackageNameToRemovedOverrides = + configs.get(configs.size() - 1).packageNameToOverridesToRemove; + assertThat(firstPackageNameToRemovedOverrides.keySet()).containsExactly(PACKAGE_1, + PACKAGE_2); + assertThat(secondPackageNameToRemovedOverrides.keySet()).containsExactly(PACKAGE_1); + // Package 1 + assertThat(firstPackageNameToRemovedOverrides.get(PACKAGE_1).changeIds).containsExactly( + 123L, 456L); + assertThat(secondPackageNameToRemovedOverrides.get(PACKAGE_1).changeIds).containsExactly( 789L); + // Package 2 + assertThat(firstPackageNameToRemovedOverrides.get(PACKAGE_2).changeIds).containsExactly( + 123L, 456L, 789L); } @Test @@ -315,34 +324,42 @@ public class AppCompatOverridesServiceTest { .setString(FLAG_REMOVE_OVERRIDES, PACKAGE_2 + "=123," + PACKAGE_3 + "=789") .setString(PACKAGE_2, "123:::true").build()); + verify(mPlatformCompat).putAllOverridesOnReleaseBuilds( + mOverridesToAddByPackageConfigCaptor.capture()); + verify(mPlatformCompat, times(2)).removeAllOverridesOnReleaseBuilds( + mOverridesToRemoveByPackageConfigCaptor.capture()); + Map packageNameToAddedOverrides = + mOverridesToAddByPackageConfigCaptor.getValue().packageNameToOverrides; + List removeConfigs = + mOverridesToRemoveByPackageConfigCaptor.getAllValues(); + assertThat(removeConfigs.size()).isAtLeast(2); + Map firstPackageNameToRemovedOverrides = + removeConfigs.get(removeConfigs.size() - 2).packageNameToOverridesToRemove; + Map secondPackageNameToRemovedOverrides = + removeConfigs.get(removeConfigs.size() - 1).packageNameToOverridesToRemove; + assertThat(packageNameToAddedOverrides.keySet()).containsExactly(PACKAGE_1, PACKAGE_3); + assertThat(firstPackageNameToRemovedOverrides.keySet()).containsExactly(PACKAGE_2, + PACKAGE_3); + assertThat(secondPackageNameToRemovedOverrides.keySet()).containsExactly(PACKAGE_1, + PACKAGE_2, + PACKAGE_3); // Package 1 - verify(mPlatformCompat).putOverridesOnReleaseBuilds(mOverridesToAddConfigCaptor.capture(), - eq(PACKAGE_1)); - verify(mPlatformCompat).removeOverridesOnReleaseBuilds( - mOverridesToRemoveConfigCaptor.capture(), eq(PACKAGE_1)); - assertThat(mOverridesToAddConfigCaptor.getValue().overrides.keySet()).containsExactly(123L, - 789L); - assertThat(mOverridesToRemoveConfigCaptor.getValue().changeIds).containsExactly(456L); + assertThat(packageNameToAddedOverrides.get(PACKAGE_1).overrides.keySet()).containsExactly( + 123L, 789L); + assertThat(secondPackageNameToRemovedOverrides.get(PACKAGE_1).changeIds).containsExactly( + 456L); // Package 2 - verify(mPlatformCompat, never()).putOverridesOnReleaseBuilds( - any(CompatibilityOverrideConfig.class), eq(PACKAGE_2)); - verify(mPlatformCompat, times(2)).removeOverridesOnReleaseBuilds( - mOverridesToRemoveConfigCaptor.capture(), eq(PACKAGE_2)); - List configs = - mOverridesToRemoveConfigCaptor.getAllValues(); - assertThat(configs.size()).isAtLeast(2); - assertThat(configs.get(configs.size() - 2).changeIds).containsExactly(123L); - assertThat(configs.get(configs.size() - 1).changeIds).containsExactly(456L, 789L); + assertThat(firstPackageNameToRemovedOverrides.get(PACKAGE_2).changeIds).containsExactly( + 123L); + assertThat(secondPackageNameToRemovedOverrides.get(PACKAGE_2).changeIds).containsExactly( + 456L, 789L); // Package 3 - verify(mPlatformCompat).putOverridesOnReleaseBuilds(mOverridesToAddConfigCaptor.capture(), - eq(PACKAGE_3)); - verify(mPlatformCompat, times(2)).removeOverridesOnReleaseBuilds( - mOverridesToRemoveConfigCaptor.capture(), eq(PACKAGE_3)); - assertThat(mOverridesToAddConfigCaptor.getValue().overrides.keySet()).containsExactly(456L); - configs = mOverridesToRemoveConfigCaptor.getAllValues(); - assertThat(configs.size()).isAtLeast(2); - assertThat(configs.get(configs.size() - 2).changeIds).containsExactly(789L); - assertThat(configs.get(configs.size() - 1).changeIds).containsExactly(123L); + assertThat(packageNameToAddedOverrides.get(PACKAGE_3).overrides.keySet()).containsExactly( + 456L); + assertThat(firstPackageNameToRemovedOverrides.get(PACKAGE_3).changeIds).containsExactly( + 789L); + assertThat(secondPackageNameToRemovedOverrides.get(PACKAGE_3).changeIds).containsExactly( + 123L); } @Test @@ -362,38 +379,41 @@ public class AppCompatOverridesServiceTest { .setString(FLAG_OWNED_CHANGE_IDS, "123,456,789") .setString(PACKAGE_2, "123:::true").build()); + verify(mPlatformCompat).putAllOverridesOnReleaseBuilds( + mOverridesToAddByPackageConfigCaptor.capture()); + verify(mPlatformCompat, times(2)).removeAllOverridesOnReleaseBuilds( + mOverridesToRemoveByPackageConfigCaptor.capture()); + Map packageNameToAddedOverrides = + mOverridesToAddByPackageConfigCaptor.getValue().packageNameToOverrides; + List removeConfigs = + mOverridesToRemoveByPackageConfigCaptor.getAllValues(); + assertThat(removeConfigs.size()).isAtLeast(2); + Map firstPackageNameToRemovedOverrides = + removeConfigs.get(removeConfigs.size() - 2).packageNameToOverridesToRemove; + Map secondPackageNameToRemovedOverrides = + removeConfigs.get(removeConfigs.size() - 1).packageNameToOverridesToRemove; + assertThat(packageNameToAddedOverrides.keySet()).containsExactly(PACKAGE_2); + assertThat(firstPackageNameToRemovedOverrides.keySet()).containsExactly(PACKAGE_1); + assertThat(secondPackageNameToRemovedOverrides.keySet()).containsExactly(PACKAGE_2); // Package 1 - verify(mPlatformCompat, never()).putOverridesOnReleaseBuilds( - any(CompatibilityOverrideConfig.class), eq(PACKAGE_1)); - verify(mPlatformCompat).removeOverridesOnReleaseBuilds( - mOverridesToRemoveConfigCaptor.capture(), eq(PACKAGE_1)); - assertThat(mOverridesToRemoveConfigCaptor.getValue().changeIds).containsExactly(123L, 456L, - 789L); + assertThat(firstPackageNameToRemovedOverrides.get(PACKAGE_1).changeIds).containsExactly( + 123L, 456L, 789L); // Package 2 - verify(mPlatformCompat).putOverridesOnReleaseBuilds(mOverridesToAddConfigCaptor.capture(), - eq(PACKAGE_2)); - verify(mPlatformCompat).removeOverridesOnReleaseBuilds( - mOverridesToRemoveConfigCaptor.capture(), eq(PACKAGE_2)); - assertThat(mOverridesToAddConfigCaptor.getValue().overrides.keySet()).containsExactly(123L); - assertThat(mOverridesToRemoveConfigCaptor.getValue().changeIds).containsExactly(456L, 789L); - // Package 3 - verify(mPlatformCompat, never()).putOverridesOnReleaseBuilds( - any(CompatibilityOverrideConfig.class), eq(PACKAGE_3)); - verify(mPlatformCompat, never()).removeOverridesOnReleaseBuilds( - any(CompatibilityOverridesToRemoveConfig.class), eq(PACKAGE_3)); + assertThat(packageNameToAddedOverrides.get(PACKAGE_2).overrides.keySet()).containsExactly( + 123L); + assertThat(secondPackageNameToRemovedOverrides.get(PACKAGE_2).changeIds).containsExactly( + 456L, 789L); } @Test - public void onPropertiesChanged_platformCompatThrowsExceptionForSomeCalls_skipsFailedCalls() + public void onPropertiesChanged_platformCompatThrowsExceptionForPutCall_skipsFailedCall() throws Exception { mockGetApplicationInfo(PACKAGE_1, /* versionCode= */ 0); mockGetApplicationInfo(PACKAGE_2, /* versionCode= */ 0); mockGetApplicationInfo(PACKAGE_3, /* versionCode= */ 0); mockGetApplicationInfo(PACKAGE_4, /* versionCode= */ 0); - doThrow(new RemoteException()).when(mPlatformCompat).putOverridesOnReleaseBuilds( - any(CompatibilityOverrideConfig.class), eq(PACKAGE_2)); - doThrow(new RemoteException()).when(mPlatformCompat).removeOverridesOnReleaseBuilds( - any(CompatibilityOverridesToRemoveConfig.class), eq(PACKAGE_3)); + doThrow(new RemoteException()).when(mPlatformCompat).putAllOverridesOnReleaseBuilds( + any(CompatibilityOverridesByPackageConfig.class)); mService.registerDeviceConfigListeners(); DeviceConfig.setProperties(new Properties.Builder(NAMESPACE_1) @@ -403,26 +423,34 @@ public class AppCompatOverridesServiceTest { .setString(PACKAGE_3, "123:::true") .setString(PACKAGE_4, "123:::true").build()); - // Package 1 - verify(mPlatformCompat).putOverridesOnReleaseBuilds(any(CompatibilityOverrideConfig.class), - eq(PACKAGE_1)); - verify(mPlatformCompat).removeOverridesOnReleaseBuilds( - any(CompatibilityOverridesToRemoveConfig.class), eq(PACKAGE_1)); - // Package 2 - verify(mPlatformCompat).putOverridesOnReleaseBuilds(any(CompatibilityOverrideConfig.class), - eq(PACKAGE_2)); - verify(mPlatformCompat).removeOverridesOnReleaseBuilds( - any(CompatibilityOverridesToRemoveConfig.class), eq(PACKAGE_2)); - // Package 3 - verify(mPlatformCompat).putOverridesOnReleaseBuilds(any(CompatibilityOverrideConfig.class), - eq(PACKAGE_3)); - verify(mPlatformCompat).removeOverridesOnReleaseBuilds( - any(CompatibilityOverridesToRemoveConfig.class), eq(PACKAGE_3)); - // Package 4 - verify(mPlatformCompat).putOverridesOnReleaseBuilds(any(CompatibilityOverrideConfig.class), - eq(PACKAGE_1)); - verify(mPlatformCompat).removeOverridesOnReleaseBuilds( - any(CompatibilityOverridesToRemoveConfig.class), eq(PACKAGE_4)); + verify(mPlatformCompat).putAllOverridesOnReleaseBuilds( + any(CompatibilityOverridesByPackageConfig.class)); + verify(mPlatformCompat).removeAllOverridesOnReleaseBuilds( + any(CompatibilityOverridesToRemoveByPackageConfig.class)); + } + + @Test + public void onPropertiesChanged_platformCompatThrowsExceptionForRemoveCall_skipsFailedCall() + throws Exception { + mockGetApplicationInfo(PACKAGE_1, /* versionCode= */ 0); + mockGetApplicationInfo(PACKAGE_2, /* versionCode= */ 0); + mockGetApplicationInfo(PACKAGE_3, /* versionCode= */ 0); + mockGetApplicationInfo(PACKAGE_4, /* versionCode= */ 0); + doThrow(new RemoteException()).when(mPlatformCompat).removeAllOverridesOnReleaseBuilds( + any(CompatibilityOverridesToRemoveByPackageConfig.class)); + + mService.registerDeviceConfigListeners(); + DeviceConfig.setProperties(new Properties.Builder(NAMESPACE_1) + .setString(FLAG_OWNED_CHANGE_IDS, "123,456") + .setString(PACKAGE_1, "123:::true") + .setString(PACKAGE_2, "123:::true") + .setString(PACKAGE_3, "123:::true") + .setString(PACKAGE_4, "123:::true").build()); + + verify(mPlatformCompat).putAllOverridesOnReleaseBuilds( + any(CompatibilityOverridesByPackageConfig.class)); + verify(mPlatformCompat).removeAllOverridesOnReleaseBuilds( + any(CompatibilityOverridesToRemoveByPackageConfig.class)); } @Test