From 9ad9486ba5f3f968ea69296792cc3a9c6ee23228 Mon Sep 17 00:00:00 2001 From: Prateek Srivastava Date: Sun, 3 Dec 2017 11:26:26 -0800 Subject: [PATCH] Revert "Create HTTP requests on a background queue." This reverts commit 2e6c57d4146ae2f25b5b372a430224de10319c54. --- Analytics/Classes/Internal/SEGHTTPClient.h | 2 +- Analytics/Classes/Internal/SEGHTTPClient.m | 13 ++--- .../Classes/Internal/SEGSegmentIntegration.m | 55 ++++++++----------- AnalyticsTests/HTTPClientTest.swift | 9 ++- 4 files changed, 34 insertions(+), 45 deletions(-) diff --git a/Analytics/Classes/Internal/SEGHTTPClient.h b/Analytics/Classes/Internal/SEGHTTPClient.h index 83d9febc6..4cd904dc2 100644 --- a/Analytics/Classes/Internal/SEGHTTPClient.h +++ b/Analytics/Classes/Internal/SEGHTTPClient.h @@ -32,7 +32,7 @@ NS_ASSUME_NONNULL_BEGIN * NOTE: You need to re-dispatch within the completionHandler onto a desired queue to avoid threading issues. * Completion handlers are called on a dispatch queue internal to SEGHTTPClient. */ -- (NSURLSessionUploadTask *)upload:(NSArray *)batch forWriteKey:(NSString *)writeKey completionHandler:(void (^)(BOOL retry))completionHandler; +- (NSURLSessionUploadTask *)upload:(JSON_DICT)batch forWriteKey:(NSString *)writeKey completionHandler:(void (^)(BOOL retry))completionHandler; - (NSURLSessionDataTask *)settingsForWriteKey:(NSString *)writeKey completionHandler:(void (^)(BOOL success, JSON_DICT _Nullable settings))completionHandler; diff --git a/Analytics/Classes/Internal/SEGHTTPClient.m b/Analytics/Classes/Internal/SEGHTTPClient.m index e2eb0ac4a..a1984fad5 100644 --- a/Analytics/Classes/Internal/SEGHTTPClient.m +++ b/Analytics/Classes/Internal/SEGHTTPClient.m @@ -62,7 +62,7 @@ - (void)dealloc } -- (NSURLSessionUploadTask *)upload:(NSArray *)batch forWriteKey:(NSString *)writeKey completionHandler:(void (^)(BOOL retry))completionHandler +- (NSURLSessionUploadTask *)upload:(NSDictionary *)batch forWriteKey:(NSString *)writeKey completionHandler:(void (^)(BOOL retry))completionHandler { // batch = SEGCoerceDictionary(batch); NSURLSession *session = [self sessionForWriteKey:writeKey]; @@ -75,16 +75,11 @@ - (NSURLSessionUploadTask *)upload:(NSArray *)batch forWriteKey:(NSString *)writ [request setHTTPMethod:@"POST"]; - // In particular, set the sentAt after the requestFactory is invoked so that it can be as up to date as possible. - NSMutableDictionary *payload = [[NSMutableDictionary alloc] init]; - [payload setObject:iso8601FormattedString([NSDate date]) forKey:@"sentAt"]; - [payload setObject:batch forKey:@"batch"]; - NSError *error = nil; NSException *exception = nil; - NSData *payloadData = nil; + NSData *payload = nil; @try { - payloadData = [NSJSONSerialization dataWithJSONObject:batch options:0 error:&error]; + payload = [NSJSONSerialization dataWithJSONObject:batch options:0 error:&error]; } @catch (NSException *exc) { exception = exc; @@ -94,7 +89,7 @@ - (NSURLSessionUploadTask *)upload:(NSArray *)batch forWriteKey:(NSString *)writ completionHandler(NO); // Don't retry this batch. return nil; } - NSData *gzippedPayload = [payloadData seg_gzippedData]; + NSData *gzippedPayload = [payload seg_gzippedData]; NSURLSessionUploadTask *task = [session uploadTaskWithRequest:request fromData:gzippedPayload completionHandler:^(NSData *_Nullable data, NSURLResponse *_Nullable response, NSError *_Nullable error) { if (error) { diff --git a/Analytics/Classes/Internal/SEGSegmentIntegration.m b/Analytics/Classes/Internal/SEGSegmentIntegration.m index f7e78132e..2212c7b9a 100644 --- a/Analytics/Classes/Internal/SEGSegmentIntegration.m +++ b/Analytics/Classes/Internal/SEGSegmentIntegration.m @@ -55,12 +55,10 @@ @interface SEGSegmentIntegration () @property (nonatomic, strong) NSMutableArray *queue; @property (nonatomic, strong) NSDictionary *cachedStaticContext; @property (nonatomic, strong) NSURLSessionUploadTask *batchRequest; -@property (nonatomic, assign) BOOL batchRequestInProgress; // Thread confined to serialQueue. @property (nonatomic, assign) UIBackgroundTaskIdentifier flushTaskID; @property (nonatomic, strong) SEGReachability *reachability; @property (nonatomic, strong) NSTimer *flushTimer; @property (nonatomic, strong) dispatch_queue_t serialQueue; -@property (nonatomic, strong) dispatch_queue_t networkQueue; @property (nonatomic, strong) dispatch_queue_t backgroundTaskQueue; @property (nonatomic, strong) NSMutableDictionary *traits; @property (nonatomic, assign) SEGAnalytics *analytics; @@ -89,9 +87,8 @@ - (id)initWithAnalytics:(SEGAnalytics *)analytics httpClient:(SEGHTTPClient *)ht self.reachability = [SEGReachability reachabilityWithHostname:@"google.com"]; [self.reachability startNotifier]; self.cachedStaticContext = [self staticContext]; - self.serialQueue = seg_dispatch_queue_create_specific("com.segment.analytics.segment", DISPATCH_QUEUE_SERIAL); - self.networkQueue = seg_dispatch_queue_create_specific("com.segment.analytics.segment.network", DISPATCH_QUEUE_SERIAL); - self.backgroundTaskQueue = seg_dispatch_queue_create_specific("com.segment.analytics.segment.backgroundTask", DISPATCH_QUEUE_SERIAL); + self.serialQueue = seg_dispatch_queue_create_specific("io.segment.analytics.segmentio", DISPATCH_QUEUE_SERIAL); + self.backgroundTaskQueue = seg_dispatch_queue_create_specific("io.segment.analytics.backgroundTask", DISPATCH_QUEUE_SERIAL); self.flushTaskID = UIBackgroundTaskInvalid; #if !TARGET_OS_TV @@ -490,7 +487,7 @@ - (void)flushWithMaxSize:(NSUInteger)maxBatchSize [self endBackgroundTask]; return; } - if (self.batchRequestInProgress) { + if (self.batchRequest != nil) { SEGLog(@"%@ API request already in progress, not flushing again.", self); return; } @@ -511,7 +508,7 @@ - (void)flushQueueByLength [self dispatchBackground:^{ SEGLog(@"%@ Length is %lu.", self, (unsigned long)self.queue.count); - if (!self.batchRequestInProgress && [self.queue count] >= self.configuration.flushAt) { + if (self.batchRequest == nil && [self.queue count] >= self.configuration.flushAt) { [self flush]; } }]; @@ -543,37 +540,31 @@ - (void)notifyForName:(NSString *)name userInfo:(id)userInfo - (void)sendData:(NSArray *)batch { - if (self.batchRequestInProgress) { - SEGLog(@"API request already in progress, not flushing again."); - return; - } - - self.batchRequestInProgress = true; + NSMutableDictionary *payload = [[NSMutableDictionary alloc] init]; + [payload setObject:iso8601FormattedString([NSDate date]) forKey:@"sentAt"]; + [payload setObject:batch forKey:@"batch"]; - seg_dispatch_specific_async(self.networkQueue, ^{ - SEGLog(@"%@ Flushing %lu of %lu queued API calls.", self, (unsigned long)batch.count, (unsigned long)self.queue.count); + SEGLog(@"%@ Flushing %lu of %lu queued API calls.", self, (unsigned long)batch.count, (unsigned long)self.queue.count); + SEGLog(@"Flushing batch %@.", payload); - self.batchRequest = [self.httpClient upload:batch forWriteKey:self.configuration.writeKey completionHandler:^(BOOL retry) { - [self dispatchBackground:^{ - self.batchRequestInProgress = false; - - if (retry) { - [self notifyForName:SEGSegmentRequestDidFailNotification userInfo:batch]; - self.batchRequest = nil; - [self endBackgroundTask]; - return; - } - - [self.queue removeObjectsInArray:batch]; - [self persistQueue]; - [self notifyForName:SEGSegmentRequestDidSucceedNotification userInfo:batch]; + self.batchRequest = [self.httpClient upload:payload forWriteKey:self.configuration.writeKey completionHandler:^(BOOL retry) { + [self dispatchBackground:^{ + if (retry) { + [self notifyForName:SEGSegmentRequestDidFailNotification userInfo:batch]; self.batchRequest = nil; [self endBackgroundTask]; - }]; + return; + } + + [self.queue removeObjectsInArray:batch]; + [self persistQueue]; + [self notifyForName:SEGSegmentRequestDidSucceedNotification userInfo:batch]; + self.batchRequest = nil; + [self endBackgroundTask]; }]; + }]; - [self notifyForName:SEGSegmentDidSendRequestNotification userInfo:batch]; - }); + [self notifyForName:SEGSegmentDidSendRequestNotification userInfo:batch]; } - (void)applicationDidEnterBackground diff --git a/AnalyticsTests/HTTPClientTest.swift b/AnalyticsTests/HTTPClientTest.swift index dfd73c563..912cc97ba 100644 --- a/AnalyticsTests/HTTPClientTest.swift +++ b/AnalyticsTests/HTTPClientTest.swift @@ -95,8 +95,11 @@ class HTTPClientTest: QuickSpec { describe("upload") { it("does not ask to retry for json error") { - // Dates cannot be serialized as is so the json serialzation will fail. - let batch = [[ "foo" : NSDate() ] ] + let batch: [String: Any] = [ + // Dates cannot be serialized as is so the json serialzation will fail. + "sentAt": NSDate(), + "batch": [["type": "track", "event": "foo"]], + ] var done = false let task = client.upload(batch, forWriteKey: "bar") { retry in expect(retry) == false @@ -106,7 +109,7 @@ class HTTPClientTest: QuickSpec { expect(done).toEventually(beTrue()) } - let batch = [["type":"track", "event":"foo"]] + let batch: [String: Any] = ["sentAt":"2016-07-19'T'19:25:06Z", "batch":[["type":"track", "event":"foo"]]] it("does not ask to retry for 2xx response") {