This is an automated email from the ASF dual-hosted git repository.
mattcasters 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 11b6d12386 Issue #8601 : Do not crash when opening a workflow
execution viewer (#8602)
11b6d12386 is described below
commit 11b6d12386ece077ec6542d6893502504425c120
Author: Matt Casters <[email protected]>
AuthorDate: Mon Sep 28 12:26:12 2026 +0200
Issue #8601 : Do not crash when opening a workflow execution viewer (#8602)
* Issue #8601 : Do not crash when opening a workflow execution viewer
* Issue #8601 : Discard a failed execution viewer open
Drop the tab-repair and stale-viewer paths. A viewer tab cannot lose its
data once it is stored before setup, and closing a tab already unregisters the
viewer. A failed open now cleans up on any throwable, and the test lives in rcp
so it can call addViewer.
---
.../ExecutionPerspectiveActiveViewerTest.java | 176 +++++++++++++++++++++
.../execution/ExecutionPerspective.java | 84 ++++++++--
.../execution/WorkflowExecutionViewer.java | 27 ++--
.../hop/ui/hopgui/shared/BaseExecutionViewer.java | 2 +-
4 files changed, 264 insertions(+), 25 deletions(-)
diff --git
a/rcp/src/test/java/org/apache/hop/ui/hopgui/perspective/execution/ExecutionPerspectiveActiveViewerTest.java
b/rcp/src/test/java/org/apache/hop/ui/hopgui/perspective/execution/ExecutionPerspectiveActiveViewerTest.java
new file mode 100644
index 0000000000..0b662684d4
--- /dev/null
+++
b/rcp/src/test/java/org/apache/hop/ui/hopgui/perspective/execution/ExecutionPerspectiveActiveViewerTest.java
@@ -0,0 +1,176 @@
+/*
+ * 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.hopgui.perspective.execution;
+
+import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertNull;
+import static org.junit.jupiter.api.Assertions.assertSame;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+import java.lang.reflect.Field;
+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.Image;
+import org.eclipse.swt.widgets.Composite;
+import org.eclipse.swt.widgets.Control;
+import org.eclipse.swt.widgets.Shell;
+import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Tag;
+import org.junit.jupiter.api.Test;
+
+/**
+ * Double-clicking a workflow execution selects its viewer tab. A tab with no
data used to throw
+ * from {@code CTabItem.getData().equals} (issue #8601). Opening a viewer
whose setup fails must not
+ * leave that composite parented to the folder.
+ */
+@Tag("uitest")
+class ExecutionPerspectiveActiveViewerTest extends SwtBotTestBase {
+
+ private Shell shell;
+ private CTabFolder folder;
+ private ExecutionPerspective perspective;
+ private ExecutionPerspective previousInstance;
+
+ @BeforeEach
+ void openFolder() throws Exception {
+ previousInstance = currentInstance();
+ perspective = new ExecutionPerspective();
+ shell = new Shell(display);
+ folder = new CTabFolder(shell, SWT.CLOSE);
+ setField(perspective, "tabFolder", folder);
+ }
+
+ @AfterEach
+ void closeFolder() throws Exception {
+ if (shell != null && !shell.isDisposed()) {
+ shell.dispose();
+ }
+ setInstance(previousInstance);
+ }
+
+ @Test
+ void nullDataTabDoesNotHideTheOpenViewer() {
+ StubViewer workflow = new StubViewer("lees-van-kafka", "dae9c3e9");
+ CTabItem empty = new CTabItem(folder, SWT.CLOSE);
+ CTabItem workflowTab = new CTabItem(folder, SWT.CLOSE);
+ workflowTab.setText(workflow.getName());
+ workflowTab.setData(workflow);
+
+ assertDoesNotThrow(() -> perspective.setActiveViewer(workflow));
+
+ assertSame(workflow, perspective.getActiveViewer());
+ assertSame(workflowTab, folder.getSelection());
+ assertEquals(1, workflow.focusCount);
+ assertNull(empty.getData());
+ }
+
+ @Test
+ void nullViewerDoesNothing() {
+ assertDoesNotThrow(() -> perspective.setActiveViewer(null));
+ assertNull(perspective.getActiveViewer());
+ }
+
+ @Test
+ void failedOpenLeavesNoTabAndDisposesTheViewer() {
+ Composite body = new Composite(folder, SWT.NONE);
+ StubViewer workflow = new StubViewer("lees-van-kafka", "dae9c3e9");
+ workflow.control = body;
+ workflow.imageFailure = new Error("icon");
+
+ Error failure = assertThrows(Error.class, () ->
perspective.addViewer(workflow));
+
+ assertSame(workflow.imageFailure, failure);
+ assertEquals(0, folder.getItemCount());
+ assertNull(perspective.findViewer(workflow.getLogChannelId(),
workflow.getName()));
+ assertTrue(body.isDisposed());
+ }
+
+ private static ExecutionPerspective currentInstance() throws Exception {
+ Field field = ExecutionPerspective.class.getDeclaredField("instance");
+ field.setAccessible(true);
+ return (ExecutionPerspective) field.get(null);
+ }
+
+ private static void setInstance(ExecutionPerspective instance) throws
Exception {
+ Field field = ExecutionPerspective.class.getDeclaredField("instance");
+ field.setAccessible(true);
+ field.set(null, instance);
+ }
+
+ private static void setField(Object target, String name, Object value)
throws Exception {
+ Field field = target.getClass().getDeclaredField(name);
+ field.setAccessible(true);
+ field.set(target, value);
+ }
+
+ /** Viewer stand-in that does not build a workflow canvas. */
+ private static final class StubViewer implements IExecutionViewer {
+ private final String name;
+ private final String id;
+ private Control control;
+ private int focusCount;
+ private Error imageFailure;
+
+ private StubViewer(String name, String id) {
+ this.name = name;
+ this.id = id;
+ }
+
+ @Override
+ public String getName() {
+ return name;
+ }
+
+ @Override
+ public String getLogChannelId() {
+ return id;
+ }
+
+ @Override
+ public Image getTitleImage() {
+ if (imageFailure != null) {
+ throw imageFailure;
+ }
+ return null;
+ }
+
+ @Override
+ public String getTitleToolTip() {
+ return null;
+ }
+
+ @Override
+ public boolean setFocus() {
+ focusCount++;
+ return true;
+ }
+
+ @Override
+ public Control getControl() {
+ return control;
+ }
+
+ @Override
+ public void refresh() {}
+ }
+}
diff --git
a/ui/src/main/java/org/apache/hop/ui/hopgui/perspective/execution/ExecutionPerspective.java
b/ui/src/main/java/org/apache/hop/ui/hopgui/perspective/execution/ExecutionPerspective.java
index 2bb85d595c..3434f8c9e3 100644
---
a/ui/src/main/java/org/apache/hop/ui/hopgui/perspective/execution/ExecutionPerspective.java
+++
b/ui/src/main/java/org/apache/hop/ui/hopgui/perspective/execution/ExecutionPerspective.java
@@ -506,16 +506,31 @@ public class ExecutionPerspective implements
IHopPerspective, TabClosable {
}
public void addViewer(IExecutionViewer viewer) {
- // Create tab item
+ if (viewer == null || tabFolder == null || tabFolder.isDisposed()) {
+ return;
+ }
+
+ // Data is set before any call that can throw. A tab left without data
makes the next
+ // double-click crash in setActiveViewer (issue #8601).
//
CTabItem tabItem = new CTabItem(tabFolder, SWT.CLOSE);
- tabItem.setFont(GuiResource.getInstance().getFontDefault());
- tabItem.setText(viewer.getName());
- tabItem.setImage(viewer.getTitleImage());
- tabItem.setToolTipText(viewer.getTitleToolTip());
-
- tabItem.setControl(viewer.getControl());
- tabItem.setData(viewer);
+ boolean ok = false;
+ try {
+ tabItem.setData(viewer);
+ tabItem.setFont(GuiResource.getInstance().getFontDefault());
+ tabItem.setText(Const.NVL(viewer.getName(), ""));
+ tabItem.setImage(viewer.getTitleImage());
+ tabItem.setToolTipText(viewer.getTitleToolTip());
+ Control control = viewer.getControl();
+ if (control != null && !control.isDisposed()) {
+ tabItem.setControl(control);
+ }
+ ok = true;
+ } finally {
+ if (!ok) {
+ discardViewerTab(tabItem, viewer);
+ }
+ }
viewers.add(viewer);
@@ -534,6 +549,32 @@ public class ExecutionPerspective implements
IHopPerspective, TabClosable {
viewer.refresh();
}
+ /**
+ * Drop a tab that never became a usable viewer. The viewer composite is a
child of the folder
+ * even when it was not attached to the tab, so it is disposed too.
+ */
+ private void discardViewerTab(CTabItem tabItem, IExecutionViewer viewer) {
+ if (tabItem != null && !tabItem.isDisposed()) {
+ try {
+ tabItem.setControl(null);
+ } catch (RuntimeException e) {
+ // Detach is best-effort; the tab is about to be disposed.
+ }
+ tabItem.dispose();
+ }
+ if (viewer == null) {
+ return;
+ }
+ try {
+ Control control = viewer.getControl();
+ if (control != null && !control.isDisposed()) {
+ control.dispose();
+ }
+ } catch (RuntimeException e) {
+ // The viewer is already unusable; the open fails with the original
exception.
+ }
+ }
+
/**
* Find a metadata editor
*
@@ -553,22 +594,33 @@ public class ExecutionPerspective implements
IHopPerspective, TabClosable {
}
public void setActiveViewer(IExecutionViewer viewer) {
+ if (viewer == null || tabFolder == null || tabFolder.isDisposed()) {
+ return;
+ }
for (CTabItem item : tabFolder.getItems()) {
- if (item.getData().equals(viewer)) {
+ if (item == null || item.isDisposed()) {
+ continue;
+ }
+ // Compare from the viewer. A tab with no data must not throw (issue
#8601).
+ //
+ if (viewer.equals(item.getData())) {
tabFolder.setSelection(item);
tabFolder.showItem(item);
-
viewer.setFocus();
}
}
}
public IExecutionViewer getActiveViewer() {
- if (tabFolder.getSelectionIndex() < 0) {
+ if (tabFolder == null || tabFolder.isDisposed() ||
tabFolder.getSelectionIndex() < 0) {
return null;
}
- return (IExecutionViewer) tabFolder.getSelection().getData();
+ Object data = tabFolder.getSelection().getData();
+ if (data instanceof IExecutionViewer viewer) {
+ return viewer;
+ }
+ return null;
}
protected void onTabClose(CTabFolderEvent event) {
@@ -1554,9 +1606,11 @@ public class ExecutionPerspective implements
IHopPerspective, TabClosable {
@Override
public void closeTab(CTabFolderEvent event, CTabItem tabItem) {
- IExecutionViewer viewer = (IExecutionViewer) tabItem.getData();
-
- boolean isRemoved = viewers.remove(viewer);
+ if (tabItem == null || tabItem.isDisposed()) {
+ return;
+ }
+ Object data = tabItem.getData();
+ boolean isRemoved = data instanceof IExecutionViewer viewer &&
viewers.remove(viewer);
tabItem.dispose();
if (isRemoved) {
diff --git
a/ui/src/main/java/org/apache/hop/ui/hopgui/perspective/execution/WorkflowExecutionViewer.java
b/ui/src/main/java/org/apache/hop/ui/hopgui/perspective/execution/WorkflowExecutionViewer.java
index 3ab912c293..124a3618f2 100644
---
a/ui/src/main/java/org/apache/hop/ui/hopgui/perspective/execution/WorkflowExecutionViewer.java
+++
b/ui/src/main/java/org/apache/hop/ui/hopgui/perspective/execution/WorkflowExecutionViewer.java
@@ -333,17 +333,26 @@ public class WorkflowExecutionViewer extends
BaseExecutionViewer
if (childIds != null) {
for (String id : childIds) {
ExecutionData actionData =
iLocation.getExecutionData(execution.getId(), id);
+ // A child id without sample data is normal while an action is still
starting. Skipping
+ // it keeps the workflow info tab on screen instead of failing the
whole refresh.
+ //
+ if (actionData == null) {
+ LogChannel.UI.logDebug("No execution data yet for action id '" +
id + "'");
+ continue;
+ }
ExecutionDataSetMeta dataSetMeta = actionData.getDataSetMeta();
- if (dataSetMeta != null) {
- String actionName = dataSetMeta.getName();
-
- // Add this one under that name
- //
- List<ExecutionData> executionDataList =
- actionExecutions.computeIfAbsent(actionName, k -> new
ArrayList<>());
- executionDataList.add(actionData);
+ if (dataSetMeta == null || dataSetMeta.getName() == null) {
+ LogChannel.UI.logDebug("Execution data for action id '" + id + "'
has no action name");
+ continue;
}
+ String actionName = dataSetMeta.getName();
+
+ // Add this one under that name
+ //
+ List<ExecutionData> executionDataList =
+ actionExecutions.computeIfAbsent(actionName, k -> new
ArrayList<>());
+ executionDataList.add(actionData);
}
}
} catch (Exception e) {
@@ -812,7 +821,7 @@ public class WorkflowExecutionViewer extends
BaseExecutionViewer
@Override
public String getActiveId() {
- if (selectedAction != null) {
+ if (selectedAction != null && selectedExecutionData != null) {
if (selectedExecutionData.getOwnerId() == null) {
return selectedExecutionData.getParentId();
} else {
diff --git
a/ui/src/main/java/org/apache/hop/ui/hopgui/shared/BaseExecutionViewer.java
b/ui/src/main/java/org/apache/hop/ui/hopgui/shared/BaseExecutionViewer.java
index 7d6816afae..3c5efefe69 100644
--- a/ui/src/main/java/org/apache/hop/ui/hopgui/shared/BaseExecutionViewer.java
+++ b/ui/src/main/java/org/apache/hop/ui/hopgui/shared/BaseExecutionViewer.java
@@ -113,7 +113,7 @@ public abstract class BaseExecutionViewer extends
DragViewZoomBase
@Override
public boolean setFocus() {
- if (canvas.isDisposed()) {
+ if (canvas == null || canvas.isDisposed()) {
return false;
}
return canvas.setFocus();
