Refactored extensions manager

[lassoan]

Reworked the extensions manager to take advantage of new Girder_v1 APIs:

• batch download of extension metadata
• server provides extension dependency metadata

These enabled implementing the following improvements:

• Automatic update check and automatic installation can now be enabled for extensions.
  Previously, hundreds of requests were submitted to query all necessary extension
  metadata, which caused slowdowns and crashes. Now all metadata can be downloaded in
  batch (in 4 requests), very robustly, in 1-2 seconds.
  For testing purpose, batch query may also be disabled with setServerQueryWithLimitEnabled.

• If updates are available then a notification marker is displayed over the extensions
  manager toolbar icon.

• Extensions can be bookmarked and all reinstalled in a different Slicer version by a
  single click.
  Previously, all extensions that the user have ever installed was offered to be restored, and there was no option to remove any of them. Bookmarked extensions can be installed without the browser widget, which may be useful in cases where proxy servers block the communication between the extensions manager frontend and backend (because only the backend needs to be accessed).

• List of bookmarks can be edited as a simple text. This makes it easy to transfer favorite extensions between computers or install a set of extensions at training events.

• Extensions Manager is displayed immediately. The user does not need to wait for any downloads.
  All data is reloaded immediately from cache or downloaded asynchronously.

• The "manage" and "restore" tabs are merged into a single tab where user can install/uninstall/update/restore.

• All metadata are downloaded from Extensions Server. The metadata is cached for offline access and to reduce extensions server load.

• Dependent extensions are installed automatically by default, to simplify extension installation.
  Manual installation (user confirmation) can be enabled in extensions settings.

• All dependent extensions (even indirect dependencies) are determined at once before asking the user to install them, saving several extra clicks for some extensions.

• Extension version and update time (when the package was built) is displayed for the installed extension, and also if there was any update.

• Close/restart button is not enabled until batch processing (install all/update all extension) are completed.

• Multiple files can be selected when installing extension from file.

• Extensions server website can be opened from the extensions manager. This makes it easier to
  download extensions for offline installation from file or when firewall or proxy settings prevent
  the extensions manager from working.

• Fixed display of icons of many extensions: icons that were hosted on servers with 301-redirect were not displayed.

[lassoan]

Here is a demo video:

https://youtu.be/sKZGeko5wGE

@muratmaga please have a look, several features were motivated by your use cases and feature requests.

@pieper @cpinter @jamesobutler check this out and let me know if you have any comments.

[lassoan]

Addresses most issues/feature requests listed at:

• Extension updates not found in stable version #5908
• Add "Update Extension" option under Manage Extensions  #5470
• Add favorite extensions list to Extensions manager #5341
• Slicer 5.0 Extension Manager: Manage Extensions Tab icons #4936
• Add last modified date for extensions. #5469

[muratmaga]

@lassoan these look awesome. Thanks.

[lassoan]

I've added extension version and date display, too:

This works so nicely that I would love to see this made available for the current stable release, and not wait another 3 months for Slicer-5.2. What do you think?

[jamesobutler]

@lassoan I'm going to test this locally. Is ENH: Refactor extensions manager the only commit of interest here in this PR? The others are from some unrelated work?

[muratmaga]

This works so nicely that I would love to see this made available for the current stable release, and not wait another 3 months for Slicer-5.2. What do you think?

Yes, please. And the check for stable release updates as well (I liked the Welcome screen option)..

[jamesobutler]

In general I'm usually strict in new features and enhancements only being in feature release versions as from a user perspective changes in functionality in a patch release like Slicer 5.0.3 is unexpected. If it was to be included then all the build improvements and documentation backports should be considered for inclusion (#6398 (review)) because the patch release would not be under strict consideration. If the desire is to get it out sooner than my choice would be to release the next feature release Slicer 5.2 sooner. This 5.2 version would just have less new features than a typical feature release.

[jcfr]

And the check for stable release updates as well

@muratmaga That specific commit (BUG: Fix extensions update check) has already been back-ported on the maintenance branch. See 9c7dc54 and #6398 (commits)

[jamesobutler]

the check for stable release updates as well (I liked the Welcome screen option)..

@jcfr What is being referred to is #5502 (comment) which is update about a new stable version of Slicer being available to download.

[muratmaga]

In general I'm usually strict in new features and enhancements only being in feature release versions as from a user perspective changes in functionality in a patch release like Slicer 5.0.3 is unexpected.

I am going to disagree with this, as we rely on update extensions functionality quite a bit to keep the SlicerMorph up to date with changes and additions. Currently this is broken, which I would say is an important issue for a stable that's going to be in place for next few months. From practice we know how long it takes to do a new release, even with best of our intentions. So this stable is going to be downloaded an used quite a while.

