Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Keep File Manager from Error State on disallowed #1455

Merged
merged 11 commits into from Oct 24, 2019
Merged

Keep File Manager from Error State on disallowed #1455

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
78fc9c6
making it so we dont error on a DISALLOWED
Oct 21, 2019
a2e3ac1
PR issues, updating error to add result code to user info
Oct 23, 2019
933d9b9
removed unneeded code
Oct 23, 2019
358cf67
fix formatting add warning log
Oct 23, 2019
b58eb3c
no message
Oct 23, 2019
47e9a2e
Update SmartDeviceLink/SDLFileManager.m
justingluck93 Oct 24, 2019
09e3d65
adding unit tests
Oct 24, 2019
d7afa64
pr issues, updating if to use any error instead
Oct 24, 2019
3af37d6
no message
Oct 24, 2019
e43aa8e
updated unit test
Oct 24, 2019
6e02171
Apply suggestions from code review
justingluck93 Oct 24, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
8 changes: 7 additions & 1 deletion SmartDeviceLink/SDLFileManager.m
View file
Open in desktop
Expand Up @@ -179,8 +179,14 @@ - (void)didEnterStateFetchingInitialList {
BLOCK_RETURN;
}

// If there was an error, we'll pass the error to the startup handler and cancel out
// If there was an error, we'll pass the error to the startup handler and cancel out other then in case of DISALLOWED
justingluck93 marked this conversation as resolved.
Show resolved Hide resolved
if (error != nil) {
// In the case we are DISALLOWED we still want to transition to a ready state
justingluck93 marked this conversation as resolved.
Show resolved Hide resolved
if([error.userInfo[@"resultCode"] isEqualToEnum:SDLResultDisallowed]) {
SDLLogW(@"ListFiles is disallowed. Certain file manager APIs may not work properly.");
[weakSelf.stateMachine transitionToState:SDLFileManagerStateReady];
BLOCK_RETURN;
}
[weakSelf.stateMachine transitionToState:SDLFileManagerStateStartupError];
BLOCK_RETURN;
}
Expand Down
9 changes: 8 additions & 1 deletion SmartDeviceLink/SDLListFilesOperation.m
View file
Open in desktop
Expand Up @@ -59,7 +59,14 @@ - (void)sdl_listFiles {
NSUInteger bytesAvailable = listFilesResponse.spaceAvailable != nil ? listFilesResponse.spaceAvailable.unsignedIntegerValue : 2000000000;

if (weakSelf.completionHandler != nil) {
weakSelf.completionHandler(success, bytesAvailable, fileNames, error);
if([response.resultCode isEqualToEnum:SDLResultDisallowed]) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of this, just always set the resultCode into the error if an error exists.

NSMutableDictionary *results = [error.userInfo mutableCopy];
results[@"resultCode"] = SDLResultDisallowed;
NSError *resultError = [NSError errorWithDomain:error.domain code:error.code userInfo:results];
weakSelf.completionHandler(success, bytesAvailable, fileNames, resultError);
} else {
weakSelf.completionHandler(success, bytesAvailable, fileNames, error);
}
}

[weakSelf finishOperation];
Expand Down
16 changes: 16 additions & 0 deletions SmartDeviceLinkTests/DevAPISpecs/SDLFileManagerSpec.m
View file
Open in desktop
Expand Up @@ -157,6 +157,22 @@ @implementation FileManagerSpecHelper
});
});

describe(@"after receiving a ListFiles error with a resulCode", ^{
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
describe(@"after receiving a ListFiles error with a resulCode", ^{
describe(@"after receiving a ListFiles error with a resultCode DISALLOWED", ^{

beforeEach(^{
SDLListFilesOperation *operation = testFileManager.pendingTransactions.firstObject;
NSMutableDictionary *userInfo = [[NSError sdl_fileManager_unableToStartError].userInfo mutableCopy];
userInfo[@"resultCode"] = SDLResultDisallowed;
NSError *errorWithResultCode = [NSError errorWithDomain:[NSError sdl_fileManager_unableToStartError].domain code:[NSError sdl_fileManager_unableToStartError].code userInfo:userInfo];
operation.completionHandler(NO, initialSpaceAvailable, testInitialFileNames, errorWithResultCode);
});

it(@"should handle the error properly", ^{
expect(testFileManager.currentState).to(match(SDLFileManagerStateReady));
expect(testFileManager.remoteFileNames).to(beEmpty());
expect(@(testFileManager.bytesAvailable)).to(equal(initialSpaceAvailable));
});
});

describe(@"after receiving a ListFiles response", ^{
beforeEach(^{
SDLListFilesOperation *operation = testFileManager.pendingTransactions.firstObject;
Expand Down
17 changes: 14 additions & 3 deletions SmartDeviceLinkTests/DevAPISpecs/SDLListFilesOperationSpec.m
View file
Open in desktop
Expand Up @@ -18,7 +18,7 @@
__block NSUInteger bytesAvailableResult = NO;
__block NSError *errorResult = nil;
__block NSArray<NSString *> *fileNamesResult = nil;

beforeEach(^{
testConnectionManager = [[TestConnectionManager alloc] init];
testOperation = [[SDLListFilesOperation alloc] initWithConnectionManager:testConnectionManager completionHandler:^(BOOL success, NSUInteger bytesAvailable, NSArray<NSString *> * _Nonnull fileNames, NSError * _Nullable error) {
Expand Down Expand Up @@ -92,13 +92,24 @@
badResponse.success = @NO;
badResponse.spaceAvailable = responseSpaceAvailable;
badResponse.filenames = responseFileNames;

[testConnectionManager respondToLastRequestWithResponse:badResponse error:[NSError sdl_lifecycle_unknownRemoteErrorWithDescription:responseErrorDescription andReason:responseErrorReason]];
});

it(@"should have called completion handler with error", ^{
[testConnectionManager respondToLastRequestWithResponse:badResponse error:[NSError sdl_lifecycle_unknownRemoteErrorWithDescription:responseErrorDescription andReason:responseErrorReason]];

expect(errorResult.localizedDescription).to(match(responseErrorDescription));
expect(errorResult.localizedFailureReason).to(match(responseErrorReason));
expect(@(successResult)).to(equal(@NO));
});

it(@"should have called completion handler with error including a resultCode ", ^{
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Based on the above change request, I think that this it is the only one needed?

badResponse.resultCode = SDLResultDisallowed;

[testConnectionManager respondToLastRequestWithResponse:badResponse error:[NSError sdl_lifecycle_unknownRemoteErrorWithDescription:responseErrorDescription andReason:responseErrorReason]];

expect(errorResult.localizedDescription).to(match(responseErrorDescription));
expect(errorResult.localizedFailureReason).to(match(responseErrorReason));
expect(errorResult.userInfo[@"resultCode"]).to(equal(@"DISALLOWED"));
expect(@(successResult)).to(equal(@NO));
});
});
Expand Down