Switch to using factories for users of ISyncThingApiClient, rather than singleton injection

This will be needed for compat with different API versions, needed for 0.11.x

It added complexity around ensuring that the event watcher and connections watcher are
instantiated / torn down at the right times, which also lead into aborting them correctly

src/SyncTrayzor/Bootstrapper.cs

@@ -37,11 +37,11 @@ namespace SyncTrayzor
             builder.Bind<IConfigurationProvider>().To<ConfigurationProvider>().InSingletonScope();
             builder.Bind<IAutostartProvider>().To<AutostartProvider>().InSingletonScope();
             builder.Bind<ConfigurationApplicator>().ToSelf().InSingletonScope();
-            builder.Bind<ISyncThingApiClient>().To<SyncThingApiClient>().InSingletonScope();
-            builder.Bind<ISyncThingEventWatcher>().To<SyncThingEventWatcher>().InSingletonScope();
+            builder.Bind<ISyncThingApiClientFactory>().To<SyncThingApiClientFactory>();
+            builder.Bind<ISyncThingEventWatcherFactory>().To<SyncThingEventWatcherFactory>();
             builder.Bind<ISyncThingProcessRunner>().To<SyncThingProcessRunner>().InSingletonScope();
             builder.Bind<ISyncThingManager>().To<SyncThingManager>().InSingletonScope();
-            builder.Bind<ISyncThingConnectionsWatcher>().To<SyncThingConnectionsWatcher>().InSingletonScope();
+            builder.Bind<ISyncThingConnectionsWatcherFactory>().To<SyncThingConnectionsWatcherFactory>();
             builder.Bind<INotifyIconManager>().To<NotifyIconManager>().InSingletonScope();
             builder.Bind<IWatchedFolderMonitor>().To<WatchedFolderMonitor>().InSingletonScope();
             builder.Bind<IGithubApiClient>().To<GithubApiClient>().InSingletonScope();

src/SyncTrayzor/SyncThing/Api/ISyncThingApi.cs

@@ -3,6 +3,7 @@ using System;
 using System.Collections.Generic;
 using System.Linq;
 using System.Text;
+using System.Threading;
 using System.Threading.Tasks;
 
 namespace SyncTrayzor.SyncThing.Api
@@ -10,10 +11,10 @@ namespace SyncTrayzor.SyncThing.Api
     public interface ISyncThingApi
     {
         [Get("/rest/events")]
-        Task<List<Event>> FetchEventsAsync(int since);
+        Task<List<Event>> FetchEventsAsync(int since, CancellationToken cancellationToken);
 
         [Get("/rest/events")]
-        Task<List<Event>> FetchEventsLimitAsync(int since, int limit);
+        Task<List<Event>> FetchEventsLimitAsync(int since, int limit, CancellationToken cancellationToken);
 
         [Get("/rest/config")]
         Task<Config> FetchConfigAsync();

src/SyncTrayzor/SyncThing/EventWatcher/SyncThingEventWatcher.cs

@@ -6,6 +6,7 @@ using System.IO;
 using System.Linq;
 using System.Net.Http;
 using System.Text;
+using System.Threading;
 using System.Threading.Tasks;
 
 namespace SyncTrayzor.SyncThing.EventWatcher
@@ -46,11 +47,11 @@ namespace SyncTrayzor.SyncThing.EventWatcher
             base.Start();
         }
 
