TestableIO / System.IO.Abstractions.Extensions

Convenience functionality on top of System.IO.Abstractions
MIT License
21 stars 6 forks source link

feat: Add extension to IFileSystem to allow using pattern to delete files / directories #41

Closed MattKotsenas closed 1 year ago

MattKotsenas commented 1 year ago

Add two new extension methods on IFileSystem:

Each creates a file / directory and returns it via an out param and returns an instance of IDisposable so that a using statement will automatically delete the file when the block goes out of scope.

An overload is also provided that automatically creates a uniquely named file using the file system's temp path and random file name.

Another overload is provided that takes a Func of the IFileInfo / IDirectoryInfo and returns the corresponding IDisposable implementation. This allows callers to provide a custom implementation of IDisposable to do things like:

As a result, the DisposableFile, DisposableDirectory, and DisposableFileSystemInfo classes are all public to allow for inheritance. Note however that the extension methods only return IDisposable, so most callers will not encounter them.

Unit tests also added to verify functionality.

Fixes #40

MattKotsenas commented 1 year ago

@gigi81, here's a first pass at the extension methods. I'm open to any and all feedback. A couple of points to note:

To minimize number of new public APIs / classes, I made DisposableDirectory and DisposableFile internal and just return an IDisposable. I like that is keeps the number of concepts small, but it also means I can't directly unit test those classes. To work around that I added the [InternalsVisibleTo] annotation via the Directory.Build.props file. If you disagree with that solution I can retarget the tests against the extension methods. That would avoid testing interal classes, at the expense of making the tests a bit more difficult to follow.

I added two calls to IFileSystemEntry.Refresh(): one after creation and one after Dispose() to ensure that the file attributes are in sync. One could probably argue that these calls are unnecessary and that users should know to make those calls before checking attributes, but they tripped me up when writing unit tests, so I think it's a common problem people will run into and is worth solving in the library.

I tried to follow the pattern of unit testing (organized by extension of type, naming convention, etc.). However if I made a mistake please let me know and I'll update.

gigi81 commented 1 year ago

Thanks @MattKotsenas. this looks great from what I can tell from my phone. Give me a couple of days until I can get in front of a computer to review properly. I'm the meantime thanks!

MattKotsenas commented 1 year ago

Note that since the PR was opened, I added another overload that takes a Func of the IFileInfo / IDirectoryInfo and returns the corresponding IDisposable implementation. This allows callers to provide a custom implementation of IDisposable to do things like:

As a result, the DisposableFile, DisposableDirectory, and DisposableFileSystemInfo classes are all public to allow for inheritance / code reuse. Note however that the extension methods only return IDisposable, so most callers will not encounter them.

Thanks to @jackhorton for the feedback on the need for custom disposable implementations.

If either of you have thoughts / ideas / suggestions / concerns with this approach, please let me know. Thanks!

gigi81 commented 1 year ago

thanks, @MattKotsenas it was quite a bit of code to review! it looks very good to me. only a few notes if you can please have a look. mainly to get rid of the codacity warnings and some better comments

MattKotsenas commented 1 year ago

Thanks @gigi81 ! I believe I made all the changes you've requested. Please take another look at your earliest convenance. If you have any questions or concerns, please let me know. Thanks!

github-actions[bot] commented 1 year ago

This is addressed in release v1.0.43.