rework configuration save, load configuration backups when available and config cannot be read

fix unnecessary config reload on save

MareSynchronos/MareConfiguration/ConfigurationSaveService.cs

@@ -0,0 +1,137 @@
+using MareSynchronos.MareConfiguration.Configurations;
+using Microsoft.Extensions.Hosting;
+using Microsoft.Extensions.Logging;
+using System.Reflection;
+using System.Text.Json;
+
+namespace MareSynchronos.MareConfiguration;
+
+public class ConfigurationSaveService : IHostedService
+{
+    private readonly HashSet<object> _configsToSave = [];
+    private readonly ILogger<ConfigurationSaveService> _logger;
+    private readonly SemaphoreSlim _configSaveSemaphore = new(1, 1);
+    private readonly CancellationTokenSource _configSaveCheckCts = new();
+    public const string BackupFolder = "config_backup";
+    private readonly MethodInfo _saveMethod;
+
+    public ConfigurationSaveService(ILogger<ConfigurationSaveService> logger, IEnumerable<IConfigService<IMareConfiguration>> configs)
+    {
+        foreach (var config in configs)
+        {
+            config.ConfigSave += OnConfigurationSave;
+        }
+        _logger = logger;
+#pragma warning disable S3011 // Reflection should not be used to increase accessibility of classes, methods, or fields
+        _saveMethod = GetType().GetMethod(nameof(SaveConfig), BindingFlags.Instance | BindingFlags.NonPublic)!;
+#pragma warning restore S3011 // Reflection should not be used to increase accessibility of classes, methods, or fields
+    }
+
+    private void OnConfigurationSave(object? sender, EventArgs e)
+    {
+        _configSaveSemaphore.Wait();
+        _configsToSave.Add(sender!);
+        _configSaveSemaphore.Release();
+    }
+
+    private async Task PeriodicSaveCheck(CancellationToken ct)
+    {
+        while (!ct.IsCancellationRequested)
+        {
+            try
+            {
+                await SaveConfigs().ConfigureAwait(false);
+            }
+            catch (Exception ex)
+            {
+                _logger.LogError(ex, "Error during SaveConfigs");
+            }
+
+            await Task.Delay(TimeSpan.FromSeconds(5), ct).ConfigureAwait(false);
+        }
+    }
+
+    private async Task SaveConfigs()
+    {
+        if (_configsToSave.Count == 0) return;
+
+        await _configSaveSemaphore.WaitAsync().ConfigureAwait(false);
+        var configList = _configsToSave.ToList();
+        _configsToSave.Clear();
+        _configSaveSemaphore.Release();
+
+        foreach (var config in configList)
+        {
+            var expectedType = config.GetType().BaseType!.GetGenericArguments()[0];
+            var save = _saveMethod.MakeGenericMethod(expectedType);
+            await ((Task)save.Invoke(this, [config])!).ConfigureAwait(false);
+        }
+    }
+
+    private async Task SaveConfig<T>(IConfigService<T> config) where T : IMareConfiguration
+    {
+        _logger.LogTrace("Saving {configName}", config.ConfigurationName);
+        var configDir = config.ConfigurationPath.Replace(config.ConfigurationName, string.Empty);
+
+        try
+        {
+            var configBackupFolder = Path.Join(configDir, BackupFolder);
+            if (!Directory.Exists(configBackupFolder))
+                Directory.CreateDirectory(configBackupFolder);
+
+            var configNameSplit = config.ConfigurationName.Split(".");
+            var existingConfigs = Directory.EnumerateFiles(
+                configBackupFolder,
+                configNameSplit[0] + "*")
+                .Select(c => new FileInfo(c))
+                .OrderByDescending(c => c.LastWriteTime).ToList();
+            if (existingConfigs.Skip(10).Any())
+            {
+                foreach (var oldBak in existingConfigs.Skip(10).ToList())
+                {
+                    oldBak.Delete();
+                }
+            }
+
+            string backupPath = Path.Combine(configBackupFolder, configNameSplit[0] + "." + DateTime.Now.ToString("yyyyMMddHHmmss") + "." + configNameSplit[1]);
+            _logger.LogTrace("Backing up current config to {backupPath}", backupPath);
+            File.Copy(config.ConfigurationPath, backupPath, overwrite: true);
+            FileInfo fi = new(backupPath);
+            fi.LastWriteTimeUtc = DateTime.UtcNow;
+        }
+        catch (Exception ex)
+        {
+            // ignore if file cannot be backupped
+            _logger.LogWarning(ex, "Could not create backup for {config}", config.ConfigurationPath);
+        }
+
+        var temp = config.ConfigurationPath + ".tmp";
+        try
+        {
+            await File.WriteAllTextAsync(temp, JsonSerializer.Serialize(config.Current, typeof(T), new JsonSerializerOptions()
+            {
+                WriteIndented = true
+            })).ConfigureAwait(false);
+            File.Move(temp, config.ConfigurationPath, true);
+            config.UpdateLastWriteTime();
+        }
+        catch (Exception ex)
+        {
+            _logger.LogWarning(ex, "Error during config save of {config}", config.ConfigurationName);
+        }
+    }
+
+    public Task StartAsync(CancellationToken cancellationToken)
+    {
+        _ = Task.Run(() => PeriodicSaveCheck(_configSaveCheckCts.Token));
+        return Task.CompletedTask;
+    }
+
+    public async Task StopAsync(CancellationToken cancellationToken)
+    {
+        await _configSaveCheckCts.CancelAsync().ConfigureAwait(false);
+        _configSaveCheckCts.Dispose();
+
+        await SaveConfigs().ConfigureAwait(false);
+    }
+}

