Merge pull request #246 from SixLabors/js/file-system-fixes

PhysicalFileSystem optimizations

Author: JimBobSquarePants

src/Directory.Build.targets

@@ -31,5 +31,6 @@
     <PackageReference Update="Microsoft.AspNetCore.WebUtilities" Version="2.2.0" />
     <PackageReference Update="Microsoft.Extensions.Caching.Abstractions" Version="2.2.0" />
     <PackageReference Update="Microsoft.Extensions.Options.ConfigurationExtensions" Version="2.2.0" />
+    <PackageReference Update="Microsoft.Extensions.FileProviders.Physical" Version="2.2.0"/>
   </ItemGroup>
 </Project>

src/ImageSharp.Web/Caching/PhysicalFileSystemCache.cs

@@ -67,7 +67,7 @@ internal static string GetCacheRoot(PhysicalFileSystemCacheOptions cacheOptions,
             string cacheRootPath = cacheOptions.CacheRootPath ?? webRootPath;
             if (string.IsNullOrEmpty(cacheRootPath))
             {
-                throw new InvalidOperationException("The cache root path can't be determined, make sure it's explicitly configured or the webroot is set.");
+                throw new InvalidOperationException("The cache root path cannot be determined, make sure it's explicitly configured or the webroot is set.");
             }
 
             if (!Path.IsPathFullyQualified(cacheRootPath))

src/ImageSharp.Web/ImageCacheMetadata.cs

@@ -156,13 +156,11 @@ public static async Task<ImageCacheMetadata> ReadAsync(Stream stream)
         /// <inheritdoc/>
         [MethodImpl(MethodImplOptions.AggressiveInlining)]
         public bool Equals(ImageCacheMetadata other)
-        {
-            return this.SourceLastWriteTimeUtc == other.SourceLastWriteTimeUtc
-                   && this.CacheLastWriteTimeUtc == other.CacheLastWriteTimeUtc
-                   && this.ContentType == other.ContentType
-                   && this.CacheControlMaxAge == other.CacheControlMaxAge
-                   && this.ContentLength == other.ContentLength;
-        }
+            => this.SourceLastWriteTimeUtc == other.SourceLastWriteTimeUtc
+            && this.CacheLastWriteTimeUtc == other.CacheLastWriteTimeUtc
+            && this.ContentType == other.ContentType
+            && this.CacheControlMaxAge == other.CacheControlMaxAge
+            && this.ContentLength == other.ContentLength;
 
         /// <inheritdoc/>
         public override bool Equals(object obj)
@@ -181,16 +179,14 @@ public override int GetHashCode() => HashCode.Combine(
         /// </summary>
         /// <returns>The <see cref="Dictionary{String, String}"/>.</returns>
         public Dictionary<string, string> ToDictionary()
-        {
-            return new Dictionary<string, string>
+            => new()
             {
                 { SourceLastModifiedKey, this.SourceLastWriteTimeUtc.ToString("o") },
                 { CacheLastModifiedKey, this.CacheLastWriteTimeUtc.ToString("o") },
                 { ContentTypeKey, this.ContentType },
                 { CacheControlKey, this.CacheControlMaxAge.TotalSeconds.ToString(NumberFormatInfo.InvariantInfo) },
                 { ContentLengthKey, this.ContentLength.ToString(NumberFormatInfo.InvariantInfo) }
             };
-        }
 
         /// <inheritdoc/>
         public override string ToString()

src/ImageSharp.Web/ImageSharp.Web.csproj

@@ -1,4 +1,4 @@
-<Project Sdk="Microsoft.NET.Sdk">
+<Project Sdk="Microsoft.NET.Sdk">
 
   <PropertyGroup>
     <AssemblyTitle>SixLabors.ImageSharp.Web</AssemblyTitle>
@@ -41,6 +41,7 @@
     <PackageReference Include="Microsoft.AspNetCore.WebUtilities" />
     <PackageReference Include="Microsoft.Extensions.Caching.Abstractions" />
     <PackageReference Include="Microsoft.Extensions.Options.ConfigurationExtensions" />
+    <PackageReference Include="Microsoft.Extensions.FileProviders.Physical" />
   </ItemGroup>
 
   <ItemGroup>

src/ImageSharp.Web/Providers/FileProviderImageProvider.cs

@@ -13,7 +13,7 @@ namespace SixLabors.ImageSharp.Web.Providers
     /// <summary>
     /// Returns images from an <see cref="IFileProvider"/> abstraction.
     /// </summary>
-    public class FileProviderImageProvider : IImageProvider
+    public abstract class FileProviderImageProvider : IImageProvider
     {
         /// <summary>
         /// The file provider abstraction.
@@ -29,21 +29,26 @@ public class FileProviderImageProvider : IImageProvider
         /// Initializes a new instance of the <see cref="FileProviderImageProvider"/> class.
         /// </summary>
         /// <param name="fileProvider">The file provider.</param>
+        /// <param name="processingBehavior">The processing behavior.</param>
         /// <param name="formatUtilities">Contains various format helper methods based on the current configuration.</param>
-        public FileProviderImageProvider(IFileProvider fileProvider, FormatUtilities formatUtilities)
+        protected FileProviderImageProvider(IFileProvider fileProvider, ProcessingBehavior processingBehavior, FormatUtilities formatUtilities)
         {
-            this.fileProvider = fileProvider ?? throw new ArgumentNullException(nameof(fileProvider));
-            this.formatUtilities = formatUtilities ?? throw new ArgumentNullException(nameof(formatUtilities));
+            Guard.NotNull(fileProvider, nameof(fileProvider));
+            Guard.NotNull(formatUtilities, nameof(formatUtilities));
+
+            this.fileProvider = fileProvider;
+            this.formatUtilities = formatUtilities;
+            this.ProcessingBehavior = processingBehavior;
         }
 
         /// <inheritdoc/>
-        public ProcessingBehavior ProcessingBehavior { get; protected set; } = ProcessingBehavior.CommandOnly;
+        public ProcessingBehavior ProcessingBehavior { get; }
 
         /// <inheritdoc/>
-        public Func<HttpContext, bool> Match { get; set; } = _ => true;
+        public virtual Func<HttpContext, bool> Match { get; set; } = _ => true;
 
         /// <inheritdoc/>
-        public bool IsValidRequest(HttpContext context)
+        public virtual bool IsValidRequest(HttpContext context)
             => this.formatUtilities.TryGetExtensionFromUri(context.Request.GetDisplayUrl(), out _);
 
         /// <inheritdoc/>

src/ImageSharp.Web/Providers/PhysicalFileSystemProvider.cs

@@ -3,30 +3,17 @@
 
 using System;
 using System.IO;
-using System.Threading.Tasks;
 using Microsoft.AspNetCore.Hosting;
-using Microsoft.AspNetCore.Http;
-using Microsoft.AspNetCore.Http.Extensions;
+using Microsoft.Extensions.FileProviders;
 using Microsoft.Extensions.Options;
-using SixLabors.ImageSharp.Web.Resolvers;
 
 namespace SixLabors.ImageSharp.Web.Providers
 {
     /// <summary>
     /// Returns images stored in the local physical file system.
     /// </summary>
-    public class PhysicalFileSystemProvider : IImageProvider
+    public sealed class PhysicalFileSystemProvider : FileProviderImageProvider
     {
-        /// <summary>
-        /// The root path for the provider.
-        /// </summary>
-        private readonly string providerRootPath;
-
-        /// <summary>
-        /// Contains various format helper methods based on the current configuration.
-        /// </summary>
-        private readonly FormatUtilities formatUtilities;
-
         /// <summary>
         /// Initializes a new instance of the <see cref="PhysicalFileSystemProvider"/> class.
         /// </summary>
@@ -41,56 +28,23 @@ public PhysicalFileSystemProvider(
             IWebHostEnvironment environment,
 #endif
             FormatUtilities formatUtilities)
+            : base(GetProvider(options, environment), options.Value.ProcessingBehavior, formatUtilities)
         {
-            Guard.NotNull(options, nameof(options));
-            Guard.NotNull(environment, nameof(environment));
-
-            this.providerRootPath = GetProviderRoot(options.Value, environment.WebRootPath, environment.ContentRootPath);
-            this.formatUtilities = formatUtilities;
-        }
-
-        /// <inheritdoc/>
-        public ProcessingBehavior ProcessingBehavior { get; } = ProcessingBehavior.CommandOnly;
-
-        /// <inheritdoc/>
-        public Func<HttpContext, bool> Match { get; set; } = _ => true;
-
-        /// <inheritdoc/>
-        public bool IsValidRequest(HttpContext context)
-            => this.formatUtilities.TryGetExtensionFromUri(context.Request.GetDisplayUrl(), out _);
-
-        /// <inheritdoc/>
-        public Task<IImageResolver> GetAsync(HttpContext context)
-        {
-            // Use Join because request path starts with a slash
-            string fullPath = Path.GetFullPath(Path.Join(this.providerRootPath, context.Request.Path.Value));
-            if (PathUtilities.IsUnderneathRoot(fullPath, this.providerRootPath))
-            {
-                // Check to see if the file exists
-                var fileInfo = new FileInfo(fullPath);
-                if (fileInfo.Exists)
-                {
-                    var metadata = new ImageMetadata(fileInfo.LastWriteTimeUtc, fileInfo.Length);
-                    return Task.FromResult<IImageResolver>(new PhysicalFileSystemResolver(fileInfo, metadata));
-                }
-            }
-
-            return Task.FromResult<IImageResolver>(null);
         }
 
         /// <summary>
         /// Determine the provider root path
         /// </summary>
-        /// <param name="providerOptions">The provider options.</param>
+        /// <param name="options">The provider options.</param>
         /// <param name="webRootPath">The web root path.</param>
         /// <param name="contentRootPath">The content root path.</param>
         /// <returns><see cref="string"/> representing the fully qualified provider root path.</returns>
-        internal static string GetProviderRoot(PhysicalFileSystemProviderOptions providerOptions, string webRootPath, string contentRootPath)
+        internal static string GetProviderRoot(PhysicalFileSystemProviderOptions options, string webRootPath, string contentRootPath)
         {
-            string providerRootPath = providerOptions.ProviderRootPath ?? webRootPath;
+            string providerRootPath = options.ProviderRootPath ?? webRootPath;
             if (string.IsNullOrEmpty(providerRootPath))
             {
-                throw new InvalidOperationException("The provider root path can't be determined, make sure it's explicitly configured or the webroot is set.");
+                throw new InvalidOperationException("The provider root path cannot be determined, make sure it's explicitly configured or the webroot is set.");
             }
 
             if (!Path.IsPathFullyQualified(providerRootPath))
@@ -101,5 +55,18 @@ internal static string GetProviderRoot(PhysicalFileSystemProviderOptions provide
 
             return PathUtilities.EnsureTrailingSlash(providerRootPath);
         }
+
+        private static PhysicalFileProvider GetProvider(
+            IOptions<PhysicalFileSystemProviderOptions> options,
+#if NETCOREAPP2_1
+            IHostingEnvironment environment)
+#else
+            IWebHostEnvironment environment)
+#endif
+        {
+            Guard.NotNull(options, nameof(options));
+            Guard.NotNull(environment, nameof(environment));
+            return new(GetProviderRoot(options.Value, environment.WebRootPath, environment.ContentRootPath));
+        }
     }
 }

src/ImageSharp.Web/Providers/PhysicalFileSystemProviderOptions.cs

@@ -21,5 +21,10 @@ public class PhysicalFileSystemProviderOptions
         /// </para>
         /// </summary>
         public string ProviderRootPath { get; set; }
+
+        /// <summary>
+        /// Gets or sets the processing behavior. Defaults to <see cref="ProcessingBehavior.CommandOnly"/>.
+        /// </summary>
+        public ProcessingBehavior ProcessingBehavior { get; set; } = ProcessingBehavior.CommandOnly;
     }
 }

src/ImageSharp.Web/Providers/WebRootImageProvider.cs

@@ -22,7 +22,7 @@ public WebRootImageProvider(
             IWebHostEnvironment environment,
 #endif
             FormatUtilities formatUtilities)
-            : base(environment.WebRootFileProvider, formatUtilities)
+            : base(environment.WebRootFileProvider, ProcessingBehavior.CommandOnly, formatUtilities)
         {
         }
     }

src/ImageSharp.Web/Resolvers/PhysicalFileSystemCacheResolver.cs

@@ -2,6 +2,7 @@
 // Licensed under the Apache License, Version 2.0.
 
 using System.IO;
+using System.Runtime.CompilerServices;
 using System.Threading.Tasks;
 using SixLabors.ImageSharp.Web.Caching;
 
@@ -32,8 +33,12 @@ public PhysicalFileSystemCacheResolver(FileInfo metaFileInfo, FormatUtilities fo
         /// <inheritdoc/>
         public async Task<ImageCacheMetadata> GetMetaDataAsync()
         {
-            using Stream stream = OpenFileStream(this.metaFileInfo.FullName);
-            this.metadata = await ImageCacheMetadata.ReadAsync(stream);
+            if (this.metadata == default)
+            {
+                using FileStream stream = OpenFileStream(this.metaFileInfo.FullName);
+                this.metadata = await ImageCacheMetadata.ReadAsync(stream);
+            }
+
             return this.metadata;
         }
 
@@ -44,14 +49,15 @@ public Task<Stream> OpenReadAsync()
                 this.metaFileInfo.FullName,
                 this.formatUtilities.GetExtensionFromContentType(this.metadata.ContentType));
 
-            return Task.FromResult(OpenFileStream(path));
+            return Task.FromResult<Stream>(OpenFileStream(path));
         }
 
-        private static Stream OpenFileStream(string path)
+        [MethodImpl(MethodImplOptions.AggressiveInlining)]
+        private static FileStream OpenFileStream(string path)
         {
             // We are setting buffer size to 1 to prevent FileStream from allocating it's internal buffer
             // 0 causes constructor to throw
-            int bufferSize = 1;
+            const int bufferSize = 1;
             return new FileStream(
                 path,
                 FileMode.Open,