Skip to content

Commit 57c400f

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 fd681b1 commit 57c400f

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
@@ -31,17 +31,8 @@ + (NSNumber *)getRevenueValue:(NSDictionary *)eventTags logger:(id<OPTLYLogger>)
3131
if ([value isKindOfClass:[NSNumber class]]) {
3232
answer = (NSNumber*)value;
3333
const char *objCType = [answer objCType];
34-
// Dispatch objCType according to one of "Type Encodings" listed here:
35-
// https://developer.apple.com/library/content/documentation/Cocoa/Conceptual/ObjCRuntimeGuide/Articles/ocrtTypeEncodings.html
36-
if ((strcmp(objCType, @encode(bool)) == 0)
37-
|| [value isEqual:@YES]
38-
|| [value isEqual:@NO]) {
39-
// NSNumber's generated by "+ (NSNumber *)numberWithBool:(BOOL)value;"
40-
// serialize to JSON booleans "true" and "false" via NSJSONSerialization .
41-
// The @YES and @NO compile to __NSCFBoolean's which (strangely enough)
42-
// are ((strcmp(objCType, @encode(char)) == 0) but these serialize as
43-
// JSON booleans "true" and "false" instead of JSON numbers.
44-
// These aren't integers, so shouldn't be sent.
34+
35+
if ([self isNSNumberBoolean:answer]) {
4536
answer = nil;
4637
[logger logMessage:OPTLYLoggerMessagesRevenueValueInvalidBoolean withLevel:OptimizelyLogLevelWarning];
4738
} else if ((strcmp(objCType, @encode(char)) == 0)
@@ -115,17 +106,8 @@ + (NSNumber *)getNumericValue:(NSDictionary *)eventTags logger:(id<OPTLYLogger>)
115106
if ([value isKindOfClass:[NSNumber class]]) {
116107
answer = (NSNumber*)value;
117108
const char *objCType = [answer objCType];
118-
// Dispatch objCType according to one of "Type Encodings" listed here:
119-
// https://developer.apple.com/library/content/documentation/Cocoa/Conceptual/ObjCRuntimeGuide/Articles/ocrtTypeEncodings.html
120-
if ((strcmp(objCType, @encode(bool)) == 0)
121-
|| [value isEqual:@YES]
122-
|| [value isEqual:@NO]) {
123-
// NSNumber's generated by "+ (NSNumber *)numberWithBool:(BOOL)value;"
124-
// serialize to JSON booleans "true" and "false" via NSJSONSerialization .
125-
// The @YES and @NO compile to __NSCFBoolean's which (strangely enough)
126-
// are ((strcmp(objCType, @encode(char)) == 0) but these serialize as
127-
// JSON booleans "true" and "false" instead of JSON numbers.
128-
// These aren't integers, so shouldn't be sent.
109+
110+
if ([self isNSNumberBoolean:answer]) {
129111
answer = nil;
130112
[logger logMessage:OPTLYLoggerMessagesNumericValueInvalidBoolean withLevel:OptimizelyLogLevelWarning];
131113
} else {
@@ -154,4 +136,25 @@ + (NSNumber *)getNumericValue:(NSDictionary *)eventTags logger:(id<OPTLYLogger>)
154136
return answer;
155137
}
156138

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

‎OptimizelySDKCore/OptimizelySDKCoreTests/OPTLYEventBuilderTest.m

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

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

641797
- (void)testBuildConversionTicketWithEventTags
@@ -1119,6 +1275,8 @@ - (void)checkEventTags:(NSDictionary *)eventTags
11191275
// check for number of tags atleast equalt to event tags
11201276
XCTAssert([event count] >= [eventTags count], @"Invalid number of event tags.");
11211277

1278+
// (1) this checks if all expected fields exists in event
1279+
11221280
if ([[eventTags allKeys] containsObject:OPTLYEventMetricNameRevenue]) {
11231281
NSNumber *expectedRevenue = eventTags[OPTLYEventMetricNameRevenue];
11241282
id revenue = event[OPTLYEventMetricNameRevenue];
@@ -1133,6 +1291,23 @@ - (void)checkEventTags:(NSDictionary *)eventTags
11331291
XCTAssert([value isKindOfClass:[NSNumber class]], @"value should be an NSNumber .");
11341292
XCTAssertEqualObjects(value, expectedValue, @"event value should equal to %@ .", expectedValue);
11351293
}
1294+
1295+
// (2) we also need to check if unexpected fields still exists in event ({revenue, value} check only)
1296+
//
1297+
// [Jira #4041] integers {1 and 0} are filtered out accidentally since @YES and @NO interpreted as @1 and @0
1298+
// we need compare with @YES and @NO only for boolean NSNumber (objCType == @encode(char))
1299+
1300+
NSMutableDictionary *eventTagsDefaultsOnly = [NSMutableDictionary new];
1301+
if(eventTags != nil) {
1302+
eventTagsDefaultsOnly[OPTLYEventMetricNameRevenue] = eventTags[OPTLYEventMetricNameRevenue];
1303+
eventTagsDefaultsOnly[OPTLYEventMetricNameValue] = eventTags[OPTLYEventMetricNameValue];
1304+
}
1305+
1306+
NSMutableDictionary *allDefaultTagsInEvent = [NSMutableDictionary new];
1307+
allDefaultTagsInEvent[OPTLYEventMetricNameRevenue] = event[OPTLYEventMetricNameRevenue];
1308+
allDefaultTagsInEvent[OPTLYEventMetricNameValue] = event[OPTLYEventMetricNameValue];
1309+
1310+
XCTAssertEqualObjects(eventTagsDefaultsOnly, allDefaultTagsInEvent, @"Invalid tag filtering");
11361311
}
11371312

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

0 commit comments

Comments
 (0)