Cache targetingData as a JSON string

android_sdk/src/main/java/co/optable/sdk/core/LocalStorage.kt

@@ -11,6 +11,8 @@ import androidx.preference.PreferenceManager
 import co.optable.sdk.OptableConfig
 import co.optable.sdk.OptableTargeting
 import com.google.gson.Gson
+import com.google.gson.annotations.SerializedName
+import org.json.JSONObject
 
 /**
  * Manages SharedPreferences for Optable SDK.
@@ -50,7 +52,7 @@ internal class LocalStorage(
                 clearTargeting()
                 return null
             }
-            return Gson().fromJson(targeting, OptableTargeting::class.java)
+            return Gson().fromJson(targeting, CachedTargeting::class.java).toTargeting()
         } catch (e: Exception) {
             Log.e("OptableSDK", "Can't parse cached targeting: ${e.message}. Clearing...")
             runCatching { clearTargeting() }
@@ -61,7 +63,7 @@ internal class LocalStorage(
     fun setTargeting(targeting: OptableTargeting) {
         try {
             prefs.edit(commit = true) {
-                putString(targetingKey, Gson().toJson(targeting))
+                putString(targetingKey, Gson().toJson(CachedTargeting.from(targeting)))
                 putLong(targetingTimestampKey, System.currentTimeMillis())
             }
         } catch (e: Exception) {
@@ -133,4 +135,24 @@ internal class LocalStorage(
         return "OPTABLE_" + kind + "_" + Base64.encodeToString(sfx.toByteArray(), 0)
     }
 
+    /**
+     * The cached form of [OptableTargeting]. Gson can't serialize a JSONObject (it writes the
+     * object's internal fields), so `targetingData` is stored as its JSON string.
+     */
+    private class CachedTargeting(
+        @SerializedName("gamTargetingKeywords") val gamTargetingKeywords: Map<String, List<String>>,
+        @SerializedName("openRtbJson") val openRtbJson: String?,
+        @SerializedName("targetingData") val targetingData: String,
+    ) {
+        fun toTargeting() = OptableTargeting(gamTargetingKeywords, openRtbJson, JSONObject(targetingData))
+
+        companion object {
+            fun from(targeting: OptableTargeting) = CachedTargeting(
+                targeting.gamTargetingKeywords,
+                targeting.openRtbJson,
+                targeting.targetingData.toString(),
+            )
+        }
+    }
+
 }

android_sdk/src/test/java/co/optable/sdk/core/LocalStorageTest.kt

@@ -7,7 +7,6 @@ import android.util.Log
 import androidx.preference.PreferenceManager
 import co.optable.sdk.OptableConfig
 import co.optable.sdk.OptableTargeting
-import com.google.gson.Gson
 import io.mockk.*
 import org.json.JSONObject
 import org.junit.After
@@ -40,6 +39,8 @@ class LocalStorageTest {
         mockContext = mockk(relaxed = true)
 
         mockkStatic(PreferenceManager::class, Base64::class, Log::class)
+        // JSONObject is a stub in local unit tests; getTargeting() rebuilds targetingData with its constructor.
+        mockkConstructor(JSONObject::class)
         every { Log.e(any(), any()) } returns 0
         every { Log.i(any(), any()) } returns 0
 
@@ -107,12 +108,10 @@ class LocalStorageTest {
     @Test
     fun `setTargeting and getTargeting should store and retrieve the value`() {
         val targetingKey = getPrivateKey(localStorage, "targetingKey")
-        val targetingJson = Gson().toJson(targeting)
+        val targetingJson = cachedTargetingJson(localStorage)
         every { mockSharedPreferences.getString(targetingKey, null) } returns targetingJson
         cacheTimestamp(localStorage, System.currentTimeMillis() - HOUR_MS)
 
-        localStorage.setTargeting(targeting)
-
         verify { mockEditor.putString(targetingKey, targetingJson) }
         val retrieved = localStorage.getTargeting()?.gamTargetingKeywords
         assertEquals(targeting.gamTargetingKeywords, retrieved)
@@ -144,7 +143,7 @@ class LocalStorageTest {
     fun `getTargeting should return null and clear the cache when the data is older than the TTL`() {
         val targetingKey = getPrivateKey(localStorage, "targetingKey")
         val timestampKey = getPrivateKey(localStorage, "targetingTimestampKey")
-        every { mockSharedPreferences.getString(targetingKey, null) } returns Gson().toJson(targeting)
+        every { mockSharedPreferences.getString(targetingKey, null) } returns cachedTargetingJson(localStorage)
         // The default TTL is 24 hours.
         cacheTimestamp(localStorage, System.currentTimeMillis() - 25 * HOUR_MS)
 
@@ -158,7 +157,7 @@ class LocalStorageTest {
     fun `getTargeting should return null and clear the cache when the timestamp is in the future`() {
         val targetingKey = getPrivateKey(localStorage, "targetingKey")
         val timestampKey = getPrivateKey(localStorage, "targetingTimestampKey")
-        every { mockSharedPreferences.getString(targetingKey, null) } returns Gson().toJson(targeting)
+        every { mockSharedPreferences.getString(targetingKey, null) } returns cachedTargetingJson(localStorage)
         // A device clock that moved backwards leaves the entry stamped in the future, making its age unusable.
         cacheTimestamp(localStorage, System.currentTimeMillis() + HOUR_MS)
 
@@ -171,7 +170,7 @@ class LocalStorageTest {
     @Test
     fun `getTargeting should return null when the cached data has no timestamp`() {
         val targetingKey = getPrivateKey(localStorage, "targetingKey")
-        every { mockSharedPreferences.getString(targetingKey, null) } returns Gson().toJson(targeting)
+        every { mockSharedPreferences.getString(targetingKey, null) } returns cachedTargetingJson(localStorage)
         cacheTimestamp(localStorage, 0L)
 
         assertNull(localStorage.getTargeting())
@@ -186,7 +185,7 @@ class LocalStorageTest {
             OptableConfig(mockContext, "testhost", "testtenant", "testslug", cacheTtl = ttlSeconds)
         )
         val targetingKey = getPrivateKey(storage, "targetingKey")
-        every { mockSharedPreferences.getString(targetingKey, null) } returns Gson().toJson(targeting)
+        every { mockSharedPreferences.getString(targetingKey, null) } returns cachedTargetingJson(storage)
 
         cacheTimestamp(storage, System.currentTimeMillis() - HOUR_MS)
         assertEquals(targeting.gamTargetingKeywords, storage.getTargeting()?.gamTargetingKeywords)
@@ -201,7 +200,7 @@ class LocalStorageTest {
             OptableConfig(mockContext, "testhost", "testtenant", "testslug", cacheTtl = Long.MAX_VALUE)
         )
         val targetingKey = getPrivateKey(storage, "targetingKey")
-        every { mockSharedPreferences.getString(targetingKey, null) } returns Gson().toJson(targeting)
+        every { mockSharedPreferences.getString(targetingKey, null) } returns cachedTargetingJson(storage)
         cacheTimestamp(storage, System.currentTimeMillis() - HOUR_MS)
 
         assertEquals(targeting.gamTargetingKeywords, storage.getTargeting()?.gamTargetingKeywords)
@@ -212,6 +211,27 @@ class LocalStorageTest {
         every { mockSharedPreferences.getLong(timestampKey, 0L) } returns timestamp
     }
 
+    /** Caches [targeting] with [storage] and returns the string it wrote to SharedPreferences. */
+    private fun cachedTargetingJson(storage: LocalStorage): String {
+        val written = slot<String>()
+        every { mockEditor.putString(getPrivateKey(storage, "targetingKey"), capture(written)) } returns mockEditor
+        storage.setTargeting(targeting)
+        return written.captured
+    }
+
+    @Test
+    fun `getTargeting should return null and clear the cache when the data is in the pre-1_3 format`() {
+        val targetingKey = getPrivateKey(localStorage, "targetingKey")
+        // Versions up to 1.2 stored the JSONObject's internal fields instead of its JSON string.
+        every { mockSharedPreferences.getString(targetingKey, null) } returns
+            """{"gamTargetingKeywords":{"foo":["bar"]},"openRtbJson":"{}","targetingData":{"nameValuePairs":{"foo":"bar"}}}"""
+        cacheTimestamp(localStorage, System.currentTimeMillis() - HOUR_MS)
+
+        assertNull(localStorage.getTargeting())
+
+        verify { mockEditor.remove(targetingKey) }
+    }
+
     @Test
     fun `getId5Signature should return null when not set`() {
         val id5SignatureKey = getPrivateKey(localStorage, "id5SignatureKey")