More API sanity for EngineVersions

src/Tgstation.Server.Api/Models/Request/ByondVersionDeleteRequest.cs

@@ -1,13 +1,15 @@
-using System;
-
-using Tgstation.Server.Api.Models.Internal;
+using Tgstation.Server.Api.Models.Internal;
 
 namespace Tgstation.Server.Api.Models.Request
 {
 	/// <summary>
-	/// A request to delete a specific <see cref="Version"/>.
+	/// A request to delete a specific <see cref="EngineVersion"/>.
 	/// </summary>
-	public class ByondVersionDeleteRequest : EngineVersion
+	public class ByondVersionDeleteRequest
 	{
+		/// <summary>
+		/// The <see cref="Internal.EngineVersion"/> to delete.
+		/// </summary>
+		public EngineVersion? EngineVersion { get; set; }
 	}
 }

src/Tgstation.Server.Api/Models/Request/ByondVersionRequest.cs

@@ -3,10 +3,15 @@
 namespace Tgstation.Server.Api.Models.Request
 {
 	/// <summary>
-	/// A request to install a <see cref="EngineVersion"/>.
+	/// A request to switch to a given <see cref="EngineVersion"/>.
 	/// </summary>
-	public sealed class ByondVersionRequest : EngineVersion
+	public sealed class ByondVersionRequest
 	{
+		/// <summary>
+		/// The <see cref="Internal.EngineVersion"/> to switch to.
+		/// </summary>
+		public EngineVersion? EngineVersion { get; set; }
+
 		/// <summary>
 		/// If a custom BYOND version is to be uploaded.
 		/// </summary>

src/Tgstation.Server.Host/Controllers/ByondController.cs

@@ -153,7 +153,8 @@ namespace Tgstation.Server.Host.Controllers
 #pragma warning restore CA1506
 #pragma warning restore CA1502
 		{
-			var earlyOut = ValidateByondVersion(model);
+			ArgumentNullException.ThrowIfNull(model);
+			var earlyOut = ValidateEngineVersion(model.EngineVersion);
 			if (earlyOut != null)
 				return earlyOut;
 
@@ -170,7 +171,7 @@ namespace Tgstation.Server.Host.Controllers
 				async instance =>
 				{
 					var byondManager = instance.EngineManager;
-					var versionAlreadyInstalled = !uploadingZip && byondManager.InstalledVersions.Any(x => x.Equals(model));
+					var versionAlreadyInstalled = !uploadingZip && byondManager.InstalledVersions.Any(x => x.Equals(model.EngineVersion));
 					if (versionAlreadyInstalled)
 					{
 						Logger.LogInformation(
@@ -181,7 +182,7 @@ namespace Tgstation.Server.Host.Controllers
 
 						try
 						{
-							await byondManager.ChangeVersion(null, model, null, false, cancellationToken);
+							await byondManager.ChangeVersion(null, model.EngineVersion, null, false, cancellationToken);
 						}
 						catch (InvalidOperationException ex)
 						{
@@ -195,7 +196,7 @@ namespace Tgstation.Server.Host.Controllers
 
 					if (!versionAlreadyInstalled)
 					{
-						if (model.CustomIteration.HasValue)
+						if (model.EngineVersion.CustomIteration.HasValue)
 							return BadRequest(new ErrorMessageResponse(ErrorCode.EngineNonExistentCustomVersion));
 
 						Logger.LogInformation(
@@ -207,7 +208,7 @@ namespace Tgstation.Server.Host.Controllers
 						// run the install through the job manager
 						var job = new Models.Job
 						{
-							Description = $"Install {(!uploadingZip ? String.Empty : "custom ")}{model.Engine.Value} version {model.Version}",
+							Description = $"Install {(!uploadingZip ? String.Empty : "custom ")}{model.EngineVersion.Engine.Value} version {model.EngineVersion.Version}",
 							StartedBy = AuthenticationContext.User,
 							CancelRightsType = RightsType.Byond,
 							CancelRight = (ulong)ByondRights.CancelInstall,
@@ -244,7 +245,7 @@ namespace Tgstation.Server.Host.Controllers
 									await using (zipFileStream)
 										await core.EngineManager.ChangeVersion(
 											progressHandler,
-											model,
+											model.EngineVersion,
 											zipFileStream,
 											true,
 											jobCancellationToken);
@@ -283,7 +284,8 @@ namespace Tgstation.Server.Host.Controllers
 		[ProducesResponseType(typeof(ErrorMessageResponse), 410)]
 		public async ValueTask<IActionResult> Delete([FromBody] ByondVersionDeleteRequest model, CancellationToken cancellationToken)
 		{
-			var earlyOut = ValidateByondVersion(model);
+			ArgumentNullException.ThrowIfNull(model);
+			var earlyOut = ValidateEngineVersion(model.EngineVersion);
 			if (earlyOut != null)
 				return earlyOut;
 
@@ -292,11 +294,11 @@ namespace Tgstation.Server.Host.Controllers
 				{
 					var byondManager = instance.EngineManager;
 
-					if (model.Equals(byondManager.ActiveVersion))
+					if (model.EngineVersion.Equals(byondManager.ActiveVersion))
 						return ValueTask.FromResult<IActionResult>(
 							Conflict(new ErrorMessageResponse(ErrorCode.EngineCannotDeleteActiveVersion)));
 
-					var versionNotInstalled = !byondManager.InstalledVersions.Any(x => x.Equals(model));
+					var versionNotInstalled = !byondManager.InstalledVersions.Any(x => x.Equals(model.EngineVersion));
 
 					return ValueTask.FromResult<IActionResult>(
 						versionNotInstalled
@@ -307,7 +309,7 @@ namespace Tgstation.Server.Host.Controllers
 			if (notInstalledResponse != null)
 				return notInstalledResponse;
 
-			var isByondVersion = model.Engine.Value == EngineType.Byond;
+			var isByondVersion = model.EngineVersion.Engine.Value == EngineType.Byond;
 
 			// run the install through the job manager
 			var job = new Models.Job
@@ -317,7 +319,7 @@ namespace Tgstation.Server.Host.Controllers
 				CancelRightsType = RightsType.Byond,
 				CancelRight = (ulong)(
 					isByondVersion
-						? model.Version.Build != -1
+						? model.EngineVersion.Version.Build != -1
 							? ByondRights.InstallOfficialOrChangeActiveByondVersion
 							: ByondRights.InstallCustomByondVersion
 						: ByondRights.InstallCustomOpenDreamVersion | ByondRights.InstallOfficialOrChangeActiveOpenDreamVersion),
@@ -327,7 +329,7 @@ namespace Tgstation.Server.Host.Controllers
 			await jobManager.RegisterOperation(
 				job,
 				(instanceCore, databaseContextFactory, job, progressReporter, jobCancellationToken)
-					=> instanceCore.EngineManager.DeleteVersion(progressReporter, model, jobCancellationToken),
+					=> instanceCore.EngineManager.DeleteVersion(progressReporter, model.EngineVersion, jobCancellationToken),
 				cancellationToken);
 
 			var apiResponse = job.ToApi();
@@ -339,11 +341,9 @@ namespace Tgstation.Server.Host.Controllers
 		/// </summary>
 		/// <param name="version">The <see cref="EngineVersion"/> to validate and normalize.</param>
 		/// <returns>The <see cref="BadRequestObjectResult"/> to return, if any.</returns>
-		BadRequestObjectResult ValidateByondVersion(EngineVersion version)
+		BadRequestObjectResult ValidateEngineVersion(EngineVersion version)
 		{
-			ArgumentNullException.ThrowIfNull(version);
-
-			if (!version.Engine.HasValue)
+			if (version == null || !version.Engine.HasValue)
 				return BadRequest(new ErrorMessageResponse(ErrorCode.ModelValidationFailure));
 
 			var isByond = version.Engine.Value == EngineType.Byond;

tests/Tgstation.Server.Client.Tests/TestApiClient.cs

@@ -51,7 +51,7 @@ namespace Tgstation.Server.Client.Tests
 
 			var result = await client.Read<ByondResponse>(Routes.Byond, default);
 			Assert.AreEqual(sample.EngineVersion, result.EngineVersion);
-			Assert.AreEqual(0, result.EngineVersion.Version.Build); // sucks but we can't do better really
+			Assert.AreEqual(-1, result.EngineVersion.Version.Build);
 			Assert.IsFalse(result.EngineVersion.CustomIteration.HasValue);
 		}
 

tests/Tgstation.Server.Tests/Live/Instance/ByondTest.cs

@@ -114,7 +114,10 @@ namespace Tgstation.Server.Tests.Live.Instance
 				() => byondClient.SetActiveVersion(
 					new ByondVersionRequest
 					{
-						Engine = testEngine,
+						EngineVersion = new EngineVersion
+						{
+							Engine = testEngine,
+						}
 					},
 					null,
 					cancellationToken),
@@ -132,36 +135,48 @@ namespace Tgstation.Server.Tests.Live.Instance
 		{
 			var deleteThisOneBecauseItWasntPartOfTheOriginalTest = await byondClient.DeleteVersion(new ByondVersionDeleteRequest
 			{
-				Engine = testEngine,
-				Version = testVersion.Version,
-				CustomIteration = 2,
+				EngineVersion = new EngineVersion
+				{
+					Engine = testEngine,
+					Version = testVersion.Version,
+					CustomIteration = 2,
+				}
 			}, cancellationToken);
 			await WaitForJob(deleteThisOneBecauseItWasntPartOfTheOriginalTest, 30, false, null, cancellationToken);
 
 			var nonExistentUninstallResponseTask = ApiAssert.ThrowsException<ConflictException, JobResponse>(() => byondClient.DeleteVersion(
 				new ByondVersionDeleteRequest
 				{
-					Version = new(509, 1000),
-					Engine = testEngine,
+					EngineVersion = new EngineVersion
+					{
+						Version = new(509, 1000),
+						Engine = testEngine,
+					}
 				},
 				cancellationToken), ErrorCode.ResourceNotPresent);
 
 			var uninstallResponseTask = byondClient.DeleteVersion(
 				new ByondVersionDeleteRequest
 				{
-					Version = testVersion.Version,
-					Engine = testVersion.Engine,
-					SourceSHA = testVersion.SourceSHA,
+					EngineVersion = new EngineVersion
+					{
+						Version = testVersion.Version,
+						Engine = testVersion.Engine,
+						SourceSHA = testVersion.SourceSHA,
+					}
 				},
 				cancellationToken);
 
 			var badBecauseActiveResponseTask = ApiAssert.ThrowsException<ConflictException, JobResponse>(() => byondClient.DeleteVersion(
 				new ByondVersionDeleteRequest
 				{
-					Version = testVersion.Version,
-					Engine = testVersion.Engine,
-					SourceSHA = testVersion.SourceSHA,
-					CustomIteration = 1,
+					EngineVersion = new EngineVersion
+					{
+						Version = testVersion.Version,
+						Engine = testVersion.Engine,
+						SourceSHA = testVersion.SourceSHA,
+						CustomIteration = 1,
+					}
 				},
 				cancellationToken), ErrorCode.EngineCannotDeleteActiveVersion);
 
@@ -190,12 +205,15 @@ namespace Tgstation.Server.Tests.Live.Instance
 		{
 			var newModel = new ByondVersionRequest
 			{
-				Version = new Version(5011, 1385)
+				EngineVersion = new EngineVersion
+				{
+					Version = new Version(5011, 1385),
+				}
 			};
 
 			await ApiAssert.ThrowsException<ApiConflictException, ByondInstallResponse>(() => byondClient.SetActiveVersion(newModel, null, cancellationToken), ErrorCode.ModelValidationFailure);
 
-			newModel.Engine = testEngine;
+			newModel.EngineVersion.Engine = testEngine;
 
 			var test = await byondClient.SetActiveVersion(newModel, null, cancellationToken);
 			Assert.IsNotNull(test.InstallJob);
@@ -206,22 +224,25 @@ namespace Tgstation.Server.Tests.Live.Instance
 		{
 			var newModel = new ByondVersionRequest
 			{
-				Version = testVersion.Version,
-				Engine = testVersion.Engine,
-				SourceSHA = testVersion.SourceSHA,
+				EngineVersion = new EngineVersion
+				{
+					Version = testVersion.Version,
+					Engine = testVersion.Engine,
+					SourceSHA = testVersion.SourceSHA,
+				}
 			};
 			var test = await byondClient.SetActiveVersion(newModel, null, cancellationToken);
 			Assert.IsNotNull(test.InstallJob);
 			await WaitForJob(test.InstallJob, 180, false, null, cancellationToken);
 			var currentShit = await byondClient.ActiveVersion(cancellationToken);
-			Assert.AreEqual(newModel, currentShit.EngineVersion);
+			Assert.AreEqual(newModel.EngineVersion, currentShit.EngineVersion);
 			Assert.IsFalse(currentShit.EngineVersion.CustomIteration.HasValue);
 
 			var dreamMaker = "DreamMaker";
 			if (new PlatformIdentifier().IsWindows)
 				dreamMaker += ".exe";
 
-			var dreamMakerDir = Path.Combine(metadata.Path, "Byond", newModel.Version.ToString(), "byond", "bin");
+			var dreamMakerDir = Path.Combine(metadata.Path, "Byond", newModel.EngineVersion.Version.ToString(), "byond", "bin");
 
 			Assert.IsTrue(Directory.Exists(dreamMakerDir), $"Directory {dreamMakerDir} does not exist!");
 			Assert.IsTrue(
@@ -271,10 +292,13 @@ namespace Tgstation.Server.Tests.Live.Instance
 			var test = await byondClient.SetActiveVersion(
 				new ByondVersionRequest
 				{
-					Engine = testVersion.Engine,
-					Version = testVersion.Version,
-					SourceSHA = testVersion.SourceSHA,
-					UploadCustomZip = true
+					EngineVersion = new EngineVersion
+					{
+						Engine = testVersion.Engine,
+						Version = testVersion.Version,
+						SourceSHA = testVersion.SourceSHA,
+					},
+					UploadCustomZip = true,
 				},
 				stableBytesMs,
 				cancellationToken);
@@ -287,10 +311,14 @@ namespace Tgstation.Server.Tests.Live.Instance
 			var test2 = await byondClient.SetActiveVersion(
 				new ByondVersionRequest
 				{
-					Version = testVersion.Version,
-					SourceSHA = testVersion.SourceSHA,
-					Engine = testVersion.Engine,
-					UploadCustomZip = true
+					EngineVersion = new EngineVersion
+					{
+						Version = testVersion.Version,
+						SourceSHA = testVersion.SourceSHA,
+						Engine = testVersion.Engine,
+					},
+					UploadCustomZip = true,
+
 				},
 				stableBytesMs,
 				cancellationToken);
@@ -305,23 +333,32 @@ namespace Tgstation.Server.Tests.Live.Instance
 			// test a few switches
 			var installResponse = await byondClient.SetActiveVersion(new ByondVersionRequest
 			{
-				Version = testVersion.Version,
-				SourceSHA = testVersion.SourceSHA,
-				Engine = testVersion.Engine,
+				EngineVersion = new EngineVersion
+				{
+					Version = testVersion.Version,
+					SourceSHA = testVersion.SourceSHA,
+					Engine = testVersion.Engine,
+				}
 			}, null, cancellationToken);
 			Assert.IsNull(installResponse.InstallJob);
 			await ApiAssert.ThrowsException<ApiConflictException, ByondInstallResponse>(() => byondClient.SetActiveVersion(new ByondVersionRequest
 			{
-				Version = testVersion.Version,
-				Engine = testEngine,
-				CustomIteration = 3,
+				EngineVersion = new EngineVersion
+				{
+					Version = testVersion.Version,
+					Engine = testEngine,
+					CustomIteration = 3,
+				}
 			}, null, cancellationToken), ErrorCode.EngineNonExistentCustomVersion);
 
 			installResponse = await byondClient.SetActiveVersion(new ByondVersionRequest
 			{
-				Version = new Version(testVersion.Version.Major, testVersion.Version.Minor),
-				Engine = testEngine,
-				CustomIteration = 1,
+				EngineVersion = new EngineVersion
+				{
+					Version = new Version(testVersion.Version.Major, testVersion.Version.Minor),
+					Engine = testEngine,
+					CustomIteration = 1,
+				}
 			}, null, cancellationToken);
 			Assert.IsNull(installResponse.InstallJob);
 		}

tests/Tgstation.Server.Tests/Live/Instance/InstanceTest.cs

@@ -182,9 +182,12 @@ namespace Tgstation.Server.Tests.Live.Instance
 				installJob2 = await instanceClient.Byond.SetActiveVersion(new ByondVersionRequest
 				{
 					UploadCustomZip = true,
-					Version = compatVersion.Version,
-					Engine = compatVersion.Engine,
-					SourceSHA = compatVersion.SourceSHA
+					EngineVersion = new EngineVersion
+					{
+						Version = compatVersion.Version,
+						Engine = compatVersion.Engine,
+						SourceSHA = compatVersion.SourceSHA,
+					},
 				}, stableBytesMs, cancellationToken);
 			}
 

tests/Tgstation.Server.Tests/Live/Instance/WatchdogTest.cs

@@ -250,9 +250,12 @@ namespace Tgstation.Server.Tests.Live.Instance
 			var setActiveResponse = await instanceClient.Byond.SetActiveVersion(
 				new ByondVersionRequest
 				{
-					Version = testCustomVersion,
-					Engine = testVersion.Engine,
-					CustomIteration = testCustomRevision,
+					EngineVersion = new EngineVersion
+					{
+						Version = testCustomVersion,
+						Engine = testVersion.Engine,
+						CustomIteration = testCustomRevision,
+					}
 				},
 				null,
 				cancellationToken);
@@ -263,9 +266,12 @@ namespace Tgstation.Server.Tests.Live.Instance
 			var deleteJob = await instanceClient.Byond.DeleteVersion(
 				new ByondVersionDeleteRequest
 				{
-					Version = testVersion.Version,
-					SourceSHA = testVersion.SourceSHA,
-					Engine = testVersion.Engine,
+					EngineVersion = new EngineVersion
+					{
+						Version = testVersion.Version,
+						SourceSHA = testVersion.SourceSHA,
+						Engine = testVersion.Engine,
+					}
 				},
 				cancellationToken);
 
@@ -280,9 +286,12 @@ namespace Tgstation.Server.Tests.Live.Instance
 			setActiveResponse = await instanceClient.Byond.SetActiveVersion(
 				new ByondVersionRequest
 				{
-					Version = testVersion.Version,
-					Engine = testVersion.Engine,
-					SourceSHA = testVersion.SourceSHA
+					EngineVersion = new EngineVersion
+					{
+						Version = testVersion.Version,
+						Engine = testVersion.Engine,
+						SourceSHA = testVersion.SourceSHA
+					}
 				},
 				null,
 				cancellationToken);
@@ -297,9 +306,12 @@ namespace Tgstation.Server.Tests.Live.Instance
 			setActiveResponse = await instanceClient.Byond.SetActiveVersion(
 				new ByondVersionRequest
 				{
-					Version = testCustomVersion,
-					Engine = testVersion.Engine,
-					CustomIteration = testCustomRevision,
+					EngineVersion = new EngineVersion
+					{
+						Version = testCustomVersion,
+						Engine = testVersion.Engine,
+						CustomIteration = testCustomRevision,
+					}
 				},
 				null,
 				cancellationToken);
@@ -310,9 +322,12 @@ namespace Tgstation.Server.Tests.Live.Instance
 			deleteJob = await instanceClient.Byond.DeleteVersion(
 				new ByondVersionDeleteRequest
 				{
-					Version = testVersion.Version,
-					Engine = testVersion.Engine,
-					SourceSHA = testVersion.SourceSHA,
+					EngineVersion = new EngineVersion
+					{
+						Version = testVersion.Version,
+						Engine = testVersion.Engine,
+						SourceSHA = testVersion.SourceSHA,
+					}
 				},
 				cancellationToken);
 
@@ -989,9 +1004,12 @@ namespace Tgstation.Server.Tests.Live.Instance
 			var byondInstallJobTask = instanceClient.Byond.SetActiveVersion(
 				new ByondVersionRequest
 				{
-					Version = versionToInstall.Version,
-					Engine = versionToInstall.Engine,
-					SourceSHA = versionToInstall.SourceSHA,
+					EngineVersion = new EngineVersion
+					{
+						Version = versionToInstall.Version,
+						Engine = versionToInstall.Engine,
+						SourceSHA = versionToInstall.SourceSHA,
+					}
 				},
 				null,
 				cancellationToken);

tools/Tgstation.Server.Migrator.Comms/Program.cs

@@ -230,7 +230,10 @@ static class Program
 				Console.WriteLine($"Found installed BYOND version: {byondVersion.Major}.{byondVersion.Minor}");
 				byondVersionRequest = new ByondVersionRequest
 				{
-					Version = byondVersion
+					EngineVersion = new EngineVersion
+					{
+						Version = byondVersion
+					}
 				};
 			}