MareSynchronos/MareConfiguration/ConfigurationServiceBase.cs

@@ -3,27 +3,28 @@ using System.Text.Json;
 
 namespace MareSynchronos.MareConfiguration;
 
-public abstract class ConfigurationServiceBase<T> : IDisposable where T : IMareConfiguration
+public abstract class ConfigurationServiceBase<T> : IConfigService<T> where T : IMareConfiguration
 {
     private readonly CancellationTokenSource _periodicCheckCts = new();
-    private bool _configIsDirty = false;
     private DateTime _configLastWriteTime;
     private Lazy<T> _currentConfigInternal;
+    private bool _disposed = false;
 
-    protected ConfigurationServiceBase(string configurationDirectory)
+    public event EventHandler? ConfigSave;
+
+    protected ConfigurationServiceBase(string configDirectory)
     {
-        ConfigurationDirectory = configurationDirectory;
+        ConfigurationDirectory = configDirectory;
 
         _ = Task.Run(CheckForConfigUpdatesInternal, _periodicCheckCts.Token);
-        _ = Task.Run(CheckForDirtyConfigInternal, _periodicCheckCts.Token);
 
         _currentConfigInternal = LazyConfig();
     }
 
     public string ConfigurationDirectory { get; init; }
     public T Current => _currentConfigInternal.Value;
-    protected abstract string ConfigurationName { get; }
+    public abstract string ConfigurationName { get; }
-    protected string ConfigurationPath => Path.Combine(ConfigurationDirectory, ConfigurationName);
+    public string ConfigurationPath => Path.Combine(ConfigurationDirectory, ConfigurationName);
 
     public void Dispose()
     {
@@ -33,14 +34,20 @@ public abstract class ConfigurationServiceBase<T> : IDisposable where T : IMareC
 
     public void Save()
     {
-        _configIsDirty = true;
+        ConfigSave?.Invoke(this, EventArgs.Empty);
+    }
+
+    public void UpdateLastWriteTime()
+    {
+        _configLastWriteTime = GetConfigLastWriteTime();
     }
 
     protected virtual void Dispose(bool disposing)
     {
+        if (!disposing || _disposed) return;
+        _disposed = true;
         _periodicCheckCts.Cancel();
         _periodicCheckCts.Dispose();
-        if (_configIsDirty) SaveDirtyConfig();
     }
 
     protected T LoadConfig()
@@ -48,8 +55,12 @@ public abstract class ConfigurationServiceBase<T> : IDisposable where T : IMareC
         T? config;
         if (!File.Exists(ConfigurationPath))
         {
-            config = (T)Activator.CreateInstance(typeof(T))!;
+            config = AttemptToLoadBackup();
-            Save();
+            if (Equals(config, default(T)))
+            {
+                config = (T)Activator.CreateInstance(typeof(T))!;
+                Save();
+            }
         }
         else
         {
@@ -60,9 +71,10 @@ public abstract class ConfigurationServiceBase<T> : IDisposable where T : IMareC
             catch
             {
                 // config failed to load for some reason
-                config = default;
+                config = AttemptToLoadBackup();
             }
-            if (config == null)
+
+            if (config == null || Equals(config, default(T)))
             {
                 config = (T)Activator.CreateInstance(typeof(T))!;
                 Save();
@@ -73,35 +85,36 @@ public abstract class ConfigurationServiceBase<T> : IDisposable where T : IMareC
         return config;
     }
 
-    protected void SaveDirtyConfig()
+    private T? AttemptToLoadBackup()
     {
-        _configIsDirty = false;
+        var configBackupFolder = Path.Join(ConfigurationDirectory, ConfigurationSaveService.BackupFolder);
-        var existingConfigs = Directory.EnumerateFiles(ConfigurationDirectory, ConfigurationName + ".bak.*").Select(c => new FileInfo(c))
+        var configNameSplit = ConfigurationName.Split(".");
-            .OrderByDescending(c => c.LastWriteTime).ToList();
+        if (!Directory.Exists(configBackupFolder))
-        if (existingConfigs.Skip(10).Any())
+            return default;
+
+        var existingBackups = Directory.EnumerateFiles(configBackupFolder, configNameSplit[0] + "*").OrderByDescending(f => new FileInfo(f).LastWriteTimeUtc);
+        foreach (var file in existingBackups)
         {
-            foreach (var config in existingConfigs.Skip(10).ToList())
+            try
             {
-                config.Delete();
+                var config = JsonSerializer.Deserialize<T>(File.ReadAllText(file));
+                if (Equals(config, default(T)))
+                {
+                    File.Delete(file);
+                }
+
+                File.Copy(file, ConfigurationPath, true);
+                return config;
             }
+            catch
+            {
+                // couldn't load backup, might as well delete it
+                File.Delete(file);
+            }
+
         }
 
-        try
+        return default;
-        {
-            File.Copy(ConfigurationPath, ConfigurationPath + ".bak." + DateTime.Now.ToString("yyyyMMddHHmmss"), overwrite: true);
-        }
-        catch
-        {
-            // ignore if file cannot be backupped once
-        }
-
-        var temp = ConfigurationPath + ".tmp";
-        File.WriteAllText(temp, JsonSerializer.Serialize(Current, new JsonSerializerOptions()
-        {
-            WriteIndented = true
-        }));
-        File.Move(temp, ConfigurationPath, true);
-        _configLastWriteTime = new FileInfo(ConfigurationPath).LastWriteTimeUtc;
     }
 
     private async Task CheckForConfigUpdatesInternal()
@@ -118,20 +131,12 @@ public abstract class ConfigurationServiceBase<T> : IDisposable where T : IMareC
         }
     }
 
-    private async Task CheckForDirtyConfigInternal()
+    private DateTime GetConfigLastWriteTime()
     {
-        while (!_periodicCheckCts.IsCancellationRequested)
+        try { return new FileInfo(ConfigurationPath).LastWriteTimeUtc; }
-        {
+        catch { return DateTime.MinValue; }
-            if (_configIsDirty)
-            {
-                SaveDirtyConfig();
-            }
-
-            await Task.Delay(TimeSpan.FromSeconds(1), _periodicCheckCts.Token).ConfigureAwait(false);
-        }
     }
 
-    private DateTime GetConfigLastWriteTime() => new FileInfo(ConfigurationPath).LastWriteTimeUtc;
 
     private Lazy<T> LazyConfig()
     {

MareSynchronos/MareConfiguration/IConfigService.cs

@@ -0,0 +1,12 @@
+using MareSynchronos.MareConfiguration.Configurations;
+
+namespace MareSynchronos.MareConfiguration;
+
+public interface IConfigService<out T> : IDisposable where T : IMareConfiguration
+{
+    T Current { get; }
+    string ConfigurationName { get; }
+    string ConfigurationPath { get; }
+    public event EventHandler? ConfigSave;
+    void UpdateLastWriteTime();
+}

MareSynchronos/MareConfiguration/MareConfigService.cs

@@ -10,5 +10,5 @@ public class MareConfigService : ConfigurationServiceBase<MareConfig>
     {
     }
 
-    protected override string ConfigurationName => ConfigName;
+    public override string ConfigurationName => ConfigName;
 }

MareSynchronos/MareConfiguration/NotesConfigService.cs

@@ -10,5 +10,5 @@ public class NotesConfigService : ConfigurationServiceBase<UidNotesConfig>
     {
     }
 
-    protected override string ConfigurationName => ConfigName;
+    public override string ConfigurationName => ConfigName;
 }

