Fix Choice Cells and Menu Cells do not take which properties are available into account for uniqueness

android/sdl_android/src/androidTest/java/com/smartdevicelink/managers/screen/choiceset/ChoiceSetManagerTests.java

@@ -40,18 +40,23 @@
 import com.smartdevicelink.managers.BaseSubManager;
 import com.smartdevicelink.managers.ISdl;
 import com.smartdevicelink.managers.file.FileManager;
+import com.smartdevicelink.proxy.rpc.ImageField;
 import com.smartdevicelink.proxy.rpc.KeyboardCapabilities;
 import com.smartdevicelink.proxy.rpc.KeyboardLayoutCapability;
 import com.smartdevicelink.proxy.rpc.KeyboardProperties;
 import com.smartdevicelink.proxy.rpc.SdlMsgVersion;
+import com.smartdevicelink.proxy.rpc.TextField;
 import com.smartdevicelink.proxy.rpc.WindowCapability;
 import com.smartdevicelink.proxy.rpc.enums.HMILevel;
+import com.smartdevicelink.proxy.rpc.enums.ImageFieldName;
 import com.smartdevicelink.proxy.rpc.enums.KeyboardInputMask;
 import com.smartdevicelink.proxy.rpc.enums.KeyboardLayout;
 import com.smartdevicelink.proxy.rpc.enums.KeypressMode;
 import com.smartdevicelink.proxy.rpc.enums.Language;
 import com.smartdevicelink.proxy.rpc.enums.SystemContext;
+import com.smartdevicelink.proxy.rpc.enums.TextFieldName;
 import com.smartdevicelink.proxy.rpc.enums.TriggerSource;
+import com.smartdevicelink.test.TestValues;
 
 import org.junit.After;
 import org.junit.Before;
@@ -231,7 +236,8 @@ public void testAddUniqueNamesToCells() {
         ChoiceCell cell4 = new ChoiceCell("McDonalds", "4 mile away", null, null, null, null);
         ChoiceCell cell5 = new ChoiceCell("Starbucks", "5 mile away", null, null, null, null);
         ChoiceCell cell6 = new ChoiceCell("Meijer", "6 mile away", null, null, null, null);
-        LinkedHashSet<ChoiceCell> cellList = new LinkedHashSet<>();
+        List<ChoiceCell> cellList = new ArrayList<>();
+
         cellList.add(cell1);
         cellList.add(cell2);
         cellList.add(cell3);
@@ -468,4 +474,51 @@ public void testDismissingQueuedKeyboard() {
         verify(testKeyboardOp, times(0)).dismissKeyboard();
         verify(testKeyboardOp2, times(1)).dismissKeyboard();
     }
+
+    @Test
+    public void testUniquenessForAvailableFields() {
+        WindowCapability windowCapability = new WindowCapability();
+        TextField secondaryText = new TextField();
+        secondaryText.setName(TextFieldName.secondaryText);
+        TextField tertiaryText = new TextField();
+        tertiaryText.setName(TextFieldName.tertiaryText);
+
+        List<TextField> textFields = new ArrayList<>();
+        textFields.add(secondaryText);
+        textFields.add(tertiaryText);
+        windowCapability.setTextFields(textFields);
+
+        ImageField choiceImage = new ImageField();
+        choiceImage.setName(ImageFieldName.choiceImage);
+        ImageField choiceSecondaryImage = new ImageField();
+        choiceSecondaryImage.setName(ImageFieldName.choiceSecondaryImage);
+        List<ImageField> imageFieldList = new ArrayList<>();
+        imageFieldList.add(choiceImage);
+        imageFieldList.add(choiceSecondaryImage);
+        windowCapability.setImageFields(imageFieldList);
+
+        csm.defaultMainWindowCapability = windowCapability;
+
+        ChoiceCell cell1 = new ChoiceCell("Item 1", "null", "tertiaryText", null, TestValues.GENERAL_ARTWORK, TestValues.GENERAL_ARTWORK);
+        ChoiceCell cell2 = new ChoiceCell("Item 1", "null2", "tertiaryText2", null, null, null);
+        List<ChoiceCell> choiceCellList = new ArrayList<>();
+        choiceCellList.add(cell1);
+        choiceCellList.add(cell2);
+
+        List<ChoiceCell> removedProperties = csm.removeUnusedProperties(choiceCellList);
+        assertNotNull(removedProperties.get(0).getSecondaryText());
+
+        textFields.remove(secondaryText);
+        textFields.remove(tertiaryText);
+        imageFieldList.remove(choiceImage);
+        imageFieldList.remove(choiceSecondaryImage);
+
+        removedProperties = csm.removeUnusedProperties(choiceCellList);
+        csm.addUniqueNamesBasedOnStrippedCells(removedProperties, choiceCellList);
+        assertEquals(choiceCellList.get(1).getUniqueText(), "Item 1 (2)");
+
+
+    }
+
+
 }

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

