Merge pull request #589 from Cyberboss/REEE

Fix things and code coverage

appveyor.yml

@@ -25,7 +25,7 @@ cache:
   - C:\ProgramData\chocolatey\bin -> appveyor.yml
   - C:\ProgramData\chocolatey\lib -> appveyor.yml
 install:
-  - choco install doxygen.portable codecov graphviz.portable
+  - choco install doxygen.portable codecov graphviz.portable opencover.portable
   - nuget restore tgstation-server.sln
 build:
   project: tgstation-server.sln
@@ -33,29 +33,29 @@ build:
   verbosity: minimal
   publish_nuget: true
 test_script:
+  - OpenCover.Console.exe -register:user -target:"C:/Program Files/dotnet/dotnet.exe" -targetargs:"test -c %CONFIGURATION% --logger:trx;LogFileName=results.trx /p:DebugType=full tests/Tgstation.Server.Api.Tests/Tgstation.Server.Api.Tests.csproj" -filter:"+[Tgstation.Server*]* -[Tgstation.Server.Api.Tests*]*" -output:".\api_coverage.xml" -oldstyle
+  - ps: $wc = New-Object 'System.Net.WebClient'
+  - ps: $wc.UploadFile("https://ci.appveyor.com/api/testresults/mstest/$($env:APPVEYOR_JOB_ID)", (Resolve-Path .\tests\Tgstation.Server.Api.Tests\TestResults\results.trx))
+  - OpenCover.Console.exe -register:user -target:"C:/Program Files/dotnet/dotnet.exe" -targetargs:"test -c %CONFIGURATION% --logger:trx;LogFileName=results.trx /p:DebugType=full tests/Tgstation.Server.Client.Tests/Tgstation.Server.Client.Tests.csproj" -filter:"+[Tgstation.Server*]* -[Tgstation.Server.Client.Tests*]*" -output:".\client_coverage.xml" -oldstyle
+  - ps: $wc = New-Object 'System.Net.WebClient'
+  - ps: $wc.UploadFile("https://ci.appveyor.com/api/testresults/mstest/$($env:APPVEYOR_JOB_ID)", (Resolve-Path .\tests\Tgstation.Server.Client.Tests\TestResults\results.trx))
+  - OpenCover.Console.exe -register:user -target:"C:/Program Files/dotnet/dotnet.exe" -targetargs:"test -c %CONFIGURATION% --logger:trx;LogFileName=results.trx /p:DebugType=full tests/Tgstation.Server.Host.Tests/Tgstation.Server.Host.Tests.csproj" -filter:"+[Tgstation.Server*]* -[Tgstation.Server.Host.Tests*]*" -output:".\host_coverage.xml" -oldstyle
+  - ps: $wc = New-Object 'System.Net.WebClient'
+  - ps: $wc.UploadFile("https://ci.appveyor.com/api/testresults/mstest/$($env:APPVEYOR_JOB_ID)", (Resolve-Path .\tests\Tgstation.Server.Host.Tests\TestResults\results.trx))
+  - OpenCover.Console.exe -register:user -target:"C:/Program Files/dotnet/dotnet.exe" -targetargs:"test -c %CONFIGURATION% --logger:trx;LogFileName=results.trx /p:DebugType=full tests/Tgstation.Server.Host.Console.Tests/Tgstation.Server.Host.Console.Tests.csproj" -filter:"+[Tgstation.Server*]* -[Tgstation.Server.Host.Console.Tests*]*" -output:".\console_coverage.xml" -oldstyle
+  - ps: $wc = New-Object 'System.Net.WebClient'
+  - ps: $wc.UploadFile("https://ci.appveyor.com/api/testresults/mstest/$($env:APPVEYOR_JOB_ID)", (Resolve-Path .\tests\Tgstation.Server.Host.Console.Tests\TestResults\results.trx))
   - set path=%ProgramFiles(x86)%\Microsoft Visual Studio\2017\TestAgent\Common7\IDE\CommonExtensions\Microsoft\TestWindow;%path%
   - copy "%ProgramFiles(x86)%\Microsoft Visual Studio\2017\Community\Common7\IDE\CommonExtensions\Microsoft\TestWindow\Extensions\appveyor.*" "%ProgramFiles(x86)%\Microsoft Visual Studio\2017\TestAgent\Common7\IDE\CommonExtensions\Microsoft\TestWindow\Extensions" /y
