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.

Reply via email to