From 6abcfe18f2707e8779ac2fd30fd4d0b5f294a657 Mon Sep 17 00:00:00 2001 From: Jordan Brown Date: Mon, 23 Aug 2021 19:10:14 -0400 Subject: [PATCH] Use IReady event for gateway tests - Fix manual connect invocations --- .../Providers/DiscordForwardingResponder.cs | 3 ++ .../Chat/Providers/DiscordProvider.cs | 44 +++++++++---------- .../Chat/Providers/IDiscordResponders.cs | 2 +- .../Chat/Providers/ProviderFactory.cs | 1 - .../Chat/Providers/TestDiscordProvider.cs | 27 +++--------- 5 files changed, 32 insertions(+), 45 deletions(-) diff --git a/src/Tgstation.Server.Host/Components/Chat/Providers/DiscordForwardingResponder.cs b/src/Tgstation.Server.Host/Components/Chat/Providers/DiscordForwardingResponder.cs index 899380df1b..42bbe937d9 100644 --- a/src/Tgstation.Server.Host/Components/Chat/Providers/DiscordForwardingResponder.cs +++ b/src/Tgstation.Server.Host/Components/Chat/Providers/DiscordForwardingResponder.cs @@ -29,5 +29,8 @@ namespace Tgstation.Server.Host.Components.Chat.Providers /// public Task RespondAsync(IMessageCreate gatewayEvent, CancellationToken ct) => targetResponder.RespondAsync(gatewayEvent, ct); + + /// + public Task RespondAsync(IReady gatewayEvent, CancellationToken ct = default) => targetResponder.RespondAsync(gatewayEvent, ct); } } diff --git a/src/Tgstation.Server.Host/Components/Chat/Providers/DiscordProvider.cs b/src/Tgstation.Server.Host/Components/Chat/Providers/DiscordProvider.cs index f59abbdd18..f73c371c7d 100644 --- a/src/Tgstation.Server.Host/Components/Chat/Providers/DiscordProvider.cs +++ b/src/Tgstation.Server.Host/Components/Chat/Providers/DiscordProvider.cs @@ -2,7 +2,6 @@ using System.Collections.Generic; using System.Drawing; using System.Linq; -using System.Reflection; using System.Threading; using System.Threading.Tasks; @@ -18,7 +17,6 @@ using Remora.Discord.Gateway.Extensions; using Remora.Results; using Tgstation.Server.Api.Models; -using Tgstation.Server.Host.Core; using Tgstation.Server.Host.Jobs; using Tgstation.Server.Host.Models; using Tgstation.Server.Host.System; @@ -50,11 +48,6 @@ namespace Tgstation.Server.Host.Components.Chat.Providers /// readonly IAssemblyInformationProvider assemblyInformationProvider; - /// - /// The for the . - /// - readonly IAsyncDelayer asyncDelayer; - /// /// The containing Discord services. /// @@ -80,6 +73,11 @@ namespace Tgstation.Server.Host.Components.Chat.Providers /// CancellationTokenSource gatewayCts; + /// + /// The for the initial gateway connection event. + /// + TaskCompletionSource gatewayReadyTcs; + /// /// The representing the lifetime of the client. /// @@ -149,19 +147,16 @@ namespace Tgstation.Server.Host.Components.Chat.Providers /// /// The for the . /// The value of . - /// The value of . /// The for the . /// The for the . public DiscordProvider( IJobManager jobManager, IAssemblyInformationProvider assemblyInformationProvider, - IAsyncDelayer asyncDelayer, ILogger logger, ChatBot chatBot) : base(jobManager, logger, chatBot) { this.assemblyInformationProvider = assemblyInformationProvider ?? throw new ArgumentNullException(nameof(assemblyInformationProvider)); - this.asyncDelayer = asyncDelayer ?? throw new ArgumentNullException(nameof(asyncDelayer)); mappedChannels = new List(); @@ -579,6 +574,13 @@ namespace Tgstation.Server.Host.Components.Chat.Providers return Result.FromSuccess(); } + /// + public Task RespondAsync(IReady readyEvent, CancellationToken cancellationToken) + { + gatewayReadyTcs?.TrySetResult(null); + return Task.FromResult(Result.FromSuccess()); + } + /// protected override async Task Connect(CancellationToken cancellationToken) { @@ -592,20 +594,15 @@ namespace Tgstation.Server.Host.Components.Chat.Providers var gatewayCancellationToken = gatewayCts.Token; var gatewayClient = serviceProvider.GetRequiredService(); - // reconnects keep happening until we stop or it faults, our auto-reconnector will handle the latter - var gatewayTaskLocal = gatewayClient.RunAsync(gatewayCancellationToken); - - // HACK: The gateway connection status isn't public, how 'bout we read it anyway? - GatewayConnectionStatus connectionStatus; - var connectionStatusField = gatewayClient.GetType().GetField("_connectionStatus", BindingFlags.NonPublic | BindingFlags.Instance); - do + gatewayReadyTcs = new TaskCompletionSource(); + using (cancellationToken.Register(() => gatewayReadyTcs.TrySetCanceled())) { - await asyncDelayer.Delay(TimeSpan.FromMilliseconds(250), cancellationToken).ConfigureAwait(false); - connectionStatus = (GatewayConnectionStatus)connectionStatusField.GetValue(gatewayClient); - } - while (!gatewayTaskLocal.IsCompleted && connectionStatus != GatewayConnectionStatus.Connected); + // reconnects keep happening until we stop or it faults, our auto-reconnector will handle the latter + gatewayTask = gatewayClient.RunAsync(gatewayCancellationToken); + + await Task.WhenAny(gatewayReadyTcs.Task, gatewayTask).ConfigureAwait(false); + } - gatewayTask = gatewayTaskLocal; if (gatewayTask.IsCompleted) { await DisconnectImpl(cancellationToken).ConfigureAwait(false); @@ -630,7 +627,8 @@ namespace Tgstation.Server.Host.Components.Chat.Providers } catch (OperationCanceledException) { - throw; + if (gatewayTask != null) + await DisconnectImpl(default).ConfigureAwait(false); // DCT: Musn't abort } catch (Exception e) { diff --git a/src/Tgstation.Server.Host/Components/Chat/Providers/IDiscordResponders.cs b/src/Tgstation.Server.Host/Components/Chat/Providers/IDiscordResponders.cs index 602f134da6..d0c320fa9e 100644 --- a/src/Tgstation.Server.Host/Components/Chat/Providers/IDiscordResponders.cs +++ b/src/Tgstation.Server.Host/Components/Chat/Providers/IDiscordResponders.cs @@ -6,7 +6,7 @@ namespace Tgstation.Server.Host.Components.Chat.Providers /// /// Combined interface for the types used by TGS. /// - interface IDiscordResponders : IResponder + interface IDiscordResponders : IResponder, IResponder { } } diff --git a/src/Tgstation.Server.Host/Components/Chat/Providers/ProviderFactory.cs b/src/Tgstation.Server.Host/Components/Chat/Providers/ProviderFactory.cs index 5647811819..2d3893ef6d 100644 --- a/src/Tgstation.Server.Host/Components/Chat/Providers/ProviderFactory.cs +++ b/src/Tgstation.Server.Host/Components/Chat/Providers/ProviderFactory.cs @@ -68,7 +68,6 @@ namespace Tgstation.Server.Host.Components.Chat.Providers ChatProvider.Discord => new DiscordProvider( jobManager, assemblyInformationProvider, - asyncDelayer, loggerFactory.CreateLogger(), settings), _ => throw new InvalidOperationException(String.Format(CultureInfo.InvariantCulture, "Invalid ChatProvider: {0}", settings.Provider)), diff --git a/tests/Tgstation.Server.Host.Tests/Components/Chat/Providers/TestDiscordProvider.cs b/tests/Tgstation.Server.Host.Tests/Components/Chat/Providers/TestDiscordProvider.cs index 61994d2504..62579ddc25 100644 --- a/tests/Tgstation.Server.Host.Tests/Components/Chat/Providers/TestDiscordProvider.cs +++ b/tests/Tgstation.Server.Host.Tests/Components/Chat/Providers/TestDiscordProvider.cs @@ -7,7 +7,6 @@ using Microsoft.Extensions.Logging; using Microsoft.VisualStudio.TestTools.UnitTesting; using Moq; -using Tgstation.Server.Host.Core; using Tgstation.Server.Host.Jobs; using Tgstation.Server.Host.Models; using Tgstation.Server.Host.System; @@ -19,7 +18,6 @@ namespace Tgstation.Server.Host.Components.Chat.Providers.Tests { ChatBot testToken1; IJobManager mockJobManager; - IAsyncDelayer mockDel; [TestInitialize] public void Initialize() @@ -41,10 +39,6 @@ namespace Tgstation.Server.Host.Components.Chat.Providers.Tests .Setup(x => x.WaitForJobCompletion(It.IsNotNull(), It.IsAny(), It.IsAny(), It.IsAny())) .Returns(Task.CompletedTask); mockJobManager = mockSetup.Object; - - var mockDelSetup = new Mock(); - mockDelSetup.Setup(x => x.Delay(It.IsAny(), It.IsAny())).Returns(Task.CompletedTask); - mockDel = mockDelSetup.Object; } [TestMethod] @@ -53,14 +47,13 @@ namespace Tgstation.Server.Host.Components.Chat.Providers.Tests if (testToken1 == null) Assert.Inconclusive("Required environment variable TGS4_TEST_DISCORD_TOKEN isn't set!"); - Assert.ThrowsException(() => new DiscordProvider(null, null, null, null, null)); - Assert.ThrowsException(() => new DiscordProvider(mockJobManager, null, null, null, null)); + Assert.ThrowsException(() => new DiscordProvider(null, null, null, null)); + Assert.ThrowsException(() => new DiscordProvider(mockJobManager, null, null, null)); var mockAss = new Mock(); - Assert.ThrowsException(() => new DiscordProvider(mockJobManager, mockAss.Object, null, null, null)); - Assert.ThrowsException(() => new DiscordProvider(mockJobManager, mockAss.Object, mockDel, null, null)); + Assert.ThrowsException(() => new DiscordProvider(mockJobManager, mockAss.Object, null, null)); var mockLogger = new Mock>(); - Assert.ThrowsException(() => new DiscordProvider(mockJobManager, null, null, mockLogger.Object, null)); - await new DiscordProvider(mockJobManager, mockAss.Object, mockDel, mockLogger.Object, testToken1).DisposeAsync(); + Assert.ThrowsException(() => new DiscordProvider(mockJobManager, null, mockLogger.Object, null)); + await new DiscordProvider(mockJobManager, mockAss.Object, mockLogger.Object, testToken1).DisposeAsync(); } static Task InvokeConnect(IProvider provider, CancellationToken cancellationToken = default) => (Task)provider.GetType().GetMethod("Connect", BindingFlags.Instance | BindingFlags.NonPublic).Invoke(provider, new object[] { cancellationToken }); @@ -69,7 +62,7 @@ namespace Tgstation.Server.Host.Components.Chat.Providers.Tests public async Task TestConnectWithFakeTokenFails() { var mockLogger = new Mock>(); - await using var provider = new DiscordProvider(mockJobManager, Mock.Of(), mockDel, mockLogger.Object, new ChatBot + await using var provider = new DiscordProvider(mockJobManager, Mock.Of(), mockLogger.Object, new ChatBot { ReconnectionInterval = 1, ConnectionString = "asdf" @@ -85,17 +78,11 @@ namespace Tgstation.Server.Host.Components.Chat.Providers.Tests Assert.Inconclusive("Required environment variable TGS4_TEST_DISCORD_TOKEN isn't set!"); var mockLogger = new Mock>(); - await using var provider = new DiscordProvider(mockJobManager, Mock.Of(), mockDel, mockLogger.Object, testToken1); + await using var provider = new DiscordProvider(mockJobManager, Mock.Of(), mockLogger.Object, testToken1); Assert.IsFalse(provider.Connected); - await provider.Disconnect(default).ConfigureAwait(false); - Assert.IsFalse(provider.Connected); - await InvokeConnect(provider).ConfigureAwait(false); - Assert.IsTrue(provider.Connected); await InvokeConnect(provider).ConfigureAwait(false); Assert.IsTrue(provider.Connected); - await provider.Disconnect(default).ConfigureAwait(false); - Assert.IsFalse(provider.Connected); await provider.Disconnect(default).ConfigureAwait(false); Assert.IsFalse(provider.Connected);