From 00d02f142aa24d9bb49adad03ea26eee99277619 Mon Sep 17 00:00:00 2001 From: Fabian Kozynski Date: Mon, 15 Apr 2019 09:48:30 -0400 Subject: [PATCH] Fixed concatenation to properly discard empty strings Before, concatenation of carrier names was done naively. Now we properly discard empty strings so there are no dangling separators. Extra mocking added because of temporary change in CarrierTextController. These should be removed with b/130246708. Test: atest Fixes: 130363633 Change-Id: I17bd437a4789eb48b3e4661f294c64e14efc9261 --- .../keyguard/CarrierTextController.java | 21 ++- .../keyguard/CarrierTextControllerTest.java | 138 +++++++++++++++++- 2 files changed, 157 insertions(+), 2 deletions(-) diff --git a/packages/SystemUI/src/com/android/keyguard/CarrierTextController.java b/packages/SystemUI/src/com/android/keyguard/CarrierTextController.java index eca39262dfb51..2e9b03c6b31e9 100644 --- a/packages/SystemUI/src/com/android/keyguard/CarrierTextController.java +++ b/packages/SystemUI/src/com/android/keyguard/CarrierTextController.java @@ -383,7 +383,7 @@ public class CarrierTextController { } if (TextUtils.isEmpty(displayText) && !airplaneMode) { - displayText = TextUtils.join(mSeparator, carrierNames); + displayText = joinNotEmpty(mSeparator, carrierNames); } final CarrierTextCallbackInfo info = new CarrierTextCallbackInfo( displayText, @@ -546,6 +546,25 @@ public class CarrierTextController { } } + /** + * Joins the strings in a sequence using a separator. Empty strings are discarded with no extra + * separator added so there are no extra separators that are not needed. + */ + private static CharSequence joinNotEmpty(CharSequence separator, CharSequence[] sequences) { + int length = sequences.length; + if (length == 0) return ""; + StringBuilder sb = new StringBuilder(); + for (int i = 0; i < length; i++) { + if (!TextUtils.isEmpty(sequences[i])) { + if (!TextUtils.isEmpty(sb)) { + sb.append(separator); + } + sb.append(sequences[i]); + } + } + return sb.toString(); + } + private static List append(List list, CharSequence string) { if (!TextUtils.isEmpty(string)) { list.add(string); diff --git a/packages/SystemUI/tests/src/com/android/keyguard/CarrierTextControllerTest.java b/packages/SystemUI/tests/src/com/android/keyguard/CarrierTextControllerTest.java index df534d79f730e..9f91a17e846dc 100644 --- a/packages/SystemUI/tests/src/com/android/keyguard/CarrierTextControllerTest.java +++ b/packages/SystemUI/tests/src/com/android/keyguard/CarrierTextControllerTest.java @@ -104,7 +104,8 @@ public class CarrierTextControllerTest extends SysuiTestCase { mCarrierTextCallbackInfo = new CarrierTextController.CarrierTextCallbackInfo("", new CharSequence[]{}, false, new int[]{}); - when(mTelephonyManager.getPhoneCount()).thenReturn(2); + when(mTelephonyManager.getPhoneCount()).thenReturn(3); + mCarrierTextController = new TestCarrierTextController(mContext, SEPARATOR, true, true, mKeyguardUpdateMonitor); // This should not start listening on any of the real dependencies @@ -130,6 +131,12 @@ public class CarrierTextControllerTest extends SysuiTestCase { reset(mCarrierTextCallback); when(mKeyguardUpdateMonitor.getSubscriptionInfo(anyBoolean())).thenReturn( new ArrayList<>()); + + // STOPSHIP(b/130246708) This line makes sure that SubscriptionManager provides the + // same answer as KeyguardUpdateMonitor. Remove when this is addressed + when(mSubscriptionManager.getActiveSubscriptionInfoList(anyBoolean())).thenReturn( + new ArrayList<>()); + when(mKeyguardUpdateMonitor.getSimState(anyInt())).thenReturn( IccCardConstants.State.CARD_IO_ERROR); // This should not produce an out of bounds error, even though there are no subscriptions @@ -173,7 +180,11 @@ public class CarrierTextControllerTest extends SysuiTestCase { list.add(TEST_SUBSCRIPTION); when(mKeyguardUpdateMonitor.getSimState(anyInt())).thenReturn(IccCardConstants.State.READY); when(mKeyguardUpdateMonitor.getSubscriptionInfo(anyBoolean())).thenReturn(list); + + // STOPSHIP(b/130246708) This line makes sure that SubscriptionManager provides the + // same answer as KeyguardUpdateMonitor. Remove when this is addressed when(mSubscriptionManager.getActiveSubscriptionInfoList(anyBoolean())).thenReturn(list); + mKeyguardUpdateMonitor.mServiceStates = new HashMap<>(); ArgumentCaptor captor = @@ -197,7 +208,11 @@ public class CarrierTextControllerTest extends SysuiTestCase { list.add(TEST_SUBSCRIPTION_ROAMING); when(mKeyguardUpdateMonitor.getSimState(anyInt())).thenReturn(IccCardConstants.State.READY); when(mKeyguardUpdateMonitor.getSubscriptionInfo(anyBoolean())).thenReturn(list); + + // STOPSHIP(b/130246708) This line makes sure that SubscriptionManager provides the + // same answer as KeyguardUpdateMonitor. Remove when this is addressed when(mSubscriptionManager.getActiveSubscriptionInfoList(anyBoolean())).thenReturn(list); + mKeyguardUpdateMonitor.mServiceStates = new HashMap<>(); ArgumentCaptor captor = @@ -219,6 +234,12 @@ public class CarrierTextControllerTest extends SysuiTestCase { reset(mCarrierTextCallback); when(mKeyguardUpdateMonitor.getSubscriptionInfo(anyBoolean())).thenReturn( new ArrayList<>()); + + // STOPSHIP(b/130246708) This line makes sure that SubscriptionManager provides the + // same answer as KeyguardUpdateMonitor. Remove when this is addressed + when(mSubscriptionManager.getActiveSubscriptionInfoList(anyBoolean())).thenReturn( + new ArrayList<>()); + ArgumentCaptor captor = ArgumentCaptor.forClass( CarrierTextController.CarrierTextCallbackInfo.class); @@ -233,6 +254,121 @@ public class CarrierTextControllerTest extends SysuiTestCase { } + @Test + public void testCarrierText_twoValidSubscriptions() { + reset(mCarrierTextCallback); + List list = new ArrayList<>(); + list.add(TEST_SUBSCRIPTION); + list.add(TEST_SUBSCRIPTION); + when(mKeyguardUpdateMonitor.getSimState(anyInt())).thenReturn(IccCardConstants.State.READY); + when(mKeyguardUpdateMonitor.getSubscriptionInfo(anyBoolean())).thenReturn(list); + + // STOPSHIP(b/130246708) This line makes sure that SubscriptionManager provides the + // same answer as KeyguardUpdateMonitor. Remove when this is addressed + when(mSubscriptionManager.getActiveSubscriptionInfoList(anyBoolean())).thenReturn(list); + + mKeyguardUpdateMonitor.mServiceStates = new HashMap<>(); + + ArgumentCaptor captor = + ArgumentCaptor.forClass( + CarrierTextController.CarrierTextCallbackInfo.class); + + mCarrierTextController.updateCarrierText(); + mTestableLooper.processAllMessages(); + verify(mCarrierTextCallback).updateCarrierInfo(captor.capture()); + + assertEquals(TEST_CARRIER + SEPARATOR + TEST_CARRIER, + captor.getValue().carrierText); + } + + @Test + public void testCarrierText_oneDisabledSub() { + reset(mCarrierTextCallback); + List list = new ArrayList<>(); + list.add(TEST_SUBSCRIPTION); + list.add(TEST_SUBSCRIPTION); + when(mKeyguardUpdateMonitor.getSimState(anyInt())) + .thenReturn(IccCardConstants.State.READY) + .thenReturn(IccCardConstants.State.NOT_READY); + when(mKeyguardUpdateMonitor.getSubscriptionInfo(anyBoolean())).thenReturn(list); + + // STOPSHIP(b/130246708) This line makes sure that SubscriptionManager provides the + // same answer as KeyguardUpdateMonitor. Remove when this is addressed + when(mSubscriptionManager.getActiveSubscriptionInfoList(anyBoolean())).thenReturn(list); + + mKeyguardUpdateMonitor.mServiceStates = new HashMap<>(); + + ArgumentCaptor captor = + ArgumentCaptor.forClass( + CarrierTextController.CarrierTextCallbackInfo.class); + + mCarrierTextController.updateCarrierText(); + mTestableLooper.processAllMessages(); + verify(mCarrierTextCallback).updateCarrierInfo(captor.capture()); + + assertEquals(TEST_CARRIER, + captor.getValue().carrierText); + } + + @Test + public void testCarrierText_firstDisabledSub() { + reset(mCarrierTextCallback); + List list = new ArrayList<>(); + list.add(TEST_SUBSCRIPTION); + list.add(TEST_SUBSCRIPTION); + when(mKeyguardUpdateMonitor.getSimState(anyInt())) + .thenReturn(IccCardConstants.State.NOT_READY) + .thenReturn(IccCardConstants.State.READY); + when(mKeyguardUpdateMonitor.getSubscriptionInfo(anyBoolean())).thenReturn(list); + + // STOPSHIP(b/130246708) This line makes sure that SubscriptionManager provides the + // same answer as KeyguardUpdateMonitor. Remove when this is addressed + when(mSubscriptionManager.getActiveSubscriptionInfoList(anyBoolean())).thenReturn(list); + + mKeyguardUpdateMonitor.mServiceStates = new HashMap<>(); + + ArgumentCaptor captor = + ArgumentCaptor.forClass( + CarrierTextController.CarrierTextCallbackInfo.class); + + mCarrierTextController.updateCarrierText(); + mTestableLooper.processAllMessages(); + verify(mCarrierTextCallback).updateCarrierInfo(captor.capture()); + + assertEquals(TEST_CARRIER, + captor.getValue().carrierText); + } + + @Test + public void testCarrierText_threeSubsMiddleDisabled() { + reset(mCarrierTextCallback); + List list = new ArrayList<>(); + list.add(TEST_SUBSCRIPTION); + list.add(TEST_SUBSCRIPTION); + list.add(TEST_SUBSCRIPTION); + when(mKeyguardUpdateMonitor.getSimState(anyInt())) + .thenReturn(IccCardConstants.State.READY) + .thenReturn(IccCardConstants.State.NOT_READY) + .thenReturn(IccCardConstants.State.READY); + when(mKeyguardUpdateMonitor.getSubscriptionInfo(anyBoolean())).thenReturn(list); + mKeyguardUpdateMonitor.mServiceStates = new HashMap<>(); + + // STOPSHIP(b/130246708) This line makes sure that SubscriptionManager provides the + // same answer as KeyguardUpdateMonitor. Remove when this is addressed + when(mSubscriptionManager.getActiveSubscriptionInfoList(anyBoolean())).thenReturn(list); + + ArgumentCaptor captor = + ArgumentCaptor.forClass( + CarrierTextController.CarrierTextCallbackInfo.class); + + mCarrierTextController.updateCarrierText(); + mTestableLooper.processAllMessages(); + verify(mCarrierTextCallback).updateCarrierInfo(captor.capture()); + + assertEquals(TEST_CARRIER + SEPARATOR + TEST_CARRIER, + captor.getValue().carrierText); + } + public static class TestCarrierTextController extends CarrierTextController { private KeyguardUpdateMonitor mKUM;