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

Commit e9aaf638 authored by Richard Uhler's avatar Richard Uhler
Browse files

Followup cleanup after refactoring rollback states. 

This CL addresses some minor comments as followup to the previous CL
"Use a single list for available and committed rollbacks".

Test: atest RollbackTest
Test: atest StagedRollbackTest
Test: atest AppDataRollbackHelperTest
Bug: 124044231
Change-Id: I6146cfac3a70017dc8d7eed6b6d3c19de932f14b
parent 6f8a33bf

services/core/java/com/android/server/rollback/AppDataRollbackHelper.java

+5 −3
Original line number Diff line number Diff line
@@ -29,9 +29,11 @@ import com.android.server.pm.Installer.InstallerException; 


import java.util.ArrayList;

import java.util.HashMap;

import java.util.HashSet;

import java.util.Iterator;

import java.util.List;

import java.util.Map;

import java.util.Set;



/**

 * Encapsulates the logic for initiating userdata snapshots and rollbacks via installd.
@@ -214,9 +216,9 @@ public class AppDataRollbackHelper { 
     * backups updates corresponding {@code changedRollbackData} with a mapping from {@code userId}

     * to a inode of theirs CE user data snapshot.

     *

     * @return a list {@code RollbackData} that have been changed and should be stored on disk.

     * @return the set of {@code RollbackData} that have been changed and should be stored on disk.

     */

    public List<RollbackData> commitPendingBackupAndRestoreForUser(int userId,

    public Set<RollbackData> commitPendingBackupAndRestoreForUser(int userId,

            List<RollbackData> rollbacks) {



        final Map<String, PackageRollbackInfo> pendingBackupPackages = new HashMap<>();
@@ -283,7 +285,7 @@ public class AppDataRollbackHelper { 
            }

        }



        final List<RollbackData> changed = pendingBackups;

        final Set<RollbackData> changed = new HashSet<>(pendingBackups);

        changed.addAll(pendingRestores);

        return changed;

    }

services/core/java/com/android/server/rollback/RollbackData.java

+6 −6
Original line number Diff line number Diff line
@@ -32,7 +32,7 @@ import java.util.ArrayList; 
 */

class RollbackData {

