diff --git a/src/Tgstation.Server.Client/ApiConflictException.cs b/src/Tgstation.Server.Client/ApiConflictException.cs index 29cdaa6aff..e107b9bc25 100644 --- a/src/Tgstation.Server.Client/ApiConflictException.cs +++ b/src/Tgstation.Server.Client/ApiConflictException.cs @@ -5,7 +5,7 @@ using Tgstation.Server.Api.Models; namespace Tgstation.Server.Client { /// - /// Occurs when the server returns an unknown response + /// Occurs when the server returns a bad request response if the is present. The server returned an unknown reponse otherwise. /// public sealed class ApiConflictException : ApiException { diff --git a/src/Tgstation.Server.Host/Controllers/ChatController.cs b/src/Tgstation.Server.Host/Controllers/ChatController.cs index 181d095328..796fb8e94d 100644 --- a/src/Tgstation.Server.Host/Controllers/ChatController.cs +++ b/src/Tgstation.Server.Host/Controllers/ChatController.cs @@ -24,6 +24,7 @@ namespace Tgstation.Server.Host.Controllers /// for managing s /// [Route(Routes.Chat)] + #pragma warning disable CA1506 // TODO: Decomplexify public sealed class ChatController : ApiController { /// @@ -84,7 +85,7 @@ namespace Tgstation.Server.Host.Controllers .ConfigureAwait(false); if (countOfExistingBotsInInstance >= Instance.ChatBotLimit.Value) - return BadRequest(new ErrorMessage(ErrorCode.ChatBotMax)); + return Conflict(new ErrorMessage(ErrorCode.ChatBotMax)); model.Enabled = model.Enabled ?? false; model.ReconnectionInterval = model.ReconnectionInterval ?? 1; @@ -98,7 +99,8 @@ namespace Tgstation.Server.Host.Controllers Channels = model.Channels?.Select(x => ConvertApiChatChannel(x)).ToList() ?? new List(), // important that this isn't null InstanceId = Instance.Id, Provider = model.Provider, - ReconnectionInterval = model.ReconnectionInterval + ReconnectionInterval = model.ReconnectionInterval, + ChannelLimit = model.ChannelLimit }; DatabaseContext.ChatBots.Add(dbModel); @@ -206,8 +208,8 @@ namespace Tgstation.Server.Host.Controllers [TgsAuthorize(ChatBotRights.WriteChannels | ChatBotRights.WriteConnectionString | ChatBotRights.WriteEnabled | ChatBotRights.WriteName | ChatBotRights.WriteProvider)] [ProducesResponseType(200)] [ProducesResponseType(typeof(Api.Models.ChatBot), 200)] - #pragma warning disable CA1502 // TODO: Decomplexify - #pragma warning disable CA1506 +#pragma warning disable CA1502 // TODO: Decomplexify +#pragma warning disable CA1506 public async Task Update([FromBody] Api.Models.ChatBot model, CancellationToken cancellationToken) #pragma warning restore CA1502 #pragma warning restore CA1506 @@ -227,7 +229,13 @@ namespace Tgstation.Server.Host.Controllers return StatusCode((int)HttpStatusCode.Gone); if ((model.Channels?.Count ?? current.Channels.Count) > (model.ChannelLimit ?? current.ChannelLimit.Value)) - return BadRequest(new ErrorMessage(ErrorCode.ChatBotMaxChannels)); + { + // 400 or 409 depends on if the client sent both + var errorMessage = new ErrorMessage(ErrorCode.ChatBotMaxChannels); + if (model.Channels != null && model.ChannelLimit.HasValue) + return BadRequest(errorMessage); + return Conflict(errorMessage); + } var userRights = (ChatBotRights)AuthenticationContext.GetRight(RightsType.ChatBots); @@ -326,4 +334,5 @@ namespace Tgstation.Server.Host.Controllers return null; } } + #pragma warning restore CA1506 } diff --git a/src/Tgstation.Server.Host/Controllers/InstanceController.cs b/src/Tgstation.Server.Host/Controllers/InstanceController.cs index 35d5ffc285..26dcddb004 100644 --- a/src/Tgstation.Server.Host/Controllers/InstanceController.cs +++ b/src/Tgstation.Server.Host/Controllers/InstanceController.cs @@ -439,7 +439,7 @@ namespace Tgstation.Server.Host.Controllers .ConfigureAwait(false); if (countOfExistingChatBots > model.ChatBotLimit.Value) - return BadRequest(new ErrorMessage(ErrorCode.ChatBotMax)); + return Conflict(new ErrorMessage(ErrorCode.ChatBotMax)); } // ensure the current user has write privilege on the instance diff --git a/src/Tgstation.Server.Host/Models/ChatBot.cs b/src/Tgstation.Server.Host/Models/ChatBot.cs index dda821709e..27cdcfaa89 100644 --- a/src/Tgstation.Server.Host/Models/ChatBot.cs +++ b/src/Tgstation.Server.Host/Models/ChatBot.cs @@ -39,7 +39,9 @@ namespace Tgstation.Server.Host.Models Enabled = Enabled, Provider = Provider, Id = Id, - Name = Name + Name = Name, + ChannelLimit = ChannelLimit, + ReconnectionInterval = ReconnectionInterval }; } } diff --git a/src/Tgstation.Server.Host/Models/Instance.cs b/src/Tgstation.Server.Host/Models/Instance.cs index 1446ee1138..19a54970d8 100644 --- a/src/Tgstation.Server.Host/Models/Instance.cs +++ b/src/Tgstation.Server.Host/Models/Instance.cs @@ -63,7 +63,8 @@ namespace Tgstation.Server.Host.Models Id = Id, Name = Name, Path = Path, - Online = Online + Online = Online, + ChatBotLimit = ChatBotLimit }; } } diff --git a/tests/Tgstation.Server.Tests/Instance/ChatTest.cs b/tests/Tgstation.Server.Tests/Instance/ChatTest.cs index be7577110c..825a5f1b6e 100644 --- a/tests/Tgstation.Server.Tests/Instance/ChatTest.cs +++ b/tests/Tgstation.Server.Tests/Instance/ChatTest.cs @@ -1,9 +1,11 @@ using System; using System.Collections.Generic; using System.Linq; +using System.Threading; using System.Threading.Tasks; using Microsoft.VisualStudio.TestTools.UnitTesting; using Tgstation.Server.Api.Models; +using Tgstation.Server.Client; using Tgstation.Server.Client.Components; namespace Tgstation.Server.Tests.Instance @@ -11,13 +13,17 @@ namespace Tgstation.Server.Tests.Instance sealed class ChatTest { readonly IChatBotsClient chatClient; + readonly IInstanceManagerClient instanceClient; + readonly Api.Models.Instance metadata; - public ChatTest(IChatBotsClient chatBotsClient) + public ChatTest(IChatBotsClient chatClient, IInstanceManagerClient instanceClient, Api.Models.Instance metadata) { - chatClient = chatBotsClient ?? throw new ArgumentNullException(nameof(chatBotsClient)); + this.chatClient = chatClient ?? throw new ArgumentNullException(nameof(chatClient)); + this.instanceClient = instanceClient ?? throw new ArgumentNullException(nameof(instanceClient)); + this.metadata = metadata ?? throw new ArgumentNullException(nameof(metadata)); } - public async Task Run() + public async Task Run(CancellationToken cancellationToken) { var firstBot = new ChatBot { @@ -25,22 +31,23 @@ namespace Tgstation.Server.Tests.Instance Enabled = false, Name = "r4407", Provider = ChatProvider.Discord, - ReconnectionInterval = 1 + ReconnectionInterval = 1, + ChannelLimit = 1 }; - firstBot = await chatClient.Create(firstBot, default); + firstBot = await chatClient.Create(firstBot, cancellationToken); Assert.AreNotEqual(0, firstBot.Id); - var bots = await chatClient.List(default); + var bots = await chatClient.List(cancellationToken); Assert.AreEqual(1, bots.Count); Assert.AreEqual(firstBot.Id, bots.First().Id); - var retrievedBot = await chatClient.GetId(firstBot, default); - Assert.AreEqual(firstBot.Id, retrievedBot); + var retrievedBot = await chatClient.GetId(firstBot, cancellationToken); + Assert.AreEqual(firstBot.Id, retrievedBot.Id); firstBot.Enabled = true; - var updatedBot = await chatClient.Update(firstBot, default); + var updatedBot = await chatClient.Update(firstBot, cancellationToken); Assert.AreEqual(true, updatedBot.Enabled); @@ -56,21 +63,55 @@ namespace Tgstation.Server.Tests.Instance DiscordChannelId = channelId } }; + - updatedBot = await chatClient.Update(firstBot, default); + updatedBot = await chatClient.Update(firstBot, cancellationToken); Assert.AreEqual(true, updatedBot.Enabled); Assert.IsNotNull(updatedBot.Channels); Assert.AreEqual(1, updatedBot.Channels.Count); Assert.AreEqual(true, updatedBot.Channels.First().IsAdminChannel); - Assert.AreEqual(true, updatedBot.Channels.First().IsUpdatesChannel); + Assert.AreEqual(false, updatedBot.Channels.First().IsUpdatesChannel); Assert.AreEqual(true, updatedBot.Channels.First().IsWatchdogChannel); Assert.AreEqual("butt", updatedBot.Channels.First().Tag); Assert.AreEqual(channelId, updatedBot.Channels.First().DiscordChannelId); Assert.IsNull(updatedBot.Channels.First().IrcChannel); - await chatClient.Delete(firstBot, default); - bots = await chatClient.List(default); + await ApiAssert.ThrowsException(() => chatClient.Create(new ChatBot + { + Name = "asdf", + ConnectionString = "asdf", + Provider = ChatProvider.Irc + }, cancellationToken), ErrorCode.ChatBotMax); + + // We limited chat bots and channels to 1, try violating them + updatedBot.Channels.Add( + new ChatChannel + { + IsAdminChannel = true, + IsUpdatesChannel = false, + IsWatchdogChannel = true, + Tag = "butt", + DiscordChannelId = channelId + }); + + await ApiAssert.ThrowsException(() => chatClient.Update(updatedBot, cancellationToken), ErrorCode.ChatBotMaxChannels); + + var oldChannels = updatedBot.Channels; + updatedBot.Channels = null; + updatedBot.ChannelLimit = 0; + await ApiAssert.ThrowsException(() => chatClient.Update(updatedBot, cancellationToken), ErrorCode.ChatBotMaxChannels); + + updatedBot.Channels = oldChannels; + updatedBot.ChannelLimit = null; + await ApiAssert.ThrowsException(() => chatClient.Update(updatedBot, cancellationToken), ErrorCode.ChatBotMaxChannels); + + var instance = metadata.CloneMetadata(); + instance.ChatBotLimit = 0; + await ApiAssert.ThrowsException(() => instanceClient.Update(instance, cancellationToken), ErrorCode.ChatBotMax); + + await chatClient.Delete(firstBot, cancellationToken); + bots = await chatClient.List(cancellationToken); Assert.AreEqual(0, bots.Count); } } diff --git a/tests/Tgstation.Server.Tests/Instance/InstanceTest.cs b/tests/Tgstation.Server.Tests/Instance/InstanceTest.cs index 8ca9076a30..b83c8be9c4 100644 --- a/tests/Tgstation.Server.Tests/Instance/InstanceTest.cs +++ b/tests/Tgstation.Server.Tests/Instance/InstanceTest.cs @@ -1,6 +1,7 @@ using System; using System.Threading; using System.Threading.Tasks; +using Tgstation.Server.Client; using Tgstation.Server.Client.Components; namespace Tgstation.Server.Tests.Instance @@ -8,20 +9,25 @@ namespace Tgstation.Server.Tests.Instance sealed class InstanceTest { readonly IInstanceClient instanceClient; + readonly IInstanceManagerClient instanceManagerClient; - public InstanceTest(IInstanceClient instanceClient) + public InstanceTest(IInstanceClient instanceClient, IInstanceManagerClient instanceManagerClient) { this.instanceClient = instanceClient ?? throw new ArgumentNullException(nameof(instanceClient)); + this.instanceManagerClient = instanceManagerClient ?? throw new ArgumentNullException(nameof(instanceManagerClient)); } public async Task RunTests(CancellationToken cancellationToken) { - var byondTests = new ByondTest(instanceClient.Byond, instanceClient.Jobs); - var configTests = new ConfigurationTest(instanceClient.Configuration, instanceClient.Metadata); + var byondTest = new ByondTest(instanceClient.Byond, instanceClient.Jobs); + var chatTest = new ChatTest(instanceClient.ChatBots, instanceManagerClient, instanceClient.Metadata.CloneMetadata()); + var configTest = new ConfigurationTest(instanceClient.Configuration, instanceClient.Metadata); - var byondTest = byondTests.Run(cancellationToken); - await configTests.Run(cancellationToken).ConfigureAwait(false); - await byondTest.ConfigureAwait(false); + var byondTests = byondTest.Run(cancellationToken); + var chatTests = chatTest.Run(cancellationToken); + await configTest.Run(cancellationToken).ConfigureAwait(false); + await byondTests.ConfigureAwait(false); + await chatTests.ConfigureAwait(false); } } } diff --git a/tests/Tgstation.Server.Tests/InstanceManagerTest.cs b/tests/Tgstation.Server.Tests/InstanceManagerTest.cs index 785769c43c..f94fbcb3d3 100644 --- a/tests/Tgstation.Server.Tests/InstanceManagerTest.cs +++ b/tests/Tgstation.Server.Tests/InstanceManagerTest.cs @@ -30,7 +30,8 @@ namespace Tgstation.Server.Tests { Name = "TestInstance-" + ++counter, Path = Path.Combine(testRootPath, Guid.NewGuid().ToString()), - Online = true + Online = true, + ChatBotLimit = 1 }, cancellationToken); public async Task Run(CancellationToken cancellationToken) @@ -115,7 +116,7 @@ namespace Tgstation.Server.Tests Path = initialPath }, cancellationToken), ErrorCode.InstanceRelocateOnline).ConfigureAwait(false); - var testSuite1 = new InstanceTest(instanceManagerClient.CreateClient(firstTest)); + var testSuite1 = new InstanceTest(instanceManagerClient.CreateClient(firstTest), instanceManagerClient); await testSuite1.RunTests(cancellationToken).ConfigureAwait(false); //can regain permissions on instance without instance user