Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Version specific store of app data, and preferences #396

Closed
wants to merge 58 commits into from

Conversation

Oliver-Loeffler
Copy link
Collaborator

@Oliver-Loeffler Oliver-Loeffler commented Aug 29, 2021

Application data, User Library and Preferences are now stored specific to each SceneBuilder version. Thus multiple versions can run in parallel.

Until now, all Scene Builder versions shared the same settings tree in file system (application data) which prevented parallel execution of Scene Builder versions. It was not possible, to compare e.g. SB11 with SB16 or SB8 side by side. This works now.

Also, user library was shared amongst all SB versions which in some cases lead to trouble as e.g. when there were multiple versions of the same control for different JDKs.

Issue

Fixes #369
Fixes #395 (partially)
Fixes #346

Progress

@Oliver-Loeffler
Copy link
Collaborator Author

There is one question - its about the "Manage Repositories" function. The configuration behind (its all in Preferences) is basically shareable between all versions. So this one could be designed as ashared element. Otherwise users probably wil complain, that they have to import their repository settings manually between different SB versions.

What do you think? This could be a different PR.

@luca-domenichini
Copy link
Contributor

As I said in #369 I think SB should store all settings per workspace location

@Oliver-Loeffler
Copy link
Collaborator Author

Okay, with the current status each SceneBuilder has its own repositories setup - which is not a workspace yet but each version has its own config. As a next step, in order to enable workspaces later, another PR could introduce the capability to save settings to files and read them from files.

@mimmoz81
For a possible workspace configuration, would you prefer a single file which holds all details or an sub directory such as .scenebuilder.workspace which holds settings in individual files?

@luca-domenichini
Copy link
Contributor

@Oliver-Loeffler I was thinking about that too..
A single json file should be enough to represent the entire ws settings and all the needed stuffs.
A folder though is more flexible..

@luca-domenichini
Copy link
Contributor

We should talk about that in a specific issue I think

@Oliver-Loeffler
Copy link
Collaborator Author

Issue #14 is probably the better space. Just found this one.

@abhinayagarwal abhinayagarwal added enhancement New feature or request major labels Aug 31, 2021
@abhinayagarwal abhinayagarwal added this to the 18 milestone Sep 1, 2021
@Oliver-Loeffler Oliver-Loeffler changed the title Version specific store of app data, and prreferences Version specific store of app data, and preferences Sep 14, 2021
@abhinayagarwal
Copy link
Collaborator

How do we make sure if an existing application is being updated, the newer version has all the settings/preferences from the older version?

@Oliver-Loeffler
Copy link
Collaborator Author

At least the following approaches are thinkable:

Approach 1

  • on first start of new SB version, scan for settings of previous versions
  • show a list of the versions found
  • ask user to import settings from one of the previous versions or to start from scratch

Approach 2

  • on first start of new SB version, scan for settings of previous versions
  • silently import settings of the most recent installed version

What do you think?

@abhinayagarwal
Copy link
Collaborator

I am more inclined towards Approach 2

@johanvos
Copy link
Contributor

johanvos commented Dec 8, 2021

I like the NetBeans approach here: when installing a new version, it says it detected an old version, and recommends copying the data from that old version.
So that would be a manual action, but the default would be to copy.
Hence, I have a slight preference for option 1, but I can live with both.

@Oliver-Loeffler
Copy link
Collaborator Author

I will have more time to dig into this again starting next week. Personally, I do prefer approach 1 over approach 2, I appreciate being asked over being forced to do things in a certain way. For that, let's see, I'll prepare a prototype for each approach so we can test this.

…c to each SceneBuilder version. Thus multiple versions can run in parallel.
@Oliver-Loeffler
Copy link
Collaborator Author

It took me a moment to get back into it but now I have a plan to create the prototype. I'm going to use the existing PR for that. Also it can be helpful to document where and how SceneBuilder stores its settings. As of now its only a a markdown snippet in docs/snippets.

