diff --git a/CHANGELOG.md b/CHANGELOG.md index f2a764b..741623a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,23 @@ # Changelog +## 0.15.3 + +ARCH-1 step 3. Third collaborator extracted from `TrayContext`. No user-visible changes. + +Changed: + +- **New `DependencyProbe` class** (`src/DependencyProbe.cs`) owns the cached `RcloneAvailable` / `WinFspInstalled` booleans plus their TTL stamp and the synchronous probes themselves (the slow `File.Exists` + `spawn rclone version` + registry lookups). Constructor takes a `Func` rclone path provider and a log callback. +- **`TrayContext.Setup.cs` delegates** the cached-state and probe methods. The async refresh worker (`RefreshDependencyStatusAsync`) stays in `TrayContext` because it composes results into the setup-status line and marshals back via `BeginUi`; it just hands the actual probing off to `DependencyProbe.PublishProbeResults`. + +## 0.15.2 + +ARCH-1 step 2. Second collaborator extracted from `TrayContext`. No user-visible changes. + +Changed: + +- **New `SettingsStore` class** (`src/SettingsStore.cs`) owns the `settings.json` read / write / in-memory cache / atomic-write / timestamped backups / prune. Profile-specific shape logic (which keys land in each profile dict) stays in `TrayContext.Settings.cs`; `SettingsStore` is the layer below — give it the root dict, it handles disk. +- **`TrayContext.ReadSettingsRoot` / `WriteSettingsRoot` / `BackupSettingsFile` delegate** to a lazily-initialised `SettingsStore` field. Lock-protected cache (PERF-3 behavior) preserved. + ## 0.15.1 ARCH-1 step 1 from the audit — first collaborator extracted from the `TrayContext` partial. No user-visible changes; internal refactor only. diff --git a/src/DependencyProbe.cs b/src/DependencyProbe.cs new file mode 100644 index 0000000..e809df6 --- /dev/null +++ b/src/DependencyProbe.cs @@ -0,0 +1,79 @@ +using System; +using System.IO; +using Microsoft.Win32; + +namespace Pixelpipe +{ + // ARCH-1 step 3 (v0.15.3, audit): third collaborator extracted from + // the TrayContext partial. Owns the cached rclone / WinFsp availability + // state plus the synchronous probes themselves. + // + // The TTL + UI-thread-friendly cached accessors stay where they were + // semantically — UI callers ask "is rclone available?" and get back + // whatever the last refresh wrote, never touching disk on the UI thread. + // The async refresh worker (RefreshDependencyStatusAsync) stays in + // TrayContext for now because it composes results into the setup-status + // line; it just hands off the actual probe calls to this class. + internal sealed class DependencyProbe + { + public const int CacheTtlSeconds = 30; + + private readonly Func _rclonePathProvider; + private readonly Action _logIssue; + private volatile bool _cachedRcloneAvailable; + private volatile bool _cachedWinfspInstalled; + private DateTime _cachedStampUtc = DateTime.MinValue; + + public DependencyProbe(Func rclonePathProvider, Action logIssue) + { + _rclonePathProvider = rclonePathProvider; + _logIssue = logIssue; + } + + public bool RcloneAvailable { get { return _cachedRcloneAvailable; } } + public bool WinFspInstalled { get { return _cachedWinfspInstalled; } } + public DateTime LastProbeUtc { get { return _cachedStampUtc; } } + public bool IsStale { get { return (DateTime.UtcNow - _cachedStampUtc).TotalSeconds >= CacheTtlSeconds; } } + + // Called by the async refresh worker after the slow probes complete. + public void PublishProbeResults(bool rcloneAvailable, bool winfspInstalled) + { + _cachedRcloneAvailable = rcloneAvailable; + _cachedWinfspInstalled = winfspInstalled; + _cachedStampUtc = DateTime.UtcNow; + } + + // Synchronous rclone probe. Tries the resolved path first; if that + // file isn't present, falls back to spawning `rclone.exe version` + // (PATH resolution). Returns true if either path responds. + public bool ProbeRcloneSync(Func runProcessCapture) + { + try + { + string resolved = _rclonePathProvider == null ? null : _rclonePathProvider(); + if (!String.IsNullOrEmpty(resolved) && File.Exists(resolved)) return true; + if (runProcessCapture == null) return false; + string version = runProcessCapture("rclone.exe", 3000); + return !String.IsNullOrEmpty(version) && version.IndexOf("rclone", StringComparison.OrdinalIgnoreCase) >= 0; + } + catch (Exception ex) { if (_logIssue != null) _logIssue("dep probe rclone", ex); return false; } + } + + // Synchronous WinFsp probe. Two on-disk dll locations + two registry + // keys; any positive signal is enough. + public bool ProbeWinFspSync() + { + try + { + string pf86 = Environment.GetFolderPath(Environment.SpecialFolder.ProgramFilesX86); + string pf = Environment.GetFolderPath(Environment.SpecialFolder.ProgramFiles); + if (File.Exists(Path.Combine(pf86, "WinFsp", "bin", "winfsp-x64.dll"))) return true; + if (File.Exists(Path.Combine(pf, "WinFsp", "bin", "winfsp-x64.dll"))) return true; + using (RegistryKey k1 = Registry.LocalMachine.OpenSubKey(@"SOFTWARE\WinFsp")) { if (k1 != null) return true; } + using (RegistryKey k2 = Registry.LocalMachine.OpenSubKey(@"SOFTWARE\WOW6432Node\WinFsp")) { if (k2 != null) return true; } + } + catch (Exception ex) { if (_logIssue != null) _logIssue("dep probe winfsp", ex); } + return false; + } + } +} diff --git a/src/Pixelpipe.Settings.cs b/src/Pixelpipe.Settings.cs index 72f2d69..6696044 100644 --- a/src/Pixelpipe.Settings.cs +++ b/src/Pixelpipe.Settings.cs @@ -110,74 +110,26 @@ private void SaveProfiles() catch (Exception ex) { LogUiIssue("save profiles", ex); } } - // PERF-3 (v0.13.4): cache the parsed settings root in memory so - // each SaveSetting / SaveProfiles doesn't re-read + re-parse the - // file. Atomic-write durability is preserved (WriteSettingsRoot - // still goes through WriteAllTextAtomic), we just skip the read + - // JavaScriptSerializer round-trip per write. Lock-protected so - // concurrent writes from worker threads don't tear the dict. - private Dictionary settingsRootCache; - private readonly object settingsRootCacheLock = new object(); - - private Dictionary ReadSettingsRoot() + // ARCH-1 step 2 (v0.15.2): the cache / read / write / lock mechanics + // moved into SettingsStore. TrayContext holds a lazily-initialised + // reference and delegates. PERF-3 cache behavior unchanged. + private SettingsStore _settingsStore; + private SettingsStore SettingsBackend { - lock (settingsRootCacheLock) + get { - if (settingsRootCache != null) return settingsRootCache; - Dictionary root; - if (TryReadSettingsRoot(settingsFile, out root)) - { - settingsRootCache = root; - return settingsRootCache; - } - string backupFile = settingsFile + ".bak"; - if (TryReadSettingsRoot(backupFile, out root)) + if (_settingsStore == null) { - LogUiWarn("read settings", "loaded backup settings file after primary file could not be read"); - settingsRootCache = root; - return settingsRootCache; + _settingsStore = new SettingsStore(settingsFile, LogUiIssue, + delegate(string area, string msg) { LogUiWarn(area, msg); }); } - settingsRootCache = new Dictionary(StringComparer.OrdinalIgnoreCase); - return settingsRootCache; + return _settingsStore; } } - private bool TryReadSettingsRoot(string path, out Dictionary root) - { - root = null; - try - { - if (!File.Exists(path)) return false; - string json = File.ReadAllText(path, Encoding.UTF8); - JavaScriptSerializer js = new JavaScriptSerializer(); - Dictionary parsed = js.DeserializeObject(json) as Dictionary; - if (parsed == null) return false; - root = new Dictionary(parsed, StringComparer.OrdinalIgnoreCase); - return true; - } - catch (Exception ex) { LogUiIssue("read settings " + Path.GetFileName(path), ex); } - return false; - } + private Dictionary ReadSettingsRoot() { return SettingsBackend.ReadRoot(); } - private void WriteSettingsRoot(Dictionary root) - { - try - { - Directory.CreateDirectory(settingsDir); - string json; - lock (settingsRootCacheLock) - { - // Caller may have built `root` from scratch (vs mutating - // the existing cache); replace the cache reference so - // the next ReadSettingsRoot returns the latest values. - settingsRootCache = root; - JavaScriptSerializer js = new JavaScriptSerializer(); - json = js.Serialize(root); - } - WriteAllTextAtomic(settingsFile, json, Encoding.UTF8); - } - catch (Exception ex) { LogUiIssue("write settings", ex); } - } + private void WriteSettingsRoot(Dictionary root) { SettingsBackend.WriteRoot(root); } // Pixelpipe already keeps the most recent settings.json as .bak via // WriteAllTextAtomic (it's overwritten on every save). Before any @@ -187,42 +139,9 @@ private void WriteSettingsRoot(Dictionary root) // recover a known-good state hours or days later. The directory is // pruned to the last `BackupRetentionCount` files so it doesn't // grow unbounded. - private const int BackupRetentionCount = 20; - - internal string BackupsDir { get { return Path.Combine(settingsDir, "backups"); } } - - private string BackupSettingsFile(string reason) - { - try - { - if (!File.Exists(settingsFile)) return null; - Directory.CreateDirectory(BackupsDir); - string stamp = DateTime.Now.ToString("yyyyMMdd-HHmmss"); - string safeReason = SafeFileName(String.IsNullOrEmpty(reason) ? "manual" : reason); - string target = Path.Combine(BackupsDir, "settings-" + stamp + "-" + safeReason + ".json"); - File.Copy(settingsFile, target, false); - PruneOldBackups(); - LogUiWarn("settings backup", "wrote " + target); - return target; - } - catch (Exception ex) { LogUiIssue("settings backup " + reason, ex); return null; } - } - - private void PruneOldBackups() - { - try - { - if (!Directory.Exists(BackupsDir)) return; - string[] files = Directory.GetFiles(BackupsDir, "settings-*.json"); - if (files.Length <= BackupRetentionCount) return; - Array.Sort(files, delegate(string a, string b) { return File.GetCreationTimeUtc(b).CompareTo(File.GetCreationTimeUtc(a)); }); - for (int i = BackupRetentionCount; i < files.Length; i++) - { - try { File.Delete(files[i]); } catch { } - } - } - catch (Exception ex) { LogUiIssue("prune backups", ex); } - } + // ARCH-1 step 2 (v0.15.2): backups + prune live in SettingsStore. + internal string BackupsDir { get { return SettingsBackend.BackupsDir; } } + private string BackupSettingsFile(string reason) { return SettingsBackend.Backup(reason); } // Tools / diagnostics menu hook. private void OpenSettingsBackupsFolder() diff --git a/src/Pixelpipe.Setup.cs b/src/Pixelpipe.Setup.cs index a999ff4..9139112 100644 --- a/src/Pixelpipe.Setup.cs +++ b/src/Pixelpipe.Setup.cs @@ -43,70 +43,45 @@ private void RunFirstLaunchSetup(bool manual) } } - // PERF-1 (v0.13.1): both RcloneAvailable and WinFspInstalled used - // to be called repeatedly from UI-thread paths (ApplyLiveState, - // UpdateMenuLiveState) every refresh tick. RcloneAvailable on a - // cold path could spawn `rclone.exe version` with a 3 s wait — - // multiple times per ~7 s refresh = visible stutter. We now cache - // the booleans with a TTL; RefreshDependencyStatusAsync (which - // already runs off-thread) is the one place that does the real - // probes and writes the cached fields. UI callers read the cached - // values via the public methods below, which never touch disk. - private const int DependencyCacheTtlSeconds = 30; - private volatile bool cachedRcloneAvailable; - private volatile bool cachedWinfspInstalled; - private DateTime cachedDependencyStampUtc = DateTime.MinValue; - - private bool RcloneAvailable() + // ARCH-1 step 3 (v0.15.3): cached dependency state + sync probes + // moved into DependencyProbe. The async refresh worker stays here + // because it composes results into the setup-status line and posts + // back via BeginUi. + private DependencyProbe _depProbe; + private DependencyProbe Deps { - EnsureDependencyCacheFresh(); - return cachedRcloneAvailable; + get + { + if (_depProbe == null) + { + _depProbe = new DependencyProbe( + delegate { rclonePath = FindRclonePath(); return rclonePath; }, + LogUiIssue); + } + return _depProbe; + } } - private bool WinFspInstalled() + private bool RcloneAvailable() { - EnsureDependencyCacheFresh(); - return cachedWinfspInstalled; + if (Deps.IsStale) RefreshDependencyStatusAsync(false); + return Deps.RcloneAvailable; } - // Lazy first-call probe: if the cache is empty / stale and we're on - // a UI thread, kick the async refresh and return whatever we have - // (false on cold start). UI updates within one refresh cycle. - private void EnsureDependencyCacheFresh() + private bool WinFspInstalled() { - if ((DateTime.UtcNow - cachedDependencyStampUtc).TotalSeconds < DependencyCacheTtlSeconds) return; - RefreshDependencyStatusAsync(false); + if (Deps.IsStale) RefreshDependencyStatusAsync(false); + return Deps.WinFspInstalled; } - // Synchronous probes — the real File.Exists / spawn-rclone / registry - // work — called only from the async refresh worker. Anything UI-bound - // must go through RcloneAvailable() / WinFspInstalled() above. + // Compatibility shims used by the refresh worker; both just forward + // to DependencyProbe. private bool ProbeRcloneAvailableSync() { - try - { - rclonePath = FindRclonePath(); - if (File.Exists(rclonePath)) return true; - string version = RunProcessCapture("rclone.exe", "version", 3000); - return version.IndexOf("rclone", StringComparison.OrdinalIgnoreCase) >= 0; - } - catch { return false; } + return Deps.ProbeRcloneSync(delegate(string exe, int timeoutMs) { return RunProcessCapture(exe, "version", timeoutMs); }); } - private bool ProbeWinFspInstalledSync() - { - try - { - string pf86 = Environment.GetFolderPath(Environment.SpecialFolder.ProgramFilesX86); - string pf = Environment.GetFolderPath(Environment.SpecialFolder.ProgramFiles); - if (File.Exists(Path.Combine(pf86, "WinFsp", "bin", "winfsp-x64.dll"))) return true; - if (File.Exists(Path.Combine(pf, "WinFsp", "bin", "winfsp-x64.dll"))) return true; - using (RegistryKey k1 = Registry.LocalMachine.OpenSubKey(@"SOFTWARE\WinFsp")) { if (k1 != null) return true; } - using (RegistryKey k2 = Registry.LocalMachine.OpenSubKey(@"SOFTWARE\WOW6432Node\WinFsp")) { if (k2 != null) return true; } - } - catch { } - return false; - } + private bool ProbeWinFspInstalledSync() { return Deps.ProbeWinFspSync(); } private bool AnyRemoteConfigured() { @@ -140,7 +115,7 @@ private string[] GetCachedRcloneRemotes(bool force) // Cheap cached lookup — never triggers a fresh disk probe; if // the dependency cache hasn't been seeded yet this returns false // and we keep the existing remotes cache rather than spinning. - if (!cachedRcloneAvailable) return cachedRcloneRemotes; + if (!Deps.RcloneAvailable) return cachedRcloneRemotes; string output; try { output = RunRcloneCapture("listremotes", 6000); } catch (Exception ex) { LogUiIssue("listremotes", ex); return cachedRcloneRemotes; } @@ -183,9 +158,7 @@ private void RefreshDependencyStatusAsync(bool force) catch (Exception ex) { LogUiIssue("dependency status", ex); text = setupStatusText; } BeginUi(delegate { - cachedRcloneAvailable = rcloneProbe; - cachedWinfspInstalled = winfspProbe; - cachedDependencyStampUtc = DateTime.UtcNow; + Deps.PublishProbeResults(rcloneProbe, winfspProbe); setupStatusText = text; lastDependencyRefreshUtc = DateTime.UtcNow; Interlocked.Exchange(ref dependencyRefreshingFlag, 0); diff --git a/src/SettingsStore.cs b/src/SettingsStore.cs new file mode 100644 index 0000000..229ea51 --- /dev/null +++ b/src/SettingsStore.cs @@ -0,0 +1,144 @@ +using System; +using System.Collections.Generic; +using System.IO; +using System.Text; +using System.Web.Script.Serialization; + +namespace Pixelpipe +{ + // ARCH-1 step 2 (v0.15.2, audit): second collaborator extracted from the + // TrayContext partial. Owns the settings.json read / write / cache / + // atomic-write / timestamped-backup machinery. + // + // Profile-specific shape logic (the LoadProfiles / SaveProfiles methods + // that know which keys live in each profile dict) stays in TrayContext; + // SettingsStore is the layer below that — give it the root dict, it + // hands you back the root dict, it handles disk. + // + // The locking + cache pattern from PERF-3 lives here unchanged: parsed + // root dict cached after first read, lock-protected so worker threads + // can write concurrently without tearing the dict. Atomic-write goes + // through WriteAllTextAtomic so durability is unchanged. + internal sealed class SettingsStore + { + private readonly string _settingsFile; + private readonly string _settingsDir; + private readonly string _backupsDir; + private readonly Action _logIssue; + private readonly Action _logWarn; + private readonly object _cacheLock = new object(); + private Dictionary _cache; + + public const int BackupRetentionCount = 20; + + public SettingsStore(string settingsFile, Action logIssue, Action logWarn) + { + _settingsFile = settingsFile; + _settingsDir = Path.GetDirectoryName(settingsFile) ?? ""; + _backupsDir = Path.Combine(_settingsDir, "backups"); + _logIssue = logIssue; + _logWarn = logWarn; + } + + public string SettingsFile { get { return _settingsFile; } } + public string BackupsDir { get { return _backupsDir; } } + + public Dictionary ReadRoot() + { + lock (_cacheLock) + { + if (_cache != null) return _cache; + Dictionary root; + if (TryReadFile(_settingsFile, out root)) + { + _cache = root; + return _cache; + } + string backupFile = _settingsFile + ".bak"; + if (TryReadFile(backupFile, out root)) + { + if (_logWarn != null) _logWarn("read settings", "loaded backup settings file after primary file could not be read"); + _cache = root; + return _cache; + } + _cache = new Dictionary(StringComparer.OrdinalIgnoreCase); + return _cache; + } + } + + public void WriteRoot(Dictionary root) + { + try + { + Directory.CreateDirectory(_settingsDir); + string json; + lock (_cacheLock) + { + _cache = root; + JavaScriptSerializer js = new JavaScriptSerializer(); + json = js.Serialize(root); + } + TrayContext.WriteAllTextAtomic(_settingsFile, json, Encoding.UTF8); + } + catch (Exception ex) { if (_logIssue != null) _logIssue("write settings", ex); } + } + + // Pass-through for the "is anything in settings.json" probe used by + // the welcome balloon path. Avoids parsing the file when callers only + // care about presence. + public bool Exists() { return File.Exists(_settingsFile); } + + // Timestamped backup before destructive operations. Returns the + // target path on success, null on failure (logged). Prunes the + // backups directory to BackupRetentionCount entries. + public string Backup(string reason) + { + try + { + if (!File.Exists(_settingsFile)) return null; + Directory.CreateDirectory(_backupsDir); + string stamp = DateTime.Now.ToString("yyyyMMdd-HHmmss"); + string safeReason = TrayContext.SafeFileName(String.IsNullOrEmpty(reason) ? "manual" : reason); + string target = Path.Combine(_backupsDir, "settings-" + stamp + "-" + safeReason + ".json"); + File.Copy(_settingsFile, target, false); + Prune(); + if (_logWarn != null) _logWarn("settings backup", "wrote " + target); + return target; + } + catch (Exception ex) { if (_logIssue != null) _logIssue("settings backup " + reason, ex); return null; } + } + + private void Prune() + { + try + { + if (!Directory.Exists(_backupsDir)) return; + string[] files = Directory.GetFiles(_backupsDir, "settings-*.json"); + if (files.Length <= BackupRetentionCount) return; + Array.Sort(files, delegate (string a, string b) { return File.GetCreationTimeUtc(b).CompareTo(File.GetCreationTimeUtc(a)); }); + for (int i = BackupRetentionCount; i < files.Length; i++) + { + try { File.Delete(files[i]); } catch { } + } + } + catch (Exception ex) { if (_logIssue != null) _logIssue("prune backups", ex); } + } + + private bool TryReadFile(string path, out Dictionary root) + { + root = null; + try + { + if (!File.Exists(path)) return false; + string json = File.ReadAllText(path, Encoding.UTF8); + JavaScriptSerializer js = new JavaScriptSerializer(); + Dictionary parsed = js.DeserializeObject(json) as Dictionary; + if (parsed == null) return false; + root = new Dictionary(parsed, StringComparer.OrdinalIgnoreCase); + return true; + } + catch (Exception ex) { if (_logIssue != null) _logIssue("read settings " + Path.GetFileName(path), ex); } + return false; + } + } +}