feat: match android type support for custom attributes #157
Conversation
| @"A Date Key":[NSDate date]}; | ||
| @"A Number Key":@(42), | ||
| @"A Date Key":[NSDate date], | ||
| @"test Dictionary": @{}}; |
There was a problem hiding this comment.
Would it be better to use a test dictionary with at least one key/value pair? Or does that not matter for this example?
| @@ -47,7 +47,8 @@ - (void)testInstance { | |||
| } | |||
|
|
|||
| NSDictionary *eventInfo = @{@"speed":@25, | |||
| @"modality":@"sprinting"}; | |||
| @"modality":@"sprinting", | |||
| @"stats":@{}}; | |||
There was a problem hiding this comment.
Same question, especially for unit test, maybe we should have an empty dictionary, then another key with a dictionary with one or two k/v pairs, then finally a key with a dictionary that contains a dictionary in one of its values (nested dictionary) for better coverage.
There was a problem hiding this comment.
Added a test case for dictionary with values and one with dictionary values in the dictionary
| @@ -163,7 +164,8 @@ - (void)testDictionaryRepresentation { | |||
| MPEvent *event = [[MPEvent alloc] initWithName:@"Dinosaur Run" type:MPEventTypeOther]; | |||
| event.duration = eventDuration; | |||
| event.customAttributes = @{@"speed":@25, | |||
| @"modality":@"sprinting"}; | |||
| @"modality":@"sprinting", | |||
| @"stats":@{}}; | |||
There was a problem hiding this comment.
Same comment re: other dictionary test cases
| @@ -72,8 +72,7 @@ - (id)valueForCaseInsensitiveKey:(NSString *)key { | |||
| transformedDictionary[key] = [[NSString alloc] initWithData:obj encoding:NSUTF8StringEncoding]; | |||
| } else { | |||
| MPILogError(@"Data type is not supported as an attribute value: %@ - %@", obj, [[obj class] description]); | |||
| NSAssert([obj isKindOfClass:[NSString class]], @"Data type is not supported as an attribute value"); | |||
| return; | |||
| transformedDictionary[key] = [obj description]; | |||
There was a problem hiding this comment.
I remember we decided on this (using description) since that's what Android is doing, though this will result in a different string than Android. I think it would be ideal to use a JSON string on both platforms, but I think we decided that was out of scope for this work and could be something we do in the future...
There was a problem hiding this comment.
I actually did this initially but was checking count (example below) which I didn't realize was what was making me not hit that statement. Thanks for making me take another look.
else if ([obj isKindOfClass:[NSDictionary class]] && [(NSDictionary *)obj count] > 0) {
transformedDictionary[key] = [obj description];
} else if ([obj isKindOfClass:[NSMutableDictionary class]] && [(NSDictionary *)obj count] > 0) {
transformedDictionary[key] = [obj description];
} else {
MPILogError(@"Data type is not supported as an attribute value: %@ - %@", obj, [[obj class] description]);
NSAssert([obj isKindOfClass:[NSString class]], @"Data type is not supported as an attribute value");
return;
}
```
BrandonStalnaker
dismissed
einsteinx2’s stale review
changes made Ready for a new look
BrandonStalnaker
deleted the
feat/4087-Match-Android-Support-For-Custom-Attributes
branch
# [8.9.0](v8.8.1...v8.9.0) (2022-11-30) ### Bug Fixes * Add class checks to initialize methods ([#165](#165)) ([065df46](065df46)) * Add Hash Methods where isEqual called ([#167](#167)) ([7302e0d](7302e0d)) * Allow null event attributes ([#163](#163)) ([5fd25c6](5fd25c6)) * Move dyld register callback to initialize ([#170](#170)) ([49d3660](49d3660)) * Remove unnecessary calls to removeObserver ([#168](#168)) ([a288ba4](a288ba4)) * Remove Unnecessary Synchronize in Dealloc ([#169](#169)) ([624891b](624891b)) ### Features * match android type support for custom attributes ([#157](#157)) ([c9a34cd](c9a34cd)) * Remove depreciate MPSegment code ([#171](#171)) ([1c04262](1c04262)) * Update Sample App for iOS 16 ([#158](#158)) ([fed8131](fed8131))
Summary
We discussed this in stand up and determined it makes sense to have iOS match Android behavior in terms of serializing dictionaries when sending to the server but leave the objects as-is when passing to kits.
Testing Plan
Updated unit test to include cases with dictionaries
Master Issue
Closes https://mparticle-eng.atlassian.net/browse/SQDSDKS-4087