From 45cb1e0de7940cbe7aef1c63aaa9cd311df4aae4 Mon Sep 17 00:00:00 2001 From: mattcasters Date: Mon, 5 Oct 2026 21:36:43 +0200 Subject: [PATCH] Issue #8758 : Do not mark loaded run configurations as changed --- .../MetaSelectionLineFillItemsTest.java | 118 ++++++++++++++++++ .../hop/ui/core/widget/MetaSelectionLine.java | 44 ++++++- 2 files changed, 158 insertions(+), 4 deletions(-) create mode 100644 rcp/src/test/java/org/apache/hop/ui/core/widget/MetaSelectionLineFillItemsTest.java diff --git a/rcp/src/test/java/org/apache/hop/ui/core/widget/MetaSelectionLineFillItemsTest.java b/rcp/src/test/java/org/apache/hop/ui/core/widget/MetaSelectionLineFillItemsTest.java new file mode 100644 index 00000000000..e5535712cc0 --- /dev/null +++ b/rcp/src/test/java/org/apache/hop/ui/core/widget/MetaSelectionLineFillItemsTest.java @@ -0,0 +1,118 @@ +/* + * 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.assertArrayEquals; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assumptions.assumeFalse; + +import java.awt.GraphicsEnvironment; +import java.util.concurrent.atomic.AtomicInteger; +import org.apache.hop.core.HopEnvironment; +import org.apache.hop.core.variables.Variables; +import org.apache.hop.execution.ExecutionInfoLocation; +import org.apache.hop.metadata.serializer.memory.MemoryMetadataProvider; +import org.apache.hop.ui.core.PropsUi; +import org.apache.hop.ui.core.gui.GuiResource; +import org.eclipse.swt.SWT; +import org.eclipse.swt.widgets.Display; +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; + +/** + * Refreshing a metadata combo is not an edit. Run configuration editors do this when their tab is + * selected, and on Windows {@code CCombo.setText} notifies Modify even when the text is unchanged + * (issue #8758). + * + *

Lives in hop-ui-rcp because constructing the widget needs the desktop look-and-feel + * implementations ({@code TextSizeUtilFacadeImpl}, {@code ToolbarFacadeImpl}). + */ +@Tag("uitest") +class MetaSelectionLineFillItemsTest { + + private Display display; + private Shell shell; + + @BeforeEach + void openShell() throws Exception { + assumeFalse(GraphicsEnvironment.isHeadless(), "No display available; skipping SWT test."); + HopEnvironment.init(); + display = Display.getDefault(); + PropsUi.getInstance(); + GuiResource.getInstance(); + shell = new Shell(display, SWT.NONE); + } + + @AfterEach + void disposeShell() { + if (shell != null && !shell.isDisposed()) { + shell.dispose(); + } + } + + @Test + void refreshingTheListDoesNotNotifyModify() throws Exception { + MemoryMetadataProvider provider = new MemoryMetadataProvider(); + ExecutionInfoLocation location = new ExecutionInfoLocation(); + location.setName("neo-location"); + provider.getSerializer(ExecutionInfoLocation.class).save(location); + + // Read-only makes setItems clear the text, so a refresh has to write it back. That write + // notifies Modify unless the refresh detaches listeners. + MetaSelectionLine line = + new MetaSelectionLine<>( + Variables.getADefaultVariableSpace(), + provider, + ExecutionInfoLocation.class, + shell, + SWT.READ_ONLY, + "Location", + "Location tooltip"); + line.fillItems(); + line.setText("neo-location"); + + AtomicInteger modifications = new AtomicInteger(); + AtomicInteger selections = new AtomicInteger(); + line.getComboWidget().addListener(SWT.Modify, event -> modifications.incrementAndGet()); + line.getComboWidget().addListener(SWT.Selection, event -> selections.incrementAndGet()); + + line.fillItems(); + + assertEquals("neo-location", line.getText()); + assertArrayEquals(new String[] {"neo-location"}, line.getItems()); + assertEquals(0, modifications.get()); + assertEquals(0, selections.get()); + + ExecutionInfoLocation added = new ExecutionInfoLocation(); + added.setName("file-location"); + provider.getSerializer(ExecutionInfoLocation.class).save(added); + line.fillItems(); + + assertEquals("neo-location", line.getText()); + assertArrayEquals(new String[] {"file-location", "neo-location"}, line.getItems()); + assertEquals(0, modifications.get()); + assertEquals(0, selections.get()); + + // Listeners are back: a real edit still marks the control changed. + line.setText("file-location"); + assertEquals(1, modifications.get()); + } +} diff --git a/ui/src/main/java/org/apache/hop/ui/core/widget/MetaSelectionLine.java b/ui/src/main/java/org/apache/hop/ui/core/widget/MetaSelectionLine.java index 11143550430..b0b2d70fa96 100644 --- a/ui/src/main/java/org/apache/hop/ui/core/widget/MetaSelectionLine.java +++ b/ui/src/main/java/org/apache/hop/ui/core/widget/MetaSelectionLine.java @@ -17,6 +17,7 @@ package org.apache.hop.ui.core.widget; +import java.util.Arrays; import java.util.Collections; import java.util.List; import org.apache.commons.lang3.StringUtils; @@ -389,18 +390,53 @@ public void fillItems() throws HopException { } repopulatingItems = true; try { - String previous = wCombo.getText(); + CCombo combo = wCombo.getCComboWidget(); + if (combo.isDisposed()) { + return; + } + String previous = Const.NVL(wCombo.getText(), ""); List elementNames = manager.getSerializer().listObjectNames(); Collections.sort(elementNames); - wCombo.setItems(elementNames.toArray(new String[0])); - if (!wCombo.getCComboWidget().isDisposed()) { - wCombo.setText(Const.NVL(previous, "")); + String[] items = elementNames.toArray(new String[0]); + // Selecting a run configuration tab refreshes these lists. On Windows, CCombo.setText + // notifies Modify even when the string is unchanged, and the editors treat that as an + // unsaved edit (issue #8758). Skip the write when nothing changed, and keep listeners + // detached while the list is rebuilt so a read-only combo can be restored quietly. + if (Arrays.equals(items, wCombo.getItems()) && previous.equals(combo.getText())) { + return; + } + Listener[] modifyListeners = combo.getListeners(SWT.Modify); + Listener[] selectionListeners = combo.getListeners(SWT.Selection); + setListeners(combo, SWT.Modify, modifyListeners, false); + setListeners(combo, SWT.Selection, selectionListeners, false); + try { + wCombo.setItems(items); + if (!combo.isDisposed()) { + wCombo.setText(previous); + } + } finally { + setListeners(combo, SWT.Modify, modifyListeners, true); + setListeners(combo, SWT.Selection, selectionListeners, true); } } finally { repopulatingItems = false; } } + /** Adds or removes the listeners captured around a programmatic combo refresh. */ + private static void setListeners(CCombo combo, int eventType, Listener[] listeners, boolean add) { + if (combo.isDisposed() || listeners == null) { + return; + } + for (Listener listener : listeners) { + if (add) { + combo.addListener(eventType, listener); + } else { + combo.removeListener(eventType, listener); + } + } + } + /** * Load the selected element and return it. In case of errors, log them to LogChannel.UI *