From 79d83afbf4cb5d8a7004c393ff60f7758741e5dc Mon Sep 17 00:00:00 2001 From: tom-englert Date: Sat, 26 Oct 2024 16:14:37 +0200 Subject: [PATCH] Fix update settings to finally remove the need for settings service singleton --- ILSpy/AboutPage.cs | 23 ++++--- ILSpy/AssemblyTree/AssemblyTreeModel.cs | 15 +++-- ILSpy/Commands/CheckForUpdatesCommand.cs | 3 +- ILSpy/MainWindow.xaml.cs | 8 +-- ILSpy/Themes/WindowStyleManagerBehavior.cs | 4 +- ILSpy/Updates/NotifyOfUpdatesStrategy.cs | 43 +++++-------- ILSpy/Updates/UpdateSettings.cs | 75 +++++++++------------- ILSpy/Util/SettingsService.cs | 27 ++------ 8 files changed, 79 insertions(+), 119 deletions(-) diff --git a/ILSpy/AboutPage.cs b/ILSpy/AboutPage.cs index 525d9b892..94ebd59c3 100644 --- a/ILSpy/AboutPage.cs +++ b/ILSpy/AboutPage.cs @@ -23,6 +23,7 @@ using System.IO; using System.Text.RegularExpressions; using System.Windows; using System.Windows.Controls; +using System.Windows.Controls.Primitives; using System.Windows.Data; using System.Windows.Input; using System.Windows.Navigation; @@ -34,7 +35,6 @@ using ICSharpCode.ILSpy.Properties; using ICSharpCode.ILSpy.TextView; using ICSharpCode.ILSpy.Themes; using ICSharpCode.ILSpy.Updates; -using ICSharpCode.ILSpyX.Settings; namespace ICSharpCode.ILSpy { @@ -53,7 +53,7 @@ namespace ICSharpCode.ILSpy [Export] [Shared] - public sealed class AboutPage(IEnumerable aboutPageAdditions) + public sealed class AboutPage(IEnumerable aboutPageAdditions, SettingsService settingsService) { public void Display(DecompilerTextView textView) { @@ -68,9 +68,10 @@ namespace ICSharpCode.ILSpy output.AddUIElement( delegate { - StackPanel stackPanel = new StackPanel(); - stackPanel.HorizontalAlignment = HorizontalAlignment.Center; - stackPanel.Orientation = Orientation.Horizontal; + StackPanel stackPanel = new() { + HorizontalAlignment = HorizontalAlignment.Center, + Orientation = Orientation.Horizontal + }; if (NotifyOfUpdatesStrategy.LatestAvailableVersion == null) { AddUpdateCheckButton(stackPanel, textView); @@ -80,11 +81,13 @@ namespace ICSharpCode.ILSpy // we already retrieved the latest version sometime earlier ShowAvailableVersion(NotifyOfUpdatesStrategy.LatestAvailableVersion, stackPanel); } - CheckBox checkBox = new CheckBox(); - checkBox.Margin = new Thickness(4); - checkBox.Content = Resources.AutomaticallyCheckUpdatesEveryWeek; - UpdateSettings settings = new UpdateSettings(SettingsService.Instance.SpySettings); - checkBox.SetBinding(CheckBox.IsCheckedProperty, new Binding("AutomaticUpdateCheckEnabled") { Source = settings }); + CheckBox checkBox = new() { + Margin = new Thickness(4), + Content = Resources.AutomaticallyCheckUpdatesEveryWeek + }; + + var settings = settingsService.GetSettings(); + checkBox.SetBinding(ToggleButton.IsCheckedProperty, new Binding("AutomaticUpdateCheckEnabled") { Source = settings }); return new StackPanel { Margin = new Thickness(0, 4, 0, 0), Cursor = Cursors.Arrow, diff --git a/ILSpy/AssemblyTree/AssemblyTreeModel.cs b/ILSpy/AssemblyTree/AssemblyTreeModel.cs index bf46861d9..8665f38e4 100644 --- a/ILSpy/AssemblyTree/AssemblyTreeModel.cs +++ b/ILSpy/AssemblyTree/AssemblyTreeModel.cs @@ -42,6 +42,7 @@ using ICSharpCode.ILSpy.Properties; using ICSharpCode.ILSpy.Search; using ICSharpCode.ILSpy.TextView; using ICSharpCode.ILSpy.TreeNodes; +using ICSharpCode.ILSpy.Updates; using ICSharpCode.ILSpy.ViewModels; using ICSharpCode.ILSpyX; using ICSharpCode.ILSpyX.Settings; @@ -169,16 +170,16 @@ namespace ICSharpCode.ILSpy.AssemblyTree /// /// Called on startup or when passed arguments via WndProc from a second instance. - /// In the format case, spySettings is non-null; in the latter it is null. + /// In the format case, updateSettings is non-null; in the latter it is null. /// - private async Task HandleCommandLineArgumentsAfterShowList(CommandLineArguments args, ISettingsProvider? spySettings = null) + private async Task HandleCommandLineArgumentsAfterShowList(CommandLineArguments args, UpdateSettings? updateSettings = null) { var sessionSettings = settingsService.SessionSettings; var relevantAssemblies = commandLineLoadedAssemblies.ToList(); commandLineLoadedAssemblies.Clear(); // clear references once we don't need them anymore - await NavigateOnLaunch(args.NavigateTo, sessionSettings.ActiveTreeViewPath, spySettings, relevantAssemblies); + await NavigateOnLaunch(args.NavigateTo, sessionSettings.ActiveTreeViewPath, updateSettings, relevantAssemblies); if (args.Search != null) { @@ -207,7 +208,7 @@ namespace ICSharpCode.ILSpy.AssemblyTree }); } - private async Task NavigateOnLaunch(string? navigateTo, string[]? activeTreeViewPath, ISettingsProvider? spySettings, List relevantAssemblies) + private async Task NavigateOnLaunch(string? navigateTo, string[]? activeTreeViewPath, UpdateSettings? updateSettings, List relevantAssemblies) { var initialSelection = SelectedItem; if (navigateTo != null) @@ -277,7 +278,7 @@ namespace ICSharpCode.ILSpy.AssemblyTree SelectNode(asmNode); } } - else if (spySettings != null) + else if (updateSettings != null) { SharpTreeNode? node = null; if (activeTreeViewPath?.Length > 0) @@ -300,7 +301,7 @@ namespace ICSharpCode.ILSpy.AssemblyTree SelectNode(node); // only if not showing the about page, perform the update check: - await App.Current.MainWindow.ShowMessageIfUpdatesAvailableAsync(spySettings); + await App.Current.MainWindow.ShowMessageIfUpdatesAvailableAsync(updateSettings); } else { @@ -399,7 +400,7 @@ namespace ICSharpCode.ILSpy.AssemblyTree private async Task OpenAssemblies() { - await HandleCommandLineArgumentsAfterShowList(App.CommandLineArguments, settingsService.SpySettings); + await HandleCommandLineArgumentsAfterShowList(App.CommandLineArguments, settingsService.GetSettings()); if (FormatExceptions(App.StartupExceptions.ToArray(), out var output)) { diff --git a/ILSpy/Commands/CheckForUpdatesCommand.cs b/ILSpy/Commands/CheckForUpdatesCommand.cs index 5ac9b1fc6..54fb1ac8a 100644 --- a/ILSpy/Commands/CheckForUpdatesCommand.cs +++ b/ILSpy/Commands/CheckForUpdatesCommand.cs @@ -20,6 +20,7 @@ using System.Composition; using ICSharpCode.ILSpy.Properties; +using ICSharpCode.ILSpy.Updates; namespace ICSharpCode.ILSpy { @@ -29,7 +30,7 @@ namespace ICSharpCode.ILSpy { public override async void Execute(object parameter) { - await App.Current.MainWindow.ShowMessageIfUpdatesAvailableAsync(settingsService.SpySettings, forceCheck: true); + await App.Current.MainWindow.ShowMessageIfUpdatesAvailableAsync(settingsService.GetSettings(), forceCheck: true); } } } diff --git a/ILSpy/MainWindow.xaml.cs b/ILSpy/MainWindow.xaml.cs index e9d392717..a9b5b67ca 100644 --- a/ILSpy/MainWindow.xaml.cs +++ b/ILSpy/MainWindow.xaml.cs @@ -152,16 +152,16 @@ namespace ICSharpCode.ILSpy string updateAvailableDownloadUrl; - public async Task ShowMessageIfUpdatesAvailableAsync(ISettingsProvider spySettings, bool forceCheck = false) + public async Task ShowMessageIfUpdatesAvailableAsync(UpdateSettings settings, bool forceCheck = false) { string downloadUrl; if (forceCheck) { - downloadUrl = await NotifyOfUpdatesStrategy.CheckForUpdatesAsync(spySettings); + downloadUrl = await NotifyOfUpdatesStrategy.CheckForUpdatesAsync(settings); } else { - downloadUrl = await NotifyOfUpdatesStrategy.CheckForUpdatesIfEnabledAsync(spySettings); + downloadUrl = await NotifyOfUpdatesStrategy.CheckForUpdatesIfEnabledAsync(settings); } // The Update Panel is only available for NotifyOfUpdatesStrategy, AutoUpdate will have differing UI requirements @@ -182,7 +182,7 @@ namespace ICSharpCode.ILSpy else { updatePanel.Visibility = Visibility.Collapsed; - string downloadUrl = await NotifyOfUpdatesStrategy.CheckForUpdatesAsync(settingsService.SpySettings); + string downloadUrl = await NotifyOfUpdatesStrategy.CheckForUpdatesAsync(settingsService.GetSettings()); AdjustUpdateUIAfterCheck(downloadUrl, true); } } diff --git a/ILSpy/Themes/WindowStyleManagerBehavior.cs b/ILSpy/Themes/WindowStyleManagerBehavior.cs index 1db815ffe..0453c0807 100644 --- a/ILSpy/Themes/WindowStyleManagerBehavior.cs +++ b/ILSpy/Themes/WindowStyleManagerBehavior.cs @@ -43,7 +43,7 @@ namespace ICSharpCode.ILSpy.Themes { base.OnAttached(); - MessageBus.Subscribers += (sender, e) => DisplaySettings_PropertyChanged(sender, e); + MessageBus.Subscribers += (sender, e) => Settings_PropertyChanged(sender, e); _foreground = AssociatedObject.Track(Control.ForegroundProperty); _background = AssociatedObject.Track(Control.BackgroundProperty); @@ -83,7 +83,7 @@ namespace ICSharpCode.ILSpy.Themes MessageBox.Show(Properties.Resources.SettingsChangeRestartRequired); } - private void DisplaySettings_PropertyChanged(object sender, PropertyChangedEventArgs e) + private void Settings_PropertyChanged(object sender, PropertyChangedEventArgs e) { if (sender is not DisplaySettings displaySettings) return; diff --git a/ILSpy/Updates/NotifyOfUpdatesStrategy.cs b/ILSpy/Updates/NotifyOfUpdatesStrategy.cs index d2ab85fc9..3aea16f2b 100644 --- a/ILSpy/Updates/NotifyOfUpdatesStrategy.cs +++ b/ILSpy/Updates/NotifyOfUpdatesStrategy.cs @@ -54,51 +54,40 @@ namespace ICSharpCode.ILSpy.Updates return LatestAvailableVersion; } - - /// /// If automatic update checking is enabled, checks if there are any updates available. /// Returns the download URL if an update is available. /// Returns null if no update is available, or if no check was performed. /// - public static async Task CheckForUpdatesIfEnabledAsync(ISettingsProvider spySettings) + public static async Task CheckForUpdatesIfEnabledAsync(UpdateSettings settings) { - UpdateSettings s = new UpdateSettings(spySettings); - // If we're in an MSIX package, updates work differently - if (s.AutomaticUpdateCheckEnabled) - { - // perform update check if we never did one before; - // or if the last check wasn't in the past 7 days - if (s.LastSuccessfulUpdateCheck == null - || s.LastSuccessfulUpdateCheck < DateTime.UtcNow.AddDays(-7) - || s.LastSuccessfulUpdateCheck > DateTime.UtcNow) - { - return await CheckForUpdateInternal(s).ConfigureAwait(false); - } - else - { - return null; - } - } - else - { + if (!settings.AutomaticUpdateCheckEnabled) return null; + + // perform update check if we never did one before; + // or if the last check wasn't in the past 7 days + if (settings.LastSuccessfulUpdateCheck == null + || settings.LastSuccessfulUpdateCheck < DateTime.UtcNow.AddDays(-7) + || settings.LastSuccessfulUpdateCheck > DateTime.UtcNow) + { + return await CheckForUpdateInternal(settings).ConfigureAwait(false); } + + return null; } - public static Task CheckForUpdatesAsync(ISettingsProvider spySettings) + public static Task CheckForUpdatesAsync(UpdateSettings settings) { - UpdateSettings s = new UpdateSettings(spySettings); - return CheckForUpdateInternal(s); + return CheckForUpdateInternal(settings); } - static async Task CheckForUpdateInternal(UpdateSettings s) + static async Task CheckForUpdateInternal(UpdateSettings settings) { try { var v = await GetLatestVersionAsync().ConfigureAwait(false); - s.LastSuccessfulUpdateCheck = DateTime.UtcNow; + settings.LastSuccessfulUpdateCheck = DateTime.UtcNow; if (v.Version > AppUpdateService.CurrentVersion) return v.DownloadUrl; else diff --git a/ILSpy/Updates/UpdateSettings.cs b/ILSpy/Updates/UpdateSettings.cs index 23b4b0663..4fd3a883a 100644 --- a/ILSpy/Updates/UpdateSettings.cs +++ b/ILSpy/Updates/UpdateSettings.cs @@ -17,73 +17,56 @@ // DEALINGS IN THE SOFTWARE. using System; -using System.ComponentModel; using System.Xml.Linq; -using ICSharpCode.ILSpyX.Settings; +using TomsToolbox.Wpf; namespace ICSharpCode.ILSpy.Updates { - sealed class UpdateSettings : INotifyPropertyChanged + public sealed class UpdateSettings : ObservableObjectBase, ISettingsSection { - public UpdateSettings(ISettingsProvider spySettings) - { - XElement s = spySettings["UpdateSettings"]; - this.automaticUpdateCheckEnabled = (bool?)s.Element("AutomaticUpdateCheckEnabled") ?? true; - try - { - this.lastSuccessfulUpdateCheck = (DateTime?)s.Element("LastSuccessfulUpdateCheck"); - } - catch (FormatException) - { - // avoid crashing on settings files invalid due to - // https://github.com/icsharpcode/ILSpy/issues/closed/#issue/2 - } - } + public XName SectionName => "UpdateSettings"; bool automaticUpdateCheckEnabled; public bool AutomaticUpdateCheckEnabled { - get { return automaticUpdateCheckEnabled; } - set { - if (automaticUpdateCheckEnabled != value) - { - automaticUpdateCheckEnabled = value; - Save(); - OnPropertyChanged(nameof(AutomaticUpdateCheckEnabled)); - } - } + get => automaticUpdateCheckEnabled; + set => SetProperty(ref automaticUpdateCheckEnabled, value); } DateTime? lastSuccessfulUpdateCheck; public DateTime? LastSuccessfulUpdateCheck { - get { return lastSuccessfulUpdateCheck; } - set { - if (lastSuccessfulUpdateCheck != value) - { - lastSuccessfulUpdateCheck = value; - Save(); - OnPropertyChanged(nameof(LastSuccessfulUpdateCheck)); - } - } + get => lastSuccessfulUpdateCheck; + set => SetProperty(ref lastSuccessfulUpdateCheck, value); } - public void Save() + public void LoadFromXml(XElement section) { - XElement updateSettings = new XElement("UpdateSettings"); - updateSettings.Add(new XElement("AutomaticUpdateCheckEnabled", automaticUpdateCheckEnabled)); - if (lastSuccessfulUpdateCheck != null) - updateSettings.Add(new XElement("LastSuccessfulUpdateCheck", lastSuccessfulUpdateCheck)); - SettingsService.Instance.SpySettings.SaveSettings(updateSettings); + AutomaticUpdateCheckEnabled = (bool?)section.Element("AutomaticUpdateCheckEnabled") ?? true; + try + { + LastSuccessfulUpdateCheck = (DateTime?)section.Element("LastSuccessfulUpdateCheck"); + } + catch (FormatException) + { + // avoid crashing on settings files invalid due to + // https://github.com/icsharpcode/ILSpy/issues/closed/#issue/2 + } } - public event PropertyChangedEventHandler PropertyChanged; - - void OnPropertyChanged(string propertyName) + public XElement SaveToXml() { - PropertyChanged?.Invoke(this, new PropertyChangedEventArgs(propertyName)); + var section = new XElement(SectionName); + + section.Add(new XElement("AutomaticUpdateCheckEnabled", AutomaticUpdateCheckEnabled)); + + if (LastSuccessfulUpdateCheck != null) + { + section.Add(new XElement("LastSuccessfulUpdateCheck", LastSuccessfulUpdateCheck)); + } + + return section; } } - } diff --git a/ILSpy/Util/SettingsService.cs b/ILSpy/Util/SettingsService.cs index 6ce11ee33..80b7dc6e0 100644 --- a/ILSpy/Util/SettingsService.cs +++ b/ILSpy/Util/SettingsService.cs @@ -19,12 +19,9 @@ using System; using System.Collections.Concurrent; using System.ComponentModel; -using System.Composition; using System.Xml.Linq; -using ICSharpCode.Decompiler; using ICSharpCode.ILSpy.Options; -using ICSharpCode.ILSpy.ViewModels; using ICSharpCode.ILSpyX; using ICSharpCode.ILSpyX.Settings; @@ -48,16 +45,11 @@ namespace ICSharpCode.ILSpy.Util XElement SaveToXml(); } - public abstract class SettingsServiceBase + public abstract class SettingsServiceBase(ISettingsProvider spySettings) { protected readonly ConcurrentDictionary sections = new(); - public ISettingsProvider SpySettings; - - protected SettingsServiceBase(ISettingsProvider spySettings) - { - SpySettings = spySettings; - } + protected ISettingsProvider SpySettings { get; set; } = spySettings; public T GetSettings() where T : ISettingsSection, new() { @@ -73,7 +65,7 @@ namespace ICSharpCode.ILSpy.Util }); } - protected void SaveSection(ISettingsSection section, XElement root) + protected static void SaveSection(ISettingsSection section, XElement root) { var element = section.SaveToXml(); @@ -89,15 +81,8 @@ namespace ICSharpCode.ILSpy.Util } } - public class SettingsSnapshot : SettingsServiceBase + public class SettingsSnapshot(SettingsService parent, ISettingsProvider spySettings) : SettingsServiceBase(spySettings) { - private readonly SettingsService parent; - - public SettingsSnapshot(SettingsService parent) : base(parent.SpySettings) - { - this.parent = parent; - } - public void Save() { SpySettings.Update(root => { @@ -113,8 +98,6 @@ namespace ICSharpCode.ILSpy.Util public class SettingsService() : SettingsServiceBase(LoadSettings()) { - public static readonly SettingsService Instance = App.ExportProvider.GetExportedValue(); - public SessionSettings SessionSettings => GetSettings(); public DecompilerSettings DecompilerSettings => GetSettings(); @@ -174,7 +157,7 @@ namespace ICSharpCode.ILSpy.Util public SettingsSnapshot CreateSnapshot() { - return new(this); + return new(this, SpySettings); } protected override void Section_PropertyChanged(object? sender, PropertyChangedEventArgs e)