Skip to content

Commit 4d86017

Browse files
samedsonsamedson
authored
Collect reports refetch (#5279)
1 parent 0306fc2 commit 4d86017

File tree

6 files changed

+50
-120
lines changed

6 files changed

+50
-120
lines changed

‎Crashlytics/Crashlytics/Controllers/FIRCLSReportManager.m

Lines changed: 44 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -326,12 +326,6 @@ - (FBLPromise *)deleteUnsentReports {
326326
return [FBLPromise resolvedWith:@NO];
327327
}
328328

329-
if (![self.settings crashReportingEnabled]) {
330-
FIRCLSInfoLog(@"Reporting is disabled");
331-
[_fileManager removeContentsOfAllPaths];
332-
return [FBLPromise resolvedWith:@NO];
333-
}
334-
335329
if (![_fileManager createReportDirectories]) {
336330
return [FBLPromise resolvedWith:@NO];
337331
}
@@ -364,11 +358,13 @@ - (FBLPromise *)deleteUnsentReports {
364358
FIRCLSDebugLog(@"Automatic data collection is enabled.");
365359
FIRCLSDebugLog(@"Unsent reports will be uploaded at startup");
366360
FIRCLSDataCollectionToken *dataCollectionToken = [FIRCLSDataCollectionToken validToken];
367-
[self startNetworkRequestsWithToken:dataCollectionToken
368-
preexistingReportPaths:preexistingReportPaths
369-
waitForSettingsRequest:NO
370-
blockingSend:launchFailure
371-
report:report];
361+
362+
[self beginSettingsAndOnboardingWithToken:dataCollectionToken waitForSettingsRequest:NO];
363+
364+
[self beginReportUploadsWithToken:dataCollectionToken
365+
preexistingReportPaths:preexistingReportPaths
366+
blockingSend:launchFailure
367+
report:report];
372368

373369
// If data collection is enabled, the SDK will not notify the user
374370
// when unsent reports are available, or respect Send / DeleteUnsentReports
@@ -401,16 +397,17 @@ - (FBLPromise *)deleteUnsentReports {
401397
BOOL waitForSetting =
402398
!self.settings.shouldUseNewReportEndpoint && !self.settings.orgID;
403399

404-
[self startNetworkRequestsWithToken:dataCollectionToken
405-
preexistingReportPaths:preexistingReportPaths
406-
waitForSettingsRequest:waitForSetting
407-
blockingSend:NO
408-
report:report];
400+
[self beginSettingsAndOnboardingWithToken:dataCollectionToken
401+
waitForSettingsRequest:waitForSetting];
402+
403+
[self beginReportUploadsWithToken:dataCollectionToken
404+
preexistingReportPaths:preexistingReportPaths
405+
blockingSend:NO
406+
report:report];
409407

410408
} else if (action == FIRCLSReportActionDelete) {
411409
FIRCLSDebugLog(@"Deleting unsent reports.");
412-
[self removeExistingReportPaths:preexistingReportPaths];
413-
[self removeContentsInOtherReportingDirectories];
410+
[self deleteUnsentReportsWithPreexisting:preexistingReportPaths];
414411
} else {
415412
FIRCLSErrorLog(@"Unknown report action: %d", action);
416413
}
@@ -456,11 +453,8 @@ - (void)checkAndRotateInstallUUIDIfNeededWithReport:(FIRCLSInternalReport *)repo
456453
}];
457454
}
458455

459-
- (void)startNetworkRequestsWithToken:(FIRCLSDataCollectionToken *)token
460-
preexistingReportPaths:(NSArray *)preexistingReportPaths
461-
waitForSettingsRequest:(BOOL)waitForSettings
462-
blockingSend:(BOOL)blockingSend
463-
report:(FIRCLSInternalReport *)report {
456+
- (void)beginSettingsAndOnboardingWithToken:(FIRCLSDataCollectionToken *)token
457+
waitForSettingsRequest:(BOOL)waitForSettings {
464458
if (self.settings.isCacheExpired) {
465459
// This method can be called more than once if the user calls
466460
// SendUnsentReports again, so don't repeat the settings fetch
@@ -471,11 +465,22 @@ - (void)startNetworkRequestsWithToken:(FIRCLSDataCollectionToken *)token
471465
waitForCompletion:waitForSettings];
472466
});
473467
}
468+
}
474469

475-
[self processExistingReportPaths:preexistingReportPaths
476-
dataCollectionToken:token
477-
asUrgent:blockingSend];
478-
[self handleContentsInOtherReportingDirectoriesWithToken:token];
470+
- (void)beginReportUploadsWithToken:(FIRCLSDataCollectionToken *)token
471+
preexistingReportPaths:(NSArray *)preexistingReportPaths
472+
blockingSend:(BOOL)blockingSend
473+
report:(FIRCLSInternalReport *)report {
474+
if (self.settings.collectReportsEnabled) {
475+
[self processExistingReportPaths:preexistingReportPaths
476+
dataCollectionToken:token
477+
asUrgent:blockingSend];
478+
[self handleContentsInOtherReportingDirectoriesWithToken:token];
479+
480+
} else {
481+
FIRCLSInfoLog(@"Collect crash reports is disabled");
482+
[self deleteUnsentReportsWithPreexisting:preexistingReportPaths];
483+
}
479484
}
480485

481486
- (BOOL)startCrashReporterWithProfilingMark:(FIRCLSProfileMark)mark
@@ -550,14 +555,6 @@ - (FIRCLSInternalReport *)setupCurrentReport:(NSString *)executionIdentifier {
550555
executionIdentifier:executionIdentifier];
551556
}
552557

553-
- (void)removeExistingReportPaths:(NSArray *)reportPaths {
554-
[self.operationQueue addOperationWithBlock:^{
555-
for (NSString *path in reportPaths) {
556-
[self.fileManager removeItemAtPath:path];
557-
}
558-
}];
559-
}
560-
561558
- (int)countSubmittableAndDeleteUnsubmittableReportPaths:(NSArray *)reportPaths {
562559
int count = 0;
563560
for (NSString *path in reportPaths) {
@@ -621,11 +618,11 @@ - (void)submitReport:(FIRCLSInternalReport *)report
621618
[self didSubmitReport];
622619
}
623620

624-
- (void)removeReport:(FIRCLSInternalReport *)report {
625-
[_fileManager removeItemAtPath:report.path];
626-
}
621+
// This is the side-effect of calling deleteUnsentReports, or collect_reports setting
622+
// being false
623+
- (void)deleteUnsentReportsWithPreexisting:(NSArray *)preexistingReportPaths {
624+
[self removeExistingReportPaths:preexistingReportPaths];
627625

628-
- (void)removeContentsInOtherReportingDirectories {
629626
[self removeExistingReportPaths:self.fileManager.processingPathContents];
630627
if (self.settings.shouldUseNewReportEndpoint) {
631628
[self removeExistingReportPaths:self.fileManager.preparedPathContents];
@@ -634,6 +631,14 @@ - (void)removeContentsInOtherReportingDirectories {
634631
}
635632
}
636633

634+
- (void)removeExistingReportPaths:(NSArray *)reportPaths {
635+
[self.operationQueue addOperationWithBlock:^{
636+
for (NSString *path in reportPaths) {
637+
[self.fileManager removeItemAtPath:path];
638+
}
639+
}];
640+
}
641+
637642
- (void)handleContentsInOtherReportingDirectoriesWithToken:(FIRCLSDataCollectionToken *)token {
638643
[self handleExistingFilesInProcessingWithToken:token];
639644
[self handleExistingFilesInPreparedWithToken:token];

‎Crashlytics/Crashlytics/Models/FIRCLSFileManager.h

Lines changed: 0 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -59,27 +59,13 @@
5959
- (BOOL)moveItemAtPath:(NSString *)path toDirectory:(NSString *)destDir;
6060
- (void)enumerateFilesInDirectory:(NSString *)directory
6161
usingBlock:(void (^)(NSString *filePath, NSString *extension))block;
62-
- (BOOL)moveItemsFromDirectory:(NSString *)srcDir toDirectory:(NSString *)destDir;
6362
- (NSNumber *)fileSizeAtPath:(NSString *)path;
6463
- (NSArray *)contentsOfDirectory:(NSString *)path;
6564

6665
// logic of managing files/directories
6766
- (BOOL)createReportDirectories;
6867
- (NSString *)setupNewPathForExecutionIdentifier:(NSString *)identifier;
6968

70-
- (void)enumerateFilesInActiveDirectoryUsingBlock:(void (^)(NSString *path,
71-
NSString *extension))block;
72-
- (void)enumerateReportsInProcessingDirectoryUsingBlock:(void (^)(FIRCLSInternalReport *report,
73-
NSString *path))block;
74-
- (void)enumerateFilesInPreparedDirectoryUsingBlock:(void (^)(NSString *path,
75-
NSString *extension))block;
76-
77-
- (BOOL)movePendingToProcessing;
78-
79-
- (BOOL)removeContentsOfProcessingPath;
80-
- (BOOL)removeContentsOfPendingPath;
81-
- (BOOL)removeContentsOfAllPaths;
82-
8369
- (BOOL)moveItemAtPath:(NSString *)srcPath toPath:(NSString *)dstPath error:(NSError **)error;
8470

8571
- (NSData *)dataWithContentsOfFile:(NSString *)path;

‎Crashlytics/Crashlytics/Models/FIRCLSFileManager.m

Lines changed: 0 additions & 61 deletions
Original file line numberDiff line numberDiff line change
@@ -146,17 +146,6 @@ - (void)enumerateFilesInDirectory:(NSString *)directory
146146
}
147147
}
148148

149-
- (BOOL)moveItemsFromDirectory:(NSString *)srcDir toDirectory:(NSString *)destDir {
150-
__block BOOL success = YES;
151-
152-
[self enumerateFilesInDirectory:srcDir
153-
usingBlock:^(NSString *filePath, NSString *extension) {
154-
success = success && [self moveItemAtPath:filePath toDirectory:destDir];
155-
}];
156-
157-
return success;
158-
}
159-
160149
- (NSNumber *)fileSizeAtPath:(NSString *)path {
161150
NSError *error = nil;
162151
NSDictionary *attrs = [[self underlyingFileManager] attributesOfItemAtPath:path error:&error];
@@ -278,56 +267,6 @@ - (NSString *)setupNewPathForExecutionIdentifier:(NSString *)identifier {
278267
return path;
279268
}
280269

281-
- (void)enumerateReportsInProcessingDirectoryUsingBlock:(void (^)(FIRCLSInternalReport *report,
282-
NSString *path))block {
283-
[self enumerateFilesInDirectory:[self processingPath]
284-
usingBlock:^(NSString *filePath, NSString *extension) {
285-
FIRCLSInternalReport *report =
286-
[FIRCLSInternalReport reportWithPath:filePath];
287-
if (block) {
288-
block(report, filePath);
289-
}
290-
}];
291-
}
292-
293-
- (void)enumerateFilesInActiveDirectoryUsingBlock:(void (^)(NSString *path,
294-
NSString *extension))block {
295-
[self enumerateFilesInDirectory:[self activePath] usingBlock:block];
296-
}
297-
298-
- (void)enumerateFilesInPreparedDirectoryUsingBlock:(void (^)(NSString *path,
299-
NSString *extension))block {
300-
[self enumerateFilesInDirectory:[self legacyPreparedPath] usingBlock:block];
301-
}
302-
303-
- (BOOL)movePendingToProcessing {
304-
return [self moveItemsFromDirectory:[self pendingPath] toDirectory:[self processingPath]];
305-
}
306-
307-
- (BOOL)removeContentsOfProcessingPath {
308-
return [self removeContentsOfDirectoryAtPath:[self processingPath]];
309-
}
310-
311-
- (BOOL)removeContentsOfPendingPath {
312-
return [self removeContentsOfDirectoryAtPath:[self pendingPath]];
313-
}
314-
315-
- (BOOL)removeContentsOfAllPaths {
316-
BOOL contentsOfProcessingPathRemoved = [self removeContentsOfProcessingPath];
317-
BOOL contentsOfPendingPathRemoved = [self removeContentsOfPendingPath];
318-
BOOL contentsOfDirectoryAtPreparedPathRemoved =
319-
[self removeContentsOfDirectoryAtPath:self.preparedPath];
320-
BOOL contentsOfDirectoryAtLegacyPreparedPathRemoved =
321-
[self removeContentsOfDirectoryAtPath:self.legacyPreparedPath];
322-
BOOL contentsOfDirectoryAtActivePathRemoved =
323-
[self removeContentsOfDirectoryAtPath:self.activePath];
324-
BOOL success = contentsOfProcessingPathRemoved && contentsOfPendingPathRemoved &&
325-
contentsOfDirectoryAtPreparedPathRemoved &&
326-
contentsOfDirectoryAtActivePathRemoved &&
327-
contentsOfDirectoryAtLegacyPreparedPathRemoved;
328-
return success;
329-
}
330-
331270
- (BOOL)moveItemAtPath:(NSString *)srcPath toPath:(NSString *)dstPath error:(NSError **)error {
332271
return [self.underlyingFileManager moveItemAtPath:srcPath toPath:dstPath error:error];
333272
}

‎Crashlytics/Crashlytics/Models/FIRCLSSettings.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -89,7 +89,7 @@ NS_ASSUME_NONNULL_BEGIN
8989
/**
9090
* When this is false, Crashlytics will not start up
9191
*/
92-
@property(nonatomic, readonly) BOOL crashReportingEnabled;
92+
@property(nonatomic, readonly) BOOL collectReportsEnabled;
9393

9494
/**
9595
* When this is false, Crashlytics will not collect non-fatal errors and errors

‎Crashlytics/Crashlytics/Models/FIRCLSSettings.m

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -293,7 +293,7 @@ - (BOOL)customExceptionsEnabled {
293293
return [self errorReportingEnabled];
294294
}
295295

296-
- (BOOL)crashReportingEnabled {
296+
- (BOOL)collectReportsEnabled {
297297
NSNumber *value = [self featuresSettings][@"collect_reports"];
298298

299299
if (value != nil) {

‎Crashlytics/UnitTests/FIRCLSSettingsTests.m

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -97,7 +97,7 @@ - (void)testDefaultSettings {
9797
XCTAssertFalse(self.settings.appNeedsOnboarding);
9898
XCTAssertFalse(self.settings.appUpdateRequired);
9999

100-
XCTAssertTrue(self.settings.crashReportingEnabled);
100+
XCTAssertTrue(self.settings.collectReportsEnabled);
101101
XCTAssertTrue(self.settings.errorReportingEnabled);
102102
XCTAssertTrue(self.settings.customExceptionsEnabled);
103103

@@ -170,7 +170,7 @@ - (void)testActivatedSettingsCached {
170170
XCTAssertFalse(self.settings.appNeedsOnboarding);
171171
XCTAssertFalse(self.settings.appUpdateRequired);
172172

173-
XCTAssertTrue(self.settings.crashReportingEnabled);
173+
XCTAssertTrue(self.settings.collectReportsEnabled);
174174
XCTAssertTrue(self.settings.errorReportingEnabled);
175175
XCTAssertTrue(self.settings.customExceptionsEnabled);
176176

@@ -197,7 +197,7 @@ - (void)testInverseDefaultSettingsCached {
197197
XCTAssertTrue(self.settings.appNeedsOnboarding);
198198
XCTAssertTrue(self.settings.appUpdateRequired);
199199

200-
XCTAssertFalse(self.settings.crashReportingEnabled);
200+
XCTAssertFalse(self.settings.collectReportsEnabled);
201201
XCTAssertFalse(self.settings.errorReportingEnabled);
202202
XCTAssertFalse(self.settings.customExceptionsEnabled);
203203

@@ -379,7 +379,7 @@ - (void)testActivatedSettingsMissingCacheKey {
379379
XCTAssertFalse(self.settings.appNeedsOnboarding);
380380
XCTAssertFalse(self.settings.appUpdateRequired);
381381

382-
XCTAssertTrue(self.settings.crashReportingEnabled);
382+
XCTAssertTrue(self.settings.collectReportsEnabled);
383383
XCTAssertTrue(self.settings.errorReportingEnabled);
384384
XCTAssertTrue(self.settings.customExceptionsEnabled);
385385

0 commit comments

Comments
 (0)