diff options
3 files changed, 67 insertions, 36 deletions
diff --git a/services/backup/java/com/android/server/backup/DataChangedJournal.java b/services/backup/java/com/android/server/backup/DataChangedJournal.java index e75eb731a73e..0e7fc93df7cc 100644 --- a/services/backup/java/com/android/server/backup/DataChangedJournal.java +++ b/services/backup/java/com/android/server/backup/DataChangedJournal.java @@ -16,17 +16,21 @@ package com.android.server.backup; +import android.annotation.NonNull; import android.annotation.Nullable; import android.util.Slog; import java.io.BufferedInputStream; import java.io.DataInputStream; +import java.io.EOFException; import java.io.File; import java.io.FileInputStream; import java.io.IOException; +import java.io.InputStream; import java.io.RandomAccessFile; import java.util.ArrayList; import java.util.List; +import java.util.Objects; import java.util.function.Consumer; /** @@ -36,7 +40,7 @@ import java.util.function.Consumer; * <p>This information is persisted to the filesystem so that it is not lost in the event of a * reboot. */ -public class DataChangedJournal { +public final class DataChangedJournal { private static final String TAG = "DataChangedJournal"; private static final String FILE_NAME_PREFIX = "journal"; @@ -50,10 +54,11 @@ public class DataChangedJournal { /** * Constructs an instance that reads from and writes to the given file. */ - DataChangedJournal(File file) { - mFile = file; + DataChangedJournal(@NonNull File file) { + mFile = Objects.requireNonNull(file); } + /** * Adds the given package to the journal. * @@ -75,15 +80,17 @@ public class DataChangedJournal { */ public void forEach(Consumer<String> consumer) throws IOException { try ( - BufferedInputStream bufferedInputStream = new BufferedInputStream( - new FileInputStream(mFile), BUFFER_SIZE_BYTES); - DataInputStream dataInputStream = new DataInputStream(bufferedInputStream) + InputStream in = new FileInputStream(mFile); + InputStream bufferedIn = new BufferedInputStream(in, BUFFER_SIZE_BYTES); + DataInputStream dataInputStream = new DataInputStream(bufferedIn) ) { - while (dataInputStream.available() > 0) { + while (true) { String packageName = dataInputStream.readUTF(); consumer.accept(packageName); } - } + } catch (EOFException tolerated) { + // no more data; we're done + } // other kinds of IOExceptions are error conditions and handled in the caller } /** @@ -107,14 +114,15 @@ public class DataChangedJournal { } @Override + public int hashCode() { + return mFile.hashCode(); + } + + @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 mFile.equals(that.mFile); } return false; } @@ -131,9 +139,10 @@ public class DataChangedJournal { * @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)); + static DataChangedJournal newJournal(@NonNull File journalDirectory) throws IOException { + Objects.requireNonNull(journalDirectory); + File file = File.createTempFile(FILE_NAME_PREFIX, null, journalDirectory); + return new DataChangedJournal(file); } /** diff --git a/services/backup/java/com/android/server/backup/UserBackupManagerService.java b/services/backup/java/com/android/server/backup/UserBackupManagerService.java index ff21a733223c..2de26b7c4bd2 100644 --- a/services/backup/java/com/android/server/backup/UserBackupManagerService.java +++ b/services/backup/java/com/android/server/backup/UserBackupManagerService.java @@ -156,6 +156,7 @@ import java.util.Collections; import java.util.Date; import java.util.HashMap; import java.util.HashSet; +import java.util.LinkedHashSet; import java.util.LinkedList; import java.util.List; import java.util.Objects; @@ -1154,23 +1155,30 @@ public class UserBackupManagerService { private void parseLeftoverJournals() { ArrayList<DataChangedJournal> journals = DataChangedJournal.listJournals(mJournalDir); + journals.removeAll(Collections.singletonList(mJournal)); + if (!journals.isEmpty()) { + Slog.i(TAG, addUserIdToLogMessage(mUserId, + "Found " + journals.size() + " stale backup journal(s), scheduling.")); + } + Set<String> packageNames = new LinkedHashSet<>(); for (DataChangedJournal journal : journals) { - if (!journal.equals(mJournal)) { - try { - journal.forEach(packageName -> { - Slog.i( - TAG, - addUserIdToLogMessage( - mUserId, "Found stale backup journal, scheduling")); - if (MORE_DEBUG) { - Slog.i(TAG, addUserIdToLogMessage(mUserId, " " + packageName)); - } + try { + journal.forEach(packageName -> { + if (packageNames.add(packageName)) { dataChangedImpl(packageName); - }); - } catch (IOException e) { - Slog.e(TAG, addUserIdToLogMessage(mUserId, "Can't read " + journal), e); - } + } + }); + } catch (IOException e) { + Slog.e(TAG, addUserIdToLogMessage(mUserId, "Can't read " + journal), e); + } + } + if (!packageNames.isEmpty()) { + String msg = "Stale backup journals: Scheduled " + packageNames.size() + + " package(s) total"; + if (MORE_DEBUG) { + msg += ": " + packageNames; } + Slog.i(TAG, addUserIdToLogMessage(mUserId, msg)); } } diff --git a/services/tests/servicestests/src/com/android/server/backup/DataChangedJournalTest.java b/services/tests/servicestests/src/com/android/server/backup/DataChangedJournalTest.java index 0e918db321e3..9daa4ba3c9c8 100644 --- a/services/tests/servicestests/src/com/android/server/backup/DataChangedJournalTest.java +++ b/services/tests/servicestests/src/com/android/server/backup/DataChangedJournalTest.java @@ -39,7 +39,7 @@ import org.mockito.MockitoAnnotations; import java.io.DataInputStream; import java.io.File; import java.io.FileInputStream; -import java.util.ArrayList; +import java.io.IOException; import java.util.function.Consumer; @SmallTest @@ -90,7 +90,13 @@ public class DataChangedJournalTest { @Test public void equals_isTrueForTheSameFile() throws Exception { - assertThat(mJournal.equals(new DataChangedJournal(mFile))).isTrue(); + assertEqualsBothWaysAndHashCode(mJournal, new DataChangedJournal(mFile)); + } + + private static <T> void assertEqualsBothWaysAndHashCode(T a, T b) { + assertEquals(a, b); + assertEquals(b, a); + assertEquals(a.hashCode(), b.hashCode()); } @Test @@ -117,9 +123,7 @@ public class DataChangedJournalTest { DataChangedJournal.newJournal(folder); DataChangedJournal.newJournal(folder); - ArrayList<DataChangedJournal> journals = DataChangedJournal.listJournals(folder); - - assertThat(journals).hasSize(2); + assertThat(DataChangedJournal.listJournals(folder)).hasSize(2); } @Test @@ -131,6 +135,16 @@ public class DataChangedJournalTest { assertThat(folder.listFiles()).hasLength(1); } + @Test(expected = NullPointerException.class) + public void newJournal_nullJournalDir() throws IOException { + DataChangedJournal.newJournal(null); + } + + @Test(expected = NullPointerException.class) + public void nullFile() { + new DataChangedJournal(null); + } + @Test public void toString_isSameAsFileToString() throws Exception { assertThat(mJournal.toString()).isEqualTo(mFile.toString()); @@ -140,6 +154,6 @@ public class DataChangedJournalTest { public void listJournals_invalidJournalFile_returnsEmptyList() throws Exception { when(invalidFile.listFiles()).thenReturn(null); - assertEquals(0, DataChangedJournal.listJournals(invalidFile).size()); + assertThat(DataChangedJournal.listJournals(invalidFile)).isEmpty(); } } |