Feature/fix #156 (#160)

* change config creator to not throw exception in there is an error......lord i hate this config creator code I need to sort it out. * Remove method that we are not using anymore.. * throw exception and add errors to message * train hacking and some refactoring * bs test for code coverage * actually return the errors in the exception
ThreeMammals · Nov 24, 2017 · 3b27bb3 · 3b27bb3
1 parent 6289992
commit 3b27bb3
Show file tree

Hide file tree

Showing 7 changed files with 227 additions and 85 deletions.
diff --git a/src/Ocelot/Configuration/Creator/FileOcelotConfigurationCreator.cs b/src/Ocelot/Configuration/Creator/FileOcelotConfigurationCreator.cs
@@ -66,35 +66,20 @@ IHttpHandlerOptionsCreator httpHandlerOptionsCreator
             _fileReRouteOptionsCreator = fileReRouteOptionsCreator;
             _httpHandlerOptionsCreator = httpHandlerOptionsCreator;
         }
-
-        public async Task<Response<IOcelotConfiguration>> Create()
-        {     
-            var config = await SetUpConfiguration(_options.Value);
-
-            return new OkResponse<IOcelotConfiguration>(config);
-        }
-
+
         public async Task<Response<IOcelotConfiguration>> Create(FileConfiguration fileConfiguration)
         {     
             var config = await SetUpConfiguration(fileConfiguration);
-
-            return new OkResponse<IOcelotConfiguration>(config);
+            return config;
         }
 
-        private async Task<IOcelotConfiguration> SetUpConfiguration(FileConfiguration fileConfiguration)
+        private async Task<Response<IOcelotConfiguration>> SetUpConfiguration(FileConfiguration fileConfiguration)
         {
             var response = await _configurationValidator.IsValid(fileConfiguration);
 
             if (response.Data.IsError)
             {
-                var errorBuilder = new StringBuilder();
-
-                foreach (var error in response.Errors)
-                {
-                    errorBuilder.AppendLine(error.Message);
-                }
-
-                throw new Exception($"Unable to start Ocelot..configuration, errors were {errorBuilder}");
+                return new ErrorResponse<IOcelotConfiguration>(response.Data.Errors);
             }
 
             var reRoutes = new List<ReRoute>();
@@ -107,7 +92,9 @@ private async Task<IOcelotConfiguration> SetUpConfiguration(FileConfiguration fi
 
             var serviceProviderConfiguration = _serviceProviderConfigCreator.Create(fileConfiguration.GlobalConfiguration);
 
-            return new OcelotConfiguration(reRoutes, fileConfiguration.GlobalConfiguration.AdministrationPath, serviceProviderConfiguration);
+            var config = new OcelotConfiguration(reRoutes, fileConfiguration.GlobalConfiguration.AdministrationPath, serviceProviderConfiguration);
+
+            return new OkResponse<IOcelotConfiguration>(config);
         }
 
         private ReRoute SetUpReRoute(FileReRoute fileReRoute, FileGlobalConfiguration globalConfiguration)

diff --git a/src/Ocelot/Configuration/Creator/IOcelotConfigurationCreator.cs b/src/Ocelot/Configuration/Creator/IOcelotConfigurationCreator.cs
@@ -6,7 +6,6 @@ namespace Ocelot.Configuration.Creator
 {
     public interface IOcelotConfigurationCreator
     {
-        Task<Response<IOcelotConfiguration>> Create();
         Task<Response<IOcelotConfiguration>> Create(FileConfiguration fileConfiguration);
     }
 }
diff --git a/src/Ocelot/Errors/Error.cs b/src/Ocelot/Errors/Error.cs
@@ -10,5 +10,10 @@ protected Error(string message, OcelotErrorCode code)
 
         public string Message { get; private set; }
         public OcelotErrorCode Code { get; private set; }
+
+        public override string ToString()
+        {
+            return Message;
+        }
     }
 }
