Fix for voice command that contains no string should be removed (#1686)

* Update sending VoiceCommandManager to strip extra whiteSpace from voiceCommands and avoid sending empty strings in the update operation

* Add unit test for empty strings and trimmed whiteSpace for voiceCommands

* update unit test

* Add null unit test check to voiceCommandManager

* Implement suggestions from code review

* Removed null voiceCommands from VoiceCommandList. Update unit test

* Fix suggestions from code review

* Update ID's for voice commands in proper list

* Strip white Space before checking if commands are unique

Co-authored-by: Julian Kast <julian@livio.com>

Author: JulianKast

android/sdl_android/src/androidTest/java/com/smartdevicelink/managers/screen/menu/VoiceCommandManagerTests.java

@@ -210,4 +210,47 @@ public void testUniqueStrings() {
         assertNull(voiceCommandManager.voiceCommands);
     }
 
+    /**
+     * Test trim whitespace from voice commands and not uploading empty strings.
+     */
+    @Test
+    public void testEmptyStringsAndWhiteSpaceRemoval() {
+        List<VoiceCommand> voiceCommandList = new ArrayList<>();
+        VoiceCommand command1 = new VoiceCommand(Arrays.asList("   Command one "), null);
+
+        voiceCommandList.add(command1);
+        voiceCommandManager.currentHMILevel = HMILevel.HMI_NONE;
+        voiceCommandManager.setVoiceCommands(voiceCommandList);
+        assertEquals(voiceCommandManager.voiceCommands.get(0).getVoiceCommands().get(0), "Command one");
+
+        voiceCommandManager.voiceCommands = null;
+        voiceCommandList = new ArrayList<>();
+        command1 = new VoiceCommand(Arrays.asList(" "), null);
+
+        voiceCommandList.add(command1);
+        voiceCommandManager.setVoiceCommands(voiceCommandList);
+        assertNull(voiceCommandManager.voiceCommands);
+
+        VoiceCommand command2 = new VoiceCommand(Arrays.asList("Command two"), null);
+        voiceCommandList.add(command2);
+        voiceCommandManager.setVoiceCommands(voiceCommandList);
+        assertEquals(voiceCommandManager.voiceCommands.size(), 1);
+
+        voiceCommandManager.setVoiceCommands(voiceCommandList);
+        assertEquals(voiceCommandManager.voiceCommands.size(), 1);
+
+        voiceCommandManager.voiceCommands = null;
+        voiceCommandManager.setVoiceCommands(null);
+        assertNull(voiceCommandManager.voiceCommands);
+
+        voiceCommandList = new ArrayList<>();
+        VoiceCommand command3 = new VoiceCommand(Arrays.asList("Command three", null, " "), null);
+        voiceCommandList.add(command3);
+        voiceCommandList.add(null);
+        voiceCommandManager.setVoiceCommands(voiceCommandList);
+        assertEquals(voiceCommandManager.voiceCommands.size(), 1);
+
+    }
+
+
 }

base/src/main/java/com/smartdevicelink/managers/screen/menu/BaseVoiceCommandManager.java

@@ -56,7 +56,7 @@
 
 abstract class BaseVoiceCommandManager extends BaseSubManager {
     private static final String TAG = "BaseVoiceCommandManager";
-    List<VoiceCommand> voiceCommands, currentVoiceCommands;
+    List<VoiceCommand> voiceCommands, currentVoiceCommands, originalVoiceCommands;
 
     int lastVoiceCommandId;
     private static final int voiceCommandIdMin = 1900000000;
@@ -132,21 +132,29 @@ private void updateTransactionQueueSuspended() {
     public void setVoiceCommands(List<VoiceCommand> voiceCommands) {
 
         // we actually need voice commands to set.
-        if (voiceCommands == null || voiceCommands.equals(this.voiceCommands)) {
+        if (voiceCommands == null || voiceCommands.equals(this.originalVoiceCommands)) {
             DebugTool.logInfo(TAG, "Voice commands list was null or matches the current voice commands");
             return;
         }
 
-        if (!isVoiceCommandsUnique(voiceCommands)) {
+        List<VoiceCommand> validatedVoiceCommands = removeEmptyVoiceCommands(voiceCommands);
+
+        if (validatedVoiceCommands.size() == 0) {
+            DebugTool.logError(TAG, "New voice commands are invalid, skipping...");
+            return;
+        }
+
+        if (!isVoiceCommandsUnique(validatedVoiceCommands)) {
             DebugTool.logError(TAG, "Not all voice command strings are unique across all voice commands. Voice commands will not be set.");
             return;
         }
 
-        updateIdsOnVoiceCommands(voiceCommands);
-        this.voiceCommands = new ArrayList<>(voiceCommands);
+        this.originalVoiceCommands = new ArrayList<>(voiceCommands);
+        this.voiceCommands = validatedVoiceCommands;
+        updateIdsOnVoiceCommands(this.voiceCommands);
 
         cleanTransactionQueue();
-        updateOperation = new VoiceCommandUpdateOperation(internalInterface, currentVoiceCommands, voiceCommands, new VoiceCommandUpdateOperation.VoiceCommandChangesListener() {
+        updateOperation = new VoiceCommandUpdateOperation(internalInterface, currentVoiceCommands, this.voiceCommands, new VoiceCommandUpdateOperation.VoiceCommandChangesListener() {
             @Override
             public void updateVoiceCommands(List<VoiceCommand> newCurrentVoiceCommands, HashMap<RPCRequest, String> errorObject) {
                 DebugTool.logInfo(TAG, "The updated list of VoiceCommands: " + newCurrentVoiceCommands);
@@ -194,6 +202,30 @@ private void updateIdsOnVoiceCommands(List<VoiceCommand> voiceCommands) {
         }
     }
 
+    List<VoiceCommand> removeEmptyVoiceCommands(List<VoiceCommand> voiceCommands) {
+        List<VoiceCommand> validatedVoiceCommands = new ArrayList<>();
+        for (VoiceCommand voiceCommand : voiceCommands) {
+            if (voiceCommand == null) {
+                continue;
+            }
+            List<String> voiceCommandStrings = new ArrayList<>();
+            for (String voiceCommandString : voiceCommand.getVoiceCommands()) {
+                if (voiceCommandString == null) {
+                    continue;
+                }
+                String trimmedString = voiceCommandString.trim();
+                if (trimmedString.length() > 0) {
+                    voiceCommandStrings.add(trimmedString);
+                }
+            }
+            if (voiceCommandStrings.size() > 0) {
+                voiceCommand.setVoiceCommands(voiceCommandStrings);
+                validatedVoiceCommands.add(voiceCommand);
+            }
+        }
+        return validatedVoiceCommands;
+    }
+
     // LISTENERS
 
     private void addListeners() {
@@ -243,6 +275,9 @@ private boolean isVoiceCommandsUnique(List<VoiceCommand> voiceCommands) {
         int voiceCommandCount = 0;
 
         for (VoiceCommand voiceCommand : voiceCommands) {
+            if (voiceCommand == null) {
+                continue;
+            }
             voiceCommandHashSet.addAll(voiceCommand.getVoiceCommands());
             voiceCommandCount += voiceCommand.getVoiceCommands().size();
         }