From 7f14edea1d84bde1824529178c609fa5961051d2 Mon Sep 17 00:00:00 2001 From: Bernardo Rufino Date: Fri, 8 Dec 2017 19:55:03 +0000 Subject: [PATCH] Binding on-demand #6: Transport attributes usage Migrate the attribute queries from the Transport to the TransportManager. Migrate all calls except currentDestinationString because that's the one that changes and we should only migrate after we have GMSCore that implements the push-from-transport model. Looking at method recordInitPendingLocked(), we only sent MSG_RETRY_INIT if the transport threw while calling transportDirName or the binder was null. With binding on-demand both of these cases can't happen - i.e. we can't fail anymore. So, I removed the message entirely. Change-Id: I45a305704274c8b0c88637e3ccafc658639b2dfa Ref: http://go/br-binding-on-demand Bug: 17140907 Test: m -j RunFrameworksServicesRoboTests Test: gts-tradefed run commandAndExit gts-dev -m GtsBackupTestCases Test: gts-tradefed run commandAndExit gts-dev -m GtsBackupHostTestCases Test: cts-tradefed run commandAndExit cts-dev -m CtsBackupTestCases Test: runtest -p com.android.server.backup frameworks-services Test: adb shell bmgr backupnow Test: adb shell bmgr fullbackup Test: adb shell cmd jobscheduler run -f android Test: adb shell bmgr enable false (being enabled before) Test: adb shell dumpsys backup Test: adb shell bmgr init Test: Observed logs and used debugger to check proper code was being Test: called in above commands --- .../RefactoredBackupManagerService.java | 210 +++++++----------- .../server/backup/TransportManager.java | 87 +++++++- .../server/backup/internal/BackupHandler.java | 10 - .../internal/PerformInitializeTask.java | 6 +- .../TransportNotRegisteredException.java | 35 +++ .../server/backup/TransportManagerTest.java | 180 ++++++++++++--- .../backup/testing/BackupTransportStub.java | 179 --------------- .../backup/BackupManagerServiceTest.java | 6 +- 8 files changed, 356 insertions(+), 357 deletions(-) create mode 100644 services/backup/java/com/android/server/backup/transport/TransportNotRegisteredException.java delete mode 100644 services/robotests/src/com/android/server/backup/testing/BackupTransportStub.java diff --git a/services/backup/java/com/android/server/backup/RefactoredBackupManagerService.java b/services/backup/java/com/android/server/backup/RefactoredBackupManagerService.java index 4adcb99f56282..94b06b67ff870 100644 --- a/services/backup/java/com/android/server/backup/RefactoredBackupManagerService.java +++ b/services/backup/java/com/android/server/backup/RefactoredBackupManagerService.java @@ -25,7 +25,6 @@ import static com.android.server.backup.internal.BackupHandler.MSG_REQUEST_BACKU import static com.android.server.backup.internal.BackupHandler.MSG_RESTORE_OPERATION_TIMEOUT; import static com.android.server.backup.internal.BackupHandler.MSG_RESTORE_SESSION_TIMEOUT; import static com.android.server.backup.internal.BackupHandler.MSG_RETRY_CLEAR; -import static com.android.server.backup.internal.BackupHandler.MSG_RETRY_INIT; import static com.android.server.backup.internal.BackupHandler.MSG_RUN_ADB_BACKUP; import static com.android.server.backup.internal.BackupHandler.MSG_RUN_ADB_RESTORE; import static com.android.server.backup.internal.BackupHandler.MSG_RUN_CLEAR; @@ -120,6 +119,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.TransportNotRegisteredException; import com.android.server.backup.utils.AppBackupUtils; import com.android.server.backup.utils.BackupManagerMonitorUtils; import com.android.server.backup.utils.BackupObserverUtils; @@ -1083,56 +1083,35 @@ public class RefactoredBackupManagerService implements BackupManagerServiceInter return mBackupPasswordManager.backupPasswordMatches(currentPw); } - // Maintain persistent state around whether need to do an initialize operation. - // Must be called with the queue lock held. - public void recordInitPendingLocked(boolean isPending, String transportName) { + /** + * Maintain persistent state around whether need to do an initialize operation. + * Must be called with the queue lock held. + */ + @GuardedBy("mQueueLock") + public void recordInitPendingLocked( + boolean isPending, String transportName, String transportDirName) { if (MORE_DEBUG) { Slog.i(TAG, "recordInitPendingLocked: " + isPending + " on transport " + transportName); } - mBackupHandler.removeMessages(MSG_RETRY_INIT); - try { - IBackupTransport transport = mTransportManager.getTransportBinder(transportName); - if (transport != null) { - String transportDirName = transport.transportDirName(); - File stateDir = new File(mBaseStateDir, transportDirName); - File initPendingFile = new File(stateDir, INIT_SENTINEL_FILE_NAME); + File stateDir = new File(mBaseStateDir, transportDirName); + File initPendingFile = new File(stateDir, INIT_SENTINEL_FILE_NAME); - if (isPending) { - // We need an init before we can proceed with sending backup data. - // Record that with an entry in our set of pending inits, as well as - // journaling it via creation of a sentinel file. - mPendingInits.add(transportName); - try { - (new FileOutputStream(initPendingFile)).close(); - } catch (IOException ioe) { - // Something is badly wrong with our permissions; just try to move on - } - } else { - // No more initialization needed; wipe the journal and reset our state. - initPendingFile.delete(); - mPendingInits.remove(transportName); - } - return; // done; don't fall through to the error case - } - } catch (Exception e) { - // transport threw when asked its name; fall through to the lookup-failed case - Slog.e(TAG, "Transport " + transportName + " failed to report name: " - + e.getMessage()); - } - - // The named transport doesn't exist or threw. This operation is - // important, so we record the need for a an init and post a message - // to retry the init later. if (isPending) { + // We need an init before we can proceed with sending backup data. + // Record that with an entry in our set of pending inits, as well as + // journaling it via creation of a sentinel file. mPendingInits.add(transportName); - mBackupHandler.sendMessageDelayed( - mBackupHandler.obtainMessage(MSG_RETRY_INIT, - (isPending ? 1 : 0), - 0, - transportName), - TRANSPORT_RETRY_INTERVAL); + try { + (new FileOutputStream(initPendingFile)).close(); + } catch (IOException ioe) { + // Something is badly wrong with our permissions; just try to move on + } + } else { + // No more initialization needed; wipe the journal and reset our state. + initPendingFile.delete(); + mPendingInits.remove(transportName); } } @@ -1614,27 +1593,9 @@ public class RefactoredBackupManagerService implements BackupManagerServiceInter return BackupManager.ERROR_BACKUP_NOT_ALLOWED; } - // We're using pieces of the new binding on-demand infra-structure and the old always-bound - // infra-structure below this comment. The TransportManager.getCurrentTransportClient() line - // is using the new one and TransportManager.getCurrentTransportBinder() is using the old. - // This is weird but there is a reason. - // This is the natural place to put TransportManager.getCurrentTransportClient() because of - // the null handling below that should be the same for TransportClient. - // TransportClient.connect() would return a IBackupTransport for us (instead of using the - // old infra), but it may block and we don't want this in this thread. - // The only usage of transport in this method is for transport.transportDirName(). When the - // push-from-transport part of binding on-demand is in place we will replace the calls for - // IBackupTransport.transportDirName() with calls for - // TransportManager.transportDirName(transportName) or similar. So we'll leave the old piece - // here until we implement that. - // TODO(brufino): Remove always-bound code mTransportManager.getCurrentTransportBinder() TransportClient transportClient = mTransportManager.getCurrentTransportClient("BMS.requestBackup()"); - IBackupTransport transport = mTransportManager.getCurrentTransportBinder(); - if (transportClient == null || transport == null) { - if (transportClient != null) { - mTransportManager.disposeOfTransportClient(transportClient, "BMS.requestBackup()"); - } + if (transportClient == null) { BackupObserverUtils.sendBackupFinished(observer, BackupManager.ERROR_TRANSPORT_ABORTED); monitor = BackupManagerMonitorUtils.monitorEvent(monitor, BackupManagerMonitor.LOG_EVENT_ID_TRANSPORT_IS_NULL, @@ -1679,15 +1640,7 @@ public class RefactoredBackupManagerService implements BackupManagerServiceInter + " k/v backups"); } - String dirName; - try { - dirName = transport.transportDirName(); - } catch (Exception e) { - Slog.e(TAG, "Transport unavailable while attempting backup: " + e.getMessage()); - BackupObserverUtils.sendBackupFinished(observer, BackupManager.ERROR_TRANSPORT_ABORTED); - return BackupManager.ERROR_TRANSPORT_ABORTED; - } - + String dirName = transportClient.getTransportDirName(); boolean nonIncrementalBackup = (flags & BackupManager.FLAG_NON_INCREMENTAL_BACKUP) != 0; Message msg = mBackupHandler.obtainMessage(MSG_REQUEST_BACKUP); @@ -1998,16 +1951,17 @@ public class RefactoredBackupManagerService implements BackupManagerServiceInter writeFullBackupScheduleAsync(); } - private boolean fullBackupAllowable(IBackupTransport transport) { - if (transport == null) { - Slog.w(TAG, "Transport not present; full data backup not performed"); + private boolean fullBackupAllowable(String transportName) { + if (!mTransportManager.isTransportRegistered(transportName)) { + Slog.w(TAG, "Transport not registered; full data backup not performed"); return false; } // Don't proceed unless we have already established package metadata // for the current dataset via a key/value backup pass. try { - File stateDir = new File(mBaseStateDir, transport.transportDirName()); + String transportDirName = mTransportManager.getTransportDirName(transportName); + File stateDir = new File(mBaseStateDir, transportDirName); File pmState = new File(stateDir, PACKAGE_MANAGER_SENTINEL); if (pmState.length() <= 0) { if (DEBUG) { @@ -2097,7 +2051,8 @@ public class RefactoredBackupManagerService implements BackupManagerServiceInter headBusy = false; - if (!fullBackupAllowable(mTransportManager.getCurrentTransportBinder())) { + String transportName = mTransportManager.getCurrentTransportName(); + if (!fullBackupAllowable(transportName)) { if (MORE_DEBUG) { Slog.i(TAG, "Preconditions not met; not running full backup"); } @@ -2545,7 +2500,8 @@ public class RefactoredBackupManagerService implements BackupManagerServiceInter throw new IllegalStateException("Restore supported only for the device owner"); } - if (!fullBackupAllowable(mTransportManager.getCurrentTransportBinder())) { + String transportName = mTransportManager.getCurrentTransportName(); + if (!fullBackupAllowable(transportName)) { Slog.i(TAG, "Full backup not currently possible -- key/value backup not yet run?"); } else { if (DEBUG) { @@ -2826,10 +2782,30 @@ public class RefactoredBackupManagerService implements BackupManagerServiceInter if (wasEnabled && mProvisioned) { // NOTE: we currently flush every registered transport, not just // the currently-active one. - String[] allTransports = mTransportManager.getBoundTransportNames(); + List transportNames = new ArrayList<>(); + List transportDirNames = new ArrayList<>(); + mTransportManager.forEachRegisteredTransport( + name -> { + final String dirName; + try { + dirName = + mTransportManager + .getTransportDirName(name); + } catch (TransportNotRegisteredException e) { + // Should never happen + Slog.e(TAG, "Unexpected unregistered transport", e); + return; + } + transportNames.add(name); + transportDirNames.add(dirName); + }); + // build the set of transports for which we are posting an init - for (String transport : allTransports) { - recordInitPendingLocked(true, transport); + for (int i = 0; i < transportNames.size(); i++) { + recordInitPendingLocked( + true, + transportNames.get(i), + transportDirNames.get(i)); } mAlarmManager.set(AlarmManager.RTC_WAKEUP, System.currentTimeMillis(), mRunInitIntent); @@ -2993,7 +2969,7 @@ public class RefactoredBackupManagerService implements BackupManagerServiceInter final long oldId = Binder.clearCallingIdentity(); try { - mTransportManager.describeTransport( + mTransportManager.updateTransportAttributes( transportComponent, name, configurationIntent, @@ -3093,23 +3069,16 @@ public class RefactoredBackupManagerService implements BackupManagerServiceInter public Intent getConfigurationIntent(String transportName) { mContext.enforceCallingOrSelfPermission(android.Manifest.permission.BACKUP, "getConfigurationIntent"); - - final IBackupTransport transport = mTransportManager.getTransportBinder(transportName); - if (transport != null) { - try { - final Intent intent = transport.configurationIntent(); - if (MORE_DEBUG) { - Slog.d(TAG, "getConfigurationIntent() returning config intent " - + intent); - } - return intent; - } catch (Exception e) { - /* fall through to return null */ - Slog.e(TAG, "Unable to get configuration intent from transport: " + e.getMessage()); + try { + Intent intent = mTransportManager.getTransportConfigurationIntent(transportName); + if (MORE_DEBUG) { + Slog.d(TAG, "getConfigurationIntent() returning intent " + intent); } + return intent; + } catch (TransportNotRegisteredException e) { + Slog.e(TAG, "Unable to get configuration intent from transport: " + e.getMessage()); + return null; } - - return null; } // Supply the configuration summary string for the given transport. If the name is @@ -3143,22 +3112,16 @@ public class RefactoredBackupManagerService implements BackupManagerServiceInter mContext.enforceCallingOrSelfPermission(android.Manifest.permission.BACKUP, "getDataManagementIntent"); - final IBackupTransport transport = mTransportManager.getTransportBinder(transportName); - if (transport != null) { - try { - final Intent intent = transport.dataManagementIntent(); - if (MORE_DEBUG) { - Slog.d(TAG, "getDataManagementIntent() returning intent " - + intent); - } - return intent; - } catch (Exception e) { - /* fall through to return null */ - Slog.e(TAG, "Unable to get management intent from transport: " + e.getMessage()); + try { + Intent intent = mTransportManager.getTransportDataManagementIntent(transportName); + if (MORE_DEBUG) { + Slog.d(TAG, "getDataManagementIntent() returning intent " + intent); } + return intent; + } catch (TransportNotRegisteredException e) { + Slog.e(TAG, "Unable to get management intent from transport: " + e.getMessage()); + return null; } - - return null; } // Supply the menu label for affordances that fire the manage-data intent @@ -3168,19 +3131,16 @@ public class RefactoredBackupManagerService implements BackupManagerServiceInter mContext.enforceCallingOrSelfPermission(android.Manifest.permission.BACKUP, "getDataManagementLabel"); - final IBackupTransport transport = mTransportManager.getTransportBinder(transportName); - if (transport != null) { - try { - final String text = transport.dataManagementLabel(); - if (MORE_DEBUG) Slog.d(TAG, "getDataManagementLabel() returning " + text); - return text; - } catch (Exception e) { - /* fall through to return null */ - Slog.e(TAG, "Unable to get management label from transport: " + e.getMessage()); + try { + String label = mTransportManager.getTransportDataManagementLabel(transportName); + if (MORE_DEBUG) { + Slog.d(TAG, "getDataManagementLabel() returning " + label); } + return label; + } catch (TransportNotRegisteredException e) { + Slog.e(TAG, "Unable to get management label from transport: " + e.getMessage()); + return null; } - - return null; } // Callback: a requested backup agent has been instantiated. This should only @@ -3497,14 +3457,16 @@ public class RefactoredBackupManagerService implements BackupManagerServiceInter pw.println("Available transports:"); final String[] transports = listAllTransports(); if (transports != null) { - for (String t : listAllTransports()) { + for (String t : transports) { pw.println((t.equals(mTransportManager.getCurrentTransportName()) ? " * " : " ") + t); try { IBackupTransport transport = mTransportManager.getTransportBinder(t); - File dir = new File(mBaseStateDir, transport.transportDirName()); + File dir = new File(mBaseStateDir, + mTransportManager.getTransportDirName(t)); pw.println(" destination: " + transport.currentDestinationString()); - pw.println(" intent: " + transport.configurationIntent()); + pw.println(" intent: " + + mTransportManager.getTransportConfigurationIntent(t)); for (File f : dir.listFiles()) { pw.println( " " + f.getName() + " - " + f.length() + " state bytes"); diff --git a/services/backup/java/com/android/server/backup/TransportManager.java b/services/backup/java/com/android/server/backup/TransportManager.java index 1f3ebf936ceb0..f1854433dcd09 100644 --- a/services/backup/java/com/android/server/backup/TransportManager.java +++ b/services/backup/java/com/android/server/backup/TransportManager.java @@ -49,12 +49,14 @@ import com.android.server.EventLogTags; import com.android.server.backup.transport.TransportClient; import com.android.server.backup.transport.TransportClientManager; import com.android.server.backup.transport.TransportConnectionListener; +import com.android.server.backup.transport.TransportNotRegisteredException; import java.util.ArrayList; import java.util.Iterator; import java.util.List; import java.util.Map; import java.util.Set; +import java.util.function.Consumer; import java.util.function.Predicate; /** @@ -236,6 +238,72 @@ public class TransportManager { return getTransportBinder(mCurrentTransportName); } + /** + * Retrieve the configuration intent of {@code transportName}. + * @throws TransportNotRegisteredException if the transport is not registered. + */ + @Nullable + public Intent getTransportConfigurationIntent(String transportName) + throws TransportNotRegisteredException { + synchronized (mTransportLock) { + return getRegisteredTransportDescriptionOrThrowLocked(transportName) + .configurationIntent; + } + } + + /** + * Retrieve the data management intent of {@code transportName}. + * @throws TransportNotRegisteredException if the transport is not registered. + */ + @Nullable + public Intent getTransportDataManagementIntent(String transportName) + throws TransportNotRegisteredException { + synchronized (mTransportLock) { + return getRegisteredTransportDescriptionOrThrowLocked(transportName) + .dataManagementIntent; + } + } + + /** + * Retrieve the data management label of {@code transportName}. + * @throws TransportNotRegisteredException if the transport is not registered. + */ + @Nullable + public String getTransportDataManagementLabel(String transportName) + throws TransportNotRegisteredException { + synchronized (mTransportLock) { + return getRegisteredTransportDescriptionOrThrowLocked(transportName) + .dataManagementLabel; + } + } + + /** + * Retrieve the transport dir name of {@code transportName}. + * @throws TransportNotRegisteredException if the transport is not registered. + */ + public String getTransportDirName(String transportName) + throws TransportNotRegisteredException { + synchronized (mTransportLock) { + return getRegisteredTransportDescriptionOrThrowLocked(transportName) + .transportDirName; + } + } + + /** + * Execute {@code transportConsumer} for each registered transport passing the transport name. + * This is called with an internal lock held, ensuring that the transport will remain registered + * while {@code transportConsumer} is being executed. Don't do heavy operations in + * {@code transportConsumer}. + */ + public void forEachRegisteredTransport(Consumer transportConsumer) { + synchronized (mTransportLock) { + for (TransportDescription transportDescription + : mRegisteredTransportsDescriptionMap.values()) { + transportConsumer.accept(transportDescription.name); + } + } + } + public String getTransportName(IBackupTransport binder) { synchronized (mTransportLock) { for (TransportConnection conn : mValidTransports.values()) { @@ -280,6 +348,17 @@ public class TransportManager { return (entry == null) ? null : entry.getValue(); } + @GuardedBy("mTransportLock") + private TransportDescription getRegisteredTransportDescriptionOrThrowLocked( + String transportName) throws TransportNotRegisteredException { + TransportDescription description = getRegisteredTransportDescriptionLocked(transportName); + if (description == null) { + throw new TransportNotRegisteredException(transportName); + } + return description; + } + + @GuardedBy("mTransportLock") @Nullable private Map.Entry getRegisteredTransportEntryLocked( @@ -385,13 +464,13 @@ public class TransportManager { * Updates given values for the transport already registered and identified with * {@param transportComponent}. If the transport is not registered it will log and return. */ - public void describeTransport( + public void updateTransportAttributes( ComponentName transportComponent, String name, @Nullable Intent configurationIntent, String currentDestinationString, @Nullable Intent dataManagementIntent, - String dataManagementLabel) { + @Nullable String dataManagementLabel) { synchronized (mTransportLock) { TransportDescription description = mRegisteredTransportsDescriptionMap.get(transportComponent); @@ -766,7 +845,7 @@ public class TransportManager { @Nullable private Intent configurationIntent; private String currentDestinationString; @Nullable private Intent dataManagementIntent; - private String dataManagementLabel; + @Nullable private String dataManagementLabel; private TransportDescription( String name, @@ -774,7 +853,7 @@ public class TransportManager { @Nullable Intent configurationIntent, String currentDestinationString, @Nullable Intent dataManagementIntent, - String dataManagementLabel) { + @Nullable String dataManagementLabel) { this.name = name; this.transportDirName = transportDirName; this.configurationIntent = configurationIntent; 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 4c78348250543..f29a9c2ecb6de 100644 --- a/services/backup/java/com/android/server/backup/internal/BackupHandler.java +++ b/services/backup/java/com/android/server/backup/internal/BackupHandler.java @@ -293,16 +293,6 @@ public class BackupHandler extends Handler { break; } - case MSG_RETRY_INIT: { - synchronized (backupManagerService.getQueueLock()) { - backupManagerService.recordInitPendingLocked(msg.arg1 != 0, (String) msg.obj); - backupManagerService.getAlarmManager().set(AlarmManager.RTC_WAKEUP, - System.currentTimeMillis(), - backupManagerService.getRunInitIntent()); - } - break; - } - case MSG_RUN_GET_RESTORE_SETS: { // Like other async operations, this is entered with the wakelock held RestoreSet[] sets = null; diff --git a/services/backup/java/com/android/server/backup/internal/PerformInitializeTask.java b/services/backup/java/com/android/server/backup/internal/PerformInitializeTask.java index 690922fd9aa90..b21b0724acc2f 100644 --- a/services/backup/java/com/android/server/backup/internal/PerformInitializeTask.java +++ b/services/backup/java/com/android/server/backup/internal/PerformInitializeTask.java @@ -98,7 +98,8 @@ public class PerformInitializeTask implements Runnable { transportDirName)); EventLog.writeEvent(EventLogTags.BACKUP_SUCCESS, 0, millis); synchronized (backupManagerService.getQueueLock()) { - backupManagerService.recordInitPendingLocked(false, transportName); + backupManagerService.recordInitPendingLocked( + false, transportName, transportDirName); } notifyResult(transportName, BackupTransport.TRANSPORT_OK); } else { @@ -107,7 +108,8 @@ public class PerformInitializeTask implements Runnable { Slog.e(TAG, "Transport error in initializeDevice()"); EventLog.writeEvent(EventLogTags.BACKUP_TRANSPORT_FAILURE, "(initialize)"); synchronized (backupManagerService.getQueueLock()) { - backupManagerService.recordInitPendingLocked(true, transportName); + backupManagerService.recordInitPendingLocked( + true, transportName, transportDirName); } notifyResult(transportName, status); result = status; diff --git a/services/backup/java/com/android/server/backup/transport/TransportNotRegisteredException.java b/services/backup/java/com/android/server/backup/transport/TransportNotRegisteredException.java new file mode 100644 index 0000000000000..26bf92cb10eb6 --- /dev/null +++ b/services/backup/java/com/android/server/backup/transport/TransportNotRegisteredException.java @@ -0,0 +1,35 @@ +/* + * 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.transport; + +import android.util.AndroidException; + +import com.android.server.backup.TransportManager; + +/** + * Exception thrown when the transport is not registered. + * + * @see TransportManager#getTransportDirName(String) + * @see TransportManager#getTransportConfigurationIntent(String) + * @see TransportManager#getTransportDataManagementIntent(String) + * @see TransportManager#getTransportDataManagementLabel(String) + */ +public class TransportNotRegisteredException extends AndroidException { + public TransportNotRegisteredException(String transportName) { + super("Transport " + transportName + " not registered"); + } +} diff --git a/services/robotests/src/com/android/server/backup/TransportManagerTest.java b/services/robotests/src/com/android/server/backup/TransportManagerTest.java index 2cb4a69cdcc15..13623e50d9c7a 100644 --- a/services/robotests/src/com/android/server/backup/TransportManagerTest.java +++ b/services/robotests/src/com/android/server/backup/TransportManagerTest.java @@ -18,9 +18,13 @@ package com.android.server.backup; import static com.google.common.truth.Truth.assertThat; +import static junit.framework.Assert.fail; + import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; import static org.robolectric.shadow.api.Shadow.extract; +import android.annotation.Nullable; import android.app.backup.BackupManager; import android.content.ComponentName; import android.content.Intent; @@ -29,15 +33,18 @@ import android.content.pm.PackageInfo; import android.content.pm.ResolveInfo; import android.content.pm.ServiceInfo; import android.os.IBinder; +import android.os.RemoteException; import android.platform.test.annotations.Presubmit; -import com.android.server.backup.testing.BackupTransportStub; +import com.android.internal.backup.IBackupTransport; +import com.android.internal.util.FunctionalUtils.ThrowingRunnable; import com.android.server.backup.testing.ShadowBackupTransportStub; import com.android.server.backup.testing.ShadowContextImplForBackup; import com.android.server.backup.testing.ShadowPackageManagerForBackup; import com.android.server.backup.testing.TransportBoundListenerStub; import com.android.server.backup.testing.TransportReadyCallbackStub; import com.android.server.backup.transport.TransportClient; +import com.android.server.backup.transport.TransportNotRegisteredException; import com.android.server.testing.FrameworkRobolectricTestRunner; import com.android.server.testing.SystemLoaderClasses; @@ -95,15 +102,29 @@ public class TransportManagerTest { (ShadowPackageManagerForBackup) extract(RuntimeEnvironment.application.getPackageManager()); - mTransport1 = new TransportInfo(PACKAGE_NAME, "transport1.name"); - mTransport2 = new TransportInfo(PACKAGE_NAME, "transport2.name"); + mTransport1 = new TransportInfo( + PACKAGE_NAME, + "transport1.name", + new Intent(), + "currentDestinationString", + new Intent(), + "dataManagementLabel"); + mTransport2 = new TransportInfo( + PACKAGE_NAME, + "transport2.name", + new Intent(), + "currentDestinationString", + new Intent(), + "dataManagementLabel"); ShadowContextImplForBackup.sComponentBinderMap.put(mTransport1.componentName, mTransport1.binder); ShadowContextImplForBackup.sComponentBinderMap.put(mTransport2.componentName, mTransport2.binder); - ShadowBackupTransportStub.sBinderTransportMap.put(mTransport1.binder, mTransport1.stub); - ShadowBackupTransportStub.sBinderTransportMap.put(mTransport2.binder, mTransport2.stub); + ShadowBackupTransportStub.sBinderTransportMap.put( + mTransport1.binder, mTransport1.binderInterface); + ShadowBackupTransportStub.sBinderTransportMap.put( + mTransport2.binder, mTransport2.binderInterface); } @After @@ -129,8 +150,10 @@ public class TransportManagerTest { Arrays.asList(mTransport1.componentName, mTransport2.componentName)); assertThat(transportManager.getBoundTransportNames()).asList().containsExactlyElementsIn( Arrays.asList(mTransport1.name, mTransport2.name)); - assertThat(mTransportBoundListenerStub.isCalledForTransport(mTransport1.stub)).isTrue(); - assertThat(mTransportBoundListenerStub.isCalledForTransport(mTransport2.stub)).isTrue(); + assertThat(mTransportBoundListenerStub.isCalledForTransport(mTransport1.binderInterface)) + .isTrue(); + assertThat(mTransportBoundListenerStub.isCalledForTransport(mTransport2.binderInterface)) + .isTrue(); } @Test @@ -153,8 +176,10 @@ public class TransportManagerTest { Collections.singleton(mTransport2.componentName)); assertThat(transportManager.getBoundTransportNames()).asList().containsExactlyElementsIn( Collections.singleton(mTransport2.name)); - assertThat(mTransportBoundListenerStub.isCalledForTransport(mTransport1.stub)).isFalse(); - assertThat(mTransportBoundListenerStub.isCalledForTransport(mTransport2.stub)).isTrue(); + assertThat(mTransportBoundListenerStub.isCalledForTransport(mTransport1.binderInterface)) + .isFalse(); + assertThat(mTransportBoundListenerStub.isCalledForTransport(mTransport2.binderInterface)) + .isTrue(); } @Test @@ -193,8 +218,10 @@ public class TransportManagerTest { Collections.singleton(mTransport2.componentName)); assertThat(transportManager.getBoundTransportNames()).asList().containsExactlyElementsIn( Collections.singleton(mTransport2.name)); - assertThat(mTransportBoundListenerStub.isCalledForTransport(mTransport1.stub)).isFalse(); - assertThat(mTransportBoundListenerStub.isCalledForTransport(mTransport2.stub)).isTrue(); + assertThat(mTransportBoundListenerStub.isCalledForTransport(mTransport1.binderInterface)) + .isFalse(); + assertThat(mTransportBoundListenerStub.isCalledForTransport(mTransport2.binderInterface)) + .isTrue(); } @Test @@ -250,8 +277,10 @@ public class TransportManagerTest { Arrays.asList(mTransport1.componentName, mTransport2.componentName)); assertThat(transportManager.getBoundTransportNames()).asList().containsExactlyElementsIn( Arrays.asList(mTransport1.name, mTransport2.name)); - assertThat(mTransportBoundListenerStub.isCalledForTransport(mTransport1.stub)).isFalse(); - assertThat(mTransportBoundListenerStub.isCalledForTransport(mTransport2.stub)).isTrue(); + assertThat(mTransportBoundListenerStub.isCalledForTransport(mTransport1.binderInterface)) + .isFalse(); + assertThat(mTransportBoundListenerStub.isCalledForTransport(mTransport2.binderInterface)) + .isTrue(); } @Test @@ -265,8 +294,10 @@ public class TransportManagerTest { Arrays.asList(mTransport1.componentName, mTransport2.componentName)); assertThat(transportManager.getBoundTransportNames()).asList().containsExactlyElementsIn( Arrays.asList(mTransport1.name, mTransport2.name)); - assertThat(mTransportBoundListenerStub.isCalledForTransport(mTransport1.stub)).isFalse(); - assertThat(mTransportBoundListenerStub.isCalledForTransport(mTransport2.stub)).isFalse(); + assertThat(mTransportBoundListenerStub.isCalledForTransport(mTransport1.binderInterface)) + .isFalse(); + assertThat(mTransportBoundListenerStub.isCalledForTransport(mTransport2.binderInterface)) + .isFalse(); } @Test @@ -280,8 +311,10 @@ public class TransportManagerTest { Arrays.asList(mTransport1.componentName, mTransport2.componentName)); assertThat(transportManager.getBoundTransportNames()).asList().containsExactlyElementsIn( Arrays.asList(mTransport1.name, mTransport2.name)); - assertThat(mTransportBoundListenerStub.isCalledForTransport(mTransport1.stub)).isFalse(); - assertThat(mTransportBoundListenerStub.isCalledForTransport(mTransport2.stub)).isFalse(); + assertThat(mTransportBoundListenerStub.isCalledForTransport(mTransport1.binderInterface)) + .isFalse(); + assertThat(mTransportBoundListenerStub.isCalledForTransport(mTransport2.binderInterface)) + .isFalse(); } @Test @@ -295,8 +328,10 @@ public class TransportManagerTest { Arrays.asList(mTransport1.componentName, mTransport2.componentName)); assertThat(transportManager.getBoundTransportNames()).asList().containsExactlyElementsIn( Arrays.asList(mTransport1.name, mTransport2.name)); - assertThat(mTransportBoundListenerStub.isCalledForTransport(mTransport1.stub)).isFalse(); - assertThat(mTransportBoundListenerStub.isCalledForTransport(mTransport2.stub)).isTrue(); + assertThat(mTransportBoundListenerStub.isCalledForTransport(mTransport1.binderInterface)) + .isFalse(); + assertThat(mTransportBoundListenerStub.isCalledForTransport(mTransport2.binderInterface)) + .isTrue(); } @Test @@ -305,9 +340,9 @@ public class TransportManagerTest { Arrays.asList(mTransport1, mTransport2), mTransport1.name); assertThat(transportManager.getTransportBinder(mTransport1.name)).isEqualTo( - mTransport1.stub); + mTransport1.binderInterface); assertThat(transportManager.getTransportBinder(mTransport2.name)).isEqualTo( - mTransport2.stub); + mTransport2.binderInterface); } @Test @@ -326,7 +361,7 @@ public class TransportManagerTest { assertThat(transportManager.getTransportBinder(mTransport1.name)).isNull(); assertThat(transportManager.getTransportBinder(mTransport2.name)).isEqualTo( - mTransport2.stub); + mTransport2.binderInterface); } @Test @@ -356,7 +391,8 @@ public class TransportManagerTest { TransportManager transportManager = createTransportManagerAndSetUpTransports( Arrays.asList(mTransport1, mTransport2), mTransport1.name); - assertThat(transportManager.getCurrentTransportBinder()).isEqualTo(mTransport1.stub); + assertThat(transportManager.getCurrentTransportBinder()) + .isEqualTo(mTransport1.binderInterface); } @Test @@ -375,8 +411,10 @@ public class TransportManagerTest { TransportManager transportManager = createTransportManagerAndSetUpTransports( Arrays.asList(mTransport1, mTransport2), mTransport1.name); - assertThat(transportManager.getTransportName(mTransport1.stub)).isEqualTo(mTransport1.name); - assertThat(transportManager.getTransportName(mTransport2.stub)).isEqualTo(mTransport2.name); + assertThat(transportManager.getTransportName(mTransport1.binderInterface)) + .isEqualTo(mTransport1.name); + assertThat(transportManager.getTransportName(mTransport2.binderInterface)) + .isEqualTo(mTransport2.name); } @Test @@ -385,8 +423,9 @@ public class TransportManagerTest { createTransportManagerAndSetUpTransports(Collections.singletonList(mTransport2), Collections.singletonList(mTransport1), mTransport1.name); - assertThat(transportManager.getTransportName(mTransport1.stub)).isNull(); - assertThat(transportManager.getTransportName(mTransport2.stub)).isEqualTo(mTransport2.name); + assertThat(transportManager.getTransportName(mTransport1.binderInterface)).isNull(); + assertThat(transportManager.getTransportName(mTransport2.binderInterface)) + .isEqualTo(mTransport2.name); } @Test @@ -499,7 +538,7 @@ public class TransportManagerTest { TransportManager transportManager = createTransportManagerAndSetUpTransports( Arrays.asList(mTransport1, mTransport2), mTransport1.name); - transportManager.describeTransport( + transportManager.updateTransportAttributes( mTransport1.componentName, "newName", null, "destinationString", null, null); TransportClient transportClient = @@ -514,7 +553,7 @@ public class TransportManagerTest { TransportManager transportManager = createTransportManagerAndSetUpTransports( Arrays.asList(mTransport1, mTransport2), mTransport1.name); - transportManager.describeTransport( + transportManager.updateTransportAttributes( mTransport1.componentName, "newName", null, "destinationString", null, null); TransportClient transportClient = @@ -529,7 +568,7 @@ public class TransportManagerTest { TransportManager transportManager = createTransportManagerAndSetUpTransports( Arrays.asList(mTransport1, mTransport2), mTransport1.name); - transportManager.describeTransport( + transportManager.updateTransportAttributes( mTransport1.componentName, "newName", null, "destinationString", null, null); String transportName = transportManager.getTransportName(mTransport1.componentName); @@ -549,6 +588,48 @@ public class TransportManagerTest { assertThat(transportManager.isTransportRegistered(mTransport2.name)).isFalse(); } + @Test + public void getTransportAttributes_forRegisteredTransport_returnsCorrectValues() + throws Exception { + TransportManager transportManager = + createTransportManagerAndSetUpTransports( + Collections.singletonList(mTransport1), + mTransport1.name); + + assertThat(transportManager.getTransportConfigurationIntent(mTransport1.name)) + .isEqualTo(mTransport1.binderInterface.configurationIntent()); + assertThat(transportManager.getTransportDataManagementIntent(mTransport1.name)) + .isEqualTo(mTransport1.binderInterface.dataManagementIntent()); + assertThat(transportManager.getTransportDataManagementLabel(mTransport1.name)) + .isEqualTo(mTransport1.binderInterface.dataManagementLabel()); + assertThat(transportManager.getTransportDirName(mTransport1.name)) + .isEqualTo(mTransport1.binderInterface.transportDirName()); + } + + @Test + public void getTransportAttributes_forUnregisteredTransport_throws() + throws Exception { + TransportManager transportManager = + createTransportManagerAndSetUpTransports( + Collections.singletonList(mTransport1), + Collections.singletonList(mTransport2), + mTransport1.name); + + expectThrows( + TransportNotRegisteredException.class, + () -> transportManager.getTransportConfigurationIntent(mTransport2.name)); + expectThrows( + TransportNotRegisteredException.class, + () -> transportManager.getTransportDataManagementIntent( + mTransport2.name)); + expectThrows( + TransportNotRegisteredException.class, + () -> transportManager.getTransportDataManagementLabel(mTransport2.name)); + expectThrows( + TransportNotRegisteredException.class, + () -> transportManager.getTransportDirName(mTransport2.name)); + } + private void setUpPackageWithTransports(String packageName, List transports, int flags) throws Exception { PackageInfo packageInfo = new PackageInfo(); @@ -616,10 +697,12 @@ public class TransportManagerTest { assertThat(transportManager.getBoundTransportNames()).asList().containsExactlyElementsIn( availableTransportsNames); for (TransportInfo transport : availableTransports) { - assertThat(mTransportBoundListenerStub.isCalledForTransport(transport.stub)).isTrue(); + assertThat(mTransportBoundListenerStub.isCalledForTransport(transport.binderInterface)) + .isTrue(); } for (TransportInfo transport : unavailableTransports) { - assertThat(mTransportBoundListenerStub.isCalledForTransport(transport.stub)).isFalse(); + assertThat(mTransportBoundListenerStub.isCalledForTransport(transport.binderInterface)) + .isFalse(); } mTransportBoundListenerStub.resetState(); @@ -627,19 +710,46 @@ public class TransportManagerTest { return transportManager; } + private static void expectThrows( + Class throwableClass, ThrowingRunnable runnable) { + try { + runnable.runOrThrow(); + fail("Expected to throw " + throwableClass.getSimpleName()); + } catch (Throwable t) { + assertThat(t).isInstanceOf(throwableClass); + } + } + private static class TransportInfo { public final String packageName; public final String name; public final ComponentName componentName; - public final BackupTransportStub stub; + public final IBackupTransport binderInterface; public final IBinder binder; - TransportInfo(String packageName, String name) { + TransportInfo( + String packageName, + String name, + @Nullable Intent configurationIntent, + String currentDestinationString, + @Nullable Intent dataManagementIntent, + String dataManagementLabel) { this.packageName = packageName; this.name = name; this.componentName = new ComponentName(packageName, name); - this.stub = new BackupTransportStub(name); this.binder = mock(IBinder.class); + IBackupTransport transport = mock(IBackupTransport.class); + try { + when(transport.name()).thenReturn(name); + when(transport.configurationIntent()).thenReturn(configurationIntent); + when(transport.currentDestinationString()).thenReturn(currentDestinationString); + when(transport.dataManagementIntent()).thenReturn(dataManagementIntent); + when(transport.dataManagementLabel()).thenReturn(dataManagementLabel); + } catch (RemoteException e) { + // Only here to mock methods that throw RemoteException + } + this.binderInterface = transport; } } + } diff --git a/services/robotests/src/com/android/server/backup/testing/BackupTransportStub.java b/services/robotests/src/com/android/server/backup/testing/BackupTransportStub.java deleted file mode 100644 index ec09f908c90d6..0000000000000 --- a/services/robotests/src/com/android/server/backup/testing/BackupTransportStub.java +++ /dev/null @@ -1,179 +0,0 @@ -/* - * 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.testing; - -import android.app.backup.RestoreDescription; -import android.app.backup.RestoreSet; -import android.content.Intent; -import android.content.pm.PackageInfo; -import android.os.IBinder; -import android.os.ParcelFileDescriptor; -import android.os.RemoteException; - -import com.android.internal.backup.IBackupTransport; - -/** - * Stub backup transport, doing nothing and returning default values. - */ -public class BackupTransportStub implements IBackupTransport { - - private final String mName; - - public BackupTransportStub(String name) { - mName = name; - } - - @Override - public IBinder asBinder() { - return null; - } - - @Override - public String name() throws RemoteException { - return mName; - } - - @Override - public Intent configurationIntent() throws RemoteException { - return null; - } - - @Override - public String currentDestinationString() throws RemoteException { - return null; - } - - @Override - public Intent dataManagementIntent() throws RemoteException { - return null; - } - - @Override - public String dataManagementLabel() throws RemoteException { - return null; - } - - @Override - public String transportDirName() throws RemoteException { - return null; - } - - @Override - public long requestBackupTime() throws RemoteException { - return 0; - } - - @Override - public int initializeDevice() throws RemoteException { - return 0; - } - - @Override - public int performBackup(PackageInfo packageInfo, ParcelFileDescriptor inFd, int flags) - throws RemoteException { - return 0; - } - - @Override - public int clearBackupData(PackageInfo packageInfo) throws RemoteException { - return 0; - } - - @Override - public int finishBackup() throws RemoteException { - return 0; - } - - @Override - public RestoreSet[] getAvailableRestoreSets() throws RemoteException { - return new RestoreSet[0]; - } - - @Override - public long getCurrentRestoreSet() throws RemoteException { - return 0; - } - - @Override - public int startRestore(long token, PackageInfo[] packages) throws RemoteException { - return 0; - } - - @Override - public RestoreDescription nextRestorePackage() throws RemoteException { - return null; - } - - @Override - public int getRestoreData(ParcelFileDescriptor outFd) throws RemoteException { - return 0; - } - - @Override - public void finishRestore() throws RemoteException { - - } - - @Override - public long requestFullBackupTime() throws RemoteException { - return 0; - } - - @Override - public int performFullBackup(PackageInfo targetPackage, ParcelFileDescriptor socket, - int flags) - throws RemoteException { - return 0; - } - - @Override - public int checkFullBackupSize(long size) throws RemoteException { - return 0; - } - - @Override - public int sendBackupData(int numBytes) throws RemoteException { - return 0; - } - - @Override - public void cancelFullBackup() throws RemoteException { - - } - - @Override - public boolean isAppEligibleForBackup(PackageInfo targetPackage, boolean isFullBackup) - throws RemoteException { - return false; - } - - @Override - public long getBackupQuota(String packageName, boolean isFullBackup) - throws RemoteException { - return 0; - } - - @Override - public int getNextFullRestoreDataChunk(ParcelFileDescriptor socket) throws RemoteException { - return 0; - } - - @Override - public int abortFullRestore() throws RemoteException { - return 0; - } -} diff --git a/services/tests/servicestests/src/com/android/server/backup/BackupManagerServiceTest.java b/services/tests/servicestests/src/com/android/server/backup/BackupManagerServiceTest.java index 362856c707c78..f4c54420853cb 100644 --- a/services/tests/servicestests/src/com/android/server/backup/BackupManagerServiceTest.java +++ b/services/tests/servicestests/src/com/android/server/backup/BackupManagerServiceTest.java @@ -117,7 +117,7 @@ public class BackupManagerServiceTest { "dataManagementLabel"); verify(mTransportManager) - .describeTransport( + .updateTransportAttributes( eq(TRANSPORT_COMPONENT), eq(TRANSPORT_NAME), eq(configurationIntent), @@ -247,7 +247,7 @@ public class BackupManagerServiceTest { null); verify(mTransportManager) - .describeTransport( + .updateTransportAttributes( eq(TRANSPORT_COMPONENT), eq(TRANSPORT_NAME), eq(configurationIntent), @@ -274,7 +274,7 @@ public class BackupManagerServiceTest { "dataManagementLabel"); verify(mTransportManager) - .describeTransport( + .updateTransportAttributes( eq(TRANSPORT_COMPONENT), eq(TRANSPORT_NAME), eq(configurationIntent),