Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor: Convert from Gson to Moshi #428

Merged
merged 1 commit into from
Feb 9, 2024
Merged

Conversation

nikclayton
Copy link
Contributor

Moshi is faster to decode JSON at runtime, is actively maintained, has a smaller memory and method footprint, and a slightly smaller APK size. Moshi also correctly creates default constructor arguments instead of leaving them null, which was a source of NullPointerExceptions when using Gson.

The conversion broadly consisted of:

  • Adding @JsonClass(generateAdapter = true) to data classes that marshall to/from JSON.

  • Replacing @SerializedName(value = ...) with @Json(name = ...).

  • Replacing Gson instances with Moshi in Retrofit, Hilt, and tests.

  • Using Moshi adapters to marshall to/from JSON instead of Gson toJson / fromJson.

  • Deleting Rfc3339DateJsonAdapter and related code, and using the equivalent adapter bundled with Moshi.

  • Rewriting GuardedBooleanAdapter as a more generic GuardedAdapter.

  • Deleting unused ProGuard rules; Moshi generates adapters using code generation, not runtime reflection.

The conversion surfaced some bugs which have been fixed.

  • Not all audio attachments have attachment size metadata. Don't show the attachment preview if the metadata is missing.

  • Some throwable were not being logged correctly.

  • The wrong type was being used when parsing the response when sending a scheduled status.

  • Exceptions other than HttpException or IoException would also cause a status to be resent. If there's a JSON error parsing a response the status would be repeatedly sent.

  • In tests strings containing error responses were not valid JSON.

  • Workaround Mastodon a bug and ensure filter.keywords is populated, FilterResult.filter.keywords not populated when included in a status' filtered property mastodon/mastodon#29142

Moshi is faster to decode JSON at runtime, is actively maintained, has
a smaller memory and method footprint, and a slightly smaller APK size.
Moshi also correctly creates default constructor arguments instead of
leaving them null, which was a source of `NullPointerExceptions` when
using Gson.

The conversion broadly consisted of:

- Adding `@JsonClass(generateAdapter = true)` to data classes that
  marshall to/from JSON.

- Replacing `@SerializedName(value = ...)` with `@Json(name = ...)`.

- Replacing Gson instances with Moshi in Retrofit, Hilt, and tests.

- Using Moshi adapters to marshall to/from JSON instead of Gson
  `toJson` / `fromJson`.

- Deleting `Rfc3339DateJsonAdapter` and related code, and using the
  equivalent adapter bundled with Moshi.

- Rewriting `GuardedBooleanAdapter` as a more generic `GuardedAdapter`.

- Deleting unused ProGuard rules; Moshi generates adapters using
  code generation, not runtime reflection.

The conversion surfaced some bugs which have been fixed.

- Not all audio attachments have attachment size metadata. Don't
  show the attachment preview if the metadata is missing.

- Some `throwable` were not being logged correctly.

- The wrong type was being used when parsing the response when
  sending a scheduled status.

- Exceptions other than `HttpException` or `IoException` would
  also cause a status to be resent. If there's a JSON error
  parsing a response the status would be repeatedly sent.

- In tests strings containing error responses were not valid
  JSON.

- Workaround Mastodon a bug and ensure `filter.keywords` is populated,
  mastodon/mastodon#29142
@nikclayton nikclayton merged commit a3d45ca into pachli:main Feb 9, 2024
6 checks passed
@nikclayton nikclayton deleted the moshi branch February 9, 2024 11:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant