cyberpresidentvanellope/tgstation-server
1
0
Fork 0
mirror of https://github.com/tgstation/tgstation-server.git synced 2026-08-30 00:22:40 +01:00
Code Issues Packages Projects Releases Wiki Activity

Got rid of that useless swarm registration abstraction AND fixed race condition in SwarmRpcMapper calls

Browse Source
This commit is contained in:
Dominion
2023-04-22 17:16:34 -04:00
parent b301b7f840
commit d0ac8f5000
7 changed files with 64 additions and 126 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/Tgstation.Server.Host/Controllers/IRequestSwarmRegistrationParser.cs
-19
View File
@@ -1,19 +0,0 @@
using System;
using Microsoft.AspNetCore.Http;
namespace Tgstation.Server.Host.Controllers
{
/// <summary>
/// Parses the swarm registration header from a <see cref="HttpRequest"/>.
/// </summary>
public interface IRequestSwarmRegistrationParser
{
/// <summary>
/// Gets the swarm registration <see cref="Guid"/> from the headers of a given <paramref name="request"/>.
/// </summary>
/// <param name="request">The <see cref="HttpRequest"/>, must contain a valid <see cref="Swarm.SwarmConstants.RegistrationIdHeader"/>.</param>
/// <returns>The parsed registration ID.</returns>
Guid GetRequestRegistrationId(HttpRequest request);
}
}
src/Tgstation.Server.Host/Controllers/RequestSwarmRegistrationParser.cs
-22
View File
@@ -1,22 +0,0 @@
using System;
using System.Linq;
using Microsoft.AspNetCore.Http;
using Tgstation.Server.Host.Swarm;
namespace Tgstation.Server.Host.Controllers
{
/// <inheritdoc />
sealed class RequestSwarmRegistrationParser : IRequestSwarmRegistrationParser
{
/// <inheritdoc />
public Guid GetRequestRegistrationId(HttpRequest request)
{
if (request == null)
throw new ArgumentNullException(nameof(request));
return Guid.Parse(request.Headers[SwarmConstants.RegistrationIdHeader].First());
}
}
}
src/Tgstation.Server.Host/Controllers/SwarmController.cs
+2 -9
View File
@@ -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
/// <summary>
/// Get the current registration <see cref="Guid"/> from the <see cref="ControllerBase.Request"/>.
/// </summary>
internal Guid RequestRegistrationId => requestRegistrationParser.GetRequestRegistrationId(Request);
internal Guid RequestRegistrationId => Guid.Parse(Request.Headers[SwarmConstants.RegistrationIdHeader].First());
/// <summary>
/// The <see cref="ISwarmOperations"/> for the <see cref="SwarmController"/>.
/// </summary>
readonly ISwarmOperations swarmOperations;
/// <summary>
/// The <see cref="IRequestSwarmRegistrationParser"/> for the <see cref="SwarmController"/>.
/// </summary>
readonly IRequestSwarmRegistrationParser requestRegistrationParser;
/// <summary>
/// The <see cref="IAssemblyInformationProvider"/> for the <see cref="SwarmController"/>.
/// </summary>
@@ -60,19 +56,16 @@ namespace Tgstation.Server.Host.Controllers
/// Initializes a new instance of the <see cref="SwarmController"/> class.
/// </summary>
/// <param name="swarmOperations">The value of <see cref="swarmOperations"/>.</param>
/// <param name="requestRegistrationParser">The value of <see cref="requestRegistrationParser"/>.</param>
/// <param name="assemblyInformationProvider">The value of <see cref="assemblyInformationProvider"/>.</param>
/// <param name="swarmConfigurationOptions">The <see cref="IOptions{TOptions}"/> containing the value of <see cref="swarmConfiguration"/>.</param>
/// <param name="logger">The value of <see cref="logger"/>.</param>
public SwarmController(
ISwarmOperations swarmOperations,
IRequestSwarmRegistrationParser requestRegistrationParser,
IAssemblyInformationProvider assemblyInformationProvider,
IOptions<SwarmConfiguration> swarmConfigurationOptions,
ILogger<SwarmController> 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;
src/Tgstation.Server.Host/Core/Application.cs
-1
View File
@@ -367,7 +367,6 @@ namespace Tgstation.Server.Host.Core
services.AddSingleton<IServerPortProvider, ServerPortProivder>();
services.AddSingleton<ITopicClientFactory, TopicClientFactory>();
services.AddSingleton<IGitHubClientFactory, GitHubClientFactory>();
services.AddSingleton<IRequestSwarmRegistrationParser, RequestSwarmRegistrationParser>();
// configure root services
services.AddSingleton<IJobManager, JobManager>();
tests/Tgstation.Server.Host.Tests/Controllers/TestRequestSwarmRegistrationParser.cs
-18
View File
@@ -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<ArgumentNullException>(() => parser.GetRequestRegistrationId(null));
}
}
}
tests/Tgstation.Server.Host.Tests/Swarm/SwarmRpcMapper.cs
+54 -47
View File
@@ -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<Guid> incomingRegistrationIds = new();
readonly Func<SwarmService, SwarmController> createSwarmController;
List<(SwarmConfiguration, TestableSwarmNode)> configToNodes;
int serverErrorCount;
public SwarmRpcMapper(Mock<IHttpClient> clientMock, ILogger logger)
public SwarmRpcMapper(Func<SwarmService, SwarmController> createSwarmController, Mock<IHttpClient> clientMock, ILogger logger)
{
this.createSwarmController = createSwarmController;
clientMock
.Setup(x => x.SendAsync(It.IsNotNull<HttpRequestMessage>(), It.IsAny<CancellationToken>()))
.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<HttpRequest>();
mockRequest.SetupGet(x => x.Headers).Returns(new HeaderDictionary
{
node.RpcMapper.incomingRegistrationIds.Push(Guid.Parse(values.First()));
var args = new List<object>();
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<HttpContext>();
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<IActionResult>), controllerMethod.ReturnType);
args.Add(cancellationToken);
var invocationTask = (Task<IActionResult>)controllerMethod.Invoke(controller, args.ToArray());
result = await invocationTask;
}
else
{
result = (IActionResult)controllerMethod.Invoke(controller, args.ToArray());
var args = new List<object>();
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<IActionResult>), controllerMethod.ReturnType);
args.Add(cancellationToken);
var invocationTask = (Task<IActionResult>)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;
}
}
}
tests/Tgstation.Server.Host.Tests/Swarm/TestableSwarmNode.cs
+8 -10
View File
@@ -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<IServerUpdater>();
RpcMapper = new SwarmRpcMapper(mockHttpClient, loggerFactory.CreateLogger($"SwarmRpcMapper-{swarmConfiguration.Identifier}"));
RpcMapper = new SwarmRpcMapper(
targetService => new SwarmController(
targetService,
mockAssemblyInformationProvider.Object,
mockOptions.Object,
loggerFactory.CreateLogger<SwarmController>()),
mockHttpClient,
loggerFactory.CreateLogger($"SwarmRpcMapper-{swarmConfiguration.Identifier}"));
mockServerUpdater
.Setup(x => x.BeginUpdate(It.IsNotNull<SwarmService>(), It.IsNotNull<Version>(), It.IsAny<CancellationToken>()))
@@ -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<SwarmController>());
}
RecreateControllerAndService();