Refactor to allow injection of fakes, and take warnings seriously (#2232)

* Create fakes that can be used during unit tests

* Create a private header for the logger

* All log storage to be injected into the log writer, and now give logs to log storage.

Also changes the tests to use the fakes

* Treat all warnings as errors, and warn pedantic

* Ok nevermind, don't warn_pedantic.

* remove trailing comma

* Remove obsolete TODOs

Not needed, because a fake is being used.

* Add fakes and injection to log storage for the uploader, implement a fast qos test

Author: mikehaney24

GoogleDataLogger.podspec

@@ -30,7 +30,8 @@ Shared library for iOS SDK data logging needs.
   s.dependency 'GoogleUtilities/Logger'
 
   s.pod_target_xcconfig = {
-    'GCC_C_LANGUAGE_STANDARD' => 'c99'
+    'GCC_C_LANGUAGE_STANDARD' => 'c99',
+    'GCC_TREAT_WARNINGS_AS_ERRORS' => 'YES'
   }
 
   # Test specs

GoogleDataLogger/GoogleDataLogger/Classes/GDLLogStorage.m

@@ -57,6 +57,7 @@ - (instancetype)init {
     _storageQueue = dispatch_queue_create("com.google.GDLLogStorage", DISPATCH_QUEUE_SERIAL);
     _logHashToLogFile = [[NSMutableDictionary alloc] init];
     _logTargetToLogFileSet = [[NSMutableDictionary alloc] init];
+    _uploader = [GDLUploader sharedInstance];
   }
   return self;
 }
@@ -90,7 +91,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<NSURL *> *allLogsForLogTarget = self.logTargetToLogFileSet[@(logTarget)];
-      [[GDLUploader sharedInstance] forceUploadLogs:allLogsForLogTarget target:logTarget];
+      [self.uploader forceUploadLogs:allLogsForLogTarget target:logTarget];
     }
 
     // Have the prioritizer prioritize the log, enforcing that they do not retain it.

GoogleDataLogger/GoogleDataLogger/Classes/GDLLogWriter.m

@@ -41,6 +41,7 @@ - (instancetype)init {
   self = [super init];
   if (self) {
     _logWritingQueue = dispatch_queue_create("com.google.GDLLogWriter", DISPATCH_QUEUE_SERIAL);
+    _storageInstance = [GDLLogStorage sharedInstance];
   }
   return self;
 }
@@ -62,7 +63,7 @@ - (void)writeLog:(GDLLogEvent *)log
         return;
       }
     }
-    // TODO(mikehaney24): [[GDLLogStorage sharedInstance] storeLog:transformedLog];
+    [self.storageInstance storeLog:transformedLog];
   });
 }
 

GoogleDataLogger/GoogleDataLogger/Classes/GDLLogger.m

@@ -15,24 +15,12 @@
  */
 
 #import "GDLLogger.h"
+#import "GDLLogger_Private.h"
 
 #import "GDLAssert.h"
 #import "GDLLogEvent.h"
 #import "GDLLogWriter.h"
 
-@interface GDLLogger ()
-
-/** The log mapping identifier that a GDLLogBackend will use to map the extension to proto. */
-@property(nonatomic) NSString *logMapID;
-
-/** The log transformers that will operate on logs logged by this logger. */
-@property(nonatomic) NSArray<id<GDLLogTransformer>> *logTransformers;
-
-/** The target backend of this logger. */
-@property(nonatomic) NSInteger logTarget;
-
-@end
-
 @implementation GDLLogger
 
 - (instancetype)initWithLogMapID:(NSString *)logMapID
@@ -45,6 +33,7 @@ - (instancetype)initWithLogMapID:(NSString *)logMapID
     _logMapID = logMapID;
     _logTransformers = logTransformers;
     _logTarget = logTarget;
+    _logWriterInstance = [GDLLogWriter sharedInstance];
   }
   return self;
 }
@@ -53,14 +42,14 @@ - (void)logTelemetryEvent:(GDLLogEvent *)logEvent {
   GDLAssert(logEvent, @"You can't log a nil event");
   GDLLogEvent *copiedLog = [logEvent copy];
   copiedLog.qosTier = GDLLogQoSTelemetry;
-  [[GDLLogWriter sharedInstance] writeLog:copiedLog afterApplyingTransformers:_logTransformers];
+  [self.logWriterInstance writeLog:copiedLog afterApplyingTransformers:_logTransformers];
 }
 
 - (void)logDataEvent:(GDLLogEvent *)logEvent {
   GDLAssert(logEvent, @"You can't log a nil event");
   GDLAssert(logEvent.qosTier != GDLLogQoSTelemetry, @"Use -logTelemetryEvent, please.");
   GDLLogEvent *copiedLog = [logEvent copy];
-  [[GDLLogWriter sharedInstance] writeLog:copiedLog afterApplyingTransformers:_logTransformers];
+  [self.logWriterInstance writeLog:copiedLog afterApplyingTransformers:_logTransformers];
 }
 
 - (GDLLogEvent *)newEvent {

GoogleDataLogger/GoogleDataLogger/Classes/Private/GDLLogStorage_Private.h

@@ -16,6 +16,8 @@
 
 #import "GDLLogStorage.h"
 
+@class GDLUploader;
+
 @interface GDLLogStorage ()
 
 /** The queue on which all storage work will occur. */
@@ -28,4 +30,7 @@
 @property(nonatomic)
     NSMutableDictionary<NSNumber *, NSMutableSet<NSURL *> *> *logTargetToLogFileSet;
 
+/** The log uploader instance to use. */
+@property(nonatomic) GDLUploader *uploader;
+
 @end

GoogleDataLogger/GoogleDataLogger/Classes/Private/GDLLogWriter_Private.h

@@ -16,9 +16,14 @@
 
 #import "GDLLogWriter.h"
 
+@class GDLLogStorage;
+
 @interface GDLLogWriter ()
 
 /** The queue on which all work will occur. */
 @property(nonatomic) dispatch_queue_t logWritingQueue;
 
+/** The log storage instance used to store logs. Should only be used to inject a testing fake. */
+@property(nonatomic) GDLLogStorage *storageInstance;
+
 @end

GoogleDataLogger/GoogleDataLogger/Classes/Private/GDLLogger_Private.h

@@ -0,0 +1,35 @@
+/*
+ * 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 <GoogleDataLogger/GDLLogger.h>
+
+@class GDLLogWriter;
+
+@interface GDLLogger ()
+
+/** The log mapping identifier that a GDLLogBackend will use to map the extension to proto. */
+@property(nonatomic) NSString *logMapID;
+
+/** The log transformers that will operate on logs logged by this logger. */
+@property(nonatomic) NSArray<id<GDLLogTransformer>> *logTransformers;
+
+/** The target backend of this logger. */
+@property(nonatomic) NSInteger logTarget;
+
+/** The log writer instance to used to write logs. Allows injecting a fake during testing. */
+@property(nonatomic) GDLLogWriter *logWriterInstance;
+
+@end

GoogleDataLogger/Tests/Fakes/GDLLogStorageFake.h

@@ -0,0 +1,26 @@
+/*
+ * 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 "GDLLogStorage.h"
+
+NS_ASSUME_NONNULL_BEGIN
+
+/** A functionless fake that can be injected into classes that need it. */
+@interface GDLLogStorageFake : GDLLogStorage
+
+@end
+
+NS_ASSUME_NONNULL_END

GoogleDataLogger/Tests/Fakes/GDLLogStorageFake.m

@@ -0,0 +1,24 @@
+/*
+ * 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 "GDLLogStorageFake.h"
+
+@implementation GDLLogStorageFake
+
+- (void)storeLog:(GDLLogEvent *)log {
+}
+
+@end

GoogleDataLogger/Tests/Fakes/GDLLogWriterFake.h

@@ -0,0 +1,26 @@
+/*
+ * 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 "GDLLogWriter.h"
+
+NS_ASSUME_NONNULL_BEGIN
+
+/** A functionless fake that can be injected into classes that need it. */
+@interface GDLLogWriterFake : GDLLogWriter
+
+@end
+
+NS_ASSUME_NONNULL_END

GoogleDataLogger/Tests/Fakes/GDLLogWriterFake.m

@@ -0,0 +1,25 @@
+/*
+ * 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 "GDLLogWriterFake.h"
+
+@implementation GDLLogWriterFake
+
+- (void)writeLog:(GDLLogEvent *)log
+    afterApplyingTransformers:(NSArray<id<GDLLogTransformer>> *)logTransformers {
+}
+
+@end

GoogleDataLogger/Tests/Fakes/GDLUploaderFake.h

@@ -0,0 +1,27 @@
+/*
+ * 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 "GDLUploader.h"
+
+NS_ASSUME_NONNULL_BEGIN
+
+@interface GDLUploaderFake : GDLUploader
+
+@property(nonatomic) BOOL forceUploadCalled;
+
+@end
+
+NS_ASSUME_NONNULL_END

GoogleDataLogger/Tests/Fakes/GDLUploaderFake.m

@@ -0,0 +1,25 @@
+/*
+ * 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 "GDLUploaderFake.h"
+
+@implementation GDLUploaderFake
+
+- (void)forceUploadLogs:(NSSet<NSURL *> *)logFiles target:(NSInteger)logTarget {
+  self.forceUploadCalled = YES;
+}
+
+@end

GoogleDataLogger/Tests/GDLLogStorageTest.m

@@ -30,6 +30,7 @@
 #import "GDLAssertHelper.h"
 #import "GDLLogStorage+Testing.h"
 #import "GDLRegistrar+Testing.h"
+#import "GDLUploaderFake.h"
 
 static NSInteger logTarget = 1337;
 
@@ -41,6 +42,8 @@ @interface GDLLogStorageTest : GDLTestCase
 /** The test prioritizer implementation. */
 @property(nullable, nonatomic) GDLTestPrioritizer *testPrioritizer;
 
+@property(nonatomic) GDLUploaderFake *uploaderFake;
+
 @end
 
 @implementation GDLLogStorageTest
@@ -51,6 +54,8 @@ - (void)setUp {
   self.testPrioritizer = [[GDLTestPrioritizer alloc] init];
   [[GDLRegistrar sharedInstance] registerBackend:_testBackend forLogTarget:logTarget];
   [[GDLRegistrar sharedInstance] registerLogPrioritizer:_testPrioritizer forLogTarget:logTarget];
+  self.uploaderFake = [[GDLUploaderFake alloc] init];
+  [GDLLogStorage sharedInstance].uploader = self.uploaderFake;
 }
 
 - (void)tearDown {
@@ -60,6 +65,8 @@ - (void)tearDown {
   self.testPrioritizer = nil;
   [[GDLRegistrar sharedInstance] reset];
   [[GDLLogStorage sharedInstance] reset];
+  [GDLLogStorage sharedInstance].uploader = [GDLUploader sharedInstance];
+  self.uploaderFake = nil;
 }
 
 /** Tests the singleton pattern. */
@@ -194,6 +201,7 @@ - (void)testLogEventDeallocationIsEnforced {
 
 /** Tests encoding and decoding the storage singleton correctly. */
 - (void)testNSSecureCoding {
+  XCTAssertTrue([GDLLogStorage supportsSecureCoding]);
   GDLLogEvent *logEvent = [[GDLLogEvent alloc] initWithLogMapID:@"404" logTarget:logTarget];
   logEvent.extensionBytes = [@"testString" dataUsingEncoding:NSUTF8StringEncoding];
   NSUInteger logHash = logEvent.hash;
@@ -215,7 +223,27 @@ - (void)testNSSecureCoding {
 
 /** Tests logging a fast log causes an upload attempt. */
 - (void)testQoSTierFast {
-  // TODO
+  NSUInteger logHash;
+  // logEvent is autoreleased, and the pool needs to drain.
+  @autoreleasepool {
+    GDLLogEvent *logEvent = [[GDLLogEvent alloc] initWithLogMapID:@"404" logTarget:logTarget];
+    logEvent.extensionBytes = [@"testString" dataUsingEncoding:NSUTF8StringEncoding];
+    logEvent.qosTier = GDLLogQoSFast;
+    logHash = logEvent.hash;
+    XCTAssertFalse(self.uploaderFake.forceUploadCalled);
+    XCTAssertNoThrow([[GDLLogStorage sharedInstance] storeLog:logEvent]);
+  }
+  dispatch_sync([GDLLogStorage sharedInstance].storageQueue, ^{
+    XCTAssertTrue(self.uploaderFake.forceUploadCalled);
+    XCTAssertEqual([GDLLogStorage sharedInstance].logHashToLogFile.count, 1);
+    XCTAssertEqual([GDLLogStorage sharedInstance].logTargetToLogFileSet[@(logTarget)].count, 1);
+    NSURL *logFile = [GDLLogStorage sharedInstance].logHashToLogFile[@(logHash)];
+    XCTAssertNotNil(logFile);
+    XCTAssertTrue([[NSFileManager defaultManager] fileExistsAtPath:logFile.path]);
+    NSError *error;
+    XCTAssertTrue([[NSFileManager defaultManager] removeItemAtURL:logFile error:&error]);
+    XCTAssertNil(error, @"There was an error deleting the logFile: %@", error);
+  });
 }
 
 @end