Migrate kvstore to kotlin (#5973)

* Get good test around the basic KvStore before starting conversion

* Converted BasicKvStore to kotlin and removed dead code

* Converted JsonKvStore to kotlin

---------

Co-authored-by: Nicolas Raoul <nicolas.raoul@gmail.com>
This commit is contained in:
Paul Hawke 2024-11-28 23:01:29 -06:00 committed by GitHub
parent d6c4cab207
commit dac3657536
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
10 changed files with 562 additions and 320 deletions

View file

@ -62,7 +62,7 @@ class SessionManager @Inject constructor(
fun forceLogin(context: Context?) =
context?.let { LoginActivity.startYourself(it) }
fun getPreference(key: String?): Boolean =
fun getPreference(key: String): Boolean =
defaultKvStore.getBoolean(key)
fun logout(): Completable = Completable.fromObservable(

View file

@ -1,215 +0,0 @@
package fr.free.nrw.commons.kvstore;
import android.content.Context;
import android.content.SharedPreferences;
import androidx.annotation.Nullable;
import java.util.HashMap;
import java.util.HashSet;
import java.util.Map;
import java.util.Set;
import timber.log.Timber;
public class BasicKvStore implements KeyValueStore {
private static final String KEY_VERSION = "__version__";
/*
This class only performs puts, sets and clears.
A commit returns a boolean indicating whether it has succeeded, we are not throwing an exception as it will
require the dev to handle it in every usage - instead we will pass on this boolean so it can be evaluated if needed.
*/
private final SharedPreferences _store;
public BasicKvStore(Context context, String storeName) {
_store = context.getSharedPreferences(storeName, Context.MODE_PRIVATE);
}
/**
* If you don't want onVersionUpdate to be called on a fresh creation, the first version supplied for the kvstore should be set to 0.
*/
public BasicKvStore(Context context, String storeName, int version) {
this(context,storeName,version,false);
}
public BasicKvStore(Context context, String storeName, int version, boolean clearAllOnUpgrade) {
_store = context.getSharedPreferences(storeName, Context.MODE_PRIVATE);
int oldVersion = getInt(KEY_VERSION);
if (version > oldVersion) {
Timber.i("version updated from %s to %s, with clearFlag %b", oldVersion, version, clearAllOnUpgrade);
onVersionUpdate(oldVersion, version, clearAllOnUpgrade);
}
if (version < oldVersion) {
throw new IllegalArgumentException(
"kvstore downgrade not allowed, old version:" + oldVersion + ", new version: " +
version);
}
//Keep this statement at the end so that clearing of store does not cause version also to get removed.
putIntInternal(KEY_VERSION, version);
}
public void onVersionUpdate(int oldVersion, int version, boolean clearAllFlag) {
if(clearAllFlag) {
clearAll();
}
}
public Set<String> getKeySet() {
Map<String, ?> allContents = new HashMap<>(_store.getAll());
allContents.remove(KEY_VERSION);
return allContents.keySet();
}
@Nullable
public Map<String, ?> getAll() {
Map<String, ?> allContents = _store.getAll();
if (allContents == null || allContents.size() == 0) {
return null;
}
allContents.remove(KEY_VERSION);
return new HashMap<>(allContents);
}
@Override
public String getString(String key) {
return getString(key, null);
}
@Override
public boolean getBoolean(String key) {
return getBoolean(key, false);
}
@Override
public long getLong(String key) {
return getLong(key, 0);
}
@Override
public int getInt(String key) {
return getInt(key, 0);
}
@Override
public String getString(String key, String defaultValue) {
return _store.getString(key, defaultValue);
}
@Override
public boolean getBoolean(String key, boolean defaultValue) {
return _store.getBoolean(key, defaultValue);
}
@Override
public long getLong(String key, long defaultValue) {
return _store.getLong(key, defaultValue);
}
@Override
public int getInt(String key, int defaultValue) {
return _store.getInt(key, defaultValue);
}
public void putAllStrings(Map<String, String> keyValuePairs) {
SharedPreferences.Editor editor = _store.edit();
for (Map.Entry<String, String> keyValuePair : keyValuePairs.entrySet()) {
putString(editor, keyValuePair.getKey(), keyValuePair.getValue(), false);
}
editor.apply();
}
@Override
public void putString(String key, String value) {
SharedPreferences.Editor editor = _store.edit();
putString(editor, key, value, true);
}
private void putString(SharedPreferences.Editor editor, String key, String value,
boolean commit) {
assertKeyNotReserved(key);
editor.putString(key, value);
if(commit) {
editor.apply();
}
}
@Override
public void putBoolean(String key, boolean value) {
assertKeyNotReserved(key);
SharedPreferences.Editor editor = _store.edit();
editor.putBoolean(key, value);
editor.apply();
}
@Override
public void putLong(String key, long value) {
assertKeyNotReserved(key);
SharedPreferences.Editor editor = _store.edit();
editor.putLong(key, value);
editor.apply();
}
@Override
public void putInt(String key, int value) {
assertKeyNotReserved(key);
putIntInternal(key, value);
}
@Override
public boolean contains(String key) {
return _store.contains(key);
}
@Override
public void remove(String key) {
SharedPreferences.Editor editor = _store.edit();
editor.remove(key);
editor.apply();
}
@Override
public void clearAll() {
int version = getInt(KEY_VERSION);
SharedPreferences.Editor editor = _store.edit();
editor.clear();
editor.apply();
putIntInternal(KEY_VERSION, version);
}
@Override
public void clearAllWithVersion() {
SharedPreferences.Editor editor = _store.edit();
editor.clear();
editor.apply();
}
private void putIntInternal(String key, int value) {
SharedPreferences.Editor editor = _store.edit();
editor.putInt(key, value);
editor.apply();
}
private void assertKeyNotReserved(String key) {
if (key.equals(KEY_VERSION)) {
throw new IllegalArgumentException(key + "is a reserved key");
}
}
public void registerChangeListener(SharedPreferences.OnSharedPreferenceChangeListener l) {
_store.registerOnSharedPreferenceChangeListener(l);
}
public void unregisterChangeListener(SharedPreferences.OnSharedPreferenceChangeListener l) {
_store.unregisterOnSharedPreferenceChangeListener(l);
}
public Set<String> getStringSet(String key){
return _store.getStringSet(key, new HashSet<>());
}
public void putStringSet(String key,Set<String> value){
_store.edit().putStringSet(key,value).apply();
}
}

View file

@ -0,0 +1,152 @@
package fr.free.nrw.commons.kvstore
import android.content.Context
import android.content.SharedPreferences
import androidx.annotation.VisibleForTesting
import androidx.core.content.edit
import timber.log.Timber
open class BasicKvStore : KeyValueStore {
/*
This class only performs puts, sets and clears.
A commit returns a boolean indicating whether it has succeeded, we are not throwing an exception as it will
require the dev to handle it in every usage - instead we will pass on this boolean so it can be evaluated if needed.
*/
private val _store: SharedPreferences
constructor(context: Context, storeName: String?) {
_store = context.getSharedPreferences(storeName, Context.MODE_PRIVATE)
}
/**
* If you don't want onVersionUpdate to be called on a fresh creation, the first version supplied for the kvstore should be set to 0.
*/
@JvmOverloads
constructor(
context: Context,
storeName: String?,
version: Int,
clearAllOnUpgrade: Boolean = false
) {
_store = context.getSharedPreferences(storeName, Context.MODE_PRIVATE)
val oldVersion = _store.getInt(KEY_VERSION, 0)
require(version >= oldVersion) {
"kvstore downgrade not allowed, old version:" + oldVersion + ", new version: " +
version
}
if (version > oldVersion) {
Timber.i(
"version updated from %s to %s, with clearFlag %b",
oldVersion,
version,
clearAllOnUpgrade
)
onVersionUpdate(oldVersion, version, clearAllOnUpgrade)
}
//Keep this statement at the end so that clearing of store does not cause version also to get removed.
_store.edit { putInt(KEY_VERSION, version) }
}
val all: Map<String, *>?
get() {
val allContents = _store.all
if (allContents == null || allContents.isEmpty()) {
return null
}
allContents.remove(KEY_VERSION)
return HashMap(allContents)
}
override fun getString(key: String): String? =
getString(key, null)
override fun getBoolean(key: String): Boolean =
getBoolean(key, false)
override fun getLong(key: String): Long =
getLong(key, 0)
override fun getInt(key: String): Int =
getInt(key, 0)
fun getStringSet(key: String?): MutableSet<String> =
_store.getStringSet(key, HashSet())!!
override fun getString(key: String, defaultValue: String?): String? =
_store.getString(key, defaultValue)
override fun getBoolean(key: String, defaultValue: Boolean): Boolean =
_store.getBoolean(key, defaultValue)
override fun getLong(key: String, defaultValue: Long): Long =
_store.getLong(key, defaultValue)
override fun getInt(key: String, defaultValue: Int): Int =
_store.getInt(key, defaultValue)
fun putAllStrings(kvData: Map<String, String>) = assertKeyNotReserved(kvData.keys) {
for ((key, value) in kvData) {
putString(key, value)
}
}
override fun putString(key: String, value: String) = assertKeyNotReserved(key) {
putString(key, value)
}
override fun putBoolean(key: String, value: Boolean) = assertKeyNotReserved(key) {
putBoolean(key, value)
}
override fun putLong(key: String, value: Long) = assertKeyNotReserved(key) {
putLong(key, value)
}
override fun putInt(key: String, value: Int) = assertKeyNotReserved(key) {
putInt(key, value)
}
fun putStringSet(key: String?, value: Set<String?>?) =
_store.edit{ putStringSet(key, value) }
override fun remove(key: String) = assertKeyNotReserved(key) {
remove(key)
}
override fun contains(key: String): Boolean {
if (key == KEY_VERSION) return false
return _store.contains(key)
}
override fun clearAll() {
val version = _store.getInt(KEY_VERSION, 0)
_store.edit {
clear()
putInt(KEY_VERSION, version)
}
}
private fun onVersionUpdate(oldVersion: Int, version: Int, clearAllFlag: Boolean) {
if (clearAllFlag) {
clearAll()
}
}
protected fun assertKeyNotReserved(key: Set<String>, block: SharedPreferences.Editor.() -> Unit) {
key.forEach { require(it != KEY_VERSION) { "$it is a reserved key" } }
_store.edit { block(this) }
}
protected fun assertKeyNotReserved(key: String, block: SharedPreferences.Editor.() -> Unit) {
require(key != KEY_VERSION) { "$key is a reserved key" }
_store.edit { block(this) }
}
companion object {
@VisibleForTesting
const val KEY_VERSION: String = "__version__"
}
}

View file

@ -1,68 +0,0 @@
package fr.free.nrw.commons.kvstore;
import android.content.Context;
import androidx.annotation.Nullable;
import com.google.gson.Gson;
import com.google.gson.JsonSyntaxException;
import java.lang.reflect.Type;
import java.util.HashMap;
import java.util.Map;
public class JsonKvStore extends BasicKvStore {
private final Gson gson;
public JsonKvStore(Context context, String storeName, Gson gson) {
super(context, storeName);
this.gson = gson;
}
public JsonKvStore(Context context, String storeName, int version, Gson gson) {
super(context, storeName, version);
this.gson = gson;
}
public JsonKvStore(Context context, String storeName, int version, boolean clearAllOnUpgrade, Gson gson) {
super(context, storeName, version, clearAllOnUpgrade);
this.gson = gson;
}
public <T> void putAllJsons(Map<String, T> jsonMap) {
Map<String, String> stringsMap = new HashMap<>(jsonMap.size());
for (Map.Entry<String, T> keyValuePair : jsonMap.entrySet()) {
String jsonString = gson.toJson(keyValuePair.getValue());
stringsMap.put(keyValuePair.getKey(), jsonString);
}
putAllStrings(stringsMap);
}
public <T> void putJson(String key, T object) {
putString(key, gson.toJson(object));
}
public <T> void putJsonWithTypeInfo(String key, T object, Type type) {
putString(key, gson.toJson(object, type));
}
@Nullable
public <T> T getJson(String key, Class<T> clazz) {
String jsonString = getString(key);
try {
return gson.fromJson(jsonString, clazz);
} catch (JsonSyntaxException e) {
return null;
}
}
@Nullable
public <T> T getJson(String key, Type type) {
String jsonString = getString(key);
try {
return gson.fromJson(jsonString, type);
} catch (JsonSyntaxException e) {
return null;
}
}
}

View file

@ -0,0 +1,52 @@
package fr.free.nrw.commons.kvstore
import android.content.Context
import com.google.gson.Gson
import com.google.gson.JsonSyntaxException
class JsonKvStore : BasicKvStore {
val gson: Gson
constructor(context: Context, storeName: String?, gson: Gson) : super(context, storeName) {
this.gson = gson
}
constructor(context: Context, storeName: String?, version: Int, gson: Gson) : super(
context, storeName, version
) {
this.gson = gson
}
constructor(
context: Context,
storeName: String?,
version: Int,
clearAllOnUpgrade: Boolean,
gson: Gson
) : super(context, storeName, version, clearAllOnUpgrade) {
this.gson = gson
}
fun <T> putJson(key: String, value: T) = assertKeyNotReserved(key) {
putString(key, gson.toJson(value))
}
@Deprecated(
message = "Migrate to newer Kotlin syntax",
replaceWith = ReplaceWith("getJson<T>(key)")
)
fun <T> getJson(key: String, clazz: Class<T>?): T? = try {
gson.fromJson(getString(key), clazz)
} catch (e: JsonSyntaxException) {
null
}
// Later, when the calls are coming from Kotlin, this will allow us to
// drop the "clazz" parameter, and just pick up the type at the call site.
// The deprecation warning should help migration!
inline fun <reified T> getJson(key: String): T? = try {
gson.fromJson(getString(key), T::class.java)
} catch (e: JsonSyntaxException) {
null
}
}

View file

@ -1,35 +0,0 @@
package fr.free.nrw.commons.kvstore;
public interface KeyValueStore {
String getString(String key);
boolean getBoolean(String key);
long getLong(String key);
int getInt(String key);
String getString(String key, String defaultValue);
boolean getBoolean(String key, boolean defaultValue);
long getLong(String key, long defaultValue);
int getInt(String key, int defaultValue);
void putString(String key, String value);
void putBoolean(String key, boolean value);
void putLong(String key, long value);
void putInt(String key, int value);
boolean contains(String key);
void remove(String key);
void clearAll();
void clearAllWithVersion();
}

View file

@ -0,0 +1,33 @@
package fr.free.nrw.commons.kvstore
interface KeyValueStore {
fun getString(key: String): String?
fun getBoolean(key: String): Boolean
fun getLong(key: String): Long
fun getInt(key: String): Int
fun getString(key: String, defaultValue: String?): String?
fun getBoolean(key: String, defaultValue: Boolean): Boolean
fun getLong(key: String, defaultValue: Long): Long
fun getInt(key: String, defaultValue: Int): Int
fun putString(key: String, value: String)
fun putBoolean(key: String, value: Boolean)
fun putLong(key: String, value: Long)
fun putInt(key: String, value: Int)
fun contains(key: String): Boolean
fun remove(key: String)
fun clearAll()
}

View file

@ -46,7 +46,7 @@ class SystemThemeUtils @Inject constructor(
// Returns true if the device is in night mode or false otherwise
fun isDeviceInNightMode(): Boolean {
return getSystemDefaultThemeBool(
applicationKvStore.getString(Prefs.KEY_THEME_VALUE, getSystemDefaultTheme())
applicationKvStore.getString(Prefs.KEY_THEME_VALUE, getSystemDefaultTheme())!!
)
}
}

View file

@ -0,0 +1,238 @@
package fr.free.nrw.commons.kvstore
import android.content.Context
import android.content.SharedPreferences
import com.nhaarman.mockitokotlin2.atLeast
import com.nhaarman.mockitokotlin2.verify
import com.nhaarman.mockitokotlin2.whenever
import fr.free.nrw.commons.kvstore.BasicKvStore.Companion.KEY_VERSION
import org.junit.Assert
import org.junit.Before
import org.junit.Test
import org.mockito.ArgumentMatchers.anyInt
import org.mockito.ArgumentMatchers.anyString
import org.mockito.Mockito.mock
class BasicKvStoreTest {
private val context = mock<Context>()
private val prefs = mock<SharedPreferences>()
private val editor = mock<SharedPreferences.Editor>()
private lateinit var store: BasicKvStore
@Before
fun setUp() {
whenever(context.getSharedPreferences(anyString(), anyInt())).thenReturn(prefs)
whenever(prefs.edit()).thenReturn(editor)
store = BasicKvStore(context, "name")
}
@Test
fun versionUpdate() {
whenever(prefs.getInt(KEY_VERSION, 0)).thenReturn(99)
BasicKvStore(context, "name", 100, true)
// It should clear itself and automatically put the new version number
verify(prefs, atLeast(2)).edit()
verify(editor).clear()
verify(editor).putInt(KEY_VERSION, 100)
verify(editor, atLeast(2)).apply()
}
@Test(expected = IllegalArgumentException::class)
fun versionDowngradeNotAllowed() {
whenever(prefs.getInt(KEY_VERSION, 0)).thenReturn(100)
BasicKvStore(context, "name", 99, true)
}
@Test
fun versionRedactedFromGetAll() {
val all = mutableMapOf("key" to "value", KEY_VERSION to 100)
whenever(prefs.all).thenReturn(all)
val result = store.all
Assert.assertEquals(mapOf("key" to "value"), result)
}
@Test
fun getAllHandlesNull() {
whenever(prefs.all).thenReturn(null)
Assert.assertNull(store.all)
}
@Test
fun getAllHandlesEmpty() {
whenever(prefs.all).thenReturn(emptyMap())
Assert.assertNull(store.all)
}
@Test
fun getString() {
whenever(prefs.getString("key", null)).thenReturn("value")
Assert.assertEquals("value", store.getString("key"))
}
@Test
fun getBoolean() {
whenever(prefs.getBoolean("key", false)).thenReturn(true)
Assert.assertTrue(store.getBoolean("key"))
}
@Test
fun getLong() {
whenever(prefs.getLong("key", 0L)).thenReturn(100)
Assert.assertEquals(100L, store.getLong("key"))
}
@Test
fun getInt() {
whenever(prefs.getInt("key", 0)).thenReturn(100)
Assert.assertEquals(100, store.getInt("key"))
}
@Test
fun getStringWithDefault() {
whenever(prefs.getString("key", "junk")).thenReturn("value")
Assert.assertEquals("value", store.getString("key", "junk"))
}
@Test
fun getBooleanWithDefault() {
whenever(prefs.getBoolean("key", true)).thenReturn(true)
Assert.assertTrue(store.getBoolean("key", true))
}
@Test
fun getLongWithDefault() {
whenever(prefs.getLong("key", 22L)).thenReturn(100)
Assert.assertEquals(100L, store.getLong("key", 22L))
}
@Test
fun getIntWithDefault() {
whenever(prefs.getInt("key", 22)).thenReturn(100)
Assert.assertEquals(100, store.getInt("key", 22))
}
@Test
fun putAllStrings() {
store.putAllStrings(
mapOf(
"one" to "fish",
"two" to "fish",
"red" to "fish",
"blue" to "fish"
)
)
verify(prefs).edit()
verify(editor).putString("one", "fish")
verify(editor).putString("two", "fish")
verify(editor).putString("red", "fish")
verify(editor).putString("blue", "fish")
verify(editor).apply()
}
@Test(expected = IllegalArgumentException::class)
fun putAllStringsWithReservedKey() {
store.putAllStrings(
mapOf(
"this" to "that",
KEY_VERSION to "something"
)
)
}
@Test
fun putString() {
store.putString("this" , "that")
verify(prefs).edit()
verify(editor).putString("this", "that")
verify(editor).apply()
}
@Test
fun putBoolean() {
store.putBoolean("this" , true)
verify(prefs).edit()
verify(editor).putBoolean("this", true)
verify(editor).apply()
}
@Test
fun putLong() {
store.putLong("this" , 123L)
verify(prefs).edit()
verify(editor).putLong("this", 123L)
verify(editor).apply()
}
@Test
fun putInt() {
store.putInt("this" , 16)
verify(prefs).edit()
verify(editor).putInt("this", 16)
verify(editor).apply()
}
@Test(expected = IllegalArgumentException::class)
fun putStringWithReservedKey() {
store.putString(KEY_VERSION, "that")
}
@Test(expected = IllegalArgumentException::class)
fun putBooleanWithReservedKey() {
store.putBoolean(KEY_VERSION, true)
}
@Test(expected = IllegalArgumentException::class)
fun putLongWithReservedKey() {
store.putLong(KEY_VERSION, 33L)
}
@Test(expected = IllegalArgumentException::class)
fun putIntWithReservedKey() {
store.putInt(KEY_VERSION, 12)
}
@Test
fun testContains() {
whenever(prefs.contains("key")).thenReturn(true)
Assert.assertTrue(store.contains("key"))
}
@Test
fun containsRedactsVersion() {
whenever(prefs.contains(KEY_VERSION)).thenReturn(true)
Assert.assertFalse(store.contains(KEY_VERSION))
}
@Test
fun remove() {
store.remove("key")
verify(prefs).edit()
verify(editor).remove("key")
verify(editor).apply()
}
@Test(expected = IllegalArgumentException::class)
fun removeWithReservedKey() {
store.remove(KEY_VERSION)
}
@Test
fun clearAllPreservesVersion() {
whenever(prefs.getInt(KEY_VERSION, 0)).thenReturn(99)
store.clearAll()
verify(prefs).edit()
verify(editor).clear()
verify(editor).putInt(KEY_VERSION, 99)
verify(editor).apply()
}
}

View file

@ -0,0 +1,85 @@
package fr.free.nrw.commons.kvstore
import android.content.Context
import android.content.SharedPreferences
import com.google.gson.Gson
import com.nhaarman.mockitokotlin2.atLeast
import com.nhaarman.mockitokotlin2.verify
import com.nhaarman.mockitokotlin2.whenever
import fr.free.nrw.commons.kvstore.BasicKvStore.Companion.KEY_VERSION
import org.junit.Assert
import org.junit.Before
import org.junit.Test
import org.mockito.ArgumentMatchers.anyInt
import org.mockito.ArgumentMatchers.anyString
import org.mockito.Mockito.mock
class JsonKvStoreTest {
private val context = mock<Context>()
private val prefs = mock<SharedPreferences>()
private val editor = mock<SharedPreferences.Editor>()
private val gson = Gson()
private val testData = Person(16, "Bob", true, Pet("Poodle", 2))
private val expected = gson.toJson(testData)
private lateinit var store: JsonKvStore
@Before
fun setUp() {
whenever(context.getSharedPreferences(anyString(), anyInt())).thenReturn(prefs)
whenever(prefs.edit()).thenReturn(editor)
store = JsonKvStore(context, "name", gson)
}
@Test
fun putJson() {
store.putJson("person", testData)
verify(prefs).edit()
verify(editor).putString("person", expected)
verify(editor).apply()
}
@Test(expected = IllegalArgumentException::class)
fun putJsonWithReservedKey() {
store.putJson(KEY_VERSION, testData)
}
@Test
fun getJson() {
whenever(prefs.getString("key", null)).thenReturn(expected)
val result = store.getJson("key", Person::class.java)
Assert.assertEquals(testData, result)
}
@Test
fun getJsonInTheFuture() {
whenever(prefs.getString("key", null)).thenReturn(expected)
val resultOne: Person? = store.getJson("key")
Assert.assertEquals(testData, resultOne)
val resultTwo = store.getJson<Person?>("key")
Assert.assertEquals(testData, resultTwo)
}
@Test
fun getJsonHandlesMalformedJson() {
whenever(prefs.getString("key", null)).thenReturn("junk")
val result = store.getJson("key", Person::class.java)
Assert.assertNull(result)
}
data class Person(
val age: Int, val name: String, val hasPets: Boolean, val pet: Pet?
)
data class Pet(
val breed: String, val age: Int
)
}