square / moshi

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

Moshi 1.5 ignores @Json annotation on Kotlin class properties #315

Closed consp1racy closed 6 years ago

consp1racy commented 7 years ago
@Keep
class RestUserSubscriptionPackage(
        val id: Int,
        @Json(name = "package_id") val package_prototype_id: Int,
        @Json(name = "package") val package_prototype: RestSubscriptionPackage?,
        val job_credit: JobCredit,
        val starts_at: OffsetDateTime,
        val ends_at: OffsetDateTime
)

package_prototype_id is always 0, package_prototype is always null.

Works out-of-the-box in Moshi 1.4.

Workaround: @field:Json

swankjesse commented 7 years ago

Try adding moshi-kotlin as a dependency and including it when you configure your Moshi instance.

    val moshi = Moshi.Builder()
        .add(KotlinJsonAdapterFactory())
        .build()
consp1racy commented 7 years ago

Not an option at the time, it crashes at runtime with Reflection on built-in Kotlin types is not yet fully supported. No metadata found for [compareTo(other: something): Int]. When I pinpoint what's happening, I'll open another issue.

swankjesse commented 7 years ago

Please do. Yeah, we wanna make sure this stuff all works well.

ligi commented 7 years ago

Dam - moshi-kotlin is not yet an option on Android as for the size of the kotlin-reflect dependency - makes me need to go back to 1.4.0 for this reason.

consp1racy commented 7 years ago

@ligi Have you tested after proguard?

ligi commented 7 years ago

@consp1racy I did not really test - was reading https://medium.com/square-corner-blog/kotlins-a-great-language-for-json-fcd6ef99256b

If you’re using JSON, Moshi and Kotlin help you to build better models with less code. Note that moshi-kotlin uses kotlin-reflect for property binding. That dependency is large by Android standards (1.7 MiB / 11,500 methods). We’re thinking of creative ways to address that!

jaredsburrows commented 7 years ago

@consp1racy Thanks for posting this. I didn't want to roll back to 1.4.0. I have tested 1.4.0 and it does work out the box. Thanks for the @field:Json work around.

2 current solutions:

I completely agree with @ligi, the moshi-kotlin dependency is too large. We should not have to add another dependency that includes transitive dependencies in order to solve this issue. See the dependency count here: http://www.methodscount.com/?lib=com.squareup.moshi%3Amoshi-kotlin%3A1.5.0.

@swankjesse, though we may not retain most of these methods post proguard, it will most likely force many development builds to use multidex, which we do not want.

JakeWharton commented 7 years ago

The method count issue aside, all development builds should be using native multidex.

On Sun, Jul 2, 2017, 6:39 PM Jared Burrows notifications@github.com wrote:

@consp1racy https://github.com/consp1racy Thanks for posting this. I didn't want to roll back to 1.4.0. I have tested 1.4.0 and it does work out the box. Thanks for the @field:Json work around.

I completely agree with @ligi https://github.com/ligi, the moshi-kotlin dependency is too large. We should not have to add another dependency that includes transitive dependencies in order to solve this issue. See the dependency count here: http://www.methodscount.com/?lib=com.squareup.moshi%3Amoshi-kotlin%3A1.5.0 .

  • com.squareup.moshi:moshi-kotlin:1.5.0 - 23043 (total)
    • org.jetbrains.kotlin:kotlin-reflect:1.1.1 - 15095 (big one)

@swankjesse https://github.com/swankjesse, though we may not retain most of these methods post proguard, it will most likely force many development builds to use multidex, which we do not want.

— 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/315#issuecomment-312521380, or mute the thread https://github.com/notifications/unsubscribe-auth/AAEEEZeWJTfdOHn0ecfG7TMesbqDxJ_hks5sKBwigaJpZM4NoAO4 .

dsdebastiani commented 7 years ago

Same problem here even with moshi-kotlin dependencies.

Dependencies:

implementation "com.squareup.moshi:moshi-kotlin:1.5.0"
implementation "com.squareup.moshi:moshi-adapters:1.5.0"

Moshi config:

val moshi = Moshi.Builder()
                .add(KotlinJsonAdapterFactory())
                .add(RealmListJsonAdapterFactory())
                .add(Date::class.java, Rfc3339DateJsonAdapter())
                .build()

