Loading services/backup/java/com/android/server/backup/keyvalue/KeyValueBackupTask.java +21 −4 Original line number Diff line number Diff line Loading @@ -649,16 +649,33 @@ public class KeyValueBackupTask implements BackupRestoreTask, Runnable { mReporter.onStartPackageBackup(PM_PACKAGE); mCurrentPackage = new PackageInfo(); mCurrentPackage.packageName = PM_PACKAGE; try { // If we can't even extractPmAgentData(), then we treat the local state as // compromised, just in case. This means that we will clear data and will // start from a clean slate in the next attempt. It's not clear whether that's // the right thing to do, but matches what we have historically done. try { extractPmAgentData(mCurrentPackage); } catch (TaskException e) { throw TaskException.stateCompromised(e); // force stateCompromised } // During sendDataToTransport, we generally trust any thrown TaskException // about whether stateCompromised because those are likely transient; // clearing state for those would have the potential to lead to cascading // failures, as discussed in http://b/144030477. // For specific status codes (e.g. TRANSPORT_NON_INCREMENTAL_BACKUP_REQUIRED), // cleanUpAgentForTransportStatus() or theoretically handleTransportStatus() // still have the opportunity to perform additional clean-up tasks. int status = sendDataToTransport(mCurrentPackage); cleanUpAgentForTransportStatus(status); } catch (AgentException | TaskException e) { mReporter.onExtractPmAgentDataError(e); cleanUpAgentForError(e); // PM agent failure is task failure. throw TaskException.stateCompromised(e); if (e instanceof TaskException) { throw (TaskException) e; } else { throw TaskException.stateCompromised(e); // PM agent failure is task failure. } } } Loading services/robotests/backup/src/com/android/server/backup/keyvalue/KeyValueBackupTaskTest.java +51 −8 Original line number Diff line number Diff line Loading @@ -122,6 +122,7 @@ import com.android.server.testing.shadows.ShadowBackupDataOutput; import com.android.server.testing.shadows.ShadowEventLog; import com.android.server.testing.shadows.ShadowSystemServiceRegistry; import com.google.common.base.Charsets; import com.google.common.truth.IterableSubject; import org.junit.After; Loading Loading @@ -1910,7 +1911,8 @@ public class KeyValueBackupTaskTest { } @Test public void testRunTask_whenTransportReturnsError_updatesFilesAndCleansUp() throws Exception { public void testRunTask_whenTransportReturnsErrorForGenericPackage_updatesFilesAndCleansUp() throws Exception { TransportMock transportMock = setUpInitializedTransport(mTransport); when(transportMock.transport.performBackup( argThat(packageInfo(PACKAGE_1)), any(), anyInt())) Loading @@ -1926,6 +1928,39 @@ public class KeyValueBackupTaskTest { assertCleansUpFilesAndAgent(mTransport, PACKAGE_1); } /** * Checks that TRANSPORT_ERROR during @pm@ backup keeps the state file untouched. * http://b/144030477 */ @Test public void testRunTask_whenTransportReturnsErrorForPm_updatesFilesAndCleansUp() throws Exception { // TODO(tobiast): Refactor this method to share code with // testRunTask_whenTransportReturnsErrorForGenericPackage_updatesFilesAndCleansUp // See patchset 7 of http://ag/11762961 final PackageData packageData = PM_PACKAGE; TransportMock transportMock = setUpInitializedTransport(mTransport); when(transportMock.transport.performBackup( argThat(packageInfo(packageData)), any(), anyInt())) .thenReturn(BackupTransport.TRANSPORT_ERROR); byte[] pmStateBytes = "fake @pm@ state for testing".getBytes(Charsets.UTF_8); Path pmStatePath = createPmStateFile(pmStateBytes.clone()); PackageManagerBackupAgent pmAgent = spy(createPmAgent()); KeyValueBackupTask task = createKeyValueBackupTask(transportMock, packageData); runTask(task); verify(pmAgent, never()).onBackup(any(), any(), any()); assertThat(Files.readAllBytes(pmStatePath)).isEqualTo(pmStateBytes.clone()); boolean existed = deletePmStateFile(); assertThat(existed).isTrue(); // unbindAgent() is skipped for @pm@. Comment in KeyValueBackupTask.java: // "For PM metadata (for which applicationInfo is null) there is no agent-bound state." assertCleansUpFiles(mTransport, packageData); } @Test public void testRunTask_whenTransportGetBackupQuotaThrowsForPm() throws Exception { TransportMock transportMock = setUpInitializedTransport(mTransport); Loading Loading @@ -2707,21 +2742,29 @@ public class KeyValueBackupTaskTest { * </ul> * </ul> */ private void createPmStateFile() throws IOException { createPmStateFile(mTransport); private Path createPmStateFile() throws IOException { return createPmStateFile("pmState".getBytes()); } private Path createPmStateFile(byte[] bytes) throws IOException { return createPmStateFile(bytes, mTransport); } private Path createPmStateFile(TransportData transport) throws IOException { return createPmStateFile("pmState".getBytes(), mTransport); } /** @see #createPmStateFile() */ private void createPmStateFile(TransportData transport) throws IOException { Files.write(getStateFile(transport, PM_PACKAGE), "pmState".getBytes()); /** @see #createPmStateFile(byte[]) */ private Path createPmStateFile(byte[] bytes, TransportData transport) throws IOException { return Files.write(getStateFile(transport, PM_PACKAGE), bytes); } /** * Forces transport initialization and call to {@link * UserBackupManagerService#resetBackupState(File)} */ private void deletePmStateFile() throws IOException { Files.deleteIfExists(getStateFile(mTransport, PM_PACKAGE)); private boolean deletePmStateFile() throws IOException { return Files.deleteIfExists(getStateFile(mTransport, PM_PACKAGE)); } /** Loading Loading
services/backup/java/com/android/server/backup/keyvalue/KeyValueBackupTask.java +21 −4 Original line number Diff line number Diff line Loading @@ -649,16 +649,33 @@ public class KeyValueBackupTask implements BackupRestoreTask, Runnable { mReporter.onStartPackageBackup(PM_PACKAGE); mCurrentPackage = new PackageInfo(); mCurrentPackage.packageName = PM_PACKAGE; try { // If we can't even extractPmAgentData(), then we treat the local state as // compromised, just in case. This means that we will clear data and will // start from a clean slate in the next attempt. It's not clear whether that's // the right thing to do, but matches what we have historically done. try { extractPmAgentData(mCurrentPackage); } catch (TaskException e) { throw TaskException.stateCompromised(e); // force stateCompromised } // During sendDataToTransport, we generally trust any thrown TaskException // about whether stateCompromised because those are likely transient; // clearing state for those would have the potential to lead to cascading // failures, as discussed in http://b/144030477. // For specific status codes (e.g. TRANSPORT_NON_INCREMENTAL_BACKUP_REQUIRED), // cleanUpAgentForTransportStatus() or theoretically handleTransportStatus() // still have the opportunity to perform additional clean-up tasks. int status = sendDataToTransport(mCurrentPackage); cleanUpAgentForTransportStatus(status); } catch (AgentException | TaskException e) { mReporter.onExtractPmAgentDataError(e); cleanUpAgentForError(e); // PM agent failure is task failure. throw TaskException.stateCompromised(e); if (e instanceof TaskException) { throw (TaskException) e; } else { throw TaskException.stateCompromised(e); // PM agent failure is task failure. } } } Loading
services/robotests/backup/src/com/android/server/backup/keyvalue/KeyValueBackupTaskTest.java +51 −8 Original line number Diff line number Diff line Loading @@ -122,6 +122,7 @@ import com.android.server.testing.shadows.ShadowBackupDataOutput; import com.android.server.testing.shadows.ShadowEventLog; import com.android.server.testing.shadows.ShadowSystemServiceRegistry; import com.google.common.base.Charsets; import com.google.common.truth.IterableSubject; import org.junit.After; Loading Loading @@ -1910,7 +1911,8 @@ public class KeyValueBackupTaskTest { } @Test public void testRunTask_whenTransportReturnsError_updatesFilesAndCleansUp() throws Exception { public void testRunTask_whenTransportReturnsErrorForGenericPackage_updatesFilesAndCleansUp() throws Exception { TransportMock transportMock = setUpInitializedTransport(mTransport); when(transportMock.transport.performBackup( argThat(packageInfo(PACKAGE_1)), any(), anyInt())) Loading @@ -1926,6 +1928,39 @@ public class KeyValueBackupTaskTest { assertCleansUpFilesAndAgent(mTransport, PACKAGE_1); } /** * Checks that TRANSPORT_ERROR during @pm@ backup keeps the state file untouched. * http://b/144030477 */ @Test public void testRunTask_whenTransportReturnsErrorForPm_updatesFilesAndCleansUp() throws Exception { // TODO(tobiast): Refactor this method to share code with // testRunTask_whenTransportReturnsErrorForGenericPackage_updatesFilesAndCleansUp // See patchset 7 of http://ag/11762961 final PackageData packageData = PM_PACKAGE; TransportMock transportMock = setUpInitializedTransport(mTransport); when(transportMock.transport.performBackup( argThat(packageInfo(packageData)), any(), anyInt())) .thenReturn(BackupTransport.TRANSPORT_ERROR); byte[] pmStateBytes = "fake @pm@ state for testing".getBytes(Charsets.UTF_8); Path pmStatePath = createPmStateFile(pmStateBytes.clone()); PackageManagerBackupAgent pmAgent = spy(createPmAgent()); KeyValueBackupTask task = createKeyValueBackupTask(transportMock, packageData); runTask(task); verify(pmAgent, never()).onBackup(any(), any(), any()); assertThat(Files.readAllBytes(pmStatePath)).isEqualTo(pmStateBytes.clone()); boolean existed = deletePmStateFile(); assertThat(existed).isTrue(); // unbindAgent() is skipped for @pm@. Comment in KeyValueBackupTask.java: // "For PM metadata (for which applicationInfo is null) there is no agent-bound state." assertCleansUpFiles(mTransport, packageData); } @Test public void testRunTask_whenTransportGetBackupQuotaThrowsForPm() throws Exception { TransportMock transportMock = setUpInitializedTransport(mTransport); Loading Loading @@ -2707,21 +2742,29 @@ public class KeyValueBackupTaskTest { * </ul> * </ul> */ private void createPmStateFile() throws IOException { createPmStateFile(mTransport); private Path createPmStateFile() throws IOException { return createPmStateFile("pmState".getBytes()); } private Path createPmStateFile(byte[] bytes) throws IOException { return createPmStateFile(bytes, mTransport); } private Path createPmStateFile(TransportData transport) throws IOException { return createPmStateFile("pmState".getBytes(), mTransport); } /** @see #createPmStateFile() */ private void createPmStateFile(TransportData transport) throws IOException { Files.write(getStateFile(transport, PM_PACKAGE), "pmState".getBytes()); /** @see #createPmStateFile(byte[]) */ private Path createPmStateFile(byte[] bytes, TransportData transport) throws IOException { return Files.write(getStateFile(transport, PM_PACKAGE), bytes); } /** * Forces transport initialization and call to {@link * UserBackupManagerService#resetBackupState(File)} */ private void deletePmStateFile() throws IOException { Files.deleteIfExists(getStateFile(mTransport, PM_PACKAGE)); private boolean deletePmStateFile() throws IOException { return Files.deleteIfExists(getStateFile(mTransport, PM_PACKAGE)); } /** Loading