Skip to content
This repository has been archived by the owner on Jan 23, 2023. It is now read-only.
/ corefx Public archive

Commit

Permalink
Directory move fix (#40570)
Browse files Browse the repository at this point in the history
* Fix to support case-insensitive directory moving/renaming

* More tests for other directories

* Include throwing win32 exp upon directory already existing

* Added ) that I dropped in previous rebase

* Directory get with pattern to check for case sensitivity

* Error message fix

* Rework to support multiple edge cases

* Rework to only support moving to the same directory case insentivity

* Handle situations for case-sensitive file systems

* Test changes

* Stopped modifying environment's current directory

* Cleanup, and tests for drive differences

* Only set route to lower, but only only compare roots ordinally

* Add additional if to check if any directory with that name exists

* Edit to what path to use

* Use destination path.

* Rework file directory scan

* Make utility function due to Any() with empty returning true

* Get the full path before getting the filename

* Check if the directory name is null or empty

* Removed extra line

* Now Defering to the OS for if the directory already exists

* Remove extra semi-colon

* Test Edit

* Comment edit

* Actually test the functionality the test is built for

* Remove platform specific tests and make them platform agnostic

* Target only OSX

* Target all platforms other than linux itself

* Changed how we detect case variants

* Remove unix specific test

* Convert OSX test into no-op test

* Code review feedback and additional tests

* Case-Sensitive file systems specific tests

* Fix to not include osx with other unixes

* Removed a ,
  • Loading branch information
Jlalond authored and JeremyKuhne committed Nov 6, 2019
1 parent e52be69 commit e9ce2f7
Show file tree
Hide file tree
Showing 3 changed files with 160 additions and 6 deletions.
26 changes: 21 additions & 5 deletions src/System.IO.FileSystem/src/System/IO/Directory.cs
Original file line number Diff line number Diff line change
Expand Up @@ -262,22 +262,38 @@ public static void Move(string sourceDirName, string destDirName)
string fulldestDirName = Path.GetFullPath(destDirName);
string destPath = PathInternal.EnsureTrailingSeparator(fulldestDirName);

StringComparison pathComparison = PathInternal.StringComparison;

if (string.Equals(sourcePath, destPath, pathComparison))
string sourceDirNameFromFullPath = Path.GetFileName(fullsourceDirName);
string destDirNameFromFullPath = Path.GetFileName(fulldestDirName);

StringComparison fileSystemSensitivity = PathInternal.StringComparison;
bool directoriesAreCaseVariants = !string.Equals(sourceDirNameFromFullPath, destDirNameFromFullPath, StringComparison.Ordinal)
&& string.Equals(sourceDirNameFromFullPath, destDirNameFromFullPath, StringComparison.OrdinalIgnoreCase);
bool sameDirectoryDifferentCase = directoriesAreCaseVariants
&& string.Equals(destDirNameFromFullPath, sourceDirNameFromFullPath, fileSystemSensitivity);

// If the destination directories are the exact same name
if (!sameDirectoryDifferentCase
&& string.Equals(sourcePath, destPath, fileSystemSensitivity))
throw new IOException(SR.IO_SourceDestMustBeDifferent);

string sourceRoot = Path.GetPathRoot(sourcePath);
string destinationRoot = Path.GetPathRoot(destPath);
if (!string.Equals(sourceRoot, destinationRoot, pathComparison))

// Compare paths for the same, skip this step if we already know the paths are identical.
if (!string.Equals(sourceRoot, destinationRoot, StringComparison.OrdinalIgnoreCase))
throw new IOException(SR.IO_SourceDestMustHaveSameRoot);

// Windows will throw if the source file/directory doesn't exist, we preemptively check
// to make sure our cross platform behavior matches NetFX behavior.
if (!FileSystem.DirectoryExists(fullsourceDirName) && !FileSystem.FileExists(fullsourceDirName))
throw new DirectoryNotFoundException(SR.Format(SR.IO_PathNotFound_Path, fullsourceDirName));

if (FileSystem.DirectoryExists(fulldestDirName))
if (!sameDirectoryDifferentCase // This check is to allowing renaming of directories
&& FileSystem.DirectoryExists(fulldestDirName))
throw new IOException(SR.Format(SR.IO_AlreadyExists_Name, fulldestDirName));

// If the directories aren't the same and the OS says the directory exists already, fail.
if (!sameDirectoryDifferentCase && Directory.Exists(fulldestDirName))
throw new IOException(SR.Format(SR.IO_AlreadyExists_Name, fulldestDirName));

FileSystem.MoveDirectory(fullsourceDirName, fulldestDirName);
Expand Down
3 changes: 3 additions & 0 deletions src/System.IO.FileSystem/src/System/IO/FileSystem.Windows.cs
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,9 @@ public static void MoveDirectory(string sourceFullPath, string destFullPath)
if (errorCode == Interop.Errors.ERROR_FILE_NOT_FOUND)
throw Win32Marshal.GetExceptionForWin32Error(Interop.Errors.ERROR_PATH_NOT_FOUND, sourceFullPath);

if (errorCode == Interop.Errors.ERROR_ALREADY_EXISTS)
throw Win32Marshal.GetExceptionForWin32Error(Interop.Errors.ERROR_ALREADY_EXISTS, destFullPath);

// This check was originally put in for Win9x (unfortunately without special casing it to be for Win9x only). We can't change the NT codepath now for backcomp reasons.
if (errorCode == Interop.Errors.ERROR_ACCESS_DENIED) // WinNT throws IOException. This check is for Win9x. We can't change it for backcomp.
throw new IOException(SR.Format(SR.UnauthorizedAccess_IODenied_Path, sourceFullPath), Win32Marshal.MakeHRFromErrorCode(errorCode));
Expand Down
137 changes: 136 additions & 1 deletion src/System.IO.FileSystem/tests/Directory/Move.cs
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,7 @@ public void TrailingDirectorySeparators()
string testDirDest = Path.Combine(TestDirectory, GetTestFileName());

Directory.CreateDirectory(testDirSource);
Move(testDirSource + Path.DirectorySeparatorChar, testDirDest + Path.DirectorySeparatorChar);
Directory.Move(testDirSource + Path.DirectorySeparatorChar, testDirDest + Path.DirectorySeparatorChar);
Assert.True(Directory.Exists(testDirDest));
}

Expand Down Expand Up @@ -212,10 +212,145 @@ public void Path_Longer_Than_MaxLongPath_Throws_Exception()
});
}

[Fact]
public void ThrowIOExceptionWhenMovingDirectoryToItself()
{
Directory.CreateDirectory(Path.Combine(TestDirectory, "foo"));
Assert.Throws<IOException>(() => Directory.Move(Path.Combine(TestDirectory, "foo"), Path.Combine(TestDirectory, "foo")));
}

[Fact]
public void ThrowIOExceptionWhenMovingToExistingDirectoryWithSameCase()
{
Directory.CreateDirectory(Path.Combine(TestDirectory, "foo"));
Directory.CreateDirectory(Path.Combine(TestDirectory, "bar", "foo"));
Assert.Throws<IOException>(() => Directory.Move(Path.Combine(TestDirectory, "foo"), Path.Combine(TestDirectory, "bar", "foo")));
}

[Fact]
public void ToNewDirectoryButWithDifferentCasing()
{
Directory.CreateDirectory(Path.Combine(TestDirectory, "foo"));
var otherDirectory = Path.Combine(TestDirectory, "bar");
Directory.CreateDirectory(Path.Combine(otherDirectory));
Directory.Move(Path.Combine(TestDirectory, "foo"), Path.Combine(otherDirectory, "FOO"));
Assert.True(Directory.Exists(Path.Combine(otherDirectory, "FOO")));
Assert.False(Directory.Exists(Path.Combine(TestDirectory, "foo")));
}

[Fact]
public void SameDirectoryWithDifferentCasing_WithFileContent()
{
var fooDirectory = Path.Combine(TestDirectory, "foo");
var fooDirectoryUppercase = Path.Combine(TestDirectory, "FOO");
Directory.CreateDirectory(fooDirectory);
File.WriteAllText(Path.Combine(fooDirectory, "bar.txt"), string.Empty);
Directory.Move(fooDirectory, fooDirectoryUppercase);
var firstFile = Directory.GetFiles(fooDirectoryUppercase);
Assert.Equal("bar.txt", Path.GetFileName(firstFile[0]));
}

[Fact]
public void WithDifferentRootCase()
{
Directory.CreateDirectory($"{TestDirectory}/bar");
var root = Path.GetPathRoot(TestDirectory);
Directory.Move($"{TestDirectory}/bar".Replace(root, root.ToLower()), $"{TestDirectory}/foo");
Assert.True(Directory.Exists($"{TestDirectory}/foo"));
Assert.False(Directory.Exists($"{TestDirectory}/bar"));
}

[Fact]
public void SameDirectoryWithDifferentCasing_WithDirectoryContent()
{
var fooDirectoryPath = Path.Combine(TestDirectory, "foo");
var fooDirectoryPathUpperCase = Path.Combine(TestDirectory, "FOO");
Directory.CreateDirectory(fooDirectoryPath);
Directory.CreateDirectory(Path.Combine(fooDirectoryPath, "bar"));
Directory.Move(fooDirectoryPath, fooDirectoryPathUpperCase);
var firstFile = Directory.GetDirectories(fooDirectoryPathUpperCase);
Assert.Equal("bar", Path.GetFileName(firstFile[0]));
}

#endregion

#region PlatformSpecific

[Fact]
[PlatformSpecific(TestPlatforms.Windows | TestPlatforms.OSX)]
public void DirectoryWithDifferentCasingThanFileSystem_ToAnotherDirectory()
{
Directory.CreateDirectory(Path.Combine(TestDirectory, "FOO"));
Directory.CreateDirectory(Path.Combine(TestDirectory, "bar"));
Directory.Move(Path.Combine(TestDirectory, "foo"), Path.Combine(TestDirectory, "bar", "FOO"));
}

[Fact]
[PlatformSpecific(TestPlatforms.Windows | TestPlatforms.OSX)]
public void DirectoryWithDifferentCasingThanFileSystem_ToItself()
{
Directory.CreateDirectory(Path.Combine(TestDirectory, "FOO"));
Directory.Move(Path.Combine(TestDirectory, "foo"), Path.Combine(TestDirectory, "FOO"));
Assert.True(Directory.Exists(Path.Combine(TestDirectory, "FOO")));
}

[Fact]
[PlatformSpecific(TestPlatforms.Linux)]
public void DirectoryWithDifferentCasingThanFileSystem_ToAnotherDirectory_CaseSensitiveOS()
{
Directory.CreateDirectory(Path.Combine(TestDirectory, "FOO"));
Directory.CreateDirectory(Path.Combine(TestDirectory, "bar"));
Assert.Throws<DirectoryNotFoundException>(() => Directory.Move(Path.Combine(TestDirectory, "foo"), Path.Combine(TestDirectory, "bar", "FOO")));
}

[Fact]
[PlatformSpecific(TestPlatforms.Linux)]
public void DirectoryWithDifferentCasingThanFileSystem_ToItself_CaseSensitiveOS()
{
Directory.CreateDirectory(Path.Combine(TestDirectory, "FOO"));
Assert.Throws<DirectoryNotFoundException>(() => Directory.Move(Path.Combine(TestDirectory, "foo"), Path.Combine(TestDirectory, "FOO")));
}

[Fact]
[PlatformSpecific(TestPlatforms.Windows)]
public void CaseVariantDirectoryNameWithCaseVariantPaths_CaseInsensitiveFileSystem()
{
var directoryToBeMoved = Path.Combine(TestDirectory, "FOO", "bar");
var newPath = Path.Combine(TestDirectory, "foo", "bar");
Directory.CreateDirectory(Path.Combine(TestDirectory, "FOO", "bar"));
Directory.CreateDirectory(Path.Combine(TestDirectory, "foo"));

Assert.Throws<IOException>(() => Directory.Move(directoryToBeMoved, Path.Combine(newPath, "bar")));
}

[Fact]
[PlatformSpecific(TestPlatforms.Windows | TestPlatforms.FreeBSD | TestPlatforms.NetBSD)]
public void MoveDirectory_FailToMoveDirectoryWithUpperCaseToOtherDirectoryWithLowerCase()
{
Directory.CreateDirectory($"{TestDirectory}/FOO");
Directory.CreateDirectory($"{TestDirectory}/bar/foo");
Assert.Throws<IOException>(() => Directory.Move($"{TestDirectory}/FOO", $"{TestDirectory}/bar/foo"));
}

[Fact]
[PlatformSpecific(TestPlatforms.OSX)]
public void MoveDirectory_NoOpWhenMovingDirectoryWithUpperCaseToOtherDirectoryWithLowerCase()
{
Directory.CreateDirectory($"{TestDirectory}/FOO");
Directory.CreateDirectory($"{TestDirectory}/bar/foo");
Directory.Move($"{TestDirectory}/FOO", $"{TestDirectory}/bar/foo");
Assert.True(Directory.Exists(Path.Combine(TestDirectory, "bar", "foo")));
}

[Fact]
[PlatformSpecific(TestPlatforms.Windows | TestPlatforms.OSX | TestPlatforms.FreeBSD | TestPlatforms.NetBSD)]
public void MoveDirectory_FailToMoveLowerCaseDirectoryWhenUpperCaseDirectoryExists()
{
Directory.CreateDirectory($"{TestDirectory}/bar/FOO");
Directory.CreateDirectory($"{TestDirectory}/foo");
Assert.Throws<IOException>(() => Directory.Move($"{TestDirectory}/foo", $"{TestDirectory}/bar/foo"));
}

[ConditionalFact(nameof(AreAllLongPathsAvailable))]
[PlatformSpecific(TestPlatforms.Windows)] // Long path succeeds
public void Path_With_Longer_Than_MaxDirectory_Succeeds()
Expand Down

0 comments on commit e9ce2f7

Please sign in to comment.