Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 9a843fba authored by tomnatan's avatar tomnatan
Browse files

Automatically remove other owned overrides when package override flag changes 

At the moment we are using remove entries in the package override flags to indicate that an override is no longer needed, this changes removes that logic and instead adds support for automatically removing overrides whose change ID is in the owned_changed_ids flag but not in the package override flag each time that flag changes.

Bug: 201058202
Test: atest FrameworksMockingServicesTests:AppCompatOverridesParserTest
Test: atest FrameworksMockingServicesTests:AppCompatOverridesServiceTest
Change-Id: I064e726e5291556ebdfddba6b2a8b1492068d74b
parent 66889d32

services/core/java/com/android/server/compat/overrides/AppCompatOverridesParser.java

+7 −53
Original line number Diff line number Diff line
@@ -175,29 +175,25 @@ final class AppCompatOverridesParser { 


    /**

     * Parses the given {@code configStr}, that is expected to be a comma separated list of changes

     * overrides, and returns a {@link PackageOverrides}.

     * overrides, and returns a map from change ID to {@link PackageOverride} instances to add.

     *

     * <p>Each change override is in the following format:

     * '<change-id>:<min-version-code?>:<max-version-code?>:<enabled?>'. If <enabled> is empty,

     * this indicates that any override for the specified change ID should be removed.

     * '<change-id>:<min-version-code?>:<max-version-code?>:<enabled>'.

     *

     * <p>If there are multiple overrides that should be added with the same change ID, the one

     * that best fits the given {@code versionCode} is added.

     *

     * <p>Any overrides whose change ID is in {@code changeIdsToSkip} are ignored.

     *

     * <p>If a change override entry in {@code configStr} is invalid, it will be ignored. If the

     * same change ID is both added and removed, i.e., has a change override entry with an empty

     * enabled and another with a non-empty enabled, the change ID will only be removed.

     * <p>If a change override entry in {@code configStr} is invalid, it will be ignored.

     */

    static PackageOverrides parsePackageOverrides(

            String configStr, long versionCode, Set<Long> changeIdsToSkip) {

    static Map<Long, PackageOverride> parsePackageOverrides(String configStr, long versionCode,

            Set<Long> changeIdsToSkip) {

        if (configStr.isEmpty()) {

            return new PackageOverrides();

            return emptyMap();

        }

        PackageOverrideComparator comparator = new PackageOverrideComparator(versionCode);

        Map<Long, PackageOverride> overridesToAdd = new ArrayMap<>();

        Set<Long> overridesToRemove = new ArraySet<>();

        for (String overrideEntryString : configStr.split(",")) {

            List<String> changeIdAndVersions = Arrays.asList(overrideEntryString.split(":", 4));

            if (changeIdAndVersions.size() != 4) {
@@ -220,16 +216,6 @@ final class AppCompatOverridesParser { 
            String maxVersionCodeStr = changeIdAndVersions.get(2);



            String enabledStr = changeIdAndVersions.get(3);

            if (enabledStr.isEmpty()) {

                if (!minVersionCodeStr.isEmpty() || !maxVersionCodeStr.isEmpty()) {

                    Slog.w(

                            TAG,

                            "min/max version code should be empty if enabled is empty: "

                                    + overrideEntryString);

                }

                overridesToRemove.add(changeId);

                continue;

            }

            if (!BOOLEAN_PATTERN.matcher(enabledStr).matches()) {

                Slog.w(TAG, "Invalid enabled string in override entry: " + overrideEntryString);

                continue;
@@ -262,39 +248,7 @@ final class AppCompatOverridesParser { 
            }

        }



        for (Long changeId : overridesToRemove) {

            if (overridesToAdd.containsKey(changeId)) {

                Slog.w(

                        TAG,

                        "Change ID ["

                                + changeId

                                + "] is both added and removed in package override flag: "

                                + configStr);

                overridesToAdd.remove(changeId);

            }

        }



        return new PackageOverrides(overridesToAdd, overridesToRemove);

    }



    /**

     * A container for a map from change ID to {@link PackageOverride} to add and a set of change

     * IDs to remove overrides for.

     *

     * <p>The map of overrides to add and the set of overrides to remove are mutually exclusive.

     */

    static final class PackageOverrides {

        public final Map<Long, PackageOverride> overridesToAdd;

        public final Set<Long> overridesToRemove;



        PackageOverrides() {

            this(emptyMap(), emptySet());

        }



        PackageOverrides(Map<Long, PackageOverride> overridesToAdd, Set<Long> overridesToRemove) {

            this.overridesToAdd = overridesToAdd;

            this.overridesToRemove = overridesToRemove;

        }

        return overridesToAdd;

    }



    /**

services/core/java/com/android/server/compat/overrides/AppCompatOverridesService.java

+51 −29
Original line number Diff line number Diff line
@@ -49,7 +49,6 @@ import com.android.internal.compat.CompatibilityOverrideConfig; 
import com.android.internal.compat.CompatibilityOverridesToRemoveConfig;

import com.android.internal.compat.IPlatformCompat;

import com.android.server.SystemService;

import com.android.server.compat.overrides.AppCompatOverridesParser.PackageOverrides;



import java.util.ArrayList;

import java.util.Arrays;
@@ -128,20 +127,25 @@ public final class AppCompatOverridesService { 
    }



    /**

     * Same as {@link #applyOverrides(Properties, Map)} except all properties of the given {@code

     * namespace} are fetched via {@link DeviceConfig#getProperties}.

     * Same as {@link #applyOverrides(Properties, Set, Map)} except all properties of the given

     * {@code namespace} are fetched via {@link DeviceConfig#getProperties}.

     */

    private void applyAllOverrides(String namespace,

    private void applyAllOverrides(String namespace, Set<Long> ownedChangeIds,

            Map<String, Set<Long>> packageToChangeIdsToSkip) {

        applyOverrides(DeviceConfig.getProperties(namespace), packageToChangeIdsToSkip);

        applyOverrides(DeviceConfig.getProperties(namespace), ownedChangeIds,

                packageToChangeIdsToSkip);

    }



    /**

     * Iterates all package override flags in the given {@code properties}, and for each flag whose

     * package is installed on the device, parses its value and applies the overrides in it with

     * package is installed on the device, parses its value and adds the overrides in it with

     * respect to the package's current installed version.

     *

     * <p>In addition, for each package, removes any override that wasn't just added, whose change

     * ID is in {@code ownedChangeIds} but not in the respective set in {@code

     * packageToChangeIdsToSkip}.

     */

    private void applyOverrides(Properties properties,

    private void applyOverrides(Properties properties, Set<Long> ownedChangeIds,

            Map<String, Set<Long>> packageToChangeIdsToSkip) {

        Set<String> packageNames = new ArraySet<>(properties.getKeyset());

        packageNames.remove(FLAG_OWNED_CHANGE_IDS);
@@ -154,15 +158,16 @@ public final class AppCompatOverridesService { 
            }



            applyPackageOverrides(properties.getString(packageName, /* defaultValue= */ ""),

                    packageName, versionCode,

                    packageToChangeIdsToSkip.getOrDefault(packageName, emptySet()));

                    packageName, versionCode, ownedChangeIds,

                    packageToChangeIdsToSkip.getOrDefault(packageName, emptySet()),

                    /* removeOtherOwnedOverrides= */ true);

        }

    }



    /**

     * Applies all overrides in all supported namespaces for the given {@code packageName}.

     * Adds all overrides in all supported namespaces for the given {@code packageName}.

     */

    private void applyAllPackageOverrides(String packageName) {

    private void addAllPackageOverrides(String packageName) {

        Long versionCode = getVersionCodeOrNull(packageName);

        if (versionCode == null) {

            return;
@@ -171,26 +176,40 @@ public final class AppCompatOverridesService { 
        for (String namespace : mSupportedNamespaces) {

            // 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<Long> ownedChangeIds = getOwnedChangeIds(namespace);

            applyPackageOverrides(

                    DeviceConfig.getString(namespace, packageName, /* defaultValue= */ ""),

                    packageName, versionCode,

                    getOverridesToRemove(namespace).getOrDefault(packageName, emptySet()));

                    packageName, versionCode, ownedChangeIds,

                    getOverridesToRemove(namespace, ownedChangeIds).getOrDefault(packageName,

                            emptySet()), /* removeOtherOwnedOverrides */ false);

        }

    }



    /**

     * Calls {@link AppCompatOverridesParser#parsePackageOverrides} on the given arguments, adds the

     * resulting {@link PackageOverrides#overridesToAdd} via {@link

     * IPlatformCompat#putOverridesOnReleaseBuilds}, and removes the resulting {@link

     * PackageOverrides#overridesToRemove} via {@link

     * IPlatformCompat#removeOverridesOnReleaseBuilds}.

     * Calls {@link AppCompatOverridesParser#parsePackageOverrides} on the given arguments and adds

     * the resulting overrides via {@link IPlatformCompat#putOverridesOnReleaseBuilds}.

     *

     * <p>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<Long> changeIdsToSkip) {

        PackageOverrides packageOverrides = AppCompatOverridesParser.parsePackageOverrides(

    private void applyPackageOverrides(String configStr, String packageName, long versionCode,

            Set<Long> ownedChangeIds, Set<Long> changeIdsToSkip,

            boolean removeOtherOwnedOverrides) {

        Map<Long, PackageOverride> overridesToAdd = AppCompatOverridesParser.parsePackageOverrides(

                configStr, versionCode, changeIdsToSkip);

        putPackageOverrides(packageName, packageOverrides.overridesToAdd);

        removePackageOverrides(packageName, packageOverrides.overridesToRemove);

        putPackageOverrides(packageName, overridesToAdd);



        if (!removeOtherOwnedOverrides) {

            return;

        }

        Set<Long> overridesToRemove = new ArraySet<>();

        for (Long changeId : ownedChangeIds) {

            if (!overridesToAdd.containsKey(changeId) && !changeIdsToSkip.contains(changeId)) {

                overridesToRemove.add(changeId);

            }

        }

        removePackageOverrides(packageName, overridesToRemove);

    }



    /**
@@ -227,10 +246,11 @@ public final class AppCompatOverridesService { 
     * {@code namespace} and parses it into a map from package name to a set of change IDs to

     * remove for that package.

     */

    private Map<String, Set<Long>> getOverridesToRemove(String namespace) {

    private Map<String, Set<Long>> getOverridesToRemove(String namespace,

            Set<Long> ownedChangeIds) {

        return mOverridesParser.parseRemoveOverrides(

                DeviceConfig.getString(namespace, FLAG_REMOVE_OVERRIDES, /* defaultValue= */ ""),

                getOwnedChangeIds(namespace));

                ownedChangeIds);

    }



    /**
@@ -333,7 +353,9 @@ public final class AppCompatOverridesService { 
            boolean ownedChangedIdsFlagChanged = properties.getKeyset().contains(

                    FLAG_OWNED_CHANGE_IDS);



            Map<String, Set<Long>> overridesToRemove = getOverridesToRemove(mNamespace);

            Set<Long> ownedChangeIds = getOwnedChangeIds(mNamespace);

            Map<String, Set<Long>> overridesToRemove = getOverridesToRemove(mNamespace,

                    ownedChangeIds);

            if (removeOverridesFlagChanged || ownedChangedIdsFlagChanged) {

                // In both cases it's possible that overrides that weren't removed before should

                // now be removed.
@@ -343,9 +365,9 @@ public final class AppCompatOverridesService { 
            if (removeOverridesFlagChanged) {

                // We need to re-apply all overrides in the namespace since the remove overrides

                // flag might have blocked some of them from being applied before.

                applyAllOverrides(mNamespace, overridesToRemove);

                applyAllOverrides(mNamespace, ownedChangeIds, overridesToRemove);

            } else {

                applyOverrides(properties, overridesToRemove);

                applyOverrides(properties, ownedChangeIds, overridesToRemove);

            }

        }

    }
@@ -392,7 +414,7 @@ public final class AppCompatOverridesService { 
            switch (action) {

                case ACTION_PACKAGE_ADDED:

                case ACTION_PACKAGE_CHANGED:

                    applyAllPackageOverrides(packageName);

                    addAllPackageOverrides(packageName);

                    break;

                case ACTION_PACKAGE_REMOVED:

                    if (!isInstalledForAnyUser(packageName)) {

services/tests/mockingservicestests/src/com/android/server/compat/overrides/AppCompatOverridesParserTest.java

+38 −71
Original line number Diff line number Diff line
@@ -31,8 +31,6 @@ import android.util.ArraySet; 


import androidx.test.filters.SmallTest;



import com.android.server.compat.overrides.AppCompatOverridesParser.PackageOverrides;



import org.junit.Before;

import org.junit.Test;

import org.junit.runner.RunWith;
@@ -183,115 +181,84 @@ public class AppCompatOverridesParserTest { 
    }



    @Test

    public void parsePackageOverrides_emptyConfig_returnsEmpty() {

        PackageOverrides result = AppCompatOverridesParser.parsePackageOverrides(/* configStr= */

                "", /* versionCode= */ 0, /* changeIdsToSkip= */ emptySet());

    public void parsePackageOverrides_emptyConfigNoOwnedChangeIds_returnsEmpty() {

        Map<Long, PackageOverride> result = AppCompatOverridesParser.parsePackageOverrides(

                /* configStr= */ "", /* versionCode= */ 0, /* changeIdsToSkip= */ emptySet());



        assertThat(result.overridesToAdd).isEmpty();

        assertThat(result.overridesToRemove).isEmpty();

        assertThat(result).isEmpty();

    }



    @Test

    public void parsePackageOverrides_configWithSingleOverride_returnsOverride() {

        PackageOverrides result = AppCompatOverridesParser.parsePackageOverrides(/* configStr= */

                "123:::true", /* versionCode= */ 5, /* changeIdsToSkip= */ emptySet());

        Map<Long, PackageOverride> result = AppCompatOverridesParser.parsePackageOverrides(

                /* configStr= */ "123:::true", /* versionCode= */ 5, /* changeIdsToSkip= */

                emptySet());



        assertThat(result.overridesToAdd).hasSize(1);

        assertThat(result.overridesToAdd.get(123L)).isEqualTo(

        assertThat(result).hasSize(1);

        assertThat(result.get(123L)).isEqualTo(

                new PackageOverride.Builder().setEnabled(true).build());

    }



    @Test

    public void parsePackageOverrides_configWithMultipleOverridesToAdd_returnsOverrides() {

        PackageOverrides result = AppCompatOverridesParser.parsePackageOverrides(/* configStr= */

                "910:3:4:false,78:10::false,12:::false,34:1:2:true,34:10::true,56::2:true,"

                        + "56:3:4:false,34:4:8:true,78:6:7:true,910:5::true,1112::5:true,"

                        + "56:6::true,1112:6:7:false", /* versionCode= */

    public void parsePackageOverrides_configWithMultipleOverrides_returnsOverrides() {

        Map<Long, PackageOverride> result = AppCompatOverridesParser.parsePackageOverrides(

                /* configStr= */ "910:3:4:false,78:10::false,12:::false,34:1:2:true,34:10::true,"

                        + "56::2:true,56:3:4:false,34:4:8:true,78:6:7:true,910:5::true,"

                        + "1112::5:true,56:6::true,1112:6:7:false", /* versionCode= */

                5, /* changeIdsToSkip= */ emptySet());



        assertThat(result.overridesToAdd).hasSize(6);

        assertThat(result.overridesToAdd.get(12L)).isEqualTo(

        assertThat(result).hasSize(6);

        assertThat(result.get(12L)).isEqualTo(

                new PackageOverride.Builder().setEnabled(false).build());

        assertThat(result.overridesToAdd.get(34L)).isEqualTo(

        assertThat(result.get(34L)).isEqualTo(

                new PackageOverride.Builder().setMinVersionCode(4).setMaxVersionCode(8).setEnabled(

                        true).build());

        assertThat(result.overridesToAdd.get(56L)).isEqualTo(

        assertThat(result.get(56L)).isEqualTo(

                new PackageOverride.Builder().setMinVersionCode(3).setMaxVersionCode(4).setEnabled(

                        false).build());

        assertThat(result.overridesToAdd.get(78L)).isEqualTo(

        assertThat(result.get(78L)).isEqualTo(

                new PackageOverride.Builder().setMinVersionCode(6).setMaxVersionCode(7).setEnabled(

                        true).build());

        assertThat(result.overridesToAdd.get(910L)).isEqualTo(

        assertThat(result.get(910L)).isEqualTo(

                new PackageOverride.Builder().setMinVersionCode(5).setEnabled(true).build());

        assertThat(result.overridesToAdd.get(1112L)).isEqualTo(

        assertThat(result.get(1112L)).isEqualTo(

                new PackageOverride.Builder().setMaxVersionCode(5).setEnabled(true).build());

        assertThat(result.overridesToRemove).isEmpty();

    }



    @Test

    public void parsePackageOverrides_configWithMultipleOverridesToRemove_returnsOverrides() {

        PackageOverrides result = AppCompatOverridesParser.parsePackageOverrides(/* configStr= */

                "12:::,34:1:2:", /* versionCode= */ 5, /* changeIdsToSkip= */ emptySet());



        assertThat(result.overridesToRemove).containsExactly(12L, 34L);

        assertThat(result.overridesToAdd).isEmpty();

    }



    @Test

    public void parsePackageOverrides_configWithBothOverridesToAddAndRemove_returnsOverrides() {

        // Note that change 56 is both added and removed, therefore it will only be removed.

        PackageOverrides result = AppCompatOverridesParser.parsePackageOverrides(/* configStr= */

                "56:::,12:::true,34:::,56:3:7:true", /* versionCode= */ 5, /* changeIdsToSkip= */

                emptySet());



        assertThat(result.overridesToAdd).hasSize(1);

        assertThat(result.overridesToAdd.get(12L)).isEqualTo(

                new PackageOverride.Builder().setEnabled(true).build());

        assertThat(result.overridesToRemove).containsExactly(34L, 56L);

    }



    @Test

    public void parsePackageOverrides_changeIdsToSkipSpecified_returnsWithoutChangeIdsToSkip() {

        ArraySet<Long> changeIdsToSkip = new ArraySet<>();

        changeIdsToSkip.add(34L);

        changeIdsToSkip.add(56L);

        changeIdsToSkip.add(910L);

        PackageOverrides result = AppCompatOverridesParser.parsePackageOverrides(/* configStr= */

                "12:::true,34:::,56:3:7:true,78:::", /* versionCode= */ 5, changeIdsToSkip);



        assertThat(result.overridesToAdd).hasSize(1);

        assertThat(result.overridesToAdd.get(12L)).isEqualTo(

        ArraySet<Long> changeIdsToSkip = new ArraySet<>(Arrays.asList(34L, 56L));

        Map<Long, PackageOverride> result = AppCompatOverridesParser.parsePackageOverrides(

                /* configStr= */ "12:::true,56:3:7:true", /* versionCode= */ 5, changeIdsToSkip);



        assertThat(result).hasSize(1);

        assertThat(result.get(12L)).isEqualTo(

                new PackageOverride.Builder().setEnabled(true).build());

        assertThat(result.overridesToRemove).containsExactly(78L);

    }



    @Test

    public void parsePackageOverrides_changeIdsToSkipContainsAllIds_returnsEmpty() {

        ArraySet<Long> changeIdsToSkip = new ArraySet<>();

        changeIdsToSkip.add(12L);

        changeIdsToSkip.add(34L);

        PackageOverrides result = AppCompatOverridesParser.parsePackageOverrides(/* configStr= */

                "12:::true,34:::", /* versionCode= */ 5, changeIdsToSkip);



        assertThat(result.overridesToAdd).isEmpty();

        assertThat(result.overridesToRemove).isEmpty();

        ArraySet<Long> changeIdsToSkip = new ArraySet<>(Arrays.asList(12L, 34L));

        Map<Long, PackageOverride> result = AppCompatOverridesParser.parsePackageOverrides(

                /* configStr= */ "12:::true", /* versionCode= */ 5, changeIdsToSkip);



        assertThat(result).isEmpty();

    }



    @Test

    public void parsePackageOverrides_someOverridesAreInvalid_returnsWithoutInvalidOverrides() {

        // We add a valid entry before and after the invalid ones to make sure they are applied.

        PackageOverrides result = AppCompatOverridesParser.parsePackageOverrides(/* configStr= */

                "12:::True,56:1:2:FALSE,56:3:true,78:4:8:true:,C1:::true,910:::no,"

                        + "1112:1:ten:true,1112:one:10:true,,1314:7:3:false,34:one:ten:",

        Map<Long, PackageOverride> result = AppCompatOverridesParser.parsePackageOverrides(

                /* configStr= */ "12:::True,56:1:2:FALSE,56:3:true,78:4:8:true:,C1:::true,910:::no,"

                        + "1112:1:ten:true,1112:one:10:true,,1314:7:3:false,34:::",

                /* versionCode= */ 5, /* changeIdsToSkip= */ emptySet());



        assertThat(result.overridesToAdd).hasSize(2);

        assertThat(result.overridesToAdd.get(12L)).isEqualTo(

        assertThat(result).hasSize(2);

        assertThat(result.get(12L)).isEqualTo(

                new PackageOverride.Builder().setEnabled(true).build());

        assertThat(result.overridesToAdd.get(56L)).isEqualTo(

        assertThat(result.get(56L)).isEqualTo(

                new PackageOverride.Builder().setMinVersionCode(1).setMaxVersionCode(2).setEnabled(

                        false).build());

        assertThat(result.overridesToRemove).containsExactly(34L);

    }



    private static ApplicationInfo createAppInfo(String packageName) {

services/tests/mockingservicestests/src/com/android/server/compat/overrides/AppCompatOverridesServiceTest.java

+92 −54

File changed.

Preview size limit exceeded, changes collapsed.