Update podspec and factor out common test sources (#2336)

* Update podspec and factor out common test sources

* Add a wifi-only QoS specifier

* Change the prioritizer protocol to include upload conditions

* Remove an unused log target.

* Call unprioritizeLog and remove an assert that wasn't helpful

* Put the upload completionBlock on the uploader queue

* Fix the CI and podspec.

* [DO NOT MERGE TO MASTER] Raise the cocoapods version to 1.6.0.rc.2

* [DO NOT MERGE TO MASTER] Update Gemfile correctly

* [DO NOT MERGE TO MASTER]  Use the tag, not the version number.

* Correct an incorrect commit

* Remove the name for standard unit tests

Author: mikehaney24

Gemfile

@@ -1,7 +1,7 @@
 # use 'bundle update' to update to latest gems
 source 'https://rubygems.org'
 
-gem 'cocoapods', :git => 'https://github.com/CocoaPods/CocoaPods.git'
-gem 'cocoapods-core', :git => 'https://github.com/CocoaPods/Core.git'
+gem 'cocoapods', :git => 'https://github.com/CocoaPods/CocoaPods.git', :tag => '1.6.0.rc.2'
+gem 'cocoapods-core', :git => 'https://github.com/CocoaPods/Core.git', :tag => '1.6.0.rc.2'
 gem 'cocoapods-generate', :git => 'https://github.com/square/cocoapods-generate'
 gem 'xcodeproj', :git => 'https://github.com/CocoaPods/Xcodeproj.git'

Gemfile.lock

@@ -1,46 +1,48 @@
 GIT
   remote: https://github.com/CocoaPods/CocoaPods.git
-  revision: 10b69dbd9b991442646944f118f569d855652b26
+  revision: 783424de392ebb327f7dc25d685b8df9605e76be
+  tag: 1.6.0.rc.2
   specs:
-    cocoapods (1.5.3)
+    cocoapods (1.6.0.rc.2)
       activesupport (>= 4.0.2, < 5)
       claide (>= 1.0.2, < 2.0)
-      cocoapods-core (= 1.5.3)
+      cocoapods-core (= 1.6.0.rc.2)
       cocoapods-deintegrate (>= 1.0.2, < 2.0)
-      cocoapods-downloader (>= 1.2.1, < 2.0)
+      cocoapods-downloader (>= 1.2.2, < 2.0)
       cocoapods-plugins (>= 1.0.0, < 2.0)
       cocoapods-search (>= 1.0.0, < 2.0)
       cocoapods-stats (>= 1.0.0, < 2.0)
-      cocoapods-trunk (>= 1.3.0, < 2.0)
+      cocoapods-trunk (>= 1.3.1, < 2.0)
       cocoapods-try (>= 1.1.0, < 2.0)
       colored2 (~> 3.1)
       escape (~> 0.0.4)
-      fourflusher (~> 2.0.1)
+      fourflusher (>= 2.2.0, < 3.0)
       gh_inspector (~> 1.0)
-      molinillo (~> 0.6.5)
+      molinillo (~> 0.6.6)
       nap (~> 1.0)
-      ruby-macho (~> 1.2)
-      xcodeproj (>= 1.5.8, < 2.0)
+      ruby-macho (~> 1.3, >= 1.3.1)
+      xcodeproj (>= 1.8.0, < 2.0)
 
 GIT
   remote: https://github.com/CocoaPods/Core.git
-  revision: 577c69f38fdb56cbdb883b44681ca2b224cad746
+  revision: 2e99d7e1ce9c36bc82c822624e34c150b48342f5
+  tag: 1.6.0.rc.2
   specs:
-    cocoapods-core (1.5.3)
+    cocoapods-core (1.6.0.rc.2)
       activesupport (>= 4.0.2, < 6)
       fuzzy_match (~> 2.0.4)
       nap (~> 1.0)
 
 GIT
   remote: https://github.com/CocoaPods/Xcodeproj.git
-  revision: cadb238767d09942a1c5eba90a3112034438ba23
+  revision: 2bc4222ff4664386363cda25c56d2b54c20d565e
   specs:
-    xcodeproj (1.5.9)
+    xcodeproj (1.8.0)
       CFPropertyList (>= 2.3.3, < 4.0)
-      atomos (~> 0.1.2)
+      atomos (~> 0.1.3)
       claide (>= 1.0.2, < 2.0)
       colored2 (~> 3.1)
-      nanaimo (~> 0.2.5)
+      nanaimo (~> 0.2.6)
 
 GIT
   remote: https://github.com/square/cocoapods-generate
@@ -52,37 +54,37 @@ GEM
   remote: https://rubygems.org/
   specs:
     CFPropertyList (3.0.0)
-    activesupport (4.2.10)
+    activesupport (4.2.11)
       i18n (~> 0.7)
       minitest (~> 5.1)
       thread_safe (~> 0.3, >= 0.3.4)
       tzinfo (~> 1.1)
-    atomos (0.1.2)
+    atomos (0.1.3)
     claide (1.0.2)
     cocoapods-deintegrate (1.0.2)
-    cocoapods-downloader (1.2.1)
+    cocoapods-downloader (1.2.2)
     cocoapods-plugins (1.0.0)
       nap
     cocoapods-search (1.0.0)
-    cocoapods-stats (1.0.0)
-    cocoapods-trunk (1.3.0)
+    cocoapods-stats (1.1.0)
+    cocoapods-trunk (1.3.1)
       nap (>= 0.8, < 2.0)
       netrc (~> 0.11)
     cocoapods-try (1.1.0)
     colored2 (3.1.2)
-    concurrent-ruby (1.0.5)
+    concurrent-ruby (1.1.4)
     escape (0.0.4)
-    fourflusher (2.0.1)
+    fourflusher (2.2.0)
     fuzzy_match (2.0.4)
     gh_inspector (1.1.3)
     i18n (0.9.5)
       concurrent-ruby (~> 1.0)
     minitest (5.11.3)
-    molinillo (0.6.5)
-    nanaimo (0.2.5)
+    molinillo (0.6.6)
+    nanaimo (0.2.6)
     nap (1.1.0)
     netrc (0.11.0)
-    ruby-macho (1.2.0)
+    ruby-macho (1.3.1)
     thread_safe (0.3.6)
     tzinfo (1.2.5)
       thread_safe (~> 0.1)

GoogleDataLogger.podspec

@@ -18,24 +18,28 @@ Shared library for iOS SDK data logging needs.
 
   s.ios.deployment_target = '8.0'
 
-  s.cocoapods_version = '>= 1.5.3'
+  s.cocoapods_version = '>= 1.6.0.rc.2'
 
-  # TODO(mikehaney24): Change to static framework after cocoapods 1.6.0 release?
-  s.static_framework = false
+  s.static_framework = true
   s.prefix_header_file = false
 
   s.source_files = 'GoogleDataLogger/GoogleDataLogger/**/*'
   s.public_header_files = 'GoogleDataLogger/GoogleDataLogger/Classes/Public/*.h'
+  s.private_header_files = 'GoogleDataLogger/GoogleDataLogger/Classes/Private/*.h'
 
   s.dependency 'GoogleUtilities/Logger'
 
   s.pod_target_xcconfig = {
     'GCC_C_LANGUAGE_STANDARD' => 'c99',
-    'GCC_TREAT_WARNINGS_AS_ERRORS' => 'YES'
+    'GCC_TREAT_WARNINGS_AS_ERRORS' => 'YES',
+    'CLANG_UNDEFINED_BEHAVIOR_SANITIZER_NULLABILITY' => 'YES'
   }
 
-  # Test specs
-  s.test_spec 'UnitTests' do |test_spec|
-    test_spec.source_files = 'GoogleDataLogger/Tests/Unit/**/*.{h,m}'
+  common_test_sources = ['GoogleDataLogger/Tests/Common/**/*.{h,m}']
+
+  # Unit test specs
+  s.test_spec do |test_spec|
+    test_spec.requires_app_host = false
+    test_spec.source_files = ['GoogleDataLogger/Tests/Unit/**/*.{h,m}'] + common_test_sources
   end
 end

GoogleDataLogger/GoogleDataLogger/Classes/GDLLogStorage.m

@@ -77,11 +77,10 @@ - (void)storeLog:(GDLLogEvent *)log {
     // Check that a log prioritizer is available for this logTarget.
     id<GDLLogPrioritizer> logPrioritizer =
         [GDLRegistrar sharedInstance].logTargetToPrioritizer[@(logTarget)];
-    GDLAssert(logPrioritizer, @"There's no scorer registered for the given logTarget.");
+    GDLAssert(logPrioritizer, @"There's no prioritizer registered for the given logTarget.");
 
     // Write the extension bytes to disk, get a filename.
     GDLAssert(shortLivedLog.extensionBytes, @"The log should have been serialized to bytes");
-    GDLAssert(shortLivedLog.extension == nil, @"The original log proto should be removed");
     NSURL *logFile = [self saveLogProtoToDisk:shortLivedLog.extensionBytes
                                       logHash:shortLivedLog.hash];
 
@@ -121,6 +120,12 @@ - (void)removeLog:(NSNumber *)logHash logTarget:(NSNumber *)logTarget {
     GDLAssert(logHashes, @"There wasn't a logSet for this logTarget.");
     [logHashes removeObject:logHash];
     // It's fine to not remove the set if it's empty.
+
+    // Check that a log prioritizer is available for this logTarget.
+    id<GDLLogPrioritizer> logPrioritizer =
+        [GDLRegistrar sharedInstance].logTargetToPrioritizer[logTarget];
+    GDLAssert(logPrioritizer, @"There's no prioritizer registered for the given logTarget.");
+    [logPrioritizer unprioritizeLog:logHash];
   });
 }
 

