Skip to content
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.

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

[API Implementation]: DirectoryInfo.CopyTo / Directory.Copy #62375

Closed
wants to merge 32 commits into from
Closed
Show file tree
Hide file tree
Changes from 31 commits
Commits
Show all changes
32 commits
Select commit Hold shift + click to select a range
2e6ebb0
FileSystem Copy (directory) logic implementation
deeprobin Dec 3, 2021
0367de3
Usage of FileSystem.Copy in static class Directory
deeprobin Dec 3, 2021
3eb9974
Fix return type in Directory.Copy
deeprobin Dec 3, 2021
c019831
Add FileSystem.Copy implementation for DirectoryInfo.CopyTo
deeprobin Dec 3, 2021
05a335c
Ref-Assembly Implementation
deeprobin Dec 12, 2021
e2a24ca
Usage of Full Paths
deeprobin Dec 12, 2021
fe409d6
Add Tests for Directory.Copy
deeprobin Dec 12, 2021
d4ead0f
Add test cases for DirectoryInfo.CopyTo
deeprobin Dec 12, 2021
3021503
Add documentation comments
deeprobin Dec 12, 2021
815aca7
Merge branch 'main' into issue-60903
deeprobin Jan 23, 2022
0298d60
Update ref assembly
deeprobin Jan 23, 2022
5a38e66
Merge branch 'main' into issue-60903
deeprobin Jan 23, 2022
0a1820d
Fix ref assembly
deeprobin Jan 23, 2022
4d9da94
Fix ref assembly
deeprobin Jan 23, 2022
fad47d0
Style change
deeprobin Feb 2, 2022
20404b4
Address feedback from @bartonjs
deeprobin Feb 2, 2022
f430130
Fix tests
deeprobin Feb 2, 2022
4bf462f
Usage of `EnumerationOptions` overload for `EnumerateDirectories`
deeprobin Feb 4, 2022
cdca890
Merge branch 'main' into issue-60903
deeprobin Feb 5, 2022
d207673
Merge branch 'main' into issue-60903
deeprobin Feb 8, 2022
69e7a3b
Fix ref
deeprobin Feb 11, 2022
727997f
Remove `IgnoreInaccessible`
deeprobin Feb 11, 2022
ce89d2d
Remove overload with non-optional parameters
deeprobin Feb 11, 2022
7952a77
Fix mutation of EnumerationOptions instance
deeprobin Feb 12, 2022
42625af
Merge branch 'main' into issue-60903
deeprobin Feb 19, 2022
d8b32cc
Merge branch 'main' into issue-60903
deeprobin Mar 10, 2022
480ea54
Improve documentation
deeprobin Mar 10, 2022
518a734
Use FileSystemEnumerable
deeprobin Mar 10, 2022
6963cb0
Improve documentation
deeprobin Mar 11, 2022
d617da8
Remove redundant using
deeprobin Mar 11, 2022
04843bd
Use FileSystemEnumerable in Copy implementation
deeprobin Mar 11, 2022
08db652
Address feedback (attention: contains a failing test)
deeprobin Mar 15, 2022
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
216 changes: 216 additions & 0 deletions src/libraries/System.IO.FileSystem/tests/Directory/Copy.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,216 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System.IO.Tests;
using System.Threading;
using Xunit;

