Add support of oneOf(primitive|object) in the json parser generator by aleksandr-gringauz · Pull Request #2972 · DataDog/dd-sdk-android

aleksandr-gringauz · 2025-10-30T10:40:28Z

What does this PR do?

This PR addresses this issue.

It implements support for the following case:

"oneOf": [
      {
        "type": "string",
        "description": "string element"
      },
      {
        "type": "integer",
        "description": "integer element"
      }
]

Options of oneOf can also be of primitive types.

The best way to look at this PR is to start with the changes in the unit tests of the generator and then to read the comments to the PR explaining specific places in code.

I will update RUM schema to the latest version in a separate PR.

Motivation

What inspired you to submit this pull request?

Additional Notes

Anything else we should know when reviewing?

Review checklist (to be filled by reviewers)

Feature or bugfix MUST have appropriate tests (unit, integration, e2e)
Make sure you discussed the feature or bugfix with the maintaining team in an Issue
Make sure each commit and the PR mention the Issue number (cf the CONTRIBUTING doc)

datadog-datadog-prod-us1 · 2025-10-30T11:05:15Z

🎯 Code Coverage
• Patch Coverage: 100.00%
• Total Coverage: 70.82% (-0.09%)

View detailed report

_{This comment will be updated automatically if new data arrives.

🔗 Commit SHA: 1585433 | Docs | Datadog PR Page | Was this helpful? Give us feedback!}

codecov-commenter · 2025-10-30T11:18:24Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 70.84%. Comparing base (ee38a9a) to head (1585433).
⚠️ Report is 1317 commits behind head on develop.

Additional details and impacted files

@@             Coverage Diff             @@
##           develop    #2972      +/-   ##
===========================================
- Coverage    70.91%   70.84%   -0.07%     
===========================================
  Files          829      829              
  Lines        30381    30381              
  Branches      5184     5184              
===========================================
- Hits         21543    21522      -21     
- Misses        7373     7385      +12     
- Partials      1465     1474       +9

see 29 files with indirect coverage changes

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

aleksandr-gringauz · 2025-10-30T11:33:44Z