@@ -43,17 +43,22 @@
 import com.smartdevicelink.protocol.enums.FunctionID;
 import com.smartdevicelink.proxy.RPCRequest;
 import com.smartdevicelink.proxy.RPCResponse;
+import com.smartdevicelink.proxy.rpc.ImageField;
 import com.smartdevicelink.proxy.rpc.OnCommand;
 import com.smartdevicelink.proxy.rpc.OnHMIStatus;
 import com.smartdevicelink.proxy.rpc.SdlMsgVersion;
 import com.smartdevicelink.proxy.rpc.SetGlobalProperties;
+import com.smartdevicelink.proxy.rpc.TextField;
 import com.smartdevicelink.proxy.rpc.WindowCapability;
 import com.smartdevicelink.proxy.rpc.enums.FileType;
 import com.smartdevicelink.proxy.rpc.enums.HMILevel;
+import com.smartdevicelink.proxy.rpc.enums.ImageFieldName;
 import com.smartdevicelink.proxy.rpc.enums.MenuLayout;
 import com.smartdevicelink.proxy.rpc.enums.SystemContext;
+import com.smartdevicelink.proxy.rpc.enums.TextFieldName;
 import com.smartdevicelink.proxy.rpc.enums.TriggerSource;
 import com.smartdevicelink.proxy.rpc.listeners.OnRPCNotificationListener;
+import com.smartdevicelink.test.TestValues;
 
 import org.junit.After;
 import org.junit.Before;
@@ -63,6 +68,7 @@
 import org.mockito.invocation.InvocationOnMock;
 import org.mockito.stubbing.Answer;
 
+import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collections;
 import java.util.List;
@@ -598,6 +604,111 @@ public void testAllowingNonUniqueTitles() {
         assertEquals(menuManager.menuCells.get(3).getSubCells().get(3).getUniqueTitle(), "A");
     }
 
