Moshi: Honor Kotlin non-null properties

[stefanmedack]

When I deserialize a json array containing a null into a kotlin list which should not accept null values, I end up with a list which contains a null.

json

{
    "data": [null]
}

kotlin data class

data class VideoPage(val data: List<Video> = listOf())

I am not sure if this is considered a bug and if throwing an exception would be a good solution.
For my use case filtering null values would be the best fit.

[stefanmedack]

Remark for others having this problem: if your desired behaviour is to filter null values from your result list, you can write an Adapter wich parses the json into an intermediate data class which can contain nulls. This intermediate data class can be parsed into the desired result and while parsing, you filter all nulls with List.filterNotNull().

For my example the Adapter looks like this:

class VideoPageAdapter {
    @ToJson
    fun toJson(videoPage: VideoPage): VideoPageJson = VideoPageJson(videoPage.data)

    @FromJson
    fun fromJson(videoPageJson: VideoPageJson): VideoPage = VideoPage(videoPageJson.data.filterNotNull())

    data class VideoPageJson(val data: List<Video?> = listOf())
}

[swankjesse]

For what it's worth, I think the most maintainable approach is to validate objects returned from Moshi manually, and take appropriate action there. Your code will be simpler if you refuse unsupported values rather than patching them, but I have no idea what your system looks like.

[stefanmedack]

Thanks for the remarks, sounds reasonable. However it should still be considered a bug, that a list with non-nullable values (List<Any>) can be initialized with null values.

[swankjesse]

Good point. Lemme rename the issue and we’ll contemplate what our options are.

[serj-lotutovici]

If we wan't to fail early may be it makes sense to add a rule mechanism to the CollectionsJsonAdapter and MapJsonAdapter that will check agains each parsed value? And enforce those rules for kotlin classes. That will give us the possibility to propagate a proper path in the error.
Another option would be to wrap those adapters and check after the values are parsed, but that will result in one extra loop which is not great.

[serj-lotutovici]

Started looking into it, and immediately stumbled upon the fact that any extra adapter for collections will add a side effect for java classes. The only idea that comes to mind at the moment is to tie through an internal api a nullability flag to the generic type for the list/set/map, but that is too smelly.
What do you guys think?

[swankjesse]

Maybe something like .nonNull() on adapters that should never return null.

[rongi]

It should just throw. It should never initialize non-nullable fields with nulls.

[williamwue]

I'm agreed with @rongi , how to make moshi throw execption now? I have tried but failed.

[rongi]

@williamwue It throws now, you just need to use KotlinJsonAdapterFactory I think.

[williamwue]

@rongi Got it, I have miss KotlinJsonAdapterFactory before.

[swankjesse]

Folding into this issues: https://github.com/square/moshi/issues/634