From: Robert Berry Date: Fri, 14 Jul 2017 13:19:21 +0000 (+0100) Subject: Move logic for backup journal into its own class X-Git-Tag: android-x86-9.0-r1~1256^2 X-Git-Url: http://git.osdn.net/view?a=commitdiff_plain;h=c31a839fd3ecc91807d735884d09fcbaf62e9244;p=android-x86%2Fframeworks-base.git Move logic for backup journal into its own class Refactor to further simplify BackupManagerService, which currently has too many responsibilities. Also adds unit tests. 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: Id433d7604c22c8b6f0d524a9bf9e83053facc0ca --- diff --git a/services/backup/java/com/android/server/backup/DataChangedJournal.java b/services/backup/java/com/android/server/backup/DataChangedJournal.java new file mode 100644 index 000000000000..9360c85aed33 --- /dev/null +++ b/services/backup/java/com/android/server/backup/DataChangedJournal.java @@ -0,0 +1,142 @@ +/* + * 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.annotation.Nullable; + +import java.io.BufferedInputStream; +import java.io.DataInputStream; +import java.io.File; +import java.io.FileInputStream; +import java.io.IOException; +import java.io.RandomAccessFile; +import java.util.ArrayList; + +/** + * A journal of packages that have indicated that their data has changed (and therefore should be + * backed up in the next scheduled K/V backup pass). + * + *

