Limit the number of unsent reports on disk to prevent disk filling (#7619)

Author: samedson

Crashlytics/CHANGELOG.md

@@ -1,5 +1,6 @@
 # Unreleased
-- [added] Added a new API checkAndUpdateUnsentReportsWithCompletion for updating the crash report from the previous run of the app if, for example, the developer wants to implement a feedback dialog to ask end-users for more information. Unsent Crashlytics Reports have familiar methods like setting custom keys and logs.
+- [added] Added a new API checkAndUpdateUnsentReportsWithCompletion for updating the crash report from the previous run of the app if, for example, the developer wants to implement a feedback dialog to ask end-users for more information. Unsent Crashlytics Reports have familiar methods like setting custom keys and logs (#7503).
+- [changed] Added a limit to the number of unsent reports on disk to prevent disk filling up when automatic data collection is off. Developers can ensure this limit is never reached by calling send/deleteUnsentReports every run (#7619).
 
 # v7.7.0
 - [added] Added a new API to allow for bulk logging of custom keys and values (#7302).

Crashlytics/Crashlytics/Controllers/FIRCLSExistingReportManager.h

@@ -21,6 +21,8 @@ NS_ASSUME_NONNULL_BEGIN
 @class FIRCLSDataCollectionToken;
 @class FIRCrashlyticsReport;
 
+FOUNDATION_EXPORT NSUInteger const FIRCLSMaxUnsentReports;
+
 @interface FIRCLSExistingReportManager : NSObject
 
 /**
@@ -60,6 +62,10 @@ NS_ASSUME_NONNULL_BEGIN
  * new report for this run of the app has been created. Any
  * reports in ExistingReportManager will be uploaded or deleted
  * and we don't want to do that for the current run of the app.
+ *
+ * If there are over MAX_UNSENT_REPORTS valid reports, this will delete them.
+ *
+ * This methods is slow and should be called only once.
  */
 - (void)collectExistingReports;
 

Crashlytics/Crashlytics/Controllers/FIRCLSExistingReportManager.m

@@ -17,23 +17,29 @@
 #import "Crashlytics/Crashlytics/Controllers/FIRCLSManagerData.h"
 #import "Crashlytics/Crashlytics/Controllers/FIRCLSReportUploader.h"
 #import "Crashlytics/Crashlytics/DataCollection/FIRCLSDataCollectionToken.h"
+#import "Crashlytics/Crashlytics/Helpers/FIRCLSLogger.h"
 #import "Crashlytics/Crashlytics/Models/FIRCLSFileManager.h"
 #import "Crashlytics/Crashlytics/Models/FIRCLSInternalReport.h"
 #import "Crashlytics/Crashlytics/Private/FIRCrashlyticsReport_Private.h"
 #import "Crashlytics/Crashlytics/Public/FirebaseCrashlytics/FIRCrashlyticsReport.h"
 
+// This value should stay in sync with the Android SDK
+NSUInteger const FIRCLSMaxUnsentReports = 4;
+
 @interface FIRCLSExistingReportManager ()
 
 @property(nonatomic, strong) FIRCLSFileManager *fileManager;
-@property(nonatomic, strong) NSOperationQueue *operationQueue;
 @property(nonatomic, strong) FIRCLSReportUploader *reportUploader;
+@property(nonatomic, strong) NSOperationQueue *operationQueue;
 
 // This list of active reports excludes the brand new active report that will be created this run of
 // the app.
 @property(nonatomic, strong) NSArray *existingUnemptyActiveReportPaths;
 @property(nonatomic, strong) NSArray *processingReportPaths;
 @property(nonatomic, strong) NSArray *preparedReportPaths;
 
+@property(nonatomic, strong) FIRCLSInternalReport *newestInternalReport;
+
 @end
 
 @implementation FIRCLSExistingReportManager
@@ -52,15 +58,16 @@ - (instancetype)initWithManagerData:(FIRCLSManagerData *)managerData
   return self;
 }
 
-NSInteger compareOlder(FIRCLSInternalReport *reportA,
+NSInteger compareNewer(FIRCLSInternalReport *reportA,
                        FIRCLSInternalReport *reportB,
                        void *context) {
-  return [reportA.dateCreated compare:reportB.dateCreated];
+  // Compare naturally sorts with oldest first, so swap A and B
+  return [reportB.dateCreated compare:reportA.dateCreated];
 }
 
 - (void)collectExistingReports {
   self.existingUnemptyActiveReportPaths =
-      [self getUnemptyExistingActiveReportsAndDeleteEmpty:self.fileManager.activePathContents];
+      [self getUnsentActiveReportsAndDeleteEmptyOrOld:self.fileManager.activePathContents];
   self.processingReportPaths = self.fileManager.processingPathContents;
   self.preparedReportPaths = self.fileManager.preparedPathContents;
 }
@@ -70,43 +77,71 @@ - (FIRCrashlyticsReport *)newestUnsentReport {
     return nil;
   }
 
-  NSMutableArray<NSString *> *allReportPaths =
-      [NSMutableArray arrayWithArray:self.existingUnemptyActiveReportPaths];
+  return [[FIRCrashlyticsReport alloc] initWithInternalReport:self.newestInternalReport];
+}
+
+- (NSUInteger)unsentReportsCount {
+  // There are nuances about why we only count active reports.
+  // See the header comment for more information.
+  return self.existingUnemptyActiveReportPaths.count;
+}
 
+/*
+ * This has the side effect of deleting any reports over the max, starting with oldest reports.
+ */
+- (NSArray<NSString *> *)getUnsentActiveReportsAndDeleteEmptyOrOld:(NSArray *)reportPaths {
   NSMutableArray<FIRCLSInternalReport *> *validReports = [NSMutableArray array];
-  for (NSString *path in allReportPaths) {
+  for (NSString *path in reportPaths) {
     FIRCLSInternalReport *_Nullable report = [FIRCLSInternalReport reportWithPath:path];
     if (!report) {
       continue;
     }
+
+    // Delete reports without any crashes or non-fatals
+    if (![report hasAnyEvents]) {
+      [self.operationQueue addOperationWithBlock:^{
+        [self.fileManager removeItemAtPath:path];
+      }];
+      continue;
+    }
+
     [validReports addObject:report];
   }
 
-  [validReports sortUsingFunction:compareOlder context:nil];
+  if (validReports.count == 0) {
+    return @[];
+  }
 
-  FIRCLSInternalReport *_Nullable internalReport = [validReports lastObject];
-  return [[FIRCrashlyticsReport alloc] initWithInternalReport:internalReport];
-}
+  // Sort with the newest at the end
+  [validReports sortUsingFunction:compareNewer context:nil];
 
-- (NSUInteger)unsentReportsCount {
-  // There are nuances about why we only count active reports.
-  // See the header comment for more information.
-  return self.existingUnemptyActiveReportPaths.count;
-}
+  // Set our report for updating in checkAndUpdateUnsentReports
+  self.newestInternalReport = [validReports firstObject];
 
-- (NSArray *)getUnemptyExistingActiveReportsAndDeleteEmpty:(NSArray *)reportPaths {
-  NSMutableArray *unemptyReports = [NSMutableArray array];
-  for (NSString *path in reportPaths) {
-    FIRCLSInternalReport *report = [FIRCLSInternalReport reportWithPath:path];
-    if ([report hasAnyEvents]) {
-      [unemptyReports addObject:path];
-    } else {
+  // Delete any reports above the limit, starting with the oldest
+  // which should be at the start of the array.
+  if (validReports.count > FIRCLSMaxUnsentReports) {
+    NSUInteger deletingCount = validReports.count - FIRCLSMaxUnsentReports;
+    FIRCLSInfoLog(@"Deleting %lu unsent reports over the limit of %lu to prevent disk space from "
+                  @"filling up. To prevent this make sure to call send/deleteUnsentReports.",
+                  deletingCount, FIRCLSMaxUnsentReports);
+  }
+
+  // Not that validReports is sorted, delete any reports at indices > MAX_UNSENT_REPORTS, and
+  // collect the rest of the reports to return.
+  NSMutableArray<NSString *> *validReportPaths = [NSMutableArray array];
+  for (int i = 0; i < validReports.count; i++) {
+    if (i >= FIRCLSMaxUnsentReports) {
       [self.operationQueue addOperationWithBlock:^{
+        NSString *path = [[validReports objectAtIndex:i] path];
         [self.fileManager removeItemAtPath:path];
       }];
+    } else {
+      [validReportPaths addObject:[[validReports objectAtIndex:i] path]];
     }
   }
-  return unemptyReports;
+
+  return validReportPaths;
 }
 
 - (void)sendUnsentReportsWithToken:(FIRCLSDataCollectionToken *)dataCollectionToken

Crashlytics/Crashlytics/Private/FIRCLSExistingReportManager_Private.h

@@ -0,0 +1,33 @@
+// Copyright 2021 Google LLC
+//
+// 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.
+
+#ifndef FIRCLSExistingReportManager_Private_h
+#define FIRCLSExistingReportManager_Private_h
+
+#import "Crashlytics/Crashlytics/Controllers/FIRCLSExistingReportManager.h"
+
+/**
+ * Visible for testing
+ */
+@interface FIRCLSExistingReportManager (Private)
+
+@property(nonatomic, strong) NSOperationQueue *operationQueue;
+
+@property(nonatomic, strong) NSArray *existingUnemptyActiveReportPaths;
+@property(nonatomic, strong) NSArray *processingReportPaths;
+@property(nonatomic, strong) NSArray *preparedReportPaths;
+
+@end
+
+#endif /* FIRCLSExistingReportManager_Private_h */