    @IntDef(flag = true, prefix = { "ROLLBACK_STATE_" }, value = {

            ROLLBACK_STATE_PENDING_AVAILABLE,

            ROLLBACK_STATE_ENABLING,

            ROLLBACK_STATE_AVAILABLE,

            ROLLBACK_STATE_COMMITTED,

    })
@@ -40,10 +40,10 @@ class RollbackData { 
    @interface RollbackState {}



    /**

     * The (staged) rollback will become available once the session with

     * stagedSessionId has been applied after a reboot.

     * The rollback is in the process of being enabled. It is not yet

     * available for use.

     */

    static final int ROLLBACK_STATE_PENDING_AVAILABLE = 0;

    static final int ROLLBACK_STATE_ENABLING = 0;



    /**

     * The rollback is currently available.
@@ -82,7 +82,7 @@ class RollbackData { 


    /**

     * The current state of the rollback.

     * PENDING_AVAILABLE, AVAILABLE, or COMMITTED.

     * ENABLING, AVAILABLE, or COMMITTED.

     */

    public @RollbackState int state;
@@ -114,7 +114,7 @@ class RollbackData { 
                /* committedSessionId */ -1);

        this.backupDir = backupDir;

        this.stagedSessionId = stagedSessionId;

        this.state = ROLLBACK_STATE_PENDING_AVAILABLE;

        this.state = ROLLBACK_STATE_ENABLING;

        this.timestamp = Instant.now();

    }

services/core/java/com/android/server/rollback/RollbackManagerServiceImpl.java

+21 −7
Original line number Diff line number Diff line
@@ -64,6 +64,7 @@ import java.util.Iterator; 
import java.util.List;

import java.util.Map;

import java.util.Random;

import java.util.Set;

import java.util.concurrent.LinkedBlockingQueue;

import java.util.concurrent.TimeUnit;
@@ -416,7 +417,18 @@ class RollbackManagerServiceImpl extends IRollbackManager.Stub { 
                            int status = result.getIntExtra(PackageInstaller.EXTRA_STATUS,

                                    PackageInstaller.STATUS_FAILURE);

                            if (status != PackageInstaller.STATUS_SUCCESS) {

                                // Committing the rollback failed, but we

                                // still have all the info we need to try

                                // rolling back again, so restore the rollback

                                // state to how it was before we tried

                                // committing.

                                // TODO: Should we just kill this rollback if

                                // commit failed? Why would we expect commit

                                // not to fail again?

                                synchronized (mLock) {

                                    // TODO: Could this cause a rollback to be

                                    // resurrected if it should otherwise have

                                    // expired by now?

                                    data.state = RollbackData.ROLLBACK_STATE_AVAILABLE;

                                    data.restoreUserDataInProgress = false;

                                }
@@ -509,7 +521,7 @@ class RollbackManagerServiceImpl extends IRollbackManager.Stub { 
                rollbacks = new ArrayList<>(mRollbacks);

            }



            final List<RollbackData> changed =

            final Set<RollbackData> changed =

                    mAppDataRollbackHelper.commitPendingBackupAndRestoreForUser(userId, rollbacks);



            for (RollbackData rd : changed) {
@@ -540,14 +552,14 @@ class RollbackManagerServiceImpl extends IRollbackManager.Stub { 
        getHandler().post(() -> {

            // Check to see if any rollback-enabled staged sessions or staged

            // rollback sessions been applied.

            List<RollbackData> pendingAvailable = new ArrayList<>();

            List<RollbackData> enabling = new ArrayList<>();

            List<RollbackData> restoreInProgress = new ArrayList<>();

            synchronized (mLock) {

                ensureRollbackDataLoadedLocked();

                for (RollbackData data : mRollbacks) {

                    if (data.isStaged()) {

                        if (data.state == RollbackData.ROLLBACK_STATE_PENDING_AVAILABLE) {

                            pendingAvailable.add(data);

                        if (data.state == RollbackData.ROLLBACK_STATE_ENABLING) {

                            enabling.add(data);

                        } else if (data.restoreUserDataInProgress) {

                            restoreInProgress.add(data);

                        }
@@ -555,10 +567,11 @@ class RollbackManagerServiceImpl extends IRollbackManager.Stub { 
                }

            }



            for (RollbackData data : pendingAvailable) {

            for (RollbackData data : enabling) {

                PackageInstaller installer = mContext.getPackageManager().getPackageInstaller();

                PackageInstaller.SessionInfo session = installer.getSessionInfo(

                        data.stagedSessionId);

                // TODO: What if session is null?

                if (session != null) {

                    if (session.isStagedSessionApplied()) {

                        makeRollbackAvailable(data);
@@ -576,12 +589,12 @@ class RollbackManagerServiceImpl extends IRollbackManager.Stub { 
                PackageInstaller installer = mContext.getPackageManager().getPackageInstaller();

                PackageInstaller.SessionInfo session = installer.getSessionInfo(

                        data.stagedSessionId);

                // TODO: What if session is null?

                if (session != null) {

                    if (session.isStagedSessionApplied() || session.isStagedSessionFailed()) {

                        synchronized (mLock) {

                            data.restoreUserDataInProgress = false;

                        }

                        mRollbackStore.deletePackageCodePaths(data);

                        saveRollbackData(data);

                    }

                }
@@ -640,8 +653,9 @@ class RollbackManagerServiceImpl extends IRollbackManager.Stub { 
            Iterator<RollbackData> iter = mRollbacks.iterator();

            while (iter.hasNext()) {

                RollbackData data = iter.next();

                // TODO: Should we remove rollbacks in the ENABLING state here?

                if (data.state == RollbackData.ROLLBACK_STATE_AVAILABLE

                        || data.state == RollbackData.ROLLBACK_STATE_PENDING_AVAILABLE) {

                        || data.state == RollbackData.ROLLBACK_STATE_ENABLING) {

                    for (PackageRollbackInfo info : data.info.getPackages()) {

                        if (info.getPackageName().equals(packageName)

                                && !packageVersionsEqual(

services/core/java/com/android/server/rollback/RollbackStore.java

+2 −2
Original line number Diff line number Diff line
@@ -404,7 +404,7 @@ class RollbackStore { 


    private static String rollbackStateToString(@RollbackData.RollbackState int state) {

        switch (state) {

            case RollbackData.ROLLBACK_STATE_PENDING_AVAILABLE: return "pending_available";

            case RollbackData.ROLLBACK_STATE_ENABLING: return "enabling";

            case RollbackData.ROLLBACK_STATE_AVAILABLE: return "available";

            case RollbackData.ROLLBACK_STATE_COMMITTED: return "committed";

        }
@@ -414,7 +414,7 @@ class RollbackStore { 
    private static @RollbackData.RollbackState int rollbackStateFromString(String state)

            throws ParseException {

        switch (state) {

            case "pending_available": return RollbackData.ROLLBACK_STATE_PENDING_AVAILABLE;

            case "enabling": return RollbackData.ROLLBACK_STATE_ENABLING;

            case "available": return RollbackData.ROLLBACK_STATE_AVAILABLE;

            case "committed": return RollbackData.ROLLBACK_STATE_COMMITTED;

        }

services/tests/servicestests/src/com/android/server/rollback/AppDataRollbackHelperTest.java

+6 −6
Original line number Diff line number Diff line
@@ -45,7 +45,7 @@ import org.mockito.Mockito; 
import java.io.File;

import java.util.ArrayList;

import java.util.Arrays;

import java.util.List;

import java.util.Set;



@RunWith(JUnit4.class)

public class AppDataRollbackHelperTest {
@@ -249,7 +249,7 @@ public class AppDataRollbackHelperTest { 
        dataForRestore.info.getPackages().add(pendingRestore);

        dataForRestore.info.getPackages().add(wasRecentlyRestored);



        List<RollbackData> changed = helper.commitPendingBackupAndRestoreForUser(37,

        Set<RollbackData> changed = helper.commitPendingBackupAndRestoreForUser(37,

                Arrays.asList(dataWithPendingBackup, dataWithRecentRestore, dataForDifferentUser,

                    dataForRestore));

        InOrder inOrder = Mockito.inOrder(installer);
@@ -265,10 +265,10 @@ public class AppDataRollbackHelperTest { 
        assertEquals(53, pendingBackup.getCeSnapshotInodes().get(37));



        // Check that changed returns correct RollbackData.

        // TODO: Figure out what this should return.

        // assertEquals(2, changed.size());

        // assertEquals(dataWithPendingBackup, changed.get(0));

        // assertEquals(dataWithRecentRestore, changed.get(1));

        assertEquals(3, changed.size());

        assertTrue(changed.contains(dataWithPendingBackup));

        assertTrue(changed.contains(dataWithRecentRestore));

        assertTrue(changed.contains(dataForRestore));



        // Check that restore was performed.

        inOrder.verify(installer).restoreAppDataSnapshot(