diff --git a/src/Tgstation.Server.Api/Models/ErrorCode.cs b/src/Tgstation.Server.Api/Models/ErrorCode.cs
index 9a350f2377..174de92a6f 100644
--- a/src/Tgstation.Server.Api/Models/ErrorCode.cs
+++ b/src/Tgstation.Server.Api/Models/ErrorCode.cs
@@ -1,4 +1,4 @@
-using System;
+using System;
using System.ComponentModel;
namespace Tgstation.Server.Api.Models
@@ -557,6 +557,12 @@ namespace Tgstation.Server.Api.Models
/// Attempted to perform an instance operation with an offline instance.
///
[Description("The instance associated with the operation is currently offline!")]
- InstanceOffline
+ InstanceOffline,
+
+ ///
+ /// An attempt to connect a chat bot failed.
+ ///
+ [Description("Failed to connect chat bot!")]
+ ChatCannotConnectProvider
}
-}
\ No newline at end of file
+}
diff --git a/src/Tgstation.Server.Host/Components/Chat/ChatManager.cs b/src/Tgstation.Server.Host/Components/Chat/ChatManager.cs
index dafe18f360..05f2c19ee4 100644
--- a/src/Tgstation.Server.Host/Components/Chat/ChatManager.cs
+++ b/src/Tgstation.Server.Host/Components/Chat/ChatManager.cs
@@ -1,4 +1,4 @@
-using Microsoft.Extensions.Logging;
+using Microsoft.Extensions.Logging;
using Newtonsoft.Json;
using Serilog.Context;
using System;
@@ -117,11 +117,6 @@ namespace Tgstation.Server.Host.Components.Chat
///
long messagesProcessed;
- ///
- /// If has been called
- ///
- bool started;
-
///
/// Construct a
///
@@ -159,13 +154,13 @@ namespace Tgstation.Server.Host.Components.Chat
}
///
- public void Dispose()
+ public async ValueTask DisposeAsync()
{
logger.LogTrace("Disposing...");
restartRegistration.Dispose();
handlerCts.Dispose();
foreach (var I in providers)
- I.Value.Dispose();
+ await I.Value.DisposeAsync().ConfigureAwait(false);
}
///
@@ -206,6 +201,21 @@ namespace Tgstation.Server.Host.Components.Chat
return provider;
}
+ async Task RemapProvider(IProvider provider, CancellationToken cancellationToken)
+ {
+ logger.LogTrace("Remapping channels for provider reconnection...");
+ IEnumerable channelsToMap;
+ long providerId;
+ lock (providers)
+ providerId = providers.Where(x => x.Value == provider).Select(x => x.Key).First();
+
+ lock (activeChatBots)
+ channelsToMap = activeChatBots.FirstOrDefault(x => x.Id == providerId)?.Channels;
+
+ if (channelsToMap?.Any() ?? false)
+ await ChangeChannels(providerId, channelsToMap, cancellationToken).ConfigureAwait(false);
+ }
+
///
/// Processes a
///
@@ -213,26 +223,20 @@ namespace Tgstation.Server.Host.Components.Chat
/// The to process. If , this indicates the provider reconnected.
/// The for the operation
/// A representing the running operation
- #pragma warning disable CA1502
+#pragma warning disable CA1502
async Task ProcessMessage(IProvider provider, Message message, CancellationToken cancellationToken)
- #pragma warning restore CA1502
+#pragma warning restore CA1502
{
+ if (!provider.Connected)
+ {
+ logger.LogTrace("Abort message processing because provider is disconnected!");
+ return;
+ }
+
// provider reconnected, remap channels.
if (message == null)
{
- logger.LogTrace("Remapping channels for provider reconnection...");
- IEnumerable channelsToMap;
- lock (activeChatBots)
- channelsToMap = activeChatBots.FirstOrDefault()?.Channels;
-
- if (channelsToMap?.Any() ?? false)
- {
- long providerId;
- lock (providers)
- providerId = providers.Where(x => x.Value == provider).Select(x => x.Key).First();
- await ChangeChannels(providerId, channelsToMap, cancellationToken).ConfigureAwait(false);
- }
-
+ await RemapProvider(provider, cancellationToken).ConfigureAwait(false);
return;
}
@@ -243,9 +247,6 @@ namespace Tgstation.Server.Host.Components.Chat
var enumerable = mappedChannels.Where(x => x.Value.ProviderId == providerId && x.Value.ProviderChannelId == message.User.Channel.RealId);
if (message.User.Channel.IsPrivateChannel)
lock (mappedChannels)
- {
- if (!provider.Connected)
- return;
if (!enumerable.Any())
{
ulong newId;
@@ -267,7 +268,6 @@ namespace Tgstation.Server.Host.Components.Chat
}
else
message.User.Channel.RealId = enumerable.First().Key;
- }
else
{
// need to add tag and isAdminChannel
@@ -285,7 +285,9 @@ namespace Tgstation.Server.Host.Components.Chat
address = address.ToUpperInvariant();
- var addressed = address == CommonMention.ToUpperInvariant() || address == provider.BotMention.ToUpperInvariant();
+ var addressed =
+ address == CommonMention.ToUpperInvariant()
+ || address == provider.BotMention.ToUpperInvariant();
// no mention
if (!addressed && !message.User.Channel.IsPrivateChannel)
@@ -406,7 +408,7 @@ namespace Tgstation.Server.Host.Components.Chat
while (!cancellationToken.IsCancellationRequested)
{
// prune disconnected providers
- foreach (var I in messageTasks.Where(x => !x.Key.Connected).ToList())
+ foreach (var I in messageTasks.Where(x => !x.Key.Disposed).ToList())
messageTasks.Remove(I.Key);
// add new ones
@@ -415,7 +417,7 @@ namespace Tgstation.Server.Host.Components.Chat
updatedTask = connectionsUpdated.Task;
lock (providers)
foreach (var I in providers)
- if (I.Value.Connected && !messageTasks.ContainsKey(I.Value))
+ if (!messageTasks.ContainsKey(I.Value))
messageTasks.Add(I.Value, I.Value.NextMessage(cancellationToken));
if (messageTasks.Count == 0)
@@ -521,7 +523,7 @@ namespace Tgstation.Server.Host.Components.Chat
}
///
- public async Task ChangeSettings(ChatBot newSettings, CancellationToken cancellationToken)
+ public async Task ChangeSettings(Models.ChatBot newSettings, CancellationToken cancellationToken)
{
if (newSettings == null)
throw new ArgumentNullException(nameof(newSettings));
@@ -537,7 +539,7 @@ namespace Tgstation.Server.Host.Components.Chat
}
finally
{
- p.Dispose();
+ await p.DisposeAsync().ConfigureAwait(false);
}
}
@@ -565,30 +567,23 @@ namespace Tgstation.Server.Host.Components.Chat
await disconnectTask.ConfigureAwait(false);
- if (started)
+ lock (synchronizationLock)
{
- if (newSettings.Enabled.Value)
- await provider.Connect(cancellationToken).ConfigureAwait(false);
- lock (synchronizationLock)
- {
- // same thread shennanigans
- var oldOne = connectionsUpdated;
- connectionsUpdated = new TaskCompletionSource
/// The to wait for
- /// The to cancel the
+ /// The to cancel the . If the TGS user will be used.
/// A that will cancel the
/// The for the operation
/// A representing the
@@ -42,10 +42,15 @@ namespace Tgstation.Server.Host.Jobs
/// Cancels a give
///
/// The to cancel
- /// The who cancelled the
+ /// The who cancelled the . If the TGS user will be used.
/// If the operation should wait until the job exits before completing
/// The for the operation
/// A resulting in the updated if it was cancelled, if it couldn't be found.
Task CancelJob(Job job, User user, bool blocking, CancellationToken cancellationToken);
+
+ ///
+ /// Activate the .
+ ///
+ void Activate();
}
}
diff --git a/src/Tgstation.Server.Host/Jobs/JobManager.cs b/src/Tgstation.Server.Host/Jobs/JobManager.cs
index 6ec0fdc7c3..ced1eb724b 100644
--- a/src/Tgstation.Server.Host/Jobs/JobManager.cs
+++ b/src/Tgstation.Server.Host/Jobs/JobManager.cs
@@ -1,4 +1,4 @@
-using Microsoft.EntityFrameworkCore;
+using Microsoft.EntityFrameworkCore;
using Microsoft.Extensions.Logging;
using Serilog.Context;
using System;
@@ -8,6 +8,7 @@ using System.Threading;
using System.Threading.Tasks;
using Tgstation.Server.Host.Components;
using Tgstation.Server.Host.Database;
+using Tgstation.Server.Host.Extensions;
using Tgstation.Server.Host.Models;
namespace Tgstation.Server.Host.Jobs
@@ -35,6 +36,11 @@ namespace Tgstation.Server.Host.Jobs
///
readonly Dictionary jobs;
+ ///
+ /// to delay starting jobs until the server is ready.
+ ///
+ readonly TaskCompletionSource activationTcs;
+
///
/// for various operations.
///
@@ -52,6 +58,7 @@ namespace Tgstation.Server.Host.Jobs
this.instanceCoreProvider = instanceCoreProvider ?? throw new ArgumentNullException(nameof(instanceCoreProvider));
this.logger = logger ?? throw new ArgumentNullException(nameof(logger));
jobs = new Dictionary();
+ activationTcs = new TaskCompletionSource();
synchronizationLock = new object();
}
@@ -89,7 +96,7 @@ namespace Tgstation.Server.Host.Jobs
using (LogContext.PushProperty("Job", job.Id))
try
{
- void LogRegularException() => logger.LogDebug("Job {0} exited with error! Exception: {1}", job.Id, job.ExceptionDetails);
+ void LogException() => logger.LogDebug("Job {0} exited with error! Exception: {1}", job.Id, job.ExceptionDetails);
try
{
var oldJob = job;
@@ -102,6 +109,8 @@ namespace Tgstation.Server.Host.Jobs
handler.Progress = progress;
}
+ await activationTcs.Task.WithToken(cancellationToken).ConfigureAwait(false);
+
await operation(
instanceCoreProvider.Value.GetInstance(oldJob.Instance),
databaseContextFactory,
@@ -120,20 +129,13 @@ namespace Tgstation.Server.Host.Jobs
catch (JobException e)
{
job.ErrorCode = e.ErrorCode;
- job.ExceptionDetails = e.Message;
- LogRegularException();
- if (e.InnerException != null)
- logger.LogDebug(
- "Inner exception for job {0}: {1}",
- job.Id,
- e.InnerException is JobException
- ? e.InnerException.Message
- : e.InnerException.ToString());
+ job.ExceptionDetails = String.IsNullOrWhiteSpace(e.Message) ? e.InnerException?.Message : e.Message;
+ LogException();
}
catch (Exception e)
{
job.ExceptionDetails = e.ToString();
- LogRegularException();
+ LogException();
}
await databaseContextFactory.UseContext(async databaseContext =>
@@ -183,10 +185,16 @@ namespace Tgstation.Server.Host.Jobs
};
databaseContext.Instances.Attach(job.Instance);
- job.StartedBy = new User
- {
- Id = job.StartedBy.Id
- };
+ if (job.StartedBy == null)
+ job.StartedBy = await databaseContext
+ .Users
+ .GetTgsUser(cancellationToken)
+ .ConfigureAwait(false);
+ else
+ job.StartedBy = new User
+ {
+ Id = job.StartedBy.Id
+ };
databaseContext.Users.Attach(job.StartedBy);
databaseContext.Jobs.Add(job);
@@ -244,11 +252,14 @@ namespace Tgstation.Server.Host.Jobs
///
public async Task StopAsync(CancellationToken cancellationToken)
{
- var joinTasks = jobs.Select(x =>
- {
- x.Value.Cancel();
- return x.Value.Wait(cancellationToken);
- });
+ var joinTasks = jobs.Select(x => CancelJob(
+ new Job
+ {
+ Id = x.Key,
+ },
+ null,
+ true,
+ cancellationToken));
await Task.WhenAll(joinTasks).ConfigureAwait(false);
}
@@ -257,8 +268,6 @@ namespace Tgstation.Server.Host.Jobs
{
if (job == null)
throw new ArgumentNullException(nameof(job));
- if (user == null)
- throw new ArgumentNullException(nameof(user));
JobHandler handler;
try
{
@@ -273,6 +282,12 @@ namespace Tgstation.Server.Host.Jobs
handler.Cancel(); // this will ensure the db update is only done once
await databaseContextFactory.UseContext(async databaseContext =>
{
+ if (user == null)
+ {
+ user = await databaseContext.Users.GetTgsUser(cancellationToken).ConfigureAwait(false);
+ databaseContext.Users.Attach(user);
+ }
+
var updatedJob = new Job { Id = job.Id };
databaseContext.Jobs.Attach(job);
var attachedUser = new User { Id = user.Id };
@@ -322,5 +337,12 @@ namespace Tgstation.Server.Host.Jobs
if (cancelTask != null)
await cancelTask.ConfigureAwait(false);
}
+
+ ///
+ public void Activate()
+ {
+ logger.LogTrace("Activating job manager...");
+ activationTcs.SetResult(null);
+ }
}
}
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 8cf32df3e6..a035eee34c 100644
--- a/tests/Tgstation.Server.Host.Tests/Components/Chat/Providers/TestDiscordProvider.cs
+++ b/tests/Tgstation.Server.Host.Tests/Components/Chat/Providers/TestDiscordProvider.cs
@@ -1,9 +1,12 @@
-using Microsoft.Extensions.Logging;
+using Microsoft.Extensions.Logging;
using Microsoft.VisualStudio.TestTools.UnitTesting;
using Moq;
using System;
+using System.Reflection;
using System.Threading;
using System.Threading.Tasks;
+using Tgstation.Server.Host.Jobs;
+using Tgstation.Server.Host.Models;
using Tgstation.Server.Host.System;
namespace Tgstation.Server.Host.Components.Chat.Providers.Tests
@@ -11,52 +14,75 @@ namespace Tgstation.Server.Host.Components.Chat.Providers.Tests
[TestClass]
public sealed class TestDiscordProvider
{
- string testToken1;
+ ChatBot testToken1;
+ IJobManager mockJobManager;
[TestInitialize]
public void Initialize()
{
- testToken1 = Environment.GetEnvironmentVariable("TGS4_TEST_DISCORD_TOKEN_1");
+ var actualToken = Environment.GetEnvironmentVariable("TGS4_TEST_DISCORD_TOKEN");
+ if(!String.IsNullOrWhiteSpace(actualToken))
+ testToken1 = new ChatBot
+ {
+ ConnectionString = actualToken,
+ ReconnectionInterval = 1
+ };
+
+ var mockSetup = new Mock();
+ mockSetup
+ .Setup(x => x.RegisterOperation(It.IsNotNull(), It.IsNotNull(), It.IsAny()))
+ .Callback((job, entrypoint, cancellationToken) => job.StartedBy ??= new User { })
+ .Returns(Task.CompletedTask);
+ mockSetup
+ .Setup(x => x.WaitForJobCompletion(It.IsNotNull(), It.IsAny(), It.IsAny(), It.IsAny()))
+ .Returns(Task.CompletedTask);
+ mockJobManager = mockSetup.Object;
}
[TestMethod]
- public void TestConstructionAndDisposal()
+ public async Task TestConstructionAndDisposal()
{
- Assert.ThrowsException(() => new DiscordProvider(null, null, null, 1));
+ if (testToken1 == null)
+ Assert.Inconclusive("Required environment variable TGS4_TEST_DISCORD_TOKEN isn't set!");
+
+ Assert.ThrowsException(() => new DiscordProvider(null, null, null, null));
+ Assert.ThrowsException(() => new DiscordProvider(mockJobManager, null, null, null));
var mockAss = new Mock();
- Assert.ThrowsException(() => new DiscordProvider(mockAss.Object, null, null, 1));
+ Assert.ThrowsException(() => new DiscordProvider(mockJobManager, mockAss.Object, null, null));
var mockLogger = new Mock>();
- Assert.ThrowsException(() => new DiscordProvider(mockAss.Object, mockLogger.Object, null, 1));
- var mockToken = "asdf";
- Assert.ThrowsException(() => new DiscordProvider(mockAss.Object, mockLogger.Object, mockToken, 0));
- new DiscordProvider(mockAss.Object, mockLogger.Object, mockToken, 1).Dispose();
+ 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 });
+
[TestMethod]
public async Task TestConnectWithFakeTokenFails()
{
var mockLogger = new Mock>();
- using var provider = new DiscordProvider(Mock.Of(), mockLogger.Object, "asdf", 1);
- Assert.IsFalse(await provider.Connect(default).ConfigureAwait(false));
+ await using var provider = new DiscordProvider(mockJobManager, Mock.Of(), mockLogger.Object, new ChatBot
+ {
+ ReconnectionInterval = 1,
+ ConnectionString = "asdf"
+ });
+ await Assert.ThrowsExceptionAsync(() => InvokeConnect(provider));
Assert.IsFalse(provider.Connected);
}
- [Ignore("Broken due to dependency issues after first call to .Connect()")]
[TestMethod]
public async Task TestConnectAndDisconnect()
{
if (testToken1 == null)
- Assert.Inconclusive("Required environment variable TGS4_TEST_DISCORD_TOKEN_1 isn't set!");
-
+ Assert.Inconclusive("Required environment variable TGS4_TEST_DISCORD_TOKEN isn't set!");
var mockLogger = new Mock>();
- using var provider = new DiscordProvider(Mock.Of(), mockLogger.Object, testToken1, 1);
+ 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);
- Assert.IsTrue(await provider.Connect(default).ConfigureAwait(false));
+ await InvokeConnect(provider).ConfigureAwait(false);
Assert.IsTrue(provider.Connected);
- Assert.IsTrue(await provider.Connect(default).ConfigureAwait(false));
+ await InvokeConnect(provider).ConfigureAwait(false);
Assert.IsTrue(provider.Connected);
await provider.Disconnect(default).ConfigureAwait(false);
@@ -68,9 +94,9 @@ namespace Tgstation.Server.Host.Components.Chat.Providers.Tests
using var cts = new CancellationTokenSource();
cts.Cancel();
var cancellationToken = cts.Token;
- await Assert.ThrowsExceptionAsync(() => provider.Connect(cancellationToken)).ConfigureAwait(false);
+ await Assert.ThrowsExceptionAsync(() => InvokeConnect(provider, cancellationToken)).ConfigureAwait(false);
Assert.IsFalse(provider.Connected);
- Assert.IsTrue(await provider.Connect(default).ConfigureAwait(false));
+ await InvokeConnect(provider).ConfigureAwait(false);
Assert.IsTrue(provider.Connected);
await Assert.ThrowsExceptionAsync(() => provider.Disconnect(cancellationToken)).ConfigureAwait(false);
Assert.IsTrue(provider.Connected);
diff --git a/tests/Tgstation.Server.Tests/IntegrationTest.cs b/tests/Tgstation.Server.Tests/IntegrationTest.cs
index 7efaace32c..a82f8783e9 100644
--- a/tests/Tgstation.Server.Tests/IntegrationTest.cs
+++ b/tests/Tgstation.Server.Tests/IntegrationTest.cs
@@ -95,7 +95,13 @@ namespace Tgstation.Server.Tests
{
try
{
- return await clientFactory.CreateFromLogin(url, User.AdminName, User.DefaultAdminPassword, attemptLoginRefresh: false).ConfigureAwait(false);
+ return await clientFactory.CreateFromLogin(
+ url,
+ User.AdminName,
+ User.DefaultAdminPassword,
+ attemptLoginRefresh: false,
+ cancellationToken: cancellationToken)
+ .ConfigureAwait(false);
}
catch (HttpRequestException)
{