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

Commit 9b1054b1 authored by Jack Yu's avatar Jack Yu
Browse files

Fixed subscription database cache out of sync

The subscription reloading from database could cause cache
to be out-of-sync. Fixed by making reloading synchronous and
only reload when necessary (e.g. Backup restore actually changes
the database).

Fix: 290295550
Test: atest SubscriptionManagerServiceTest TelephonyProviderTests
Test: Manually test backup/restore
Test: Telephony basic functionality tests
Test: Re-tested passed on b/290295550#comment#23
Change-Id: Iee0355a4069ce6d5c85ecc1bd727bc457c3bb902
parent 3a6b7ea6
Loading
Loading
Loading
Loading
+8 −8
Original line number Diff line number Diff line
@@ -2018,21 +2018,21 @@ public class SubscriptionDatabaseManager extends Handler {
    }

    /**
     * Reload the database from content provider to the cache.
     * Reload the database from content provider to the cache. This must be a synchronous operation
     * to prevent cache/database out-of-sync. Callers should be cautious to call this method because
     * it might take longer time to complete.
     */
    public void reloadDatabase() {
        if (mAsyncMode) {
            post(this::loadDatabaseInternal);
        } else {
    public void reloadDatabaseSync() {
        logl("reloadDatabaseSync");
        // Synchronously load the database into the cache.
        loadDatabaseInternal();
    }
    }

    /**
     * Load the database from content provider to the cache.
     */
    private void loadDatabaseInternal() {
        log("loadDatabaseInternal");
        logl("loadDatabaseInternal");
        try (Cursor cursor = mContext.getContentResolver().query(
                SimInfo.CONTENT_URI, null, null, null, null)) {
            mReadWriteLock.writeLock().lock();
+13 −6
Original line number Diff line number Diff line
@@ -1424,12 +1424,15 @@ public class SubscriptionManagerService extends ISub.Stub {
                    }

                    // Attempt to restore SIM specific settings when SIM is loaded.
                    mContext.getContentResolver().call(
                    Bundle result = mContext.getContentResolver().call(
                            SubscriptionManager.SIM_INFO_BACKUP_AND_RESTORE_CONTENT_URI,
                            SubscriptionManager.RESTORE_SIM_SPECIFIC_SETTINGS_METHOD_NAME,
                            iccId, null);
                    log("Reload the database.");
                    mSubscriptionDatabaseManager.reloadDatabase();
                    if (result != null && result.getBoolean(
                            SubscriptionManager.RESTORE_SIM_SPECIFIC_SETTINGS_DATABASE_UPDATED)) {
                        logl("Sim specific settings changed the database.");
                        mSubscriptionDatabaseManager.reloadDatabaseSync();
                    }
                }

                log("updateSubscription: " + mSubscriptionDatabaseManager
@@ -3711,12 +3714,16 @@ public class SubscriptionManagerService extends ISub.Stub {
            Bundle bundle = new Bundle();
            bundle.putByteArray(SubscriptionManager.KEY_SIM_SPECIFIC_SETTINGS_DATA, data);
            logl("restoreAllSimSpecificSettingsFromBackup");
            mContext.getContentResolver().call(
            Bundle result = mContext.getContentResolver().call(
                    SubscriptionManager.SIM_INFO_BACKUP_AND_RESTORE_CONTENT_URI,
                    SubscriptionManager.RESTORE_SIM_SPECIFIC_SETTINGS_METHOD_NAME,
                    null, bundle);
            // After restoring, we need to reload the content provider into the cache.
            mSubscriptionDatabaseManager.reloadDatabase();

            if (result != null && result.getBoolean(
                    SubscriptionManager.RESTORE_SIM_SPECIFIC_SETTINGS_DATABASE_UPDATED)) {
                logl("Sim specific settings changed the database.");
                mSubscriptionDatabaseManager.reloadDatabaseSync();
            }
        } finally {
            Binder.restoreCallingIdentity(token);
        }
+14 −2
Original line number Diff line number Diff line
@@ -269,6 +269,8 @@ public class SubscriptionDatabaseManagerTest extends TelephonyTest {

        private final List<String> mAllColumns;

        private boolean mDatabaseChanged;

        SubscriptionProvider() {
            mAllColumns = SimInfo.getAllColumns();
        }
@@ -378,7 +380,17 @@ public class SubscriptionDatabaseManagerTest extends TelephonyTest {

        @Override
        public Bundle call(String method, @Nullable String args, @Nullable Bundle bundle) {
            return new Bundle();
            Bundle result = new Bundle();
            if (method.equals(SubscriptionManager.RESTORE_SIM_SPECIFIC_SETTINGS_METHOD_NAME)) {
                result.putBoolean(
                        SubscriptionManager.RESTORE_SIM_SPECIFIC_SETTINGS_DATABASE_UPDATED,
                        mDatabaseChanged);
            }
            return result;
        }

        public void setRestoreDatabaseChanged(boolean changed) {
            mDatabaseChanged = changed;
        }
    }

@@ -422,7 +434,7 @@ public class SubscriptionDatabaseManagerTest extends TelephonyTest {
                .that(mDatabaseManagerUT.getSubscriptionInfoInternal(subId)).isEqualTo(subInfo);

        // Load subscription info from the database.
        mDatabaseManagerUT.reloadDatabase();
        mDatabaseManagerUT.reloadDatabaseSync();
        processAllMessages();

        // Verify the database value is same as the inserted one.
+29 −3
Original line number Diff line number Diff line
@@ -69,8 +69,10 @@ import android.annotation.NonNull;
import android.app.AppOpsManager;
import android.app.PropertyInvalidatedCache;
import android.compat.testing.PlatformCompatChangeRule;
import android.content.ContentValues;
import android.content.Intent;
import android.content.pm.PackageManager;
import android.net.Uri;
import android.os.Build;
import android.os.Bundle;
import android.os.Handler;
@@ -2215,16 +2217,40 @@ public class SubscriptionManagerServiceTest extends TelephonyTest {
    }

    @Test
    public void testRestoreAllSimSpecificSettingsFromBackup() {
    public void testRestoreAllSimSpecificSettingsFromBackup() throws Exception {
        assertThrows(SecurityException.class, ()
                -> mSubscriptionManagerServiceUT.restoreAllSimSpecificSettingsFromBackup(
                        new byte[0]));
        insertSubscription(FAKE_SUBSCRIPTION_INFO1);
        mContextFixture.addCallingOrSelfPermission(Manifest.permission.MODIFY_PHONE_STATE);

        // TODO: Briefly copy the logic from TelephonyProvider to
        //  SubscriptionDatabaseManagerTest.SubscriptionProvider

        // getSubscriptionDatabaseManager().setWifiCallingEnabled(1, 0);

        // Simulate restoration altered the database directly.
        ContentValues cvs = new ContentValues();
        cvs.put(SimInfo.COLUMN_WFC_IMS_ENABLED, 0);
        mSubscriptionProvider.update(Uri.withAppendedPath(SimInfo.CONTENT_URI, "1"), cvs, null,
                null);

        // Setting this to false to prevent database reload.
        mSubscriptionProvider.setRestoreDatabaseChanged(false);
        mSubscriptionManagerServiceUT.restoreAllSimSpecificSettingsFromBackup(
                new byte[0]);

        SubscriptionInfoInternal subInfo = mSubscriptionManagerServiceUT
                .getSubscriptionInfoInternal(1);
        // Since reload didn't happen, WFC should remains enabled.
        assertThat(subInfo.getWifiCallingEnabled()).isEqualTo(1);

        // Now the database reload should happen
        mSubscriptionProvider.setRestoreDatabaseChanged(true);
        mSubscriptionManagerServiceUT.restoreAllSimSpecificSettingsFromBackup(
                new byte[0]);

        subInfo = mSubscriptionManagerServiceUT.getSubscriptionInfoInternal(1);
        // Since reload didn't happen, WFC should remains enabled.
        assertThat(subInfo.getWifiCallingEnabled()).isEqualTo(0);
    }

    @Test