Skip to content

Commit

Permalink
ScalarRepoRegistry : improve naming
Browse files Browse the repository at this point in the history 
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'.
  • Loading branch information
@wilbaker
wilbaker committed Nov 5, 2019
1 parent 8915f90 commit 5126ef3
Show file tree
Hide file tree
Showing 8 changed files with 41 additions and 41 deletions.
4 changes: 2 additions & 2 deletions Scalar.Common/RepoRegistry/IScalarRepoRegistry.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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<ScalarRepoRegistration> GetRegisteredRepos();
}
}
4 changes: 2 additions & 2 deletions Scalar.Common/RepoRegistry/IScalarRepoRegistryExtensions.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,9 @@ namespace Scalar.Common.RepoRegistry
{
public static class IScalarRepoRegistryExtensions
{
public static IEnumerable<ScalarRepoRegistration> GetRegisteredReposForUser(this IScalarRepoRegistry registry, string ownerSID)
public static IEnumerable<ScalarRepoRegistration> 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));
}
}
}
12 changes: 6 additions & 6 deletions Scalar.Common/RepoRegistry/ScalarRepoRegistration.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
{
Expand All @@ -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()
Expand Down
42 changes: 21 additions & 21 deletions Scalar.Common/RepoRegistry/ScalarRepoRegistry.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
{
Expand All @@ -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(
Expand All @@ -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");
Expand All @@ -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,
Expand All @@ -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,
Expand Down Expand Up @@ -175,20 +175,20 @@ public IEnumerable<ScalarRepoRegistration> 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);
}

Expand Down
2 changes: 1 addition & 1 deletion Scalar.Platform.POSIX/POSIXFileSystem.Shared.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
12 changes: 6 additions & 6 deletions Scalar.Service/MaintenanceTaskScheduler.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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));
Expand All @@ -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,
Expand All @@ -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",
Expand Down
4 changes: 2 additions & 2 deletions Scalar.UnitTests/Common/RepoRegistry/ScalarRepoRegistryTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}
}
2 changes: 1 addition & 1 deletion Scalar/CommandLine/ServiceVerb.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ private IEnumerable<string> GetRepoList()
new PhysicalFileSystem(),
repoRegistryLocation);

return repoRegistry.GetRegisteredRepos().Select(x => x.EnlistmentRoot);
return repoRegistry.GetRegisteredRepos().Select(x => x.NormalizedRepoRoot);
}
}
}
Expand Down

0 comments on commit 5126ef3

Please sign in to comment.