diff options
| author | 2019-09-18 17:07:21 +0100 | |
|---|---|---|
| committer | 2019-09-19 11:52:49 +0100 | |
| commit | 3143ee3f987f7c4ca6318aa644b5434b66eb053c (patch) | |
| tree | 97d8bfd8c38937d47e85d79790cb4a32d061d8c5 | |
| parent | 8c0084d980961b0eb3845e0a9a85595f74ad5a72 (diff) | |
Add methods to Rollback to retrieve and check package names.
This adds methods to the Rollback class that allow lists of package names
to be retrieved. Methods to check is a certain package is present are
also added.
This allows several cases where code in RollbackManagerServiceImpl iterated
over the list of packages in a Rollback to be replaced by Rollback method calls.
Once all such cases have been replaced, access to the list of PackageRollbackInfo
in a Rollback will be internal to Rollback, which will allow synchronisation to be
implemented within the Rollback class. It will also give a cleaner Rollback API where
the internal state of rollbacks is not exposed to other classes.
For now, synchronisation is still performed within RollbackManagerServiceImpl for
consistency. This will be changed once future CLs to move other Rollback
functionality (e.g. lifecycle functions such as commit, delete) has also been moved.
Bug: 136241838
Test: unit tests added to RollbackUnitTest
Test: atest RollbackTest
Change-Id: If00f36da4c0b51e9a579ab09e3d44f1a6370a912
3 files changed, 137 insertions, 37 deletions
diff --git a/services/core/java/com/android/server/rollback/Rollback.java b/services/core/java/com/android/server/rollback/Rollback.java index 80b18ea1b92e..2dc495197254 100644 --- a/services/core/java/com/android/server/rollback/Rollback.java +++ b/services/core/java/com/android/server/rollback/Rollback.java @@ -29,6 +29,7 @@ import java.lang.annotation.RetentionPolicy; import java.text.ParseException; import java.time.Instant; import java.util.ArrayList; +import java.util.List; /** @@ -282,6 +283,60 @@ class Rollback { mRestoreUserDataInProgress = restoreUserDataInProgress; } + /** + * Returns true if this rollback includes the package with the provided {@code packageName}. + */ + @GuardedBy("getLock") + boolean includesPackage(String packageName) { + for (PackageRollbackInfo info : info.getPackages()) { + if (info.getPackageName().equals(packageName)) { + return true; + } + } + return false; + } + + /** + * Returns true if this rollback includes the package with the provided {@code packageName} + * with a <i>version rolled back from</i> that is not {@code versionCode}. + */ + @GuardedBy("getLock") + boolean includesPackageWithDifferentVersion(String packageName, long versionCode) { + for (PackageRollbackInfo info : info.getPackages()) { + if (info.getPackageName().equals(packageName) + && info.getVersionRolledBackFrom().getLongVersionCode() != versionCode) { + return true; + } + } + return false; + } + + /** + * Returns a list containing the names of all the packages included in this rollback. + */ + @GuardedBy("getLock") + List<String> getPackageNames() { + List<String> result = new ArrayList<>(); + for (PackageRollbackInfo info : info.getPackages()) { + result.add(info.getPackageName()); + } + return result; + } + + /** + * Returns a list containing the names of all the apex packages included in this rollback. + */ + @GuardedBy("getLock") + List<String> getApexPackageNames() { + List<String> result = new ArrayList<>(); + for (PackageRollbackInfo info : info.getPackages()) { + if (info.isApex()) { + result.add(info.getPackageName()); + } + } + return result; + } + static String rollbackStateToString(@RollbackState int state) { switch (state) { case Rollback.ROLLBACK_STATE_ENABLING: return "enabling"; diff --git a/services/core/java/com/android/server/rollback/RollbackManagerServiceImpl.java b/services/core/java/com/android/server/rollback/RollbackManagerServiceImpl.java index 12a5c8bff03e..e8e448aa118e 100644 --- a/services/core/java/com/android/server/rollback/RollbackManagerServiceImpl.java +++ b/services/core/java/com/android/server/rollback/RollbackManagerServiceImpl.java @@ -550,22 +550,16 @@ class RollbackManagerServiceImpl extends IRollbackManager.Stub { while (iter.hasNext()) { Rollback rollback = iter.next(); synchronized (rollback.getLock()) { - for (PackageRollbackInfo info : rollback.info.getPackages()) { - if (info.getPackageName().equals(packageName)) { - iter.remove(); - deleteRollback(rollback); - break; - } + if (rollback.includesPackage(packageName)) { + iter.remove(); + deleteRollback(rollback); } } } for (NewRollback newRollback : mNewRollbacks) { synchronized (newRollback.rollback.getLock()) { - for (PackageRollbackInfo info : newRollback.rollback.info.getPackages()) { - if (info.getPackageName().equals(packageName)) { - newRollback.isCancelled = true; - break; - } + if (newRollback.rollback.includesPackage(packageName)) { + newRollback.isCancelled = true; } } } @@ -648,11 +642,7 @@ class RollbackManagerServiceImpl extends IRollbackManager.Stub { restoreInProgress.add(rollback); } - for (PackageRollbackInfo info : rollback.info.getPackages()) { - if (info.isApex()) { - apexPackageNames.add(info.getPackageName()); - } - } + apexPackageNames.addAll(rollback.getApexPackageNames()); } } } @@ -714,7 +704,7 @@ class RollbackManagerServiceImpl extends IRollbackManager.Stub { private void onPackageReplaced(String packageName) { // TODO: Could this end up incorrectly deleting a rollback for a // package that is about to be installed? - VersionedPackage installedVersion = getInstalledPackageVersion(packageName); + long installedVersion = getInstalledPackageVersion(packageName); synchronized (mLock) { Iterator<Rollback> iter = mRollbacks.iterator(); @@ -722,17 +712,11 @@ class RollbackManagerServiceImpl extends IRollbackManager.Stub { Rollback rollback = iter.next(); synchronized (rollback.getLock()) { // TODO: Should we remove rollbacks in the ENABLING state here? - if (rollback.isEnabling() || rollback.isAvailable()) { - for (PackageRollbackInfo info : rollback.info.getPackages()) { - if (info.getPackageName().equals(packageName) - && !packageVersionsEqual( - info.getVersionRolledBackFrom(), - installedVersion)) { - iter.remove(); - deleteRollback(rollback); - break; - } - } + if ((rollback.isEnabling() || rollback.isAvailable()) + && rollback.includesPackageWithDifferentVersion(packageName, + installedVersion)) { + iter.remove(); + deleteRollback(rollback); } } } @@ -1251,18 +1235,18 @@ class RollbackManagerServiceImpl extends IRollbackManager.Stub { /** * Gets the version of the package currently installed. - * Returns null if the package is not currently installed. + * Returns -1 if the package is not currently installed. */ - private VersionedPackage getInstalledPackageVersion(String packageName) { + private long getInstalledPackageVersion(String packageName) { PackageManager pm = mContext.getPackageManager(); PackageInfo pkgInfo = null; try { pkgInfo = getPackageInfo(packageName); } catch (PackageManager.NameNotFoundException e) { - return null; + return -1; } - return new VersionedPackage(packageName, pkgInfo.getLongVersionCode()); + return pkgInfo.getLongVersionCode(); } /** @@ -1391,11 +1375,7 @@ class RollbackManagerServiceImpl extends IRollbackManager.Stub { // should document in PackageInstaller.SessionParams#setEnableRollback // After enabling and commiting any rollback, observe packages and // prepare to rollback if packages crashes too frequently. - List<String> packages = new ArrayList<>(); - for (int i = 0; i < rollback.info.getPackages().size(); i++) { - packages.add(rollback.info.getPackages().get(i).getPackageName()); - } - mPackageHealthObserver.startObservingHealth(packages, + mPackageHealthObserver.startObservingHealth(rollback.getPackageNames(), mRollbackLifetimeDurationInMillis); scheduleExpiration(mRollbackLifetimeDurationInMillis); } diff --git a/services/tests/servicestests/src/com/android/server/rollback/RollbackUnitTest.java b/services/tests/servicestests/src/com/android/server/rollback/RollbackUnitTest.java index d27f1c7e0ce7..b5925a6e750f 100644 --- a/services/tests/servicestests/src/com/android/server/rollback/RollbackUnitTest.java +++ b/services/tests/servicestests/src/com/android/server/rollback/RollbackUnitTest.java @@ -18,11 +18,18 @@ package com.android.server.rollback; import static com.google.common.truth.Truth.assertThat; +import android.content.pm.VersionedPackage; +import android.content.rollback.PackageRollbackInfo; +import android.util.IntArray; +import android.util.SparseLongArray; + import org.junit.Test; import org.junit.runner.RunWith; import org.junit.runners.JUnit4; import java.io.File; +import java.util.ArrayList; +import java.util.Arrays; @RunWith(JUnit4.class) public class RollbackUnitTest { @@ -74,4 +81,62 @@ public class RollbackUnitTest { assertThat(rollback.isCommitted()).isTrue(); } + @Test + public void getPackageNamesAllAndJustApex() { + String pkg1 = "test.testpackage.pkg1"; + String pkg2 = "test.testpackage.pkg2"; + String pkg3 = "com.blah.hello.three"; + String pkg4 = "com.something.4pack"; + + Rollback rollback = new Rollback(123, new File("/test/testing"), -1); + PackageRollbackInfo pkgInfo1 = pkgInfoFor(pkg1, 12, 10, false); + PackageRollbackInfo pkgInfo2 = pkgInfoFor(pkg2, 12, 10, true); + PackageRollbackInfo pkgInfo3 = pkgInfoFor(pkg3, 12, 10, false); + PackageRollbackInfo pkgInfo4 = pkgInfoFor(pkg4, 12, 10, true); + + rollback.info.getPackages().addAll(Arrays.asList(pkgInfo1, pkgInfo2, pkgInfo3, pkgInfo4)); + + assertThat(rollback.getPackageNames()).containsExactly(pkg1, pkg2, pkg3, pkg4); + assertThat(rollback.getApexPackageNames()).containsExactly(pkg2, pkg4); + } + + @Test + public void includesPackages() { + String pkg1 = "test.testpackage.pkg1"; + String pkg2 = "test.testpackage.pkg2"; + String pkg3 = "com.blah.hello.three"; + String pkg4 = "com.something.4pack"; + + Rollback rollback = new Rollback(123, new File("/test/testing"), -1); + PackageRollbackInfo pkgInfo1 = pkgInfoFor(pkg1, 12, 10, false); + PackageRollbackInfo pkgInfo2 = pkgInfoFor(pkg2, 18, 12, true); + PackageRollbackInfo pkgInfo3 = pkgInfoFor(pkg3, 157, 156, false); + PackageRollbackInfo pkgInfo4 = pkgInfoFor(pkg4, 99, 1, true); + + rollback.info.getPackages().addAll(Arrays.asList(pkgInfo1, pkgInfo2, pkgInfo3, pkgInfo4)); + + assertThat(rollback.includesPackage(pkg2)).isTrue(); + assertThat(rollback.includesPackage(pkg3)).isTrue(); + assertThat(rollback.includesPackage("com.something.else")).isFalse(); + + assertThat(rollback.includesPackageWithDifferentVersion(pkg1, 12)).isFalse(); + assertThat(rollback.includesPackageWithDifferentVersion(pkg1, 1)).isTrue(); + + assertThat(rollback.includesPackageWithDifferentVersion(pkg2, 18)).isFalse(); + assertThat(rollback.includesPackageWithDifferentVersion(pkg2, 12)).isTrue(); + + assertThat(rollback.includesPackageWithDifferentVersion(pkg3, 157)).isFalse(); + assertThat(rollback.includesPackageWithDifferentVersion(pkg3, 156)).isTrue(); + assertThat(rollback.includesPackageWithDifferentVersion(pkg3, 15)).isTrue(); + + assertThat(rollback.includesPackageWithDifferentVersion(pkg4, 99)).isFalse(); + assertThat(rollback.includesPackageWithDifferentVersion(pkg4, 100)).isTrue(); + } + + private static PackageRollbackInfo pkgInfoFor( + String packageName, long fromVersion, long toVersion, boolean isApex) { + return new PackageRollbackInfo(new VersionedPackage(packageName, fromVersion), + new VersionedPackage(packageName, toVersion), + new IntArray(), new ArrayList<>(), isApex, new IntArray(), new SparseLongArray()); + } } |