From de7c3b31c70693900251ff4b9df6bd060f6faa3b Mon Sep 17 00:00:00 2001 From: Jordan Brown Date: Thu, 3 Dec 2020 11:41:59 -0500 Subject: [PATCH 1/6] Adds additional null checking Likely fixes #1127 --- src/Tgstation.Server.Host/Controllers/RepositoryController.cs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/Tgstation.Server.Host/Controllers/RepositoryController.cs b/src/Tgstation.Server.Host/Controllers/RepositoryController.cs index 88de87bbce..880a8efc96 100644 --- a/src/Tgstation.Server.Host/Controllers/RepositoryController.cs +++ b/src/Tgstation.Server.Host/Controllers/RepositoryController.cs @@ -744,6 +744,9 @@ namespace Tgstation.Server.Host.Controllers revInfoWereLookingFor = dbPull .Where(testRevInfo => { + if (testRevInfo.PrimaryTestMerge == null) + return false; + var testMergeMatch = model.NewTestMerges.Any(testTestMerge => { var numberMatch = testRevInfo.PrimaryTestMerge.Number == testTestMerge.Number; From 88b16e1323bf788127c2f17ce15bd2ea19f9c03f Mon Sep 17 00:00:00 2001 From: Jordan Brown Date: Thu, 3 Dec 2020 11:43:25 -0500 Subject: [PATCH 2/6] Version bump to 4.6.1 --- build/Version.props | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/build/Version.props b/build/Version.props index 3b64b3ff67..306f7948c9 100644 --- a/build/Version.props +++ b/build/Version.props @@ -3,7 +3,7 @@ - 4.6.0 + 4.6.1 2.1.1 7.4.0 8.4.0 From 21c9e5f01a6c2e4c357ec039c00e20e34f663736 Mon Sep 17 00:00:00 2001 From: Jordan Brown Date: Wed, 9 Dec 2020 15:50:29 -0500 Subject: [PATCH 3/6] Fix downgrade migrations for the last time --- .../Database/DatabaseContext.cs | 109 +++++++++++++++--- .../Database/DatabaseSeeder.cs | 6 +- .../Database/IDatabaseContext.cs | 8 +- tests/Tgstation.Server.Tests/VersionsTest.cs | 59 ++++++++++ 4 files changed, 164 insertions(+), 18 deletions(-) diff --git a/src/Tgstation.Server.Host/Database/DatabaseContext.cs b/src/Tgstation.Server.Host/Database/DatabaseContext.cs index 85f00bd68a..1a1aa6c0e8 100644 --- a/src/Tgstation.Server.Host/Database/DatabaseContext.cs +++ b/src/Tgstation.Server.Host/Database/DatabaseContext.cs @@ -315,30 +315,111 @@ namespace Tgstation.Server.Host.Database return wasEmpty; } +#if DEBUG + /// + /// Used by unit tests to remind us to setup the correct MSSQL migration downgrades. + /// + public static readonly Type MSLatestMigration = typeof(MSAddAdditionalDDParameters); + + /// + /// Used by unit tests to remind us to setup the correct MSSQL migration downgrades. + /// + public static readonly Type MYLatestMigration = typeof(MYAddAdditionalDDParameters); + + /// + /// Used by unit tests to remind us to setup the correct MSSQL migration downgrades. + /// + public static readonly Type PGLatestMigration = typeof(PGAddAdditionalDDParameters); + + /// + /// Used by unit tests to remind us to setup the correct MSSQL migration downgrades. + /// + public static readonly Type SLLatestMigration = typeof(SLAddAdditionalDDParameters); +#endif + /// public async Task SchemaDowngradeForServerVersion( ILogger logger, - Version version, + Version targetVersion, DatabaseType currentDatabaseType, CancellationToken cancellationToken) { if(logger == null) throw new ArgumentNullException(nameof(logger)); - if (version == null) - throw new ArgumentNullException(nameof(version)); - if (version < new Version(4, 0)) - throw new ArgumentOutOfRangeException(nameof(version), version, "Not a valid V4 version!"); + if (targetVersion == null) + throw new ArgumentNullException(nameof(targetVersion)); + if (targetVersion < new Version(4, 0)) + throw new ArgumentOutOfRangeException(nameof(targetVersion), targetVersion, "Not a valid V4 version!"); + + if (currentDatabaseType == DatabaseType.PostgresSql && targetVersion < new Version(4, 3, 0)) + throw new NotSupportedException("Cannot migrate below version 4.3.0 with PostgresSql!"); + + if (targetVersion < new Version(4, 1, 0)) + throw new NotSupportedException("Cannot migrate below version 4.1.0!"); // Update this with new migrations as they are made string targetMigration = null; + if (targetVersion < new Version(4, 7, 0)) + switch (currentDatabaseType) + { + case DatabaseType.MariaDB: + case DatabaseType.MySql: + targetMigration = nameof(MYAddAdditionalDDParameters); + break; + case DatabaseType.PostgresSql: + targetMigration = nameof(PGAddAdditionalDDParameters); + break; + case DatabaseType.SqlServer: + targetMigration = nameof(MSAddAdditionalDDParameters); + break; + case DatabaseType.Sqlite: + targetMigration = nameof(SLAddAdditionalDDParameters); + break; + default: + throw new ArgumentException($"Invalid DatabaseType: {currentDatabaseType}", nameof(currentDatabaseType)); + } - if (currentDatabaseType == DatabaseType.PostgresSql && version < new Version(4, 3, 0)) - throw new NotSupportedException("Cannot migrate below version 4.3.0 with PostgresSql!"); + if (targetVersion < new Version(4, 6, 0)) + switch (currentDatabaseType) + { + case DatabaseType.MariaDB: + case DatabaseType.MySql: + targetMigration = nameof(MYAddDeploymentColumns); + break; + case DatabaseType.PostgresSql: + targetMigration = nameof(PGAddDeploymentColumns); + break; + case DatabaseType.SqlServer: + targetMigration = nameof(MSAddDeploymentColumns); + break; + case DatabaseType.Sqlite: + targetMigration = nameof(SLAddDeploymentColumns); + break; + default: + throw new ArgumentException($"Invalid DatabaseType: {currentDatabaseType}", nameof(currentDatabaseType)); + } - if (version < new Version(4, 1, 0)) - throw new NotSupportedException("Cannot migrate below version 4.1.0!"); + if (targetVersion < new Version(4, 5, 0)) + switch (currentDatabaseType) + { + case DatabaseType.MariaDB: + case DatabaseType.MySql: + targetMigration = nameof(MYAllowNullDMApi); + break; + case DatabaseType.PostgresSql: + targetMigration = nameof(PGAllowNullDMApi); + break; + case DatabaseType.SqlServer: + targetMigration = nameof(MSAllowNullDMApi); + break; + case DatabaseType.Sqlite: + targetMigration = nameof(SLAllowNullDMApi); + break; + default: + throw new ArgumentException($"Invalid DatabaseType: {currentDatabaseType}", nameof(currentDatabaseType)); + } - if(version < new Version(4, 4, 0)) + if (targetVersion < new Version(4, 4, 0)) switch (currentDatabaseType) { case DatabaseType.MariaDB: @@ -349,14 +430,16 @@ namespace Tgstation.Server.Host.Database targetMigration = nameof(PGCreate); break; case DatabaseType.SqlServer: - case DatabaseType.Sqlite: targetMigration = nameof(MSRemoveSoftColumns); break; + case DatabaseType.Sqlite: + targetMigration = nameof(SLRemoveSoftColumns); + break; default: throw new ArgumentException($"Invalid DatabaseType: {currentDatabaseType}", nameof(currentDatabaseType)); } - if (version < new Version(4, 2, 0)) + if (targetVersion < new Version(4, 2, 0)) targetMigration = currentDatabaseType == DatabaseType.Sqlite ? nameof(SLRebuild) : nameof(MSFixCascadingDelete); if (targetMigration == null) @@ -393,7 +476,7 @@ namespace Tgstation.Server.Host.Database var dbServiceProvider = ((IInfrastructure)Database).Instance; var migrator = dbServiceProvider.GetRequiredService(); - logger.LogInformation("Migrating down to version {0}. Target: {1}", version, targetMigration); + logger.LogInformation("Migrating down to version {0}. Target: {1}", targetVersion, targetMigration); try { await migrator.MigrateAsync(targetMigration, cancellationToken).ConfigureAwait(false); diff --git a/src/Tgstation.Server.Host/Database/DatabaseSeeder.cs b/src/Tgstation.Server.Host/Database/DatabaseSeeder.cs index 962e88e1df..44b598ca8e 100644 --- a/src/Tgstation.Server.Host/Database/DatabaseSeeder.cs +++ b/src/Tgstation.Server.Host/Database/DatabaseSeeder.cs @@ -295,7 +295,11 @@ namespace Tgstation.Server.Host.Database if (downgradeVersion == null) throw new ArgumentNullException(nameof(downgradeVersion)); - return databaseContext.SchemaDowngradeForServerVersion(databaseLogger, downgradeVersion, databaseConfiguration.DatabaseType, cancellationToken); + return databaseContext.SchemaDowngradeForServerVersion( + databaseLogger, + downgradeVersion, + databaseConfiguration.DatabaseType, + cancellationToken); } } } diff --git a/src/Tgstation.Server.Host/Database/IDatabaseContext.cs b/src/Tgstation.Server.Host/Database/IDatabaseContext.cs index b7679599cc..b5f6809d79 100644 --- a/src/Tgstation.Server.Host/Database/IDatabaseContext.cs +++ b/src/Tgstation.Server.Host/Database/IDatabaseContext.cs @@ -1,4 +1,4 @@ -using Microsoft.EntityFrameworkCore; +using Microsoft.EntityFrameworkCore; using Microsoft.Extensions.Logging; using System; using System.Threading; @@ -96,16 +96,16 @@ namespace Tgstation.Server.Host.Database Task Migrate(ILogger logger, CancellationToken cancellationToken); /// - /// Attempt to downgrade the schema to the migration used for a given server + /// Attempt to downgrade the schema to the migration used for a given server /// /// The to use. - /// The tgstation-server that the schema should downgrade for + /// The tgstation-server that the schema should downgrade for /// The in use. /// The for the operation /// A representing the running operation Task SchemaDowngradeForServerVersion( ILogger logger, - Version version, + Version targetVersion, DatabaseType currentDatabaseType, CancellationToken cancellationToken); } diff --git a/tests/Tgstation.Server.Tests/VersionsTest.cs b/tests/Tgstation.Server.Tests/VersionsTest.cs index 0d1eec02a9..f6502984be 100644 --- a/tests/Tgstation.Server.Tests/VersionsTest.cs +++ b/tests/Tgstation.Server.Tests/VersionsTest.cs @@ -1,3 +1,4 @@ +using Microsoft.EntityFrameworkCore.Migrations; using Microsoft.VisualStudio.TestTools.UnitTesting; using Newtonsoft.Json.Linq; using System; @@ -10,6 +11,7 @@ using Tgstation.Server.Client; using Tgstation.Server.Host; using Tgstation.Server.Host.Components.Interop; using Tgstation.Server.Host.Configuration; +using Tgstation.Server.Host.Database; namespace Tgstation.Server.Tests { @@ -143,5 +145,62 @@ namespace Tgstation.Server.Tests var line = scriptLines.FirstOrDefault(x => x.Trim().Contains($"SCRIPT_VERSION=\"{expected.Semver()}\"")); Assert.IsNotNull(line); } + +#if DEBUG + [TestMethod] + public void TestDowngradeMigrations() + { + static string GetMigrationTimestampString(Type type) => type + ?.GetCustomAttributes(typeof(MigrationAttribute), false) + .OfType() + .SingleOrDefault() + ?.Id + .Split('_') + .First() + ?? String.Empty; + + var allTypesWithMigrationAttributes = typeof(Program) + .Assembly + .GetTypes() + .ToDictionary( + x => x, + x => GetMigrationTimestampString(x)); + + Type latestMigrationMS = null; + Type latestMigrationMY = null; + Type latestMigrationPG = null; + Type latestMigrationSL = null; + foreach (var kvp in allTypesWithMigrationAttributes) + { + var migrationType = kvp.Key; + var migrationTimestamp = kvp.Value; + + switch(migrationType.Name.Substring(0, 2)) + { + case "MS": + if (String.Compare(GetMigrationTimestampString(latestMigrationMS), migrationTimestamp) < 0) + latestMigrationMS = migrationType; + break; + case "MY": + if (String.Compare(GetMigrationTimestampString(latestMigrationMY), migrationTimestamp) < 0) + latestMigrationMY = migrationType; + break; + case "PG": + if (String.Compare(GetMigrationTimestampString(latestMigrationPG), migrationTimestamp) < 0) + latestMigrationPG = migrationType; + break; + case "SL": + if (String.Compare(GetMigrationTimestampString(latestMigrationSL), migrationTimestamp) < 0) + latestMigrationSL = migrationType; + break; + } + } + + Assert.AreEqual(latestMigrationMS, DatabaseContext.MSLatestMigration); + Assert.AreEqual(latestMigrationMY, DatabaseContext.MYLatestMigration); + Assert.AreEqual(latestMigrationPG, DatabaseContext.PGLatestMigration); + Assert.AreEqual(latestMigrationSL, DatabaseContext.SLLatestMigration); + } +#endif } } From 774a5ba3319493ff11e2fb0c32368727c2f9289b Mon Sep 17 00:00:00 2001 From: Jordan Brown Date: Wed, 9 Dec 2020 15:50:45 -0500 Subject: [PATCH 4/6] Version bump to 4.6.2 --- build/Version.props | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/build/Version.props b/build/Version.props index 306f7948c9..43a29c4998 100644 --- a/build/Version.props +++ b/build/Version.props @@ -3,7 +3,7 @@ - 4.6.1 + 4.6.2 2.1.1 7.4.0 8.4.0 From db7b1cc2425780b5a9afef35dd3b20c518926126 Mon Sep 17 00:00:00 2001 From: Jordan Brown Date: Wed, 9 Dec 2020 16:21:52 -0500 Subject: [PATCH 5/6] Disable CA1502 on SchemaDowngradeForServerVersion --- src/Tgstation.Server.Host/Database/DatabaseContext.cs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/Tgstation.Server.Host/Database/DatabaseContext.cs b/src/Tgstation.Server.Host/Database/DatabaseContext.cs index 1a1aa6c0e8..23d07da4e2 100644 --- a/src/Tgstation.Server.Host/Database/DatabaseContext.cs +++ b/src/Tgstation.Server.Host/Database/DatabaseContext.cs @@ -338,6 +338,7 @@ namespace Tgstation.Server.Host.Database #endif /// +#pragma warning disable CA1502 // TODO: Decomplexify public async Task SchemaDowngradeForServerVersion( ILogger logger, Version targetVersion, @@ -486,5 +487,6 @@ namespace Tgstation.Server.Host.Database logger.LogCritical(e, "Failed to migrate!"); } } +#pragma warning restore CA1502 } } From 2e56b2a5009f00ba5999c0e09397e7ef731b8e7c Mon Sep 17 00:00:00 2001 From: Jordan Brown Date: Wed, 9 Dec 2020 22:34:41 -0500 Subject: [PATCH 6/6] Reduces PortAllocator logspam --- .../Core/PortAllocator.cs | 53 +++++++++++-------- 1 file changed, 31 insertions(+), 22 deletions(-) diff --git a/src/Tgstation.Server.Host/Core/PortAllocator.cs b/src/Tgstation.Server.Host/Core/PortAllocator.cs index 4181334204..8ff8feba81 100644 --- a/src/Tgstation.Server.Host/Core/PortAllocator.cs +++ b/src/Tgstation.Server.Host/Core/PortAllocator.cs @@ -1,6 +1,7 @@ using Microsoft.EntityFrameworkCore; using Microsoft.Extensions.Logging; using System; +using System.Collections.Generic; using System.Linq; using System.Threading; using System.Threading.Tasks; @@ -59,33 +60,41 @@ namespace Tgstation.Server.Host.Core .ToListAsync(cancellationToken) .ConfigureAwait(false); - for (var I = basePort; I < UInt16.MaxValue; ++I) + var exceptions = new List(); + ushort I = 0; + try { - if (checkOne && I != basePort) - break; - - if (I == serverPortProvider.HttpApiPort - || ddPorts.Contains(I) - || dmPorts.Contains(I)) - continue; - - try + for (I = basePort; I < UInt16.MaxValue; ++I) { - logger.LogTrace("Bind test: {0}", I); - SocketExtensions.BindTest(I, false); - } - catch (Exception ex) - { - logger.LogDebug(ex, "Not using port {0}", I); - continue; + if (checkOne && I != basePort) + break; + + if (I == serverPortProvider.HttpApiPort + || ddPorts.Contains(I) + || dmPorts.Contains(I)) + continue; + + try + { + SocketExtensions.BindTest(I, false); + } + catch (Exception ex) + { + exceptions.Add(ex); + continue; + } + + logger.LogInformation("Allocated port {0}", I); + return I; } - logger.LogInformation("Allocated port {0}", I); - return I; + logger.LogWarning("Unable to allocate port >= {0}!", basePort); + return null; + } + finally + { + logger.LogDebug(new AggregateException(exceptions), "Failed to allocate ports {0}-{1}!", basePort, I - 1); } - - logger.LogWarning("Unable to allocate port >= {0}!", basePort); - return null; } } }