MareSynchronos/MareConfiguration/PlayerPerformanceConfigService.cs

@@ -7,5 +7,5 @@ public class PlayerPerformanceConfigService : ConfigurationServiceBase<PlayerPer
     public const string ConfigName = "playerperformance.json";
     public PlayerPerformanceConfigService(string configDir) : base(configDir) { }
 
-    protected override string ConfigurationName => ConfigName;
+    public override string ConfigurationName => ConfigName;
 }

MareSynchronos/MareConfiguration/ServerBlockConfigService.cs

@@ -10,5 +10,5 @@ public class ServerBlockConfigService : ConfigurationServiceBase<ServerBlockConf
     {
     }
 
-    protected override string ConfigurationName => ConfigName;
+    public override string ConfigurationName => ConfigName;
 }

MareSynchronos/MareConfiguration/ServerConfigService.cs

@@ -10,5 +10,5 @@ public class ServerConfigService : ConfigurationServiceBase<ServerConfig>
     {
     }
 
-    protected override string ConfigurationName => ConfigName;
+    public override string ConfigurationName => ConfigName;
 }

MareSynchronos/MareConfiguration/ServerTagConfigService.cs

@@ -10,5 +10,5 @@ public class ServerTagConfigService : ConfigurationServiceBase<ServerTagConfig>
     {
     }
 