-  - vstest.console /logger:trx;LogFileName=results.trx "tests\Tgstation.Server.Api.Tests\bin\%CONFIGURATION%\netcoreapp2.0\Tgstation.Server.Api.Tests.dll" /Enablecodecoverage /inIsolation /Platform:x64
-  - ps: $wc = New-Object 'System.Net.WebClient'
-  - ps: $wc.UploadFile("https://ci.appveyor.com/api/testresults/mstest/$($env:APPVEYOR_JOB_ID)", (Resolve-Path .\TestResults\results.trx))
-  - vstest.console /logger:trx;LogFileName=results.trx "tests\Tgstation.Server.Client.Tests\bin\%CONFIGURATION%\netcoreapp2.0\Tgstation.Server.Client.Tests.dll" /Enablecodecoverage /inIsolation /Platform:x64
-  - ps: $wc = New-Object 'System.Net.WebClient'
-  - ps: $wc.UploadFile("https://ci.appveyor.com/api/testresults/mstest/$($env:APPVEYOR_JOB_ID)", (Resolve-Path .\TestResults\results.trx))
-  - vstest.console /logger:trx;LogFileName=results.trx "tests\Tgstation.Server.Host.Tests\bin\%CONFIGURATION%\netcoreapp2.0\Tgstation.Server.Host.Tests.dll" /Enablecodecoverage /inIsolation /Platform:x64
-  - ps: $wc = New-Object 'System.Net.WebClient'
-  - ps: $wc.UploadFile("https://ci.appveyor.com/api/testresults/mstest/$($env:APPVEYOR_JOB_ID)", (Resolve-Path .\TestResults\results.trx))
-  - vstest.console /logger:trx;LogFileName=results.trx "tests\Tgstation.Server.Host.Console.Tests\bin\%CONFIGURATION%\netcoreapp2.0\Tgstation.Server.Host.Console.Tests.dll" /Enablecodecoverage /inIsolation /Platform:x64
-  - ps: $wc = New-Object 'System.Net.WebClient'
-  - ps: $wc.UploadFile("https://ci.appveyor.com/api/testresults/mstest/$($env:APPVEYOR_JOB_ID)", (Resolve-Path .\TestResults\results.trx))
   - vstest.console /logger:trx;LogFileName=results.trx "tests\Tgstation.Server.Host.Service.Tests\bin\%CONFIGURATION%\Tgstation.Server.Host.Service.Tests.dll" /Enablecodecoverage /inIsolation /Platform:x64
   - ps: $wc = New-Object 'System.Net.WebClient'
   - ps: $wc.UploadFile("https://ci.appveyor.com/api/testresults/mstest/$($env:APPVEYOR_JOB_ID)", (Resolve-Path .\TestResults\results.trx))
-  - vstest.console /logger:trx;LogFileName=results.trx "tests\Tgstation.Server.Host.Watchdog.Tests\bin\%CONFIGURATION%\netcoreapp2.0\Tgstation.Server.Host.Watchdog.Tests.dll" /Enablecodecoverage /inIsolation /Platform:x64
+  - OpenCover.Console.exe -register:user -target:"C:/Program Files/dotnet/dotnet.exe" -targetargs:"test -c %CONFIGURATION% --logger:trx;LogFileName=results.trx /p:DebugType=full tests/Tgstation.Server.Host.Watchdog.Tests/Tgstation.Server.Host.Watchdog.Tests.csproj" -filter:"+[Tgstation.Server*]* -[Tgstation.Server.Host.Watchdog.Tests*]*" -output:".\watchdog_coverage.xml" -oldstyle
   - ps: $wc = New-Object 'System.Net.WebClient'
-  - ps: $wc.UploadFile("https://ci.appveyor.com/api/testresults/mstest/$($env:APPVEYOR_JOB_ID)", (Resolve-Path .\TestResults\results.trx))
-  - vstest.console /logger:trx;LogFileName=results.trx "tests\Tgstation.Server.Tests\bin\%CONFIGURATION%\netcoreapp2.0\Tgstation.Server.Tests.dll" /Enablecodecoverage /inIsolation /Platform:x64
+  - ps: $wc.UploadFile("https://ci.appveyor.com/api/testresults/mstest/$($env:APPVEYOR_JOB_ID)", (Resolve-Path .\tests\Tgstation.Server.Host.Watchdog.Tests\TestResults\results.trx))
+  - OpenCover.Console.exe -register:user -target:"C:/Program Files/dotnet/dotnet.exe" -targetargs:"test -c %CONFIGURATION% --logger:trx;LogFileName=results.trx /p:DebugType=full tests/Tgstation.Server.Tests/Tgstation.Server.Tests.csproj" -filter:"+[Tgstation.Server*]* -[Tgstation.Server.Tests*]*" -output:".\server_coverage.xml" -oldstyle
   - ps: $wc = New-Object 'System.Net.WebClient'
