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

(#494) Refresh sources from file #955

Draft
wants to merge 5 commits into
base: develop
Choose a base branch
from

Conversation

corbob
Copy link
Member

@corbob corbob commented Aug 30, 2022

Description Of Changes

Update to get sources from the configuration file every time they're requested.

Motivation and Context

When the sources are updated outside of Chocolatey GUI (or within Chocolatey GUI when Chocolatey Licensed Extension is installed), only the sources that were present when Chocolatey GUI started are loaded, no new sources are found.

Testing

Note: Because Chocolatey GUI interacts with Chocolatey through the Chocolatey library, and because there are extensions that can change behaviour, all testing should be done with Chocolatey CLI, then with Chocolatey Licensed Extension installed, and finally with Chocolatey GUI Licensed Extension installed. The below steps were taken with each of those configurations.

Chocolatey Licensed Extension Note: I have noted in my testing that if you install Licensed Extension, then immediately open Chocolatey GUI without running choco beforehand, you end up with a duplicated list of remote sources on the left. Any action that makes changes to the Chocolatey config file will correct this duplication. I believe this is due to the list being updated on application load, and since that (effectively) runs choco, the config is updated with the Licensed Extension configurations which fires the FileSystemWatcher and results in a second refresh occurring in rapid succession. This is the only time I have been able to get this behaviour to occur since correcting it in the FileSystemWatcher.

  1. Open a PowerShell window as administrator and Chocolatey GUI
  2. Position them such that you can see Chocolatey GUI and the PowerShell window with minimal overlap
  3. Run choco sources list
  4. Note that the list of enabled sources matches the left panel of Chocolatey GUI
  5. Run choco source add -n test -s c:\
  6. Note that Chocolatey GUI left panel updates with new test source
  7. Run choco source disable -n test
  8. Note that Chocolatey GUI left panel updates without the test source
  9. Navigate to Settings > Sources in Chocolatey GUI
  10. Note that test source is there, but disabled.
  11. Enable test source and navigate back to Chocolatey GUI
  12. Note that test is back in the list
  13. Run choco source remove -n test
  14. Note that test is removed from the list on the left panel of Chocolatey GUI
  15. Navigate to Settings > Sources in Chocolatey GUI
  16. Note that test source is no longer there
  17. Close Chocolatey GUI

Chocolatey Licensed Extension specific tests

Due to some of the restrictions that the Chocolatey Licensed Extension provides, we need to test the following with the Licensed Extension, and with the Chocolatey GUI Licensed Extension. Use the following code to setup a few settings:

choco install chocolatey-agent
choco feature disable -n showNonElevatedWarnings
choco feature enable -n useBackgroundService
choco feature enable -n useBackgroundServiceWithNonAdministratorsOnly
choco source add -n admin-only -s c:\temp --adminonly
choco source add -n self-serve -s c:\temp --allowselfservice

With Chocolatey configured, perform the following tests:

  1. Launch Chocolatey GUI as a no-administrative user
  2. Verify that only self-serve source is the only source listed on the left
  3. In administrative PowerShell run: choco source disable -n self-serve
  4. Note that the source disappears on the left
  5. In administrative PowerShell run: choco source enable -n self-serve
  6. Note that the source re-appears.
  7. In administrative PowerShell run: choco source add -n new-self-serve -s c\temp --allowselfservice
  8. Note that the new source should appear on the left
  9. In administrative PowerShell run: choco source add -n admin-only -s c:\temp
  10. Note that the admin-only source should appear in the left of Chocolatey GUI

Change Types Made

  • Bug fix (non-breaking change)
  • Feature / Enhancement (non-breaking change)
  • Breaking change (fix or feature that could cause existing functionality to change)
  • PowerShell code changes.

Related Issue

Fixes #494

Change Checklist

  • Requires a change to the documentation
  • Documentation has been updated
  • Tests to cover my changes, have been added
  • All new and existing tests passed.
  • PowerShell v2 compatibility checked.

@corbob corbob force-pushed the refresh-sources-from-file branch 3 times, most recently from 7ca4fe9 to ca20e8d Compare September 2, 2022 03:33
@corbob corbob changed the title Refresh sources from file (#494) Refresh sources from file Sep 2, 2022
@corbob corbob requested a review from gep13 September 2, 2022 17:28
Sometimes (it looks like maybe when the Chocolatey Licensed Extension is
in play) we don't get the sources currently in the configuration file.
If instead of reusing the configuration settings service we instantiate
a new one each time, then it will ensure the configuration file is read
again. This resolves the issue of settings not applying in GUI if
they're made while GUI is running.
When Chocolatey CLI makes changes to the configuration file while
Chocolatey GUI is running, Chocolatey GUI should refresh the sources so
that they're accurate. This adds a FileSystemWatcher to monitor the
configuration file and update the GUI if there's updates to the file.
With the FileSystemWatcher, we don't need to manually tell Chocolatey
GUI to update the sources, as they'll be updated by the
FileSystemWatcher. Leaving them in results in duplicated entries that we
don't want.
@corbob corbob force-pushed the refresh-sources-from-file branch from ca20e8d to dcb3405 Compare September 2, 2022 18:48
@corbob corbob marked this pull request as ready for review September 2, 2022 20:21
Renamed configuration file variable to indicate it's the path to the
configuration file. Made ConfigFileChanged public and added it to the
interface. Also renamed it to OnConfigFileChanged as that seems to be a
better name to indicate it's the action for when the file changes.
@corbob corbob requested a review from gep13 September 6, 2022 14:40
@corbob
Copy link
Member Author

corbob commented Sep 6, 2022

After reverting commit 583b1b9 I performed the tests in the PR. My observations are:

  1. With Just Chocolatey CLI installed, all existing sources in the file when Chocolatey GUI is launched successfully maintain state. If you enable or disable them outside of Chocolatey GUI, they are updated inside of Chocolatey GUI. However, if you add a new source, it fails to be detected.
  2. With Chocolatey Licensed Extension installed, the tests behaved the same as Chocolatey CLI. The Licensed specific tests however did not work as desired. The only change that was correctly picked up is enable, disable, or removal of an existing source. Changes to the Admin Only or Self Serve status does not get reflected.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Source credential changes outside of Chocolatey GUI cause issues
2 participants