Merge pull request #1593 from bemusementpark/more-snodes

Optimise SnodeAPI and Add Snode.Version and tests

app/src/main/java/org/thoughtcrime/securesms/database/LokiAPIDatabase.kt

@@ -166,8 +166,6 @@ class LokiAPIDatabase(context: Context, helper: SQLCipherOpenHelper) : Database(
 
         const val RESET_SEQ_NO = "UPDATE $lastMessageServerIDTable SET $lastMessageServerID = 0;"
 
-        const val EMPTY_VERSION = "0.0.0"
-
         // endregion
     }
 
@@ -175,15 +173,7 @@ class LokiAPIDatabase(context: Context, helper: SQLCipherOpenHelper) : Database(
         val database = databaseHelper.readableDatabase
         return database.get(snodePoolTable, "${Companion.dummyKey} = ?", wrap("dummy_key")) { cursor ->
             val snodePoolAsString = cursor.getString(cursor.getColumnIndexOrThrow(snodePool))
-            snodePoolAsString.split(", ").mapNotNull { snodeAsString ->
+            snodePoolAsString.split(", ").mapNotNull(::Snode)
-                val components = snodeAsString.split("-")
-                val address = components[0]
-                val port = components.getOrNull(1)?.toIntOrNull() ?: return@mapNotNull null
-                val ed25519Key = components.getOrNull(2) ?: return@mapNotNull null
-                val x25519Key = components.getOrNull(3) ?: return@mapNotNull null
-                val version = components.getOrNull(4) ?: EMPTY_VERSION
-                Snode(address, port, Snode.KeySet(ed25519Key, x25519Key), version)
-            }
         }?.toSet() ?: setOf()
     }
 