-    protected override string ConfigurationName => ConfigName;
+    public override string ConfigurationName => ConfigName;
 }

MareSynchronos/MareConfiguration/SyncshellConfigService.cs

@@ -10,5 +10,5 @@ public class SyncshellConfigService : ConfigurationServiceBase<SyncshellConfig>
     {
     }
 
-    protected override string ConfigurationName => ConfigName;
+    public override string ConfigurationName => ConfigName;
 }

MareSynchronos/MareConfiguration/TransientConfigService.cs

@@ -10,5 +10,5 @@ public class TransientConfigService : ConfigurationServiceBase<TransientConfig>
     {
     }
 
-    protected override string ConfigurationName => ConfigName;
+    public override string ConfigurationName => ConfigName;
 }

MareSynchronos/MareConfiguration/XivDataStorageService.cs

@@ -8,5 +8,5 @@ public class XivDataStorageService : ConfigurationServiceBase<XivDataStorageConf
 
     public XivDataStorageService(string configDir) : base(configDir) { }
 
-    protected override string ConfigurationName => ConfigName;
+    public override string ConfigurationName => ConfigName;
 }

MareSynchronos/Plugin.cs

@@ -7,6 +7,7 @@ using MareSynchronos.FileCache;
 using MareSynchronos.Interop;
 using MareSynchronos.Interop.Ipc;
 using MareSynchronos.MareConfiguration;
+using MareSynchronos.MareConfiguration.Configurations;
 using MareSynchronos.PlayerData.Export;
 using MareSynchronos.PlayerData.Factories;
 using MareSynchronos.PlayerData.Pairs;
@@ -140,7 +141,18 @@ public sealed class Plugin : IDalamudPlugin
             collection.AddSingleton((s) => new XivDataStorageService(pluginInterface.ConfigDirectory.FullName));
             collection.AddSingleton((s) => new PlayerPerformanceConfigService(pluginInterface.ConfigDirectory.FullName));
             collection.AddSingleton((s) => new ServerBlockConfigService(pluginInterface.ConfigDirectory.FullName));
+            collection.AddSingleton<IConfigService<IMareConfiguration>>(s => s.GetRequiredService<MareConfigService>());
+            collection.AddSingleton<IConfigService<IMareConfiguration>>(s => s.GetRequiredService<ServerConfigService>());
+            collection.AddSingleton<IConfigService<IMareConfiguration>>(s => s.GetRequiredService<NotesConfigService>());
+            collection.AddSingleton<IConfigService<IMareConfiguration>>(s => s.GetRequiredService<ServerTagConfigService>());
+            collection.AddSingleton<IConfigService<IMareConfiguration>>(s => s.GetRequiredService<SyncshellConfigService>());
+            collection.AddSingleton<IConfigService<IMareConfiguration>>(s => s.GetRequiredService<TransientConfigService>());
+            collection.AddSingleton<IConfigService<IMareConfiguration>>(s => s.GetRequiredService<XivDataStorageService>());
+            collection.AddSingleton<IConfigService<IMareConfiguration>>(s => s.GetRequiredService<PlayerPerformanceConfigService>());
+            collection.AddSingleton<IConfigService<IMareConfiguration>>(s => s.GetRequiredService<ServerBlockConfigService>());
             collection.AddSingleton<ConfigurationMigrator>();
+            collection.AddSingleton<ConfigurationSaveService>();
+
             collection.AddSingleton<HubFactory>();
 
             // add scoped services
@@ -168,6 +180,7 @@ public sealed class Plugin : IDalamudPlugin
             collection.AddScoped<ChatService>();
             collection.AddScoped<GuiHookService>();
 
+            collection.AddHostedService(p => p.GetRequiredService<ConfigurationSaveService>());
             collection.AddHostedService(p => p.GetRequiredService<MareMediator>());
             collection.AddHostedService(p => p.GetRequiredService<NotificationService>());
             collection.AddHostedService(p => p.GetRequiredService<FileCacheManager>());

MareSynchronos/UI/SyncshellAdminUI.cs

@@ -351,7 +351,7 @@ public class SyncshellAdminUI : WindowMediatorSubscriberBase
                             using var pushId = ImRaii.PushId(bannedUser.UID);
                             if (_uiSharedService.IconTextButton(FontAwesomeIcon.Check, "Unban"))
                             {
-                                _ = _apiController.GroupUnbanUser(bannedUser);
+                                _apiController.GroupUnbanUser(bannedUser);
                                 _bannedUsers.RemoveAll(b => string.Equals(b.UID, bannedUser.UID, StringComparison.Ordinal));
                             }
                         }