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 80c5f99fdf issue #7414 : Shell states not cleared on restart (#7416)
80c5f99fdf is described below
commit 80c5f99fdf46523dad742faee248f19c3b9bb3ad
Author: Matt Casters <[email protected]>
AuthorDate: Fri Jul 3 07:34:09 2026 +0200
issue #7414 : Shell states not cleared on restart (#7416)
---
.../java/org/apache/hop/history/AuditManager.java | 11 ++++
.../java/org/apache/hop/history/IAuditManager.java | 9 +++
.../hop/history/local/LocalAuditManager.java | 15 +++++
.../org/apache/hop/history/AuditManagerTest.java | 22 +++++++
.../org/apache/hop/ui/hopgui/HopWebEntryPoint.java | 4 +-
.../main/java/org/apache/hop/ui/core/PropsUi.java | 67 +++++++++++++---------
.../main/java/org/apache/hop/ui/hopgui/HopGui.java | 4 +-
.../tabs/ConfigGeneralOptionsTab.java | 35 ++++++++++-
.../ui/pipeline/transform/BaseTransformDialog.java | 27 +--------
.../core/dialog/messages/messages_en_US.properties | 4 +-
10 files changed, 141 insertions(+), 57 deletions(-)
diff --git a/engine/src/main/java/org/apache/hop/history/AuditManager.java
b/engine/src/main/java/org/apache/hop/history/AuditManager.java
index 40c2c3c6ee..cc1d95a5c0 100644
--- a/engine/src/main/java/org/apache/hop/history/AuditManager.java
+++ b/engine/src/main/java/org/apache/hop/history/AuditManager.java
@@ -151,4 +151,15 @@ public class AuditManager {
public static final void clearEvents() throws HopException {
getActive().clearEvents();
}
+
+ /**
+ * Convenience method for clearing stored states for a given group and type.
+ *
+ * @param group The group (namespace, environment)
+ * @param type The type (shell, perspective, ...)
+ * @throws HopException
+ */
+ public static final void clearStates(String group, String type) throws
HopException {
+ getActive().clearStates(group, type);
+ }
}
diff --git a/engine/src/main/java/org/apache/hop/history/IAuditManager.java
b/engine/src/main/java/org/apache/hop/history/IAuditManager.java
index 3d12c8c16b..888f383ba8 100644
--- a/engine/src/main/java/org/apache/hop/history/IAuditManager.java
+++ b/engine/src/main/java/org/apache/hop/history/IAuditManager.java
@@ -128,4 +128,13 @@ public interface IAuditManager {
* @throws HopException
*/
void clearEvents() throws HopException;
+
+ /**
+ * Clear all stored states for a given group and type.
+ *
+ * @param group The group (namespace, environment)
+ * @param type The type (shell, perspective, ...)
+ * @throws HopException
+ */
+ void clearStates(String group, String type) throws HopException;
}
diff --git
a/engine/src/main/java/org/apache/hop/history/local/LocalAuditManager.java
b/engine/src/main/java/org/apache/hop/history/local/LocalAuditManager.java
index 2d68c71678..8721740c56 100644
--- a/engine/src/main/java/org/apache/hop/history/local/LocalAuditManager.java
+++ b/engine/src/main/java/org/apache/hop/history/local/LocalAuditManager.java
@@ -357,4 +357,19 @@ public class LocalAuditManager implements IAuditManager {
throw new HopException(e);
}
}
+
+ @Override
+ public void clearStates(String group, String type) throws HopException {
+ if (StringUtils.isEmpty(group)) {
+ throw new HopException("An audit state clear needs a group");
+ }
+ if (StringUtils.isEmpty(type)) {
+ throw new HopException("An audit state clear needs a type");
+ }
+ String filename = calculateStateFilename(group, type);
+ File file = new File(filename);
+ if (file.exists() && !file.delete()) {
+ throw new HopException("Unable to delete state file '" + filename + "'");
+ }
+ }
}
diff --git a/engine/src/test/java/org/apache/hop/history/AuditManagerTest.java
b/engine/src/test/java/org/apache/hop/history/AuditManagerTest.java
index 1b416ce6d3..8fd67ca6f5 100644
--- a/engine/src/test/java/org/apache/hop/history/AuditManagerTest.java
+++ b/engine/src/test/java/org/apache/hop/history/AuditManagerTest.java
@@ -18,6 +18,7 @@ package org.apache.hop.history;
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 static org.mockito.ArgumentMatchers.any;
import static org.mockito.Mockito.times;
import static org.mockito.Mockito.verify;
@@ -26,7 +27,9 @@ import static org.mockito.Mockito.when;
import java.nio.file.Path;
import java.util.ArrayList;
import java.util.Date;
+import java.util.HashMap;
import java.util.List;
+import java.util.Map;
import org.apache.hop.core.exception.HopException;
import org.apache.hop.history.local.LocalAuditManager;
import org.junit.jupiter.api.BeforeEach;
@@ -158,4 +161,23 @@ class AuditManagerTest {
AuditManager.findEvents(group, "type1", null, 10, false).size(),
"Problem in clearing events");
}
+
+ @Test
+ void testClearStates() throws HopException {
+ String group = "hop-gui";
+ String type = "shells";
+ Map<String, Object> stateProperties = new HashMap<>();
+ stateProperties.put("x", 100);
+ stateProperties.put("y", 200);
+ AuditManager.storeState(
+ org.apache.hop.core.logging.LogChannel.GENERAL, group, type,
"TestDialog", stateProperties);
+
+ AuditStateMap stateMap = AuditManager.getActive().loadAuditStateMap(group,
type);
+ assertEquals(1, stateMap.getNameStateMap().size(), "State should be
stored");
+
+ AuditManager.clearStates(group, type);
+
+ stateMap = AuditManager.getActive().loadAuditStateMap(group, type);
+ assertTrue(stateMap.getNameStateMap().isEmpty(), "State should be
cleared");
+ }
}
diff --git a/rap/src/main/java/org/apache/hop/ui/hopgui/HopWebEntryPoint.java
b/rap/src/main/java/org/apache/hop/ui/hopgui/HopWebEntryPoint.java
index c0a500d52d..d1ca2573f8 100644
--- a/rap/src/main/java/org/apache/hop/ui/hopgui/HopWebEntryPoint.java
+++ b/rap/src/main/java/org/apache/hop/ui/hopgui/HopWebEntryPoint.java
@@ -152,7 +152,9 @@ public class HopWebEntryPoint extends AbstractEntryPoint {
HopGui.getInstance().setCommandLineArguments(args);
HopGui.getInstance().setShell(parent.getShell());
- HopGui.getInstance().setProps(PropsUi.getInstance());
+ PropsUi props = PropsUi.getInstance();
+ HopGui.getInstance().setProps(props);
+ props.clearPersistedDialogPositionsOnStartupIfConfigured();
// When user changes theme in Configuration → GUI options, redirect so the
new theme takes
// effect. Boolean null = "follow system" (run system redirect, don't use
dark flag).
diff --git a/ui/src/main/java/org/apache/hop/ui/core/PropsUi.java
b/ui/src/main/java/org/apache/hop/ui/core/PropsUi.java
index d44f89d34a..2501ace11c 100644
--- a/ui/src/main/java/org/apache/hop/ui/core/PropsUi.java
+++ b/ui/src/main/java/org/apache/hop/ui/core/PropsUi.java
@@ -30,7 +30,6 @@ import org.apache.hop.core.logging.LogChannel;
import org.apache.hop.core.util.Utils;
import org.apache.hop.history.AuditManager;
import org.apache.hop.history.AuditState;
-import org.apache.hop.history.AuditStateMap;
import org.apache.hop.ui.core.gui.GuiResource;
import org.apache.hop.ui.core.gui.WindowProperty;
import org.apache.hop.ui.core.widget.OsHelper;
@@ -93,6 +92,7 @@ public class PropsUi extends Props {
private static final String METRICS_ABOVE_SELECTED_TRANSFORMS =
"MetricsAboveSelectedTransforms";
private static final String ENABLE_INFINITE_CANVAS_MOVE =
"EnableInfiniteCanvasMove";
private static final String USE_ADVANCED_TERMINAL = "UseAdvancedTerminal";
+ private static final String REMEMBER_DIALOG_POSITIONS =
"RememberDialogPositions";
private static final String RESET_DIALOG_POSITIONS_ON_RESTART =
"ResetDialogPositionsOnRestart";
/** Max characters shown in a preview grid cell before truncation (0 = no
truncation). */
@@ -413,6 +413,9 @@ public class PropsUi extends Props {
}
public void setScreen(WindowProperty windowProperty) {
+ if (!getRememberDialogPositions()) {
+ return;
+ }
AuditManager.storeState(
LogChannel.UI,
HopGui.DEFAULT_HOP_GUI_NAMESPACE,
@@ -422,7 +425,7 @@ public class PropsUi extends Props {
}
public WindowProperty getScreen(String windowName) {
- if (windowName == null) {
+ if (windowName == null || !getRememberDialogPositions()) {
return null;
}
AuditState auditState =
@@ -442,6 +445,9 @@ public class PropsUi extends Props {
* @param windowProperty the window property containing position/size
information
*/
public void setSessionScreen(WindowProperty windowProperty) {
+ if (!getRememberDialogPositions()) {
+ return;
+ }
if (windowProperty != null && windowProperty.getName() != null) {
sessionWindowProperties.put(windowProperty.getName(), windowProperty);
}
@@ -455,7 +461,7 @@ public class PropsUi extends Props {
* @return the stored WindowProperty, or null if not found in session storage
*/
public WindowProperty getSessionScreen(String windowName) {
- if (windowName == null) {
+ if (windowName == null || !getRememberDialogPositions()) {
return null;
}
return sessionWindowProperties.get(windowName);
@@ -470,37 +476,46 @@ public class PropsUi extends Props {
}
/**
- * Clears all persisted window positions from storage (except the main
window). This removes saved
- * dialog positions from the audit state, forcing dialogs to center on their
next opening.
+ * Clears all persisted window positions from storage. This removes saved
dialog positions from
+ * the audit state, forcing dialogs to center on their next opening.
*/
public void clearPersistedDialogScreens() {
try {
- // Load all stored shell states
- AuditStateMap shellStates =
-
AuditManager.getActive().loadAuditStateMap(HopGui.DEFAULT_HOP_GUI_NAMESPACE,
"shells");
-
- if (shellStates != null && shellStates.getNameStateMap() != null) {
- // Create a new map with only the main window state
- AuditStateMap filteredStates = new AuditStateMap();
- Map<String, AuditState> stateMap = shellStates.getNameStateMap();
-
- for (Map.Entry<String, AuditState> entry : stateMap.entrySet()) {
- String windowName = entry.getKey();
- // Keep the main window position (identified by "Hop" in the name)
- if (windowName != null && (windowName.equals("Hop") ||
windowName.contains("Hop"))) {
- filteredStates.getNameStateMap().put(windowName, entry.getValue());
- }
- }
-
- // Save the filtered map (only main window)
- AuditManager.getActive()
- .saveAuditStateMap(HopGui.DEFAULT_HOP_GUI_NAMESPACE, "shells",
filteredStates);
- }
+ AuditManager.clearStates(HopGui.DEFAULT_HOP_GUI_NAMESPACE, "shells");
} catch (Exception e) {
LogChannel.UI.logError("Error clearing persisted dialog positions", e);
}
}
+ /**
+ * Clears persisted dialog positions on application startup when dialog
positions are not
+ * persisted to disk. Must be called before the main shell is sized.
+ */
+ public void clearPersistedDialogPositionsOnStartupIfConfigured() {
+ if (!getResetDialogPositionsOnRestart()) {
+ return;
+ }
+ clearPersistedDialogScreens();
+ }
+
+ /**
+ * Gets whether dialog positions should be remembered at all.
+ *
+ * @return true if dialog positions should be remembered, false otherwise
+ */
+ public boolean getRememberDialogPositions() {
+ return YES.equalsIgnoreCase(getProperty(REMEMBER_DIALOG_POSITIONS, YES));
+ }
+
+ /**
+ * Sets whether dialog positions should be remembered at all.
+ *
+ * @param remember true to remember dialog positions, false to always use
defaults
+ */
+ public void setRememberDialogPositions(boolean remember) {
+ setProperty(REMEMBER_DIALOG_POSITIONS, remember ? YES : NO);
+ }
+
/**
* Gets whether dialog positions should be reset on application restart.
When true, dialogs will
* only remember their position during the current session. When false,
dialog positions will be
diff --git a/ui/src/main/java/org/apache/hop/ui/hopgui/HopGui.java
b/ui/src/main/java/org/apache/hop/ui/hopgui/HopGui.java
index 94a264a450..920bbd46a8 100644
--- a/ui/src/main/java/org/apache/hop/ui/hopgui/HopGui.java
+++ b/ui/src/main/java/org/apache/hop/ui/hopgui/HopGui.java
@@ -409,7 +409,9 @@ public class HopGui
HopGui hopGui = HopGui.getInstance();
hopGui.getCommandLineArguments().addAll(Arrays.asList(arguments));
- hopGui.setProps(PropsUi.getInstance());
+ PropsUi props = PropsUi.getInstance();
+ hopGui.setProps(props);
+ props.clearPersistedDialogPositionsOnStartupIfConfigured();
// Add and load the Hop GUI Plugins...
// - Load perspectives
diff --git
a/ui/src/main/java/org/apache/hop/ui/hopgui/perspective/configuration/tabs/ConfigGeneralOptionsTab.java
b/ui/src/main/java/org/apache/hop/ui/hopgui/perspective/configuration/tabs/ConfigGeneralOptionsTab.java
index 7b6c3cda35..6375f842e9 100644
---
a/ui/src/main/java/org/apache/hop/ui/hopgui/perspective/configuration/tabs/ConfigGeneralOptionsTab.java
+++
b/ui/src/main/java/org/apache/hop/ui/hopgui/perspective/configuration/tabs/ConfigGeneralOptionsTab.java
@@ -68,6 +68,7 @@ public class ConfigGeneralOptionsTab {
private Button wbUseGlobalFileBookmarks;
private Button wSortFieldByName;
private Text wMaxExecutionLoggingTextSize;
+ private Button wRememberDialogPositions;
private Button wResetDialogPositions;
private boolean isReloading = false; // Flag to prevent saving during reload
@@ -108,9 +109,13 @@ public class ConfigGeneralOptionsTab {
wMaxExecutionLoggingTextSize.setText(
Integer.toString(props.getMaxExecutionLoggingTextSize()));
- // Only reload if widget is initialized
+ // Only reload if widgets are initialized
+ if (wRememberDialogPositions != null &&
!wRememberDialogPositions.isDisposed()) {
+
wRememberDialogPositions.setSelection(props.getRememberDialogPositions());
+ }
if (wResetDialogPositions != null &&
!wResetDialogPositions.isDisposed()) {
wResetDialogPositions.setSelection(props.getResetDialogPositionsOnRestart());
+ wResetDialogPositions.setEnabled(props.getRememberDialogPositions());
}
} finally {
// Always reset the flag
@@ -443,8 +448,28 @@ public class ConfigGeneralOptionsTab {
dialogPosLayout.marginHeight = PropsUi.getFormMargin();
dialogPosContent.setLayout(dialogPosLayout);
- // Reset dialog positions checkbox
+ // Dialog position checkboxes
Control lastDialogPosControl = null;
+ wRememberDialogPositions =
+ createCheckbox(
+ dialogPosContent,
+ "EnterOptionsDialog.RememberDialogPositions.Label",
+ "EnterOptionsDialog.RememberDialogPositions.Tooltip",
+ props.getRememberDialogPositions(),
+ lastDialogPosControl,
+ margin);
+ lastDialogPosControl = wRememberDialogPositions;
+ wRememberDialogPositions.addListener(
+ SWT.Selection,
+ e -> {
+ boolean remember = wRememberDialogPositions.getSelection();
+ wResetDialogPositions.setEnabled(remember);
+ if (!remember) {
+ props.clearSessionScreens();
+ props.clearPersistedDialogScreens();
+ }
+ });
+
wResetDialogPositions =
createCheckbox(
dialogPosContent,
@@ -453,6 +478,7 @@ public class ConfigGeneralOptionsTab {
props.getResetDialogPositionsOnRestart(),
lastDialogPosControl,
margin);
+ wResetDialogPositions.setEnabled(props.getRememberDialogPositions());
lastDialogPosControl = wResetDialogPositions;
// Clear dialog positions button
@@ -717,7 +743,10 @@ public class ConfigGeneralOptionsTab {
wMaxExecutionLoggingTextSize.getText(),
PropsUi.DEFAULT_MAX_EXECUTION_LOGGING_TEXT_SIZE));
- // Only save if widget is initialized (it's created after other widgets)
+ // Only save if widgets are initialized (they are created after other
widgets)
+ if (wRememberDialogPositions != null &&
!wRememberDialogPositions.isDisposed()) {
+
props.setRememberDialogPositions(wRememberDialogPositions.getSelection());
+ }
if (wResetDialogPositions != null && !wResetDialogPositions.isDisposed()) {
props.setResetDialogPositionsOnRestart(wResetDialogPositions.getSelection());
}
diff --git
a/ui/src/main/java/org/apache/hop/ui/pipeline/transform/BaseTransformDialog.java
b/ui/src/main/java/org/apache/hop/ui/pipeline/transform/BaseTransformDialog.java
index b7b7a385f8..3dfac5bccb 100644
---
a/ui/src/main/java/org/apache/hop/ui/pipeline/transform/BaseTransformDialog.java
+++
b/ui/src/main/java/org/apache/hop/ui/pipeline/transform/BaseTransformDialog.java
@@ -417,13 +417,8 @@ public abstract class BaseTransformDialog extends Dialog
implements ITransformDi
public void dispose() {
WindowProperty winprop = new WindowProperty(shell);
- // Always save to session storage for immediate reopening during current
session
props.setSessionScreen(winprop);
-
- // If user wants to persist dialog positions across restarts, also save to
persistent storage
- if (!props.getResetDialogPositionsOnRestart()) {
- props.setScreen(winprop);
- }
+ props.setScreen(winprop);
shell.dispose();
}
@@ -835,17 +830,6 @@ public abstract class BaseTransformDialog extends Dialog
implements ITransformDi
setSize(shell, minWidth, minHeight, false);
}
- /**
- * Checks if a window name represents the main Hop GUI window.
- *
- * @param windowName the window title to check
- * @return true if this is the main window, false otherwise
- */
- private static boolean isMainWindow(String windowName) {
- // The main window is identified by the "Hop" title or the localized
application name
- return windowName != null && (windowName.equals("Hop") ||
windowName.contains("Hop"));
- }
-
/**
* Sets the size of this dialog with respect to the given parameters.
*
@@ -857,16 +841,9 @@ public abstract class BaseTransformDialog extends Dialog
implements ITransformDi
public static void setSize(Shell shell, int minWidth, int minHeight, boolean
packIt) {
PropsUi props = PropsUi.getInstance();
- // Check session-only storage first (for dialogs during current session)
WindowProperty winprop = props.getSessionScreen(shell.getText());
-
- // If not in session storage, check persistent storage if user wants to
persist positions
- // (or if it's the main window - main window should always restore from
persistent storage)
if (winprop == null) {
- // Only check persistent storage if reset setting is disabled, OR for
main window
- if (!props.getResetDialogPositionsOnRestart() ||
isMainWindow(shell.getText())) {
- winprop = props.getScreen(shell.getText());
- }
+ winprop = props.getScreen(shell.getText());
}
if (winprop != null) {
diff --git
a/ui/src/main/resources/org/apache/hop/ui/core/dialog/messages/messages_en_US.properties
b/ui/src/main/resources/org/apache/hop/ui/core/dialog/messages/messages_en_US.properties
index b6d4cbe6eb..36e49f5d15 100644
---
a/ui/src/main/resources/org/apache/hop/ui/core/dialog/messages/messages_en_US.properties
+++
b/ui/src/main/resources/org/apache/hop/ui/core/dialog/messages/messages_en_US.properties
@@ -136,8 +136,10 @@ EnterOptionsDialog.ResetConfirmations.Tooltip=Enable all
confirmation dialogs an
EnterOptionsDialog.ResetConfirmations.Success=All confirmation dialogs have
been enabled.
EnterOptionsDialog.ResetTooltips.Label=Reset all to enabled
EnterOptionsDialog.ResetTooltips.Tooltip=Enable all tooltip options
+EnterOptionsDialog.RememberDialogPositions.Label=Remember dialog positions
+EnterOptionsDialog.RememberDialogPositions.Tooltip=When enabled, dialogs
remember their position and size.\nWhen disabled, dialogs always open at their
default position and size.
EnterOptionsDialog.ResetDialogPositions.Label=Reset dialog positions after
restart
-EnterOptionsDialog.ResetDialogPositions.Tooltip=When enabled, dialog positions
will only be remembered during the current session.\nWhen disabled, dialog
positions will be persisted across application restarts.
+EnterOptionsDialog.ResetDialogPositions.Tooltip=When enabled, dialog positions
are remembered during the current session only and shells-state.json is cleared
on restart.\nWhen disabled, dialog positions are also saved to
shells-state.json and restored after restart.\nOnly applies when dialog
positions are remembered.
EnterOptionsDialog.ClearDialogPositions.Label=Clear dialog positions
EnterOptionsDialog.ClearDialogPositions.Tooltip=Clears all remembered dialog
positions and resets the perspective tree-panel widths to their defaults
EnterOptionsDialog.ClearDialogPositions.Confirmation=All dialog positions have
been cleared and the perspective tree-panel widths have been reset to their
defaults.