Enhance EmbeddedFileProvider to support multiple assemblies and can be overridden by physical files.

[mnguyen284]

As suggested by @davidfowl I'm opening an issue to make some enhancements to EmbeddedFileProvider

1. Embedded views can be loaded from multiple assemblies.
2. Embedded views can be overridden by physical file views MVC's default or a specified provider.
3. List of assemblies and base namespaces, and physical files root default provider are configured via new EmbeddedFileProviderOptions class by DI.

I have forked the project and will start working on it. Any suggestion please let me know.

Thanks a lot.

[mnguyen284]

Hi @davidfowl,

I have completed implementation, unit test will be pushed soon. Do I need to let you have a look at it before making a pull request?

Thanks,
Minh

[MartinJohns]

Wouldn't it make more sense to implement #49 (Implement CombinedFileSystemProvider) instead of this?
With #49 you could just create a CombinedFileProvider, which receives the PhysicalFileProvider as the first file provider, and the EmbeddedFileProvider as a second. It would check for physical files first, and fall back to the embedded file. This way you also could easily provide files from Azure Blob storage or any other resource.

I think this would keep the separation much cleaner, as well as the code for the EmbeddedFileProvider. I don't think the EmbeddedFileProvider should know anything about other file providers. The job is clear: provide access to embedded files.

[mnguyen284]

I actually prefer CombinedFileSystemProvider approach. That would be a general solution for any provider. However, without being able to support multiple assemblies and can be overridden by a default provider, the usage of EmbeddedFileProvider is very limited. This might be why @davidfowl suggested me to go with this option in #130.

With current implementation, all views for a website must come from the same assembly. And the website cannot use views from another assembly or it's own views. Also, it doesn't make sense to use that view assembly for a different website as this is a very tight couple.

The most useful of EmbeddedFileProvider is for code reuse. As requested in aspnet/Mvc#3237, people want the ability to create an assembly that contains View Components (or Controllers) and their views to make a complete module and use it in multiple projects. A project should also be able to use multiple modules as well.

I think this would keep the separation much cleaner, as well as the code for the EmbeddedFileProvider. I don't think the EmbeddedFileProvider should know anything about other file providers. The job is clear: provide access to embedded files.

You are right. A provider shouldn't know anything about another provider. In my implementation, EmbeddedFileProvider accepts an IFileProvider as a default provider and a list of assemblies to find files from. Files will be searched for in assemblies only when it is not found in the default provider. This way, the default provider can be any IFileProvider, not only PhysicalFileProvider.

[MartinJohns]

With the CombinedFileSystemProvider approach you could also just specify multiple EmbeddedFileProvider for different assemblies. I'm unsure which approach I would choose. Having multiple EmbeddedFileProvider seems like the cleaner approach, but having one EmbeddedFileProvider for multiple assemblies is an easier-to-use approach.

Regardless of that, I'd definitely remove the IFileProvider usage from the EmbeddedFileProvider.

[davidfowl]

Yea I wasn't suggesting to change EmbeddedFileProvider to have an IFileProvider fallback, that's weird. CombinedFileProvider is what you want plus the ability to use multiple assemblies for embedded resources (potentially).

[mnguyen284]

Sounds reasonable. I have assumed that EmbeddedFileProvider would be useless without the ability to be overridden, which can be wrong. I still prefer having a provider that supports the scenario above but I can always make it a project based custom provider if I really need it. My remaining concern is that without CombinedFileSystemProvider EmbeddedFileProvider seems quite useless. So how should I go further with this then?

[davidfowl]

I think you should file an issue on the MVC repository and call out the specific use cases before adding a generic CombinedFileSystemProvider to this repository.

[vncastanheira]

I've been using a custom IFileProvider for multiple assemblies and it worked fine until beta5.
It was nothing more than a modified EmbeddedFileProvider that checked a list of assemblies instead of one.
But it seems that you must specify your namespace now and that won't work so well for external assemblies...

[muratg]

Putting this in 1.0.0 backlog. Please link to any MVC bug that you may open for specific use cases.

[Antaris]

I've been designing a composite file provider which takes a set of ordered file providers and returns the first file instance that exists. My use case here is I am developing a CMS based on ASP.NET and for my modules I am deploying resources such as views, scripts, images etc. as embedded resources. I want to enable scenarios whereby if we want to customise a particular resource, we can bring that resource into the file system. Essentially the composite file provider will work as such:

var fileProvider = new CompositeFileProvider(
  new[]
  {
    new PhysicalFileProvider(),
    new ModuleFileProvider(moduleProvider)
  });

The implementation of ModuleFileProvider isn't important for this issue, but what is, is a facade over a set of EmbeddedFileProvider instances for my loaded modules.

Everything works a treat except I am confused about the best approach to the Watch method. Given that I accept a set of file providers, I am unsure how I should resolve the change token. Currently I am naively returning the change token from the first file provider, the PhysicalFileProvider, but is this the right approach? For MVC, reading files like Views (through a custom ViewLocationExpander) is the Watch method actually called, and will this have an impact on anything?

My current implementation is:

using System.Collections.Generic;
using System.Linq;
using Microsoft.AspNet.FileProviders;
using Microsoft.Framework.Primitives;

/// <summary>
/// Represents a file provider that pulls from a set of ordered file providers.
/// </summary>
public class CompositeFileProvider : IFileProvider
{
    private readonly IEnumerable<IFileProvider> _fileProviders;

    /// <summary>
    /// Initialises a new instance of <see cref="CompositeFileProvider"/>.
    /// </summary>
    /// <param name="fileProviders">The set of ordered file providers.</param>
    public CompositeFileProvider(IEnumerable<IFileProvider> fileProviders)
    {
        _fileProviders = Ensure.ArgumentNotNull(fileProviders, nameof(fileProviders));
    }

    /// <inheritdocs />
    public IDirectoryContents GetDirectoryContents(string subpath)
    {
        IDirectoryContents directoryContents = null;
        foreach (var fileProvider in _fileProviders)
        {
            directoryContents = fileProvider.GetDirectoryContents(subpath);
            if (directoryContents != null && directoryContents.Exists)
            {
                return directoryContents;
            }
        }

        return new NotFoundDirectoryContents();
    }

    /// <inheritdocs />
    public IFileInfo GetFileInfo(string subpath)
    {
        IFileInfo fileInfo = null;
        foreach (var fileProvider in _fileProviders)
        {
            fileInfo = fileProvider.GetFileInfo(subpath);
            if (fileInfo != null && fileInfo.Exists)
            {
                return fileInfo;
            }
        }

        return new NotFoundFileInfo(subpath);
    }

    /// <inheritdocs />
    public IChangeToken Watch(string filter)
    {
        // MA - Unsure of how the change token should be generated in this instance, so delegating to the first provider instance.
        return _fileProviders.ElementAtOrDefault(0)?.Watch(filter);
    }
}

[MartinJohns]

I would vote to close this issue as a duplicate of #49 (Implement CombinedFileSystemProvider), as the requested feature would be solved with that approach. Currently the same approach is discussed separately in two different issues, which is not beneficial for finding the best solution.

[muratg]

@MartinJohns Agreed, https://github.com/aspnet/FileSystem/pull/142/files fixes this.