-  - ps: $wc.UploadFile("https://ci.appveyor.com/api/testresults/mstest/$($env:APPVEYOR_JOB_ID)", (Resolve-Path .\TestResults\results.trx))
+  - ps: $wc.UploadFile("https://ci.appveyor.com/api/testresults/mstest/$($env:APPVEYOR_JOB_ID)", (Resolve-Path .\tests\Tgstation.Server.Tests\TestResults\results.trx))
 after_test:
   - ps: build/UploadCoverage.ps1
   - ps: build/BuildDox.ps1

build/UploadCoverage.ps1

@@ -1,10 +1,16 @@
-$coverageFilePath = Resolve-Path -path "TestResults\*\*.coverage"
- 
-$coverageFilePath = $coverageFilePath.ToString()
+$coverageFilePaths = Get-ChildItem -Path TestResults -Filter *.coverage -Recurse -ErrorAction SilentlyContinue -Force | %{ $_.fullname }
+
+$coverageFilePathList = [string]$coverageFilePaths
 
 Write-Host "Running CodeCoverage.exe..."
-&"C:\Program Files (x86)\Microsoft Visual Studio\2017\TestAgent\Team Tools\Dynamic Code Coverage Tools\CodeCoverage.exe" analyze /output:coverage.coveragexml "$coverageFilePath"
+&"C:\Program Files (x86)\Microsoft Visual Studio\2017\TestAgent\Team Tools\Dynamic Code Coverage Tools\CodeCoverage.exe" analyze /output:service.coveragexml "$coverageFilePathList"
 
 rm -r TestResults
 
-codecov -f coverage.coveragexml
+codecov -f api_coverage.xml --flag unittests
+codecov -f client_coverage.xml --flag unittests
+codecov -f host_coverage.xml  --flag unittests
+codecov -f console_coverage.xml --flag unittests
+codecov -f watchdog_coverage.xml --flag unittests
+codecov -f service.coveragexml --flag unittests
+codecov -f server_coverage.xml --flag integration

src/Tgstation.Server.Host/Components/Repository/IRepositoryManager.cs

@@ -9,6 +9,11 @@ namespace Tgstation.Server.Host.Components.Repository
 	/// </summary>
 	public interface IRepositoryManager : IDisposable
 	{
+		/// <summary>
+		/// If something is holding a lock on the repository
+		/// </summary>
+		bool InUse { get; }
+
 		/// <summary>
 		/// If a <see cref="CloneRepository(Uri, string, string, string, Action{int}, CancellationToken)"/> operation is in progress
 		/// </summary>

src/Tgstation.Server.Host/Components/Repository/RepositoryManager.cs

@@ -11,6 +11,9 @@ namespace Tgstation.Server.Host.Components.Repository
 	/// <inheritdoc />
 	sealed class RepositoryManager : IRepositoryManager
 	{
+		/// <inheritdoc />
+		public bool InUse => semaphore.CurrentCount == 0;
+
 		/// <inheritdoc />
 		public bool CloneInProgress { get; private set; }
 

src/Tgstation.Server.Host/Components/StaticFiles/Configuration.cs

@@ -216,10 +216,6 @@ namespace Tgstation.Server.Host.Components.StaticFiles
 							Path = configurationRelativePath
 						};
 					}
-					catch (IOException e)
-					{
-						logger.LogDebug("IOException while reading {0}: {1}", path, e);
-					}
 					catch (UnauthorizedAccessException)
 					{
 						//this happens on windows, dunno about linux
@@ -308,10 +304,6 @@ namespace Tgstation.Server.Host.Components.StaticFiles
 							Path = configurationRelativePath
 						};
 					}
