Skip to content

Conversation

@palexdev
Copy link
Collaborator

This PR aims at reworking the workspace API, moving from a generic map of strings to a dedicated class that manages both reads and writes in a safe and typed way.

Details:

  1. The WorkspaceController loads a workspace JSON file and keeps a reference to the current loaded workspace, which is a dedicated class: WorkspaceData.
  2. Then it sends the event to all the tools to restore their state from the workspace data. The LoadWorkspaceResponse has been changed to carry the WorkspaceData instead of the file and the "plain" data.
  3. Since PDFSam uses nested maps to carry states to each individual tool, there's another dedicated class inside WorkspaceData which is: ToolData. This class wraps the workspace file and the specific tool data as a Map<String, String>. A tool can retrieve its state from the workspace data by calling workspaceData.getToolData(this). I preferred this way rather than having a generic getToolData(String toolId) method which may be error prone, less secure.
  4. In BaseToolPanel, the onLoadWorkspace(...) method has been changed to accept the WorkspaceData instance. Each tool panel retrieves the tool data via the aforementioned method and calls restoreStateFrom(...) on its subcomponents (the AlternateMixToolPanel being an exception since it also manipulates the data for backwards compatibility)
  5. The restoreStateFrom(...) method from RestorableView has also been changed to accept ToolData rather than a generic map

Flow

WorkspaceController -> LoadWorkspaceResponse(WorkspaceData) -> BaseToolPanel(WorkspaceData -> ToolData) -> RestorableView(ToolData)

Summary

All in all, WorkspaceData and ToolData are just wrappers for plain, generic maps. In fact, the goal of the first commit was to preserve as much as possible of the existing code, while just moving the logic to a single place so that it can more easily be tested, managed, changed.

Three more things to note:

  1. The first commit says 'Part 1' because for, now only the loading API has been reworked. The save API still accepts maps.
  2. WorkspaceData already implements an enhancement for issue Relative paths in .json files #710, and as requested it's not a fallback system, but expects a relative.paths property to be set to true in the workspace file. Since ToolData is responsible for resolving the paths (because it's the class carrying the data to the specific tool), the wrapped workspace file determines if the relative path resolution is enabled. If the file is null, then it's disabled.
  3. This enhancement follows a similar path as PR WIP: Ask the user to save the workspace if it changed before closing the app #763, which defines a separate class to keep track of the loaded workspace and its hash. We could consider merging that PR here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants