Add FIRCLSReportUploader unit tests for supporting multiple endpoints (#4978)

jasonhu-g · web-flow · commit 10a7766436b6 · 2020-02-27T10:50:18.000-05:00
diff --git a/Crashlytics/Crashlytics/Controllers/FIRCLSReportUploader.m b/Crashlytics/Crashlytics/Controllers/FIRCLSReportUploader.m
@@ -114,12 +114,10 @@ - (BOOL)prepareAndSubmitReport:(FIRCLSInternalReport *)report
 
         if (self.dataSource.settings.shouldUseNewReportEndpoint) {
           // For the new endpoint, just move the .clsrecords from "processing" -> "prepared"
-          packagedPath =
-              [self.fileManager.preparedPath stringByAppendingPathComponent:report.directoryName];
-          NSError *moveError = nil;
-          [self.fileManager moveItemAtPath:report.path toPath:packagedPath error:&moveError];
-          if (moveError) {
+          if (![self.fileManager moveItemAtPath:report.path
+                                    toDirectory:self.fileManager.preparedPath]) {
             FIRCLSErrorLog(@"Unable to move report to prepared");
+            return;
           }
         } else {
           // For the legacy endpoint, continue generate the multipartmime file in "prepared-legacy"
@@ -203,6 +201,11 @@ - (BOOL)uploadPackagedReportAtPath:(NSString *)path
   BOOL isNewPreparedPath = ![path containsString:self.fileManager.legacyPreparedPath];
 
   if (isNewPreparedPath && self.dataSource.settings.shouldUseNewReportEndpoint) {
+    if (![dataCollectionToken isValid]) {
+      FIRCLSErrorLog(@"A report upload was requested with an invalid data collection token.");
+      return NO;
+    }
+
     FIRCLSReportAdapter *adapter =
         [[FIRCLSReportAdapter alloc] initWithPath:path
                                       googleAppId:self.dataSource.googleAppID
diff --git a/Crashlytics/UnitTests/FIRCLSReportUploaderTests.m b/Crashlytics/UnitTests/FIRCLSReportUploaderTests.m
@@ -19,8 +19,12 @@
 #import "FABMockApplicationIdentifierModel.h"
 #import "FIRCLSApplication.h"
 #include "FIRCLSConstants.h"
+#include "FIRCLSDataCollectionToken.h"
 #include "FIRCLSDefines.h"
 #include "FIRCLSFileManager.h"
+#include "FIRCLSInternalReport.h"
+#include "FIRCLSMockFileManager.h"
+#include "FIRCLSMockNetworkClient.h"
 #include "FIRCLSMockSettings.h"
 #include "FIRCLSSettings.h"
 #include "FIRMockGDTCoreTransport.h"
@@ -31,9 +35,13 @@ @interface FIRCLSReportUploaderTests
     : XCTestCase <FIRCLSReportUploaderDelegate, FIRCLSReportUploaderDataSource>
 
 @property(nonatomic, strong) FIRCLSReportUploader *uploader;
-@property(nonatomic, strong) FIRCLSFileManager *fileManager;
+@property(nonatomic, strong) FIRCLSMockFileManager *fileManager;
 @property(nonatomic, strong) NSOperationQueue *queue;
-@property(nonatomic, strong) NSURL *url;
+@property(nonatomic, strong) FIRCLSMockNetworkClient *networkClient;
+
+// Add mock prefix to names as there are naming conflicts with FIRCLSReportUploaderDelegate
+@property(nonatomic, strong) FIRMockGDTCORTransport *mockDataTransport;
+@property(nonatomic, strong) FIRCLSMockSettings *mockSettings;
 
 @end
 
@@ -42,21 +50,24 @@ @implementation FIRCLSReportUploaderTests
 - (void)setUp {
   [super setUp];
 
+  FABMockApplicationIdentifierModel *appIDModel = [[FABMockApplicationIdentifierModel alloc] init];
   self.queue = [NSOperationQueue new];
-
-  self.fileManager = [[FIRCLSFileManager alloc] init];
+  self.mockSettings = [[FIRCLSMockSettings alloc] initWithFileManager:self.fileManager
+                                                           appIDModel:appIDModel];
+  self.mockSettings.fetchedBundleID = self.bundleIdentifier;
+  self.networkClient = [[FIRCLSMockNetworkClient alloc] initWithQueue:self.queue
+                                                          fileManager:self.fileManager
+                                                             delegate:nil];
+  self.fileManager = [[FIRCLSMockFileManager alloc] init];
   self.uploader = [[FIRCLSReportUploader alloc] initWithQueue:self.queue
                                                      delegate:self
                                                    dataSource:self
-                                                       client:nil
-                                                  fileManager:nil
+                                                       client:self.networkClient
+                                                  fileManager:self.fileManager
                                                     analytics:nil];
-
-  // glue together a string that will work for both platforms
-  NSString *urlString =
-      [NSString stringWithFormat:@"%@/sdk-api/v1/platforms/%@/apps/%@/reports", TestEndpoint,
-                                 FIRCLSApplicationGetPlatform(), self.bundleIdentifier];
-  self.url = [NSURL URLWithString:urlString];
+  self.mockDataTransport = [[FIRMockGDTCORTransport alloc] initWithMappingID:@"mappingID"
+                                                                transformers:nil
+                                                                      target:1206];
 }
 
 - (void)tearDown {
@@ -65,8 +76,153 @@ - (void)tearDown {
   [super tearDown];
 }
 
+#pragma mark - Tests
+
 - (void)testURLGeneration {
-  XCTAssertEqualObjects([self.uploader reportURL], _url);
+  NSString *urlString =
+      [NSString stringWithFormat:@"%@/sdk-api/v1/platforms/%@/apps/%@/reports", TestEndpoint,
+                                 FIRCLSApplicationGetPlatform(), self.bundleIdentifier];
+  NSURL *url = [NSURL URLWithString:urlString];
+
+  XCTAssertEqualObjects([self.uploader reportURL], url);
+}
+
+- (void)testPrepareReport {
+  NSString *path = [self.fileManager.activePath stringByAppendingPathComponent:@"pkg_uuid"];
+  FIRCLSInternalReport *report = [[FIRCLSInternalReport alloc] initWithPath:path];
+  self.mockSettings.orgID = @"orgID";
+  self.mockSettings.shouldUseNewReportEndpoint = YES;
+  self.fileManager.moveItemAtPathResult = [NSNumber numberWithInt:1];
+
+  [self.uploader prepareAndSubmitReport:report
+                    dataCollectionToken:FIRCLSDataCollectionToken.validToken
+                               asUrgent:YES
+                         withProcessing:YES];
+
+  // Verify with the last move operation is from processing -> prepared
+  XCTAssertTrue(
+      [self.fileManager.moveItemAtPath_destDir containsString:self.fileManager.preparedPath]);
+}
+
+- (void)testPrepareLegacyReport {
+  NSString *path = [self.fileManager.activePath stringByAppendingPathComponent:@"pkg_uuid"];
+  FIRCLSInternalReport *report = [[FIRCLSInternalReport alloc] initWithPath:path];
+  self.mockSettings.orgID = @"orgID";
+  self.mockSettings.shouldUseNewReportEndpoint = NO;
+  self.fileManager.moveItemAtPathResult = [NSNumber numberWithInt:1];
+
+  [self.uploader prepareAndSubmitReport:report
+                    dataCollectionToken:FIRCLSDataCollectionToken.validToken
+                               asUrgent:YES
+                         withProcessing:YES];
+
+  // Verify with the last move operation is from active -> processing for the legacy workflow
+  // FIRCLSPackageReportOperation will then move the report from processing -> prepared-legacy
+  XCTAssertTrue(
+      [self.fileManager.moveItemAtPath_destDir containsString:self.fileManager.processingPath]);
+}
+
+- (void)testUploadPackagedReportWithPath {
+  [self runUploadPackagedReportWithUrgency:NO];
+}
+
+- (void)testUploadPackagedReportWithLegacyPath {
+  [self runUploadPackagedReportLegacyWithUrgency:NO];
+}
+
+- (void)testUrgentUploadPackagedReportWithPath {
+  [self runUploadPackagedReportWithUrgency:YES];
+}
+
+- (void)testUrgentUploadPackagedReportWithLegacyPath {
+  [self runUploadPackagedReportLegacyWithUrgency:YES];
+}
+
+- (void)testUploadPackagedReportWithMismatchPathAndSettings {
+  [self setUpForLegacyUpload];
+
+  BOOL success = [self.uploader uploadPackagedReportAtPath:[self packagePath]
+                                       dataCollectionToken:FIRCLSDataCollectionToken.validToken
+                                                  asUrgent:NO];
+  XCTAssertFalse(success);
+  XCTAssertNil(self.mockDataTransport.sendDataEvent_event);
+  XCTAssertNil(self.networkClient.startUploadRequest);
+}
+
+- (void)testUploadPackagedReportWithoutDataCollectionToken {
+  [self setUpForUpload];
+
+  BOOL success = [self.uploader uploadPackagedReportAtPath:[self packagePath]
+                                       dataCollectionToken:nil
+                                                  asUrgent:NO];
+  XCTAssertFalse(success);
+  XCTAssertNil(self.mockDataTransport.sendDataEvent_event);
+  XCTAssertNil(self.networkClient.startUploadRequest);
+}
+
+- (void)testUploadPackagedReportNotGDTWritten {
+  [self setUpForUpload];
+  self.mockDataTransport.sendDataEvent_wasWritten = NO;
+
+  [self.uploader uploadPackagedReportAtPath:[self packagePath] dataCollectionToken:nil asUrgent:NO];
+
+  // Did not delete report
+  XCTAssertNil(self.fileManager.removedItemAtPath_path);
+}
+
+- (void)testUploadPackagedReportGDTError {
+  [self setUpForUpload];
+  self.mockDataTransport.sendDataEvent_error = [[NSError alloc] initWithDomain:@"domain"
+                                                                          code:1
+                                                                      userInfo:nil];
+
+  [self.uploader uploadPackagedReportAtPath:[self packagePath] dataCollectionToken:nil asUrgent:NO];
+
+  // Did not delete report
+  XCTAssertNil(self.fileManager.removedItemAtPath_path);
+}
+
+#pragma mark - Helper functions
+
+- (NSString *)packagePath {
+  return [self.fileManager.preparedPath stringByAppendingPathComponent:@"pkg_uuid"];
+}
+
+- (void)runUploadPackagedReportWithUrgency:(BOOL)urgent {
+  [self setUpForUpload];
+
+  BOOL success = [self.uploader uploadPackagedReportAtPath:[self packagePath]
+                                       dataCollectionToken:FIRCLSDataCollectionToken.validToken
+                                                  asUrgent:urgent];
+  XCTAssertTrue(success);
+  XCTAssertNotNil(self.mockDataTransport.sendDataEvent_event);
+  XCTAssertNil(self.networkClient.startUploadRequest);
+  XCTAssertEqualObjects(self.fileManager.removedItemAtPath_path, [self packagePath]);
+}
+
+- (void)runUploadPackagedReportLegacyWithUrgency:(BOOL)urgent {
+  NSString *packagePath =
+      [self.fileManager.legacyPreparedPath stringByAppendingPathComponent:@"pkg_uuid"];
+
+  [self setUpForLegacyUpload];
+
+  BOOL success = [self.uploader uploadPackagedReportAtPath:packagePath
+                                       dataCollectionToken:FIRCLSDataCollectionToken.validToken
+                                                  asUrgent:urgent];
+  XCTAssertTrue(success);
+  XCTAssertNil(self.mockDataTransport.sendDataEvent_event);
+  XCTAssertNotNil(self.networkClient.startUploadRequest);
+}
+
+- (void)setUpForUpload {
+  self.mockSettings.shouldUseNewReportEndpoint = YES;
+  self.mockDataTransport.sendDataEvent_wasWritten = YES;
+}
+
+- (void)setUpForLegacyUpload {
+  self.mockSettings.shouldUseNewReportEndpoint = NO;
+  self.mockDataTransport.sendDataEvent_wasWritten = YES;
+  self.fileManager.fileSizeAtPathResult = [NSNumber numberWithInt:1];
 }
 
 #pragma mark - FIRCLSReportUploaderDelegate
@@ -76,6 +232,8 @@ - (void)didCompletePackageSubmission:(NSString *)path
                                error:(NSError *)error {
 }
 
+#pragma mark - FIRCLSReportUploaderDataSource
+
 - (NSString *)bundleIdentifier {
   return @"com.test.TestApp";
 }
@@ -84,16 +242,12 @@ - (NSString *)googleAppID {
   return @"someGoogleAppId";
 }
 
-- (FIRCLSSettings *)settings {
-  FABMockApplicationIdentifierModel *appIDModel = [[FABMockApplicationIdentifierModel alloc] init];
-  FIRCLSMockSettings *settings = [[FIRCLSMockSettings alloc] initWithFileManager:self.fileManager
-                                                                      appIDModel:appIDModel];
-  settings.fetchedBundleID = self.bundleIdentifier;
-  return settings;
+- (GDTCORTransport *)googleTransport {
+  return self.mockDataTransport;
 }
 
-- (GDTCORTransport *)googleTransport {
-  return [[FIRMockGDTCORTransport alloc] initWithMappingID:@"mappingID" transformers:nil target:0];
+- (FIRCLSSettings *)settings {
+  return self.mockSettings;
 }
 
 - (void)didCompleteAllSubmissions {
diff --git a/Crashlytics/UnitTests/Mocks/FIRCLSMockFileManager.h b/Crashlytics/UnitTests/Mocks/FIRCLSMockFileManager.h
@@ -26,13 +26,25 @@
 // Incremented when a remove happens with removeItemAtPath
 @property(nonatomic) NSInteger removeCount;
 
+// Overrides fileSizeAtPath if set
+@property(nonatomic, copy) NSNumber *fileSizeAtPathResult;
+
 // Will be fulfilled when the expected number of removes have happened
 // using removeItemAtPath
 //
 // Users should initialize this in their test.
 @property(nonatomic, strong) XCTestExpectation *removeExpectation;
 
+@property(nonatomic, copy) NSString *removedItemAtPath_path;
+
 // Overriding the method for testing Settings
 - (BOOL)removeItemAtPath:(NSString *)path;
 
+// Overrides moveItemAtPath if set
+@property(nonatomic) NSNumber *moveItemAtPathResult;
+@property(nonatomic, copy) NSString *moveItemAtPath_path;
+@property(nonatomic, copy) NSString *moveItemAtPath_destDir;
+
+- (BOOL)moveItemAtPath:(NSString *)path toDirectory:(NSString *)destDir;
+
 @end
diff --git a/Crashlytics/UnitTests/Mocks/FIRCLSMockFileManager.m b/Crashlytics/UnitTests/Mocks/FIRCLSMockFileManager.m
@@ -31,6 +31,8 @@ - (instancetype)init {
 }
 
 - (BOOL)removeItemAtPath:(NSString *)path {
+  self.removedItemAtPath_path = path;
+
   [super removeItemAtPath:path];
 
   self.removeCount += 1;
@@ -44,4 +46,23 @@ - (BOOL)removeItemAtPath:(NSString *)path {
   return YES;
 }
 
+- (NSNumber *)fileSizeAtPath:(NSString *)path {
+  if (self.fileSizeAtPathResult) {
+    return self.fileSizeAtPathResult;
+  }
+
+  return [super fileSizeAtPath:path];
+}
+
+- (BOOL)moveItemAtPath:(NSString *)path toDirectory:(NSString *)destDir {
+  self.moveItemAtPath_path = path;
+  self.moveItemAtPath_destDir = destDir;
+
+  if (self.moveItemAtPathResult) {
+    return self.moveItemAtPathResult.intValue > 0;
+  }
+
+  return [super moveItemAtPath:path toDirectory:destDir];
+}
+
 @end
diff --git a/Crashlytics/UnitTests/Mocks/FIRCLSMockNetworkClient.h b/Crashlytics/UnitTests/Mocks/FIRCLSMockNetworkClient.h
@@ -16,4 +16,6 @@
 
 @interface FIRCLSMockNetworkClient : FIRCLSNetworkClient
 
+@property(nonatomic, strong) NSURLRequest *startUploadRequest;
+
 @end
diff --git a/Crashlytics/UnitTests/Mocks/FIRCLSMockNetworkClient.m b/Crashlytics/UnitTests/Mocks/FIRCLSMockNetworkClient.m
@@ -18,7 +18,9 @@ @implementation FIRCLSMockNetworkClient
 
 - (void)startUploadRequest:(NSURLRequest *)request
                   filePath:(NSString *)path
+       dataCollectionToken:(FIRCLSDataCollectionToken *)dataCollectionToken
                immediately:(BOOL)immediate {
+  self.startUploadRequest = request;
   NSLog(@"intercepted request");
 }
 
diff --git a/Crashlytics/UnitTests/Mocks/FIRCLSMockSettings.h b/Crashlytics/UnitTests/Mocks/FIRCLSMockSettings.h
@@ -18,5 +18,6 @@
 
 @property(nonatomic, copy) NSString* orgID;
 @property(nonatomic, copy) NSString* fetchedBundleID;
+@property(nonatomic) BOOL shouldUseNewReportEndpoint;
 
 @end
diff --git a/Crashlytics/UnitTests/Mocks/FIRCLSMockSettings.m b/Crashlytics/UnitTests/Mocks/FIRCLSMockSettings.m
@@ -20,5 +20,6 @@ @implementation FIRCLSMockSettings
 // and bypass the normal functionality
 @synthesize orgID;
 @synthesize fetchedBundleID;
+@synthesize shouldUseNewReportEndpoint;
 
 @end
diff --git a/Crashlytics/UnitTests/Mocks/FIRMockGDTCoreTransport.h b/Crashlytics/UnitTests/Mocks/FIRMockGDTCoreTransport.h
@@ -16,9 +16,18 @@
 
 @interface FIRMockGDTCORTransport : GDTCORTransport
 
-- (nullable instancetype)initWithMappingID:(NSString *_Nonnull)mappingID
-                              transformers:
-                                  (nullable NSArray<id<GDTCOREventTransformer>> *)transformers
-                                    target:(NSInteger)target NS_DESIGNATED_INITIALIZER;
+@property(nonatomic, copy) NSString *mappingID;
+@property(nonatomic) NSInteger target;
+
+@property(nonatomic, strong) GDTCOREvent *sendDataEvent_event;
+@property(nonatomic, strong) NSError *sendDataEvent_error;
+@property(nonatomic) BOOL sendDataEvent_wasWritten;
+
+- (instancetype)initWithMappingID:(NSString *)mappingID
+                     transformers:(NSArray<id<GDTCOREventTransformer>> *)transformers
+                           target:(NSInteger)target NS_DESIGNATED_INITIALIZER;
+
+- (void)sendDataEvent:(GDTCOREvent *)event
+           onComplete:(void (^)(BOOL wasWritten, NSError *error))completion;
 
 @end
diff --git a/Crashlytics/UnitTests/Mocks/FIRMockGDTCoreTransport.m b/Crashlytics/UnitTests/Mocks/FIRMockGDTCoreTransport.m

Original file line number	Diff line number	Diff line change
`@@ -18,7 +18,9 @@ @implementation FIRCLSMockNetworkClient`
`18`	`18`
`19`	`19`	`- (void)startUploadRequest:(NSURLRequest *)request`
`20`	`20`	`filePath:(NSString *)path`
	`21`	`+ dataCollectionToken:(FIRCLSDataCollectionToken *)dataCollectionToken`
`21`	`22`	`immediately:(BOOL)immediate {`
	`23`	`+ self.startUploadRequest = request;`
`22`	`24`	`NSLog(@"intercepted request");`
`23`	`25`	`}`
`24`	`26`