RUM-6195: Allow ResourcesLruCache to work with any key

DataDog · Nov 28, 2024 · 8743eb1 · 8743eb1
1 parent 4590058
commit 8743eb1
Show file tree

Hide file tree

Showing 8 changed files with 63 additions and 61 deletions.
diff --git a/...tlin/com/datadog/android/sessionreplay/internal/recorder/resources/BitmapCachesManager.kt b/...tlin/com/datadog/android/sessionreplay/internal/recorder/resources/BitmapCachesManager.kt
@@ -51,15 +51,19 @@ internal class BitmapCachesManager(
         isBitmapPoolRegisteredForCallbacks = true
     }
 
-    internal fun putInResourceCache(drawable: Drawable, resourceId: String) {
-        resourcesLRUCache.put(drawable, resourceId.toByteArray(Charsets.UTF_8))
+    internal fun putInResourceCache(key: String, resourceId: String) {
+        resourcesLRUCache.put(key, resourceId.toByteArray(Charsets.UTF_8))
     }
 
-    internal fun getFromResourceCache(drawable: Drawable): String? {
-        val resourceId = resourcesLRUCache.get(drawable) ?: return null
+    internal fun getFromResourceCache(key: String): String? {
+        val resourceId = resourcesLRUCache.get(key) ?: return null
         return String(resourceId, Charsets.UTF_8)
     }
 
+    internal fun generateResourceKeyFromDrawable(drawable: Drawable): String? {
+        return (resourcesLRUCache as? ResourcesLRUCache)?.generateKeyFromDrawable(drawable)
+    }
+
     internal fun putInBitmapPool(bitmap: Bitmap) {
         bitmapPool.put(bitmap)
     }

diff --git a/...c/main/kotlin/com/datadog/android/sessionreplay/internal/recorder/resources/BitmapPool.kt b/...c/main/kotlin/com/datadog/android/sessionreplay/internal/recorder/resources/BitmapPool.kt
@@ -91,8 +91,8 @@ internal class BitmapPool(
     }
 
     @Synchronized
-    override fun get(element: String): Bitmap? {
-        val bitmapsWithReqDimensions = bitmapsBySize[element] ?: return null
+    override fun get(key: String): Bitmap? {
+        val bitmapsWithReqDimensions = bitmapsBySize[key] ?: return null
 
         // find the first unused bitmap, mark it as used and return it
         return bitmapsWithReqDimensions.find {

diff --git a/...ay/src/main/kotlin/com/datadog/android/sessionreplay/internal/recorder/resources/Cache.kt b/...ay/src/main/kotlin/com/datadog/android/sessionreplay/internal/recorder/resources/Cache.kt
@@ -9,8 +9,8 @@ package com.datadog.android.sessionreplay.internal.recorder.resources
 internal interface Cache<K : Any, V : Any> {
 
     fun put(value: V) {}
-    fun put(element: K, value: V) {}
-    fun get(element: K): V? = null
+    fun put(key: String, value: V) {}
+    fun get(key: String): V? = null
     fun size(): Int
     fun clear()
 

diff --git a/.../kotlin/com/datadog/android/sessionreplay/internal/recorder/resources/ResourceResolver.kt b/.../kotlin/com/datadog/android/sessionreplay/internal/recorder/resources/ResourceResolver.kt
@@ -233,7 +233,8 @@ internal class ResourceResolver(
             bitmapCachesManager.putInBitmapPool(bitmap)
         }
 
-        bitmapCachesManager.putInResourceCache(drawable, resourceId)
+        val key = bitmapCachesManager.generateResourceKeyFromDrawable(drawable) ?: return
+        bitmapCachesManager.putInResourceCache(key, resourceId)
     }
 
     @WorkerThread
@@ -321,7 +322,10 @@ internal class ResourceResolver(
 
     private fun tryToGetResourceFromCache(
         drawable: Drawable
-    ): String? = bitmapCachesManager.getFromResourceCache(drawable)
+    ): String? {
+        val key = bitmapCachesManager.generateResourceKeyFromDrawable(drawable) ?: return null
+        return bitmapCachesManager.getFromResourceCache(key)
+    }
 
     private fun shouldUseDrawableBitmap(drawable: BitmapDrawable): Boolean {
         return drawable.bitmap != null &&

diff --git a/...kotlin/com/datadog/android/sessionreplay/internal/recorder/resources/ResourcesLRUCache.kt b/...kotlin/com/datadog/android/sessionreplay/internal/recorder/resources/ResourcesLRUCache.kt
@@ -12,7 +12,6 @@ import android.graphics.drawable.AnimationDrawable
 import android.graphics.drawable.Drawable
 import android.graphics.drawable.DrawableContainer
 import android.graphics.drawable.LayerDrawable
-import androidx.annotation.VisibleForTesting
 import androidx.collection.LruCache
 import com.datadog.android.sessionreplay.internal.recorder.safeGetDrawable
 import com.datadog.android.sessionreplay.internal.utils.CacheUtils
@@ -45,9 +44,7 @@ internal class ResourcesLRUCache(
     }
 
     @Synchronized
-    override fun put(element: Drawable, value: ByteArray) {
-        val key = generateKey(element)
-
+    override fun put(key: String, value: ByteArray) {
         @Suppress("UnsafeThirdPartyFunctionCall") // Called within a try/catch block
         invocationUtils.safeCallWithErrorLogging(
             call = { cache.put(key, value) },
@@ -56,11 +53,11 @@ internal class ResourcesLRUCache(
     }
 
     @Synchronized
-    override fun get(element: Drawable): ByteArray? =
+    override fun get(key: String): ByteArray? =
         @Suppress("UnsafeThirdPartyFunctionCall") // Called within a try/catch block
         invocationUtils.safeCallWithErrorLogging(
             call = {
-                cache.get(generateKey(element))
+                cache.get(key)
             },
             failureMessage = FAILURE_MSG_GET_CACHE
         )
@@ -76,9 +73,8 @@ internal class ResourcesLRUCache(
         )
     }
 
-    @VisibleForTesting
-    internal fun generateKey(drawable: Drawable): String =
-        generatePrefix(drawable) + System.identityHashCode(drawable)
+    internal fun generateKeyFromDrawable(element: Drawable): String =
+        generatePrefix(element) + System.identityHashCode(element)
 
     private fun generatePrefix(drawable: Drawable): String {
         return when (drawable) {

diff --git a/.../com/datadog/android/sessionreplay/internal/recorder/resources/BitmapCachesManagerTest.kt b/.../com/datadog/android/sessionreplay/internal/recorder/resources/BitmapCachesManagerTest.kt
@@ -24,6 +24,7 @@ import org.junit.jupiter.api.extension.Extensions
 import org.mockito.Mock
 import org.mockito.junit.jupiter.MockitoExtension
 import org.mockito.junit.jupiter.MockitoSettings
+import org.mockito.kotlin.any
 import org.mockito.kotlin.verify
 import org.mockito.kotlin.whenever
 import org.mockito.quality.Strictness
@@ -58,8 +59,13 @@ internal class BitmapCachesManagerTest {
     @StringForgery
     lateinit var fakeResourceId: String
 
+    @StringForgery
+    lateinit var fakeResourceKey: String
+
     @BeforeEach
     fun `set up`() {
+        whenever(mockResourcesCache.generateKeyFromDrawable(any())).thenReturn(fakeResourceId)
+
         testedCachesManager = createBitmapCachesManager(
             bitmapPool = mockBitmapPool,
             resourcesLRUCache = mockResourcesCache,
@@ -103,20 +109,20 @@ internal class BitmapCachesManagerTest {
     @Test
     fun `M put in resource cache W putInResourceCache`() {
         // When
-        testedCachesManager.putInResourceCache(mockDrawable, fakeResourceId)
+        testedCachesManager.putInResourceCache(fakeResourceKey, fakeResourceId)
 
         // Then
-        verify(mockResourcesCache).put(mockDrawable, fakeResourceId.toByteArray(Charsets.UTF_8))
+        verify(mockResourcesCache).put(fakeResourceKey, fakeResourceId.toByteArray(Charsets.UTF_8))
     }
 
     @Test
     fun `M get resource from resource cache W getFromResourceCache { resource exists in cache }`() {
         // Given
         val fakeCacheData = fakeResourceId.toByteArray(Charsets.UTF_8)
-        whenever(mockResourcesCache.get(mockDrawable)).thenReturn(fakeCacheData)
+        whenever(mockResourcesCache.get(fakeResourceKey)).thenReturn(fakeCacheData)
 
         // When
-        val result = testedCachesManager.getFromResourceCache(mockDrawable)
+        val result = testedCachesManager.getFromResourceCache(fakeResourceKey)
 
         // Then
         assertThat(result).isEqualTo(fakeResourceId)
@@ -125,10 +131,10 @@ internal class BitmapCachesManagerTest {
     @Test
     fun `M get null from resource cache W getFromResourceCache { resource not in cache }`() {
         // When
-        val result = testedCachesManager.getFromResourceCache(mockDrawable)
+        val result = testedCachesManager.getFromResourceCache(fakeResourceKey)
 
         // Then
-        verify(mockResourcesCache).get(mockDrawable)
+        verify(mockResourcesCache).get(fakeResourceKey)
         assertThat(result).isNull()
     }
 

diff --git a/...lin/com/datadog/android/sessionreplay/internal/recorder/resources/ResourceResolverTest.kt b/...lin/com/datadog/android/sessionreplay/internal/recorder/resources/ResourceResolverTest.kt
@@ -11,7 +11,6 @@ import android.content.res.Resources
 import android.graphics.Bitmap
 import android.graphics.drawable.BitmapDrawable
 import android.graphics.drawable.Drawable
-import android.graphics.drawable.Drawable.ConstantState
 import android.graphics.drawable.LayerDrawable
 import android.graphics.drawable.StateListDrawable
 import android.util.DisplayMetrics
@@ -108,16 +107,16 @@ internal class ResourceResolverTest {
     @Mock
     lateinit var mockResources: Resources
 
-    @Mock
-    lateinit var mockBitmapConstantState: ConstantState
-
     private var fakeBitmapWidth: Int = 1
 
     private var fakeBitmapHeight: Int = 1
 
     @Forgery
     lateinit var fakeApplicationid: UUID
 
+    @StringForgery
+    lateinit var fakeResourceKey: String
+
     @StringForgery
     lateinit var fakeResourceId: String
 
@@ -128,6 +127,7 @@ internal class ResourceResolverTest {
         whenever(mockDrawableCopier.copy(eq(mockBitmapDrawable), any())).thenReturn(
             mockBitmapDrawable
         )
+        whenever(mockBitmapCachesManager.generateResourceKeyFromDrawable(mockDrawable)).thenReturn(fakeResourceKey)
         whenever(mockDrawableCopier.copy(eq(mockDrawable), any())).thenReturn(mockDrawable)
         fakeImageCompressionByteArray = forge.aString().toByteArray()
 
@@ -175,7 +175,7 @@ internal class ResourceResolverTest {
     @Test
     fun `M get data from cache W resolveResourceId() { cache hit with resourceId }`() {
         // Given
-        whenever(mockBitmapCachesManager.getFromResourceCache(mockDrawable)).thenReturn(fakeResourceId)
+        whenever(mockBitmapCachesManager.getFromResourceCache(fakeResourceKey)).thenReturn(fakeResourceId)
 
         whenever(mockWebPImageCompression.compressBitmap(any()))
             .thenReturn(fakeImageCompressionByteArray)
@@ -210,7 +210,7 @@ internal class ResourceResolverTest {
 
         val emptyByteArray = ByteArray(0)
 
-        whenever(mockBitmapCachesManager.getFromResourceCache(mockBitmapDrawable))
+        whenever(mockBitmapCachesManager.getFromResourceCache(fakeResourceKey))
             .thenReturn(null)
 
         whenever(mockWebPImageCompression.compressBitmap(any()))
@@ -321,7 +321,7 @@ internal class ResourceResolverTest {
     @Test
     fun `M calculate resourceId W resolveResourceId() { cache miss }`() {
         // Given
-        whenever(mockResourcesLRUCache.get(mockDrawable)).thenReturn(null)
+        whenever(mockResourcesLRUCache.get(fakeResourceKey)).thenReturn(null)
 
         // When
         testedResourceResolver.resolveResourceId(
@@ -351,7 +351,7 @@ internal class ResourceResolverTest {
     @Test
     fun `M return failure W resolveResourceId { createBitmapOfApproxSizeFromDrawable failed }`() {
         // Given
-        whenever(mockResourcesLRUCache.get(mockDrawable)).thenReturn(null)
+        whenever(mockResourcesLRUCache.get(fakeResourceKey)).thenReturn(null)
         whenever(
             mockDrawableUtils.createBitmapOfApproxSizeFromDrawable(
                 resources = any(),
@@ -670,7 +670,7 @@ internal class ResourceResolverTest {
     @Test
     fun `M cache bitmap W resolveResourceId() { from BitmapDrawable with null bitmap }`() {
         // Given
-        whenever(mockBitmapCachesManager.getFromResourceCache(mockBitmapDrawable))
+        whenever(mockBitmapCachesManager.getFromResourceCache(fakeResourceKey))
             .thenReturn(null)
         whenever(mockBitmapDrawable.bitmap).thenReturn(null)
 
@@ -718,12 +718,16 @@ internal class ResourceResolverTest {
         @Mock mockFirstDrawable: Drawable,
         @Mock mockSecondDrawable: Drawable,
         @StringForgery fakeFirstResourceId: String,
-        @StringForgery fakeSecondResourceId: String
+        @StringForgery fakeSecondResourceId: String,
+        @StringForgery fakeFirstKey: String,
+        @StringForgery fakeSecondKey: String
     ) {
         // Given
-        whenever(mockBitmapCachesManager.getFromResourceCache(mockFirstDrawable))
+        whenever(mockBitmapCachesManager.generateResourceKeyFromDrawable(mockFirstDrawable)).thenReturn(fakeFirstKey)
+        whenever(mockBitmapCachesManager.generateResourceKeyFromDrawable(mockSecondDrawable)).thenReturn(fakeSecondKey)
+        whenever(mockBitmapCachesManager.getFromResourceCache(fakeFirstKey))
             .thenReturn(fakeFirstResourceId)
-        whenever(mockBitmapCachesManager.getFromResourceCache(mockSecondDrawable))
+        whenever(mockBitmapCachesManager.getFromResourceCache(fakeSecondKey))
             .thenReturn(fakeSecondResourceId)
 
         val countDownLatch = CountDownLatch(2)

diff --git a/...in/com/datadog/android/sessionreplay/internal/recorder/resources/ResourcesLRUCacheTest.kt b/...in/com/datadog/android/sessionreplay/internal/recorder/resources/ResourcesLRUCacheTest.kt
@@ -47,6 +47,9 @@ internal class ResourcesLRUCacheTest {
     @Mock
     lateinit var mockInvocationUtils: InvocationUtils
 
+    @StringForgery
+    lateinit var fakeResourceKey: String
+
     val argumentCaptor = argumentCaptor<String>()
 
     @BeforeEach
@@ -61,7 +64,7 @@ internal class ResourcesLRUCacheTest {
     @Test
     fun `M return null W get() { item not in cache }`() {
         // When
-        val cacheItem = testedCache.get(mockDrawable)
+        val cacheItem = testedCache.get(fakeResourceKey)
 
         // Then
         assertThat(cacheItem).isNull()
@@ -73,57 +76,47 @@ internal class ResourcesLRUCacheTest {
     ) {
         // Given
         val fakeResourceIdByteArray = fakeResourceId.toByteArray(Charsets.UTF_8)
-        testedCache.put(mockDrawable, fakeResourceIdByteArray)
+        testedCache.put(fakeResourceKey, fakeResourceIdByteArray)
 
         // When
-        val cacheItem = testedCache.get(mockDrawable)
+        val cacheItem = testedCache.get(fakeResourceKey)
 
         // Then
         assertThat(cacheItem).isEqualTo(fakeResourceIdByteArray)
     }
 
     @Test
-    fun `M not generate prefix W put() { animationDrawable }`(
-        @StringForgery fakeResourceId: String
-    ) {
+    fun `M not generate prefix W put() { animationDrawable }`() {
         // Given
-        val fakeResourceIdByteArray = fakeResourceId.toByteArray(Charsets.UTF_8)
         val mockAnimationDrawable: AnimationDrawable = mock()
 
         // When
-        testedCache.put(mockAnimationDrawable, fakeResourceIdByteArray)
+        val key = testedCache.generateKeyFromDrawable(mockAnimationDrawable)
 
         // Then
-        val key = testedCache.generateKey(mockAnimationDrawable)
         assertThat(key).doesNotContain("-")
     }
 
     @Test
     fun `M generate key prefix with state W put() { drawableContainer }`(
-        @StringForgery fakeResourceId: String,
         forge: Forge
     ) {
         // Given
-        val fakeResourceIdByteArray = fakeResourceId.toByteArray(Charsets.UTF_8)
         val mockStatelistDrawable: StateListDrawable = mock()
         val fakeStateArray = intArrayOf(forge.aPositiveInt())
         val expectedPrefix = fakeStateArray[0].toString() + "-"
         whenever(mockStatelistDrawable.state).thenReturn(fakeStateArray)
 
         // When
-        testedCache.put(mockStatelistDrawable, fakeResourceIdByteArray)
+        val key = testedCache.generateKeyFromDrawable(mockStatelistDrawable)
 
         // Then
-        val key = testedCache.generateKey(mockStatelistDrawable)
         assertThat(key).startsWith(expectedPrefix)
     }
 
     @Test
-    fun `M generate key prefix with layer hash W put() { layerDrawable }`(
-        @StringForgery fakeResourceId: String
-    ) {
+    fun `M generate key prefix with layer hash W put() { layerDrawable }`() {
         // Given
-        val fakeResourceIdByteArray = fakeResourceId.toByteArray(Charsets.UTF_8)
         val mockRippleDrawable: RippleDrawable = mock()
         val mockBackgroundLayer: Drawable = mock()
         val mockForegroundLayer: Drawable = mock()
@@ -134,36 +127,31 @@ internal class ResourcesLRUCacheTest {
             .thenReturn(mockForegroundLayer)
         whenever(mockRippleDrawable.numberOfLayers).thenReturn(2)
 
-        testedCache.put(mockRippleDrawable, fakeResourceIdByteArray)
-
         val expectedPrefix = System.identityHashCode(mockBackgroundLayer).toString() + "-" +
             System.identityHashCode(mockForegroundLayer).toString() + "-"
         val expectedHash = System.identityHashCode(mockRippleDrawable).toString()
 
         // When
-        val key = testedCache.generateKey(mockRippleDrawable)
+        val key = testedCache.generateKeyFromDrawable(mockRippleDrawable)
 
         // Then
         assertThat(key).isEqualTo(expectedPrefix + expectedHash)
     }
 
     @Test
     fun `M not generate key prefix W put() { layerDrawable with only one layer }`(
-        @StringForgery fakeResourceId: String,
         @Mock mockRippleDrawable: RippleDrawable,
         @Mock mockBackgroundLayer: Drawable
     ) {
         // Given
-        val fakeResourceIdByteArray = fakeResourceId.toByteArray(Charsets.UTF_8)
         whenever(mockRippleDrawable.numberOfLayers).thenReturn(1)
         whenever(mockRippleDrawable.safeGetDrawable(0)).thenReturn(mockBackgroundLayer)
-        testedCache.put(mockRippleDrawable, fakeResourceIdByteArray)
 
         val expectedPrefix = System.identityHashCode(mockBackgroundLayer).toString() + "-"
         val drawableHash = System.identityHashCode(mockRippleDrawable).toString()
 
         // When
-        val key = testedCache.generateKey(mockRippleDrawable)
+        val key = testedCache.generateKeyFromDrawable(mockRippleDrawable)
 
         // Then
         assertThat(key).isEqualTo(expectedPrefix + drawableHash)