Refactor event fileURLs to be derived (#5137)

* Refactor event fileURLs to be derived

Apparently, since iOS 8, it's possible for the app's bundle to change directories as often as every launch. This is a mechanism that's hopefully slightly more robust against that, as data is supposed to carry over, the UUID component of the app's absolute directory might just change.

* Convert a couple of tests, actually use a non-absolute URL in GDTCOREvent

* Make sure to use the event fileURL property as the source of truth.

Add some additional logs and improve the integration test

* Fix KVC not working for the removed property

Author: mikehaney24

GoogleDataTransport/GDTCORLibrary/GDTCOREvent.m

@@ -19,6 +19,7 @@
 #import <GoogleDataTransport/GDTCORAssert.h>
 #import <GoogleDataTransport/GDTCORClock.h>
 #import <GoogleDataTransport/GDTCORConsoleLogger.h>
+#import <GoogleDataTransport/GDTCORPlatform.h>
 
 #import "GDTCORLibrary/Private/GDTCORDataFuture.h"
 #import "GDTCORLibrary/Private/GDTCOREvent_Private.h"
@@ -47,7 +48,7 @@ - (instancetype)copy {
   copy.qosTier = _qosTier;
   copy.clockSnapshot = _clockSnapshot;
   copy.customPrioritizationParams = _customPrioritizationParams;
-  copy->_fileURL = _fileURL;
+  copy->_GDTFilePath = _GDTFilePath;
   GDTCORLogDebug("Copying event %@ to event %@", self, copy);
   return copy;
 }
@@ -57,7 +58,7 @@ - (NSUInteger)hash {
   NSUInteger mappingIDHash = [_mappingID hash];
   NSUInteger timeHash = [_clockSnapshot hash];
   NSInteger dataObjectHash = [_dataObject hash];
-  NSUInteger fileURL = [_fileURL hash];
+  NSUInteger fileURL = [_GDTFilePath hash];
 
   return mappingIDHash ^ _target ^ _qosTier ^ timeHash ^ dataObjectHash ^ fileURL;
 }
@@ -66,6 +67,8 @@ - (BOOL)isEqual:(id)object {
   return [self hash] == [object hash];
 }
 
+#pragma mark - Property overrides
+
 - (void)setDataObject:(id<GDTCOREventDataObject>)dataObject {
   // If you're looking here because of a performance issue in -transportBytes slowing the assignment
   // of -dataObject, one way to address this is to add a queue to this class,
@@ -75,22 +78,31 @@ - (void)setDataObject:(id<GDTCOREventDataObject>)dataObject {
   }
 }
 
-- (BOOL)writeToURL:(NSURL *)fileURL error:(NSError **)error {
+- (NSURL *)fileURL {
+  if (!_GDTFilePath) {
+    _GDTFilePath = [NSString stringWithFormat:@"event-%lu", (unsigned long)self.hash];
+  }
+  return [GDTCORRootDirectory() URLByAppendingPathComponent:_GDTFilePath];
+}
+
+#pragma mark - Private methods
+
+- (BOOL)writeToGDTPath:(NSString *)filePath error:(NSError **)error {
   NSData *dataTransportBytes = [_dataObject transportBytes];
   if (dataTransportBytes == nil) {
-    _fileURL = nil;
+    _GDTFilePath = nil;
     _dataObject = nil;
     return NO;
   }
+  NSURL *fileURL = [GDTCORRootDirectory() URLByAppendingPathComponent:filePath];
   BOOL writingSuccess = [dataTransportBytes writeToURL:fileURL
                                                options:NSDataWritingAtomic
                                                  error:error];
   if (!writingSuccess) {
     GDTCORLogError(GDTCORMCEFileWriteError, @"An event file could not be written: %@", fileURL);
-    _fileURL = nil;
     return NO;
   }
-  _fileURL = fileURL;
+  _GDTFilePath = filePath;
   _dataObject = nil;
   return YES;
 }
@@ -112,6 +124,9 @@ - (BOOL)writeToURL:(NSURL *)fileURL error:(NSError **)error {
 /** NSCoding key for fileURL property. */
 static NSString *fileURLKey = @"_fileURL";
 
+/** NSCoding key for GDTFilePath property. */
+static NSString *kGDTFilePathKey = @"_GDTFilePath";
+
 /** NSCoding key for customPrioritizationParams property. */
 static NSString *customPrioritizationParams = @"_customPrioritizationParams";
 
@@ -152,7 +167,12 @@ - (id)initWithCoder:(NSCoder *)aDecoder {
   if (self) {
     _qosTier = [aDecoder decodeIntegerForKey:qosTierKey];
     _clockSnapshot = [aDecoder decodeObjectOfClass:[GDTCORClock class] forKey:clockSnapshotKey];
-    _fileURL = [aDecoder decodeObjectOfClass:[NSURL class] forKey:fileURLKey];
+    NSURL *fileURL = [aDecoder decodeObjectOfClass:[NSURL class] forKey:fileURLKey];
+    if (fileURL) {
+      _GDTFilePath = [fileURL lastPathComponent];
+    } else {
+      _GDTFilePath = [aDecoder decodeObjectOfClass:[NSString class] forKey:kGDTFilePathKey];
+    }
     _customPrioritizationParams = [aDecoder decodeObjectOfClass:[NSDictionary class]
                                                          forKey:customPrioritizationParams];
   }
@@ -171,7 +191,11 @@ - (id)initWithCoderForStoredEventBackwardCompatibility:(NSCoder *)aDecoder
                                        forKey:kStoredEventQosTierKey] integerValue];
     _clockSnapshot = [aDecoder decodeObjectOfClass:[GDTCORClock class]
                                             forKey:kStoredEventClockSnapshotKey];
-    _fileURL = fileURL;
+    if (fileURL) {
+      _GDTFilePath = [fileURL lastPathComponent];
+    } else {
+      _GDTFilePath = [aDecoder decodeObjectOfClass:[NSString class] forKey:kGDTFilePathKey];
+    }
     _customPrioritizationParams =
         [aDecoder decodeObjectOfClass:[NSDictionary class]
                                forKey:kStoredEventCustomPrioritizationParamsKey];
@@ -184,7 +208,7 @@ - (void)encodeWithCoder:(NSCoder *)aCoder {
   [aCoder encodeInteger:_target forKey:targetKey];
   [aCoder encodeInteger:_qosTier forKey:qosTierKey];
   [aCoder encodeObject:_clockSnapshot forKey:clockSnapshotKey];
-  [aCoder encodeObject:_fileURL forKey:fileURLKey];
+  [aCoder encodeObject:_GDTFilePath forKey:kGDTFilePathKey];
   [aCoder encodeObject:_customPrioritizationParams forKey:customPrioritizationParams];
 }
 

GoogleDataTransport/GDTCORLibrary/GDTCORPlatform.m

@@ -40,6 +40,20 @@
 
 NSString *const kGDTCORApplicationWillTerminateNotification =
     @"GDTCORApplicationWillTerminateNotification";
+
+NSURL *GDTCORRootDirectory(void) {
+  static NSURL *GDTPath;
+  static dispatch_once_t onceToken;
+  dispatch_once(&onceToken, ^{
+    NSString *cachePath =
+        NSSearchPathForDirectoriesInDomains(NSCachesDirectory, NSUserDomainMask, YES)[0];
+    GDTPath =
+        [NSURL fileURLWithPath:[NSString stringWithFormat:@"%@/google-sdks-events", cachePath]];
+    GDTCORLogDebug("GDT's state will be saved to: %@", GDTPath);
+  });
+  return GDTPath;
+}
+
 #if !TARGET_OS_WATCH
 BOOL GDTCORReachabilityFlagsContainWWAN(SCNetworkReachabilityFlags flags) {
 #if TARGET_OS_IOS
@@ -134,9 +148,18 @@ GDTCORNetworkMobileSubtype GDTCORNetworkMobileSubTypeMessage() {
     resultData = [NSKeyedArchiver archivedDataWithRootObject:obj
                                        requiringSecureCoding:YES
                                                        error:error];
-    BOOL result = [resultData writeToFile:archivePath atomically:YES];
-    result = result;  // To get rid of the warning.
-    GDTCORLogDebug(@"Attempt to write archive. successful:%@ path:%@", result, archivePath);
+    if (*error) {
+      GDTCORLogDebug(@"Encoding an object failed: %@", *error);
+      return nil;
+    }
+    if (archivePath) {
+      BOOL result = [resultData writeToFile:archivePath options:NSDataWritingAtomic error:error];
+      if (result == NO || *error) {
+        GDTCORLogDebug(@"Attempt to write archive failed: URL:%@ error:%@", archivePath, *error);
+      } else {
+        GDTCORLogDebug(@"Writing archive succeeded: %@", archivePath);
+      }
+    }
   } else {
 #endif
     BOOL result = NO;
@@ -145,15 +168,23 @@ GDTCORNetworkMobileSubtype GDTCORNetworkMobileSubTypeMessage() {
 #pragma clang diagnostic ignored "-Wdeprecated-declarations"
       resultData = [NSKeyedArchiver archivedDataWithRootObject:obj];
 #pragma clang diagnostic pop
-      result = [resultData writeToFile:archivePath atomically:YES];
+      if (archivePath) {
+        result = [resultData writeToFile:archivePath options:NSDataWritingAtomic error:error];
+        if (result == NO || *error) {
+          GDTCORLogDebug(@"Attempt to write archive failed: URL:%@ error:%@", archivePath, *error);
+        } else {
+          GDTCORLogDebug(@"Writing archive succeeded: %@", archivePath);
+        }
+      }
     } @catch (NSException *exception) {
       NSString *errorString =
           [NSString stringWithFormat:@"An exception was thrown during encoding: %@", exception];
       *error = [NSError errorWithDomain:NSCocoaErrorDomain
                                    code:-1
                                userInfo:@{NSLocalizedFailureReasonErrorKey : errorString}];
     }
-    GDTCORLogDebug(@"Attempt to write archive. successful:%@ path:%@", result, archivePath);
+    GDTCORLogDebug(@"Attempt to write archive. successful:%@ URL:%@ error:%@",
+                   result ? @"YES" : @"NO", archivePath, *error);
   }
   return resultData;
 }

GoogleDataTransport/GDTCORLibrary/GDTCORStorage.m

@@ -27,29 +27,13 @@
 #import "GDTCORLibrary/Private/GDTCORRegistrar_Private.h"
 #import "GDTCORLibrary/Private/GDTCORUploadCoordinator.h"
 
-/** Creates and/or returns a singleton NSString that is the shared storage path.
- *
- * @return The SDK event storage path.
- */
-static NSString *GDTCORStoragePath() {
-  static NSString *storagePath;
-  static dispatch_once_t onceToken;
-  dispatch_once(&onceToken, ^{
-    NSString *cachePath =
-        NSSearchPathForDirectoriesInDomains(NSCachesDirectory, NSUserDomainMask, YES)[0];
-    storagePath = [NSString stringWithFormat:@"%@/google-sdks-events", cachePath];
-    GDTCORLogDebug("Events will be saved to %@", storagePath);
-  });
-  return storagePath;
-}
-
 @implementation GDTCORStorage
 
 + (NSString *)archivePath {
   static NSString *archivePath;
   static dispatch_once_t onceToken;
   dispatch_once(&onceToken, ^{
-    archivePath = [GDTCORStoragePath() stringByAppendingPathComponent:@"GDTCORStorageArchive"];
+    archivePath = [GDTCORRootDirectory() URLByAppendingPathComponent:@"GDTCORStorageArchive"].path;
   });
   return archivePath;
 }
@@ -169,10 +153,10 @@ - (void)removeEvents:(NSSet<GDTCOREvent *> *)events {
 /** Creates the storage directory if it does not exist. */
 - (void)createEventDirectoryIfNotExists {
   NSError *error;
-  BOOL result = [[NSFileManager defaultManager] createDirectoryAtPath:GDTCORStoragePath()
-                                          withIntermediateDirectories:YES
-                                                           attributes:0
-                                                                error:&error];
+  BOOL result = [[NSFileManager defaultManager] createDirectoryAtURL:GDTCORRootDirectory()
+                                         withIntermediateDirectories:YES
+                                                          attributes:0
+                                                               error:&error];
   if (!result || error) {
     GDTCORLogError(GDTCORMCEDirectoryCreationError, @"Error creating the directory: %@", error);
   }
@@ -190,16 +174,13 @@ - (void)createEventDirectoryIfNotExists {
 - (NSURL *)saveEventBytesToDisk:(GDTCOREvent *)event
                       eventHash:(NSUInteger)eventHash
                           error:(NSError **)error {
-  NSString *storagePath = GDTCORStoragePath();
   NSString *eventFileName = [NSString stringWithFormat:@"event-%lu", (unsigned long)eventHash];
-  NSURL *eventFilePath =
-      [NSURL fileURLWithPath:[storagePath stringByAppendingPathComponent:eventFileName]];
-
-  GDTCORAssert(![[NSFileManager defaultManager] fileExistsAtPath:eventFilePath.path],
-               @"An event shouldn't already exist at this path: %@", eventFilePath.path);
-
-  [event writeToURL:eventFilePath error:error];
-  return eventFilePath;
+  NSError *writingError;
+  [event writeToGDTPath:eventFileName error:&writingError];
+  if (writingError) {
+    GDTCORLogDebug(@"There was an error saving an event to disk: %@", writingError);
+  }
+  return event.fileURL;
 }
 
 /** Adds the event to internal tracking collections.
@@ -242,6 +223,8 @@ - (void)appWillBackground:(GDTCORApplication *)app {
     GDTCOREncodeArchive(self, [GDTCORStorage archivePath], &error);
     if (error) {
       GDTCORLogDebug(@"Serializing GDTCORStorage to an archive failed: %@", error);
+    } else {
+      GDTCORLogDebug(@"Serialized GDTCORStorage to %@", [GDTCORStorage archivePath]);
     }
 
     // End the background task if it's still valid.
@@ -256,6 +239,8 @@ - (void)appWillTerminate:(GDTCORApplication *)application {
     GDTCOREncodeArchive(self, [GDTCORStorage archivePath], &error);
     if (error) {
       GDTCORLogDebug(@"Serializing GDTCORStorage to an archive failed: %@", error);
+    } else {
+      GDTCORLogDebug(@"Serialized GDTCORStorage to %@", [GDTCORStorage archivePath]);
     }
   });
 }

GoogleDataTransport/GDTCORLibrary/Private/GDTCOREvent_Private.h

@@ -22,14 +22,17 @@ NS_ASSUME_NONNULL_BEGIN
 
 @interface GDTCOREvent ()
 
+/** The GDT relative file path of the event. */
+@property(nullable, nonatomic, readonly) NSString *GDTFilePath;
+
 /** Writes [dataObject transportBytes] to the given URL, populates fileURL with the filename, then
  * nils the dataObject property. This method should not be called twice on the same event.
  *
- * @param fileURL The fileURL that dataObject will be written to.
+ * @param filePath The GDTCORRootDirectory-relative path that dataObject will be written to.
  * @param error If populated, the error encountered during writing to disk.
  * @return YES if writing dataObject to disk was successful, NO otherwise.
  */
-- (BOOL)writeToURL:(NSURL *)fileURL error:(NSError **)error;
+- (BOOL)writeToGDTPath:(NSString *)filePath error:(NSError **)error;
 
 @end
 

GoogleDataTransport/GDTCORLibrary/Public/GDTCORPlatform.h

@@ -66,6 +66,12 @@ typedef NS_ENUM(NSInteger, GDTCORNetworkMobileSubtype) {
   GDTCORNetworkMobileSubtypeLTE = 11,
 };
 
+/** Returns a URL to the root directory under which all GDT-associated data must be saved.
+ *
+ * @return A URL to the root directory under which all GDT-associated data must be saved.
+ */
+NSURL *GDTCORRootDirectory(void);
+
 #if !TARGET_OS_WATCH
 /** Compares flags with the WWAN reachability flag, if available, and returns YES if present.
  *

GoogleDataTransport/GDTCORTests/Integration/GDTCORIntegrationTest.m

@@ -102,7 +102,7 @@ - (void)testEndToEndEvent {
   [testServer registerTestPaths];
   [testServer start];
 
-  // Create eventgers.
+  // Create events.
   self.transport1 = [[GDTCORTransport alloc] initWithMappingID:@"eventMap1"
                                                   transformers:nil
                                                         target:kGDTCORIntegrationTestTarget];
@@ -168,7 +168,8 @@ - (void)testEndToEndEvent {
 
 /** Generates a bunch of random events. */
 - (void)generateEvents {
-  for (int i = 0; i < arc4random_uniform(10) + 1; i++) {
+  int limit = arc4random_uniform(10) + 1;
+  for (int i = 0; i < limit; i++) {
     // Choose a random transport, and randomly choose if it's a telemetry event.
     GDTCORTransport *transport = arc4random_uniform(2) ? self.transport1 : self.transport2;
     BOOL isTelemetryEvent = arc4random_uniform(2);

GoogleDataTransport/GDTCORTests/Unit/GDTCOREventTest.m

@@ -92,7 +92,7 @@ - (void)testIsEqualAndHash {
   event1.qosTier = GDTCOREventQosDefault;
   event1.customPrioritizationParams = @{@"customParam1" : @"aValue1"};
   NSError *error1;
-  [event1 writeToURL:[NSURL fileURLWithPath:@"/tmp/fake.txt"] error:&error1];
+  [event1 writeToGDTPath:@"/tmp/fake.txt" error:&error1];
   XCTAssertNil(error1);
 
   GDTCOREvent *event2 = [[GDTCOREvent alloc] initWithMappingID:@"1018" target:1];
@@ -104,7 +104,7 @@ - (void)testIsEqualAndHash {
   event2.qosTier = GDTCOREventQosDefault;
   event2.customPrioritizationParams = @{@"customParam1" : @"aValue1"};
   NSError *error2;
-  [event2 writeToURL:[NSURL fileURLWithPath:@"/tmp/fake.txt"] error:&error2];
+  [event2 writeToGDTPath:@"/tmp/fake.txt" error:&error2];
   XCTAssertNil(error2);
 
   XCTAssertEqual([event1 hash], [event2 hash]);

GoogleDataTransport/GDTCORTests/Unit/GDTCORStorageTest.m

@@ -596,18 +596,11 @@ - (void)testMigrationFromOldVersion {
   NSData *v1ArchiveData = [[NSData alloc] initWithBase64EncodedString:base64EncodedArchive
                                                               options:0];
   XCTAssertNotNil(v1ArchiveData);
-  GDTCORStorage *archiveStorage;
-  if (@available(macOS 10.13, iOS 11.0, tvOS 11.0, *)) {
-    NSError *error;
-    XCTAssertNoThrow(archiveStorage =
-                         [NSKeyedUnarchiver unarchivedObjectOfClass:[GDTCORStorage class]
-                                                           fromData:v1ArchiveData
-                                                              error:&error]);
-  } else {
-#if !TARGET_OS_MACCATALYST && !TARGET_OS_WATCH
-    XCTAssertNoThrow(archiveStorage = [NSKeyedUnarchiver unarchiveObjectWithData:v1ArchiveData]);
-#endif
-  }
+  NSError *error;
+  GDTCORStorage *archiveStorage =
+      (GDTCORStorage *)GDTCORDecodeArchive([GDTCORStorage class], nil, v1ArchiveData, &error);
+  XCTAssertNil(error);
+  XCTAssertNotNil(archiveStorage);
   XCTAssertEqual(archiveStorage.targetToEventSet[@(kGDTCORTargetCCT)].count, 6);
   XCTAssertEqual(archiveStorage.targetToEventSet[@(kGDTCORTargetFLL)].count, 12);
   XCTAssertEqual(archiveStorage.storedEvents.count, 18);

GoogleDataTransport/GDTCORTests/Unit/GDTCORUploadPackageTest.m

@@ -113,23 +113,15 @@ - (void)testEncoding {
   NSMutableSet<GDTCOREvent *> *set = [GDTCOREventGenerator generate3Events];
   uploadPackage.events = set;
   uploadPackage.handler = self;
-  GDTCORUploadPackage *recreatedPackage;
   NSError *error;
-
-  if (@available(macOS 10.13, iOS 11.0, tvOS 11.0, *)) {
-    NSData *packageData = [NSKeyedArchiver archivedDataWithRootObject:uploadPackage
-                                                requiringSecureCoding:YES
-                                                                error:&error];
-    recreatedPackage = [NSKeyedUnarchiver unarchivedObjectOfClass:[GDTCORUploadPackage class]
-                                                         fromData:packageData
-                                                            error:&error];
-    XCTAssertNil(error);
-  } else {
-#if !TARGET_OS_MACCATALYST
-    NSData *packageData = [NSKeyedArchiver archivedDataWithRootObject:uploadPackage];
-    recreatedPackage = [NSKeyedUnarchiver unarchiveObjectWithData:packageData];
-#endif
-  }
+  NSData *packageData = GDTCOREncodeArchive(uploadPackage, nil, &error);
+  XCTAssertNil(error);
+  XCTAssertNotNil(packageData);
+  error = nil;
+  GDTCORUploadPackage *recreatedPackage = (GDTCORUploadPackage *)GDTCORDecodeArchive(
+      [GDTCORUploadPackage class], nil, packageData, &error);
+  XCTAssertNil(error);
+  XCTAssertNotNil(recreatedPackage);
   XCTAssertEqualObjects(uploadPackage, recreatedPackage);
 }