From d21f14a002c12ef466a77878fa3c2e868701241a Mon Sep 17 00:00:00 2001 From: SongFerngWang Date: Tue, 19 Sep 2023 01:51:31 +0800 Subject: [PATCH] Remove the cached display name If the display name is changed, then the Settings should remove the cached display name. Bug: 296157273 Test: [pass] atest SubscriptionUtilTest [pass]Build and manual test Change-Id: I3b1297ddddf9f9051dd16523b97fc27255cf3923 --- .../settings/network/SubscriptionUtil.java | 43 +++++-- .../network/SubscriptionUtilTest.java | 111 ++++++++++++++++-- 2 files changed, 133 insertions(+), 21 deletions(-) diff --git a/src/com/android/settings/network/SubscriptionUtil.java b/src/com/android/settings/network/SubscriptionUtil.java index 0cd12fe6354..9974ba27708 100644 --- a/src/com/android/settings/network/SubscriptionUtil.java +++ b/src/com/android/settings/network/SubscriptionUtil.java @@ -18,7 +18,6 @@ package com.android.settings.network; import static android.telephony.SubscriptionManager.INVALID_SIM_SLOT_INDEX; import static android.telephony.UiccSlotInfo.CARD_STATE_INFO_PRESENT; - import static com.android.internal.util.CollectionUtils.emptyIfNull; import android.annotation.Nullable; @@ -56,6 +55,8 @@ import java.util.Map; import java.util.Set; import java.util.function.Function; import java.util.function.Supplier; +import java.util.regex.Matcher; +import java.util.regex.Pattern; import java.util.stream.Collectors; import java.util.stream.Stream; @@ -66,6 +67,9 @@ public class SubscriptionUtil { static final String SUB_ID = "sub_id"; @VisibleForTesting static final String KEY_UNIQUE_SUBSCRIPTION_DISPLAYNAME = "unique_subscription_displayName"; + private static final String REGEX_DISPLAY_NAME_PREFIXES = "^"; + private static final String REGEX_DISPLAY_NAME_SUFFIXES = "\\s[0-9]+"; + private static List sAvailableResultsForTesting; private static List sActiveResultsForTesting; @@ -281,8 +285,8 @@ public class SubscriptionUtil { String displayName = i.getDisplayName().toString(); info.originalName = TextUtils.equals(displayName, PROFILE_GENERIC_DISPLAY_NAME) - ? context.getResources().getString(R.string.sim_card) - : displayName.trim(); + ? context.getResources().getString(R.string.sim_card) + : displayName.trim(); return info; }); @@ -298,12 +302,17 @@ public class SubscriptionUtil { // If a display name is duplicate, append the final 4 digits of the phone number. // Creates a mapping of Subscription id to original display name + phone number display name final Supplier> uniqueInfos = () -> originalInfos.get().map(info -> { + int infoSubId = info.subscriptionInfo.getSubscriptionId(); String cachedDisplayName = getDisplayNameFromSharedPreference( - context, info.subscriptionInfo.getSubscriptionId()); - if (!TextUtils.isEmpty(cachedDisplayName)) { - Log.d(TAG, "use cached display name : " + cachedDisplayName); + context, infoSubId); + if (isValidCachedDisplayName(cachedDisplayName, info.originalName.toString())) { + Log.d(TAG, "use cached display name : for subId : " + infoSubId + + "cached display name : " + cachedDisplayName); info.uniqueName = cachedDisplayName; return info; + } else { + Log.d(TAG, "remove cached display name : " + infoSubId); + removeItemFromDisplayNameSharedPreference(context, infoSubId); } if (duplicateOriginalNames.contains(info.originalName)) { @@ -320,9 +329,8 @@ public class SubscriptionUtil { } else { info.uniqueName = info.originalName + " " + lastFourDigits; Log.d(TAG, "Cache display name [" + info.uniqueName + "] for sub id " - + info.subscriptionInfo.getSubscriptionId()); - saveDisplayNameToSharedPreference( - context, info.subscriptionInfo.getSubscriptionId(), info.uniqueName); + + infoSubId); + saveDisplayNameToSharedPreference(context, infoSubId, info.uniqueName); } } else { info.uniqueName = info.originalName; @@ -404,10 +412,27 @@ public class SubscriptionUtil { .apply(); } + private static void removeItemFromDisplayNameSharedPreference(Context context, int subId) { + getDisplayNameSharedPreferenceEditor(context) + .remove(SUB_ID + subId) + .commit(); + } + private static String getDisplayNameFromSharedPreference(Context context, int subid) { return getDisplayNameSharedPreferences(context).getString(SUB_ID + subid, ""); } + @VisibleForTesting + static boolean isValidCachedDisplayName(String cachedDisplayName, String originalName) { + if (TextUtils.isEmpty(cachedDisplayName) || TextUtils.isEmpty(originalName)) { + return false; + } + String regex = REGEX_DISPLAY_NAME_PREFIXES + originalName + REGEX_DISPLAY_NAME_SUFFIXES; + Pattern pattern = Pattern.compile(regex); + Matcher matcher = pattern.matcher(cachedDisplayName); + return matcher.matches(); + } + public static String getDisplayName(SubscriptionInfo info) { final CharSequence name = info.getDisplayName(); if (name != null) { diff --git a/tests/unit/src/com/android/settings/network/SubscriptionUtilTest.java b/tests/unit/src/com/android/settings/network/SubscriptionUtilTest.java index f0630423953..587e7349ad8 100644 --- a/tests/unit/src/com/android/settings/network/SubscriptionUtilTest.java +++ b/tests/unit/src/com/android/settings/network/SubscriptionUtilTest.java @@ -18,9 +18,7 @@ package com.android.settings.network; import static com.android.settings.network.SubscriptionUtil.KEY_UNIQUE_SUBSCRIPTION_DISPLAYNAME; import static com.android.settings.network.SubscriptionUtil.SUB_ID; - import static com.google.common.truth.Truth.assertThat; - import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertTrue; import static org.mockito.ArgumentMatchers.anyString; @@ -185,7 +183,7 @@ public class SubscriptionUtilTest { @Ignore @Test public void getUniqueDisplayNames_identicalCarriers_fourDigitsUsed() { - // Both subscriptoins have the same display name. + // Both subscriptions have the same display name. final SubscriptionInfo info1 = mock(SubscriptionInfo.class); final SubscriptionInfo info2 = mock(SubscriptionInfo.class); when(info1.getSubscriptionId()).thenReturn(SUBID_1); @@ -215,7 +213,7 @@ public class SubscriptionUtilTest { @Ignore @Test public void getUniqueDisplayNames_identicalCarriersAfterTrim_fourDigitsUsed() { - // Both subscriptoins have the same display name. + // Both subscriptions have the same display name. final SubscriptionInfo info1 = mock(SubscriptionInfo.class); final SubscriptionInfo info2 = mock(SubscriptionInfo.class); when(info1.getSubscriptionId()).thenReturn(SUBID_1); @@ -244,8 +242,8 @@ public class SubscriptionUtilTest { @Ignore @Test - public void getUniqueDisplayNames_phoneNumberBlocked_subscriptoinIdFallback() { - // Both subscriptoins have the same display name. + public void getUniqueDisplayNames_phoneNumberBlocked_subscriptionIdFallback() { + // Both subscriptions have the same display name. final SubscriptionInfo info1 = mock(SubscriptionInfo.class); final SubscriptionInfo info2 = mock(SubscriptionInfo.class); when(info1.getSubscriptionId()).thenReturn(SUBID_1); @@ -273,9 +271,9 @@ public class SubscriptionUtilTest { @Ignore @Test - public void getUniqueDisplayNames_phoneNumberIdentical_subscriptoinIdFallback() { + public void getUniqueDisplayNames_phoneNumberIdentical_subscriptionIdFallback() { // TODO have three here from the same carrier - // Both subscriptoins have the same display name. + // Both subscriptions have the same display name. final SubscriptionInfo info1 = mock(SubscriptionInfo.class); final SubscriptionInfo info2 = mock(SubscriptionInfo.class); final SubscriptionInfo info3 = mock(SubscriptionInfo.class); @@ -464,8 +462,8 @@ public class SubscriptionUtilTest { SharedPreferences sp = mock(SharedPreferences.class); when(mContext.getSharedPreferences( KEY_UNIQUE_SUBSCRIPTION_DISPLAYNAME, Context.MODE_PRIVATE)).thenReturn(sp); - when(sp.getString(eq(SUB_ID + SUBID_1), anyString())).thenReturn(CARRIER_1 + "6789"); - when(sp.getString(eq(SUB_ID + SUBID_2), anyString())).thenReturn(CARRIER_1 + "4321"); + when(sp.getString(eq(SUB_ID + SUBID_1), anyString())).thenReturn(CARRIER_1 + " 6789"); + when(sp.getString(eq(SUB_ID + SUBID_2), anyString())).thenReturn(CARRIER_1 + " 4321"); final CharSequence nameOfSub1 = @@ -475,8 +473,41 @@ public class SubscriptionUtilTest { assertThat(nameOfSub1).isNotNull(); assertThat(nameOfSub2).isNotNull(); - assertEquals(CARRIER_1 + "6789", nameOfSub1.toString()); - assertEquals(CARRIER_1 + "4321", nameOfSub2.toString()); + assertEquals(CARRIER_1 + " 6789", nameOfSub1.toString()); + assertEquals(CARRIER_1 + " 4321", nameOfSub2.toString()); + } + + @Test + public void getUniqueDisplayName_hasRecordAndNameIsChanged_doesNotUseRecordBeTheResult() { + final SubscriptionInfo info1 = mock(SubscriptionInfo.class); + final SubscriptionInfo info2 = mock(SubscriptionInfo.class); + when(info1.getSubscriptionId()).thenReturn(SUBID_1); + when(info2.getSubscriptionId()).thenReturn(SUBID_2); + when(info1.getDisplayName()).thenReturn(CARRIER_1); + when(info2.getDisplayName()).thenReturn(CARRIER_2); + when(mSubMgr.getAvailableSubscriptionInfoList()).thenReturn( + Arrays.asList(info1, info2)); + + SharedPreferences sp = mock(SharedPreferences.class); + SharedPreferences.Editor editor = mock(SharedPreferences.Editor.class); + when(mContext.getSharedPreferences( + KEY_UNIQUE_SUBSCRIPTION_DISPLAYNAME, Context.MODE_PRIVATE)).thenReturn(sp); + when(sp.edit()).thenReturn(editor); + when(editor.remove(anyString())).thenReturn(editor); + + when(sp.getString(eq(SUB_ID + SUBID_1), anyString())).thenReturn(CARRIER_1 + " 6789"); + when(sp.getString(eq(SUB_ID + SUBID_2), anyString())).thenReturn(CARRIER_1 + " 4321"); + + + final CharSequence nameOfSub1 = + SubscriptionUtil.getUniqueSubscriptionDisplayName(info1, mContext); + final CharSequence nameOfSub2 = + SubscriptionUtil.getUniqueSubscriptionDisplayName(info2, mContext); + + assertThat(nameOfSub1).isNotNull(); + assertThat(nameOfSub2).isNotNull(); + assertEquals(CARRIER_1 + " 6789", nameOfSub1.toString()); + assertEquals(CARRIER_2.toString(), nameOfSub2.toString()); } @Test @@ -501,4 +532,60 @@ public class SubscriptionUtilTest { assertTrue(SubscriptionUtil.isSimHardwareVisible(mContext)); } + + @Test + public void isValidCachedDisplayName_matchesRule1_returnTrue() { + String originalName = "originalName"; + String cacheString = "originalName 1234"; + + assertThat(SubscriptionUtil.isValidCachedDisplayName(cacheString, originalName)).isTrue(); + } + + @Test + public void isValidCachedDisplayName_matchesRule2_returnTrue() { + String originalName = "original Name"; + String cacheString = originalName + " " + 1234; + + assertThat(SubscriptionUtil.isValidCachedDisplayName(cacheString, originalName)).isTrue(); + } + + @Test + public void isValidCachedDisplayName_nameIsEmpty1_returnFalse() { + String originalName = "original Name"; + String cacheString = ""; + + assertThat(SubscriptionUtil.isValidCachedDisplayName(cacheString, originalName)).isFalse(); + } + + @Test + public void isValidCachedDisplayName_nameIsEmpty2_returnFalse() { + String originalName = ""; + String cacheString = "originalName 1234"; + + assertThat(SubscriptionUtil.isValidCachedDisplayName(cacheString, originalName)).isFalse(); + } + + @Test + public void isValidCachedDisplayName_nameIsDifferent_returnFalse() { + String originalName = "original Name"; + String cacheString = "originalName 1234"; + + assertThat(SubscriptionUtil.isValidCachedDisplayName(cacheString, originalName)).isFalse(); + } + + @Test + public void isValidCachedDisplayName_noNumber_returnFalse() { + String originalName = "original Name"; + String cacheString = originalName; + + assertThat(SubscriptionUtil.isValidCachedDisplayName(cacheString, originalName)).isFalse(); + } + + @Test + public void isValidCachedDisplayName_noSpace_returnFalse() { + String originalName = "original Name"; + String cacheString = originalName; + + assertThat(SubscriptionUtil.isValidCachedDisplayName(cacheString, originalName)).isFalse(); + } }