@@ -231,18 +221,7 @@ class LokiAPIDatabase(context: Context, helper: SQLCipherOpenHelper) : Database(
         val database = databaseHelper.readableDatabase
         fun get(indexPath: String): Snode? {
             return database.get(onionRequestPathTable, "${Companion.indexPath} = ?", wrap(indexPath)) { cursor ->
-                val snodeAsString = cursor.getString(cursor.getColumnIndexOrThrow(snode))
+                Snode(cursor.getString(cursor.getColumnIndexOrThrow(snode)))
-                val components = snodeAsString.split("-")
-                val address = components[0]
-                val port = components.getOrNull(1)?.toIntOrNull()
-                val ed25519Key = components.getOrNull(2)
-                val x25519Key = components.getOrNull(3)
-                val version = components.getOrNull(4) ?: EMPTY_VERSION
-                if (port != null && ed25519Key != null && x25519Key != null) {
-                    Snode(address, port, Snode.KeySet(ed25519Key, x25519Key), version)
-                } else {
-                    null
-                }
             }
         }
         val result = mutableListOf<List<Snode>>()
@@ -276,15 +255,7 @@ class LokiAPIDatabase(context: Context, helper: SQLCipherOpenHelper) : Database(
         val database = databaseHelper.readableDatabase
         return database.get(swarmTable, "${Companion.swarmPublicKey} = ?", wrap(publicKey)) { cursor ->
             val swarmAsString = cursor.getString(cursor.getColumnIndexOrThrow(swarm))
-            swarmAsString.split(", ").mapNotNull { targetAsString ->
+            swarmAsString.split(", ").mapNotNull(::Snode)
-                val components = targetAsString.split("-")
-                val address = components[0]
-                val port = components.getOrNull(1)?.toIntOrNull() ?: return@mapNotNull null
-                val ed25519Key = components.getOrNull(2) ?: return@mapNotNull null
-                val x25519Key = components.getOrNull(3) ?: return@mapNotNull null
-                val version = components.getOrNull(4) ?: EMPTY_VERSION
-                Snode(address, port, Snode.KeySet(ed25519Key, x25519Key), version)
-            }
         }?.toSet()
     }
 

app/src/test/java/org/session/libsignal/utilities/SnodeTest.kt

@@ -0,0 +1,52 @@
+package org.session.libsignal.utilities
+
+import org.hamcrest.MatcherAssert.assertThat
+import org.hamcrest.core.IsEqual.equalTo
+import org.junit.Test
+import org.junit.runner.RunWith
+import org.junit.runners.Parameterized
+
+@RunWith(Parameterized::class)
+class SnodeVersionTest(
+    private val v1: String,
+    private val v2: String,
+    private val expectedEqual: Boolean,
+    private val expectedLessThan: Boolean
+) {
+    companion object {
+        @JvmStatic
+        @Parameterized.Parameters(name = "{index}: testVersion({0},{1}) = (equalTo: {2}, lessThan: {3})")
+        fun data(): Collection<Array<Any>> = listOf(
+            arrayOf("1", "1", true, false),
+            arrayOf("1", "2", false, true),
+            arrayOf("2", "1", false, false),
+            arrayOf("1.0", "1", true, false),
+            arrayOf("1.0", "1.0.0", true, false),
+            arrayOf("1.0", "1.0.0.0", true, false),
+            arrayOf("1.0", "1.0.0.0.0.0", true, false),
+            arrayOf("2.0", "1.2", false, false),
+            arrayOf("1.0.0.0", "1.0.0.1", false, true),
+            // Snode.Version only considers the first 4 integers, so these are equal
+            arrayOf("1.0.0.0", "1.0.0.0.1", true, false),
+            arrayOf("1.0.0.1", "1.0.0.1", true, false),
+            // parts can be up to 16 bits, around 65,535
+            arrayOf("65535.65535.65535.65535", "65535.65535.65535.65535", true, false),
+            // values higher than this are coerced to 65535 (:
+            arrayOf("65535.65535.65535.65535", "65535.65535.65535.99999", true, false),
+        )
+    }
+
+    @Test
+    fun testVersionEqual() {
+        val version1 = Snode.Version(v1)
+        val version2 = Snode.Version(v2)
+        assertThat(version1 == version2, equalTo(expectedEqual))
+    }
+
+    @Test
+    fun testVersionOnePartLessThan() {
+        val version1 = Snode.Version(v1)
+        val version2 = Snode.Version(v2)
+        assertThat(version1 < version2, equalTo(expectedLessThan))
+    }
+}

libsession/src/main/java/org/session/libsession/messaging/jobs/ConfigurationSyncJob.kt

@@ -63,7 +63,6 @@ data class ConfigurationSyncJob(val destination: Destination): Job {
             // return a list of batch request objects
             val snodeMessage = MessageSender.buildConfigMessageToSnode(destination.destinationPublicKey(), message)
             val authenticated = SnodeAPI.buildAuthenticatedStoreBatchInfo(
-                destination.destinationPublicKey(),
                 config.configNamespace(),
                 snodeMessage
             ) ?: return@map null // this entry will be null otherwise

libsession/src/main/java/org/session/libsession/messaging/sending_receiving/pollers/Poller.kt

@@ -140,8 +140,7 @@ class Poller(private val configFactory: ConfigFactoryProtocol, debounceTimer: Ti
         val messages = rawMessages["messages"] as? List<*>
         val processed = if (!messages.isNullOrEmpty()) {
             SnodeAPI.updateLastMessageHashValueIfPossible(snode, userPublicKey, messages, namespace)
-            SnodeAPI.removeDuplicates(userPublicKey, messages, namespace, true).mapNotNull { messageBody ->
+            SnodeAPI.removeDuplicates(userPublicKey, messages, namespace, true).mapNotNull { rawMessageAsJSON ->
-                val rawMessageAsJSON = messageBody as? Map<*, *> ?: return@mapNotNull null
                 val hashValue = rawMessageAsJSON["hash"] as? String ?: return@mapNotNull null
                 val b64EncodedBody = rawMessageAsJSON["data"] as? String ?: return@mapNotNull null
                 val timestamp = rawMessageAsJSON["t"] as? Long ?: SnodeAPI.nowWithOffset

libsession/src/main/java/org/session/libsession/utilities/Util.kt

@@ -358,34 +358,6 @@ object Util {
         val digitGroups = (Math.log10(sizeBytes.toDouble()) / Math.log10(1024.0)).toInt()
         return DecimalFormat("#,##0.#").format(sizeBytes / Math.pow(1024.0, digitGroups.toDouble())) + " " + units[digitGroups]
     }
-
-    /**
-     * Compares two version strings (for example "1.8.0")
-     *
-     * @param version1 the first version string to compare.
-     * @param version2 the second version string to compare.
-     * @return an integer indicating the result of the comparison:
-     *         - 0 if the versions are equal
-     *         - a positive number if version1 is greater than version2
-     *         - a negative number if version1 is less than version2
-     */
-    @JvmStatic
-    fun compareVersions(version1: String, version2: String): Int {
-        val parts1 = version1.split(".").map { it.toIntOrNull() ?: 0 }
-        val parts2 = version2.split(".").map { it.toIntOrNull() ?: 0 }
-
-        val maxLength = maxOf(parts1.size, parts2.size)
-        val paddedParts1 = parts1 + List(maxLength - parts1.size) { 0 }
-        val paddedParts2 = parts2 + List(maxLength - parts2.size) { 0 }
-
-        for (i in 0 until maxLength) {
-            val compare = paddedParts1[i].compareTo(paddedParts2[i])
-            if (compare != 0) {
-                return compare
-            }
-        }
-        return 0
-    }
 }
 
 fun <T, R> T.runIf(condition: Boolean, block: T.() -> R): R where T: R = if (condition) block() else this
@@ -422,6 +394,12 @@ fun <E, K: Any, V: Any> Iterable<E>.associateByNotNull(
     for(e in this) { it[keySelector(e) ?: continue] = valueTransform(e) ?: continue }
 }
 
+fun <K: Any, V: Any, W : Any> Map<K, V>.mapValuesNotNull(
+    valueTransform: (Map.Entry<K, V>) -> W?
+): Map<K, W> = mutableMapOf<K, W>().also {
+    for(e in this) { it[e.key] = valueTransform(e) ?: continue }
+}
+
 /**
  * Groups elements of the original collection by the key returned by the given [keySelector] function
  * applied to each element and returns a map where each group key is associated with a list of
@@ -432,3 +410,23 @@ fun <E, K: Any, V: Any> Iterable<E>.associateByNotNull(
 inline fun <E, K> Iterable<E>.groupByNotNull(keySelector: (E) -> K?): Map<K, List<E>> = LinkedHashMap<K, MutableList<E>>().also {
     forEach { e -> keySelector(e)?.let { k -> it.getOrPut(k) { mutableListOf() } += e } }
 }
+
+/**
+ * Analogous to [buildMap], this function creates a [MutableMap] and populates it using the given [action].
+ */
+inline fun <K, V> buildMutableMap(action: MutableMap<K, V>.() -> Unit): MutableMap<K, V> =
+    mutableMapOf<K, V>().apply(action)
+
+/**
+ * Converts a list of Pairs into a Map, filtering out any Pairs where the value is null.
+ *
+ * @param pairs The list of Pairs to convert.
+ * @return A Map with non-null values.
+ */
+fun <K : Any, V : Any> Iterable<Pair<K, V?>>.toMapNotNull(): Map<K, V> =
+    associateByNotNull(Pair<K, V?>::first, Pair<K, V?>::second)
+
+fun Sequence<String>.toByteArray(): ByteArray = ByteArrayOutputStream().use { output ->
+    forEach { it.byteInputStream().use { input -> input.copyTo(output) } }
+    output.toByteArray()
+}

libsignal/src/main/java/org/session/libsignal/utilities/Base64.java

@@ -1,5 +1,7 @@
 package org.session.libsignal.utilities;
 
+import androidx.annotation.NonNull;
+
 /**
  * <p>Encodes and decodes to and from Base64 notation.</p>
  * <p>Homepage: <a href="http://iharder.net/base64">http://iharder.net/base64</a>.</p>
@@ -714,7 +716,7 @@ public class Base64
      * @throws NullPointerException if source array is null
      * @since 1.4
      */
-    public static String encodeBytes( byte[] source ) {
+    public static String encodeBytes(@NonNull byte[] source ) {
         // Since we're not going to have the GZIP encoding turned on,
         // we're not going to have an java.io.IOException thrown, so
         // we should not force the user to have to catch it.

libsignal/src/main/java/org/session/libsignal/utilities/Snode.kt

@@ -1,9 +1,25 @@
 package org.session.libsignal.utilities
 
-class Snode(val address: String, val port: Int, val publicKeySet: KeySet?, val version: String) {
+import android.annotation.SuppressLint
+import android.util.LruCache
+
+/**
+ * Create a Snode from a "-" delimited String if valid, null otherwise.
+ */
+fun Snode(string: String): Snode? {
+    val components = string.split("-")
+    val address = components[0]
+    val port = components.getOrNull(1)?.toIntOrNull() ?: return null
+    val ed25519Key = components.getOrNull(2) ?: return null
+    val x25519Key = components.getOrNull(3) ?: return null
+    val version = components.getOrNull(4)?.let(Snode::Version) ?: Snode.Version.ZERO
+    return Snode(address, port, Snode.KeySet(ed25519Key, x25519Key), version)
+}
+
+class Snode(val address: String, val port: Int, val publicKeySet: KeySet?, val version: Version) {
     val ip: String get() = address.removePrefix("https://")
 
-    public enum class Method(val rawValue: String) {
+    enum class Method(val rawValue: String) {
         GetSwarm("get_snodes_for_pubkey"),
         Retrieve("retrieve"),
         SendMessage("store"),
@@ -19,17 +35,37 @@ class Snode(val address: String, val port: Int, val publicKeySet: KeySet?, val v
 
     data class KeySet(val ed25519Key: String, val x25519Key: String)
 
-    override fun equals(other: Any?): Boolean {
+    override fun equals(other: Any?) = other is Snode && address == other.address && port == other.port
-        return if (other is Snode) {
+    override fun hashCode(): Int = address.hashCode() xor port.hashCode()
-            address == other.address && port == other.port
+    override fun toString(): String = "$address:$port"
-        } else {
+
-            false
+    companion object {
-        }
+        private val CACHE = LruCache<String, Version>(100)
+
+        @SuppressLint("NotConstructor")
+        @Synchronized
+        fun Version(value: String) = CACHE[value] ?: Snode.Version(value).also { CACHE.put(value, it) }
+
+        fun Version(parts: List<Int>) = Version(parts.joinToString("."))
     }
 
-    override fun hashCode(): Int {
+    @JvmInline
-        return address.hashCode() xor port.hashCode()
+    value class Version(val value: ULong) {
+        companion object {
+            val ZERO = Version(0UL)
+            private const val MASK_BITS = 16
+            private const val MASK = 0xFFFFUL
         }
 
-    override fun toString(): String { return "$address:$port" }
+        internal constructor(value: String): this(
+            value.splitToSequence(".")
+                .take(4)
+                .map { it.toULongOrNull() ?: 0UL }
+                .foldIndexed(0UL) { i, acc, it ->
+                    it.coerceAtMost(MASK) shl (3 - i) * MASK_BITS or acc
+                }
+        )
+
+        operator fun compareTo(other: Version): Int = value.compareTo(other.value)
+    }
 }