+    @Test
+    public void testUniquenessForAvailableFields() {
+        WindowCapability windowCapability = new WindowCapability();
+        TextField menuSubMenuSecondaryText = new TextField();
+        menuSubMenuSecondaryText.setName(TextFieldName.menuSubMenuSecondaryText);
+        TextField menuSubMenuTertiaryText = new TextField();
+        menuSubMenuTertiaryText.setName(TextFieldName.menuSubMenuTertiaryText);
+        TextField menuCommandSecondaryText = new TextField();
+        menuCommandSecondaryText.setName(TextFieldName.menuCommandSecondaryText);
+        TextField menuCommandTertiaryText = new TextField();
+        menuCommandTertiaryText.setName(TextFieldName.menuCommandTertiaryText);
+        List<TextField> textFields = new ArrayList<>();
+        textFields.add(menuSubMenuSecondaryText);
+        textFields.add(menuSubMenuTertiaryText);
+        textFields.add(menuCommandSecondaryText);
+        textFields.add(menuCommandTertiaryText);
+        windowCapability.setTextFields(textFields);
+
+        ImageField cmdIcon = new ImageField();
+        cmdIcon.setName(ImageFieldName.cmdIcon);
+        ImageField menuSubMenuSecondaryImage = new ImageField();
+        menuSubMenuSecondaryImage.setName(ImageFieldName.menuSubMenuSecondaryImage);
+        ImageField menuCommandSecondaryImage = new ImageField();
+        menuCommandSecondaryImage.setName(ImageFieldName.menuCommandSecondaryImage);
+        List<ImageField> imageFieldList = new ArrayList<>();
+        imageFieldList.add(cmdIcon);
+        imageFieldList.add(menuSubMenuSecondaryImage);
+        imageFieldList.add(menuCommandSecondaryImage);
+        windowCapability.setImageFields(imageFieldList);
+        menuManager.defaultMainWindowCapability = windowCapability;
+
+        assertNull(menuManager.removeUnusedProperties(null));
+
+        MenuCell cell1 = new MenuCell("Text1", "SecondaryText", "TText", TestValues.GENERAL_ARTWORK, TestValues.GENERAL_ARTWORK, null, new MenuSelectionListener() {
+            @Override
+            public void onTriggered(TriggerSource trigger) {
+
+            }
+        });
+
+        MenuCell cell2 = new MenuCell("Text1", "SecondaryText2", "TText2", null, null, null, new MenuSelectionListener() {
+            @Override
+            public void onTriggered(TriggerSource trigger) {
+
+            }
+        });
+
+        MenuCell subCell1 = new MenuCell("SubCell1", "Secondary Text", "TText", TestValues.GENERAL_ARTWORK, TestValues.GENERAL_ARTWORK, null, new MenuSelectionListener() {
+            @Override
+            public void onTriggered(TriggerSource trigger) {
+            }
+        });
+
+        MenuCell subCell2 = new MenuCell("SubCell1", "Secondary Text2", "TText2", null, null, null, new MenuSelectionListener() {
+            @Override
+            public void onTriggered(TriggerSource trigger) {
+            }
+        });
+
+        List<MenuCell> subCellList = new ArrayList<>();
+        subCellList.add(subCell1);
+        subCellList.add(subCell2);
+
+
+        MenuCell cell3 = new MenuCell("Test Cell 3 (sub menu)", "SecondaryText", "TText", MenuLayout.LIST, TestValues.GENERAL_ARTWORK, TestValues.GENERAL_ARTWORK, subCellList);
+        MenuCell cell4 = new MenuCell("Test Cell 3 (sub menu)", null, null, MenuLayout.LIST, null, null, subCellList);
+
+        List<MenuCell> menuCellList = new ArrayList<>();
+        menuCellList.add(cell1);
+        menuCellList.add(cell2);
+        menuCellList.add(cell3);
+        menuCellList.add(cell4);
+
+        List<MenuCell> removedProperties = menuManager.removeUnusedProperties(menuCellList);
+        assertNotNull(removedProperties.get(0).getSecondaryText());
+        menuManager.addUniqueNamesBasedOnStrippedCells(removedProperties, menuCellList);
+        assertEquals(menuCellList.get(1).getUniqueTitle(), "Text1");
+
+        // Remove menuCommandSecondaryText as a supported TextField
+        textFields.remove(menuCommandSecondaryText);
+        textFields.remove(menuCommandTertiaryText);
+        imageFieldList.remove(cmdIcon);
+        imageFieldList.remove(menuCommandSecondaryImage);
+        imageFieldList.remove(menuSubMenuSecondaryImage);
+        textFields.remove(menuSubMenuSecondaryText);
+        textFields.remove(menuSubMenuTertiaryText);
+        textFields.remove(menuSubMenuSecondaryImage);
+
+        // Test removeUnusedProperties
+        removedProperties = menuManager.removeUnusedProperties(menuCellList);
+        assertNull(removedProperties.get(0).getSecondaryText());
+        assertNull(removedProperties.get(0).getTertiaryText());
+
+        menuManager.addUniqueNamesBasedOnStrippedCells(removedProperties, menuCellList);
+        assertEquals(menuCellList.get(1).getUniqueTitle(), "Text1 (2)");
+
+        // SubCell test
+        assertEquals(menuCellList.get(3).getUniqueTitle(), "Test Cell 3 (sub menu) (2)");
+        assertEquals(menuCellList.get(2).getSubCells().get(1).getUniqueTitle(), "SubCell1 (2)");
+
+
+    }
+
+
+
     // HELPERS
 
     // Emulate what happens when Core sends OnHMIStatus notification
