fix: Improve parsing of Friendica (and other server) version formats

.editorconfig

@@ -25,3 +25,8 @@ max_line_length = off
 
 [*.{yml,yaml}]
 indent_size = 2
+
+# Disable ktlint on generated source code, see
+# https://github.com/JLLeitschuh/ktlint-gradle/issues/746
+[**/build/generated/source/**]
+ktlint = disabled

app/lint-baseline.xml

@@ -3534,7 +3534,7 @@
         errorLine2="                                               ~~~~~~~~~~~~~~~~~~~">
         <location
             file="src/main/java/app/pachli/fragment/SFragment.kt"
-            line="239"
+            line="240"
             column="48"/>
     </issue>
 

app/src/main/java/app/pachli/fragment/SFragment.kt

@@ -127,6 +127,7 @@ abstract class SFragment : Fragment() {
                     result.onFailure {
                         val msg = getString(
                             R.string.server_repository_error,
+                            accountManager.activeAccount!!.domain,
                             it.msg(requireContext()),
                         )
                         Timber.e(msg)

app/src/main/java/app/pachli/network/ServerRepository.kt

@@ -27,7 +27,7 @@ import app.pachli.core.network.model.nodeinfo.NodeInfo
 import app.pachli.core.network.retrofit.MastodonApi
 import app.pachli.core.network.retrofit.NodeInfoApi
 import app.pachli.network.ServerRepository.Error.Capabilities
-import app.pachli.network.ServerRepository.Error.GetInstanceInfo
+import app.pachli.network.ServerRepository.Error.GetInstanceInfoV1
 import app.pachli.network.ServerRepository.Error.GetNodeInfo
 import app.pachli.network.ServerRepository.Error.GetWellKnownNodeInfo
 import app.pachli.network.ServerRepository.Error.UnsupportedSchema
@@ -75,8 +75,8 @@ class ServerRepository @Inject constructor(
     }
 
     /**
-     * @return the current server or a [Server.Error] subclass error if the
-     * server can not be determined.
+     * @return the server info or a [Server.Error] if the server info can not
+     * be determined.
      */
     private suspend fun getServer(): Result<Server, Error> = binding {
         // Fetch the /.well-known/nodeinfo document
@@ -107,7 +107,7 @@ class ServerRepository @Inject constructor(
             {
                 mastodonApi.getInstanceV1().fold(
                     { Server.from(nodeInfo.software, it).mapError(::Capabilities) },
-                    { Err(GetInstanceInfo(it)) },
+                    { Err(GetInstanceInfoV1(it)) },
                 )
             },
         ).bind()
@@ -139,7 +139,7 @@ class ServerRepository @Inject constructor(
             source = error,
         )
 
-        data class GetInstanceInfo(val throwable: Throwable) : Error(
+        data class GetInstanceInfoV1(val throwable: Throwable) : Error(
             R.string.server_repository_error_get_instance_info,
             throwable.localizedMessage,
         )

app/src/main/res/values/strings.xml

@@ -766,7 +766,7 @@ Your description here:\n\n
 ----\n
 </string>
 
-    <string name="server_repository_error">Could not fetch server info: %1$s</string>
+    <string name="server_repository_error">Could not fetch server info for %1$s: %2$s</string>
     <string name="server_repository_error_get_well_known_node_info">fetching /.well-known/nodeinfo failed: %1$s</string>
     <string name="server_repository_error_unsupported_schema">/.well-known/nodeinfo did not contain understandable schemas</string>
     <string name="server_repository_error_get_node_info">fetching nodeinfo %1$s failed: %2$s</string>

core/common/src/main/kotlin/app/pachli/core/common/PachliError.kt

@@ -19,12 +19,6 @@ package app.pachli.core.common
 
 import android.content.Context
 import androidx.annotation.StringRes
-import com.github.michaelbull.result.Err
-import com.github.michaelbull.result.Ok
-import com.github.michaelbull.result.Result
-import com.github.michaelbull.result.getError
-import com.github.michaelbull.result.getOr
-import kotlin.coroutines.cancellation.CancellationException
 
 /**
  * Base class for errors throughout the app.
@@ -83,46 +77,3 @@ open class PachliError(
         return context.getString(resourceId, *args.toTypedArray())
     }
 }
-
-// See https://www.jacobras.nl/2022/04/resilient-use-cases-with-kotlin-result-coroutines-and-annotations/
-
-/**
- * Like [runCatching], but with proper coroutines cancellation handling. Also only catches [Exception] instead of [Throwable].
- *
- * Cancellation exceptions need to be rethrown. See https://github.com/Kotlin/kotlinx.coroutines/issues/1814.
- */
-inline fun <R> resultOf(block: () -> R): Result<R, Exception> {
-    return try {
-        Ok(block())
-    } catch (e: CancellationException) {
-        throw e
-    } catch (e: Exception) {
-        Err(e)
-    }
-}
-
-/**
- * Like [runCatching], but with proper coroutines cancellation handling. Also only catches [Exception] instead of [Throwable].
- *
- * Cancellation exceptions need to be rethrown. See https://github.com/Kotlin/kotlinx.coroutines/issues/1814.
- */
-inline fun <T, R> T.resultOf(block: T.() -> R): Result<R, Exception> {
-    return try {
-        Ok(block())
-    } catch (e: CancellationException) {
-        throw e
-    } catch (e: Exception) {
-        Err(e)
-    }
-}
-
-/**
- * Like [mapCatching], but uses [resultOf] instead of [runCatching].
- */
-inline fun <R, T> Result<T, Exception>.mapResult(transform: (value: T) -> R): Result<R, Exception> {
-    val successResult = getOr { null } // getOrNull()
-    return when {
-        successResult != null -> resultOf { transform(successResult) }
-        else -> Err(getError() ?: error("Unreachable state"))
-    }
-}

core/network/src/main/kotlin/app/pachli/core/network/Server.kt

@@ -18,28 +18,41 @@
 package app.pachli.core.network
 
 import androidx.annotation.StringRes
+import androidx.annotation.VisibleForTesting
+import androidx.annotation.VisibleForTesting.Companion.PRIVATE
 import app.pachli.core.common.PachliError
-import app.pachli.core.common.resultOf
 import app.pachli.core.network.Server.Error.UnparseableVersion
 import app.pachli.core.network.ServerKind.AKKOMA
+import app.pachli.core.network.ServerKind.FEDIBIRD
+import app.pachli.core.network.ServerKind.FRIENDICA
+import app.pachli.core.network.ServerKind.GLITCH
 import app.pachli.core.network.ServerKind.GOTOSOCIAL
+import app.pachli.core.network.ServerKind.HOMETOWN
+import app.pachli.core.network.ServerKind.ICESHRIMP
 import app.pachli.core.network.ServerKind.MASTODON
+import app.pachli.core.network.ServerKind.PIXELFED
 import app.pachli.core.network.ServerKind.PLEROMA
+import app.pachli.core.network.ServerKind.SHARKEY
 import app.pachli.core.network.ServerKind.UNKNOWN
 import app.pachli.core.network.ServerOperation.ORG_JOINMASTODON_FILTERS_CLIENT
 import app.pachli.core.network.ServerOperation.ORG_JOINMASTODON_FILTERS_SERVER
 import app.pachli.core.network.ServerOperation.ORG_JOINMASTODON_STATUSES_TRANSLATE
 import app.pachli.core.network.model.InstanceV1
 import app.pachli.core.network.model.InstanceV2
 import app.pachli.core.network.model.nodeinfo.NodeInfo
+import com.github.michaelbull.result.Ok
 import com.github.michaelbull.result.Result
+import com.github.michaelbull.result.andThen
 import com.github.michaelbull.result.binding
-import com.github.michaelbull.result.getOrElse
+import com.github.michaelbull.result.coroutines.runSuspendCatching
 import com.github.michaelbull.result.mapError
+import com.github.michaelbull.result.recover
+import com.github.michaelbull.result.toResultOr
 import io.github.z4kn4fein.semver.Version
 import io.github.z4kn4fein.semver.constraints.Constraint
 import io.github.z4kn4fein.semver.satisfies
 import io.github.z4kn4fein.semver.toVersion
+import java.text.ParseException
 import kotlin.collections.set
 
 /**
@@ -97,7 +110,7 @@ data class Server(
          * Constructs a server from its [NodeInfo] and [InstanceV2] details.
          */
         fun from(software: NodeInfo.Software, instanceV2: InstanceV2): Result<Server, Error> = binding {
-            val serverKind = ServerKind.from(software.name)
+            val serverKind = ServerKind.from(software)
             val version = parseVersionString(serverKind, software.version).bind()
             val capabilities = capabilitiesFromServerVersion(serverKind, version)
 
@@ -120,7 +133,7 @@ data class Server(
          * Constructs a server from its [NodeInfo] and [InstanceV1] details.
          */
         fun from(software: NodeInfo.Software, instanceV1: InstanceV1): Result<Server, Error> = binding {
-            val serverKind = ServerKind.from(software.name)
+            val serverKind = ServerKind.from(software)
             val version = parseVersionString(serverKind, software.version).bind()
             val capabilities = capabilitiesFromServerVersion(serverKind, version)
 
@@ -130,37 +143,108 @@ data class Server(
         /**
          * Parse a [version] string from the given [serverKind] in to a [Version].
          */
-        private fun parseVersionString(serverKind: ServerKind, version: String): Result<Version, UnparseableVersion> = binding {
-            // Real world examples of version strings from nodeinfo
-            // pleroma - 2.6.50-875-g2eb5c453.service-origin+soapbox
-            // akkoma - 3.9.3-0-gd83f5f66f-blob
-            // firefish - 1.1.0-dev29-hf1
-            // hometown - 4.0.10+hometown-1.1.1
-            // cherrypick - 4.6.0+cs-8f0ba0f
-            // gotosocial - 0.13.1-SNAPSHOT git-dfc7656
-
-            val semver = when (serverKind) {
-                // These servers have semver compatible versions
-                AKKOMA, MASTODON, PLEROMA, UNKNOWN -> {
-                    resultOf { Version.parse(version, strict = false) }
-                        .mapError { UnparseableVersion(version, it) }.bind()
+        @VisibleForTesting(otherwise = PRIVATE)
+        fun parseVersionString(serverKind: ServerKind, version: String): Result<Version, Error> {
+            val result = runSuspendCatching {
+                Version.parse(version, strict = false)
+            }.mapError { UnparseableVersion(version, it) }
+
+            if (result is Ok) return result
+
+            return when (serverKind) {
+                // These servers should have semver compatible versions, but perhaps
+                // the server operator has changed them. Try looking for a matching
+                // <major>.<minor>.<patch> somewhere in the version string and hope
+                // it's correct
+                AKKOMA, FEDIBIRD, GLITCH, HOMETOWN, MASTODON, PIXELFED, UNKNOWN -> {
+                    val rx = """(?<major>\d+)\.(?<minor>\d+).(?<patch>\d+)""".toRegex()
+                    rx.find(version)
+                        .toResultOr { UnparseableVersion(version, ParseException("unexpected null", 0)) }
+                        .andThen {
+                            val adjusted = "${it.groups["major"]?.value}.${it.groups["minor"]?.value}.${it.groups["patch"]?.value}"
+                            runSuspendCatching { Version.parse(adjusted, strict = false) }
+                                .mapError { UnparseableVersion(version, it) }
+                        }
+                }
+
+                // Friendica does not report a semver compatible version, expect something
+                // where the version looks like "yyyy.mm", with an optional suffix that
+                // starts with a "-". The date-like parts of the string may have leading
+                // zeros.
+                //
+                // Try to extract the "yyyy.mm", without any leading zeros, append ".0".
+                // https://github.com/friendica/friendica/issues/11264
+                FRIENDICA -> {
+                    val rx = """^0*(?<major>\d+)\.0*(?<minor>\d+)""".toRegex()
+                    rx.find(version)
+                        .toResultOr { UnparseableVersion(version, ParseException("unexpected null", 0)) }
+                        .andThen {
+                            val adjusted = "${it.groups["major"]?.value}.${it.groups["minor"]?.value}.0"
+                            runSuspendCatching { Version.parse(adjusted, strict = false) }
+                                .mapError { UnparseableVersion(version, it) }
+                        }
                 }
-                // GoToSocial does not report a semver compatible version, expect something
-                // where the possible version number is space-separated, like "0.13.1 git-ccecf5a"
+
+                // GoToSocial does not always report a semver compatible version, and is all
+                // over the place, including:
+                //
+                // - "" (empty)
+                // - "git-8ab30d0"
+                // - "kalaclista git-212fecf"
+                // - "f4fcffc8b56ef73c184ae17892b69181961c15c7"
+                //
+                // as well as instances where the version number is semver compatible, but is
+                // separated by whitespace or a "_".
+                //
                 // https://github.com/superseriousbusiness/gotosocial/issues/1953
+                //
+                // Since GoToSocial has comparatively few features at the moment just fall
+                // back to "0.0.0" if there are problems.
                 GOTOSOCIAL -> {
-                    // Try and parse as semver, just in case
-                    resultOf { Version.parse(version, strict = false) }
-                        .getOrElse {
-                            // Didn't parse, use the first component, fall back to 0.0.0
-                            val components = version.split(" ")
-                            resultOf { Version.parse(components[0], strict = false) }
-                                .getOrElse { "0.0.0".toVersion() }
+                    // Failed, split on spaces and parse the first component
+                    val components = version.split(" ", "_")
+                    runSuspendCatching { Version.parse(components[0], strict = false) }
+                        .recover { "0.0.0".toVersion() }
+                }
+
+                // IceShrimp uses "yyyy.mm.dd" with leading zeros in the month and day
+                // components, similar to Friendica.
+                // https://iceshrimp.dev/iceshrimp/iceshrimp/issues/502 and
+                // https://iceshrimp.dev/iceshrimp/iceshrimp-rewrite/issues/1
+                ICESHRIMP -> {
+                    val rx = """^0*(?<major>\d+)\.0*(?<minor>\d+)\.0*(?<patch>\d+)""".toRegex()
+                    rx.find(version).toResultOr { UnparseableVersion(version, ParseException("unexpected null", 0)) }
+                        .andThen {
+                            val adjusted = "${it.groups["major"]?.value}.${it.groups["minor"]?.value ?: 0}.${it.groups["patch"]?.value ?: 0}"
+                            runSuspendCatching { Version.parse(adjusted, strict = false) }
+                                .mapError { UnparseableVersion(adjusted, it) }
+                        }
+                }
+
+                // Seen "Pleroma 0.9.0 d93789dfde3c44c76a56732088a897ddddfe9716" in
+                // the wild
+                PLEROMA -> {
+                    val rx = """Pleroma (?<major>\d+)\.(?<minor>\d+)\.(?<patch>\d+)""".toRegex()
+                    rx.find(version).toResultOr { UnparseableVersion(version, ParseException("unexpected null", 0)) }
+                        .andThen {
+                            val adjusted = "${it.groups["major"]?.value}.${it.groups["minor"]?.value}.${it.groups["patch"]?.value}"
+                            runSuspendCatching { Version.parse(adjusted, strict = false) }
+                                .mapError { UnparseableVersion(adjusted, it) }
                         }
                 }
-            }
 
-            semver
+                // Uses format "yyyy.mm.dd" with an optional ".beta..." suffix.
+                // https://git.joinsharkey.org/Sharkey/Sharkey/issues/371
+                SHARKEY -> {
+                    val rx = """^(?<major>\d+)\.(?<minor>\d+)\.(?<patch>\d+)""".toRegex()
+                    rx.find(version).toResultOr { UnparseableVersion(version, ParseException("unexpected null", 0)) }
+                        .andThen {
+                            val adjusted = "${it.groups["major"]?.value}.${it.groups["minor"]?.value}.${it.groups["patch"]?.value}"
+                            runSuspendCatching { Version.parse(adjusted, strict = false) }
+                                .mapError { UnparseableVersion(adjusted, it) }
+                        }
+                }
+            }
         }
 
         /**
@@ -188,7 +272,7 @@ data class Server(
 
                 // Everything else. Assume server side filtering and no translation. This may be an
                 // incorrect assumption.
-                AKKOMA, PLEROMA, UNKNOWN -> {
+                AKKOMA, FEDIBIRD, FRIENDICA, GLITCH, HOMETOWN, ICESHRIMP, PIXELFED, PLEROMA, SHARKEY, UNKNOWN -> {
                     c[ORG_JOINMASTODON_FILTERS_SERVER] = "1.0.0".toVersion()
                 }
             }
@@ -210,20 +294,45 @@ data class Server(
     }
 }
 
+/**
+ * Servers that are known to implement the Mastodon client API
+ */
 enum class ServerKind {
     AKKOMA,
+    FEDIBIRD,
+    FRIENDICA,
+    GLITCH,
     GOTOSOCIAL,
+    HOMETOWN,
+    ICESHRIMP,
     MASTODON,
     PLEROMA,
+    PIXELFED,
+    SHARKEY,
+
+    /**
+     * Catch-all for servers we don't recognise but that responded to either
+     * /api/v1/instance or /api/v2/instance
+     */
     UNKNOWN,
     ;
 
     companion object {
-        fun from(s: String) = when (s.lowercase()) {
+        fun from(s: NodeInfo.Software) = when (s.name.lowercase()) {
             "akkoma" -> AKKOMA
+            "fedibird" -> FEDIBIRD
+            "friendica" -> FRIENDICA
             "gotosocial" -> GOTOSOCIAL
-            "mastodon" -> MASTODON
+            "hometown" -> HOMETOWN
+            "iceshrimp" -> ICESHRIMP
+            "mastodon" -> {
+                // Glitch doesn't report a different software name it stuffs it
+                // in the version (https://github.com/glitch-soc/mastodon/issues/2582).
+                if (s.version.contains("+glitch")) GLITCH else MASTODON
+            }
+            "pixelfed" -> PIXELFED
             "pleroma" -> PLEROMA
+            "sharkey" -> SHARKEY
             else -> UNKNOWN
         }
     }