Fix manager to not delete and set identical voice commands

lib/js/src/manager/screen/_ScreenManagerBase.js

@@ -438,7 +438,7 @@ class _ScreenManagerBase extends _SubManagerBase {
     }
 
     /**
-     * Get the currently set voice commands
+     * Gets the voice commands set as part of the last initiated update operation
      * @returns {VoiceCommand[]} - a List of Voice Command objects
      */
     getVoiceCommands () {

lib/js/src/manager/screen/_VoiceCommandManagerBase.js

@@ -84,24 +84,27 @@ class _VoiceCommandManagerBase extends _SubManagerBase {
      */
     async setVoiceCommands (voiceCommands) {
         // we actually need voice commands to set. checks if the array of voice commands passed in contains the same content as the current voice commands
-        if (!Array.isArray(voiceCommands) || (voiceCommands.length > 0 && !voiceCommands.map((vc, index) => vc.equals(this._originalVoiceCommands[index])).includes(false))) {
-            console.log('Voice commands list non-existent or matches the current voice commands');
+        if (!Array.isArray(voiceCommands)) {
+            console.log('Voice commands list non-existent');
             return;
         }
 
-        const validatedVoiceCommands = this._removeEmptyVoiceCommands(voiceCommands);
+        this._voiceCommands = voiceCommands.map((vc) => vc.clone());
+
+        const validatedVoiceCommands = this._removeEmptyVoiceCommands(this._voiceCommands);
         if (voiceCommands.length > 0 && validatedVoiceCommands.length === 0) {
             console.log('New voice commands are invalid, skipping...');
+            this._voiceCommands = null;
             return;
         }
 
         // check uniqueness before updating the IDs since changing the IDs would make them all unique
         if (!this._arePendingVoiceCommandsUnique(validatedVoiceCommands)) {
             console.log('Not all voice command strings are unique across all voice commands. Voice commands will not be set.');
+            this._voiceCommands = null;
             return;
         }
 
-        this._originalVoiceCommands = voiceCommands;
         this._voiceCommands = validatedVoiceCommands;
 
         this._updateIdsOnVoiceCommands(this._voiceCommands);
@@ -136,7 +139,7 @@ class _VoiceCommandManagerBase extends _SubManagerBase {
     }
 
     /**
-     * Gets all the voice commands currently set
+     * Gets all the voice commands set as part of the last initiated update operation
      * @returns {VoiceCommand[]} - An array of VoiceCommand instances.
      */
     getVoiceCommands () {
@@ -163,7 +166,7 @@ class _VoiceCommandManagerBase extends _SubManagerBase {
             if (task.getState() === _Task.IN_PROGRESS) {
                 return;
             }
-            task._oldVoiceCommands = voiceCommands;
+            task._setOldVoiceCommands(voiceCommands);
         });
     }
 
@@ -197,7 +200,7 @@ class _VoiceCommandManagerBase extends _SubManagerBase {
         this._commandListener = (onCommand) => {
             // find and invoke the listener of the matching command
             const targetCommandId = onCommand.getCmdID();
-            for (const command of this._voiceCommands) {
+            for (const command of this._currentVoiceCommands) {
                 if (targetCommandId === command._getCommandId()) {
                     const listener = command.getVoiceCommandSelectionListener();
                     if (typeof listener === 'function') {

lib/js/src/manager/screen/utils/VoiceCommand.js

@@ -129,10 +129,6 @@ class VoiceCommand {
         if (!(other instanceof VoiceCommand)) {
             return false;
         }
-        // main comparison check
-        if (this._getCommandId() !== other._getCommandId()) {
-            return false;
-        }
         const voiceCommands = this.getVoiceCommands();
         const otherVoiceCommands = other.getVoiceCommands();
         if (voiceCommands.length !== otherVoiceCommands.length) {
@@ -145,6 +141,27 @@ class VoiceCommand {
         }
         return true;
     }
+
+    /**
+     * Creates a deep copy of the object
+     * @returns {VoiceCommand} - A deep clone of the object
+     */
+    clone () {
+        const clone = new VoiceCommand(
+            JSON.parse(JSON.stringify(this.getVoiceCommands()))
+        );
+        if (typeof this._getCommandId() === 'number') {
+            clone._setCommandId(
+                parseInt(JSON.stringify(this._getCommandId()))
+            );
+        }
+
+        if (typeof this.getVoiceCommandSelectionListener() === 'function') {
+            // Re-bind the context of the listener to make a clone of the method
+            clone.setVoiceCommandSelectionListener(this.getVoiceCommandSelectionListener().bind(clone));
+        }
+        return clone;
+    }
 }
 
 export { VoiceCommand };

lib/js/src/manager/screen/utils/_VoiceCommandUpdateOperation.js

@@ -68,6 +68,17 @@ class _VoiceCommandUpdateOperation extends _Task {
             this.onFinished();
             return;
         }
+
+        if (Array.isArray(this._pendingVoiceCommands) && this._pendingVoiceCommands.length > 0) {
+            for (const voiceCommand of this._pendingVoiceCommands) {
+                this._currentVoiceCommands.forEach((vc) => {
+                    if (vc.equals(voiceCommand)) {
+                        voiceCommand.setVoiceCommandSelectionListener(vc.getVoiceCommandSelectionListener());
+                    }
+                });
+            }
+        }
+
         await this._sendDeleteCurrentVoiceCommands();
         if (this.getState() === _Task.CANCELED) {
             this.onFinished();
@@ -89,11 +100,16 @@ class _VoiceCommandUpdateOperation extends _Task {
      */
     async _sendDeleteCurrentVoiceCommands () {
         if (!Array.isArray(this._oldVoiceCommands) || this._oldVoiceCommands.length === 0) {
+            return true;
+        }
+
+        const voiceCommandsToDelete = this._voiceCommandsNotInSecondArray(this._oldVoiceCommands, this._pendingVoiceCommands);
+        if (voiceCommandsToDelete.length === 0) {
             return true; // nothing to delete
         }
 
         // make a DeleteCommand request for every voice command
-        const deleteCommands = this._oldVoiceCommands.map(voiceCommand => {
+        const deleteCommands = voiceCommandsToDelete.map(voiceCommand => {
             return new DeleteCommand().setCmdID(voiceCommand._getCommandId());
         });
 
@@ -119,6 +135,31 @@ class _VoiceCommandUpdateOperation extends _Task {
         return this._errorArray.length === 0;
     }
 
+    /**
+     * Returns an array of VoiceCommands that are in the first array but not the second array
+     * @param {VoiceCommmand[]} firstArray - an array of VoiceCommands
+     * @param {VoiceCommand[]} secondArray - an array of VoiceCommands
+     * @returns {VoiceCommand[]} - An array of VoiceCommands that are in the first array but not the second array
+     */
+    _voiceCommandsNotInSecondArray (firstArray = null, secondArray = null) {
+        if (!Array.isArray(firstArray) || firstArray.length === 0) {
+            return secondArray;
+        }
+        if (!Array.isArray(secondArray) || secondArray.length === 0) {
+            return firstArray;
+        }
+
+        const differenceArray = [];
+
+        firstArray.forEach((checkVC) => {
+            if (!secondArray.map((secondVC) => checkVC.equals(secondVC)).includes(true)) {
+                differenceArray.push(checkVC);
+            }
+        });
+
+        return Array.from(differenceArray);
+    }
+
     /**
      * Removes this delete command from the current voice commands array
      * @private
@@ -140,12 +181,13 @@ class _VoiceCommandUpdateOperation extends _Task {
      * @returns {Promise} - A promise which returns a Boolean of whether the operation is a success
      */
     async _sendCurrentVoiceCommands () {
-        if (!Array.isArray(this._pendingVoiceCommands) || this._pendingVoiceCommands.length === 0) {
-            return true; // nothing to delete
+        const voiceCommandsToAdd = this._voiceCommandsNotInSecondArray(this._pendingVoiceCommands, this._oldVoiceCommands);
+        if (!Array.isArray(voiceCommandsToAdd) || voiceCommandsToAdd.length === 0) {
+            return true; // nothing to send
         }
 
         // filter the voice command list of any voice commands with duplicate items
-        const addCommands = this._pendingVoiceCommands.map(voiceCommand => {
+        const addCommands = voiceCommandsToAdd.map(voiceCommand => {
             // make an AddCommand request for every voice command
             return new AddCommand()
                 .setCmdID(voiceCommand._getCommandId())
@@ -189,6 +231,15 @@ class _VoiceCommandUpdateOperation extends _Task {
             }
         }
     }
+
+    /**
+     * Updates the voice commands in the task in case another operation has made updates
+     * @param {VoiceCommand[]} oldVoiceCommands - An Array of VoiceCommands
+     */
+    _setOldVoiceCommands (oldVoiceCommands) {
+        this._oldVoiceCommands = oldVoiceCommands;
+        this._currentVoiceCommands = Array.from(oldVoiceCommands);
+    }
 }
 
 export { _VoiceCommandUpdateOperation };

tests/managers/screen/VoiceCommandManagerTests.js

@@ -66,23 +66,34 @@ module.exports = async function (appClient) {
             Validator.assertEquals(voiceCommandManager._currentHmiLevel, SDL.rpc.enums.HMILevel.HMI_FULL);
         });
 
-        it('testUpdatingCommands', function () {
-            const callback = sinon.fake(() => {});
+        it('testUpdatingCommands', async function () {
+            let timesCallbackWasCalled = 0;
+            const callback = () => {
+                timesCallbackWasCalled++;
+            };
             const voiceCommand3 = new SDL.manager.screen.utils.VoiceCommand(['Command 5', 'Command 6'], callback);
 
             voiceCommandManager._currentHmiLevel = SDL.rpc.enums.HMILevel.HMI_NONE; // don't act on processing voice commands
-            voiceCommandManager.setVoiceCommands([voiceCommand3]);
-
-            // Fake onCommand - we want to make sure that we can pass back onCommand events to our VoiceCommand Objects
-            voiceCommandManager._commandListener(new SDL.rpc.messages.OnCommand()
-                .setCmdID(voiceCommand3._getCommandId())
-                .setTriggerSource(SDL.rpc.enums.TriggerSource.TS_VR)); // these are voice commands
-
-            // verify the mock listener has been hit once
-            Validator.assertEquals(callback.calledOnce, true);
+            await voiceCommandManager.setVoiceCommands([voiceCommand3]);
+
+            // there's only one voice command at the moment
+            const commandId = voiceCommandManager.getVoiceCommands()[0]._getCommandId();
+
+            // the commands take time to set
+            await new Promise ((resolve) => {
+                setTimeout(() => {
+                    // Fake onCommand - we want to make sure that we can pass back onCommand events to our VoiceCommand Objects
+                    voiceCommandManager._commandListener(new SDL.rpc.messages.OnCommand()
+                        .setCmdID(commandId)
+                        .setTriggerSource(SDL.rpc.enums.TriggerSource.TS_VR)); // these are voice commands
+
+                    // verify the mock listener has been hit once
+                    Validator.assertEquals(timesCallbackWasCalled, 1);
+                    resolve();
+                }, 1000);
+            });
         });
 
-<<<<<<< HEAD
         it('testEmptyVoiceCommandsShouldAddTask', async function () {
             const callback = sinon.fake(() => {});
             const stub = sinon.stub(voiceCommandManager, '_addTask')
@@ -91,7 +102,8 @@ module.exports = async function (appClient) {
 
             Validator.assertTrue(callback.called);
             stub.restore();
-=======
+        });
+
         describe('if any of the voice commands contains an empty string', function () {
             it('should remove the empty strings and queue another operation', async function () {
                 await voiceCommandManager.setVoiceCommands([voiceCommand2, voiceCommand3, voiceCommand4, voiceCommand5, voiceCommand6]);
@@ -106,9 +118,7 @@ module.exports = async function (appClient) {
                 await voiceCommandManager.setVoiceCommands([voiceCommand1]);
                 // these commands are empty and should be ignored entirely
                 await voiceCommandManager.setVoiceCommands([voiceCommand4, voiceCommand5]);
-                Validator.assertEquals(voiceCommandManager.getVoiceCommands().length, 1);
-                Validator.assertEquals(voiceCommandManager.getVoiceCommands()[0].getVoiceCommands().length, 2);
-                Validator.assertEquals(voiceCommandManager.getVoiceCommands()[0].getVoiceCommands(), ['Command 1', 'Command 2']);
+                Validator.assertNull(voiceCommandManager.getVoiceCommands());
             });
         });
 
@@ -123,7 +133,15 @@ module.exports = async function (appClient) {
                 Validator.assertEquals(voiceCommandManager._getTasks().length, 1);
                 Validator.assertTrue(!voiceCommandManager._arePendingVoiceCommandsUnique([voiceCommand2, voiceCommand7]));
             });
->>>>>>> develop
+        });
+
+        it('clone should not keep reference', function (done) {
+            const voiceCommand = new SDL.manager.screen.utils.VoiceCommand(['Command 1', 'Command 2'], () => {});
+            const clone = voiceCommand.clone();
+            Validator.assertTrue(clone.equals(voiceCommand));
+            clone.setVoiceCommands(['Command 3']);
+            Validator.assertTrue(!clone.equals(voiceCommand));
+            done();
         });
 
         after(function () {