Skip to content

JSON serialization of default value for fields with explicit presence #238

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.

Sign up for GitHub

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

Merged
garyp merged 18 commits into from
Oct 25, 2022
Merged

JSON serialization of default value for fields with explicit presence #238

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
42c6a4a
Repro unit test
antongrbin Oct 20, 2022
15467d9
add another case
antongrbin Oct 20, 2022
cbd188b
add a test for proto3 as well
antongrbin Oct 20, 2022
1a97cff
self review
antongrbin Oct 21, 2022
6fa3c58
change order of params for assertEquals
antongrbin Oct 21, 2022
a210518
Change expected values in unit tests and fix
antongrbin Oct 21, 2022
278a78e
fix 2
antongrbin Oct 21, 2022
5e3f046
add test binary json roundtrip test
antongrbin Oct 21, 2022
2c879fd
Merge branch 'nc-151/test-bug' into nc-151/fix-it-2
antongrbin Oct 21, 2022
c2e705a
remove test
antongrbin Oct 21, 2022
5af171e
Merge branch 'nc-151/test-bug' into nc-151/fix-it-2
antongrbin Oct 21, 2022
98bc4e0
simplfiy by introducing a var
antongrbin Oct 21, 2022
e749c6a
address comments
antongrbin Oct 25, 2022
ceae7e6
add comment that for proto3 the two tests are equivalent
antongrbin Oct 25, 2022
9045c29
Merge branch 'nc-151/test-bug' into nc-151/fix-it-2
antongrbin Oct 25, 2022
b8f938e
apply test fixes
antongrbin Oct 25, 2022
e30c000
Merge branch 'master' into nc-151/fix-it-2
antongrbin Oct 25, 2022
1cc3146
address comment about oneOf presence
antongrbin Oct 25, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions runtime/src/commonMain/kotlin/pbandk/internal/json/JsonMessageEncoder.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,8 @@ internal class JsonMessageEncoder(private val jsonConfig: JsonConfig) : MessageE
@Suppress("UNCHECKED_CAST")
val value = (fd.value as KProperty1<T, *>).get(message)

if (value == null && fd.oneofMember) continue
if (!fd.oneofMember && !jsonConfig.outputDefaultValues && fd.type.isDefaultValue(value)) continue
if (value == null && fd.type.hasPresence) continue
if (!fd.type.hasPresence && !jsonConfig.outputDefaultValues && fd.type.isDefaultValue(value)) continue

val jsonValue = value
?.takeUnless {
Expand Down
8 changes: 4 additions & 4 deletions runtime/src/commonTest/kotlin/pbandk/json/OutputDefaultValuesTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,12 +32,12 @@ class OutputDefaultValuesTest {
assertEquals(JsonPrimitive(""), parsedJson["optionalBytes"])
assertEquals(JsonPrimitive(TestAllTypesProto3.NestedEnum.fromValue(0).name!!), parsedJson["optionalNestedEnum"])

assertEquals(JsonNull, parsedJson["optionalNestedMessage"])
assertFalse("optionalNestedMessage" in parsedJson)
Copy link
Contributor Author

@antongrbin antongrbin Oct 25, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I looked deeper into this, please see explanation in PR description about this.


assertEquals(JsonArray(emptyList()), parsedJson["repeatedString"])
assertEquals(JsonObject(emptyMap()), parsedJson["mapBoolBool"])

assertEquals(JsonNull, parsedJson["optionalStringWrapper"])
assertFalse("optionalStringWrapper" in parsedJson)
assertEquals(JsonPrimitive(false), parsedJson["optionalBoolWrapper"])
}

Expand All @@ -55,13 +55,13 @@ class OutputDefaultValuesTest {
@Test
fun testProto2EnumUnsetField() {
val message = MessageWithEnum()
assertEquals("{\"value\":null}", message.encodeToJsonString(jsonConfigCompactOutput))
assertEquals("{}", message.encodeToJsonString(jsonConfigCompactOutput))
}

@Test
fun testProto2EnumFieldSetToDefaultValue() {
val message = MessageWithEnum(value = MessageWithEnum.EnumType.FOO)
assertEquals("{}", message.encodeToJsonString(jsonConfigCompactOutput))
assertEquals("{\"value\":\"FOO\"}", message.encodeToJsonString(jsonConfigCompactOutput))
}

@Test
Expand Down