diff --git a/src/Tgstation.Server.Host/Controllers/IRequestSwarmRegistrationParser.cs b/src/Tgstation.Server.Host/Controllers/IRequestSwarmRegistrationParser.cs deleted file mode 100644 index a91734f1a2..0000000000 --- a/src/Tgstation.Server.Host/Controllers/IRequestSwarmRegistrationParser.cs +++ /dev/null @@ -1,19 +0,0 @@ -using System; - -using Microsoft.AspNetCore.Http; - -namespace Tgstation.Server.Host.Controllers -{ - /// - /// Parses the swarm registration header from a . - /// - public interface IRequestSwarmRegistrationParser - { - /// - /// Gets the swarm registration from the headers of a given . - /// - /// The , must contain a valid . - /// The parsed registration ID. - Guid GetRequestRegistrationId(HttpRequest request); - } -} diff --git a/src/Tgstation.Server.Host/Controllers/RequestSwarmRegistrationParser.cs b/src/Tgstation.Server.Host/Controllers/RequestSwarmRegistrationParser.cs deleted file mode 100644 index 5e15059d3b..0000000000 --- a/src/Tgstation.Server.Host/Controllers/RequestSwarmRegistrationParser.cs +++ /dev/null @@ -1,22 +0,0 @@ -using System; -using System.Linq; - -using Microsoft.AspNetCore.Http; - -using Tgstation.Server.Host.Swarm; - -namespace Tgstation.Server.Host.Controllers -{ - /// - sealed class RequestSwarmRegistrationParser : IRequestSwarmRegistrationParser - { - /// - public Guid GetRequestRegistrationId(HttpRequest request) - { - if (request == null) - throw new ArgumentNullException(nameof(request)); - - return Guid.Parse(request.Headers[SwarmConstants.RegistrationIdHeader].First()); - } - } -} diff --git a/src/Tgstation.Server.Host/Controllers/SwarmController.cs b/src/Tgstation.Server.Host/Controllers/SwarmController.cs index c339a45a67..abd329de50 100644 --- a/src/Tgstation.Server.Host/Controllers/SwarmController.cs +++ b/src/Tgstation.Server.Host/Controllers/SwarmController.cs @@ -9,6 +9,7 @@ using Microsoft.AspNetCore.Mvc; using Microsoft.AspNetCore.Mvc.Filters; using Microsoft.Extensions.Logging; using Microsoft.Extensions.Options; + using Serilog.Context; using Tgstation.Server.Host.Configuration; @@ -29,18 +30,13 @@ namespace Tgstation.Server.Host.Controllers /// /// Get the current registration from the . /// - internal Guid RequestRegistrationId => requestRegistrationParser.GetRequestRegistrationId(Request); + internal Guid RequestRegistrationId => Guid.Parse(Request.Headers[SwarmConstants.RegistrationIdHeader].First()); /// /// The for the . /// readonly ISwarmOperations swarmOperations; - /// - /// The for the . - /// - readonly IRequestSwarmRegistrationParser requestRegistrationParser; - /// /// The for the . /// @@ -60,19 +56,16 @@ namespace Tgstation.Server.Host.Controllers /// Initializes a new instance of the class. /// /// The value of . - /// The value of . /// The value of . /// The containing the value of . /// The value of . public SwarmController( ISwarmOperations swarmOperations, - IRequestSwarmRegistrationParser requestRegistrationParser, IAssemblyInformationProvider assemblyInformationProvider, IOptions swarmConfigurationOptions, ILogger logger) { this.swarmOperations = swarmOperations ?? throw new ArgumentNullException(nameof(swarmOperations)); - this.requestRegistrationParser = requestRegistrationParser ?? throw new ArgumentNullException(nameof(requestRegistrationParser)); this.assemblyInformationProvider = assemblyInformationProvider ?? throw new ArgumentNullException(nameof(assemblyInformationProvider)); swarmConfiguration = swarmConfigurationOptions?.Value ?? throw new ArgumentNullException(nameof(swarmConfigurationOptions)); this.logger = logger; diff --git a/src/Tgstation.Server.Host/Core/Application.cs b/src/Tgstation.Server.Host/Core/Application.cs index 8a24863112..eb88be12f7 100644 --- a/src/Tgstation.Server.Host/Core/Application.cs +++ b/src/Tgstation.Server.Host/Core/Application.cs @@ -367,7 +367,6 @@ namespace Tgstation.Server.Host.Core services.AddSingleton(); services.AddSingleton(); services.AddSingleton(); - services.AddSingleton(); // configure root services services.AddSingleton(); diff --git a/tests/Tgstation.Server.Host.Tests/Controllers/TestRequestSwarmRegistrationParser.cs b/tests/Tgstation.Server.Host.Tests/Controllers/TestRequestSwarmRegistrationParser.cs deleted file mode 100644 index 5436058b6a..0000000000 --- a/tests/Tgstation.Server.Host.Tests/Controllers/TestRequestSwarmRegistrationParser.cs +++ /dev/null @@ -1,18 +0,0 @@ -using System; - -using Microsoft.VisualStudio.TestTools.UnitTesting; - -namespace Tgstation.Server.Host.Controllers.Tests -{ - [TestClass] - public sealed class TestRequestSwarmRegistrationParser - { - [TestMethod] - public void TestConstructorThrows() - { - var parser = new RequestSwarmRegistrationParser(); - - Assert.ThrowsException(() => parser.GetRequestRegistrationId(null)); - } - } -} diff --git a/tests/Tgstation.Server.Host.Tests/Swarm/SwarmRpcMapper.cs b/tests/Tgstation.Server.Host.Tests/Swarm/SwarmRpcMapper.cs index 8eb8d9a249..f736a7ace6 100644 --- a/tests/Tgstation.Server.Host.Tests/Swarm/SwarmRpcMapper.cs +++ b/tests/Tgstation.Server.Host.Tests/Swarm/SwarmRpcMapper.cs @@ -12,6 +12,7 @@ using Microsoft.AspNetCore.Mvc; using Microsoft.AspNetCore.Mvc.Infrastructure; using Microsoft.AspNetCore.Mvc.Routing; using Microsoft.Extensions.Logging; +using Microsoft.Extensions.Primitives; using Microsoft.VisualStudio.TestTools.UnitTesting; using Moq; @@ -24,20 +25,21 @@ using Tgstation.Server.Host.Controllers; namespace Tgstation.Server.Host.Swarm.Tests { - sealed class SwarmRpcMapper : IRequestSwarmRegistrationParser, IDisposable + sealed class SwarmRpcMapper : IDisposable { public bool AsyncRequests { get; set; } readonly ILogger logger; - readonly Stack incomingRegistrationIds = new(); + readonly Func createSwarmController; List<(SwarmConfiguration, TestableSwarmNode)> configToNodes; int serverErrorCount; - public SwarmRpcMapper(Mock clientMock, ILogger logger) + public SwarmRpcMapper(Func createSwarmController, Mock clientMock, ILogger logger) { + this.createSwarmController = createSwarmController; clientMock .Setup(x => x.SendAsync(It.IsNotNull(), It.IsAny())) .Returns(MapRequest); @@ -50,8 +52,6 @@ namespace Tgstation.Server.Host.Swarm.Tests Assert.AreEqual(0, serverErrorCount); } - public Guid GetRequestRegistrationId(HttpRequest request) => incomingRegistrationIds.Peek(); - public void Register(List<(SwarmConfiguration, TestableSwarmNode)> configToNodes) { this.configToNodes = configToNodes; @@ -77,7 +77,7 @@ namespace Tgstation.Server.Host.Swarm.Tests throw new HttpRequestException("Can't connect to shutdown node!"); } - var controller = node.Controller; + var controller = createSwarmController(node.Service); Type targetAttribute = null; bool isDataRequest = false; @@ -128,58 +128,59 @@ namespace Tgstation.Server.Host.Swarm.Tests Assert.Fail($"SwarmController has no method with attribute {targetAttribute}!"); IActionResult result; - var hasRegistrationHeader = request.Headers.TryGetValues(SwarmConstants.RegistrationIdHeader, out var values) && values.Count() == 1; + var hasRegistrationHeader = request.Headers.TryGetValues(SwarmConstants.RegistrationIdHeader, out var values) + && values.Count() == 1; + var response = new HttpResponseMessage(); try { - var response = new HttpResponseMessage(); - try + // We're not testing OnActionExecutingAsync, that's covered by integration. + if (hasRegistrationHeader) { - // We're not testing OnActionExecutingAsync, that's covered by integration. - if (hasRegistrationHeader) + var mockRequest = new Mock(); + mockRequest.SetupGet(x => x.Headers).Returns(new HeaderDictionary { - node.RpcMapper.incomingRegistrationIds.Push(Guid.Parse(values.First())); - var args = new List(); - if (isDataRequest && request.Content != null) { - var dataType = controllerMethod.GetParameters().First().ParameterType; - var json = await request.Content.ReadAsStringAsync(cancellationToken); - var parameter = JsonConvert.DeserializeObject(json, dataType, SwarmService.SerializerSettings); - args.Add(parameter); - } + SwarmConstants.RegistrationIdHeader, + new StringValues(values.First()) + }, + }); + var mockHttpContext = new Mock(); + mockHttpContext.SetupGet(x => x.Request).Returns(mockRequest.Object); - if (AsyncRequests) - await Task.Yield(); + controller + .ControllerContext + .HttpContext = mockHttpContext.Object; - if (controllerMethod.ReturnType != typeof(IActionResult)) - { - Assert.AreEqual(typeof(Task), controllerMethod.ReturnType); - args.Add(cancellationToken); - var invocationTask = (Task)controllerMethod.Invoke(controller, args.ToArray()); - result = await invocationTask; - } - else - { - result = (IActionResult)controllerMethod.Invoke(controller, args.ToArray()); + var args = new List(); + if (isDataRequest && request.Content != null) + { + var dataType = controllerMethod.GetParameters().First().ParameterType; + var json = await request.Content.ReadAsStringAsync(cancellationToken); + var parameter = JsonConvert.DeserializeObject(json, dataType, SwarmService.SerializerSettings); + args.Add(parameter); + } - // simulate worst case, request completed but was aborted before server replied - cancellationToken.ThrowIfCancellationRequested(); - } + if (AsyncRequests) + await Task.Yield(); + + if (controllerMethod.ReturnType != typeof(IActionResult)) + { + Assert.AreEqual(typeof(Task), controllerMethod.ReturnType); + args.Add(cancellationToken); + var invocationTask = (Task)controllerMethod.Invoke(controller, args.ToArray()); + result = await invocationTask; } else { - result = controller.BadRequest(); + result = (IActionResult)controllerMethod.Invoke(controller, args.ToArray()); + + // simulate worst case, request completed but was aborted before server replied + cancellationToken.ThrowIfCancellationRequested(); } } - catch (Exception ex) + else { - if (ex is not OperationCanceledException) - { - logger.LogCritical(ex, "Error in request to {nodeId}!", config.Identifier); - ++serverErrorCount; - } - - response.Dispose(); - throw; + result = controller.BadRequest(); } // manually checked all controller response types @@ -196,10 +197,16 @@ namespace Tgstation.Server.Host.Swarm.Tests return response; } - finally + catch (Exception ex) { - if (hasRegistrationHeader) - node.RpcMapper.incomingRegistrationIds.Pop(); + if (ex is not OperationCanceledException) + { + logger.LogCritical(ex, "Error in request to {nodeId}!", config.Identifier); + ++serverErrorCount; + } + + response.Dispose(); + throw; } } } diff --git a/tests/Tgstation.Server.Host.Tests/Swarm/TestableSwarmNode.cs b/tests/Tgstation.Server.Host.Tests/Swarm/TestableSwarmNode.cs index 7e8b728f34..828675fbe0 100644 --- a/tests/Tgstation.Server.Host.Tests/Swarm/TestableSwarmNode.cs +++ b/tests/Tgstation.Server.Host.Tests/Swarm/TestableSwarmNode.cs @@ -22,8 +22,6 @@ namespace Tgstation.Server.Host.Swarm.Tests { sealed class TestableSwarmNode : IAsyncDisposable { - public SwarmController Controller { get; private set; } - public SwarmService Service { get; private set; } public SwarmConfiguration Config { get; } @@ -106,7 +104,14 @@ namespace Tgstation.Server.Host.Swarm.Tests var mockServerUpdater = new Mock(); - RpcMapper = new SwarmRpcMapper(mockHttpClient, loggerFactory.CreateLogger($"SwarmRpcMapper-{swarmConfiguration.Identifier}")); + RpcMapper = new SwarmRpcMapper( + targetService => new SwarmController( + targetService, + mockAssemblyInformationProvider.Object, + mockOptions.Object, + loggerFactory.CreateLogger()), + mockHttpClient, + loggerFactory.CreateLogger($"SwarmRpcMapper-{swarmConfiguration.Identifier}")); mockServerUpdater .Setup(x => x.BeginUpdate(It.IsNotNull(), It.IsNotNull(), It.IsAny())) @@ -144,13 +149,6 @@ namespace Tgstation.Server.Host.Swarm.Tests mockAsyncDelayer.Object, mockOptions.Object, serviceLogger); - - Controller = new SwarmController( - Service, - RpcMapper, - mockAssemblyInformationProvider.Object, - mockOptions.Object, - loggerFactory.CreateLogger()); } RecreateControllerAndService();