There is a bug in the onClick handler in SelectionManager. We assume the row index is 1:1 with the available table objects. However, this is not true in two cases, at least:
1) Re-sorting the table causes the "current" row to be sent in, but the underlying object collection does not change its order. 2) When metahosts are added to the table, they are not present in the available table. We therefore end up altering arbitrary available host row objects, which leads to eventual errors in execution. To fix this, associated the "id" of the underlying row object with the widget, and notify listeners onClick that a given "id" has been updated to a given "profile". Add an implementation of the onClick handler in HostSelector that updates the selectedHostData, looked-up by that "id". Stop modifying the SelectionManager's row objects, which represent the available table's rows. Signed-off-by: Nishanth Aravamudan <[email protected]> --- I'm not 100% happy with this patch, as I feel like it breaks some of the abstraction between the HostSelector and SelectionManager. But I don't really see any way around it. I'm passing some pretty specific values to the onClick handler -- I suppose we could also just associate the row object itself to the widget and then just pass the object in with the new property (perhaps even as a dictionary, rather than just assuming only the profile is being updated?). That can be done as a follow-on fix, though, I think. It's better to fix the bug ASAP. diff --git a/frontend/client/src/autotest/afe/HostSelector.java b/frontend/client/src/autotest/afe/HostSelector.java index 8b75056..413fc1b 100644 --- a/frontend/client/src/autotest/afe/HostSelector.java +++ b/frontend/client/src/autotest/afe/HostSelector.java @@ -18,6 +18,7 @@ import com.google.gwt.json.client.JSONArray; import com.google.gwt.json.client.JSONNumber; import com.google.gwt.json.client.JSONObject; import com.google.gwt.json.client.JSONString; +import com.google.gwt.json.client.JSONValue; import com.google.gwt.user.client.ui.Anchor; import com.google.gwt.user.client.ui.HasText; import com.google.gwt.user.client.ui.HasValue; @@ -121,6 +122,17 @@ public class HostSelector implements ClickHandler { } selectionRefresh(); } + + public void onClick(JSONValue id, String profile) { + for (JSONObject row : selectedHostData.getItems()) { + if (row.get("id").equals(id)) { + selectedHostData.removeItem(row); + row.put("profile", new JSONString(profile)); + selectedHostData.addItem(row); + break; + } + } + } }); selectedTable.addListener(new DynamicTableListener() { diff --git a/frontend/client/src/autotest/afe/TestSelector.java b/frontend/client/src/autotest/afe/TestSelector.java index 5deddb9..f671937 100644 --- a/frontend/client/src/autotest/afe/TestSelector.java +++ b/frontend/client/src/autotest/afe/TestSelector.java @@ -16,6 +16,7 @@ import com.google.gwt.event.dom.client.ChangeEvent; import com.google.gwt.event.dom.client.ChangeHandler; import com.google.gwt.json.client.JSONArray; import com.google.gwt.json.client.JSONObject; +import com.google.gwt.json.client.JSONValue; import com.google.gwt.user.client.ui.Composite; import com.google.gwt.user.client.ui.HasHTML; import com.google.gwt.user.client.ui.Widget; @@ -239,4 +240,6 @@ public class TestSelector extends Composite implements DataTableListener, Change selectedTests.removeAll(objects); notifyListener(); } + + public void onClick(JSONValue id, String profile) { } } diff --git a/frontend/client/src/autotest/common/table/SelectionManager.java b/frontend/client/src/autotest/common/table/SelectionManager.java index 2e65ee9..8015101 100644 --- a/frontend/client/src/autotest/common/table/SelectionManager.java +++ b/frontend/client/src/autotest/common/table/SelectionManager.java @@ -9,6 +9,7 @@ import autotest.common.ui.TableSelectionPanel.SelectionPanelListener; import com.google.gwt.json.client.JSONObject; import com.google.gwt.json.client.JSONArray; import com.google.gwt.json.client.JSONString; +import com.google.gwt.json.client.JSONValue; import com.google.gwt.user.client.ui.CheckBox; import com.google.gwt.user.client.ui.ListBox; import com.google.gwt.user.client.ui.Widget; @@ -39,6 +40,7 @@ public class SelectionManager implements TableWidgetFactory, TableWidgetClickLis public interface SelectionListener { public void onAdd(Collection<JSONObject> objects); public void onRemove(Collection<JSONObject> objects); + public void onClick(JSONValue id, String profile); } public interface SelectableRowFilter { @@ -182,6 +184,7 @@ public class SelectionManager implements TableWidgetFactory, TableWidgetClickLis } if (type == DataTable.WidgetType.ListBox) { + TableClickWidget w; int i; ListBox listBox = new ListBox(); listBox.setVisibleItemCount(1); @@ -197,18 +200,25 @@ public class SelectionManager implements TableWidgetFactory, TableWidgetClickLis if (rowObject.containsKey("profile")) { s = rowObject.get("profile").isString().toString(); } else { - s = rowObject.get("current_profile").isString().toString(); + if (rowObject.containsKey("current_profile")) { + s = rowObject.get("current_profile").isString().toString(); + } else { + // when we are using a metahost or select-by-hostname, we + // are putting all profiles in, so just select the first one + s = profiles.get(0).toString(); + } } s = s.substring(1, s.length() - 1); for (i = 0; i < listBox.getItemCount(); i++) { - if (listBox.getItemText(i) == s) { + if (listBox.getItemText(i).equals(s)) { rowObject.put("profile", new JSONString(listBox.getItemText(i))); listBox.setSelectedIndex(i); break; } } - attachedTable.setRow(row, rowObject); - return new TableClickWidget(listBox, this, row, cell); + w = new TableClickWidget(listBox, this, row, cell); + w.setAssociatedId(rowObject.get("id")); + return w; } else { // Should really check if type is CheckBox, but make this the default // scenario if type != DataTable.WidgetType.ListBox @@ -226,9 +236,9 @@ public class SelectionManager implements TableWidgetFactory, TableWidgetClickLis refreshSelection(); } else { ListBox l = (ListBox)widget.getContainedWidget(); - JSONObject row = attachedTable.getRow(widget.getRow()); - row.put("profile", new JSONString(l.getItemText(l.getSelectedIndex()))); - attachedTable.setRow(widget.getRow(), row); + for (SelectionListener listener : listeners) { + listener.onClick(widget.getAssociatedId(), l.getItemText(l.getSelectedIndex())); + } } } diff --git a/frontend/client/src/autotest/common/table/TableClickWidget.java b/frontend/client/src/autotest/common/table/TableClickWidget.java index f2b4b5d..31071c7 100644 --- a/frontend/client/src/autotest/common/table/TableClickWidget.java +++ b/frontend/client/src/autotest/common/table/TableClickWidget.java @@ -2,6 +2,7 @@ package autotest.common.table; import com.google.gwt.event.dom.client.ClickEvent; import com.google.gwt.event.dom.client.ClickHandler; +import com.google.gwt.json.client.JSONValue; import com.google.gwt.user.client.ui.Composite; import com.google.gwt.user.client.ui.FocusWidget; @@ -12,6 +13,7 @@ public class TableClickWidget extends Composite implements ClickHandler { private TableWidgetClickListener listener; private int row; private int cell; + private JSONValue associatedId; public static interface TableWidgetClickListener { public void onClick(TableClickWidget widget); @@ -43,4 +45,12 @@ public class TableClickWidget extends Composite implements ClickHandler { public FocusWidget getContainedWidget() { return widget; } + + public void setAssociatedId(JSONValue id) { + this.associatedId = id; + } + + public JSONValue getAssociatedId() { + return this.associatedId; + } } _______________________________________________ Autotest-kernel mailing list [email protected] https://www.redhat.com/mailman/listinfo/autotest-kernel