-					catch (IOException e)
-					{
-						logger.LogDebug("IOException while writing {0}: {1}", path, e);
-					}
 					catch (UnauthorizedAccessException)
 					{
 						//this happens on windows, dunno about linux

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

@@ -1,6 +1,7 @@
 using Microsoft.AspNetCore.Mvc;
 using Microsoft.Extensions.Logging;
 using System;
+using System.IO;
 using System.Net;
 using System.Threading;
 using System.Threading.Tasks;
@@ -54,12 +55,23 @@ namespace Tgstation.Server.Host.Controllers
 			{
 				var newFile = await config.Write(model.Path, AuthenticationContext.SystemIdentity, model.Content, model.LastReadHash, cancellationToken).ConfigureAwait(false);
 				if (newFile == null)
-					return Conflict();
+					return Conflict(new ErrorMessage
+					{
+						Message = ""
+					});
 
 				newFile.Content = null;
 
 				return model.LastReadHash == null ? (IActionResult)StatusCode((int)HttpStatusCode.Created, newFile) : Json(newFile);
 			}
+			catch(IOException e)
+			{
+				Logger.LogInformation("IOException while updating file {0}: {1}", model.Path, e);
+				return Conflict(new ErrorMessage
+				{
+					Message = e.Message
+				});
+			}
 			catch (NotImplementedException)
 			{
 				return StatusCode((int)HttpStatusCode.NotImplemented);
@@ -87,6 +99,14 @@ namespace Tgstation.Server.Host.Controllers
 
 				return Json(result);
 			}
+			catch (IOException e)
+			{
+				Logger.LogInformation("IOException while reading file {0}: {1}", filePath, e);
+				return Conflict(new ErrorMessage
+				{
+					Message = e.Message
+				});
+			}
 			catch (NotImplementedException)
 			{
 				return StatusCode((int)HttpStatusCode.NotImplemented);

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

@@ -92,7 +92,7 @@ namespace Tgstation.Server.Host.Controllers
 			return needsDbUpdate;
 		}
 
-		static async Task<bool> PopulateApi(Repository model, Components.Repository.IRepository repository, IDatabaseContext databaseContext, Models.Instance instance, string lastOriginCommitSha, Action<Models.RevisionInformation> revInfoSink, CancellationToken cancellationToken)
+		static async Task<bool> PopulateApi(Repository model, Components.Repository.IRepository repository, IDatabaseContext databaseContext, Models.Instance instance, CancellationToken cancellationToken)
 		{
 			model.IsGitHub = repository.IsGitHubRepository;
 			model.Origin = repository.Origin;
@@ -100,9 +100,8 @@ namespace Tgstation.Server.Host.Controllers
 
 			//rev info stuff
 			Models.RevisionInformation revisionInfo = null;
-			var needsDbUpdate = await LoadRevisionInformation(repository, databaseContext, instance, lastOriginCommitSha, x => revisionInfo = x, cancellationToken).ConfigureAwait(false);
+			var needsDbUpdate = await LoadRevisionInformation(repository, databaseContext, instance, null, x => revisionInfo = x, cancellationToken).ConfigureAwait(false);
 			model.RevisionInformation = revisionInfo.ToApi();
-			revInfoSink?.Invoke(revisionInfo);
 			return needsDbUpdate;
 		}
 
@@ -140,13 +139,25 @@ namespace Tgstation.Server.Host.Controllers
 			var repoManager = instanceManager.GetInstance(Instance).RepositoryManager;
 
 			if (repoManager.CloneInProgress)
-				return Conflict();
+				return Conflict(new ErrorMessage
+				{
+					Message = "A clone operation is in progress!"
+				});
+
+			if(repoManager.InUse)
+				return Conflict(new ErrorMessage
+				{
+					Message = "The repo is busy!"
+				});
 
 			using (var repo = await repoManager.LoadRepository(cancellationToken).ConfigureAwait(false))
 			{
 				if (repo != null)
 					//clone conflict
-					return Conflict();
+					return Conflict(new ErrorMessage
+					{
+						Message = "The repository already exists!"
+					});
 
 				var job = new Models.Job
 				{
@@ -164,7 +175,7 @@ namespace Tgstation.Server.Host.Controllers
 						if (repos == null)
 							throw new JobException("Filesystem conflict while cloning repository!");
 						var db = serviceProvider.GetRequiredService<IDatabaseContext>();
-						if (await PopulateApi(api, repos, db, Instance, null, null, cancellationToken).ConfigureAwait(false))
+						if (await PopulateApi(api, repos, db, Instance, cancellationToken).ConfigureAwait(false))
 							await db.Save(cancellationToken).ConfigureAwait(false);
 					}
 				}, cancellationToken).ConfigureAwait(false);