This information is persisted to the filesystem so that it is not lost in the event of a + * reboot. + */ +public final class DataChangedJournal { + private static final String FILE_NAME_PREFIX = "journal"; + + /** + * Journals tend to be on the order of a few kilobytes, hence setting the buffer size to 8kb. + */ + private static final int BUFFER_SIZE_BYTES = 8 * 1024; + + private final File mFile; + + /** + * Constructs an instance that reads from and writes to the given file. + */ + DataChangedJournal(File file) { + mFile = file; + } + + /** + * Adds the given package to the journal. + * + * @param packageName The name of the package whose data has changed. + * @throws IOException if there is an IO error writing to the journal file. + */ + public void addPackage(String packageName) throws IOException { + try (RandomAccessFile out = new RandomAccessFile(mFile, "rws")) { + out.seek(out.length()); + out.writeUTF(packageName); + } + } + + /** + * Invokes {@link Consumer#accept(String)} with every package name in the journal file. + * + * @param consumer The callback. + * @throws IOException If there is an IO error reading from the file. + */ + public void forEach(Consumer consumer) throws IOException { + try ( + BufferedInputStream bufferedInputStream = new BufferedInputStream( + new FileInputStream(mFile), BUFFER_SIZE_BYTES); + DataInputStream dataInputStream = new DataInputStream(bufferedInputStream) + ) { + while (dataInputStream.available() > 0) { + String packageName = dataInputStream.readUTF(); + consumer.accept(packageName); + } + } + } + + /** + * Deletes the journal from the filesystem. + * + * @return {@code true} if successfully deleted journal. + */ + public boolean delete() { + return mFile.delete(); + } + + @Override + public boolean equals(@Nullable Object object) { + if (object instanceof DataChangedJournal) { + DataChangedJournal that = (DataChangedJournal) object; + try { + return this.mFile.getCanonicalPath().equals(that.mFile.getCanonicalPath()); + } catch (IOException exception) { + return false; + } + } + return false; + } + + @Override + public String toString() { + return mFile.toString(); + } + + /** + * Consumer for iterating over package names in the journal. + */ + @FunctionalInterface + public interface Consumer { + void accept(String packageName); + } + + /** + * Creates a new journal with a random file name in the given journal directory. + * + * @param journalDirectory The directory where journals are kept. + * @return The journal. + * @throws IOException if there is an IO error creating the file. + */ + static DataChangedJournal newJournal(File journalDirectory) throws IOException { + return new DataChangedJournal( + File.createTempFile(FILE_NAME_PREFIX, null, journalDirectory)); + } + + /** + * Returns a list of journals in the given journal directory. + */ + static ArrayList listJournals(File journalDirectory) { + ArrayList journals = new ArrayList<>(); + for (File file : journalDirectory.listFiles()) { + journals.add(new DataChangedJournal(file)); + } + return journals; + } +} diff --git a/services/backup/java/com/android/server/backup/RefactoredBackupManagerService.java b/services/backup/java/com/android/server/backup/RefactoredBackupManagerService.java index 7e28f610e565..9aa62296585a 100644 --- a/services/backup/java/com/android/server/backup/RefactoredBackupManagerService.java +++ b/services/backup/java/com/android/server/backup/RefactoredBackupManagerService.java @@ -32,6 +32,7 @@ import static com.android.server.backup.internal.BackupHandler.MSG_RUN_CLEAR; import static com.android.server.backup.internal.BackupHandler.MSG_RUN_RESTORE; import static com.android.server.backup.internal.BackupHandler.MSG_SCHEDULE_BACKUP_PACKAGE; +import android.annotation.Nullable; import android.app.ActivityManager; import android.app.AlarmManager; import android.app.AppGlobals; @@ -482,11 +483,11 @@ public class RefactoredBackupManagerService implements BackupManagerServiceInter mDataDir = dataDir; } - public File getJournal() { + public DataChangedJournal getJournal() { return mJournal; } - public void setJournal(File journal) { + public void setJournal(@Nullable DataChangedJournal journal) { mJournal = journal; } @@ -630,7 +631,7 @@ public class RefactoredBackupManagerService implements BackupManagerServiceInter private File mBaseStateDir; private File mDataDir; private File mJournalDir; - private File mJournal; + @Nullable private DataChangedJournal mJournal; // Backup password, if any, and the file where it's saved. What is stored is not the // password text itself; it's the result of a PBKDF2 hash with a randomly chosen (but @@ -1105,35 +1106,17 @@ public class RefactoredBackupManagerService implements BackupManagerServiceInter } private void parseLeftoverJournals() { - for (File f : mJournalDir.listFiles()) { - if (mJournal == null || f.compareTo(mJournal) != 0) { - // This isn't the current journal, so it must be a leftover. Read - // out the package names mentioned there and schedule them for - // backup. - DataInputStream in = null; + ArrayList journals = DataChangedJournal.listJournals(mJournalDir); + for (DataChangedJournal journal : journals) { + if (!journal.equals(mJournal)) { try { - Slog.i(TAG, "Found stale backup journal, scheduling"); - // Journals will tend to be on the order of a few kilobytes(around 4k), hence, - // setting the buffer size to 8192. - InputStream bufferedInputStream = new BufferedInputStream( - new FileInputStream(f), 8192); - in = new DataInputStream(bufferedInputStream); - while (true) { - String packageName = in.readUTF(); + journal.forEach(packageName -> { + Slog.i(TAG, "Found stale backup journal, scheduling"); if (MORE_DEBUG) Slog.i(TAG, " " + packageName); dataChangedImpl(packageName); - } - } catch (EOFException e) { - // no more data; we're done - } catch (Exception e) { - Slog.e(TAG, "Can't read " + f, e); - } finally { - // close/delete the file - try { - if (in != null) in.close(); - } catch (IOException e) { - } - f.delete(); + }); + } catch (IOException e) { + Slog.e(TAG, "Can't read " + journal, e); } } } @@ -2507,20 +2490,12 @@ public class RefactoredBackupManagerService implements BackupManagerServiceInter } private void writeToJournalLocked(String str) { - RandomAccessFile out = null; try { - if (mJournal == null) mJournal = File.createTempFile("journal", null, mJournalDir); - out = new RandomAccessFile(mJournal, "rws"); - out.seek(out.length()); - out.writeUTF(str); + if (mJournal == null) mJournal = DataChangedJournal.newJournal(mJournalDir); + mJournal.addPackage(str); } catch (IOException e) { Slog.e(TAG, "Can't write " + str + " to backup journal", e); mJournal = null; - } finally { - try { - if (out != null) out.close(); - } catch (IOException e) { - } } } diff --git a/services/backup/java/com/android/server/backup/internal/BackupHandler.java b/services/backup/java/com/android/server/backup/internal/BackupHandler.java index edd389403f02..8f823004d993 100644 --- a/services/backup/java/com/android/server/backup/internal/BackupHandler.java +++ b/services/backup/java/com/android/server/backup/internal/BackupHandler.java @@ -36,6 +36,7 @@ import android.util.Slog; import com.android.internal.backup.IBackupTransport; import com.android.server.EventLogTags; import com.android.server.backup.BackupRestoreTask; +import com.android.server.backup.DataChangedJournal; import com.android.server.backup.RefactoredBackupManagerService; import com.android.server.backup.fullbackup.PerformAdbBackupTask; import com.android.server.backup.fullbackup.PerformFullTransportBackupTask; @@ -107,7 +108,7 @@ public class BackupHandler extends Handler { // snapshot the pending-backup set and work on that ArrayList queue = new ArrayList<>(); - File oldJournal = backupManagerService.getJournal(); + DataChangedJournal oldJournal = backupManagerService.getJournal(); synchronized (backupManagerService.getQueueLock()) { // Do we have any work to do? Construct the work queue // then release the synchronization lock to actually run diff --git a/services/backup/java/com/android/server/backup/internal/PerformBackupTask.java b/services/backup/java/com/android/server/backup/internal/PerformBackupTask.java index a996e2d96492..5d4fcf4c16d9 100644 --- a/services/backup/java/com/android/server/backup/internal/PerformBackupTask.java +++ b/services/backup/java/com/android/server/backup/internal/PerformBackupTask.java @@ -28,6 +28,7 @@ import static com.android.server.backup.RefactoredBackupManagerService.TIMEOUT_B import static com.android.server.backup.internal.BackupHandler.MSG_BACKUP_OPERATION_TIMEOUT; import static com.android.server.backup.internal.BackupHandler.MSG_BACKUP_RESTORE_STEP; +import android.annotation.Nullable; import android.app.ApplicationThreadConstants; import android.app.IBackupAgent; import android.app.backup.BackupDataInput; @@ -57,6 +58,7 @@ import com.android.internal.backup.IBackupTransport; import com.android.server.AppWidgetBackupBridge; import com.android.server.EventLogTags; import com.android.server.backup.BackupRestoreTask; +import com.android.server.backup.DataChangedJournal; import com.android.server.backup.KeyValueBackupJob; import com.android.server.backup.PackageManagerBackupAgent; import com.android.server.backup.RefactoredBackupManagerService; @@ -114,7 +116,7 @@ public class PerformBackupTask implements BackupRestoreTask { ArrayList mQueue; ArrayList mOriginalQueue; File mStateDir; - File mJournal; + @Nullable DataChangedJournal mJournal; BackupState mCurrentState; List mPendingFullBackups; IBackupObserver mObserver; @@ -142,9 +144,9 @@ public class PerformBackupTask implements BackupRestoreTask { public PerformBackupTask(RefactoredBackupManagerService backupManagerService, IBackupTransport transport, String dirName, - ArrayList queue, File journal, IBackupObserver observer, - IBackupManagerMonitor monitor, List pendingFullBackups, - boolean userInitiated, boolean nonIncremental) { + ArrayList queue, @Nullable DataChangedJournal journal, + IBackupObserver observer, IBackupManagerMonitor monitor, + List pendingFullBackups, boolean userInitiated, boolean nonIncremental) { this.backupManagerService = backupManagerService; mTransport = transport; mOriginalQueue = queue; diff --git a/services/tests/servicestests/src/com/android/server/backup/DataChangedJournalTest.java b/services/tests/servicestests/src/com/android/server/backup/DataChangedJournalTest.java new file mode 100644 index 000000000000..c27fd079bb89 --- /dev/null +++ b/services/tests/servicestests/src/com/android/server/backup/DataChangedJournalTest.java @@ -0,0 +1,132 @@ +/* + * 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.google.common.truth.Truth.assertThat; + +import android.platform.test.annotations.Presubmit; +import android.support.test.filters.SmallTest; +import android.support.test.runner.AndroidJUnit4; + +import org.junit.Before; +import org.junit.Rule; +import org.junit.Test; +import org.junit.rules.TemporaryFolder; +import org.junit.runner.RunWith; +import org.mockito.InOrder; +import org.mockito.Mock; +import org.mockito.Mockito; +import org.mockito.MockitoAnnotations; + +import java.io.DataInputStream; +import java.io.File; +import java.io.FileInputStream; +import java.util.ArrayList; + +@SmallTest +@Presubmit +@RunWith(AndroidJUnit4.class) +public class DataChangedJournalTest { + private static final String GMAIL = "com.google.gmail"; + private static final String DOCS = "com.google.docs"; + private static final String GOOGLE_PLUS = "com.google.plus"; + + @Rule public TemporaryFolder mTemporaryFolder = new TemporaryFolder(); + + @Mock private DataChangedJournal.Consumer mConsumer; + + private File mFile; + private DataChangedJournal mJournal; + + @Before + public void setUp() throws Exception { + MockitoAnnotations.initMocks(this); + mFile = mTemporaryFolder.newFile(); + mJournal = new DataChangedJournal(mFile); + } + + @Test + public void addPackage_addsPackagesToEndOfFile() throws Exception { + mJournal.addPackage(GMAIL); + mJournal.addPackage(DOCS); + mJournal.addPackage(GOOGLE_PLUS); + + FileInputStream fos = new FileInputStream(mFile); + DataInputStream dos = new DataInputStream(fos); + assertThat(dos.readUTF()).isEqualTo(GMAIL); + assertThat(dos.readUTF()).isEqualTo(DOCS); + assertThat(dos.readUTF()).isEqualTo(GOOGLE_PLUS); + assertThat(dos.available()).isEqualTo(0); + } + + @Test + public void delete_deletesTheFile() throws Exception { + mJournal.addPackage(GMAIL); + + mJournal.delete(); + + assertThat(mFile.exists()).isFalse(); + } + + @Test + public void equals_isTrueForTheSameFile() throws Exception { + assertThat(mJournal.equals(new DataChangedJournal(mFile))).isTrue(); + } + + @Test + public void equals_isFalseForDifferentFiles() throws Exception { + assertThat(mJournal.equals(new DataChangedJournal(mTemporaryFolder.newFile()))).isFalse(); + } + + @Test + public void forEach_iteratesThroughPackagesInFileInOrder() throws Exception { + mJournal.addPackage(GMAIL); + mJournal.addPackage(DOCS); + + mJournal.forEach(mConsumer); + + InOrder inOrder = Mockito.inOrder(mConsumer); + inOrder.verify(mConsumer).accept(GMAIL); + inOrder.verify(mConsumer).accept(DOCS); + inOrder.verifyNoMoreInteractions(); + } + + @Test + public void listJournals_returnsJournalsForEveryFileInDirectory() throws Exception { + File folder = mTemporaryFolder.newFolder(); + DataChangedJournal.newJournal(folder); + DataChangedJournal.newJournal(folder); + + ArrayList journals = DataChangedJournal.listJournals(folder); + + assertThat(journals).hasSize(2); + } + + @Test + public void newJournal_createsANewTemporaryFile() throws Exception { + File folder = mTemporaryFolder.newFolder(); + + DataChangedJournal.newJournal(folder); + + assertThat(folder.listFiles()).hasLength(1); + } + + @Test + public void toString_isSameAsFileToString() throws Exception { + assertThat(mJournal.toString()).isEqualTo(mFile.toString()); + } +}