Handled dollar prefixed values for "createOrReplace" Mongo implementation #207
Conversation
suresh-prakash
requested review from
avinashkolluru,
kotharironak and
skjindal93
as code owners
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #207 +/- ##
============================================
+ Coverage 79.60% 79.70% +0.09%
- Complexity 1021 1022 +1
============================================
Files 194 194
Lines 4865 4879 +14
Branches 404 408 +4
============================================
+ Hits 3873 3889 +16
+ Misses 707 706 -1
+ Partials 285 284 -1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
| @@ -113,6 +124,17 @@ public static JsonNode recursiveClone( | |||
| if (value.isObject()) { | |||
| newValue = recursiveClone(value, function, emptyObjectConverter); | |||
| } | |||
| if (value.isArray()) { | |||
There was a problem hiding this comment.
Typically in this kind of function you'd have object/array/primitives as sibling code paths, with recursion inside object and array. Here it looks like we handle primitives, else assume it's an object and only treat array differently inside the processing of object fields? Am I reading that right and if so, is there a reason to do it that way?
There was a problem hiding this comment.
We are treating "primitive values" differently.
Also, we are treating "object keys" differently.
Both needs encoding so that a $ prefix is (treated literally and) not treated as the special field lookup operator.
We are still doing similar thing. Nested inside objects and arrays and handling primitives differently. Because, we also want object keys to be encoded, this method turned out this way.
In this PR, I'm just handling the missing case of "primitive values" (especially when they are strings). So, just piggy bagging onto the existing method.
| } | ||
| } | ||
| if (newValue.isTextual()) { | ||
| newValue = new TextNode(function.apply(newValue.asText())); |
There was a problem hiding this comment.
Why do we need this, we have already applied transformation to each value?
There was a problem hiding this comment.
Sorry my mistake. I read it as array node.
| } | ||
| } | ||
| if (newValue.isTextual()) { | ||
| newValue = new TextNode(function.apply(newValue.asText())); |
There was a problem hiding this comment.
Sorry my mistake. I read it as array node.
No description provided.