From dbe626f83db63e6f904e99c4b633f54b4803009d Mon Sep 17 00:00:00 2001 From: Jordan Brown Date: Fri, 8 May 2020 03:16:22 -0400 Subject: [PATCH 1/6] Fix being unable to set chat bot channel limits. Fixes #967 --- src/Tgstation.Server.Host/Controllers/ChatController.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/Tgstation.Server.Host/Controllers/ChatController.cs b/src/Tgstation.Server.Host/Controllers/ChatController.cs index 796fb8e94d..8af345403f 100644 --- a/src/Tgstation.Server.Host/Controllers/ChatController.cs +++ b/src/Tgstation.Server.Host/Controllers/ChatController.cs @@ -264,6 +264,7 @@ namespace Tgstation.Server.Host.Controllers || CheckModified(x => x.Name, ChatBotRights.WriteName) || CheckModified(x => x.Provider, ChatBotRights.WriteProvider) || CheckModified(x => x.ReconnectionInterval, ChatBotRights.WriteReconnectionInterval) + || CheckModified(x => x.ChannelLimit, ChatBotRights.WriteChannelLimit) || (model.Channels != null && !userRights.HasFlag(ChatBotRights.WriteChannels))) return Forbid(); From 73df4f93257ebfa4d0095d27581a227f1de4aa3b Mon Sep 17 00:00:00 2001 From: Jordan Brown Date: Fri, 8 May 2020 03:17:22 -0400 Subject: [PATCH 2/6] Fix ChatBotLimit being unable to update instances Fixes #968 --- src/Tgstation.Server.Host/Controllers/InstanceController.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Tgstation.Server.Host/Controllers/InstanceController.cs b/src/Tgstation.Server.Host/Controllers/InstanceController.cs index d207b07d5c..15f6eec39f 100644 --- a/src/Tgstation.Server.Host/Controllers/InstanceController.cs +++ b/src/Tgstation.Server.Host/Controllers/InstanceController.cs @@ -349,7 +349,7 @@ namespace Tgstation.Server.Host.Controllers /// Instance updated successfully. /// Instance updated successfully and relocation job created. [HttpPost] - [TgsAuthorize(InstanceManagerRights.Relocate | InstanceManagerRights.Rename | InstanceManagerRights.SetAutoUpdate | InstanceManagerRights.SetConfiguration | InstanceManagerRights.SetOnline)] + [TgsAuthorize(InstanceManagerRights.Relocate | InstanceManagerRights.Rename | InstanceManagerRights.SetAutoUpdate | InstanceManagerRights.SetConfiguration | InstanceManagerRights.SetOnline | InstanceManagerRights.SetChatBotLimit)] [ProducesResponseType(typeof(Api.Models.Instance), 200)] [ProducesResponseType(typeof(Api.Models.Instance), 202)] [ProducesResponseType(410)] From 95e786b198ee599775add66f744b3b858cac7b28 Mon Sep 17 00:00:00 2001 From: Jordan Brown Date: Fri, 8 May 2020 03:17:43 -0400 Subject: [PATCH 3/6] Version bump to 4.1.1 --- build/Version.props | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/build/Version.props b/build/Version.props index b9cd0eadbd..393bf27ba0 100644 --- a/build/Version.props +++ b/build/Version.props @@ -2,7 +2,7 @@ - 4.1.0 + 4.1.1 6.1.0 6.0.0 5.0.0 From 0c89b99418157de06dd78b679f86f79864631f68 Mon Sep 17 00:00:00 2001 From: Jordan Brown Date: Fri, 8 May 2020 03:18:06 -0400 Subject: [PATCH 4/6] Re-enable update test --- tests/Tgstation.Server.Tests/IntegrationTest.cs | 2 -- 1 file changed, 2 deletions(-) diff --git a/tests/Tgstation.Server.Tests/IntegrationTest.cs b/tests/Tgstation.Server.Tests/IntegrationTest.cs index 99f4b4318d..f4df4fcd61 100644 --- a/tests/Tgstation.Server.Tests/IntegrationTest.cs +++ b/tests/Tgstation.Server.Tests/IntegrationTest.cs @@ -78,8 +78,6 @@ namespace Tgstation.Server.Tests Assert.IsTrue(discordProvider.Connected, "Discord provider not connected!"); } - // Disabled until 4.1.0 due to changes in version handling - [Ignore] [TestMethod] public async Task TestServerUpdate() { From 7de1c45aa8a0a9eac240f49dece6f8de594c5cc9 Mon Sep 17 00:00:00 2001 From: Jordan Brown Date: Fri, 8 May 2020 03:25:33 -0400 Subject: [PATCH 5/6] Update integration tests with regressions --- .../Instance/ChatTest.cs | 3 +++ .../InstanceManagerTest.cs | 24 ++++++++++++++++--- .../Tgstation.Server.Tests/IntegrationTest.cs | 6 ++--- 3 files changed, 27 insertions(+), 6 deletions(-) diff --git a/tests/Tgstation.Server.Tests/Instance/ChatTest.cs b/tests/Tgstation.Server.Tests/Instance/ChatTest.cs index 89330c64a3..f6a54b595f 100644 --- a/tests/Tgstation.Server.Tests/Instance/ChatTest.cs +++ b/tests/Tgstation.Server.Tests/Instance/ChatTest.cs @@ -196,6 +196,9 @@ namespace Tgstation.Server.Tests.Instance var instance = metadata.CloneMetadata(); instance.ChatBotLimit = 0; await ApiAssert.ThrowsException(() => instanceClient.Update(instance, cancellationToken), ErrorCode.ChatBotMax); + + discordBot.ChannelLimit = 20; + await chatClient.Update(discordBot, cancellationToken); } } } diff --git a/tests/Tgstation.Server.Tests/InstanceManagerTest.cs b/tests/Tgstation.Server.Tests/InstanceManagerTest.cs index 59c6e4e09c..3f0012d296 100644 --- a/tests/Tgstation.Server.Tests/InstanceManagerTest.cs +++ b/tests/Tgstation.Server.Tests/InstanceManagerTest.cs @@ -5,6 +5,7 @@ using System.Reflection; using System.Threading; using System.Threading.Tasks; using Tgstation.Server.Api.Models; +using Tgstation.Server.Api.Rights; using Tgstation.Server.Client; using Tgstation.Server.Host.Controllers; using Tgstation.Server.Tests.Instance; @@ -14,13 +15,15 @@ namespace Tgstation.Server.Tests sealed class InstanceManagerTest { readonly IInstanceManagerClient instanceManagerClient; + readonly IUsersClient usersClient; readonly string testRootPath; long counter; - public InstanceManagerTest(IInstanceManagerClient instanceManagerClient, string testRootPath) + public InstanceManagerTest(IInstanceManagerClient instanceManagerClient, IUsersClient usersClient, string testRootPath) { this.instanceManagerClient = instanceManagerClient ?? throw new ArgumentNullException(nameof(instanceManagerClient)); + this.usersClient = usersClient ?? throw new ArgumentNullException(nameof(usersClient)); this.testRootPath = testRootPath ?? throw new ArgumentNullException(nameof(testRootPath)); counter = 0; @@ -110,7 +113,7 @@ namespace Tgstation.Server.Tests await Task.Delay(TimeSpan.FromSeconds(1), cancellationToken).ConfigureAwait(false); } while (firstTest.MoveJob != null); - + //online it for real for component tests firstTest.Online = true; firstTest.ConfigurationType = ConfigurationType.HostWrite; @@ -152,10 +155,25 @@ namespace Tgstation.Server.Tests var instanceAttachFileName = (string)typeof(InstanceController).GetField("InstanceAttachFileName", BindingFlags.NonPublic | BindingFlags.Static).GetValue(null); var attachPath = Path.Combine(firstTest.Path, instanceAttachFileName); Assert.IsTrue(File.Exists(attachPath)); - + //can recreate detached instance firstTest = await instanceManagerClient.CreateOrAttach(firstTest, cancellationToken).ConfigureAwait(false); + // Test updating only with SetChatBotLimit works + var current = await usersClient.Read(cancellationToken); + var update = new UserUpdate + { + Id = current.Id, + InstanceManagerRights = InstanceManagerRights.SetChatBotLimit + }; + await usersClient.Update(update, cancellationToken); + firstTest.ChatBotLimit = 77; + var newThing = await instanceManagerClient.Update(firstTest, cancellationToken); + Assert.AreEqual(77, newThing.ChatBotLimit); + + update.InstanceManagerRights |= InstanceManagerRights.Delete; + await usersClient.Update(update, cancellationToken); + //but only if the attach file exists await instanceManagerClient.Detach(firstTest, cancellationToken).ConfigureAwait(false); File.Delete(attachPath); diff --git a/tests/Tgstation.Server.Tests/IntegrationTest.cs b/tests/Tgstation.Server.Tests/IntegrationTest.cs index f4df4fcd61..fe31c266d5 100644 --- a/tests/Tgstation.Server.Tests/IntegrationTest.cs +++ b/tests/Tgstation.Server.Tests/IntegrationTest.cs @@ -123,7 +123,7 @@ namespace Tgstation.Server.Tests } } while (true); - var testUpdateVersion = new Version(4, 0, 0, 6); + var testUpdateVersion = new Version(4, 1, 0); using (adminClient) //attempt to update to stable await adminClient.Administration.Update(new Administration @@ -142,7 +142,7 @@ namespace Tgstation.Server.Tests Assert.IsTrue(File.Exists(updatedAssemblyPath), "Updated assembly missing!"); var updatedAssemblyVersion = FileVersionInfo.GetVersionInfo(updatedAssemblyPath); - Assert.AreEqual(testUpdateVersion, Version.Parse(updatedAssemblyVersion.FileVersion)); + Assert.AreEqual(testUpdateVersion, Version.Parse(updatedAssemblyVersion.FileVersion).Semver()); } finally { @@ -220,7 +220,7 @@ namespace Tgstation.Server.Tests var adminTest = new AdministrationTest(adminClient.Administration).Run(cancellationToken); var usersTest = new UsersTest(adminClient.Users).Run(cancellationToken); - await new InstanceManagerTest(adminClient.Instances, server.Directory).Run(cancellationToken).ConfigureAwait(false); + await new InstanceManagerTest(adminClient.Instances, adminClient.Users, server.Directory).Run(cancellationToken).ConfigureAwait(false); await adminTest.ConfigureAwait(false); await usersTest.ConfigureAwait(false); From be71c56eb8afb7cff59e3452fd807fb6d7adda1c Mon Sep 17 00:00:00 2001 From: Jordan Brown Date: Fri, 8 May 2020 04:10:57 -0400 Subject: [PATCH 6/6] Test updates --- .../Controllers/ChatController.cs | 6 +++--- tests/Tgstation.Server.Tests/Instance/ChatTest.cs | 1 + tests/Tgstation.Server.Tests/InstanceManagerTest.cs | 11 +++++++---- 3 files changed, 11 insertions(+), 7 deletions(-) diff --git a/src/Tgstation.Server.Host/Controllers/ChatController.cs b/src/Tgstation.Server.Host/Controllers/ChatController.cs index 8af345403f..321a004b71 100644 --- a/src/Tgstation.Server.Host/Controllers/ChatController.cs +++ b/src/Tgstation.Server.Host/Controllers/ChatController.cs @@ -87,8 +87,8 @@ namespace Tgstation.Server.Host.Controllers if (countOfExistingBotsInInstance >= Instance.ChatBotLimit.Value) return Conflict(new ErrorMessage(ErrorCode.ChatBotMax)); - model.Enabled = model.Enabled ?? false; - model.ReconnectionInterval = model.ReconnectionInterval ?? 1; + model.Enabled ??= false; + model.ReconnectionInterval ??= 1; // try to update das db first var dbModel = new Models.ChatBot @@ -330,7 +330,7 @@ namespace Tgstation.Server.Host.Controllers return BadRequest(new ErrorMessage(ErrorCode.ChatBotMaxChannels)); if (forCreation) - model.ChannelLimit = model.ChannelLimit ?? (ushort)defaultMaxChannels; + model.ChannelLimit ??= (ushort)defaultMaxChannels; return null; } diff --git a/tests/Tgstation.Server.Tests/Instance/ChatTest.cs b/tests/Tgstation.Server.Tests/Instance/ChatTest.cs index f6a54b595f..a31441d0b8 100644 --- a/tests/Tgstation.Server.Tests/Instance/ChatTest.cs +++ b/tests/Tgstation.Server.Tests/Instance/ChatTest.cs @@ -198,6 +198,7 @@ namespace Tgstation.Server.Tests.Instance await ApiAssert.ThrowsException(() => instanceClient.Update(instance, cancellationToken), ErrorCode.ChatBotMax); discordBot.ChannelLimit = 20; + discordBot.Channels = null; await chatClient.Update(discordBot, cancellationToken); } } diff --git a/tests/Tgstation.Server.Tests/InstanceManagerTest.cs b/tests/Tgstation.Server.Tests/InstanceManagerTest.cs index 3f0012d296..da75591d08 100644 --- a/tests/Tgstation.Server.Tests/InstanceManagerTest.cs +++ b/tests/Tgstation.Server.Tests/InstanceManagerTest.cs @@ -167,11 +167,14 @@ namespace Tgstation.Server.Tests InstanceManagerRights = InstanceManagerRights.SetChatBotLimit }; await usersClient.Update(update, cancellationToken); - firstTest.ChatBotLimit = 77; - var newThing = await instanceManagerClient.Update(firstTest, cancellationToken); - Assert.AreEqual(77, newThing.ChatBotLimit); + var update2 = new Api.Models.Instance + { + Id = firstTest.Id, + ChatBotLimit = 77 + }; + var newThing = await instanceManagerClient.Update(update2, cancellationToken); - update.InstanceManagerRights |= InstanceManagerRights.Delete; + update.InstanceManagerRights |= InstanceManagerRights.Delete | InstanceManagerRights.Create; await usersClient.Update(update, cancellationToken); //but only if the attach file exists