6200 and 6201: [easyForm] fix incorrect value and type (#6206)

* #6200: fix reporting value in easy form list

* #6201: fix type of List Single

kernel/base/src/main/java/com/twosigma/beakerx/easyform/EasyForm.java

@@ -36,14 +36,13 @@
 import org.apache.commons.lang3.StringUtils;
 
 import java.util.Collection;
+import java.util.Collections;
 import java.util.HashMap;
 import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Map;
 import java.util.stream.Collectors;
 
-import static java.util.Collections.singletonList;
-
 @SuppressWarnings("unchecked")
 public class EasyForm extends ObservableMap<String, Object> implements DisplayableWidget {
 
@@ -177,11 +176,7 @@ public EasyFormComponent addList(final String label,
     list.setSize(size);
     list.setMultipleSelection(multipleSelection);
     list.setValues(values);
-    if (values != null && values.size() > 0 && multipleSelection) {
-      list.setValue(singletonList(values.iterator().next()));
-    } else {
-      list.setValue(values.iterator().next());
-    }
+    list.setValue(Collections.EMPTY_LIST);
     return addComponentOrThrow(label, list);
   }
 

kernel/base/src/main/java/com/twosigma/beakerx/easyform/formitem/widgets/SelectMultipleSingleWidget.java

@@ -18,17 +18,20 @@
 import com.twosigma.beakerx.easyform.formitem.ListComponent;
 import com.twosigma.beakerx.widgets.selections.SelectMultipleSingle;
 
+import java.util.ArrayList;
 import java.util.Collection;
 
+import static java.util.Arrays.asList;
+
 public class SelectMultipleSingleWidget extends ListComponent<SelectMultipleSingle> {
 
   public SelectMultipleSingleWidget() {
     super(new SelectMultipleSingle());
   }
 
   @Override
-  public String getValue() {
-    return this.widget.getValue();
+  public Object getValue() {
+    return new ArrayList<>(asList(this.widget.getValue()));
   }
 
   @Override

kernel/base/src/main/java/com/twosigma/beakerx/widgets/selections/MultipleSelectionWidget.java

@@ -54,7 +54,7 @@ public String[] updateValueFromObject(Object input) {
     return collect.toArray(new String[collect.size()]);
   }
 
-  private Collection<Integer> indexes(Object input) {
+  protected Collection<Integer> indexes(Object input) {
     if (input instanceof Object[]) {
       return Arrays.stream((Object[]) input).map(i -> (Integer) i).collect(Collectors.toList());
     } else if (input instanceof List) {

kernel/base/src/main/java/com/twosigma/beakerx/widgets/selections/SelectMultipleSingle.java

@@ -17,16 +17,29 @@
 
 import com.twosigma.beakerx.widgets.BeakerxWidget;
 
-public class SelectMultipleSingle extends SingleSelectionWidget {
+import java.util.Collection;
+
+import static java.util.Collections.singletonList;
+
+public class SelectMultipleSingle extends MultipleSelectionWidget {
 
   public static String VIEW_NAME_VALUE = "SelectMultipleSingleView";
   public static String MODEL_NAME_VALUE = "SelectMultipleSingleModel";
 
   public SelectMultipleSingle() {
     super();
+    this.defaultIndex = -1;
     openComm();
   }
 
+  @Override
+  protected Collection<Integer> indexes(Object input) {
+    if (input instanceof Integer) {
+      return singletonList((Integer) input);
+    }
+    throw new RuntimeException("Index for SelectMultipleSingle should be Integer, your index: " + input);
+  }
+
   @Override
   public String getModelModuleValue() {
     return BeakerxWidget.MODEL_MODULE_VALUE;

kernel/base/src/main/java/com/twosigma/beakerx/widgets/selections/SelectionWidget.java

@@ -16,6 +16,7 @@
 package com.twosigma.beakerx.widgets.selections;
 
 import com.twosigma.beakerx.widgets.ValueWidget;
+
 import java.io.Serializable;
 import java.util.HashMap;
 
@@ -26,6 +27,7 @@ public abstract class SelectionWidget<T extends Serializable> extends ValueWidge
 
   private String[] options = new String[0];
   private Integer size;
+  protected int defaultIndex = 0;
 
   @Override
   protected HashMap<String, Serializable> content(HashMap<String, Serializable> content) {
@@ -42,11 +44,11 @@ public String[] getOptions() {
   public void setOptions(Object options) {
     this.options = getStringArray(options);
     sendUpdate(OPTIONS_LABELS, options);
-    sendUpdate(INDEX, 0);
+    sendUpdate(INDEX, defaultIndex);
   }
 
   public Integer getSelectedOptionIndex(String option) {
-    for (int i=0; options.length > 0 && i<options.length+1; i++) {
+    for (int i = 0; options.length > 0 && i < options.length + 1; i++) {
       if (options[i].equals(option)) {
         return i;
       }

kernel/base/src/test/java/com/twosigma/beakerx/easyform/EasyFormTest.java

@@ -48,6 +48,7 @@
 import java.util.List;
 import java.util.Map;
 
+import org.assertj.core.api.Assertions;
 import org.junit.After;
 import org.junit.Before;
 import org.junit.Test;
@@ -153,14 +154,15 @@ public void shouldCreateEasyFormWithMultipleSelection() throws Exception {
     easyForm.addList(label, asList("1", "2", "3"));
     easyForm.display();
     //then
-    verifyMultipleSelection(kernel.getPublishedMessages());
+    Object multipleSelectionValue = easyForm.get(label);
+    verifyMultipleSelection(kernel.getPublishedMessages(), (List) multipleSelectionValue);
     verifyEasyForm(kernel.getPublishedMessages(), easyForm.getCommFunctionalities());
     verifyDisplayMsg(kernel.getPublishedMessages());
   }
 
-  private void verifyMultipleSelection(List<Message> messages) {
-    verifyInternalOpenCommMsgWitLayout(messages, SelectMultiple.MODEL_NAME_VALUE,
-            SelectMultiple.VIEW_NAME_VALUE);
+  private void verifyMultipleSelection(List<Message> messages, List multipleSelectionValue) {
+    verifyInternalOpenCommMsgWitLayout(messages, SelectMultiple.MODEL_NAME_VALUE, SelectMultiple.VIEW_NAME_VALUE);
+    Assertions.assertThat(multipleSelectionValue).isEmpty();
   }
 
   @Test
@@ -172,13 +174,16 @@ public void shouldCreateEasyFormWithSelectMultipleSingle() throws Exception {
     easyForm.addList(label, asList("1", "2", "3"), Boolean.FALSE);
     easyForm.display();
     //then
-    verifySelectMultipleSingle(kernel.getPublishedMessages());
+    Object multipleSelectionSingleValue = easyForm.get(label);
+    verifySelectMultipleSingle(kernel.getPublishedMessages(), (List) multipleSelectionSingleValue);
     verifyEasyForm(kernel.getPublishedMessages(), easyForm.getCommFunctionalities());
     verifyDisplayMsg(kernel.getPublishedMessages());
   }
-  private void verifySelectMultipleSingle(List<Message> messages) {
+
+  private void verifySelectMultipleSingle(List<Message> messages, List multipleSelectionSingleValue) {
     verifyInternalOpenCommMsgWitLayout(messages, SelectMultipleSingle.MODEL_NAME_VALUE,
             SelectMultipleSingle.VIEW_NAME_VALUE);
+    Assertions.assertThat(multipleSelectionSingleValue).isEmpty();
   }
 
   @Test

kernel/base/src/test/java/com/twosigma/beakerx/easyform/formitem/widgets/SelectMultipleSingleWidgetTest.java

@@ -18,9 +18,11 @@
 import com.twosigma.beakerx.easyform.EasyFormComponent;
 import com.twosigma.beakerx.widgets.selections.SelectMultiple;
 import com.twosigma.beakerx.widgets.selections.SelectMultipleSingle;
-import org.assertj.core.api.Assertions;
 import org.junit.Test;
 
+import java.util.Collections;
+import java.util.List;
+
 import static com.twosigma.beakerx.widgets.TestWidgetUtils.verifyMsgForProperty;
 import static java.util.Arrays.asList;
 import static org.assertj.core.api.Assertions.assertThat;
@@ -30,15 +32,15 @@ public class SelectMultipleSingleWidgetTest extends EasyFormWidgetTest {
   @Test
   public void setValue() throws Exception {
     //given
-    String newValue = "2";
+    List<String> newValue = Collections.singletonList("2");
     SelectMultipleSingleWidget widget = new SelectMultipleSingleWidget();
     widget.setValues(asList("1", "2", "3"));
     kernel.clearPublishedMessages();
     //when
     widget.setValue(newValue);
     //then
-    verifyMsgForProperty(kernel, SelectMultiple.VALUE, "2");
-    assertThat(widget.getValue()).isEqualTo("2");
+    verifyMsgForProperty(kernel, SelectMultiple.VALUE, newValue);
+    assertThat(widget.getValue()).isEqualTo(newValue);
   }
 
   @Test

kernel/base/src/test/java/com/twosigma/beakerx/widgets/selections/SelectMultipleSingleTest.java

@@ -17,14 +17,19 @@
 
 import com.twosigma.beakerx.KernelTest;
 import com.twosigma.beakerx.kernel.KernelManager;
+import com.twosigma.beakerx.widgets.Widget;
 import org.junit.After;
 import org.junit.Before;
 import org.junit.Test;
 
 import java.security.NoSuchAlgorithmException;
 
+import static com.twosigma.beakerx.widgets.TestWidgetUtils.getValueForProperty;
 import static com.twosigma.beakerx.widgets.TestWidgetUtils.verifyInternalOpenCommMsgWitLayout;
 import static com.twosigma.beakerx.widgets.TestWidgetUtils.verifyMsgForProperty;
+import static java.util.Arrays.asList;
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.junit.Assert.fail;
 
 public class SelectMultipleSingleTest {
 
@@ -41,6 +46,43 @@ public void tearDown() throws Exception {
     KernelManager.register(null);
   }
 
+  @Test
+  public void noSelection() throws Exception {
+    SelectMultipleSingle widget = selectMultipleSingle();
+    kernel.clearPublishedMessages();
+    //when
+    widget.setOptions(new String[]{"1", "2", "3"});
+    //then
+    Integer index = getValueForProperty(kernel.getPublishedMessages().get(1), Widget.INDEX, Integer.class);
+    assertThat(index).isEqualTo(-1);
+  }
+
+  @Test
+  public void updateValue() throws Exception {
+    SelectMultipleSingle widget = selectMultipleSingle();
+    widget.setOptions(new String[]{"1", "2", "3"});
+    kernel.clearPublishedMessages();
+    //when
+    widget.updateValue(1);
+    //then
+    assertThat(widget.getValue()).isEqualTo(new String[]{"2"});
+  }
+
+  @Test
+  public void shouldNotUpdateValueWhenArrayOfIndexes() throws Exception {
+    SelectMultipleSingle widget = selectMultipleSingle();
+    widget.setOptions(new String[]{"1", "2", "3"});
+    kernel.clearPublishedMessages();
+    //when
+    try {
+      widget.updateValue(asList(1, 2));
+      fail("should not update value when list of indexes");
+    } catch (Exception e) {
+      // then
+      // test passes
+    }
+  }
+
   @Test
   public void shouldSendCommOpenWhenCreate() throws Exception {
     //given
@@ -54,7 +96,7 @@ public void shouldSendCommOpenWhenCreate() throws Exception {
   public void shouldSendCommMsgWhenValueChange() throws Exception {
     //given
     SelectMultipleSingle widget = selectMultipleSingle();
-    widget.setOptions(new String[]{"1","2", "3"});
+    widget.setOptions(new String[]{"1", "2", "3"});
     kernel.clearPublishedMessages();
     //when
     widget.setValue("2");