@@ -220,10 +231,23 @@ namespace Tgstation.Server.Host.Controllers
 				return StatusCode((int)HttpStatusCode.Gone);
 
 			var api = currentModel.ToApi();
+			var repoManager = instanceManager.GetInstance(Instance).RepositoryManager;
 
-			using (var repo = await instanceManager.GetInstance(Instance).RepositoryManager.LoadRepository(cancellationToken).ConfigureAwait(false))
+			if (repoManager.CloneInProgress)
+				return Conflict(new ErrorMessage
+				{
+					Message = "A clone operation is in progress!"
+				});
+
+			if (repoManager.InUse)
+				return Conflict(new ErrorMessage
+				{
+					Message = "The repo is busy!"
+				});
+
+			using (var repo = await repoManager.LoadRepository(cancellationToken).ConfigureAwait(false))
 			{
-				if (repo != null && await PopulateApi(api, repo, DatabaseContext, Instance, null, null, cancellationToken).ConfigureAwait(false))
+				if (repo != null && await PopulateApi(api, repo, DatabaseContext, Instance, cancellationToken).ConfigureAwait(false))
 				{
 					//user may have fucked with the repo without telling us, do what we can
 					await DatabaseContext.Save(cancellationToken).ConfigureAwait(false);
@@ -301,6 +325,36 @@ namespace Tgstation.Server.Host.Controllers
 				currentModel.AccessToken = null;
 			}
 
+			var canRead = userRights.HasFlag(RepositoryRights.Read);
+
+			var api = canRead ? currentModel.ToApi() : new Repository();
+			var repoManager = instanceManager.GetInstance(Instance).RepositoryManager;
+
+			if (canRead)
+			{
+				if (repoManager.CloneInProgress)
+					return Conflict(new ErrorMessage
+					{
+						Message = "A clone operation is in progress!"
+					});
+
+				if (repoManager.InUse)
+					return Conflict(new ErrorMessage
+					{
+						Message = "The repo is busy!"
+					});
+
+				using (var repo = await repoManager.LoadRepository(cancellationToken).ConfigureAwait(false))
+				{
+					if (repo == null)
+						return Conflict(new ErrorMessage
+						{
+							Message = "Repository could not be loaded!"
+						});
+					await PopulateApi(api, repo, DatabaseContext, Instance, cancellationToken).ConfigureAwait(false);
+				}
+			}
+						
 			//this is just db stuf so stow it away
 			await DatabaseContext.Save(cancellationToken).ConfigureAwait(false);
 
@@ -315,7 +369,7 @@ namespace Tgstation.Server.Host.Controllers
 
 			await jobManager.RegisterOperation(job, async (paramJob, serviceProvider, progressReporter, ct) =>
 			{
-				using (var repo = await instanceManager.GetInstance(Instance).RepositoryManager.LoadRepository(ct).ConfigureAwait(false))
+				using (var repo = await repoManager.LoadRepository(ct).ConfigureAwait(false))
 				{
 					if (repo == null)
 						throw new JobException("Repository could not be loaded!");
@@ -598,7 +652,8 @@ namespace Tgstation.Server.Host.Controllers
 				}
 			}, cancellationToken).ConfigureAwait(false);
 
-			return Accepted(job.ToApi());
+			api.ActiveJob = job.ToApi();
+			return Accepted(api);
 		}
 	}
 }

src/Tgstation.Server.Host/Core/SemaphoreSlimContext.cs

@@ -52,11 +52,14 @@ namespace Tgstation.Server.Host.Core
 		/// </summary>
 		public void Dispose()
 		{
-			if (disposed)
-				return;
-			lockedSemaphore.Release();
+			lock (this)
+			{
+				if (disposed)
+					return;
+				disposed = true;
+			}
 			GC.SuppressFinalize(this);
-			disposed = true;
+			lockedSemaphore.Release();
 		}
 	}
 }

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

@@ -0,0 +1,17 @@
+using Microsoft.VisualStudio.TestTools.UnitTesting;
+using System;
+using System.Net.Http.Headers;
+
+namespace Tgstation.Server.Client.Tests
+{
+	[TestClass]
+	public sealed class TestServerClientFactory
+	{
+		[TestMethod]
+		public void TestConstruction()
+		{
+			Assert.ThrowsException<ArgumentNullException>(() => new ServerClientFactory(null));
+			new ServerClientFactory(new ProductHeaderValue("Tgstation.Server.Client.Tests", GetType().Assembly.GetName().Version.ToString()));
+		}
+	}
+}