-        protected override async Task PollAsync()
+        protected override async Task PollAsync(CancellationToken cancellationToken)
         {
             try
             {
-                var events = await this.apiClient.FetchEventsAsync(this.lastEventId);
+                var events = await this.apiClient.FetchEventsAsync(this.lastEventId, cancellationToken);
 
                 // We can be aborted in the time it takes to fetch the events
                 if (!this.Running)

src/SyncTrayzor/SyncThing/EventWatcher/SyncThingEventWatcherFactory.cs

@@ -0,0 +1,23 @@
+using SyncTrayzor.SyncThing;
+using SyncTrayzor.SyncThing.EventWatcher;
+using System;
+using System.Collections.Generic;
+using System.Linq;
+using System.Text;
+using System.Threading.Tasks;
+
+namespace SyncTrayzor.SyncThing.EventWatcher
+{
+    public interface ISyncThingEventWatcherFactory
+    {
+        ISyncThingEventWatcher CreateEventWatcher(ISyncThingApiClient apiClient);
+    }
+
+    public class SyncThingEventWatcherFactory : ISyncThingEventWatcherFactory
+    {
+        public ISyncThingEventWatcher CreateEventWatcher(ISyncThingApiClient apiClient)
+        {
+            return new SyncThingEventWatcher(apiClient);
+        }
+    }
+}

src/SyncTrayzor/SyncThing/SyncThingApiClient.cs

@@ -8,16 +8,16 @@ using System.Collections.Generic;
 using System.Linq;
 using System.Net.Http;
 using System.Text;
+using System.Threading;
 using System.Threading.Tasks;
 
 namespace SyncTrayzor.SyncThing
 {
     public interface ISyncThingApiClient
     {
-        void SetConnectionDetails(Uri baseAddress, string apiKey);
-
         Task ShutdownAsync();
-        Task<List<Event>> FetchEventsAsync(int since, int? limit = null);
+        Task<List<Event>> FetchEventsAsync(int since, int limit, CancellationToken cancellationToken);
+        Task<List<Event>> FetchEventsAsync(int since, CancellationToken cancellationToken);
         Task<Config> FetchConfigAsync();
         Task ScanAsync(string folderId, string subPath);
         Task<SystemInfo> FetchSystemInfoAsync();
@@ -32,7 +32,7 @@ namespace SyncTrayzor.SyncThing
         private static readonly Logger logger = LogManager.GetCurrentClassLogger();
         private ISyncThingApi api;
 
-        public void SetConnectionDetails(Uri baseAddress, string apiKey)
+        public SyncThingApiClient(Uri baseAddress, string apiKey)
         {
             var httpClient = new HttpClient(new AuthenticatedHttpClientHandler(apiKey))
             {
@@ -51,22 +51,21 @@ namespace SyncTrayzor.SyncThing
         public Task ShutdownAsync()
         {
             logger.Info("Requesting API shutdown");
-            this.EnsureSetup();
             return this.api.ShutdownAsync();
         }
 
-        public Task<List<Event>> FetchEventsAsync(int since, int? limit)
+        public Task<List<Event>> FetchEventsAsync(int since, int limit, CancellationToken cancellationToken)
+        {
+            return this.api.FetchEventsLimitAsync(since, limit, cancellationToken);
+        }
+
+        public Task<List<Event>> FetchEventsAsync(int since, CancellationToken cancellationToken)
         {
-            this.EnsureSetup();
-            if (limit == null)
-                return this.api.FetchEventsAsync(since);
-            else
-                return this.api.FetchEventsLimitAsync(since, limit.Value);
+            return this.api.FetchEventsAsync(since, cancellationToken);
         }
 
         public async Task<Config> FetchConfigAsync()
         {
-            this.EnsureSetup();
             var config = await this.api.FetchConfigAsync();
             logger.Debug("Fetched configuration: {0}", config);
             return config;
@@ -75,13 +74,11 @@ namespace SyncTrayzor.SyncThing
         public Task ScanAsync(string folderId, string subPath)
         {
             logger.Debug("Scanning folder: {0} subPath: {1}", folderId, subPath);
-            this.EnsureSetup();
             return this.api.ScanAsync(folderId, subPath);
         }
 
         public async Task<SystemInfo> FetchSystemInfoAsync()
         {
-            this.EnsureSetup();
             var systemInfo = await this.api.FetchSystemInfoAsync();
             logger.Debug("Fetched system info: {0}", systemInfo);
             return systemInfo;
@@ -89,13 +86,11 @@ namespace SyncTrayzor.SyncThing
 
         public Task<Connections> FetchConnectionsAsync()
         {
-            this.EnsureSetup();
             return this.api.FetchConnectionsAsync();
         }
 
         public async Task<SyncthingVersion> FetchVersionAsync()
         {
-            this.EnsureSetup();
             var version = await this.api.FetchVersionAsync();
             logger.Debug("Fetched version: {0}", version);
             return version;
@@ -103,7 +98,6 @@ namespace SyncTrayzor.SyncThing
 
         public async Task<Ignores> FetchIgnoresAsync(string folderId)
         {
-            this.EnsureSetup();
             var ignores = await this.api.FetchIgnoresAsync(folderId);
             logger.Debug("Fetched ignores for folderid {0}: {1}", folderId, ignores);
             return ignores;
@@ -111,17 +105,10 @@ namespace SyncTrayzor.SyncThing
 
         public Task RestartAsync()
         {
-            this.EnsureSetup();
             logger.Debug("Restarting Syncthing");
             return this.api.RestartAsync();
         }
 
-        private void EnsureSetup()
-        {
-            if (this.api == null)
-                throw new InvalidOperationException("SetConnectionDetails not called");
-        }
-
         private class AuthenticatedHttpClientHandler : WebRequestHandler
         {
             private readonly string apiKey;

src/SyncTrayzor/SyncThing/SyncThingApiClientFactory.cs

@@ -0,0 +1,21 @@
+using System;
+using System.Collections.Generic;
+using System.Linq;
+using System.Text;
+using System.Threading.Tasks;
+
+namespace SyncTrayzor.SyncThing
+{
+    public interface ISyncThingApiClientFactory
+    {
+        ISyncThingApiClient CreateApiClient(Uri baseAddress, string apiKey);
+    }
+
+    public class SyncThingApiClientFactory : ISyncThingApiClientFactory
+    {
+        public ISyncThingApiClient CreateApiClient(Uri baseAddress, string apiKey)
+        {
+            return new SyncThingApiClient(baseAddress, apiKey);
+        }
+    }
+}

src/SyncTrayzor/SyncThing/SyncThingConnectionsWatcher.cs

@@ -3,6 +3,7 @@ using System;
 using System.Collections.Generic;
 using System.Linq;
 using System.Text;
+using System.Threading;
 using System.Threading.Tasks;
 
 namespace SyncTrayzor.SyncThing
@@ -38,7 +39,7 @@ namespace SyncTrayzor.SyncThing
             this.apiClient = apiClient;
         }
 
-        protected override async Task PollAsync()
+        protected override async Task PollAsync(CancellationToken cancellationToken)
         {
             var connections = await this.apiClient.FetchConnectionsAsync();
 

src/SyncTrayzor/SyncThing/SyncThingConnectionsWatcherFactory.cs

@@ -0,0 +1,23 @@
+using StyletIoC;
+using SyncTrayzor.SyncThing;
+using System;
+using System.Collections.Generic;
+using System.Linq;
+using System.Text;
+using System.Threading.Tasks;
+
+namespace SyncTrayzor.SyncThing
+{
+    public interface ISyncThingConnectionsWatcherFactory
+    {
+        ISyncThingConnectionsWatcher CreateConnectionsWatcher(ISyncThingApiClient apiClient);
+    }
+
+    public class SyncThingConnectionsWatcherFactory : ISyncThingConnectionsWatcherFactory
+    {
+        public ISyncThingConnectionsWatcher CreateConnectionsWatcher(ISyncThingApiClient apiClient)
+        {
+            return new SyncThingConnectionsWatcher(apiClient);
+        }
+    }
+}

src/SyncTrayzor/SyncThing/SyncThingManager.cs

@@ -63,9 +63,14 @@ namespace SyncTrayzor.SyncThing
 
         private readonly SynchronizedEventDispatcher eventDispatcher;
         private readonly ISyncThingProcessRunner processRunner;
-        private readonly ISyncThingApiClient apiClient;
-        private readonly ISyncThingEventWatcher eventWatcher;
-        private readonly ISyncThingConnectionsWatcher connectionsWatcher;
+        private readonly ISyncThingApiClientFactory apiClientFactory;
+        private readonly ISyncThingEventWatcherFactory eventWatcherFactory;
+        private readonly ISyncThingConnectionsWatcherFactory connectionsWatcherFactory;
+
+        private ISyncThingEventWatcher eventWatcher;
+        private ISyncThingConnectionsWatcher connectionsWatcher;
+        private ISyncThingApiClient apiClient;
+        private CancellationTokenSource apiAbortCts;
 
         private DateTime _startedTime;
         private readonly object startedTimeLock = new object();
@@ -143,25 +148,23 @@ namespace SyncTrayzor.SyncThing
 
         public SyncThingManager(
             ISyncThingProcessRunner processRunner,
-            ISyncThingApiClient apiClient,
-            ISyncThingEventWatcher eventWatcher,
-            ISyncThingConnectionsWatcher connectionsWatcher)
+            ISyncThingApiClientFactory apiClientFactory,
+            ISyncThingEventWatcherFactory eventWatcherFactory,
+            ISyncThingConnectionsWatcherFactory connectionsWatcherFactory)
         {
             this.StartedTime = DateTime.MinValue;
             this.LastConnectivityEventTime = DateTime.MinValue;
 
             this.eventDispatcher = new SynchronizedEventDispatcher(this);
             this.processRunner = processRunner;
-            this.apiClient = apiClient;
-            this.eventWatcher = eventWatcher;
-            this.connectionsWatcher = connectionsWatcher;
+            this.apiClientFactory = apiClientFactory;
+            this.eventWatcherFactory = eventWatcherFactory;
+            this.connectionsWatcherFactory = connectionsWatcherFactory;
 
             this.processRunner.ProcessStopped += (o, e) => this.ProcessStopped(e.ExitStatus);
             this.processRunner.MessageLogged += (o, e) => this.OnMessageLogged(e.LogMessage);
             this.processRunner.Starting += (o, e) =>
             {
-                // This is fired on restarts, too, so we can update config
-                this.apiClient.SetConnectionDetails(this.Address, this.ApiKey);
                 this.processRunner.ApiKey = this.ApiKey;
                 this.processRunner.HostAddress = this.Address.ToString();
                 this.processRunner.ExecutablePath = this.ExecutablePath;
@@ -173,21 +176,13 @@ namespace SyncTrayzor.SyncThing
 
                 this.SetState(SyncThingState.Starting);
             };
-
-            this.eventWatcher.StartupComplete += (o, e) => { var t = this.StartupCompleteAsync(); };
-            this.eventWatcher.SyncStateChanged += (o, e) => this.OnFolderSyncStateChanged(e);
-            this.eventWatcher.ItemStarted += (o, e) => this.ItemStarted(e.Folder, e.Item);
-            this.eventWatcher.ItemFinished += (o, e) => this.ItemFinished(e.Folder, e.Item);
-            this.eventWatcher.DeviceConnected += (o, e) => this.OnDeviceConnected(e);
-            this.eventWatcher.DeviceDisconnected += (o, e) => this.OnDeviceDisconnected(e);
-
-            this.connectionsWatcher.TotalConnectionStatsChanged += (o, e) => this.OnTotalConnectionStatsChanged(e.TotalConnectionStats);
         }
 
         public void Start()
         {
             try
             {
+                this.apiAbortCts = new CancellationTokenSource();
                 this.processRunner.Start();
             }
             catch (Exception e)
@@ -197,13 +192,13 @@ namespace SyncTrayzor.SyncThing
             }
         }
 
-        public Task StopAsync()
+        public async Task StopAsync()
         {
             if (this.State != SyncThingState.Running)
-                return Task.FromResult(false);
+                return;
 
+            await this.apiClient.ShutdownAsync();
             this.SetState(SyncThingState.Stopping);
-            return this.apiClient.ShutdownAsync();
         }
 
         public Task RestartAsync()
@@ -278,18 +273,51 @@ namespace SyncTrayzor.SyncThing
                 if (this._state == SyncThingState.Stopped && state == SyncThingState.Running)
                     return;
 
+                if ((this._state == SyncThingState.Running || this._state == SyncThingState.Starting) && state == SyncThingState.Stopped)
+                    this.apiAbortCts.Cancel();
+
                 this._state = state;
             }
 
-            this.UpdateWatchersState(state);
+            this.UpdateWatchersState(oldState, state);
             this.eventDispatcher.Raise(this.StateChanged, new SyncThingStateChangedEventArgs(oldState, state));
         }
 
-        private void UpdateWatchersState(SyncThingState state)
+        private void UpdateWatchersState(SyncThingState oldState, SyncThingState newState)
         {
-            var running = (state == SyncThingState.Starting || state == SyncThingState.Running);
-            this.eventWatcher.Running = running;
-            this.connectionsWatcher.Running = running;
+            if (newState == SyncThingState.Starting || (oldState != SyncThingState.Starting && newState == SyncThingState.Running))
+            {
+                this.apiClient = this.apiClientFactory.CreateApiClient(this.Address, this.ApiKey);
+
+                if (this.connectionsWatcher != null)
+                    this.connectionsWatcher.Dispose();
+                this.connectionsWatcher = this.connectionsWatcherFactory.CreateConnectionsWatcher(this.apiClient);
+                this.connectionsWatcher.TotalConnectionStatsChanged += (o, e) => this.OnTotalConnectionStatsChanged(e.TotalConnectionStats);
+                this.connectionsWatcher.Running = true;
+
+                if (this.eventWatcher != null)
+                    this.eventWatcher.Dispose();
+                this.eventWatcher = this.eventWatcherFactory.CreateEventWatcher(this.apiClient);
+                this.eventWatcher.StartupComplete += (o, e) => this.OnStartupComplete();
+                this.eventWatcher.SyncStateChanged += (o, e) => this.OnFolderSyncStateChanged(e);
+                this.eventWatcher.ItemStarted += (o, e) => this.ItemStarted(e.Folder, e.Item);
+                this.eventWatcher.ItemFinished += (o, e) => this.ItemFinished(e.Folder, e.Item);
+                this.eventWatcher.DeviceConnected += (o, e) => this.OnDeviceConnected(e);
+                this.eventWatcher.DeviceDisconnected += (o, e) => this.OnDeviceDisconnected(e);
+                this.eventWatcher.Running = true;
+            }
+            else if (newState == SyncThingState.Stopping || newState == SyncThingState.Stopped)
+            {
+                this.apiClient = null;
+
+                if (this.connectionsWatcher != null)
+                    this.connectionsWatcher.Dispose();
+                this.connectionsWatcher = null;
+
+                if (this.eventWatcher != null)
+                    this.eventWatcher.Dispose();
+                this.eventWatcher = null;
+            }
         }
 
         private void ProcessStopped(SyncThingExitStatus exitStatus)
@@ -299,7 +327,18 @@ namespace SyncTrayzor.SyncThing
                 this.OnProcessExitedWithError();
         }
 
-        private async Task StartupCompleteAsync()
+        private async void OnStartupComplete()
+        {
+            // We'll get cancelled if the startup aborts - e.g. if Syncthing can't lock its config directory
+            try
+            {
+                await this.LoadStartupDataAsync(this.apiAbortCts.Token);
+            }
+            catch (OperationCanceledException)
+            { }
+        }
+
+        private async Task LoadStartupDataAsync(CancellationToken cancellationToken)
         {
             this.SetState(SyncThingState.Running);
 
@@ -307,6 +346,8 @@ namespace SyncTrayzor.SyncThing
             var systemTask = this.apiClient.FetchSystemInfoAsync();
             var versionTask = this.apiClient.FetchVersionAsync();
             var connectionsTask = this.apiClient.FetchConnectionsAsync();
+
+            cancellationToken.ThrowIfCancellationRequested();
             await Task.WhenAll(configTask, systemTask, versionTask, connectionsTask);
 
             this.devices = new ConcurrentDictionary<string, Device>(configTask.Result.Devices.Select(device =>
@@ -329,11 +370,13 @@ namespace SyncTrayzor.SyncThing
                 return new Folder(folder.ID, path, new FolderIgnores(ignores.IgnorePatterns, ignores.RegexPatterns));
             });
 
+            cancellationToken.ThrowIfCancellationRequested();
             var folders = await Task.WhenAll(folderConstructionTasks);
             this.folders = new ConcurrentDictionary<string, Folder>(folders.Select(x => new KeyValuePair<string, Folder>(x.FolderId, x)));
 
             this.Version = versionTask.Result;
 
+            cancellationToken.ThrowIfCancellationRequested();
             this.OnDataLoaded();
             this.StartedTime = DateTime.UtcNow;
             this.IsDataLoaded = true;
@@ -422,6 +465,10 @@ namespace SyncTrayzor.SyncThing
         public void Dispose()
         {
             this.processRunner.Dispose();
+            if (this.connectionsWatcher != null)
+                this.connectionsWatcher.Dispose();
+            if (this.eventWatcher != null)
+                this.eventWatcher.Dispose();
         }
     }
 }

src/SyncTrayzor/SyncThing/SyncThingPoller.cs

@@ -4,11 +4,12 @@ using System.IO;
 using System.Linq;
 using System.Net.Http;
 using System.Text;
+using System.Threading;
 using System.Threading.Tasks;
 
 namespace SyncTrayzor.SyncThing
 {
-    public interface ISyncThingPoller
+    public interface ISyncThingPoller : IDisposable
     {
         bool Running { get; set; }
     }
@@ -19,6 +20,7 @@ namespace SyncTrayzor.SyncThing
         private readonly TimeSpan erroredWaitInterval;
 
         private readonly object runningLock = new object();
+        private CancellationTokenSource cancelCts;
         private bool _running;
         public bool Running
         {
@@ -33,8 +35,13 @@ namespace SyncTrayzor.SyncThing
                     this._running = value;
                     if (value)
                     {
+                        this.cancelCts = new CancellationTokenSource();
                         this.Start();
                     }
+                    else
+                    {
+                        this.cancelCts.Cancel();
+                    }
                 }
             }
         }
@@ -59,9 +66,9 @@ namespace SyncTrayzor.SyncThing
 
                     try
                     {
-                        await this.PollAsync();
+                        await this.PollAsync(this.cancelCts.Token);
                         if (this.pollingInterval.Ticks > 0)
-                            await Task.Delay(this.pollingInterval);
+                            await Task.Delay(this.pollingInterval, this.cancelCts.Token);
                     }
                     catch (HttpRequestException)
                     {
@@ -79,7 +86,14 @@ namespace SyncTrayzor.SyncThing
                     }
 
                     if (errored)
-                        await Task.Delay(this.erroredWaitInterval);
+                    {
+                        try
+                        {
+                            await Task.Delay(this.erroredWaitInterval, this.cancelCts.Token);
+                        }
+                        catch (OperationCanceledException)
+                        { }
+                    }
                 }
             }
             finally
@@ -88,6 +102,11 @@ namespace SyncTrayzor.SyncThing
             }
         }
 
