fix: ensure correct behavior when moving or replacing a file with an …

…open handle (#555) * Adapt tests for Copy, Move and Replace for File and FileInfo * Ignore FileShare for Linux and MacOS when calling Move or Replace methods
Testably · Apr 9, 2024 · fb0420f · fb0420f
1 parent 2952883
commit fb0420f
Show file tree

Hide file tree

Showing 11 changed files with 180 additions and 36 deletions.
diff --git a/Source/Testably.Abstractions.Testing/Storage/IStorageContainer.cs b/Source/Testably.Abstractions.Testing/Storage/IStorageContainer.cs
@@ -83,6 +83,7 @@ internal interface IStorageContainer : IFileSystemEntity, ITimeSystemEntity
 	/// <returns>An <see cref="IStorageAccessHandle" /> that is used to release the access lock on dispose.</returns>
 	IStorageAccessHandle RequestAccess(FileAccess access, FileShare share,
 		bool deleteAccess = false,
+		bool ignoreFileShare = false,
 		bool ignoreMetadataErrors = true,
 		int? hResult = null);
 

diff --git a/Source/Testably.Abstractions.Testing/Storage/InMemoryContainer.cs b/Source/Testably.Abstractions.Testing/Storage/InMemoryContainer.cs
@@ -137,9 +137,10 @@ public void Encrypt()
 	/// <inheritdoc cref="IStorageContainer.GetBytes()" />
 	public byte[] GetBytes() => _bytes;
 
-	/// <inheritdoc cref="IStorageContainer.RequestAccess(FileAccess, FileShare, bool, bool, int?)" />
+	/// <inheritdoc cref="IStorageContainer.RequestAccess(FileAccess, FileShare, bool, bool, bool, int?)" />
 	public IStorageAccessHandle RequestAccess(FileAccess access, FileShare share,
 		bool deleteAccess = false,
+		bool ignoreFileShare = false,
 		bool ignoreMetadataErrors = true,
 		int? hResult = null)
 	{
@@ -163,7 +164,7 @@ public IStorageAccessHandle RequestAccess(FileAccess access, FileShare share,
 			throw ExceptionFactory.AclAccessToPathDenied(_location.FullPath);
 		}
 
-		if (CanGetAccess(access, share, deleteAccess))
+		if (CanGetAccess(access, share, deleteAccess, ignoreFileShare))
 		{
 			Guid guid = Guid.NewGuid();
 			FileHandle fileHandle =
@@ -289,11 +290,11 @@ internal FileAttributes AdjustAttributes(FileAttributes attributes)
 		return attributes;
 	}
 
-	private bool CanGetAccess(FileAccess access, FileShare share, bool deleteAccess)
+	private bool CanGetAccess(FileAccess access, FileShare share, bool deleteAccess, bool ignoreFileShare)
 	{
 		foreach (KeyValuePair<Guid, FileHandle> fileHandle in _fileHandles)
 		{
-			if (!fileHandle.Value.GrantAccess(access, share, deleteAccess))
+			if (!fileHandle.Value.GrantAccess(access, share, deleteAccess, ignoreFileShare))
 			{
 				return false;
 			}
@@ -385,17 +386,20 @@ public void Dispose()
 
 		#endregion
 
-		public bool GrantAccess(FileAccess access, FileShare share, bool deleteAccess)
+		public bool GrantAccess(FileAccess access, FileShare share, bool deleteAccess, bool ignoreFileShare)
 		{
 			FileShare usedShare = share;
+			FileShare currentShare = Share;
 			_fileSystem.Execute.NotOnWindows(()
 				=> usedShare = FileShare.ReadWrite);
+			_fileSystem.Execute.NotOnWindowsIf(ignoreFileShare, ()
+				=> currentShare = FileShare.ReadWrite);
 			if (deleteAccess)
 			{
 				return !_fileSystem.Execute.IsWindows || Share == FileShare.Delete;
 			}
 
-			return CheckAccessWithShare(access, Share) &&
+			return CheckAccessWithShare(access, currentShare) &&
 			       CheckAccessWithShare(Access, usedShare);
 		}
 

diff --git a/Source/Testably.Abstractions.Testing/Storage/InMemoryStorage.cs b/Source/Testably.Abstractions.Testing/Storage/InMemoryStorage.cs
@@ -409,12 +409,14 @@ public IStorageContainer GetOrCreateContainer(
 		using (_ = sourceContainer.RequestAccess(
 			FileAccess.ReadWrite,
 			FileShare.None,
-			ignoreMetadataErrors: ignoreMetadataErrors))
+			ignoreMetadataErrors: ignoreMetadataErrors,
+			ignoreFileShare: true))
 		{
 			using (_ = destinationContainer.RequestAccess(
 				FileAccess.ReadWrite,
 				FileShare.None,
-				ignoreMetadataErrors: ignoreMetadataErrors))
+				ignoreMetadataErrors: ignoreMetadataErrors,
+				ignoreFileShare: true))
 			{
 				if (_containers.TryRemove(destination,
 					out IStorageContainer? existingDestinationContainer))
@@ -664,6 +666,7 @@ private void CreateParents(MockFileSystem fileSystem, IStorageLocation location)
 		}
 
 		using (container.RequestAccess(FileAccess.Write, FileShare.None,
+			ignoreFileShare: true,
 			hResult: sourceType == FileSystemTypes.Directory ? -2147024891 : -2147024864))
 		{
 			if (children.Any() && recursive)

diff --git a/Source/Testably.Abstractions.Testing/Storage/NullContainer.cs b/Source/Testably.Abstractions.Testing/Storage/NullContainer.cs
@@ -91,9 +91,10 @@ public void Encrypt()
 	public byte[] GetBytes()
 		=> Array.Empty<byte>();
 
-	/// <inheritdoc cref="IStorageContainer.RequestAccess(FileAccess, FileShare, bool, bool, int?)" />
+	/// <inheritdoc cref="IStorageContainer.RequestAccess(FileAccess, FileShare, bool, bool, bool, int?)" />
 	public IStorageAccessHandle RequestAccess(FileAccess access, FileShare share,
 		bool deleteAccess = false,
+		bool ignoreFileShare = false,
 		bool ignoreMetadataErrors = true,
 		int? hResult = null)
 		=> new NullStorageAccessHandle(access, share, deleteAccess);

diff --git a/Tests/Testably.Abstractions.Testing.Tests/TestHelpers/LockableContainer.cs b/Tests/Testably.Abstractions.Testing.Tests/TestHelpers/LockableContainer.cs
@@ -12,7 +12,7 @@ namespace Testably.Abstractions.Testing.Tests.TestHelpers;
 ///     A <see cref="IStorageContainer" /> for testing purposes.
 ///     <para />
 ///     Set <see cref="IsLocked" /> to <see langword="true" /> to simulate a locked file
-///     (<see cref="RequestAccess(FileAccess, FileShare, bool, bool, int?)" /> throws an <see cref="IOException" />).
+///     (<see cref="RequestAccess(FileAccess, FileShare, bool, bool, bool, int?)" /> throws an <see cref="IOException" />).
 /// </summary>
 internal sealed class LockableContainer(
 	MockFileSystem fileSystem,
@@ -24,7 +24,7 @@ internal sealed class LockableContainer(
 
 	/// <summary>
 	///     Simulate a locked file, if set to <see langword="true" />.<br />
-	///     In this case <see cref="RequestAccess(FileAccess, FileShare, bool, bool, int?)" /> throws
+	///     In this case <see cref="RequestAccess(FileAccess, FileShare, bool, bool, bool, int?)" /> throws
 	///     an <see cref="IOException" />, otherwise it will succeed.
 	/// </summary>
 	public bool IsLocked { get; set; }
@@ -89,9 +89,10 @@ public void Encrypt()
 	public byte[] GetBytes()
 		=> _bytes;
 
-	/// <inheritdoc cref="IStorageContainer.RequestAccess(FileAccess, FileShare, bool, bool, int?)" />
+	/// <inheritdoc cref="IStorageContainer.RequestAccess(FileAccess, FileShare, bool, bool, bool, int?)" />
 	public IStorageAccessHandle RequestAccess(FileAccess access, FileShare share,
 		bool deleteAccess = false,
+		bool ignoreFileShare = false,
 		bool ignoreMetadataErrors = true,
 		int? hResult = null)
 	{

diff --git a/Tests/Testably.Abstractions.Tests/FileSystem/File/CopyTests.cs b/Tests/Testably.Abstractions.Tests/FileSystem/File/CopyTests.cs
@@ -266,22 +266,17 @@ public void Copy_ShouldCopyFileWithContent(
 	[SkippableTheory]
 	[InlineAutoData(FileAccess.Read, FileShare.Read)]
 	[InlineAutoData(FileAccess.Read, FileShare.ReadWrite)]
-	[InlineAutoData(FileAccess.Read, FileShare.Write)]
 	[InlineAutoData(FileAccess.ReadWrite, FileShare.Read)]
 	[InlineAutoData(FileAccess.ReadWrite, FileShare.ReadWrite)]
-	[InlineAutoData(FileAccess.ReadWrite, FileShare.Write)]
 	[InlineAutoData(FileAccess.Write, FileShare.Read)]
 	[InlineAutoData(FileAccess.Write, FileShare.ReadWrite)]
-	[InlineAutoData(FileAccess.Write, FileShare.Write)]
 	public void Copy_SourceAccessedWithReadShare_ShouldNotThrow(
 		FileAccess fileAccess,
 		FileShare fileShare,
 		string sourcePath,
 		string destinationPath,
 		string sourceContents)
 	{
-		Skip.If(Test.RunsOnWindows && fileShare == FileShare.Write);
-
 		FileSystem.Initialize().WithFile(sourcePath)
 			.Which(f => f.HasStringContent(sourceContents));
 		using (FileSystem.FileStream
@@ -294,6 +289,30 @@ public void Copy_SourceAccessedWithReadShare_ShouldNotThrow(
 		FileSystem.File.ReadAllText(destinationPath).Should().Be(sourceContents);
 	}
 
+	[SkippableTheory]
+	[InlineAutoData(FileAccess.Read)]
+	[InlineAutoData(FileAccess.ReadWrite)]
+	[InlineAutoData(FileAccess.Write)]
+	public void Copy_SourceAccessedWithWriteShare_ShouldNotThrowOnLinuxOrMac(
+		FileAccess fileAccess,
+		string sourcePath,
+		string destinationPath,
+		string sourceContents)
+	{
+		Skip.If(Test.RunsOnWindows, "see https://github.com/dotnet/runtime/issues/52700");
+
+		FileSystem.Initialize().WithFile(sourcePath)
+			.Which(f => f.HasStringContent(sourceContents));
+		using (FileSystem.FileStream
+			.New(sourcePath, FileMode.Open, fileAccess, FileShare.Write))
+		{
+			FileSystem.File.Copy(sourcePath, destinationPath);
+		}
+
+		FileSystem.File.Exists(destinationPath).Should().BeTrue();
+		FileSystem.File.ReadAllText(destinationPath).Should().Be(sourceContents);
+	}
+
 	[SkippableTheory]
 	[AutoData]
 	public void Copy_SourceIsDirectory_ShouldThrowUnauthorizedAccessException_AndNotCopyFile(
@@ -324,11 +343,12 @@ public void Copy_SourceLocked_ShouldThrowIOException(
 		string sourceName,
 		string destinationName)
 	{
-		Skip.If(!Test.RunsOnWindows && fileShare == FileShare.Write);
+		Skip.If(!Test.RunsOnWindows && fileShare == FileShare.Write,
+			"see https://github.com/dotnet/runtime/issues/52700");
 
 		FileSystem.File.WriteAllText(sourceName, null);
-		using FileSystemStream stream = FileSystem.File.Open(sourceName, FileMode.Open,
-			FileAccess.Read, fileShare);
+		using FileSystemStream stream = FileSystem.File.Open(
+			sourceName, FileMode.Open, FileAccess.Read, fileShare);
 
 		Exception? exception = Record.Exception(() =>
 		{

diff --git a/Tests/Testably.Abstractions.Tests/FileSystem/File/MoveTests.cs b/Tests/Testably.Abstractions.Tests/FileSystem/File/MoveTests.cs
@@ -181,14 +181,27 @@ public void Move_SourceDirectoryMissing_ShouldThrowFileNotFoundException(
 	}
 
 	[SkippableTheory]
-	[AutoData]
+	[InlineAutoData(FileAccess.Read, FileShare.None)]
+	[InlineAutoData(FileAccess.Read, FileShare.Read)]
+	[InlineAutoData(FileAccess.Read, FileShare.ReadWrite)]
+	[InlineAutoData(FileAccess.Read, FileShare.Write)]
+	[InlineAutoData(FileAccess.ReadWrite, FileShare.None)]
+	[InlineAutoData(FileAccess.ReadWrite, FileShare.Read)]
+	[InlineAutoData(FileAccess.ReadWrite, FileShare.ReadWrite)]
+	[InlineAutoData(FileAccess.ReadWrite, FileShare.Write)]
+	[InlineAutoData(FileAccess.Write, FileShare.None)]
+	[InlineAutoData(FileAccess.Write, FileShare.Read)]
+	[InlineAutoData(FileAccess.Write, FileShare.ReadWrite)]
+	[InlineAutoData(FileAccess.Write, FileShare.Write)]
 	public void Move_SourceLocked_ShouldThrowIOException_OnWindows(
+		FileAccess fileAccess,
+		FileShare fileShare,
 		string sourceName,
 		string destinationName)
 	{
 		FileSystem.File.WriteAllText(sourceName, null);
-		using FileSystemStream stream = FileSystem.File.Open(sourceName, FileMode.Open,
-			FileAccess.Read, FileShare.Read);
+		using FileSystemStream stream = FileSystem.File.Open(
+			sourceName, FileMode.Open, fileAccess, fileShare);
 
 		Exception? exception = Record.Exception(() =>
 		{
@@ -197,12 +210,13 @@ public void Move_SourceLocked_ShouldThrowIOException_OnWindows(
 
 		if (Test.RunsOnWindows)
 		{
-			exception.Should().BeException<IOException>(
-				hResult: -2147024864);
+			exception.Should().BeException<IOException>(hResult: -2147024864);
+			FileSystem.Should().HaveFile(sourceName);
 			FileSystem.Should().NotHaveFile(destinationName);
 		}
 		else
 		{
+			// https://github.com/dotnet/runtime/issues/52700
 			FileSystem.Should().NotHaveFile(sourceName);
 			FileSystem.Should().HaveFile(destinationName);
 		}

diff --git a/Tests/Testably.Abstractions.Tests/FileSystem/File/ReplaceTests.cs b/Tests/Testably.Abstractions.Tests/FileSystem/File/ReplaceTests.cs
@@ -174,8 +174,21 @@ public void Replace_SourceIsDirectory_ShouldThrowUnauthorizedAccessException(
 	}
 
 	[SkippableTheory]
-	[AutoData]
+	[InlineAutoData(FileAccess.Read, FileShare.None)]
+	[InlineAutoData(FileAccess.Read, FileShare.Read)]
+	[InlineAutoData(FileAccess.Read, FileShare.ReadWrite)]
+	[InlineAutoData(FileAccess.Read, FileShare.Write)]
+	[InlineAutoData(FileAccess.ReadWrite, FileShare.None)]
+	[InlineAutoData(FileAccess.ReadWrite, FileShare.Read)]
+	[InlineAutoData(FileAccess.ReadWrite, FileShare.ReadWrite)]
+	[InlineAutoData(FileAccess.ReadWrite, FileShare.Write)]
+	[InlineAutoData(FileAccess.Write, FileShare.None)]
+	[InlineAutoData(FileAccess.Write, FileShare.Read)]
+	[InlineAutoData(FileAccess.Write, FileShare.ReadWrite)]
+	[InlineAutoData(FileAccess.Write, FileShare.Write)]
 	public void Replace_SourceLocked_ShouldThrowIOException_OnWindows(
+		FileAccess fileAccess,
+		FileShare fileShare,
 		string sourceName,
 		string destinationName,
 		string backupName,
@@ -186,8 +199,8 @@ public void Replace_SourceLocked_ShouldThrowIOException_OnWindows(
 		FileSystem.File.WriteAllText(destinationName, destinationContents);
 
 		Exception? exception;
-		using (FileSystemStream _ = FileSystem.File.Open(sourceName,
-			FileMode.Open, FileAccess.Read, FileShare.Read))
+		using (FileSystemStream _ = FileSystem.File.Open(
+			sourceName, FileMode.Open, fileAccess, fileShare))
 		{
 			exception = Record.Exception(() =>
 			{
@@ -208,6 +221,7 @@ public void Replace_SourceLocked_ShouldThrowIOException_OnWindows(
 		}
 		else
 		{
+			// https://github.com/dotnet/runtime/issues/52700
 			FileSystem.Should().NotHaveFile(sourceName);
 			FileSystem.Should().HaveFile(destinationName)
 				.Which.HasContent(sourceContents);

diff --git a/Tests/Testably.Abstractions.Tests/FileSystem/FileInfo/CopyToTests.cs b/Tests/Testably.Abstractions.Tests/FileSystem/FileInfo/CopyToTests.cs
@@ -173,6 +173,58 @@ public void CopyTo_ShouldKeepMetadata(
 			.Should().Be(sourceLastWriteTime);
 	}
 
+	[SkippableTheory]
+	[InlineAutoData(FileAccess.Read, FileShare.Read)]
+	[InlineAutoData(FileAccess.Read, FileShare.ReadWrite)]
+	[InlineAutoData(FileAccess.ReadWrite, FileShare.Read)]
+	[InlineAutoData(FileAccess.ReadWrite, FileShare.ReadWrite)]
+	[InlineAutoData(FileAccess.Write, FileShare.Read)]
+	[InlineAutoData(FileAccess.Write, FileShare.ReadWrite)]
+	public void CopyTo_SourceAccessedWithReadShare_ShouldNotThrow(
+		FileAccess fileAccess,
+		FileShare fileShare,
+		string sourcePath,
+		string destinationPath,
+		string sourceContents)
+	{
+		FileSystem.Initialize().WithFile(sourcePath)
+			.Which(f => f.HasStringContent(sourceContents));
+		IFileInfo sut = FileSystem.FileInfo.New(sourcePath);
+		using (FileSystem.FileStream
+			.New(sourcePath, FileMode.Open, fileAccess, fileShare))
+		{
+			sut.CopyTo(destinationPath);
+		}
+
+		FileSystem.File.Exists(destinationPath).Should().BeTrue();
+		FileSystem.File.ReadAllText(destinationPath).Should().Be(sourceContents);
+	}
+
+	[SkippableTheory]
+	[InlineAutoData(FileAccess.Read)]
+	[InlineAutoData(FileAccess.ReadWrite)]
+	[InlineAutoData(FileAccess.Write)]
+	public void CopyTo_SourceAccessedWithWriteShare_ShouldNotThrowOnLinuxOrMac(
+		FileAccess fileAccess,
+		string sourcePath,
+		string destinationPath,
+		string sourceContents)
+	{
+		Skip.If(Test.RunsOnWindows, "see https://github.com/dotnet/runtime/issues/52700");
+
+		FileSystem.Initialize().WithFile(sourcePath)
+			.Which(f => f.HasStringContent(sourceContents));
+		IFileInfo sut = FileSystem.FileInfo.New(sourcePath);
+		using (FileSystem.FileStream
+			.New(sourcePath, FileMode.Open, fileAccess, FileShare.Write))
+		{
+			sut.CopyTo(destinationPath);
+		}
+
+		FileSystem.File.Exists(destinationPath).Should().BeTrue();
+		FileSystem.File.ReadAllText(destinationPath).Should().Be(sourceContents);
+	}
+
 	[SkippableTheory]
 	[AutoData]
 	public void CopyTo_SourceIsDirectory_ShouldThrowUnauthorizedAccessException_AndNotCopyFile(
@@ -195,14 +247,19 @@ public void CopyTo_SourceIsDirectory_ShouldThrowUnauthorizedAccessException_AndN
 	}
 
 	[SkippableTheory]
-	[AutoData]
+	[InlineAutoData(FileShare.None)]
+	[InlineAutoData(FileShare.Write)]
 	public void CopyTo_SourceLocked_ShouldThrowIOException(
+		FileShare fileShare,
 		string sourceName,
 		string destinationName)
 	{
+		Skip.If(!Test.RunsOnWindows && fileShare == FileShare.Write,
+			"see https://github.com/dotnet/runtime/issues/52700");
+
 		FileSystem.File.WriteAllText(sourceName, null);
 		using FileSystemStream stream = FileSystem.File.Open(sourceName, FileMode.Open,
-			FileAccess.Read, FileShare.None);
+			FileAccess.Read, fileShare);
 		IFileInfo sut = FileSystem.FileInfo.New(sourceName);
 
 		Exception? exception = Record.Exception(() =>