GoogleDataLogger/GoogleDataLogger/Classes/GDLUploadCoordinator.m

@@ -92,25 +92,27 @@ - (GDLUploaderCompletionBlock)onCompleteBlock {
     onCompleteBlock = ^(GDLLogTarget target, GDLClock *nextUploadAttemptUTC, NSError *error) {
       GDLUploadCoordinator *strongSelf = weakSelf;
       if (strongSelf) {
-        NSNumber *logTarget = @(target);
-        if (error) {
-          GDLLogWarning(GDLMCWUploadFailed, @"Error during upload: %@", error);
+        dispatch_async(strongSelf.coordinationQueue, ^{
+          NSNumber *logTarget = @(target);
+          if (error) {
+            GDLLogWarning(GDLMCWUploadFailed, @"Error during upload: %@", error);
+            [strongSelf->_logTargetToInFlightLogSet removeObjectForKey:logTarget];
+            return;
+          }
+          strongSelf->_logTargetToNextUploadTimes[logTarget] = nextUploadAttemptUTC;
+          NSSet<NSNumber *> *logHashSet =
+              [strongSelf->_logTargetToInFlightLogSet objectForKey:logTarget];
+          [strongSelf.logStorage removeLogs:logHashSet logTarget:logTarget];
           [strongSelf->_logTargetToInFlightLogSet removeObjectForKey:logTarget];
-          return;
-        }
-        strongSelf->_logTargetToNextUploadTimes[logTarget] = nextUploadAttemptUTC;
-        NSSet<NSNumber *> *logHashSet =
-            [strongSelf->_logTargetToInFlightLogSet objectForKey:logTarget];
-        [strongSelf.logStorage removeLogs:logHashSet logTarget:logTarget];
-        [strongSelf->_logTargetToInFlightLogSet removeObjectForKey:logTarget];
-        if (strongSelf->_forcedUploadQueue.count) {
-          GDLUploadCoordinatorForceUploadBlock queuedBlock =
-              [strongSelf->_forcedUploadQueue lastObject];
-          if (queuedBlock) {
-            queuedBlock();
+          if (strongSelf->_forcedUploadQueue.count) {
+            GDLUploadCoordinatorForceUploadBlock queuedBlock =
+                [strongSelf->_forcedUploadQueue lastObject];
+            if (queuedBlock) {
+              queuedBlock();
+            }
+            [strongSelf->_forcedUploadQueue removeLastObject];
           }
-          [strongSelf->_forcedUploadQueue removeLastObject];
-        }
+        });
       }
     };
   });
