The Moshi converter option doesn't support moshi-codegen

[gobetti]

Hi, this plugin is super awesome but I found this bug today where all of the annotated properties in my generated model failed to be deserialised. As of the way the classes are generated today, one needs to use the KotlinJsonAdapterFactory:

.addConverterFactory(MoshiConverterFactory.create(Moshi.Builder().add(KotlinJsonAdapterFactory()).build()))

i.e. adding a dependency to kotlin-reflect, a considerably heavy library. It would be nice to just do:

.addConverterFactory(MoshiConverterFactory.create())

@tfcporciuncula pointed me to a simple solution which is using the more explicit annotation @field:Json instead of simply @Json.

EDIT:
however that is NOT a recommended solution! It would work with these Gradle dependencies:

implementation "com.squareup.retrofit2:retrofit:$retrofitVersion"
implementation "com.squareup.retrofit2:converter-moshi:$retrofitVersion"
implementation 'com.squareup.moshi:moshi-kotlin:1.6.0'

By supporting the moshi codegen, the need of KotlinJsonAdapterFactory can be removed and the simple annotation @Json can be kept, one just needs to annotate the class with @JsonClass(generateAdapter = true) and update the dependencies to:

implementation "com.squareup.retrofit2:retrofit:$retrofitVersion"
implementation "com.squareup.retrofit2:converter-moshi:$retrofitVersion"
implementation "com.squareup.moshi:moshi:$moshiVersion"
kapt "com.squareup.moshi:moshi-kotlin-codegen:$moshiVersion"

[tfcporciuncula]

Apparently that's not an official solution, though. So... my bad.

It seems one should either stick to moshi-kotlin or use the Kotlin codegen from Moshi 1.6. And a plain @Json works in both worlds. More info in their docs: https://github.com/square/moshi#kotlin

[wuseal]

@gobetti @tfcporciuncula Thanks for your feedback.Em. now you could use customize annotation option to solve your problem at first. (#19 )
I'll try to test the case you described. and fix the issue, if it does exist.

[wuseal]

@gobetti @tfcporciuncula As I tested.the result like this. It works with @Json

data class Model(
        @field:Json(name = "total_pages") val totalPages: Int = 0
)

data class NoFieldModel(
    @Json(name = "total_pages") val totalPages: Int = 0
)

Is there anything I was missing? 😢

[gobetti]

Hi @wuseal , yes, you are using KotlinJsonAdapterFactory when you set up moshi - as I mention at the beginning of this ticket, this could be avoidable by either using @field:Json or do the better solution mentioned by @tfcporciuncula above. I tested both solutions and neither required the KotlinJsonAdapterFactory, but both need modifications in the generated Kotlin class.

• here was my first solution which was to add KotlinJsonAdapterFactory
• then I could remove it by using @field:Json
• and finally I reverted to @Json after installing moshi-kotlin-codegen and adding @JsonClass(generateAdapter = true) to the generated classes

Thanks!

[wuseal]

@gobetti Yes, when using Moshi with kotlin we also add KotlinJsonAdapterFactory, Do you mean you won't like to use this method for the big size kotlin-reflect dependency? As you said you could make it by using @field:Json , Em. for this purpose, you could do this in the annotation tab setting

Then JsonToKotlinClassPlugin will generate data class as you want like this:

data class Model(
        @field:Json(name = "total_pages") val totalPages: Int = 0
)

Or as the last method you mentioned, you could do this setting

Then JsonToKotlinClassPlugin will generate data class as you want like this:

@JsonClass(generateAdapter = true)
data class Model(
        @Json(name = "total_pages") val totalPages: Int = 0
)

Hope it works for you 😄

[gobetti]

Hi @wuseal! Yes, I mean there is nowadays a better solution than using KotlinJsonAdapterFactory but it requires changes in the models - and since these models are generated by JsonToKotlinClass, I thought why not having these changes in ;)

It's okay that one can click "Others by customize" but when you already have an option "Moshi", IMHO selecting "Others" wouldn't be expected to be the correct choice.

Thanks again!

[wuseal]

@gobetti Did you meaning making @field:Json to be the default annotation when selecting Moshi option would be the correct choice? or the last solution, which one do you think would be better?

[gobetti]

Hi @wuseal, based on the comments in the issue shared by @tfcporciuncula the solution using codegen is the best, and it is also the last chronological commit I had when fixing the deserialization issue (shared in the comments above) :D

So adding the annotation for the class will be a final solution for this and should work for anyone using Moshi and Moshi codegen without requiring reflection 👍

[tfcporciuncula]

Bottom line is: the current behavior is correct and this issue can be closed :)

[wuseal]

@gobetti Yes, use codegen would be the most performance solution, Agree with you. but it is not the simplest way I think, the simplest way is just to use KotlinJsonAdapterFactory and the origin annotation, And the reflection option(Use KotlinJsonAdapterFactory ) is the first option in official documentation, So I'll let the Moshi Config keep nowadays state temporarily. If the official documentation was changed about this, we will keep the step with official documentation, And when that time welcomes to remind us to update Moshi default config. 😃

Thanks.

[gobetti]

@tfcporciuncula I'm not sure it should be closed because it is still a welcome change, but @wuseal I agree to postpone it until the documentation states that codegen is the only way to go.

Another option is to let users of this plugin to choose between codegen and reflection.

The reason why both options are still mentioned in the docs is this (quoting from there):

Prefer codegen for better performance and to avoid the kotlin-reflect dependency; prefer reflection to convert both private and protected properties

I'll update the title to reflect our discussion above. Thanks guys!

[tfcporciuncula]

Yeah, I was referring to the original issue (i.e. considering supporting @field:Json besides the plain @Json). Things changed a little and I agree it makes sense to do something about it.

the simplest way is just to use KotlinJsonAdapterFactory and the origin annotation, And the reflection option(Use KotlinJsonAdapterFactory ) is the first option in official documentation

I personally don't think these are strong arguments to keep things as they are today. If you look from other perspective, you can say the codegen alternative is actually simpler since you don't need to add any custom adapter. And it might be the first option in the docs just because codegen is more recent - it was added only in the last release.

Based on the docs, the only reason one should stick to reflection is if they have private and protected properties. Otherwise, codegen should be an obvious choice since we get more performance and less dependencies.

[wuseal]

@tfcporciuncula First of all, thank you for your answer.And then again, you said that this feature was recently added, so in order to maintain compatibility, in order to maintain and the previous functional finance, I think it would be better to use the current annotation.Because one of our goals is to enable users to achieve their goals more smoothly, so that most users can quickly and smoothly achieve their goals.If you change it to a new annotation, then the previous version of the library, it may not resolve the new annotation.This will lead to new problems, the compiler did not pass.

[gobetti]

@wuseal what about my previous comment:

Another option is to let users of this plugin to choose between codegen and reflection.

[wuseal]

@gobetti How to choose? Add another option?

[gobetti]

Yes, today we have a single option called Moshi which implies using reflection.
There is no option for Moshi with codegen, even though Moshi’s docs provide two options as you pointed out.
Can we have two options instead of just Moshi?

• Moshi with reflection
• Moshi with codegen

[wuseal]

@gobetti Ok, Let's add Moshi (codegen) option in next version

[gobetti]

Hi @wuseal , I noticed this was released in 2.3.0, thank you!
(You may want to remove the "fixed but not release" label 😉)