Skip to content

Commit

Permalink
RUM-6195: Allow ResourcesLruCache to work with any key
Browse files Browse the repository at this point in the history
  • Loading branch information
jonathanmos committed Nov 28, 2024
1 parent 4590058 commit d4f5994
Show file tree
Hide file tree
Showing 7 changed files with 56 additions and 53 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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 generateResourceLruKey(drawable: Drawable): String? {
return (resourcesLRUCache as? ResourcesLRUCache)?.generateKey(drawable)
}

internal fun putInBitmapPool(bitmap: Bitmap) {
bitmapPool.put(bitmap)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,9 @@ 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 put(key: String, value: V) {}
fun get(element: K): V? = null
fun get(key: String): V? = null
fun size(): Int
fun clear()

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -233,7 +233,8 @@ internal class ResourceResolver(
bitmapCachesManager.putInBitmapPool(bitmap)
}

bitmapCachesManager.putInResourceCache(drawable, resourceId)
val key = bitmapCachesManager.generateResourceLruKey(drawable) ?: return
bitmapCachesManager.putInResourceCache(key, resourceId)
}

@WorkerThread
Expand Down Expand Up @@ -321,7 +322,10 @@ internal class ResourceResolver(

private fun tryToGetResourceFromCache(
drawable: Drawable
): String? = bitmapCachesManager.getFromResourceCache(drawable)
): String? {
val key = bitmapCachesManager.generateResourceLruKey(drawable) ?: return null
return bitmapCachesManager.getFromResourceCache(key)
}

private fun shouldUseDrawableBitmap(drawable: BitmapDrawable): Boolean {
return drawable.bitmap != null &&
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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) },
Expand All @@ -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
)
Expand All @@ -76,9 +73,8 @@ internal class ResourcesLRUCache(
)
}

@VisibleForTesting
internal fun generateKey(drawable: Drawable): String =
generatePrefix(drawable) + System.identityHashCode(drawable)
internal fun generateKey(element: Drawable): String =
generatePrefix(element) + System.identityHashCode(element)

private fun generatePrefix(drawable: Drawable): String {
return when (drawable) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -58,8 +59,13 @@ internal class BitmapCachesManagerTest {
@StringForgery
lateinit var fakeResourceId: String

@StringForgery
lateinit var fakeResourceKey: String

@BeforeEach
fun `set up`() {
whenever(mockResourcesCache.generateKey(any())).thenReturn(fakeResourceId)

testedCachesManager = createBitmapCachesManager(
bitmapPool = mockBitmapPool,
resourcesLRUCache = mockResourcesCache,
Expand Down Expand Up @@ -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)
Expand All @@ -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()
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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

Expand All @@ -128,6 +127,7 @@ internal class ResourceResolverTest {
whenever(mockDrawableCopier.copy(eq(mockBitmapDrawable), any())).thenReturn(
mockBitmapDrawable
)
whenever(mockBitmapCachesManager.generateResourceLruKey(mockDrawable)).thenReturn(fakeResourceKey)
whenever(mockDrawableCopier.copy(eq(mockDrawable), any())).thenReturn(mockDrawable)
fakeImageCompressionByteArray = forge.aString().toByteArray()

Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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()))
Expand Down Expand Up @@ -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)

Expand Down Expand Up @@ -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.generateResourceLruKey(mockFirstDrawable)).thenReturn(fakeFirstKey)
whenever(mockBitmapCachesManager.generateResourceLruKey(mockSecondDrawable)).thenReturn(fakeSecondKey)
whenever(mockBitmapCachesManager.getFromResourceCache(fakeFirstKey))
.thenReturn(fakeFirstResourceId)
whenever(mockBitmapCachesManager.getFromResourceCache(mockSecondDrawable))
whenever(mockBitmapCachesManager.getFromResourceCache(fakeSecondKey))
.thenReturn(fakeSecondResourceId)

val countDownLatch = CountDownLatch(2)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,9 @@ internal class ResourcesLRUCacheTest {
@Mock
lateinit var mockInvocationUtils: InvocationUtils

@StringForgery
lateinit var fakeResourceKey: String

val argumentCaptor = argumentCaptor<String>()

@BeforeEach
Expand All @@ -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.generateKey(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.generateKey(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()
Expand All @@ -134,8 +127,6 @@ 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()
Expand All @@ -149,15 +140,12 @@ internal class ResourcesLRUCacheTest {

@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()
Expand Down

0 comments on commit d4f5994

Please sign in to comment.