From 8b332fb5c97a67a4c4ddcd0549fdc704a8367c77 Mon Sep 17 00:00:00 2001 From: William Baker Date: Fri, 1 Nov 2019 10:27:44 -0700 Subject: [PATCH 01/15] RepoRegistry: refactor format and access directly from verbs Previously, the repo registry was owned completely by the service, and verbs that needed to register repos (e.g. 'scalar clone') would make a request to the service to perform the registration. As a consequence of this design, if the service were not running when repo was cloned, the repo would never be added to the registry, and the service would never run maintenance tasks for it. With the changes in this commit Scalar uses a new approach for the repo registry: - The registry is a directory populated with .repo files - 'scalar clone' will create a new .repo file for its repo (the name of the file being the SHA1 hash of its path) - The .repo file stores the repo's path and owner in JSON format - Scalar.Service will discover all the registered repos by scanning the registry directory each time a maintenance task runs As part of this change, the following code could be removed: - All registration related messages to/from the service and the scalar verbs - The concept of "active" registered repos. Now that there's not a mount process there's no need to track which repos are mounted (i.e. "active") - All methods in the registry interface that are no longer required Future work: - Add a new verb for registering and unregistering repos - Move the '--list-registered' option from the 'service' verb to the new verb mentioned above --- Scalar.Common/NamedPipes/NamedPipeMessages.cs | 76 ---- .../RepoRegistry/IScalarRepoRegistry.cs | 12 + .../RepoRegistry/ScalarRepoRegistration.cs | 21 +- .../RepoRegistry/ScalarRepoRegistry.cs | 225 ++++++++++ Scalar.Common/ScalarConstants.cs | 5 + .../Handlers/GetActiveRepoListHandler.cs | 93 ----- Scalar.Service/Handlers/MessageHandler.cs | 16 - .../Handlers/RegisterRepoHandler.cs | 45 -- Scalar.Service/Handlers/RequestHandler.cs | 35 +- .../Handlers/UnregisterRepoHandler.cs | 45 -- Scalar.Service/IRepoRegistry.cs | 15 - Scalar.Service/MaintenanceTaskScheduler.cs | 117 +++++- Scalar.Service/Program.Mac.cs | 9 +- Scalar.Service/RepoRegistry.cs | 394 ------------------ Scalar.Service/Scalar.Service.Mac.csproj | 7 - Scalar.Service/Scalar.Service.Windows.csproj | 7 - Scalar.Service/ScalarService.Mac.cs | 14 +- Scalar.Service/ScalarService.Windows.cs | 33 +- .../RepoRegistry/ScalarRepoRegistryTests.cs | 54 +++ .../Service/Mac/MacServiceTests.cs | 120 +++--- Scalar.UnitTests/Service/RepoRegistryTests.cs | 190 --------- Scalar/CommandLine/CloneVerb.cs | 73 +--- Scalar/CommandLine/ServiceVerb.cs | 78 +--- Scripts/Mac/Scalar_Mount.sh | 10 - Scripts/Mac/Scalar_Unmount.sh | 10 - 25 files changed, 536 insertions(+), 1168 deletions(-) create mode 100644 Scalar.Common/RepoRegistry/IScalarRepoRegistry.cs rename Scalar.Service/RepoRegistration.cs => Scalar.Common/RepoRegistry/ScalarRepoRegistration.cs (51%) create mode 100644 Scalar.Common/RepoRegistry/ScalarRepoRegistry.cs delete mode 100644 Scalar.Service/Handlers/GetActiveRepoListHandler.cs delete mode 100644 Scalar.Service/Handlers/MessageHandler.cs delete mode 100644 Scalar.Service/Handlers/RegisterRepoHandler.cs delete mode 100644 Scalar.Service/Handlers/UnregisterRepoHandler.cs delete mode 100644 Scalar.Service/IRepoRegistry.cs delete mode 100644 Scalar.Service/RepoRegistry.cs create mode 100644 Scalar.UnitTests/Common/RepoRegistry/ScalarRepoRegistryTests.cs delete mode 100644 Scalar.UnitTests/Service/RepoRegistryTests.cs delete mode 100755 Scripts/Mac/Scalar_Mount.sh delete mode 100755 Scripts/Mac/Scalar_Unmount.sh diff --git a/Scalar.Common/NamedPipes/NamedPipeMessages.cs b/Scalar.Common/NamedPipes/NamedPipeMessages.cs index dde3aa5ac1..1322191826 100644 --- a/Scalar.Common/NamedPipes/NamedPipeMessages.cs +++ b/Scalar.Common/NamedPipes/NamedPipeMessages.cs @@ -116,82 +116,6 @@ public override string ToString() } } - public class UnregisterRepoRequest - { - public const string Header = nameof(UnregisterRepoRequest); - - public string EnlistmentRoot { get; set; } - - public static UnregisterRepoRequest FromMessage(Message message) - { - return JsonConvert.DeserializeObject(message.Body); - } - - public Message ToMessage() - { - return new Message(Header, JsonConvert.SerializeObject(this)); - } - - public class Response : BaseResponse - { - public static Response FromMessage(Message message) - { - return JsonConvert.DeserializeObject(message.Body); - } - } - } - - public class RegisterRepoRequest - { - public const string Header = nameof(RegisterRepoRequest); - - public string EnlistmentRoot { get; set; } - public string OwnerSID { get; set; } - - public static RegisterRepoRequest FromMessage(Message message) - { - return JsonConvert.DeserializeObject(message.Body); - } - - public Message ToMessage() - { - return new Message(Header, JsonConvert.SerializeObject(this)); - } - - public class Response : BaseResponse - { - public static Response FromMessage(Message message) - { - return JsonConvert.DeserializeObject(message.Body); - } - } - } - - public class GetActiveRepoListRequest - { - public const string Header = nameof(GetActiveRepoListRequest); - - public static GetActiveRepoListRequest FromMessage(Message message) - { - return JsonConvert.DeserializeObject(message.Body); - } - - public Message ToMessage() - { - return new Message(Header, JsonConvert.SerializeObject(this)); - } - - public class Response : BaseResponse - { - public List RepoList { get; set; } - - public static Response FromMessage(Message message) - { - return JsonConvert.DeserializeObject(message.Body); - } - } - } - public class BaseResponse { public const string Header = nameof(TRequest) + ResponseSuffix; diff --git a/Scalar.Common/RepoRegistry/IScalarRepoRegistry.cs b/Scalar.Common/RepoRegistry/IScalarRepoRegistry.cs new file mode 100644 index 0000000000..374f06052e --- /dev/null +++ b/Scalar.Common/RepoRegistry/IScalarRepoRegistry.cs @@ -0,0 +1,12 @@ +using System.Collections.Generic; + +namespace Scalar.Common.RepoRegistry +{ + public interface IScalarRepoRegistry + { + bool TryRegisterRepo(string repoRoot, string ownerSID, out string errorMessage); + List GetRegisteredRepos(); + List GetRegisteredReposForUser(string ownerSID); + bool TryRemoveRepo(string repoRoot, out string errorMessage); + } +} diff --git a/Scalar.Service/RepoRegistration.cs b/Scalar.Common/RepoRegistry/ScalarRepoRegistration.cs similarity index 51% rename from Scalar.Service/RepoRegistration.cs rename to Scalar.Common/RepoRegistry/ScalarRepoRegistration.cs index e175725161..2d329d3a22 100644 --- a/Scalar.Service/RepoRegistration.cs +++ b/Scalar.Common/RepoRegistry/ScalarRepoRegistration.cs @@ -1,27 +1,25 @@ using Newtonsoft.Json; -namespace Scalar.Service +namespace Scalar.Common.RepoRegistry { - public class RepoRegistration + public class ScalarRepoRegistration { - public RepoRegistration() + public ScalarRepoRegistration() { } - public RepoRegistration(string enlistmentRoot, string ownerSID) + public ScalarRepoRegistration(string enlistmentRoot, string ownerSID) { this.EnlistmentRoot = enlistmentRoot; this.OwnerSID = ownerSID; - this.IsActive = true; } public string EnlistmentRoot { get; set; } public string OwnerSID { get; set; } - public bool IsActive { get; set; } - public static RepoRegistration FromJson(string json) + public static ScalarRepoRegistration FromJson(string json) { - return JsonConvert.DeserializeObject( + return JsonConvert.DeserializeObject( json, new JsonSerializerSettings { @@ -31,12 +29,7 @@ public static RepoRegistration FromJson(string json) public override string ToString() { - return - string.Format( - "({0} - {1}) {2}", - this.IsActive ? "Active" : "Inactive", - this.OwnerSID, - this.EnlistmentRoot); + return $"({this.OwnerSID}) {this.EnlistmentRoot}"; } public string ToJson() diff --git a/Scalar.Common/RepoRegistry/ScalarRepoRegistry.cs b/Scalar.Common/RepoRegistry/ScalarRepoRegistry.cs new file mode 100644 index 0000000000..e2f62c83f0 --- /dev/null +++ b/Scalar.Common/RepoRegistry/ScalarRepoRegistry.cs @@ -0,0 +1,225 @@ +using Scalar.Common.FileSystem; +using Scalar.Common.Tracing; +using System; +using System.Collections.Generic; +using System.ComponentModel; +using System.IO; +using System.Linq; + +namespace Scalar.Common.RepoRegistry +{ + public class ScalarRepoRegistry : IScalarRepoRegistry + { + private const string EtwArea = nameof(ScalarRepoRegistry); + private const string RegistryFileExtension = ".repo"; + private const string RegistryTempFileExtension = ".temp"; + + private string registryFolderPath; + private ITracer tracer; + private PhysicalFileSystem fileSystem; + + public ScalarRepoRegistry( + ITracer tracer, + PhysicalFileSystem fileSystem, + string repoRegistryLocation) + { + this.tracer = tracer; + this.fileSystem = fileSystem; + this.registryFolderPath = repoRegistryLocation; + } + + public bool TryRegisterRepo(string repoRoot, string ownerSID, out string errorMessage) + { + try + { + if (!this.fileSystem.DirectoryExists(this.registryFolderPath)) + { + EventMetadata metadata = CreateEventMetadata(); + metadata.Add(nameof(this.registryFolderPath), this.registryFolderPath); + this.tracer.RelatedEvent( + EventLevel.Informational, + $"{nameof(this.TryRegisterRepo)}_CreatingRegistryDirectory", + metadata); + + // TODO #136: Make sure this does the right thing with ACLs on Windows + this.fileSystem.CreateDirectory(this.registryFolderPath); + } + } + catch (Exception e) + { + errorMessage = $"Error while ensuring registry directory '{this.registryFolderPath}' exists: {e.Message}"; + + EventMetadata metadata = CreateEventMetadata(e); + metadata.Add(nameof(repoRoot), repoRoot); + metadata.Add(nameof(this.registryFolderPath), this.registryFolderPath); + this.tracer.RelatedError(metadata, $"{nameof(this.TryRegisterRepo)}: Exception while ensuring registry directory exists"); + return false; + } + + string tempRegistryPath = this.GetRepoRegistryTempFilePath(repoRoot); + + try + { + ScalarRepoRegistration repoRegistration = new ScalarRepoRegistration(repoRoot, ownerSID); + string registryFileContents = repoRegistration.ToJson(); + + using (Stream tempFile = this.fileSystem.OpenFileStream( + tempRegistryPath, + FileMode.Create, + FileAccess.Write, + FileShare.None, + callFlushFileBuffers: true)) + using (StreamWriter writer = new StreamWriter(tempFile)) + { + writer.WriteLine(registryFileContents); + tempFile.Flush(); + } + } + catch (Exception e) + { + errorMessage = $"Error while registering repo {repoRoot}: {e.Message}"; + + EventMetadata metadata = CreateEventMetadata(e); + metadata.Add(nameof(repoRoot), repoRoot); + metadata.Add(nameof(tempRegistryPath), tempRegistryPath); + this.tracer.RelatedError(metadata, $"{nameof(this.TryRegisterRepo)}: Exception while writing temp registry file"); + return false; + } + + string registryFilePath = this.GetRepoRegistryFilePath(repoRoot); + try + { + this.fileSystem.MoveAndOverwriteFile(tempRegistryPath, registryFilePath); + } + catch (Win32Exception e) + { + errorMessage = $"Error while registering repo {repoRoot}: {e.Message}"; + + EventMetadata metadata = CreateEventMetadata(e); + metadata.Add(nameof(repoRoot), repoRoot); + metadata.Add(nameof(tempRegistryPath), tempRegistryPath); + metadata.Add(nameof(registryFilePath), registryFilePath); + this.tracer.RelatedError(metadata, $"{nameof(this.TryRegisterRepo)}: Exception while renaming temp registry file"); + return false; + } + + errorMessage = null; + return true; + } + + public bool TryRemoveRepo(string repoRoot, out string errorMessage) + { + string registryPath = this.GetRepoRegistryFilePath(repoRoot); + if (!this.fileSystem.FileExists(registryPath)) + { + errorMessage = $"Attempted to remove non-existent repo '{repoRoot}'"; + + EventMetadata metadata = CreateEventMetadata(); + metadata.Add(nameof(repoRoot), repoRoot); + metadata.Add(nameof(registryPath), registryPath); + this.tracer.RelatedWarning( + metadata, + $"{nameof(this.TryRemoveRepo)}: Attempted to remove non-existent repo"); + + return false; + } + + try + { + this.fileSystem.DeleteFile(registryPath); + } + catch (Exception e) + { + errorMessage = $"Error while removing repo {repoRoot}: {e.Message}"; + + EventMetadata metadata = CreateEventMetadata(e); + metadata.Add(nameof(repoRoot), repoRoot); + metadata.Add(nameof(registryPath), registryPath); + this.tracer.RelatedWarning( + metadata, + $"{nameof(this.TryRemoveRepo)}: Exception while removing repo"); + + return false; + } + + errorMessage = null; + return true; + } + + public List GetRegisteredRepos() + { + List repoList = new List(); + if (!this.fileSystem.DirectoryExists(this.registryFolderPath)) + { + return repoList; + } + + IEnumerable registryDirContents = this.fileSystem.ItemsInDirectory(this.registryFolderPath); + foreach (DirectoryItemInfo dirItem in registryDirContents) + { + if (dirItem.IsDirectory) + { + continue; + } + + if (!Path.GetExtension(dirItem.Name).Equals(RegistryFileExtension, StringComparison.OrdinalIgnoreCase)) + { + continue; + } + + try + { + string repoData = this.fileSystem.ReadAllText(dirItem.FullName); + ScalarRepoRegistration registration = ScalarRepoRegistration.FromJson(repoData); + repoList.Add(registration); + } + catch (Exception e) + { + EventMetadata metadata = CreateEventMetadata(e); + metadata.Add(nameof(dirItem.FullName), dirItem.FullName); + this.tracer.RelatedWarning( + metadata, + $"{nameof(this.GetRegisteredRepos)}: Failed to read registry file"); + } + } + + return repoList; + } + + public List GetRegisteredReposForUser(string ownerSID) + { + List registeredRepos = this.GetRegisteredRepos(); + IEnumerable registryDirContents = this.fileSystem.ItemsInDirectory(this.registryFolderPath); + return registeredRepos.Where(x => x.OwnerSID.Equals(ownerSID, StringComparison.CurrentCultureIgnoreCase)).ToList(); + } + + internal static string GetRepoRootSha(string repoRoot) + { + return SHA1Util.SHA1HashStringForUTF8String(repoRoot.ToLowerInvariant()); + } + + internal string GetRepoRegistryTempFilePath(string repoRoot) + { + string repoTempFilename = $"{GetRepoRootSha(repoRoot)}{RegistryTempFileExtension}"; + return Path.Combine(this.registryFolderPath, repoTempFilename); + } + + internal string GetRepoRegistryFilePath(string repoRoot) + { + string repoFilename = $"{GetRepoRootSha(repoRoot)}{RegistryFileExtension}"; + return Path.Combine(this.registryFolderPath, repoFilename); + } + + private static EventMetadata CreateEventMetadata(Exception e = null) + { + EventMetadata metadata = new EventMetadata(); + metadata.Add("Area", EtwArea); + if (e != null) + { + metadata.Add("Exception", e.ToString()); + } + + return metadata; + } + } +} diff --git a/Scalar.Common/ScalarConstants.cs b/Scalar.Common/ScalarConstants.cs index 8972068a10..a265801b9a 100644 --- a/Scalar.Common/ScalarConstants.cs +++ b/Scalar.Common/ScalarConstants.cs @@ -54,6 +54,11 @@ public static class Service public const string UIName = "Scalar.Service.UI"; } + public static class RepoRegistry + { + public const string RegistryDirectoryName = "Scalar.RepoRegistry"; + } + public static class MediaTypes { public const string PrefetchPackFilesAndIndexesMediaType = "application/x-gvfs-timestamped-packfiles-indexes"; diff --git a/Scalar.Service/Handlers/GetActiveRepoListHandler.cs b/Scalar.Service/Handlers/GetActiveRepoListHandler.cs deleted file mode 100644 index 78c4ff6302..0000000000 --- a/Scalar.Service/Handlers/GetActiveRepoListHandler.cs +++ /dev/null @@ -1,93 +0,0 @@ -using Scalar.Common; -using Scalar.Common.NamedPipes; -using Scalar.Common.Tracing; -using System.Collections.Generic; -using System.Linq; - -namespace Scalar.Service.Handlers -{ - public class GetActiveRepoListHandler : MessageHandler - { - private NamedPipeServer.Connection connection; - private NamedPipeMessages.GetActiveRepoListRequest request; - private ITracer tracer; - private IRepoRegistry registry; - - public GetActiveRepoListHandler( - ITracer tracer, - IRepoRegistry registry, - NamedPipeServer.Connection connection, - NamedPipeMessages.GetActiveRepoListRequest request) - { - this.tracer = tracer; - this.registry = registry; - this.connection = connection; - this.request = request; - } - - public void Run() - { - string errorMessage; - NamedPipeMessages.GetActiveRepoListRequest.Response response = new NamedPipeMessages.GetActiveRepoListRequest.Response(); - response.State = NamedPipeMessages.CompletionState.Success; - response.RepoList = new List(); - - List repos; - if (this.registry.TryGetActiveRepos(out repos, out errorMessage)) - { - List tempRepoList = repos.Select(repo => repo.EnlistmentRoot).ToList(); - - foreach (string repoRoot in tempRepoList) - { - if (!this.IsValidRepo(repoRoot)) - { - if (!this.registry.TryRemoveRepo(repoRoot, out errorMessage)) - { - this.tracer.RelatedInfo("Removing an invalid repo failed with error: " + response.ErrorMessage); - } - else - { - this.tracer.RelatedInfo("Removed invalid repo entry from registry: " + repoRoot); - } - } - else - { - response.RepoList.Add(repoRoot); - } - } - } - else - { - response.ErrorMessage = errorMessage; - response.State = NamedPipeMessages.CompletionState.Failure; - this.tracer.RelatedError("Get active repo list failed with error: " + response.ErrorMessage); - } - - this.WriteToClient(response.ToMessage(), this.connection, this.tracer); - } - - private bool IsValidRepo(string repoRoot) - { - string gitBinPath = ScalarPlatform.Instance.GitInstallation.GetInstalledGitBinPath(); - try - { - ScalarEnlistment enlistment = ScalarEnlistment.CreateFromDirectory( - repoRoot, - gitBinPath, - authentication: null); - } - catch (InvalidRepoException e) - { - EventMetadata metadata = new EventMetadata(); - metadata.Add(nameof(repoRoot), repoRoot); - metadata.Add(nameof(gitBinPath), gitBinPath); - metadata.Add("Exception", e.ToString()); - this.tracer.RelatedInfo(metadata, $"{nameof(this.IsValidRepo)}: Found invalid repo"); - - return false; - } - - return true; - } - } -} diff --git a/Scalar.Service/Handlers/MessageHandler.cs b/Scalar.Service/Handlers/MessageHandler.cs deleted file mode 100644 index e6289b50a8..0000000000 --- a/Scalar.Service/Handlers/MessageHandler.cs +++ /dev/null @@ -1,16 +0,0 @@ -using Scalar.Common.NamedPipes; -using Scalar.Common.Tracing; - -namespace Scalar.Service.Handlers -{ - public abstract class MessageHandler - { - protected void WriteToClient(NamedPipeMessages.Message message, NamedPipeServer.Connection connection, ITracer tracer) - { - if (!connection.TrySendResponse(message)) - { - tracer.RelatedError("Failed to send line to client: {0}", message); - } - } - } -} diff --git a/Scalar.Service/Handlers/RegisterRepoHandler.cs b/Scalar.Service/Handlers/RegisterRepoHandler.cs deleted file mode 100644 index 744028e83b..0000000000 --- a/Scalar.Service/Handlers/RegisterRepoHandler.cs +++ /dev/null @@ -1,45 +0,0 @@ -using Scalar.Common.NamedPipes; -using Scalar.Common.Tracing; - -namespace Scalar.Service.Handlers -{ - public class RegisterRepoHandler : MessageHandler - { - private NamedPipeServer.Connection connection; - private NamedPipeMessages.RegisterRepoRequest request; - private ITracer tracer; - private IRepoRegistry registry; - - public RegisterRepoHandler( - ITracer tracer, - IRepoRegistry registry, - NamedPipeServer.Connection connection, - NamedPipeMessages.RegisterRepoRequest request) - { - this.tracer = tracer; - this.registry = registry; - this.connection = connection; - this.request = request; - } - - public void Run() - { - string errorMessage = string.Empty; - NamedPipeMessages.RegisterRepoRequest.Response response = new NamedPipeMessages.RegisterRepoRequest.Response(); - - if (this.registry.TryRegisterRepo(this.request.EnlistmentRoot, this.request.OwnerSID, out errorMessage)) - { - response.State = NamedPipeMessages.CompletionState.Success; - this.tracer.RelatedInfo("Registered repo {0}", this.request.EnlistmentRoot); - } - else - { - response.ErrorMessage = errorMessage; - response.State = NamedPipeMessages.CompletionState.Failure; - this.tracer.RelatedError("Failed to register repo {0} with error: {1}", this.request.EnlistmentRoot, errorMessage); - } - - this.WriteToClient(response.ToMessage(), this.connection, this.tracer); - } - } -} diff --git a/Scalar.Service/Handlers/RequestHandler.cs b/Scalar.Service/Handlers/RequestHandler.cs index 38c76e0965..63015922b4 100644 --- a/Scalar.Service/Handlers/RequestHandler.cs +++ b/Scalar.Service/Handlers/RequestHandler.cs @@ -7,29 +7,20 @@ namespace Scalar.Service.Handlers /// /// RequestHandler - Routes client requests that reach Scalar.Service to /// appropriate MessageHandler object. - /// Example requests - scalar mount/unmount command sends requests to - /// register/un-register repositories for automount. RequestHandler - /// routes them to RegisterRepoHandler and UnRegisterRepoHandler - /// respectively. /// public class RequestHandler { protected string requestDescription; - private const string MountRequestDescription = "mount"; - private const string UnmountRequestDescription = "unmount"; - private const string RepoListRequestDescription = "repo list"; private const string UnknownRequestDescription = "unknown"; private string etwArea; private ITracer tracer; - private IRepoRegistry repoRegistry; - public RequestHandler(ITracer tracer, string etwArea, IRepoRegistry repoRegistry) + public RequestHandler(ITracer tracer, string etwArea) { this.tracer = tracer; this.etwArea = etwArea; - this.repoRegistry = repoRegistry; } public void HandleRequest(ITracer tracer, string request, NamedPipeServer.Connection connection) @@ -65,30 +56,6 @@ protected virtual void HandleMessage( { switch (message.Header) { - case NamedPipeMessages.RegisterRepoRequest.Header: - this.requestDescription = MountRequestDescription; - NamedPipeMessages.RegisterRepoRequest mountRequest = NamedPipeMessages.RegisterRepoRequest.FromMessage(message); - RegisterRepoHandler mountHandler = new RegisterRepoHandler(tracer, this.repoRegistry, connection, mountRequest); - mountHandler.Run(); - - break; - - case NamedPipeMessages.UnregisterRepoRequest.Header: - this.requestDescription = UnmountRequestDescription; - NamedPipeMessages.UnregisterRepoRequest unmountRequest = NamedPipeMessages.UnregisterRepoRequest.FromMessage(message); - UnregisterRepoHandler unmountHandler = new UnregisterRepoHandler(tracer, this.repoRegistry, connection, unmountRequest); - unmountHandler.Run(); - - break; - - case NamedPipeMessages.GetActiveRepoListRequest.Header: - this.requestDescription = RepoListRequestDescription; - NamedPipeMessages.GetActiveRepoListRequest repoListRequest = NamedPipeMessages.GetActiveRepoListRequest.FromMessage(message); - GetActiveRepoListHandler excludeHandler = new GetActiveRepoListHandler(tracer, this.repoRegistry, connection, repoListRequest); - excludeHandler.Run(); - - break; - default: this.requestDescription = UnknownRequestDescription; EventMetadata metadata = new EventMetadata(); diff --git a/Scalar.Service/Handlers/UnregisterRepoHandler.cs b/Scalar.Service/Handlers/UnregisterRepoHandler.cs deleted file mode 100644 index 749fc158ee..0000000000 --- a/Scalar.Service/Handlers/UnregisterRepoHandler.cs +++ /dev/null @@ -1,45 +0,0 @@ -using Scalar.Common.NamedPipes; -using Scalar.Common.Tracing; - -namespace Scalar.Service.Handlers -{ - public class UnregisterRepoHandler : MessageHandler - { - private NamedPipeServer.Connection connection; - private NamedPipeMessages.UnregisterRepoRequest request; - private ITracer tracer; - private IRepoRegistry registry; - - public UnregisterRepoHandler( - ITracer tracer, - IRepoRegistry registry, - NamedPipeServer.Connection connection, - NamedPipeMessages.UnregisterRepoRequest request) - { - this.tracer = tracer; - this.registry = registry; - this.connection = connection; - this.request = request; - } - - public void Run() - { - string errorMessage = string.Empty; - NamedPipeMessages.UnregisterRepoRequest.Response response = new NamedPipeMessages.UnregisterRepoRequest.Response(); - - if (this.registry.TryDeactivateRepo(this.request.EnlistmentRoot, out errorMessage)) - { - response.State = NamedPipeMessages.CompletionState.Success; - this.tracer.RelatedInfo("Deactivated repo {0}", this.request.EnlistmentRoot); - } - else - { - response.ErrorMessage = errorMessage; - response.State = NamedPipeMessages.CompletionState.Failure; - this.tracer.RelatedError("Failed to deactivate repo {0} with error: {1}", this.request.EnlistmentRoot, errorMessage); - } - - this.WriteToClient(response.ToMessage(), this.connection, this.tracer); - } - } -} diff --git a/Scalar.Service/IRepoRegistry.cs b/Scalar.Service/IRepoRegistry.cs deleted file mode 100644 index 38595863ca..0000000000 --- a/Scalar.Service/IRepoRegistry.cs +++ /dev/null @@ -1,15 +0,0 @@ -using System.Collections.Generic; -using Scalar.Common.Maintenance; - -namespace Scalar.Service -{ - public interface IRepoRegistry - { - bool TryRegisterRepo(string repoRoot, string ownerSID, out string errorMessage); - bool TryDeactivateRepo(string repoRoot, out string errorMessage); - bool TryGetActiveRepos(out List repoList, out string errorMessage); - bool TryRemoveRepo(string repoRoot, out string errorMessage); - void RunMaintenanceTaskForRepos(MaintenanceTasks.Task task, string userId, int sessionId); - void TraceStatus(); - } -} diff --git a/Scalar.Service/MaintenanceTaskScheduler.cs b/Scalar.Service/MaintenanceTaskScheduler.cs index eeb8b93b23..c6e43238ae 100644 --- a/Scalar.Service/MaintenanceTaskScheduler.cs +++ b/Scalar.Service/MaintenanceTaskScheduler.cs @@ -1,8 +1,11 @@ using Scalar.Common; +using Scalar.Common.FileSystem; using Scalar.Common.Maintenance; +using Scalar.Common.RepoRegistry; using Scalar.Common.Tracing; using System; using System.Collections.Generic; +using System.IO; using System.Threading; namespace Scalar.Service @@ -21,15 +24,25 @@ public class MaintenanceTaskScheduler : IDisposable, IRegisteredUserStore private readonly TimeSpan fetchCommitsAndTreesPeriod = TimeSpan.FromMinutes(15); private readonly ITracer tracer; + private readonly PhysicalFileSystem fileSystem; + private readonly IScalarVerbRunner scalarVerb; + private readonly IScalarRepoRegistry repoRegistry; private ServiceTaskQueue taskQueue; private List taskTimers; - public MaintenanceTaskScheduler(ITracer tracer, IRepoRegistry repoRegistry) + public MaintenanceTaskScheduler( + ITracer tracer, + PhysicalFileSystem fileSystem, + IScalarVerbRunner scalarVerb, + IScalarRepoRegistry repoRegistry) { this.tracer = tracer; + this.fileSystem = fileSystem; + this.scalarVerb = scalarVerb; + this.repoRegistry = repoRegistry; this.taskTimers = new List(); this.taskQueue = new ServiceTaskQueue(this.tracer); - this.ScheduleRecurringTasks(repoRegistry); + this.ScheduleRecurringTasks(); } public UserAndSession RegisteredUser { get; private set; } @@ -64,7 +77,7 @@ public void Dispose() this.taskTimers = null; } - private void ScheduleRecurringTasks(IRepoRegistry repoRegistry) + private void ScheduleRecurringTasks() { if (ScalarEnlistment.IsUnattended(this.tracer)) { @@ -98,7 +111,9 @@ private void ScheduleRecurringTasks(IRepoRegistry repoRegistry) (state) => this.taskQueue.TryEnqueue( new MaintenanceTask( this.tracer, - repoRegistry, + this.fileSystem, + this.scalarVerb, + this.repoRegistry, this, schedule.Task)), state: null, @@ -123,18 +138,24 @@ public MaintenanceSchedule(MaintenanceTasks.Task task, TimeSpan dueTime, TimeSpa private class MaintenanceTask : IServiceTask { - private readonly MaintenanceTasks.Task task; - private readonly IRepoRegistry repoRegistry; private readonly ITracer tracer; + private readonly PhysicalFileSystem fileSystem; + private readonly IScalarVerbRunner scalarVerb; + private readonly IScalarRepoRegistry repoRegistry; private readonly IRegisteredUserStore registeredUserStore; + private readonly MaintenanceTasks.Task task; public MaintenanceTask( ITracer tracer, - IRepoRegistry repoRegistry, + PhysicalFileSystem fileSystem, + IScalarVerbRunner scalarVerb, + IScalarRepoRegistry repoRegistry, IRegisteredUserStore registeredUserStore, MaintenanceTasks.Task task) { this.tracer = tracer; + this.fileSystem = fileSystem; + this.scalarVerb = scalarVerb; this.repoRegistry = repoRegistry; this.registeredUserStore = registeredUserStore; this.task = task; @@ -155,8 +176,9 @@ public void Execute() $"{nameof(MaintenanceTaskScheduler)}.{nameof(this.Execute)}", metadata); - this.repoRegistry.RunMaintenanceTaskForRepos( + this.RunMaintenanceTaskForRepos( this.task, + this.repoRegistry, registeredUser.UserId, registeredUser.SessionId); } @@ -170,6 +192,85 @@ public void Stop() { // TODO: #185 - Kill the currently running maintenance verb } + + public void RunMaintenanceTaskForRepos( + MaintenanceTasks.Task task, + IScalarRepoRegistry repoRegistry, + string userId, + int sessionId) + { + EventMetadata metadata = new EventMetadata(); + metadata.Add(nameof(task), MaintenanceTasks.GetVerbTaskName(task)); + metadata.Add(nameof(userId), userId); + metadata.Add(nameof(sessionId), sessionId); + + List activeRepos = repoRegistry.GetRegisteredReposForUser(userId); + if (activeRepos.Count == 0) + { + metadata.Add(TracingConstants.MessageKey.InfoMessage, "No active repos for user"); + this.tracer.RelatedEvent( + EventLevel.Informational, + $"{nameof(this.RunMaintenanceTaskForRepos)}_NoRepos", + metadata); + } + else + { + string rootPath; + string errorMessage; + + foreach (ScalarRepoRegistration repo in activeRepos) + { + rootPath = Path.GetPathRoot(repo.EnlistmentRoot); + + metadata[nameof(repo.EnlistmentRoot)] = repo.EnlistmentRoot; + metadata[nameof(task)] = task; + metadata[nameof(rootPath)] = rootPath; + metadata.Remove(nameof(errorMessage)); + + if (!string.IsNullOrWhiteSpace(rootPath) && !this.fileSystem.DirectoryExists(rootPath)) + { + // If the volume does not exist we'll assume the drive was removed or is encrypted, + // and we'll leave the repo in the registry (but we won't run maintenance on it). + this.tracer.RelatedEvent( + EventLevel.Informational, + $"{nameof(this.RunMaintenanceTaskForRepos)}_SkippedRepoWithMissingVolume", + metadata); + + continue; + } + + if (!this.fileSystem.DirectoryExists(repo.EnlistmentRoot)) + { + // The repo is no longer on disk (but its volume is present) + // Unregister the repo + if (repoRegistry.TryRemoveRepo(repo.EnlistmentRoot, out errorMessage)) + { + this.tracer.RelatedEvent( + EventLevel.Informational, + $"{nameof(this.RunMaintenanceTaskForRepos)}_RemovedMissingRepo", + metadata); + } + else + { + metadata[nameof(errorMessage)] = errorMessage; + this.tracer.RelatedEvent( + EventLevel.Informational, + $"{nameof(this.RunMaintenanceTaskForRepos)}_FailedToRemoveRepo", + metadata); + } + + continue; + } + + this.tracer.RelatedEvent( + EventLevel.Informational, + $"{nameof(this.RunMaintenanceTaskForRepos)}_CallingMaintenance", + metadata); + + this.scalarVerb.CallMaintenance(task, repo.EnlistmentRoot, sessionId); + } + } + } } } } diff --git a/Scalar.Service/Program.Mac.cs b/Scalar.Service/Program.Mac.cs index ef5f7901a3..0ef92d8bb4 100644 --- a/Scalar.Service/Program.Mac.cs +++ b/Scalar.Service/Program.Mac.cs @@ -1,8 +1,8 @@ using Scalar.Common; using Scalar.Common.FileSystem; +using Scalar.Common.RepoRegistry; using Scalar.Common.Tracing; using Scalar.PlatformLoader; -using Scalar.Service.Handlers; using System; using System.IO; using System.Linq; @@ -47,12 +47,11 @@ private static ScalarService CreateService(JsonTracer tracer, string[] args) EventLevel.Informational, Keywords.Any); - string serviceDataLocation = scalarPlatform.GetDataRootForScalarComponent(serviceName); - RepoRegistry repoRegistry = new RepoRegistry( + string repoRegistryLocation = scalarPlatform.GetDataRootForScalarComponent(ScalarConstants.RepoRegistry.RegistryDirectoryName); + ScalarRepoRegistry repoRegistry = new ScalarRepoRegistry( tracer, new PhysicalFileSystem(), - serviceDataLocation, - new ScalarVerbRunner(tracer)); + repoRegistryLocation); return new ScalarService(tracer, serviceName, repoRegistry); } diff --git a/Scalar.Service/RepoRegistry.cs b/Scalar.Service/RepoRegistry.cs deleted file mode 100644 index 8461f78236..0000000000 --- a/Scalar.Service/RepoRegistry.cs +++ /dev/null @@ -1,394 +0,0 @@ -using Scalar.Common.FileSystem; -using Scalar.Common.Maintenance; -using Scalar.Common.Tracing; -using System; -using System.Collections.Generic; -using System.IO; -using System.Linq; - -namespace Scalar.Service -{ - public class RepoRegistry : IRepoRegistry - { - public const string RegistryName = "repo-registry"; - private const string EtwArea = nameof(RepoRegistry); - private const string RegistryTempName = "repo-registry.lock"; - private const int RegistryVersion = 2; - - private string registryParentFolderPath; - private ITracer tracer; - private PhysicalFileSystem fileSystem; - private object repoLock = new object(); - private IScalarVerbRunner scalarVerb; - - public RepoRegistry( - ITracer tracer, - PhysicalFileSystem fileSystem, - string serviceDataLocation, - IScalarVerbRunner scalarVerbRunner) - { - this.tracer = tracer; - this.fileSystem = fileSystem; - this.registryParentFolderPath = serviceDataLocation; - this.scalarVerb = scalarVerbRunner; - - EventMetadata metadata = new EventMetadata(); - metadata.Add("Area", EtwArea); - metadata.Add("registryParentFolderPath", this.registryParentFolderPath); - metadata.Add(TracingConstants.MessageKey.InfoMessage, "RepoRegistry created"); - this.tracer.RelatedEvent(EventLevel.Informational, "RepoRegistry_Created", metadata); - } - - public bool TryRegisterRepo(string repoRoot, string ownerSID, out string errorMessage) - { - errorMessage = null; - - try - { - lock (this.repoLock) - { - Dictionary allRepos = this.ReadRegistry(); - RepoRegistration repo; - if (allRepos.TryGetValue(repoRoot, out repo)) - { - if (!repo.IsActive) - { - repo.IsActive = true; - repo.OwnerSID = ownerSID; - this.WriteRegistry(allRepos); - } - } - else - { - allRepos[repoRoot] = new RepoRegistration(repoRoot, ownerSID); - this.WriteRegistry(allRepos); - } - } - - return true; - } - catch (Exception e) - { - errorMessage = string.Format("Error while registering repo {0}: {1}", repoRoot, e.ToString()); - } - - return false; - } - - public void TraceStatus() - { - try - { - lock (this.repoLock) - { - Dictionary allRepos = this.ReadRegistry(); - foreach (RepoRegistration repo in allRepos.Values) - { - this.tracer.RelatedInfo(repo.ToString()); - } - } - } - catch (Exception e) - { - this.tracer.RelatedError("Error while tracing repos: {0}", e.ToString()); - } - } - - public bool TryDeactivateRepo(string repoRoot, out string errorMessage) - { - errorMessage = null; - - try - { - lock (this.repoLock) - { - Dictionary allRepos = this.ReadRegistry(); - RepoRegistration repo; - if (allRepos.TryGetValue(repoRoot, out repo)) - { - if (repo.IsActive) - { - repo.IsActive = false; - this.WriteRegistry(allRepos); - } - - return true; - } - else - { - errorMessage = string.Format("Attempted to deactivate non-existent repo at '{0}'", repoRoot); - } - } - } - catch (Exception e) - { - errorMessage = string.Format("Error while deactivating repo {0}: {1}", repoRoot, e.ToString()); - } - - return false; - } - - public bool TryRemoveRepo(string repoRoot, out string errorMessage) - { - errorMessage = null; - - try - { - lock (this.repoLock) - { - Dictionary allRepos = this.ReadRegistry(); - if (allRepos.Remove(repoRoot)) - { - this.WriteRegistry(allRepos); - return true; - } - else - { - errorMessage = string.Format("Attempted to remove non-existent repo at '{0}'", repoRoot); - } - } - } - catch (Exception e) - { - errorMessage = string.Format("Error while removing repo {0}: {1}", repoRoot, e.ToString()); - } - - return false; - } - - public void RunMaintenanceTaskForRepos(MaintenanceTasks.Task task, string userId, int sessionId) - { - EventMetadata metadata = CreateEventMetadata(); - metadata.Add(nameof(task), MaintenanceTasks.GetVerbTaskName(task)); - metadata.Add(nameof(userId), userId); - metadata.Add(nameof(sessionId), sessionId); - - List activeRepos = this.GetActiveReposForUser(userId); - if (activeRepos.Count == 0) - { - metadata.Add(TracingConstants.MessageKey.InfoMessage, "No active repos for user"); - this.tracer.RelatedEvent( - EventLevel.Informational, - $"{nameof(this.RunMaintenanceTaskForRepos)}_NoRepos", - metadata); - } - else - { - string rootPath; - string errorMessage; - - foreach (RepoRegistration repo in activeRepos) - { - rootPath = Path.GetPathRoot(repo.EnlistmentRoot); - - metadata[nameof(repo.EnlistmentRoot)] = repo.EnlistmentRoot; - metadata[nameof(task)] = task; - metadata[nameof(rootPath)] = rootPath; - metadata.Remove(nameof(errorMessage)); - - if (!string.IsNullOrWhiteSpace(rootPath) && !this.fileSystem.DirectoryExists(rootPath)) - { - // If the volume does not exist we'll assume the drive was removed or is encrypted, - // and we'll leave the repo in the registry (but we won't run maintenance on it). - this.tracer.RelatedEvent( - EventLevel.Informational, - $"{nameof(this.RunMaintenanceTaskForRepos)}_SkippedRepoWithMissingVolume", - metadata); - - continue; - } - - if (!this.fileSystem.DirectoryExists(repo.EnlistmentRoot)) - { - // The repo is no longer on disk (but its volume is present) - // Unregister the repo - if (this.TryRemoveRepo(repo.EnlistmentRoot, out errorMessage)) - { - this.tracer.RelatedEvent( - EventLevel.Informational, - $"{nameof(this.RunMaintenanceTaskForRepos)}_RemovedMissingRepo", - metadata); - } - else - { - metadata[nameof(errorMessage)] = errorMessage; - this.tracer.RelatedEvent( - EventLevel.Informational, - $"{nameof(this.RunMaintenanceTaskForRepos)}_FailedToRemoveRepo", - metadata); - } - - continue; - } - - this.tracer.RelatedEvent( - EventLevel.Informational, - $"{nameof(this.RunMaintenanceTaskForRepos)}_CallingMaintenance", - metadata); - - this.scalarVerb.CallMaintenance(task, repo.EnlistmentRoot, sessionId); - } - } - } - - public Dictionary ReadRegistry() - { - Dictionary allRepos = new Dictionary(StringComparer.OrdinalIgnoreCase); - - using (Stream stream = this.fileSystem.OpenFileStream( - Path.Combine(this.registryParentFolderPath, RegistryName), - FileMode.OpenOrCreate, - FileAccess.Read, - FileShare.Read, - callFlushFileBuffers: false)) - { - using (StreamReader reader = new StreamReader(stream)) - { - string versionString = reader.ReadLine(); - int version; - if (!int.TryParse(versionString, out version) || - version > RegistryVersion) - { - if (versionString != null) - { - EventMetadata metadata = CreateEventMetadata(); - metadata.Add("OnDiskVersion", versionString); - metadata.Add("ExpectedVersion", versionString); - this.tracer.RelatedError(metadata, "ReadRegistry: Unsupported version"); - } - - return allRepos; - } - - while (!reader.EndOfStream) - { - string entry = reader.ReadLine(); - if (entry.Length > 0) - { - try - { - RepoRegistration registration = RepoRegistration.FromJson(entry); - - string errorMessage; - string normalizedEnlistmentRootPath = registration.EnlistmentRoot; - if (this.fileSystem.TryGetNormalizedPath(registration.EnlistmentRoot, out normalizedEnlistmentRootPath, out errorMessage)) - { - if (!normalizedEnlistmentRootPath.Equals(registration.EnlistmentRoot, StringComparison.OrdinalIgnoreCase)) - { - EventMetadata metadata = CreateEventMetadata(); - metadata.Add("registration.EnlistmentRoot", registration.EnlistmentRoot); - metadata.Add(nameof(normalizedEnlistmentRootPath), normalizedEnlistmentRootPath); - metadata.Add(TracingConstants.MessageKey.InfoMessage, $"{nameof(this.ReadRegistry)}: Mapping registered enlistment root to final path"); - this.tracer.RelatedEvent(EventLevel.Informational, $"{nameof(this.ReadRegistry)}_NormalizedPathMapping", metadata); - } - } - else - { - EventMetadata metadata = CreateEventMetadata(); - metadata.Add("registration.EnlistmentRoot", registration.EnlistmentRoot); - metadata.Add("NormalizedEnlistmentRootPath", normalizedEnlistmentRootPath); - metadata.Add("ErrorMessage", errorMessage); - this.tracer.RelatedWarning(metadata, $"{nameof(this.ReadRegistry)}: Failed to get normalized path name for registed enlistment root"); - } - - if (normalizedEnlistmentRootPath != null) - { - allRepos[normalizedEnlistmentRootPath] = registration; - } - } - catch (Exception e) - { - EventMetadata metadata = CreateEventMetadata(e); - metadata.Add("entry", entry); - this.tracer.RelatedError(metadata, "ReadRegistry: Failed to read entry"); - } - } - } - } - } - - return allRepos; - } - - public bool TryGetActiveRepos(out List repoList, out string errorMessage) - { - repoList = null; - errorMessage = null; - - lock (this.repoLock) - { - try - { - Dictionary repos = this.ReadRegistry(); - repoList = repos - .Values - .Where(repo => repo.IsActive) - .ToList(); - return true; - } - catch (Exception e) - { - errorMessage = string.Format("Unable to get list of active repos: {0}", e.ToString()); - return false; - } - } - } - - private static EventMetadata CreateEventMetadata(Exception e = null) - { - EventMetadata metadata = new EventMetadata(); - metadata.Add("Area", EtwArea); - if (e != null) - { - metadata.Add("Exception", e.ToString()); - } - - return metadata; - } - - private List GetActiveReposForUser(string ownerSID) - { - lock (this.repoLock) - { - try - { - Dictionary repos = this.ReadRegistry(); - return repos - .Values - .Where(repo => repo.IsActive) - .Where(repo => string.Equals(repo.OwnerSID, ownerSID, StringComparison.InvariantCultureIgnoreCase)) - .ToList(); - } - catch (Exception e) - { - this.tracer.RelatedError("Unable to get list of active repos for user {0}: {1}", ownerSID, e.ToString()); - return new List(); - } - } - } - - private void WriteRegistry(Dictionary registry) - { - string tempFilePath = Path.Combine(this.registryParentFolderPath, RegistryTempName); - using (Stream stream = this.fileSystem.OpenFileStream( - tempFilePath, - FileMode.Create, - FileAccess.Write, - FileShare.None, - callFlushFileBuffers: true)) - using (StreamWriter writer = new StreamWriter(stream)) - { - writer.WriteLine(RegistryVersion); - - foreach (RepoRegistration repo in registry.Values) - { - writer.WriteLine(repo.ToJson()); - } - - stream.Flush(); - } - - this.fileSystem.MoveAndOverwriteFile(tempFilePath, Path.Combine(this.registryParentFolderPath, RegistryName)); - } - } -} diff --git a/Scalar.Service/Scalar.Service.Mac.csproj b/Scalar.Service/Scalar.Service.Mac.csproj index 7c0ee5df85..1c1022c6d1 100644 --- a/Scalar.Service/Scalar.Service.Mac.csproj +++ b/Scalar.Service/Scalar.Service.Mac.csproj @@ -26,15 +26,8 @@ - - - - - - - diff --git a/Scalar.Service/Scalar.Service.Windows.csproj b/Scalar.Service/Scalar.Service.Windows.csproj index f15c2e7922..68b6337a78 100644 --- a/Scalar.Service/Scalar.Service.Windows.csproj +++ b/Scalar.Service/Scalar.Service.Windows.csproj @@ -64,20 +64,13 @@ - - - - - - - diff --git a/Scalar.Service/ScalarService.Mac.cs b/Scalar.Service/ScalarService.Mac.cs index 390b16c51e..2f8f4cf6fa 100644 --- a/Scalar.Service/ScalarService.Mac.cs +++ b/Scalar.Service/ScalarService.Mac.cs @@ -1,5 +1,7 @@ using Scalar.Common; +using Scalar.Common.FileSystem; using Scalar.Common.NamedPipes; +using Scalar.Common.RepoRegistry; using Scalar.Common.Tracing; using Scalar.Service.Handlers; using System; @@ -17,14 +19,14 @@ public class ScalarService private Thread serviceThread; private ManualResetEvent serviceStopped; private string serviceName; - private IRepoRegistry repoRegistry; + private IScalarRepoRegistry repoRegistry; private RequestHandler requestHandler; private MaintenanceTaskScheduler maintenanceTaskScheduler; public ScalarService( ITracer tracer, string serviceName, - IRepoRegistry repoRegistry) + IScalarRepoRegistry repoRegistry) { this.tracer = tracer; this.repoRegistry = repoRegistry; @@ -32,7 +34,7 @@ public ScalarService( this.serviceStopped = new ManualResetEvent(false); this.serviceThread = new Thread(this.ServiceThreadMain); - this.requestHandler = new RequestHandler(this.tracer, EtwArea, this.repoRegistry); + this.requestHandler = new RequestHandler(this.tracer, EtwArea); } public void Run() @@ -91,7 +93,11 @@ private void ServiceThreadMain() { try { - this.maintenanceTaskScheduler = new MaintenanceTaskScheduler(this.tracer, this.repoRegistry); + this.maintenanceTaskScheduler = new MaintenanceTaskScheduler( + this.tracer, + new PhysicalFileSystem(), + new ScalarVerbRunner(this.tracer), + this.repoRegistry); // On Mac, there is no separate session Id. currentUser is used as sessionId this.maintenanceTaskScheduler.RegisterUser(new UserAndSession(currentUser, sessionId)); diff --git a/Scalar.Service/ScalarService.Windows.cs b/Scalar.Service/ScalarService.Windows.cs index 53037c8a24..cee5bc23d2 100644 --- a/Scalar.Service/ScalarService.Windows.cs +++ b/Scalar.Service/ScalarService.Windows.cs @@ -1,6 +1,7 @@ using Scalar.Common; using Scalar.Common.FileSystem; using Scalar.Common.NamedPipes; +using Scalar.Common.RepoRegistry; using Scalar.Common.Tracing; using Scalar.Platform.Windows; using Scalar.Service.Handlers; @@ -24,7 +25,8 @@ public class ScalarService : ServiceBase private ManualResetEvent serviceStopped; private string serviceName; private string serviceDataLocation; - private RepoRegistry repoRegistry; + private string repoRegistryLocation; + private ScalarRepoRegistry repoRegistry; private ProductUpgradeTimer productUpgradeTimer; private RequestHandler requestHandler; private MaintenanceTaskScheduler maintenanceTaskScheduler; @@ -47,15 +49,15 @@ public void Run() metadata.Add("Version", ProcessHelper.GetCurrentProcessVersion()); this.tracer.RelatedEvent(EventLevel.Informational, $"{nameof(ScalarService)}_{nameof(this.Run)}", metadata); - this.repoRegistry = new RepoRegistry( + PhysicalFileSystem fileSystem = new PhysicalFileSystem(); + this.repoRegistry = new ScalarRepoRegistry( this.tracer, - new PhysicalFileSystem(), - this.serviceDataLocation, - new ScalarVerbRunner(this.tracer)); + fileSystem, + this.repoRegistryLocation); - this.maintenanceTaskScheduler = new MaintenanceTaskScheduler(this.tracer, this.repoRegistry); + this.maintenanceTaskScheduler = new MaintenanceTaskScheduler(this.tracer, fileSystem, new ScalarVerbRunner(this.tracer), this.repoRegistry); - this.requestHandler = new RequestHandler(this.tracer, EtwArea, this.repoRegistry); + this.requestHandler = new RequestHandler(this.tracer, EtwArea); string pipeName = ScalarPlatform.Instance.GetScalarServiceNamedPipeName(this.serviceName); this.tracer.RelatedInfo("Starting pipe server with name: " + pipeName); @@ -187,6 +189,7 @@ protected override void OnStart(string[] args) try { this.serviceDataLocation = ScalarPlatform.Instance.GetDataRootForScalarComponent(this.serviceName); + this.repoRegistryLocation = ScalarPlatform.Instance.GetDataRootForScalarComponent(ScalarConstants.RepoRegistry.RegistryDirectoryName); this.CreateAndConfigureProgramDataDirectories(); this.Start(); } @@ -257,11 +260,12 @@ private void CreateServiceLogsDirectory(string serviceLogsDirectoryPath) private void CreateAndConfigureProgramDataDirectories() { - string serviceDataRootPath = Path.GetDirectoryName(this.serviceDataLocation); + string serviceDataRootPath = ScalarPlatform.Instance.GetDataRootForScalar(); DirectorySecurity serviceDataRootSecurity = this.GetServiceDirectorySecurity(serviceDataRootPath); - // Create Scalar.Service and Scalar.Upgrade related directories (if they don't already exist) + // Create Scalar, Scalar.Service, and Scalar.Upgrade related directories (if they don't already exist) + // TODO #136: Determine if we still should be creating Scalar.Service here Directory.CreateDirectory(serviceDataRootPath, serviceDataRootSecurity); Directory.CreateDirectory(this.serviceDataLocation, serviceDataRootSecurity); Directory.CreateDirectory(ProductUpgraderInfo.GetUpgradeProtectedDataDirectory(), serviceDataRootSecurity); @@ -269,12 +273,13 @@ private void CreateAndConfigureProgramDataDirectories() // Ensure the ACLs are set correctly on any files or directories that were already created (e.g. after upgrading Scalar) Directory.SetAccessControl(serviceDataRootPath, serviceDataRootSecurity); - // Special rules for the upgrader logs, as non-elevated users need to be be able to write - this.CreateAndConfigureLogDirectory(ProductUpgraderInfo.GetLogDirectoryPath()); - this.CreateAndConfigureLogDirectory(ScalarPlatform.Instance.GetDataRootForScalarComponent(ScalarConstants.Service.UIName)); + // Special rules for the upgrader logs and registry, as non-elevated users need to be be able to write + this.CreateAndConfigureUserWriteableDirectory(this.repoRegistryLocation); + this.CreateAndConfigureUserWriteableDirectory(ProductUpgraderInfo.GetLogDirectoryPath()); + this.CreateAndConfigureUserWriteableDirectory(ScalarPlatform.Instance.GetDataRootForScalarComponent(ScalarConstants.Service.UIName)); } - private void CreateAndConfigureLogDirectory(string path) + private void CreateAndConfigureUserWriteableDirectory(string path) { string upgradeLogsPath = ProductUpgraderInfo.GetLogDirectoryPath(); @@ -287,7 +292,7 @@ private void CreateAndConfigureLogDirectory(string path) metadata.Add(nameof(error), error); this.tracer.RelatedWarning( metadata, - $"{nameof(this.CreateAndConfigureLogDirectory)}: Failed to create upgrade logs directory", + $"{nameof(this.CreateAndConfigureUserWriteableDirectory)}: Failed to create upgrade logs directory", Keywords.Telemetry); } } diff --git a/Scalar.UnitTests/Common/RepoRegistry/ScalarRepoRegistryTests.cs b/Scalar.UnitTests/Common/RepoRegistry/ScalarRepoRegistryTests.cs new file mode 100644 index 0000000000..61ea224be8 --- /dev/null +++ b/Scalar.UnitTests/Common/RepoRegistry/ScalarRepoRegistryTests.cs @@ -0,0 +1,54 @@ +using Moq; +using NUnit.Framework; +using Scalar.Common.RepoRegistry; +using Scalar.Tests.Should; +using Scalar.UnitTests.Mock.Common; +using Scalar.UnitTests.Mock.FileSystem; +using System; +using System.Collections.Generic; +using System.IO; + +namespace Scalar.UnitTests.Common.RepoRegistry +{ + [TestFixture] + public class ScalarRepoRegistryTests + { + [SetUp] + public void Setup() + { + } + + [TearDown] + public void TearDown() + { + } + + ////[TestCase] + ////public void TryRegisterRepo_EmptyRegistry() + ////{ + //// string dataLocation = Path.Combine("mock:", "registryDataFolder"); + //// + //// MockFileSystem fileSystem = new MockFileSystem(new MockDirectory(dataLocation, null, null)); + //// RepoRegistry registry = new RepoRegistry( + //// new MockTracer(), + //// fileSystem, + //// dataLocation); + //// + //// string repoRoot = Path.Combine("c:", "test"); + //// string ownerSID = Guid.NewGuid().ToString(); + //// + //// string errorMessage; + //// registry.TryRegisterRepo(repoRoot, ownerSID, out errorMessage).ShouldEqual(true); + //// + //// Dictionary verifiableRegistry = registry.ReadRegistry(); + //// verifiableRegistry.Count.ShouldEqual(1); + //// this.VerifyRepo(verifiableRegistry[repoRoot], ownerSID); + ////} + + private void VerifyRepo(ScalarRepoRegistration repo, string expectedOwnerSID) + { + repo.ShouldNotBeNull(); + repo.OwnerSID.ShouldEqual(expectedOwnerSID); + } + } +} diff --git a/Scalar.UnitTests/Service/Mac/MacServiceTests.cs b/Scalar.UnitTests/Service/Mac/MacServiceTests.cs index cebad8993f..902463014d 100644 --- a/Scalar.UnitTests/Service/Mac/MacServiceTests.cs +++ b/Scalar.UnitTests/Service/Mac/MacServiceTests.cs @@ -2,6 +2,7 @@ using NUnit.Framework; using Scalar.Common; using Scalar.Common.Maintenance; +using Scalar.Common.RepoRegistry; using Scalar.Service; using Scalar.Tests.Should; using Scalar.UnitTests.Mock.Common; @@ -31,50 +32,49 @@ public void SetUp() this.scalarPlatform.MockCurrentUser = ExpectedActiveUserId.ToString(); } - [TestCase] - public void RepoRegistryRemovesRegisteredRepoIfMissingFromDisk() - { - Mock repoMounterMock = new Mock(MockBehavior.Strict); - - this.fileSystem.DirectoryExists(ExpectedActiveRepoPath).ShouldBeFalse($"{ExpectedActiveRepoPath} should not exist"); - - MaintenanceTasks.Task task = MaintenanceTasks.Task.FetchCommitsAndTrees; - - this.CreateTestRepos(ServiceDataLocation); - RepoRegistry repoRegistry = new RepoRegistry( - this.tracer, - this.fileSystem, - ServiceDataLocation, - repoMounterMock.Object); - - repoRegistry.RunMaintenanceTaskForRepos(task, ExpectedActiveUserId.ToString(), ExpectedSessionId); - repoMounterMock.VerifyAll(); - - repoRegistry.ReadRegistry().ShouldNotContain(entry => entry.Key.Equals(ExpectedActiveRepoPath)); - } - - [TestCase] - public void RepoRegistryCallsMaintenanceVerbOnlyForRegisteredRepos() - { - Mock repoMounterMock = new Mock(MockBehavior.Strict); - - this.fileSystem.CreateDirectory(ExpectedActiveRepoPath); - - MaintenanceTasks.Task task = MaintenanceTasks.Task.FetchCommitsAndTrees; - repoMounterMock.Setup(mp => mp.CallMaintenance(task, ExpectedActiveRepoPath, ExpectedActiveUserId)).Returns(true); - - this.CreateTestRepos(ServiceDataLocation); - - RepoRegistry repoRegistry = new RepoRegistry( - this.tracer, - this.fileSystem, - ServiceDataLocation, - repoMounterMock.Object); - - repoRegistry.RunMaintenanceTaskForRepos(task, ExpectedActiveUserId.ToString(), ExpectedSessionId); - - repoMounterMock.VerifyAll(); - } + ////[TestCase] + ////public void RepoRegistryRemovesRegisteredRepoIfMissingFromDisk() + ////{ + //// Mock repoMounterMock = new Mock(MockBehavior.Strict); + //// + //// this.fileSystem.DirectoryExists(ExpectedActiveRepoPath).ShouldBeFalse($"{ExpectedActiveRepoPath} should not exist"); + //// + //// MaintenanceTasks.Task task = MaintenanceTasks.Task.FetchCommitsAndTrees; + //// + //// this.CreateTestRepos(ServiceDataLocation); + //// RepoRegistry repoRegistry = new RepoRegistry( + //// this.tracer, + //// this.fileSystem, + //// ServiceDataLocation); + //// + //// repoRegistry.RunMaintenanceTaskForRepos(task, ExpectedActiveUserId.ToString(), ExpectedSessionId); + //// repoMounterMock.VerifyAll(); + //// + //// repoRegistry.ReadRegistry().ShouldNotContain(entry => entry.Key.Equals(ExpectedActiveRepoPath)); + ////} + + ////[TestCase] + ////public void RepoRegistryCallsMaintenanceVerbOnlyForRegisteredRepos() + ////{ + //// Mock repoMounterMock = new Mock(MockBehavior.Strict); + //// + //// this.fileSystem.CreateDirectory(ExpectedActiveRepoPath); + //// + //// MaintenanceTasks.Task task = MaintenanceTasks.Task.FetchCommitsAndTrees; + //// repoMounterMock.Setup(mp => mp.CallMaintenance(task, ExpectedActiveRepoPath, ExpectedActiveUserId)).Returns(true); + //// + //// this.CreateTestRepos(ServiceDataLocation); + //// + //// RepoRegistry repoRegistry = new RepoRegistry( + //// this.tracer, + //// this.fileSystem, + //// ServiceDataLocation, + //// repoMounterMock.Object); + //// + //// repoRegistry.RunMaintenanceTaskForRepos(task, ExpectedActiveUserId.ToString(), ExpectedSessionId); + //// + //// repoMounterMock.VerifyAll(); + ////} [TestCase] public void MaintenanceVerbLaunchedUsingCorrectArgs() @@ -99,21 +99,21 @@ public void MaintenanceVerbLaunchedUsingCorrectArgs() mountLauncherMock.VerifyAll(); } - private void CreateTestRepos(string dataLocation) - { - string repo1 = Path.Combine("mock:", "code", "repo1"); - string repo2 = ExpectedActiveRepoPath; - string repo3 = Path.Combine("mock:", "code", "repo3"); - string repo4 = Path.Combine("mock:", "code", "repo4"); - - this.fileSystem.WriteAllText( - Path.Combine(dataLocation, RepoRegistry.RegistryName), - $@"1 - {{""EnlistmentRoot"":""{repo1.Replace("\\", "\\\\")}"",""OwnerSID"":502,""IsActive"":false}} - {{""EnlistmentRoot"":""{repo2.Replace("\\", "\\\\")}"",""OwnerSID"":502,""IsActive"":true}} - {{""EnlistmentRoot"":""{repo3.Replace("\\", "\\\\")}"",""OwnerSID"":501,""IsActive"":false}} - {{""EnlistmentRoot"":""{repo4.Replace("\\", "\\\\")}"",""OwnerSID"":501,""IsActive"":true}} - "); - } + ////private void CreateTestRepos(string dataLocation) + ////{ + //// string repo1 = Path.Combine("mock:", "code", "repo1"); + //// string repo2 = ExpectedActiveRepoPath; + //// string repo3 = Path.Combine("mock:", "code", "repo3"); + //// string repo4 = Path.Combine("mock:", "code", "repo4"); + //// + //// this.fileSystem.WriteAllText( + //// Path.Combine(dataLocation, RepoRegistry.RegistryName), + //// $@"1 + //// {{""EnlistmentRoot"":""{repo1.Replace("\\", "\\\\")}"",""OwnerSID"":502,""IsActive"":false}} + //// {{""EnlistmentRoot"":""{repo2.Replace("\\", "\\\\")}"",""OwnerSID"":502,""IsActive"":true}} + //// {{""EnlistmentRoot"":""{repo3.Replace("\\", "\\\\")}"",""OwnerSID"":501,""IsActive"":false}} + //// {{""EnlistmentRoot"":""{repo4.Replace("\\", "\\\\")}"",""OwnerSID"":501,""IsActive"":true}} + //// "); + ////} } } diff --git a/Scalar.UnitTests/Service/RepoRegistryTests.cs b/Scalar.UnitTests/Service/RepoRegistryTests.cs deleted file mode 100644 index eaecda4040..0000000000 --- a/Scalar.UnitTests/Service/RepoRegistryTests.cs +++ /dev/null @@ -1,190 +0,0 @@ -using Moq; -using NUnit.Framework; -using Scalar.Service; -using Scalar.Tests.Should; -using Scalar.UnitTests.Mock.Common; -using Scalar.UnitTests.Mock.FileSystem; -using System; -using System.Collections.Generic; -using System.IO; -using System.Linq; - -namespace Scalar.UnitTests.Service -{ - [TestFixture] - public class RepoRegistryTests - { - private Mock mockRepoMounter; - - [SetUp] - public void Setup() - { - this.mockRepoMounter = new Mock(MockBehavior.Strict); - } - - [TearDown] - public void TearDown() - { - this.mockRepoMounter.VerifyAll(); - } - - [TestCase] - public void TryRegisterRepo_EmptyRegistry() - { - string dataLocation = Path.Combine("mock:", "registryDataFolder"); - - MockFileSystem fileSystem = new MockFileSystem(new MockDirectory(dataLocation, null, null)); - RepoRegistry registry = new RepoRegistry( - new MockTracer(), - fileSystem, - dataLocation, - this.mockRepoMounter.Object); - - string repoRoot = Path.Combine("c:", "test"); - string ownerSID = Guid.NewGuid().ToString(); - - string errorMessage; - registry.TryRegisterRepo(repoRoot, ownerSID, out errorMessage).ShouldEqual(true); - - Dictionary verifiableRegistry = registry.ReadRegistry(); - verifiableRegistry.Count.ShouldEqual(1); - this.VerifyRepo(verifiableRegistry[repoRoot], ownerSID, expectedIsActive: true); - } - - [TestCase] - public void TryGetActiveRepos_BeforeAndAfterActivateAndDeactivate() - { - string dataLocation = Path.Combine("mock:", "registryDataFolder"); - MockFileSystem fileSystem = new MockFileSystem(new MockDirectory(dataLocation, null, null)); - RepoRegistry registry = new RepoRegistry( - new MockTracer(), - fileSystem, - dataLocation, - this.mockRepoMounter.Object); - - string repo1Root = Path.Combine("mock:", "test", "repo1"); - string owner1SID = Guid.NewGuid().ToString(); - string repo2Root = Path.Combine("mock:", "test", "repo2"); - string owner2SID = Guid.NewGuid().ToString(); - string repo3Root = Path.Combine("mock:", "test", "repo3"); - string owner3SID = Guid.NewGuid().ToString(); - - // Register all 3 repos - string errorMessage; - registry.TryRegisterRepo(repo1Root, owner1SID, out errorMessage).ShouldEqual(true); - registry.TryRegisterRepo(repo2Root, owner2SID, out errorMessage).ShouldEqual(true); - registry.TryRegisterRepo(repo3Root, owner3SID, out errorMessage).ShouldEqual(true); - - // Confirm all 3 active - List activeRepos; - registry.TryGetActiveRepos(out activeRepos, out errorMessage); - activeRepos.Count.ShouldEqual(3); - this.VerifyRepo(activeRepos.SingleOrDefault(repo => repo.EnlistmentRoot.Equals(repo1Root)), owner1SID, expectedIsActive: true); - this.VerifyRepo(activeRepos.SingleOrDefault(repo => repo.EnlistmentRoot.Equals(repo2Root)), owner2SID, expectedIsActive: true); - this.VerifyRepo(activeRepos.SingleOrDefault(repo => repo.EnlistmentRoot.Equals(repo3Root)), owner3SID, expectedIsActive: true); - - // Deactive repo 2 - registry.TryDeactivateRepo(repo2Root, out errorMessage).ShouldEqual(true); - - // Confirm repos 1 and 3 still active - registry.TryGetActiveRepos(out activeRepos, out errorMessage); - activeRepos.Count.ShouldEqual(2); - this.VerifyRepo(activeRepos.SingleOrDefault(repo => repo.EnlistmentRoot.Equals(repo1Root)), owner1SID, expectedIsActive: true); - this.VerifyRepo(activeRepos.SingleOrDefault(repo => repo.EnlistmentRoot.Equals(repo3Root)), owner3SID, expectedIsActive: true); - - // Activate repo 2 - registry.TryRegisterRepo(repo2Root, owner2SID, out errorMessage).ShouldEqual(true); - - // Confirm all 3 active - registry.TryGetActiveRepos(out activeRepos, out errorMessage); - activeRepos.Count.ShouldEqual(3); - this.VerifyRepo(activeRepos.SingleOrDefault(repo => repo.EnlistmentRoot.Equals(repo1Root)), owner1SID, expectedIsActive: true); - this.VerifyRepo(activeRepos.SingleOrDefault(repo => repo.EnlistmentRoot.Equals(repo2Root)), owner2SID, expectedIsActive: true); - this.VerifyRepo(activeRepos.SingleOrDefault(repo => repo.EnlistmentRoot.Equals(repo3Root)), owner3SID, expectedIsActive: true); - } - - [TestCase] - public void TryDeactivateRepo() - { - string dataLocation = Path.Combine("mock:", "registryDataFolder"); - MockFileSystem fileSystem = new MockFileSystem(new MockDirectory(dataLocation, null, null)); - RepoRegistry registry = new RepoRegistry( - new MockTracer(), - fileSystem, - dataLocation, - this.mockRepoMounter.Object); - - string repo1Root = Path.Combine("mock:", "test", "repo1"); - string owner1SID = Guid.NewGuid().ToString(); - string errorMessage; - registry.TryRegisterRepo(repo1Root, owner1SID, out errorMessage).ShouldEqual(true); - - List activeRepos; - registry.TryGetActiveRepos(out activeRepos, out errorMessage); - activeRepos.Count.ShouldEqual(1); - this.VerifyRepo(activeRepos.SingleOrDefault(repo => repo.EnlistmentRoot.Equals(repo1Root)), owner1SID, expectedIsActive: true); - - // Deactivate repo - registry.TryDeactivateRepo(repo1Root, out errorMessage).ShouldEqual(true); - registry.TryGetActiveRepos(out activeRepos, out errorMessage); - activeRepos.Count.ShouldEqual(0); - - // Deactivate repo again (no-op) - registry.TryDeactivateRepo(repo1Root, out errorMessage).ShouldEqual(true); - registry.TryGetActiveRepos(out activeRepos, out errorMessage); - activeRepos.Count.ShouldEqual(0); - - // Repo should still be in the registry - Dictionary verifiableRegistry = registry.ReadRegistry(); - verifiableRegistry.Count.ShouldEqual(1); - this.VerifyRepo(verifiableRegistry[repo1Root], owner1SID, expectedIsActive: false); - - // Deactivate non-existent repo should fail - string nonExistantPath = Path.Combine("mock:", "test", "doesNotExist"); - registry.TryDeactivateRepo(nonExistantPath, out errorMessage).ShouldEqual(false); - errorMessage.ShouldContain("Attempted to deactivate non-existent repo"); - } - - [TestCase] - public void TraceStatus() - { - string dataLocation = Path.Combine("mock:", "registryDataFolder"); - MockFileSystem fileSystem = new MockFileSystem(new MockDirectory(dataLocation, null, null)); - MockTracer tracer = new MockTracer(); - RepoRegistry registry = new RepoRegistry( - tracer, - fileSystem, - dataLocation, - this.mockRepoMounter.Object); - - string repo1Root = Path.Combine("mock:", "test", "repo1"); - string owner1SID = Guid.NewGuid().ToString(); - string repo2Root = Path.Combine("mock:", "test", "repo2"); - string owner2SID = Guid.NewGuid().ToString(); - string repo3Root = Path.Combine("mock:", "test", "repo3"); - string owner3SID = Guid.NewGuid().ToString(); - - string errorMessage; - registry.TryRegisterRepo(repo1Root, owner1SID, out errorMessage).ShouldEqual(true); - registry.TryRegisterRepo(repo2Root, owner2SID, out errorMessage).ShouldEqual(true); - registry.TryRegisterRepo(repo3Root, owner3SID, out errorMessage).ShouldEqual(true); - registry.TryDeactivateRepo(repo2Root, out errorMessage).ShouldEqual(true); - - registry.TraceStatus(); - - Dictionary repos = registry.ReadRegistry(); - repos.Count.ShouldEqual(3); - foreach (KeyValuePair kvp in repos) - { - tracer.RelatedInfoEvents.SingleOrDefault(message => message.Equals(kvp.Value.ToString())).ShouldNotBeNull(); - } - } - - private void VerifyRepo(RepoRegistration repo, string expectedOwnerSID, bool expectedIsActive) - { - repo.ShouldNotBeNull(); - repo.OwnerSID.ShouldEqual(expectedOwnerSID); - repo.IsActive.ShouldEqual(expectedIsActive); - } - } -} diff --git a/Scalar/CommandLine/CloneVerb.cs b/Scalar/CommandLine/CloneVerb.cs index 63c912ef01..23e00d4db5 100644 --- a/Scalar/CommandLine/CloneVerb.cs +++ b/Scalar/CommandLine/CloneVerb.cs @@ -4,6 +4,7 @@ using Scalar.Common.Git; using Scalar.Common.Http; using Scalar.Common.NamedPipes; +using Scalar.Common.RepoRegistry; using Scalar.Common.Tracing; using System; using System.Diagnostics; @@ -281,7 +282,7 @@ private Result DoClone(string fullEnlistmentRootPathParameter, string normalized cloneResult = this.CheckoutRepo(); - this.RegisterWithService(); + this.RegisterRepo(); } return cloneResult; @@ -575,80 +576,36 @@ private Result CreateClone() return new Result(true); } - private void RegisterWithService() + private void RegisterRepo() { if (!this.Unattended) { - this.tracer.RelatedInfo($"{nameof(this.Execute)}: Registering with service"); + this.tracer.RelatedInfo($"{nameof(this.Execute)}: Registering repo"); string errorMessage = string.Empty; if (this.ShowStatusWhileRunning( - () => { return this.RegisterRepoWithService(out errorMessage); }, - "Registering with service")) + () => { return this.RegisterRepo(out errorMessage); }, + "Registering repo")) { - this.tracer.RelatedInfo($"{nameof(this.Execute)}: Registered with service"); + this.tracer.RelatedInfo($"{nameof(this.Execute)}: Registration succeeded"); } else { this.Output.WriteLine(" WARNING: " + errorMessage); - this.tracer.RelatedInfo($"{nameof(this.Execute)}: Failed to register with service"); + this.tracer.RelatedInfo($"{nameof(this.Execute)}: Failed to register repo"); } } } - private bool RegisterRepoWithService(out string errorMessage) + private bool RegisterRepo(out string errorMessage) { - errorMessage = string.Empty; - - NamedPipeMessages.RegisterRepoRequest request = new NamedPipeMessages.RegisterRepoRequest(); - request.EnlistmentRoot = this.enlistment.EnlistmentRoot; - - request.OwnerSID = ScalarPlatform.Instance.GetCurrentUser(); - - using (NamedPipeClient client = new NamedPipeClient(this.ServicePipeName)) - { - if (!client.Connect()) - { - errorMessage = "Unable to register repo because Scalar.Service is not responding."; - return false; - } - - try - { - client.SendRequest(request.ToMessage()); - NamedPipeMessages.Message response = client.ReadResponse(); - if (response.Header == NamedPipeMessages.RegisterRepoRequest.Response.Header) - { - NamedPipeMessages.RegisterRepoRequest.Response message = NamedPipeMessages.RegisterRepoRequest.Response.FromMessage(response); - - if (!string.IsNullOrEmpty(message.ErrorMessage)) - { - errorMessage = message.ErrorMessage; - return false; - } + string repoRegistryLocation = ScalarPlatform.Instance.GetDataRootForScalarComponent(ScalarConstants.RepoRegistry.RegistryDirectoryName); + ScalarRepoRegistry repoRegistry = new ScalarRepoRegistry( + this.tracer, + this.fileSystem, + repoRegistryLocation); - if (message.State != NamedPipeMessages.CompletionState.Success) - { - errorMessage = "Unable to register repo. " + errorMessage; - return false; - } - else - { - return true; - } - } - else - { - errorMessage = string.Format("Scalar.Service responded with unexpected message: {0}", response); - return false; - } - } - catch (BrokenPipeException e) - { - errorMessage = "Unable to communicate with Scalar.Service: " + e.ToString(); - return false; - } - } + return repoRegistry.TryRegisterRepo(this.enlistment.EnlistmentRoot, ScalarPlatform.Instance.GetCurrentUser(), out errorMessage); } private Result TryInitRepo() diff --git a/Scalar/CommandLine/ServiceVerb.cs b/Scalar/CommandLine/ServiceVerb.cs index 5336720828..7d4b26febb 100644 --- a/Scalar/CommandLine/ServiceVerb.cs +++ b/Scalar/CommandLine/ServiceVerb.cs @@ -1,9 +1,9 @@ using CommandLine; using Scalar.Common; -using Scalar.Common.NamedPipes; -using System; +using Scalar.Common.FileSystem; +using Scalar.Common.RepoRegistry; +using Scalar.Common.Tracing; using System.Collections.Generic; -using System.IO; using System.Linq; namespace Scalar.CommandLine @@ -37,73 +37,25 @@ public override void Execute() this.ReportErrorAndExit($"Error: You cannot specify multiple arguments. Run 'scalar {ServiceVerbName} --help' for details."); } - string errorMessage; - List repoList; - if (!this.TryGetRepoList(out repoList, out errorMessage)) + List repoList = this.GetRepoList(); + foreach (string repoRoot in repoList) { - this.ReportErrorAndExit("Error getting repo list: " + errorMessage); - } - - if (this.List) - { - foreach (string repoRoot in repoList) - { - this.Output.WriteLine(repoRoot); - } + this.Output.WriteLine(repoRoot); } } - private bool TryGetRepoList(out List repoList, out string errorMessage) + private List GetRepoList() { - repoList = null; - errorMessage = string.Empty; - - NamedPipeMessages.GetActiveRepoListRequest request = new NamedPipeMessages.GetActiveRepoListRequest(); - - using (NamedPipeClient client = new NamedPipeClient(this.ServicePipeName)) + string repoRegistryLocation = ScalarPlatform.Instance.GetDataRootForScalarComponent(ScalarConstants.RepoRegistry.RegistryDirectoryName); + using (JsonTracer tracer = new JsonTracer(ScalarConstants.ScalarEtwProviderName, "ServiceVerb")) { - if (!client.Connect()) - { - errorMessage = "Scalar.Service is not responding."; - return false; - } - - try - { - client.SendRequest(request.ToMessage()); - NamedPipeMessages.Message response = client.ReadResponse(); - if (response.Header == NamedPipeMessages.GetActiveRepoListRequest.Response.Header) - { - NamedPipeMessages.GetActiveRepoListRequest.Response message = NamedPipeMessages.GetActiveRepoListRequest.Response.FromMessage(response); - - if (!string.IsNullOrEmpty(message.ErrorMessage)) - { - errorMessage = message.ErrorMessage; - } - else - { - if (message.State != NamedPipeMessages.CompletionState.Success) - { - errorMessage = "Unable to retrieve repo list."; - } - else - { - repoList = message.RepoList; - return true; - } - } - } - else - { - errorMessage = string.Format("Scalar.Service responded with unexpected message: {0}", response); - } - } - catch (BrokenPipeException e) - { - errorMessage = "Unable to communicate with Scalar.Service: " + e.ToString(); - } + ScalarRepoRegistry repoRegistry = new ScalarRepoRegistry( + tracer, + new PhysicalFileSystem(), + repoRegistryLocation); - return false; + List registeredRepos = repoRegistry.GetRegisteredRepos(); + return registeredRepos.Select(x => x.EnlistmentRoot).ToList(); } } } diff --git a/Scripts/Mac/Scalar_Mount.sh b/Scripts/Mac/Scalar_Mount.sh deleted file mode 100755 index 64dd7c69f5..0000000000 --- a/Scripts/Mac/Scalar_Mount.sh +++ /dev/null @@ -1,10 +0,0 @@ -#!/bin/bash - -. "$(dirname ${BASH_SOURCE[0]})/InitializeEnvironment.sh" - -CONFIGURATION=$1 -if [ -z $CONFIGURATION ]; then - CONFIGURATION=Debug -fi - -$Scalar_PUBLISHDIR/scalar mount ~/ScalarTest diff --git a/Scripts/Mac/Scalar_Unmount.sh b/Scripts/Mac/Scalar_Unmount.sh deleted file mode 100755 index 1379b54ffe..0000000000 --- a/Scripts/Mac/Scalar_Unmount.sh +++ /dev/null @@ -1,10 +0,0 @@ -#!/bin/bash - -. "$(dirname ${BASH_SOURCE[0]})/InitializeEnvironment.sh" - -CONFIGURATION=$1 -if [ -z $CONFIGURATION ]; then - CONFIGURATION=Debug -fi - -$Scalar_PUBLISHDIR/scalar unmount ~/ScalarTest From 6301685ac2ffdbaef0ba0c0fd18f3c6ea3e3b770 Mon Sep 17 00:00:00 2001 From: William Baker Date: Tue, 5 Nov 2019 14:09:10 -0800 Subject: [PATCH 02/15] ScalarRepoRegistry: switch to IEnumerable For consistency with the underlying API, update GetRegisteredRepos to return an IEnumerable rather than a List. Additionally, update GetRegisteredRepos to use Directory.EnumerateFiles rather than DirectoryInfo.GetFileSystemInfos so that the code can lazily walk the .repo files. --- .../FileSystem/PhysicalFileSystem.cs | 5 + .../RepoRegistry/IScalarRepoRegistry.cs | 3 +- .../IScalarRepoRegistryExtensions.cs | 14 +++ .../RepoRegistry/ScalarRepoRegistry.cs | 64 ++++------- Scalar.Service/MaintenanceTaskScheduler.cs | 102 +++++++++--------- .../Mock/FileSystem/MockFileSystem.cs | 45 ++++++++ Scalar/CommandLine/ServiceVerb.cs | 14 ++- 7 files changed, 145 insertions(+), 102 deletions(-) create mode 100644 Scalar.Common/RepoRegistry/IScalarRepoRegistryExtensions.cs diff --git a/Scalar.Common/FileSystem/PhysicalFileSystem.cs b/Scalar.Common/FileSystem/PhysicalFileSystem.cs index 7740de4356..b6b23c6786 100644 --- a/Scalar.Common/FileSystem/PhysicalFileSystem.cs +++ b/Scalar.Common/FileSystem/PhysicalFileSystem.cs @@ -196,6 +196,11 @@ public virtual IEnumerable EnumerateDirectories(string path) return Directory.EnumerateDirectories(path); } + public virtual IEnumerable EnumerateFiles(string path, string searchPattern) + { + return Directory.EnumerateFiles(path, searchPattern); + } + public virtual FileProperties GetFileProperties(string path) { FileInfo entry = new FileInfo(path); diff --git a/Scalar.Common/RepoRegistry/IScalarRepoRegistry.cs b/Scalar.Common/RepoRegistry/IScalarRepoRegistry.cs index 374f06052e..7ee688fab3 100644 --- a/Scalar.Common/RepoRegistry/IScalarRepoRegistry.cs +++ b/Scalar.Common/RepoRegistry/IScalarRepoRegistry.cs @@ -5,8 +5,7 @@ namespace Scalar.Common.RepoRegistry public interface IScalarRepoRegistry { bool TryRegisterRepo(string repoRoot, string ownerSID, out string errorMessage); - List GetRegisteredRepos(); - List GetRegisteredReposForUser(string ownerSID); bool TryRemoveRepo(string repoRoot, out string errorMessage); + IEnumerable GetRegisteredRepos(); } } diff --git a/Scalar.Common/RepoRegistry/IScalarRepoRegistryExtensions.cs b/Scalar.Common/RepoRegistry/IScalarRepoRegistryExtensions.cs new file mode 100644 index 0000000000..6fa95d452a --- /dev/null +++ b/Scalar.Common/RepoRegistry/IScalarRepoRegistryExtensions.cs @@ -0,0 +1,14 @@ +using System; +using System.Collections.Generic; +using System.Linq; + +namespace Scalar.Common.RepoRegistry +{ + public static class IScalarRepoRegistryExtensions + { + public static IEnumerable GetRegisteredReposForUser(this IScalarRepoRegistry registry, string ownerSID) + { + return registry.GetRegisteredRepos().Where(x => x.OwnerSID.Equals(ownerSID, StringComparison.CurrentCultureIgnoreCase)); + } + } +} diff --git a/Scalar.Common/RepoRegistry/ScalarRepoRegistry.cs b/Scalar.Common/RepoRegistry/ScalarRepoRegistry.cs index e2f62c83f0..a4814c4501 100644 --- a/Scalar.Common/RepoRegistry/ScalarRepoRegistry.cs +++ b/Scalar.Common/RepoRegistry/ScalarRepoRegistry.cs @@ -4,7 +4,6 @@ using System.Collections.Generic; using System.ComponentModel; using System.IO; -using System.Linq; namespace Scalar.Common.RepoRegistry { @@ -146,51 +145,34 @@ public bool TryRemoveRepo(string repoRoot, out string errorMessage) return true; } - public List GetRegisteredRepos() + public IEnumerable GetRegisteredRepos() { - List repoList = new List(); - if (!this.fileSystem.DirectoryExists(this.registryFolderPath)) + if (this.fileSystem.DirectoryExists(this.registryFolderPath)) { - return repoList; - } - - IEnumerable registryDirContents = this.fileSystem.ItemsInDirectory(this.registryFolderPath); - foreach (DirectoryItemInfo dirItem in registryDirContents) - { - if (dirItem.IsDirectory) - { - continue; - } - - if (!Path.GetExtension(dirItem.Name).Equals(RegistryFileExtension, StringComparison.OrdinalIgnoreCase)) + IEnumerable registryFilePaths = this.fileSystem.EnumerateFiles(this.registryFolderPath, $"*{RegistryFileExtension}"); + foreach (string registryFilePath in registryFilePaths) { - continue; - } - - try - { - string repoData = this.fileSystem.ReadAllText(dirItem.FullName); - ScalarRepoRegistration registration = ScalarRepoRegistration.FromJson(repoData); - repoList.Add(registration); - } - catch (Exception e) - { - EventMetadata metadata = CreateEventMetadata(e); - metadata.Add(nameof(dirItem.FullName), dirItem.FullName); - this.tracer.RelatedWarning( - metadata, - $"{nameof(this.GetRegisteredRepos)}: Failed to read registry file"); + ScalarRepoRegistration registration = null; + try + { + string repoData = this.fileSystem.ReadAllText(registryFilePath); + registration = ScalarRepoRegistration.FromJson(repoData); + } + catch (Exception e) + { + EventMetadata metadata = CreateEventMetadata(e); + metadata.Add(nameof(registryFilePath), registryFilePath); + this.tracer.RelatedWarning( + metadata, + $"{nameof(this.GetRegisteredRepos)}: Failed to read registry file"); + } + + if (registration != null) + { + yield return registration; + } } } - - return repoList; - } - - public List GetRegisteredReposForUser(string ownerSID) - { - List registeredRepos = this.GetRegisteredRepos(); - IEnumerable registryDirContents = this.fileSystem.ItemsInDirectory(this.registryFolderPath); - return registeredRepos.Where(x => x.OwnerSID.Equals(ownerSID, StringComparison.CurrentCultureIgnoreCase)).ToList(); } internal static string GetRepoRootSha(string repoRoot) diff --git a/Scalar.Service/MaintenanceTaskScheduler.cs b/Scalar.Service/MaintenanceTaskScheduler.cs index c6e43238ae..6385ec69be 100644 --- a/Scalar.Service/MaintenanceTaskScheduler.cs +++ b/Scalar.Service/MaintenanceTaskScheduler.cs @@ -204,71 +204,71 @@ public void RunMaintenanceTaskForRepos( metadata.Add(nameof(userId), userId); metadata.Add(nameof(sessionId), sessionId); - List activeRepos = repoRegistry.GetRegisteredReposForUser(userId); - if (activeRepos.Count == 0) - { - metadata.Add(TracingConstants.MessageKey.InfoMessage, "No active repos for user"); - this.tracer.RelatedEvent( - EventLevel.Informational, - $"{nameof(this.RunMaintenanceTaskForRepos)}_NoRepos", - metadata); - } - else + int reposForUserCount = 0; + string rootPath; + string errorMessage; + + foreach (ScalarRepoRegistration repoRegistration in repoRegistry.GetRegisteredReposForUser(userId)) { - string rootPath; - string errorMessage; + ++reposForUserCount; - foreach (ScalarRepoRegistration repo in activeRepos) + rootPath = Path.GetPathRoot(repoRegistration.EnlistmentRoot); + + metadata[nameof(repoRegistration.EnlistmentRoot)] = repoRegistration.EnlistmentRoot; + metadata[nameof(task)] = task; + metadata[nameof(rootPath)] = rootPath; + metadata.Remove(nameof(errorMessage)); + + if (!string.IsNullOrWhiteSpace(rootPath) && !this.fileSystem.DirectoryExists(rootPath)) { - rootPath = Path.GetPathRoot(repo.EnlistmentRoot); + // If the volume does not exist we'll assume the drive was removed or is encrypted, + // and we'll leave the repo in the registry (but we won't run maintenance on it). + this.tracer.RelatedEvent( + EventLevel.Informational, + $"{nameof(this.RunMaintenanceTaskForRepos)}_SkippedRepoWithMissingVolume", + metadata); - metadata[nameof(repo.EnlistmentRoot)] = repo.EnlistmentRoot; - metadata[nameof(task)] = task; - metadata[nameof(rootPath)] = rootPath; - metadata.Remove(nameof(errorMessage)); + continue; + } - if (!string.IsNullOrWhiteSpace(rootPath) && !this.fileSystem.DirectoryExists(rootPath)) + if (!this.fileSystem.DirectoryExists(repoRegistration.EnlistmentRoot)) + { + // The repo is no longer on disk (but its volume is present) + // Unregister the repo + if (repoRegistry.TryRemoveRepo(repoRegistration.EnlistmentRoot, out errorMessage)) { - // If the volume does not exist we'll assume the drive was removed or is encrypted, - // and we'll leave the repo in the registry (but we won't run maintenance on it). this.tracer.RelatedEvent( EventLevel.Informational, - $"{nameof(this.RunMaintenanceTaskForRepos)}_SkippedRepoWithMissingVolume", + $"{nameof(this.RunMaintenanceTaskForRepos)}_RemovedMissingRepo", metadata); - - continue; } - - if (!this.fileSystem.DirectoryExists(repo.EnlistmentRoot)) + else { - // The repo is no longer on disk (but its volume is present) - // Unregister the repo - if (repoRegistry.TryRemoveRepo(repo.EnlistmentRoot, out errorMessage)) - { - this.tracer.RelatedEvent( - EventLevel.Informational, - $"{nameof(this.RunMaintenanceTaskForRepos)}_RemovedMissingRepo", - metadata); - } - else - { - metadata[nameof(errorMessage)] = errorMessage; - this.tracer.RelatedEvent( - EventLevel.Informational, - $"{nameof(this.RunMaintenanceTaskForRepos)}_FailedToRemoveRepo", - metadata); - } - - continue; + metadata[nameof(errorMessage)] = errorMessage; + this.tracer.RelatedEvent( + EventLevel.Informational, + $"{nameof(this.RunMaintenanceTaskForRepos)}_FailedToRemoveRepo", + metadata); } - this.tracer.RelatedEvent( - EventLevel.Informational, - $"{nameof(this.RunMaintenanceTaskForRepos)}_CallingMaintenance", - metadata); - - this.scalarVerb.CallMaintenance(task, repo.EnlistmentRoot, sessionId); + continue; } + + this.tracer.RelatedEvent( + EventLevel.Informational, + $"{nameof(this.RunMaintenanceTaskForRepos)}_CallingMaintenance", + metadata); + + this.scalarVerb.CallMaintenance(task, repoRegistration.EnlistmentRoot, sessionId); + } + + if (reposForUserCount == 0) + { + metadata.Add(TracingConstants.MessageKey.InfoMessage, "No active repos for user"); + this.tracer.RelatedEvent( + EventLevel.Informational, + $"{nameof(this.RunMaintenanceTaskForRepos)}_NoRepos", + metadata); } } } diff --git a/Scalar.UnitTests/Mock/FileSystem/MockFileSystem.cs b/Scalar.UnitTests/Mock/FileSystem/MockFileSystem.cs index 62a7afabb5..d158bc06cc 100644 --- a/Scalar.UnitTests/Mock/FileSystem/MockFileSystem.cs +++ b/Scalar.UnitTests/Mock/FileSystem/MockFileSystem.cs @@ -284,6 +284,51 @@ public override IEnumerable EnumerateDirectories(string path) } } + public override IEnumerable EnumerateFiles(string path, string searchPattern) + { + string searchSuffix = null; + bool matchAll = string.IsNullOrEmpty(searchPattern) || searchPattern == "*"; + + if (!matchAll) + { + // Only support matching "*" + if (!searchPattern.StartsWith("*", StringComparison.Ordinal)) + { + throw new NotImplementedException("Unsupported search pattern"); + } + + if (searchPattern.IndexOf('*', startIndex: 1) != -1) + { + throw new NotImplementedException("Unsupported search pattern"); + } + + if (searchPattern.Contains("?")) + { + throw new NotImplementedException("Unsupported search pattern"); + } + + searchSuffix = searchPattern.Substring(1); + } + + MockDirectory directory = this.RootDirectory.FindDirectory(path); + directory.ShouldNotBeNull(); + + if (directory != null) + { + foreach (MockFile file in directory.Files.Values) + { + if (matchAll) + { + yield return file.Name; + } + else if (file.Name.EndsWith(searchSuffix, StringComparison.OrdinalIgnoreCase)) + { + yield return file.Name; + } + } + } + } + public override FileProperties GetFileProperties(string path) { MockFile file = this.RootDirectory.FindFile(path); diff --git a/Scalar/CommandLine/ServiceVerb.cs b/Scalar/CommandLine/ServiceVerb.cs index 7d4b26febb..b08f56bc17 100644 --- a/Scalar/CommandLine/ServiceVerb.cs +++ b/Scalar/CommandLine/ServiceVerb.cs @@ -37,25 +37,23 @@ public override void Execute() this.ReportErrorAndExit($"Error: You cannot specify multiple arguments. Run 'scalar {ServiceVerbName} --help' for details."); } - List repoList = this.GetRepoList(); - foreach (string repoRoot in repoList) + foreach (string repoRoot in this.GetRepoList()) { this.Output.WriteLine(repoRoot); } } - private List GetRepoList() + private IEnumerable GetRepoList() { string repoRegistryLocation = ScalarPlatform.Instance.GetDataRootForScalarComponent(ScalarConstants.RepoRegistry.RegistryDirectoryName); using (JsonTracer tracer = new JsonTracer(ScalarConstants.ScalarEtwProviderName, "ServiceVerb")) { ScalarRepoRegistry repoRegistry = new ScalarRepoRegistry( - tracer, - new PhysicalFileSystem(), - repoRegistryLocation); + tracer, + new PhysicalFileSystem(), + repoRegistryLocation); - List registeredRepos = repoRegistry.GetRegisteredRepos(); - return registeredRepos.Select(x => x.EnlistmentRoot).ToList(); + return repoRegistry.GetRegisteredRepos().Select(x => x.EnlistmentRoot); } } } From 8915f9085ead4151a9be98f5819f739ae8b554e5 Mon Sep 17 00:00:00 2001 From: William Baker Date: Tue, 5 Nov 2019 14:15:37 -0800 Subject: [PATCH 03/15] ScalarService.Windows.cs: use repo registry interface For consistency with ScalarService.Mac.cs (and as a first step toward making the Windows service more unit testable) interact with the ScalarRepoRegistry via its interface rather than directly. --- Scalar.Service/ScalarService.Windows.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Scalar.Service/ScalarService.Windows.cs b/Scalar.Service/ScalarService.Windows.cs index cee5bc23d2..3bd5ea3cb9 100644 --- a/Scalar.Service/ScalarService.Windows.cs +++ b/Scalar.Service/ScalarService.Windows.cs @@ -26,7 +26,7 @@ public class ScalarService : ServiceBase private string serviceName; private string serviceDataLocation; private string repoRegistryLocation; - private ScalarRepoRegistry repoRegistry; + private IScalarRepoRegistry repoRegistry; private ProductUpgradeTimer productUpgradeTimer; private RequestHandler requestHandler; private MaintenanceTaskScheduler maintenanceTaskScheduler; From 5126ef392b9a795564a387f7404d02262647edc4 Mon Sep 17 00:00:00 2001 From: William Baker Date: Tue, 5 Nov 2019 15:03:30 -0800 Subject: [PATCH 04/15] ScalarRepoRegistry : improve naming Improve the naming used for repository root variables to make it clear that the registry expects the paths to already be normalized. Additionally, use the term 'userId' consistently, and remove the use of 'ownerSID'. --- .../RepoRegistry/IScalarRepoRegistry.cs | 4 +- .../IScalarRepoRegistryExtensions.cs | 4 +- .../RepoRegistry/ScalarRepoRegistration.cs | 12 +++--- .../RepoRegistry/ScalarRepoRegistry.cs | 42 +++++++++---------- .../POSIXFileSystem.Shared.cs | 2 +- Scalar.Service/MaintenanceTaskScheduler.cs | 12 +++--- .../RepoRegistry/ScalarRepoRegistryTests.cs | 4 +- Scalar/CommandLine/ServiceVerb.cs | 2 +- 8 files changed, 41 insertions(+), 41 deletions(-) diff --git a/Scalar.Common/RepoRegistry/IScalarRepoRegistry.cs b/Scalar.Common/RepoRegistry/IScalarRepoRegistry.cs index 7ee688fab3..0a12a851a9 100644 --- a/Scalar.Common/RepoRegistry/IScalarRepoRegistry.cs +++ b/Scalar.Common/RepoRegistry/IScalarRepoRegistry.cs @@ -4,8 +4,8 @@ namespace Scalar.Common.RepoRegistry { public interface IScalarRepoRegistry { - bool TryRegisterRepo(string repoRoot, string ownerSID, out string errorMessage); - bool TryRemoveRepo(string repoRoot, out string errorMessage); + bool TryRegisterRepo(string normalizedRepoRoot, string userId, out string errorMessage); + bool TryRemoveRepo(string normalizedRepoRoot, out string errorMessage); IEnumerable GetRegisteredRepos(); } } diff --git a/Scalar.Common/RepoRegistry/IScalarRepoRegistryExtensions.cs b/Scalar.Common/RepoRegistry/IScalarRepoRegistryExtensions.cs index 6fa95d452a..dd82390c76 100644 --- a/Scalar.Common/RepoRegistry/IScalarRepoRegistryExtensions.cs +++ b/Scalar.Common/RepoRegistry/IScalarRepoRegistryExtensions.cs @@ -6,9 +6,9 @@ namespace Scalar.Common.RepoRegistry { public static class IScalarRepoRegistryExtensions { - public static IEnumerable GetRegisteredReposForUser(this IScalarRepoRegistry registry, string ownerSID) + public static IEnumerable GetRegisteredReposForUser(this IScalarRepoRegistry registry, string userId) { - return registry.GetRegisteredRepos().Where(x => x.OwnerSID.Equals(ownerSID, StringComparison.CurrentCultureIgnoreCase)); + return registry.GetRegisteredRepos().Where(x => x.UserId.Equals(userId, StringComparison.CurrentCultureIgnoreCase)); } } } diff --git a/Scalar.Common/RepoRegistry/ScalarRepoRegistration.cs b/Scalar.Common/RepoRegistry/ScalarRepoRegistration.cs index 2d329d3a22..123e468a3d 100644 --- a/Scalar.Common/RepoRegistry/ScalarRepoRegistration.cs +++ b/Scalar.Common/RepoRegistry/ScalarRepoRegistration.cs @@ -8,14 +8,14 @@ public ScalarRepoRegistration() { } - public ScalarRepoRegistration(string enlistmentRoot, string ownerSID) + public ScalarRepoRegistration(string normalizedRepoRoot, string userId) { - this.EnlistmentRoot = enlistmentRoot; - this.OwnerSID = ownerSID; + this.NormalizedRepoRoot = normalizedRepoRoot; + this.UserId = userId; } - public string EnlistmentRoot { get; set; } - public string OwnerSID { get; set; } + public string NormalizedRepoRoot { get; set; } + public string UserId { get; set; } public static ScalarRepoRegistration FromJson(string json) { @@ -29,7 +29,7 @@ public static ScalarRepoRegistration FromJson(string json) public override string ToString() { - return $"({this.OwnerSID}) {this.EnlistmentRoot}"; + return $"({this.UserId}) {this.NormalizedRepoRoot}"; } public string ToJson() diff --git a/Scalar.Common/RepoRegistry/ScalarRepoRegistry.cs b/Scalar.Common/RepoRegistry/ScalarRepoRegistry.cs index a4814c4501..f2153b4997 100644 --- a/Scalar.Common/RepoRegistry/ScalarRepoRegistry.cs +++ b/Scalar.Common/RepoRegistry/ScalarRepoRegistry.cs @@ -27,7 +27,7 @@ public ScalarRepoRegistry( this.registryFolderPath = repoRegistryLocation; } - public bool TryRegisterRepo(string repoRoot, string ownerSID, out string errorMessage) + public bool TryRegisterRepo(string normalizedRepoRoot, string userId, out string errorMessage) { try { @@ -49,17 +49,17 @@ public bool TryRegisterRepo(string repoRoot, string ownerSID, out string errorMe errorMessage = $"Error while ensuring registry directory '{this.registryFolderPath}' exists: {e.Message}"; EventMetadata metadata = CreateEventMetadata(e); - metadata.Add(nameof(repoRoot), repoRoot); + metadata.Add(nameof(normalizedRepoRoot), normalizedRepoRoot); metadata.Add(nameof(this.registryFolderPath), this.registryFolderPath); this.tracer.RelatedError(metadata, $"{nameof(this.TryRegisterRepo)}: Exception while ensuring registry directory exists"); return false; } - string tempRegistryPath = this.GetRepoRegistryTempFilePath(repoRoot); + string tempRegistryPath = this.GetRepoRegistryTempFilePath(normalizedRepoRoot); try { - ScalarRepoRegistration repoRegistration = new ScalarRepoRegistration(repoRoot, ownerSID); + ScalarRepoRegistration repoRegistration = new ScalarRepoRegistration(normalizedRepoRoot, userId); string registryFileContents = repoRegistration.ToJson(); using (Stream tempFile = this.fileSystem.OpenFileStream( @@ -76,26 +76,26 @@ public bool TryRegisterRepo(string repoRoot, string ownerSID, out string errorMe } catch (Exception e) { - errorMessage = $"Error while registering repo {repoRoot}: {e.Message}"; + errorMessage = $"Error while registering repo {normalizedRepoRoot}: {e.Message}"; EventMetadata metadata = CreateEventMetadata(e); - metadata.Add(nameof(repoRoot), repoRoot); + metadata.Add(nameof(normalizedRepoRoot), normalizedRepoRoot); metadata.Add(nameof(tempRegistryPath), tempRegistryPath); this.tracer.RelatedError(metadata, $"{nameof(this.TryRegisterRepo)}: Exception while writing temp registry file"); return false; } - string registryFilePath = this.GetRepoRegistryFilePath(repoRoot); + string registryFilePath = this.GetRepoRegistryFilePath(normalizedRepoRoot); try { this.fileSystem.MoveAndOverwriteFile(tempRegistryPath, registryFilePath); } catch (Win32Exception e) { - errorMessage = $"Error while registering repo {repoRoot}: {e.Message}"; + errorMessage = $"Error while registering repo {normalizedRepoRoot}: {e.Message}"; EventMetadata metadata = CreateEventMetadata(e); - metadata.Add(nameof(repoRoot), repoRoot); + metadata.Add(nameof(normalizedRepoRoot), normalizedRepoRoot); metadata.Add(nameof(tempRegistryPath), tempRegistryPath); metadata.Add(nameof(registryFilePath), registryFilePath); this.tracer.RelatedError(metadata, $"{nameof(this.TryRegisterRepo)}: Exception while renaming temp registry file"); @@ -106,15 +106,15 @@ public bool TryRegisterRepo(string repoRoot, string ownerSID, out string errorMe return true; } - public bool TryRemoveRepo(string repoRoot, out string errorMessage) + public bool TryRemoveRepo(string normalizedRepoRoot, out string errorMessage) { - string registryPath = this.GetRepoRegistryFilePath(repoRoot); + string registryPath = this.GetRepoRegistryFilePath(normalizedRepoRoot); if (!this.fileSystem.FileExists(registryPath)) { - errorMessage = $"Attempted to remove non-existent repo '{repoRoot}'"; + errorMessage = $"Attempted to remove non-existent repo '{normalizedRepoRoot}'"; EventMetadata metadata = CreateEventMetadata(); - metadata.Add(nameof(repoRoot), repoRoot); + metadata.Add(nameof(normalizedRepoRoot), normalizedRepoRoot); metadata.Add(nameof(registryPath), registryPath); this.tracer.RelatedWarning( metadata, @@ -129,10 +129,10 @@ public bool TryRemoveRepo(string repoRoot, out string errorMessage) } catch (Exception e) { - errorMessage = $"Error while removing repo {repoRoot}: {e.Message}"; + errorMessage = $"Error while removing repo {normalizedRepoRoot}: {e.Message}"; EventMetadata metadata = CreateEventMetadata(e); - metadata.Add(nameof(repoRoot), repoRoot); + metadata.Add(nameof(normalizedRepoRoot), normalizedRepoRoot); metadata.Add(nameof(registryPath), registryPath); this.tracer.RelatedWarning( metadata, @@ -175,20 +175,20 @@ public IEnumerable GetRegisteredRepos() } } - internal static string GetRepoRootSha(string repoRoot) + internal static string GetRepoRootSha(string normalizedRepoRoot) { - return SHA1Util.SHA1HashStringForUTF8String(repoRoot.ToLowerInvariant()); + return SHA1Util.SHA1HashStringForUTF8String(normalizedRepoRoot.ToLowerInvariant()); } - internal string GetRepoRegistryTempFilePath(string repoRoot) + internal string GetRepoRegistryTempFilePath(string normalizedRepoRoot) { - string repoTempFilename = $"{GetRepoRootSha(repoRoot)}{RegistryTempFileExtension}"; + string repoTempFilename = $"{GetRepoRootSha(normalizedRepoRoot)}{RegistryTempFileExtension}"; return Path.Combine(this.registryFolderPath, repoTempFilename); } - internal string GetRepoRegistryFilePath(string repoRoot) + internal string GetRepoRegistryFilePath(string normalizedRepoRoot) { - string repoFilename = $"{GetRepoRootSha(repoRoot)}{RegistryFileExtension}"; + string repoFilename = $"{GetRepoRootSha(normalizedRepoRoot)}{RegistryFileExtension}"; return Path.Combine(this.registryFolderPath, repoFilename); } diff --git a/Scalar.Platform.POSIX/POSIXFileSystem.Shared.cs b/Scalar.Platform.POSIX/POSIXFileSystem.Shared.cs index f6eb6c75dd..c8ff350073 100644 --- a/Scalar.Platform.POSIX/POSIXFileSystem.Shared.cs +++ b/Scalar.Platform.POSIX/POSIXFileSystem.Shared.cs @@ -4,7 +4,7 @@ public partial class POSIXFileSystem { public static bool TryGetNormalizedPathImplementation(string path, out string normalizedPath, out string errorMessage) { - // TODO(#1358): Properly determine normalized paths (e.g. across links) + // TODO(#217): Properly determine normalized paths (e.g. across links) errorMessage = null; normalizedPath = path; return true; diff --git a/Scalar.Service/MaintenanceTaskScheduler.cs b/Scalar.Service/MaintenanceTaskScheduler.cs index 6385ec69be..11be8033d9 100644 --- a/Scalar.Service/MaintenanceTaskScheduler.cs +++ b/Scalar.Service/MaintenanceTaskScheduler.cs @@ -212,9 +212,9 @@ public void RunMaintenanceTaskForRepos( { ++reposForUserCount; - rootPath = Path.GetPathRoot(repoRegistration.EnlistmentRoot); + rootPath = Path.GetPathRoot(repoRegistration.NormalizedRepoRoot); - metadata[nameof(repoRegistration.EnlistmentRoot)] = repoRegistration.EnlistmentRoot; + metadata[nameof(repoRegistration.NormalizedRepoRoot)] = repoRegistration.NormalizedRepoRoot; metadata[nameof(task)] = task; metadata[nameof(rootPath)] = rootPath; metadata.Remove(nameof(errorMessage)); @@ -231,11 +231,11 @@ public void RunMaintenanceTaskForRepos( continue; } - if (!this.fileSystem.DirectoryExists(repoRegistration.EnlistmentRoot)) + if (!this.fileSystem.DirectoryExists(repoRegistration.NormalizedRepoRoot)) { // The repo is no longer on disk (but its volume is present) // Unregister the repo - if (repoRegistry.TryRemoveRepo(repoRegistration.EnlistmentRoot, out errorMessage)) + if (repoRegistry.TryRemoveRepo(repoRegistration.NormalizedRepoRoot, out errorMessage)) { this.tracer.RelatedEvent( EventLevel.Informational, @@ -259,12 +259,12 @@ public void RunMaintenanceTaskForRepos( $"{nameof(this.RunMaintenanceTaskForRepos)}_CallingMaintenance", metadata); - this.scalarVerb.CallMaintenance(task, repoRegistration.EnlistmentRoot, sessionId); + this.scalarVerb.CallMaintenance(task, repoRegistration.NormalizedRepoRoot, sessionId); } if (reposForUserCount == 0) { - metadata.Add(TracingConstants.MessageKey.InfoMessage, "No active repos for user"); + metadata.Add(TracingConstants.MessageKey.InfoMessage, "No registered repos for user"); this.tracer.RelatedEvent( EventLevel.Informational, $"{nameof(this.RunMaintenanceTaskForRepos)}_NoRepos", diff --git a/Scalar.UnitTests/Common/RepoRegistry/ScalarRepoRegistryTests.cs b/Scalar.UnitTests/Common/RepoRegistry/ScalarRepoRegistryTests.cs index 61ea224be8..4d8a760d3b 100644 --- a/Scalar.UnitTests/Common/RepoRegistry/ScalarRepoRegistryTests.cs +++ b/Scalar.UnitTests/Common/RepoRegistry/ScalarRepoRegistryTests.cs @@ -45,10 +45,10 @@ public void TearDown() //// this.VerifyRepo(verifiableRegistry[repoRoot], ownerSID); ////} - private void VerifyRepo(ScalarRepoRegistration repo, string expectedOwnerSID) + private void VerifyRepo(ScalarRepoRegistration repo, string expectedUserId) { repo.ShouldNotBeNull(); - repo.OwnerSID.ShouldEqual(expectedOwnerSID); + repo.UserId.ShouldEqual(expectedUserId); } } } diff --git a/Scalar/CommandLine/ServiceVerb.cs b/Scalar/CommandLine/ServiceVerb.cs index b08f56bc17..06178d6823 100644 --- a/Scalar/CommandLine/ServiceVerb.cs +++ b/Scalar/CommandLine/ServiceVerb.cs @@ -53,7 +53,7 @@ private IEnumerable GetRepoList() new PhysicalFileSystem(), repoRegistryLocation); - return repoRegistry.GetRegisteredRepos().Select(x => x.EnlistmentRoot); + return repoRegistry.GetRegisteredRepos().Select(x => x.NormalizedRepoRoot); } } } From 20336e69e701adf1997d3adb0b67a54bf603621b Mon Sep 17 00:00:00 2001 From: William Baker Date: Tue, 5 Nov 2019 16:59:57 -0800 Subject: [PATCH 05/15] ScalarRepoRegistry: add first unit test Add an initial unit test to ScalarRepoRegistry that validates the registry creates its directory if it's missing from disk. --- .../Should/EnumerableShouldExtensions.cs | 5 +- .../RepoRegistry/ScalarRepoRegistryTests.cs | 59 +++++++++---------- .../Mock/FileSystem/MockFileSystem.cs | 4 +- 3 files changed, 34 insertions(+), 34 deletions(-) diff --git a/Scalar.Tests/Should/EnumerableShouldExtensions.cs b/Scalar.Tests/Should/EnumerableShouldExtensions.cs index c3dca5f90e..966d38e8f4 100644 --- a/Scalar.Tests/Should/EnumerableShouldExtensions.cs +++ b/Scalar.Tests/Should/EnumerableShouldExtensions.cs @@ -148,7 +148,10 @@ public bool Equals(T x, T y) public int GetHashCode(T obj) { - return obj.GetHashCode(); + // Return a constant to force Equals(...) to be called. + // This is required for custom T types that do not implement + // GetHashCode + return 1; } } } diff --git a/Scalar.UnitTests/Common/RepoRegistry/ScalarRepoRegistryTests.cs b/Scalar.UnitTests/Common/RepoRegistry/ScalarRepoRegistryTests.cs index 4d8a760d3b..83d10df883 100644 --- a/Scalar.UnitTests/Common/RepoRegistry/ScalarRepoRegistryTests.cs +++ b/Scalar.UnitTests/Common/RepoRegistry/ScalarRepoRegistryTests.cs @@ -13,42 +13,39 @@ namespace Scalar.UnitTests.Common.RepoRegistry [TestFixture] public class ScalarRepoRegistryTests { - [SetUp] - public void Setup() + [TestCase] + public void TryRegisterRepo_CreatesMissingRegistryDirectory() { - } + string registryFolderPath = Path.Combine("mock:", "root", "UnitTests.RepoRegistry"); - [TearDown] - public void TearDown() - { - } + MockFileSystem fileSystem = new MockFileSystem(new MockDirectory(Path.GetDirectoryName(registryFolderPath), null, null)); + ScalarRepoRegistry registry = new ScalarRepoRegistry( + new MockTracer(), + fileSystem, + registryFolderPath); + + string testRepoRoot = Path.Combine("mock:", "Repos", "Repo1"); + string testUserId = "testUser"; - ////[TestCase] - ////public void TryRegisterRepo_EmptyRegistry() - ////{ - //// string dataLocation = Path.Combine("mock:", "registryDataFolder"); - //// - //// MockFileSystem fileSystem = new MockFileSystem(new MockDirectory(dataLocation, null, null)); - //// RepoRegistry registry = new RepoRegistry( - //// new MockTracer(), - //// fileSystem, - //// dataLocation); - //// - //// string repoRoot = Path.Combine("c:", "test"); - //// string ownerSID = Guid.NewGuid().ToString(); - //// - //// string errorMessage; - //// registry.TryRegisterRepo(repoRoot, ownerSID, out errorMessage).ShouldEqual(true); - //// - //// Dictionary verifiableRegistry = registry.ReadRegistry(); - //// verifiableRegistry.Count.ShouldEqual(1); - //// this.VerifyRepo(verifiableRegistry[repoRoot], ownerSID); - ////} + fileSystem.DirectoryExists(registryFolderPath).ShouldBeFalse(); + registry.TryRegisterRepo(testRepoRoot, testUserId, out string errorMessage).ShouldBeTrue(); + errorMessage.ShouldBeNull(); + fileSystem.DirectoryExists(registryFolderPath).ShouldBeTrue("Registering a repo should have created the missing registry directory"); + + List expectedRegistrations = new List + { + new ScalarRepoRegistration(testRepoRoot, testUserId) + }; + + registry.GetRegisteredRepos().ShouldMatchInOrder(expectedRegistrations, RepoRegistrationsEqual); + registry.GetRegisteredReposForUser(testUserId).ShouldMatchInOrder(expectedRegistrations, RepoRegistrationsEqual); + } - private void VerifyRepo(ScalarRepoRegistration repo, string expectedUserId) + private static bool RepoRegistrationsEqual(ScalarRepoRegistration repo1, ScalarRepoRegistration repo2) { - repo.ShouldNotBeNull(); - repo.UserId.ShouldEqual(expectedUserId); + return + repo1.NormalizedRepoRoot.Equals(repo2.NormalizedRepoRoot, StringComparison.Ordinal) && + repo1.UserId.Equals(repo2.UserId, StringComparison.Ordinal); } } } diff --git a/Scalar.UnitTests/Mock/FileSystem/MockFileSystem.cs b/Scalar.UnitTests/Mock/FileSystem/MockFileSystem.cs index d158bc06cc..9ae5459c7e 100644 --- a/Scalar.UnitTests/Mock/FileSystem/MockFileSystem.cs +++ b/Scalar.UnitTests/Mock/FileSystem/MockFileSystem.cs @@ -319,11 +319,11 @@ public override IEnumerable EnumerateFiles(string path, string searchPat { if (matchAll) { - yield return file.Name; + yield return file.FullName; } else if (file.Name.EndsWith(searchSuffix, StringComparison.OrdinalIgnoreCase)) { - yield return file.Name; + yield return file.FullName; } } } From dd103b16b748d98c3d03ded138f8103370a7e92d Mon Sep 17 00:00:00 2001 From: William Baker Date: Wed, 6 Nov 2019 12:39:22 -0800 Subject: [PATCH 06/15] ScalarRepoRegistry: complete unit tests Complete the unit tests for ScalarRepoRegistry. Additionally, the following improvements were made: - Renamed "TryRemoveRepo" to "TryUnregisterRepo" to better match the method for registering repos - Updated several Should extensions in the test to provide better validation --- .../RepoRegistry/IScalarRepoRegistry.cs | 2 +- .../RepoRegistry/ScalarRepoRegistry.cs | 30 +-- Scalar.Service/MaintenanceTaskScheduler.cs | 2 +- .../Should/EnumerableShouldExtensions.cs | 51 +++- Scalar.Tests/Should/StringShouldExtensions.cs | 6 + .../RepoRegistry/ScalarRepoRegistryTests.cs | 234 +++++++++++++++++- 6 files changed, 286 insertions(+), 39 deletions(-) diff --git a/Scalar.Common/RepoRegistry/IScalarRepoRegistry.cs b/Scalar.Common/RepoRegistry/IScalarRepoRegistry.cs index 0a12a851a9..ad28eab592 100644 --- a/Scalar.Common/RepoRegistry/IScalarRepoRegistry.cs +++ b/Scalar.Common/RepoRegistry/IScalarRepoRegistry.cs @@ -5,7 +5,7 @@ namespace Scalar.Common.RepoRegistry public interface IScalarRepoRegistry { bool TryRegisterRepo(string normalizedRepoRoot, string userId, out string errorMessage); - bool TryRemoveRepo(string normalizedRepoRoot, out string errorMessage); + bool TryUnregisterRepo(string normalizedRepoRoot, out string errorMessage); IEnumerable GetRegisteredRepos(); } } diff --git a/Scalar.Common/RepoRegistry/ScalarRepoRegistry.cs b/Scalar.Common/RepoRegistry/ScalarRepoRegistry.cs index f2153b4997..4b3ada7828 100644 --- a/Scalar.Common/RepoRegistry/ScalarRepoRegistry.cs +++ b/Scalar.Common/RepoRegistry/ScalarRepoRegistry.cs @@ -106,7 +106,7 @@ public bool TryRegisterRepo(string normalizedRepoRoot, string userId, out string return true; } - public bool TryRemoveRepo(string normalizedRepoRoot, out string errorMessage) + public bool TryUnregisterRepo(string normalizedRepoRoot, out string errorMessage) { string registryPath = this.GetRepoRegistryFilePath(normalizedRepoRoot); if (!this.fileSystem.FileExists(registryPath)) @@ -118,7 +118,7 @@ public bool TryRemoveRepo(string normalizedRepoRoot, out string errorMessage) metadata.Add(nameof(registryPath), registryPath); this.tracer.RelatedWarning( metadata, - $"{nameof(this.TryRemoveRepo)}: Attempted to remove non-existent repo"); + $"{nameof(this.TryUnregisterRepo)}: Attempted to remove non-existent repo"); return false; } @@ -136,7 +136,7 @@ public bool TryRemoveRepo(string normalizedRepoRoot, out string errorMessage) metadata.Add(nameof(registryPath), registryPath); this.tracer.RelatedWarning( metadata, - $"{nameof(this.TryRemoveRepo)}: Exception while removing repo"); + $"{nameof(this.TryUnregisterRepo)}: Exception while removing repo"); return false; } @@ -180,18 +180,6 @@ internal static string GetRepoRootSha(string normalizedRepoRoot) return SHA1Util.SHA1HashStringForUTF8String(normalizedRepoRoot.ToLowerInvariant()); } - internal string GetRepoRegistryTempFilePath(string normalizedRepoRoot) - { - string repoTempFilename = $"{GetRepoRootSha(normalizedRepoRoot)}{RegistryTempFileExtension}"; - return Path.Combine(this.registryFolderPath, repoTempFilename); - } - - internal string GetRepoRegistryFilePath(string normalizedRepoRoot) - { - string repoFilename = $"{GetRepoRootSha(normalizedRepoRoot)}{RegistryFileExtension}"; - return Path.Combine(this.registryFolderPath, repoFilename); - } - private static EventMetadata CreateEventMetadata(Exception e = null) { EventMetadata metadata = new EventMetadata(); @@ -203,5 +191,17 @@ private static EventMetadata CreateEventMetadata(Exception e = null) return metadata; } + + private string GetRepoRegistryTempFilePath(string normalizedRepoRoot) + { + string repoTempFilename = $"{GetRepoRootSha(normalizedRepoRoot)}{RegistryTempFileExtension}"; + return Path.Combine(this.registryFolderPath, repoTempFilename); + } + + private string GetRepoRegistryFilePath(string normalizedRepoRoot) + { + string repoFilename = $"{GetRepoRootSha(normalizedRepoRoot)}{RegistryFileExtension}"; + return Path.Combine(this.registryFolderPath, repoFilename); + } } } diff --git a/Scalar.Service/MaintenanceTaskScheduler.cs b/Scalar.Service/MaintenanceTaskScheduler.cs index 11be8033d9..2870fe8afb 100644 --- a/Scalar.Service/MaintenanceTaskScheduler.cs +++ b/Scalar.Service/MaintenanceTaskScheduler.cs @@ -235,7 +235,7 @@ public void RunMaintenanceTaskForRepos( { // The repo is no longer on disk (but its volume is present) // Unregister the repo - if (repoRegistry.TryRemoveRepo(repoRegistration.NormalizedRepoRoot, out errorMessage)) + if (repoRegistry.TryUnregisterRepo(repoRegistration.NormalizedRepoRoot, out errorMessage)) { this.tracer.RelatedEvent( EventLevel.Informational, diff --git a/Scalar.Tests/Should/EnumerableShouldExtensions.cs b/Scalar.Tests/Should/EnumerableShouldExtensions.cs index 966d38e8f4..281deaf6b7 100644 --- a/Scalar.Tests/Should/EnumerableShouldExtensions.cs +++ b/Scalar.Tests/Should/EnumerableShouldExtensions.cs @@ -89,6 +89,36 @@ public static IEnumerable ShouldMatchInOrder(this IEnumerable group, pa } public static IEnumerable ShouldMatchInOrder(this IEnumerable group, IEnumerable expectedValues, Func equals, string message = "") + { + return group.ShouldMatch(expectedValues, equals, shouldMatchInOrder: true, message: message); + } + + public static IEnumerable ShouldMatchInOrder(this IEnumerable group, params T[] expectedValues) + { + return group.ShouldMatchInOrder((IEnumerable)expectedValues); + } + + public static IEnumerable ShouldMatchInOrder(this IEnumerable group, IEnumerable expectedValues) + { + return group.ShouldMatchInOrder(expectedValues, (t1, t2) => t1.Equals(t2)); + } + + public static IEnumerable ShouldMatch(this IEnumerable group, IEnumerable expectedValues, Func equals, string message = "") + { + return group.ShouldMatch(expectedValues, equals, shouldMatchInOrder: false, message: message); + } + + public static IEnumerable ShouldMatch(this IEnumerable group, params T[] expectedValues) + { + return group.ShouldMatch((IEnumerable)expectedValues); + } + + public static IEnumerable ShouldMatch(this IEnumerable group, IEnumerable expectedValues) + { + return group.ShouldMatch(expectedValues, (t1, t2) => t1.Equals(t2)); + } + + private static IEnumerable ShouldMatch(this IEnumerable group, IEnumerable expectedValues, Func equals, bool shouldMatchInOrder, string message = "") { List groupList = new List(group); List expectedValuesList = new List(expectedValues); @@ -114,6 +144,17 @@ public static IEnumerable ShouldMatchInOrder(this IEnumerable group, IE errorMessage.AppendLine(string.Format("Missing: {0}", groupMissingItem)); } + if (shouldMatchInOrder) + { + for (int i = 0; i < groupList.Count; ++i) + { + if (!equals(groupList[i], expectedValuesList[i])) + { + errorMessage.AppendLine($"Items ordered differently, found: {groupList[i]} expected: {expectedValuesList[i]}"); + } + } + } + if (errorMessage.Length > 0) { Assert.Fail("{0}\r\n{1}", message, errorMessage); @@ -122,16 +163,6 @@ public static IEnumerable ShouldMatchInOrder(this IEnumerable group, IE return group; } - public static IEnumerable ShouldMatchInOrder(this IEnumerable group, params T[] expectedValues) - { - return group.ShouldMatchInOrder((IEnumerable)expectedValues); - } - - public static IEnumerable ShouldMatchInOrder(this IEnumerable group, IEnumerable expectedValues) - { - return group.ShouldMatchInOrder(expectedValues, (t1, t2) => t1.Equals(t2)); - } - private class Comparer : IEqualityComparer { private Func equals; diff --git a/Scalar.Tests/Should/StringShouldExtensions.cs b/Scalar.Tests/Should/StringShouldExtensions.cs index 8c7eb6b761..983c66e6f6 100644 --- a/Scalar.Tests/Should/StringShouldExtensions.cs +++ b/Scalar.Tests/Should/StringShouldExtensions.cs @@ -64,5 +64,11 @@ public static string ShouldContainOneOf(this string actualValue, params string[] Assert.Fail("No expected substrings found in '{0}'", actualValue); return actualValue; } + + public static string ShouldBeNonEmpty(this string value) + { + Assert.IsFalse(string.IsNullOrEmpty(value)); + return value; + } } } diff --git a/Scalar.UnitTests/Common/RepoRegistry/ScalarRepoRegistryTests.cs b/Scalar.UnitTests/Common/RepoRegistry/ScalarRepoRegistryTests.cs index 83d10df883..18973d61e0 100644 --- a/Scalar.UnitTests/Common/RepoRegistry/ScalarRepoRegistryTests.cs +++ b/Scalar.UnitTests/Common/RepoRegistry/ScalarRepoRegistryTests.cs @@ -1,44 +1,223 @@ using Moq; using NUnit.Framework; +using Scalar.Common.FileSystem; using Scalar.Common.RepoRegistry; using Scalar.Tests.Should; +using Scalar.UnitTests.Category; using Scalar.UnitTests.Mock.Common; using Scalar.UnitTests.Mock.FileSystem; using System; using System.Collections.Generic; using System.IO; +using System.Linq; namespace Scalar.UnitTests.Common.RepoRegistry { [TestFixture] public class ScalarRepoRegistryTests { + private readonly string registryFolderPath = Path.Combine("mock:", "root", "UnitTests.RepoRegistry"); + [TestCase] public void TryRegisterRepo_CreatesMissingRegistryDirectory() { - string registryFolderPath = Path.Combine("mock:", "root", "UnitTests.RepoRegistry"); - - MockFileSystem fileSystem = new MockFileSystem(new MockDirectory(Path.GetDirectoryName(registryFolderPath), null, null)); + MockFileSystem fileSystem = new MockFileSystem(new MockDirectory(Path.GetDirectoryName(this.registryFolderPath), null, null)); ScalarRepoRegistry registry = new ScalarRepoRegistry( new MockTracer(), fileSystem, - registryFolderPath); + this.registryFolderPath); + + List registrations = new List + { + new ScalarRepoRegistration(Path.Combine("mock:", "Repos", "Repo1"), "testUser") + }; + + fileSystem.DirectoryExists(this.registryFolderPath).ShouldBeFalse(); + this.RegisterRepos(registry, registrations); + fileSystem.DirectoryExists(this.registryFolderPath).ShouldBeTrue("Registering a repo should have created the missing registry directory"); + + this.RegistryShouldContainRegistrations(registry, registrations); + } + + [TestCase] + [Category(CategoryConstants.ExceptionExpected)] + public void TryRegisterRepo_FailsIfMissingRegistryDirectoryCantBeCreated() + { + Mock mockFileSystem; + mockFileSystem = new Mock(MockBehavior.Strict); + mockFileSystem.Setup(fileSystem => fileSystem.DirectoryExists(this.registryFolderPath)).Returns(false); + mockFileSystem.Setup(fileSystem => fileSystem.CreateDirectory(this.registryFolderPath)).Throws(new UnauthorizedAccessException()); + + ScalarRepoRegistry registry = new ScalarRepoRegistry( + new MockTracer(), + mockFileSystem.Object, + this.registryFolderPath); string testRepoRoot = Path.Combine("mock:", "Repos", "Repo1"); string testUserId = "testUser"; - fileSystem.DirectoryExists(registryFolderPath).ShouldBeFalse(); - registry.TryRegisterRepo(testRepoRoot, testUserId, out string errorMessage).ShouldBeTrue(); - errorMessage.ShouldBeNull(); - fileSystem.DirectoryExists(registryFolderPath).ShouldBeTrue("Registering a repo should have created the missing registry directory"); + registry.TryRegisterRepo(testRepoRoot, testUserId, out string errorMessage).ShouldBeFalse(); + errorMessage.ShouldBeNonEmpty(); + + mockFileSystem.VerifyAll(); + } + + [TestCase] + public void TryRegisterRepo_RegisterMultipleRepos() + { + MockFileSystem fileSystem = new MockFileSystem(new MockDirectory(Path.GetDirectoryName(this.registryFolderPath), null, null)); + ScalarRepoRegistry registry = new ScalarRepoRegistry( + new MockTracer(), + fileSystem, + this.registryFolderPath); + + List repoRegistrations = new List + { + new ScalarRepoRegistration(Path.Combine("mock:", "Repos", "Repo1"), "testUser"), + new ScalarRepoRegistration(Path.Combine("mock:", "Repos", "Repo2"), "testUser"), + new ScalarRepoRegistration(Path.Combine("mock:", "MoreRepos", "Repo1"), "user2"), + new ScalarRepoRegistration(Path.Combine("mock:", "MoreRepos", "Repo2"), "user2"), + new ScalarRepoRegistration(Path.Combine("mock:", "Repos2", "Repo1"), "testUser"), + new ScalarRepoRegistration(Path.Combine("mock:", "Repos3", "Repo1"), "ThirdUser"), + new ScalarRepoRegistration(Path.Combine("mock:", "Repos3", "Repo2"), "ThirdUser") + }; + + this.RegisterRepos(registry, repoRegistrations); + this.RegistryShouldContainRegistrations(registry, repoRegistrations); + } - List expectedRegistrations = new List + [TestCase] + public void TryRegisterRepo_UpdatesUsersForExistingRegistrations() + { + MockFileSystem fileSystem = new MockFileSystem(new MockDirectory(Path.GetDirectoryName(this.registryFolderPath), null, null)); + ScalarRepoRegistry registry = new ScalarRepoRegistry( + new MockTracer(), + fileSystem, + this.registryFolderPath); + + List registrationsPart1 = new List + { + new ScalarRepoRegistration(Path.Combine("mock:", "Repos", "Repo1"), "testUser"), + new ScalarRepoRegistration(Path.Combine("mock:", "MoreRepos", "Repo1"), "user2"), + new ScalarRepoRegistration(Path.Combine("mock:", "Repos2", "Repo1"), "testUser") + }; + + List registrationsPart2 = new List { - new ScalarRepoRegistration(testRepoRoot, testUserId) + new ScalarRepoRegistration(Path.Combine("mock:", "Repos", "Repo2"), "testUser"), + new ScalarRepoRegistration(Path.Combine("mock:", "MoreRepos", "Repo2"), "user2"), + new ScalarRepoRegistration(Path.Combine("mock:", "Repos3", "Repo1"), "ThirdUser"), + new ScalarRepoRegistration(Path.Combine("mock:", "Repos3", "Repo2"), "ThirdUser") }; - registry.GetRegisteredRepos().ShouldMatchInOrder(expectedRegistrations, RepoRegistrationsEqual); - registry.GetRegisteredReposForUser(testUserId).ShouldMatchInOrder(expectedRegistrations, RepoRegistrationsEqual); + this.RegisterRepos(registry, registrationsPart1.Concat(registrationsPart2)); + this.RegistryShouldContainRegistrations(registry, registrationsPart1.Concat(registrationsPart2)); + + // Update the users on some registrations + foreach (ScalarRepoRegistration registration in registrationsPart2) + { + registration.UserId = $"UPDATED_{registration.UserId}"; + } + + // Just register the updates + this.RegisterRepos(registry, registrationsPart2); + + // The unchanged + updated entries should be present + this.RegistryShouldContainRegistrations(registry, registrationsPart1.Concat(registrationsPart2)); + } + + [TestCase] + public void TryUnregisterRepo_RemovesRegisteredRepos() + { + MockFileSystem fileSystem = new MockFileSystem(new MockDirectory(Path.GetDirectoryName(this.registryFolderPath), null, null)); + ScalarRepoRegistry registry = new ScalarRepoRegistry( + new MockTracer(), + fileSystem, + this.registryFolderPath); + + List registrationsPart1 = new List + { + new ScalarRepoRegistration(Path.Combine("mock:", "Repos", "Repo1"), "testUser"), + new ScalarRepoRegistration(Path.Combine("mock:", "MoreRepos", "Repo1"), "user2"), + new ScalarRepoRegistration(Path.Combine("mock:", "Repos2", "Repo1"), "testUser") + }; + + List registrationsPart2 = new List + { + new ScalarRepoRegistration(Path.Combine("mock:", "Repos", "Repo2"), "testUser"), + new ScalarRepoRegistration(Path.Combine("mock:", "MoreRepos", "Repo2"), "user2"), + new ScalarRepoRegistration(Path.Combine("mock:", "Repos3", "Repo1"), "ThirdUser"), + new ScalarRepoRegistration(Path.Combine("mock:", "Repos3", "Repo2"), "ThirdUser") + }; + + this.RegisterRepos(registry, registrationsPart1.Concat(registrationsPart2)); + this.RegistryShouldContainRegistrations(registry, registrationsPart1.Concat(registrationsPart2)); + this.UnregisterRepos(registry, registrationsPart2); + this.RegistryShouldContainRegistrations(registry, registrationsPart1); + } + + [TestCase] + public void TryUnregisterRepo_FailsIfRegistryDirectoryMissing() + { + MockFileSystem fileSystem = new MockFileSystem(new MockDirectory(Path.GetDirectoryName(this.registryFolderPath), null, null)); + ScalarRepoRegistry registry = new ScalarRepoRegistry( + new MockTracer(), + fileSystem, + this.registryFolderPath); + fileSystem.DirectoryExists(this.registryFolderPath).ShouldBeFalse(); + registry.TryUnregisterRepo(Path.Combine("mock:", "Repos", "Repo1"), out string errorMessage).ShouldBeFalse(); + errorMessage.ShouldBeNonEmpty(); + fileSystem.DirectoryExists(this.registryFolderPath).ShouldBeFalse(); + } + + [TestCase] + public void TryUnregisterRepo_FailsForUnregisteredRepo() + { + MockFileSystem fileSystem = new MockFileSystem(new MockDirectory(Path.GetDirectoryName(this.registryFolderPath), null, null)); + ScalarRepoRegistry registry = new ScalarRepoRegistry( + new MockTracer(), + fileSystem, + this.registryFolderPath); + + List registrations = new List + { + new ScalarRepoRegistration(Path.Combine("mock:", "Repos", "Repo1"), "testUser") + }; + + this.RegisterRepos(registry, registrations); + registry.TryUnregisterRepo(Path.Combine("mock:", "Repos", "Repo2"), out string errorMessage).ShouldBeFalse(); + errorMessage.ShouldBeNonEmpty(); + this.RegistryShouldContainRegistrations(registry, registrations); + } + + [TestCase] + [Category(CategoryConstants.ExceptionExpected)] + public void TryUnregisterRepo_FailsIfDeleteFails() + { + string repoPath = Path.Combine("mock:", "Repos", "Repo1"); + string registrationFilePath = Path.Combine("mock:", "root", "UnitTests.RepoRegistry", ScalarRepoRegistry.GetRepoRootSha(repoPath) + ".repo"); + + Mock mockFileSystem; + mockFileSystem = new Mock(MockBehavior.Strict); + mockFileSystem.Setup(fileSystem => fileSystem.FileExists(registrationFilePath)).Returns(true); + mockFileSystem.Setup(fileSystem => fileSystem.DeleteFile(registrationFilePath)).Throws(new UnauthorizedAccessException()); + + ScalarRepoRegistry registry = new ScalarRepoRegistry( + new MockTracer(), + mockFileSystem.Object, + this.registryFolderPath); + + registry.TryUnregisterRepo(repoPath, out string errorMessage).ShouldBeFalse(); + errorMessage.ShouldBeNonEmpty(); + + mockFileSystem.VerifyAll(); + } + + [TestCase] + public void GetRepoRootSha_IsStable() + { + ScalarRepoRegistry.GetRepoRootSha("mock:/Repos/Repo1").ShouldEqual("7fe00a3479e25557a439fc5a2b19a1ec2605fb59"); + ScalarRepoRegistry.GetRepoRootSha("mock:/folder/repoRoot").ShouldEqual("7e83aa2e7baab0b4bcd39eaeb3c8b189e5167297"); } private static bool RepoRegistrationsEqual(ScalarRepoRegistration repo1, ScalarRepoRegistration repo2) @@ -47,5 +226,36 @@ private static bool RepoRegistrationsEqual(ScalarRepoRegistration repo1, ScalarR repo1.NormalizedRepoRoot.Equals(repo2.NormalizedRepoRoot, StringComparison.Ordinal) && repo1.UserId.Equals(repo2.UserId, StringComparison.Ordinal); } + + private void RegisterRepos(ScalarRepoRegistry registry, IEnumerable registrations) + { + foreach (ScalarRepoRegistration registration in registrations) + { + registry.TryRegisterRepo(registration.NormalizedRepoRoot, registration.UserId, out string errorMessage).ShouldBeTrue(); + errorMessage.ShouldBeNull(); + } + } + + private void UnregisterRepos(ScalarRepoRegistry registry, IEnumerable registrations) + { + foreach (ScalarRepoRegistration registration in registrations) + { + registry.TryUnregisterRepo(registration.NormalizedRepoRoot, out string errorMessage).ShouldBeTrue(); + errorMessage.ShouldBeNull(); + } + } + + private void RegistryShouldContainRegistrations(ScalarRepoRegistry registry, IEnumerable registrations) + { + registry.GetRegisteredRepos().ShouldMatch(registrations, RepoRegistrationsEqual); + + IEnumerable registeredUsers = registrations.Select(r => r.UserId).Distinct(); + foreach (string user in registeredUsers) + { + registry.GetRegisteredReposForUser(user).ShouldMatch( + registrations.Where(x => x.UserId.Equals(user)), + RepoRegistrationsEqual); + } + } } } From e9a86e89883cc9a3e54b6e90d83aaaaa229ec176 Mon Sep 17 00:00:00 2001 From: William Baker Date: Wed, 6 Nov 2019 13:13:37 -0800 Subject: [PATCH 07/15] UnitTests: remove MacServiceTests and add ScalarVerbRunnerTests The one test still active in MacServiceTests was actually a test of ScalarVerbRunner.Mac. Remove MacServiceTests and replace it with ScalarVerbRunnerTests --- .../Service/Mac/MacServiceTests.cs | 119 ------------------ .../Service/Mac/ScalarVerbRunnerTests.cs | 51 ++++++++ 2 files changed, 51 insertions(+), 119 deletions(-) delete mode 100644 Scalar.UnitTests/Service/Mac/MacServiceTests.cs create mode 100644 Scalar.UnitTests/Service/Mac/ScalarVerbRunnerTests.cs diff --git a/Scalar.UnitTests/Service/Mac/MacServiceTests.cs b/Scalar.UnitTests/Service/Mac/MacServiceTests.cs deleted file mode 100644 index 902463014d..0000000000 --- a/Scalar.UnitTests/Service/Mac/MacServiceTests.cs +++ /dev/null @@ -1,119 +0,0 @@ -using Moq; -using NUnit.Framework; -using Scalar.Common; -using Scalar.Common.Maintenance; -using Scalar.Common.RepoRegistry; -using Scalar.Service; -using Scalar.Tests.Should; -using Scalar.UnitTests.Mock.Common; -using Scalar.UnitTests.Mock.FileSystem; -using System.IO; - -namespace Scalar.UnitTests.Service.Mac -{ - [TestFixture] - public class MacServiceTests - { - private const int ExpectedActiveUserId = 502; - private const int ExpectedSessionId = 502; - private static readonly string ExpectedActiveRepoPath = Path.Combine("mock:", "code", "repo2"); - private static readonly string ServiceDataLocation = Path.Combine("mock:", "registryDataFolder"); - - private MockFileSystem fileSystem; - private MockTracer tracer; - private MockPlatform scalarPlatform; - - [SetUp] - public void SetUp() - { - this.tracer = new MockTracer(); - this.fileSystem = new MockFileSystem(new MockDirectory(ServiceDataLocation, null, null)); - this.scalarPlatform = (MockPlatform)ScalarPlatform.Instance; - this.scalarPlatform.MockCurrentUser = ExpectedActiveUserId.ToString(); - } - - ////[TestCase] - ////public void RepoRegistryRemovesRegisteredRepoIfMissingFromDisk() - ////{ - //// Mock repoMounterMock = new Mock(MockBehavior.Strict); - //// - //// this.fileSystem.DirectoryExists(ExpectedActiveRepoPath).ShouldBeFalse($"{ExpectedActiveRepoPath} should not exist"); - //// - //// MaintenanceTasks.Task task = MaintenanceTasks.Task.FetchCommitsAndTrees; - //// - //// this.CreateTestRepos(ServiceDataLocation); - //// RepoRegistry repoRegistry = new RepoRegistry( - //// this.tracer, - //// this.fileSystem, - //// ServiceDataLocation); - //// - //// repoRegistry.RunMaintenanceTaskForRepos(task, ExpectedActiveUserId.ToString(), ExpectedSessionId); - //// repoMounterMock.VerifyAll(); - //// - //// repoRegistry.ReadRegistry().ShouldNotContain(entry => entry.Key.Equals(ExpectedActiveRepoPath)); - ////} - - ////[TestCase] - ////public void RepoRegistryCallsMaintenanceVerbOnlyForRegisteredRepos() - ////{ - //// Mock repoMounterMock = new Mock(MockBehavior.Strict); - //// - //// this.fileSystem.CreateDirectory(ExpectedActiveRepoPath); - //// - //// MaintenanceTasks.Task task = MaintenanceTasks.Task.FetchCommitsAndTrees; - //// repoMounterMock.Setup(mp => mp.CallMaintenance(task, ExpectedActiveRepoPath, ExpectedActiveUserId)).Returns(true); - //// - //// this.CreateTestRepos(ServiceDataLocation); - //// - //// RepoRegistry repoRegistry = new RepoRegistry( - //// this.tracer, - //// this.fileSystem, - //// ServiceDataLocation, - //// repoMounterMock.Object); - //// - //// repoRegistry.RunMaintenanceTaskForRepos(task, ExpectedActiveUserId.ToString(), ExpectedSessionId); - //// - //// repoMounterMock.VerifyAll(); - ////} - - [TestCase] - public void MaintenanceVerbLaunchedUsingCorrectArgs() - { - MaintenanceTasks.Task task = MaintenanceTasks.Task.FetchCommitsAndTrees; - string taskVerbName = MaintenanceTasks.GetVerbTaskName(task); - string executable = @"/bin/launchctl"; - string scalarBinPath = Path.Combine(this.scalarPlatform.Constants.ScalarBinDirectoryPath, this.scalarPlatform.Constants.ScalarExecutableName); - string expectedArgs = - $"asuser {ExpectedActiveUserId} {scalarBinPath} maintenance \"{ExpectedActiveRepoPath}\" --{ScalarConstants.VerbParameters.Maintenance.Task} {taskVerbName} --{ScalarConstants.VerbParameters.InternalUseOnly} {new InternalVerbParameters(startedByService: true).ToJson()}"; - - Mock mountLauncherMock = new Mock(MockBehavior.Strict, this.tracer); - mountLauncherMock.Setup(mp => mp.LaunchProcess( - executable, - expectedArgs, - ExpectedActiveRepoPath)) - .Returns(new ProcessResult(output: string.Empty, errors: string.Empty, exitCode: 0)); - - ScalarVerbRunner verbProcess = new ScalarVerbRunner(this.tracer, mountLauncherMock.Object); - verbProcess.CallMaintenance(task, ExpectedActiveRepoPath, ExpectedActiveUserId); - - mountLauncherMock.VerifyAll(); - } - - ////private void CreateTestRepos(string dataLocation) - ////{ - //// string repo1 = Path.Combine("mock:", "code", "repo1"); - //// string repo2 = ExpectedActiveRepoPath; - //// string repo3 = Path.Combine("mock:", "code", "repo3"); - //// string repo4 = Path.Combine("mock:", "code", "repo4"); - //// - //// this.fileSystem.WriteAllText( - //// Path.Combine(dataLocation, RepoRegistry.RegistryName), - //// $@"1 - //// {{""EnlistmentRoot"":""{repo1.Replace("\\", "\\\\")}"",""OwnerSID"":502,""IsActive"":false}} - //// {{""EnlistmentRoot"":""{repo2.Replace("\\", "\\\\")}"",""OwnerSID"":502,""IsActive"":true}} - //// {{""EnlistmentRoot"":""{repo3.Replace("\\", "\\\\")}"",""OwnerSID"":501,""IsActive"":false}} - //// {{""EnlistmentRoot"":""{repo4.Replace("\\", "\\\\")}"",""OwnerSID"":501,""IsActive"":true}} - //// "); - ////} - } -} diff --git a/Scalar.UnitTests/Service/Mac/ScalarVerbRunnerTests.cs b/Scalar.UnitTests/Service/Mac/ScalarVerbRunnerTests.cs new file mode 100644 index 0000000000..b1e7f44070 --- /dev/null +++ b/Scalar.UnitTests/Service/Mac/ScalarVerbRunnerTests.cs @@ -0,0 +1,51 @@ +using Moq; +using NUnit.Framework; +using Scalar.Common; +using Scalar.Common.Maintenance; +using Scalar.Service; +using Scalar.UnitTests.Mock.Common; +using System.IO; + +namespace Scalar.UnitTests.Service.Mac +{ + [TestFixture] + public class ScalarVerbRunnerTests + { + private const int ExpectedActiveUserId = 502; + private static readonly string ExpectedActiveRepoPath = Path.Combine("mock:", "code", "repo2"); + + private MockTracer tracer; + private MockPlatform scalarPlatform; + + [SetUp] + public void SetUp() + { + this.tracer = new MockTracer(); + this.scalarPlatform = (MockPlatform)ScalarPlatform.Instance; + this.scalarPlatform.MockCurrentUser = ExpectedActiveUserId.ToString(); + } + + [TestCase] + public void CallMaintenance_LaunchesVerbUsingCorrectArgs() + { + MaintenanceTasks.Task task = MaintenanceTasks.Task.FetchCommitsAndTrees; + string taskVerbName = MaintenanceTasks.GetVerbTaskName(task); + string executable = @"/bin/launchctl"; + string scalarBinPath = Path.Combine(this.scalarPlatform.Constants.ScalarBinDirectoryPath, this.scalarPlatform.Constants.ScalarExecutableName); + string expectedArgs = + $"asuser {ExpectedActiveUserId} {scalarBinPath} maintenance \"{ExpectedActiveRepoPath}\" --{ScalarConstants.VerbParameters.Maintenance.Task} {taskVerbName} --{ScalarConstants.VerbParameters.InternalUseOnly} {new InternalVerbParameters(startedByService: true).ToJson()}"; + + Mock mountLauncherMock = new Mock(MockBehavior.Strict, this.tracer); + mountLauncherMock.Setup(mp => mp.LaunchProcess( + executable, + expectedArgs, + ExpectedActiveRepoPath)) + .Returns(new ProcessResult(output: string.Empty, errors: string.Empty, exitCode: 0)); + + ScalarVerbRunner verbProcess = new ScalarVerbRunner(this.tracer, mountLauncherMock.Object); + verbProcess.CallMaintenance(task, ExpectedActiveRepoPath, ExpectedActiveUserId); + + mountLauncherMock.VerifyAll(); + } + } +} From 9c086a2f258c6144624041001165dc1b65db18ec Mon Sep 17 00:00:00 2001 From: William Baker Date: Wed, 6 Nov 2019 15:04:56 -0800 Subject: [PATCH 08/15] MaintenanceTaskScheduler: add unit tests Add unit tests for MaintenanceTaskScheduler and its nested class MaintenanceTask. Additionally: - Update MockTracer to track calls to RelatedEvent so that they can be validated by the unit tests. - Update the code that generates CommonAssemblyVersion.cs so that the unit tests have access to internal members of Scalar.Service - Add GetTestRoot() to MockFileSystem that will return an appropriate mock root (based on the system's directory separator char) --- Scalar.Build/GenerateVersionInfo.cs | 2 + Scalar.Service/MaintenanceTaskScheduler.cs | 55 ++--- .../RepoRegistry/ScalarRepoRegistryTests.cs | 6 +- Scalar.UnitTests/Mock/Common/MockTracer.cs | 6 + .../Mock/FileSystem/MockFileSystem.cs | 17 ++ .../Service/MaintenanceTaskSchedulerTests.cs | 213 ++++++++++++++++++ Scripts/Mac/GenerateCommonAssemblyVersion.sh | 2 + 7 files changed, 265 insertions(+), 36 deletions(-) create mode 100644 Scalar.UnitTests/Service/MaintenanceTaskSchedulerTests.cs diff --git a/Scalar.Build/GenerateVersionInfo.cs b/Scalar.Build/GenerateVersionInfo.cs index 1895ecde1e..c8966c8486 100644 --- a/Scalar.Build/GenerateVersionInfo.cs +++ b/Scalar.Build/GenerateVersionInfo.cs @@ -27,9 +27,11 @@ public override bool Execute() this.AssemblyVersion, string.Format( @"using System.Reflection; +using System.Runtime.CompilerServices; [assembly: AssemblyVersion(""{0}"")] [assembly: AssemblyFileVersion(""{0}"")] +[assembly: InternalsVisibleTo(""Scalar.UnitTests"")] ", this.Version)); diff --git a/Scalar.Service/MaintenanceTaskScheduler.cs b/Scalar.Service/MaintenanceTaskScheduler.cs index 2870fe8afb..a4802ef8f3 100644 --- a/Scalar.Service/MaintenanceTaskScheduler.cs +++ b/Scalar.Service/MaintenanceTaskScheduler.cs @@ -122,21 +122,7 @@ private void ScheduleRecurringTasks() } } - private class MaintenanceSchedule - { - public MaintenanceSchedule(MaintenanceTasks.Task task, TimeSpan dueTime, TimeSpan period) - { - this.Task = task; - this.DueTime = dueTime; - this.Period = period; - } - - public MaintenanceTasks.Task Task { get; } - public TimeSpan DueTime { get; } - public TimeSpan Period { get; } - } - - private class MaintenanceTask : IServiceTask + internal class MaintenanceTask : IServiceTask { private readonly ITracer tracer; private readonly PhysicalFileSystem fileSystem; @@ -176,11 +162,7 @@ public void Execute() $"{nameof(MaintenanceTaskScheduler)}.{nameof(this.Execute)}", metadata); - this.RunMaintenanceTaskForRepos( - this.task, - this.repoRegistry, - registeredUser.UserId, - registeredUser.SessionId); + this.RunMaintenanceTaskForRepos(registeredUser); } else { @@ -193,29 +175,24 @@ public void Stop() // TODO: #185 - Kill the currently running maintenance verb } - public void RunMaintenanceTaskForRepos( - MaintenanceTasks.Task task, - IScalarRepoRegistry repoRegistry, - string userId, - int sessionId) + public void RunMaintenanceTaskForRepos(UserAndSession registeredUser) { EventMetadata metadata = new EventMetadata(); - metadata.Add(nameof(task), MaintenanceTasks.GetVerbTaskName(task)); - metadata.Add(nameof(userId), userId); - metadata.Add(nameof(sessionId), sessionId); + metadata.Add(nameof(this.task), MaintenanceTasks.GetVerbTaskName(this.task)); + metadata.Add(nameof(registeredUser.UserId), registeredUser.UserId); + metadata.Add(nameof(registeredUser.SessionId), registeredUser.SessionId); int reposForUserCount = 0; string rootPath; string errorMessage; - foreach (ScalarRepoRegistration repoRegistration in repoRegistry.GetRegisteredReposForUser(userId)) + foreach (ScalarRepoRegistration repoRegistration in this.repoRegistry.GetRegisteredReposForUser(registeredUser.UserId)) { ++reposForUserCount; rootPath = Path.GetPathRoot(repoRegistration.NormalizedRepoRoot); metadata[nameof(repoRegistration.NormalizedRepoRoot)] = repoRegistration.NormalizedRepoRoot; - metadata[nameof(task)] = task; metadata[nameof(rootPath)] = rootPath; metadata.Remove(nameof(errorMessage)); @@ -235,7 +212,7 @@ public void RunMaintenanceTaskForRepos( { // The repo is no longer on disk (but its volume is present) // Unregister the repo - if (repoRegistry.TryUnregisterRepo(repoRegistration.NormalizedRepoRoot, out errorMessage)) + if (this.repoRegistry.TryUnregisterRepo(repoRegistration.NormalizedRepoRoot, out errorMessage)) { this.tracer.RelatedEvent( EventLevel.Informational, @@ -259,7 +236,7 @@ public void RunMaintenanceTaskForRepos( $"{nameof(this.RunMaintenanceTaskForRepos)}_CallingMaintenance", metadata); - this.scalarVerb.CallMaintenance(task, repoRegistration.NormalizedRepoRoot, sessionId); + this.scalarVerb.CallMaintenance(this.task, repoRegistration.NormalizedRepoRoot, registeredUser.SessionId); } if (reposForUserCount == 0) @@ -272,5 +249,19 @@ public void RunMaintenanceTaskForRepos( } } } + + private class MaintenanceSchedule + { + public MaintenanceSchedule(MaintenanceTasks.Task task, TimeSpan dueTime, TimeSpan period) + { + this.Task = task; + this.DueTime = dueTime; + this.Period = period; + } + + public MaintenanceTasks.Task Task { get; } + public TimeSpan DueTime { get; } + public TimeSpan Period { get; } + } } } diff --git a/Scalar.UnitTests/Common/RepoRegistry/ScalarRepoRegistryTests.cs b/Scalar.UnitTests/Common/RepoRegistry/ScalarRepoRegistryTests.cs index 18973d61e0..e3db3f975b 100644 --- a/Scalar.UnitTests/Common/RepoRegistry/ScalarRepoRegistryTests.cs +++ b/Scalar.UnitTests/Common/RepoRegistry/ScalarRepoRegistryTests.cs @@ -43,8 +43,7 @@ public void TryRegisterRepo_CreatesMissingRegistryDirectory() [Category(CategoryConstants.ExceptionExpected)] public void TryRegisterRepo_FailsIfMissingRegistryDirectoryCantBeCreated() { - Mock mockFileSystem; - mockFileSystem = new Mock(MockBehavior.Strict); + Mock mockFileSystem = new Mock(MockBehavior.Strict); mockFileSystem.Setup(fileSystem => fileSystem.DirectoryExists(this.registryFolderPath)).Returns(false); mockFileSystem.Setup(fileSystem => fileSystem.CreateDirectory(this.registryFolderPath)).Throws(new UnauthorizedAccessException()); @@ -197,8 +196,7 @@ public void TryUnregisterRepo_FailsIfDeleteFails() string repoPath = Path.Combine("mock:", "Repos", "Repo1"); string registrationFilePath = Path.Combine("mock:", "root", "UnitTests.RepoRegistry", ScalarRepoRegistry.GetRepoRootSha(repoPath) + ".repo"); - Mock mockFileSystem; - mockFileSystem = new Mock(MockBehavior.Strict); + Mock mockFileSystem = new Mock(MockBehavior.Strict); mockFileSystem.Setup(fileSystem => fileSystem.FileExists(registrationFilePath)).Returns(true); mockFileSystem.Setup(fileSystem => fileSystem.DeleteFile(registrationFilePath)).Throws(new UnauthorizedAccessException()); diff --git a/Scalar.UnitTests/Mock/Common/MockTracer.cs b/Scalar.UnitTests/Mock/Common/MockTracer.cs index fbc525714e..608878e804 100644 --- a/Scalar.UnitTests/Mock/Common/MockTracer.cs +++ b/Scalar.UnitTests/Mock/Common/MockTracer.cs @@ -13,6 +13,7 @@ public class MockTracer : ITracer public MockTracer() { this.waitEvent = new AutoResetEvent(false); + this.RelatedEvents = new List(); this.RelatedInfoEvents = new List(); this.RelatedWarningEvents = new List(); this.RelatedErrorEvents = new List(); @@ -21,6 +22,7 @@ public MockTracer() public MockTracer StartActivityTracer { get; private set; } public string WaitRelatedEventName { get; set; } + public List RelatedEvents { get; } public List RelatedInfoEvents { get; } public List RelatedWarningEvents { get; } public List RelatedErrorEvents { get; } @@ -36,6 +38,8 @@ public void RelatedEvent(EventLevel error, string eventName, EventMetadata metad { this.waitEvent.Set(); } + + this.RelatedEvents.Add($"EventName:'{eventName}', metadata: {JsonConvert.SerializeObject(metadata)}"); } public void RelatedEvent(EventLevel error, string eventName, EventMetadata metadata, Keywords keyword) @@ -44,6 +48,8 @@ public void RelatedEvent(EventLevel error, string eventName, EventMetadata metad { this.waitEvent.Set(); } + + this.RelatedEvents.Add($"EventName:'{eventName}', metadata: {JsonConvert.SerializeObject(metadata)}"); } public void RelatedInfo(string message) diff --git a/Scalar.UnitTests/Mock/FileSystem/MockFileSystem.cs b/Scalar.UnitTests/Mock/FileSystem/MockFileSystem.cs index 9ae5459c7e..54860e623e 100644 --- a/Scalar.UnitTests/Mock/FileSystem/MockFileSystem.cs +++ b/Scalar.UnitTests/Mock/FileSystem/MockFileSystem.cs @@ -1,3 +1,4 @@ +using NUnit.Framework; using Scalar.Common; using Scalar.Common.FileSystem; using Scalar.Common.Tracing; @@ -39,6 +40,22 @@ public MockFileSystem(MockDirectory rootDirectory) /// public bool DeleteNonExistentFileThrowsException { get; set; } + public static string GetTestRoot() + { + switch (Path.DirectorySeparatorChar) + { + case '/': + return "/mock"; + + case '\\': + return "B:"; + + default: + Assert.Fail($"Unknown DirectorySeparatorChar '{Path.DirectorySeparatorChar}'"); + return null; + } + } + public override void DeleteDirectory(string path, bool recursive = true) { if (!recursive) diff --git a/Scalar.UnitTests/Service/MaintenanceTaskSchedulerTests.cs b/Scalar.UnitTests/Service/MaintenanceTaskSchedulerTests.cs new file mode 100644 index 0000000000..9482d75714 --- /dev/null +++ b/Scalar.UnitTests/Service/MaintenanceTaskSchedulerTests.cs @@ -0,0 +1,213 @@ +using Moq; +using NUnit.Framework; +using Scalar.Common.FileSystem; +using Scalar.Common.Maintenance; +using Scalar.Common.RepoRegistry; +using Scalar.Service; +using Scalar.Tests.Should; +using Scalar.UnitTests.Mock.Common; +using Scalar.UnitTests.Mock.FileSystem; +using System.Collections.Generic; +using System.IO; + +namespace Scalar.UnitTests.Service +{ + [TestFixture] + public class MaintenanceTaskSchedulerTests + { + private MockTracer mockTracer; + private Mock mockFileSystem; + private Mock mockVerbRunner; + private Mock mockRepoRegistry; + private Mock mockRegisteredUserStore; + + [SetUp] + public void Setup() + { + this.mockTracer = new MockTracer(); + this.mockFileSystem = new Mock(MockBehavior.Strict); + this.mockVerbRunner = new Mock(MockBehavior.Strict); + this.mockRepoRegistry = new Mock(MockBehavior.Strict); + this.mockRegisteredUserStore = new Mock(MockBehavior.Strict); + } + + [TearDown] + public void TearDown() + { + this.mockFileSystem.VerifyAll(); + this.mockVerbRunner.VerifyAll(); + this.mockRepoRegistry.VerifyAll(); + this.mockRegisteredUserStore.VerifyAll(); + } + + [TestCase] + public void RegisterUser() + { + using (MaintenanceTaskScheduler taskScheduler = new MaintenanceTaskScheduler( + this.mockTracer, + this.mockFileSystem.Object, + this.mockVerbRunner.Object, + this.mockRepoRegistry.Object)) + { + taskScheduler.RegisteredUser.ShouldBeNull(); + + UserAndSession testUser1 = new UserAndSession("testUser1", sessionId: 1); + UserAndSession testUser2 = new UserAndSession("testUser2", sessionId: 2); + + taskScheduler.RegisterUser(testUser1); + taskScheduler.RegisteredUser.UserId.ShouldEqual(testUser1.UserId); + taskScheduler.RegisteredUser.SessionId.ShouldEqual(testUser1.SessionId); + + taskScheduler.RegisterUser(testUser2); + taskScheduler.RegisteredUser.UserId.ShouldEqual(testUser2.UserId); + taskScheduler.RegisteredUser.SessionId.ShouldEqual(testUser2.SessionId); + } + } + + [TestCase] + public void MaintenanceTask_Execute_NoRegisteredUser() + { + MaintenanceTasks.Task task = MaintenanceTasks.Task.PackFiles; + + this.mockRegisteredUserStore.SetupGet(mrus => mrus.RegisteredUser).Returns((UserAndSession)null); + + MaintenanceTaskScheduler.MaintenanceTask maintenanceTask = new MaintenanceTaskScheduler.MaintenanceTask( + this.mockTracer, + this.mockFileSystem.Object, + this.mockVerbRunner.Object, + this.mockRepoRegistry.Object, + this.mockRegisteredUserStore.Object, + task); + + maintenanceTask.Execute(); + this.mockTracer.RelatedInfoEvents.ShouldContain(entry => entry.Contains($"Skipping '{task}', no registered user")); + } + + [TestCase] + public void MaintenanceTask_Execute_SkipsReposThatDoNotMatchRegisteredUser() + { + MaintenanceTasks.Task task = MaintenanceTasks.Task.PackFiles; + + UserAndSession testUser = new UserAndSession("testUserId", sessionId: 1); + this.mockRegisteredUserStore.SetupGet(mrus => mrus.RegisteredUser).Returns(testUser); + + this.mockRepoRegistry.Setup(reg => reg.GetRegisteredRepos()).Returns( + new List + { + new ScalarRepoRegistration(Path.Combine(MockFileSystem.GetTestRoot(), "Repos", "repoRoot"), "nonMatchingUser"), + new ScalarRepoRegistration(Path.Combine(MockFileSystem.GetTestRoot(), "Repos", "repoRoot2"), "nonMatchingUser2") + }); + + MaintenanceTaskScheduler.MaintenanceTask maintenanceTask = new MaintenanceTaskScheduler.MaintenanceTask( + this.mockTracer, + this.mockFileSystem.Object, + this.mockVerbRunner.Object, + this.mockRepoRegistry.Object, + this.mockRegisteredUserStore.Object, + task); + + maintenanceTask.Execute(); + this.mockTracer.RelatedEvents.ShouldContain(entry => entry.Contains("No registered repos for user")); + } + + [TestCase] + public void MaintenanceTask_RunMaintenanceTaskForRepos_SkipsRegisteredRepoIfVolumeDoesNotExist() + { + MaintenanceTasks.Task task = MaintenanceTasks.Task.PackFiles; + + UserAndSession testUser = new UserAndSession("testUserId", sessionId: 1); + string repoPath = Path.Combine(MockFileSystem.GetTestRoot(), "Repos", "repoRoot"); + this.mockRepoRegistry.Setup(reg => reg.GetRegisteredRepos()).Returns( + new List + { + new ScalarRepoRegistration(repoPath, testUser.UserId) + }); + + this.mockFileSystem.Setup(fs => fs.DirectoryExists(Path.GetPathRoot(repoPath))).Returns(false); + + MaintenanceTaskScheduler.MaintenanceTask maintenanceTask = new MaintenanceTaskScheduler.MaintenanceTask( + this.mockTracer, + this.mockFileSystem.Object, + this.mockVerbRunner.Object, + this.mockRepoRegistry.Object, + this.mockRegisteredUserStore.Object, + task); + + maintenanceTask.RunMaintenanceTaskForRepos(testUser); + this.mockTracer.RelatedEvents.ShouldContain(entry => entry.Contains("SkippedRepoWithMissingVolume")); + } + + [TestCase] + public void MaintenanceTask_RunMaintenanceTaskForRepos_UnregistersRepoIfMissing() + { + MaintenanceTasks.Task task = MaintenanceTasks.Task.PackFiles; + + UserAndSession testUser = new UserAndSession("testUserId", sessionId: 1); + string repoPath = Path.Combine(MockFileSystem.GetTestRoot(), "Repos", "repoRoot"); + this.mockRepoRegistry.Setup(reg => reg.GetRegisteredRepos()).Returns( + new List + { + new ScalarRepoRegistration(repoPath, testUser.UserId) + }); + + // Validate that TryUnregisterRepo will be called for repoPath + string emptyString = string.Empty; + this.mockRepoRegistry.Setup(reg => reg.TryUnregisterRepo(repoPath, out emptyString)).Returns(true); + + // The root volume should exist + this.mockFileSystem.Setup(fs => fs.DirectoryExists(Path.GetPathRoot(repoPath))).Returns(true); + + // The repo itself does not exist + this.mockFileSystem.Setup(fs => fs.DirectoryExists(repoPath)).Returns(false); + + MaintenanceTaskScheduler.MaintenanceTask maintenanceTask = new MaintenanceTaskScheduler.MaintenanceTask( + this.mockTracer, + this.mockFileSystem.Object, + this.mockVerbRunner.Object, + this.mockRepoRegistry.Object, + this.mockRegisteredUserStore.Object, + task); + + maintenanceTask.RunMaintenanceTaskForRepos(testUser); + this.mockTracer.RelatedEvents.ShouldContain(entry => entry.Contains("RemovedMissingRepo")); + } + + [TestCase] + public void MaintenanceTask_RunMaintenanceTaskForRepos_CallsMaintenanceVerbOnlyForRegisteredRepos() + { + MaintenanceTasks.Task task = MaintenanceTasks.Task.PackFiles; + + UserAndSession testUser = new UserAndSession("testUserId", sessionId: 1); + UserAndSession secondUser = new UserAndSession("testUserId2", sessionId: 1); + string repoPath1 = Path.Combine(MockFileSystem.GetTestRoot(), "Repos", "repoRoot"); + string repoPath2 = Path.Combine(MockFileSystem.GetTestRoot(), "Repos", "repoRoot2"); + string secondUsersRepoPath = Path.Combine(MockFileSystem.GetTestRoot(), "Repos", "secondUsersRepo"); + + this.mockRepoRegistry.Setup(reg => reg.GetRegisteredRepos()).Returns( + new List + { + new ScalarRepoRegistration(repoPath1, testUser.UserId), + new ScalarRepoRegistration(secondUsersRepoPath, secondUser.UserId), + new ScalarRepoRegistration(repoPath2, testUser.UserId) + }); + + // The root volume and repos exist + this.mockFileSystem.Setup(fs => fs.DirectoryExists(Path.GetPathRoot(repoPath1))).Returns(true); + this.mockFileSystem.Setup(fs => fs.DirectoryExists(repoPath1)).Returns(true); + this.mockFileSystem.Setup(fs => fs.DirectoryExists(repoPath2)).Returns(true); + + this.mockVerbRunner.Setup(vr => vr.CallMaintenance(task, repoPath1, testUser.SessionId)).Returns(true); + this.mockVerbRunner.Setup(vr => vr.CallMaintenance(task, repoPath2, testUser.SessionId)).Returns(true); + + MaintenanceTaskScheduler.MaintenanceTask maintenanceTask = new MaintenanceTaskScheduler.MaintenanceTask( + this.mockTracer, + this.mockFileSystem.Object, + this.mockVerbRunner.Object, + this.mockRepoRegistry.Object, + this.mockRegisteredUserStore.Object, + task); + + maintenanceTask.RunMaintenanceTaskForRepos(testUser); + } + } +} diff --git a/Scripts/Mac/GenerateCommonAssemblyVersion.sh b/Scripts/Mac/GenerateCommonAssemblyVersion.sh index acb15f9c12..887564cbbe 100755 --- a/Scripts/Mac/GenerateCommonAssemblyVersion.sh +++ b/Scripts/Mac/GenerateCommonAssemblyVersion.sh @@ -10,7 +10,9 @@ sed -i "" -E "s@[0-9]+(\.[0-9]+)*@ # Then generate CommonAssemblyVersion.cs cat >$Scalar_OUTPUTDIR/CommonAssemblyVersion.cs <