chinwag/chinwag-android
chinwag/chinwag-android
2
0
Fork 0
Code Issues Pull requests Releases 5 Activity

Replace Gson library with Moshi (#4309)

Browse source 
**! ! Warning**: Do not merge before testing every API call and database
read involving JSON !

**Gson** is obsolete and has been superseded by **Moshi**. But more
importantly, parsing Kotlin objects using Gson is _dangerous_ because
Gson uses Java serialization and is **not Kotlin-aware**. This has two
main consequences:

- Fields of non-null types may end up null at runtime. Parsing will
succeed, but the code may crash later with a `NullPointerException` when
trying to access a field member;
- Default values of constructor parameters are always ignored. When
absent, reference types will be null, booleans will be false and
integers will be zero.

On the other hand, Kotlin-aware parsers like **Moshi** or **Kotlin
Serialization** will validate at parsing time that all received fields
comply with the Kotlin contract and avoid errors at runtime, making apps
more stable and schema mismatches easier to detect (as long as logs are
accessible):

- Receiving a null value for a non-null type will generate a parsing
error;
- Optional types are declared explicitly by adding a default value. **A
missing value with no default value declaration will generate a parsing
error.**

Migrating the entity declarations from Gson to Moshi will make the code
more robust but is not an easy task because of the semantic differences.

With Gson, both nullable and optional fields are represented with a null
value. After converting to Moshi, some nullable entities can become
non-null with a default value (if they are optional and not nullable),
others can stay nullable with no default value (if they are mandatory
and nullable), and others can become **nullable with a default value of
null** (if they are optional _or_ nullable _or_ both). That third option
is the safest bet when it's not clear if a field is optional or not,
except for lists which can usually be declared as non-null with a
default value of an empty list (I have yet to see a nullable array type
in the Mastodon API).

Fields that are currently declared as non-null present another
challenge. In theory, they should remain as-is and everything will work
fine. In practice, **because Gson is not aware of nullable types at
all**, it's possible that some non-null fields currently hold a null
value in some cases but the app does not report any error because the
field is not accessed by Kotlin code in that scenario. After migrating
to Moshi however, parsing such a field will now fail early if a null
value or no value is received.

These fields will have to be identified by heavily testing the app and
looking for parsing errors (`JsonDataException`) and/or by going through
the Mastodon documentation. A default value needs to be added for
missing optional fields, and their type could optionally be changed to
nullable, depending on the case.

Gson is also currently used to serialize and deserialize objects to and
from the local database, which is also challenging because backwards
compatibility needs to be preserved. Fortunately, by default Gson omits
writing null fields, so a field of type `List<T>?` could be replaced
with a field of type `List<T>` with a default value of `emptyList()` and
reading back the old data should still work. However, nullable lists
that are written directly (not as a field of another object) will still
be serialized to JSON as `"null"` so the deserializing code must still
be handling null properly.

Finally, changing the database schema is out of scope for this pull
request, so database entities that also happen to be serialized with
Gson will keep their original types even if they could be made non-null
as an improvement.

In the end this is all for the best, because the app will be more
reliable and errors will be easier to detect by showing up earlier with
a clear error message. Not to mention the performance benefits of using
Moshi compared to Gson.

- Replace Gson reflection with Moshi Kotlin codegen to generate all
parsers at compile time.
- Replace custom `Rfc3339DateJsonAdapter` with the one provided by
moshi-adapters.
- Replace custom `JsonDeserializer` classes for Enum types with
`EnumJsonAdapter.create(T).withUnknownFallback()` from moshi-adapters to
support fallback values.
- Replace `GuardedBooleanAdapter` with the more generic `GuardedAdapter`
which works with any type. Any nullable field may now be annotated with
`@Guarded`.
- Remove Proguard rules related to Json entities. Each Json entity needs
to be annotated with `@JsonClass` with no exception, and adding this
annotation will ensure that R8/Proguard will handle the entities
properly.
- Replace some nullable Boolean fields with non-null Boolean fields with
a default value where possible.
- Replace some nullable list fields with non-null list fields with a
default value of `emptyList()` where possible.
- Update `TimelineDao` to perform all Json conversions internally using
`Converters` so no Gson or Moshi instance has to be passed to its
methods.
- ~~Create a custom `DraftAttachmentJsonAdapter` to serialize and
deserialize `DraftAttachment` which is a special entity that supports
more than one json name per field. A custom adapter is necessary because
there is not direct equivalent of `@SerializedName(alternate = [...])`
in Moshi.~~ Remove alternate names for some `DraftAttachment` fields
which were used as a workaround to deserialize local data in 2-years old
builds of Tusky.
- Update tests to make them work with Moshi.
- Simplify a few `equals()` implementations.
- Change a few functions to `val`s
- Turn `NetworkModule` into an `object` (since it contains no abstract
methods).

Please test the app thoroughly before merging. There may be some fields
currently declared as mandatory that are actually optional.
This commit is contained in:
Christophe Beyls 2024-04-02 21:01:04 +02:00 • committed by GitHub
commit df7b11afc3
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
87 changed files with 767 additions and 992 deletions
Download patch file Download diff file Expand all files Collapse all files

29
app/src/main/java/com/keylesspalace/tusky/json/GuardedBooleanAdapter.kt → app/src/main/java/com/keylesspalace/tusky/json/Guarded.kt
View file

@ -1,4 +1,5 @@
/* Copyright 2022 Tusky Contributors
/*
* Copyright 2024 Tusky Contributors
*
* This file is a part of Tusky.
*
@ -11,27 +12,13 @@
* Public License for more details.
*
* You should have received a copy of the GNU General Public License along with Tusky; if not,
* see <http://www.gnu.org/licenses>. */
* see <http://www.gnu.org/licenses>.
*/
package com.keylesspalace.tusky.json
import com.google.gson.JsonDeserializationContext
import com.google.gson.JsonDeserializer
import com.google.gson.JsonElement
import com.google.gson.JsonParseException
import java.lang.reflect.Type
import com.squareup.moshi.JsonQualifier
class GuardedBooleanAdapter : JsonDeserializer<Boolean?> {
@Throws(JsonParseException::class)
override fun deserialize(
json: JsonElement,
typeOfT: Type,
context: JsonDeserializationContext
): Boolean? {
return if (json.isJsonObject) {
null
} else {
json.asBoolean
}
}
}
@Retention(AnnotationRetention.RUNTIME)
@JsonQualifier
internal annotation class Guarded

63
app/src/main/java/com/keylesspalace/tusky/json/GuardedAdapter.kt Normal file
View file

@ -0,0 +1,63 @@
/*
* Copyright 2024 Tusky Contributors
*
* This file is a part of Tusky.
*
* 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.
*
* Tusky 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 Tusky; if not,
* see <http://www.gnu.org/licenses>.
*/
package com.keylesspalace.tusky.json
import com.squareup.moshi.JsonAdapter
import com.squareup.moshi.JsonDataException
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
/**
* This adapter tries to parse the value using a delegated parser
* and returns null in case of error.
*/
class GuardedAdapter<T> private constructor(
private val delegate: JsonAdapter<T>
) : JsonAdapter<T>() {
override fun fromJson(reader: JsonReader): T? {
return try {
delegate.fromJson(reader)
} catch (e: JsonDataException) {
reader.skipValue()
null
}
}
override fun toJson(writer: JsonWriter, value: T?) {
delegate.toJson(writer, value)
}
companion object {
val ANNOTATION_FACTORY = object : Factory {
override fun create(
type: Type,
annotations: Set<Annotation>,
moshi: Moshi
): JsonAdapter<*>? {
val delegateAnnotations =
Types.nextAnnotations(annotations, Guarded::class.java) ?: return null
val delegate = moshi.nextAdapter<Any?>(this, type, delegateAnnotations)
return GuardedAdapter(delegate)
}
}
}
}

313
app/src/main/java/com/keylesspalace/tusky/json/Iso8601Utils.kt
View file

@ -1,313 +0,0 @@
package com.keylesspalace.tusky.json
/*
* Copyright (C) 2011 FasterXML, LLC
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* https://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
import com.google.gson.JsonParseException
import java.util.Calendar
import java.util.Date
import java.util.GregorianCalendar
import java.util.Locale
import java.util.TimeZone
import kotlin.math.min
import kotlin.math.pow
/*
* Jackson’s date formatter, pruned to Moshi's needs. Forked from this file:
* https://github.com/FasterXML/jackson-databind/blob/67ebf7305f492285a8f9f4de31545f5f16fc7c3a/src/main/java/com/fasterxml/jackson/databind/util/ISO8601Utils.java
*
* Utilities methods for manipulating dates in iso8601 format. This is much much faster and GC
* friendly than using SimpleDateFormat so highly suitable if you (un)serialize lots of date
* objects.
*
* Supported parse format:
* `[yyyy-MM-dd|yyyyMMdd][T(hh:mm[:ss[.sss]]|hhmm[ss[.sss]])]?[Z|[+-]hh[:]mm]]`
*
* @see [this specification](http://www.w3.org/TR/NOTE-datetime)
*/
/** ID to represent the 'GMT' string */
private const val GMT_ID = "GMT"
/** The GMT timezone, prefetched to avoid more lookups. */
private val TIMEZONE_Z: TimeZone = TimeZone.getTimeZone(GMT_ID)
/** Returns `date` formatted as yyyy-MM-ddThh:mm:ss.sssZ */
internal fun Date.formatIsoDate(): String {
val calendar: Calendar = GregorianCalendar(TIMEZONE_Z, Locale.US)
calendar.time = this
// estimate capacity of buffer as close as we can (yeah, that's pedantic ;)
val capacity = "yyyy-MM-ddThh:mm:ss.sssZ".length
val formatted = StringBuilder(capacity)
padInt(formatted, calendar[Calendar.YEAR], "yyyy".length)
formatted.append('-')
padInt(formatted, calendar[Calendar.MONTH] + 1, "MM".length)
formatted.append('-')
padInt(formatted, calendar[Calendar.DAY_OF_MONTH], "dd".length)
formatted.append('T')
padInt(formatted, calendar[Calendar.HOUR_OF_DAY], "hh".length)
formatted.append(':')
padInt(formatted, calendar[Calendar.MINUTE], "mm".length)
formatted.append(':')
padInt(formatted, calendar[Calendar.SECOND], "ss".length)
formatted.append('.')
padInt(formatted, calendar[Calendar.MILLISECOND], "sss".length)
formatted.append('Z')
return formatted.toString()
}
/**
* Parse a date from ISO-8601 formatted string. It expects a format
* `[yyyy-MM-dd|yyyyMMdd][T(hh:mm[:ss[.sss]]|hhmm[ss[.sss]])]?[Z|[+-]hh:mm]]`
*
* @receiver ISO string to parse in the appropriate format.
* @return the parsed date
*/
internal fun String.parseIsoDate(): Date {
return try {
var offset = 0
// extract year
val year = parseInt(
this,
offset,
4.let {
offset += it
offset
}
)
if (checkOffset(this, offset, '-')) {
offset += 1
}
// extract month
val month = parseInt(
this,
offset,
2.let {
offset += it
offset
}
)
if (checkOffset(this, offset, '-')) {
offset += 1
}
// extract day
val day = parseInt(
this,
offset,
2.let {
offset += it
offset
}
)
// default time value
var hour = 0
var minutes = 0
var seconds = 0
// always use 0 otherwise returned date will include millis of current time
var milliseconds = 0
// if the value has no time component (and no time zone), we are done
val hasT = checkOffset(this, offset, 'T')
if (!hasT && this.length <= offset) {
return GregorianCalendar(year, month - 1, day).time
}
if (hasT) {
// extract hours, minutes, seconds and milliseconds
hour = parseInt(
this,
1.let {
offset += it
offset
},
2.let {
offset += it
offset
}
)
if (checkOffset(this, offset, ':')) {
offset += 1
}
minutes = parseInt(
this, offset,
2.let {
offset += it
offset
}
)
if (checkOffset(this, offset, ':')) {
offset += 1
}
// second and milliseconds can be optional
if (this.length > offset) {
val c = this[offset]
if (c != 'Z' && c != '+' && c != '-') {
seconds = parseInt(
this, offset,
2.let {
offset += it
offset
}
)
if (seconds in 60..62) seconds = 59 // truncate up to 3 leap seconds
// milliseconds can be optional in the format
if (checkOffset(this, offset, '.')) {
offset += 1
val endOffset = indexOfNonDigit(
this,
offset + 1
) // assume at least one digit
val parseEndOffset = min(endOffset, offset + 3) // parse up to 3 digits
val fraction = parseInt(this, offset, parseEndOffset)
milliseconds =
(10.0.pow((3 - (parseEndOffset - offset)).toDouble()) * fraction).toInt()
offset = endOffset
}
}
}
}
// extract timezone
require(this.length > offset) { "No time zone indicator" }
val timezone: TimeZone
val timezoneIndicator = this[offset]
if (timezoneIndicator == 'Z') {
timezone = TIMEZONE_Z
} else if (timezoneIndicator == '+' || timezoneIndicator == '-') {
val timezoneOffset = this.substring(offset)
// 18-Jun-2015, tatu: Minor simplification, skip offset of "+0000"/"+00:00"
if ("+0000" == timezoneOffset || "+00:00" == timezoneOffset) {
timezone = TIMEZONE_Z
} else {
// 18-Jun-2015, tatu: Looks like offsets only work from GMT, not UTC...
// not sure why, but it is what it is.
val timezoneId = GMT_ID + timezoneOffset
timezone = TimeZone.getTimeZone(timezoneId)
val act = timezone.id
if (act != timezoneId) {
/*
* 22-Jan-2015, tatu: Looks like canonical version has colons, but we may be given
* one without. If so, don't sweat.
* Yes, very inefficient. Hopefully not hit often.
* If it becomes a perf problem, add 'loose' comparison instead.
*/
val cleaned = act.replace(":", "")
if (cleaned != timezoneId) {
throw IndexOutOfBoundsException(
"Mismatching time zone indicator: $timezoneId given, resolves to ${timezone.id}"
)
}
}
}
} else {
throw IndexOutOfBoundsException(
"Invalid time zone indicator '$timezoneIndicator'"
)
}
val calendar: Calendar = GregorianCalendar(timezone)
calendar.isLenient = false
calendar[Calendar.YEAR] = year
calendar[Calendar.MONTH] = month - 1
calendar[Calendar.DAY_OF_MONTH] = day
calendar[Calendar.HOUR_OF_DAY] = hour
calendar[Calendar.MINUTE] = minutes
calendar[Calendar.SECOND] = seconds
calendar[Calendar.MILLISECOND] = milliseconds
calendar.time
// If we get a ParseException it'll already have the right message/offset.
// Other exception types can convert here.
} catch (e: IndexOutOfBoundsException) {
throw JsonParseException("Not an RFC 3339 date: $this", e)
} catch (e: IllegalArgumentException) {
throw JsonParseException("Not an RFC 3339 date: $this", e)
}
}
/**
* Check if the expected character exist at the given offset in the value.
*
* @param value the string to check at the specified offset
* @param offset the offset to look for the expected character
* @param expected the expected character
* @return true if the expected character exist at the given offset
*/
private fun checkOffset(value: String, offset: Int, expected: Char): Boolean {
return offset < value.length && value[offset] == expected
}
/**
* Parse an integer located between 2 given offsets in a string
*
* @param value the string to parse
* @param beginIndex the start index for the integer in the string
* @param endIndex the end index for the integer in the string
* @return the int
* @throws NumberFormatException if the value is not a number
*/
private fun parseInt(value: String, beginIndex: Int, endIndex: Int): Int {
if (beginIndex < 0 || endIndex > value.length || beginIndex > endIndex) {
throw NumberFormatException(value)
}
// use same logic as in Integer.parseInt() but less generic we're not supporting negative values
var i = beginIndex
var result = 0
var digit: Int
if (i < endIndex) {
digit = Character.digit(value[i++], 10)
if (digit < 0) {
throw NumberFormatException("Invalid number: " + value.substring(beginIndex, endIndex))
}
result = -digit
}
while (i < endIndex) {
digit = Character.digit(value[i++], 10)
if (digit < 0) {
throw NumberFormatException("Invalid number: " + value.substring(beginIndex, endIndex))
}
result *= 10
result -= digit
}
return -result
}
/**
* Zero pad a number to a specified length
*
* @param buffer buffer to use for padding
* @param value the integer value to pad if necessary.
* @param length the length of the string we should zero pad
*/
private fun padInt(buffer: StringBuilder, value: Int, length: Int) {
val strValue = value.toString()
for (i in length - strValue.length downTo 1) {
buffer.append('0')
}
buffer.append(strValue)
}
/**
* Returns the index of the first character in the string that is not a digit, starting at offset.
*/
private fun indexOfNonDigit(string: String, offset: Int): Int {
for (i in offset until string.length) {
val c = string[i]
if (c < '0' || c > '9') return i
}
return string.length
}

56
app/src/main/java/com/keylesspalace/tusky/json/Rfc3339DateJsonAdapter.kt
View file

@ -1,56 +0,0 @@
// https://github.com/google/gson/blob/master/extras/src/main/java/com/google/gson/typeadapters/UtcDateTypeAdapter.java
/*
* Copyright (C) 2011 Google Inc.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package com.keylesspalace.tusky.json
import android.util.Log
import com.google.gson.JsonParseException
import com.google.gson.TypeAdapter
import com.google.gson.stream.JsonReader
import com.google.gson.stream.JsonToken
import com.google.gson.stream.JsonWriter
import java.io.IOException
import java.util.Date
class Rfc3339DateJsonAdapter : TypeAdapter<Date?>() {
@Throws(IOException::class)
override fun write(writer: JsonWriter, date: Date?) {
if (date == null) {
writer.nullValue()
} else {
writer.value(date.formatIsoDate())
}
}
@Throws(IOException::class)
override fun read(reader: JsonReader): Date? {
return when (reader.peek()) {
JsonToken.NULL -> {
reader.nextNull()
null
}
else -> {
try {
reader.nextString().parseIsoDate()
} catch (jpe: JsonParseException) {
Log.w("Rfc3339DateJsonAdapter", jpe)
null
}
}
}
}
}