From e2c11595fb078244477fa338a73005791accef8a Mon Sep 17 00:00:00 2001 From: Michael Haney Date: Wed, 16 Jan 2019 14:07:59 -0800 Subject: [PATCH 1/7] Rename fields related to the previous notion of 'backends' to 'uploaders'. Also changes the declaration of the uploader callback block. --- .../GoogleDataLogger/Classes/GDLRegistrar.m | 4 +-- .../Classes/Private/GDLRegistrar_Private.h | 2 +- .../Classes/Public/GDLLogTargets.h | 28 +++++++++++++++++++ .../Classes/Public/GDLLogUploader.h | 8 ++++-- .../Classes/Public/GDLRegistrar.h | 8 +----- .../Unit/Categories/GDLRegistrar+Testing.m | 2 +- .../Tests/Unit/Helpers/GDLTestUploader.h | 2 +- .../Tests/Unit/Helpers/GDLTestUploader.m | 4 +-- 8 files changed, 41 insertions(+), 17 deletions(-) create mode 100644 GoogleDataLogger/GoogleDataLogger/Classes/Public/GDLLogTargets.h diff --git a/GoogleDataLogger/GoogleDataLogger/Classes/GDLRegistrar.m b/GoogleDataLogger/GoogleDataLogger/Classes/GDLRegistrar.m index 3d647301c91..d23baf42467 100644 --- a/GoogleDataLogger/GoogleDataLogger/Classes/GDLRegistrar.m +++ b/GoogleDataLogger/GoogleDataLogger/Classes/GDLRegistrar.m @@ -33,13 +33,13 @@ - (instancetype)init { self = [super init]; if (self) { _logTargetToPrioritizer = [[NSMutableDictionary alloc] init]; - _logTargetToBackend = [[NSMutableDictionary alloc] init]; + _logTargetToUploader = [[NSMutableDictionary alloc] init]; } return self; } - (void)registerBackend:(id)backend forLogTarget:(GDLLogTarget)logTarget { - self.logTargetToBackend[@(logTarget)] = backend; + self.logTargetToUploader[@(logTarget)] = backend; } - (void)registerLogPrioritizer:(id)prioritizer diff --git a/GoogleDataLogger/GoogleDataLogger/Classes/Private/GDLRegistrar_Private.h b/GoogleDataLogger/GoogleDataLogger/Classes/Private/GDLRegistrar_Private.h index 509ea9b9253..f133cffd2b4 100644 --- a/GoogleDataLogger/GoogleDataLogger/Classes/Private/GDLRegistrar_Private.h +++ b/GoogleDataLogger/GoogleDataLogger/Classes/Private/GDLRegistrar_Private.h @@ -19,7 +19,7 @@ @interface GDLRegistrar () /** A map of logTargets to backend implementations. */ -@property(nonatomic) NSMutableDictionary> *logTargetToBackend; +@property(nonatomic) NSMutableDictionary> *logTargetToUploader; /** A map of logTargets to prioritizer implementations. */ @property(nonatomic) NSMutableDictionary> *logTargetToPrioritizer; diff --git a/GoogleDataLogger/GoogleDataLogger/Classes/Public/GDLLogTargets.h b/GoogleDataLogger/GoogleDataLogger/Classes/Public/GDLLogTargets.h new file mode 100644 index 00000000000..c373e5252be --- /dev/null +++ b/GoogleDataLogger/GoogleDataLogger/Classes/Public/GDLLogTargets.h @@ -0,0 +1,28 @@ +/* + * Copyright 2019 Google + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#import + +/** The list of targets supported by the shared logging infrastructure. If adding a new target, + * please use the previous value +1, and increment GDLLogTargetLast by 1. + */ +typedef NS_ENUM(NSInteger, GDLLogTarget) { + + /** The CCT log target. */ + kGDLLogTargetCCT = 1000, + + GDLLogTargetLast = 1001 +}; diff --git a/GoogleDataLogger/GoogleDataLogger/Classes/Public/GDLLogUploader.h b/GoogleDataLogger/GoogleDataLogger/Classes/Public/GDLLogUploader.h index fded1deddb1..b395a56bb06 100644 --- a/GoogleDataLogger/GoogleDataLogger/Classes/Public/GDLLogUploader.h +++ b/GoogleDataLogger/GoogleDataLogger/Classes/Public/GDLLogUploader.h @@ -16,13 +16,15 @@ #import +#import +#import + NS_ASSUME_NONNULL_BEGIN /** A convenient typedef to define the block to be called upon completion of an upload to the * backend. */ -typedef void (^GDLBackendCompletionBlock)(NSSet *_Nullable successfulUploads, - NSSet *_Nullable unsuccessfulUploads); +typedef void (^GDLUploaderCompletionBlock)(GDLLogTarget target, GDLClock *nextUploadAttemptUTC, NSSet *_Nullable successfulUploads); /** This protocol defines the common interface for logging backend implementations. */ @protocol GDLLogUploader @@ -36,7 +38,7 @@ typedef void (^GDLBackendCompletionBlock)(NSSet *_Nullable successfulUp * - successfulUploads: The set of filenames uploaded successfully. * - unsuccessfulUploads: The set of filenames not uploaded successfully. */ -- (void)uploadLogs:(NSSet *)logFiles onComplete:(GDLBackendCompletionBlock)onComplete; +- (void)uploadLogs:(NSSet *)logFiles onComplete:(GDLUploaderCompletionBlock)onComplete; @end diff --git a/GoogleDataLogger/GoogleDataLogger/Classes/Public/GDLRegistrar.h b/GoogleDataLogger/GoogleDataLogger/Classes/Public/GDLRegistrar.h index 75c578da0d2..d61e26957f0 100644 --- a/GoogleDataLogger/GoogleDataLogger/Classes/Public/GDLRegistrar.h +++ b/GoogleDataLogger/GoogleDataLogger/Classes/Public/GDLRegistrar.h @@ -18,16 +18,10 @@ #import #import +#import NS_ASSUME_NONNULL_BEGIN -/** The list of targets supported by the shared logging infrastructure. */ -typedef NS_ENUM(NSInteger, GDLLogTarget) { - - /** The CCT log target. */ - kGDLLogTargetCCT = 1000 -}; - /** Manages the registration of log targets with the logging SDK. */ @interface GDLRegistrar : NSObject diff --git a/GoogleDataLogger/Tests/Unit/Categories/GDLRegistrar+Testing.m b/GoogleDataLogger/Tests/Unit/Categories/GDLRegistrar+Testing.m index 57a88b5e5f6..5aa9a2d5a10 100644 --- a/GoogleDataLogger/Tests/Unit/Categories/GDLRegistrar+Testing.m +++ b/GoogleDataLogger/Tests/Unit/Categories/GDLRegistrar+Testing.m @@ -22,7 +22,7 @@ @implementation GDLRegistrar (Testing) - (void)reset { [self.logTargetToPrioritizer removeAllObjects]; - [self.logTargetToBackend removeAllObjects]; + [self.logTargetToUploader removeAllObjects]; } @end diff --git a/GoogleDataLogger/Tests/Unit/Helpers/GDLTestUploader.h b/GoogleDataLogger/Tests/Unit/Helpers/GDLTestUploader.h index 194d9a55c9f..40945237b0f 100644 --- a/GoogleDataLogger/Tests/Unit/Helpers/GDLTestUploader.h +++ b/GoogleDataLogger/Tests/Unit/Helpers/GDLTestUploader.h @@ -27,7 +27,7 @@ NS_ASSUME_NONNULL_BEGIN /** A block that can be ran in -uploadLogs:onComplete:. */ @property(nullable, nonatomic) void (^uploadLogsBlock) - (NSSet *logFiles, GDLBackendCompletionBlock completionBlock); + (NSSet *logFiles, GDLUploaderCompletionBlock completionBlock); @end diff --git a/GoogleDataLogger/Tests/Unit/Helpers/GDLTestUploader.m b/GoogleDataLogger/Tests/Unit/Helpers/GDLTestUploader.m index 3c2bfd61b9a..0b013f443da 100644 --- a/GoogleDataLogger/Tests/Unit/Helpers/GDLTestUploader.m +++ b/GoogleDataLogger/Tests/Unit/Helpers/GDLTestUploader.m @@ -18,11 +18,11 @@ @implementation GDLTestUploader -- (void)uploadLogs:(NSSet *)logFiles onComplete:(GDLBackendCompletionBlock)onComplete { +- (void)uploadLogs:(NSSet *)logFiles onComplete:(GDLUploaderCompletionBlock)onComplete { if (_uploadLogsBlock) { _uploadLogsBlock(logFiles, onComplete); } else if (onComplete) { - onComplete(nil, nil); + onComplete(kGDLLogTargetCCT, [GDLClock snapshot], nil); } } From 7d5a76977b205b302c776fd26fdc6a0d2e71b24d Mon Sep 17 00:00:00 2001 From: Michael Haney Date: Wed, 16 Jan 2019 14:42:21 -0800 Subject: [PATCH 2/7] Add the ability to delete a set of logs by filename, and a conversion method for hashes to files. --- .../GoogleDataLogger/Classes/GDLLogStorage.h | 14 ++++++++++ .../GoogleDataLogger/Classes/GDLLogStorage.m | 26 +++++++++++++++++++ 2 files changed, 40 insertions(+) diff --git a/GoogleDataLogger/GoogleDataLogger/Classes/GDLLogStorage.h b/GoogleDataLogger/GoogleDataLogger/Classes/GDLLogStorage.h index 42cd66a8c5f..3ebb6a1684a 100644 --- a/GoogleDataLogger/GoogleDataLogger/Classes/GDLLogStorage.h +++ b/GoogleDataLogger/GoogleDataLogger/Classes/GDLLogStorage.h @@ -45,6 +45,20 @@ NS_ASSUME_NONNULL_BEGIN */ - (void)removeLog:(NSNumber *)logHash logTarget:(NSNumber *)logTarget; +/** Removes a set of log fields specified by their filenames. + * + * @param logFiles The set of log files to remove. + * @param logTarget The log target the log files correspond to. + */ +- (void)removeLogFiles:(NSSet *)logFiles logTarget:(NSNumber *)logTarget; + +/** Converts a set of log hashes to a set of log files. + * + * @param logHashes A set of log hashes to get the files of. + * @return A set of equivalent length, containing all the filenames corresponding to the hashes. + */ +- (NSSet *)logHashesToFiles:(NSSet *)logHashes; + @end NS_ASSUME_NONNULL_END diff --git a/GoogleDataLogger/GoogleDataLogger/Classes/GDLLogStorage.m b/GoogleDataLogger/GoogleDataLogger/Classes/GDLLogStorage.m index a590386fdcc..1e7a4616dd4 100644 --- a/GoogleDataLogger/GoogleDataLogger/Classes/GDLLogStorage.m +++ b/GoogleDataLogger/GoogleDataLogger/Classes/GDLLogStorage.m @@ -124,6 +124,32 @@ - (void)removeLog:(NSNumber *)logHash logTarget:(NSNumber *)logTarget { }); } +- (void)removeLogFiles:(NSSet *)logFiles logTarget:(NSNumber *)logTarget { + NSMutableSet *logHashes = [[NSMutableSet alloc] init]; + [_logHashToLogFile enumerateKeysAndObjectsUsingBlock:^(NSNumber * _Nonnull key, + NSURL * _Nonnull obj, + BOOL * _Nonnull stop) { + if ([logFiles containsObject:obj]) { + [logHashes addObject:key]; + } + }]; + for (NSNumber *logHash in logHashes) { + [self removeLog:logHash logTarget:logTarget]; + } +} + +- (NSSet *)logHashesToFiles:(NSSet *)logHashes { + NSMutableSet *logFiles = [[NSMutableSet alloc] init]; + dispatch_sync(_storageQueue, ^{ + for (NSNumber *hashNumber in logHashes) { + NSURL *logURL = self.logHashToLogFile[hashNumber]; + GDLAssert(logURL, @"A log file URL couldn't be found for the given hash"); + [logFiles addObject:logURL]; + } + }); + return logFiles; +} + #pragma mark - Private helper methods /** Creates the log directory if it does not exist. */ From 9894bd2cc11101ac64985da719967e1d15a65e75 Mon Sep 17 00:00:00 2001 From: Michael Haney Date: Wed, 16 Jan 2019 20:24:39 -0800 Subject: [PATCH 3/7] Change the log storage removeLogs method to be log hashes instead of URLS --- .../GoogleDataLogger/Classes/GDLLogStorage.h | 4 +- .../GoogleDataLogger/Classes/GDLLogStorage.m | 49 +++++++++---------- .../Classes/Private/GDLLogStorage_Private.h | 2 +- .../Unit/Categories/GDLLogStorage+Testing.m | 2 +- .../Tests/Unit/GDLLogStorageTest.m | 10 ++-- 5 files changed, 32 insertions(+), 35 deletions(-) diff --git a/GoogleDataLogger/GoogleDataLogger/Classes/GDLLogStorage.h b/GoogleDataLogger/GoogleDataLogger/Classes/GDLLogStorage.h index 3ebb6a1684a..5622d03dc71 100644 --- a/GoogleDataLogger/GoogleDataLogger/Classes/GDLLogStorage.h +++ b/GoogleDataLogger/GoogleDataLogger/Classes/GDLLogStorage.h @@ -47,10 +47,10 @@ NS_ASSUME_NONNULL_BEGIN /** Removes a set of log fields specified by their filenames. * - * @param logFiles The set of log files to remove. + * @param logHashes The set of log files to remove. * @param logTarget The log target the log files correspond to. */ -- (void)removeLogFiles:(NSSet *)logFiles logTarget:(NSNumber *)logTarget; +- (void)removeLogs:(NSSet *)logHashes logTarget:(NSNumber *)logTarget; /** Converts a set of log hashes to a set of log files. * diff --git a/GoogleDataLogger/GoogleDataLogger/Classes/GDLLogStorage.m b/GoogleDataLogger/GoogleDataLogger/Classes/GDLLogStorage.m index 1e7a4616dd4..8bace15a825 100644 --- a/GoogleDataLogger/GoogleDataLogger/Classes/GDLLogStorage.m +++ b/GoogleDataLogger/GoogleDataLogger/Classes/GDLLogStorage.m @@ -56,7 +56,7 @@ - (instancetype)init { if (self) { _storageQueue = dispatch_queue_create("com.google.GDLLogStorage", DISPATCH_QUEUE_SERIAL); _logHashToLogFile = [[NSMutableDictionary alloc] init]; - _logTargetToLogFileSet = [[NSMutableDictionary alloc] init]; + _logTargetToLogHashSet = [[NSMutableDictionary alloc] init]; _uploader = [GDLUploadCoordinator sharedInstance]; } return self; @@ -90,7 +90,7 @@ - (void)storeLog:(GDLLogEvent *)log { // Check the QoS, if it's high priority, notify the log target that it has a high priority log. if (shortLivedLog.qosTier == GDLLogQoSFast) { - NSSet *allLogsForLogTarget = self.logTargetToLogFileSet[@(logTarget)]; + NSSet *allLogsForLogTarget = self.logTargetToLogHashSet[@(logTarget)]; [self.uploader forceUploadLogs:allLogsForLogTarget target:logTarget]; } @@ -117,25 +117,19 @@ - (void)removeLog:(NSNumber *)logHash logTarget:(NSNumber *)logTarget { // Remove from the tracking collections. [self.logHashToLogFile removeObjectForKey:logHash]; - NSMutableSet *logFiles = self.logTargetToLogFileSet[logTarget]; - GDLAssert(logFiles, @"There wasn't a logSet for this logTarget."); - [logFiles removeObject:logFile]; + NSMutableSet *logHashes = self.logTargetToLogHashSet[logTarget]; + GDLAssert(logHashes, @"There wasn't a logSet for this logTarget."); + [logHashes removeObject:logHash]; // It's fine to not remove the set if it's empty. }); } -- (void)removeLogFiles:(NSSet *)logFiles logTarget:(NSNumber *)logTarget { - NSMutableSet *logHashes = [[NSMutableSet alloc] init]; - [_logHashToLogFile enumerateKeysAndObjectsUsingBlock:^(NSNumber * _Nonnull key, - NSURL * _Nonnull obj, - BOOL * _Nonnull stop) { - if ([logFiles containsObject:obj]) { - [logHashes addObject:key]; +- (void)removeLogs:(NSSet *)logHashes logTarget:(NSNumber *)logTarget { + dispatch_async(_storageQueue, ^{ + for (NSNumber *logHash in logHashes) { + [self removeLog:logHash logTarget:logTarget]; } - }]; - for (NSNumber *logHash in logHashes) { - [self removeLog:logHash logTarget:logTarget]; - } + }); } - (NSSet *)logHashesToFiles:(NSSet *)logHashes { @@ -196,13 +190,15 @@ - (NSURL *)saveLogProtoToDisk:(NSData *)logProtoBytes logHash:(NSUInteger)logHas */ - (void)addLogToTrackingCollections:(GDLLogEvent *)log logFile:(NSURL *)logFile { NSInteger logTarget = log.logTarget; - self.logHashToLogFile[@(log.hash)] = logFile; - NSMutableSet *logs = self.logTargetToLogFileSet[@(logTarget)]; + NSNumber *logHash = @(log.hash); + NSNumber *logTargetNumber = @(logTarget); + self.logHashToLogFile[logHash] = logFile; + NSMutableSet *logs = self.logTargetToLogHashSet[logTargetNumber]; if (logs) { - [logs addObject:logFile]; + [logs addObject:logHash]; } else { - NSMutableSet *logSet = [NSMutableSet setWithObject:logFile]; - self.logTargetToLogFileSet[@(logTarget)] = logSet; + NSMutableSet *logSet = [NSMutableSet setWithObject:logHash]; + self.logTargetToLogHashSet[logTargetNumber] = logSet; } } @@ -211,8 +207,8 @@ - (void)addLogToTrackingCollections:(GDLLogEvent *)log logFile:(NSURL *)logFile /** The NSKeyedCoder key for the logHashToFile property. */ static NSString *const kGDLLogHashToLogFileKey = @"logHashToLogFileKey"; -/** The NSKeyedCoder key for the logTargetToLogFileSet property. */ -static NSString *const kGDLLogTargetToLogSetKey = @"logTargetToLogFileSetKey"; +/** The NSKeyedCoder key for the logTargetToLogHashSet property. */ +static NSString *const kGDLLogTargetToLogHashSetKey = @"logTargetToLogHashSetKey"; + (BOOL)supportsSecureCoding { return YES; @@ -225,8 +221,8 @@ - (instancetype)initWithCoder:(NSCoder *)aDecoder { Class NSMutableDictionaryClass = [NSMutableDictionary class]; sharedInstance->_logHashToLogFile = [aDecoder decodeObjectOfClass:NSMutableDictionaryClass forKey:kGDLLogHashToLogFileKey]; - sharedInstance->_logTargetToLogFileSet = - [aDecoder decodeObjectOfClass:NSMutableDictionaryClass forKey:kGDLLogTargetToLogSetKey]; + sharedInstance->_logTargetToLogHashSet = + [aDecoder decodeObjectOfClass:NSMutableDictionaryClass forKey:kGDLLogTargetToLogHashSetKey]; }); return sharedInstance; } @@ -235,7 +231,8 @@ - (void)encodeWithCoder:(NSCoder *)aCoder { GDLLogStorage *sharedInstance = [self.class sharedInstance]; dispatch_sync(sharedInstance.storageQueue, ^{ [aCoder encodeObject:sharedInstance->_logHashToLogFile forKey:kGDLLogHashToLogFileKey]; - [aCoder encodeObject:sharedInstance->_logTargetToLogFileSet forKey:kGDLLogTargetToLogSetKey]; + [aCoder encodeObject:sharedInstance->_logTargetToLogHashSet + forKey:kGDLLogTargetToLogHashSetKey]; }); } diff --git a/GoogleDataLogger/GoogleDataLogger/Classes/Private/GDLLogStorage_Private.h b/GoogleDataLogger/GoogleDataLogger/Classes/Private/GDLLogStorage_Private.h index 6f3ff0a7585..24b90ad4183 100644 --- a/GoogleDataLogger/GoogleDataLogger/Classes/Private/GDLLogStorage_Private.h +++ b/GoogleDataLogger/GoogleDataLogger/Classes/Private/GDLLogStorage_Private.h @@ -28,7 +28,7 @@ /** A map of logTargets to a set of log hash values. */ @property(nonatomic) - NSMutableDictionary *> *logTargetToLogFileSet; + NSMutableDictionary *> *logTargetToLogHashSet; /** The log uploader instance to use. */ @property(nonatomic) GDLUploadCoordinator *uploader; diff --git a/GoogleDataLogger/Tests/Unit/Categories/GDLLogStorage+Testing.m b/GoogleDataLogger/Tests/Unit/Categories/GDLLogStorage+Testing.m index e728748fe66..22bec982c07 100644 --- a/GoogleDataLogger/Tests/Unit/Categories/GDLLogStorage+Testing.m +++ b/GoogleDataLogger/Tests/Unit/Categories/GDLLogStorage+Testing.m @@ -20,7 +20,7 @@ @implementation GDLLogStorage (Testing) - (void)reset { dispatch_sync(self.storageQueue, ^{ - [self.logTargetToLogFileSet removeAllObjects]; + [self.logTargetToLogHashSet removeAllObjects]; [self.logHashToLogFile removeAllObjects]; }); } diff --git a/GoogleDataLogger/Tests/Unit/GDLLogStorageTest.m b/GoogleDataLogger/Tests/Unit/GDLLogStorageTest.m index 32a46d9ebee..8d4e0dcca75 100644 --- a/GoogleDataLogger/Tests/Unit/GDLLogStorageTest.m +++ b/GoogleDataLogger/Tests/Unit/GDLLogStorageTest.m @@ -87,7 +87,7 @@ - (void)testStoreLog { } dispatch_sync([GDLLogStorage sharedInstance].storageQueue, ^{ XCTAssertEqual([GDLLogStorage sharedInstance].logHashToLogFile.count, 1); - XCTAssertEqual([GDLLogStorage sharedInstance].logTargetToLogFileSet[@(logTarget)].count, 1); + XCTAssertEqual([GDLLogStorage sharedInstance].logTargetToLogHashSet[@(logTarget)].count, 1); NSURL *logFile = [GDLLogStorage sharedInstance].logHashToLogFile[@(logHash)]; XCTAssertNotNil(logFile); XCTAssertTrue([[NSFileManager defaultManager] fileExistsAtPath:logFile.path]); @@ -116,7 +116,7 @@ - (void)testRemoveLog { dispatch_sync([GDLLogStorage sharedInstance].storageQueue, ^{ XCTAssertFalse([[NSFileManager defaultManager] fileExistsAtPath:logFile.path]); XCTAssertEqual([GDLLogStorage sharedInstance].logHashToLogFile.count, 0); - XCTAssertEqual([GDLLogStorage sharedInstance].logTargetToLogFileSet[@(logTarget)].count, 0); + XCTAssertEqual([GDLLogStorage sharedInstance].logTargetToLogHashSet[@(logTarget)].count, 0); }); } @@ -143,7 +143,7 @@ - (void)testStoreMultipleLogs { } dispatch_sync([GDLLogStorage sharedInstance].storageQueue, ^{ XCTAssertEqual([GDLLogStorage sharedInstance].logHashToLogFile.count, 3); - XCTAssertEqual([GDLLogStorage sharedInstance].logTargetToLogFileSet[@(logTarget)].count, 3); + XCTAssertEqual([GDLLogStorage sharedInstance].logTargetToLogHashSet[@(logTarget)].count, 3); NSURL *log1File = [GDLLogStorage sharedInstance].logHashToLogFile[@(log1Hash)]; XCTAssertNotNil(log1File); @@ -196,7 +196,7 @@ - (void)testLogEventDeallocationIsEnforced { dispatch_sync([GDLLogStorage sharedInstance].storageQueue, ^{ XCTAssertFalse([[NSFileManager defaultManager] fileExistsAtPath:logFile.path]); XCTAssertEqual([GDLLogStorage sharedInstance].logHashToLogFile.count, 0); - XCTAssertEqual([GDLLogStorage sharedInstance].logTargetToLogFileSet[@(logTarget)].count, 0); + XCTAssertEqual([GDLLogStorage sharedInstance].logTargetToLogHashSet[@(logTarget)].count, 0); }); } @@ -237,7 +237,7 @@ - (void)testQoSTierFast { dispatch_sync([GDLLogStorage sharedInstance].storageQueue, ^{ XCTAssertTrue(self.uploaderFake.forceUploadCalled); XCTAssertEqual([GDLLogStorage sharedInstance].logHashToLogFile.count, 1); - XCTAssertEqual([GDLLogStorage sharedInstance].logTargetToLogFileSet[@(logTarget)].count, 1); + XCTAssertEqual([GDLLogStorage sharedInstance].logTargetToLogHashSet[@(logTarget)].count, 1); NSURL *logFile = [GDLLogStorage sharedInstance].logHashToLogFile[@(logHash)]; XCTAssertNotNil(logFile); XCTAssertTrue([[NSFileManager defaultManager] fileExistsAtPath:logFile.path]); From 4cf7092cc8f27df3a83a727a146e847fee610837 Mon Sep 17 00:00:00 2001 From: Michael Haney Date: Wed, 16 Jan 2019 20:25:18 -0800 Subject: [PATCH 4/7] Style, and change the completion block to include an error --- .../GoogleDataLogger/Classes/Public/GDLLogUploader.h | 8 +++++++- .../GoogleDataLogger/Classes/Public/GDLRegistrar.h | 2 +- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/GoogleDataLogger/GoogleDataLogger/Classes/Public/GDLLogUploader.h b/GoogleDataLogger/GoogleDataLogger/Classes/Public/GDLLogUploader.h index b395a56bb06..0d41b1a9e6f 100644 --- a/GoogleDataLogger/GoogleDataLogger/Classes/Public/GDLLogUploader.h +++ b/GoogleDataLogger/GoogleDataLogger/Classes/Public/GDLLogUploader.h @@ -23,8 +23,14 @@ NS_ASSUME_NONNULL_BEGIN /** A convenient typedef to define the block to be called upon completion of an upload to the * backend. + * + * target: The log target that was uploading. + * nextUploadAttemptUTC: The desired next upload attempt time. + * uploadError: Populated with any upload error. If non-nil, a retry will be attempted. */ -typedef void (^GDLUploaderCompletionBlock)(GDLLogTarget target, GDLClock *nextUploadAttemptUTC, NSSet *_Nullable successfulUploads); +typedef void (^GDLUploaderCompletionBlock)(GDLLogTarget target, + GDLClock *nextUploadAttemptUTC, + NSError *_Nullable uploadError); /** This protocol defines the common interface for logging backend implementations. */ @protocol GDLLogUploader diff --git a/GoogleDataLogger/GoogleDataLogger/Classes/Public/GDLRegistrar.h b/GoogleDataLogger/GoogleDataLogger/Classes/Public/GDLRegistrar.h index d61e26957f0..da3f583a41e 100644 --- a/GoogleDataLogger/GoogleDataLogger/Classes/Public/GDLRegistrar.h +++ b/GoogleDataLogger/GoogleDataLogger/Classes/Public/GDLRegistrar.h @@ -17,8 +17,8 @@ #import #import -#import #import +#import NS_ASSUME_NONNULL_BEGIN From 98147a192152dcedf0fa0ca455d943710324c474 Mon Sep 17 00:00:00 2001 From: Michael Haney Date: Wed, 16 Jan 2019 21:02:57 -0800 Subject: [PATCH 5/7] Change to sync, since async to adding more async created race condition opportunity. --- GoogleDataLogger/GoogleDataLogger/Classes/GDLLogStorage.m | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/GoogleDataLogger/GoogleDataLogger/Classes/GDLLogStorage.m b/GoogleDataLogger/GoogleDataLogger/Classes/GDLLogStorage.m index 8bace15a825..10864651e72 100644 --- a/GoogleDataLogger/GoogleDataLogger/Classes/GDLLogStorage.m +++ b/GoogleDataLogger/GoogleDataLogger/Classes/GDLLogStorage.m @@ -125,7 +125,7 @@ - (void)removeLog:(NSNumber *)logHash logTarget:(NSNumber *)logTarget { } - (void)removeLogs:(NSSet *)logHashes logTarget:(NSNumber *)logTarget { - dispatch_async(_storageQueue, ^{ + dispatch_sync(_storageQueue, ^{ for (NSNumber *logHash in logHashes) { [self removeLog:logHash logTarget:logTarget]; } From 742f9bd174681e470868d162667e8b051a468384 Mon Sep 17 00:00:00 2001 From: Michael Haney Date: Wed, 16 Jan 2019 21:03:08 -0800 Subject: [PATCH 6/7] Test new storage methods --- .../Tests/Unit/GDLLogStorageTest.m | 68 +++++++++++++++++++ 1 file changed, 68 insertions(+) diff --git a/GoogleDataLogger/Tests/Unit/GDLLogStorageTest.m b/GoogleDataLogger/Tests/Unit/GDLLogStorageTest.m index 8d4e0dcca75..9e5f5b61cac 100644 --- a/GoogleDataLogger/Tests/Unit/GDLLogStorageTest.m +++ b/GoogleDataLogger/Tests/Unit/GDLLogStorageTest.m @@ -120,6 +120,42 @@ - (void)testRemoveLog { }); } +/** Tests removing a set of logs */ +- (void)testRemoveLogs { + GDLLogStorage *storage = [GDLLogStorage sharedInstance]; + NSUInteger log1Hash, log2Hash, log3Hash; + + // logEvents are autoreleased, and the pool needs to drain. + @autoreleasepool { + GDLLogEvent *logEvent = [[GDLLogEvent alloc] initWithLogMapID:@"404" logTarget:logTarget]; + logEvent.extensionBytes = [@"testString1" dataUsingEncoding:NSUTF8StringEncoding]; + log1Hash = logEvent.hash; + XCTAssertNoThrow([storage storeLog:logEvent]); + + logEvent = [[GDLLogEvent alloc] initWithLogMapID:@"100" logTarget:logTarget]; + logEvent.extensionBytes = [@"testString2" dataUsingEncoding:NSUTF8StringEncoding]; + log2Hash = logEvent.hash; + XCTAssertNoThrow([storage storeLog:logEvent]); + + logEvent = [[GDLLogEvent alloc] initWithLogMapID:@"404" logTarget:logTarget]; + logEvent.extensionBytes = [@"testString3" dataUsingEncoding:NSUTF8StringEncoding]; + log3Hash = logEvent.hash; + XCTAssertNoThrow([storage storeLog:logEvent]); + } + NSSet *logHashSet = [NSSet setWithObjects:@(log1Hash), @(log2Hash), @(log3Hash), nil]; + NSSet *logFiles = [storage logHashesToFiles:logHashSet]; + [storage removeLogs:logHashSet logTarget:@(logTarget)]; + dispatch_sync(storage.storageQueue, ^{ + XCTAssertNil(storage.logHashToLogFile[@(log1Hash)]); + XCTAssertNil(storage.logHashToLogFile[@(log2Hash)]); + XCTAssertNil(storage.logHashToLogFile[@(log3Hash)]); + XCTAssertEqual(storage.logTargetToLogHashSet[@(logTarget)].count, 0); + for (NSURL *logFile in logFiles) { + XCTAssertFalse([[NSFileManager defaultManager] fileExistsAtPath:logFile.path]); + } + }); +} + /** Tests storing a few different logs. */ - (void)testStoreMultipleLogs { NSUInteger log1Hash, log2Hash, log3Hash; @@ -247,4 +283,36 @@ - (void)testQoSTierFast { }); } +/** Tests convert a set of log hashes to a set of log file URLS. */ +- (void)testLogHashesToFiles { + GDLLogStorage *storage = [GDLLogStorage sharedInstance]; + NSUInteger log1Hash, log2Hash, log3Hash; + + // logEvents are autoreleased, and the pool needs to drain. + @autoreleasepool { + GDLLogEvent *logEvent = [[GDLLogEvent alloc] initWithLogMapID:@"404" logTarget:logTarget]; + logEvent.extensionBytes = [@"testString1" dataUsingEncoding:NSUTF8StringEncoding]; + log1Hash = logEvent.hash; + XCTAssertNoThrow([storage storeLog:logEvent]); + + logEvent = [[GDLLogEvent alloc] initWithLogMapID:@"100" logTarget:logTarget]; + logEvent.extensionBytes = [@"testString2" dataUsingEncoding:NSUTF8StringEncoding]; + log2Hash = logEvent.hash; + XCTAssertNoThrow([storage storeLog:logEvent]); + + logEvent = [[GDLLogEvent alloc] initWithLogMapID:@"404" logTarget:logTarget]; + logEvent.extensionBytes = [@"testString3" dataUsingEncoding:NSUTF8StringEncoding]; + log3Hash = logEvent.hash; + XCTAssertNoThrow([storage storeLog:logEvent]); + } + NSSet *logHashSet = [NSSet setWithObjects:@(log1Hash), @(log2Hash), @(log3Hash), nil]; + NSSet *logFiles = [storage logHashesToFiles:logHashSet]; + dispatch_sync(storage.storageQueue, ^{ + XCTAssertEqual(logFiles.count, 3); + XCTAssertTrue([logFiles containsObject:storage.logHashToLogFile[@(log1Hash)]]); + XCTAssertTrue([logFiles containsObject:storage.logHashToLogFile[@(log2Hash)]]); + XCTAssertTrue([logFiles containsObject:storage.logHashToLogFile[@(log3Hash)]]); + }); +} + @end From 2c12ee5040286dbe3062db8d2dd09a1a89c640ad Mon Sep 17 00:00:00 2001 From: Michael Haney Date: Wed, 16 Jan 2019 21:03:31 -0800 Subject: [PATCH 7/7] Fix coordinator method declarations --- .../GoogleDataLogger/Classes/GDLUploadCoordinator.h | 5 ++++- .../GoogleDataLogger/Classes/GDLUploadCoordinator.m | 2 +- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/GoogleDataLogger/GoogleDataLogger/Classes/GDLUploadCoordinator.h b/GoogleDataLogger/GoogleDataLogger/Classes/GDLUploadCoordinator.h index 8ecaf34d872..71e4d0f6648 100644 --- a/GoogleDataLogger/GoogleDataLogger/Classes/GDLUploadCoordinator.h +++ b/GoogleDataLogger/GoogleDataLogger/Classes/GDLUploadCoordinator.h @@ -33,8 +33,11 @@ NS_ASSUME_NONNULL_BEGIN /** Forces the backend specified by the target to upload the provided set of logs. This should only * ever happen when the QoS tier of a log requires it. + * + * @param logHashes The set of log hashes to force upload. + * @param logTarget The log target that should force an upload. */ -- (void)forceUploadLogs:(NSSet *)logFiles target:(GDLLogTarget)logTarget; +- (void)forceUploadLogs:(NSSet *)logHashes target:(GDLLogTarget)logTarget; @end diff --git a/GoogleDataLogger/GoogleDataLogger/Classes/GDLUploadCoordinator.m b/GoogleDataLogger/GoogleDataLogger/Classes/GDLUploadCoordinator.m index 277da32f223..f1baa389ee9 100644 --- a/GoogleDataLogger/GoogleDataLogger/Classes/GDLUploadCoordinator.m +++ b/GoogleDataLogger/GoogleDataLogger/Classes/GDLUploadCoordinator.m @@ -29,7 +29,7 @@ + (instancetype)sharedInstance { return sharedUploader; } -- (void)forceUploadLogs:(NSSet *)logFiles target:(GDLLogTarget)logTarget { +- (void)forceUploadLogs:(NSSet *)logHashes target:(GDLLogTarget)logTarget { // Ask the prioritizer of the target for some logs }