-
Notifications
You must be signed in to change notification settings - Fork 762
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
Ignore null value and use the default value when deserializing from json string #843
Comments
See #762 |
It's not what i want. My bean is: class Foo @SuppressWarnings("unused") constructor() {
@Json(name = "id")
var id: Int = 0
@Json(name = "name")
var name: String = ""
} And the serve return json :
About the "name" filed, I want to skip the null value and use the the default "", and also null array filed. |
there are a few ways to achieve what you want. here's a relatively short way that comes to mind: class Foo {
var id: Int = 0
internal var name: String? = ""
fun getName(): String = name ?: ""
fun setName(name: String) {
this.name = name
}
}
@Test fun foo() {
val encoded = """
{
"id": 1,
"name": null
}"""
val decoded = Foo()
val moshi = Moshi.Builder().add(KotlinJsonAdapterFactory()).build()
val adapter = moshi.adapter(Foo::class.java)
adapter.fromJson(encoded)!!.run {
assertThat(id).isEqualTo(1)
assertThat(getName()).isEqualTo("")
}
assertThat(adapter.toJson(decoded)).isEqualTo("""{"id":0,"name":""}""")
} |
@NightlyNexus although it works fine, but I think it isn't a best solution. Because the Foo class generated by tools, not by hand usually, your solution is not convenient. I think the best solution is provide a ignore JSONNULL strategy by moshi lib itself. |
@ildar2 to answer your FR in #1000, the best solution is for you to either
I don't think it's in our interest to effectively break moshi's behavior to accommodate unscrupulous APIs that conflate absence and nullability. |
@ZacSweers |
@condesales default values are used only if api responce misses this field. If the field was explicitly null, then your field will be null, regardless of kotlin nullability. But you can work around that with reserialization - null fields get stripped. Here is a passing test, where
|
This is the case (absent key) that Moshi's Kotlin support handles. However, |
@ZacSweers @condesales What does |
It has one answer in Moshi and this is Moshi's opinion :). |
@ZacSweers a clear answer. |
That's not what I said, so let me be clear. Moshi's opinion in Kotlin is that present keys with literal Here's one that works via intermediary delegating adapter that manually removes nulls from the json blob: @Retention(RUNTIME)
@Target(CLASS)
annotation class DefaultIfNull
class DefaultIfNullFactory : JsonAdapter.Factory {
override fun create(type: Type, annotations: MutableSet<out Annotation>,
moshi: Moshi): JsonAdapter<*>? {
if (!Types.getRawType(type).isAnnotationPresent(
DefaultIfNull::class.java)) {
return null
}
val delegate = moshi.nextAdapter<Any>(this, type, annotations)
return object : JsonAdapter<Any>() {
override fun fromJson(reader: JsonReader): Any? {
@Suppress("UNCHECKED_CAST")
val blob = reader.readJsonValue() as Map<String, Any?>
val noNulls = blob.filterValues { it != null }
return delegate.fromJsonValue(noNulls)
}
override fun toJson(writer: JsonWriter, value: Any?) {
return delegate.toJson(writer, value)
}
}
}
}
class NullSkipperTest {
@DefaultIfNull
data class ClassWithDefaults(val foo: String, val bar: String? = "defaultBar")
@Test
fun skipNulls() {
//language=JSON
val json = """{"foo": "fooValue", "bar": null}"""
val adapter = Moshi.Builder()
.add(DefaultIfNullFactory())
.add(KotlinJsonAdapterFactory())
.build()
.adapter(ClassWithDefaults::class.java)
val instance = adapter.fromJson(json)!!
check(instance.bar == "defaultBar")
}
} |
Ah thank you for the clarification! P.S., sorry, didn't mean seem so harsh ... definitely appreciate the great work you all do. |
@ZacSweers this question comes up a lot. Maybe we can add a section under the Kotlin heading in the docs to more thoroughly explain "It understands Kotlin's non-nullable types and default parameter values" When you said that Moshi respects explicit nulls sent by the server... This made perfect sense to me. In the perfect world I do indeed want default values only when a key is absent, and the rest of the time... I want to respect null. Unfortunately I work with a backend that has given us a "guarantee" of omitting nulls, yet every now and then one sneaks in. In this case, I rather ship my release builds with an adapter that will just strip out any nulls so that we don't hit any crashes in production. But I'm okay with omitting a StripNullAdapter in our development stage. Do you think it'd be worthwhile to call this out in the docs (specifically moshis opinion on this) and potentially provide the strip null adapter for users to quickly find it... Also potentially mentioning that it may be best to only enable in release mode? Just my opinion since I keep hitting needless crashes because my prod backend keeps breaking the contract. Edit: just saw you provided an annotation and not an adapter. So my point still stands, but maybe highlight the option to perform this on the adapter level or class level with annotation Edit 2: wait. Your code is an annotation and an adapter factory? Consider me lost. 😁 Wouldn't you only need one of those? Edit 3: okay. Reread the code. Looks like your adapter only defaults the values if the annotation is present. Makes sense. I did try to run it and it crashes on the |
@ZacSweers by any chance do you think this modification to your adapter you posted above, is completely awful or terrible for some reason? Basically I'm in an old legacy app, with like 200+ models, and so I'm looking for a way to touch as little code as possible. I just want nulls from my backend to be removed so that they turn into the default values in kotlin. Hoping to not have to use any annotations or anything like that. Hopefully one adapter to do the trick. The below seems to work without any real negative effect on perf. Do you think it's completely terrible, or it's probably the best I can get?
|
@ZacSweers while I use this DefaultIfNull annotation it shows error on field saying |
@saqibmirza2007 that's not enough to help you, and sort of out of scope of this original issue. Should post it on stackoverflow with the exact error messages and sample code if you want help |
@ZacSweers thanks for your reply I have sorted out the issue but can you please tell me that can I use this if I am using retrofit android which automatically parses response. |
@saqibmirza2007 I would recommend not using that factory. I have had major issues in production because of it. My findings in the edit here. https://stackoverflow.com/questions/60377758/moshi-factory-to-ignore-null-values-and-use-kotlin-default-value-when-deserializ |
@ColtonIdle your problem isn't really with the factory, seems your problem is with how numbers work in JSON and caring about the intermediate number representations in that map. A downstream adapter will be able to handle those number conversions just fine, I think that post is misleading. Any API using the jsonValue API would have this behavior, and I use it for a number of projects without any issue. |
@ZacSweers I put together a small test that shows my issue. There's a good chance I'm not understanding something correctly or using something correctly, but would love to at least know that for sure. Here is my factory. Similar to yours except I removed the fact that you need to be opted in via an annotation.
Here are my 3 test cases
If it helps, I can just put up a sample repo so theres no miscommunication here. As you can see, in the second test that fails, my server sends me a long timestamp for a birthday and my class that this gets adapted to has the timestamp defined as a String. This works totally fine with no DefaultIfNullFactory(). I get the String on the other side with an untouched value. Hoorah! If I add in the DefaultIfNullFactory(), then I still get a String, but it's been modified out from under me. Can you see where my confusion comes from or am I going insane? 😄 I have a test that passes (and that means my android code is working), but then I added the DefaultIfNullfactory, and now my android app doesn't work because it's not expecting scientific notation in my string. I also updated my SO question to say that I may not be right in my verdict of it not working. |
@ColtonIdle Thanks for your reply. I am using it with retrofit and it was not working well maybe I have missed something. |
I have made some changes https://github.com/uchhabra3/Moshi-Issue-843-Solution If anyone want to check and comment |
@uchhabra3 use in moshi 1.11.0 not work
|
@ZacSweers use in moshi 1.11.0 not work
OR
Neither of the above methods will work |
this is my working and fast solution
|
@morder your solution seemed the best to me but it doesn't compile in moshi 1.11.0 since the JsonReader constructor is package private. |
yep, change the package name to |
Check my answer here and see if it will help you |
Here is what I'll use, based on @ColtonIdle 's suggestion:
Perhaps its not as fast as @morder but speed isnt a big issue in my use case. |
Ignore null value and use the default value when deserializing from json string
The text was updated successfully, but these errors were encountered: