Skip to content

Commit 7ee99da

Browse files
baumatronbaumatron
committed
Fix duplicated events with multiple upload calls
Fix a problem where multiple calls to uploadWithFinishedBlock: can cause duplicate uploads when a subsequent call starts uploading before a prior call finishes and deletes successfully uploaded events.
1 parent 2b7efa1 commit 7ee99da

File tree

5 files changed

+217
-80
lines changed

5 files changed

+217
-80
lines changed

‎KeenClient.xcodeproj/project.pbxproj‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -82,6 +82,7 @@
8282
480FEB741E846F7500641112 /* KIOUtil.m in Sources */ = {isa = PBXBuildFile; fileRef = 480FEB701E846F7500641112 /* KIOUtil.m */; };
8383
480FEB751E846F7500641112 /* KIOUtil.m in Sources */ = {isa = PBXBuildFile; fileRef = 480FEB701E846F7500641112 /* KIOUtil.m */; };
8484
480FEB761E846F7500641112 /* KIOUtil.m in Sources */ = {isa = PBXBuildFile; fileRef = 480FEB701E846F7500641112 /* KIOUtil.m */; };
85+
481794741EE8F66500586007 /* MockNSURLSession.m in Sources */ = {isa = PBXBuildFile; fileRef = 481794731EE8F66500586007 /* MockNSURLSession.m */; };
8586
481A9B7C1E5690950094B985 /* KeenLogSinkNSLog.h in Headers */ = {isa = PBXBuildFile; fileRef = 481A9B7A1E5690950094B985 /* KeenLogSinkNSLog.h */; };
8687
481A9B7D1E5690950094B985 /* KeenLogSinkNSLog.m in Sources */ = {isa = PBXBuildFile; fileRef = 481A9B7B1E5690950094B985 /* KeenLogSinkNSLog.m */; };
8788
481A9B7E1E5691270094B985 /* KeenLogSink.h in Headers */ = {isa = PBXBuildFile; fileRef = 481A9B711E5687EA0094B985 /* KeenLogSink.h */; settings = {ATTRIBUTES = (Public, ); }; };
@@ -193,6 +194,8 @@
193194
3EE9A7301C59873F00B7B2D9 /* Info.plist */ = {isa = PBXFileReference; lastKnownFileType = text.plist.xml; path = Info.plist; sourceTree = "<group>"; };
194195
480FEB6F1E846F7500641112 /* KIOUtil.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = KIOUtil.h; sourceTree = "<group>"; };
195196
480FEB701E846F7500641112 /* KIOUtil.m */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.objc; path = KIOUtil.m; sourceTree = "<group>"; };
197+
481794721EE8F66500586007 /* MockNSURLSession.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = MockNSURLSession.h; sourceTree = "<group>"; };
198+
481794731EE8F66500586007 /* MockNSURLSession.m */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.objc; path = MockNSURLSession.m; sourceTree = "<group>"; };
196199
481A9B711E5687EA0094B985 /* KeenLogSink.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = KeenLogSink.h; sourceTree = "<group>"; };
197200
481A9B7A1E5690950094B985 /* KeenLogSinkNSLog.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = KeenLogSinkNSLog.h; sourceTree = "<group>"; };
198201
481A9B7B1E5690950094B985 /* KeenLogSinkNSLog.m */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.objc; path = KeenLogSinkNSLog.m; sourceTree = "<group>"; };
@@ -383,6 +386,8 @@
383386
3E63D9F61AE7425000E57A17 /* KIOQueryTests.m */,
384387
48583E0D1E5904FD002CFD99 /* KeenLoggerTests.h */,
385388
48583E0E1E5904FD002CFD99 /* KeenLoggerTests.m */,
389+
481794721EE8F66500586007 /* MockNSURLSession.h */,
390+
481794731EE8F66500586007 /* MockNSURLSession.m */,
386391
);
387392
path = KeenClientTests;
388393
sourceTree = "<group>";
@@ -713,6 +718,7 @@
713718
isa = PBXSourcesBuildPhase;
714719
buildActionMask = 2147483647;
715720
files = (
721+
481794741EE8F66500586007 /* MockNSURLSession.m in Sources */,
716722
CA6410E218E39F3A00E53E3C /* KIODBStoreTests.m in Sources */,
717723
3E63D9F71AE7425000E57A17 /* KIOQueryTests.m in Sources */,
718724
48583E0F1E5904FD002CFD99 /* KeenLoggerTests.m in Sources */,

‎KeenClient/KIOUploader.m‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,10 @@ - (void)handleEventAPIResponse:(NSURLResponse *)response
4040

4141
@property (nonatomic) KIONetwork *network;
4242

43+
@property BOOL isUploading;
44+
45+
@property NSCondition *isUploadingCondition;
46+
4347
@end
4448

4549

@@ -74,6 +78,9 @@ - (instancetype)initWithNetwork:(KIONetwork *)network
7478

7579
self.maxEventUploadAttempts = 3;
7680

81+
self.isUploadingCondition = [NSCondition new];
82+
self.isUploading = NO;
83+
7784
self.network = network;
7885

7986
self.store = store;
@@ -179,6 +186,10 @@ - (void)uploadEventsForProjectID:(NSString *)projectID
179186
}
180187
}
181188

