This is an automated email from the ASF dual-hosted git repository.
hansva pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/hop.git
The following commit(s) were added to refs/heads/main by this push:
new 6402b5cf62 fix tableview issue, fixes #7843 (#7846)
6402b5cf62 is described below
commit 6402b5cf6264d47b84fbe30e536d08875561fcee
Author: Hans Van Akelyen <[email protected]>
AuthorDate: Fri Aug 7 20:35:30 2026 +0200
fix tableview issue, fixes #7843 (#7846)
---
.../SelectValuesDialogMetaTabTest.java | 249 +++++++++++++++++++++
.../ui/core/widget/TableViewLeaveEditorTest.java | 220 ++++++++++++++++++
.../HopGuiPipelineGraphHopCreationTest.java | 216 +++++++++++++++++-
.../org/apache/hop/ui/core/widget/TableView.java | 33 +--
.../hopgui/file/pipeline/HopGuiPipelineGraph.java | 19 +-
.../hopgui/file/workflow/HopGuiWorkflowGraph.java | 13 +-
6 files changed, 724 insertions(+), 26 deletions(-)
diff --git
a/plugins/transforms/selectvalues/src/test/java/org/apache/hop/pipeline/transforms/selectvalues/SelectValuesDialogMetaTabTest.java
b/plugins/transforms/selectvalues/src/test/java/org/apache/hop/pipeline/transforms/selectvalues/SelectValuesDialogMetaTabTest.java
new file mode 100644
index 0000000000..7d4f931f32
--- /dev/null
+++
b/plugins/transforms/selectvalues/src/test/java/org/apache/hop/pipeline/transforms/selectvalues/SelectValuesDialogMetaTabTest.java
@@ -0,0 +1,249 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hop.pipeline.transforms.selectvalues;
+
+import static
org.eclipse.swtbot.swt.finder.matchers.WidgetMatcherFactory.widgetOfType;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertNotNull;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+import java.util.Arrays;
+import java.util.concurrent.atomic.AtomicReference;
+import java.util.function.Consumer;
+import org.apache.hop.core.plugins.PluginRegistry;
+import org.apache.hop.core.plugins.TransformPluginType;
+import org.apache.hop.core.variables.Variables;
+import org.apache.hop.i18n.BaseMessages;
+import org.apache.hop.pipeline.PipelineMeta;
+import org.apache.hop.pipeline.transform.TransformMeta;
+import org.apache.hop.ui.testing.SwtBotTestBase;
+import org.eclipse.swt.SWT;
+import org.eclipse.swt.custom.CTabFolder;
+import org.eclipse.swt.custom.CTabItem;
+import org.eclipse.swt.graphics.Rectangle;
+import org.eclipse.swt.widgets.Combo;
+import org.eclipse.swt.widgets.Control;
+import org.eclipse.swt.widgets.Event;
+import org.eclipse.swt.widgets.Shell;
+import org.eclipse.swt.widgets.Table;
+import org.eclipse.swt.widgets.TableItem;
+import org.eclipse.swtbot.swt.finder.SWTBot;
+import org.eclipse.swtbot.swt.finder.widgets.SWTBotTable;
+import org.junit.jupiter.api.Tag;
+import org.junit.jupiter.api.Test;
+
+/**
+ * SWTBot coverage for the Meta-data tab of {@link SelectValuesDialog}, in
particular for what
+ * happens to the cell being edited when the user leaves it by clicking the
empty area under the
+ * grid.
+ *
+ * <p>The Format column is a {@code COLUMN_TYPE_FORMAT} column, edited with
the same combo as a
+ * {@code COLUMN_TYPE_CCOMBO} column - but the TableView mouse listener that
commits an open editor
+ * before handling the click only knows the TEXT and CCOMBO types. The format
combo is therefore
+ * still open, and still what TableView's shared {@code combo} field points
at, when the click below
+ * the rows moves the active cell to the Fieldname column and inserts a row;
the commit that the
+ * insert then does reads that stale combo and writes the mask into the
Fieldname cell. {@link
+ * #editingTheFormatCellMustNotLeakIntoTheFieldnameColumn()} pins that down;
{@link
+ * #editingTheTypeCellStaysInTheTypeColumn()} is the same scene on a plain
CCOMBO column, which is
+ * committed correctly today.
+ *
+ * <p>Tagged {@code uitest} so it is skipped on headless machines; run with
{@code mvn -pl
+ * plugins/transforms/selectvalues -Puitest test}.
+ */
+@Tag("uitest")
+class SelectValuesDialogMetaTabTest extends SwtBotTestBase {
+
+ private static final String TRANSFORM_NAME = "select values";
+
+ private static final String DIALOG_TITLE =
+ BaseMessages.getString(SelectValuesMeta.class,
"SelectValuesDialog.Shell.Label");
+ private static final String META_TAB =
+ BaseMessages.getString(SelectValuesMeta.class,
"SelectValuesDialog.MetaTab.TabItem");
+ private static final String FORMAT_HEADER =
+ BaseMessages.getString(SelectValuesMeta.class,
"SelectValuesDialog.ColumnInfo.Format");
+
+ /** Grid layout of the Meta-data tab: column 0 holds the row number, the
fields follow. */
+ private static final int FIELDNAME_COLUMN = 1;
+
+ private static final int TYPE_COLUMN = 3;
+ private static final int FORMAT_COLUMN = 7;
+
+ /** The Meta-data tab is the third of the dialog's tabs. */
+ private static final int META_TAB_INDEX = 2;
+
+ private static final String MASK = "MM/dd/yyyy HH:mm:ss";
+
+ /**
+ * Issue: typing a format mask and then clicking outside the editor copies
the mask into the
+ * Fieldname column of another row.
+ */
+ @Test
+ void editingTheFormatCellMustNotLeakIntoTheFieldnameColumn() {
+ SelectValuesMeta meta = metaWith(change("sdfsdfsdf", "Timestamp"),
change("other", "String"));
+
+ withDialog(
+ openerFor(meta),
+ bot -> {
+ SWTBot dialog = dialogBot(bot);
+ activateTheMetaTab(dialog);
+
+ SWTBotTable grid = metaGrid(dialog);
+ grid.click(0, FORMAT_COLUMN);
+ typeIntoTheOpenComboEditor(grid.widget, MASK);
+
+ clickTheEmptyAreaUnderTheGrid(grid.widget);
+
+ assertEquals(
+ MASK, grid.cell(0, FORMAT_COLUMN), "the mask belongs to the cell
it was typed in");
+ assertEquals(
+ "other",
+ grid.cell(1, FIELDNAME_COLUMN),
+ "leaving the format editor must not overwrite another row's
fieldname");
+
+ dialog.button(buttonLabel("System.Button.Cancel")).click();
+ });
+ }
+
+ /** The same scene on a plain CCOMBO column, which TableView does commit
before the click. */
+ @Test
+ void editingTheTypeCellStaysInTheTypeColumn() {
+ SelectValuesMeta meta = metaWith(change("sdfsdfsdf", "Timestamp"),
change("other", "String"));
+
+ withDialog(
+ openerFor(meta),
+ bot -> {
+ SWTBot dialog = dialogBot(bot);
+ activateTheMetaTab(dialog);
+
+ SWTBotTable grid = metaGrid(dialog);
+ grid.click(0, TYPE_COLUMN);
+ typeIntoTheOpenComboEditor(grid.widget, "Date");
+
+ clickTheEmptyAreaUnderTheGrid(grid.widget);
+
+ assertEquals(
+ "Date", grid.cell(0, TYPE_COLUMN), "the type belongs to the cell
it was typed in");
+ assertEquals(
+ "other",
+ grid.cell(1, FIELDNAME_COLUMN),
+ "leaving the type editor must not overwrite another row's
fieldname");
+
+ dialog.button(buttonLabel("System.Button.Cancel")).click();
+ });
+ }
+
+ private SWTBot dialogBot(SWTBot bot) {
+ return bot.shell(DIALOG_TITLE).activate().bot();
+ }
+
+ /**
+ * Brings the Meta-data tab to the front. Done on the folder rather than
through {@code
+ * bot.cTabItem(...)} because SWTBot's widget finder only walks controls and
never sees the tab
+ * items of a dialog's CTabFolder.
+ */
+ private void activateTheMetaTab(SWTBot dialog) {
+ CTabFolder tabFolder = (CTabFolder)
dialog.widget(widgetOfType(CTabFolder.class));
+ display.syncExec(
+ () -> {
+ CTabItem metaTab = tabFolder.getItem(META_TAB_INDEX);
+ // The look-and-feel pads tab labels with spaces, hence the trim().
+ assertEquals(META_TAB, metaTab.getText().trim(), "expected the
Meta-data tab");
+ tabFolder.setSelection(metaTab);
+ tabFolder.layout();
+ });
+ }
+
+ /**
+ * The Meta-data grid. Every tab of the dialog holds one, but SWTBot only
looks at visible
+ * controls, so with the Meta-data tab up front this one is the only grid it
finds.
+ */
+ private SWTBotTable metaGrid(SWTBot dialog) {
+ SWTBotTable grid = dialog.table();
+ assertEquals(
+ FORMAT_HEADER,
+ grid.columns().get(FORMAT_COLUMN),
+ "expected the Meta-data grid, whose column " + FORMAT_COLUMN + " is
the format mask");
+ return grid;
+ }
+
+ /**
+ * Writes into the combo the click on a cell opened, which is what typing in
it amounts to: the
+ * TableView modify listener copies the text into the cell as it is entered.
+ */
+ private void typeIntoTheOpenComboEditor(Table table, String value) {
+ AtomicReference<Combo> editor = new AtomicReference<>();
+ display.syncExec(
+ () -> {
+ for (Control child : table.getChildren()) {
+ if (child instanceof Combo combo) {
+ editor.set(combo);
+ }
+ }
+ });
+ assertNotNull(editor.get(), "clicking the cell should have opened a combo
editor on the grid");
+ display.syncExec(() -> editor.get().setText(value));
+ }
+
+ /**
+ * Clicks the empty space below the last row - the "click outside the
editor" of the report. The
+ * grid answers that with a new row, which is where the still-open editor's
text can end up.
+ */
+ private void clickTheEmptyAreaUnderTheGrid(Table table) {
+ display.syncExec(
+ () -> {
+ TableItem lastRow = table.getItem(table.getItemCount() - 1);
+ Rectangle bounds = lastRow.getBounds(FIELDNAME_COLUMN);
+ Event click = new Event();
+ click.widget = table;
+ click.button = 1;
+ click.count = 1;
+ click.x = bounds.x + bounds.width / 2;
+ click.y = bounds.y + bounds.height + bounds.height / 2;
+ assertTrue(
+ table.getClientArea().contains(click.x, click.y),
+ "the grid must be tall enough to have empty space under its
rows");
+ table.notifyListeners(SWT.MouseDown, click);
+ });
+ }
+
+ private Consumer<Shell> openerFor(SelectValuesMeta meta) {
+ PipelineMeta pipelineMeta = pipelineWith(meta);
+ return parent -> new SelectValuesDialog(parent, new Variables(), meta,
pipelineMeta).open();
+ }
+
+ private static SelectMetadataChange change(String name, String type) {
+ SelectMetadataChange change = new SelectMetadataChange();
+ change.setName(name);
+ change.setType(type);
+ return change;
+ }
+
+ private static SelectValuesMeta metaWith(SelectMetadataChange... changes) {
+ SelectValuesMeta meta = new SelectValuesMeta();
+ meta.getSelectOption().getMeta().addAll(Arrays.asList(changes));
+ return meta;
+ }
+
+ private static PipelineMeta pipelineWith(SelectValuesMeta meta) {
+ String pluginId =
PluginRegistry.getInstance().getPluginId(TransformPluginType.class, meta);
+ assertNotNull(pluginId, "Select values transform must be registered via
HopEnvironment.init()");
+ PipelineMeta pipelineMeta = new PipelineMeta();
+ pipelineMeta.addTransform(new TransformMeta(pluginId, TRANSFORM_NAME,
meta));
+ return pipelineMeta;
+ }
+}
diff --git
a/rcp/src/test/java/org/apache/hop/ui/core/widget/TableViewLeaveEditorTest.java
b/rcp/src/test/java/org/apache/hop/ui/core/widget/TableViewLeaveEditorTest.java
new file mode 100644
index 0000000000..5cba616fb9
--- /dev/null
+++
b/rcp/src/test/java/org/apache/hop/ui/core/widget/TableViewLeaveEditorTest.java
@@ -0,0 +1,220 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hop.ui.core.widget;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertNotNull;
+
+import java.util.concurrent.atomic.AtomicReference;
+import java.util.function.BiConsumer;
+import java.util.function.Supplier;
+import org.apache.hop.core.variables.Variables;
+import org.apache.hop.ui.core.PropsUi;
+import org.apache.hop.ui.testing.SwtBotTestBase;
+import org.eclipse.swt.SWT;
+import org.eclipse.swt.events.SelectionAdapter;
+import org.eclipse.swt.graphics.Rectangle;
+import org.eclipse.swt.layout.FillLayout;
+import org.eclipse.swt.widgets.Composite;
+import org.eclipse.swt.widgets.Control;
+import org.eclipse.swt.widgets.Event;
+import org.eclipse.swt.widgets.TableItem;
+import org.eclipse.swt.widgets.Text;
+import org.eclipse.swtbot.swt.finder.SWTBot;
+import org.junit.jupiter.api.Tag;
+import org.junit.jupiter.api.Test;
+
+/**
+ * Covers what happens to an open cell editor when the user leaves it by
clicking the empty area
+ * under the grid - the click that makes the grid add a row.
+ *
+ * <p>That click first moves the active cell to column 1 of the last row and
only then commits, so
+ * an editor the commit does not recognise is applied to the cell that was
moved to rather than the
+ * one it belongs to. The commit has to know every editor {@code edit()} can
open; this pins down
+ * the {@code COLUMN_TYPE_TEXT_BUTTON} one, which is a {@link TextVarButton} -
a {@link TextVar},
+ * not the plain {@link Text} that the type mismatch makes {@code
getTextWidgetValue} cast to.
+ *
+ * <p>Tagged {@code uitest} so it is skipped on headless machines; run with
{@code mvn -pl rcp
+ * -Puitest test}.
+ */
+@Tag("uitest")
+class TableViewLeaveEditorTest extends SwtBotTestBase {
+
+ /** Grid layout: column 0 holds the row number, then the name and the
browsable value. */
+ private static final int NAME_COLUMN = 1;
+
+ private static final int GENERATOR_COLUMN = 2;
+
+ private static final int EDITED_ROW = 0;
+ private static final int LAST_ROW = 1;
+
+ private static final String TYPED = "uuid";
+
+ /**
+ * With a plain text column next to it the misapplied commit does not even
reach the wrong cell:
+ * it casts the browse editor to the widget type that column would have used.
+ */
+ @Test
+ void aTextButtonCellIsCommittedToItsOwnCellNextToAPlainColumn() {
+ assertTextButtonCellSurvivesLeavingTheEditor(false);
+ }
+
+ /** With a variables column next to it the same commit silently lands in the
wrong cell. */
+ @Test
+ void aTextButtonCellIsCommittedToItsOwnCellNextToAVariablesColumn() {
+ assertTextButtonCellSurvivesLeavingTheEditor(true);
+ }
+
+ private void assertTextButtonCellSurvivesLeavingTheEditor(boolean
nameColumnUsesVariables) {
+ withTableView(
+ nameColumnUsesVariables,
+ (tableView, bot) -> {
+ editCell(tableView, EDITED_ROW, GENERATOR_COLUMN);
+ setOpenEditorText(tableView, TYPED);
+
+ clickTheEmptyAreaUnderTheGrid(tableView);
+
+ assertEquals(
+ TYPED,
+ cellValue(tableView, EDITED_ROW, GENERATOR_COLUMN),
+ "what was typed belongs to the cell it was typed in");
+ assertEquals(
+ "second",
+ cellValue(tableView, LAST_ROW, NAME_COLUMN),
+ "leaving the editor must not overwrite another row's name");
+ });
+ }
+
+ // --- scene
-----------------------------------------------------------------------------------
+
+ /** Builds a two-row grid whose second column is browsable, the way e.g. the
Fake dialog is. */
+ private void withTableView(boolean nameColumnUsesVariables,
BiConsumer<TableView, SWTBot> body) {
+ AtomicReference<TableView> tableViewRef = new AtomicReference<>();
+ withScene(
+ shell -> {
+ shell.setLayout(new FillLayout());
+ shell.setSize(900, 320);
+ ColumnInfo[] columns = {
+ new ColumnInfo("Name", ColumnInfo.COLUMN_TYPE_TEXT, false, false),
+ new ColumnInfo("Generator", ColumnInfo.COLUMN_TYPE_TEXT_BUTTON,
false, false),
+ };
+ columns[0].setUsingVariables(nameColumnUsesVariables);
+ // A TEXT_BUTTON column only renders its browse button when the
column uses variables,
+ // so its editor is a TextVarButton rather than a plain Text.
+ columns[1].setUsingVariables(true);
+ columns[1].setTextVarButtonSelectionListener(new SelectionAdapter()
{});
+ TableView tableView =
+ new TableView(
+ new Variables(),
+ shell,
+ SWT.BORDER | SWT.FULL_SELECTION,
+ columns,
+ 2,
+ null,
+ PropsUi.getInstance());
+ TableItem first = tableView.table.getItem(EDITED_ROW);
+ first.setText(NAME_COLUMN, "first");
+ first.setText(GENERATOR_COLUMN, "random");
+ TableItem second = tableView.table.getItem(LAST_ROW);
+ second.setText(NAME_COLUMN, "second");
+ tableView.optWidth(true);
+ tableViewRef.set(tableView);
+ },
+ bot -> body.accept(tableViewRef.get(), bot));
+ }
+
+ // --- interactions
----------------------------------------------------------------------------
+
+ private void editCell(TableView tableView, int rowNr, int colNr) {
+ onUi(
+ () -> {
+ tableView.edit(rowNr, colNr);
+ return null;
+ });
+ }
+
+ /** Types into the editor the edit opened, wherever in the cell's holder it
lives. */
+ private void setOpenEditorText(TableView tableView, String value) {
+ onUi(
+ () -> {
+ Text editor = findTextIn(tableView.table);
+ assertNotNull(editor, "no editor is open on the grid");
+ editor.setText(value);
+ return null;
+ });
+ }
+
+ /**
+ * Clicks the empty space below the last row. The grid answers that with a
new row, and it is the
+ * commit done on the way there that can pick up the editor left open.
+ */
+ private void clickTheEmptyAreaUnderTheGrid(TableView tableView) {
+ onUi(
+ () -> {
+ TableItem lastRow =
tableView.table.getItem(tableView.table.getItemCount() - 1);
+ Rectangle bounds = lastRow.getBounds(NAME_COLUMN);
+ Event click = new Event();
+ click.widget = tableView.table;
+ click.button = 1;
+ click.count = 1;
+ click.x = bounds.x + bounds.width / 2;
+ click.y = bounds.y + bounds.height + bounds.height / 2;
+ tableView.table.notifyListeners(SWT.MouseDown, click);
+ return null;
+ });
+ }
+
+ // --- lookups
---------------------------------------------------------------------------------
+
+ private String cellValue(TableView tableView, int rowNr, int colNr) {
+ return onUi(() -> tableView.table.getItem(rowNr).getText(colNr));
+ }
+
+ private static Text findTextIn(Composite parent) {
+ for (Control child : parent.getChildren()) {
+ if (child instanceof Text text) {
+ return text;
+ }
+ if (child instanceof Composite composite) {
+ Text found = findTextIn(composite);
+ if (found != null) {
+ return found;
+ }
+ }
+ }
+ return null;
+ }
+
+ /** Runs {@code supplier} on the UI thread and hands its result back to the
SWTBot worker. */
+ private static <T> T onUi(Supplier<T> supplier) {
+ AtomicReference<T> result = new AtomicReference<>();
+ AtomicReference<RuntimeException> failure = new AtomicReference<>();
+ display.syncExec(
+ () -> {
+ try {
+ result.set(supplier.get());
+ } catch (RuntimeException e) {
+ failure.set(e);
+ }
+ });
+ if (failure.get() != null) {
+ throw failure.get();
+ }
+ return result.get();
+ }
+}
diff --git
a/rcp/src/test/java/org/apache/hop/ui/hopgui/file/pipeline/HopGuiPipelineGraphHopCreationTest.java
b/rcp/src/test/java/org/apache/hop/ui/hopgui/file/pipeline/HopGuiPipelineGraphHopCreationTest.java
index ee21a99d9b..d4adc8a136 100644
---
a/rcp/src/test/java/org/apache/hop/ui/hopgui/file/pipeline/HopGuiPipelineGraphHopCreationTest.java
+++
b/rcp/src/test/java/org/apache/hop/ui/hopgui/file/pipeline/HopGuiPipelineGraphHopCreationTest.java
@@ -23,19 +23,30 @@ import static
org.junit.jupiter.api.Assertions.assertNotNull;
import static org.junit.jupiter.api.Assertions.assertNull;
import static org.junit.jupiter.api.Assertions.assertTrue;
+import java.util.ArrayList;
+import java.util.HashSet;
import java.util.LinkedHashMap;
+import java.util.List;
import java.util.Map;
import java.util.Set;
import java.util.concurrent.atomic.AtomicReference;
+import java.util.function.Supplier;
import org.apache.hop.core.exception.HopException;
import org.apache.hop.core.gui.AreaOwner;
import org.apache.hop.core.gui.Point;
+import org.apache.hop.core.row.IRowMeta;
+import org.apache.hop.core.row.value.ValueMetaString;
+import org.apache.hop.core.variables.IVariables;
+import org.apache.hop.i18n.BaseMessages;
+import org.apache.hop.metadata.api.IHopMetadataProvider;
+import org.apache.hop.pipeline.PipelineHopMeta;
import org.apache.hop.pipeline.PipelineMeta;
import org.apache.hop.pipeline.transform.BaseTransformMeta;
import org.apache.hop.pipeline.transform.ITransform;
import org.apache.hop.pipeline.transform.ITransformData;
import org.apache.hop.pipeline.transform.TransformMeta;
import org.apache.hop.ui.core.PropsUi;
+import org.apache.hop.ui.hopgui.HopGui;
import org.apache.hop.ui.hopgui.HopGuiEnvironment;
import org.apache.hop.ui.hopgui.file.GraphCanvasTestBase;
import
org.apache.hop.ui.hopgui.file.pipeline.context.HopGuiPipelineTransformContext;
@@ -63,8 +74,10 @@ class HopGuiPipelineGraphHopCreationTest extends
GraphCanvasTestBase {
private static final String SOURCE_TRANSFORM = "Query Delete";
private static final String TARGET_TRANSFORM = "Write to log";
+ private static final String OTHER_INPUT_TRANSFORM = "Second input";
private static final Point SOURCE_LOCATION = new Point(60, 60);
private static final Point TARGET_LOCATION = new Point(280, 60);
+ private static final Point OTHER_INPUT_LOCATION = new Point(500, 60);
@BeforeAll
static void registerGuiPlugins() throws HopException {
@@ -436,6 +449,90 @@ class HopGuiPipelineGraphHopCreationTest extends
GraphCanvasTestBase {
});
}
+ // ------------------------------------------------- target reading another
row layout already
+
+ /**
+ * The target already reads from a transform that sends a different row
layout, so completing the
+ * hop runs the layout check, which reports the mismatch and offers to merge
the streams. Those
+ * two are the whole story: the click drew a hop, so the context dialog of
the transform it landed
+ * on must not appear on top of them.
+ *
+ * <p>The check runs from the mouseDown that completes the hop, and its
dialogs run their own
+ * event loop - which is what dispatches the release of that same click,
long before mouseDown is
+ * done with it. Getting that release ignored is the point here.
+ */
+ @Test
+ void
createHopActionThenClickingATargetMixingRowLayoutsOnlyRunsTheLayoutCheck() {
+ onCanvas(
+ HopGuiPipelineGraphHopCreationTest::buildPipelineMixingRowLayouts,
+ (bot, graph, pipelineMeta, spots) -> {
+ TransformMeta source = pipelineMeta.findTransform(SOURCE_TRANSFORM);
+ TransformMeta target = pipelineMeta.findTransform(TARGET_TRANSFORM);
+
+ clickTransformAndPickCreateHop(graph, spots, source);
+
+ fire(spots.canvas, SWT.MouseMove, spots.scale, spots.target, 0,
SWT.NONE);
+ List<String> popups = clickAndCatchDialogs(bot, spots, spots.target,
SWT.NONE);
+
+ assertAll(
+ () -> assertHopExists(pipelineMeta, source, target),
+ () -> assertOnlyTheLayoutCheckDialogs(popups),
+ () -> assertNoFailures(),
+ () -> assertCanvasIsIdle(graph));
+ });
+ }
+
+ @Test
+ void shiftClickingATargetMixingRowLayoutsOnlyRunsTheLayoutCheck() {
+ onCanvas(
+ HopGuiPipelineGraphHopCreationTest::buildPipelineMixingRowLayouts,
+ (bot, graph, pipelineMeta, spots) -> {
+ TransformMeta source = pipelineMeta.findTransform(SOURCE_TRANSFORM);
+ TransformMeta target = pipelineMeta.findTransform(TARGET_TRANSFORM);
+
+ fire(spots.canvas, SWT.MouseDown, spots.scale, spots.source, 1,
SWT.SHIFT);
+ fire(spots.canvas, SWT.MouseUp, spots.scale, spots.source, 1,
SWT.SHIFT | SWT.BUTTON1);
+ fire(spots.canvas, SWT.MouseMove, spots.scale, spots.target, 0,
SWT.SHIFT);
+ List<String> popups = clickAndCatchDialogs(bot, spots, spots.target,
SWT.SHIFT);
+
+ assertAll(
+ () -> assertHopExists(pipelineMeta, source, target),
+ () -> assertOnlyTheLayoutCheckDialogs(popups),
+ () -> assertNoFailures(),
+ () -> assertCanvasIsIdle(graph));
+ });
+ }
+
+ @Test
+ void shiftDraggingOntoATargetMixingRowLayoutsOnlyRunsTheLayoutCheck() {
+ onCanvas(
+ HopGuiPipelineGraphHopCreationTest::buildPipelineMixingRowLayouts,
+ (bot, graph, pipelineMeta, spots) -> {
+ TransformMeta source = pipelineMeta.findTransform(SOURCE_TRANSFORM);
+ TransformMeta target = pipelineMeta.findTransform(TARGET_TRANSFORM);
+
+ fire(spots.canvas, SWT.MouseDown, spots.scale, spots.source, 1,
SWT.SHIFT);
+ fire(
+ spots.canvas,
+ SWT.MouseMove,
+ spots.scale,
+ midpoint(spots.source, spots.target),
+ 0,
+ SWT.SHIFT | SWT.BUTTON1);
+ fire(spots.canvas, SWT.MouseMove, spots.scale, spots.target, 0,
SWT.SHIFT | SWT.BUTTON1);
+ Set<Shell> before = openShells();
+ fireAsync(
+ spots.canvas, SWT.MouseUp, spots.scale, spots.target, 1,
SWT.SHIFT | SWT.BUTTON1);
+ List<String> popups = catchDialogs(bot, before);
+
+ assertAll(
+ () -> assertHopExists(pipelineMeta, source, target),
+ () -> assertOnlyTheLayoutCheckDialogs(popups),
+ () -> assertNoFailures(),
+ () -> assertCanvasIsIdle(graph));
+ });
+ }
+
// ------------------------------------------------------------------
gesture building blocks
/**
@@ -485,6 +582,59 @@ class HopGuiPipelineGraphHopCreationTest extends
GraphCanvasTestBase {
return title;
}
+ /**
+ * Presses and releases the left button without waiting for the handlers.
Both halves of the click
+ * are posted up front on purpose: the press may open a dialog, and that
dialog runs its own event
+ * loop, which is what dispatches the release. Waiting for the press would
deadlock the test, and
+ * holding the release back until the dialog is gone would hide the very
ordering under test.
+ */
+ private List<String> clickAndCatchDialogs(SWTBot bot, Spots spots, Point at,
int stateMask) {
+ Set<Shell> before = openShells();
+ fireAsync(spots.canvas, SWT.MouseDown, spots.scale, at, 1, stateMask);
+ fireAsync(spots.canvas, SWT.MouseUp, spots.scale, at, 1, stateMask |
SWT.BUTTON1);
+ return catchDialogs(bot, before);
+ }
+
+ /**
+ * Collects the titles of the dialogs that opened since {@code before}, in
the order they
+ * appeared, and closes every one of them so the event loops underneath are
handed back.
+ *
+ * <p>Dialogs both stack and follow one another here: a dialog runs its own
event loop, so
+ * anything that loop dispatches can open a second dialog on top of the
first, while the code
+ * after the first dialog can open yet another one once it is gone. So a
round gathers everything
+ * that is up at the same time, closes the newest first - an older one
cannot return while a newer
+ * loop sits on top of it - and then looks again for whatever that let
through.
+ *
+ * <p>Closing a dialog is the answer a test wants: Hop dialogs treat it as
cancel, so a question
+ * like "replace this transform?" is answered with no.
+ */
+ private List<String> catchDialogs(SWTBot bot, Set<Shell> before) {
+ List<String> titles = new ArrayList<>();
+ Set<Shell> seen = new HashSet<>(before);
+ for (List<Shell> round = awaitNewShells(bot, seen);
+ !round.isEmpty();
+ round = awaitNewShells(bot, seen)) {
+ round.forEach(popup -> titles.add(titleOf(popup)));
+ for (int i = round.size() - 1; i >= 0; i--) {
+ closeShell(bot, round.get(i));
+ }
+ }
+ return titles;
+ }
+
+ /**
+ * Every dialog that is open at the same time, in the order it appeared.
Adds them to {@code
+ * seen}.
+ */
+ private List<Shell> awaitNewShells(SWTBot bot, Set<Shell> seen) {
+ List<Shell> found = new ArrayList<>();
+ for (Shell popup = awaitNewShell(bot, seen); popup != null; popup =
awaitNewShell(bot, seen)) {
+ found.add(popup);
+ seen.add(popup);
+ }
+ return found;
+ }
+
// ------------------------------------------------------------------
assertions
private void assertHopExists(PipelineMeta meta, TransformMeta from,
TransformMeta to) {
@@ -501,6 +651,20 @@ class HopGuiPipelineGraphHopCreationTest extends
GraphCanvasTestBase {
assertNull(dialogTitle, "no dialog may open here, but one did");
}
+ /**
+ * Mixing row layouts is worth reporting, and a Dummy target is worth an
offer to replace it with
+ * a Stream Schema Merge. Those two are all: the click completed a hop, so
it may not also be read
+ * as a plain click on the transform and open the context dialog on top of
them.
+ */
+ private void assertOnlyTheLayoutCheckDialogs(List<String> dialogTitles) {
+ assertEquals(
+ List.of(
+ BaseMessages.getString(HopGui.class,
"HopGui.LayoutCheck.Dialog.MismatchTitle"),
+ BaseMessages.getString(HopGui.class,
"HopGui.LayoutCheck.Dialog.ReplaceDummyTitle")),
+ dialogTitles,
+ "the layout check is all this gesture may report");
+ }
+
private void assertNoFailures() {
assertTrue(swallowed.isEmpty(), "the canvas must not throw, but got " +
swallowed);
}
@@ -548,6 +712,10 @@ class HopGuiPipelineGraphHopCreationTest extends
GraphCanvasTestBase {
}
private void onCanvas(CanvasTest test) {
+ onCanvas(HopGuiPipelineGraphHopCreationTest::buildPipeline, test);
+ }
+
+ private void onCanvas(Supplier<PipelineMeta> scene, CanvasTest test) {
AtomicReference<HopGuiPipelineGraph> graphRef = new AtomicReference<>();
AtomicReference<PipelineMeta> metaRef = new AtomicReference<>();
@@ -558,7 +726,7 @@ class HopGuiPipelineGraphHopCreationTest extends
GraphCanvasTestBase {
// Keep the single click on the canvas synchronous and deterministic.
PropsUi.getInstance().setUseDoubleClickOnCanvas(false);
- PipelineMeta pipelineMeta = buildPipeline();
+ PipelineMeta pipelineMeta = scene.get();
metaRef.set(pipelineMeta);
graphRef.set(
new HopGuiPipelineGraph(
@@ -619,10 +787,56 @@ class HopGuiPipelineGraphHopCreationTest extends
GraphCanvasTestBase {
return pipelineMeta;
}
+ /**
+ * The same two transforms, plus a third one that already feeds the target -
and sends a different
+ * row layout than the source does. Drawing the second hop into the target
is what makes the row
+ * layout check report a mismatch.
+ *
+ * <p>The target is a {@code Dummy}, exactly as in the report, so the layout
check also offers to
+ * replace it with a Stream Schema Merge. The tests decline that offer.
+ */
+ private static PipelineMeta buildPipelineMixingRowLayouts() {
+ PipelineMeta pipelineMeta = new PipelineMeta();
+ pipelineMeta.setName("hop-creation-mixed-layouts");
+
+ TransformMeta source = fieldTransform(SOURCE_TRANSFORM, SOURCE_LOCATION,
"one");
+ TransformMeta other = fieldTransform(OTHER_INPUT_TRANSFORM,
OTHER_INPUT_LOCATION, "one", "two");
+ TransformMeta target = transform(TARGET_TRANSFORM, TARGET_LOCATION);
+ pipelineMeta.addTransform(source);
+ pipelineMeta.addTransform(other);
+ pipelineMeta.addTransform(target);
+ pipelineMeta.addPipelineHop(new PipelineHopMeta(other, target));
+ return pipelineMeta;
+ }
+
private static TransformMeta transform(String name, Point location) {
TransformMeta transformMeta =
new TransformMeta("Dummy", name, new BaseTransformMeta<ITransform,
ITransformData>());
transformMeta.setLocation(location.x, location.y);
return transformMeta;
}
+
+ /** A transform putting the given fields on the stream, so that two of them
can disagree. */
+ private static TransformMeta fieldTransform(String name, Point location,
String... fields) {
+ TransformMeta transformMeta =
+ new TransformMeta(
+ "Dummy",
+ name,
+ new BaseTransformMeta<ITransform, ITransformData>() {
+ @Override
+ public void getFields(
+ IRowMeta inputRowMeta,
+ String transformName,
+ IRowMeta[] info,
+ TransformMeta nextTransform,
+ IVariables variables,
+ IHopMetadataProvider metadataProvider) {
+ for (String field : fields) {
+ inputRowMeta.addValueMeta(new ValueMetaString(field));
+ }
+ }
+ });
+ transformMeta.setLocation(location.x, location.y);
+ return transformMeta;
+ }
}
diff --git a/ui/src/main/java/org/apache/hop/ui/core/widget/TableView.java
b/ui/src/main/java/org/apache/hop/ui/core/widget/TableView.java
index 96f2ba39df..47fbc5d709 100644
--- a/ui/src/main/java/org/apache/hop/ui/core/widget/TableView.java
+++ b/ui/src/main/java/org/apache/hop/ui/core/widget/TableView.java
@@ -741,18 +741,10 @@ public class TableView extends Composite {
return new MouseAdapter() {
@Override
public void mouseDown(MouseEvent event) {
- if (activeTableItem != null
- && !activeTableItem.isDisposed()
- && editor != null
- && editor.getEditor() != null
- && !editor.getEditor().isDisposed()
- && activeTableColumn > 0) {
- if (columns[activeTableColumn - 1].getType() ==
ColumnInfo.COLUMN_TYPE_TEXT) {
- applyTextChange(activeTableItem, activeTableRow,
activeTableColumn);
- } else if (columns[activeTableColumn - 1].getType() ==
ColumnInfo.COLUMN_TYPE_CCOMBO) {
- applyComboChange(activeTableItem, activeTableRow,
activeTableColumn);
- }
- }
+ // Commit whatever cell is being edited before the click moves the
active cell somewhere
+ // else: an editor left open here would be applied to the cell we are
about to move to.
+ applyAllChanges();
+
boolean rightClick = event.button == 3;
if (event.button == 1 || rightClick) {
boolean shift = (event.stateMask & SWT.SHIFT) != 0;
@@ -821,10 +813,19 @@ public class TableView extends Composite {
&& editor.getEditor() != null
&& !editor.getEditor().isDisposed()
&& activeTableColumn > 0) {
- if (columns[activeTableColumn - 1].getType() ==
ColumnInfo.COLUMN_TYPE_TEXT) {
- applyTextChange(activeTableItem, activeTableRow, activeTableColumn);
- } else if (columns[activeTableColumn - 1].getType() ==
ColumnInfo.COLUMN_TYPE_CCOMBO) {
- applyComboChange(activeTableItem, activeTableRow, activeTableColumn);
+ // Mirror the editors edit() opens: a format column is edited with a
combo and a text-button
+ // column with a text field, just like the plain combo and text columns,
so they have to be
+ // committed (and disposed) the same way. Leaving one open makes the
next commit read that
+ // stale editor and write its text into another cell.
+ switch (columns[activeTableColumn - 1].getType()) {
+ case ColumnInfo.COLUMN_TYPE_TEXT, ColumnInfo.COLUMN_TYPE_TEXT_BUTTON:
+ applyTextChange(activeTableItem, activeTableRow, activeTableColumn);
+ break;
+ case ColumnInfo.COLUMN_TYPE_CCOMBO, ColumnInfo.COLUMN_TYPE_FORMAT:
+ applyComboChange(activeTableItem, activeTableRow, activeTableColumn);
+ break;
+ default:
+ break;
}
}
}
diff --git
a/ui/src/main/java/org/apache/hop/ui/hopgui/file/pipeline/HopGuiPipelineGraph.java
b/ui/src/main/java/org/apache/hop/ui/hopgui/file/pipeline/HopGuiPipelineGraph.java
index cc5ab9c7ce..b1889fce9c 100644
---
a/ui/src/main/java/org/apache/hop/ui/hopgui/file/pipeline/HopGuiPipelineGraph.java
+++
b/ui/src/main/java/org/apache/hop/ui/hopgui/file/pipeline/HopGuiPipelineGraph.java
@@ -905,13 +905,20 @@ public class HopGuiPipelineGraph extends
HopGuiAbstractGraph
// If we click on the start hop transform or a forbidden
transform, then we don't have a
// candidate hop, but we need to ignore this click to not start a
drag operation.
if (candidate != null) {
- addCandidateAsHop(event.x, event.y);
// Single-stream hop completes on mouseDown and clears
startHopTransform; without
- // this,
- // the following mouseUp would look like a plain transform click
and open the action
- // dialog.
- if (startHopTransform == null) {
- avoidContextDialog = true;
+ // this, the following mouseUp would look like a plain transform
click and open the
+ // action dialog. Claim that release up front: completing the
hop can put a dialog on
+ // screen - the row layout report, the copy/distribute question
- and a dialog runs
+ // its own event loop, which dispatches the release of this very
click before we get
+ // back here.
+ avoidContextDialog = true;
+ addCandidateAsHop(event.x, event.y);
+ if (avoidContextDialog && startHopTransform != null) {
+ // The hop is not done yet: it is the pop-up menu of stream
options that completes
+ // it, so the release is an ordinary one after all. Should the
release already have
+ // been handled from a dialog's event loop, the flag is
cleared by now and has to
+ // stay that way, or it would swallow the next click.
+ avoidContextDialog = false;
}
}
} else if (event.button == 1 && alt &&
currentTransform.supportsErrorHandling()) {
diff --git
a/ui/src/main/java/org/apache/hop/ui/hopgui/file/workflow/HopGuiWorkflowGraph.java
b/ui/src/main/java/org/apache/hop/ui/hopgui/file/workflow/HopGuiWorkflowGraph.java
index 4d907a94b3..c230175cd5 100644
---
a/ui/src/main/java/org/apache/hop/ui/hopgui/file/workflow/HopGuiWorkflowGraph.java
+++
b/ui/src/main/java/org/apache/hop/ui/hopgui/file/workflow/HopGuiWorkflowGraph.java
@@ -755,11 +755,18 @@ public class HopGuiWorkflowGraph extends
HopGuiAbstractGraph
// If we click on the start hop action or a forbidden action, then
we don't have a
// candidate hop, but we need to ignore this click to not start a
drag operation
if (hopCandidate != null) {
- addCandidateAsHop();
// The hop completes on mouseDown and clears startHopAction;
without this, the
// following mouseUp would look like a plain action click and
open the context dialog.
- if (startHopAction == null) {
- avoidContextDialog = true;
+ // Claim that release up front: completing the hop can put a
dialog on screen - the
+ // hop exists already, the hop causes a loop - and a dialog runs
its own event loop,
+ // which dispatches the release of this very click before we get
back here.
+ avoidContextDialog = true;
+ addCandidateAsHop();
+ if (avoidContextDialog && startHopAction != null) {
+ // The hop was not made, so the release is an ordinary one
after all. Should it
+ // already have been handled from a dialog's event loop, the
flag is cleared by now
+ // and has to stay that way, or it would swallow the next
click.
+ avoidContextDialog = false;
}
}
} else if (event.button == 2 || (event.button == 1 && shift)) {