@@ -849,4 +960,5 @@ private List<MenuCell> createDynamicMenu6_forUniqueNamesTest() {
         return Arrays.asList(A, B, C, D);
     }
 
+
 }

base/src/main/java/com/smartdevicelink/managers/screen/choiceset/BaseChoiceSetManager.java

@@ -42,6 +42,7 @@
 import com.smartdevicelink.managers.BaseSubManager;
 import com.smartdevicelink.managers.CompletionListener;
 import com.smartdevicelink.managers.ISdl;
+import com.smartdevicelink.managers.ManagerUtility;
 import com.smartdevicelink.managers.file.FileManager;
 import com.smartdevicelink.managers.lifecycle.OnSystemCapabilityListener;
 import com.smartdevicelink.managers.lifecycle.SystemCapabilityManager;
@@ -54,18 +55,21 @@
 import com.smartdevicelink.proxy.rpc.OnHMIStatus;
 import com.smartdevicelink.proxy.rpc.WindowCapability;
 import com.smartdevicelink.proxy.rpc.enums.HMILevel;
+import com.smartdevicelink.proxy.rpc.enums.ImageFieldName;
 import com.smartdevicelink.proxy.rpc.enums.InteractionMode;
 import com.smartdevicelink.proxy.rpc.enums.KeyboardLayout;
 import com.smartdevicelink.proxy.rpc.enums.KeypressMode;
 import com.smartdevicelink.proxy.rpc.enums.Language;
 import com.smartdevicelink.proxy.rpc.enums.PredefinedWindows;
 import com.smartdevicelink.proxy.rpc.enums.SystemCapabilityType;
 import com.smartdevicelink.proxy.rpc.enums.SystemContext;
+import com.smartdevicelink.proxy.rpc.enums.TextFieldName;
 import com.smartdevicelink.proxy.rpc.enums.TriggerSource;
 import com.smartdevicelink.proxy.rpc.listeners.OnRPCNotificationListener;
 import com.smartdevicelink.util.DebugTool;
 
 import java.lang.ref.WeakReference;
+import java.util.ArrayList;
 import java.util.HashMap;
 import java.util.HashSet;
 import java.util.LinkedHashSet;