189+
[self.isUploadingCondition lock];
190+
self.isUploading = YES;
191+
[self.isUploadingCondition unlock];
192+
182193
// then make an http request to the keen server.
183194
[self.network sendEvents:data
184195
withProjectID:projectID
@@ -188,7 +199,20 @@ - (void)uploadEventsForProjectID:(NSString *)projectID
188199
[self handleEventAPIResponse:response andData:data forEvents:eventIDs];
189200

190201
[self runUploadFinishedBlock:block];
202+
203+
[self.isUploadingCondition lock];
204+
self.isUploading = NO;
205+
[self.isUploadingCondition signal];
206+
[self.isUploadingCondition unlock];
191207
}];
208+
209+
// Block the queue until uploading has finished.
210+
// Otherwise we'll pick up events that are in flight and try to upload them again
211+
[self.isUploadingCondition lock];
212+
while (self.isUploading) {
213+
[self.isUploadingCondition waitUntilDate:[NSDate dateWithTimeIntervalSinceNow:60]];
214+
}
215+
[self.isUploadingCondition unlock];
192216
}
193217
});
194218
}

‎KeenClientTests/KeenClientTests.m‎

Lines changed: 113 additions & 80 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
#import "KIOFileStore.h"
1818
#import "KIONetwork.h"
1919
#import "KIOUploader.h"
20+
#import "MockNSURLSession.h"
2021

2122
NSString* kDefaultProjectID = @"id";
2223
NSString* kDefaultWriteKey = @"wk";
@@ -44,7 +45,9 @@ @interface KeenClient (testability)
4445
@property (nonatomic, strong) NSString *writeKey;
4546
@property (nonatomic, strong) NSString *readKey;
4647

47-
@property (nonatomic) KIONetwork* network;
48+
@property (nonatomic) KIONetwork *network;
49+
50+
@property (nonatomic) KIODBStore *store;
4851

4952
// If we're running tests.
5053
@property (nonatomic) BOOL isRunningTests;
@@ -502,21 +505,8 @@ - (id)mockUrlSessionWithResponse:(NSHTTPURLResponse*)response
502505
andResponseData:(NSData*)responseData
503506
andRequestValidator:(BOOL (^)(id requestObject))requestValidator {
504507
// Mock the NSURLSession to be used for the request
505-
id urlSessionMock = [OCMockObject partialMockForObject:[[NSURLSession alloc] init]];
506-
507-
// Set up fake response data and request validation
508-
if (nil != requestValidator) {
509-
// Set up validation of the request
510-
[[urlSessionMock expect] dataTaskWithRequest:[OCMArg checkWithBlock:requestValidator]
511-
completionHandler:[OCMArg invokeBlockWithArgs:responseData, response, [NSNull null], nil]];
512-
} else {
513-
// We won't check that the request contained anything specific
514-
[[urlSessionMock stub] dataTaskWithRequest:[OCMArg any]
515-
completionHandler:[OCMArg invokeBlockWithArgs:responseData, response, [NSNull null], nil]];
516-
517-
}
518-
519-
return urlSessionMock;
508+
// If a validator is provided, it will check the actual request made
509+
return [[MockNSURLSession alloc] initWithValidator:requestValidator data:responseData response:response error:nil];
520510
}
521511

522512
- (id)createClientWithResponseData:(id)data
@@ -583,7 +573,7 @@ - (void)addSimpleEventAndUploadWithMock:(id)mock andFinishedBlock:(void (^)())fi
583573

