From 8dd32ab07d94fad6dae3c336e2bfe9be38be144e Mon Sep 17 00:00:00 2001 From: Chaohui Wang Date: Tue, 11 Jul 2023 17:49:18 +0800 Subject: [PATCH] [BT] Correct the filter when addCachedDevices In Change Ia9750adb6b4c1424d084381e9d7c2ca8e7912391, addCachedDevices() becomes async, but the filter is set outside of addCachedDevices(), which makes the filter not apply to addCachedDevices(). Direct pass the filter to addCachedDevices() to fix this issue. Also migrate the test to Kotlin so we can test coroutine. Fix: 289876965 Test: manual - check BT pairing page Test: m RunSettingsRoboTests Change-Id: I95b16840881747ec9f69e5cd778e456bcc8a7626 --- .../bluetooth/BluetoothPairingDetail.java | 4 +- .../bluetooth/DeviceListPreferenceFragment.kt | 23 +- .../DeviceListPreferenceFragmentTest.java | 237 ---------------- .../DeviceListPreferenceFragmentTest.kt | 260 ++++++++++++++++++ 4 files changed, 272 insertions(+), 252 deletions(-) delete mode 100644 tests/robotests/src/com/android/settings/bluetooth/DeviceListPreferenceFragmentTest.java create mode 100644 tests/robotests/src/com/android/settings/bluetooth/DeviceListPreferenceFragmentTest.kt diff --git a/src/com/android/settings/bluetooth/BluetoothPairingDetail.java b/src/com/android/settings/bluetooth/BluetoothPairingDetail.java index a78bf27101c..234d6d2eb45 100644 --- a/src/com/android/settings/bluetooth/BluetoothPairingDetail.java +++ b/src/com/android/settings/bluetooth/BluetoothPairingDetail.java @@ -101,10 +101,8 @@ public class BluetoothPairingDetail extends BluetoothDevicePairingDetailBase imp if (bluetoothState == BluetoothAdapter.STATE_ON) { if (mInitialScanStarted) { // Don't show bonded devices when screen turned back on - setFilter(BluetoothDeviceFilter.UNBONDED_DEVICE_FILTER); - addCachedDevices(); + addCachedDevices(BluetoothDeviceFilter.UNBONDED_DEVICE_FILTER); } - setFilter(BluetoothDeviceFilter.ALL_FILTER); updateFooterPreference(mFooterPreference); mAlwaysDiscoverable.start(); } diff --git a/src/com/android/settings/bluetooth/DeviceListPreferenceFragment.kt b/src/com/android/settings/bluetooth/DeviceListPreferenceFragment.kt index 9c86e4398f6..42c24ea2140 100644 --- a/src/com/android/settings/bluetooth/DeviceListPreferenceFragment.kt +++ b/src/com/android/settings/bluetooth/DeviceListPreferenceFragment.kt @@ -28,7 +28,6 @@ import android.text.BidiFormatter import android.util.Log import android.view.View import androidx.annotation.VisibleForTesting -import androidx.lifecycle.LifecycleCoroutineScope import androidx.lifecycle.lifecycleScope import androidx.preference.Preference import androidx.preference.PreferenceCategory @@ -41,6 +40,7 @@ import com.android.settingslib.bluetooth.CachedBluetoothDevice import com.android.settingslib.bluetooth.CachedBluetoothDeviceManager import com.android.settingslib.bluetooth.LocalBluetoothManager import java.util.concurrent.ConcurrentHashMap +import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.launch import kotlinx.coroutines.withContext @@ -85,11 +85,10 @@ abstract class DeviceListPreferenceFragment(restrictedKey: String?) : @JvmField val mSelectedList: MutableList = ArrayList() - private var showDevicesWithoutNames = false + @VisibleForTesting + var lifecycleScope: CoroutineScope? = null - protected fun setFilter(filter: BluetoothDeviceFilter.Filter?) { - this.filter = filter - } + private var showDevicesWithoutNames = false protected fun setFilter(filterType: Int) { filter = BluetoothDeviceFilter.getFilter(filterType) @@ -125,8 +124,6 @@ abstract class DeviceListPreferenceFragment(restrictedKey: String?) : /** find and update preference that already existed in preference screen */ protected abstract fun initPreferencesFromPreferenceScreen() - private var lifecycleScope: LifecycleCoroutineScope? = null - override fun onViewCreated(view: View, savedInstanceState: Bundle?) { super.onViewCreated(view, savedInstanceState) lifecycleScope = viewLifecycleOwner.lifecycleScope @@ -154,13 +151,15 @@ abstract class DeviceListPreferenceFragment(restrictedKey: String?) : mDeviceListGroup!!.removeAll() } - fun addCachedDevices() { + @JvmOverloads + fun addCachedDevices(filterForCachedDevices: BluetoothDeviceFilter.Filter? = null) { lifecycleScope?.launch { withContext(Dispatchers.Default) { - val cachedDevices = mCachedDeviceManager!!.cachedDevicesCopy - for (cachedDevice in cachedDevices) { - onDeviceAdded(cachedDevice) - } + mCachedDeviceManager!!.cachedDevicesCopy + .filter { + filterForCachedDevices == null || filterForCachedDevices.matches(it.device) + } + .forEach(::onDeviceAdded) } } } diff --git a/tests/robotests/src/com/android/settings/bluetooth/DeviceListPreferenceFragmentTest.java b/tests/robotests/src/com/android/settings/bluetooth/DeviceListPreferenceFragmentTest.java deleted file mode 100644 index 4f46ce93d03..00000000000 --- a/tests/robotests/src/com/android/settings/bluetooth/DeviceListPreferenceFragmentTest.java +++ /dev/null @@ -1,237 +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.settings.bluetooth; - -import static com.google.common.truth.Truth.assertThat; - -import static org.mockito.ArgumentMatchers.any; -import static org.mockito.ArgumentMatchers.eq; -import static org.mockito.Mockito.doReturn; -import static org.mockito.Mockito.spy; -import static org.mockito.Mockito.times; -import static org.mockito.Mockito.verify; - -import android.bluetooth.BluetoothAdapter; -import android.bluetooth.BluetoothUuid; -import android.bluetooth.le.BluetoothLeScanner; -import android.bluetooth.le.ScanCallback; -import android.bluetooth.le.ScanFilter; -import android.bluetooth.le.ScanSettings; -import android.content.Context; -import android.content.res.Resources; - -import androidx.preference.Preference; - -import com.android.settings.R; -import com.android.settings.testutils.shadow.ShadowBluetoothAdapter; -import com.android.settingslib.bluetooth.CachedBluetoothDevice; -import com.android.settingslib.core.AbstractPreferenceController; - -import org.junit.Before; -import org.junit.Test; -import org.junit.runner.RunWith; -import org.mockito.Mock; -import org.mockito.MockitoAnnotations; -import org.robolectric.RobolectricTestRunner; -import org.robolectric.RuntimeEnvironment; -import org.robolectric.annotation.Config; - -import java.util.Collections; -import java.util.List; - -@RunWith(RobolectricTestRunner.class) -@Config(shadows = {ShadowBluetoothAdapter.class}) -public class DeviceListPreferenceFragmentTest { - - private static final String FOOTAGE_MAC_STRING = "Bluetooth mac: xxxx"; - - @Mock - private Resources mResource; - @Mock - private Context mContext; - @Mock - private BluetoothLeScanner mBluetoothLeScanner; - - private TestFragment mFragment; - private Preference mMyDevicePreference; - - - private BluetoothAdapter mBluetoothAdapter; - @Before - public void setUp() { - MockitoAnnotations.initMocks(this); - - mFragment = spy(new TestFragment()); - doReturn(mContext).when(mFragment).getContext(); - doReturn(mResource).when(mFragment).getResources(); - mBluetoothAdapter = spy(BluetoothAdapter.getDefaultAdapter()); - mFragment.mBluetoothAdapter = mBluetoothAdapter; - - mMyDevicePreference = new Preference(RuntimeEnvironment.application); - } - - @Test - public void setUpdateMyDevicePreference_setTitleCorrectly() { - doReturn(FOOTAGE_MAC_STRING).when(mFragment) - .getString(eq(R.string.bluetooth_footer_mac_message), any()); - - mFragment.updateFooterPreference(mMyDevicePreference); - - assertThat(mMyDevicePreference.getTitle()).isEqualTo(FOOTAGE_MAC_STRING); - } - - @Test - public void testEnableDisableScanning_testStateAfterEanbleDisable() { - mFragment.enableScanning(); - verify(mFragment).startScanning(); - assertThat(mFragment.mScanEnabled).isTrue(); - - mFragment.disableScanning(); - verify(mFragment).stopScanning(); - assertThat(mFragment.mScanEnabled).isFalse(); - } - - @Test - public void testScanningStateChanged_testScanStarted() { - mFragment.enableScanning(); - assertThat(mFragment.mScanEnabled).isTrue(); - verify(mFragment).startScanning(); - - mFragment.onScanningStateChanged(true); - verify(mFragment, times(1)).startScanning(); - } - - @Test - public void testScanningStateChanged_testScanFinished() { - // Could happen when last scanning not done while current scan gets enabled - mFragment.enableScanning(); - verify(mFragment).startScanning(); - assertThat(mFragment.mScanEnabled).isTrue(); - - mFragment.onScanningStateChanged(false); - verify(mFragment, times(2)).startScanning(); - } - - @Test - public void testScanningStateChanged_testScanStateMultiple() { - // Could happen when last scanning not done while current scan gets enabled - mFragment.enableScanning(); - assertThat(mFragment.mScanEnabled).isTrue(); - verify(mFragment).startScanning(); - - mFragment.onScanningStateChanged(true); - verify(mFragment, times(1)).startScanning(); - - mFragment.onScanningStateChanged(false); - verify(mFragment, times(2)).startScanning(); - - mFragment.onScanningStateChanged(true); - verify(mFragment, times(2)).startScanning(); - - mFragment.disableScanning(); - verify(mFragment).stopScanning(); - - mFragment.onScanningStateChanged(false); - verify(mFragment, times(2)).startScanning(); - - mFragment.onScanningStateChanged(true); - verify(mFragment, times(2)).startScanning(); - } - - @Test - public void testScanningStateChanged_testScanFinishedAfterDisable() { - mFragment.enableScanning(); - verify(mFragment).startScanning(); - assertThat(mFragment.mScanEnabled).isTrue(); - - mFragment.disableScanning(); - verify(mFragment).stopScanning(); - assertThat(mFragment.mScanEnabled).isFalse(); - - mFragment.onScanningStateChanged(false); - verify(mFragment, times(1)).startScanning(); - } - - @Test - public void testScanningStateChanged_testScanStartedAfterDisable() { - mFragment.enableScanning(); - verify(mFragment).startScanning(); - assertThat(mFragment.mScanEnabled).isTrue(); - - mFragment.disableScanning(); - verify(mFragment).stopScanning(); - assertThat(mFragment.mScanEnabled).isFalse(); - - mFragment.onScanningStateChanged(true); - verify(mFragment, times(1)).startScanning(); - } - - @Test - public void startScanning_setLeScanFilter_shouldStartLeScan() { - final ScanFilter leScanFilter = new ScanFilter.Builder() - .setServiceData(BluetoothUuid.HEARING_AID, new byte[]{0}, new byte[]{0}) - .build(); - doReturn(mBluetoothLeScanner).when(mBluetoothAdapter).getBluetoothLeScanner(); - - mFragment.setFilter(Collections.singletonList(leScanFilter)); - mFragment.startScanning(); - - verify(mBluetoothLeScanner).startScan(eq(Collections.singletonList(leScanFilter)), - any(ScanSettings.class), any(ScanCallback.class)); - } - - /** - * Fragment to test since {@code DeviceListPreferenceFragment} is abstract - */ - public static class TestFragment extends DeviceListPreferenceFragment { - - public TestFragment() { - super(""); - } - - @Override - public int getMetricsCategory() { - return 0; - } - - @Override - public void onDeviceBondStateChanged(CachedBluetoothDevice cachedDevice, int bondState) {} - - @Override - protected void initPreferencesFromPreferenceScreen() {} - - @Override - public String getDeviceListKey() { - return null; - } - - @Override - protected String getLogTag() { - return null; - } - - @Override - protected int getPreferenceScreenResId() { - return 0; - } - - @Override - protected List createPreferenceControllers(Context context) { - return null; - } - } -} diff --git a/tests/robotests/src/com/android/settings/bluetooth/DeviceListPreferenceFragmentTest.kt b/tests/robotests/src/com/android/settings/bluetooth/DeviceListPreferenceFragmentTest.kt new file mode 100644 index 00000000000..5a21aff2dc1 --- /dev/null +++ b/tests/robotests/src/com/android/settings/bluetooth/DeviceListPreferenceFragmentTest.kt @@ -0,0 +1,260 @@ +/* + * Copyright (C) 2023 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.settings.bluetooth + +import android.bluetooth.BluetoothAdapter +import android.bluetooth.BluetoothDevice +import android.bluetooth.BluetoothUuid +import android.bluetooth.le.BluetoothLeScanner +import android.bluetooth.le.ScanCallback +import android.bluetooth.le.ScanFilter +import android.content.Context +import android.content.res.Resources +import androidx.preference.Preference +import com.android.settings.R +import com.android.settings.testutils.shadow.ShadowBluetoothAdapter +import com.android.settingslib.bluetooth.BluetoothDeviceFilter +import com.android.settingslib.bluetooth.CachedBluetoothDevice +import com.android.settingslib.bluetooth.CachedBluetoothDeviceManager +import com.google.common.truth.Truth.assertThat +import kotlinx.coroutines.delay +import kotlinx.coroutines.runBlocking +import org.junit.Before +import org.junit.Rule +import org.junit.Test +import org.junit.runner.RunWith +import org.mockito.ArgumentMatchers.any +import org.mockito.ArgumentMatchers.eq +import org.mockito.Mock +import org.mockito.Mockito.doNothing +import org.mockito.Mockito.doReturn +import org.mockito.Mockito.mock +import org.mockito.Mockito.never +import org.mockito.Mockito.spy +import org.mockito.Mockito.times +import org.mockito.Mockito.verify +import org.mockito.Spy +import org.mockito.junit.MockitoJUnit +import org.mockito.junit.MockitoRule +import org.robolectric.RobolectricTestRunner +import org.robolectric.RuntimeEnvironment +import org.robolectric.annotation.Config +import org.mockito.Mockito.`when` as whenever + +@RunWith(RobolectricTestRunner::class) +@Config(shadows = [ShadowBluetoothAdapter::class]) +class DeviceListPreferenceFragmentTest { + @get:Rule + val mockito: MockitoRule = MockitoJUnit.rule() + + @Mock + private lateinit var resource: Resources + + @Mock + private lateinit var context: Context + + @Mock + private lateinit var bluetoothLeScanner: BluetoothLeScanner + + @Mock + private lateinit var cachedDeviceManager: CachedBluetoothDeviceManager + + @Mock + private lateinit var cachedDevice: CachedBluetoothDevice + + @Spy + private var fragment = TestFragment() + + private lateinit var myDevicePreference: Preference + private lateinit var bluetoothAdapter: BluetoothAdapter + + @Before + fun setUp() { + doReturn(context).`when`(fragment).context + doReturn(resource).`when`(fragment).resources + doNothing().`when`(fragment).onDeviceAdded(cachedDevice) + bluetoothAdapter = spy(BluetoothAdapter.getDefaultAdapter()) + fragment.mBluetoothAdapter = bluetoothAdapter + fragment.mCachedDeviceManager = cachedDeviceManager + + myDevicePreference = Preference(RuntimeEnvironment.application) + } + + @Test + fun setUpdateMyDevicePreference_setTitleCorrectly() { + doReturn(FOOTAGE_MAC_STRING).`when`(fragment) + .getString(eq(R.string.bluetooth_footer_mac_message), any()) + + fragment.updateFooterPreference(myDevicePreference) + + assertThat(myDevicePreference.title).isEqualTo(FOOTAGE_MAC_STRING) + } + + @Test + fun testEnableDisableScanning_testStateAfterEnableDisable() { + fragment.enableScanning() + verify(fragment).startScanning() + assertThat(fragment.mScanEnabled).isTrue() + + fragment.disableScanning() + verify(fragment).stopScanning() + assertThat(fragment.mScanEnabled).isFalse() + } + + @Test + fun testScanningStateChanged_testScanStarted() { + fragment.enableScanning() + assertThat(fragment.mScanEnabled).isTrue() + verify(fragment).startScanning() + + fragment.onScanningStateChanged(true) + verify(fragment, times(1)).startScanning() + } + + @Test + fun testScanningStateChanged_testScanFinished() { + // Could happen when last scanning not done while current scan gets enabled + fragment.enableScanning() + verify(fragment).startScanning() + assertThat(fragment.mScanEnabled).isTrue() + + fragment.onScanningStateChanged(false) + verify(fragment, times(2)).startScanning() + } + + @Test + fun testScanningStateChanged_testScanStateMultiple() { + // Could happen when last scanning not done while current scan gets enabled + fragment.enableScanning() + assertThat(fragment.mScanEnabled).isTrue() + verify(fragment).startScanning() + + fragment.onScanningStateChanged(true) + verify(fragment, times(1)).startScanning() + + fragment.onScanningStateChanged(false) + verify(fragment, times(2)).startScanning() + + fragment.onScanningStateChanged(true) + verify(fragment, times(2)).startScanning() + + fragment.disableScanning() + verify(fragment).stopScanning() + + fragment.onScanningStateChanged(false) + verify(fragment, times(2)).startScanning() + + fragment.onScanningStateChanged(true) + verify(fragment, times(2)).startScanning() + } + + @Test + fun testScanningStateChanged_testScanFinishedAfterDisable() { + fragment.enableScanning() + verify(fragment).startScanning() + assertThat(fragment.mScanEnabled).isTrue() + + fragment.disableScanning() + verify(fragment).stopScanning() + assertThat(fragment.mScanEnabled).isFalse() + + fragment.onScanningStateChanged(false) + verify(fragment, times(1)).startScanning() + } + + @Test + fun testScanningStateChanged_testScanStartedAfterDisable() { + fragment.enableScanning() + verify(fragment).startScanning() + assertThat(fragment.mScanEnabled).isTrue() + + fragment.disableScanning() + verify(fragment).stopScanning() + assertThat(fragment.mScanEnabled).isFalse() + + fragment.onScanningStateChanged(true) + verify(fragment, times(1)).startScanning() + } + + @Test + fun startScanning_setLeScanFilter_shouldStartLeScan() { + val leScanFilter = ScanFilter.Builder() + .setServiceData(BluetoothUuid.HEARING_AID, byteArrayOf(0), byteArrayOf(0)) + .build() + doReturn(bluetoothLeScanner).`when`(bluetoothAdapter).bluetoothLeScanner + + fragment.setFilter(listOf(leScanFilter)) + fragment.startScanning() + + verify(bluetoothLeScanner).startScan(eq(listOf(leScanFilter)), any(), any()) + } + + @Test + fun addCachedDevices_whenFilterIsNull_onDeviceAddedIsCalled() = runBlocking { + val mockCachedDevice = mock(CachedBluetoothDevice::class.java) + whenever(cachedDeviceManager.cachedDevicesCopy).thenReturn(listOf(mockCachedDevice)) + fragment.lifecycleScope = this + + fragment.addCachedDevices(filterForCachedDevices = null) + delay(100) + + verify(fragment).onDeviceAdded(mockCachedDevice) + } + + @Test + fun addCachedDevices_whenFilterMatched_onDeviceAddedIsCalled() = runBlocking { + val mockBluetoothDevice = mock(BluetoothDevice::class.java) + whenever(mockBluetoothDevice.bondState).thenReturn(BluetoothDevice.BOND_NONE) + whenever(cachedDevice.device).thenReturn(mockBluetoothDevice) + whenever(cachedDeviceManager.cachedDevicesCopy).thenReturn(listOf(cachedDevice)) + fragment.lifecycleScope = this + + fragment.addCachedDevices(BluetoothDeviceFilter.UNBONDED_DEVICE_FILTER) + delay(100) + + verify(fragment).onDeviceAdded(cachedDevice) + } + + @Test + fun addCachedDevices_whenFilterNoMatch_onDeviceAddedNotCalled() = runBlocking { + val mockBluetoothDevice = mock(BluetoothDevice::class.java) + whenever(mockBluetoothDevice.bondState).thenReturn(BluetoothDevice.BOND_BONDED) + whenever(cachedDevice.device).thenReturn(mockBluetoothDevice) + whenever(cachedDeviceManager.cachedDevicesCopy).thenReturn(listOf(cachedDevice)) + fragment.lifecycleScope = this + + fragment.addCachedDevices(BluetoothDeviceFilter.UNBONDED_DEVICE_FILTER) + delay(100) + + verify(fragment, never()).onDeviceAdded(cachedDevice) + } + + /** + * Fragment to test since `DeviceListPreferenceFragment` is abstract + */ + open class TestFragment : DeviceListPreferenceFragment(null) { + override fun getMetricsCategory() = 0 + override fun initPreferencesFromPreferenceScreen() {} + override val deviceListKey = "device_list" + override fun getLogTag() = null + override fun getPreferenceScreenResId() = 0 + } + + private companion object { + const val FOOTAGE_MAC_STRING = "Bluetooth mac: xxxx" + } +}