Skip to content

Add test-related functionality, make GDLRegistrar thread-safe and tested #2285

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Jan 18, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
54 changes: 48 additions & 6 deletions GoogleDataLogger/GoogleDataLogger/Classes/GDLRegistrar.m
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,13 @@

#import "GDLRegistrar_Private.h"

@implementation GDLRegistrar
@implementation GDLRegistrar {
/** Backing ivar for logTargetToUploader property. */
NSMutableDictionary<NSNumber *, id<GDLLogUploader>> *_logTargetToUploader;

/** Backing ivar for logTargetToPrioritizer property. */
NSMutableDictionary<NSNumber *, id<GDLLogPrioritizer>> *_logTargetToPrioritizer;
}

+ (instancetype)sharedInstance {
static GDLRegistrar *sharedInstance;
Expand All @@ -32,19 +38,55 @@ + (instancetype)sharedInstance {
- (instancetype)init {
self = [super init];
if (self) {
_registrarQueue = dispatch_queue_create("com.google.GDLRegistrar", DISPATCH_QUEUE_CONCURRENT);
_logTargetToPrioritizer = [[NSMutableDictionary alloc] init];
_logTargetToUploader = [[NSMutableDictionary alloc] init];
}
return self;
}

- (void)registerBackend:(id<GDLLogUploader>)backend forLogTarget:(GDLLogTarget)logTarget {
self.logTargetToUploader[@(logTarget)] = backend;
- (void)registerUploader:(id<GDLLogUploader>)backend logTarget:(GDLLogTarget)logTarget {
__weak GDLRegistrar *weakSelf = self;
dispatch_barrier_async(_registrarQueue, ^{
GDLRegistrar *strongSelf = weakSelf;
if (strongSelf) {
strongSelf->_logTargetToUploader[@(logTarget)] = backend;
}
});
}

- (void)registerPrioritizer:(id<GDLLogPrioritizer>)prioritizer logTarget:(GDLLogTarget)logTarget {
__weak GDLRegistrar *weakSelf = self;
dispatch_barrier_async(_registrarQueue, ^{
GDLRegistrar *strongSelf = weakSelf;
if (strongSelf) {
strongSelf->_logTargetToPrioritizer[@(logTarget)] = prioritizer;
}
});
}

- (void)registerLogPrioritizer:(id<GDLLogPrioritizer>)prioritizer
forLogTarget:(GDLLogTarget)logTarget {
self.logTargetToPrioritizer[@(logTarget)] = prioritizer;
- (NSMutableDictionary<NSNumber *, id<GDLLogUploader>> *)logTargetToUploader {
__block NSMutableDictionary<NSNumber *, id<GDLLogUploader>> *logTargetToUploader;
__weak GDLRegistrar *weakSelf = self;
dispatch_sync(_registrarQueue, ^{
GDLRegistrar *strongSelf = weakSelf;
if (strongSelf) {
logTargetToUploader = strongSelf->_logTargetToUploader;
}
});
return logTargetToUploader;
}

- (NSMutableDictionary<NSNumber *, id<GDLLogPrioritizer>> *)logTargetToPrioritizer {
__block NSMutableDictionary<NSNumber *, id<GDLLogPrioritizer>> *logTargetToPrioritizer;
__weak GDLRegistrar *weakSelf = self;
dispatch_sync(_registrarQueue, ^{
GDLRegistrar *strongSelf = weakSelf;
if (strongSelf) {
logTargetToPrioritizer = strongSelf->_logTargetToPrioritizer;
}
});
return logTargetToPrioritizer;
}

@end
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,15 @@

@interface GDLRegistrar ()

/** The concurrent queue on which all registration occurs. */
@property(nonatomic, readonly) dispatch_queue_t registrarQueue;

/** A map of logTargets to backend implementations. */
@property(nonatomic) NSMutableDictionary<NSNumber *, id<GDLLogUploader>> *logTargetToUploader;
@property(atomic, readonly)
NSMutableDictionary<NSNumber *, id<GDLLogUploader>> *logTargetToUploader;

/** A map of logTargets to prioritizer implementations. */
@property(nonatomic) NSMutableDictionary<NSNumber *, id<GDLLogPrioritizer>> *logTargetToPrioritizer;
@property(atomic, readonly)
NSMutableDictionary<NSNumber *, id<GDLLogPrioritizer>> *logTargetToPrioritizer;

@end
Original file line number Diff line number Diff line change
Expand Up @@ -36,15 +36,14 @@ NS_ASSUME_NONNULL_BEGIN
* @param backend The backend object to register.
* @param logTarget The logTarget this backend object will be responsible for.
*/
- (void)registerBackend:(id<GDLLogUploader>)backend forLogTarget:(GDLLogTarget)logTarget;
- (void)registerUploader:(id<GDLLogUploader>)backend logTarget:(GDLLogTarget)logTarget;

/** Registers a log prioritizer implementation with the GoogleDataLogger infrastructure.
*
* @param prioritizer The prioritizer object to register.
* @param logTarget The logTarget this prioritizer object will be responsible for.
*/
- (void)registerLogPrioritizer:(id<GDLLogPrioritizer>)prioritizer
forLogTarget:(GDLLogTarget)logTarget;
- (void)registerPrioritizer:(id<GDLLogPrioritizer>)prioritizer logTarget:(GDLLogTarget)logTarget;

@end

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,9 @@ typedef NS_ENUM(NSInteger, GDLMessageCode) {
/** For warning messages concerning protoBytes: not being implemented by a log extension. */
GDLMCWExtensionMissingBytesImpl = 1,

/** For warning message concerning a failed log upload. */
GDLMCWUploadFailed = 2,

/** For error messages concerning transform: not being implemented by a log transformer. */
GDLMCETransformerDoesntImplementTransform = 1000,

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
*/

#import "GDLRegistrar.h"
#import "GDLRegistrar_Private.h"

NS_ASSUME_NONNULL_BEGIN

Expand Down
6 changes: 4 additions & 2 deletions GoogleDataLogger/Tests/Unit/Categories/GDLRegistrar+Testing.m
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,10 @@
@implementation GDLRegistrar (Testing)

- (void)reset {
[self.logTargetToPrioritizer removeAllObjects];
[self.logTargetToUploader removeAllObjects];
dispatch_sync(self.registrarQueue, ^{
[self.logTargetToPrioritizer removeAllObjects];
[self.logTargetToUploader removeAllObjects];
});
}

@end
3 changes: 3 additions & 0 deletions GoogleDataLogger/Tests/Unit/Fakes/GDLLogStorageFake.h
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,9 @@ NS_ASSUME_NONNULL_BEGIN
/** A functionless fake that can be injected into classes that need it. */
@interface GDLLogStorageFake : GDLLogStorage

/** The logs to return from -logHashesToFiles. */
@property(nonatomic) NSSet<NSURL *> *logsToReturnFromLogHashesToFiles;

@end

NS_ASSUME_NONNULL_END
14 changes: 14 additions & 0 deletions GoogleDataLogger/Tests/Unit/Fakes/GDLLogStorageFake.m
Original file line number Diff line number Diff line change
Expand Up @@ -21,4 +21,18 @@ @implementation GDLLogStorageFake
- (void)storeLog:(GDLLogEvent *)log {
}

- (NSSet<NSURL *> *)logHashesToFiles:(NSSet<NSNumber *> *)logHashes {
if (_logsToReturnFromLogHashesToFiles) {
return _logsToReturnFromLogHashesToFiles;
} else {
return [[NSSet alloc] init];
}
}

- (void)removeLog:(NSNumber *)logHash logTarget:(NSNumber *)logTarget {
}

- (void)removeLogs:(NSSet<NSNumber *> *)logHashes logTarget:(NSNumber *)logTarget {
}

@end
4 changes: 2 additions & 2 deletions GoogleDataLogger/Tests/Unit/GDLLogStorageTest.m
Original file line number Diff line number Diff line change
Expand Up @@ -53,8 +53,8 @@ - (void)setUp {
[super setUp];
self.testBackend = [[GDLTestUploader alloc] init];
self.testPrioritizer = [[GDLTestPrioritizer alloc] init];
[[GDLRegistrar sharedInstance] registerBackend:_testBackend forLogTarget:logTarget];
[[GDLRegistrar sharedInstance] registerLogPrioritizer:_testPrioritizer forLogTarget:logTarget];
[[GDLRegistrar sharedInstance] registerUploader:_testBackend logTarget:logTarget];
[[GDLRegistrar sharedInstance] registerPrioritizer:_testPrioritizer logTarget:logTarget];
self.uploaderFake = [[GDLUploadCoordinatorFake alloc] init];
[GDLLogStorage sharedInstance].uploader = self.uploaderFake;
}
Expand Down
26 changes: 26 additions & 0 deletions GoogleDataLogger/Tests/Unit/GDLRegistrarTest.m
Original file line number Diff line number Diff line change
Expand Up @@ -18,15 +18,41 @@

#import <GoogleDataLogger/GDLRegistrar.h>

#import "GDLRegistrar_Private.h"
#import "GDLTestPrioritizer.h"
#import "GDLTestUploader.h"

@interface GDLRegistrarTest : GDLTestCase

@property(nonatomic) GDLLogTarget logTarget;

@end

@implementation GDLRegistrarTest

- (void)setUp {
_logTarget = 23;
}

/** Tests the default initializer. */
- (void)testInit {
XCTAssertNotNil([[GDLRegistrarTest alloc] init]);
}

/** Test registering an uploader. */
- (void)testRegisterUpload {
GDLRegistrar *registrar = [GDLRegistrar sharedInstance];
GDLTestUploader *uploader = [[GDLTestUploader alloc] init];
XCTAssertNoThrow([registrar registerUploader:uploader logTarget:self.logTarget]);
XCTAssertEqual(uploader, registrar.logTargetToUploader[@(_logTarget)]);
}

/** Test registering a prioritizer. */
- (void)testRegisterPrioritizer {
GDLRegistrar *registrar = [GDLRegistrar sharedInstance];
GDLTestPrioritizer *prioritizer = [[GDLTestPrioritizer alloc] init];
XCTAssertNoThrow([registrar registerPrioritizer:prioritizer logTarget:self.logTarget]);
XCTAssertEqual(prioritizer, registrar.logTargetToPrioritizer[@(_logTarget)]);
}

@end
3 changes: 3 additions & 0 deletions GoogleDataLogger/Tests/Unit/Helpers/GDLTestPrioritizer.h
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,9 @@ NS_ASSUME_NONNULL_BEGIN
/** Allows the running of a block of code during -prioritizeLog. */
@property(nullable, nonatomic) void (^prioritizeLogBlock)(GDLLogEvent *logEvent);

/** A block that can run before -logsForNextUpload completes. */
@property(nullable, nonatomic) void (^logsForNextUploadBlock)(void);

@end

NS_ASSUME_NONNULL_END
3 changes: 3 additions & 0 deletions GoogleDataLogger/Tests/Unit/Helpers/GDLTestPrioritizer.m
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,9 @@ - (instancetype)init {
}

- (NSSet<NSNumber *> *)logsForNextUpload {
if (_logsForNextUploadBlock) {
_logsForNextUploadBlock();
}
return _logsForNextUploadFake;
}

Expand Down