Merge pull request #1776 from noah-livio/bugfix/issue_1774

Fix soft button object invalid states

3 files changed, 174 insertions, 24 deletions

diff --git a/android/sdl_android/src/androidTest/java/com/smartdevicelink/managers/screen/ScreenManagerTests.java b/android/sdl_android/src/androidTest/java/com/smartdevicelink/managers/screen/ScreenManagerTests.java
index 99eb359b4..b5c0059c4 100644
--- a/android/sdl_android/src/androidTest/java/com/smartdevicelink/managers/screen/ScreenManagerTests.java
+++ b/android/sdl_android/src/androidTest/java/com/smartdevicelink/managers/screen/ScreenManagerTests.java
@@ -213,14 +213,15 @@ public class ScreenManagerTests {
 
     @Test
     public void testAssigningIdsToSoftButtonObjects() {
+        SoftButtonState defaultState = new SoftButtonState("default", "hi", null);
         SoftButtonObject sbo1, sbo2, sbo3, sbo4, sbo5;
 
         // Case 1 - don't set id for any button (Manager should set ids automatically starting from 1 and up)
-        sbo1 = new SoftButtonObject(null, Collections.EMPTY_LIST, null, null);
-        sbo2 = new SoftButtonObject(null, Collections.EMPTY_LIST, null, null);
-        sbo3 = new SoftButtonObject(null, Collections.EMPTY_LIST, null, null);
-        sbo4 = new SoftButtonObject(null, Collections.EMPTY_LIST, null, null);
-        sbo5 = new SoftButtonObject(null, Collections.EMPTY_LIST, null, null);
+        sbo1 = new SoftButtonObject(null, defaultState, null);
+        sbo2 = new SoftButtonObject(null, defaultState, null);
+        sbo3 = new SoftButtonObject(null, defaultState, null);
+        sbo4 = new SoftButtonObject(null, defaultState, null);
+        sbo5 = new SoftButtonObject(null, defaultState, null);
         screenManager.checkAndAssignButtonIds(Arrays.asList(sbo1, sbo2, sbo3, sbo4, sbo5), BaseScreenManager.ManagerLocation.SOFTBUTTON_MANAGER);
         assertEquals("SoftButtonObject id doesn't match the expected value", 1, sbo1.getButtonId());
         assertEquals("SoftButtonObject id doesn't match the expected value", 2, sbo2.getButtonId());
@@ -230,15 +231,15 @@ public class ScreenManagerTests {
 
 
         // Case 2 - Set ids for all buttons (Manager shouldn't alter the ids set by developer)
-        sbo1 = new SoftButtonObject(null, Collections.EMPTY_LIST, null, null);
+        sbo1 = new SoftButtonObject(null, defaultState, null);
         sbo1.setButtonId(100);
-        sbo2 = new SoftButtonObject(null, Collections.EMPTY_LIST, null, null);
+        sbo2 = new SoftButtonObject(null, defaultState, null);
         sbo2.setButtonId(200);
-        sbo3 = new SoftButtonObject(null, Collections.EMPTY_LIST, null, null);
+        sbo3 = new SoftButtonObject(null, defaultState, null);
         sbo3.setButtonId(300);
-        sbo4 = new SoftButtonObject(null, Collections.EMPTY_LIST, null, null);
+        sbo4 = new SoftButtonObject(null, defaultState, null);
         sbo4.setButtonId(400);
-        sbo5 = new SoftButtonObject(null, Collections.EMPTY_LIST, null, null);
+        sbo5 = new SoftButtonObject(null, defaultState, null);
         sbo5.setButtonId(500);
         screenManager.checkAndAssignButtonIds(Arrays.asList(sbo1, sbo2, sbo3, sbo4, sbo5), BaseScreenManager.ManagerLocation.SOFTBUTTON_MANAGER);
         assertEquals("SoftButtonObject id doesn't match the expected value", 100, sbo1.getButtonId());
@@ -249,13 +250,13 @@ public class ScreenManagerTests {
 
 
         // Case 3 - Set ids for some buttons (Manager shouldn't alter the ids set by developer. And it should assign ids for the ones that don't have id)
-        sbo1 = new SoftButtonObject(null, Collections.EMPTY_LIST, null, null);
+        sbo1 = new SoftButtonObject(null, defaultState, null);
         sbo1.setButtonId(50);
-        sbo2 = new SoftButtonObject(null, Collections.EMPTY_LIST, null, null);
-        sbo3 = new SoftButtonObject(null, Collections.EMPTY_LIST, null, null);
-        sbo4 = new SoftButtonObject(null, Collections.EMPTY_LIST, null, null);
+        sbo2 = new SoftButtonObject(null, defaultState, null);
+        sbo3 = new SoftButtonObject(null, defaultState, null);
+        sbo4 = new SoftButtonObject(null, defaultState, null);
         sbo4.setButtonId(100);
-        sbo5 = new SoftButtonObject(null, Collections.EMPTY_LIST, null, null);
+        sbo5 = new SoftButtonObject(null, defaultState, null);
         screenManager.checkAndAssignButtonIds(Arrays.asList(sbo1, sbo2, sbo3, sbo4, sbo5), BaseScreenManager.ManagerLocation.SOFTBUTTON_MANAGER);
         assertEquals("SoftButtonObject id doesn't match the expected value", 50, sbo1.getButtonId());
         assertEquals("SoftButtonObject id doesn't match the expected value", 101, sbo2.getButtonId());
diff --git a/android/sdl_android/src/androidTest/java/com/smartdevicelink/managers/screen/SoftButtonManagerTests.java b/android/sdl_android/src/androidTest/java/com/smartdevicelink/managers/screen/SoftButtonManagerTests.java
index becc01a74..67e7384e9 100644
--- a/android/sdl_android/src/androidTest/java/com/smartdevicelink/managers/screen/SoftButtonManagerTests.java
+++ b/android/sdl_android/src/androidTest/java/com/smartdevicelink/managers/screen/SoftButtonManagerTests.java
@@ -44,6 +44,7 @@ import java.util.Arrays;
 import java.util.Collections;
 import java.util.List;
 
+
 import static junit.framework.TestCase.assertEquals;
 import static junit.framework.TestCase.assertFalse;
 import static junit.framework.TestCase.assertNull;
@@ -350,8 +351,8 @@ public class SoftButtonManagerTests {
         softButtonStateList.add(softButtonState1);
         softButtonStateList2.add(softButtonState1);
         softButtonStateList2.add(softButtonState2);
-        softButtonObject1 = new SoftButtonObject("hi", softButtonStateList, "Hi", null);
-        softButtonObject2 = new SoftButtonObject("hi", softButtonStateList2, "Hi", null);
+        softButtonObject1 = new SoftButtonObject("hi", softButtonStateList, softButtonStateList.get(0).getName(), null);
+        softButtonObject2 = new SoftButtonObject("hi", softButtonStateList2, softButtonStateList2.get(0).getName(), null);
         assertNotEquals(softButtonObject1, softButtonObject2);
 
         // Case 5: SoftButtonStates are not the same, assertFalse
@@ -365,8 +366,8 @@ public class SoftButtonManagerTests {
         assertNotEquals(softButtonObject1, softButtonObject2);
 
         // Case 7: SoftButtonObject currentStateName not same, assertFalse
-        softButtonObject1 = new SoftButtonObject("hi", softButtonStateList, "Hi", null);
-        softButtonObject2 = new SoftButtonObject("hi", softButtonStateList, "Hi2", null);
+        softButtonObject1 = new SoftButtonObject("hi", softButtonStateList2, softButtonStateList2.get(0).getName(), null);
+        softButtonObject2 = new SoftButtonObject("hi", softButtonStateList2, softButtonStateList2.get(1).getName(), null);
         assertNotEquals(softButtonObject1, softButtonObject2);
     }
 
@@ -402,4 +403,117 @@ public class SoftButtonManagerTests {
         softButtonState2 = new SoftButtonState("object1-state1", "o1s1", artwork1);
         assertEquals(softButtonState1, softButtonState2);
     }
+
+    /**
+     * Test constructing SoftButtonObject with an empty state list
+     */
+    @Test
+    public void testConstructSoftButtonObjectWithEmptyStateList() {
+        List<SoftButtonState> stateList = new ArrayList<>();
+        SoftButtonObject softButtonObject = new SoftButtonObject("hello_there", stateList, "general_kenobi", null);
+        assertNull(softButtonObject.getStates());
+    }
+
+    /**
+     * Test constructing SoftButtonObject with an nonempty state list
+     */
+    @Test
+    public void testConstructSoftButtonObjectWithNonEmptyStateList() {
+        List<SoftButtonState> stateList = new ArrayList<>();
+        SoftButtonState softButtonState = new SoftButtonState("general_kenobi", "General Kenobi", null);
+        stateList.add(softButtonState);
+        SoftButtonObject softButtonObject = new SoftButtonObject("hello_there", stateList, "general_kenobi", null);
+        assertEquals(stateList, softButtonObject.getStates());
+    }
+
+    /**
+     * Test constructing SoftButtonObject with an invalid initialStateName
+     */
+    @Test
+    public void testConstructSoftButtonObjectWithInvalidInitialStateName() {
+        List<SoftButtonState> stateList = new ArrayList<>();
+        SoftButtonState softButtonState = new SoftButtonState("general_kenobi", "General Kenobi", null);
+        stateList.add(softButtonState);
+        SoftButtonObject softButtonObject = new SoftButtonObject("hello_there", stateList, "hello_there", null);
+        assertNull(softButtonObject.getStates());
+    }
+
+    /**
+     * Test assigning an empty state list to existing SoftButtonObject
+     */
+    @Test
+    public void testAssignEmptyStateListToSoftButtonObject() {
+        List<SoftButtonState> nonEmptyStateList = new ArrayList<>();
+        List<SoftButtonState> emptyStateList = new ArrayList<>();
+        SoftButtonState softButtonState = new SoftButtonState("general_kenobi", "General Kenobi", null);
+        nonEmptyStateList.add(softButtonState);
+
+        SoftButtonObject softButtonObject = new SoftButtonObject("hello_there", nonEmptyStateList, "general_kenobi", null);
+
+        softButtonObject.setStates(emptyStateList);
+        assertEquals(nonEmptyStateList, softButtonObject.getStates());
+    }
+
+    /**
+     * Test assigning a state list with the current state to existing SoftButtonObject
+     */
+    @Test
+    public void testAssignStateListWithCurrentStateToSoftButtonObject() {
+        List<SoftButtonState> stateList1 = new ArrayList<>();
+        SoftButtonState softButtonState1 = new SoftButtonState("hello_there", "Hello there", null);
+        stateList1.add(softButtonState1);
+
+        List<SoftButtonState> stateList2 = new ArrayList<>();
+        SoftButtonState softButtonState2 = new SoftButtonState("general_kenobi", "General Kenobi", null);
+        stateList2.add(softButtonState1);
+        stateList2.add(softButtonState2);
+
+        SoftButtonObject softButtonObject = new SoftButtonObject("general_kenobi", stateList1, "hello_there", null);
+
+        softButtonObject.setStates(stateList2);
+
+        assertEquals(stateList2, softButtonObject.getStates());
+    }
+
+    /**
+     * Test assigning a state list without the current state to existing SoftButtonObject
+     */
+    @Test
+    public void testAssignStateListWithoutCurrentStateToSoftButtonObject() {
+        List<SoftButtonState> stateList1 = new ArrayList<>();
+        SoftButtonState softButtonState1 = new SoftButtonState("hello_there", "Hello there", null);
+        stateList1.add(softButtonState1);
+
+        List<SoftButtonState> stateList2 = new ArrayList<>();
+        SoftButtonState softButtonState2 = new SoftButtonState("general_kenobi", "General Kenobi", null);
+        stateList2.add(softButtonState2);
+
+        SoftButtonObject softButtonObject = new SoftButtonObject("general_kenobi", stateList1, "hello_there", null);
+
+        softButtonObject.setStates(stateList2);
+
+        assertEquals(stateList2, softButtonObject.getStates());
+    }
+
+    /**
+     * Test assigning a state list with states that have the same name to existing SoftButtonObject
+     */
+    @Test
+    public void testAssignSameNameStateListToSoftButtonObject() {
+        List<SoftButtonState> stateListUnique = new ArrayList<>();
+        SoftButtonState softButtonState1 = new SoftButtonState("hello_there", "Hello there", null);
+        stateListUnique.add(softButtonState1);
+
+        List<SoftButtonState> stateListDuplicateNames = new ArrayList<>();
+        SoftButtonState softButtonState2 = new SoftButtonState("general_kenobi", "General Kenobi", null);
+        stateListDuplicateNames.add(softButtonState2);
+        SoftButtonState softButtonState3 = new SoftButtonState("general_kenobi", "General Kenobi Again", null);
+        stateListDuplicateNames.add(softButtonState3);
+
+        SoftButtonObject softButtonObject = new SoftButtonObject("general_kenobi", stateListUnique, "hello_there", null);
+
+        softButtonObject.setStates(stateListDuplicateNames);
+
+        assertEquals(stateListUnique, softButtonObject.getStates());
+    }
 }
diff --git a/base/src/main/java/com/smartdevicelink/managers/screen/SoftButtonObject.java b/base/src/main/java/com/smartdevicelink/managers/screen/SoftButtonObject.java
index 68d486080..9eac0a1ff 100644
--- a/base/src/main/java/com/smartdevicelink/managers/screen/SoftButtonObject.java
+++ b/base/src/main/java/com/smartdevicelink/managers/screen/SoftButtonObject.java
@@ -38,7 +38,6 @@ import com.smartdevicelink.proxy.rpc.OnButtonEvent;
 import com.smartdevicelink.proxy.rpc.OnButtonPress;
 import com.smartdevicelink.proxy.rpc.SoftButton;
 import com.smartdevicelink.util.DebugTool;
-
 import java.util.Collections;
 import java.util.List;
 
@@ -73,12 +72,24 @@ public class SoftButtonObject implements Cloneable {
      */
     public SoftButtonObject(@NonNull String name, @NonNull List<SoftButtonState> states, @NonNull String initialStateName, OnEventListener onEventListener) {
 
-        // Make sure there aren't two states with the same name
-        if (hasTwoStatesOfSameName(states)) {
-            DebugTool.logError(TAG, "Two states have the same name in states list for soft button object");
-            return;
+        boolean repeatedStateNames = hasTwoStatesOfSameName(states);
+
+        boolean hasStateWithInitialName = false;
+        for (SoftButtonState state : states) {
+            if(state.getName().equals(initialStateName)) {
+                hasStateWithInitialName = true;
+                break;
+            }
         }
 
+        if (repeatedStateNames) {
+            DebugTool.logError(TAG, "A SoftButtonObject must have states with different names.");
+            return;
+        }
+        if (!hasStateWithInitialName) {
+            DebugTool.logError(TAG, "A SoftButtonObject must have a state with initialStateName.");
+            return;
+        }
         this.name = name;
         this.states = states;
         this.currentStateName = initialStateName;
@@ -264,6 +275,30 @@ public class SoftButtonObject implements Cloneable {
      * @param states a list of the object's soft button states. <strong>states should be unique for every SoftButtonObject. A SoftButtonState instance cannot be reused for multiple SoftButtonObjects.</strong>
      */
     public void setStates(@NonNull List<SoftButtonState> states) {
+
+        boolean repeatedStateNames = hasTwoStatesOfSameName(states);
+
+        if (repeatedStateNames) {
+            DebugTool.logError(TAG, "A SoftButtonObject must have states with different names.");
+            return;
+        }
+
+        if (states.isEmpty()) {
+            DebugTool.logError(TAG, "A SoftButtonObject must contain at least one state");
+            return;
+        }
+
+        boolean hasStateWithCurrentName = false;
+        for (SoftButtonState state : states) {
+            if(state.getName().equals(currentStateName)) {
+                hasStateWithCurrentName = true;
+                break;
+            }
+        }
+        if (!hasStateWithCurrentName) {
+            DebugTool.logError(TAG, "A SoftButtonObject setting states must contain a state with the name " + currentStateName + ".");
+        }
+
         this.states = states;
     }