fix: Handle null properties in `Attachment.Focus` (#449)

The `x` and `y` properties in `Attachment.Focus` may be null (not
documented as such, but observed in the wild).

Provide a `DefaultIfNull` adapter that can be applied to these to
replace null values with a sensible default.
This commit is contained in:
Nik Clayton 2024-02-15 20:32:22 +01:00 committed by GitHub
parent c7895cf2db
commit 11bf47e3b3
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
7 changed files with 226 additions and 3 deletions

View File

@ -2,6 +2,7 @@ package app.pachli
import androidx.test.ext.junit.runners.AndroidJUnit4 import androidx.test.ext.junit.runners.AndroidJUnit4
import app.pachli.core.database.model.TranslationState import app.pachli.core.database.model.TranslationState
import app.pachli.core.network.json.DefaultIfNullAdapter.Companion.DefaultIfNullAdapterFactory
import app.pachli.core.network.json.GuardedAdapter.Companion.GuardedAdapterFactory import app.pachli.core.network.json.GuardedAdapter.Companion.GuardedAdapterFactory
import app.pachli.core.network.model.Status import app.pachli.core.network.model.Status
import app.pachli.viewdata.StatusViewData import app.pachli.viewdata.StatusViewData
@ -18,7 +19,9 @@ import org.junit.runner.RunWith
class StatusComparisonTest { class StatusComparisonTest {
private val moshi = Moshi.Builder() private val moshi = Moshi.Builder()
.add(Date::class.java, Rfc3339DateJsonAdapter()) .add(Date::class.java, Rfc3339DateJsonAdapter())
.add(GuardedAdapterFactory()).build() .add(GuardedAdapterFactory())
.add(DefaultIfNullAdapterFactory())
.build()
@Test @Test
fun `two equal statuses - should be equal`() { fun `two equal statuses - should be equal`() {

View File

@ -20,6 +20,7 @@ import app.pachli.core.database.model.AccountEntity
import app.pachli.core.database.model.RemoteKeyEntity import app.pachli.core.database.model.RemoteKeyEntity
import app.pachli.core.database.model.RemoteKeyKind import app.pachli.core.database.model.RemoteKeyKind
import app.pachli.core.database.model.TimelineStatusWithAccount import app.pachli.core.database.model.TimelineStatusWithAccount
import app.pachli.core.network.json.DefaultIfNullAdapter.Companion.DefaultIfNullAdapterFactory
import app.pachli.core.network.json.GuardedAdapter.Companion.GuardedAdapterFactory import app.pachli.core.network.json.GuardedAdapter.Companion.GuardedAdapterFactory
import com.google.common.truth.Truth.assertThat import com.google.common.truth.Truth.assertThat
import com.squareup.moshi.Moshi import com.squareup.moshi.Moshi
@ -66,6 +67,7 @@ class CachedTimelineRemoteMediatorTest {
private val moshi: Moshi = Moshi.Builder() private val moshi: Moshi = Moshi.Builder()
.add(Date::class.java, Rfc3339DateJsonAdapter()) .add(Date::class.java, Rfc3339DateJsonAdapter())
.add(GuardedAdapterFactory()) .add(GuardedAdapterFactory())
.add(DefaultIfNullAdapterFactory())
.build() .build()
@Before @Before

View File

@ -5,6 +5,7 @@ import app.pachli.core.database.model.TimelineAccountEntity
import app.pachli.core.database.model.TimelineStatusEntity import app.pachli.core.database.model.TimelineStatusEntity
import app.pachli.core.database.model.TimelineStatusWithAccount import app.pachli.core.database.model.TimelineStatusWithAccount
import app.pachli.core.database.model.TranslationState import app.pachli.core.database.model.TranslationState
import app.pachli.core.network.json.DefaultIfNullAdapter.Companion.DefaultIfNullAdapterFactory
import app.pachli.core.network.json.GuardedAdapter.Companion.GuardedAdapterFactory import app.pachli.core.network.json.GuardedAdapter.Companion.GuardedAdapterFactory
import app.pachli.core.network.model.Status import app.pachli.core.network.model.Status
import app.pachli.core.network.model.TimelineAccount import app.pachli.core.network.model.TimelineAccount
@ -101,6 +102,7 @@ fun mockStatusEntityWithAccount(
val moshi = Moshi.Builder() val moshi = Moshi.Builder()
.add(Date::class.java, Rfc3339DateJsonAdapter()) .add(Date::class.java, Rfc3339DateJsonAdapter())
.add(GuardedAdapterFactory()) .add(GuardedAdapterFactory())
.add(DefaultIfNullAdapterFactory())
.build() .build()
return TimelineStatusWithAccount( return TimelineStatusWithAccount(

View File

@ -22,6 +22,7 @@ import android.os.Build
import app.pachli.core.common.util.versionName import app.pachli.core.common.util.versionName
import app.pachli.core.mastodon.model.MediaUploadApi import app.pachli.core.mastodon.model.MediaUploadApi
import app.pachli.core.network.BuildConfig import app.pachli.core.network.BuildConfig
import app.pachli.core.network.json.DefaultIfNullAdapter.Companion.DefaultIfNullAdapterFactory
import app.pachli.core.network.json.GuardedAdapter.Companion.GuardedAdapterFactory import app.pachli.core.network.json.GuardedAdapter.Companion.GuardedAdapterFactory
import app.pachli.core.network.retrofit.InstanceSwitchAuthInterceptor import app.pachli.core.network.retrofit.InstanceSwitchAuthInterceptor
import app.pachli.core.network.retrofit.MastodonApi import app.pachli.core.network.retrofit.MastodonApi
@ -63,6 +64,7 @@ object NetworkModule {
fun providesMoshi(): Moshi = Moshi.Builder() fun providesMoshi(): Moshi = Moshi.Builder()
.add(Date::class.java, Rfc3339DateJsonAdapter()) .add(Date::class.java, Rfc3339DateJsonAdapter())
.add(GuardedAdapterFactory()) .add(GuardedAdapterFactory())
.add(DefaultIfNullAdapterFactory())
.build() .build()
@Provides @Provides

View File

@ -0,0 +1,66 @@
/*
* 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.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
@Retention(AnnotationRetention.RUNTIME)
@JsonQualifier
annotation class DefaultIfNull
class DefaultIfNullAdapter(private val delegate: JsonAdapter<Any>) : JsonAdapter<Any>() {
override fun fromJson(reader: JsonReader): Any? {
val value = reader.readJsonValue()
if (value is Map<*, *>) {
val withoutNulls = value.filterValues { it != null }
return delegate.fromJsonValue(withoutNulls)
}
return delegate.fromJsonValue(value)
}
override fun toJson(writer: JsonWriter, value: Any?) {
return delegate.toJson(writer, value)
}
companion object {
class DefaultIfNullAdapterFactory : Factory {
override fun create(
type: Type,
annotations: MutableSet<out Annotation>,
moshi: Moshi,
): JsonAdapter<*>? {
val delegateAnnotations = Types.nextAnnotations(
annotations,
DefaultIfNull::class.java,
) ?: return null
val delegate = moshi.nextAdapter<Any>(
this,
type,
delegateAnnotations,
)
return DefaultIfNullAdapter(delegate)
}
}
}
}

View File

@ -18,6 +18,7 @@
package app.pachli.core.network.model package app.pachli.core.network.model
import android.os.Parcelable import android.os.Parcelable
import app.pachli.core.network.json.DefaultIfNull
import com.squareup.moshi.Json import com.squareup.moshi.Json
import com.squareup.moshi.JsonClass import com.squareup.moshi.JsonClass
import kotlinx.parcelize.Parcelize import kotlinx.parcelize.Parcelize
@ -58,6 +59,8 @@ data class Attachment(
@Parcelize @Parcelize
@JsonClass(generateAdapter = true) @JsonClass(generateAdapter = true)
data class MetaData( data class MetaData(
// Fields in Focus may be null, see https://github.com/mastodon/mastodon/issues/29222
@DefaultIfNull
val focus: Focus?, val focus: Focus?,
val duration: Float?, val duration: Float?,
val original: Size?, val original: Size?,
@ -73,8 +76,8 @@ data class Attachment(
@Parcelize @Parcelize
@JsonClass(generateAdapter = true) @JsonClass(generateAdapter = true)
data class Focus( data class Focus(
val x: Float, val x: Float = 0f,
val y: Float, val y: Float = 0f,
) : Parcelable { ) : Parcelable {
fun toMastodonApiString(): String = "$x,$y" fun toMastodonApiString(): String = "$x,$y"
} }

View File

@ -0,0 +1,145 @@
package app.pachli.core.network.json
import app.pachli.core.network.json.DefaultIfNullAdapter.Companion.DefaultIfNullAdapterFactory
import com.google.common.truth.Truth.assertThat
import com.squareup.moshi.JsonClass
import com.squareup.moshi.Moshi
import com.squareup.moshi.adapter
import org.junit.Test
@OptIn(ExperimentalStdlibApi::class)
class DefaultIfNullTest {
private val moshi = Moshi.Builder()
.add(DefaultIfNullAdapterFactory())
.build()
@JsonClass(generateAdapter = true)
data class Wrapper(
@DefaultIfNull
val data: Data,
)
@JsonClass(generateAdapter = true)
data class Data(
val x: Int = 1,
val y: Float = 2f,
val z: String = "hello, world",
val nullable: String? = null,
)
@Test
fun `null x defaults to 1`() {
val jsonInput = """
{
"data": {
"x": null,
"y": 3,
"z": "foo",
"nullable": "bar"
}
}
""".trimIndent()
assertThat(moshi.adapter<Wrapper>().fromJson(jsonInput)).isEqualTo(
Wrapper(
data = Data(
x = 1,
y = 3f,
z = "foo",
nullable = "bar",
),
),
)
}
@Test
fun `null y defaults to 2f`() {
val jsonInput = """
{
"data": {
"x": 1,
"y": null,
"z": "foo",
"nullable": "bar"
}
}
""".trimIndent()
assertThat(moshi.adapter<Wrapper>().fromJson(jsonInput)).isEqualTo(
Wrapper(
data = Data(
x = 1,
y = 2f,
z = "foo",
nullable = "bar",
),
),
)
}
@Test
fun `null z defaults to "hello, world"`() {
val jsonInput = """
{
"data": {
"x": 1,
"y": 2,
"z": null,
"nullable": "bar"
}
}
""".trimIndent()
assertThat(moshi.adapter<Wrapper>().fromJson(jsonInput)).isEqualTo(
Wrapper(
data = Data(
x = 1,
y = 2f,
z = "hello, world",
nullable = "bar",
),
),
)
}
@Test
fun `nullable remains null`() {
val jsonInput = """
{
"data": {
"x": 1,
"y": 2,
"z": "foo",
"nullable": null
}
}
""".trimIndent()
assertThat(moshi.adapter<Wrapper>().fromJson(jsonInput)).isEqualTo(
Wrapper(
data = Data(
x = 1,
y = 2f,
z = "foo",
nullable = null,
),
),
)
}
@Test
fun `null everything returns default`() {
val jsonInput = """
{
"data": {
"x": null,
"y": null,
"z": null,
"nullable": null
}
}
""".trimIndent()
assertThat(moshi.adapter<Wrapper>().fromJson(jsonInput)).isEqualTo(
Wrapper(
data = Data(),
),
)
}
}