feat(DirectoryInfoExtensions): add recursive parameter #6358
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Reviewer's GuideThis PR enhances the DirectoryInfoExtensions.Copy method by introducing a recursive flag, enforcing source directory validation, refactoring the copy implementation for files and subdirectories, and extending the unit tests to cover these new behaviors. Class diagram for updated DirectoryInfoExtensions.Copy methodclassDiagram
class DirectoryInfoExtensions {
+void Copy(DirectoryInfo dir, string destinationDir, bool recursive = true)
}
Flow diagram for DirectoryInfoExtensions.Copy with recursive parameterflowchart TD
A[Check if source directory exists] -->|No| B[Throw DirectoryNotFoundException]
A -->|Yes| C[Create destination directory]
C --> D[Copy files from source to destination]
D --> E{recursive?}
E -- Yes --> F[For each subdirectory, call Copy recursively]
E -- No --> G[End]
F --> G
File-Level Changes
Assessment against linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey @ArgoZhang - I've reviewed your changes and they look great!
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location> `src/BootstrapBlazor/Extensions/DirectoryInfoExtensions.cs:39` </location>
<code_context>
- }
- else if (info is DirectoryInfo di)
+ string targetFilePath = Path.Combine(destinationDir, file.Name);
+ file.CopyTo(targetFilePath);
+ }
+
</code_context>
<issue_to_address>
File.CopyTo does not overwrite existing files, which may cause issues if destination files already exist.
The previous code allowed overwriting by passing 'true' to 'CopyTo'. To maintain this behavior and prevent exceptions when files exist, add 'true' as the second argument.
</issue_to_address>
### Comment 2
<location> `test/UnitTest/Extensions/DirectoryInfoExtensionsTest.cs:35` </location>
<code_context>
var sourceDirInfo = new DirectoryInfo(sourceDir);
- sourceDirInfo.Copy(destDir);
+ sourceDirInfo.Copy(destDir, true);
Assert.True(Directory.Exists(destDir));
+
</code_context>
<issue_to_address>
Missing test for non-recursive copy (recursive = false).
Please add a test with 'recursive' set to false to confirm subdirectories are not copied when recursion is disabled.
</issue_to_address>
<suggested_fix>
<<<<<<< SEARCH
sourceDirInfo.Copy(destDir, true);
Assert.True(Directory.Exists(destDir));
// 测试源文件夹不存在的情况
var sourceDirNotExists = new DirectoryInfo(Path.Combine(rootDir, "notexists"));
var ex = Assert.Throws<DirectoryNotFoundException>(() => sourceDirNotExists.Copy(destDir, true));
Assert.NotNull(ex);
=======
sourceDirInfo.Copy(destDir, true);
Assert.True(Directory.Exists(destDir));
// Test non-recursive copy (recursive = false)
var nonRecursiveDestDir = Path.Combine(rootDir, "nonRecursiveDest");
sourceDirInfo.Copy(nonRecursiveDestDir, false);
Assert.True(Directory.Exists(nonRecursiveDestDir));
// Check that files in the root of sourceDir are copied
foreach (var file in Directory.GetFiles(sourceDir))
{
var fileName = Path.GetFileName(file);
Assert.True(File.Exists(Path.Combine(nonRecursiveDestDir, fileName)));
}
// Check that subdirectories are NOT copied
foreach (var subDir in Directory.GetDirectories(sourceDir))
{
var subDirName = Path.GetFileName(subDir);
Assert.False(Directory.Exists(Path.Combine(nonRecursiveDestDir, subDirName)));
}
// 测试源文件夹不存在的情况
var sourceDirNotExists = new DirectoryInfo(Path.Combine(rootDir, "notexists"));
var ex = Assert.Throws<DirectoryNotFoundException>(() => sourceDirNotExists.Copy(destDir, true));
Assert.NotNull(ex);
>>>>>>> REPLACE
</suggested_fix>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| } | ||
| else if (info is DirectoryInfo di) | ||
| string targetFilePath = Path.Combine(destinationDir, file.Name); | ||
| file.CopyTo(targetFilePath); |
There was a problem hiding this comment.
issue (bug_risk): File.CopyTo does not overwrite existing files, which may cause issues if destination files already exist.
The previous code allowed overwriting by passing 'true' to 'CopyTo'. To maintain this behavior and prevent exceptions when files exist, add 'true' as the second argument.
Comment on lines
+35
to
+41
| sourceDirInfo.Copy(destDir, true); | ||
| Assert.True(Directory.Exists(destDir)); | ||
|
|
||
| // 测试源文件夹不存在的情况 | ||
| var sourceDirNotExists = new DirectoryInfo(Path.Combine(rootDir, "notexists")); | ||
| var ex = Assert.Throws<DirectoryNotFoundException>(() => sourceDirNotExists.Copy(destDir, true)); | ||
| Assert.NotNull(ex); |
There was a problem hiding this comment.
suggestion (testing): Missing test for non-recursive copy (recursive = false).
Please add a test with 'recursive' set to false to confirm subdirectories are not copied when recursion is disabled.
Suggested change
| sourceDirInfo.Copy(destDir, true); | |
| Assert.True(Directory.Exists(destDir)); | |
| // 测试源文件夹不存在的情况 | |
| var sourceDirNotExists = new DirectoryInfo(Path.Combine(rootDir, "notexists")); | |
| var ex = Assert.Throws<DirectoryNotFoundException>(() => sourceDirNotExists.Copy(destDir, true)); | |
| Assert.NotNull(ex); | |
| sourceDirInfo.Copy(destDir, true); | |
| Assert.True(Directory.Exists(destDir)); | |
| // Test non-recursive copy (recursive = false) | |
| var nonRecursiveDestDir = Path.Combine(rootDir, "nonRecursiveDest"); | |
| sourceDirInfo.Copy(nonRecursiveDestDir, false); | |
| Assert.True(Directory.Exists(nonRecursiveDestDir)); | |
| // Check that files in the root of sourceDir are copied | |
| foreach (var file in Directory.GetFiles(sourceDir)) | |
| { | |
| var fileName = Path.GetFileName(file); | |
| Assert.True(File.Exists(Path.Combine(nonRecursiveDestDir, fileName))); | |
| } | |
| // Check that subdirectories are NOT copied | |
| foreach (var subDir in Directory.GetDirectories(sourceDir)) | |
| { | |
| var subDirName = Path.GetFileName(subDir); | |
| Assert.False(Directory.Exists(Path.Combine(nonRecursiveDestDir, subDirName))); | |
| } | |
| // 测试源文件夹不存在的情况 | |
| var sourceDirNotExists = new DirectoryInfo(Path.Combine(rootDir, "notexists")); | |
| var ex = Assert.Throws<DirectoryNotFoundException>(() => sourceDirNotExists.Copy(destDir, true)); | |
| Assert.NotNull(ex); |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #6358 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 718 718
Lines 31607 31608 +1
Branches 4461 4461
=========================================
+ Hits 31607 31608 +1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Link issues
fixes #6357
Summary By Copilot
Regression?
Risk
Verification
Packaging changes reviewed?
☑️ Self Check before Merge
Summary by Sourcery
Introduce a recursive option and improve error handling for the DirectoryInfoExtensions.Copy method
New Features:
recursiveparameter to enable optional deep copying of subdirectoriesBug Fixes:
Enhancements:
Documentation:
Tests: