Skip to content

Allow interception of File.Open / new FileStream() - #52

Merged
LordMike merged 2 commits into
DiscUtils:masterfrom
ahdde:filelocator
Jul 22, 2017
Merged

Allow interception of File.Open / new FileStream()#52
LordMike merged 2 commits into
DiscUtils:masterfrom
ahdde:filelocator

Conversation

@zivillian

Copy link
Copy Markdown
Contributor

This adds the option to modify the parameters for any FileStream, which is opened inside DiscUtils.

The first commit adds the LocalFileLocator (I've skipped the standalone binaries from Utilities), to reduce the number of places, where files are opened. The second commit introduces an event, which can be used to modify the parameters (filename, mode, share & access), wrap the filestream or return a completly different stream.

This allows us to modify the FileShare parameter for vhdx files (as described in #24) or wrap all FileStreams to workaround a huge performance penalty (factor > 1000) in the FileStream.Length Property for files residing on a Cluster Shared Volume (CSV).

This may also be useful for unit testing, because the files to be opened don't even have to exist.

@LordMike

Copy link
Copy Markdown
Member

Would an event here and there be the best way to convey flexibility like this?

In other projects, patterns used are f.ex. dependency injection. I do like the proposed option better than f.ex. a duplicated settings object everywhere.

@zivillian

zivillian commented Jul 20, 2017

Copy link
Copy Markdown
Contributor Author

My first approach was a Func<> Property in SetupHelper, but there is no Func<> in net20. My second idea was to add a RegisterFileLocator() method for FileLocator, but FileLocator is internal and it doesn't make sense to create a new FileLocator, just to override a parameter for new FileStream(). My first implementation would have been an EventFileLocator, which would have done exactly the same.

That's why I've added the FileLocator wherever it was missing (and reasonable), and add a single place to add specific handling for opening local files (that's also, why I've chosen SetupHelper, because I guess this is the place the user has already touched to register the required modules).

This event may be replaced with something more elegant, if there are additional "global" settings or overrides but I guess for now it's the most simple and elegant solution.

I also thought about adding some kind of warning, because it's a static event, which may prevent garbage collection if not unsubscribed. On the other side, you need a very special use case to subscribe to this event, and in this case you hopefully know about the internals of discutils and the downsides of static events.

@LordMike

Copy link
Copy Markdown
Member

Regarding Func, it can be introduced as needed.

@LordMike
LordMike merged commit f204326 into DiscUtils:master Jul 22, 2017
@zivillian
zivillian deleted the filelocator branch July 22, 2017 11:58
LordMike added a commit that referenced this pull request Aug 16, 2017
Includes PRs:
#17, #18, #21, #22, #23, #27, #30, #31, #33, #34, #35, #36, #38, #39, #40, #41, #48, #51, #52, #55, #60
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants