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 80864ced76 Fixes #8872 : Remove the lint results listener when Show
Lint Results closes, widen Severity (#8878)
80864ced76 is described below
commit 80864ced7686f58215cf6c040eea3040e2233339
Author: Bart Maertens <[email protected]>
AuthorDate: Tue Oct 13 22:21:55 2026 +0200
Fixes #8872 : Remove the lint results listener when Show Lint Results
closes, widen Severity (#8878)
---
.../org/apache/hop/lint/LintResultsManager.java | 6 +-
.../java/org/apache/hop/lint/LintResultsPanel.java | 132 ++++++++++++---------
.../hop/lint/LintResultsPanelDisposeTest.java | 79 ++++++++++++
3 files changed, 157 insertions(+), 60 deletions(-)
diff --git
a/plugins/misc/lint/src/main/java/org/apache/hop/lint/LintResultsManager.java
b/plugins/misc/lint/src/main/java/org/apache/hop/lint/LintResultsManager.java
index e193bff97f..d48c82dfeb 100644
---
a/plugins/misc/lint/src/main/java/org/apache/hop/lint/LintResultsManager.java
+++
b/plugins/misc/lint/src/main/java/org/apache/hop/lint/LintResultsManager.java
@@ -23,6 +23,7 @@ import java.util.List;
import java.util.Map;
import java.util.Set;
import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.CopyOnWriteArrayList;
import org.apache.hop.core.logging.ILogChannel;
import org.apache.hop.core.logging.LogChannel;
import org.apache.hop.ui.hopgui.HopGui;
@@ -64,8 +65,9 @@ public class LintResultsManager {
// Track when results were last updated
private long lastUpdateTime = 0;
- // Listeners for result changes
- private final List<LintResultsListener> listeners = new ArrayList<>();
+ // Listeners for result changes. Notified from lint threads while a window
closing on the UI
+ // thread removes its own, so iteration has to tolerate a concurrent removal.
+ private final List<LintResultsListener> listeners = new
CopyOnWriteArrayList<>();
private LintResultsManager() {
// Singleton
diff --git
a/plugins/misc/lint/src/main/java/org/apache/hop/lint/LintResultsPanel.java
b/plugins/misc/lint/src/main/java/org/apache/hop/lint/LintResultsPanel.java
index 6b6bc49d7e..8142163369 100644
--- a/plugins/misc/lint/src/main/java/org/apache/hop/lint/LintResultsPanel.java
+++ b/plugins/misc/lint/src/main/java/org/apache/hop/lint/LintResultsPanel.java
@@ -26,10 +26,12 @@ import org.eclipse.swt.SWT;
import org.eclipse.swt.custom.SashForm;
import org.eclipse.swt.events.SelectionAdapter;
import org.eclipse.swt.events.SelectionEvent;
+import org.eclipse.swt.graphics.GC;
import org.eclipse.swt.layout.FillLayout;
import org.eclipse.swt.layout.GridData;
import org.eclipse.swt.layout.GridLayout;
import org.eclipse.swt.widgets.Composite;
+import org.eclipse.swt.widgets.Display;
import org.eclipse.swt.widgets.Group;
import org.eclipse.swt.widgets.Label;
import org.eclipse.swt.widgets.Shell;
@@ -46,6 +48,13 @@ public class LintResultsPanel extends Composite implements
LintResultsManager.Li
private static final ILogChannel log = LogChannel.GENERAL;
private LintResultsManager resultsManager;
+
+ /**
+ * Kept from the start: results arrive on lint threads, and asking a panel
that has just been
+ * disposed for its display throws.
+ */
+ private final Display display;
+
private Tree resultsTree;
private LintResultGrouping.SortKey sortKey = LintResultGrouping.SortKey.FILE;
private boolean sortAscending = true;
@@ -56,9 +65,13 @@ public class LintResultsPanel extends Composite implements
LintResultsManager.Li
public LintResultsPanel(Composite parent, int style) {
super(parent, style);
+ this.display = parent.getDisplay();
this.resultsManager = LintResultsManager.getInstance();
this.resultsManager.addListener(this);
+ // A dispose listener, not an override of dispose(): closing the window
disposes the panel
+ // without calling its dispose().
+ addDisposeListener(e -> resultsManager.removeListener(this));
createContents();
refreshResults();
@@ -128,7 +141,18 @@ public class LintResultsPanel extends Composite implements
LintResultsManager.Li
// Tree columns
TreeColumn severityColumn = new TreeColumn(resultsTree, SWT.LEFT);
severityColumn.setText(BaseMessages.getString(PKG,
"LintResultsPanel.Column.Severity"));
- severityColumn.setWidth(80);
+ // The widest text in the column, WARNING or the header, plus room for the
expand arrow, the
+ // indent of the child rows and the cell margins.
+ GC gc = new GC(resultsTree);
+ try {
+ int textWidth =
+ Math.max(
+ gc.textExtent(LintSeverity.Level.WARNING.getCode()).x,
+ gc.textExtent(severityColumn.getText()).x);
+ severityColumn.setWidth(textWidth + 40);
+ } finally {
+ gc.dispose();
+ }
TreeColumn ruleColumn = new TreeColumn(resultsTree, SWT.LEFT);
ruleColumn.setText(BaseMessages.getString(PKG,
"LintResultsPanel.Column.Rule"));
@@ -182,58 +206,58 @@ public class LintResultsPanel extends Composite
implements LintResultsManager.Li
}
private void refreshResults() {
+ if (isDisposed() || display.isDisposed()) {
+ return;
+ }
// The panel's own display, not Display.getDefault(): results arrive on
lint threads, and in
// Hop Web a thread that was started without a session has no default
display to find.
- getDisplay()
- .asyncExec(
- () -> {
- if (isDisposed()) {
- return;
- }
-
- resultsTree.removeAll();
-
- List<LintResult> displayedResults = getDisplayedResults();
-
- if (displayedResults.isEmpty()) {
- if (fileFilter != null) {
- statusLabel.setText(
- "No lint issues for "
- + LintEditorGraphHelper.displayName(fileFilter)
- + ". Run lint on this file or check Tools → Lint →
Show Lint Results.");
- } else {
- statusLabel.setText(
- BaseMessages.getString(PKG,
"LintResultsPanel.Status.NoResults"));
- }
- detailsText.setText(
- BaseMessages.getString(PKG,
"LintResultsPanel.Details.NoResults"));
- return;
- }
-
- int errorCount = 0;
- int warningCount = 0;
- for (LintResult result : displayedResults) {
- if ("ERROR".equalsIgnoreCase(result.getSeverity())) {
- errorCount++;
- } else if ("WARNING".equalsIgnoreCase(result.getSeverity())) {
- warningCount++;
- }
- }
-
- String scope =
- fileFilter != null ? " for " +
LintEditorGraphHelper.displayName(fileFilter) : "";
+ display.asyncExec(
+ () -> {
+ if (isDisposed()) {
+ return;
+ }
+
+ resultsTree.removeAll();
+
+ List<LintResult> displayedResults = getDisplayedResults();
+
+ if (displayedResults.isEmpty()) {
+ if (fileFilter != null) {
statusLabel.setText(
- String.format(
- "Found %d issues%s (%d errors, %d warnings)",
- displayedResults.size(), scope, errorCount,
warningCount));
-
- Map<String, List<LintResult>> resultsBySeverity =
groupBySeverity(displayedResults);
- List<String> severities = new
java.util.ArrayList<>(resultsBySeverity.keySet());
- severities.sort(LintResultGrouping.severityOrder());
- for (String severity : severities) {
- addSeverityGroup(severity, resultsBySeverity.get(severity));
- }
- });
+ "No lint issues for "
+ + LintEditorGraphHelper.displayName(fileFilter)
+ + ". Run lint on this file or check Tools → Lint → Show
Lint Results.");
+ } else {
+ statusLabel.setText(BaseMessages.getString(PKG,
"LintResultsPanel.Status.NoResults"));
+ }
+ detailsText.setText(BaseMessages.getString(PKG,
"LintResultsPanel.Details.NoResults"));
+ return;
+ }
+
+ int errorCount = 0;
+ int warningCount = 0;
+ for (LintResult result : displayedResults) {
+ if ("ERROR".equalsIgnoreCase(result.getSeverity())) {
+ errorCount++;
+ } else if ("WARNING".equalsIgnoreCase(result.getSeverity())) {
+ warningCount++;
+ }
+ }
+
+ String scope =
+ fileFilter != null ? " for " +
LintEditorGraphHelper.displayName(fileFilter) : "";
+ statusLabel.setText(
+ String.format(
+ "Found %d issues%s (%d errors, %d warnings)",
+ displayedResults.size(), scope, errorCount, warningCount));
+
+ Map<String, List<LintResult>> resultsBySeverity =
groupBySeverity(displayedResults);
+ List<String> severities = new
java.util.ArrayList<>(resultsBySeverity.keySet());
+ severities.sort(LintResultGrouping.severityOrder());
+ for (String severity : severities) {
+ addSeverityGroup(severity, resultsBySeverity.get(severity));
+ }
+ });
}
private void addSeverityGroup(String severity, List<LintResult> results) {
@@ -345,12 +369,4 @@ public class LintResultsPanel extends Composite implements
LintResultsManager.Li
public void onResultsUpdated() {
refreshResults();
}
-
- @Override
- public void dispose() {
- if (resultsManager != null) {
- resultsManager.removeListener(this);
- }
- super.dispose();
- }
}
diff --git
a/plugins/misc/lint/src/test/java/org/apache/hop/lint/LintResultsPanelDisposeTest.java
b/plugins/misc/lint/src/test/java/org/apache/hop/lint/LintResultsPanelDisposeTest.java
new file mode 100644
index 0000000000..7f3b9092dc
--- /dev/null
+++
b/plugins/misc/lint/src/test/java/org/apache/hop/lint/LintResultsPanelDisposeTest.java
@@ -0,0 +1,79 @@
+/*
+ * 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.lint;
+
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+import java.lang.reflect.Field;
+import java.util.List;
+import java.util.concurrent.atomic.AtomicReference;
+import org.apache.hop.ui.testing.SwtBotTestBase;
+import org.eclipse.swt.SWT;
+import org.eclipse.swt.layout.FillLayout;
+import org.junit.jupiter.api.Tag;
+import org.junit.jupiter.api.Test;
+
+/**
+ * Closing Show Lint Results disposes the panel without calling its dispose().
The panel stayed
+ * registered, and every later lint update logged "Widget is disposed".
+ *
+ * @see <a href="https://github.com/apache/hop/issues/8872";>#8872</a>
+ */
+@Tag("uitest")
+class LintResultsPanelDisposeTest extends SwtBotTestBase {
+
+ @Test
+ void closingTheWindowRemovesThePanelsListener() throws Exception {
+ AtomicReference<LintResultsPanel> panel = new AtomicReference<>();
+
+ withScene(
+ shell -> {
+ shell.setLayout(new FillLayout());
+ panel.set(new LintResultsPanel(shell, SWT.NONE));
+ },
+ bot -> {});
+
+ assertTrue(panel.get().isDisposed(), "the window was closed");
+ assertFalse(listeners().contains(panel.get()), "a closed panel is still
notified");
+ }
+
+ @Test
+ void anOpenPanelIsNotified() throws Exception {
+ AtomicReference<Boolean> registered = new AtomicReference<>();
+
+ withScene(
+ shell -> {
+ shell.setLayout(new FillLayout());
+ LintResultsPanel open = new LintResultsPanel(shell, SWT.NONE);
+ registered.set(listeners().contains(open));
+ },
+ bot -> {});
+
+ assertTrue(registered.get());
+ }
+
+ private static List<?> listeners() {
+ try {
+ Field field = LintResultsManager.class.getDeclaredField("listeners");
+ field.setAccessible(true);
+ return (List<?>) field.get(LintResultsManager.getInstance());
+ } catch (ReflectiveOperationException e) {
+ throw new IllegalStateException(e);
+ }
+ }
+}