diff --git a/src/Ocelot/Middleware/OcelotMiddlewareExtensions.cs b/src/Ocelot/Middleware/OcelotMiddlewareExtensions.cs
@@ -23,6 +23,7 @@
 namespace Ocelot.Middleware
 {
     using System;
+    using System.Linq;
     using System.Threading.Tasks;
     using Authorisation.Middleware;
     using Microsoft.AspNetCore.Hosting;
@@ -150,61 +151,108 @@ public static async Task<IApplicationBuilder> UseOcelot(this IApplicationBuilder
 
         private static async Task<IOcelotConfiguration> CreateConfiguration(IApplicationBuilder builder)
         {
-            var fileConfig = (IOptions<FileConfiguration>)builder.ApplicationServices.GetService(typeof(IOptions<FileConfiguration>));
-
-            var configSetter = (IFileConfigurationSetter)builder.ApplicationServices.GetService(typeof(IFileConfigurationSetter));
-
-            var configProvider = (IOcelotConfigurationProvider)builder.ApplicationServices.GetService(typeof(IOcelotConfigurationProvider));
+            var deps = GetDependencies(builder);
 
-            var ocelotConfiguration = await configProvider.Get();
+            var ocelotConfiguration = await deps.provider.Get();
 
-            if (ocelotConfiguration == null || ocelotConfiguration.Data == null || ocelotConfiguration.IsError)
+            if (ConfigurationNotSetUp(ocelotConfiguration))
             {
-                Response config = null;
-                var fileConfigRepo = builder.ApplicationServices.GetService(typeof(IFileConfigurationRepository));
-                if (fileConfigRepo.GetType() == typeof(ConsulFileConfigurationRepository))
-                {
-                    var consulFileConfigRepo = (ConsulFileConfigurationRepository) fileConfigRepo;
-                    var ocelotConfigurationRepository =
-                        (IOcelotConfigurationRepository) builder.ApplicationServices.GetService(
-                            typeof(IOcelotConfigurationRepository));
-                    var ocelotConfigurationCreator =
-                        (IOcelotConfigurationCreator) builder.ApplicationServices.GetService(
-                            typeof(IOcelotConfigurationCreator));
-
-                    var fileConfigFromConsul = await consulFileConfigRepo.Get();
-                    if (fileConfigFromConsul.Data == null)
-                    {
-                        config = await configSetter.Set(fileConfig.Value);
-                    }
-                    else
-                    {
-                        var ocelotConfig = await ocelotConfigurationCreator.Create(fileConfigFromConsul.Data);
-                        config = await ocelotConfigurationRepository.AddOrReplace(ocelotConfig.Data);
-                        var hack = builder.ApplicationServices.GetService(typeof(ConsulFileConfigurationPoller));
-                    }
-                }
-                else
+                var response = await SetConfig(builder, deps.fileConfiguration, deps.setter, deps.provider, deps.repo);
+
+                if (UnableToSetConfig(response))
                 {
-                    config = await configSetter.Set(fileConfig.Value);
+                    ThrowToStopOcelotStarting(response);
                 }
+            }
 
-                if (config == null || config.IsError)
-                {
-                    throw new Exception("Unable to start Ocelot: configuration was not set up correctly.");
-                }
+            return await GetOcelotConfigAndReturn(deps.provider);
+        }
+
+        private static async Task<Response> SetConfig(IApplicationBuilder builder, IOptions<FileConfiguration> fileConfiguration, IFileConfigurationSetter setter, IOcelotConfigurationProvider provider, IFileConfigurationRepository repo)
+        {
+            if (UsingConsul(repo))
+            {
+                return await SetUpConfigFromConsul(builder, repo, setter, fileConfiguration);
             }
+
+            return await setter.Set(fileConfiguration.Value);
+        }
+
+        private static bool UnableToSetConfig(Response response)
+        {
+            return response == null || response.IsError;
+        }
+
+        private static bool ConfigurationNotSetUp(Response<IOcelotConfiguration> ocelotConfiguration)
+        {
+            return ocelotConfiguration == null || ocelotConfiguration.Data == null || ocelotConfiguration.IsError;
+        }
+
+        private static (IOptions<FileConfiguration> fileConfiguration, IFileConfigurationSetter setter, IOcelotConfigurationProvider provider, IFileConfigurationRepository repo) GetDependencies(IApplicationBuilder builder)
+        {
+            var fileConfiguration = (IOptions<FileConfiguration>)builder.ApplicationServices.GetService(typeof(IOptions<FileConfiguration>));
+
+            var setter = (IFileConfigurationSetter)builder.ApplicationServices.GetService(typeof(IFileConfigurationSetter));
+
+            var provider = (IOcelotConfigurationProvider)builder.ApplicationServices.GetService(typeof(IOcelotConfigurationProvider));
 
-            ocelotConfiguration = await configProvider.Get();
+            var repo = (IFileConfigurationRepository)builder.ApplicationServices.GetService(typeof(IFileConfigurationRepository));
+
+            return (fileConfiguration, setter, provider, repo);
+        }
+
+        private static async Task<IOcelotConfiguration> GetOcelotConfigAndReturn(IOcelotConfigurationProvider provider)
+        {
+            var ocelotConfiguration = await provider.Get();
 
             if(ocelotConfiguration == null || ocelotConfiguration.Data == null || ocelotConfiguration.IsError)
             {
-                throw new Exception("Unable to start Ocelot: ocelot configuration was not returned by provider.");
+                ThrowToStopOcelotStarting(ocelotConfiguration);
             }
 
             return ocelotConfiguration.Data;
         }
 
+        private static void ThrowToStopOcelotStarting(Response config)
+        {
+            throw new Exception($"Unable to start Ocelot, errors are: {string.Join(",", config.Errors.Select(x => x.ToString()))}");
+        }
+
+        private static bool UsingConsul(IFileConfigurationRepository fileConfigRepo)
+        {
+            return fileConfigRepo.GetType() == typeof(ConsulFileConfigurationRepository);
+        }
+
+        private static async Task<Response> SetUpConfigFromConsul(IApplicationBuilder builder, IFileConfigurationRepository consulFileConfigRepo, IFileConfigurationSetter setter, IOptions<FileConfiguration> fileConfig)
+        {
+            Response config = null;
+
+            var ocelotConfigurationRepository =
+                (IOcelotConfigurationRepository) builder.ApplicationServices.GetService(
+                    typeof(IOcelotConfigurationRepository));
+            var ocelotConfigurationCreator =
+                (IOcelotConfigurationCreator) builder.ApplicationServices.GetService(
+                    typeof(IOcelotConfigurationCreator));
+
+            var fileConfigFromConsul = await consulFileConfigRepo.Get();
+            if (fileConfigFromConsul.Data == null)
+            {
+                config = await setter.Set(fileConfig.Value);
+            }
+            else
+            {
+                var ocelotConfig = await ocelotConfigurationCreator.Create(fileConfigFromConsul.Data);
+                if(ocelotConfig.IsError)
+                {
+                    return new ErrorResponse(ocelotConfig.Errors);
+                }
+                config = await ocelotConfigurationRepository.AddOrReplace(ocelotConfig.Data);
+                var hack = builder.ApplicationServices.GetService(typeof(ConsulFileConfigurationPoller));
+            }
+
+            return new OkResponse();
+        }
+
         private static async Task CreateAdministrationArea(IApplicationBuilder builder, IOcelotConfiguration configuration)
         {
             var identityServerConfiguration = (IIdentityServerConfiguration)builder.ApplicationServices.GetService(typeof(IIdentityServerConfiguration));

diff --git a/test/Ocelot.AcceptanceTests/CannotStartOcelotTests.cs b/test/Ocelot.AcceptanceTests/CannotStartOcelotTests.cs
@@ -0,0 +1,58 @@
+
+using System;
+using System.Collections.Generic;
+using Microsoft.AspNetCore.Hosting;
+using Ocelot.Configuration.File;
+using Shouldly;
+using TestStack.BDDfy;
+using Xunit;
+
+namespace Ocelot.AcceptanceTests
+{
+    public class CannotStartOcelotTests : IDisposable
+    {
+        private IWebHost _builder;
+        private readonly Steps _steps;
+        private string _downstreamPath;
+
+        public CannotStartOcelotTests()
+        {
+            _steps = new Steps();
+        }
+
+        [Fact]
+        public void should_throw_exception_if_cannot_start()
+        {
+            var invalidConfig = new FileConfiguration()
+            {
+                ReRoutes = new List<FileReRoute>
+                {
+                    new FileReRoute
+                    {
+                        UpstreamPathTemplate = "api",
+                        DownstreamPathTemplate = "test"
+                    }
+                }
+            };
+
+            Exception exception = null;
+            _steps.GivenThereIsAConfiguration(invalidConfig);
+            try
+            {
+                _steps.GivenOcelotIsRunning();
+            }
+            catch(Exception ex)
+            {
+                exception = ex;
+            }
+
+            exception.ShouldNotBeNull();
+        }
+
+        public void Dispose()
+        {
+            _builder?.Dispose();
+            _steps.Dispose();
+        }
+    }
+}