Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit be9589e2 authored by Sal Savage's avatar Sal Savage
Browse files

Storage Refactor: Rename similar objects in preparation for refactor 

The addition of a storage abstraction allows us to shift
responsibilities across a few objects to better promote code health and
testability.

In particular, the obex client no longer needs to also store contacts,
and the service and state machine no longer both need to worry about
user unlocks and account readiness. The existing PullRequest completion
processors that own storage currently are now redundant.

This change moves several objects over in naming to make room for the
new implementations. Objects with name changes were too difficult to
edit in place with flagging. Other downstream objects have been marked
for removal with warning messages.

Bug: 365626536
Bug: 376461939
Test: atest com.android.bluetooth.pbapclient
Test: m com.android.btservices
Change-Id: I3b1ebd509a4ba56935a46d885d6972a1925a7313
parent 55843315

android/app/src/com/android/bluetooth/pbapclient/PbapClientService.java

+33 −35
Original line number Diff line number Diff line
@@ -58,7 +58,7 @@ public class PbapClientService extends ProfileService { 
    private static final int MAXIMUM_DEVICES = 10;



    @VisibleForTesting

    final Map<BluetoothDevice, PbapClientStateMachine> mPbapClientStateMachineMap =

    final Map<BluetoothDevice, PbapClientStateMachineOld> mPbapClientStateMachineOldMap =

            new ConcurrentHashMap<>();



    private static PbapClientService sPbapClientService;
@@ -73,8 +73,8 @@ public class PbapClientService extends ProfileService { 
            Log.i(TAG, "onAccountsChanged: old=" + oldAccounts + ", new=" + newAccounts);

            if (oldAccounts == null) {

                removeUncleanAccounts();

                for (PbapClientStateMachine stateMachine : mPbapClientStateMachineMap.values()) {

                    stateMachine.tryDownloadIfConnected();

                for (PbapClientStateMachineOld smOld : mPbapClientStateMachineOldMap.values()) {

                    smOld.tryDownloadIfConnected();

                }

            }

        }
@@ -125,10 +125,10 @@ public class PbapClientService extends ProfileService { 
        setPbapClientService(null);

        cleanUpSdpRecord();



        for (PbapClientStateMachine pbapClientStateMachine : mPbapClientStateMachineMap.values()) {

            pbapClientStateMachine.doQuit();

        for (PbapClientStateMachineOld smOld : mPbapClientStateMachineOldMap.values()) {

            smOld.doQuit();

        }

        mPbapClientStateMachineMap.clear();

        mPbapClientStateMachineOldMap.clear();



        // Unregister Handler and stop all queued messages.

        if (mHandler != null) {
@@ -186,11 +186,11 @@ public class PbapClientService extends ProfileService { 


    void cleanupDevice(BluetoothDevice device) {

        Log.d(TAG, "Cleanup device: " + device);

        synchronized (mPbapClientStateMachineMap) {

            PbapClientStateMachine pbapClientStateMachine = mPbapClientStateMachineMap.get(device);

            if (pbapClientStateMachine != null) {

                mPbapClientStateMachineMap.remove(device);

                pbapClientStateMachine.doQuit();

        synchronized (mPbapClientStateMachineOldMap) {

            PbapClientStateMachineOld smOld = mPbapClientStateMachineOldMap.get(device);

            if (smOld != null) {

                mPbapClientStateMachineOldMap.remove(device);

                smOld.doQuit();

            }

        }

    }
@@ -293,8 +293,8 @@ public class PbapClientService extends ProfileService { 
    @Override

    public void dump(StringBuilder sb) {

        super.dump(sb);

        for (PbapClientStateMachine stateMachine : mPbapClientStateMachineMap.values()) {

            stateMachine.dump(sb);

        for (PbapClientStateMachineOld smOld : mPbapClientStateMachineOldMap.values()) {

            smOld.dump(sb);

        }

        ProfileService.println(sb, mPbapClientAccountManager.dump());

    }
@@ -353,16 +353,15 @@ public class PbapClientService extends ProfileService { 
                        + BluetoothUuid.PBAP_PSE.toString()

                        + ")");

        if (uuid.equals(BluetoothUuid.PBAP_PSE)) {

            PbapClientStateMachine stateMachine = mPbapClientStateMachineMap.get(device);

            if (stateMachine == null) {

            PbapClientStateMachineOld smOld = mPbapClientStateMachineOldMap.get(device);

            if (smOld == null) {

                Log.e(TAG, "No Statemachine found for the device=" + device.toString());

                return;

            }

            SdpPseRecord pseRecord = (SdpPseRecord) record;

            if (pseRecord != null) {

                stateMachine

                        .obtainMessage(

                                PbapClientStateMachine.MSG_SDP_COMPLETE,

                smOld.obtainMessage(

                                PbapClientStateMachineOld.MSG_SDP_COMPLETE,

                                new PbapSdpRecord(device, pseRecord))

                        .sendToTarget();

            } else {
@@ -413,13 +412,12 @@ public class PbapClientService extends ProfileService { 
        if (getConnectionPolicy(device) <= BluetoothProfile.CONNECTION_POLICY_FORBIDDEN) {

            return false;

        }

        synchronized (mPbapClientStateMachineMap) {

            PbapClientStateMachine pbapClientStateMachine = mPbapClientStateMachineMap.get(device);

            if (pbapClientStateMachine == null

                    && mPbapClientStateMachineMap.size() < MAXIMUM_DEVICES) {

                pbapClientStateMachine = new PbapClientStateMachine(this, device);

                pbapClientStateMachine.start();

                mPbapClientStateMachineMap.put(device, pbapClientStateMachine);

        synchronized (mPbapClientStateMachineOldMap) {

            PbapClientStateMachineOld smOld = mPbapClientStateMachineOldMap.get(device);

            if (smOld == null && mPbapClientStateMachineOldMap.size() < MAXIMUM_DEVICES) {

                smOld = new PbapClientStateMachineOld(this, device);

                smOld.start();

                mPbapClientStateMachineOldMap.put(device, smOld);

                return true;

            } else {

                Log.w(TAG, "Received connect request while already connecting/connected.");
@@ -438,9 +436,9 @@ public class PbapClientService extends ProfileService { 
        if (device == null) {

            throw new IllegalArgumentException("Null device");

        }

        PbapClientStateMachine pbapClientStateMachine = mPbapClientStateMachineMap.get(device);

        if (pbapClientStateMachine != null) {

            pbapClientStateMachine.disconnect(device);

        PbapClientStateMachineOld smOld = mPbapClientStateMachineOldMap.get(device);

        if (smOld != null) {

            smOld.disconnect(device);

            return true;

        } else {

            Log.w(TAG, "disconnect() called on unconnected device.");
@@ -467,12 +465,12 @@ public class PbapClientService extends ProfileService { 
     */

    public List<BluetoothDevice> getDevicesMatchingConnectionStates(int[] states) {

        List<BluetoothDevice> deviceList = new ArrayList<BluetoothDevice>(0);

        for (Map.Entry<BluetoothDevice, PbapClientStateMachine> stateMachineEntry :

                mPbapClientStateMachineMap.entrySet()) {

            int currentDeviceState = stateMachineEntry.getValue().getConnectionState();

        for (Map.Entry<BluetoothDevice, PbapClientStateMachineOld> stateMachineEntryOld :

                mPbapClientStateMachineOldMap.entrySet()) {

            int currentDeviceState = stateMachineEntryOld.getValue().getConnectionState();

            for (int state : states) {

                if (currentDeviceState == state) {

                    deviceList.add(stateMachineEntry.getKey());

                    deviceList.add(stateMachineEntryOld.getKey());

                    break;

                }

            }
@@ -493,11 +491,11 @@ public class PbapClientService extends ProfileService { 
        if (device == null) {

            throw new IllegalArgumentException("Null device");

        }

        PbapClientStateMachine pbapClientStateMachine = mPbapClientStateMachineMap.get(device);

        if (pbapClientStateMachine == null) {

        PbapClientStateMachineOld smOld = mPbapClientStateMachineOldMap.get(device);

        if (smOld == null) {

            return BluetoothProfile.STATE_DISCONNECTED;

        } else {

            return pbapClientStateMachine.getConnectionState(device);

            return smOld.getConnectionState(device);

        }

    }

android/app/src/com/android/bluetooth/pbapclient/PbapClientStateMachine.java→android/app/src/com/android/bluetooth/pbapclient/PbapClientStateMachineOld.java

+10 −5
Original line number Diff line number Diff line
@@ -62,6 +62,7 @@ import com.android.bluetooth.Utils; 
import com.android.bluetooth.btservice.AdapterService;

import com.android.bluetooth.btservice.MetricsLogger;

import com.android.bluetooth.btservice.ProfileService;

import com.android.bluetooth.flags.Flags;

import com.android.internal.annotations.VisibleForTesting;

import com.android.internal.util.IState;

import com.android.internal.util.State;
@@ -70,8 +71,8 @@ import com.android.internal.util.StateMachine; 
import java.util.ArrayList;

import java.util.List;



class PbapClientStateMachine extends StateMachine {

    private static final String TAG = PbapClientStateMachine.class.getSimpleName();

class PbapClientStateMachineOld extends StateMachine {

    private static final String TAG = PbapClientStateMachineOld.class.getSimpleName();



    // Messages for handling connect/disconnect requests.

    private static final int MSG_DISCONNECT = 2;
@@ -109,17 +110,21 @@ class PbapClientStateMachine extends StateMachine { 
    // mMostRecentState maintains previous state for broadcasting transitions.

    private int mMostRecentState = BluetoothProfile.STATE_DISCONNECTED;



    PbapClientStateMachine(PbapClientService svc, BluetoothDevice device) {

    PbapClientStateMachineOld(PbapClientService svc, BluetoothDevice device) {

        this(svc, device, null);

    }



    @VisibleForTesting

    PbapClientStateMachine(

    PbapClientStateMachineOld(

            PbapClientService svc,

            BluetoothDevice device,

            PbapClientConnectionHandler connectionHandler) {

        super(TAG);



        if (Flags.pbapClientStorageRefactor()) {

            Log.w(TAG, "This object is no longer used in this configuration");

        }



        mService = svc;

        mCurrentDevice = device;

        mConnectionHandler = connectionHandler;
@@ -173,7 +178,7 @@ class PbapClientStateMachine extends StateMachine { 
                                .setLooper(looper)

                                .setLocalSupportedFeatures(LOCAL_SUPPORTED_FEATURES)

                                .setService(mService)

                                .setClientSM(PbapClientStateMachine.this)

                                .setClientSM(PbapClientStateMachineOld.this)

                                .setRemoteDevice(mCurrentDevice)

                                .build();

            }

android/app/src/com/android/bluetooth/pbapclient/obex/PbapClientConnectionHandler.java

+14 −7
Original line number Diff line number Diff line
@@ -30,6 +30,7 @@ import android.util.Log; 
import com.android.bluetooth.BluetoothObexTransport;

import com.android.bluetooth.ObexAppParameters;

import com.android.bluetooth.R;

import com.android.bluetooth.flags.Flags;

import com.android.internal.annotations.VisibleForTesting;

import com.android.obex.ClientSession;

import com.android.obex.HeaderSet;
@@ -92,7 +93,7 @@ class PbapClientConnectionHandler extends Handler { 
    private ClientSession mObexSession;

    private PbapClientService mService;

    private PbapClientObexAuthenticator mAuth = null;

    private final PbapClientStateMachine mPbapClientStateMachine;

    private final PbapClientStateMachineOld mPbapClientStateMachine;

    private boolean mAccountCreated;



    /**
@@ -102,6 +103,11 @@ class PbapClientConnectionHandler extends Handler { 
     */

    PbapClientConnectionHandler(Builder pceHandlerbuild) {

        super(pceHandlerbuild.mLooper);



        if (Flags.pbapClientStorageRefactor()) {

            Log.w(TAG, "This object is no longer used in this configuration");

        }



        mDevice = pceHandlerbuild.mDevice;

        mLocalSupportedFeatures = pceHandlerbuild.mLocalSupportedFeatures;

        mService = pceHandlerbuild.mService;
@@ -119,7 +125,7 @@ class PbapClientConnectionHandler extends Handler { 
        private PbapClientService mService;

        private BluetoothDevice mDevice;

        private int mLocalSupportedFeatures;

        private PbapClientStateMachine mClientStateMachine;

        private PbapClientStateMachineOld mClientStateMachine;



        public Builder setLooper(Looper loop) {

            this.mLooper = loop;
@@ -131,7 +137,7 @@ class PbapClientConnectionHandler extends Handler { 
            return this;

        }



        public Builder setClientSM(PbapClientStateMachine clientStateMachine) {

        public Builder setClientSM(PbapClientStateMachineOld clientStateMachine) {

            this.mClientStateMachine = clientStateMachine;

            return this;

        }
@@ -165,16 +171,16 @@ class PbapClientConnectionHandler extends Handler { 
                } else {

                    Log.w(TAG, "Socket CONNECT Failure ");

                    mPbapClientStateMachine.sendMessage(

                            PbapClientStateMachine.MSG_CONNECTION_FAILED);

                            PbapClientStateMachineOld.MSG_CONNECTION_FAILED);

                    return;

                }



                if (connectObexSession()) {

                    mPbapClientStateMachine.sendMessage(

                            PbapClientStateMachine.MSG_CONNECTION_COMPLETE);

                            PbapClientStateMachineOld.MSG_CONNECTION_COMPLETE);

                } else {

                    mPbapClientStateMachine.sendMessage(

                            PbapClientStateMachine.MSG_CONNECTION_FAILED);

                            PbapClientStateMachineOld.MSG_CONNECTION_FAILED);

                }

                break;
@@ -198,7 +204,8 @@ class PbapClientConnectionHandler extends Handler { 
                }

                removeCallLog();



                mPbapClientStateMachine.sendMessage(PbapClientStateMachine.MSG_CONNECTION_CLOSED);

                mPbapClientStateMachine.sendMessage(

                        PbapClientStateMachineOld.MSG_CONNECTION_CLOSED);

                break;



            case MSG_DOWNLOAD:

android/app/tests/unit/src/com/android/bluetooth/pbapclient/PbapClientConnectionHandlerTest.java

+2 −2
Original line number Diff line number Diff line
@@ -70,7 +70,7 @@ public class PbapClientConnectionHandlerTest { 


    @Mock private ContentResolver mMockContentResolver;



    @Mock private PbapClientStateMachine mStateMachine;

    @Mock private PbapClientStateMachineOld mStateMachine;



    private PbapClientConnectionHandler mHandler;
@@ -164,6 +164,6 @@ public class PbapClientConnectionHandlerTest { 
    public void createAndDisconnectWithoutAddingAccount_doesNotCrash() {

        mHandler.obtainMessage(PbapClientConnectionHandler.MSG_DISCONNECT).sendToTarget();

        TestUtils.waitForLooperToFinishScheduledTask(mHandler.getLooper());

        verify(mStateMachine).sendMessage(PbapClientStateMachine.MSG_CONNECTION_CLOSED);

        verify(mStateMachine).sendMessage(PbapClientStateMachineOld.MSG_CONNECTION_CLOSED);

    }

}

android/app/tests/unit/src/com/android/bluetooth/pbapclient/PbapClientServiceTest.java

+15 −15
Original line number Diff line number Diff line
@@ -169,8 +169,8 @@ public class PbapClientServiceTest { 


    @Test

    public void onAccountsChanged_fromNulltoEmpty_tryDownloadIfConnectedCalled() {

        PbapClientStateMachine sm = mock(PbapClientStateMachine.class);

        mService.mPbapClientStateMachineMap.put(mRemoteDevice, sm);

        PbapClientStateMachineOld sm = mock(PbapClientStateMachineOld.class);

        mService.mPbapClientStateMachineOldMap.put(mRemoteDevice, sm);



        PbapClientService.PbapClientAccountManagerCallback callback =

                mService.new PbapClientAccountManagerCallback();
@@ -181,8 +181,8 @@ public class PbapClientServiceTest { 


    @Test

    public void onAccountsChanged_fromEmptyToOne_tryDownloadIfConnectedNotCalled() {

        PbapClientStateMachine sm = mock(PbapClientStateMachine.class);

        mService.mPbapClientStateMachineMap.put(mRemoteDevice, sm);

        PbapClientStateMachineOld sm = mock(PbapClientStateMachineOld.class);

        mService.mPbapClientStateMachineOldMap.put(mRemoteDevice, sm);



        PbapClientService.PbapClientAccountManagerCallback callback =

                mService.new PbapClientAccountManagerCallback();
@@ -197,8 +197,8 @@ public class PbapClientServiceTest { 
    @Test

    public void aclDisconnected_withLeTransport_doesNotCallDisconnect() {

        int connectionState = BluetoothProfile.STATE_CONNECTED;

        PbapClientStateMachine sm = mock(PbapClientStateMachine.class);

        mService.mPbapClientStateMachineMap.put(mRemoteDevice, sm);

        PbapClientStateMachineOld sm = mock(PbapClientStateMachineOld.class);

        mService.mPbapClientStateMachineOldMap.put(mRemoteDevice, sm);

        when(sm.getConnectionState(mRemoteDevice)).thenReturn(connectionState);



        mService.aclDisconnected(mRemoteDevice, BluetoothDevice.TRANSPORT_LE);
@@ -210,8 +210,8 @@ public class PbapClientServiceTest { 
    @Test

    public void aclDisconnected_withBrEdrTransport_callsDisconnect() {

        int connectionState = BluetoothProfile.STATE_CONNECTED;

        PbapClientStateMachine sm = mock(PbapClientStateMachine.class);

        mService.mPbapClientStateMachineMap.put(mRemoteDevice, sm);

        PbapClientStateMachineOld sm = mock(PbapClientStateMachineOld.class);

        mService.mPbapClientStateMachineOldMap.put(mRemoteDevice, sm);

        when(sm.getConnectionState(mRemoteDevice)).thenReturn(connectionState);



        mService.aclDisconnected(mRemoteDevice, BluetoothDevice.TRANSPORT_BREDR);
@@ -237,12 +237,12 @@ public class PbapClientServiceTest { 


    @Test

    public void cleanUpDevice() {

        PbapClientStateMachine sm = mock(PbapClientStateMachine.class);

        mService.mPbapClientStateMachineMap.put(mRemoteDevice, sm);

        PbapClientStateMachineOld sm = mock(PbapClientStateMachineOld.class);

        mService.mPbapClientStateMachineOldMap.put(mRemoteDevice, sm);



        mService.cleanupDevice(mRemoteDevice);



        assertThat(mService.mPbapClientStateMachineMap).doesNotContainKey(mRemoteDevice);

        assertThat(mService.mPbapClientStateMachineOldMap).doesNotContainKey(mRemoteDevice);

    }



    // *********************************************************************************************
@@ -320,8 +320,8 @@ public class PbapClientServiceTest { 


    @Test

    public void testDisconnect_whenConnected_returnsTrue() {

        PbapClientStateMachine sm = mock(PbapClientStateMachine.class);

        mService.mPbapClientStateMachineMap.put(mRemoteDevice, sm);

        PbapClientStateMachineOld sm = mock(PbapClientStateMachineOld.class);

        mService.mPbapClientStateMachineOldMap.put(mRemoteDevice, sm);



        assertThat(mService.disconnect(mRemoteDevice)).isTrue();
@@ -340,8 +340,8 @@ public class PbapClientServiceTest { 


    @Test

    public void dump_callsStateMachineDump() {

        PbapClientStateMachine sm = mock(PbapClientStateMachine.class);

        mService.mPbapClientStateMachineMap.put(mRemoteDevice, sm);

        PbapClientStateMachineOld sm = mock(PbapClientStateMachineOld.class);

        mService.mPbapClientStateMachineOldMap.put(mRemoteDevice, sm);

        StringBuilder builder = new StringBuilder();



        mService.dump(builder);