+
+            @JvmStatic
+            @Throws(JsonParseException::class)
+            public fun fromJsonObject(jsonElement: JsonElement): Path {


The json element corresponding to oneOf now is either JsonObject or JsonPrimitive. Their common parent is JsonElement.

aleksandr-gringauz · 2025-10-30T11:34:13Z

+                    null
+                }
+                val asBoolean = try {
+                    if (jsonElement is JsonPrimitive) {


First checking jsonElement is JsonPrimitive, then trying to parse a Boolean

aleksandr-gringauz · 2025-10-30T11:34:47Z

+                    null
+                }
+                val asPoint = try {
+                    if (jsonElement is JsonObject) {


First checking jsonElement is JsonObject, then trying to parse a Point

aleksandr-gringauz · 2025-10-30T11:35:45Z

    data class OneOfClass(
        val name: String,
-        val options: List<Class>,
+        val options: List<Option>,


Now options of oneOf can be either a class or a primitive.

aleksandr-gringauz · 2025-10-30T11:37:19Z

+import com.squareup.kotlinpoet.TypeSpec
+import com.squareup.kotlinpoet.jvm.throws
+
+class OneOfPrimitiveOptionGenerator(


This class is responsible for generating code of primitive options of oneOf. It is used in MultiClassGenerator.

Code for class options of oneOf is generated in MultiClassGenerator as before.

aleksandr-gringauz · 2025-10-30T14:50:09Z

+                @Throws(JsonParseException::class)
+                public fun fromJsonObject(jsonObject: JsonPrimitive): String {
+                    try {
+                        if (jsonObject.isString) {


This isString check is important. Otherwise is jsonObject contains Int or Boolean jsonObject.asString will successfully "parse" it. This isn't what we want.

https://github.com/google/gson/blob/72537db9a6095720aca3a67abe77ec5a16200081/gson/src/main/java/com/google/gson/JsonPrimitive.java#L163

aleksandr-gringauz · 2025-10-30T14:50:22Z

+                @Throws(JsonParseException::class)
+                public fun fromJsonObject(jsonObject: JsonPrimitive): Boolean {
+                    try {
+                        if (jsonObject.isBoolean) {


This isBoolean check is important. Otherwise .asBoolean it will try doing this.

aleksandr-gringauz · 2025-10-30T14:59:21Z

+                @Throws(JsonParseException::class)
+                public fun fromJsonObject(jsonObject: JsonPrimitive): Long {
+                    try {
+                        if (jsonObject.isNumber) {


This isNumber check is important. Otherwise .asLong will try to extract a number from a string link.

0xnm

nice!

0xnm · 2025-10-31T08:02:11Z

    companion object 
      fun fromJson(kotlin.String): Vital
-      fun fromJsonObject(com.google.gson.JsonObject): Vital
+      fun fromJsonObject(com.google.gson.JsonElement): Vital


we probably need to rename fromJsonObject to fromJsonElement then, but such change will break Flutter, for example https://github.com/search?q=repo%3ADataDog%2Fdd-sdk-flutter%20fromJsonObject&type=code

Good point.

I will try to rename, but I think we should rename fromJsonObject to fromJsonElement only in places where JsonElement is passed as an argument.

We can also rename here to fromJsonPrimitive (this file is introduced in the current PR).

but such change will break Flutter

LogEvent isn't affected by my change. It is still parsed from jsonObject, so no need to rename its method.

IIUC the usage in flutter that you mentioned is the only place where we use fromJsonObject link. So it should be safe to rename.

renamed to fromJsonElement and fromJsonPrimitive where needed

0xnm · 2025-10-31T08:09:38Z

+                    ClassNameRef.IllegalStateException,
+                    ClassNameRef.UnsupportedOperationException
+                )
+                JsonPrimitiveType.DOUBLE -> error("Double is not supported")


maybe add comment why?

Actually, I can entirely remove JsonPrimitiveType.DOUBLE from the code.

It is never used, we don't assign it anywhere when we parse the json link.

And JSON Schema actually has only integer and number. https://json-schema.org/understanding-json-schema/reference/numeric

I don't know why JsonPrimitiveType.DOUBLE was introduced in the first place.

removed JsonPrimitiveType.DOUBLE from the codebase

aleksandr-gringauz · 2025-10-31T11:44:31Z

        rootTypeName: String
    ) {
        val opt = if (nullable) "?" else ""
-        beginControlFlow("$assignee = $getter$opt.asJsonObject$opt.let")


This is an important place that I missed. It corresponds to the situation when a oneOf is a field of some object.

When we are deserializing a oneOf, we need to treat it as JsonElement now. But here it was cast to JsonObject and it failed if the option of oneOf was of primitive type.

My new tests only tested the case when oneOf(primitive) was an element of an array.

I modified an old test here to test this scenario.

aleksandr-gringauz · 2025-10-31T12:21:17Z

+                    ClassNameRef.IllegalStateException,
+                    ClassNameRef.UnsupportedOperationException
+                )
+                JsonPrimitiveType.DOUBLE -> error("Double is not supported")


removed JsonPrimitiveType.DOUBLE from the codebase

aleksandr-gringauz · 2025-10-31T12:21:40Z

    companion object 
      fun fromJson(kotlin.String): Vital
-      fun fromJsonObject(com.google.gson.JsonObject): Vital
+      fun fromJsonObject(com.google.gson.JsonElement): Vital


renamed to fromJsonElement and fromJsonPrimitive where needed

aleksandr-gringauz changed the title ~~Add support of oneOf(primitive|object types) in the json parser generator~~ Add support of oneOf(primitive|object) in the json parser generator Oct 30, 2025

aleksandr-gringauz force-pushed the aleksandr-gringauz/boxed-primitive-generation branch 3 times, most recently from 5bac644 to dd8cc53 Compare October 30, 2025 14:48

Add support of oneOf(primitive types) in the json parser generator

67dcf0d

aleksandr-gringauz force-pushed the aleksandr-gringauz/boxed-primitive-generation branch from dd8cc53 to 67dcf0d Compare October 30, 2025 15:42

aleksandr-gringauz commented Oct 30, 2025

View reviewed changes

aleksandr-gringauz marked this pull request as ready for review October 30, 2025 15:43

aleksandr-gringauz requested review from a team as code owners October 30, 2025 15:43

0xnm previously approved these changes Oct 31, 2025

View reviewed changes

ambushwork previously approved these changes Oct 31, 2025

View reviewed changes

aleksandr-gringauz added 2 commits October 31, 2025 10:42

Remove Double

978b851

better naming for path_array

bf22e3c

aleksandr-gringauz dismissed stale reviews from ambushwork and 0xnm via 7cf8f4c October 31, 2025 11:40

rename fromJsonObject to fromJsonPrimitive or fromJsonElement

1585433

aleksandr-gringauz force-pushed the aleksandr-gringauz/boxed-primitive-generation branch from 7cf8f4c to 1585433 Compare October 31, 2025 12:14

aleksandr-gringauz commented Oct 31, 2025

View reviewed changes

aleksandr-gringauz requested review from 0xnm and ambushwork October 31, 2025 12:23

0xnm approved these changes Oct 31, 2025

View reviewed changes

ambushwork approved these changes Oct 31, 2025

View reviewed changes

aleksandr-gringauz merged commit df659af into develop Oct 31, 2025
26 of 27 checks passed

aleksandr-gringauz deleted the aleksandr-gringauz/boxed-primitive-generation branch October 31, 2025 16:17

Uh oh!

Conversation

aleksandr-gringauz commented Oct 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What does this PR do?

Motivation

Additional Notes

Review checklist (to be filled by reviewers)

Uh oh!

datadog-datadog-prod-us1 Bot commented Oct 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov-commenter commented Oct 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

0xnm left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

aleksandr-gringauz Oct 31, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

aleksandr-gringauz Oct 31, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

aleksandr-gringauz commented Oct 30, 2025 •

edited

Loading

datadog-datadog-prod-us1 Bot commented Oct 30, 2025 •

edited

Loading

codecov-commenter commented Oct 30, 2025 •

edited

Loading

aleksandr-gringauz Oct 31, 2025 •

edited

Loading

aleksandr-gringauz Oct 31, 2025 •

edited

Loading