Skip to content

Remove temp paths from ModifiedPathsDatabase #207

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
kewillford merged 14 commits into from
Sep 26, 2018
Merged

Remove temp paths from ModifiedPathsDatabase #207

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
3f8fd9e
Add tests for removing entries from the modified files database
Aug 22, 2018
a0f9b40
Add TryRemove method to the ModifiedPathsDatabase
Aug 21, 2018
ae5f9ba
Handle the file system callbacks to track created then deleted files
Sep 5, 2018
b50dd1a
Add handling of rename of files for modified paths cleanup
Sep 6, 2018
848e6d3
Handle cleaning up modified paths when a folder is renamed
Sep 7, 2018
c189b2c
Add null check when creating trace metadata using and exception
Sep 8, 2018
1772519
Fix broken tests
Sep 11, 2018
9f8944e
Simplify checking the newly created files and removing from modified …
Sep 12, 2018
f7e268a
Fix broken tests for cleaning modified paths
Sep 20, 2018
a0023aa
Remove extra return statments and adding to new paths during rename
Sep 21, 2018
dcf925f
Add PreDelete handler in BG operation to handle Mac
Sep 24, 2018
45159db
Add comment explaining only one of predelete or delete should be queued
Sep 25, 2018
472904c
Fix case only folder rename test
Sep 25, 2018
243f621
Fix test that moved new folder outside the repo
Sep 25, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
99 changes: 80 additions & 19 deletions GVFS/GVFS.Common/ModifiedPathsDatabase.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,30 +75,39 @@ public bool TryAdd(string path, bool isFolder, out bool isRetryable)
}
catch (IOException e)
{
if (this.Tracer != null)
{
EventMetadata metadata = new EventMetadata();
metadata.Add("Area", "ModifiedPathsDatabase");
metadata.Add(nameof(entry), entry);
metadata.Add(nameof(isFolder), isFolder);
metadata.Add("Exception", e.ToString());
this.Tracer.RelatedWarning(metadata, $"IOException caught while processing {nameof(this.TryAdd)}");
}

this.TraceWarning(isFolder, entry, e, nameof(this.TryAdd));
return false;
}
catch (Exception e)
{
if (this.Tracer != null)
{
EventMetadata metadata = new EventMetadata();
metadata.Add("Area", "ModifiedPathsDatabase");
metadata.Add(nameof(entry), entry);
metadata.Add(nameof(isFolder), isFolder);
metadata.Add("Exception", e.ToString());
this.Tracer.RelatedError(metadata, $"Exception caught while processing {nameof(this.TryAdd)}");
}
this.TraceError(isFolder, entry, e, nameof(this.TryAdd));
isRetryable = false;
return false;
}
}

return true;
}

public bool TryRemove(string path, bool isFolder, out bool isRetryable)
{
isRetryable = true;
string entry = this.NormalizeEntryString(path, isFolder);
if (this.modifiedPaths.Contains(entry))
{
isRetryable = true;
try
{
this.WriteRemoveEntry(entry, () => this.modifiedPaths.TryRemove(entry));
}
catch (IOException e)
{
this.TraceWarning(isFolder, entry, e, nameof(this.TryRemove));
return false;
}
catch (Exception e)
{
this.TraceError(isFolder, entry, e, nameof(this.TryRemove));
isRetryable = false;
return false;
}
Expand All @@ -107,6 +116,58 @@ public bool TryAdd(string path, bool isFolder, out bool isRetryable)
return true;
}

public void WriteAllEntriesAndFlush()
{
try
{
this.WriteAndReplaceDataFile(this.GenerateDataLines);
}
catch (Exception e)
{
throw new FileBasedCollectionException(e);
}
}

private static EventMetadata CreateEventMetadata(bool isFolder, string entry, Exception e)
{
EventMetadata metadata = new EventMetadata();
metadata.Add("Area", "ModifiedPathsDatabase");
metadata.Add(nameof(entry), entry);
metadata.Add(nameof(isFolder), isFolder);
if (e != null)
{
metadata.Add("Exception", e.ToString());
}

return metadata;
}

private IEnumerable<string> GenerateDataLines()
{
foreach (string entry in this.modifiedPaths)
{
yield return this.FormatAddLine(entry);
Comment thread
kewillford marked this conversation as resolved.
}
}

private void TraceWarning(bool isFolder, string entry, Exception e, string method)
{
if (this.Tracer != null)
{
EventMetadata metadata = CreateEventMetadata(isFolder, entry, e);
this.Tracer.RelatedWarning(metadata, $"{e.GetType().Name} caught while processing {method}");
}
}

private void TraceError(bool isFolder, string entry, Exception e, string method)
{
if (this.Tracer != null)
{
EventMetadata metadata = CreateEventMetadata(isFolder, entry, e);
this.Tracer.RelatedError(metadata, $"{e.GetType().Name} caught while processing {method}");
}
}

private bool TryParseAddLine(string line, out string key, out string value, out string error)
{
key = line;
Expand Down
42 changes: 24 additions & 18 deletions GVFS/GVFS.FunctionalTests.Windows/Windows/Tests/DiskLayoutUpgradeTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -324,24 +324,30 @@ public void MountCreatesModifiedPathsDatabase()
this.Enlistment.MountGVFS();
this.Enlistment.UnmountGVFS();

string expectedModifiedPaths = @"A .gitattributes
A developer/me/
A developer/me/JLANGE9._prerazzle
A developer/me/StateSwitch.Save
A tools/x86/remote.exe
A tools/x86/runelevated.exe
A tools/amd64/remote.exe
A tools/amd64/runelevated.exe
A tools/perllib/MS/TraceLogging.dll
A tools/managed/v2.0/midldd.CheckedInExe
A tools/managed/v4.0/sdapi.dll
A tools/managed/v2.0/midlpars.dll
A tools/managed/v2.0/RPCDataSupport.dll
A tools/managed/v2.0/MidlStaticAnalysis.dll
A tools/perllib/MS/Somefile.txt
";

modifiedPathsDatabasePath.ShouldBeAFile(this.fileSystem).WithContents(expectedModifiedPaths);
string[] expectedModifiedPaths =
{
"A .gitattributes",
"A developer/me/",
"A developer/me/JLANGE9._prerazzle",
"A developer/me/StateSwitch.Save",
"A tools/x86/remote.exe",
"A tools/x86/runelevated.exe",
"A tools/amd64/remote.exe",
"A tools/amd64/runelevated.exe",
"A tools/perllib/MS/TraceLogging.dll",
"A tools/managed/v2.0/midldd.CheckedInExe",
"A tools/managed/v4.0/sdapi.dll",
"A tools/managed/v2.0/midlpars.dll",
"A tools/managed/v2.0/RPCDataSupport.dll",
"A tools/managed/v2.0/MidlStaticAnalysis.dll",
"A tools/perllib/MS/Somefile.txt",
};

modifiedPathsDatabasePath.ShouldBeAFile(this.fileSystem);
this.fileSystem.ReadAllText(modifiedPathsDatabasePath)
.Split(new[] { '\r', '\n' }, StringSplitOptions.RemoveEmptyEntries)
Comment thread
kewillford marked this conversation as resolved.
.OrderBy(x => x)
.ShouldMatchInOrder(expectedModifiedPaths.OrderBy(x => x));

this.ValidatePersistedVersionMatchesCurrentVersion();
}
Expand Down
30 changes: 17 additions & 13 deletions GVFS/GVFS.FunctionalTests/Tests/EnlistmentPerFixture/GitFilesTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,6 @@ public void RenameEmptyFolderTest()
string renamedFolderName = "folder3b";
string[] expectedModifiedEntries =
{
folderName + "/",
renamedFolderName + "/",
};

Expand All @@ -110,19 +109,14 @@ public void RenameFolderTest()
string[] fileNames = { "a", "b", "c" };
string[] expectedModifiedEntries =
{
renamedFolderName + "/" + fileNames[0],
renamedFolderName + "/" + fileNames[1],
renamedFolderName + "/" + fileNames[2],
folderName + "/" + fileNames[0],
folderName + "/" + fileNames[1],
folderName + "/" + fileNames[2],
renamedFolderName + "/",
};

this.Enlistment.GetVirtualPathTo(folderName).ShouldNotExistOnDisk(this.fileSystem);
this.fileSystem.CreateDirectory(this.Enlistment.GetVirtualPathTo(folderName));
foreach (string fileName in fileNames)
{
string filePath = folderName + "\\" + fileName;
string filePath = Path.Combine(folderName, fileName);
this.fileSystem.CreateEmptyFile(this.Enlistment.GetVirtualPathTo(filePath));
this.Enlistment.GetVirtualPathTo(filePath).ShouldBeAFile(this.fileSystem);
}
Expand All @@ -138,22 +132,32 @@ public void RenameFolderTest()
[Category(Categories.MacTODO.M2)]
public void CaseOnlyRenameOfNewFolderKeepsModifiedPathsEntries()
{
string[] expectedModifiedPathsEntries =
if (this.fileSystem is PowerShellRunner)
{
Assert.Ignore("Powershell does not support case only renames.");
}

string[] expectedModifiedPathsEntriesAfterCreate =
{
"A Folder/",
"A Folder/testfile",
};

string[] expectedModifiedPathsEntriesAfterRename =
{
"Folder/",
"Folder/testfile",
"A folder/",
};

this.fileSystem.CreateDirectory(Path.Combine(this.Enlistment.RepoRoot, "Folder"));
this.fileSystem.CreateEmptyFile(Path.Combine(this.Enlistment.RepoRoot, "Folder", "testfile"));
this.Enlistment.WaitForBackgroundOperations().ShouldEqual(true, "Background operations failed to complete.");

GVFSHelpers.ModifiedPathsShouldContain(this.fileSystem, this.Enlistment.DotGVFSRoot, expectedModifiedPathsEntries);
GVFSHelpers.ModifiedPathsShouldContain(this.fileSystem, this.Enlistment.DotGVFSRoot, expectedModifiedPathsEntriesAfterCreate);

this.fileSystem.RenameDirectory(this.Enlistment.RepoRoot, "Folder", "folder");
this.Enlistment.WaitForBackgroundOperations().ShouldEqual(true, "Background operations failed to complete.");

GVFSHelpers.ModifiedPathsShouldContain(this.fileSystem, this.Enlistment.DotGVFSRoot, expectedModifiedPathsEntries);
GVFSHelpers.ModifiedPathsShouldContain(this.fileSystem, this.Enlistment.DotGVFSRoot, expectedModifiedPathsEntriesAfterRename);
}

[TestCase, Order(7)]
Expand Down
119 changes: 91 additions & 28 deletions GVFS/GVFS.FunctionalTests/Tests/EnlistmentPerTestCase/ModifiedPathsTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,9 @@
using GVFS.FunctionalTests.Tools;
using GVFS.Tests.Should;
using NUnit.Framework;
using System;
using System.IO;
using System.Linq;

namespace GVFS.FunctionalTests.Tests.EnlistmentPerTestCase
{
Expand All @@ -23,26 +25,62 @@ public class ModifiedPathsTests : TestsWithEnlistmentPerTestCase
private static readonly string FileToCreateOutsideRepo = $"{nameof(ModifiedPathsTests)}_outsideRepo.txt";
private static readonly string FolderToCreateOutsideRepo = $"{nameof(ModifiedPathsTests)}_outsideFolder";
private static readonly string FolderToDelete = "Scripts";
private static readonly string ExpectedModifiedFilesContentsAfterRemount =
$@"A .gitattributes
A {GVFSHelpers.ConvertPathToGitFormat(FileToAdd)}
A {GVFSHelpers.ConvertPathToGitFormat(FileToUpdate)}
A {FileToDelete}
A {GVFSHelpers.ConvertPathToGitFormat(FileToRename)}
A {GVFSHelpers.ConvertPathToGitFormat(RenameFileTarget)}
A {FolderToCreate}/
A {FolderToRename}/
A {RenameFolderTarget}/
A {RenameNewDotGitFileTarget}
A {FileToCreateOutsideRepo}
A {FolderToCreateOutsideRepo}/
A {FolderToDelete}/CreateCommonAssemblyVersion.bat
A {FolderToDelete}/CreateCommonCliAssemblyVersion.bat
A {FolderToDelete}/CreateCommonVersionHeader.bat
A {FolderToDelete}/RunFunctionalTests.bat
A {FolderToDelete}/RunUnitTests.bat
A {FolderToDelete}/
";
private static readonly string[] ExpectedModifiedFilesContentsAfterRemount =
{
$"A .gitattributes",
$"A {GVFSHelpers.ConvertPathToGitFormat(FileToAdd)}",
$"A {GVFSHelpers.ConvertPathToGitFormat(FileToUpdate)}",
$"A {FileToDelete}",
$"A {GVFSHelpers.ConvertPathToGitFormat(FileToRename)}",
$"A {GVFSHelpers.ConvertPathToGitFormat(RenameFileTarget)}",
$"A {FolderToCreate}/",
$"A {RenameNewDotGitFileTarget}",
$"A {FileToCreateOutsideRepo}",
$"A {FolderToCreateOutsideRepo}/",
$"A {FolderToDelete}/CreateCommonAssemblyVersion.bat",
$"A {FolderToDelete}/CreateCommonCliAssemblyVersion.bat",
$"A {FolderToDelete}/CreateCommonVersionHeader.bat",
$"A {FolderToDelete}/RunFunctionalTests.bat",
$"A {FolderToDelete}/RunUnitTests.bat",
$"A {FolderToDelete}/",
};

[TestCaseSource(typeof(FileSystemRunner), FileSystemRunner.TestRunners)]
public void DeletedTempFileIsRemovedFromModifiedFiles(FileSystemRunner fileSystem)
{
string tempFile = this.CreateFile(fileSystem, "temp.txt");
fileSystem.DeleteFile(tempFile);
tempFile.ShouldNotExistOnDisk(fileSystem);

this.Enlistment.UnmountGVFS();
this.ValidateModifiedPathsDoNotContain(fileSystem, "temp.txt");
}

[TestCaseSource(typeof(FileSystemRunner), FileSystemRunner.TestRunners)]
public void DeletedTempFolderIsRemovedFromModifiedFiles(FileSystemRunner fileSystem)
{
string tempFolder = this.CreateDirectory(fileSystem, "Temp");
fileSystem.DeleteDirectory(tempFolder);
tempFolder.ShouldNotExistOnDisk(fileSystem);

this.Enlistment.UnmountGVFS();
this.ValidateModifiedPathsDoNotContain(fileSystem, "Temp/");
}

[TestCaseSource(typeof(FileSystemRunner), FileSystemRunner.TestRunners)]
public void DeletedTempFolderDeletesFilesFromModifiedFiles(FileSystemRunner fileSystem)
{
string tempFolder = this.CreateDirectory(fileSystem, "Temp");
string tempFile1 = this.CreateFile(fileSystem, Path.Combine("Temp", "temp1.txt"));
string tempFile2 = this.CreateFile(fileSystem, Path.Combine("Temp", "temp2.txt"));
fileSystem.DeleteDirectory(tempFolder);
tempFolder.ShouldNotExistOnDisk(fileSystem);
tempFile1.ShouldNotExistOnDisk(fileSystem);
tempFile2.ShouldNotExistOnDisk(fileSystem);

this.Enlistment.UnmountGVFS();
this.ValidateModifiedPathsDoNotContain(fileSystem, "Temp/", "Temp/temp1.txt", "Temp/temp2.txt");
}

[Category(Categories.MacTODO.M2)]
[TestCaseSource(typeof(FileSystemRunner), FileSystemRunner.TestRunners)]
Expand All @@ -69,7 +107,7 @@ public void ModifiedPathsSavedAfterRemount(FileSystemRunner fileSystem)
string folderToRenameTarget = this.Enlistment.GetVirtualPathTo(RenameFolderTarget);
fileSystem.MoveDirectory(folderToRename, folderToRenameTarget);

// Moving the new folder out of the repo should not change the modified paths
// Moving the new folder out of the repo will remove it from the modified paths file
string folderTargetOutsideSrc = Path.Combine(this.Enlistment.EnlistmentRoot, RenameFolderTarget);
folderTargetOutsideSrc.ShouldNotExistOnDisk(fileSystem);
fileSystem.MoveDirectory(folderToRenameTarget, folderTargetOutsideSrc);
Expand Down Expand Up @@ -114,18 +152,20 @@ public void ModifiedPathsSavedAfterRemount(FileSystemRunner fileSystem)
modifiedPathsDatabase.ShouldBeAFile(fileSystem);
using (StreamReader reader = new StreamReader(File.Open(modifiedPathsDatabase, FileMode.Open, FileAccess.Read, FileShare.ReadWrite)))
{
reader.ReadToEnd().ShouldEqual(ExpectedModifiedFilesContentsAfterRemount);
reader.ReadToEnd().Split(new char[] { '\r', '\n' }, StringSplitOptions.RemoveEmptyEntries).OrderBy(x => x)
.ShouldMatchInOrder(ExpectedModifiedFilesContentsAfterRemount.OrderBy(x => x));
}
}

[TestCaseSource(typeof(FileSystemRunner), FileSystemRunner.TestRunners)]
public void ModifiedPathsCorrectAfterHardLinking(FileSystemRunner fileSystem)
{
const string ExpectedModifiedFilesContentsAfterHardlinks =
@"A .gitattributes
A LinkToReadme.md
A LinkToFileOutsideSrc.txt
";
string[] expectedModifiedFilesContentsAfterHardlinks =
{
"A .gitattributes",
"A LinkToReadme.md",
"A LinkToFileOutsideSrc.txt",
};

// Create a link from src\LinkToReadme.md to src\Readme.md
string existingFileInRepoPath = this.Enlistment.GetVirtualPathTo("Readme.md");
Expand Down Expand Up @@ -157,8 +197,31 @@ A LinkToFileOutsideSrc.txt
modifiedPathsDatabase.ShouldBeAFile(fileSystem);
using (StreamReader reader = new StreamReader(File.Open(modifiedPathsDatabase, FileMode.Open, FileAccess.Read, FileShare.ReadWrite)))
{
reader.ReadToEnd().ShouldEqual(ExpectedModifiedFilesContentsAfterHardlinks);
reader.ReadToEnd().Split(new char[] { '\r', '\n' }, StringSplitOptions.RemoveEmptyEntries).OrderBy(x => x)
.ShouldMatchInOrder(expectedModifiedFilesContentsAfterHardlinks.OrderBy(x => x));
}
}

private string CreateDirectory(FileSystemRunner fileSystem, string relativePath)
{
string tempFolder = this.Enlistment.GetVirtualPathTo(relativePath);
fileSystem.CreateDirectory(tempFolder);
tempFolder.ShouldBeADirectory(fileSystem);
return tempFolder;
}

private string CreateFile(FileSystemRunner fileSystem, string relativePath)
{
string tempFile = this.Enlistment.GetVirtualPathTo(relativePath);
fileSystem.WriteAllText(tempFile, $"Contents for the {relativePath} file");
tempFile.ShouldBeAFile(fileSystem);
return tempFile;
}

private void ValidateModifiedPathsDoNotContain(FileSystemRunner fileSystem, params string[] paths)
{
GVFSHelpers.ModifiedPathsShouldNotContain(fileSystem, this.Enlistment.DotGVFSRoot, paths.Select(x => $"A {x}" + Environment.NewLine).ToArray());
GVFSHelpers.ModifiedPathsShouldNotContain(fileSystem, this.Enlistment.DotGVFSRoot, paths.Select(x => $"D {x}" + Environment.NewLine).ToArray());
}
}
}
Loading