Replace json-simple with Jackson - #234
Conversation
Replace org.json.simple dependency with Jackson (jackson-databind).
Add NON_NULL property inclusion to objectMapper so null fields are omitted from JSON output, matching the behavior expected by tests (and previously implicit via json-simple's null semantics).
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #234 +/- ##
============================================
+ Coverage 33.89% 34.08% +0.19%
Complexity 162 162
============================================
Files 36 36
Lines 1366 1370 +4
Branches 140 140
============================================
+ Hits 463 467 +4
Misses 870 870
Partials 33 33
Continue to review full report in Codecov by Harness.
🚀 New features to boost your workflow:
|
jicoco pinned jackson 2.17.1 while jitsi-utils and jicofo use 2.19.0. In a coordinated multi-repo build, dependency mediation produced a skew (jackson-databind 2.17.1 from jicoco + jackson-module-kotlin 2.19.0), which makes the Kotlin module fail to construct data classes: 'Cannot construct instance ... (no Creators ...)'. Align to 2.19.0.
| val parsed = Event.parse( | ||
| """ | ||
| { | ||
| { |
There was a problem hiding this comment.
You removed the trailing spaces - was that on purpose? I would hope the tests still pass with the trailing spaces?
There was a problem hiding this comment.
It wasn't on purpose, but shouldn't cause any problems -- this is parsing JSON. Tests pass locally, failures we see on GH are probably because of the jitsi-utils version (but the CI build should pick up the right version automatically)
Replace mapper.createObjectNode()/createArrayNode() with JsonNodeFactory.instance.objectNode()/arrayNode() everywhere. Remove now-unused ObjectMapper instances and imports.
| open val jsonString: String | ||
| get() = JSONObject(metrics.filter { it.value.supportsJson }.mapValues { it.value.get() }).toJSONString() | ||
| get() = jsonMapper.valueToTree<com.fasterxml.jackson.databind.node.ObjectNode>( | ||
| metrics.filter { it.value.supportsJson }.mapValues { it.value.get() } |
There was a problem hiding this comment.
Can you import ObjectNode here?
Replace the
org.json.simpledependency with Jackson (jackson-databind).MetricsContainerandHistogramMetricto useObjectNode/ArrayNodemediajsonserialization; addNON_NULLproperty inclusion to match previous null-omission behaviorjitsi-utilsto1.0-SNAPSHOT