From 5888044c288acdab2ab80539417c3db83c6bb325 Mon Sep 17 00:00:00 2001 From: gardusig Date: Thu, 15 Dec 2022 13:52:54 -0300 Subject: [PATCH 1/6] Dependency injection fix attempt --- .../DependencyInjectionExtensions.cs | 37 +++++++++---------- Conductor/Client/OrkesApiClient.cs | 12 +++--- Conductor/conductor-csharp.csproj | 1 + Tests/Util/ApiUtil.cs | 28 ++++---------- Tests/Worker/WorkerTests.cs | 6 +-- 5 files changed, 35 insertions(+), 49 deletions(-) diff --git a/Conductor/Client/Extensions/DependencyInjectionExtensions.cs b/Conductor/Client/Extensions/DependencyInjectionExtensions.cs index bd0ada2a..37fbceb7 100644 --- a/Conductor/Client/Extensions/DependencyInjectionExtensions.cs +++ b/Conductor/Client/Extensions/DependencyInjectionExtensions.cs @@ -9,41 +9,31 @@ namespace Conductor.Client.Extensions { public static class DependencyInjectionExtensions { - public static IServiceCollection WithConductorWorker(this IServiceCollection services) where T : IWorkflowTask + public static IServiceCollection AddConductorWorkflowTask(this IServiceCollection services) where T : IWorkflowTask { services.AddTransient(typeof(IWorkflowTask), typeof(T)); services.AddTransient(typeof(T)); return services; } - public static IServiceCollection WithOrkesApiClient(this IServiceCollection services, OrkesApiClient orkesApiClient) + public static IServiceCollection AddConductorWorker(this IServiceCollection services, Configuration configuration = null, Action configureHttpClient = null) { services.AddHttpClient(); services.AddOptions(); - services.AddSingleton(orkesApiClient); - services.AddTransient(); + services.AddSingleton(configuration != null ? configuration : new Configuration()); + services.AddSingleton(new OrkesApiClient(configuration)); services.AddSingleton(); + services.AddTransient(); services.AddTransient(); - services.AddSingleton(orkesApiClient); - return services; - } - - public static IServiceCollection WithHostedService(this IServiceCollection services) where T : BackgroundService - { - services.AddHostedService(); - return services; + return services.AddConductorClient(configureHttpClient); } - public static IServiceCollection WithConfiguration(this IServiceCollection services, Configuration configuration) + public static IServiceCollection AddConductorClient(this IServiceCollection services, Func serverUrl) { - if (configuration == null) - { - services.AddSingleton(); - } - else + services.AddHttpClient((provider, client) => { - services.AddSingleton(configuration); - } + client.BaseAddress = new Uri(serverUrl(provider)); + }); return services; } @@ -59,5 +49,12 @@ public static IServiceCollection AddConductorClient(this IServiceCollection serv } return services; } + + public static IServiceCollection WithHostedService(this IServiceCollection services) where T : BackgroundService + { + services.AddHostedService(); + return services; + } + } } diff --git a/Conductor/Client/OrkesApiClient.cs b/Conductor/Client/OrkesApiClient.cs index 7140f03b..a79572cf 100644 --- a/Conductor/Client/OrkesApiClient.cs +++ b/Conductor/Client/OrkesApiClient.cs @@ -10,22 +10,24 @@ public class OrkesApiClient { private Configuration _configuration; private MemoryCache _memoryCache; - - private OrkesAuthenticationSettings _authenticationSettings; private TokenResourceApi _tokenClient; + private OrkesAuthenticationSettings _authenticationSettings; private OrkesApiClient() { _memoryCache = new MemoryCache(new MemoryCacheOptions()); _configuration = null; - _authenticationSettings = null; _tokenClient = null; + _authenticationSettings = null; } - public OrkesApiClient(Configuration configuration = null, OrkesAuthenticationSettings authenticationSettings = null) : this() + public OrkesApiClient(Configuration configuration = null) : this() { - _authenticationSettings = authenticationSettings; _configuration = configuration; + if (_configuration != null && !string.IsNullOrEmpty(_configuration.keyId) && !string.IsNullOrEmpty(_configuration.keySecret)) + { + _authenticationSettings = new OrkesAuthenticationSettings(_configuration.keyId, _configuration.keySecret); + } RefreshAuthenticationHeader(); } diff --git a/Conductor/conductor-csharp.csproj b/Conductor/conductor-csharp.csproj index 39f4a5a5..7c57d162 100644 --- a/Conductor/conductor-csharp.csproj +++ b/Conductor/conductor-csharp.csproj @@ -15,5 +15,6 @@ + \ No newline at end of file diff --git a/Tests/Util/ApiUtil.cs b/Tests/Util/ApiUtil.cs index c2bf4bf8..303f7dda 100644 --- a/Tests/Util/ApiUtil.cs +++ b/Tests/Util/ApiUtil.cs @@ -1,6 +1,5 @@ using Conductor.Api; using Conductor.Client; -using Conductor.Client.Authentication; using Conductor.Executor; using System; using System.Diagnostics; @@ -34,30 +33,17 @@ public static WorkflowExecutor GetWorkflowExecutor() public static T GetClient() where T : IApiAccessor, new() { - OrkesApiClient apiClient = GetApiClient(); + OrkesApiClient apiClient = new OrkesApiClient(GetConfiguration()); return apiClient.GetClient(); } - public static OrkesApiClient GetApiClient() + public static Configuration GetConfiguration() { - return GetApiClient( - basePath: _basePath, - keyId: _keyId, - keySecret: _keySecret - ); - } - - private static OrkesApiClient GetApiClient(string basePath, string keyId, string keySecret) - { - return new OrkesApiClient( - configuration: new Configuration() - { - BasePath = basePath - }, - authenticationSettings: new OrkesAuthenticationSettings( - keyId, keySecret - ) - ); + Configuration configuration = new Configuration(); + configuration.keyId = _keyId; + configuration.keySecret = _keySecret; + configuration.BasePath = _basePath; + return configuration; } private static string GetEnvironmentVariable(string variable) diff --git a/Tests/Worker/WorkerTests.cs b/Tests/Worker/WorkerTests.cs index c47cdb8d..35faf40b 100644 --- a/Tests/Worker/WorkerTests.cs +++ b/Tests/Worker/WorkerTests.cs @@ -38,7 +38,7 @@ public void TestWorkflowAsyncExecution() { ConductorWorkflow workflow = GetConductorWorkflow(); _workflowExecutor.RegisterWorkflow(workflow, true); - GetWorkerHost().RunAsync(); + GetWorkerHost().Run(); List workflowIds = StartWorkflows(workflow); Thread.Sleep(WORKFLOW_EXECUTION_TIMEOUT_SECONDS * 1000); foreach (string workflowId in workflowIds) @@ -83,8 +83,8 @@ private IHost GetWorkerHost() .ConfigureServices( (ctx, services) => { - services.WithOrkesApiClient(ApiUtil.GetApiClient()); - services.WithConductorWorker(); + services.AddConductorWorker(ApiUtil.GetConfiguration()); + services.AddConductorWorkflowTask(); services.WithHostedService(); } ).ConfigureLogging( From 56357f564a78ca482735eee2822352ba2271738e Mon Sep 17 00:00:00 2001 From: Manan Bhatt Date: Thu, 15 Dec 2022 22:41:46 +0530 Subject: [PATCH 2/6] Test fix --- Tests/Worker/WorkerTests.cs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/Tests/Worker/WorkerTests.cs b/Tests/Worker/WorkerTests.cs index 35faf40b..c60f9108 100644 --- a/Tests/Worker/WorkerTests.cs +++ b/Tests/Worker/WorkerTests.cs @@ -1,8 +1,11 @@ using Conductor.Api; +using Conductor.Client; using Conductor.Client.Extensions; +using Conductor.Client.Interfaces; using Conductor.Definition; using Conductor.Definition.TaskType; using Conductor.Executor; +using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Hosting; using Microsoft.Extensions.Logging; using System; @@ -86,6 +89,7 @@ private IHost GetWorkerHost() services.AddConductorWorker(ApiUtil.GetConfiguration()); services.AddConductorWorkflowTask(); services.WithHostedService(); + services.AddTransient(); } ).ConfigureLogging( logging => From cb3de0811f1fd60a4c97a5a60f459551344e1bb8 Mon Sep 17 00:00:00 2001 From: gardusig Date: Thu, 15 Dec 2022 14:18:45 -0300 Subject: [PATCH 3/6] Update worker to run async --- Tests/Worker/WorkerTests.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Tests/Worker/WorkerTests.cs b/Tests/Worker/WorkerTests.cs index c60f9108..32f75040 100644 --- a/Tests/Worker/WorkerTests.cs +++ b/Tests/Worker/WorkerTests.cs @@ -41,7 +41,7 @@ public void TestWorkflowAsyncExecution() { ConductorWorkflow workflow = GetConductorWorkflow(); _workflowExecutor.RegisterWorkflow(workflow, true); - GetWorkerHost().Run(); + GetWorkerHost().RunAsync(); List workflowIds = StartWorkflows(workflow); Thread.Sleep(WORKFLOW_EXECUTION_TIMEOUT_SECONDS * 1000); foreach (string workflowId in workflowIds) From a3bce9162d8106d00c574b8c5ac34cd257684dfb Mon Sep 17 00:00:00 2001 From: gardusig Date: Thu, 15 Dec 2022 14:49:14 -0300 Subject: [PATCH 4/6] Fixed tests --- .../Extensions/DependencyInjectionExtensions.cs | 13 ++++++++++--- .../Client/Worker/ConductorWorkerRestClient.cs | 2 +- Conductor/Client/Worker/WorkflowTaskExecutor.cs | 4 ++-- Tests/Worker/WorkerTests.cs | 1 - 4 files changed, 13 insertions(+), 7 deletions(-) diff --git a/Conductor/Client/Extensions/DependencyInjectionExtensions.cs b/Conductor/Client/Extensions/DependencyInjectionExtensions.cs index 37fbceb7..a28816f7 100644 --- a/Conductor/Client/Extensions/DependencyInjectionExtensions.cs +++ b/Conductor/Client/Extensions/DependencyInjectionExtensions.cs @@ -20,10 +20,17 @@ public static IServiceCollection AddConductorWorker(this IServiceCollection serv { services.AddHttpClient(); services.AddOptions(); - services.AddSingleton(configuration != null ? configuration : new Configuration()); - services.AddSingleton(new OrkesApiClient(configuration)); - services.AddSingleton(); + if (configuration == null) + { + configuration = new Configuration(); + } + services.AddSingleton(configuration); + OrkesApiClient orkesApiClient = new OrkesApiClient(configuration); + services.AddSingleton(orkesApiClient); services.AddTransient(); + ConductorWorkerRestClient conductorWorkerRestClient = new ConductorWorkerRestClient(orkesApiClient); + services.AddSingleton(conductorWorkerRestClient); + services.AddSingleton(); services.AddTransient(); return services.AddConductorClient(configureHttpClient); } diff --git a/Conductor/Client/Worker/ConductorWorkerRestClient.cs b/Conductor/Client/Worker/ConductorWorkerRestClient.cs index 85f54af2..2d7530c8 100644 --- a/Conductor/Client/Worker/ConductorWorkerRestClient.cs +++ b/Conductor/Client/Worker/ConductorWorkerRestClient.cs @@ -1,7 +1,7 @@ +using Conductor.Api; using Conductor.Client.Interfaces; using Conductor.Client.Models; using System.Threading.Tasks; -using Conductor.Api; namespace Conductor.Client { diff --git a/Conductor/Client/Worker/WorkflowTaskExecutor.cs b/Conductor/Client/Worker/WorkflowTaskExecutor.cs index 5b634158..e7c6ddd6 100644 --- a/Conductor/Client/Worker/WorkflowTaskExecutor.cs +++ b/Conductor/Client/Worker/WorkflowTaskExecutor.cs @@ -13,6 +13,7 @@ namespace Conductor.Client.Worker { internal class WorkflowTaskExecutor : IWorkflowTaskExecutor { + private IServiceProvider _serviceProvider; private List workers; private ILogger logger; private readonly Configuration configuration; @@ -24,12 +25,11 @@ internal class WorkflowTaskExecutor : IWorkflowTaskExecutor private readonly string workerId = Environment.MachineName + "_" + new Random(epoch).Next(); public WorkflowTaskExecutor( - IConductorWorkerRestClient taskClient, IServiceProvider serviceProvider, ILogger logger, IOptions configuration) { - this.taskClient = taskClient; + this.taskClient = serviceProvider.GetService(typeof(ConductorWorkerRestClient)) as ConductorWorkerRestClient; this.serviceProvider = serviceProvider; this.logger = logger; this.configuration = configuration.Value; diff --git a/Tests/Worker/WorkerTests.cs b/Tests/Worker/WorkerTests.cs index 32f75040..1c4ac166 100644 --- a/Tests/Worker/WorkerTests.cs +++ b/Tests/Worker/WorkerTests.cs @@ -89,7 +89,6 @@ private IHost GetWorkerHost() services.AddConductorWorker(ApiUtil.GetConfiguration()); services.AddConductorWorkflowTask(); services.WithHostedService(); - services.AddTransient(); } ).ConfigureLogging( logging => From e0174efa5bca689478bc95a1ea7951b06e9e582a Mon Sep 17 00:00:00 2001 From: gardusig Date: Thu, 15 Dec 2022 14:54:29 -0300 Subject: [PATCH 5/6] Removed unnecessary service at task coordinator --- Conductor/Client/Worker/WorkflowTaskCoordinator.cs | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/Conductor/Client/Worker/WorkflowTaskCoordinator.cs b/Conductor/Client/Worker/WorkflowTaskCoordinator.cs index acbcbcab..fdfd8e39 100644 --- a/Conductor/Client/Worker/WorkflowTaskCoordinator.cs +++ b/Conductor/Client/Worker/WorkflowTaskCoordinator.cs @@ -11,14 +11,13 @@ namespace Conductor.Client.Worker internal class WorkflowTaskCoordinator : IWorkflowTaskCoordinator { private int _concurrentWorkers; - private IServiceProvider _serviceProvider; private ILogger _logger; + private IWorkflowTaskExecutor _workflowTaskExecutor; private HashSet _workerDefinitions; private TaskResourceApi _client; public WorkflowTaskCoordinator(IServiceProvider serviceProvider, ILogger logger, OrkesApiClient orkesApiClient, int? concurrentWorkers = null) { - _serviceProvider = serviceProvider; _logger = logger; _workerDefinitions = new HashSet(); if (concurrentWorkers == null) @@ -27,6 +26,7 @@ public WorkflowTaskCoordinator(IServiceProvider serviceProvider, ILogger(); + _workflowTaskExecutor = serviceProvider.GetService(typeof(IWorkflowTaskExecutor)) as IWorkflowTaskExecutor; } public async Task Start() @@ -35,8 +35,7 @@ public async Task Start() var pollers = new List(); for (var i = 0; i < _concurrentWorkers; i++) { - var executor = _serviceProvider.GetService(typeof(IWorkflowTaskExecutor)) as IWorkflowTaskExecutor; - pollers.Add(executor.StartPoller(_workerDefinitions.ToList())); + pollers.Add(_workflowTaskExecutor.StartPoller(_workerDefinitions.ToList())); } await Task.WhenAll(pollers); } From ba125e1f3081434941dfeb447265b086158c41b4 Mon Sep 17 00:00:00 2001 From: Manan Bhatt Date: Thu, 15 Dec 2022 23:43:41 +0530 Subject: [PATCH 6/6] remove unnecessary injection --- Conductor/Client/Extensions/DependencyInjectionExtensions.cs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/Conductor/Client/Extensions/DependencyInjectionExtensions.cs b/Conductor/Client/Extensions/DependencyInjectionExtensions.cs index a28816f7..24a766e4 100644 --- a/Conductor/Client/Extensions/DependencyInjectionExtensions.cs +++ b/Conductor/Client/Extensions/DependencyInjectionExtensions.cs @@ -27,9 +27,7 @@ public static IServiceCollection AddConductorWorker(this IServiceCollection serv services.AddSingleton(configuration); OrkesApiClient orkesApiClient = new OrkesApiClient(configuration); services.AddSingleton(orkesApiClient); - services.AddTransient(); - ConductorWorkerRestClient conductorWorkerRestClient = new ConductorWorkerRestClient(orkesApiClient); - services.AddSingleton(conductorWorkerRestClient); + services.AddSingleton(new ConductorWorkerRestClient(orkesApiClient)); services.AddSingleton(); services.AddTransient(); return services.AddConductorClient(configureHttpClient);