Fix obj_c types to not overwrite jsonDict with nil
#201
Conversation
When the inner subType serializer returns nil as it does with `return [jsonDict count] > 0 ? jsonDict : nil`, this causes the outer/parent serializer to overwrite the mutable `jsonDict` to nil, resulting in a nil result when the `.tag` field was going to be added... In our case the `DBCAMERAUPLOADSMOBILEMediaMetadataSerializer` serializer returned nil when it needed a `.tag` field to be successful server-side
Codecov Report
@@ Coverage Diff @@
## main #201 +/- ##
=======================================
Coverage 51.60% 51.60%
=======================================
Files 37 37
Lines 8400 8400
Branches 1790 1790
=======================================
Hits 4335 4335
Misses 3751 3751
Partials 314 314
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
| @@ -1120,7 +1120,7 @@ def emit_serializer(): | |||
| if is_user_defined_type(data_type): | |||
| if is_struct_type(data_type) and \ | |||
| not data_type.has_enumerated_subtypes(): | |||
| self.emit('jsonDict = [{} mutableCopy];'. | |||
There was a problem hiding this comment.
The only thing i'd be worried about here is that in the old version, we are relying on the fact that we either get a brand new dictionary or nil. I the new code, we are adding the serialized dictionary to whatever jsonDict already contains. Are we 100% certain that every instance where this is used does not have an instance of jsonDict with some values already in it??
Something safer (if your unsure about my question above) would be to do something like:
jsonDict = [{} mutableCopy] ?: [[NSMutableDictionary alloc] init];
There was a problem hiding this comment.
this is the deleted code, I think your saying that I should do
let additionDict = [{} mutableCopy] ?: [[NSMutableDictionary alloc] init]; // to guarantee non-nilness
[jsonDict addEntriesFromDictionary: additionDict]
There was a problem hiding this comment.
if we are adding nil entries like [jsonDict addEntriesFromDictionary:nil], the json will still be non-nil. so we don't need a additionDict to be non-nil
|
@yuxiang-he could I have a review please? |
When the inner subType serializer returns nil as it does with
return [jsonDict count] > 0 ? jsonDict : nil, this causes the outer/parent serializer to overwrite the mutablejsonDictto nil, resulting in a nil result when the.tagfield was going to be added... In our case theDBCAMERAUPLOADSMOBILEMediaMetadataSerializerserializer returned nil when it needed a.tagfield to be successful server-sideGeneral Contributing
Is This a Code Change?
Validation
tox?