scenebuilder icon indicating copy to clipboard operation
scenebuilder copied to clipboard

feat: Version specific store of app data, and preferences

Open Oliver-Loeffler opened this issue 2 years ago • 19 comments

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 avatar Aug 29 '21 10:08 Oliver-Loeffler

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.

Oliver-Loeffler avatar Aug 29 '21 10:08 Oliver-Loeffler

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

luca-domenichini avatar Aug 29 '21 22:08 luca-domenichini

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?

Oliver-Loeffler avatar Aug 30 '21 09:08 Oliver-Loeffler

@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 avatar Aug 30 '21 12:08 luca-domenichini

We should talk about that in a specific issue I think

luca-domenichini avatar Aug 30 '21 12:08 luca-domenichini

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

Oliver-Loeffler avatar Aug 30 '21 13:08 Oliver-Loeffler

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

abhinayagarwal avatar Dec 07 '21 14:12 abhinayagarwal

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?

Oliver-Loeffler avatar Dec 07 '21 14:12 Oliver-Loeffler

I am more inclined towards Approach 2

abhinayagarwal avatar Dec 08 '21 06:12 abhinayagarwal

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.

johanvos avatar Dec 08 '21 09:12 johanvos

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.

Oliver-Loeffler avatar Dec 09 '21 22:12 Oliver-Loeffler

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 avatar Dec 22 '21 21:12 Oliver-Loeffler

:tada: Happy new year to all friends of Scene Builder! :tada:

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 #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 avatar Jan 04 '22 20:01 Oliver-Loeffler

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?

Oliver-Loeffler avatar Jan 05 '22 20:01 Oliver-Loeffler

Feedback is always important to the user. If an application doesn't respond for a few seconds, it can lead to bad UX. Therefore, if you think that import will take more than a second, I preference would be to add a progress indicator somewhere.

abhinayagarwal avatar Jan 13 '22 07:01 abhinayagarwal

Okay, so let me prepare a dialog with progress bar for the import (only for the part when libraries are imported), the preferences part works really quick.

Oliver-Loeffler avatar Jan 13 '22 17:01 Oliver-Loeffler

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.

Oliver-Loeffler avatar Jan 23 '22 20:01 Oliver-Loeffler

@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?

Oliver-Loeffler avatar Mar 09 '22 20:03 Oliver-Loeffler

@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?

AlmasB avatar Mar 10 '22 12:03 AlmasB

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

Oliver-Loeffler avatar Oct 01 '22 14:10 Oliver-Loeffler