Working on better handling of passing back errors / state

5 files changed, 130 insertions, 29 deletions

diff --git a/SmartDeviceLink-iOS.xcodeproj/project.pbxproj b/SmartDeviceLink-iOS.xcodeproj/project.pbxproj
index 7c8a3a341..ad8a675d3 100644
--- a/SmartDeviceLink-iOS.xcodeproj/project.pbxproj
+++ b/SmartDeviceLink-iOS.xcodeproj/project.pbxproj
@@ -1385,6 +1385,7 @@
 		4ABB2BA724F850AE0061BF55 /* SDLImage.h in Headers */ = {isa = PBXBuildFile; fileRef = 4ABB2B9924F850AD0061BF55 /* SDLImage.h */; settings = {ATTRIBUTES = (Public, ); }; };
 		4ABB2BA824F850AE0061BF55 /* SDLLightState.m in Sources */ = {isa = PBXBuildFile; fileRef = 4ABB2B9A24F850AD0061BF55 /* SDLLightState.m */; };
 		4ABB2BA924F850AE0061BF55 /* SDLImageResolution.h in Headers */ = {isa = PBXBuildFile; fileRef = 4ABB2B9B24F850AD0061BF55 /* SDLImageResolution.h */; settings = {ATTRIBUTES = (Public, ); }; };
+		4AD1F1742559957100637FE1 /* SDLVoiceCommandOperationSpec.m in Sources */ = {isa = PBXBuildFile; fileRef = 4AD1F1732559957100637FE1 /* SDLVoiceCommandOperationSpec.m */; };
 		4AE8A7022537796E000666C0 /* SmartDeviceLink.h in Headers */ = {isa = PBXBuildFile; fileRef = 4AE8A7012537796E000666C0 /* SmartDeviceLink.h */; settings = {ATTRIBUTES = (Public, ); }; };
 		5D0A9F911F15550400CC80DD /* SDLSystemCapabilityTypeSpec.m in Sources */ = {isa = PBXBuildFile; fileRef = 5D0A9F901F15550400CC80DD /* SDLSystemCapabilityTypeSpec.m */; };
 		5D0A9F931F15560B00CC80DD /* SDLNavigationCapabilitySpec.m in Sources */ = {isa = PBXBuildFile; fileRef = 5D0A9F921F15560B00CC80DD /* SDLNavigationCapabilitySpec.m */; };
@@ -3170,6 +3171,7 @@
 		4ABB2B9924F850AD0061BF55 /* SDLImage.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; name = SDLImage.h; path = public/SDLImage.h; sourceTree = "<group>"; };
 		4ABB2B9A24F850AD0061BF55 /* SDLLightState.m */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.objc; name = SDLLightState.m; path = public/SDLLightState.m; sourceTree = "<group>"; };
 		4ABB2B9B24F850AD0061BF55 /* SDLImageResolution.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; name = SDLImageResolution.h; path = public/SDLImageResolution.h; sourceTree = "<group>"; };
+		4AD1F1732559957100637FE1 /* SDLVoiceCommandOperationSpec.m */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.objc; name = SDLVoiceCommandOperationSpec.m; path = DevAPISpecs/SDLVoiceCommandOperationSpec.m; sourceTree = "<group>"; };
 		4AE8A7012537796E000666C0 /* SmartDeviceLink.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; name = SmartDeviceLink.h; path = public/SmartDeviceLink.h; sourceTree = "<group>"; };
 		4AE8A707253779F9000666C0 /* EAAccessory+OCMock.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = "EAAccessory+OCMock.h"; sourceTree = "<group>"; };
 		5D0A9F901F15550400CC80DD /* SDLSystemCapabilityTypeSpec.m */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.objc; path = SDLSystemCapabilityTypeSpec.m; sourceTree = "<group>"; };
@@ -4256,6 +4258,16 @@
 			name = "Status Manager";
 			sourceTree = "<group>";
 		};
