Skip to content

Add provider-independent middleware and CDN media URL provider #11

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 5 commits into from

Conversation

ronaldbarendse
Copy link
Contributor

This builds on top of PR #9 and moves provider-independent code to the new Umbraco.StorageProviders project.

It also adds an abstraction/wrapper to make Umbraco's IFileSystem (read/write) compatible with the ASP.NET Core IFileProvider (read-only), so it can be used by the built-in StaticFileMiddleware to serve files.

Adding support for a new storage provider would then only require implementing the IFileSystem and optionally add some helper methods to easily configure it.

We're also taking advantage of all features already available in the StaticFileMiddleware (like compression and altering the response, e.g. to set cache control headers).

I'm creating this as a draft, because there's still some things to improve and test:

  • The general CDN options are now loaded from Umbraco:Storage:Media:Cdn and the Azure Blob Storage version adds the configuration from Umbraco:Storage:AzureBlob:Media:Cdn on top of that and configures the CDN URL to use the container name as path. I'm not sure if the naming and cascading make sense though...
  • The extension methods configuring UseStaticFiles() can be make more generic and should expose the options, so it's actually possible to alter the response headers, etc.
  • Initial performance testing shows no noticeable changes with the previous AzureBlobFileSystemMiddleware, but this is something that needs more extensive testing.

@ronaldbarendse ronaldbarendse changed the base branch from initial-review to main August 27, 2021 13:47
@ronaldbarendse
Copy link
Contributor Author

Just a thought: it makes more sense to directly implement the IFileProvider instead of wrapping/proxying calls to the IFileSystem. Because the IFileProvider interface is used for serving the files (using the StaticFileMiddleware), it should be optimized for read-performance and not go through another layer of abstraction. The IFileSystem could also use some cleanup, so I would propose to:

  • Introduce a new IStorageProvider interface that extends IFileProvider and adds a clean API for writing files;
  • Write a FileSystemStorageProvider wrapper for compatibility with existing IFileSystem implementations;
  • Add new extension methods that setup both the Umbraco file system (using builder.SetMediaFileSystem() or builder.ConfigureFileSystems()) and serving the files (using UseStaticFiles()).

This way you only need to implement a single IStorageProvider interface to support a new storage type and most will already have an IFileProvider implementation, like the PhysicalFileProvider.

In the future, we might replace the IFileSystem with the IStorageProvider within Umbraco itself. In the meantime, having it in a Umbraco.StorageProviders package and providing a backwards compatibility layer is perfectly fine!

@ronaldbarendse ronaldbarendse added the enhancement New feature or request label Oct 5, 2021
@olibos
Copy link

olibos commented Oct 7, 2021

Hello @ronaldbarendse , I love your idea to access the storage through the IFileProvider!

@rickbutterfield
Copy link

I've pulled down this branch and included it in my project and it works perfectly for my use case! I migrated a site from WordPress to Umbraco about a year ago, but not all the content came across into Umbraco's media. The existing /wp-content/uploads folder was migrated to Azure blob storage and I used the v8 Azure File System Provider to access it with a custom IFileSystem. Here's how I'm using this new one as an example...

Startup.cs

public void ConfigureServices(IServiceCollection services)
{
    services
        .AddAzureBlobMediaFileSystem()
        .AddAzureBlobFileSystem("WpContent", "~/wp-content");
}

public void Configure(IApplicationBuilder app, IWebHostEnvironment env)
{
    app.UseUmbraco()
        .WithMiddleware(u =>
        {
            u.UseAzureBlobMediaFileSystem();
            u.UseAzureBlobMediaCustomFileSystem("WpContent");
        });
}

AzureBlobMediaFileSystemExtensions.cs

public static IUmbracoApplicationBuilderContext UseAzureBlobMediaCustomFileSystem(this IUmbracoApplicationBuilderContext builder, string fileSystemName)
{
    if (builder == null) throw new ArgumentNullException(nameof(builder));

    UseAzureBlobMediaCustomFileSystem(builder.AppBuilder, fileSystemName);

    return builder;
}

public static IApplicationBuilder UseAzureBlobMediaCustomFileSystem(this IApplicationBuilder app, string fileSystemName)
{
    if (app == null) throw new ArgumentNullException(nameof(app));

    var fileSystem = app.ApplicationServices.GetRequiredService<IAzureBlobFileSystemProvider>().GetFileSystem(fileSystemName);
    var options = app.ApplicationServices.GetRequiredService<IOptionsFactory<AzureBlobFileSystemOptions>>().Create(fileSystemName);
    var hostingEnvironment = app.ApplicationServices.GetRequiredService<IHostingEnvironment>();

    var requestPath = hostingEnvironment.ToAbsolute(options.VirtualPath);

    app.UseStaticFiles(new StaticFileOptions()
    {
        FileProvider = new FileSystemFileProvider(fileSystem, requestPath),
        RequestPath = requestPath,
        OnPrepareResponse = ctx =>
        {
            var headers = ctx.Context.Response.GetTypedHeaders();
            headers.CacheControl = new CacheControlHeaderValue
            {
                Public = true,
                MustRevalidate = true,
                MaxAge = TimeSpan.FromDays(7)
            };
        }
    });

    return app;
}

Apologies for the big code block but hopefully this helps someone!

@ronaldbarendse
Copy link
Contributor Author

This is now obsoleted by the work done in #36.

@ronaldbarendse ronaldbarendse deleted the feature/fileproviderwrapper branch March 23, 2022 10:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants