Made the file config poller use IHostedService, bit more generic, not… (#507)

* Made the file config poller use IHostedService, bit more generic, not just need to provide the correct implementations of the repo services and it will poll anything..this means we can open up redis for #458

* removed comments

src/Ocelot/Configuration/Repository/FileConfigurationPoller.cs

@@ -2,34 +2,45 @@ using System;
 using System.Linq;
 using System.Threading;
 using System.Threading.Tasks;
+using Microsoft.Extensions.Hosting;
 using Newtonsoft.Json;
+using Ocelot.Configuration.Creator;
 using Ocelot.Configuration.File;
 using Ocelot.Configuration.Setter;
 using Ocelot.Logging;
 
 namespace Ocelot.Configuration.Repository
 {
-    public class ConsulFileConfigurationPoller : IDisposable
+    public class FileConfigurationPoller : IHostedService, IDisposable
     {
         private readonly IOcelotLogger _logger; 
         private readonly IFileConfigurationRepository _repo;
-        private readonly IFileConfigurationSetter _setter;
         private string _previousAsJson;
-        private readonly Timer _timer;
+        private Timer _timer;
         private bool _polling;
-        private readonly IConsulPollerConfiguration _config;
+        private readonly IFileConfigurationPollerOptions _options;
+        private readonly IInternalConfigurationRepository _internalConfigRepo;
+        private readonly IInternalConfigurationCreator _internalConfigCreator;
 
-        public ConsulFileConfigurationPoller(
+        public FileConfigurationPoller(
             IOcelotLoggerFactory factory, 
             IFileConfigurationRepository repo, 
-            IFileConfigurationSetter setter, 
+            IFileConfigurationPollerOptions options,
-            IConsulPollerConfiguration config)
+            IInternalConfigurationRepository internalConfigRepo,
+            IInternalConfigurationCreator internalConfigCreator)
         {
-            _setter = setter;
+            _internalConfigRepo = internalConfigRepo;
-            _config = config;
+            _internalConfigCreator = internalConfigCreator;
-            _logger = factory.CreateLogger<ConsulFileConfigurationPoller>();
+            _options = options;
+            _logger = factory.CreateLogger<FileConfigurationPoller>();
             _repo = repo;
             _previousAsJson = "";
+        }
+        
+        public Task StartAsync(CancellationToken cancellationToken)
+        {
+            _logger.LogInformation($"{nameof(FileConfigurationPoller)} is starting.");
+
             _timer = new Timer(async x =>
             {
                 if(_polling)
@@ -40,7 +51,18 @@ namespace Ocelot.Configuration.Repository
                 _polling = true;
                 await Poll();
                 _polling = false;
-            }, null, _config.Delay, _config.Delay);
+            }, null, _options.Delay, _options.Delay);
+
+            return Task.CompletedTask;
+        }
+
+        public Task StopAsync(CancellationToken cancellationToken)
+        {
+            _logger.LogInformation($"{nameof(FileConfigurationPoller)} is stopping.");
+
+            _timer?.Change(Timeout.Infinite, 0);
+
+            return Task.CompletedTask;        
         }
 
         private async Task Poll()
@@ -59,7 +81,13 @@ namespace Ocelot.Configuration.Repository
 
             if(!fileConfig.IsError && asJson != _previousAsJson)
             {
-                await _setter.Set(fileConfig.Data);
+                var config = await _internalConfigCreator.Create(fileConfig.Data);
+
+                if(!config.IsError)
+                {
+                    _internalConfigRepo.AddOrReplace(config.Data);
+                }
+
                 _previousAsJson = asJson;
             }
 

src/Ocelot/Configuration/Repository/IConsulPollerConfiguration.cs

@@ -1,6 +1,6 @@
 namespace Ocelot.Configuration.Repository
 {
-    public interface IConsulPollerConfiguration
+    public interface IFileConfigurationPollerOptions
     {
         int Delay { get; }
     }

src/Ocelot/Configuration/Repository/InMemoryFileConfigurationPollerOptions.cs

@@ -1,6 +1,6 @@
 namespace Ocelot.Configuration.Repository
 {
-    public class InMemoryConsulPollerConfiguration : IConsulPollerConfiguration
+    public class InMemoryFileConfigurationPollerOptions : IFileConfigurationPollerOptions
     {
         public int Delay => 1000;
     }

src/Ocelot/Configuration/Setter/FileAndInternalConfigurationSetter.cs

@@ -8,7 +8,7 @@ namespace Ocelot.Configuration.Setter
 {
     public class FileAndInternalConfigurationSetter : IFileConfigurationSetter
     {
-        private readonly IInternalConfigurationRepository _configRepo;
+        private readonly IInternalConfigurationRepository internalConfigRepo;
         private readonly IInternalConfigurationCreator _configCreator;
         private readonly IFileConfigurationRepository _repo;
 
@@ -17,7 +17,7 @@ namespace Ocelot.Configuration.Setter
             IInternalConfigurationCreator configCreator, 
             IFileConfigurationRepository repo)
         {
-            _configRepo = configRepo;
+            internalConfigRepo = configRepo;
             _configCreator = configCreator;
             _repo = repo;
         }
@@ -35,7 +35,7 @@ namespace Ocelot.Configuration.Setter
 
             if(!config.IsError)
             {
-                _configRepo.AddOrReplace(config.Data);
+                internalConfigRepo.AddOrReplace(config.Data);
             }
 
             return new ErrorResponse(config.Errors);

src/Ocelot/DependencyInjection/OcelotBuilder.cs

@@ -160,7 +160,7 @@ namespace Ocelot.DependencyInjection
 
             // We add this here so that we can always inject something into the factory for IoC..
             _services.AddSingleton<IServiceTracer, FakeServiceTracer>();
-            _services.TryAddSingleton<IConsulPollerConfiguration, InMemoryConsulPollerConfiguration>();
+            _services.TryAddSingleton<IFileConfigurationPollerOptions, InMemoryFileConfigurationPollerOptions>();
             _services.TryAddSingleton<IAddHeadersToResponse, AddHeadersToResponse>();
             _services.TryAddSingleton<IPlaceholders, Placeholders>();
             _services.TryAddSingleton<IConsulClientFactory, ConsulClientFactory>();
@@ -245,7 +245,7 @@ namespace Ocelot.DependencyInjection
 
         public IOcelotBuilder AddStoreOcelotConfigurationInConsul()
         {
-            _services.AddSingleton<ConsulFileConfigurationPoller>();
+            _services.AddHostedService<FileConfigurationPoller>();
             _services.AddSingleton<IFileConfigurationRepository, ConsulFileConfigurationRepository>();
             return this;
         }

src/Ocelot/Middleware/OcelotMiddlewareExtensions.cs

@@ -183,9 +183,6 @@
                     ThrowToStopOcelotStarting(internalConfig);
                 }
             }
-
-            //todo - this starts the poller if it has been registered...please this is so bad.
-            var hack = builder.ApplicationServices.GetService(typeof(ConsulFileConfigurationPoller));
         }
 
         private static async Task SetFileConfig(IFileConfigurationSetter fileConfigSetter, IOptions<FileConfiguration> fileConfig)

test/Ocelot.AcceptanceTests/ConfigurationInConsulTests.cs

@@ -122,6 +122,7 @@ namespace Ocelot.AcceptanceTests
         public void should_load_configuration_out_of_consul()
         {
             var consulPort = 8500;
+            
             var configuration = new FileConfiguration
             {
                 GlobalConfiguration = new FileGlobalConfiguration()

test/Ocelot.UnitTests/Configuration/FileConfigurationPollerTests.cs

@@ -12,35 +12,37 @@ using TestStack.BDDfy;
 using Xunit;
 using Shouldly;
 using static Ocelot.Infrastructure.Wait;
+using Ocelot.Configuration.Creator;
+using Ocelot.Configuration;
 
 namespace Ocelot.UnitTests.Configuration
 {
-    public class ConsulFileConfigurationPollerTests : IDisposable
+    public class FileConfigurationPollerTests : IDisposable
     {
-        private readonly ConsulFileConfigurationPoller _poller;
+        private readonly FileConfigurationPoller _poller;
         private Mock<IOcelotLoggerFactory> _factory;
         private readonly Mock<IFileConfigurationRepository> _repo;
-        private readonly Mock<IFileConfigurationSetter> _setter;
         private readonly FileConfiguration _fileConfig;
-        private Mock<IConsulPollerConfiguration> _config;
+        private Mock<IFileConfigurationPollerOptions> _config;
+        private readonly Mock<IInternalConfigurationRepository> _internalConfigRepo;
+        private readonly Mock<IInternalConfigurationCreator> _internalConfigCreator;
+        private IInternalConfiguration _internalConfig;
 
-        public ConsulFileConfigurationPollerTests()
+        public FileConfigurationPollerTests()
         {
             var logger = new Mock<IOcelotLogger>();
             _factory = new Mock<IOcelotLoggerFactory>();
-            _factory.Setup(x => x.CreateLogger<ConsulFileConfigurationPoller>()).Returns(logger.Object);
+            _factory.Setup(x => x.CreateLogger<FileConfigurationPoller>()).Returns(logger.Object);
             _repo = new Mock<IFileConfigurationRepository>();
-            _setter = new Mock<IFileConfigurationSetter>();
             _fileConfig = new FileConfiguration();
-            _config = new Mock<IConsulPollerConfiguration>();
+            _config = new Mock<IFileConfigurationPollerOptions>();
             _repo.Setup(x => x.Get()).ReturnsAsync(new OkResponse<FileConfiguration>(_fileConfig));
             _config.Setup(x => x.Delay).Returns(100);
-            _poller = new ConsulFileConfigurationPoller(_factory.Object, _repo.Object, _setter.Object, _config.Object);
+            _internalConfigRepo = new Mock<IInternalConfigurationRepository>();
-        }
+            _internalConfigCreator = new Mock<IInternalConfigurationCreator>();
-        
+            _internalConfigCreator.Setup(x => x.Create(It.IsAny<FileConfiguration>())).ReturnsAsync(new OkResponse<IInternalConfiguration>(_internalConfig));
-        public void Dispose()
+            _poller = new FileConfigurationPoller(_factory.Object, _repo.Object, _config.Object, _internalConfigRepo.Object, _internalConfigCreator.Object);
-        {
+            _poller.StartAsync(new CancellationToken());
-            _poller.Dispose();
         }
     
         [Fact]
@@ -141,10 +143,11 @@ namespace Ocelot.UnitTests.Configuration
 
         private void ThenTheSetterIsCalled(FileConfiguration fileConfig, int times)
         {
-            var result = WaitFor(2000).Until(() => {
+            var result = WaitFor(4000).Until(() => {
                 try
                 {
-                    _setter.Verify(x => x.Set(fileConfig), Times.Exactly(times));
+                    _internalConfigRepo.Verify(x => x.AddOrReplace(_internalConfig), Times.Exactly(times));
+                    _internalConfigCreator.Verify(x => x.Create(fileConfig), Times.Exactly(times));
                     return true;
                 }
                 catch(Exception)
@@ -157,10 +160,11 @@ namespace Ocelot.UnitTests.Configuration
 
         private void ThenTheSetterIsCalledAtLeast(FileConfiguration fileConfig, int times)
         {
-            var result = WaitFor(2000).Until(() => {
+            var result = WaitFor(4000).Until(() => {
                 try
                 {
-                    _setter.Verify(x => x.Set(fileConfig), Times.AtLeast(times));
+                    _internalConfigRepo.Verify(x => x.AddOrReplace(_internalConfig), Times.AtLeast(times));
+                    _internalConfigCreator.Verify(x => x.Create(fileConfig), Times.AtLeast(times));
                     return true;
                 }
                 catch(Exception)
@@ -170,5 +174,10 @@ namespace Ocelot.UnitTests.Configuration
             });
             result.ShouldBeTrue();
         }
+
+        public void Dispose()
+        {
+            _poller.Dispose();
+        }
     }
 }