From 9c4144eb9825950774c610ef859273896bdbdd40 Mon Sep 17 00:00:00 2001 From: Jordan Brown Date: Sun, 9 May 2021 09:58:56 -0400 Subject: [PATCH 1/6] Fix bad dereference of nullable value Fixes #1255 --- .../Components/Chat/Providers/IrcProvider.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Tgstation.Server.Host/Components/Chat/Providers/IrcProvider.cs b/src/Tgstation.Server.Host/Components/Chat/Providers/IrcProvider.cs index a554d12637..8cac39f923 100644 --- a/src/Tgstation.Server.Host/Components/Chat/Providers/IrcProvider.cs +++ b/src/Tgstation.Server.Host/Components/Chat/Providers/IrcProvider.cs @@ -121,7 +121,7 @@ namespace Tgstation.Server.Host.Components.Chat.Providers nickname = ircBuilder.Nickname; password = ircBuilder.Password; - passwordType = ircBuilder.PasswordType.Value; + passwordType = ircBuilder.PasswordType; client = new IrcFeatures { From a250efefa6b7d03a661a0812a149d86305be65cc Mon Sep 17 00:00:00 2001 From: Jordan Brown Date: Sun, 9 May 2021 10:08:13 -0400 Subject: [PATCH 2/6] Regression test for #1255 --- .../Chat/Providers/TestIrcProvider.cs | 51 +++++++++++++++++++ 1 file changed, 51 insertions(+) create mode 100644 tests/Tgstation.Server.Host.Tests/Components/Chat/Providers/TestIrcProvider.cs diff --git a/tests/Tgstation.Server.Host.Tests/Components/Chat/Providers/TestIrcProvider.cs b/tests/Tgstation.Server.Host.Tests/Components/Chat/Providers/TestIrcProvider.cs new file mode 100644 index 0000000000..198bd78fa8 --- /dev/null +++ b/tests/Tgstation.Server.Host.Tests/Components/Chat/Providers/TestIrcProvider.cs @@ -0,0 +1,51 @@ +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.Api.Models; +using Tgstation.Server.Host.Core; +using Tgstation.Server.Host.Jobs; +using Tgstation.Server.Host.Models; +using Tgstation.Server.Host.System; + +namespace Tgstation.Server.Host.Components.Chat.Providers.Tests +{ + [TestClass] + public sealed class TestIrcProvider + { + [TestMethod] + public async Task TestConstructionAndDisposal() + { + Assert.ThrowsException(() => new IrcProvider(null, null, null, null, null)); + var mockJobManager = new Mock(); + Assert.ThrowsException(() => new IrcProvider(mockJobManager.Object, null, null, null, null)); + var mockAss = new Mock(); + Assert.ThrowsException(() => new IrcProvider(mockJobManager.Object, mockAss.Object, null, null, null)); + var mockAsyncDelayer = new Mock(); + Assert.ThrowsException(() => new IrcProvider(mockJobManager.Object, mockAss.Object, mockAsyncDelayer.Object, null, null)); + var mockLogger = new Mock>(); + Assert.ThrowsException(() => new IrcProvider(mockJobManager.Object, mockAss.Object, mockAsyncDelayer.Object, mockLogger.Object, null)); + + var mockBot = new ChatBot + { + Name = "test", + Provider = ChatProvider.Irc + }; + + Assert.ThrowsException(() => new IrcProvider(mockJobManager.Object, mockAss.Object, mockAsyncDelayer.Object, mockLogger.Object, mockBot)); + + mockBot.ConnectionString = new IrcConnectionStringBuilder + { + Address = "localhost", + Nickname = "test", + UseSsl = true, + Port = 6667 + }.ToString(); + + await new IrcProvider(mockJobManager.Object, mockAss.Object, mockAsyncDelayer.Object, mockLogger.Object, mockBot).DisposeAsync(); + } + } +} From 60e8a3ade8e4705c944809f8e70d3c4ed4bdb3fb Mon Sep 17 00:00:00 2001 From: Jordan Brown Date: Sun, 9 May 2021 10:08:25 -0400 Subject: [PATCH 3/6] Minor whitespace cleanup --- .../Components/Chat/Providers/IrcProvider.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/Tgstation.Server.Host/Components/Chat/Providers/IrcProvider.cs b/src/Tgstation.Server.Host/Components/Chat/Providers/IrcProvider.cs index 8cac39f923..93bf03a260 100644 --- a/src/Tgstation.Server.Host/Components/Chat/Providers/IrcProvider.cs +++ b/src/Tgstation.Server.Host/Components/Chat/Providers/IrcProvider.cs @@ -110,6 +110,7 @@ namespace Tgstation.Server.Host.Components.Chat.Providers { if (assemblyInformationProvider == null) throw new ArgumentNullException(nameof(assemblyInformationProvider)); + this.asyncDelayer = asyncDelayer ?? throw new ArgumentNullException(nameof(asyncDelayer)); var builder = chatBot.CreateConnectionStringBuilder(); From 42686d6d34f7e8427dc4e0f8f2823df1b10a8f52 Mon Sep 17 00:00:00 2001 From: Jordan Brown Date: Sun, 9 May 2021 10:10:27 -0400 Subject: [PATCH 4/6] Remove unused usings --- .../Components/Chat/Providers/TestIrcProvider.cs | 2 -- 1 file changed, 2 deletions(-) diff --git a/tests/Tgstation.Server.Host.Tests/Components/Chat/Providers/TestIrcProvider.cs b/tests/Tgstation.Server.Host.Tests/Components/Chat/Providers/TestIrcProvider.cs index 198bd78fa8..68413a0809 100644 --- a/tests/Tgstation.Server.Host.Tests/Components/Chat/Providers/TestIrcProvider.cs +++ b/tests/Tgstation.Server.Host.Tests/Components/Chat/Providers/TestIrcProvider.cs @@ -2,8 +2,6 @@ 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.Api.Models; using Tgstation.Server.Host.Core; From 1e9e97f0cb50b1ac16a3d33b6d6fe2101616dc52 Mon Sep 17 00:00:00 2001 From: Jordan Brown Date: Sun, 9 May 2021 10:29:34 -0400 Subject: [PATCH 5/6] Regression test for #1256 --- .../InstanceManagerTest.cs | 50 +++++++++++++++++-- .../Tgstation.Server.Tests/IntegrationTest.cs | 4 +- 2 files changed, 49 insertions(+), 5 deletions(-) diff --git a/tests/Tgstation.Server.Tests/InstanceManagerTest.cs b/tests/Tgstation.Server.Tests/InstanceManagerTest.cs index 82d99d3ed9..08fae66d40 100644 --- a/tests/Tgstation.Server.Tests/InstanceManagerTest.cs +++ b/tests/Tgstation.Server.Tests/InstanceManagerTest.cs @@ -1,9 +1,15 @@ using Microsoft.VisualStudio.TestTools.UnitTesting; +using Newtonsoft.Json; using System; using System.IO; using System.Linq; +using System.Net; +using System.Net.Http; +using System.Net.Http.Headers; +using System.Net.Mime; using System.Threading; using System.Threading.Tasks; +using Tgstation.Server.Api; using Tgstation.Server.Api.Models; using Tgstation.Server.Api.Models.Request; using Tgstation.Server.Api.Models.Response; @@ -17,14 +23,16 @@ namespace Tgstation.Server.Tests { public const string TestInstanceName = "IntegrationTestInstance"; + readonly IServerClient serverClient; readonly IInstanceManagerClient instanceManagerClient; readonly IUsersClient usersClient; readonly string testRootPath; - public InstanceManagerTest(IInstanceManagerClient instanceManagerClient, IUsersClient usersClient, string testRootPath) + public InstanceManagerTest(IServerClient serverClient, string testRootPath) { - this.instanceManagerClient = instanceManagerClient ?? throw new ArgumentNullException(nameof(instanceManagerClient)); - this.usersClient = usersClient ?? throw new ArgumentNullException(nameof(usersClient)); + this.serverClient = serverClient ?? throw new ArgumentNullException(nameof(serverClient)); + this.instanceManagerClient = serverClient.Instances; + this.usersClient = serverClient.Users; this.testRootPath = testRootPath ?? throw new ArgumentNullException(nameof(testRootPath)); } @@ -150,9 +158,45 @@ namespace Tgstation.Server.Tests }, cancellationToken), ErrorCode.InstanceRelocateOnline).ConfigureAwait(false); Assert.IsTrue(Directory.Exists(firstTest.Path)); + await RegressionTest1256(cancellationToken).ConfigureAwait(false); + return firstTest; } + async Task RegressionTest1256(CancellationToken cancellationToken) + { + var allInstances = await instanceManagerClient.List(null, cancellationToken).ConfigureAwait(false); + Assert.IsTrue(allInstances.Count <= 6, "Need less than or 6 instances at this point"); + + for (var I = allInstances.Count; I < 6; ++I) + await instanceManagerClient.CreateOrAttach(new InstanceCreateRequest + { + Name = $"RegressionTest1256-{I}", + Path = Path.Combine(testRootPath, Guid.NewGuid().ToString()), + }, cancellationToken).ConfigureAwait(false); + + var url = serverClient.Url; + var token = serverClient.Token.Bearer; + // check that 400s are returned appropriately + using var httpClient = new HttpClient(); + using (var request = new HttpRequestMessage(HttpMethod.Get, url.ToString() + Routes.ListRoute(Routes.InstanceManager).Substring(1) + "?pageSize=2")) + { + request.Headers.Accept.Clear(); + request.Headers.UserAgent.Add(new ProductInfoHeaderValue("RegressionTest1256", "1.0.0")); + request.Headers.Accept.Add(new MediaTypeWithQualityHeaderValue(MediaTypeNames.Application.Json)); + request.Headers.Add(ApiHeaders.ApiVersionHeader, "Tgstation.Server.Api/" + ApiHeaders.Version); + request.Headers.Authorization = new AuthenticationHeaderValue(ApiHeaders.BearerAuthenticationScheme, token); + using var response = await httpClient.SendAsync(request, cancellationToken); + response.EnsureSuccessStatusCode(); + + var json = await response.Content.ReadAsStringAsync(); + var paginated = JsonConvert.DeserializeObject>(json); + + Assert.AreEqual(2, paginated.PageSize); + Assert.AreEqual(3, paginated.TotalPages); + } + } + public async Task RunPostTest(CancellationToken cancellationToken) { var instances = await instanceManagerClient.List(null, cancellationToken); diff --git a/tests/Tgstation.Server.Tests/IntegrationTest.cs b/tests/Tgstation.Server.Tests/IntegrationTest.cs index a61532bca8..eeeffeb764 100644 --- a/tests/Tgstation.Server.Tests/IntegrationTest.cs +++ b/tests/Tgstation.Server.Tests/IntegrationTest.cs @@ -767,7 +767,7 @@ namespace Tgstation.Server.Tests var rootTest = FailFast(new RawRequestTests().Run(clientFactory, adminClient, cancellationToken)); var adminTest = FailFast(new AdministrationTest(adminClient.Administration).Run(cancellationToken)); var usersTest = FailFast(new UsersTest(adminClient).Run(cancellationToken)); - instance = await new InstanceManagerTest(adminClient.Instances, adminClient.Users, server.Directory).RunPreInstanceTest(cancellationToken); + instance = await new InstanceManagerTest(adminClient, server.Directory).RunPreInstanceTest(cancellationToken); Assert.IsTrue(Directory.Exists(instance.Path)); var instanceClient = adminClient.Instances.CreateClient(instance); @@ -884,7 +884,7 @@ namespace Tgstation.Server.Tests await new ChatTest(instanceClient.ChatBots, adminClient.Instances, instance).RunPostTest(cancellationToken); await repoTest; - await new InstanceManagerTest(adminClient.Instances, adminClient.Users, server.Directory).RunPostTest(cancellationToken); + await new InstanceManagerTest(adminClient, server.Directory).RunPostTest(cancellationToken); } } catch(ApiException ex) From e75df2877da3345dcf4a3e75b9c739854918d27e Mon Sep 17 00:00:00 2001 From: Jordan Brown Date: Sun, 9 May 2021 10:34:39 -0400 Subject: [PATCH 6/6] Fixes pagination total pages response If division matched perfectly, an extra page would be added. Fixes #1256 --- src/Tgstation.Server.Host/Controllers/ApiController.cs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/Tgstation.Server.Host/Controllers/ApiController.cs b/src/Tgstation.Server.Host/Controllers/ApiController.cs index 659f7da2e8..ce3c596459 100644 --- a/src/Tgstation.Server.Host/Controllers/ApiController.cs +++ b/src/Tgstation.Server.Host/Controllers/ApiController.cs @@ -406,12 +406,15 @@ namespace Tgstation.Server.Host.Controllers foreach (var I in finalResults) resultTransformer(I); + var carryTheOne = totalResults % pageSize != 0 + ? 1 + : 0; return Json( new PaginatedResponse { Content = finalResults, PageSize = pageSize, - TotalPages = (ushort)((totalResults / pageSize) + 1) + TotalPages = (ushort)(totalResults / pageSize) + carryTheOne }); } }