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

Add ability to check for, download, and apply add-on updates #6930

Closed
wants to merge 94 commits into from

Conversation

josephsl
Copy link
Collaborator

@josephsl josephsl commented Mar 5, 2017

Hi,

Added (July 2018): reformatted to use PR template.

Link to issue number:

#3208

Summary of the issue:

The client portion of add-on update facility

Description of how this pull request fixes the issue:

Add abilitty to check for, download, and apply add-on updates from NVDA itself.

Note that this is a prototype client implementation. This does not support automatic updates nor actual download as the server portion and data exchange format isn't available yet.

Automatic add-on update checks require careful design, as NVDA Core update should be given priority when it comes to checking for updates (that is, if NVDA Core is being updated or an update is available, add-on updates should not take place).

Testing performed:

Checked for, downloaded, and applied various add-on updates from community add-ons website (through use of fake JSON dictionaary+PHP file provided by the add-ons website).

Known issues with pull request:

This is a prototype implementation and will be ready to go once the server, database, and exchange format are published. Also, this PR will rely on changes made in #7491 (checkable list box), as well as looking at minimum NVDA version (#6275).

Change log entry:

New feature: NVDA can now check, download, and apply add-on updates. (#3208)
Changes for developers: Add-ons can specify update channel in their manifest, used to look up add-on update channel.

Thanks.

… shows a message. re nvaccess#3208.

First of a series of foundations for add-on update facility: add a 'Check for Add-on Update' (shortcut: U) that'll let users update installed add-ons. For now, the button is non-functional - shows a message box until the client is working.
…ns manager progress dialog changes. re nvaccess#3208.

Add-on handler: add a new function (checkForAddonUpdates) that'll allow NvDA to check for add-on updates. This is to be used from add-ons manager (manual) or when NVDA starts (automatic). The goal of this function is to gather add-on listings, connect online to retrieve add-on update information, and return this info in a format that a new add-on update result dialog will use to display and download add-on bundles. The add-on update check should not run if NVDA itself is being updated (NvDA Core takes precedence).
In add-ons manager, the progress bar dialog used for add-on installation is now an attribute of the dialog itself, as it is now used for update status message as well. Also, update check button briefly displays progress dialog and returns to add-ons manager. Eventually a new add-on update result dialog will be added that'll display add-on update info returned by addonHandler.checkForAddonUpdates and download new bundles.
…on check emulation in addonHandler.checkForAddonUpdates function. re nvaccess#3208.

A prototype of add-on update check results dialog has been implemented:
* When 'update add-ons' button is pressed, a background thread will call addonHandler.checkForAddonUpdates. This function will emulate a typical add-on update check routine by sleeping for three seconds, then populating the results dictionary with records of installed add-ons, with add-on names (identifiers) as keys and containing summary and version information.
* Once update info is retrieved, the update progress bar stops, and a results dialog will be shown. While checking for updates, Add-ons Manager window will be hidden until a suitable way of showing add-on update check progress and results with Add-ons Manager window disabled is found.
* The AddonUpdatesDialog (the results dialog class) borrows heavily from NVDA Core update results dialog, displaying a list of add-ons with updates (at least emulated) or a message if update info is filled in or not, respectively.
* For now, when 'update' button is pressed from results dialog, it'll do nothing. Eventually the update button will use a refactored file downloader to download and install updates, using the file path and hash that'll be supplied by the update info dictionary.
…. re nvaccess#3208.

Similar to add-on instalaltion routines but with a difference: no prompts will be shown when updating add-ons unless serious errors occur. The new updateAddons function takes a map of new add-ons to be installed, which is a map with add-on identifiers (names) as keys and info such as path as subkeys. This function will be called when user tells NVDA to update add-ons from update results dialog.
… involves sending and receiving data in JSON format. re nvaccess#3208.

Javascript Object Notation (JSON) is a handy way to serialize and deserialize data across various systems, including sending data over a network. It can be used to send an receive various data, such as nested dictionaries, lists and so on.
In case of add-on update checker, JSON wil be employed to store add-on update information from a local NVDA installation, as well as to parse what the update server says about a possible update. A blueprint (pseudo-code) has been provided to spell out what the add-on update checker should do to accomplish its task.
…s will be fetched from info url value. re nvaccess#3208.

An add-on downloader, based on updateCheck's downloader has been implemented. The only differences are title and content of the progress dialogs, destination path name and extension, and what happens once download completes (installs add-ons).
In terms of update URL's, the same approach as in NVDA Core: url list will be stored in the update info dictionary.
The only showstopper is the problem where progress dialogs for add-on downloads won't go away if multiple add-ons are being downloaded and if at least one of them has an installation UI. Once this is fixed, the manual add-on update check is considered complete (the remaining item for the client is automatic update checks, and the rest will be server's job).
@josephsl josephsl requested a review from jcsteh March 5, 2017 01:19
@josephsl
Copy link
Collaborator Author

josephsl commented Mar 5, 2017

@derekriemer, may I request additional reviews such as catching unforseen errors, design versus implementation and so on? Thanks.

