Merge changes I7641cf9e,I1756add0,I94f35f9a,Ia1343e4c
* changes:
DataChangedJournal.forEach(): Fix fragile loop condition.
DataChangedJournal: Disallow null files/directories.
Fix DataChangedJournalTest.equals().
Log only a summary 'Found stale backup journal' message.
diff --git a/services/backup/java/com/android/server/backup/DataChangedJournal.java b/services/backup/java/com/android/server/backup/DataChangedJournal.java
index e75eb73..0e7fc93 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 @@
* <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 @@
/**
* 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 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 @@
}
@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 @@
* @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 ff21a73..2de26b7 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.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,24 +1155,31 @@
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));
+ }
}
public Set<String> getExcludedRestoreKeys(String packageName) {
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 0e918db..9daa4ba 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 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 @@
@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 @@
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 @@
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 void listJournals_invalidJournalFile_returnsEmptyList() throws Exception {
when(invalidFile.listFiles()).thenReturn(null);
- assertEquals(0, DataChangedJournal.listJournals(invalidFile).size());
+ assertThat(DataChangedJournal.listJournals(invalidFile)).isEmpty();
}
}