And the attributes:

 @Json(name = "picture_url")
    var pictureUrl: String? = null
NightlyNexus commented 7 years ago

@dsdebastiani Can you make a minimum reproducible test case to show what's wrong?

recoverrelax commented 7 years ago

data class LoginSessionAuthDto( @Json(name = "access_token") val accessToken: String )

For me this works in debug mode, but in release it says accessToken can't be null. that class isn't being obfuscated, and i have the proper proguard rules (as stated in the main page). Only with @field:Json it works

dsdebastiani commented 7 years ago

Some solution for this? Only works with @field:Json annotation.

@field:Json(name = "delivery_time")
var deliveryTime: Int? = null
NightlyNexus commented 7 years ago

@dsdebastiani Are you using the KotlinJsonAdapterFactory? If so, can you provide a test case?

The following works:

@Test fun foo() {
  class Foo {
    @Json(name = "delivery_time")
    var deliveryTime: Int? = null
  }
  val moshi = Moshi.Builder().add(KotlinJsonAdapterFactory()).build()
  val adapter = moshi.adapter(Foo::class.java)
  assertThat(adapter.fromJson("{\"delivery_time\":60}")!!.deliveryTime).isEqualTo(60)
  assertThat(adapter.fromJson("{}")!!.deliveryTime).isNull()
  assertThat(adapter.toJson(Foo())).isEqualTo("{}")
  assertThat(adapter.toJson(Foo().apply { deliveryTime = 60 })).isEqualTo("{\"delivery_time\":60}")
}
GediminasZukas commented 7 years ago

I have the same problem when trying to convert API field (in snake case) to entity class field (in camel case). Some example cases:

@Json(name = "url_z") val urlImage640: String? - doesn't work (urlImage640 is null) @field:Json(name = "url_z") val urlImage640: String? - doesn't work (urlImage640 is null) @Json(name = "url_z") val urlZ: String? - doesn't work (urlZ is null) @Json(name = "url_z") val url_z: String? - Works @Json(name = "owner") val ownerId: String - Works

Project info:

implementation 'com.squareup.retrofit2:converter-moshi:2.3.0'
implementation 'com.squareup.moshi:moshi-kotlin:1.5.0'

Retrofit & Moshi configuration in Dagger Di module:

    @Singleton
    @Provides
    fun provideJsonConverter(): Moshi {
        return Moshi.Builder()
                .add(KotlinJsonAdapterFactory())
                .build()
    }

    @Singleton
    @Provides
    fun provideRestClient(moshi: Moshi): Retrofit {
        return Retrofit.Builder()
                .baseUrl(BuildConfig.FLICKR_ENDPOINT)
                .addConverterFactory(MoshiConverterFactory.create(moshi))
                .addCallAdapterFactory(RxJava2CallAdapterFactory.create())
                .build()
    }

Json example:

{
  "photos": {   
    "photo": [
      {
        "id": "1111",
        "owner": "11111DD",
        "title": "sss",
        "ownername": "bob",
        "iconserver": "0001",
        "iconfarm": 1,
        "url_z": "urlz",
        "url_n": "urln",  
        "url_m": "urlm"
      }
    ]
  }
}  

Entity classes for this Json parsing:

data class PhotosResponse constructor(@Json(name = "photos") val photosEntity: PhotosEntity)

data class PhotosEntity constructor(@Json(name = "photo") val photoList: List<PhotoEntity>)

data class PhotoEntity constructor(@Json(name = "id") val id: Long,
                              @Json(name = "title") val title: String,
                              @Json(name = "url_m") val urlImage240: String?,
                              @Json(name = "url_n") val urlImage320: String?,
                              @Json(name = "url_z") val urlImage640: String?,
                              @Json(name = "ownername") val ownerName: String,
                              @Json(name = "owner") val ownerId: String,
                              @Json(name = "iconserver") val iconServer: Long,
                              @Json(name = "iconfarm") val iconFarm: Long)

Is it a bug or I am doing something wrong?

NightlyNexus commented 7 years ago

@GediminasZukas