584574
-(void)testUploadWithNoEvents {
585575
XCTestExpectation* uploadFinishedBlockCalled = [self expectationWithDescription:@"Upload should finish."];
586-
576+
587577
id mock = [self createClientWithResponseData:nil andStatusCode:HTTPCode200OK];
588578

589579
[mock uploadWithFinishedBlock:^{
@@ -883,6 +873,46 @@ - (void)testUploadMultipleEventsSameCollectionSuccess {
883873
}];
884874
}
885875

876+
- (void)testUploadEventDoesntDuplicateWithMultipleCallsToUpload {
877+
NSDictionary *eventResult = [self buildResultWithSuccess:YES andErrorCode:nil andDescription:nil];
878+
NSDictionary *result =
879+
[NSDictionary dictionaryWithObject:[NSArray arrayWithObjects:eventResult, nil] forKey:@"foo"];
880+
881+
__block NSInteger requestCount = 0;
882+
__block NSInteger uploadCallCount = 0;
883+
const NSInteger totalUploadCallCount = 10;
884+
KeenClient* client = [self createClientWithResponseData:result andStatusCode:HTTPCode200OK andNetworkConnected:@YES andRequestValidator:^BOOL(id obj) {
885+
requestCount++;
886+
return @YES;
887+
}];
888+
889+
// add an event
890+
[client addEvent:[NSDictionary dictionaryWithObject:@"apple" forKey:@"a"] toEventCollection:@"foo" error:nil];
891+
892+
NSUInteger pendingEvents = [client.store getTotalEventCountWithProjectID:kDefaultProjectID];
893+
XCTAssertEqual(pendingEvents, 1, @"There should be an event awaiting upload.");
894+
895+
// and "upload" it
896+
XCTestExpectation *responseArrived = [self expectationWithDescription:@"response of async request has arrived"];
897+
for (int i = 0; i < totalUploadCallCount; ++i) {
898+
[client uploadWithFinishedBlock:^{
899+
uploadCallCount++;
900+
if (uploadCallCount == totalUploadCallCount) {
901+
[responseArrived fulfill];
902+
}
903+
}];
904+
}
905+
906+
[self waitForExpectationsWithTimeout:_asyncTimeInterval
907+
handler:^(NSError *_Nullable error) {
908+
// make sure the events were deleted locally
909+
XCTAssertTrue([client.store getTotalEventCountWithProjectID:nil] == 0,
910+
@"There should be no files after a successful upload.");
911+
}];
912+
913+
XCTAssertEqual(requestCount, 1, @"Only one request should have been made");
914+
}
915+
886916
- (void)testUploadMultipleEventsSameCollectionSuccessInstanceClient {
887917
NSDictionary *result1 = [self buildResultWithSuccess:YES
888918
andErrorCode:nil
@@ -1492,7 +1522,7 @@ - (void)testUploadMultipleTimes {
14921522
XCTestExpectation* uploadFinishedBlockCalled1 = [self expectationWithDescription:@"Upload 1 should run to completion."];
14931523
XCTestExpectation* uploadFinishedBlockCalled2 = [self expectationWithDescription:@"Upload 2 should run to completion."];
14941524
XCTestExpectation* uploadFinishedBlockCalled3 = [self expectationWithDescription:@"Upload 3 should run to completion."];
1495-
1525+
14961526
KeenClient *client = [KeenClient sharedClientWithProjectID:kDefaultProjectID andWriteKey:kDefaultWriteKey andReadKey:kDefaultReadKey];
14971527
client.isRunningTests = YES;
14981528

@@ -1505,7 +1535,7 @@ - (void)testUploadMultipleTimes {
15051535
[client uploadWithFinishedBlock:^ {
15061536
[uploadFinishedBlockCalled3 fulfill];
15071537
}];
1508-
1538+
15091539
[self waitForExpectationsWithTimeout:_asyncTimeInterval handler:nil];
15101540
}
15111541

@@ -1526,7 +1556,7 @@ - (void)testUploadMultipleTimesInstanceClient {
15261556
[client uploadWithFinishedBlock:^ {
15271557
[uploadFinishedBlockCalled3 fulfill];
15281558
}];
1529-
1559+
15301560
[self waitForExpectationsWithTimeout:_asyncTimeInterval handler:nil];
15311561
}
15321562

@@ -1946,90 +1976,93 @@ - (void)validateSdkVersionHeaderFieldForRequest:(id)requestObject {
19461976
}
19471977

19481978
- (void)testSdkTrackingHeadersOnUpload {
1979+
XCTestExpectation *requestValidated = [self expectationWithDescription:@"request was validated"];
19491980
// mock an empty response from the server
1950-
1951-
KeenClient* client = [self createClientWithRequestValidator:^BOOL(id obj) {
1981+
KeenClient *client = [self createClientWithRequestValidator:^BOOL(id obj) {
19521982
[self validateSdkVersionHeaderFieldForRequest:obj];
1983+
[requestValidated fulfill];
19531984
return @YES;
19541985
}];
19551986

1956-
// Get the mock url session. We'll check the request it gets passed by sendEvents for the version header
1957-
id urlSessionMock = client.network.urlSession;
1958-
19591987
// add an event
19601988
[client addEvent:[NSDictionary dictionaryWithObject:@"apple" forKey:@"a"] toEventCollection:@"foo" error:nil];
19611989

1962-
XCTestExpectation* responseArrived = [self expectationWithDescription:@"response of async request has arrived"];
1990+
XCTestExpectation *responseArrived = [self expectationWithDescription:@"response of async request has arrived"];
19631991
// and "upload" it
19641992
[client uploadWithFinishedBlock:^{
1965-
// Check for the sdk version header
1966-
[urlSessionMock verify];
1967-
19681993
[responseArrived fulfill];
19691994
}];
19701995

1971-
[self waitForExpectationsWithTimeout:_asyncTimeInterval handler:^(NSError * _Nullable error) {
1972-
XCTAssertNil(error, @"Test should complete within expected interval.");
1973-
}];
1996+
[self waitForExpectationsWithTimeout:_asyncTimeInterval
1997+
handler:^(NSError *_Nullable error) {
1998+
XCTAssertNil(error, @"Test should complete within expected interval.");
1999+
}];
19742000
}
19752001

19762002
- (void)testSdkTrackingHeadersOnQuery {
1977-
KeenClient* client = [self createClientWithResponseData:@{@"result": @10}
1978-
andStatusCode:HTTPCode200OK
1979-
andNetworkConnected:@YES
1980-
andRequestValidator:^BOOL(id obj) {
1981-
[self validateSdkVersionHeaderFieldForRequest:obj];
1982-
return @YES;
1983-
}];
1984-
1985-
// Get the mock url session. We'll check the request it gets passed by sendEvents for the version header
1986-
id urlSessionMock = client.network.urlSession;
1987-
1988-
KIOQuery *query = [[KIOQuery alloc] initWithQuery:@"count" andPropertiesDictionary:@{@"event_collection": @"event_collection"}];
2003+
XCTestExpectation *requestValidated = [self expectationWithDescription:@"request was validated"];
2004+
KeenClient *client = [self createClientWithResponseData:@{
2005+
@"result": @10
2006+
}
2007+
andStatusCode:HTTPCode200OK
2008+
andNetworkConnected:@YES
2009+
andRequestValidator:^BOOL(id obj) {
2010+
[self validateSdkVersionHeaderFieldForRequest:obj];
2011+
[requestValidated fulfill];
2012+
return @YES;
2013+
}];
19892014

1990-
XCTestExpectation* responseArrived = [self expectationWithDescription:@"response of async request has arrived"];
1991-
[client runQuery:query completionHandler:^(NSData *queryResponseData, NSURLResponse *response, NSError *error) {
1992-
// Check for the sdk version header
1993-
[urlSessionMock verify];
2015+
KIOQuery *query =
2016+
[[KIOQuery alloc] initWithQuery:@"count" andPropertiesDictionary:@{
2017+
@"event_collection": @"event_collection"
2018+
}];
19942019

1995-
[responseArrived fulfill];
1996-
}];
2020+
XCTestExpectation *responseArrived = [self expectationWithDescription:@"response of async request has arrived"];
2021+
[client runAsyncQuery:query
2022+
block:^(NSData *queryResponseData, NSURLResponse *response, NSError *error) {
2023+
[responseArrived fulfill];
2024+
}];
19972025

1998-
[self waitForExpectationsWithTimeout:_asyncTimeInterval handler:^(NSError * _Nullable error) {
1999-
XCTAssertNil(error, @"Test should complete within expected interval.");
2000-
}];
2026+
[self waitForExpectationsWithTimeout:_asyncTimeInterval
2027+
handler:^(NSError *_Nullable error) {
2028+
XCTAssertNil(error, @"Test should complete within expected interval.");
2029+
}];
20012030
}
20022031

20032032
- (void)testSdkTrackingHeadersOnMultiAnalysis {
2004-
KeenClient* client = [self createClientWithResponseData:@{@"result": @{@"query1": @10, @"query2": @1}}
2005-
andStatusCode:HTTPCode200OK
2006-
andNetworkConnected:@YES
2007-
andRequestValidator:^BOOL(id obj) {
2008-
[self validateSdkVersionHeaderFieldForRequest:obj];
2009-
return @YES;
2010-
}];
2011-
2012-
// Get the mock url session. We'll check the request it gets passed by sendEvents for the version header
2013-
id urlSessionMock = client.network.urlSession;
2014-
2015-
KIOQuery* countQuery = [[KIOQuery alloc] initWithQuery:@"count"
2016-
andPropertiesDictionary:@{@"event_collection": @"event_collection"}];
2017-
2018-
KIOQuery* averageQuery = [[KIOQuery alloc] initWithQuery:@"count_unique"
2019-
andPropertiesDictionary:@{@"event_collection": @"event_collection", @"target_property": @"something"}];
2020-
2021-
XCTestExpectation* responseArrived = [self expectationWithDescription:@"response of async request has arrived"];
2022-
[client runMultiAnalysisWithQueries:@[countQuery, averageQuery]
2023-
completionHandler:^(NSData* queryResponseData, NSURLResponse* response, NSError* error) {
2024-
// Check for the sdk version header
2025-
[urlSessionMock verify];
2033+
XCTestExpectation *requestValidated = [self expectationWithDescription:@"request was validated"];
2034+
KeenClient *client = [self createClientWithResponseData:@{
2035+
@"result": @{@"query1": @10, @"query2": @1}
2036+
}
2037+
andStatusCode:HTTPCode200OK
2038+
andNetworkConnected:@YES
2039+
andRequestValidator:^BOOL(id obj) {
2040+
[self validateSdkVersionHeaderFieldForRequest:obj];
2041+
[requestValidated fulfill];
2042+
return @YES;
2043+
}];
20262044

2027-
[responseArrived fulfill];
2028-
}];
2045+
KIOQuery *countQuery =
2046+
[[KIOQuery alloc] initWithQuery:@"count" andPropertiesDictionary:@{
2047+
@"event_collection": @"event_collection"
2048+
}];
20292049

2030-
[self waitForExpectationsWithTimeout:_asyncTimeInterval handler:^(NSError * _Nullable error) {
2031-
XCTAssertNil(error, @"Test should complete within expected interval.");
2032-
}];
2050+
KIOQuery *averageQuery = [[KIOQuery alloc] initWithQuery:@"count_unique"
2051+
andPropertiesDictionary:@{
2052+
@"event_collection": @"event_collection",
2053+
@"target_property": @"something"
2054+
}];
2055+
2056+
XCTestExpectation *responseArrived = [self expectationWithDescription:@"response of async request has arrived"];
2057+
[client runAsyncMultiAnalysisWithQueries:@[countQuery, averageQuery]
2058+
block:^(NSData *queryResponseData, NSURLResponse *response, NSError *error) {
2059+
[responseArrived fulfill];
2060+
}];
2061+
2062+
[self waitForExpectationsWithTimeout:_asyncTimeInterval
2063+
handler:^(NSError *_Nullable error) {
2064+
XCTAssertNil(error, @"Test should complete within expected interval.");
2065+
}];
20332066
}
20342067

20352068
# pragma mark - test filesystem utility methods

‎KeenClientTests/MockNSURLSession.h‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
//
2+
// MockNSURLSession.h
3+
// KeenClient
4+
//
5+
// Created by Brian Baumhover on 6/7/17.
6+
// Copyright © 2017 Keen Labs. All rights reserved.
7+
//
8+
9+
#import <Foundation/Foundation.h>
10+
11+
@interface MockNSURLSession : NSObject
12+
13+
- (instancetype)initWithValidator:(BOOL (^)(id requestObject))validator data:(NSData *)data response:(NSURLResponse *)response error:(NSError *)error;
14+
15+
- (NSURLSessionDataTask *)dataTaskWithRequest:(NSURLRequest *)request
16+
completionHandler:(void (^)(NSData *_Nullable data,
17+
NSURLResponse *_Nullable response,
18+
NSError *_Nullable error))completionHandler;
19+
20+
@end
21+

0 commit comments

Comments
 (0)