namespace System.IO.FileSystem.Tests
{
public sealed class Directory_CopyDirectory : FileSystemTest
{
[Fact]
public void Copy()
{
const string sourceDirName = $"Test_source_{nameof(Copy)}";
const string destDirName = $"Test_target_{nameof(Copy)}";
deeprobin marked this conversation as resolved.
Show resolved Hide resolved
CancellationToken cancellationToken = new();

// Prerequisites
// Create source folder
if (Directory.Exists(sourceDirName))
{
Directory.Delete(sourceDirName, true);
}
Directory.CreateDirectory(sourceDirName);
deeprobin marked this conversation as resolved.
Show resolved Hide resolved

// Create destination folder
if (Directory.Exists(destDirName))
{
Directory.Delete(destDirName, true);
}
Directory.CreateDirectory(destDirName);

// Create file which does not exist in target folder
string firstPath = Path.Combine(sourceDirName, "first.txt");
using (StreamWriter firstStreamSource = File.CreateText(firstPath))
{
firstStreamSource.WriteLine("Test");
}
deeprobin marked this conversation as resolved.
Show resolved Hide resolved

// Create file which does exist in target folder
string secondPathSource = Path.Combine(sourceDirName, "test.txt");
using (StreamWriter secondStreamSource = File.CreateText(secondPathSource))
{
secondStreamSource.WriteLine("Test");
}

string secondPathDest = Path.Combine(destDirName, "test.txt");
using (StreamWriter secondStreamDest = File.CreateText(secondPathDest))
{
secondStreamDest.WriteLine("Test");
}

// Do file system operation
Directory.Copy(sourceDirName, destDirName, false, false, cancellationToken);
deeprobin marked this conversation as resolved.
Show resolved Hide resolved

// Validate cancellation
Assert.False(cancellationToken.IsCancellationRequested);
}

[Fact]
public void Copy_SkipExistingFiles()
{
const string sourceDirName = $"Test_source_{nameof(Copy_SkipExistingFiles)}";
const string destDirName = $"Test_target_{nameof(Copy_SkipExistingFiles)}";
CancellationToken cancellationToken = new();

// Prerequisites
// Create source folder
if (Directory.Exists(sourceDirName))
{
Directory.Delete(sourceDirName, true);
}
Directory.CreateDirectory(sourceDirName);

// Create destination folder
if (Directory.Exists(destDirName))
{
Directory.Delete(destDirName, true);
}
Directory.CreateDirectory(destDirName);

// Create file which does not exist in target folder
string firstPath = Path.Combine(sourceDirName, "first.txt");
using (StreamWriter firstStreamSource = File.CreateText(firstPath))
{
firstStreamSource.WriteLine("Test");
}

// Create file which does exist in target folder
string secondPathSource = Path.Combine(sourceDirName, "test.txt");
using (StreamWriter secondStreamSource = File.CreateText(secondPathSource))
{
secondStreamSource.WriteLine("Test");
}

string secondPathDest = Path.Combine(destDirName, "test.txt");
using (StreamWriter secondStreamDest = File.CreateText(secondPathDest))
{
secondStreamDest.WriteLine("Test");
}
deeprobin marked this conversation as resolved.
Show resolved Hide resolved

// Do file system operation
Directory.Copy(sourceDirName, destDirName, false, true, cancellationToken);

// Validate cancellation
Assert.False(cancellationToken.IsCancellationRequested);
}

[Fact]
public void Copy_Recursive()
{
const string sourceDirName = $"Test_source_{nameof(Copy_Recursive)}";
const string destDirName = $"Test_target_{nameof(Copy_Recursive)}";
CancellationToken cancellationToken = new();

// Prerequisites
// Create source folder
if (Directory.Exists(sourceDirName))
{
Directory.Delete(sourceDirName, true);
}
Directory.CreateDirectory(sourceDirName);

// Create destination folder
if (Directory.Exists(destDirName))
{
Directory.Delete(destDirName, true);
}
Directory.CreateDirectory(destDirName);

string sourceTopLevelFilePath = Path.Combine(sourceDirName, "top-level.txt");
using (StreamWriter sourceTopLevelFileStream = File.CreateText(sourceTopLevelFilePath))
{
sourceTopLevelFileStream.WriteLine("Test");
}

string sourceSecondLevelDirectoryPath = Path.Combine(sourceDirName, "subdir");
Directory.CreateDirectory(sourceSecondLevelDirectoryPath);

string sourceThirdLevelFilePath = Path.Combine(sourceSecondLevelDirectoryPath, "test.txt");
using (StreamWriter sourceThirdLevelFileStream = File.CreateText(sourceThirdLevelFilePath))
{
sourceThirdLevelFileStream.WriteLine("Test");
}

// Do file system opreation
Directory.Copy(sourceDirName, destDirName, true, false, cancellationToken);

// Validate cancellation
Assert.False(cancellationToken.IsCancellationRequested);

// Validate copied files
string destTopLevelFilePath = Path.Combine(destDirName, "top-level.txt");
string destSecondLevelDirectoryPath = Path.Combine(destDirName, "subdir");
string destThirdLevelFilePath = Path.Combine(destSecondLevelDirectoryPath, "test.txt");

Assert.True(File.Exists(destTopLevelFilePath));
Assert.True(File.Exists(destThirdLevelFilePath));
}

[Fact]
public void Copy_NonRecursive()
{
const string sourceDirName = $"Test_source_{nameof(Copy_NonRecursive)}";
const string destDirName = $"Test_target_{nameof(Copy_NonRecursive)}";
CancellationToken cancellationToken = new();

// Prerequisites
// Create source folder
if (Directory.Exists(sourceDirName))
{
Directory.Delete(sourceDirName, true);
}
Directory.CreateDirectory(sourceDirName);

// Create destination folder
if (Directory.Exists(destDirName))
{
Directory.Delete(destDirName, true);
}
Directory.CreateDirectory(destDirName);

string sourceTopLevelFilePath = Path.Combine(sourceDirName, "top-level.txt");
using (StreamWriter sourceTopLevelFileStream = File.CreateText(sourceTopLevelFilePath))
{
sourceTopLevelFileStream.WriteLine("Test");
}

string sourceSecondLevelDirectoryPath = Path.Combine(sourceDirName, "subdir");
Directory.CreateDirectory(sourceSecondLevelDirectoryPath);

string sourceThirdLevelFilePath = Path.Combine(sourceSecondLevelDirectoryPath, "test.txt");
using (StreamWriter sourceThirdLevelFileStream = File.CreateText(sourceThirdLevelFilePath))
{
sourceThirdLevelFileStream.WriteLine("Test");
}

// Do file system opreation
Directory.Copy(sourceDirName, destDirName, false, false, cancellationToken);

// Validate cancellation
Assert.False(cancellationToken.IsCancellationRequested);

// Validate copied files
string destTopLevelFilePath = Path.Combine(destDirName, "top-level.txt");
string destSecondLevelDirectoryPath = Path.Combine(destDirName, "subdir");
string destThirdLevelFilePath = Path.Combine(destSecondLevelDirectoryPath, "test.txt");

Assert.True(File.Exists(destTopLevelFilePath));

// False because we only want to copy top level files (= non-recursive)
Assert.False(File.Exists(destThirdLevelFilePath));
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You need to add tests for read and write errors, and for the simpler Directory.Copy overload.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In my last commit, I added these. However, the two tests that check for it will fail.

I don't know exactly why but I guess it's still due to the problem that I don't know exactly how to clearly distinguish the read and write failures....

Maybe you can find another approach.

Feel free to cancel the CI, I'm sure it won't pass.

}
}
43 changes: 43 additions & 0 deletions src/libraries/System.IO.FileSystem/tests/DirectoryInfo/CopyTo.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System.IO.Tests;
using System.Threading;
using Xunit;

namespace System.IO.FileSystem.Tests
{
public sealed class CopyTo : Directory_CreateDirectory
deeprobin marked this conversation as resolved.
Show resolved Hide resolved
{
[Fact]
public void SimpleCopyToTest()
{
CancellationToken cancellationToken = new();
const string sourceFolderPath = $"test_source_{nameof(SimpleCopyToTest)}";
const string destFolderPath = $"test_dest_{nameof(SimpleCopyToTest)}";

// Prerequisites
var sourceFolder = Directory.CreateDirectory(sourceFolderPath);
var destFolder = Directory.CreateDirectory(destFolderPath);

// Fill source folder with test files
const int n = 10;
for (int i = 0; i < n; i++)
{
string fileName = Path.Combine(sourceFolderPath, $"{i}.txt");

using StreamWriter writer = File.CreateText(fileName);
writer.WriteLine("Lorem ipsum dolor sit a test");
}

// Do file system operation
sourceFolder.CopyTo(destFolderPath, false, false, cancellationToken);

// Validate cancellation
Assert.False(cancellationToken.IsCancellationRequested);

// Validate existing files
Assert.Equal(n, destFolder.GetFiles().Length);
}
}
deeprobin marked this conversation as resolved.
Show resolved Hide resolved
}
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@
<Compile Include="Base\SymbolicLinks\BaseSymbolicLinks.cs" />
<Compile Include="Base\SymbolicLinks\BaseSymbolicLinks.FileSystem.cs" />
<Compile Include="Base\SymbolicLinks\BaseSymbolicLinks.FileSystemInfo.cs" />
<Compile Include="DirectoryInfo\CopyTo.cs" />
<Compile Include="Directory\Copy.cs" />
<Compile Include="Directory\EnumerableTests.cs" />
<Compile Include="Directory\SymbolicLinks.cs" />
<Compile Include="DirectoryInfo\SymbolicLinks.cs" />
Expand Down
34 changes: 34 additions & 0 deletions src/libraries/System.Private.CoreLib/src/System/IO/Directory.cs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
using System.Diagnostics.CodeAnalysis;
using System.IO;
using System.IO.Enumeration;
using System.Threading;

namespace System.IO
{
Expand Down Expand Up @@ -284,5 +285,38 @@ public static FileSystemInfo CreateSymbolicLink(string path, string pathToTarget
FileSystem.VerifyValidPath(linkPath, nameof(linkPath));
return FileSystem.ResolveLinkTarget(linkPath, returnFinalTarget, isDirectory: true);
}

/// <summary>
/// Copies a directory into another directory.
/// </summary>
/// <param name="sourcePath">The source path</param>
deeprobin marked this conversation as resolved.
Show resolved Hide resolved
/// <param name="destinationPath">The destination path</param>
deeprobin marked this conversation as resolved.
Show resolved Hide resolved
/// <param name="recursive"><see langword="true" /> to copy directories, subdirectories, and files in <paramref name="sourcePath"/>; otherwise, <see langword="false" />.</param>
/// <returns><see langword="true"/> if the operation succeeded in copying all the specified files and directories; otherwise, <see langword="false"/>.</returns>
/// <exception cref="IOException">A write operation fails</exception>
deeprobin marked this conversation as resolved.
Show resolved Hide resolved
public static bool Copy(string sourcePath, string destinationPath, bool recursive) =>
Copy(sourcePath, destinationPath, recursive, true);
deeprobin marked this conversation as resolved.
Show resolved Hide resolved

/// <summary>
/// Copies a directory into another directory.
/// </summary>
/// <param name="sourcePath">The source path</param>
deeprobin marked this conversation as resolved.
Show resolved Hide resolved
/// <param name="destinationPath">The destination path</param>
deeprobin marked this conversation as resolved.
Show resolved Hide resolved
/// <param name="recursive"><see langword="true" /> to copy directories, subdirectories, and files in <paramref name="sourcePath"/>; otherwise, <see langword="false" />.</param>
/// <param name="skipExistingFiles"><see langword="true" /> to skip overwrite of existing files in <paramref name="destinationPath"/>; otherwise, <see langword="false" />.</param>
/// <param name="cancellationToken">The token to monitor for cancellation requests. The default value is <see cref="CancellationToken.None" /></param>
deeprobin marked this conversation as resolved.
Show resolved Hide resolved
/// <returns><see langword="true"/> if the operation succeeded in copying all the specified files and directories; otherwise, <see langword="false"/>.</returns>
/// <exception cref="IOException">A write operation fails</exception>
/// <remarks>
/// <paramref name="recursive"/> specifies whether only the top-level files (<see langword="false"/>) should be copied or also subordinate ones (<see langword="true"/>).
/// If <paramref name="skipExistingFiles"/> is <see langword="false"/>, an <see cref="IOException"/> is thrown on an already existing file.
deeprobin marked this conversation as resolved.
Show resolved Hide resolved
/// </remarks>
public static bool Copy(string sourcePath, string destinationPath, bool recursive, bool skipExistingFiles = true, CancellationToken cancellationToken = default)
{
string fullSourcePath = Path.GetFullPath(sourcePath);
string fullDestinationPath = Path.GetFullPath(destinationPath);

return FileSystem.Copy(fullSourcePath, fullDestinationPath, recursive, skipExistingFiles, cancellationToken);
}
jozkee marked this conversation as resolved.
Show resolved Hide resolved
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
using System.Collections.Generic;
using System.Diagnostics;
using System.IO.Enumeration;
using System.Threading;

namespace System.IO
{
Expand Down Expand Up @@ -206,5 +207,37 @@ public void Delete(bool recursive)
FileSystem.RemoveDirectory(FullPath, recursive);
Invalidate();
}

/// <summary>
/// Copies this directory into another directory.
/// </summary>
///
deeprobin marked this conversation as resolved.
Show resolved Hide resolved
/// <param name="destinationPath">The destination path</param>
deeprobin marked this conversation as resolved.
Show resolved Hide resolved
/// <param name="recursive"><see langword="true" /> to copy directories, subdirectories, and files in <see cref="FileSystemInfo.OriginalPath"/>; otherwise, <see langword="false" />.</param>
/// <returns>If the operation succeeded, <see langword="true"/>. Otherwise <see langword="false"/>.</returns>
deeprobin marked this conversation as resolved.
Show resolved Hide resolved
/// <exception cref="IOException">A write operation fails</exception>
public bool CopyTo(string destinationPath, bool recursive) => CopyTo(destinationPath, recursive, true);
jozkee marked this conversation as resolved.
Show resolved Hide resolved

/// <summary>
/// Copies this directory into another directory.
///
deeprobin marked this conversation as resolved.
Show resolved Hide resolved
/// <paramref name="recursive"/> specifies whether only the top-level files (<see langword="false"/>) should be copied or also subordinate ones (<see langword="true"/>).
/// If <paramref name="skipExistingFiles"/> is <see langword="false"/>, an <see cref="IOException"/> is thrown on an already existing file.
deeprobin marked this conversation as resolved.
Show resolved Hide resolved
/// </summary>
/// <param name="destinationPath">The destination path</param>
deeprobin marked this conversation as resolved.
Show resolved Hide resolved
/// <param name="recursive"><see langword="true" /> to copy directories, subdirectories, and files in <see cref="FileSystemInfo.OriginalPath"/>; otherwise, <see langword="false" />.</param>
/// <param name="skipExistingFiles"><see langword="true" /> to skip overwrite of existing files in <paramref name="destinationPath"/>; otherwise, <see langword="false" />.</param>
/// <param name="cancellationToken">The token to monitor for cancellation requests. The default value is <see cref="CancellationToken.None" /></param>
deeprobin marked this conversation as resolved.
Show resolved Hide resolved
/// <returns><see langword="true"/> if the operation succeeded in copying all the specified files and directories; otherwise, <see langword="false"/>.</returns>
/// <exception cref="IOException">A write operation fails</exception>
public bool CopyTo(string destinationPath, bool recursive, bool skipExistingFiles = true,
CancellationToken cancellationToken = default)
Comment on lines +233 to +234
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit

Suggested change
public bool CopyTo(string destinationPath, bool recursive, bool skipExistingFiles = true,
CancellationToken cancellationToken = default)
public bool CopyTo(string destinationPath, bool recursive, bool skipExistingFiles = true, CancellationToken cancellationToken = default)

{
string fullSourcePath = FullPath;
string fullDestinationPath = Path.GetFullPath(destinationPath);

return FileSystem.Copy(fullSourcePath, fullDestinationPath, recursive, skipExistingFiles,
cancellationToken);
Comment on lines +239 to +240
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit

Suggested change
return FileSystem.Copy(fullSourcePath, fullDestinationPath, recursive, skipExistingFiles,
cancellationToken);
return FileSystem.Copy(fullSourcePath, fullDestinationPath, recursive, skipExistingFiles, cancellationToken);

}
}
jozkee marked this conversation as resolved.
Show resolved Hide resolved
}
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ public class EnumerationOptions
internal static EnumerationOptions Compatible { get; } =
new EnumerationOptions { MatchType = MatchType.Win32, AttributesToSkip = 0, IgnoreInaccessible = false };

private static EnumerationOptions CompatibleRecursive { get; } =
internal static EnumerationOptions CompatibleRecursive { get; } =
new EnumerationOptions { RecurseSubdirectories = true, MatchType = MatchType.Win32, AttributesToSkip = 0, IgnoreInaccessible = false };

/// <summary>
Expand Down
Loading