@Test fun f() {
  val moshi = Moshi.Builder().add(KotlinJsonAdapterFactory()).build()
  val adapter = moshi.adapter(PhotosResponse::class.java)
  val response = adapter.fromJson("""
    {
      "photos": {
        "photo": [
          {
            "id": "1111",
            "owner": "11111DD",
            "title": "sss",
            "ownername": "bob",
            "iconserver": "0001",
            "iconfarm": 1,
            "url_z": "urlz",
            "url_n": "urln",
            "url_m": "urlm"
          }
        ]
      }
    }
  """.trimIndent())
  assertThat(response).isEqualTo(PhotosResponse(PhotosEntity(listOf(PhotoEntity(
      1111, "sss", "urlm", "urln", "urlz",
      "bob","11111DD", 1, 1)))))
}

passes.

What specifically doesn't work (thrown exception, unexpected result)? Can you form it in a test case?

GediminasZukas commented 7 years ago

@NightlyNexus , your test case clarifies everything. It was my side issue, thank you.

premnirmal commented 6 years ago

@GediminasZukas what was the fix for the issue on your side? I believe I'm facing the same issue.

png6 commented 6 years ago

@premnirmal bug in the KotlisJsonAdapterFactory? I noticed the @field:Json issue as well when proguard is used.

jaredsburrows commented 6 years ago

I am not sure why this is closed. In Moshi 1.4 we did not need to do the @field.

swankjesse commented 6 years ago

Moshi 1.4 didn’t support Kotlin. It sort of worked because Moshi’s did Java reflection on the objects. But it was fragile, especially around nulls.

jaredsburrows commented 6 years ago

@swankjesse Ok. Thanks for the explanation. Adding @field: is no big deal.

NightlyNexus commented 6 years ago

In Moshi 1.4 we did not need to do the @field.

right, cuz @Target for fields was removed from @Json. but, yeah, Kotlin was not officially supported.

bug in the KotlinJsonAdapterFactory? I noticed the @field:Json issue as well when proguard is used.

sounds like a ProGuard issue, not a bug here. adding the KotlinJsonAdapterFactory makes @Json work as expected.

png6 commented 6 years ago

@NightlyNexus so is this a matter of setting the correct rules?

NightlyNexus commented 6 years ago

@png6 Unsure. I don't use ProGuard, personally. Somebody on Stack Overflow or a ProGuard forum might know?

flux87 commented 6 years ago

Well, had the same issue ...

// Not working
@Json(name = "first_name")
val firstName: String = "" // VAL
// Working
@Json(name = "first_name")
var firstName: String = "" // VAR

Maybe because it was read-only ...

amitav13 commented 6 years ago

Is using @field:Json now the recommended way if we don't want to use moshi-kotlin?

swankjesse commented 6 years ago

Use moshi-kotlin. Nothing else is tested.

nachtien commented 6 years ago

@swankjesse, but that has the Kotlin reflect library and is huge right? Also a bit slow to initialize especially on older devices? We opted out of using it for these reasons.

swankjesse commented 6 years ago

Then use the Kotlin codegen from the most recent release?

nachtien commented 6 years ago

Does that fix this issue? We were going to bring in codgen for just the null support.

swankjesse commented 6 years ago

Yes, Kotlin support is the solution to mistargetted @Json annotations.

nachtien commented 6 years ago

Ok thanks 👌 On Sun, Jun 24, 2018 at 11:34 AM Jesse Wilson notifications@github.com wrote:

Yes, Kotlin support is the solution to mistargetted @Json annotations.

— 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/315#issuecomment-399777408, or mute the thread https://github.com/notifications/unsubscribe-auth/AAHH9l_EZ-0CZU2ARG9M-3X9II29JPwhks5t_9uZgaJpZM4NoAO4 .

abbasiehsan1991 commented 4 years ago

I just faced the same problem, I had some variables with underline like row_type and its value was null all the time. I fixed this problem by setting this annotation for my data class: @JsonClass(generateAdapter = true)

desgraci commented 4 years ago

@flux87 I had to add a var and use a dummy value to my fields.

using: implementation "com.squareup.moshi:moshi-kotlin:1.9.2"

jewom commented 4 years ago

Not working :

@Json(name = "image_url")
    val imageUrl: String

Working :

@field:Json(name = "image_url")
    val imageUrl: String

Do I have to use the field:Json anotation ?

yatoooon commented 1 year ago

same problem

ZacSweers commented 1 year ago

Locking this as it's no longer constructive. Please use discussions to ask usage questions, and if you have a bug to report please do so with a minimally reproducible sample.