…oblem, update results diallg will now handle the case of no add-on updates. re nvaccess#3208.

Somehow, either because of lack of proficiency in GUI programming (from me) or what not, when add-on update results are displayed, add-ons manager is not focused once that window is closed. This is more so after multiple add-ons are updated at the same time.
Also, update results dialog will present a message (not a list of add-ons) when no add-on updates are available, as well as laying the foundation for automatic update checks for add-ons via 'auto' variable.
@josephsl
Copy link
Collaborator Author

josephsl commented Mar 5, 2017

Hi,

Somehow, when checking for updates, Add-ons Manager must be hidden, otherwise update status dialog will not be shown properly. Also, after downloading multiple updates, add-ons manager window will not receive focus afterwards (perhaps due to GUI programming), as well as the progress dialogs not closing if at least one add-on has an installation UI that users must deal with. Apart from that and the automatic check issue outlined above, the client side is done.

At the moment the highest priority is to fix add-on update experience when updating multiple add-ons, then deal with window focus problem.

Thanks.

@josephsl
Copy link
Collaborator Author

josephsl commented Mar 5, 2017

Hi,

In regards to serialization issue discussed in the original ticket, one possible workaround might be use of events (where the download initiator will wait until the previous add-on installs or fails to install). I spent all day today trying to come up with a suitable fix for that, but getting tired.

Thanks.

@josephsl
Copy link
Collaborator Author

josephsl commented Mar 8, 2017

I'll reopen this once GUI and synchronization issues are fixed. Thanks.

@josephsl josephsl closed this Mar 8, 2017
… shows a message. re nvaccess#3208.

First of a series of foundations for add-on update facility: add a 'Check for Add-on Update' (shortcut: U) that'll let users update installed add-ons. For now, the button is non-functional - shows a message box until the client is working.
…ns manager progress dialog changes. re nvaccess#3208.

Add-on handler: add a new function (checkForAddonUpdates) that'll allow NvDA to check for add-on updates. This is to be used from add-ons manager (manual) or when NVDA starts (automatic). The goal of this function is to gather add-on listings, connect online to retrieve add-on update information, and return this info in a format that a new add-on update result dialog will use to display and download add-on bundles. The add-on update check should not run if NVDA itself is being updated (NvDA Core takes precedence).
In add-ons manager, the progress bar dialog used for add-on installation is now an attribute of the dialog itself, as it is now used for update status message as well. Also, update check button briefly displays progress dialog and returns to add-ons manager. Eventually a new add-on update result dialog will be added that'll display add-on update info returned by addonHandler.checkForAddonUpdates and download new bundles.
…on check emulation in addonHandler.checkForAddonUpdates function. re nvaccess#3208.

A prototype of add-on update check results dialog has been implemented:
* When 'update add-ons' button is pressed, a background thread will call addonHandler.checkForAddonUpdates. This function will emulate a typical add-on update check routine by sleeping for three seconds, then populating the results dictionary with records of installed add-ons, with add-on names (identifiers) as keys and containing summary and version information.
* Once update info is retrieved, the update progress bar stops, and a results dialog will be shown. While checking for updates, Add-ons Manager window will be hidden until a suitable way of showing add-on update check progress and results with Add-ons Manager window disabled is found.
* The AddonUpdatesDialog (the results dialog class) borrows heavily from NVDA Core update results dialog, displaying a list of add-ons with updates (at least emulated) or a message if update info is filled in or not, respectively.
* For now, when 'update' button is pressed from results dialog, it'll do nothing. Eventually the update button will use a refactored file downloader to download and install updates, using the file path and hash that'll be supplied by the update info dictionary.
…. re nvaccess#3208.

Similar to add-on instalaltion routines but with a difference: no prompts will be shown when updating add-ons unless serious errors occur. The new updateAddons function takes a map of new add-ons to be installed, which is a map with add-on identifiers (names) as keys and info such as path as subkeys. This function will be called when user tells NVDA to update add-ons from update results dialog.
… involves sending and receiving data in JSON format. re nvaccess#3208.

Javascript Object Notation (JSON) is a handy way to serialize and deserialize data across various systems, including sending data over a network. It can be used to send an receive various data, such as nested dictionaries, lists and so on.
In case of add-on update checker, JSON wil be employed to store add-on update information from a local NVDA installation, as well as to parse what the update server says about a possible update. A blueprint (pseudo-code) has been provided to spell out what the add-on update checker should do to accomplish its task.
…s will be fetched from info url value. re nvaccess#3208.

An add-on downloader, based on updateCheck's downloader has been implemented. The only differences are title and content of the progress dialogs, destination path name and extension, and what happens once download completes (installs add-ons).
In terms of update URL's, the same approach as in NVDA Core: url list will be stored in the update info dictionary.
The only showstopper is the problem where progress dialogs for add-on downloads won't go away if multiple add-ons are being downloaded and if at least one of them has an installation UI. Once this is fixed, the manual add-on update check is considered complete (the remaining item for the client is automatic update checks, and the rest will be server's job).
…oblem, update results diallg will now handle the case of no add-on updates. re nvaccess#3208.

