Merge "[Sb refactor] Add a reusable method to TableLogBufferFactory" into tm-qpr-dev

packages/SystemUI/src/com/android/systemui/log/table/TableLogBufferFactory.kt

@@ -29,6 +29,18 @@ constructor(
     private val dumpManager: DumpManager,
     private val systemClock: SystemClock,
 ) {
+    private val existingBuffers = mutableMapOf<String, TableLogBuffer>()
+
+    /**
+     * Creates a new [TableLogBuffer]. This method should only be called from static contexts, where
+     * it is guaranteed only to be created one time. See [getOrCreate] for a cache-aware method of
+     * obtaining a buffer.
+     *
+     * @param name a unique table name
+     * @param maxSize the buffer max size. See [adjustMaxSize]
+     *
+     * @return a new [TableLogBuffer] registered with [DumpManager]
+     */
     fun create(
         name: String,
         maxSize: Int,
@@ -37,4 +49,23 @@ constructor(
         dumpManager.registerNormalDumpable(name, tableBuffer)
         return tableBuffer
     }
+
+    /**
+     * Log buffers are retained indefinitely by [DumpManager], so that they can be represented in
+     * bugreports. Because of this, many of them are created statically in the Dagger graph.
+     *
+     * In the case where you have to create a logbuffer with a name only known at runtime, this
+     * method can be used to lazily create a table log buffer which is then cached for reuse.
+     *
+     * @return a [TableLogBuffer] suitable for reuse
+     */
+    fun getOrCreate(
+        name: String,
+        maxSize: Int,
+    ): TableLogBuffer =
+        existingBuffers.getOrElse(name) {
+            val buffer = create(name, maxSize)
+            existingBuffers[name] = buffer
+            buffer
+        }
 }

packages/SystemUI/src/com/android/systemui/statusbar/pipeline/mobile/data/repository/demo/DemoMobileConnectionsRepository.kt

@@ -149,7 +149,7 @@ constructor(
     }
 
     private fun createDemoMobileConnectionRepo(subId: Int): DemoMobileConnectionRepository {
-        val tableLogBuffer = logFactory.create("DemoMobileConnectionLog [$subId]", 100)
+        val tableLogBuffer = logFactory.getOrCreate("DemoMobileConnectionLog [$subId]", 100)
 
         return DemoMobileConnectionRepository(
             subId,

packages/SystemUI/src/com/android/systemui/statusbar/pipeline/mobile/data/repository/prod/MobileConnectionRepositoryImpl.kt

@@ -308,7 +308,7 @@ class MobileConnectionRepositoryImpl(
             networkNameSeparator: String,
             globalMobileDataSettingChangedEvent: Flow<Unit>,
         ): MobileConnectionRepository {
-            val mobileLogger = logFactory.create(tableBufferLogName(subId), 100)
+            val mobileLogger = logFactory.getOrCreate(tableBufferLogName(subId), 100)
 
             return MobileConnectionRepositoryImpl(
                 context,

packages/SystemUI/tests/src/com/android/systemui/log/table/TableLogBufferFactoryTest.kt

@@ -0,0 +1,64 @@
+/*
+ * 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.systemui.log.table
+
+import androidx.test.filters.SmallTest
+import com.android.systemui.SysuiTestCase
+import com.android.systemui.dump.DumpManager
+import com.android.systemui.util.mockito.mock
+import com.android.systemui.util.time.FakeSystemClock
+import com.google.common.truth.Truth.assertThat
+import org.junit.Test
+
+@SmallTest
+class TableLogBufferFactoryTest : SysuiTestCase() {
+    private val dumpManager: DumpManager = mock()
+    private val systemClock = FakeSystemClock()
+    private val underTest = TableLogBufferFactory(dumpManager, systemClock)
+
+    @Test
+    fun `create - always creates new instance`() {
+        val b1 = underTest.create(NAME_1, SIZE)
+        val b1_copy = underTest.create(NAME_1, SIZE)
+        val b2 = underTest.create(NAME_2, SIZE)
+        val b2_copy = underTest.create(NAME_2, SIZE)
+
+        assertThat(b1).isNotSameInstanceAs(b1_copy)
+        assertThat(b1).isNotSameInstanceAs(b2)
+        assertThat(b2).isNotSameInstanceAs(b2_copy)
+    }
+
+    @Test
+    fun `getOrCreate - reuses instance`() {
+        val b1 = underTest.getOrCreate(NAME_1, SIZE)
+        val b1_copy = underTest.getOrCreate(NAME_1, SIZE)
+        val b2 = underTest.getOrCreate(NAME_2, SIZE)
+        val b2_copy = underTest.getOrCreate(NAME_2, SIZE)
+
+        assertThat(b1).isSameInstanceAs(b1_copy)
+        assertThat(b2).isSameInstanceAs(b2_copy)
+        assertThat(b1).isNotSameInstanceAs(b2)
+        assertThat(b1_copy).isNotSameInstanceAs(b2_copy)
+    }
+
+    companion object {
+        const val NAME_1 = "name 1"
+        const val NAME_2 = "name 2"
+
+        const val SIZE = 8
+    }
+}

packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/mobile/data/repository/prod/MobileConnectionsRepositoryTest.kt

@@ -37,6 +37,7 @@ import com.android.systemui.SysuiTestCase
 import com.android.systemui.log.table.TableLogBuffer
 import com.android.systemui.log.table.TableLogBufferFactory
 import com.android.systemui.statusbar.pipeline.mobile.data.model.MobileConnectivityModel
+import com.android.systemui.statusbar.pipeline.mobile.data.model.NetworkNameModel
 import com.android.systemui.statusbar.pipeline.mobile.data.model.SubscriptionModel
 import com.android.systemui.statusbar.pipeline.mobile.util.FakeMobileMappingsProxy
 import com.android.systemui.statusbar.pipeline.shared.ConnectivityPipelineLogger
@@ -46,6 +47,7 @@ import com.android.systemui.util.mockito.eq
 import com.android.systemui.util.mockito.mock
 import com.android.systemui.util.mockito.whenever
 import com.android.systemui.util.settings.FakeSettings
+import com.android.systemui.util.time.FakeSystemClock
 import com.google.common.truth.Truth.assertThat
 import kotlinx.coroutines.CoroutineScope
 import kotlinx.coroutines.Dispatchers
@@ -94,7 +96,7 @@ class MobileConnectionsRepositoryTest : SysuiTestCase() {
             }
         }
 
-        whenever(logBufferFactory.create(anyString(), anyInt())).thenAnswer { _ ->
+        whenever(logBufferFactory.getOrCreate(anyString(), anyInt())).thenAnswer { _ ->
             mock<TableLogBuffer>()
         }
 
@@ -292,13 +294,13 @@ class MobileConnectionsRepositoryTest : SysuiTestCase() {
             // Get repos to trigger creation
             underTest.getRepoForSubId(SUB_1_ID)
             verify(logBufferFactory)
-                .create(
+                .getOrCreate(
                     eq(MobileConnectionRepositoryImpl.tableBufferLogName(SUB_1_ID)),
                     anyInt(),
                 )
             underTest.getRepoForSubId(SUB_2_ID)
             verify(logBufferFactory)
-                .create(
+                .getOrCreate(
                     eq(MobileConnectionRepositoryImpl.tableBufferLogName(SUB_2_ID)),
                     anyInt(),
                 )
@@ -306,6 +308,46 @@ class MobileConnectionsRepositoryTest : SysuiTestCase() {
             job.cancel()
         }
 
+    @Test
+    fun `connection repository factory - reuses log buffers for same connection`() =
+        runBlocking(IMMEDIATE) {
+            val realLoggerFactory = TableLogBufferFactory(mock(), FakeSystemClock())
+
+            connectionFactory =
+                MobileConnectionRepositoryImpl.Factory(
+                    fakeBroadcastDispatcher,
+                    context = context,
+                    telephonyManager = telephonyManager,
+                    bgDispatcher = IMMEDIATE,
+                    globalSettings = globalSettings,
+                    logger = logger,
+                    mobileMappingsProxy = mobileMappings,
+                    scope = scope,
+                    logFactory = realLoggerFactory,
+                )
+
+            // Create two connections for the same subId. Similar to if the connection appeared
+            // and disappeared from the connectionFactory's perspective
+            val connection1 =
+                connectionFactory.build(
+                    1,
+                    NetworkNameModel.Default("default_name"),
+                    "-",
+                    underTest.globalMobileDataSettingChangedEvent,
+                )
+
+            val connection1_repeat =
+                connectionFactory.build(
+                    1,
+                    NetworkNameModel.Default("default_name"),
+                    "-",
+                    underTest.globalMobileDataSettingChangedEvent,
+                )
+
+            assertThat(connection1.tableLogBuffer)
+                .isSameInstanceAs(connection1_repeat.tableLogBuffer)
+        }
+
     @Test
     fun mobileConnectivity_default() {
         assertThat(underTest.defaultMobileNetworkConnectivity.value)