diff options
author | NicoleYarroch <nicole@livio.io> | 2019-05-23 16:07:07 -0400 |
---|---|---|
committer | NicoleYarroch <nicole@livio.io> | 2019-05-23 16:07:07 -0400 |
commit | 25f8f76a73900c1a8e8640172add522ffd73e873 (patch) | |
tree | 103269c5500d7c24411793bb92f18f11492ea88b | |
parent | c080f3e0084ea502930c22749e18eb1e0f41f524 (diff) | |
download | sdl_ios-25f8f76a73900c1a8e8640172add522ffd73e873.tar.gz |
Fixed the file manager race condition
Fixed the file manager adding operations to the queue after shutdown
-rw-r--r-- | SmartDeviceLink/SDLFileManager.m | 23 | ||||
-rw-r--r-- | SmartDeviceLinkTests/DevAPISpecs/SDLFileManagerSpec.m | 31 |
2 files changed, 50 insertions, 4 deletions
diff --git a/SmartDeviceLink/SDLFileManager.m b/SmartDeviceLink/SDLFileManager.m index 911ec889f..60b6192d0 100644 --- a/SmartDeviceLink/SDLFileManager.m +++ b/SmartDeviceLink/SDLFileManager.m @@ -404,8 +404,6 @@ SDLFileManagerState *const SDLFileManagerStateStartupError = @"StartupError"; if ([self sdl_canFileBeUploadedAgain:file maxUploadCount:maxUploadCount failedFileUploadsCount:self.failedFileUploadsCount]) { SDLLogD(@"Attempting to resend file with name %@ after a failed upload attempt", file.name); return [self sdl_uploadFile:file completionHandler:handler]; - } else { - SDLLogE(@"File named %@ failed to upload. Max number of upload attempts reached", file.name); } } @@ -526,11 +524,28 @@ SDLFileManagerState *const SDLFileManagerStateStartupError = @"StartupError"; * @return True if the file still needs to be (re)sent to Core; false if not. */ - (BOOL)sdl_canFileBeUploadedAgain:(nullable SDLFile *)file maxUploadCount:(UInt8)maxUploadCount failedFileUploadsCount:(NSMutableDictionary<SDLFileName *, NSNumber<SDLUInt> *> *)failedFileUploadsCount { - if (!file || [self hasUploadedFile:file]) { + if (![self.currentState isEqualToString:SDLFileManagerStateReady]) { + SDLLogE(@"File named %@ failed to upload. The file manager has shutdown so the file will not be sent again.", file.name); + return NO; + } + + if (!file) { + SDLLogE(@"File named %@ can not be uploaded because it is not a valid file.", file.name); + return NO; + } + + if ([self hasUploadedFile:file]) { + SDLLogE(@"File named %@ has already been uploaded.", file.name); return NO; } + NSNumber *failedUploadCount = failedFileUploadsCount[file.name]; - return (failedUploadCount == nil) ? YES : (failedUploadCount.integerValue < maxUploadCount); + BOOL canFileBeUploadedAgain = (failedUploadCount == nil) ? YES : (failedUploadCount.integerValue < maxUploadCount); + if (!canFileBeUploadedAgain) { + SDLLogE(@"File named %@ failed to upload. Max number of upload attempts reached.", file.name); + } + + return canFileBeUploadedAgain; } /** diff --git a/SmartDeviceLinkTests/DevAPISpecs/SDLFileManagerSpec.m b/SmartDeviceLinkTests/DevAPISpecs/SDLFileManagerSpec.m index afbd93873..c7dd86483 100644 --- a/SmartDeviceLinkTests/DevAPISpecs/SDLFileManagerSpec.m +++ b/SmartDeviceLinkTests/DevAPISpecs/SDLFileManagerSpec.m @@ -1,5 +1,6 @@ #import <Quick/Quick.h> #import <Nimble/Nimble.h> +#import <OCMock/OCMock.h> #import "SDLDeleteFileResponse.h" #import "SDLError.h" @@ -14,6 +15,7 @@ #import "SDLPutFile.h" #import "SDLPutFileResponse.h" #import "SDLRPCResponse.h" +#import "SDLStateMachine.h" #import "TestConnectionManager.h" #import "TestMultipleFilesConnectionManager.h" #import "TestFileProgressResponse.h" @@ -31,6 +33,7 @@ SDLFileManagerState *const SDLFileManagerStateStartupError = @"StartupError"; @property (strong, nonatomic) NSMutableDictionary<SDLFileName *, NSNumber<SDLUInt> *> *failedFileUploadsCount; @property (assign, nonatomic) UInt8 maxFileUploadAttempts; @property (assign, nonatomic) UInt8 maxArtworkUploadAttempts; +@property (strong, nonatomic) SDLStateMachine *stateMachine; - (BOOL)sdl_canFileBeUploadedAgain:(nullable SDLFile *)file maxUploadCount:(int)maxRetryCount failedFileUploadsCount:(NSMutableDictionary<SDLFileName *, NSNumber<SDLUInt> *> *)failedFileUploadsCount; + (NSMutableDictionary<SDLFileName *, NSNumber<SDLUInt> *> *)sdl_incrementFailedUploadCountForFileName:(SDLFileName *)fileName failedFileUploadsCount:(NSMutableDictionary<SDLFileName *, NSNumber<SDLUInt> *> *)failedFileUploadsCount; @@ -1675,12 +1678,33 @@ describe(@"SDLFileManager reupload failed files", ^{ describe(@"the file cannot be uploaded again", ^{ it(@"should not upload a file that is nil", ^{ + // Make sure we are in the ready state + testFileManager.stateMachine = OCMClassMock([SDLStateMachine class]); + OCMStub([testFileManager.stateMachine currentState]).andReturn(SDLFileManagerStateReady); + expect([testFileManager.currentState isEqualToEnum:SDLFileManagerStateReady]).to(beTrue()); + testFile = nil; BOOL canUploadAgain = [testFileManager sdl_canFileBeUploadedAgain:testFile maxUploadCount:5 failedFileUploadsCount:testFailedFileUploadsCount]; expect(canUploadAgain).to(equal(NO)); }); it(@"should not upload a file that has already been uploaded the max number of times", ^{ + // Make sure we are in the ready state + testFileManager.stateMachine = OCMClassMock([SDLStateMachine class]); + OCMStub([testFileManager.stateMachine currentState]).andReturn(SDLFileManagerStateReady); + expect([testFileManager.currentState isEqualToEnum:SDLFileManagerStateReady]).to(beTrue()); + + testFailedFileUploadsCount[testFileName] = @4; + BOOL canUploadAgain = [testFileManager sdl_canFileBeUploadedAgain:testFile maxUploadCount:4 failedFileUploadsCount:testFailedFileUploadsCount]; + expect(canUploadAgain).to(equal(NO)); + }); + + it(@"should not upload a file if the file manager is not in state ready", ^{ + // Make sure we are NOT in the ready state + testFileManager.stateMachine = OCMClassMock([SDLStateMachine class]); + OCMStub([testFileManager.stateMachine currentState]).andReturn(SDLFileManagerStateShutdown); + expect([testFileManager.currentState isEqualToEnum:SDLFileManagerStateShutdown]).to(beTrue()); + testFailedFileUploadsCount[testFileName] = @4; BOOL canUploadAgain = [testFileManager sdl_canFileBeUploadedAgain:testFile maxUploadCount:4 failedFileUploadsCount:testFailedFileUploadsCount]; expect(canUploadAgain).to(equal(NO)); @@ -1688,6 +1712,13 @@ describe(@"SDLFileManager reupload failed files", ^{ }); describe(@"the file can be uploaded again", ^{ + beforeEach(^{ + // Make sure we are in the ready state + testFileManager.stateMachine = OCMClassMock([SDLStateMachine class]); + OCMStub([testFileManager.stateMachine currentState]).andReturn(SDLFileManagerStateReady); + expect([testFileManager.currentState isEqualToEnum:SDLFileManagerStateReady]).to(beTrue()); + }); + it(@"should upload a file that has not yet failed to upload", ^{ testFailedFileUploadsCount = [NSMutableDictionary dictionary]; BOOL canUploadAgain = [testFileManager sdl_canFileBeUploadedAgain:testFile maxUploadCount:2 failedFileUploadsCount:testFailedFileUploadsCount]; |