[jamesobutler]

9c7dc54 is an appropriate bug fix to include in Slicer 5.0.3 which will solve the extension update issue present in Slicer 5.0.2. However improved functionality about updating extensions through one new tab, favorited/book marked extensions, etc all of that is new features which should be in Slicer 5.2.

[jcfr]

on update extensions functionality quite a bit [...] changes and additions. Currently this is broken

@muratmaga As indicated above by @jamesobutler , if you are talking about the issue we discussed during our last meeting, I this has been fixed and backported on the maintenance branch and will be part of v5.0.3. See #6417 (comment)

[jamesobutler]

• When I installed the "SegmentEditorExtraEffects" extension from the install tab it downloaded and install "SegmentEditorExtraEffects" and dependent extension "MarkupsToModel", but prior to restart the "Manage Extensions" tab indicated that there were 2 items, but there was only just the primary extension. After restarting both extensions were present and the manage extension count number matched the number of items in the table.

Before Restart	After Restart

Update: I think this was because it was still filtering using the terms I used for the "Install" extensions tab, so no issue here.

• I think having just the "Check for updates" button is sufficient where you can remove the action in the options menu to reduce the number of items there.
  

• When there are no bookmarks the "Install all bookmarks" button is disabled, however when then are no extensions the "Check for updates" button is still enabled. I would expect consistent logic applied where "Check for updates" is disabled until there are installed extensions to check for.
  => AL: fixed in 665e885

• Is there a functional need for the new option of "Automatically install all dependencies"? This seems like only allowing users to get into trouble? When would there be a reason to not install a dependency? Won't things not work? If things will work, then is it a true dependency?
  

• I manually entered in a fake entry of <input> into the "Edit Bookmarks" widget and noticed that when the entry can't be found there is still a "More" hyperlink in the row, but it doesn't go anywhere.
  => AL: fixed in 2e48952

[lassoan]

The patch makes it possible to update extensions, but without auto-update notification and the option being so much hidden, the feature is not practically available. We cannot expect that users will notice updates and update extensions.

I agree that there are tons of new features in this PR and lots of changes, so there is significant risk of regressions. These would be good reasons for excluding it from Slicer-5.0. However, preview releases don't have extension updates, so this feature could only be tested in a stable release. If we wait until Slicer-5.2, we just prevent from users accessing the new feature, but the feature will not get much testing, so it won't get more mature or more stable. Slicer5 has not been announced anyway yet, so our Slicer-5.0.x versions are kind of release candidates, so we don't violate our policies very harshly by allowing in some new features.

[lassoan]

When there are no bookmarks the "Install all bookmarks" button is disabled, however when then are no extensions the "Check for updates" button is still enabled. I would expect consistent logic applied where "Check for updates" is disabled until there are installed extensions to check for.

There is an important difference between bookmarks and udpates. Updates require network connection. Check for updates button must be always enabled because network connectivity comes and goes and the user must be able to trigger an update anytime.

When I installed the "SegmentEditorExtraEffects" extension from the install tab it downloaded and install "SegmentEditorExtraEffects" and dependent extension "MarkupsToModel", but prior to restart the "Manage Extensions" tab indicated that there were 2 items

The "(2)" means that clicking that button will install two updates, so that looks correct to me. We don't want filter&search to affect what extensions are will be installed/uninstalled/updated. If we find big inconsistencies then we can change the implementation, but if it's just a few questions/uncertainties here and there then it may be sufficient to document them in the user guide and/or tooltips.

Is there a functional need for the new option of "Automatically install all dependencies"? This seems like only allowing users to get into trouble? When would there be a reason to not install a dependency? Won't things not work? If things will work, then is it a true dependency?

Maybe we can remove the option. I added an option to allow preserving the current behavior. I agree that most of the time user should just install all required dependencies. We can remove the option to simplify the GUI if we all agree that there is no need to give users much control.

[jamesobutler]

Check for updates button must be always enabled because network connectivity comes and goes and the user must be able to trigger an update anytime.

• Does the "Check for updates" button need to be enabled when there are no extensions to manage in the table? My suggestion would be to have it enabled when there is n>0 extensions. => AL: fixed in 665e885

[jamesobutler]

• Previously the "Install from file" toolbutton was a normal toolbutton of just an icon + tooltip. Was there a reason for including text on your new toolbuttons? Do you think the new icons don't convey clearly the actions that they represent? The "Check for updates" icon is familiar enough to me, though "Install bookmarked" is not recognizable to me. The general bookmark icon is not the base shape of it so I wouldn't expect it to be bookmarked related unless I read the text. Therefore the icon for "Install bookmarked" seems pointless at the moment.
  
  

[jcfr]

I am finalizing #6082, once done. I will rebase this PR.

[lassoan]

Does the "Check for updates" button need to be enabled when there are no extensions to manage in the table? My suggestion would be to have it enabled when there is n>0 extensions.

Good point. Although clicking the button has a somewhat useful function (check if the server is available, if yes, download extension metadata, if not, display an error) there no visible result of the operation is expected, so it may make sense to disable.

• The general bookmark icon is not the base shape of it so I wouldn't expect it to be bookmarked related unless I read the text. Therefore the icon for "Install bookmarked" seems pointless at the moment.

All the buttons are standard google material design icons for update, install, and add operations, but their meaning are probably not obvious enough, and we have lots of space there, so adding the text is useful. Since we have the text, we could remove the icons, but I think they make the GUI look a bit nicer. If people don't like the icons then we can remove them.

[lassoan]

I am finalizing #6082, once done. I will rebase this PR.

Only the last commit is needed, the rest were just some unrelated experimental stuff from my development branch that I forgot to remove from my latest push (you can remove the other commits). The only thing I still need to fix is the extensions manager test.

[lassoan]

I think having just the "Check for updates" button is sufficient where you can remove the action in the options menu to reduce the number of items there.

In the button row, "Check for updates" turns into "Update all" button. This is nice because it simplifies the button row.
I've added "Check for updates" to the dropdown menu because sometimes I wanted to check for updates again, even if some updates have been already found. We could remove the update check from the dropdown menu, but maybe we could all use it for a while and then finalize these kind of tweaks.

[lassoan]

Is there a functional need for the new option of "Automatically install all dependencies"? This seems like only allowing users to get into trouble? When would there be a reason to not install a dependency? Won't things not work? If things will work, then is it a true dependency?

This is an interesting topic. Currently, we cannot distinguish between optional and required dependencies, so we can either consider all dependencies are optional, or all dependencies are required. I've added a note to #3704 where we can discuss this topic further.

[lassoan]

I've rebased the code, it should be all good except the extensions server test and documentation has not been updated yet.

[jcfr]

Summary of changes:

• Re-based
• Use of convertExtensionMetadata and filterExtensionMetadata
• Updated tests
• Added support for disabling batched query introducing setServerQueryWithOffsetEnabled/serverQueryWithOffsetEnabled, this was required to keep using a single file as URL substitute

Possible follow up changes:

• Add test for loaded and bookmarked states
• Consolidate convertExtensionMetadata and filterExtensionMetadata
• Revisit use of DescriptionKey name
• Simplify support for waitForCompletion using qRestAPI::sync

@lassoan After review the last commit adding setServerQueryWithOffsetEnabled/serverQueryWithOffsetEnabled, I will squash it.

[lassoan]

Looks good, just added some trivial comments.

[jcfr]

@lassoan This is now ready.

Once reviewed, I will squash the last three commits together.

I fixed the approach to instead pass limit=-1 when ServerQueryWithLimit is false, this effectively ensure to have all data retrieved at once.

[jcfr]

After I am done rebuilding and doing a last round of testing, I will squash and integrate.

[jcfr]

@lassoan The last commit I pushed ensure install works when AutoCheck has not been enabled already.

[jcfr]

@jamesobutler I plan on integrating this later this evening before the round of nightly build, if you have a chance to quickly check on your side as well. Would be nice 🙏

[jamesobutler]

Ok I will take a look again here soon and will report back so you have feedback before you integrate later tonight.

[jcfr]

Merci 🙏

[jamesobutler]

I don't see any issues with the changes in this PR. Below are some issues I have observed though that are still affecting the extensions manager for users.

I still get a stream of these warnings when using the extensions manager as previously mentioned in:
https://discourse.slicer.org/t/extension-manager-in-linux-empty/22343/7?u=jamesobutler
https://discourse.slicer.org/t/new-extension-manager-and-issues-with-corporate-certificates/19361?u=jamesobutler

Mixed Content: The page at 'https://extensions.slicer.org/catalog/All/31027/win?q=' was loaded over HTTPS, but requested an insecure image 'http://www.slicer.org/slicerWiki/images/c/c2/VolumeClipLogo.png'. This content should also be served over HTTPS.

@jcfr You mentioned the following in one of the above threads. Is this expected to be fixed as well or no?

This is because some of the extension icons are server over http.
This will be fixed in the next few weeks.

