From c0d81e615c9f582ac211c0e467bd0882db1cff0c Mon Sep 17 00:00:00 2001
From: Evan Laird <evanlaird@google.com>
Date: Tue, 20 Jun 2023 15:36:34 -0400
Subject: [PATCH] [Sb refactor] Remove throwing condition when requesting a
 repo

Initially it seemed unlikely that we would ever request a mobile
connections repository for a subscription id that we had not-yet been
told about. I.e., a view model requesting state for subId N should only
happen after we are told that N is an existing subscription.

This check is fine in the majority of cases, but it's prone to race
conditions since to check if a subscription is valid we need to check
the _current_ list of subscriptions.

Removing this thrown exception since it doesn't meet the criteria of
catching actionable exceptions, nor does it represent an actual bad
state anymore. If we ever end up with a view model tracking a
subscription that does not exist, then it will show up in logs and we
can trace where it came from then.

Test: all tests in com.android.systemui.statusbar.pipeline
Fixes: 273863830
Change-Id: I2453b5ebc08f738e6cf25689860558a7a5c47c44
---
 .../prod/MobileConnectionsRepositoryImpl.kt           | 11 ++---------
 .../prod/MobileConnectionsRepositoryTest.kt           |  8 +++-----
 2 files changed, 5 insertions(+), 14 deletions(-)

diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/mobile/data/repository/prod/MobileConnectionsRepositoryImpl.kt b/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/mobile/data/repository/prod/MobileConnectionsRepositoryImpl.kt
index a47f95d69c65b..302580ab0c53d 100644
--- a/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/mobile/data/repository/prod/MobileConnectionsRepositoryImpl.kt
+++ b/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/mobile/data/repository/prod/MobileConnectionsRepositoryImpl.kt
@@ -255,15 +255,8 @@ constructor(
             .distinctUntilChanged()
             .onEach { logger.logDefaultMobileIconGroup(it) }
 
-    override fun getRepoForSubId(subId: Int): FullMobileConnectionRepository {
-        if (!isValidSubId(subId)) {
-            throw IllegalArgumentException(
-                "subscriptionId $subId is not in the list of valid subscriptions"
-            )
-        }
-
-        return getOrCreateRepoForSubId(subId)
-    }
+    override fun getRepoForSubId(subId: Int): FullMobileConnectionRepository =
+        getOrCreateRepoForSubId(subId)
 
     private fun getOrCreateRepoForSubId(subId: Int) =
         subIdRepositoryCache[subId]
diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/mobile/data/repository/prod/MobileConnectionsRepositoryTest.kt b/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/mobile/data/repository/prod/MobileConnectionsRepositoryTest.kt
index fd156d86c19eb..4ac9a581e764a 100644
--- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/mobile/data/repository/prod/MobileConnectionsRepositoryTest.kt
+++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/mobile/data/repository/prod/MobileConnectionsRepositoryTest.kt
@@ -73,7 +73,6 @@ import kotlinx.coroutines.test.StandardTestDispatcher
 import kotlinx.coroutines.test.TestScope
 import kotlinx.coroutines.test.runCurrent
 import kotlinx.coroutines.test.runTest
-import org.junit.Assert.assertThrows
 import org.junit.Assert.assertTrue
 import org.junit.Before
 import org.junit.Test
@@ -588,11 +587,10 @@ class MobileConnectionsRepositoryTest : SysuiTestCase() {
         }
 
     @Test
-    fun testConnectionRepository_invalidSubId_throws() =
+    fun testConnectionRepository_invalidSubId_doesNotThrow() =
         testScope.runTest {
-            assertThrows(IllegalArgumentException::class.java) {
-                underTest.getRepoForSubId(SUB_1_ID)
-            }
+            underTest.getRepoForSubId(SUB_1_ID)
+            // No exception
         }
 
     @Test