Somehow, either because of lack of proficiency in GUI programming (from me) or what not, when add-on update results are displayed, add-ons manager is not focused once that window is closed. This is more so after multiple add-ons are updated at the same time.
Also, update results dialog will present a message (not a list of add-ons) when no add-on updates are available, as well as laying the foundation for automatic update checks for add-ons via 'auto' variable.
…me. re nvaccess#3208.

Noted by Jamie Teh (NV Access) and Leonard from Netherlands: generators are great for doing one thing at a time. For this task, an updateAddonsGenerator will be used to update (download and install) add-ons one at a time. The generator will receive a list of add-ons to be updated, the add-on info for the to be updated add-on will be gather via list.pop, then add-on downloader and instlaler will be told to update add-ons via two kwargs: auto (automatic updates) and and list of add-ons to be updated.

This means:
* While add-ons are being updated, add-ons manager will be destroyed.
* Add-ons will be updated one at a time.
* Success/error methods in add-on downloader will call the generator defined above.
@josephsl
Copy link
Collaborator Author

Hi,

Major technical hurdle fixed: thanks to @jcsteh and @LeonarddeR for generator suggestion (it works). I'm reopening this PR to gather additional comments.

At this point, the issues to be resolved has to do with automatic add-on updates when NVDA starts, as well as making sure users don't update add-ons when NVDA is waiting for add-ons to be installed. Thanks.

@josephsl josephsl reopened this Mar 24, 2017
nvaccess#3208.

If for some reason an add-on fails to update, don't just quit right away - allow other add-ons to be updated. This is done by separating the generator caller to a separate function.
@AppVeyorBot
Copy link

PR introduces Flake8 errors 😲

See test results for Failed build of commit 9568e7e3c7

Based on flake8: lint errors such as unused urllib import, long lines, and bare exception were fixed.
Lint work include removing unused queue handler import, catching unused variable in update add-on screen, spacing, line length, and reworked logic for update add-ons button.
@AppVeyorBot
Copy link

PR introduces Flake8 errors 😲

See test results for Failed build of commit a2e9a3397d

Lint fixes include splitting function calls into multiple lines, spaces around operators, and splitting long lines."
For function complexity note in addonGui.installAddon, this is caused by installer UI. Thus find a way to separate the instlaler from the UI.
For E402 in update check, remant from nvaccess#6275, so no need to worry.
…t least one add-on is installed. Re nvaccess#3208.

Enable add-on update check settings if add-ons are installed (perviously it was showing all the time due to a logic error regarding add-on list refresh).
@feerrenrut
Copy link
Contributor

We will leave this PR open, hopefully it can be adapted to work with the Addon store proposal when that is implemented in the near future.

Before we review this, it would be good to know more (updated description) about the cases this PR specifically handles.

@josephsl
Copy link
Collaborator Author

josephsl commented Apr 1, 2020

Hi,

Ah yes...

The mechanism in this PR is similar to Add-on Updater add-on except that this PR does not specify where to obtain the add-on update metadata. This is intentional, as it stems from a long discussion in #3208 regarding server-side mechanics of this PR.

Specifically, this PR introduces a new button in Add-ons Manager to check for updates, along with a way to configure update channels and such (at least the foundation is there).

If the add-ons store proposal is adopted, I recommend not merging this PR directly at first - I think it would be best to integrate the work done on this PR into the larger Store proposal work, which by then can piece together the missing puzzle pieces in this PR.

Thanks.

@josephsl
Copy link
Collaborator Author

josephsl commented Apr 1, 2020

Hi,

As for the cases it covers:

  • Downloading an update from a URL specified.
  • Possibility to update add-ons automatically at startup after NVDA Core checks for its own updates.
  • Enabling/disabling add-on update checks globally or on per add-on basis (the latter done through a button in Add-ons Manager).

Let me know if you need more clarifications.

Add-on Updater repo:
https://github.com/josephsl/addonupdater

@feerrenrut feerrenrut added Addon/management In NVDA management of addons by users. feature labels Apr 8, 2020
@feerrenrut feerrenrut marked this pull request as draft June 10, 2020 16:48
@feerrenrut
Copy link
Contributor

Given this is blocked and really depends on the server-side before it can be finished, I've converted it to a draft.

@josephsl
Copy link
Collaborator Author

josephsl commented Jun 1, 2021

Hi,

As a gift to NV Access and NVDA community, I would like to donate this entire branch for further refinements in preparation for a future add-ons store implementation (and until add-ons store opens, I'll maintain Add-on Updater add-on, which provided basis for this branch).

Thanks.

@feerrenrut
Copy link
Contributor

If I'm interpreting you right @josephsl, you are saying that you won't continue to work on this? In that case I'm going to close it. Given we have our own plans at NV Access to fill this need it might be simpler if we take it on anyway.

@feerrenrut feerrenrut closed this Jun 1, 2021
@josephsl
Copy link
Collaborator Author

josephsl commented Jun 1, 2021 via email

@josephsl josephsl deleted the i3208-updateAddonsClient branch June 12, 2024 20:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Addon/management In NVDA management of addons by users. blocked feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants