Implement additional tests and enhance GDLLogEvent (#2231)

* Move qosTier to the public API and add a custom prioritization dict

* Set the default qosTier in each logging API

* Change a missing transform: impl to an error, rearrange error enums

We can only rearrange the enums because we've not shipped anything yet.

* Implement additional tests

* Remove extra space

Damned flat macbook keyboards.

Author: mikehaney24

GoogleDataLogger/GoogleDataLogger/Classes/GDLLogEvent.m

@@ -28,6 +28,7 @@ - (instancetype)initWithLogMapID:(NSString *)logMapID logTarget:(NSInteger)logTa
   if (self) {
     _logMapID = logMapID;
     _logTarget = logTarget;
+    _qosTier = GDLLogQosDefault;
   }
   return self;
 }
@@ -38,6 +39,7 @@ - (instancetype)copy {
   copy.extensionBytes = _extensionBytes;
   copy.qosTier = _qosTier;
   copy.clockSnapshot = _clockSnapshot;
+  copy.customPrioritizationParams = _customPrioritizationParams;
   return copy;
 }
 

GoogleDataLogger/GoogleDataLogger/Classes/GDLLogWriter.m

@@ -57,8 +57,8 @@ - (void)writeLog:(GDLLogEvent *)log
           return;
         }
       } else {
-        GDLLogWarning(GDLMCWTransformerDoesntImplementTransform,
-                      @"Transformer doesn't implement transform: %@", transformer);
+        GDLLogError(GDLMCETransformerDoesntImplementTransform,
+                    @"Transformer doesn't implement transform: %@", transformer);
         return;
       }
     }

GoogleDataLogger/GoogleDataLogger/Classes/GDLLogger.m

@@ -52,11 +52,13 @@ - (instancetype)initWithLogMapID:(NSString *)logMapID
 - (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];
 }
 
 - (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];
 }

GoogleDataLogger/GoogleDataLogger/Classes/Private/GDLLogEvent_Private.h

@@ -25,9 +25,6 @@ NS_ASSUME_NONNULL_BEGIN
 /** The serialized bytes of the log object. */
 @property(nonatomic) NSData *extensionBytes;
 
-/** The quality of service tier this log belongs to. */
-@property(nonatomic) GDLLogQoS qosTier;
-
 /** The clock snapshot at the time of logging. */
 @property(nonatomic) GDLLogClockSnapshot clockSnapshot;
 

GoogleDataLogger/GoogleDataLogger/Classes/Public/GDLLogEvent.h

@@ -50,6 +50,14 @@ typedef NS_ENUM(NSInteger, GDLLogQoS) {
  * the GDLLogProto protocol. */
 @property(nullable, nonatomic) id<GDLLogProto> extension;
 
+/** The quality of service tier this log belongs to. */
+@property(nonatomic) GDLLogQoS qosTier;
+
+/** A dictionary provided to aid prioritizers by allowing the passing of arbitrary data. It will be
+ * retained by a copy in -copy, but not used for -hash.
+ */
+@property(nullable, nonatomic) NSDictionary *customPrioritizationParams;
+
 // Please use the designated initializer.
 - (instancetype)init NS_UNAVAILABLE;
 

GoogleDataLogger/GoogleDataLogger/DependencyWrappers/GDLConsoleLogger.h

@@ -30,20 +30,20 @@ static GULLoggerService kGDLConsoleLogger = @"[GoogleDataLogger]";
  */
 typedef NS_ENUM(NSInteger, GDLMessageCode) {
 
-  /** For warning messages concerning transform: not being implemented by a log transformer. */
-  GDLMCWTransformerDoesntImplementTransform = 1,
-
   /** For warning messages concerning protoBytes: not being implemented by a log extension. */
-  GDLMCWExtensionMissingBytesImpl = 2,
+  GDLMCWExtensionMissingBytesImpl = 1,
+
+  /** For error messages concerning transform: not being implemented by a log transformer. */
+  GDLMCETransformerDoesntImplementTransform = 1000,
 
   /** For error messages concerning a GDLLogEvent living past the storeLog: invocation. */
-  GDLMCELogEventWasIllegallyRetained = 1000,
+  GDLMCELogEventWasIllegallyRetained = 1001,
 
   /** For error messages concerning the creation of a directory failing. */
-  GDLMCEDirectoryCreationError = 1001,
+  GDLMCEDirectoryCreationError = 1002,
 
-  /** For error messages concerning the writing of  a log file. */
-  GDLMCEFileWriteError = 1002
+  /** For error messages concerning the writing of a log file. */
+  GDLMCEFileWriteError = 1003
 };
 
 /** */

GoogleDataLogger/Tests/GDLLogStorageTest.m

@@ -27,6 +27,7 @@
 #import "GDLTestBackend.h"
 #import "GDLTestPrioritizer.h"
 
+#import "GDLAssertHelper.h"
 #import "GDLLogStorage+Testing.h"
 #import "GDLRegistrar+Testing.h"
 
@@ -45,13 +46,15 @@ @interface GDLLogStorageTest : GDLTestCase
 @implementation GDLLogStorageTest
 
 - (void)setUp {
+  [super setUp];
   self.testBackend = [[GDLTestBackend alloc] init];
   self.testPrioritizer = [[GDLTestPrioritizer alloc] init];
   [[GDLRegistrar sharedInstance] registerBackend:_testBackend forLogTarget:logTarget];
   [[GDLRegistrar sharedInstance] registerLogPrioritizer:_testPrioritizer forLogTarget:logTarget];
 }
 
 - (void)tearDown {
+  [super tearDown];
   // Destroy these objects before the next test begins.
   self.testBackend = nil;
   self.testPrioritizer = nil;
@@ -109,9 +112,84 @@ - (void)testRemoveLog {
   });
 }
 
+/** Tests storing a few different logs. */
+- (void)testStoreMultipleLogs {
+  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([[GDLLogStorage sharedInstance] storeLog:logEvent]);
+
+    logEvent = [[GDLLogEvent alloc] initWithLogMapID:@"100" logTarget:logTarget];
+    logEvent.extensionBytes = [@"testString2" dataUsingEncoding:NSUTF8StringEncoding];
+    log2Hash = logEvent.hash;
+    XCTAssertNoThrow([[GDLLogStorage sharedInstance] storeLog:logEvent]);
+
+    logEvent = [[GDLLogEvent alloc] initWithLogMapID:@"404" logTarget:logTarget];
+    logEvent.extensionBytes = [@"testString3" dataUsingEncoding:NSUTF8StringEncoding];
+    log3Hash = logEvent.hash;
+    XCTAssertNoThrow([[GDLLogStorage sharedInstance] storeLog:logEvent]);
+  }
+  dispatch_sync([GDLLogStorage sharedInstance].storageQueue, ^{
+    XCTAssertEqual([GDLLogStorage sharedInstance].logHashToLogFile.count, 3);
+    XCTAssertEqual([GDLLogStorage sharedInstance].logTargetToLogFileSet[@(logTarget)].count, 3);
+
+    NSURL *log1File = [GDLLogStorage sharedInstance].logHashToLogFile[@(log1Hash)];
+    XCTAssertNotNil(log1File);
+    XCTAssertTrue([[NSFileManager defaultManager] fileExistsAtPath:log1File.path]);
+    NSError *error;
+    XCTAssertTrue([[NSFileManager defaultManager] removeItemAtURL:log1File error:&error]);
+    XCTAssertNil(error, @"There was an error deleting the logFile: %@", error);
+
+    NSURL *log2File = [GDLLogStorage sharedInstance].logHashToLogFile[@(log2Hash)];
+    XCTAssertNotNil(log2File);
+    XCTAssertTrue([[NSFileManager defaultManager] fileExistsAtPath:log2File.path]);
+    error = nil;
+    XCTAssertTrue([[NSFileManager defaultManager] removeItemAtURL:log2File error:&error]);
+    XCTAssertNil(error, @"There was an error deleting the logFile: %@", error);
+
+    NSURL *log3File = [GDLLogStorage sharedInstance].logHashToLogFile[@(log3Hash)];
+    XCTAssertNotNil(log3File);
+    XCTAssertTrue([[NSFileManager defaultManager] fileExistsAtPath:log3File.path]);
+    error = nil;
+    XCTAssertTrue([[NSFileManager defaultManager] removeItemAtURL:log3File error:&error]);
+    XCTAssertNil(error, @"There was an error deleting the logFile: %@", error);
+  });
+}
+
 /** Tests enforcing that a log prioritizer does not retain a log in memory. */
 - (void)testLogEventDeallocationIsEnforced {
-  // TODO
+  XCTestExpectation *errorExpectation = [self expectationWithDescription:@"log retain error"];
+  [GDLAssertHelper setAssertionBlock:^{
+    [errorExpectation fulfill];
+  }];
+
+  // logEvent is referenced past -storeLog, ensuring it's retained, which should assert.
+  GDLLogEvent *logEvent = [[GDLLogEvent alloc] initWithLogMapID:@"404" logTarget:logTarget];
+  logEvent.extensionBytes = [@"testString" dataUsingEncoding:NSUTF8StringEncoding];
+
+  // Store the log and wait for the expectation.
+  [[GDLLogStorage sharedInstance] storeLog:logEvent];
+  [self waitForExpectations:@[ errorExpectation ] timeout:5.0];
+
+  NSURL *logFile;
+  logFile = [GDLLogStorage sharedInstance].logHashToLogFile[@(logEvent.hash)];
+
+  // This isn't strictly necessary because of the -waitForExpectations above.
+  dispatch_sync([GDLLogStorage sharedInstance].storageQueue, ^{
+    XCTAssertTrue([[NSFileManager defaultManager] fileExistsAtPath:logFile.path]);
+  });
+
+  // Ensure log was removed.
+  [[GDLLogStorage sharedInstance] removeLog:@(logEvent.hash) logTarget:@(logTarget)];
+  dispatch_sync([GDLLogStorage sharedInstance].storageQueue, ^{
+    XCTAssertFalse([[NSFileManager defaultManager] fileExistsAtPath:logFile.path]);
+    XCTAssertEqual([GDLLogStorage sharedInstance].logHashToLogFile.count, 0);
+    XCTAssertEqual([GDLLogStorage sharedInstance].logTargetToLogFileSet[@(logTarget)].count, 0);
+  });
 }
 
 /** Tests encoding and decoding the storage singleton correctly. */

GoogleDataLogger/Tests/GDLLogWriterTest.m

@@ -23,6 +23,8 @@
 #import "GDLLogWriter.h"
 #import "GDLLogWriter_Private.h"
 
+#import "GDLAssertHelper.h"
+
 @interface GDLLogWriterTestNilingTransformer : NSObject <GDLLogTransformer>
 
 @end
@@ -106,14 +108,22 @@ - (void)testWriteLogWithBadTransformer {
   GDLLogEvent *log = [[GDLLogEvent alloc] initWithLogMapID:@"2" logTarget:1];
   log.extension = [[GDLLogExtensionTesterSimple alloc] init];
   NSArray *transformers = @[ [[NSObject alloc] init] ];
-  @try {
-    dispatch_sync(writer.logWritingQueue, ^{
-      // TODO(mikehaney24): Assert that storage contains the new log.
-      [writer writeLog:log afterApplyingTransformers:transformers];
-    });
-  } @catch (NSException *exception) {
-    NSLog(@"");
-  }
+
+  XCTestExpectation *errorExpectation = [self expectationWithDescription:@"transform: is missing"];
+  [GDLAssertHelper setAssertionBlock:^{
+    [errorExpectation fulfill];
+  }];
+  [writer writeLog:log afterApplyingTransformers:transformers];
+  [self waitForExpectations:@[ errorExpectation ] timeout:5.0];
+  dispatch_sync(writer.logWritingQueue, ^{
+                    // TODO(mikehaney24): Assert that storage contains the new log.
+                });
+}
+
+/** Tests that writing a nil log throws. */
+- (void)testWritingANilLogThrows {
+  GDLLogWriter *writer = [GDLLogWriter sharedInstance];
+  XCTAssertThrows([writer writeLog:nil afterApplyingTransformers:nil]);
 }
 
 @end