OSDN Git Service

Remove transport dir name from TransportClient
authorBernardo Rufino <brufino@google.com>
Thu, 4 Jan 2018 14:16:32 +0000 (14:16 +0000)
committerBernardo Rufino <brufino@google.com>
Fri, 5 Jan 2018 16:57:51 +0000 (16:57 +0000)
To be able to re-use the TransportClient infra for transport
registration, I need to remove transport dir name property from
TransportClient because it's not available before registration
itself. As a result callsites that used getTransportDirName()
from TransportClient will have to go through the
TransportManager for that. Bryan suggested that the
TransportClient wasn't the best place for the property before.

Ref: http://go/br-binding-on-demand
Bug: 17140907
Test: m -j RunFrameworksServicesTests
Change-Id: I3fa335faf97d63adfad1a929336073a70fc8bc02

services/backup/java/com/android/server/backup/RefactoredBackupManagerService.java
services/backup/java/com/android/server/backup/TransportManager.java
services/backup/java/com/android/server/backup/internal/PerformClearTask.java
services/backup/java/com/android/server/backup/internal/PerformInitializeTask.java
services/backup/java/com/android/server/backup/restore/PerformUnifiedRestoreTask.java
services/backup/java/com/android/server/backup/transport/TransportClient.java
services/backup/java/com/android/server/backup/transport/TransportClientManager.java
services/backup/java/com/android/server/backup/transport/TransportNotRegisteredException.java
services/robotests/src/com/android/server/backup/internal/PerformInitializeTaskTest.java
services/robotests/src/com/android/server/backup/testing/TransportTestUtils.java
services/robotests/src/com/android/server/backup/transport/TransportClientTest.java

index 6024e38..d6a4f19 100644 (file)
@@ -1604,9 +1604,15 @@ public class RefactoredBackupManagerService implements BackupManagerServiceInter
             return BackupManager.ERROR_BACKUP_NOT_ALLOWED;
         }
 
-        TransportClient transportClient =
-                mTransportManager.getCurrentTransportClient("BMS.requestBackup()");
-        if (transportClient == null) {
+        final TransportClient transportClient;
+        final String transportDirName;
+        try {
+            transportDirName =
+                    mTransportManager.getTransportDirName(
+                            mTransportManager.getCurrentTransportName());
+            transportClient =
+                    mTransportManager.getCurrentTransportClientOrThrow("BMS.requestBackup()");
+        } catch (TransportNotRegisteredException e) {
             BackupObserverUtils.sendBackupFinished(observer, BackupManager.ERROR_TRANSPORT_ABORTED);
             monitor = BackupManagerMonitorUtils.monitorEvent(monitor,
                     BackupManagerMonitor.LOG_EVENT_ID_TRANSPORT_IS_NULL,
@@ -1651,12 +1657,11 @@ public class RefactoredBackupManagerService implements BackupManagerServiceInter
                     + " k/v backups");
         }
 
-        String dirName = transportClient.getTransportDirName();
         boolean nonIncrementalBackup = (flags & BackupManager.FLAG_NON_INCREMENTAL_BACKUP) != 0;
 
         Message msg = mBackupHandler.obtainMessage(MSG_REQUEST_BACKUP);
-        msg.obj = new BackupParams(transportClient, dirName, kvBackupList, fullBackupList, observer,
-                monitor, listener, true, nonIncrementalBackup);
+        msg.obj = new BackupParams(transportClient, transportDirName, kvBackupList, fullBackupList,
+                observer, monitor, listener, true, nonIncrementalBackup);
         mBackupHandler.sendMessage(msg);
         return BackupManager.SUCCESS;
     }
index 58270a6..72a3a32 100644 (file)
@@ -49,6 +49,7 @@ 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.TransportNotAvailableException;
 import com.android.server.backup.transport.TransportNotRegisteredException;
 
 import java.util.ArrayList;
@@ -302,6 +303,18 @@ public class TransportManager {
     }
 
     /**
+     * Retrieve the transport dir name of {@code transportComponent}.
+     * @throws TransportNotRegisteredException if the transport is not registered.
+     */
+    public String getTransportDirName(ComponentName transportComponent)
+            throws TransportNotRegisteredException {
+        synchronized (mTransportLock) {
+            return getRegisteredTransportDescriptionOrThrowLocked(transportComponent)
+                    .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
@@ -370,7 +383,6 @@ public class TransportManager {
         return description;
     }
 
-
     @GuardedBy("mTransportLock")
     @Nullable
     private Map.Entry<ComponentName, TransportDescription> getRegisteredTransportEntryLocked(
@@ -385,17 +397,35 @@ public class TransportManager {
         return null;
     }
 
+    @GuardedBy("mTransportLock")
+    private TransportDescription getRegisteredTransportDescriptionOrThrowLocked(
+            ComponentName transportComponent) throws TransportNotRegisteredException {
+        TransportDescription description =
+                mRegisteredTransportsDescriptionMap.get(transportComponent);
+        if (description == null) {
+            throw new TransportNotRegisteredException(transportComponent);
+        }
+        return description;
+    }
+
     @Nullable
     public TransportClient getTransportClient(String transportName, String caller) {
+        try {
+            return getTransportClientOrThrow(transportName, caller);
+        } catch (TransportNotRegisteredException e) {
+            Slog.w(TAG, "Transport " + transportName + " not registered");
+            return null;
+        }
+    }
+
+    public TransportClient getTransportClientOrThrow(String transportName, String caller)
+            throws TransportNotRegisteredException {
         synchronized (mTransportLock) {
             ComponentName component = getRegisteredTransportComponentLocked(transportName);
             if (component == null) {
-                Slog.w(TAG, "Transport " + transportName + " not registered");
-                return null;
+                throw new TransportNotRegisteredException(transportName);
             }
-            TransportDescription description = mRegisteredTransportsDescriptionMap.get(component);
-            return mTransportClientManager.getTransportClient(
-                    component, description.transportDirName, caller);
+            return mTransportClientManager.getTransportClient(component, caller);
         }
     }
 
@@ -415,7 +445,25 @@ public class TransportManager {
      */
     @Nullable
     public TransportClient getCurrentTransportClient(String caller) {
-        return getTransportClient(mCurrentTransportName, caller);
+        synchronized (mTransportLock) {
+            return getTransportClient(mCurrentTransportName, caller);
+        }
+    }
+
+    /**
+     * Returns a {@link TransportClient} for the current transport or throws if not registered.
+     *
+     * @param caller A {@link String} identifying the caller for logging/debugging purposes. Check
+     *     {@link TransportClient#connectAsync(TransportConnectionListener, String)} for more
+     *     details.
+     * @return A {@link TransportClient}.
+     * @throws TransportNotRegisteredException if the transport is not registered.
+     */
+    public TransportClient getCurrentTransportClientOrThrow(String caller)
+            throws TransportNotRegisteredException {
+        synchronized (mTransportLock) {
+            return getTransportClientOrThrow(mCurrentTransportName, caller);
+        }
     }
 
     /**
index 84ca59b..140d728 100644 (file)
@@ -23,12 +23,14 @@ import android.util.Slog;
 
 import com.android.internal.backup.IBackupTransport;
 import com.android.server.backup.RefactoredBackupManagerService;
+import com.android.server.backup.TransportManager;
 import com.android.server.backup.transport.TransportClient;
 
 import java.io.File;
 
 public class PerformClearTask implements Runnable {
     private final RefactoredBackupManagerService mBackupManagerService;
+    private final TransportManager mTransportManager;
     private final TransportClient mTransportClient;
     private final PackageInfo mPackage;
     private final OnTaskFinishedListener mListener;
@@ -37,6 +39,7 @@ public class PerformClearTask implements Runnable {
             TransportClient transportClient, PackageInfo packageInfo,
             OnTaskFinishedListener listener) {
         mBackupManagerService = backupManagerService;
+        mTransportManager = backupManagerService.getTransportManager();
         mTransportClient = transportClient;
         mPackage = packageInfo;
         mListener = listener;
@@ -47,8 +50,9 @@ public class PerformClearTask implements Runnable {
         IBackupTransport transport = null;
         try {
             // Clear the on-device backup state to ensure a full backup next time
-            File stateDir = new File(mBackupManagerService.getBaseStateDir(),
-                    mTransportClient.getTransportDirName());
+            String transportDirName =
+                    mTransportManager.getTransportDirName(mTransportClient.getTransportComponent());
+            File stateDir = new File(mBackupManagerService.getBaseStateDir(), transportDirName);
             File stateFile = new File(stateDir, mPackage.packageName);
             stateFile.delete();
 
index c624698..2f2af98 100644 (file)
@@ -122,7 +122,9 @@ public class PerformInitializeTask implements Runnable {
                 transportClientsToDisposeOf.add(transportClient);
 
                 Slog.i(TAG, "Initializing (wiping) backup transport storage: " + transportName);
-                String transportDirName = transportClient.getTransportDirName();
+                String transportDirName =
+                        mTransportManager.getTransportDirName(
+                                transportClient.getTransportComponent());
                 EventLog.writeEvent(EventLogTags.BACKUP_START, transportDirName);
                 long startRealtime = SystemClock.elapsedRealtime();
 
index 86866dc..88f9ead 100644 (file)
@@ -61,6 +61,7 @@ import com.android.server.backup.BackupUtils;
 import com.android.server.backup.PackageManagerBackupAgent;
 import com.android.server.backup.PackageManagerBackupAgent.Metadata;
 import com.android.server.backup.RefactoredBackupManagerService;
+import com.android.server.backup.TransportManager;
 import com.android.server.backup.internal.OnTaskFinishedListener;
 import com.android.server.backup.transport.TransportClient;
 import com.android.server.backup.utils.AppBackupUtils;
@@ -78,6 +79,7 @@ import java.util.List;
 public class PerformUnifiedRestoreTask implements BackupRestoreTask {
 
     private RefactoredBackupManagerService backupManagerService;
+    private final TransportManager mTransportManager;
     // Transport client we're working with to do the restore
     private final TransportClient mTransportClient;
 
@@ -164,6 +166,7 @@ public class PerformUnifiedRestoreTask implements BackupRestoreTask {
             int pmToken, boolean isFullSystemRestore, String[] filterSet,
             OnTaskFinishedListener listener) {
         this.backupManagerService = backupManagerService;
+        mTransportManager = backupManagerService.getTransportManager();
         mEphemeralOpToken = backupManagerService.generateRandomIntegerToken();
         mState = UnifiedRestoreState.INITIAL;
         mStartRealtime = SystemClock.elapsedRealtime();
@@ -349,8 +352,9 @@ public class PerformUnifiedRestoreTask implements BackupRestoreTask {
         }
 
         try {
-            String transportDir = mTransportClient.getTransportDirName();
-            mStateDir = new File(backupManagerService.getBaseStateDir(), transportDir);
+            String transportDirName =
+                    mTransportManager.getTransportDirName(mTransportClient.getTransportComponent());
+            mStateDir = new File(backupManagerService.getBaseStateDir(), transportDirName);
 
             // Fetch the current metadata from the dataset first
             PackageInfo pmPackage = new PackageInfo();
index 2c7a0eb..7bd9111 100644 (file)
@@ -68,7 +68,6 @@ public class TransportClient {
     private final Intent mBindIntent;
     private final String mIdentifier;
     private final ComponentName mTransportComponent;
-    private final String mTransportDirName;
     private final Handler mListenerHandler;
     private final String mPrefixForLog;
     private final Object mStateLock = new Object();
@@ -87,13 +86,11 @@ public class TransportClient {
             Context context,
             Intent bindIntent,
             ComponentName transportComponent,
-            String transportDirName,
             String identifier) {
         this(
                 context,
                 bindIntent,
                 transportComponent,
-                transportDirName,
                 identifier,
                 new Handler(Looper.getMainLooper()));
     }
@@ -103,12 +100,10 @@ public class TransportClient {
             Context context,
             Intent bindIntent,
             ComponentName transportComponent,
-            String transportDirName,
             String identifier,
             Handler listenerHandler) {
         mContext = context;
         mTransportComponent = transportComponent;
-        mTransportDirName = transportDirName;
         mBindIntent = bindIntent;
         mIdentifier = identifier;
         mListenerHandler = listenerHandler;
@@ -122,10 +117,6 @@ public class TransportClient {
         return mTransportComponent;
     }
 
-    public String getTransportDirName() {
-        return mTransportDirName;
-    }
-
     // Calls to onServiceDisconnected() or onBindingDied() turn TransportClient UNUSABLE. After one
     // of these calls, if a binding happen again the new service can be a different instance. Since
     // transports are stateful, we don't want a new instance responding for an old instance's state.
index bb550f6..1cbe747 100644 (file)
@@ -23,7 +23,6 @@ import android.content.Context;
 import android.content.Intent;
 import android.util.Log;
 
-import com.android.internal.backup.IBackupTransport;
 import com.android.server.backup.TransportManager;
 
 /**
@@ -48,17 +47,12 @@ public class TransportClientManager {
      * transportComponent}.
      *
      * @param transportComponent The {@link ComponentName} of the transport.
-     * @param transportDirName The {@link String} returned by
-     *     {@link IBackupTransport#transportDirName()} at registration.
      * @param caller A {@link String} identifying the caller for logging/debugging purposes. Check
      *     {@link TransportClient#connectAsync(TransportConnectionListener, String)} for more
      *     details.
      * @return A {@link TransportClient}.
      */
-    public TransportClient getTransportClient(
-            ComponentName transportComponent,
-            String transportDirName,
-            String caller) {
+    public TransportClient getTransportClient(ComponentName transportComponent, String caller) {
         Intent bindIntent =
                 new Intent(SERVICE_ACTION_TRANSPORT_HOST).setComponent(transportComponent);
         synchronized (mTransportClientsLock) {
@@ -67,7 +61,6 @@ public class TransportClientManager {
                             mContext,
                             bindIntent,
                             transportComponent,
-                            transportDirName,
                             Integer.toString(mTransportClientsCreated));
             mTransportClientsCreated++;
             TransportUtils.log(Log.DEBUG, TAG, caller, "Retrieving " + transportClient);
index 26bf92c..02766de 100644 (file)
@@ -16,6 +16,7 @@
 
 package com.android.server.backup.transport;
 
+import android.content.ComponentName;
 import android.util.AndroidException;
 
 import com.android.server.backup.TransportManager;
@@ -32,4 +33,8 @@ public class TransportNotRegisteredException extends AndroidException {
     public TransportNotRegisteredException(String transportName) {
         super("Transport " + transportName + " not registered");
     }
+
+    public TransportNotRegisteredException(ComponentName transportComponent) {
+        super("Transport for host " + transportComponent + " not registered");
+    }
 }
index a5bff3e..dfca901 100644 (file)
@@ -108,10 +108,13 @@ public class PerformInitializeTaskTest {
 
         verify(mBackupManagerService)
                 .recordInitPending(
-                        false, TRANSPORT_NAME, TransportTestUtils.dirName(TRANSPORT_NAME));
+                        false, TRANSPORT_NAME, TransportTestUtils.transportDirName(TRANSPORT_NAME));
         verify(mBackupManagerService)
                 .resetBackupState(
-                        eq(new File(mBaseStateDir, TransportTestUtils.dirName(TRANSPORT_NAME))));
+                        eq(
+                                new File(
+                                        mBaseStateDir,
+                                        TransportTestUtils.transportDirName(TRANSPORT_NAME))));
     }
 
     @Test
@@ -139,7 +142,7 @@ public class PerformInitializeTaskTest {
         verify(mTransport, never()).finishBackup();
         verify(mBackupManagerService)
                 .recordInitPending(
-                        true, TRANSPORT_NAME, TransportTestUtils.dirName(TRANSPORT_NAME));
+                        true, TRANSPORT_NAME, TransportTestUtils.transportDirName(TRANSPORT_NAME));
     }
 
     @Test
@@ -179,7 +182,7 @@ public class PerformInitializeTaskTest {
         verify(mTransport).finishBackup();
         verify(mBackupManagerService)
                 .recordInitPending(
-                        true, TRANSPORT_NAME, TransportTestUtils.dirName(TRANSPORT_NAME));
+                        true, TRANSPORT_NAME, TransportTestUtils.transportDirName(TRANSPORT_NAME));
     }
 
     @Test
index b784159..9770e40 100644 (file)
@@ -22,24 +22,27 @@ import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.when;
 
 import android.annotation.Nullable;
+import android.content.ComponentName;
 
 import com.android.internal.backup.IBackupTransport;
 import com.android.server.backup.TransportManager;
 import com.android.server.backup.transport.TransportClient;
 import com.android.server.backup.transport.TransportNotAvailableException;
+import com.android.server.backup.transport.TransportNotRegisteredException;
 
 import java.util.Arrays;
 import java.util.List;
 
 public class TransportTestUtils {
     public static final String[] TRANSPORT_NAMES = {
-            "android/com.android.internal.backup.LocalTransport",
-            "com.google.android.gms/.backup.migrate.service.D2dTransport",
-            "com.google.android.gms/.backup.BackupTransportService"
+        "android/com.android.internal.backup.LocalTransport",
+        "com.google.android.gms/.backup.migrate.service.D2dTransport",
+        "com.google.android.gms/.backup.BackupTransportService"
     };
 
     public static final String TRANSPORT_NAME = TRANSPORT_NAMES[0];
 
+    /** {@code transportName} has to be in the {@link ComponentName} format (with '/') */
     public static TransportData setUpCurrentTransport(
             TransportManager transportManager, String transportName) throws Exception {
         TransportData transport = setUpTransports(transportManager, transportName).get(0);
@@ -48,6 +51,7 @@ public class TransportTestUtils {
         return transport;
     }
 
+    /** {@code transportName} has to be in the {@link ComponentName} format (with '/') */
     public static List<TransportData> setUpTransports(
             TransportManager transportManager, String... transportNames) throws Exception {
         return setUpTransports(
@@ -70,39 +74,61 @@ public class TransportTestUtils {
      * Configures transport according to {@link TransportData}:
      *
      * <ul>
-     *   <li>{@link TransportData#transportMock} {@code null} means {@link
-     *       TransportClient#connectOrThrow(String)} throws {@link TransportNotAvailableException}.
-     *   <li>{@link TransportData#transportClientMock} {@code null} means {@link
-     *       TransportManager#getTransportClient(String, String)} returns {@code null}.
+     *   <li>{@link TransportData#transportMock} {@code null} means transport not available.
+     *   <li>{@link TransportData#transportClientMock} {@code null} means transport not registered.
      * </ul>
      */
     public static void setUpTransport(TransportManager transportManager, TransportData transport)
             throws Exception {
         String transportName = transport.transportName;
-        String transportDirName = dirName(transportName);
+        String transportDirName = transportDirName(transportName);
+        ComponentName transportComponent = transportComponentName(transportName);
         IBackupTransport transportMock = transport.transportMock;
         TransportClient transportClientMock = transport.transportClientMock;
 
-        if (transportMock != null) {
-            when(transportMock.name()).thenReturn(transportName);
-            when(transportMock.transportDirName()).thenReturn(transportDirName);
-        }
-
         if (transportClientMock != null) {
-            when(transportClientMock.getTransportDirName()).thenReturn(transportDirName);
+            // Transport registered
+            when(transportManager.getTransportClient(eq(transportName), any()))
+                    .thenReturn(transportClientMock);
+            when(transportManager.getTransportClientOrThrow(eq(transportName), any()))
+                    .thenReturn(transportClientMock);
+            when(transportManager.getTransportName(transportComponent)).thenReturn(transportName);
+            when(transportManager.getTransportDirName(eq(transportName)))
+                    .thenReturn(transportDirName);
+            when(transportManager.getTransportDirName(eq(transportComponent)))
+                    .thenReturn(transportDirName);
+            when(transportClientMock.getTransportComponent()).thenReturn(transportComponent);
+
             if (transportMock != null) {
+                // Transport registered and available
                 when(transportClientMock.connectOrThrow(any())).thenReturn(transportMock);
+                when(transportMock.name()).thenReturn(transportName);
+                when(transportMock.transportDirName()).thenReturn(transportDirName);
             } else {
+                // Transport registered but unavailable
                 when(transportClientMock.connectOrThrow(any()))
                         .thenThrow(TransportNotAvailableException.class);
             }
+        } else {
+            // Transport not registered
+            when(transportManager.getTransportClient(eq(transportName), any())).thenReturn(null);
+            when(transportManager.getTransportClientOrThrow(eq(transportName), any()))
+                    .thenThrow(TransportNotRegisteredException.class);
+            when(transportManager.getTransportName(transportComponent))
+                    .thenThrow(TransportNotRegisteredException.class);
+            when(transportManager.getTransportDirName(eq(transportName)))
+                    .thenThrow(TransportNotRegisteredException.class);
+            when(transportManager.getTransportDirName(eq(transportComponent)))
+                    .thenThrow(TransportNotRegisteredException.class);
         }
+    }
 
-        when(transportManager.getTransportClient(eq(transportName), any()))
-                .thenReturn(transportClientMock);
+    /** {@code transportName} has to be in the {@link ComponentName} format (with '/') */
+    public static ComponentName transportComponentName(String transportName) {
+        return ComponentName.unflattenFromString(transportName);
     }
 
-    public static String dirName(String transportName) {
+    public static String transportDirName(String transportName) {
         return transportName + "_dir_name";
     }
 
index 4462d2a..3bc0b30 100644 (file)
@@ -43,7 +43,6 @@ import com.android.server.testing.FrameworkRobolectricTestRunner;
 import com.android.server.testing.SystemLoaderClasses;
 
 import org.junit.Before;
-import org.junit.BeforeClass;
 import org.junit.Test;
 import org.junit.runner.RunWith;
 import org.mockito.ArgumentCaptor;
@@ -65,7 +64,6 @@ public class TransportClientTest {
     @Mock private IBackupTransport.Stub mIBackupTransport;
     private TransportClient mTransportClient;
     private ComponentName mTransportComponent;
-    private String mTransportDirName;
     private Intent mBindIntent;
     private ShadowLooper mShadowLooper;
 
@@ -77,14 +75,12 @@ public class TransportClientTest {
         mShadowLooper = shadowOf(mainLooper);
         mTransportComponent =
                 new ComponentName(PACKAGE_NAME, PACKAGE_NAME + ".transport.Transport");
-        mTransportDirName = mTransportComponent.toString();
         mBindIntent = new Intent(SERVICE_ACTION_TRANSPORT_HOST).setComponent(mTransportComponent);
         mTransportClient =
                 new TransportClient(
                         mContext,
                         mBindIntent,
                         mTransportComponent,
-                        mTransportDirName,
                         "1",
                         new Handler(mainLooper));
 
@@ -97,11 +93,6 @@ public class TransportClientTest {
     }
 
     @Test
-    public void testGetTransportDirName_returnsTransportDirName() {
-        assertThat(mTransportClient.getTransportDirName()).isEqualTo(mTransportDirName);
-    }
-
-    @Test
     public void testGetTransportComponent_returnsTransportComponent() {
         assertThat(mTransportClient.getTransportComponent()).isEqualTo(mTransportComponent);
     }