@Oliver-Loeffler
Copy link
Collaborator Author

Oliver-Loeffler commented Jan 4, 2022

🎉 Happy new year to all friends of Scene Builder! 🎉

There is now the first prototype. It detects settings of older SB versions (well, until now its always SB_2.0) and asks one single time at first start if an import is desired. The user decision is memorized.

It works like that:

  • There is a type AppVersion which knows how to handle version numbers such as 1.0 or 1.1.1, a version such as 1.1.0 is considered to be more recent than 1.1 or 1.0.
  • Then there is also a type VersionedPreferences which combines an AppVersion with a Preferences node so that we once can handle version specific import behavior.
  • The PreferencesController now has the ability to provide a PreferencesImporter.
  • The import process takes place in SceneBuilderApp.handleLaunch() method, there the method PreferencesImporter .askForActionAndRun() is called where the process takes place.

The PreferencesController parametrizes the PreferencesImporter accordingly, so that the necessary initialization steps after import can take place (e.g. reload global prefs record, reload Maven settings or Repository settings). Otherwise a restart of SB would be needed after import.

I was thinking about approach 2, silently doing the import as @abhinayagarwal mentioned this. A silent import would match the way Scene Builder works as of now. So user experience would be as usual but with the benefit of the ability to operate different versions in parallel.

What works:

  • Scene Builder asks on start to import previous version Preferences (when available).
  • Global application preferences are imported
  • Maven artifact settings are imported
  • Repository configuration is imported
  • Import/transfer of custom user JAR files
  • All settings are refreshed after import, so that no restart of Scene Builder is needed

The MessageBox contents is not imported.

Remark:

  • For the sake of having Java records available some places, I enabled the compiler support for JDK17 in POM.xml. Well, it made me think that this could possibly a not desired change, thus I opened an issue and PR Upgrade Scene Builder Java compiler version to Java 17 #431 for discussion. If it is desired to turn back to Java 11, I'd rework the few places where the 17 features are use.

@abhinayagarwal & @johanvos What do you think?

@Oliver-Loeffler
Copy link
Collaborator Author

Oliver-Loeffler commented Jan 5, 2022

May be you can help me @abhinayagarwal:
The library directory can be big, so importing a user library can be time consuming. Would it be desirable to indicate progress for this cases?

interface to access platform directories in order to make the OS
specific behavior testable.
@Oliver-Loeffler
Copy link
Collaborator Author

Oliver-Loeffler commented Jan 23, 2022

Hello,

first of all my apologies, this one turned out getting larger than expected - the import was partially kind of tricky.
But now this one has reached a state of work, where I'd like to wait for your feedback before continuing.

After playing a while, I'm with @abhinayagarwal (approach 2) to do all the imports silently and never ask.

