Skip to content

Commit

Permalink
Scalar.Service: unregister repo when it's not on disk
Browse files Browse the repository at this point in the history
Update the RepoRegistry to check if the enlistment root
exists on disk before attempting to run the maintenance
verb against it.  If the enlistment root does not exist
on disk, but it's root directory *does* exist, then
also remove the repo from the registry.

Checking that the root of the enlistment path exists
before removing the repo from the registry ensures that
we will not remove repos that are temporarily unavailable
(e.g. due to Bitlocker or a removable drive not being
present). See the discussion in
microsoft/VFSForGit#359 for details.
  • Loading branch information
wilbaker committed Oct 31, 2019
1 parent f1e7fce commit 920a1b0
Show file tree
Hide file tree
Showing 2 changed files with 70 additions and 8 deletions.
53 changes: 45 additions & 8 deletions Scalar.Service/RepoRegistry.cs
Original file line number Diff line number Diff line change
Expand Up @@ -174,22 +174,59 @@ public void RunMaintenanceTaskForRepos(MaintenanceTasks.Task task, string userId
}
else
{
metadata.Add(TracingConstants.MessageKey.InfoMessage, "Calling maintenance verb");
string rootPath;
string errorMessage;

foreach (RepoRegistration repo in activeRepos)
{
rootPath = Path.GetPathRoot(repo.EnlistmentRoot);

metadata[nameof(repo.EnlistmentRoot)] = repo.EnlistmentRoot;
metadata[nameof(task)] = task;
this.tracer.RelatedEvent(
EventLevel.Informational,
$"{nameof(this.RunMaintenanceTaskForRepos)}_CallingMaintenance",
metadata);
metadata[nameof(rootPath)] = rootPath;
metadata.Remove(nameof(errorMessage));

if (!this.scalarVerb.CallMaintenance(task, repo.EnlistmentRoot, sessionId))
if (!string.IsNullOrWhiteSpace(rootPath) && !this.fileSystem.DirectoryExists(rootPath))
{
// TODO: #111 - If the maintenance verb failed because the repo is no longer
// on disk, it should be removed from the registry
// 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);
}
}
}
Expand Down
25 changes: 25 additions & 0 deletions Scalar.UnitTests/Service/Mac/MacServiceTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
using Scalar.Common;
using Scalar.Common.Maintenance;
using Scalar.Service;
using Scalar.Tests.Should;
using Scalar.UnitTests.Mock.Common;
using Scalar.UnitTests.Mock.FileSystem;
using System.IO;
Expand Down Expand Up @@ -30,11 +31,35 @@ public void SetUp()
this.scalarPlatform.MockCurrentUser = ExpectedActiveUserId.ToString();
}

[TestCase]
public void RepoRegistryRemovesRegisteredRepoIfMissingFromDisk()
{
Mock<IScalarVerbRunner> repoMounterMock = new Mock<IScalarVerbRunner>(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<IScalarVerbRunner> repoMounterMock = new Mock<IScalarVerbRunner>(MockBehavior.Strict);

this.fileSystem.CreateDirectory(ExpectedActiveRepoPath);

MaintenanceTasks.Task task = MaintenanceTasks.Task.FetchCommitsAndTrees;
repoMounterMock.Setup(mp => mp.CallMaintenance(task, ExpectedActiveRepoPath, ExpectedActiveUserId)).Returns(true);

Expand Down

0 comments on commit 920a1b0

Please sign in to comment.