Implement a stored event object to simplify in-memory storage (#2497)

* Implement a stored event object to simplify in-memory storage

This will make passing data to the prioritizers and uploaders easier, and it significantly simplifies logic in the storage system while reducing memory footprint.

* Remove two files that always seem to sneak in somehow.

* Style and a needed cast

Author: mikehaney24

GoogleDataTransport/GoogleDataTransport/Classes/GDTClock.m

@@ -118,8 +118,7 @@ - (BOOL)isAfter:(GDTClock *)otherClock {
 }
 
 - (NSUInteger)hash {
-  // These casts lose some precision, but it's probably fine.
-  return (NSUInteger)_kernelBootTime ^ (NSUInteger)_uptime ^ (NSUInteger)_timeMillis;
+  return [@(_kernelBootTime) hash] ^ [@(_uptime) hash] ^ [@(_timeMillis) hash];
 }
 
 - (BOOL)isEqual:(id)object {

GoogleDataTransport/GoogleDataTransport/Classes/GDTEvent.m

@@ -16,6 +16,8 @@
 
 #import <GoogleDataTransport/GDTEvent.h>
 
+#import <GoogleDataTransport/GDTStoredEvent.h>
+
 #import "GDTAssert.h"
 #import "GDTEvent_Private.h"
 
@@ -61,6 +63,10 @@ - (void)setDataObject:(id<GDTEventDataObject>)dataObject {
   }
 }
 
+- (GDTStoredEvent *)storedEventWithFileURL:(NSURL *)fileURL {
+  return [[GDTStoredEvent alloc] initWithFileURL:fileURL event:self];
+}
+
 #pragma mark - NSSecureCoding and NSCoding Protocols
 
 /** NSCoding key for mappingID property. */

GoogleDataTransport/GoogleDataTransport/Classes/GDTStorage.h

@@ -17,6 +17,7 @@
 #import <Foundation/Foundation.h>
 
 @class GDTEvent;
+@class GDTStoredEvent;
 
 NS_ASSUME_NONNULL_BEGIN
 
@@ -30,27 +31,17 @@ NS_ASSUME_NONNULL_BEGIN
 + (instancetype)sharedInstance;
 
 /** Stores event.dataObjectTransportBytes into a shared on-device folder and tracks the event via
- * its hash and target properties.
- *
- * @note The event param is expected to be deallocated during this method.
+ * a GDTStoredEvent instance.
  *
  * @param event The event to store.
  */
 - (void)storeEvent:(GDTEvent *)event;
 
-/** Removes a set of event from storage specified by their hash.
- *
- * @param eventHashes The set of event hashes to remove.
- * @param target The upload target the event files correspond to.
- */
-- (void)removeEvents:(NSSet<NSNumber *> *)eventHashes target:(NSNumber *)target;
-
-/** Converts a set of event hashes to a set of event files.
+/** Removes a set of events from storage specified by their hash.
  *
- * @param eventHashes A set of event hashes to get the files of.
- * @return A set of equivalent length, containing all the filenames corresponding to the hashes.
+ * @param events The set of stored events to remove.
  */
-- (NSDictionary<NSNumber *, NSURL *> *)eventHashesToFiles:(NSSet<NSNumber *> *)eventHashes;
+- (void)removeEvents:(NSSet<GDTStoredEvent *> *)events;
 
 @end
 

GoogleDataTransport/GoogleDataTransport/Classes/GDTStorage.m

@@ -18,6 +18,7 @@
 #import "GDTStorage_Private.h"
 
 #import <GoogleDataTransport/GDTPrioritizer.h>
+#import <GoogleDataTransport/GDTStoredEvent.h>
 
 #import "GDTAssert.h"
 #import "GDTConsoleLogger.h"
@@ -55,8 +56,8 @@ - (instancetype)init {
   self = [super init];
   if (self) {
     _storageQueue = dispatch_queue_create("com.google.GDTStorage", DISPATCH_QUEUE_SERIAL);
-    _eventHashToFile = [[NSMutableDictionary alloc] init];
-    _targetToEventHashSet = [[NSMutableDictionary alloc] init];
+    _targetToEventSet = [[NSMutableDictionary alloc] init];
+    _storedEvents = [[NSMutableOrderedSet alloc] init];
     _uploader = [GDTUploadCoordinator sharedInstance];
   }
   return self;
@@ -65,94 +66,64 @@ - (instancetype)init {
 - (void)storeEvent:(GDTEvent *)event {
   [self createEventDirectoryIfNotExists];
 
-  // This is done to ensure that event is deallocated at the end of the ensuing block.
-  __block GDTEvent *shortLivedEvent = event;
-  __weak GDTEvent *weakShortLivedEvent = event;
-  event = nil;
-
   dispatch_async(_storageQueue, ^{
     // Check that a backend implementation is available for this target.
-    NSInteger target = shortLivedEvent.target;
+    NSInteger target = event.target;
 
     // Check that a prioritizer is available for this target.
     id<GDTPrioritizer> prioritizer = [GDTRegistrar sharedInstance].targetToPrioritizer[@(target)];
     GDTAssert(prioritizer, @"There's no prioritizer registered for the given target.");
 
     // Write the transport bytes to disk, get a filename.
-    GDTAssert(shortLivedEvent.dataObjectTransportBytes,
-              @"The event should have been serialized to bytes");
-    NSURL *eventFile = [self saveEventBytesToDisk:shortLivedEvent.dataObjectTransportBytes
-                                        eventHash:shortLivedEvent.hash];
+    GDTAssert(event.dataObjectTransportBytes, @"The event should have been serialized to bytes");
+    NSURL *eventFile = [self saveEventBytesToDisk:event.dataObjectTransportBytes
+                                        eventHash:event.hash];
+    GDTStoredEvent *storedEvent = [event storedEventWithFileURL:eventFile];
 
     // Add event to tracking collections.
-    [self addEventToTrackingCollections:shortLivedEvent eventFile:eventFile];
+    [self addEventToTrackingCollections:storedEvent];
 
     // Check the QoS, if it's high priority, notify the target that it has a high priority event.
-    if (shortLivedEvent.qosTier == GDTEventQoSFast) {
-      NSSet<NSNumber *> *allEventsForTarget = self.targetToEventHashSet[@(target)];
+    if (event.qosTier == GDTEventQoSFast) {
+      NSSet<GDTStoredEvent *> *allEventsForTarget = self.targetToEventSet[storedEvent.target];
       [self.uploader forceUploadEvents:allEventsForTarget target:target];
     }
 
-    // Have the prioritizer prioritize the event, enforcing that they do not retain it.
-    @autoreleasepool {
-      [prioritizer prioritizeEvent:shortLivedEvent];
-      shortLivedEvent = nil;
-    }
-    if (weakShortLivedEvent) {
-      GDTLogError(GDTMCEEventWasIllegallyRetained, @"%@",
-                  @"An event should not be retained outside of storage.");
-    };
+    // Have the prioritizer prioritize the event.
+    [prioritizer prioritizeEvent:storedEvent];
   });
 }
 
-- (void)removeEvents:(NSSet<NSNumber *> *)eventHashes target:(NSNumber *)target {
-  dispatch_sync(_storageQueue, ^{
-    for (NSNumber *eventHash in eventHashes) {
-      [self removeEvent:eventHash target:target];
-    }
-  });
-}
-
-- (NSDictionary<NSNumber *, NSURL *> *)eventHashesToFiles:(NSSet<NSNumber *> *)eventHashes {
-  NSMutableDictionary<NSNumber *, NSURL *> *eventHashesToFiles = [[NSMutableDictionary alloc] init];
-  dispatch_sync(_storageQueue, ^{
-    for (NSNumber *hashNumber in eventHashes) {
-      NSURL *eventURL = self.eventHashToFile[hashNumber];
-      GDTAssert(eventURL, @"An event file URL couldn't be found for the given hash");
-      eventHashesToFiles[hashNumber] = eventURL;
+- (void)removeEvents:(NSSet<GDTStoredEvent *> *)events {
+  NSSet<GDTStoredEvent *> *eventsToRemove = [events copy];
+  dispatch_async(_storageQueue, ^{
+    // Check that a prioritizer is available for this target.
+    id<GDTPrioritizer> prioritizer;
+
+    for (GDTStoredEvent *event in eventsToRemove) {
+      // Remove from disk, first and foremost.
+      NSError *error;
+      [[NSFileManager defaultManager] removeItemAtURL:event.eventFileURL error:&error];
+      GDTAssert(error == nil, @"There was an error removing an event file: %@", error);
+
+      if (!prioritizer) {
+        prioritizer = [GDTRegistrar sharedInstance].targetToPrioritizer[event.target];
+      } else {
+        GDTAssert(prioritizer == [GDTRegistrar sharedInstance].targetToPrioritizer[event.target],
+                  @"All logs within an upload set should have the same prioritizer.");
+      }
+
+      // Remove from the tracking collections.
+      [self.storedEvents removeObject:event];
+      [self.targetToEventSet[event.target] removeObject:event];
     }
+    GDTAssert(prioritizer, @"There's no prioritizer registered for the given target.");
+    [prioritizer unprioritizeEvents:events];
   });
-  return eventHashesToFiles;
 }
 
 #pragma mark - Private helper methods
 
-/** Removes the corresponding event file from disk.
- *
- * @param eventHash The hash value of the original event.
- * @param target The target of the original event.
- */
-- (void)removeEvent:(NSNumber *)eventHash target:(NSNumber *)target {
-  NSURL *eventFile = self.eventHashToFile[eventHash];
-
-  // Remove from disk, first and foremost.
-  NSError *error;
-  [[NSFileManager defaultManager] removeItemAtURL:eventFile error:&error];
-  GDTAssert(error == nil, @"There was an error removing an event file: %@", error);
-
-  // Remove from the tracking collections.
-  [self.eventHashToFile removeObjectForKey:eventHash];
-  NSMutableSet<NSNumber *> *eventHashes = self.targetToEventHashSet[target];
-  GDTAssert(eventHashes, @"There wasn't an event set for this target.");
-  [eventHashes removeObject:eventHash];
-  // It's fine to not remove the set if it's empty.
-
-  // Check that a prioritizer is available for this target.
-  id<GDTPrioritizer> prioritizer = [GDTRegistrar sharedInstance].targetToPrioritizer[target];
-  GDTAssert(prioritizer, @"There's no prioritizer registered for the given target.");
-  [prioritizer unprioritizeEvent:eventHash];
-}
-
 /** Creates the storage directory if it does not exist. */
 - (void)createEventDirectoryIfNotExists {
   NSError *error;
@@ -179,6 +150,9 @@ - (NSURL *)saveEventBytesToDisk:(NSData *)transportBytes eventHash:(NSUInteger)e
   NSString *event = [NSString stringWithFormat:@"event-%lu", (unsigned long)eventHash];
   NSURL *eventFilePath = [NSURL fileURLWithPath:[storagePath stringByAppendingPathComponent:event]];
 
+  GDTAssert(![[NSFileManager defaultManager] fileExistsAtPath:eventFilePath.path],
+            @"An event shouldn't already exist at this path: %@", eventFilePath.path);
+
   BOOL writingSuccess = [transportBytes writeToURL:eventFilePath atomically:YES];
   if (!writingSuccess) {
     GDTLogError(GDTMCEFileWriteError, @"An event file could not be written: %@", eventFilePath);
@@ -193,29 +167,22 @@ - (NSURL *)saveEventBytesToDisk:(NSData *)transportBytes eventHash:(NSUInteger)e
  * thread safety.
  *
  * @param event The event to track.
- * @param eventFile The file the event has been saved to.
  */
-- (void)addEventToTrackingCollections:(GDTEvent *)event eventFile:(NSURL *)eventFile {
-  NSInteger target = event.target;
-  NSNumber *eventHash = @(event.hash);
-  NSNumber *targetNumber = @(target);
-  self.eventHashToFile[eventHash] = eventFile;
-  NSMutableSet<NSNumber *> *events = self.targetToEventHashSet[targetNumber];
-  if (events) {
-    [events addObject:eventHash];
-  } else {
-    NSMutableSet<NSNumber *> *eventSet = [NSMutableSet setWithObject:eventHash];
-    self.targetToEventHashSet[targetNumber] = eventSet;
-  }
+- (void)addEventToTrackingCollections:(GDTStoredEvent *)event {
+  [_storedEvents addObject:event];
+  NSMutableSet<GDTStoredEvent *> *events = self.targetToEventSet[event.target];
+  events = events ? events : [[NSMutableSet alloc] init];
+  [events addObject:event];
+  _targetToEventSet[event.target] = events;
 }
 
 #pragma mark - NSSecureCoding
 
-/** The NSKeyedCoder key for the eventHashToFile property. */
-static NSString *const kGDTEventHashToFileKey = @"eventHashToFileKey";
+/** The NSKeyedCoder key for the storedEvents property. */
+static NSString *const kGDTStorageStoredEventsKey = @"GDTStorageStoredEventsKey";
 
-/** The NSKeyedCoder key for the targetToEventHashSet property. */
-static NSString *const kGDTTargetToEventHashSetKey = @"targetToEventHashSetKey";
+/** The NSKeyedCoder key for the targetToEventSet property. */
+static NSString *const kGDTStorageTargetToEventSetKey = @"GDTStorageTargetToEventSetKey";
 
 + (BOOL)supportsSecureCoding {
   return YES;
@@ -225,20 +192,20 @@ - (instancetype)initWithCoder:(NSCoder *)aDecoder {
   // Create the singleton and populate its ivars.
   GDTStorage *sharedInstance = [self.class sharedInstance];
   dispatch_sync(sharedInstance.storageQueue, ^{
-    Class NSMutableDictionaryClass = [NSMutableDictionary class];
-    sharedInstance->_eventHashToFile = [aDecoder decodeObjectOfClass:NSMutableDictionaryClass
-                                                              forKey:kGDTEventHashToFileKey];
-    sharedInstance->_targetToEventHashSet =
-        [aDecoder decodeObjectOfClass:NSMutableDictionaryClass forKey:kGDTTargetToEventHashSetKey];
+    sharedInstance->_storedEvents = [aDecoder decodeObjectOfClass:[NSMutableOrderedSet class]
+                                                           forKey:kGDTStorageStoredEventsKey];
+    sharedInstance->_targetToEventSet =
+        [aDecoder decodeObjectOfClass:[NSMutableDictionary class]
+                               forKey:kGDTStorageTargetToEventSetKey];
   });
   return sharedInstance;
 }
 
 - (void)encodeWithCoder:(NSCoder *)aCoder {
   GDTStorage *sharedInstance = [self.class sharedInstance];
   dispatch_sync(sharedInstance.storageQueue, ^{
-    [aCoder encodeObject:sharedInstance->_eventHashToFile forKey:kGDTEventHashToFileKey];
-    [aCoder encodeObject:sharedInstance->_targetToEventHashSet forKey:kGDTTargetToEventHashSetKey];
+    [aCoder encodeObject:sharedInstance->_storedEvents forKey:kGDTStorageStoredEventsKey];
+    [aCoder encodeObject:sharedInstance->_targetToEventSet forKey:kGDTStorageTargetToEventSetKey];
   });
 }
 

GoogleDataTransport/GoogleDataTransport/Classes/GDTStoredEvent.m

@@ -0,0 +1,84 @@
+/*
+ * 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 "GDTStoredEvent.h"
+
+#import <GoogleDataTransport/GDTClock.h>
+
+@implementation GDTStoredEvent
+
+- (instancetype)initWithFileURL:(NSURL *)URL event:(GDTEvent *)event {
+  self = [super init];
+  if (self) {
+    _eventFileURL = URL;
+    _mappingID = event.mappingID;
+    _target = @(event.target);
+    _qosTier = event.qosTier;
+    _clockSnapshot = event.clockSnapshot;
+    _customPrioritizationParams = event.customPrioritizationParams;
+  }
+  return self;
+}
+
+#pragma mark - NSSecureCoding
+
+/** Coding key for eventFileURL ivar. */
+static NSString *kEventFileURLKey = @"GDTStoredEventEventFileURLKey";
+
+/** Coding key for mappingID ivar. */
+static NSString *kMappingIDKey = @"GDTStoredEventMappingIDKey";
+
+/** Coding key for target ivar. */
+static NSString *kTargetKey = @"GDTStoredEventTargetKey";
+
+/** Coding key for qosTier ivar. */
+static NSString *kQosTierKey = @"GDTStoredEventQosTierKey";
+
+/** Coding key for clockSnapshot ivar. */
+static NSString *kClockSnapshotKey = @"GDTStoredEventClockSnapshotKey";
+
+/** Coding key for customPrioritizationParams ivar. */
+static NSString *kCustomPrioritizationParamsKey = @"GDTStoredEventcustomPrioritizationParamsKey";
+
++ (BOOL)supportsSecureCoding {
+  return YES;
+}
+
+- (void)encodeWithCoder:(nonnull NSCoder *)aCoder {
+  [aCoder encodeObject:_eventFileURL forKey:kEventFileURLKey];
+  [aCoder encodeObject:_mappingID forKey:kMappingIDKey];
+  [aCoder encodeObject:_target forKey:kTargetKey];
+  [aCoder encodeObject:@(_qosTier) forKey:kQosTierKey];
+  [aCoder encodeObject:_clockSnapshot forKey:kClockSnapshotKey];
+  [aCoder encodeObject:_customPrioritizationParams forKey:kCustomPrioritizationParamsKey];
+}
+
+- (nullable instancetype)initWithCoder:(nonnull NSCoder *)aDecoder {
+  self = [self init];
+  if (self) {
+    _eventFileURL = [aDecoder decodeObjectOfClass:[NSURL class] forKey:kEventFileURLKey];
+    _mappingID = [aDecoder decodeObjectOfClass:[NSString class] forKey:kMappingIDKey];
+    _target = [aDecoder decodeObjectOfClass:[NSNumber class] forKey:kTargetKey];
+    NSNumber *qosTier = [aDecoder decodeObjectOfClass:[NSNumber class] forKey:kQosTierKey];
+    _qosTier = [qosTier intValue];
+    _clockSnapshot = [aDecoder decodeObjectOfClass:[GDTClock class] forKey:kClockSnapshotKey];
+    _customPrioritizationParams = [aDecoder decodeObjectOfClass:[NSDictionary class]
+                                                         forKey:kCustomPrioritizationParamsKey];
+  }
+  return self;
+}
+
+@end

GoogleDataTransport/GoogleDataTransport/Classes/GDTTransformer.m

@@ -21,7 +21,6 @@
 
 #import "GDTAssert.h"
 #import "GDTConsoleLogger.h"
-#import "GDTEvent_Private.h"
 #import "GDTStorage.h"
 
 @implementation GDTTransformer

GoogleDataTransport/GoogleDataTransport/Classes/GDTTransport.m

@@ -18,8 +18,8 @@
 #import "GDTTransport_Private.h"
 
 #import "GDTAssert.h"
+#import "GDTClock.h"
 #import "GDTEvent.h"
-#import "GDTEvent_Private.h"
 #import "GDTTransformer.h"
 
 @implementation GDTTransport