Goals of this PR:

  • enable co-existance of different Scene Builder versions with their individual settings
  • each version should have its own settings
  • newer versions can import existing settings and user library contents (added during discussion)
  • all settings directories are now version specific (so its no longer AppData\Roaming\Scene Builder but instead AppData\Roaming\Scene Builder 17.0.0.
  • newer versions do not touch any settings of older versions, both can coexist
  • The user "dot" directory ~/.scenebuilder remains the same but the log file is now version specific (i.e. scenebuilder-17.0.0.log.0)

It works as follows:

  • Scene Builder launches
  • The PreferencesImporter searches for preferences of older versions.
  • It will ask to import - if answered yes it will do so
  • The import decision is shared with UserLibraryImporter and it will start a JavaFX task to import user library in background.
  • 2 preference values are stored to prevent future imports (one for preferences import, one for user library import).
  • If one wants to enforce an import, run Scene Builder with -DforceImport=true option (currently fixed in parent POM).

ImportDecision

This PR consists of 3 larger changes:

To achieve this goal and to get most components of this tested, following work has been performed:

1 - version specific settings

  • Operating system detection moved into an enum type OperatingSystem
  • All the platform specific handling of settings directories and log files has been moved into PlatformSpecificDirectories class.
  • This all is used inside AppPlatform and now it allows to test almost everything.
  • The message box directory is not part of AppPlatformDirectories interface as the MB directory resides inside the application data dir.
  • Where which settings are stored and what each folder means is documented in AppConfiguration.md

AppPlatformDirectories

2 - Import of application specific preferences

  • There is now a class PreferencesImporter which can import the full application preferences tree of a previous verstion.
  • To distinguish versions and to provide an order (what is the previous one...) a record type AppVersion was introduced.
  • When multiple preferences trees exist, for each preferences tree a version can be assigned (e.g. SB_2.0 or SB_17.0.1 or SB_18.0.0-SNAPSHOT, hence a record VersionedPreferences was introduced.
  • The VersionedPreferencesFinder class then walks the Scene Builder preferences tree and searches for possibly existing older version settings.
  • Due to use of records, switched the project to Java-17 compiler level. However, if this is not desired, I'll revert the records to normal classes.
  • The app PreferencesController now allows to get an instance of the PreferencesImporter and there one can do the job like:
    PreferencesImporter prefsImporter = PreferencesController.getSingleton().getImporter();
    prefsImporter.askForActionAndRun();  
    // if the user opts out, the import action for User Library  is disabled via a preferences key
  • The PreferencesImporter also ensures that the PreferencesController reads all the settings again after completing the import. Otherwise a restart of Scene Builder would be needed.
  • If an import is needed is also stored in preferences.
  • For test purposes the import can be enforced by starting Scene Builder with -DforceImport=true (I've set this in POM.xml or ran it from IDE with that setting)
  • The alert asking for action has not received much care yet as I think its better to review the existing concept first.

PreferencesImport

3 - Import of user library

  • This task is handled by com.oracle.javafx.scenebuilder.app.library.user.UserLibraryImporter
  • This will only be executed if previous Scene Builder preferences exist - so if only the library directory exists it won't work.
  • As runtime was a concert here, I've selected a JavaFX task to do the job, thus all work is performed in background and the user basically won't notice it during startup
    // The import will start as soon as the user library is initialized.
    // Import status, if triggered, if failed or completed, will be written to log.
    // Only files are copied which do not exist in target library directory
    Platform.runLater(UserLibraryImporter.createImportTask());

UserLibImporter1

UserLibraryImporter2

The import progress and possible issues are stored in Scene Builder log file. Here it is then possible to see from which version the import was started and also which JARs were imported. I'll add a flow / state diagram to illustrate what where happens.

User library is imported using a task, so it runs in the background.
Progress and results are logged.
@Oliver-Loeffler Oliver-Loeffler marked this pull request as ready for review January 23, 2022 22:26
@Oliver-Loeffler Oliver-Loeffler changed the title Version specific store of app data, and preferences feat: Version specific store of app data, and preferences Feb 9, 2022
@Oliver-Loeffler
Copy link
Collaborator Author

@AlmasB Hi Almas, I'll make this one fit to properly merge with the current main line.
Would it possibly be helpful If I start a new PR with the desired change so that we can review the whole change set?
The previous discussion has lead to changes I had not anticipated upfront. What do you think?

@AlmasB
Copy link
Collaborator

AlmasB commented Mar 10, 2022

@Oliver-Loeffler

I've only skimmed through this, but I was wondering if you can split this into multiple smaller PRs: one for tests, one for production code change, one for refactor, etc. That way it's easier to review and manage scope. See if that is possible?

@Oliver-Loeffler
Copy link
Collaborator Author

Hello @AlmasB, exactly as you proposed, I'll rework this one and re-issue smaller PRs.

@Oliver-Loeffler Oliver-Loeffler deleted the issue-346 branch September 30, 2024 21:47
@Oliver-Loeffler Oliver-Loeffler restored the issue-346 branch September 30, 2024 21:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request major
Projects
None yet
6 participants