diff options
author | Ruslan Tkhakokhov <rthakohov@google.com> | 2021-02-20 10:43:21 +0000 |
---|---|---|
committer | Ruslan Tkhakokhov <rthakohov@google.com> | 2021-02-24 23:21:21 +0000 |
commit | a2fdf3f5fbafd47d17cd31b51f782aa89cd59189 (patch) | |
tree | 0779c07cf0a462532432b1724940910b0ac39d92 | |
parent | c3bf79a2cd92634cc18a2983e63b5f5c9193cdfd (diff) |
Get OperationType from transport
This CL was merged earlier (ag/13484966) and then reverted due to the
new behaviour breaking D2D transfers.
Merge it again with all changes being controlled by a flag (default
off), see UserBackupManagerService:getOperationTypeFromTransport in this
CL. View the diff between patchsets 1 and 2 to only see what's changed
between the earlier reverted code and the fixed version of it (i.e. with
the flag).
The flag can be changed via adb for now, we will set it to true by
default once other components are ready.
Bug: 174216309
Test: atest UserBackupManagerServiceTest
Change-Id: I7473c9b4f8d0c4d20155be76930279184ffb17c4
7 files changed, 111 insertions, 117 deletions
diff --git a/cmds/bmgr/src/com/android/commands/bmgr/Bmgr.java b/cmds/bmgr/src/com/android/commands/bmgr/Bmgr.java index 4b7eda096e54..ed717c491467 100644 --- a/cmds/bmgr/src/com/android/commands/bmgr/Bmgr.java +++ b/cmds/bmgr/src/com/android/commands/bmgr/Bmgr.java @@ -19,7 +19,6 @@ package com.android.commands.bmgr; import android.annotation.IntDef; import android.annotation.UserIdInt; import android.app.backup.BackupManager; -import android.app.backup.BackupManager.OperationType; import android.app.backup.BackupManagerMonitor; import android.app.backup.BackupProgress; import android.app.backup.BackupTransport; @@ -667,7 +666,7 @@ public class Bmgr { // The rest of the 'list' options work with a restore session on the current transport try { - mRestore = mBmgr.beginRestoreSessionForUser(userId, null, null, OperationType.BACKUP); + mRestore = mBmgr.beginRestoreSessionForUser(userId, null, null); if (mRestore == null) { System.err.println(BMGR_ERR_NO_RESTORESESSION_FOR_USER + userId); return; @@ -822,7 +821,7 @@ public class Bmgr { try { boolean didRestore = false; - mRestore = mBmgr.beginRestoreSessionForUser(userId, null, null, OperationType.BACKUP); + mRestore = mBmgr.beginRestoreSessionForUser(userId, null, null); if (mRestore == null) { System.err.println(BMGR_ERR_NO_RESTORESESSION_FOR_USER + userId); return; diff --git a/core/java/android/app/backup/BackupManager.java b/core/java/android/app/backup/BackupManager.java index 673de8fa7c8c..dae565e12fd7 100644 --- a/core/java/android/app/backup/BackupManager.java +++ b/core/java/android/app/backup/BackupManager.java @@ -361,36 +361,7 @@ public class BackupManager { try { // All packages, current transport IRestoreSession binder = - sService.beginRestoreSessionForUser(mContext.getUserId(), null, null, - OperationType.BACKUP); - if (binder != null) { - session = new RestoreSession(mContext, binder); - } - } catch (RemoteException e) { - Log.e(TAG, "beginRestoreSession() couldn't connect"); - } - } - return session; - } - - /** - * Begin the process of restoring data from backup. See the - * {@link android.app.backup.RestoreSession} class for documentation on that process. - * - * @param operationType Type of the operation, see {@link OperationType} - * - * @hide - */ - @RequiresPermission(android.Manifest.permission.BACKUP) - public RestoreSession beginRestoreSession(@OperationType int operationType) { - RestoreSession session = null; - checkServiceBinder(); - if (sService != null) { - try { - // All packages, current transport - IRestoreSession binder = - sService.beginRestoreSessionForUser(mContext.getUserId(), null, null, - operationType); + sService.beginRestoreSessionForUser(mContext.getUserId(), null, null); if (binder != null) { session = new RestoreSession(mContext, binder); } @@ -801,7 +772,7 @@ public class BackupManager { @SystemApi @RequiresPermission(android.Manifest.permission.BACKUP) public int requestBackup(String[] packages, BackupObserver observer) { - return requestBackup(packages, observer, null, 0, OperationType.BACKUP); + return requestBackup(packages, observer, null, 0); } /** @@ -826,31 +797,6 @@ public class BackupManager { @RequiresPermission(android.Manifest.permission.BACKUP) public int requestBackup(String[] packages, BackupObserver observer, BackupManagerMonitor monitor, int flags) { - return requestBackup(packages, observer, monitor, flags, OperationType.BACKUP); - } - - /** - * Request an immediate backup, providing an observer to which results of the backup operation - * will be published. The Android backup system will decide for each package whether it will - * be full app data backup or key/value-pair-based backup. - * - * <p>If this method returns {@link BackupManager#SUCCESS}, the OS will attempt to backup all - * provided packages using the remote transport. - * - * @param packages List of package names to backup. - * @param observer The {@link BackupObserver} to receive callbacks during the backup - * operation. Could be {@code null}. - * @param monitor The {@link BackupManagerMonitorWrapper} to receive callbacks of important - * events during the backup operation. Could be {@code null}. - * @param flags {@link #FLAG_NON_INCREMENTAL_BACKUP}. - * @param operationType {@link OperationType} - * @return {@link BackupManager#SUCCESS} on success; nonzero on error. - * @throws IllegalArgumentException on null or empty {@code packages} param. - * @hide - */ - @RequiresPermission(android.Manifest.permission.BACKUP) - public int requestBackup(String[] packages, BackupObserver observer, - BackupManagerMonitor monitor, int flags, @OperationType int operationType) { checkServiceBinder(); if (sService != null) { try { @@ -860,8 +806,7 @@ public class BackupManager { BackupManagerMonitorWrapper monitorWrapper = monitor == null ? null : new BackupManagerMonitorWrapper(monitor); - return sService.requestBackup(packages, observerWrapper, monitorWrapper, flags, - operationType); + return sService.requestBackup(packages, observerWrapper, monitorWrapper, flags); } catch (RemoteException e) { Log.e(TAG, "requestBackup() couldn't connect"); } diff --git a/core/java/android/app/backup/IBackupManager.aidl b/core/java/android/app/backup/IBackupManager.aidl index e1bbc08e72f3..bf5be95c4ab0 100644 --- a/core/java/android/app/backup/IBackupManager.aidl +++ b/core/java/android/app/backup/IBackupManager.aidl @@ -547,11 +547,9 @@ interface IBackupManager { * set can be restored. * @param transportID The name of the transport to use for the restore operation. * May be null, in which case the current active transport is used. - * @param operationType Type of the operation, see {@link BackupManager#OperationType} * @return An interface to the restore session, or null on error. */ - IRestoreSession beginRestoreSessionForUser(int userId, String packageName, String transportID, - int operationType); + IRestoreSession beginRestoreSessionForUser(int userId, String packageName, String transportID); /** * Notify the backup manager that a BackupAgent has completed the operation @@ -680,7 +678,7 @@ interface IBackupManager { * {@link android.app.backup.IBackupManager.requestBackupForUser} for the calling user id. */ int requestBackup(in String[] packages, IBackupObserver observer, IBackupManagerMonitor monitor, - int flags, int operationType); + int flags); /** * Cancel all running backups. After this call returns, no currently running backups will diff --git a/services/backup/java/com/android/server/backup/BackupManagerService.java b/services/backup/java/com/android/server/backup/BackupManagerService.java index 6c30999f63a4..38275f7cd348 100644 --- a/services/backup/java/com/android/server/backup/BackupManagerService.java +++ b/services/backup/java/com/android/server/backup/BackupManagerService.java @@ -1245,10 +1245,9 @@ public class BackupManagerService extends IBackupManager.Stub { @Override public IRestoreSession beginRestoreSessionForUser( - int userId, String packageName, String transportID, - @OperationType int operationType) throws RemoteException { + int userId, String packageName, String transportID) throws RemoteException { return isUserReadyForBackup(userId) - ? beginRestoreSession(userId, packageName, transportID, operationType) : null; + ? beginRestoreSession(userId, packageName, transportID) : null; } /** @@ -1257,15 +1256,13 @@ public class BackupManagerService extends IBackupManager.Stub { */ @Nullable public IRestoreSession beginRestoreSession( - @UserIdInt int userId, String packageName, String transportName, - @OperationType int operationType) { + @UserIdInt int userId, String packageName, String transportName) { UserBackupManagerService userBackupManagerService = getServiceForUserIfCallerHasPermission(userId, "beginRestoreSession()"); return userBackupManagerService == null ? null - : userBackupManagerService.beginRestoreSession(packageName, transportName, - operationType); + : userBackupManagerService.beginRestoreSession(packageName, transportName); } @Override @@ -1350,15 +1347,15 @@ public class BackupManagerService extends IBackupManager.Stub { if (!isUserReadyForBackup(userId)) { return BackupManager.ERROR_BACKUP_NOT_ALLOWED; } - return requestBackup(userId, packages, observer, monitor, flags, OperationType.BACKUP); + return requestBackup(userId, packages, observer, monitor, flags); } @Override public int requestBackup(String[] packages, IBackupObserver observer, - IBackupManagerMonitor monitor, int flags, @OperationType int operationType) + IBackupManagerMonitor monitor, int flags) throws RemoteException { return requestBackup(binderGetCallingUserId(), packages, - observer, monitor, flags, operationType); + observer, monitor, flags); } /** @@ -1370,15 +1367,13 @@ public class BackupManagerService extends IBackupManager.Stub { String[] packages, IBackupObserver observer, IBackupManagerMonitor monitor, - int flags, - @OperationType int operationType) { + int flags) { UserBackupManagerService userBackupManagerService = getServiceForUserIfCallerHasPermission(userId, "requestBackup()"); return userBackupManagerService == null ? BackupManager.ERROR_BACKUP_NOT_ALLOWED - : userBackupManagerService.requestBackup(packages, observer, monitor, flags, - operationType); + : userBackupManagerService.requestBackup(packages, observer, monitor, flags); } @Override diff --git a/services/backup/java/com/android/server/backup/UserBackupManagerService.java b/services/backup/java/com/android/server/backup/UserBackupManagerService.java index 136cd22fad83..9ee0159e903a 100644 --- a/services/backup/java/com/android/server/backup/UserBackupManagerService.java +++ b/services/backup/java/com/android/server/backup/UserBackupManagerService.java @@ -96,6 +96,7 @@ import android.text.TextUtils; import android.util.ArraySet; import android.util.AtomicFile; import android.util.EventLog; +import android.util.FeatureFlagUtils; import android.util.Pair; import android.util.Slog; import android.util.SparseArray; @@ -127,6 +128,7 @@ import com.android.server.backup.params.RestoreParams; import com.android.server.backup.restore.ActiveRestoreSession; import com.android.server.backup.restore.PerformUnifiedRestoreTask; import com.android.server.backup.transport.TransportClient; +import com.android.server.backup.transport.TransportNotAvailableException; import com.android.server.backup.transport.TransportNotRegisteredException; import com.android.server.backup.utils.BackupEligibilityRules; import com.android.server.backup.utils.BackupManagerMonitorUtils; @@ -1860,19 +1862,10 @@ public class UserBackupManagerService { /** * Requests a backup for the inputted {@code packages} with a specified {@link - * IBackupManagerMonitor}. - */ - public int requestBackup(String[] packages, IBackupObserver observer, - IBackupManagerMonitor monitor, int flags) { - return requestBackup(packages, observer, monitor, flags, OperationType.BACKUP); - } - - /** - * Requests a backup for the inputted {@code packages} with a specified {@link * IBackupManagerMonitor} and {@link OperationType}. */ public int requestBackup(String[] packages, IBackupObserver observer, - IBackupManagerMonitor monitor, int flags, @OperationType int operationType) { + IBackupManagerMonitor monitor, int flags) { mContext.enforceCallingPermission(android.Manifest.permission.BACKUP, "requestBackup"); if (packages == null || packages.length < 1) { @@ -1903,13 +1896,16 @@ public class UserBackupManagerService { final TransportClient transportClient; final String transportDirName; + int operationType; try { transportDirName = mTransportManager.getTransportDirName( mTransportManager.getCurrentTransportName()); transportClient = mTransportManager.getCurrentTransportClientOrThrow("BMS.requestBackup()"); - } catch (TransportNotRegisteredException e) { + operationType = getOperationTypeFromTransport(transportClient); + } catch (TransportNotRegisteredException | TransportNotAvailableException + | RemoteException e) { BackupObserverUtils.sendBackupFinished(observer, BackupManager.ERROR_TRANSPORT_ABORTED); monitor = BackupManagerMonitorUtils.monitorEvent(monitor, BackupManagerMonitor.LOG_EVENT_ID_TRANSPORT_IS_NULL, @@ -4024,15 +4020,13 @@ public class UserBackupManagerService { } /** Hand off a restore session. */ - public IRestoreSession beginRestoreSession(String packageName, String transport, - @OperationType int operationType) { + public IRestoreSession beginRestoreSession(String packageName, String transport) { if (DEBUG) { Slog.v( TAG, addUserIdToLogMessage( mUserId, - "beginRestoreSession: pkg=" + packageName + " transport=" + transport - + "operationType=" + operationType)); + "beginRestoreSession: pkg=" + packageName + " transport=" + transport)); } boolean needPermission = true; @@ -4073,6 +4067,17 @@ public class UserBackupManagerService { } } + int operationType; + try { + operationType = getOperationTypeFromTransport( + mTransportManager.getTransportClientOrThrow(transport, /* caller */ + "BMS.beginRestoreSession")); + } catch (TransportNotAvailableException | TransportNotRegisteredException + | RemoteException e) { + Slog.w(TAG, "Failed to get operation type from transport: " + e); + return null; + } + synchronized (this) { if (mActiveRestoreSession != null) { Slog.i( @@ -4356,6 +4361,34 @@ public class UserBackupManagerService { } } + @VisibleForTesting + @OperationType int getOperationTypeFromTransport(TransportClient transportClient) + throws TransportNotAvailableException, RemoteException { + if (!shouldUseNewBackupEligibilityRules()) { + // Return the default to stick to the legacy behaviour. + return OperationType.BACKUP; + } + + long oldCallingId = Binder.clearCallingIdentity(); + try { + IBackupTransport transport = transportClient.connectOrThrow( + /* caller */ "BMS.getOperationTypeFromTransport"); + if ((transport.getTransportFlags() & BackupAgent.FLAG_DEVICE_TO_DEVICE_TRANSFER) != 0) { + return OperationType.MIGRATION; + } else { + return OperationType.BACKUP; + } + } finally { + Binder.restoreCallingIdentity(oldCallingId); + } + } + + @VisibleForTesting + boolean shouldUseNewBackupEligibilityRules() { + return FeatureFlagUtils.isEnabled(mContext, + FeatureFlagUtils.SETTINGS_USE_NEW_BACKUP_ELIGIBILITY_RULES); + } + private static String addUserIdToLogMessage(int userId, String message) { return "[UserID:" + userId + "] " + message; } diff --git a/services/robotests/backup/src/com/android/server/backup/BackupManagerServiceRoboTest.java b/services/robotests/backup/src/com/android/server/backup/BackupManagerServiceRoboTest.java index cbebe6984794..2219d477630e 100644 --- a/services/robotests/backup/src/com/android/server/backup/BackupManagerServiceRoboTest.java +++ b/services/robotests/backup/src/com/android/server/backup/BackupManagerServiceRoboTest.java @@ -38,7 +38,6 @@ import static org.testng.Assert.expectThrows; import android.annotation.UserIdInt; import android.app.Application; -import android.app.backup.BackupManager.OperationType; import android.app.backup.IBackupManagerMonitor; import android.app.backup.IBackupObserver; import android.app.backup.IFullBackupRestoreObserver; @@ -874,8 +873,7 @@ public class BackupManagerServiceRoboTest { SecurityException.class, () -> backupManagerService.requestBackup( - mUserTwoId, packages, observer, monitor, 0, - OperationType.BACKUP)); + mUserTwoId, packages, observer, monitor, 0)); } /** @@ -893,11 +891,9 @@ public class BackupManagerServiceRoboTest { IBackupManagerMonitor monitor = mock(IBackupManagerMonitor.class); setCallerAndGrantInteractUserPermission(mUserOneId, /* shouldGrantPermission */ true); - backupManagerService.requestBackup(mUserTwoId, packages, observer, monitor, /* flags */ 0, - OperationType.BACKUP); + backupManagerService.requestBackup(mUserTwoId, packages, observer, monitor, /* flags */ 0); - verify(mUserTwoService).requestBackup(packages, observer, monitor, /* flags */ 0, - OperationType.BACKUP); + verify(mUserTwoService).requestBackup(packages, observer, monitor, /* flags */ 0); } /** Test that the backup service routes methods correctly to the user that requests it. */ @@ -910,11 +906,9 @@ public class BackupManagerServiceRoboTest { IBackupManagerMonitor monitor = mock(IBackupManagerMonitor.class); setCallerAndGrantInteractUserPermission(mUserOneId, /* shouldGrantPermission */ false); - backupManagerService.requestBackup(mUserOneId, packages, observer, monitor, /* flags */ 0, - OperationType.BACKUP); + backupManagerService.requestBackup(mUserOneId, packages, observer, monitor, /* flags */ 0); - verify(mUserOneService).requestBackup(packages, observer, monitor, /* flags */ 0, - OperationType.BACKUP); + verify(mUserOneService).requestBackup(packages, observer, monitor, /* flags */ 0); } /** Test that the backup service routes methods correctly to the user that requests it. */ @@ -927,11 +921,9 @@ public class BackupManagerServiceRoboTest { IBackupManagerMonitor monitor = mock(IBackupManagerMonitor.class); setCallerAndGrantInteractUserPermission(mUserTwoId, /* shouldGrantPermission */ false); - backupManagerService.requestBackup(mUserTwoId, packages, observer, monitor, /* flags */ 0, - OperationType.BACKUP); + backupManagerService.requestBackup(mUserTwoId, packages, observer, monitor, /* flags */ 0); - verify(mUserOneService, never()).requestBackup(packages, observer, monitor, /* flags */ 0, - OperationType.BACKUP); + verify(mUserOneService, never()).requestBackup(packages, observer, monitor, /* flags */ 0); } /** @@ -1092,11 +1084,9 @@ public class BackupManagerServiceRoboTest { registerUser(backupManagerService, mUserOneId, mUserOneService); setCallerAndGrantInteractUserPermission(mUserOneId, /* shouldGrantPermission */ false); - backupManagerService.beginRestoreSession(mUserOneId, TEST_PACKAGE, TEST_TRANSPORT, - OperationType.BACKUP); + backupManagerService.beginRestoreSession(mUserOneId, TEST_PACKAGE, TEST_TRANSPORT); - verify(mUserOneService).beginRestoreSession(TEST_PACKAGE, TEST_TRANSPORT, - OperationType.BACKUP); + verify(mUserOneService).beginRestoreSession(TEST_PACKAGE, TEST_TRANSPORT); } /** Test that the backup service does not route methods for non-registered users. */ @@ -1106,11 +1096,9 @@ public class BackupManagerServiceRoboTest { registerUser(backupManagerService, mUserOneId, mUserOneService); setCallerAndGrantInteractUserPermission(mUserTwoId, /* shouldGrantPermission */ false); - backupManagerService.beginRestoreSession(mUserTwoId, TEST_PACKAGE, TEST_TRANSPORT, - OperationType.BACKUP); + backupManagerService.beginRestoreSession(mUserTwoId, TEST_PACKAGE, TEST_TRANSPORT); - verify(mUserOneService, never()).beginRestoreSession(TEST_PACKAGE, TEST_TRANSPORT, - OperationType.BACKUP); + verify(mUserOneService, never()).beginRestoreSession(TEST_PACKAGE, TEST_TRANSPORT); } /** Test that the backup service routes methods correctly to the user that requests it. */ diff --git a/services/tests/servicestests/src/com/android/server/backup/UserBackupManagerServiceTest.java b/services/tests/servicestests/src/com/android/server/backup/UserBackupManagerServiceTest.java index b98f0257d7b7..205ff300c543 100644 --- a/services/tests/servicestests/src/com/android/server/backup/UserBackupManagerServiceTest.java +++ b/services/tests/servicestests/src/com/android/server/backup/UserBackupManagerServiceTest.java @@ -23,6 +23,7 @@ import static org.mockito.ArgumentMatchers.anyInt; import static org.mockito.ArgumentMatchers.anyString; import static org.mockito.Mockito.when; +import android.app.backup.BackupAgent; import android.app.backup.BackupManager.OperationType; import android.app.backup.IBackupManagerMonitor; import android.app.backup.IBackupObserver; @@ -30,6 +31,7 @@ import android.content.Context; import android.content.pm.ApplicationInfo; import android.content.pm.PackageInfo; import android.content.pm.PackageManager; +import com.android.internal.backup.IBackupTransport; import android.platform.test.annotations.Presubmit; import androidx.test.runner.AndroidJUnit4; @@ -56,6 +58,7 @@ public class UserBackupManagerServiceTest { @Mock IBackupObserver mBackupObserver; @Mock PackageManager mPackageManager; @Mock TransportClient mTransportClient; + @Mock IBackupTransport mBackupTransport; @Mock BackupEligibilityRules mBackupEligibilityRules; @@ -132,6 +135,33 @@ public class UserBackupManagerServiceTest { assertThat(params.mBackupEligibilityRules).isEqualTo(mBackupEligibilityRules); } + @Test + public void testGetOperationTypeFromTransport_returnsBackupByDefault() + throws Exception { + when(mTransportClient.connectOrThrow(any())).thenReturn(mBackupTransport); + when(mBackupTransport.getTransportFlags()).thenReturn(0); + + int operationType = mService.getOperationTypeFromTransport(mTransportClient); + + assertThat(operationType).isEqualTo(OperationType.BACKUP); + } + + @Test + public void testGetOperationTypeFromTransport_returnsMigrationForMigrationTransport() + throws Exception { + // This is a temporary flag to control the new behaviour until it's ready to be fully + // rolled out. + mService.shouldUseNewBackupEligibilityRules = true; + + when(mTransportClient.connectOrThrow(any())).thenReturn(mBackupTransport); + when(mBackupTransport.getTransportFlags()).thenReturn( + BackupAgent.FLAG_DEVICE_TO_DEVICE_TRANSFER); + + int operationType = mService.getOperationTypeFromTransport(mTransportClient); + + assertThat(operationType).isEqualTo(OperationType.MIGRATION); + } + private static PackageInfo getPackageInfo(String packageName) { PackageInfo packageInfo = new PackageInfo(); packageInfo.applicationInfo = new ApplicationInfo(); @@ -141,6 +171,7 @@ public class UserBackupManagerServiceTest { private static class TestBackupService extends UserBackupManagerService { boolean isEnabledStatePersisted = false; + boolean shouldUseNewBackupEligibilityRules = false; TestBackupService(Context context, PackageManager packageManager) { super(context, packageManager); @@ -158,5 +189,10 @@ public class UserBackupManagerServiceTest { @Override void updateStateOnBackupEnabled(boolean wasEnabled, boolean enable) {} + + @Override + boolean shouldUseNewBackupEligibilityRules() { + return shouldUseNewBackupEligibilityRules; + } } } |