The app currently uses a custom callback system for requesting
permissions, located in `BaseActivity`.
This code doesn't work when the activity gets re-created before the
permission is granted: the callback will be lost with the Activity and
no action will be performed after the permission is granted.
To avoid these issues while still simplifying the code, Google
recommends to use the ActivityResultContract APIs to request
permissions, which allow to register persistent callbacks. This pull
request removes the legacy API and replaces the calls with
`registerForActivityResult(ActivityResultContracts.RequestPermission())`.
- `ActivityResultContracts.RequestPermission` will check if the
permission is already granted and call the callback synchronously when
possible. So this check doesn't need to be performed anymore.
- In `SearchStatusesFragment` and `SFragment`, the download action to
launch after granting the permission requires an argument (a list of
media URLs). This argument is temporarily stored in a Fragment field and
saved and restored as part of the instance state, so the action can
properly resume after an Activity re-creation.
This refactors the NotificationsFragment and related classes to Kotlin &
paging.
While trying to preserve as much of the original behavior as possible,
this adds the following improvements as well:
- The "show notifications filter" preference was added again
- The "load more" button now has a background ripple effect when clicked
- The "legal" report category of Mastodon 4.2 is now supported in report
notifications
- Unknown notifications now display "unknown notification type" instead
of an empty line
Other code quality improvements:
- All views from xml layouts are now referenced via ViewBindings
- the classes responsible for showing system notifications were moved to
a new package `systemnotifications` while the classes from this
refactoring are in `notifications`
- the id of the local Tusky account is now called `tuskyAccountId` in
all places I could find
closes https://github.com/tuskyapp/Tusky/issues/3429
---------
Co-authored-by: Zongle Wang <wangzongler@gmail.com>
- Use `TooltipCompat.setTooltipText()` instead of setting an
`OnLongClickListener` showing a Toast, to show the attachment
description. This method will display native tooltips on API 26+, and
set an `OnLongClickListener` on older versions to display a special
Toast anchored to the view. In both cases this provides a better user
experience.
- Simplify `Attachment.getFormattedDescription()` by using Kotlin's
`Duration`. Since it's an inline class, no extra memory will be
allocated on the heap. Also, ensure that the calculation of minutes and
hours use the rounded number of seconds instead of the non-rounded one.
Steps to reproduce: Quickly unfollow a bunch of hashtags, try clicking
unfollow buttons multiple times. If you are faster than the network, the
app crashes.
```
java.lang.ArrayIndexOutOfBoundsException: length=20; index=-1
at java.util.ArrayList.remove(ArrayList.java:506)
at com.keylesspalace.tusky.components.followedtags.FollowedTagsActivity$unfollow$1.invokeSuspend(FollowedTagsActivity.kt:152)
at kotlin.coroutines.jvm.internal.BaseContinuationImpl.resumeWith(ContinuationImpl.kt:33)
at kotlinx.coroutines.DispatchedTask.run(DispatchedTask.kt:104)
at android.os.Handler.handleCallback(Handler.java:942)
at android.os.Handler.dispatchMessage(Handler.java:99)
at android.os.Looper.loopOnce(Looper.java:201)
at android.os.Looper.loop(Looper.java:288)
at android.app.ActivityThread.main(ActivityThread.java:7872)
at java.lang.reflect.Method.invoke(Native Method)
at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:548)
at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:936)
```
Fragments don't necessarily have the same lifecycle as their views, so
their is a rare race condition when collecting a Flow from the
`lifecycleScope` of the Fragment where the view is already disposed but
the collect lambda is still executed. Accessing the view from such a
lambda can therefore led to a crash.
Collecting from the `viewLifecycleOwner.lifecycleScope` fixes the
problem.
```
Exception java.lang.IllegalStateException:
at androidx.fragment.app.Fragment.requireView (Fragment.java:2062)
at com.keylesspalace.tusky.util.ViewLifecycleLazy.getValue (ViewBindingExtensions.kt:32)
at com.keylesspalace.tusky.components.viewthread.ViewThreadFragment.getBinding (ViewThreadFragment.kt:79)
at com.keylesspalace.tusky.components.viewthread.ViewThreadFragment.access$getBinding (ViewThreadFragment.kt:67)
at com.keylesspalace.tusky.components.viewthread.ViewThreadFragment$onViewCreated$3$1.emit (ViewThreadFragment.java:250)
at com.keylesspalace.tusky.components.viewthread.ViewThreadFragment$onViewCreated$3$1.emit (ViewThreadFragment.java:248)
at kotlinx.coroutines.flow.SharedFlowImpl.collect$suspendImpl (SharedFlow.kt:392)
at kotlinx.coroutines.flow.SharedFlowImpl$collect$1.invokeSuspend (SharedFlow.kt:13)
at kotlin.coroutines.jvm.internal.BaseContinuationImpl.resumeWith (ContinuationImpl.kt:33)
at kotlinx.coroutines.DispatchedTaskKt.resume (DispatchedTask.kt:231)
at kotlinx.coroutines.DispatchedTaskKt.resumeUnconfined (DispatchedTask.kt:187)
at kotlinx.coroutines.DispatchedTaskKt.dispatch (DispatchedTask.kt:159)
at kotlinx.coroutines.CancellableContinuationImpl.dispatchResume (CancellableContinuationImpl.kt:470)
at kotlinx.coroutines.CancellableContinuationImpl.resumeImpl (CancellableContinuationImpl.kt:504)
at kotlinx.coroutines.CancellableContinuationImpl.resumeImpl$default (CancellableContinuationImpl.kt:493)
at kotlinx.coroutines.CancellableContinuationImpl.resumeWith (CancellableContinuationImpl.kt:364)
at kotlinx.coroutines.flow.SharedFlowImpl.tryEmit (SharedFlow.kt:409)
at kotlinx.coroutines.flow.SharedFlowImpl.emit$suspendImpl (SharedFlow.kt:414)
at kotlinx.coroutines.flow.SharedFlowImpl.emit (SharedFlow.kt:1)
at com.keylesspalace.tusky.components.viewthread.ViewThreadViewModel$loadThread$1.invokeSuspend (ViewThreadViewModel.kt:172)
at kotlin.coroutines.jvm.internal.BaseContinuationImpl.resumeWith (ContinuationImpl.kt:33)
at kotlinx.coroutines.DispatchedTask.run (DispatchedTask.kt:104)
at android.os.Handler.handleCallback (Handler.java:938)
at android.os.Handler.dispatchMessage (Handler.java:99)
at android.os.Looper.loopOnce (Looper.java:210)
at android.os.Looper.loop (Looper.java:299)
at android.app.ActivityThread.main (ActivityThread.java:8319)
at java.lang.reflect.Method.invoke
at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run (RuntimeInit.java:556)
at com.android.internal.os.ZygoteInit.main (ZygoteInit.java:1038)
```
Currently translated at 100.0% (639 of 639 strings)
Translated using Weblate (Turkish)
Currently translated at 100.0% (639 of 639 strings)
Co-authored-by: Ümit Solmaz <usnotv@gmail.com>
Translate-URL: https://weblate.tusky.app/projects/tusky/tusky/tr/
Translation: Tusky/Tusky
Akkoma does not always set all attributes for Cards and when they are
loaded from the database, the app crashes. When they are loaded from the
network, Tusky displays an error. Adding more default values fixes the
problem.
```
com.squareup.moshi.JsonDataException: Required value 'authorName' (JSON name 'author_name') missing at $
at com.squareup.moshi.internal.Util.missingProperty(Util.java:660)
at com.keylesspalace.tusky.entity.CardJsonAdapter.fromJson(CardJsonAdapter.kt:122)
at com.keylesspalace.tusky.entity.CardJsonAdapter.fromJson(CardJsonAdapter.kt:22)
at com.squareup.moshi.internal.NullSafeJsonAdapter.fromJson(NullSafeJsonAdapter.java:41)
at com.squareup.moshi.JsonAdapter.fromJson(JsonAdapter.java:70)
at com.keylesspalace.tusky.components.timeline.TimelineTypeMappersKt.toViewData(TimelineTypeMappers.kt:168)
at com.keylesspalace.tusky.components.timeline.viewmodel.CachedTimelineViewModel$statuses$2$1.invoke(CachedTimelineViewModel.kt:110)
at com.keylesspalace.tusky.components.timeline.viewmodel.CachedTimelineViewModel$statuses$2$1.invoke(CachedTimelineViewModel.kt:108)
at androidx.paging.PagingDataTransforms$map$2$1$1.invokeSuspend(PagingDataTransforms.kt:58)
at androidx.paging.PagingDataTransforms$map$2$1$1.invoke(Unknown Source:8)
at androidx.paging.PagingDataTransforms$map$2$1$1.invoke(Unknown Source:2)
at androidx.paging.PageEvent$Insert.map(PageEvent.kt:128)
at androidx.paging.PagingDataTransforms$map$2$1.invokeSuspend(PagingDataTransforms.kt:58)
at kotlin.coroutines.jvm.internal.BaseContinuationImpl.resumeWith(ContinuationImpl.kt:33)
at kotlinx.coroutines.DispatchedTask.run(DispatchedTask.kt:104)
at kotlinx.coroutines.scheduling.CoroutineScheduler.runSafely(CoroutineScheduler.kt:585)
at kotlinx.coroutines.scheduling.CoroutineScheduler$Worker.executeTask(CoroutineScheduler.kt:802)
at kotlinx.coroutines.scheduling.CoroutineScheduler$Worker.runWorker(CoroutineScheduler.kt:706)
at kotlinx.coroutines.scheduling.CoroutineScheduler$Worker.run(CoroutineScheduler.kt:693)
Suppressed: kotlinx.coroutines.internal.DiagnosticCoroutineContextException: [StandaloneCoroutine{Cancelling}@de2cbd5, Dispatchers.Main.immediate]
```
closes#4393
Mastodon returns different reponses when posting normally and when
scheduling. This was previously ignored silently, but Moshi is more
correct than Gson and fails, which causes the `SendStatusService` to
retry sending forever and a lot of posts are scheduled.
Mastodon should actually ignore multiple attempts at scheduling the same
post, but doesn't so I filed this
https://github.com/mastodon/mastodon/issues/30039
cc @cbeyls
The only crash so far in the 25.0-beta1 crash reports. Probably not a
regression though as that code did not change in a while.
```
Exception java.lang.IllegalArgumentException: Max number of dynamic shortcuts exceeded
at android.os.Parcel.createExceptionOrNull (Parcel.java:3032)
at android.os.Parcel.createException (Parcel.java:3012)
at android.os.Parcel.readException (Parcel.java:2995)
at android.os.Parcel.readException (Parcel.java:2937)
at android.content.pm.IShortcutService$Stub$Proxy.addDynamicShortcuts (IShortcutService.java:618)
at android.content.pm.ShortcutManager.addDynamicShortcuts (ShortcutManager.java:240)
at androidx.core.content.pm.ShortcutManagerCompat.addDynamicShortcuts (ShortcutManagerCompat.java:334)
at com.keylesspalace.tusky.util.ShareShortcutHelper$updateShortcut$1.invokeSuspend (ShareShortcutHelper.kt:96)
at kotlin.coroutines.jvm.internal.BaseContinuationImpl.resumeWith (ContinuationImpl.kt:33)
at kotlinx.coroutines.DispatchedTask.run (DispatchedTask.kt:104)
at android.os.Handler.handleCallback (Handler.java:984)
at android.os.Handler.dispatchMessage (Handler.java:104)
at android.os.Looper.loopOnce (Looper.java:238)
at android.os.Looper.loop (Looper.java:357)
at android.app.ActivityThread.main (ActivityThread.java:8094)
at java.lang.reflect.Method.invoke
at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run (RuntimeInit.java:548)
at com.android.internal.os.ZygoteInit.main (ZygoteInit.java:957)
Caused by android.os.RemoteException: Remote stack trace:
at com.android.server.pm.ShortcutService.enforceMaxActivityShortcuts (ShortcutService.java:1768)
at com.android.server.pm.ShortcutPackage.enforceShortcutCountsBeforeOperation (ShortcutPackage.java:1551)
at com.android.server.pm.ShortcutService.addDynamicShortcuts (ShortcutService.java:2161)
at android.content.pm.IShortcutService$Stub.onTransact (IShortcutService.java:281)
at android.os.Binder.execTransactInternal (Binder.java:1294)
```
All subtitle formats used to be enabled by default in older versions of
media3.
After the media3 library was upgraded to 1.3.0, Extractors should
specify a `SubtitleParser.Factory` explicitly. By default, no subtitle
formats are supported when using the now deprecated empty constructor of
Extractors.
Limit support to **WebVTT** and **TTML** which are the only true web
standards amongst all the subtitle formats supported by ExoPlayer.
Note that only subtitles embedded in MP4 and WebM files are supported
until Mastodon provides a way to upload subtitles separately.
- Read license resource using Okio inside a coroutine (instead of the
main thread) in `LicenseActivity`
- Use Okio and its buffer system to copy ContentProvider streams and
files to a temporary file in `MediaUploader.prepareMedia()`
- Properly close the input file after copying it to a temporary file in
`MediaUploader.prepareMedia()`
- Properly close sink in case of null body source during file copy in
`Uri.copyToFolder()` in `DraftHelper.kt`
- Add comment explaining the current value of `DEFAULT_CHUNK_SIZE` in
`UriRequestBody.kt` and indent the file properly
- Replace hardcoded `Charset` and `Int` byte size with the proper
constants, and align the `hashCode()` implementation with other
`BitmapTransformation` implementations in
`CompositeWithOpaqueBackground`
- Properly close `InputStream` in case of error during Bitmap size
decoding in `getImageSquarePixels()`
- return `Int` instead of `Long` in `getImageSquarePixels()`, since the
current code simply converts the `Int` result to a `Long` _after_
multiplication and not before (and `Int.MAX_VALUE` is already way above
the maximum number of pixels a decoded Bitmap could return)
- Simplify `getImageOrientation()`
- Add explicit dependency to the Okio library and upgrade it to its
latest version.
This pull request takes advantage of the Okio library to simplify, fix
or improve performance of some I/O related code in Tusky.
- Return early or throw `FileNotFoundException` early in case
`contentResolver.openInputStream()` returns `null` instead of throwing
`NullPointerException` later. Change the signature of
`Closeable.closeQuietly()` to only accept a non-null `Closeable`.
- Reimplement `Uri.copyToFile()` using Okio. This takes advantage of the
built-in high-performance buffers of the library so a buffer doesn't
need to be allocated or managed manually. The new implementation also
makes sure that the input and output streams are always closed, as the
original code could in some cases return without properly closing a
stream.
- Reimplement `ProgressRequestBody` as `Uri.asRequestBody()` (adding to
the existing extension functions available in the Okio library to create
a `RequestBody`). The new implementation uses Okio's `Buffer` instead of
a manually managed byte array, which allows to avoid copying bytes from
one buffer to the next. The max number of bytes read at once was
increased from 2K to 8K to improve performance. Avoid division by zero
in case `contentLength` is `0`. Finally, this implementation now takes a
`Uri` as input instead of an `InputStream`, because a `RequestBody` must
be replayable in case Okio retries the request, and an `InputStream` can
only be used once.
This also improves randomness by avoiding to reinitialize the random
number generator repeatedly from a seed based on the current time.
Typically, if the number generator is reinitialized repeatedly at
non-random times (like multiple times in a row), then generated numbers
have a higher chance of repeating.
The Kotlin Random object is only initialized once, using the best seed
available for the current Android version.
closes#4361
```
com.squareup.moshi.JsonDataException: Required value 'width' missing at $.statuses[0].media_attachments[0].meta.original
at com.squareup.moshi.internal.Util.missingProperty(Util.java:660)
at com.keylesspalace.tusky.entity.Attachment_SizeJsonAdapter.fromJson(Attachment_SizeJsonAdapter.kt:81)
at com.keylesspalace.tusky.entity.Attachment_SizeJsonAdapter.fromJson(Attachment_SizeJsonAdapter.kt:23)
at com.squareup.moshi.internal.NullSafeJsonAdapter.fromJson(NullSafeJsonAdapter.java:41)
at com.keylesspalace.tusky.entity.Attachment_MetaDataJsonAdapter.fromJson(Attachment_MetaDataJsonAdapter.kt:64)
at com.keylesspalace.tusky.entity.Attachment_MetaDataJsonAdapter.fromJson(Attachment_MetaDataJsonAdapter.kt:23)
at com.squareup.moshi.internal.NullSafeJsonAdapter.fromJson(NullSafeJsonAdapter.java:41)
at com.keylesspalace.tusky.entity.AttachmentJsonAdapter.fromJson(AttachmentJsonAdapter.kt:66)
at com.keylesspalace.tusky.entity.AttachmentJsonAdapter.fromJson(AttachmentJsonAdapter.kt:22)
at com.squareup.moshi.internal.NullSafeJsonAdapter.fromJson(NullSafeJsonAdapter.java:41)
at com.squareup.moshi.CollectionJsonAdapter.fromJson(CollectionJsonAdapter.java:81)
at com.squareup.moshi.CollectionJsonAdapter$2.fromJson(CollectionJsonAdapter.java:55)
at com.squareup.moshi.internal.NullSafeJsonAdapter.fromJson(NullSafeJsonAdapter.java:41)
at com.keylesspalace.tusky.entity.StatusJsonAdapter.fromJson(StatusJsonAdapter.kt:195)
at com.keylesspalace.tusky.entity.StatusJsonAdapter.fromJson(StatusJsonAdapter.kt:26)
at com.squareup.moshi.internal.NullSafeJsonAdapter.fromJson(NullSafeJsonAdapter.java:41)
at com.squareup.moshi.CollectionJsonAdapter.fromJson(CollectionJsonAdapter.java:81)
at com.squareup.moshi.CollectionJsonAdapter$2.fromJson(CollectionJsonAdapter.java:55)
at com.squareup.moshi.internal.NullSafeJsonAdapter.fromJson(NullSafeJsonAdapter.java:41)
at com.keylesspalace.tusky.entity.SearchResultJsonAdapter.fromJson(SearchResultJsonAdapter.kt:51)
at com.keylesspalace.tusky.entity.SearchResultJsonAdapter.fromJson(SearchResultJsonAdapter.kt:21)
at com.squareup.moshi.internal.NullSafeJsonAdapter.fromJson(NullSafeJsonAdapter.java:41)
at retrofit2.converter.moshi.MoshiResponseBodyConverter.convert(MoshiResponseBodyConverter.java:46)
at retrofit2.converter.moshi.MoshiResponseBodyConverter.convert(MoshiResponseBodyConverter.java:27)
at retrofit2.OkHttpCall.parseResponse(OkHttpCall.java:246)
at retrofit2.OkHttpCall$1.onResponse(OkHttpCall.java:156)
at okhttp3.internal.connection.RealCall$AsyncCall.run(RealCall.kt:519)
at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1145)
at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:644)
at java.lang.Thread.run(Thread.java:1012)
```
This helps playing some media even if there is a problem with the
primary decoder.
E.g. [this
video](https://mastodon.social/@krzyzanowskim/112208964123517040) fails
on my Fairphone 4 without this change.
<img
src="https://github.com/tuskyapp/Tusky/assets/10157047/215d932c-9ed1-4ee8-8be7-e6ca28ddec23"
width="200"/>
<details>
<summary>Stacktrace</summary>
```
androidx.media3.exoplayer.ExoPlaybackException: MediaCodecVideoRenderer error, index=0, format=Format(1, null, null, video/avc, avc1.640034, -1, null, [1920, 1440, 119.99593, ColorInfo(BT709, Limited range, sRGB, false, 8bit Luma, 8bit Chroma)], [-1, -1]), format_supported=YES
at androidx.media3.exoplayer.ExoPlayerImplInternal.handleMessage(ExoPlayerImplInternal.java:620)
at android.os.Handler.dispatchMessage(Handler.java:102)
at android.os.Looper.loopOnce(Looper.java:201)
at android.os.Looper.loop(Looper.java:288)
at android.os.HandlerThread.run(HandlerThread.java:67)
Caused by: androidx.media3.exoplayer.mediacodec.MediaCodecRenderer$DecoderInitializationException: Decoder init failed: OMX.qcom.video.decoder.avc, Format(1, null, null, video/avc, avc1.640034, -1, null, [1920, 1440, 119.99593, ColorInfo(BT709, Limited range, sRGB, false, 8bit Luma, 8bit Chroma)], [-1, -1])
at androidx.media3.exoplayer.mediacodec.MediaCodecRenderer.maybeInitCodecWithFallback(MediaCodecRenderer.java:1114)
at androidx.media3.exoplayer.mediacodec.MediaCodecRenderer.maybeInitCodecOrBypass(MediaCodecRenderer.java:551)
at androidx.media3.exoplayer.mediacodec.MediaCodecRenderer.onInputFormatChanged(MediaCodecRenderer.java:1560)
at androidx.media3.exoplayer.video.MediaCodecVideoRenderer.onInputFormatChanged(MediaCodecVideoRenderer.java:1152)
at androidx.media3.exoplayer.mediacodec.MediaCodecRenderer.readSourceOmittingSampleData(MediaCodecRenderer.java:994)
at androidx.media3.exoplayer.mediacodec.MediaCodecRenderer.render(MediaCodecRenderer.java:814)
at androidx.media3.exoplayer.video.MediaCodecVideoRenderer.render(MediaCodecVideoRenderer.java:940)
at androidx.media3.exoplayer.ExoPlayerImplInternal.doSomeWork(ExoPlayerImplInternal.java:1102)
at androidx.media3.exoplayer.ExoPlayerImplInternal.handleMessage(ExoPlayerImplInternal.java:541)
at android.os.Handler.dispatchMessage(Handler.java:102)
at android.os.Looper.loopOnce(Looper.java:201)
at android.os.Looper.loop(Looper.java:288)
at android.os.HandlerThread.run(HandlerThread.java:67)
Caused by: android.media.MediaCodec$CodecException: Error 0xfffffff4
at android.media.MediaCodec.native_configure(Native Method)
at android.media.MediaCodec.configure(MediaCodec.java:2215)
at android.media.MediaCodec.configure(MediaCodec.java:2131)
at androidx.media3.exoplayer.mediacodec.AsynchronousMediaCodecAdapter.initialize(AsynchronousMediaCodecAdapter.java:174)
at androidx.media3.exoplayer.mediacodec.AsynchronousMediaCodecAdapter.access$100(AsynchronousMediaCodecAdapter.java:54)
at androidx.media3.exoplayer.mediacodec.AsynchronousMediaCodecAdapter$Factory.createAdapter(AsynchronousMediaCodecAdapter.java:119)
at androidx.media3.exoplayer.mediacodec.DefaultMediaCodecAdapterFactory.createAdapter(DefaultMediaCodecAdapterFactory.java:117)
at androidx.media3.exoplayer.mediacodec.MediaCodecRenderer.initCodec(MediaCodecRenderer.java:1195)
at androidx.media3.exoplayer.mediacodec.MediaCodecRenderer.maybeInitCodecWithFallback(MediaCodecRenderer.java:1103)
at androidx.media3.exoplayer.mediacodec.MediaCodecRenderer.maybeInitCodecOrBypass(MediaCodecRenderer.java:551)
at androidx.media3.exoplayer.mediacodec.MediaCodecRenderer.onInputFormatChanged(MediaCodecRenderer.java:1560)
at androidx.media3.exoplayer.video.MediaCodecVideoRenderer.onInputFormatChanged(MediaCodecVideoRenderer.java:1152)
at androidx.media3.exoplayer.mediacodec.MediaCodecRenderer.readSourceOmittingSampleData(MediaCodecRenderer.java:994)
at androidx.media3.exoplayer.mediacodec.MediaCodecRenderer.render(MediaCodecRenderer.java:814)
at androidx.media3.exoplayer.video.MediaCodecVideoRenderer.render(MediaCodecVideoRenderer.java:940)
at androidx.media3.exoplayer.ExoPlayerImplInternal.doSomeWork(ExoPlayerImplInternal.java:1102)
at androidx.media3.exoplayer.ExoPlayerImplInternal.handleMessage(ExoPlayerImplInternal.java:541)
at android.os.Handler.dispatchMessage(Handler.java:102)
at android.os.Looper.loopOnce(Looper.java:201)
at android.os.Looper.loop(Looper.java:288)
at android.os.HandlerThread.run(HandlerThread.java:67)
```
</details>
**! ! 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.
The flow must emit every update even if the values are the same, so use
SharedFlow instead of StateFlow.
Regression from https://github.com/tuskyapp/Tusky/pull/4337 cc @Goooler
Christophe Beyls
Konrad Pozniak
Conny Duck
Ümit Solmaz
Connyduck
Balázs Meskó
Weblate
XoseM
Zongle Wang
Weblate
fin-w
xzFantom
Deleted User
