Skip to content

Commit cbe3f1c

Browse files
jaeoptthomaszurkan-optimizely
jaeopt
authored and
thomaszurkan-optimizely
committed
fix event tags {1 and 0} unexpected filtered out [Jira #4041] (#363)
* fix event tags {1 and 0} unexpected filtered out [Jira #4041] * refactor boolean checking
1 parent 1425d4b commit cbe3f1c

File tree

2 files changed

+200
-22
lines changed

2 files changed

+200
-22
lines changed

‎OptimizelySDKCore/OptimizelySDKCore/OPTLYEventTagUtil.m

Lines changed: 25 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -32,17 +32,8 @@ + (NSNumber *)getRevenueValue:(NSDictionary *)eventTags logger:(id<OPTLYLogger>)
3232
if ([value isKindOfClass:[NSNumber class]]) {
3333
answer = (NSNumber*)value;
3434
const char *objCType = [answer objCType];
35-
// Dispatch objCType according to one of "Type Encodings" listed here:
36-
// https://developer.apple.com/library/content/documentation/Cocoa/Conceptual/ObjCRuntimeGuide/Articles/ocrtTypeEncodings.html
37-
if ((strcmp(objCType, @encode(bool)) == 0)
38-
|| [value isEqual:@YES]
39-
|| [value isEqual:@NO]) {
40-
// NSNumber's generated by "+ (NSNumber *)numberWithBool:(BOOL)value;"
41-
// serialize to JSON booleans "true" and "false" via NSJSONSerialization .
42-
// The @YES and @NO compile to __NSCFBoolean's which (strangely enough)
43-
// are ((strcmp(objCType, @encode(char)) == 0) but these serialize as
44-
// JSON booleans "true" and "false" instead of JSON numbers.
45-
// These aren't integers, so shouldn't be sent.
35+
36+
if ([self isNSNumberBoolean:answer]) {
4637
answer = nil;
4738
[logger logMessage:OPTLYLoggerMessagesRevenueValueInvalidBoolean withLevel:OptimizelyLogLevelWarning];
4839
} else if ((strcmp(objCType, @encode(char)) == 0)
@@ -116,17 +107,8 @@ + (NSNumber *)getNumericValue:(NSDictionary *)eventTags logger:(id<OPTLYLogger>)
116107
if ([value isKindOfClass:[NSNumber class]]) {
117108
answer = (NSNumber*)value;
118109
const char *objCType = [answer objCType];
119-
// Dispatch objCType according to one of "Type Encodings" listed here:
120-
// https://developer.apple.com/library/content/documentation/Cocoa/Conceptual/ObjCRuntimeGuide/Articles/ocrtTypeEncodings.html
121-
if ((strcmp(objCType, @encode(bool)) == 0)
122-
|| [value isEqual:@YES]
123-
|| [value isEqual:@NO]) {
124-
// NSNumber's generated by "+ (NSNumber *)numberWithBool:(BOOL)value;"
125-
// serialize to JSON booleans "true" and "false" via NSJSONSerialization .
126-
// The @YES and @NO compile to __NSCFBoolean's which (strangely enough)
127-
// are ((strcmp(objCType, @encode(char)) == 0) but these serialize as
128-
// JSON booleans "true" and "false" instead of JSON numbers.
129-
// These aren't integers, so shouldn't be sent.
110+
111+
if ([self isNSNumberBoolean:answer]) {
130112
answer = nil;
131113
[logger logMessage:OPTLYLoggerMessagesNumericValueInvalidBoolean withLevel:OptimizelyLogLevelWarning];
132114
} else {
@@ -155,4 +137,25 @@ + (NSNumber *)getNumericValue:(NSDictionary *)eventTags logger:(id<OPTLYLogger>)
155137
return answer;
156138
}
157139

140+
+ (BOOL)isNSNumberBoolean:(NSNumber*)value {
141+
const char *objCType = [value objCType];
142+
// Dispatch objCType according to one of "Type Encodings" listed here:
143+
// https://developer.apple.com/library/content/documentation/Cocoa/Conceptual/ObjCRuntimeGuide/Articles/ocrtTypeEncodings.html
144+
145+
// NSNumber's generated by "+ (NSNumber *)numberWithBool:(BOOL)value;"
146+
// serialize to JSON booleans "true" and "false" via NSJSONSerialization .
147+
// The @YES and @NO compile to __NSCFBoolean's which (strangely enough)
148+
// are ((strcmp(objCType, @encode(char)) == 0) but these serialize as
149+
// JSON booleans "true" and "false" instead of JSON numbers.
150+
// These aren't integers, so shouldn't be sent.
151+
152+
//
153+
// [Jira #4041] integers {1 and 0} are filtered out accidentally since @YES and @NO interpreted as @1 and @0
154+
// we need compare with @YES and @NO only for boolean NSNumber (objCType == @encode(char))
155+
156+
return (strcmp(objCType, @encode(bool)) == 0) ||
157+
((strcmp(objCType, @encode(char)) == 0) && [value isEqual:@YES]) ||
158+
((strcmp(objCType, @encode(char)) == 0) && [value isEqual:@NO]);
159+
}
160+
158161
@end

‎OptimizelySDKCore/OptimizelySDKCoreTests/OPTLYEventBuilderTest.m

Lines changed: 175 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -634,6 +634,162 @@ - (void)testRevenueMetricAndValueMetric
634634
[self commonBuildConversionTicketTest:tags sentEventTags:tags sentTags:tags];
635635
}
636636

637+
638+
639+
#pragma mark - Test integer values as boolean
640+
641+
// these set of tests are for filtering bug report (Jira #4041): integer 1 is filtered out as boolean unexpectedly
642+
643+
- (void)testRevenueMetricAndValueMetricWithRevenueIntegerOne
644+
{
645+
// Test creating event containing both "revenue" and "value". Imagine
646+
// "revenue" == money received
647+
// "value" == temperature measured
648+
// There isn't a good reason why both can't be sent in the same event.
649+
NSDictionary *tags = @{OPTLYEventMetricNameRevenue:@1,
650+
OPTLYEventMetricNameValue:@(kEventValue)};
651+
NSDictionary *sentTags = tags;
652+
[self commonBuildConversionTicketTest:tags
653+
sentEventTags:sentTags
654+
sentTags:tags];
655+
}
656+
- (void)testRevenueMetricAndValueMetricWithRevenueIntegerZero
657+
{
658+
// Test creating event containing both "revenue" and "value". Imagine
659+
// "revenue" == money received
660+
// "value" == temperature measured
661+
// There isn't a good reason why both can't be sent in the same event.
662+
NSDictionary *tags = @{OPTLYEventMetricNameRevenue:@0,
663+
OPTLYEventMetricNameValue:@(kEventValue)};
664+
NSDictionary *sentTags = tags;
665+
[self commonBuildConversionTicketTest:tags
666+
sentEventTags:sentTags
667+
sentTags:tags];
668+
}
669+
- (void)testRevenueMetricAndValueMetricWithRevenueIntegerTwo
670+
{
671+
// Test creating event containing both "revenue" and "value". Imagine
672+
// "revenue" == money received
673+
// "value" == temperature measured
674+
// There isn't a good reason why both can't be sent in the same event.
675+
NSDictionary *tags = @{OPTLYEventMetricNameRevenue:@2,
676+
OPTLYEventMetricNameValue:@(kEventValue)};
677+
NSDictionary *sentTags = tags;
678+
[self commonBuildConversionTicketTest:tags
679+
sentEventTags:sentTags
680+
sentTags:tags];
681+
}
682+
- (void)testRevenueMetricAndValueMetricWithRevenueBooleanYes
683+
{
684+
// Test creating event containing both "revenue" and "value". Imagine
685+
// "revenue" == money received
686+
// "value" == temperature measured
687+
// There isn't a good reason why both can't be sent in the same event.
688+
NSDictionary *tags = @{OPTLYEventMetricNameRevenue:@YES,
689+
OPTLYEventMetricNameValue:@(kEventValue)};
690+
NSDictionary *sentTags = @{OPTLYEventMetricNameValue:@(kEventValue)};
691+
[self commonBuildConversionTicketTest:tags
692+
sentEventTags:sentTags
693+
sentTags:tags];
694+
}
695+
- (void)testRevenueMetricAndValueMetricWithRevenueBooleanNo
696+
{
697+
// Test creating event containing both "revenue" and "value". Imagine
698+
// "revenue" == money received
699+
// "value" == temperature measured
700+
// There isn't a good reason why both can't be sent in the same event.
701+
NSDictionary *tags = @{OPTLYEventMetricNameRevenue:@NO,
702+
OPTLYEventMetricNameValue:@(kEventValue)};
703+
NSDictionary *sentTags = @{OPTLYEventMetricNameValue:@(kEventValue)};
704+
[self commonBuildConversionTicketTest:tags
705+
sentEventTags: sentTags
706+
sentTags:tags];
707+
}
708+
709+
// value tag checking
710+
711+
- (void)testRevenueMetricAndValueMetricWithValueIntegerOne
712+
{
713+
// Test creating event containing both "revenue" and "value". Imagine
714+
// "revenue" == money received
715+
// "value" == temperature measured
716+
// There isn't a good reason why both can't be sent in the same event.
717+
NSDictionary *tags = @{OPTLYEventMetricNameRevenue:@(kEventRevenue),
718+
OPTLYEventMetricNameValue:@1};
719+
NSDictionary *sentTags = tags;
720+
[self commonBuildConversionTicketTest:tags
721+
sentEventTags:sentTags
722+
sentTags:tags];
723+
}
724+
- (void)testRevenueMetricAndValueMetricWithValueIntegerZero
725+
{
726+
// Test creating event containing both "revenue" and "value". Imagine
727+
// "revenue" == money received
728+
// "value" == temperature measured
729+
// There isn't a good reason why both can't be sent in the same event.
730+
NSDictionary *tags = @{OPTLYEventMetricNameRevenue:@(kEventRevenue),
731+
OPTLYEventMetricNameValue:@0};
732+
NSDictionary *sentTags = tags;
733+
[self commonBuildConversionTicketTest:tags
734+
sentEventTags:sentTags
735+
sentTags:tags];
736+
}
737+
- (void)testRevenueMetricAndValueMetricWithValueIntegerTwo
738+
{
739+
// Test creating event containing both "revenue" and "value". Imagine
740+
// "revenue" == money received
741+
// "value" == temperature measured
742+
// There isn't a good reason why both can't be sent in the same event.
743+
NSDictionary *tags = @{OPTLYEventMetricNameRevenue:@(kEventRevenue),
744+
OPTLYEventMetricNameValue:@2};
745+
NSDictionary *sentTags = tags;
746+
[self commonBuildConversionTicketTest:tags
747+
sentEventTags:sentTags
748+
sentTags:tags];
749+
}
750+
- (void)testRevenueMetricAndValueMetricWithValueIntegerNegativeOne
751+
{
752+
// Test creating event containing both "revenue" and "value". Imagine
753+
// "revenue" == money received
754+
// "value" == temperature measured
755+
// There isn't a good reason why both can't be sent in the same event.
756+
NSDictionary *tags = @{OPTLYEventMetricNameRevenue:@(kEventRevenue),
757+
OPTLYEventMetricNameValue:@(-1)};
758+
NSDictionary *sentTags = tags;
759+
[self commonBuildConversionTicketTest:tags
760+
sentEventTags:sentTags
761+
sentTags:tags];
762+
}
763+
- (void)testRevenueMetricAndValueMetricWithValueBooleanYes
764+
{
765+
// Test creating event containing both "revenue" and "value". Imagine
766+
// "revenue" == money received
767+
// "value" == temperature measured
768+
// There isn't a good reason why both can't be sent in the same event.
769+
NSDictionary *tags = @{OPTLYEventMetricNameRevenue:@(kEventRevenue),
770+
OPTLYEventMetricNameValue:@YES};
771+
NSDictionary *sentTags = @{OPTLYEventMetricNameRevenue:@(kEventRevenue)};
772+
[self commonBuildConversionTicketTest:tags
773+
sentEventTags:sentTags
774+
sentTags:tags];
775+
}
776+
- (void)testRevenueMetricAndValueMetricWithValueBooleanNo
777+
{
778+
// Test creating event containing both "revenue" and "value". Imagine
779+
// "revenue" == money received
780+
// "value" == temperature measured
781+
// There isn't a good reason why both can't be sent in the same event.
782+
NSDictionary *tags = @{OPTLYEventMetricNameRevenue:@(kEventRevenue),
783+
OPTLYEventMetricNameValue:@NO};
784+
NSDictionary *sentTags = @{OPTLYEventMetricNameRevenue:@(kEventRevenue)};
785+
[self commonBuildConversionTicketTest:tags
786+
sentEventTags: sentTags
787+
sentTags:tags];
788+
}
789+
790+
791+
792+
637793
#pragma mark - Test BuildConversionTicket:... with Multiple eventTags
638794

639795
- (void)testBuildConversionTicketWithEventTags
@@ -1110,6 +1266,8 @@ - (void)checkEventTags:(NSDictionary *)eventTags
11101266
// check for number of tags atleast equalt to event tags
11111267
XCTAssert([event count] >= [eventTags count], @"Invalid number of event tags.");
11121268

1269+
// (1) this checks if all expected fields exists in event
1270+
11131271
if ([[eventTags allKeys] containsObject:OPTLYEventMetricNameRevenue]) {
11141272
NSNumber *expectedRevenue = eventTags[OPTLYEventMetricNameRevenue];
11151273
id revenue = event[OPTLYEventMetricNameRevenue];
@@ -1124,6 +1282,23 @@ - (void)checkEventTags:(NSDictionary *)eventTags
11241282
XCTAssert([value isKindOfClass:[NSNumber class]], @"value should be an NSNumber .");
11251283
XCTAssertEqualObjects(value, expectedValue, @"event value should equal to %@ .", expectedValue);
11261284
}
1285+
1286+
// (2) we also need to check if unexpected fields still exists in event ({revenue, value} check only)
1287+
//
1288+
// [Jira #4041] integers {1 and 0} are filtered out accidentally since @YES and @NO interpreted as @1 and @0
1289+
// we need compare with @YES and @NO only for boolean NSNumber (objCType == @encode(char))
1290+
1291+
NSMutableDictionary *eventTagsDefaultsOnly = [NSMutableDictionary new];
1292+
if(eventTags != nil) {
1293+
eventTagsDefaultsOnly[OPTLYEventMetricNameRevenue] = eventTags[OPTLYEventMetricNameRevenue];
1294+
eventTagsDefaultsOnly[OPTLYEventMetricNameValue] = eventTags[OPTLYEventMetricNameValue];
1295+
}
1296+
1297+
NSMutableDictionary *allDefaultTagsInEvent = [NSMutableDictionary new];
1298+
allDefaultTagsInEvent[OPTLYEventMetricNameRevenue] = event[OPTLYEventMetricNameRevenue];
1299+
allDefaultTagsInEvent[OPTLYEventMetricNameValue] = event[OPTLYEventMetricNameValue];
1300+
1301+
XCTAssertEqualObjects(eventTagsDefaultsOnly, allDefaultTagsInEvent, @"Invalid tag filtering");
11271302
}
11281303

11291304
- (void)checkCommonParams:(NSDictionary *)params withAttributes:(NSDictionary *)attributes {

0 commit comments

Comments
 (0)