-        protected abstract Task PollAsync();
+        protected abstract Task PollAsync(CancellationToken cancellationToken);
+
+        public void Dispose()
+        {
+            this.Running = false;
+        }
     }
 }

src/SyncTrayzor/SyncTrayzor.csproj

@@ -120,6 +120,9 @@
       <DependentUpon>App.xaml</DependentUpon>
       <SubType>Code</SubType>
     </Compile>
+    <Compile Include="SyncThing\SyncThingApiClientFactory.cs" />
+    <Compile Include="SyncThing\SyncThingConnectionsWatcherFactory.cs" />
+    <Compile Include="SyncThing\EventWatcher\SyncThingEventWatcherFactory.cs" />
     <Compile Include="Localization\LocalizeConverter.cs" />
     <Compile Include="Localization\Localizer.cs" />
     <Compile Include="Localization\LocExtension.cs" />
@@ -366,6 +369,9 @@
   <ItemGroup>
     <EmbeddedResource Include="Resources\Licenses\FluentValidation.txt" />
   </ItemGroup>
+  <ItemGroup>
+    <Folder Include="Core\" />
+  </ItemGroup>
   <Import Project="$(MSBuildToolsPath)\Microsoft.CSharp.targets" />
   <Import Project="..\packages\Fody.1.26.4\build\Fody.targets" Condition="Exists('..\packages\Fody.1.26.4\build\Fody.targets')" />
   <Target Name="EnsureNuGetPackageBuildImports" BeforeTargets="PrepareForBuild">