From fcc2f71b91061182fdcd8bec67253dba138e4b3c Mon Sep 17 00:00:00 2001
From: Frank Elias <francois.elias@livio.io>
Date: Wed, 7 Apr 2021 16:28:35 -0400
Subject: Comments review

---
 SmartDeviceLink/private/SDLVoiceCommandManager.m     | 20 ++++++++++++++++++++
 .../private/SDLVoiceCommandUpdateOperation.m         | 15 ---------------
 .../DevAPISpecs/SDLVoiceCommandManagerSpec.m         | 16 ++++++++++++++++
 3 files changed, 36 insertions(+), 15 deletions(-)

diff --git a/SmartDeviceLink/private/SDLVoiceCommandManager.m b/SmartDeviceLink/private/SDLVoiceCommandManager.m
index fc0048736..258653ca2 100644
--- a/SmartDeviceLink/private/SDLVoiceCommandManager.m
+++ b/SmartDeviceLink/private/SDLVoiceCommandManager.m
@@ -117,6 +117,10 @@ UInt32 const VoiceCommandIdMin = 1900000000;
 
     // Create the operation, cancel previous ones and set this one
     __weak typeof(self) weakSelf = self;
+    if (![weakSelf sdl_arePendingVoiceCommandsUnique:voiceCommands]) {
+        return;
+    }
+
     SDLVoiceCommandUpdateOperation *updateOperation = [[SDLVoiceCommandUpdateOperation alloc] initWithConnectionManager:self.connectionManager pendingVoiceCommands:voiceCommands oldVoiceCommands:_currentVoiceCommands updateCompletionHandler:^(NSArray<SDLVoiceCommand *> *newCurrentVoiceCommands, NSError * _Nullable error) {
         weakSelf.currentVoiceCommands = newCurrentVoiceCommands;
         [weakSelf sdl_updatePendingOperationsWithNewCurrentVoiceCommands:newCurrentVoiceCommands];
@@ -146,6 +150,22 @@ UInt32 const VoiceCommandIdMin = 1900000000;
     }
 }
 
+/// Evaluate the pendingVoiceCommands to check if there is two or more voiceCommands with the same string
+- (BOOL)sdl_arePendingVoiceCommandsUnique:(NSArray<SDLVoiceCommand *> *)voiceCommands {
+    NSMutableSet<NSString *> *voiceCommandSets = [[NSMutableSet alloc] init];
+    for (SDLVoiceCommand *voiceCommand in voiceCommands) {
+        for (NSString *voiceCommandString in voiceCommand.voiceCommands) {
+            if ([voiceCommandSets containsObject:voiceCommandString]) {
+                SDLLogE(@"Failed to upload voice commands for having duplicate strings in different voiceCommands %@", nil);
+                return NO;
+            } else {
+                [voiceCommandSets addObject:voiceCommandString];
+            }
+        }
+    }
+    return YES;
+}
+
 #pragma mark - Observers
 
 - (void)sdl_commandNotification:(SDLRPCNotificationNotification *)notification {
diff --git a/SmartDeviceLink/private/SDLVoiceCommandUpdateOperation.m b/SmartDeviceLink/private/SDLVoiceCommandUpdateOperation.m
index 16b3a8fdb..184615d63 100644
--- a/SmartDeviceLink/private/SDLVoiceCommandUpdateOperation.m
+++ b/SmartDeviceLink/private/SDLVoiceCommandUpdateOperation.m
@@ -57,21 +57,6 @@ NS_ASSUME_NONNULL_BEGIN
     }
 
     __weak typeof(self) weakSelf = self;
-
-    // Evaluate the pendingVoiceCommands to check if there is two or more voiceCommands with the same string
-    NSMutableSet<NSString *> *voiceCommandSets = [[NSMutableSet alloc] init];
-    for (SDLVoiceCommand *voiceCommand in weakSelf.pendingVoiceCommands) {
-        for (NSString *voiceCommandString in voiceCommand.voiceCommands) {
-            if ([voiceCommandSets containsObject:voiceCommandString]) {
-                SDLLogE(@"Failed to upload voice commands for having duplicate strings in different voiceCommands %@", nil);
-                [weakSelf finishOperation];
-                return;
-            } else {
-                [voiceCommandSets addObject:voiceCommandString];
-            }
-        }
-    }
-
     [self sdl_sendDeleteCurrentVoiceCommands:^{
         // If the operation has been canceled, then don't send the new commands and finish the operation
         if (self.isCancelled) {
diff --git a/SmartDeviceLinkTests/DevAPISpecs/SDLVoiceCommandManagerSpec.m b/SmartDeviceLinkTests/DevAPISpecs/SDLVoiceCommandManagerSpec.m
index 3fed04f1c..365878969 100644
--- a/SmartDeviceLinkTests/DevAPISpecs/SDLVoiceCommandManagerSpec.m
+++ b/SmartDeviceLinkTests/DevAPISpecs/SDLVoiceCommandManagerSpec.m
@@ -37,6 +37,8 @@
 @property (assign, nonatomic) UInt32 lastVoiceCommandId;
 @property (copy, nonatomic) NSArray<SDLVoiceCommand *> *currentVoiceCommands;
 
+- (BOOL)sdl_arePendingVoiceCommandsUnique:(NSArray<SDLVoiceCommand *> *)voiceCommands;
+
 @end
 
 UInt32 const VoiceCommandIdMin = 1900000000;
@@ -49,6 +51,7 @@ describe(@"voice command manager", ^{
 
     __block SDLVoiceCommand *testVoiceCommand = [[SDLVoiceCommand alloc] initWithVoiceCommands:@[@"Test 1"] handler:^{}];
     __block SDLVoiceCommand *testVoiceCommand2 = [[SDLVoiceCommand alloc] initWithVoiceCommands:@[@"Test 2"] handler:^{}];
+    __block SDLVoiceCommand *testVoiceCommand3 = [[SDLVoiceCommand alloc] initWithVoiceCommands:@[@"Test 1", @"Test 2"] handler:^{}];
     __block SDLOnHMIStatus *newHMIStatus = [[SDLOnHMIStatus alloc] init];
     __block NSArray<SDLVoiceCommand *> *testVCArray = nil;
 
@@ -155,6 +158,19 @@ describe(@"voice command manager", ^{
                 });
             });
         });
+
+        // updating voice commands with duplicate string in different voice commands
+        describe(@"when new voice commands are set and have duplicate strings in different voice commands", ^{
+            beforeEach(^{
+                testManager.voiceCommands = @[testVoiceCommand2, testVoiceCommand3];
+            });
+
+            // should queue another operation
+            fit(@"should only have one operation", ^{
+                expect(testManager.transactionQueue.operations).to(haveCount(1));
+                expect(testManager.sdl_arePendingVoiceCommandsUnique:@[testVoiceCommand2, testVoiceCommand3]).to(equal(NO));
+            });
+        });
     });
 
     // on disconnect
-- 
cgit v1.2.1