Skip to content
This repository was archived by the owner on Nov 6, 2018. It is now read-only.

Implement CombinedFileSystemProvider #49

Closed
glennc opened this issue Jan 21, 2015 · 12 comments
Closed

Implement CombinedFileSystemProvider #49

glennc opened this issue Jan 21, 2015 · 12 comments

Comments

@glennc
Copy link
Member

glennc commented Jan 21, 2015

The CombinedFileSystemProvider would accept multiple FileSystemProviders and return files if they exist in one of them.

@kichalla
Copy link
Member

@muratg Fixing this issue can help in fixing aspnet/Diagnostics#157 where razor files could be(as MVC gives this option) coming from physical or embedded resources

cc @davidfowl

@kichalla
Copy link
Member

Actually ignore my previous comment, the fix for aspnet/Diagnostics#157 is not dependent on fixing this issue...but from an end user perspective would be nice to have..

@pranavkm pranavkm self-assigned this Aug 5, 2015
@pranavkm
Copy link
Contributor

pranavkm commented Aug 5, 2015

Need this for aspnet/Mvc#2551

@pranavkm pranavkm removed their assignment Aug 5, 2015
@glennc glennc added this to the Backlog milestone Sep 2, 2015
@glennc
Copy link
Member Author

glennc commented Sep 2, 2015

This is backlog for now. If it is definitely needed for MVC then come talk to us.

@mgrosperrin
Copy link
Contributor

Hi,
I'm really interested by this CombinedFileProvider.
Currently, I have my own implementation, but have to duplicate the not found file and directory (from the Microsoft.AspNet.FileProviders.Sources project).
Can I submit you a PR with this new file provider or is already someone working on it ?

@pranavkm
Copy link
Contributor

PR sounds great. I tried playing with this at one point and sort of dropped it because it was missing some design -

  1. For GetFileInfo - does it keep querying until it finds a file?
  2. For GetDirectoryContents - does it combine the results of multiple IFileProvider instances?

These might not be an issue if individual IFileProviders were isolated in separate hives (had to have distinct roots) but that didn't work for the scenarios we were trying to use it in Mvc.

@mgrosperrin
Copy link
Contributor

Hi,
OK for merging the content of multiple directories.
When I do this, should I consider that the Name property (of IFileInfo) is unique for a directory (ie when I have a file with a given name from a directory, I skip all files with the same name in the later directories)?

@MartinJohns
Copy link

I was thinking a bit about this issue and here are my two cents:

  1. GetFileInfo: This should iterate over all provided IFileProvider and return the first result where Exists is true. If there was no existing file found, then a FileNotFound class should be returned, as it is happening with all providers currently. This potentially leads to a lot of unnecessary garbage, because each provider will always instantiate a FileNotFound class which is thrown away immediately.
  2. GetDirectoryContents: This should merge the contents of all provided IFileProvider. On duplicate entries (uniqueness on the Name property) it should take the first entry it found (again by the order of provided file providers).
  3. Watch: The biggest trouble I see is with this method that returns the IChangeToken which notifies about changes. I think simply not providing this functionality is a poor cheap way out and would not help with some requirements. I think this should combine the changes of all file providers. This would allow nifty things like having an embedded version, which can be overridden by a physical file without restarting the application. I would even think further and say that a change notification from provider B should not be propagated, when the actual file was returned by provider A (since A takes priority and it did not change) - but that would seem impossible the given interface, and changes in such an elementary interface are unlikely at RC1.

@muratg
Copy link

muratg commented Nov 19, 2015

Thanks, @MartinJohns. FWIV, RC1 is already released BTW! :)

@pranavkm
Copy link
Contributor

cc @loudej

I would even think further and say that a change notification from provider B should not be propagated, when the actual file was returned by provider A

This was specifically the issue that stopped me from creating a CombinedFileProvider based solution for precompiled views in Mvc.

@mgrosperrin
Copy link
Contributor

Hi,
for the Watch method, I decided to create a new IChangeToken that "merges" all tokens.

The caller will potentially be notified for files that cannot be accessed, but I don't see how to do differently 😞

@pranavkm
Copy link
Contributor

@mgrosperrin that sounds fine. Over expiring shouldn't be a problem. The first two bullet points from @MartinJohns sound fine.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

6 participants