+		4AD1F16A2559952D00637FE1 /* Voice Command */ = {
+			isa = PBXGroup;
+			children = (
+				5DF40B27208FDA9700DD6FDA /* SDLVoiceCommandManagerSpec.m */,
+				5DAB5F5220989A8300A020C8 /* SDLVoiceCommandSpec.m */,
+				4AD1F1732559957100637FE1 /* SDLVoiceCommandOperationSpec.m */,
+			);
+			name = "Voice Command";
+			sourceTree = "<group>";
+		};
 		5D0218EB1A8E795700D1BF62 /* UI */ = {
 			isa = PBXGroup;
 			children = (
@@ -4478,8 +4490,8 @@
 			children = (
 				5D4019B11A76EC350006B0C2 /* Examples */,
 				5D61FA1D1A84237100846EE7 /* SmartDeviceLink */,
-				5D61FA2C1A84237100846EE7 /* SmartDeviceLinkTests */,
 				5D4346621E6F38E600B639C6 /* SmartDeviceLinkSwift */,
+				5D61FA2C1A84237100846EE7 /* SmartDeviceLinkTests */,
 				5D4019B01A76EC350006B0C2 /* Products */,
 			);
 			sourceTree = "<group>";
@@ -6121,6 +6133,7 @@
 				5DAD5F8220507DED0025624C /* Soft Button */,
 				88D0E5D42478656B009469AB /* Subscribe Button */,
 				5DAD5F8320507DF30025624C /* Text and Graphic */,
+				4AD1F16A2559952D00637FE1 /* Voice Command */,
 				5DAD5F8420507E1F0025624C /* SDLScreenManagerSpec.m */,
 			);
 			name = Screen;
@@ -6481,9 +6494,7 @@
 			isa = PBXGroup;
 			children = (
 				5DF40B25208FA7DE00DD6FDA /* SDLMenuManagerSpec.m */,
-				5DF40B27208FDA9700DD6FDA /* SDLVoiceCommandManagerSpec.m */,
 				5DAB5F502098994C00A020C8 /* SDLMenuCellSpec.m */,
-				5DAB5F5220989A8300A020C8 /* SDLVoiceCommandSpec.m */,
 				752ECDB8228C42E100D945F4 /* SDLMenuRunScoreSpec.m */,
 				752ECDBA228C532600D945F4 /* SDLMenuUpdateAlgorithmSpec.m */,
 				5D76751522D920FD00E8D71A /* SDLMenuConfigurationSpec.m */,
@@ -8736,6 +8747,7 @@
 				162E831E1A9BDE8B00906325 /* SDLOnTBTClientStateSpec.m in Sources */,
 				162E83351A9BDE8B00906325 /* SDLReadDIDSpec.m in Sources */,
 				5DF40B28208FDA9700DD6FDA /* SDLVoiceCommandManagerSpec.m in Sources */,
+				4AD1F1742559957100637FE1 /* SDLVoiceCommandOperationSpec.m in Sources */,
 				88B3BFA020DA8FD000943565 /* SDLFuelTypeSpec.m in Sources */,
 				162E836F1A9BDE8B00906325 /* SDLUnsubscribeVehicleDataResponseSpec.m in Sources */,
 				162E82DB1A9BDE8B00906325 /* SDLECallConfirmationStatusSpec.m in Sources */,
diff --git a/SmartDeviceLink/SDLVoiceCommandUpdateOperation.h b/SmartDeviceLink/SDLVoiceCommandUpdateOperation.h
index aa0fd1ced..b9b09f3db 100644
--- a/SmartDeviceLink/SDLVoiceCommandUpdateOperation.h
+++ b/SmartDeviceLink/SDLVoiceCommandUpdateOperation.h
@@ -14,7 +14,7 @@
 
 NS_ASSUME_NONNULL_BEGIN
 
-typedef void(^SDLVoiceCommandUpdateCompletionHandler)(NSError *__nullable error);
+typedef void(^SDLVoiceCommandUpdateCompletionHandler)(NSArray<SDLVoiceCommand *> *newCurrentVoiceCommands, NSError *__nullable error);
 
 @interface SDLVoiceCommandUpdateOperation : SDLAsynchronousOperation
 
diff --git a/SmartDeviceLink/SDLVoiceCommandUpdateOperation.m b/SmartDeviceLink/SDLVoiceCommandUpdateOperation.m
index 766b71206..c13105e5b 100644
--- a/SmartDeviceLink/SDLVoiceCommandUpdateOperation.m
+++ b/SmartDeviceLink/SDLVoiceCommandUpdateOperation.m
@@ -25,8 +25,7 @@ NS_ASSUME_NONNULL_BEGIN
 
 @property (weak, nonatomic) id<SDLConnectionManagerType> connectionManager;
 @property (strong, nonatomic, nullable) NSArray<SDLVoiceCommand *> *updatedVoiceCommands;
-@property (strong, nonatomic, nullable) NSArray<SDLVoiceCommand *> *oldVoiceCommands;
-@property (assign, nonatomic) UInt32 firstNewVoiceCommandId;
+@property (strong, nonatomic, nullable) NSArray<SDLVoiceCommand *> *currentVoiceCommands;
 @property (copy, nonatomic) SDLVoiceCommandUpdateCompletionHandler completionHandler;
 
 @property (copy, nonatomic, nullable) NSError *internalError;
@@ -41,7 +40,7 @@ NS_ASSUME_NONNULL_BEGIN
 
     _connectionManager = connectionManager;
     _updatedVoiceCommands = newVoiceCommands;
-    _oldVoiceCommands = oldVoiceCommands;
+    _currentVoiceCommands = oldVoiceCommands;
     _completionHandler = completionHandler;
 
     return self;
@@ -52,13 +51,11 @@ NS_ASSUME_NONNULL_BEGIN
     if (self.isCancelled) { return; }
 
     __weak typeof(self) weakSelf = self;
-    [self sdl_sendDeleteCurrentVoiceCommands:^(NSError * _Nullable error) {
+    [self sdl_sendDeleteCurrentVoiceCommands:^(NSArray<SDLVoiceCommand *> *currentVoiceCommands, NSError * _Nullable error) {
         // If any of the deletions failed, don't send the new commands
         if (error != nil) {
-            SDLLogD(@"Some voice commands failed to delete; aborting operation");
+            SDLLogE(@"Some voice commands failed to delete; going to attempt to set new voice commands");
             weakSelf.internalError = error;
-            [weakSelf finishOperation];
-
             return;
         }
 
@@ -68,7 +65,8 @@ NS_ASSUME_NONNULL_BEGIN
         }
 
         // If no error, send the new commands
-        [weakSelf sdl_sendCurrentVoiceCommands:^(NSError * _Nullable error) {
+        [weakSelf sdl_sendCurrentVoiceCommands:^(NSArray<SDLVoiceCommand *> *currentVoiceCommands, NSError * _Nullable error) {
+            SDLLogE(@"Some voice commands failed to send; aborting operation");
             if (weakSelf.completionHandler != nil) {
                 weakSelf.completionHandler(error);
                 [weakSelf finishOperation];
@@ -79,33 +77,41 @@ NS_ASSUME_NONNULL_BEGIN
 
 #pragma mark - Sending RPCs
 
-- (void)sdl_sendDeleteCurrentVoiceCommands:(void(^)(NSError * _Nullable))completionHandler {
-    if (self.oldVoiceCommands.count == 0) {
+- (void)sdl_sendDeleteCurrentVoiceCommands:(SDLVoiceCommandUpdateCompletionHandler)completionHandler {
+    if (self.currentVoiceCommands.count == 0) {
         SDLLogD(@"No voice commands to delete");
-        return completionHandler(nil);
+        return completionHandler(self.currentVoiceCommands, nil);
     }
 
-    NSArray<SDLRPCRequest *> *deleteVoiceCommands = [self sdl_deleteCommandsForVoiceCommands:self.oldVoiceCommands];
-
+    NSArray<SDLRPCRequest *> *deleteVoiceCommands = [self sdl_deleteCommandsForVoiceCommands:self.currentVoiceCommands];
     __block NSMutableDictionary<SDLRPCRequest *, NSError *> *errors = [NSMutableDictionary dictionary];
     __weak typeof(self) weakSelf = self;
     [self.connectionManager sendRequests:deleteVoiceCommands progressHandler:^(__kindof SDLRPCRequest * _Nonnull request, __kindof SDLRPCResponse * _Nullable response, NSError * _Nullable error, float percentComplete) {
         if (error != nil) {
-            errors[request] = error;
+            firstRunErrors[request] = error;
         }
+
+        SDLLogV(@"Deleting voice commands progress: %f", percentComplete);
     } completionHandler:^(BOOL success) {
         if (!success) {
             SDLLogE(@"Error deleting old voice commands: %@", errors);
-            return completionHandler([NSError sdl_menuManager_failedToUpdateWithDictionary:errors]);
-        }
 
-        weakSelf.oldVoiceCommands = @[];
-        SDLLogD(@"Finished deleting old voice commands");
-        return completionHandler(nil);
+            // Not all the voice command deletes succeeded, subtract the ones that did from the voice commands current state
+            NSArray<SDLDeleteCommand *> *deletedCommands = [self.class sdl_deleteCommands:deleteVoiceCommands subtractDeleteCommands:errors.allKeys];
+            [self sdl_voiceCommandsSubtractDeleteCommands:deletedCommands];
+
+            return completionHandler(nil);
+        } else {
+            SDLLogD(@"Finished deleting old voice commands");
+
+            // All the voice command deletes succeeded, subtract them from the voice commands current state
+            [self sdl_voiceCommandsSubtractDeleteCommands:deleteVoiceCommands];
+            return completionHandler(nil);
+        }
     }];
 }
 
-- (void)sdl_sendCurrentVoiceCommands:(void(^)(NSError * _Nullable))completionHandler {
+- (void)sdl_sendCurrentVoiceCommands:(void(^)(SDLVoiceCommandUpdateCompletionHandler))completionHandler {
     if (self.updatedVoiceCommands.count == 0) {
         SDLLogD(@"No voice commands to send");
         return completionHandler(nil);
@@ -119,13 +125,15 @@ NS_ASSUME_NONNULL_BEGIN
         if (error != nil) {
             errors[request] = error;
         }
+
+        SDLLogV(@"Sending voice commands progress: %f", percentComplete);
     } completionHandler:^(BOOL success) {
         if (!success) {
             SDLLogE(@"Failed to send main menu commands: %@", errors);
             return completionHandler([NSError sdl_menuManager_failedToUpdateWithDictionary:errors]);
         }
 
-        weakSelf.oldVoiceCommands = weakSelf.updatedVoiceCommands;
+        weakSelf.currentVoiceCommands = weakSelf.updatedVoiceCommands; // TODO: This needs to happen if some succeed
         SDLLogD(@"Finished updating voice commands");
         return completionHandler(nil);
     }];
@@ -163,13 +171,38 @@ NS_ASSUME_NONNULL_BEGIN
     return command;
 }
 
+#pragma mark - Managing list of commands on head unit
+
+- (void)sdl_voiceCommandsSubtractDeleteCommands:(NSArray<SDLDeleteCommand *> *)deleteCommands {
+    NSMutableArray<SDLVoiceCommand *> *mutableOldVoiceCommands = [self.currentVoiceCommands mutableCopy];
+    for (SDLDeleteCommand *deleteCommand in deleteCommands) {
+        for (SDLVoiceCommand *voiceCommand in mutableOldVoiceCommands) {
+            if (voiceCommand.commandId == deleteCommand.cmdID.unsignedIntValue) {
+                [mutableOldVoiceCommands removeObject:voiceCommand];
+                break;
+            }
+        }
+    }
+}
+
++ (NSArray<SDLDeleteCommand *> *)sdl_deleteCommands:(NSArray<SDLDeleteCommand *> *)deleteCommands subtractDeleteCommands:(NSArray<SDLDeleteCommand *> *)subtractCommands {
+    NSMutableArray<SDLDeleteCommand *> *mutableDeleteCommands = deleteCommands.mutableCopy;
+    [mutableDeleteCommands removeObjectsInArray:subtractCommands];
+
+    return [mutableDeleteCommands copy];
+}
+
+- (void)sdl_voiceCommandsAddAddCommands:(NSArray<SDLAddCommand *> *)addCommands {
+
+}
+
 #pragma mark - Operation Overrides
 
 - (void)finishOperation {
     SDLLogV(@"Finishing text and graphic update operation");
     if (self.isCancelled) {
         self.internalError = [NSError sdl_voiceCommandManager_pendingUpdateSuperseded];
-        self.completionHandler(self.error);
+        self.completionHandler(self.currentVoiceCommands, self.error);
     }
 
     [super finishOperation];
diff --git a/SmartDeviceLink/private/SDLVoiceCommandManager.m b/SmartDeviceLink/private/SDLVoiceCommandManager.m
index 073a972db..0025f9a09 100644
--- a/SmartDeviceLink/private/SDLVoiceCommandManager.m
+++ b/SmartDeviceLink/private/SDLVoiceCommandManager.m
@@ -110,19 +110,21 @@ UInt32 const VoiceCommandIdMin = 1900000000;
         return;
     }
 
+    // TODO: Cancel existing operations
+
     // Set the ids
-    self.lastVoiceCommandId = VoiceCommandIdMin;
+    self.lastVoiceCommandId = VoiceCommandIdMin; // TODO: This looks very wrong
     [self sdl_updateIdsOnVoiceCommands:voiceCommands];
 
     _voiceCommands = voiceCommands;
 
     __weak typeof(self) weakSelf = self;
-    SDLVoiceCommandUpdateOperation *updateOperation = [[SDLVoiceCommandUpdateOperation alloc] initWithConnectionManager:self.connectionManager newVoiceCommands:voiceCommands oldVoiceCommands:_currentVoiceCommands updateCompletionHandler:^(NSError * _Nullable error) {
+    SDLVoiceCommandUpdateOperation *updateOperation = [[SDLVoiceCommandUpdateOperation alloc] initWithConnectionManager:self.connectionManager newVoiceCommands:voiceCommands oldVoiceCommands:_currentVoiceCommands updateCompletionHandler:^(NSArray<SDLVoiceCommand *> *newCurrentVoiceCommands, NSError * _Nullable error) {
         if (error != nil) {
             return;
         }
 
-        weakSelf.currentVoiceCommands = voiceCommands;
+        weakSelf.currentVoiceCommands = newCurrentVoiceCommands; // TODO: This won't get set if only some succeed
     }];
 
     [self.transactionQueue addOperation:updateOperation];
diff --git a/SmartDeviceLinkTests/DevAPISpecs/SDLVoiceCommandOperationSpec.m b/SmartDeviceLinkTests/DevAPISpecs/SDLVoiceCommandOperationSpec.m
new file mode 100644
index 000000000..e8fe9bd4a
--- /dev/null
+++ b/SmartDeviceLinkTests/DevAPISpecs/SDLVoiceCommandOperationSpec.m
@@ -0,0 +1,54 @@
+#import <Quick/Quick.h>
+#import <Nimble/Nimble.h>
+#import <OCMock/OCMock.h>
+
+#import "SDLAddCommand.h"
+#import "SDLAddCommandResponse.h"
+#import "SDLDeleteCommand.h"
+#import "SDLFileManager.h"
+#import "SDLHMILevel.h"
+#import "SDLVoiceCommand.h"
+#import "SDLVoiceCommandManager.h"
+#import "TestConnectionManager.h"
+
+@interface SDLVoiceCommand()
+
+@property (assign, nonatomic) UInt32 commandId;
+
+@end
+
+QuickSpecBegin(SDLVoiceCommandOperationSpec)
+
+describe(@"a voice command operation", ^{
+    it(@"should have a priority of 'normal'", ^{
+
+    });
+
+    describe(@"initializing the operation", ^{
+
+    });
+
+    describe(@"starting the operation", ^{
+        context(@"if it starts cancelled", ^{
+            it(@"should return immediately with an error", ^{
+
+            });
+        });
+
+        context(@"if it has voice commands to delete", ^{
+            context(@"and the delete succeeds", ^{
+                <#code#>
+            });
+
+            context(@"and the delete fails", ^{
+                <#code#>
+            });
+        });
+
+        context(@"if it doesn't have any voice commands to delete", ^{
+            <#code#>
+        });
+    });
+});
+
+QuickSpecEnd