Locking changes in ProcessedPackagesJournal
A follow-up CL to apply the code feedback from the first one.
Bug: 36850431
Test: adb shell am instrument -w -e package com.android.server.backup com.android.frameworks.servicestests/android.support.test.runner.AndroidJUnitRunner
Change-Id: I8785eb358658757b329ac871194243c47e6d87a9
diff --git a/services/backup/java/com/android/server/backup/AppsBackedUpOnThisDeviceJournal.java b/services/backup/java/com/android/server/backup/AppsBackedUpOnThisDeviceJournal.java
deleted file mode 100644
index c942bb2..0000000
--- a/services/backup/java/com/android/server/backup/AppsBackedUpOnThisDeviceJournal.java
+++ /dev/null
@@ -1,125 +0,0 @@
-/*
- * Copyright (C) 2017 The Android Open Source Project
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- * http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License
- */
-
-package com.android.server.backup;
-
-import static com.android.server.backup.RefactoredBackupManagerService.DEBUG;
-
-import android.util.Slog;
-
-import com.android.internal.annotations.GuardedBy;
-
-import java.io.EOFException;
-import java.io.File;
-import java.io.IOException;
-import java.io.RandomAccessFile;
-import java.util.HashSet;
-
-/**
- * Records which apps have been backed up on this device, persisting it to disk so that it can be
- * read at subsequent boots. This class is threadsafe.
- *
- * <p>This is used to decide, when restoring a package at install time, whether it has been
- * previously backed up on the current device. If it has been previously backed up it should
- * restore from the same restore set that the current device has been backing up to. If it has not
- * been previously backed up, it should restore from the ancestral restore set (i.e., the restore
- * set that the user's previous device was backing up to).
- *
- * <p>NB: this is always backed by the same files within the state directory supplied at
- * construction.
- */
-final class AppsBackedUpOnThisDeviceJournal {
- private static final String TAG = "AppsBackedUpOnThisDeviceJournal";
- private static final String JOURNAL_FILE_NAME = "processed";
-
- @GuardedBy("this")
- private final HashSet<String> mProcessedPackages = new HashSet<>();
- private final File mStateDirectory;
-
- /**
- * Constructs a new journal, loading state from disk if it has been previously persisted.
- *
- * @param stateDirectory The directory in which backup state (including journals) is stored.
- */
- AppsBackedUpOnThisDeviceJournal(File stateDirectory) {
- mStateDirectory = stateDirectory;
- loadFromDisk();
- }
-
- /**
- * Returns {@code true} if {@code packageName} has previously been backed up.
- */
- synchronized boolean hasBeenProcessed(String packageName) {
- return mProcessedPackages.contains(packageName);
- }
-
- synchronized void addPackage(String packageName) {
- if (!mProcessedPackages.add(packageName)) {
- // This package has already been processed - no need to add it to the journal.
- return;
- }
-
- File journalFile = new File(mStateDirectory, JOURNAL_FILE_NAME);
-
- try (RandomAccessFile out = new RandomAccessFile(journalFile, "rws")) {
- out.seek(out.length());
- out.writeUTF(packageName);
- } catch (IOException e) {
- Slog.e(TAG, "Can't log backup of " + packageName + " to " + journalFile);
- }
- }
-
- /**
- * A copy of the current state of the journal.
- *
- * <p>Used only for dumping out information for logging. {@link #hasBeenProcessed(String)}
- * should be used for efficiently checking whether a package has been backed up before by this
- * device.
- *
- * @return The current set of packages that have been backed up previously.
- */
- synchronized HashSet<String> getPackagesCopy() {
- return new HashSet<>(mProcessedPackages);
- }
-
- synchronized void reset() {
- mProcessedPackages.clear();
- File journalFile = new File(mStateDirectory, JOURNAL_FILE_NAME);
- journalFile.delete();
- }
-
- private void loadFromDisk() {
- File journalFile = new File(mStateDirectory, JOURNAL_FILE_NAME);
-
- if (!journalFile.exists()) {
- return;
- }
-
- try (RandomAccessFile oldJournal = new RandomAccessFile(journalFile, "r")) {
- while (true) {
- String packageName = oldJournal.readUTF();
- if (DEBUG) {
- Slog.v(TAG, " + " + packageName);
- }
- mProcessedPackages.add(packageName);
- }
- } catch (EOFException e) {
- // Successfully loaded journal file
- } catch (IOException e) {
- Slog.e(TAG, "Error reading processed packages journal", e);
- }
- }
-}
diff --git a/services/backup/java/com/android/server/backup/ProcessedPackagesJournal.java b/services/backup/java/com/android/server/backup/ProcessedPackagesJournal.java
new file mode 100644
index 0000000..187d5d9
--- /dev/null
+++ b/services/backup/java/com/android/server/backup/ProcessedPackagesJournal.java
@@ -0,0 +1,147 @@
+/*
+ * Copyright (C) 2017 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License
+ */
+
+package com.android.server.backup;
+
+import android.util.Slog;
+
+import com.android.internal.annotations.GuardedBy;
+import com.android.server.backup.RefactoredBackupManagerService;
+
+import java.io.EOFException;
+import java.io.File;
+import java.io.IOException;
+import java.io.RandomAccessFile;
+import java.util.HashSet;
+import java.util.Set;
+
+/**
+ * Records which apps have been backed up on this device, persisting it to disk so that it can be
+ * read at subsequent boots. This class is threadsafe.
+ *
+ * <p>This is used to decide, when restoring a package at install time, whether it has been
+ * previously backed up on the current device. If it has been previously backed up it should
+ * restore from the same restore set that the current device has been backing up to. If it has not
+ * been previously backed up, it should restore from the ancestral restore set (i.e., the restore
+ * set that the user's previous device was backing up to).
+ *
+ * <p>NB: this is always backed by the same files within the state directory supplied at
+ * construction.
+ */
+final class ProcessedPackagesJournal {
+ private static final String TAG = "ProcessedPackagesJournal";
+ private static final String JOURNAL_FILE_NAME = "processed";
+ private static final boolean DEBUG = RefactoredBackupManagerService.DEBUG || false;
+
+ // using HashSet instead of ArraySet since we expect 100-500 elements range
+ @GuardedBy("mProcessedPackages")
+ private final Set<String> mProcessedPackages = new HashSet<>();
+ // TODO: at some point consider splitting the bookkeeping to be per-transport
+ private final File mStateDirectory;
+
+ /**
+ * Constructs a new journal.
+ *
+ * After constructing the object one should call {@link #init()} to load state from disk if
+ * it has been previously persisted.
+ *
+ * @param stateDirectory The directory in which backup state (including journals) is stored.
+ */
+ ProcessedPackagesJournal(File stateDirectory) {
+ mStateDirectory = stateDirectory;
+ }
+
+ /**
+ * Loads state from disk if it has been previously persisted.
+ */
+ void init() {
+ synchronized (mProcessedPackages) {
+ loadFromDisk();
+ }
+ }
+
+ /**
+ * Returns {@code true} if {@code packageName} has previously been backed up.
+ */
+ boolean hasBeenProcessed(String packageName) {
+ synchronized (mProcessedPackages) {
+ return mProcessedPackages.contains(packageName);
+ }
+ }
+
+ void addPackage(String packageName) {
+ synchronized (mProcessedPackages) {
+ if (!mProcessedPackages.add(packageName)) {
+ // This package has already been processed - no need to add it to the journal.
+ return;
+ }
+
+ File journalFile = new File(mStateDirectory, JOURNAL_FILE_NAME);
+
+ try (RandomAccessFile out = new RandomAccessFile(journalFile, "rws")) {
+ out.seek(out.length());
+ out.writeUTF(packageName);
+ } catch (IOException e) {
+ Slog.e(TAG, "Can't log backup of " + packageName + " to " + journalFile);
+ }
+ }
+ }
+
+ /**
+ * A copy of the current state of the journal.
+ *
+ * <p>Used only for dumping out information for logging. {@link #hasBeenProcessed(String)}
+ * should be used for efficiently checking whether a package has been backed up before by this
+ * device.
+ *
+ * @return The current set of packages that have been backed up previously.
+ */
+ Set<String> getPackagesCopy() {
+ synchronized (mProcessedPackages) {
+ return new HashSet<>(mProcessedPackages);
+ }
+ }
+
+ void reset() {
+ synchronized (mProcessedPackages) {
+ mProcessedPackages.clear();
+ File journalFile = new File(mStateDirectory, JOURNAL_FILE_NAME);
+ journalFile.delete();
+ }
+ }
+
+ private void loadFromDisk() {
+ File journalFile = new File(mStateDirectory, JOURNAL_FILE_NAME);
+
+ if (!journalFile.exists()) {
+ return;
+ }
+
+ try (RandomAccessFile oldJournal = new RandomAccessFile(journalFile, "r")) {
+ while (true) {
+ String packageName = oldJournal.readUTF();
+ if (DEBUG) {
+ Slog.v(TAG, " + " + packageName);
+ }
+ mProcessedPackages.add(packageName);
+ }
+ } catch (EOFException e) {
+ // Successfully loaded journal file
+ } catch (IOException e) {
+ Slog.e(TAG, "Error reading processed packages journal", e);
+ }
+ }
+}
diff --git a/services/backup/java/com/android/server/backup/RefactoredBackupManagerService.java b/services/backup/java/com/android/server/backup/RefactoredBackupManagerService.java
index 39f7232..b01cfc5 100644
--- a/services/backup/java/com/android/server/backup/RefactoredBackupManagerService.java
+++ b/services/backup/java/com/android/server/backup/RefactoredBackupManagerService.java
@@ -629,7 +629,7 @@
// Keep a log of all the apps we've ever backed up, and what the dataset tokens are for both
// the current backup dataset and the ancestral dataset.
- private AppsBackedUpOnThisDeviceJournal mAppsBackedUpOnThisDeviceJournal;
+ private ProcessedPackagesJournal mProcessedPackagesJournal;
private static final int CURRENT_ANCESTRAL_RECORD_VERSION = 1;
// increment when the schema changes
@@ -816,7 +816,8 @@
Slog.w(TAG, "Unable to read token file", e);
}
- mAppsBackedUpOnThisDeviceJournal = new AppsBackedUpOnThisDeviceJournal(mBaseStateDir);
+ mProcessedPackagesJournal = new ProcessedPackagesJournal(mBaseStateDir);
+ mProcessedPackagesJournal.init();
synchronized (mQueueLock) {
// Resume the full-data backup queue
@@ -1068,7 +1069,7 @@
// so we must re-upload all saved settings.
public void resetBackupState(File stateFileDir) {
synchronized (mQueueLock) {
- mAppsBackedUpOnThisDeviceJournal.reset();
+ mProcessedPackagesJournal.reset();
mCurrentToken = 0;
writeRestoreTokens();
@@ -1363,7 +1364,7 @@
public void logBackupComplete(String packageName) {
if (packageName.equals(PACKAGE_MANAGER_SENTINEL)) return;
- mAppsBackedUpOnThisDeviceJournal.addPackage(packageName);
+ mProcessedPackagesJournal.addPackage(packageName);
}
// Persistently record the current and ancestral backup tokens as well
@@ -1500,7 +1501,7 @@
long token = mAncestralToken;
synchronized (mQueueLock) {
- if (mAppsBackedUpOnThisDeviceJournal.hasBeenProcessed(packageName)) {
+ if (mProcessedPackagesJournal.hasBeenProcessed(packageName)) {
if (MORE_DEBUG) {
Slog.i(TAG, "App in ever-stored, so using current token");
}
@@ -3303,9 +3304,9 @@
}
}
- HashSet<String> processedApps = mAppsBackedUpOnThisDeviceJournal.getPackagesCopy();
- pw.println("Ever backed up: " + processedApps.size());
- for (String pkg : processedApps) {
+ Set<String> processedPackages = mProcessedPackagesJournal.getPackagesCopy();
+ pw.println("Ever backed up: " + processedPackages.size());
+ for (String pkg : processedPackages) {
pw.println(" " + pkg);
}
diff --git a/services/tests/servicestests/src/com/android/server/backup/AppsBackedUpOnThisDeviceJournalTest.java b/services/tests/servicestests/src/com/android/server/backup/ProcessedPackagesJournalTest.java
similarity index 66%
rename from services/tests/servicestests/src/com/android/server/backup/AppsBackedUpOnThisDeviceJournalTest.java
rename to services/tests/servicestests/src/com/android/server/backup/ProcessedPackagesJournalTest.java
index 093f920..b4a1f18 100644
--- a/services/tests/servicestests/src/com/android/server/backup/AppsBackedUpOnThisDeviceJournalTest.java
+++ b/services/tests/servicestests/src/com/android/server/backup/ProcessedPackagesJournalTest.java
@@ -43,7 +43,7 @@
@SmallTest
@Presubmit
@RunWith(AndroidJUnit4.class)
-public class AppsBackedUpOnThisDeviceJournalTest {
+public class ProcessedPackagesJournalTest {
private static final String JOURNAL_FILE_NAME = "processed";
private static final String GOOGLE_PHOTOS = "com.google.photos";
@@ -53,21 +53,23 @@
@Rule public TemporaryFolder mTemporaryFolder = new TemporaryFolder();
private File mStateDirectory;
- private AppsBackedUpOnThisDeviceJournal mAppsBackedUpOnThisDeviceJournal;
+ private ProcessedPackagesJournal mProcessedPackagesJournal;
@Before
public void setUp() throws Exception {
MockitoAnnotations.initMocks(this);
mStateDirectory = mTemporaryFolder.newFolder();
- mAppsBackedUpOnThisDeviceJournal = new AppsBackedUpOnThisDeviceJournal(mStateDirectory);
+ mProcessedPackagesJournal = new ProcessedPackagesJournal(mStateDirectory);
+ mProcessedPackagesJournal.init();
}
@Test
public void constructor_loadsAnyPreviousJournalFromDisk() throws Exception {
writePermanentJournalPackages(Sets.newHashSet(GOOGLE_PHOTOS, GMAIL));
- AppsBackedUpOnThisDeviceJournal journalFromDisk =
- new AppsBackedUpOnThisDeviceJournal(mStateDirectory);
+ ProcessedPackagesJournal journalFromDisk =
+ new ProcessedPackagesJournal(mStateDirectory);
+ journalFromDisk.init();
assertThat(journalFromDisk.hasBeenProcessed(GOOGLE_PHOTOS)).isTrue();
assertThat(journalFromDisk.hasBeenProcessed(GMAIL)).isTrue();
@@ -75,61 +77,61 @@
@Test
public void hasBeenProcessed_isFalseForAnyPackageFromBlankInit() {
- assertThat(mAppsBackedUpOnThisDeviceJournal.hasBeenProcessed(GOOGLE_PHOTOS)).isFalse();
- assertThat(mAppsBackedUpOnThisDeviceJournal.hasBeenProcessed(GMAIL)).isFalse();
- assertThat(mAppsBackedUpOnThisDeviceJournal.hasBeenProcessed(GOOGLE_PLUS)).isFalse();
+ assertThat(mProcessedPackagesJournal.hasBeenProcessed(GOOGLE_PHOTOS)).isFalse();
+ assertThat(mProcessedPackagesJournal.hasBeenProcessed(GMAIL)).isFalse();
+ assertThat(mProcessedPackagesJournal.hasBeenProcessed(GOOGLE_PLUS)).isFalse();
}
@Test
public void addPackage_addsPackageToObjectState() {
- mAppsBackedUpOnThisDeviceJournal.addPackage(GOOGLE_PHOTOS);
+ mProcessedPackagesJournal.addPackage(GOOGLE_PHOTOS);
- assertThat(mAppsBackedUpOnThisDeviceJournal.hasBeenProcessed(GOOGLE_PHOTOS)).isTrue();
+ assertThat(mProcessedPackagesJournal.hasBeenProcessed(GOOGLE_PHOTOS)).isTrue();
}
@Test
public void addPackage_addsPackageToFileSystem() throws Exception {
- mAppsBackedUpOnThisDeviceJournal.addPackage(GOOGLE_PHOTOS);
+ mProcessedPackagesJournal.addPackage(GOOGLE_PHOTOS);
assertThat(readJournalPackages()).contains(GOOGLE_PHOTOS);
}
@Test
public void getPackagesCopy_returnsTheCurrentState() throws Exception {
- mAppsBackedUpOnThisDeviceJournal.addPackage(GOOGLE_PHOTOS);
- mAppsBackedUpOnThisDeviceJournal.addPackage(GMAIL);
+ mProcessedPackagesJournal.addPackage(GOOGLE_PHOTOS);
+ mProcessedPackagesJournal.addPackage(GMAIL);
- assertThat(mAppsBackedUpOnThisDeviceJournal.getPackagesCopy())
+ assertThat(mProcessedPackagesJournal.getPackagesCopy())
.isEqualTo(Sets.newHashSet(GOOGLE_PHOTOS, GMAIL));
}
@Test
public void getPackagesCopy_returnsACopy() throws Exception {
- mAppsBackedUpOnThisDeviceJournal.getPackagesCopy().add(GMAIL);
+ mProcessedPackagesJournal.getPackagesCopy().add(GMAIL);
- assertThat(mAppsBackedUpOnThisDeviceJournal.hasBeenProcessed(GMAIL)).isFalse();
+ assertThat(mProcessedPackagesJournal.hasBeenProcessed(GMAIL)).isFalse();
}
@Test
public void reset_removesAllPackagesFromObjectState() {
- mAppsBackedUpOnThisDeviceJournal.addPackage(GOOGLE_PHOTOS);
- mAppsBackedUpOnThisDeviceJournal.addPackage(GOOGLE_PLUS);
- mAppsBackedUpOnThisDeviceJournal.addPackage(GMAIL);
+ mProcessedPackagesJournal.addPackage(GOOGLE_PHOTOS);
+ mProcessedPackagesJournal.addPackage(GOOGLE_PLUS);
+ mProcessedPackagesJournal.addPackage(GMAIL);
- mAppsBackedUpOnThisDeviceJournal.reset();
+ mProcessedPackagesJournal.reset();
- assertThat(mAppsBackedUpOnThisDeviceJournal.hasBeenProcessed(GOOGLE_PHOTOS)).isFalse();
- assertThat(mAppsBackedUpOnThisDeviceJournal.hasBeenProcessed(GMAIL)).isFalse();
- assertThat(mAppsBackedUpOnThisDeviceJournal.hasBeenProcessed(GOOGLE_PLUS)).isFalse();
+ assertThat(mProcessedPackagesJournal.hasBeenProcessed(GOOGLE_PHOTOS)).isFalse();
+ assertThat(mProcessedPackagesJournal.hasBeenProcessed(GMAIL)).isFalse();
+ assertThat(mProcessedPackagesJournal.hasBeenProcessed(GOOGLE_PLUS)).isFalse();
}
@Test
public void reset_removesAllPackagesFromFileSystem() throws Exception {
- mAppsBackedUpOnThisDeviceJournal.addPackage(GOOGLE_PHOTOS);
- mAppsBackedUpOnThisDeviceJournal.addPackage(GOOGLE_PLUS);
- mAppsBackedUpOnThisDeviceJournal.addPackage(GMAIL);
+ mProcessedPackagesJournal.addPackage(GOOGLE_PHOTOS);
+ mProcessedPackagesJournal.addPackage(GOOGLE_PLUS);
+ mProcessedPackagesJournal.addPackage(GMAIL);
- mAppsBackedUpOnThisDeviceJournal.reset();
+ mProcessedPackagesJournal.reset();
assertThat(readJournalPackages()).isEmpty();
}