Also some extension icons aren't being rendered which I'm not sure why. It is an issue present in the extensions manager and in the online extensions catalog. For example BreastImplantAnalyzer specifies an icon in the s4ext file of the ExtensionsIndex and going to the path directly will show an icon, but that isn't rendered here.
https://github.com/Slicer/ExtensionsIndex/blob/96a5d4734de34995fe5d99028f5b89741ce32a9b/BreastImplantAnalyzer.s4ext#L31

[jcfr]

Thanks for the feedback.

Regarding the mixed content warnings, I indeed have a work in progress notebook going though all extensions and s4ext and effectively listing and/or fixing them.

Thanks for the reminder, it would indeed be nice to address this before announcing the release more broadly.

[lassoan]

Probably the icons are not taken from the s4ext file in the extension index but from the top-level CMakeLists.txt file.

BreastImplantAnalyzer has an invalid icon: https://github.com/lancelevine/SlicerBreastImplantAnalyzer/blob/master/CMakeLists.txt

[jcfr]

Indeed, I have a notebook almost finalized for checking sources, updating source and creating pull request

[lassoan]

I had a look at a couple of extensions and submitted a few PRs to fix icons - they are all some trivial issues (using outdated Slicer wiki links, using the github file's page instead of the download link, etc).

PR submitted:

• BUG: Fix links to extension icon and screenshot AIM-Harvard/SlicerRadiomics#73
• Fix links to icons ETS-vis-interactive/SlicerPRISMRendering#21
• Fix links to icons hina-shah/SlicerBatchAnonymize#17
• Fix extension icon and screenshot URLs DCBIA-OrthoLab/ShapeVariationAnalyzer#66
• Fix links to icons and move HowTo file content to the main markdown page SNRLab/SNRMeasurement#4
• https://github.com/lassoan/SlicerDRRGenerator.git

@jcfr could you take care of these Kitware extensions?

• https://github.com/NIRALUser/SPHARM-PDM/blob/master/CMakeLists.txt
• https://github.com/KitwareMedical/OsteotomyPlanner/wiki

@jamesobutler could you help submitting PRs to fix icons of these extensions? (maybe not all, just as much time as you can contribute)

• https://github.com/gsi-biomotion/Slicer-RegQA.git
• https://github.com/AldrickF/SlicerRegularizedFastMarching
• https://github.com/Ting-Jia/ModelClip.git
• https://github.com/JonathanRamos/SlicerCleverSegmentation.git
• https://github.com/lancelevine/SlicerBreastImplantAnalyzer.git
• https://github.com/CSIM-Toolkits/SlicerBrainVolumeRefinement.git
• https://github.com/DCBIA-OrthoLab/DatabaseInteractorExtension.git

[jamesobutler]

Probably the icons are not taken from the s4ext file in the extension index but from the top-level CMakeLists.txt file.

Are the icons for the ExtensionsManager/ExtensionsCatalog supposed to be using the links specified in the Extensions Index s4ext files? Is the icon url specified in the s4ext in the Extensions Index used for anything?

[lassoan]

Are the icons for the ExtensionsManager/ExtensionsCatalog supposed to be using the links specified in the Extensions Index s4ext files? Is the icon url specified in the s4ext in the Extensions Index used for anything?

The s4ext file in the ExtensionsIndex repository is only used by the factory machine for getting repository URLs and dependencies. All other information is ignored.

The Extensions Server does not know about s4ext files in the ExtensionsIndex repository, but instead it populates its database from the ExtensionName.zip/share/Slicer-5.0/ExtensionName.s4ext file of each uploaded package. This s4ext file created from the content of the CMakeLists.txt file during extension build.

I find the redundancy between the CMakeLists.txt and s4ext files in ExtensionsIndex quite confusing and it is a frequent cause of errors. Maybe we could remove all the other fields from these s4ext files to avoid redundancy. Or we could replace the s4ext files by a single json or csv file in the extensions index that contains the list of extension names, repository urls, and dependencies. That said, I would do this only when we touch the extensions infrastructure anyway. I've created an issue to keep track of this: #6434

[lassoan]

Since this PR is closed and we already have an issue for the extension icons, I copy the details to that ticket (as a checklist) and we can continue the discussion there: #4936.

[jamesobutler]

@jamesobutler could you help submitting PRs to fix icons of these extensions? (maybe not all, just as much time as you can contribute)

@lassoan I've completed submitting PRs to all the ones you assigned to me. I've provided links to the PRs in #4936 where it is being tracked.

[lassoan]

Awesome, you @jamesobutler. Hopefully maintainers will respond to our PRs.