@@ -144,6 +146,8 @@ - (void)startTimer {
 - (void)checkPrioritizersAndUploadLogs {
   __weak GDLUploadCoordinator *weakSelf = self;
   dispatch_async(_coordinationQueue, ^{
+    static int count = 0;
+    count++;
     GDLUploadCoordinator *strongSelf = weakSelf;
     if (strongSelf) {
       NSArray<NSNumber *> *logTargetsReadyForUpload = [self logTargetsReadyForUpload];
@@ -153,10 +157,14 @@ - (void)checkPrioritizersAndUploadLogs {
         id<GDLLogUploader> uploader = strongSelf->_registrar.logTargetToUploader[logTarget];
         GDLAssert(prioritizer && uploader, @"log target '%@' is missing an implementation",
                   logTarget);
-        NSSet<NSNumber *> *logHashesToUpload = [prioritizer logsForNextUpload];
+        GDLUploadConditions conds = [self uploadConditions];
+        NSSet<NSNumber *> *logHashesToUpload = [prioritizer logsToUploadGivenConditions:conds];
         if (logHashesToUpload && logHashesToUpload.count > 0) {
+          NSAssert(logHashesToUpload.count > 0, @"");
           NSSet<NSURL *> *logFilesToUpload =
               [strongSelf.logStorage logHashesToFiles:logHashesToUpload];
+          NSAssert(logFilesToUpload.count == logHashesToUpload.count,
+                   @"There should be the same number of files to logs");
           [uploader uploadLogs:logFilesToUpload onComplete:self.onCompleteBlock];
           strongSelf->_logTargetToInFlightLogSet[logTarget] = logHashesToUpload;
         }
@@ -165,6 +173,12 @@ - (void)checkPrioritizersAndUploadLogs {
   });
 }
 
+/** */
+- (GDLUploadConditions)uploadConditions {
+  // TODO: Compute the real upload conditions.
+  return GDLUploadConditionMobileData;
+}
+
 /** Checks the next upload time for each log target and returns an array of log targets that are
  * able to make an upload attempt.
  *
@@ -174,6 +188,10 @@ - (void)checkPrioritizersAndUploadLogs {
   NSMutableArray *logTargetsReadyForUpload = [[NSMutableArray alloc] init];
   GDLClock *currentTime = [GDLClock snapshot];
   for (NSNumber *logTarget in self.registrar.logTargetToPrioritizer) {
+    // Log targets in flight are not ready.
+    if (_logTargetToInFlightLogSet[logTarget]) {
+      continue;
+    }
     GDLClock *nextUploadTime = _logTargetToNextUploadTimes[logTarget];
 
     // If no next upload time was specified or if the currentTime > nextUpload time, mark as ready.

GoogleDataLogger/GoogleDataLogger/Classes/Public/GDLLogEvent.h

@@ -35,7 +35,10 @@ typedef NS_ENUM(NSInteger, GDLLogQoS) {
   GDLLogQosDefault = 3,
 
   /** This log should be sent immediately along with any other data that can be batched. */
-  GDLLogQoSFast = 4
+  GDLLogQoSFast = 4,
+
+  /** This log should only be uploaded on wifi. */
+  GDLLogQoSWifiOnly = 5,
 };
 
 @interface GDLLogEvent : NSObject <NSSecureCoding>

GoogleDataLogger/GoogleDataLogger/Classes/Public/GDLLogPrioritizer.h

@@ -20,6 +20,18 @@
 
 NS_ASSUME_NONNULL_BEGIN
 
+/** Options that define a set of upload conditions. This is used to help minimize end user data
+ * consumption impact.
+ */
+typedef NS_OPTIONS(NSInteger, GDLUploadConditions) {
+
+  /** An upload would likely use mobile data. */
+  GDLUploadConditionMobileData,
+
+  /** An upload would likely use wifi data. */
+  GDLUploadConditionWifiData,
+};
+
 /** This protocol defines the common interface of a log prioritization. Log prioritizers are
  * stateful objects that prioritize logs upon insertion into storage and remain prepared to return a
  * set of log filenames to the storage system.
@@ -40,11 +52,17 @@ NS_ASSUME_NONNULL_BEGIN
  */
 - (void)prioritizeLog:(GDLLogEvent *)logEvent;
 
-/** Returns a set of logs based on the logic of the prioritizer.
+/** Unprioritizes a log. This method is called when a log has been removed from storage and should
+ * no longer be given as a log to upload.
+ */
+- (void)unprioritizeLog:(NSNumber *)logHash;
+
+/** Returns a set of logs to upload given a set of conditions.
  *
- * @return A set of log hashes to upload, presumably based on the logs' priority.
+ * @param conditions A bit mask specifying the current upload conditions.
+ * @return A set of logs to upload with respect to the current conditions.
  */
-- (NSSet<NSNumber *> *)logsForNextUpload;
+- (NSSet<NSNumber *> *)logsToUploadGivenConditions:(GDLUploadConditions)conditions;
 
 @end
 

GoogleDataLogger/GoogleDataLogger/Classes/Public/GDLLogTargets.h

@@ -23,6 +23,4 @@ typedef NS_ENUM(NSInteger, GDLLogTarget) {
 
   /** The CCT log target. */
   kGDLLogTargetCCT = 1000,
-
-  GDLLogTargetLast = 1001
 };

GoogleDataLogger/Tests/Unit/Categories/GDLLogStorage+Testing.h renamed to GoogleDataLogger/Tests/Common/Categories/GDLLogStorage+Testing.h

@@ -26,7 +26,7 @@ - (instancetype)init {
   return self;
 }
 
-- (NSSet<NSNumber *> *)logsForNextUpload {
+- (NSSet<NSNumber *> *)logsToUploadGivenConditions:(GDLUploadConditions)conditions {
   if (_logsForNextUploadBlock) {
     _logsForNextUploadBlock();
   }
@@ -39,4 +39,7 @@ - (void)prioritizeLog:(GDLLogEvent *)logEvent {
   }
 }
 
+- (void)unprioritizeLog:(nonnull NSNumber *)logHash {
+}
+
 @end