square / moshi

A modern JSON library for Kotlin and Java.
https://square.github.io/moshi/1.x/
Apache License 2.0
9.77k stars 761 forks source link

Kotlin extensions for JsonReader and listAdapter and other suggestions #785

Open dave08 opened 5 years ago

dave08 commented 5 years ago

For JsonReader I currently have: https://gist.github.com/dave08/32632fe438998f845713df789115171d

And since there's a good number of uses for a simple List adapter and it's a multi-liner, it might be nice to add such an extension:

inline fun <reified T> Moshi.listAdapter(): JsonAdapter<List<T>> =
        Types.newParameterizedType(
                List::class.java, T::class.java
        ).let { this.adapter<List<T>>(it) }

It might also be a good idea to make an enum for JsonReader.Options.of() values and then provide something like this (selectName would handle the JsonReader.Options.of() conversions and key resolving... this could save quite a bit of bugs in adding keys and management of ids:


enum class OptionEnum { key1, key2 }

reader.selectName<OptionEnum> {
    when(it) {
        key1 -> do1()
        key2 -> do2()
    }
}
ZacSweers commented 5 years ago

I like the idea, but it can quickly become a footgun if T is also a generic type. Could check at runtime, but that's a bit late. Does kotlin give us more power with generic types to say something like "where T !is ParameterizedType"? Pretty sure it doesn't but thinking out loud just in case.

dave08 commented 5 years ago

True but probably not any worse than the current adapter function... if documented hopefully people will realize its limitations. Of course, if there's a way to limit it with some IDE lint or compile error, that would be even better.

swankjesse commented 5 years ago

I'd call this a seductive API. It makes the common cases nicer but makes uncommon cases awkward.

Thus far my preference has been to lean on libraries like Retrofit to call into Moshi for you.

dave08 commented 5 years ago

Which part are you referring to, the listAdapter or the JsonReader extensions?

Does retrofit handle the generics part better? If you still need a newParamerizedType to handle a simple List then why not make it easier... and if it does handle it, maybe it should be done the same way here. There's a few contexts where Moshi is used directly such as caching, where there's no other libraries that do it.

In the reader extensions, I don't see any real disadvantage, it just makes things less error prone using Kotlin features.

NightlyNexus commented 5 years ago
fun JsonReader.readJsonArray(block: JsonReader.() -> Unit) {
  beginArray()
  block()
  endArray()
}

fun JsonWriter.writeJsonArray(block: JsonWriter.() -> Unit) {
  beginArray()
  block()
  endArray()
}

for arrays and similar for objects are useful, since otherwise users need to remember to call endArray/endObject for JSON.

naturalwarren commented 5 years ago

Since this seems like the extension tracking issue I thought I'd throw this bit of sugar in that I'm using for fallback adapters:

inline fun <reified T : Enum<T>> Moshi.Builder.addFallbackAdapter(value: T) =
        this.apply {
            add(T::class.java, EnumJsonAdapter.create(T::class.java).withUnknownFallback(value))
        }

Thoughts?

swankjesse commented 5 years ago

Oooh it turns out Jackson does something quite neat with this function:

inline fun <reified T> jacksonTypeRef(): TypeReference<T> = object: TypeReference<T>() {}

This supports List<Long>!

JakeWharton commented 5 years ago

Just wait 4 weeks and Kotlin 1.3.40 will have typeOf() which handles this without effort.

On Sun, Apr 14, 2019, 2:31 PM Jesse Wilson notifications@github.com wrote:

Oooh it turns out Jackson does something quite neat:

https://github.com/FasterXML/jackson-module-kotlin/blob/master/src/main/kotlin/com/fasterxml/jackson/module/kotlin/Extensions.kt

This supports List!

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/square/moshi/issues/785#issuecomment-483036245, or mute the thread https://github.com/notifications/unsubscribe-auth/AAEEEfpxpEzX75Hvns9Zvz2WWetJpYpaks5vg3QAgaJpZM4Z3eyU .

PaulWoitaschek commented 5 years ago

So this typeOf is not really usable as it returns a KType, and we need reflection to get the java type?

ZacSweers commented 5 years ago

Yep I don't think ktype helps if you don't have kotlin-reflect

On Mon, Jun 24, 2019, 9:58 AM Paul Woitaschek notifications@github.com wrote:

So this typeOf is not really usable as it returns a KType, and we need reflection to get the java type?

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/square/moshi/issues/785?email_source=notifications&email_token=AAKMJPV6PLXQWZSH4YKDGRTP4DOJPA5CNFSM4GO55SKKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODYNGWWI#issuecomment-505047897, or mute the thread https://github.com/notifications/unsubscribe-auth/AAKMJPXUZA7UP33LSR5EEW3P4DOJPANCNFSM4GO55SKA .

ZacSweers commented 5 years ago

The next release will have a couple of typeOf()-using APIs available, namely fun <reified T> Moshi.adapter() and a fun <reified T> Moshi.Builder.add(adapter: JsonAdapter<T>). Unfortunately we can't really escape having to lean on kotlin-reflect for these, as almost everything in KType requires it.

We could add non-reflect APIs to the main artifact without a rewrite (similar to Retrofit), but unsure if that's a direction we'd want to head in.

This was punted

Talhaali00 commented 4 years ago

Here's a list of extension functions that I've been using and might be helpful to add.

object JsonExtensions {
    fun JsonWriter.jsonObject(objectName: String? = null, body: () -> Unit) {
        if (!objectName.isNullOrBlank()) {
            name(objectName)
        }

        beginObject()
        body()
        endObject()
    }

    fun JsonWriter.jsonArray(objectName: String? = null, body: () -> Unit) {
        if (!objectName.isNullOrBlank()) {
            name(objectName)
        }

        beginArray()
        body()
        endArray()
    }

    fun JsonWriter.attribute(name: String, value: Any?) {
        name(name)

        when (value) {
            is Long -> value(value)
            is Double -> value(value)
            is Number? -> value(value)
            is Boolean -> value(value)
            is Boolean? -> value(value)
            is String? -> value(value)
            is BufferedSource -> value(value)
            else -> throw Exception("Cannot Write Unsupported Type")
        }
    }

    fun JsonReader.jsonObject(body: () -> Unit) {
        beginObject()
        body()
        endObject()
    }

    fun JsonReader.jsonArray(body: () -> Unit) {
        beginArray()
        body()
        endArray()
    }
}