Refactor event saving and searching (#5773)

mikehaney24 · web-flow · commit a61786cbf1be · 2020-06-09T11:19:38.000-07:00
* Refactor event saving and searching

No more symlinks to try and speed searching. Just get a directory's contents and check the filename, which will contain metadata about the event.

* Change one string formatter

* Fix the type of some params, remove unused code

* Minor update

* Continue running after failing to process a file
diff --git a/GoogleDataTransport/GDTCORLibrary/GDTCORFlatFileStorage.m b/GoogleDataTransport/GDTCORLibrary/GDTCORFlatFileStorage.m
@@ -102,74 +102,71 @@ + (NSString *)libraryDataStoragePath {
   return libraryDataPath;
 }
 
-+ (NSDictionary<NSString *, NSString *> *)pathsForEvent:(GDTCOREvent *)event {
-  NSString *dataPath =
-      [NSString stringWithFormat:@"%@/%ld/%@", [GDTCORFlatFileStorage baseEventStoragePath],
-                                 (long)event.target, event.eventID];
-  NSString *mappingIDPath =
-      [NSString stringWithFormat:@"%@/%ld/%@/%@", [GDTCORFlatFileStorage baseEventStoragePath],
-                                 (long)event.target, event.mappingID, event.eventID];
-  NSString *qosTierPath =
-      [NSString stringWithFormat:@"%@/%ld/%ld/%@", [GDTCORFlatFileStorage baseEventStoragePath],
-                                 (long)event.target, (long)event.qosTier, event.eventID];
-  return @{
-    gGDTCORFlatFileStorageEventDataPathKey : dataPath,
-    gGDTCORFlatFileStorageMappingIDPathKey : mappingIDPath,
-    gGDTCORFlatFileStorageQoSTierPathKey : qosTierPath
-  };
-}
-
 + (NSString *)pathForTarget:(GDTCORTarget)target
-                    qosTier:(nullable NSNumber *)qosTier
-                  mappingID:(nullable NSString *)mappingID {
-  NSString *baseEventPath = [GDTCORFlatFileStorage baseEventStoragePath];
-  // If only a target was given, return the target path.
-  if (qosTier == nil && mappingID == nil) {
-    return [NSString stringWithFormat:@"%@/%ld", baseEventPath, (long)target];
-  }
-
-  // If only a target and mappingID were given, return the mapping ID path.
-  if (qosTier == nil) {
-    return [NSString stringWithFormat:@"%@/%ld/%@", baseEventPath, (long)target, mappingID];
-  }
-
-  // If only a target and qosTier were given, return the QoS tier path.
-  if (mappingID == nil) {
-    return [NSString stringWithFormat:@"%@/%ld/%@", baseEventPath, (long)target, qosTier];
-  }
-
-  // If a target, mappingID, and qosTier were all given, return a single target/qosTier/mappingID
-  // directory.
+                    eventID:(NSNumber *)eventID
+                    qosTier:(NSNumber *)qosTier
+                  mappingID:(NSString *)mappingID {
   return
-      [NSString stringWithFormat:@"%@/%ld/%@/%@", baseEventPath, (long)target, qosTier, mappingID];
+      [NSString stringWithFormat:@"%@/%ld/%@.%@.%@", [GDTCORFlatFileStorage baseEventStoragePath],
+                                 (long)target, eventID, qosTier, mappingID];
 }
 
-+ (NSArray<NSString *> *)searchPathsWithEventSelector:(GDTCORStorageEventSelector *)eventSelector {
-  NSMutableArray<NSString *> *searchPaths = [[NSMutableArray alloc] init];
-  if (eventSelector.selectedQosTiers && eventSelector.selectedQosTiers.count > 0) {
-    for (NSNumber *qosTier in eventSelector.selectedQosTiers) {
-      NSString *searchPath = [self pathForTarget:eventSelector.selectedTarget
-                                         qosTier:qosTier
-                                       mappingID:eventSelector.selectedMappingID];
-      BOOL isDirectory;
-      if ([[NSFileManager defaultManager] fileExistsAtPath:searchPath isDirectory:&isDirectory]) {
-        if (isDirectory) {
-          [searchPaths addObject:searchPath];
-        }
-      }
++ (NSSet<NSString *> *)pathsForTarget:(GDTCORTarget)target
+                             eventIDs:(nullable NSSet<NSNumber *> *)eventIDs
+                             qosTiers:(nullable NSSet<NSNumber *> *)qosTiers
+                           mappingIDs:(nullable NSSet<NSString *> *)mappingIDs {
+  NSMutableSet<NSString *> *paths = [[NSMutableSet alloc] init];
+  NSFileManager *fileManager = [NSFileManager defaultManager];
+  NSString *targetPath = [NSString
+      stringWithFormat:@"%@/%ld", [GDTCORFlatFileStorage baseEventStoragePath], (long)target];
+  NSError *error;
+  NSArray<NSString *> *dirPaths = [fileManager contentsOfDirectoryAtPath:targetPath error:&error];
+  if (error) {
+    GDTCORLogDebug(@"There was an error reading the contents of the target path: %@", error);
+    return paths;
+  }
+  BOOL checkingIDs = eventIDs.count > 0;
+  BOOL checkingQosTiers = qosTiers.count > 0;
+  BOOL checkingMappingIDs = mappingIDs.count > 0;
+  BOOL checkingAnything = checkingIDs == NO && checkingQosTiers == NO && checkingMappingIDs == NO;
+  for (NSString *path in dirPaths) {
+    if (checkingAnything) {
+      [paths addObject:path];
+      continue;
     }
-  } else {
-    NSString *searchPath = [self pathForTarget:eventSelector.selectedTarget
-                                       qosTier:nil
-                                     mappingID:eventSelector.selectedMappingID];
-    BOOL isDirectory;
-    if ([[NSFileManager defaultManager] fileExistsAtPath:searchPath isDirectory:&isDirectory]) {
-      if (isDirectory) {
-        [searchPaths addObject:searchPath];
-      }
+    NSString *filename = [path lastPathComponent];
+    NSArray<NSString *> *components = [filename componentsSeparatedByString:@"."];
+    if (components.count != 3) {
+      GDTCORLogDebug(@"There was an error reading the filename components: %@", components);
+      [[NSFileManager defaultManager] removeItemAtPath:path error:nil];
+      continue;
+    }
+    NSNumber *eventID = @(components[0].integerValue);
+    NSNumber *qosTier = @(components[1].integerValue);
+    NSString *mappingID = components[2];
+    if (eventID == nil || qosTier == nil) {
+      GDTCORLogDebug(@"There was an error parsing the filename components: %@", components);
+      [[NSFileManager defaultManager] removeItemAtPath:path error:nil];
+      continue;
+    }
+    NSNumber *eventIDMatch = checkingIDs ? @([eventIDs containsObject:eventID]) : nil;
+    NSNumber *qosTierMatch = checkingQosTiers ? @([qosTiers containsObject:qosTier]) : nil;
+    NSNumber *mappingIDMatch = checkingMappingIDs ? @([mappingIDs containsObject:mappingID]) : nil;
+    if ((eventIDMatch == nil || eventIDMatch.boolValue) &&
+        (qosTierMatch == nil || qosTierMatch.boolValue) &&
+        (mappingIDMatch == nil || mappingIDMatch.boolValue)) {
+      [paths addObject:path];
     }
   }
-  return searchPaths;
+  return paths;
+}
+
++ (NSArray<NSString *> *)searchPathsWithEventSelector:(GDTCORStorageEventSelector *)eventSelector {
+  NSSet *searchPaths = [GDTCORFlatFileStorage pathsForTarget:eventSelector.selectedTarget
+                                                    eventIDs:eventSelector.selectedEventIDs
+                                                    qosTiers:eventSelector.selectedQosTiers
+                                                  mappingIDs:eventSelector.selectedMappingIDs];
+  return [searchPaths allObjects];
 }
 
 + (instancetype)sharedInstance {
@@ -357,38 +354,23 @@ - (void)removeLibraryDataForKey:(nonnull NSString *)key
 }
 
 - (BOOL)hasEventsForTarget:(GDTCORTarget)target {
-  NSString *searchPath = [GDTCORFlatFileStorage pathForTarget:target qosTier:nil mappingID:nil];
-  NSFileManager *fm = [NSFileManager defaultManager];
-  NSDirectoryEnumerator *enumerator = [fm enumeratorAtPath:searchPath];
-  while ([enumerator nextObject]) {
-    if ([enumerator.fileAttributes[NSFileType] isEqual:NSFileTypeRegular]) {
-      return YES;
-    }
-  }
-  return NO;
+  // TODO(mikehaney24): Increase the performance of this call.
+  GDTCORStorageEventSelector *eventSelector =
+      [[GDTCORStorageEventSelector alloc] initWithTarget:target
+                                                eventIDs:nil
+                                              mappingIDs:nil
+                                                qosTiers:nil];
+  id<GDTCORStorageEventIterator> iter = [self iteratorWithSelector:eventSelector];
+  return [iter nextEvent] != nil;
 }
 
 - (nullable id<GDTCORStorageEventIterator>)iteratorWithSelector:
     (nonnull GDTCORStorageEventSelector *)eventSelector {
   __block GDTCORFlatFileStorageIterator *iterator;
   dispatch_sync(_storageQueue, ^{
-    NSMutableArray<NSString *> *filePaths;
     NSArray<NSString *> *searchPaths =
         [GDTCORFlatFileStorage searchPathsWithEventSelector:eventSelector];
-    for (NSString *searchPath in searchPaths) {
-      NSDirectoryEnumerator *enumerator =
-          [[NSFileManager defaultManager] enumeratorAtPath:searchPath];
-      NSString *nextFile;
-      while ((nextFile = [enumerator nextObject])) {
-        NSFileAttributeType fileType = enumerator.fileAttributes[NSFileType];
-        if ([fileType isEqual:NSFileTypeRegular]) {
-          [filePaths addObject:nextFile];
-        }
-      }
-      iterator = [[GDTCORFlatFileStorageIterator alloc] initWithTarget:eventSelector.selectedTarget
-                                                                 queue:self->_storageQueue];
-      iterator.eventFiles = filePaths;
-    }
+    iterator.eventFiles = searchPaths;
   });
   return iterator;
 }
diff --git a/GoogleDataTransport/GDTCORLibrary/GDTCORStorageEventSelector.m b/GoogleDataTransport/GDTCORLibrary/GDTCORStorageEventSelector.m
@@ -19,14 +19,14 @@
 @implementation GDTCORStorageEventSelector
 
 - (instancetype)initWithTarget:(GDTCORTarget)target
-                eventIDEqualTo:(nullable NSNumber *)eventID
-              mappingIDEqualTo:(nullable NSString *)mappingID
-                      qosTiers:(nullable NSArray<NSNumber *> *)qosTiers {
+                      eventIDs:(nullable NSSet<NSNumber *> *)eventIDs
+                    mappingIDs:(nullable NSSet<NSString *> *)mappingIDs
+                      qosTiers:(nullable NSSet<NSNumber *> *)qosTiers {
   self = [super init];
   if (self) {
     _selectedTarget = target;
-    _selectedEventID = eventID;
-    _selectedMappingID = mappingID;
+    _selectedEventIDs = eventIDs;
+    _selectedMappingIDs = mappingIDs;
     _selectedQosTiers = qosTiers;
   }
   return self;
diff --git a/GoogleDataTransport/GDTCORLibrary/Private/GDTCORFlatFileStorage.h b/GoogleDataTransport/GDTCORLibrary/Private/GDTCORFlatFileStorage.h
@@ -25,21 +25,10 @@
 
 NS_ASSUME_NONNULL_BEGIN
 
-/** The key of the event data path if a path dictionary is returned. */
-FOUNDATION_EXPORT NSString *const gGDTCORFlatFileStorageEventDataPathKey;
-
-/** The key of the event's mapping ID path if a path dictionary is returned. */
-FOUNDATION_EXPORT NSString *const gGDTCORFlatFileStorageMappingIDPathKey;
-
-/** The key of the event's qos tier path if a path dictionary is returned. */
-FOUNDATION_EXPORT NSString *const gGDTCORFlatFileStorageQoSTierPathKey;
-
 /** Manages the storage of events. This class is thread-safe.
  *
  * Event files will be stored as follows:
- *   <app cache>/gdt_event_data/<target>/<eventID> as a normal file write
- *   <app cache>/gdt_event_data/<target>/<qosTier>/<eventID> as a symbolic link
- *   <app cache>/gdt_event_data/<target>/<mappingID>/<eventID> as a symbolic link
+ *   <app cache>/gdt_event_data/<target>/<eventID>.<qosTier>.<mappingID>
  *
  * Library data will be stored as follows:
  *   <app cache>/gdt_library_data/<key of library data>
@@ -85,24 +74,29 @@ FOUNDATION_EXPORT NSString *const gGDTCORFlatFileStorageQoSTierPathKey;
  */
 + (NSString *)libraryDataStoragePath;
 
-/** Returns storage paths for the given event, though the paths may not exist.
- *
- * @note The keys of this dictionary are declared in this header.
- * @param event The event to map to storage paths.
- */
-+ (NSDictionary<NSString *, NSString *> *)pathsForEvent:(GDTCOREvent *)event;
-
-/** Returns a storage path to events for the given target, qosTier, and mapping ID. The path may not
- * exist.
+/** Returns a constructed storage path based on the given values. This path may not exist.
  *
  * @param target The target, which is necessary to be given a path.
- * @param qosTier An optional parameter to get a more specific path.
- * @param mappingID An optional parameter to get a more specific path.
+ * @param eventID The eventID.
+ * @param qosTier The qosTier.
+ * @param mappingID The mappingID.
  * @return The path representing the combination of the given parameters.
  */
 + (NSString *)pathForTarget:(GDTCORTarget)target
-                    qosTier:(nullable NSNumber *)qosTier
-                  mappingID:(nullable NSString *)mappingID;
+                    eventID:(NSNumber *)eventID
+                    qosTier:(NSNumber *)qosTier
+                  mappingID:(NSString *)mappingID;
+
+/** Returns extant paths that match all of the given parameters.
+ *
+ * @param eventIDs The list of eventIDs to look for, or nil for any.
+ * @param qosTiers The list of qosTiers to look for, or nil for any.
+ * @param mappingIDs The list of mappingIDs to look for, or nil for any.
+ */
++ (NSSet<NSString *> *)pathsForTarget:(GDTCORTarget)target
+                             eventIDs:(nullable NSSet<NSNumber *> *)eventIDs
+                             qosTiers:(nullable NSSet<NSNumber *> *)qosTiers
+                           mappingIDs:(nullable NSSet<NSString *> *)mappingIDs;
 
 /** Returns a list of paths that will contain events for the given event selector.
  *
diff --git a/GoogleDataTransport/GDTCORLibrary/Public/GDTCORStorageEventSelector.h b/GoogleDataTransport/GDTCORLibrary/Public/GDTCORStorageEventSelector.h
@@ -28,26 +28,26 @@ NS_ASSUME_NONNULL_BEGIN
 @property(readonly, nonatomic) GDTCORTarget selectedTarget;
 
 /** Finds a specific event. */
-@property(nullable, readonly, nonatomic) NSNumber *selectedEventID;
+@property(nullable, readonly, nonatomic) NSSet<NSNumber *> *selectedEventIDs;
 
 /** Finds all events of a mappingID. */
-@property(nullable, readonly, nonatomic) NSString *selectedMappingID;
+@property(nullable, readonly, nonatomic) NSSet<NSString *> *selectedMappingIDs;
 
 /** Finds all events matching the qosTiers in this list. */
-@property(nullable, readonly, nonatomic) NSArray<NSNumber *> *selectedQosTiers;
+@property(nullable, readonly, nonatomic) NSSet<NSNumber *> *selectedQosTiers;
 
 /** Instantiates an event selector.
  *
  * @param target The selected target.
- * @param eventID Optional param to find an event matching this eventID.
- * @param mappingID Optional param to find events matching this mappingID.
+ * @param eventIDs Optional param to find an event matching this eventID.
+ * @param mappingIDs Optional param to find events matching this mappingID.
  * @param qosTiers Optional param to find events matching the given QoS tiers.
  * @return An immutable event selector instance.
  */
 - (instancetype)initWithTarget:(GDTCORTarget)target
-                eventIDEqualTo:(nullable NSNumber *)eventID
-              mappingIDEqualTo:(nullable NSString *)mappingID
-                      qosTiers:(nullable NSArray<NSNumber *> *)qosTiers;
+                      eventIDs:(nullable NSSet<NSNumber *> *)eventIDs
+                    mappingIDs:(nullable NSSet<NSString *> *)mappingIDs
+                      qosTiers:(nullable NSSet<NSNumber *> *)qosTiers;
 @end
 
 NS_ASSUME_NONNULL_END
diff --git a/GoogleDataTransport/GDTCORTests/Unit/GDTCORFlatFileStorageTest.m b/GoogleDataTransport/GDTCORTests/Unit/GDTCORFlatFileStorageTest.m
@@ -525,49 +525,23 @@ - (void)testSaveAndRemoveLibraryData {
   [self waitForExpectations:@[ expectation ] timeout:10.0];
 }
 
-/** Tests that -pathsForEvent returns 3 file paths of the appropriate structure. */
-- (void)testPathsForEvent {
-  GDTCOREvent *event = [[GDTCOREvent alloc] initWithMappingID:@"test" target:kGDTCORTargetTest];
-  NSDictionary<NSString *, NSString *> *eventPaths = [GDTCORFlatFileStorage pathsForEvent:event];
+/** Tests that -pathForTarget:qosTier:mappingID: returns the correctly structured path. */
+- (void)testPathsForTargetEventIDQoSTierMappingID {
+  // TODO(mikehaney24): Complete below tests once events are being saved to disk.
 
-  NSString *dataPathSuffix = event.eventID.stringValue;
-  XCTAssertTrue([eventPaths[gGDTCORFlatFileStorageEventDataPathKey] hasSuffix:dataPathSuffix]);
+  // Store some predictably generated events across > 1 target.
 
-  NSString *qosTierSuffix =
-      [NSString stringWithFormat:@"%ld/%@/%@", (long)event.target, @(event.qosTier), event.eventID];
-  XCTAssertTrue([eventPaths[gGDTCORFlatFileStorageQoSTierPathKey] hasSuffix:qosTierSuffix]);
+  // Search for all events (by target).
 
-  NSString *mappingIDSuffix =
-      [NSString stringWithFormat:@"%ld/%@/%@", (long)event.target, event.mappingID, event.eventID];
-  XCTAssertTrue([eventPaths[gGDTCORFlatFileStorageMappingIDPathKey] hasSuffix:mappingIDSuffix]);
-}
+  // Search for events by eventID.
 
-/** Tests that -pathForTarget:qosTier:mappingID: returns the correctly structured path. */
-- (void)testPathForTargetQoSTier {
-  // Target only.
-  NSString *expectedSuffix = [NSString stringWithFormat:@"%ld", (long)kGDTCORTargetTest];
-  NSString *path = [GDTCORFlatFileStorage pathForTarget:kGDTCORTargetTest
-                                                qosTier:nil
-                                              mappingID:nil];
-  XCTAssertTrue([path hasSuffix:expectedSuffix]);
-
-  // qosTier is given.
-  path = [GDTCORFlatFileStorage pathForTarget:kGDTCORTargetTest
-                                      qosTier:@(GDTCOREventQoSFast)
-                                    mappingID:nil];
-  XCTAssertTrue([path hasSuffix:@(GDTCOREventQoSFast).stringValue]);
-
-  // mappingID is given.
-  path = [GDTCORFlatFileStorage pathForTarget:kGDTCORTargetTest qosTier:nil mappingID:@"test"];
-  XCTAssertTrue([path hasSuffix:@"test"]);
-
-  // Both qosTier and mappingID are given.
-  path = [GDTCORFlatFileStorage pathForTarget:kGDTCORTargetTest
-                                      qosTier:@(GDTCOREventQoSFast)
-                                    mappingID:@"test"];
-  expectedSuffix = [NSString
-      stringWithFormat:@"%ld/%ld/%@", (long)kGDTCORTargetTest, (long)GDTCOREventQoSFast, @"test"];
-  XCTAssertTrue([path hasSuffix:expectedSuffix]);
+  // Search for events by qosTier.
+
+  // Search for events by mappingID.
+
+  // Search for events by eventID and qosTier.
+
+  // Search for events by qosTier and mappingID.
 }
 
 /** Tests that searchPathsWithEventSelector: returns the appropriate event search paths. */
@@ -586,8 +560,8 @@ - (void)testHasEventsForTarget {
 - (void)testIteratorWithSelector {
   GDTCORStorageEventSelector *eventSelector =
       [[GDTCORStorageEventSelector alloc] initWithTarget:kGDTCORTargetTest
-                                          eventIDEqualTo:nil
-                                        mappingIDEqualTo:nil
+                                                eventIDs:nil
+                                              mappingIDs:nil
                                                 qosTiers:nil];
   id<GDTCORStorageEventIterator> iter =
       [[GDTCORFlatFileStorage sharedInstance] iteratorWithSelector:eventSelector];
