fix: Handle JSON enums with unknown values (#462)

Previous code expected all incoming enums values to map directly to
Kotlin enum constants.

This is a problem for servers with additional features -- e.g.,
"reaction" as a notification type.

Fix this with a new Moshi adapter that will set the incoming value to a
given constant if it's not recognised.

Apply this to the enum constants in core.network to ensure they are
handled.

Clean up enum handling in Converters.kt, ComposeViewModel.kt, and
Status.kt by using the existing `.ordinal` property and some extension
functions for idiomatic code.

Fixes #461

app/src/main/java/app/pachli/components/compose/ComposeViewModel.kt

@@ -427,8 +427,8 @@ class ComposeViewModel @Inject constructor(
         val preferredVisibility = accountManager.activeAccount!!.defaultPostPrivacy
 
         val replyVisibility = composeOptions?.replyVisibility ?: Status.Visibility.UNKNOWN
-        startingVisibility = Status.Visibility.byNum(
+        startingVisibility = Status.Visibility.getOrUnknown(
-            preferredVisibility.num.coerceAtLeast(replyVisibility.num),
+            preferredVisibility.ordinal.coerceAtLeast(replyVisibility.ordinal),
         )
 
         inReplyToId = composeOptions?.inReplyToId
@@ -471,7 +471,7 @@ class ComposeViewModel @Inject constructor(
         postLanguage = composeOptions?.language
 
         val tootVisibility = composeOptions?.visibility ?: Status.Visibility.UNKNOWN
-        if (tootVisibility.num != Status.Visibility.UNKNOWN.num) {
+        if (tootVisibility != Status.Visibility.UNKNOWN) {
             startingVisibility = tootVisibility
         }
         statusVisibility.value = startingVisibility

core/common/src/main/kotlin/app/pachli/core/common/extensions/EnumExtensions.kt

@@ -0,0 +1,54 @@
+/*
+ * Copyright 2024 Pachli Association
+ *
+ * This file is a part of Pachli.
+ *
+ * This program is free software; you can redistribute it and/or modify it under the terms of the
+ * GNU General Public License as published by the Free Software Foundation; either version 3 of the
+ * License, or (at your option) any later version.
+ *
+ * Pachli is distributed in the hope that it will be useful, but WITHOUT ANY WARRANTY; without even
+ * the implied warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU General
+ * Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along with Pachli; if not,
+ * see <http://www.gnu.org/licenses>.
+ */
+
+package app.pachli.core.common.extensions
+
+// Note: Technically these don't have to be extension methods on
+// Enum.Companion. You could remove that and write:
+//
+//   get<SomeEnum>(0)
+//
+// But the bare "get" (getOrElse, getOrNull) in calling code strikes
+// me as a little weird. Installing these on Enum.Companion makes
+// the calling code:
+//
+//   Enum.get<SomeEnum>(0)
+//
+// which is a little more legible.
+
+/**
+ * Returns the enum constant with the given [ordinal] value.
+ *
+ * If the ordinal is out of bounds of this enum, throws
+ * an IndexOutOfBoundsException except in Kotlin/JS where the behavior is unspecified.
+ *
+ * @see [kotlin.Array.get]
+ */
+inline fun <reified E : Enum<E>> Enum.Companion.get(ordinal: Int) = enumValues<E>()[ordinal]
+
+/**
+ * Returns the enum constant with the given [ordinal] value or the result
+ * of calling the [defaultValue] function if the [ordinal] is out of bounds of
+ * this enum.
+ */
+inline fun <reified E : Enum<E>> Enum.Companion.getOrElse(ordinal: Int, defaultValue: (Int) -> E) = enumValues<E>().getOrElse(ordinal, defaultValue)
+
+/**
+ * Returns the enum constant with the given [ordinal] value or `null` if the
+ * [ordinal] is out of bounds of this enum
+ */
+inline fun <reified E : Enum<E>> Enum.Companion.getOrNull(ordinal: Int) = enumValues<E>().getOrNull(ordinal)

core/database/src/main/kotlin/app/pachli/core/database/Converters.kt

@@ -58,12 +58,12 @@ class Converters @Inject constructor(
 
     @TypeConverter
     fun visibilityToInt(visibility: Status.Visibility?): Int {
-        return visibility?.num ?: Status.Visibility.UNKNOWN.num
+        return visibility?.ordinal ?: Status.Visibility.UNKNOWN.ordinal
     }
 
     @TypeConverter
     fun intToVisibility(visibility: Int): Status.Visibility {
-        return Status.Visibility.byNum(visibility)
+        return Status.Visibility.getOrUnknown(visibility)
     }
 
     @TypeConverter

core/network/src/main/kotlin/app/pachli/core/network/di/NetworkModule.kt

@@ -25,6 +25,7 @@ import app.pachli.core.network.BuildConfig
 import app.pachli.core.network.json.BooleanIfNull
 import app.pachli.core.network.json.DefaultIfNull
 import app.pachli.core.network.json.Guarded
+import app.pachli.core.network.json.HasDefault
 import app.pachli.core.network.retrofit.InstanceSwitchAuthInterceptor
 import app.pachli.core.network.retrofit.MastodonApi
 import app.pachli.core.preferences.PrefKeys.HTTP_PROXY_ENABLED
@@ -65,6 +66,7 @@ object NetworkModule {
     fun providesMoshi(): Moshi = Moshi.Builder()
         .add(Date::class.java, Rfc3339DateJsonAdapter())
         .add(Guarded.Factory())
+        .add(HasDefault.Factory())
         .add(DefaultIfNull.Factory())
         .add(BooleanIfNull.Factory())
         .build()

core/network/src/main/kotlin/app/pachli/core/network/json/HasDefault.kt

@@ -0,0 +1,133 @@
+/*
+ * Copyright 2024 Pachli Association
+ *
+ * This file is a part of Pachli.
+ *
+ * This program is free software; you can redistribute it and/or modify it under the terms of the
+ * GNU General Public License as published by the Free Software Foundation; either version 3 of the
+ * License, or (at your option) any later version.
+ *
+ * Pachli is distributed in the hope that it will be useful, but WITHOUT ANY WARRANTY; without even
+ * the implied warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU General
+ * Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along with Pachli; if not,
+ * see <http://www.gnu.org/licenses>.
+ */
+
+package app.pachli.core.network.json
+
+import com.squareup.moshi.JsonAdapter
+import com.squareup.moshi.JsonDataException
+import com.squareup.moshi.JsonQualifier
+import com.squareup.moshi.JsonReader
+import com.squareup.moshi.JsonWriter
+import com.squareup.moshi.Moshi
+import com.squareup.moshi.Types
+import java.lang.reflect.Type
+
+/**
+ * Marks the enum constant to use as the default value if the parsed JSON
+ * contains an invalid value.
+ *
+ * The `enum class` must be annotated with [HasDefault].
+ */
+@Retention(AnnotationRetention.RUNTIME)
+@Target(AnnotationTarget.FIELD)
+annotation class Default
+
+/**
+ * A [JsonQualifier] for use with [Enum] declarations to indicate that incoming
+ * JSON values that are not valid enum constants should be mapped to a default
+ * value instead of throwing a [JsonDataException].
+ *
+ * Usage:
+ * ```
+ * val moshi = Moshi.Builder()
+ *     .add(HasDefault.Factory())
+ *     .build()
+ *
+ * @HasDefault
+ * enum class SomeEnum {
+ *     @Default
+ *     FOO,
+ *     BAR
+ * }
+ *
+ * @JsonClass(generateAdapter = true)
+ * data class Data(
+ *     @Json(name = "some_enum") someEnum: SomeEnum
+ * )
+ * ```
+ *
+ * JSON of the form `{ "some_enum": "unknown" }` will parse to a
+ *
+ * ```
+ * Data(someEnum = SomeEnum.FOO)
+ * ```
+ *
+ * because `SomeEnum.FOO` has the `@Default` annotation. Move it to another constant
+ * to change it.
+ *
+ * This is similar to Moshi's existing [com.squareup.moshi.adapters.EnumJsonAdapter]
+ * which also supports fallbacks. The primary difference is that you define the
+ * default value at the point where the `enum class` is declared, not at the point
+ * where the Moshi instance is created.
+ */
+@Retention(AnnotationRetention.RUNTIME)
+@Target(AnnotationTarget.CLASS)
+@JsonQualifier
+annotation class HasDefault() {
+    class Factory : JsonAdapter.Factory {
+        override fun create(
+            type: Type,
+            annotations: MutableSet<out Annotation>,
+            moshi: Moshi,
+        ): JsonAdapter<*>? {
+            if (annotations.isNotEmpty()) return null
+            val rawType = Types.getRawType(type)
+            if (!rawType.isEnum) return null
+
+            rawType.getAnnotation(HasDefault::class.java) ?: return null
+
+            val delegateAnnotations = Types.nextAnnotations(
+                annotations,
+                HasDefault::class.java,
+            ) ?: emptySet()
+
+            val delegate = moshi.nextAdapter<Any>(
+                this,
+                type,
+                delegateAnnotations,
+            )
+
+            val enumType = rawType as Class<out Enum<*>>
+
+            val default = enumType.enumConstants.firstOrNull {
+                it::class.java.getField(it.name).getAnnotation(Default::class.java) != null
+            } ?: throw AssertionError("Missing @Default on ${enumType.name}")
+
+            return Adapter(delegate, default)
+        }
+
+        private class Adapter<T : Enum<T>>(
+            private val delegate: JsonAdapter<Any>,
+            val default: Enum<*>,
+        ) : JsonAdapter<T>() {
+            override fun fromJson(reader: JsonReader): T {
+                val peeked = reader.peekJson()
+                val result = try {
+                    delegate.fromJson(peeked) as T
+                } catch (_: JsonDataException) {
+                    default
+                } finally {
+                    peeked.close()
+                }
+                reader.skipValue()
+                return result as T
+            }
+
+            override fun toJson(writer: JsonWriter, value: T?) = delegate.toJson(writer, value)
+        }
+    }
+}

core/network/src/main/kotlin/app/pachli/core/network/model/Attachment.kt

@@ -18,7 +18,9 @@
 package app.pachli.core.network.model
 
 import android.os.Parcelable
+import app.pachli.core.network.json.Default
 import app.pachli.core.network.json.DefaultIfNull
+import app.pachli.core.network.json.HasDefault
 import com.squareup.moshi.Json
 import com.squareup.moshi.JsonClass
 import kotlinx.parcelize.Parcelize
@@ -36,6 +38,7 @@ data class Attachment(
     val blurhash: String?,
 ) : Parcelable {
 
+    @HasDefault
     enum class Type {
         @Json(name = "image")
         IMAGE,
@@ -50,6 +53,7 @@ data class Attachment(
         AUDIO,
 
         @Json(name = "unknown")
+        @Default
         UNKNOWN,
     }
 

core/network/src/main/kotlin/app/pachli/core/network/model/Filter.kt

@@ -1,6 +1,8 @@
 package app.pachli.core.network.model
 
 import android.os.Parcelable
+import app.pachli.core.network.json.Default
+import app.pachli.core.network.json.HasDefault
 import com.squareup.moshi.Json
 import com.squareup.moshi.JsonClass
 import java.util.Date
@@ -22,26 +24,33 @@ data class Filter(
     val keywords: List<FilterKeyword> = emptyList(),
     // val statuses: List<FilterStatus>,
 ) : Parcelable {
+    @HasDefault
     enum class Action(val action: String) {
         NONE("none"),
+
+        @Default
         WARN("warn"),
         HIDE("hide"),
         ;
 
         companion object {
-            fun from(action: String): Action = values().firstOrNull { it.action == action } ?: WARN
+            fun from(action: String): Action = entries.firstOrNull { it.action == action } ?: WARN
         }
     }
+
+    @HasDefault
     enum class Kind(val kind: String) {
         HOME("home"),
         NOTIFICATIONS("notifications"),
+
+        @Default
         PUBLIC("public"),
         THREAD("thread"),
         ACCOUNT("account"),
         ;
 
         companion object {
-            fun from(kind: String): Kind = values().firstOrNull { it.kind == kind } ?: PUBLIC
+            fun from(kind: String): Kind = entries.firstOrNull { it.kind == kind } ?: PUBLIC
 
             fun from(kind: TimelineKind): Kind = when (kind) {
                 is TimelineKind.Home, is TimelineKind.UserList -> HOME

core/network/src/main/kotlin/app/pachli/core/network/model/Notification.kt

@@ -17,6 +17,8 @@
 
 package app.pachli.core.network.model
 
+import app.pachli.core.network.json.Default
+import app.pachli.core.network.json.HasDefault
 import com.squareup.moshi.Json
 import com.squareup.moshi.JsonClass
 
@@ -33,8 +35,10 @@ data class Notification(
 
     /** From https://docs.joinmastodon.org/entities/Notification/#type */
     @JsonClass(generateAdapter = false)
+    @HasDefault
     enum class Type(val presentation: String) {
         @Json(name = "unknown")
+        @Default
         UNKNOWN("unknown"),
 
         /** Someone mentioned you */
@@ -80,14 +84,7 @@ data class Notification(
 
         companion object {
             @JvmStatic
-            fun byString(s: String): Type {
+            fun byString(s: String) = entries.firstOrNull { s == it.presentation } ?: UNKNOWN
-                values().forEach {
-                    if (s == it.presentation) {
-                        return it
-                    }
-                }
-                return UNKNOWN
-            }
 
             /** Notification types for UI display (omits UNKNOWN) */
             val visibleTypes = listOf(MENTION, REBLOG, FAVOURITE, FOLLOW, FOLLOW_REQUEST, POLL, STATUS, SIGN_UP, UPDATE, REPORT)
@@ -113,10 +110,7 @@ data class Notification(
     // for Pleroma compatibility that uses Mention type
     fun rewriteToStatusTypeIfNeeded(accountId: String): Notification {
         if (type == Type.MENTION && status != null) {
-            return if (status.mentions.any {
+            return if (status.mentions.any { it.id == accountId }) {
-                    it.id == accountId
-                }
-            ) {
                 this
             } else {
                 copy(type = Type.STATUS)

core/network/src/main/kotlin/app/pachli/core/network/model/Status.kt

@@ -19,6 +19,9 @@ package app.pachli.core.network.model
 
 import android.text.SpannableStringBuilder
 import android.text.style.URLSpan
+import app.pachli.core.common.extensions.getOrNull
+import app.pachli.core.network.json.Default
+import app.pachli.core.network.json.HasDefault
 import app.pachli.core.network.parseAsMastodonHtml
 import com.squareup.moshi.Json
 import com.squareup.moshi.JsonClass
@@ -64,20 +67,26 @@ data class Status(
     val actionableStatus: Status
         get() = reblog ?: this
 
-    enum class Visibility(val num: Int) {
+    // Note: These are deliberately listed in order, most public to least public.
-        UNKNOWN(0),
+    // These are currently serialised to the database by the ordinal value, and
+    // compared by ordinal value, so be extremely careful when adding anything
+    // to this list.
+    @HasDefault
+    enum class Visibility {
+        @Default
+        UNKNOWN,
 
         @Json(name = "public")
-        PUBLIC(1),
+        PUBLIC,
 
         @Json(name = "unlisted")
-        UNLISTED(2),
+        UNLISTED,
 
         @Json(name = "private")
-        PRIVATE(3),
+        PRIVATE,
 
         @Json(name = "direct")
-        DIRECT(4),
+        DIRECT,
         ;
 
         fun serverString(): String {
@@ -92,16 +101,7 @@ data class Status(
 
         companion object {
             @JvmStatic
-            fun byNum(num: Int): Visibility {
+            fun getOrUnknown(index: Int) = Enum.getOrNull<Visibility>(index) ?: UNKNOWN
-                return when (num) {
-                    4 -> DIRECT
-                    3 -> PRIVATE
-                    2 -> UNLISTED
-                    1 -> PUBLIC
-                    0 -> UNKNOWN
-                    else -> UNKNOWN
-                }
-            }
 
             @JvmStatic
             fun byString(s: String): Visibility {

core/network/src/main/kotlin/app/pachli/core/network/model/TrendsLink.kt

@@ -17,10 +17,16 @@
 
 package app.pachli.core.network.model
 
+import app.pachli.core.network.json.Default
+import app.pachli.core.network.json.HasDefault
 import com.squareup.moshi.Json
 import com.squareup.moshi.JsonClass
 
+@HasDefault
 enum class PreviewCardKind {
+    @Default
+    UNKNOWN,
+
     @Json(name = "link")
     LINK,
 

core/network/src/test/kotlin/app/pachli/core/network/json/HasDefaultTest.kt

@@ -0,0 +1,109 @@
+/*
+ * Copyright 2024 Pachli Association
+ *
+ * This file is a part of Pachli.
+ *
+ * This program is free software; you can redistribute it and/or modify it under the terms of the
+ * GNU General Public License as published by the Free Software Foundation; either version 3 of the
+ * License, or (at your option) any later version.
+ *
+ * Pachli is distributed in the hope that it will be useful, but WITHOUT ANY WARRANTY; without even
+ * the implied warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU General
+ * Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along with Pachli; if not,
+ * see <http://www.gnu.org/licenses>.
+ */
+
+package app.pachli.core.network.json
+
+import com.google.common.truth.Truth.assertThat
+import com.squareup.moshi.Json
+import com.squareup.moshi.JsonClass
+import com.squareup.moshi.JsonDataException
+import com.squareup.moshi.Moshi
+import com.squareup.moshi.adapter
+import kotlin.test.fail
+import org.junit.Test
+
+@OptIn(ExperimentalStdlibApi::class)
+class HasDefaultTest {
+    private val moshi = Moshi.Builder()
+        .add(HasDefault.Factory())
+        .build()
+
+    enum class NoAnnotation { FOO, BAR }
+
+    @HasDefault
+    enum class Annotated {
+        @Default
+        FOO,
+        BAR,
+    }
+
+    @Test
+    fun `unannotated enum parses`() {
+        assertThat(moshi.adapter<NoAnnotation>().fromJson("\"FOO\"")).isEqualTo(NoAnnotation.FOO)
+    }
+
+    @Test
+    fun `unannotated enum throws an exception on unrecognised value`() {
+        try {
+            moshi.adapter<NoAnnotation>().fromJson("\"UNKNOWN\"")
+            fail()
+        } catch (e: Exception) {
+            assertThat(e).isInstanceOf(JsonDataException::class.java)
+        }
+    }
+
+    @Test
+    fun `annotated enum parses as normal`() {
+        val adapter = moshi.adapter<Annotated>()
+        assertThat(adapter.fromJson("\"FOO\"")).isEqualTo(Annotated.FOO)
+        assertThat(adapter.fromJson("\"BAR\"")).isEqualTo(Annotated.BAR)
+    }
+
+    @Test
+    fun `annotated enum with unknown value parses as FOO`() {
+        val adapter = moshi.adapter<Annotated>()
+        assertThat(adapter.fromJson("\"unknown\"")).isEqualTo(Annotated.FOO)
+    }
+
+    @Test
+    fun `annotated enum emits correct JSON for valid values`() {
+        val adapter = moshi.adapter<Annotated>()
+        assertThat(adapter.toJson(Annotated.FOO)).isEqualTo("\"FOO\"")
+        assertThat(adapter.toJson(Annotated.BAR)).isEqualTo("\"BAR\"")
+    }
+
+    @JsonClass(generateAdapter = true)
+    data class Data(@Json(name = "some_enum") val someEnum: Annotated)
+
+    @Test
+    fun `annotated enum as property of class with unknown value parses as FOO`() {
+        val adapter = moshi.adapter<Data>()
+        assertThat(adapter.fromJson("{ \"some_enum\": \"unknown\" }")).isEqualTo(
+            Data(someEnum = Annotated.FOO),
+        )
+    }
+
+    // This definition has @HasDefault but no @Default constant, so should error
+    @HasDefault
+    enum class MissingDefaultAnnotation {
+        FOO,
+        BAR,
+    }
+
+    @Test
+    fun `unannotated enum constant throws exception when creating adapter`() {
+        try {
+            assertThat(moshi.adapter<MissingDefaultAnnotation>().fromJson("\"FOO\"")).isEqualTo(
+                MissingDefaultAnnotation.FOO,
+            )
+            fail()
+        } catch (e: Error) {
+            assertThat(e).isInstanceOf(AssertionError::class.java)
+            assertThat(e.message).contains("Missing @Default")
+        }
+    }
+}