@@ -540,7 +544,7 @@ HashSet<ChoiceCell> choicesToBeRemovedFromPendingWithArray(List<ChoiceCell> choi
      * E.g. Choices param contains 2 cells with text/title "Address" will be handled by updating the uniqueText/uniqueTitle of the second cell to "Address (2)".
      * @param choices The list of choiceCells to be uploaded.
      */
-    void addUniqueNamesToCells(LinkedHashSet<ChoiceCell> choices) {
+    void addUniqueNamesToCells(List<ChoiceCell> choices) {
         HashMap<String, Integer> dictCounter = new HashMap<>();
 
         for (ChoiceCell cell : choices) {
@@ -556,16 +560,80 @@ void addUniqueNamesToCells(LinkedHashSet<ChoiceCell> choices) {
         }
     }
 
+    void addUniqueNamesBasedOnStrippedCells(List<ChoiceCell> strippedCells, List<ChoiceCell> unstrippedCells) {
+        if (strippedCells == null || unstrippedCells == null || strippedCells.size() != unstrippedCells.size()) {
+          return;
+        }
+        // Tracks how many of each cell primary text there are so that we can append numbers to make each unique as necessary
+        HashMap<ChoiceCell, Integer> dictCounter = new HashMap<>();
+        for (int i = 0; i < strippedCells.size(); i++) {
+            ChoiceCell cell = strippedCells.get(i);
+            Integer counter = dictCounter.get(cell);
+            if (counter != null) {
+                counter++;
+                dictCounter.put(cell, counter);
+            } else {
+                dictCounter.put(cell, 1);
+            }
+
+            counter = dictCounter.get(cell);
+
+            if (counter > 1) {
+                unstrippedCells.get(i).setUniqueText(unstrippedCells.get(i).getText() + " (" + counter + ")");
+            }
+        }
+    }
+
+    private List<ChoiceCell> cloneChoiceCellList(List<ChoiceCell> originalList) {
+        if (originalList == null) {
+            return null;
+        }
+
+        List<ChoiceCell> clone = new ArrayList<>();
+        for (ChoiceCell choiceCell : originalList) {
+            clone.add(choiceCell.clone());
+        }
+        return clone;
+    }
+
     private LinkedHashSet<ChoiceCell> getChoicesToBeUploadedWithArray(List<ChoiceCell> choices) {
-        LinkedHashSet<ChoiceCell> choiceSet = new LinkedHashSet<>(choices);
-        // If we're running on a connection < RPC 7.1, we need to de-duplicate cells because presenting them will fail if we have the same cell primary text.
+        // Clone choices
+        List<ChoiceCell> choicesClone = cloneChoiceCellList(choices);
         if (choices != null && internalInterface.getSdlMsgVersion() != null
                 && (internalInterface.getSdlMsgVersion().getMajorVersion() < 7
                 || (internalInterface.getSdlMsgVersion().getMajorVersion() == 7 && internalInterface.getSdlMsgVersion().getMinorVersion() == 0))) {
-            addUniqueNamesToCells(choiceSet);
+            // If we're on < RPC 7.1, all primary texts need to be unique, so we don't need to check removed properties and duplicate cells
+            addUniqueNamesToCells(choicesClone);
+        } else {
+            List<ChoiceCell> strippedCellsClone = removeUnusedProperties(choicesClone);
+            addUniqueNamesBasedOnStrippedCells(strippedCellsClone, choicesClone);
         }
-        choiceSet.removeAll(preloadedChoices);
-        return choiceSet;
+        LinkedHashSet<ChoiceCell> choiceCloneLinkedHash = new LinkedHashSet<>(choicesClone);
+        choiceCloneLinkedHash.removeAll(preloadedChoices);
+        return choiceCloneLinkedHash;
+    }
+
+    List<ChoiceCell> removeUnusedProperties(List<ChoiceCell> choiceCells) {
+        List<ChoiceCell> strippedCellsClone = cloneChoiceCellList(choiceCells);
+        //Clone Cells
+        for (ChoiceCell cell : strippedCellsClone) {
+            // Strip away fields that cannot be used to determine uniqueness visually including fields not supported by the HMI
+            cell.setVoiceCommands(null);
+
+            if (!hasImageFieldOfName(ImageFieldName.choiceImage)) {
+                cell.setArtwork(null);
+            }
+            if (!hasTextFieldOfName(TextFieldName.secondaryText)) {
+                cell.setSecondaryText(null);
+            }
+            if (!hasTextFieldOfName(TextFieldName.tertiaryText)) {
+                cell.setTertiaryText(null);
+            }
+            if (!hasImageFieldOfName(ImageFieldName.choiceSecondaryImage)) {
+                cell.setSecondaryArtwork(null);
+            }
+        }
+        return strippedCellsClone;
     }
 
     void updateIdsOnChoices(LinkedHashSet<ChoiceCell> choices) {
@@ -672,6 +740,14 @@ public void onNotified(RPCNotification notification) {
 
     // ADDITIONAL HELPERS
 
+    private boolean hasImageFieldOfName(ImageFieldName imageFieldName) {
+        return defaultMainWindowCapability == null || ManagerUtility.WindowCapabilityUtility.hasImageFieldOfName(defaultMainWindowCapability, imageFieldName);
+    }
+
+    private boolean hasTextFieldOfName(TextFieldName textFieldName) {
+        return defaultMainWindowCapability == null || ManagerUtility.WindowCapabilityUtility.hasTextFieldOfName(defaultMainWindowCapability, textFieldName);
+    }
+
     boolean setUpChoiceSet(ChoiceSet choiceSet) {
 
         List<ChoiceCell> choices = choiceSet.getChoices();
@@ -695,9 +771,8 @@ boolean setUpChoiceSet(ChoiceSet choiceSet) {
         int choiceCellWithVoiceCommandCount = 0;
 
         for (ChoiceCell cell : choices) {
-
             uniqueChoiceCells.add(cell);
-
+            // Not using cloned cell here because we set the clone's VoiceCommands to null for visual check only
             if (cell.getVoiceCommands() != null) {
                 uniqueVoiceCommands.addAll(cell.getVoiceCommands());
                 choiceCellWithVoiceCommandCount += 1;