From 22080a1df54027d42f87f3004d79dc42c5337fa5 Mon Sep 17 00:00:00 2001 From: Shunta Sato Date: Mon, 19 Dec 2016 19:30:46 +0800 Subject: [PATCH] Fix Wi-Fi list adds same AP repeatedly When switch AP security mode, several same APs are shown. To fix this issue, append security type to preference key for avoiding different APs have same key. git fetch Cherrypick of: https://partner-android-review.googlesource.com/#/c/799829/ Bug: 37558394 Test: runtest --path packages/apps/Settings/tests/unit/src/com/android/settings/wifi/WifiSettingsUiTest.java Change-Id: I39621636f14b29e45ba96ff76dc3c21a4996a136 --- .../android/settings/wifi/WifiSettings.java | 17 +++-- .../settings/wifi/WifiSettingsUiTest.java | 68 +++++++++++++------ 2 files changed, 62 insertions(+), 23 deletions(-) diff --git a/src/com/android/settings/wifi/WifiSettings.java b/src/com/android/settings/wifi/WifiSettings.java index 3cfb17dd5a6..92846a602f9 100644 --- a/src/com/android/settings/wifi/WifiSettings.java +++ b/src/com/android/settings/wifi/WifiSettings.java @@ -749,10 +749,7 @@ public class WifiSettings extends RestrictedSettingsFragment AccessPoint accessPoint = accessPoints.get(index); // Ignore access points that are out of range. if (accessPoint.isReachable()) { - String key = accessPoint.getBssid(); - if (TextUtils.isEmpty(key)) { - key = accessPoint.getSsidStr(); - } + String key = generateKey(accessPoint); hasAvailableAccessPoints = true; LongPressAccessPointPreference pref = (LongPressAccessPointPreference) getCachedPreference(key); @@ -794,6 +791,18 @@ public class WifiSettings extends RestrictedSettingsFragment } } + private String generateKey(AccessPoint accessPoint) { + StringBuilder key = new StringBuilder(); + String bssid = accessPoint.getBssid(); + if (TextUtils.isEmpty(bssid)) { + key.append(accessPoint.getSsidStr()); + } else { + key.append(bssid); + } + key.append(',').append(accessPoint.getSecurity()); + return key.toString(); + } + @NonNull private LongPressAccessPointPreference createLongPressActionPointPreference( AccessPoint accessPoint) { diff --git a/tests/unit/src/com/android/settings/wifi/WifiSettingsUiTest.java b/tests/unit/src/com/android/settings/wifi/WifiSettingsUiTest.java index 640c8849c6f..a85d5917d15 100644 --- a/tests/unit/src/com/android/settings/wifi/WifiSettingsUiTest.java +++ b/tests/unit/src/com/android/settings/wifi/WifiSettingsUiTest.java @@ -15,6 +15,24 @@ */ package com.android.settings.wifi; +import static android.support.test.InstrumentationRegistry.getInstrumentation; +import static android.support.test.espresso.Espresso.onView; +import static android.support.test.espresso.assertion.ViewAssertions.doesNotExist; +import static android.support.test.espresso.assertion.ViewAssertions.matches; +import static android.support.test.espresso.matcher.ViewMatchers.Visibility.VISIBLE; +import static android.support.test.espresso.matcher.ViewMatchers.isDisplayed; +import static android.support.test.espresso.matcher.ViewMatchers.withEffectiveVisibility; +import static android.support.test.espresso.matcher.ViewMatchers.withText; + +import static com.google.common.truth.Truth.assertThat; + +import static org.hamcrest.Matchers.allOf; +import static org.hamcrest.Matchers.not; +import static org.hamcrest.Matchers.startsWith; +import static org.mockito.Mockito.atMost; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + import android.app.Activity; import android.app.Fragment; import android.content.Context; @@ -31,6 +49,7 @@ import android.support.test.runner.AndroidJUnit4; import com.android.settings.Settings.WifiSettingsActivity; import com.android.settingslib.wifi.AccessPoint; +import com.android.settingslib.wifi.TestAccessPointBuilder; import com.android.settingslib.wifi.WifiTracker; import com.android.settingslib.wifi.WifiTracker.WifiListener; import com.android.settingslib.wifi.WifiTrackerFactory; @@ -46,25 +65,6 @@ import org.mockito.MockitoAnnotations; import java.util.List; -import static android.support.test.InstrumentationRegistry.getInstrumentation; -import static android.support.test.espresso.Espresso.onView; -import static android.support.test.espresso.assertion.ViewAssertions.doesNotExist; -import static android.support.test.espresso.assertion.ViewAssertions.matches; -import static android.support.test.espresso.matcher.ViewMatchers.Visibility.VISIBLE; -import static android.support.test.espresso.matcher.ViewMatchers.isDisplayed; -import static android.support.test.espresso.matcher.ViewMatchers.withEffectiveVisibility; -import static android.support.test.espresso.matcher.ViewMatchers.withText; - -import static com.google.common.truth.Truth.assertThat; - -import static org.hamcrest.Matchers.allOf; -import static org.hamcrest.Matchers.not; -import static org.hamcrest.Matchers.startsWith; - -import static org.mockito.Mockito.atMost; -import static org.mockito.Mockito.verify; -import static org.mockito.Mockito.when; - @RunWith(AndroidJUnit4.class) public class WifiSettingsUiTest { @@ -242,4 +242,34 @@ public class WifiSettingsUiTest { getInstrumentation().callActivityOnStart(activity); verify(mWifiTracker, atMost(1)).forceUpdate(); } + + @Test + public void changingSecurityStateOnApShouldNotCauseMultipleListItems() { + setWifiState(WifiManager.WIFI_STATE_ENABLED); + TestAccessPointBuilder builder = new TestAccessPointBuilder(mContext) + .setSsid(TEST_SSID).setSecurity(AccessPoint.SECURITY_NONE); + AccessPoint open = builder.build(); + + builder.setSecurity(AccessPoint.SECURITY_EAP); + AccessPoint eap = builder.build(); + + builder.setSecurity(AccessPoint.SECURITY_WEP); + AccessPoint wep = builder.build(); + + // Return a different security state each time getAccessPoints is invoked + when(mWifiTracker.getAccessPoints()) + .thenReturn(Lists.newArrayList(open, eap)) + .thenReturn(Lists.newArrayList(eap)) + .thenReturn(Lists.newArrayList(wep)); + + launchActivity(); + + onView(withText(TEST_SSID)).check(matches(isDisplayed())); + + mWifiListener.onAccessPointsChanged(); + onView(withText(TEST_SSID)).check(matches(isDisplayed())); + + mWifiListener.onAccessPointsChanged(); + onView(withText(TEST_SSID)).check(matches(isDisplayed())); + } }