From 2f7e31ab5be5522aaeb508c818e4de0e59b2a30e Mon Sep 17 00:00:00 2001 From: Hao Kung Date: Fri, 4 Mar 2016 15:16:21 -0800 Subject: [PATCH] Remove GetService calls in Static Files --- .../DefaultFilesMiddleware.cs | 8 ++++--- .../DefaultFilesOptions.cs | 2 +- .../DirectoryBrowserMiddleware.cs | 23 +++++++++++-------- .../DirectoryBrowserOptions.cs | 3 +-- .../FileExtensionContentTypeProvider.cs | 1 - .../FileServerOptions.cs | 2 +- .../Helpers.cs | 11 +++++++++ .../HtmlDirectoryFormatter.cs | 14 +++++++---- .../Infrastructure/SharedOptionsBase.cs | 16 +------------ .../Resources.Designer.cs | 16 ------------- .../Resources.resx | 3 --- .../StaticFileContext.cs | 10 +++++--- .../StaticFileMiddleware.cs | 13 +++++------ .../StaticFileOptions.cs | 4 +--- .../DirectoryBrowserMiddlewareTests.cs | 5 ++-- .../StaticFileContextTest.cs | 6 ++--- .../StaticFileMiddlewareTests.cs | 3 ++- 17 files changed, 64 insertions(+), 76 deletions(-) diff --git a/src/Microsoft.AspNetCore.StaticFiles/DefaultFilesMiddleware.cs b/src/Microsoft.AspNetCore.StaticFiles/DefaultFilesMiddleware.cs index ecd51ef831..ab4af41e8b 100644 --- a/src/Microsoft.AspNetCore.StaticFiles/DefaultFilesMiddleware.cs +++ b/src/Microsoft.AspNetCore.StaticFiles/DefaultFilesMiddleware.cs @@ -6,6 +6,7 @@ using System.Threading.Tasks; using Microsoft.AspNetCore.Builder; using Microsoft.AspNetCore.Hosting; using Microsoft.AspNetCore.Http; +using Microsoft.Extensions.FileProviders; using Microsoft.Extensions.Options; using Microsoft.Net.Http.Headers; @@ -21,6 +22,7 @@ namespace Microsoft.AspNetCore.StaticFiles private readonly DefaultFilesOptions _options; private readonly PathString _matchUrl; private readonly RequestDelegate _next; + private readonly IFileProvider _fileProvider; /// /// Creates a new instance of the DefaultFilesMiddleware. @@ -47,7 +49,7 @@ namespace Microsoft.AspNetCore.StaticFiles _next = next; _options = options.Value; - _options.ResolveFileProvider(hostingEnv); + _fileProvider = _options.FileProvider ?? Helpers.ResolveFileProvider(hostingEnv); _matchUrl = _options.RequestPath; } @@ -64,14 +66,14 @@ namespace Microsoft.AspNetCore.StaticFiles if (Helpers.IsGetOrHeadMethod(context.Request.Method) && Helpers.TryMatchPath(context, _matchUrl, forDirectory: true, subpath: out subpath)) { - var dirContents = _options.FileProvider.GetDirectoryContents(subpath.Value); + var dirContents = _fileProvider.GetDirectoryContents(subpath.Value); if (dirContents.Exists) { // Check if any of our default files exist. for (int matchIndex = 0; matchIndex < _options.DefaultFileNames.Count; matchIndex++) { string defaultFile = _options.DefaultFileNames[matchIndex]; - var file = _options.FileProvider.GetFileInfo(subpath + defaultFile); + var file = _fileProvider.GetFileInfo(subpath + defaultFile); // TryMatchPath will make sure subpath always ends with a "/" by adding it if needed. if (file.Exists) { diff --git a/src/Microsoft.AspNetCore.StaticFiles/DefaultFilesOptions.cs b/src/Microsoft.AspNetCore.StaticFiles/DefaultFilesOptions.cs index 6019c77971..72b577dfcc 100644 --- a/src/Microsoft.AspNetCore.StaticFiles/DefaultFilesOptions.cs +++ b/src/Microsoft.AspNetCore.StaticFiles/DefaultFilesOptions.cs @@ -9,7 +9,7 @@ namespace Microsoft.AspNetCore.Builder /// /// Options for selecting default file names. /// - public class DefaultFilesOptions : SharedOptionsBase + public class DefaultFilesOptions : SharedOptionsBase { /// /// Configuration for the DefaultFilesMiddleware. diff --git a/src/Microsoft.AspNetCore.StaticFiles/DirectoryBrowserMiddleware.cs b/src/Microsoft.AspNetCore.StaticFiles/DirectoryBrowserMiddleware.cs index 479043a15c..dd13fe4ba7 100644 --- a/src/Microsoft.AspNetCore.StaticFiles/DirectoryBrowserMiddleware.cs +++ b/src/Microsoft.AspNetCore.StaticFiles/DirectoryBrowserMiddleware.cs @@ -2,6 +2,7 @@ // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System; +using System.Text.Encodings.Web; using System.Threading.Tasks; using Microsoft.AspNetCore.Builder; using Microsoft.AspNetCore.Hosting; @@ -20,14 +21,17 @@ namespace Microsoft.AspNetCore.StaticFiles private readonly DirectoryBrowserOptions _options; private readonly PathString _matchUrl; private readonly RequestDelegate _next; + private readonly IDirectoryFormatter _formatter; + private readonly IFileProvider _fileProvider; /// /// Creates a new instance of the SendFileMiddleware. /// /// The next middleware in the pipeline. /// The used by this middleware. + /// The used by the default . /// The configuration for this middleware. - public DirectoryBrowserMiddleware(RequestDelegate next, IHostingEnvironment hostingEnv, IOptions options) + public DirectoryBrowserMiddleware(RequestDelegate next, IHostingEnvironment hostingEnv, HtmlEncoder encoder, IOptions options) { if (next == null) { @@ -39,19 +43,20 @@ namespace Microsoft.AspNetCore.StaticFiles throw new ArgumentNullException(nameof(hostingEnv)); } + if (encoder == null) + { + throw new ArgumentNullException(nameof(encoder)); + } + if (options == null) { throw new ArgumentNullException(nameof(options)); } - if (options.Value.Formatter == null) - { - throw new ArgumentException(Resources.Args_NoFormatter); - } - _next = next; _options = options.Value; - _options.ResolveFileProvider(hostingEnv); + _fileProvider = _options.FileProvider ?? Helpers.ResolveFileProvider(hostingEnv); + _formatter = options.Value.Formatter ?? new HtmlDirectoryFormatter(encoder); _matchUrl = _options.RequestPath; } @@ -78,7 +83,7 @@ namespace Microsoft.AspNetCore.StaticFiles return Constants.CompletedTask; } - return _options.Formatter.GenerateContentAsync(context, contents); + return _formatter.GenerateContentAsync(context, contents); } return _next(context); @@ -86,7 +91,7 @@ namespace Microsoft.AspNetCore.StaticFiles private bool TryGetDirectoryInfo(PathString subpath, out IDirectoryContents contents) { - contents = _options.FileProvider.GetDirectoryContents(subpath.Value); + contents = _fileProvider.GetDirectoryContents(subpath.Value); return contents.Exists; } } diff --git a/src/Microsoft.AspNetCore.StaticFiles/DirectoryBrowserOptions.cs b/src/Microsoft.AspNetCore.StaticFiles/DirectoryBrowserOptions.cs index f3a33d3fc9..611df33e54 100644 --- a/src/Microsoft.AspNetCore.StaticFiles/DirectoryBrowserOptions.cs +++ b/src/Microsoft.AspNetCore.StaticFiles/DirectoryBrowserOptions.cs @@ -9,7 +9,7 @@ namespace Microsoft.AspNetCore.Builder /// /// Directory browsing options /// - public class DirectoryBrowserOptions : SharedOptionsBase + public class DirectoryBrowserOptions : SharedOptionsBase { /// /// Enabled directory browsing for all request paths @@ -26,7 +26,6 @@ namespace Microsoft.AspNetCore.Builder public DirectoryBrowserOptions(SharedOptions sharedOptions) : base(sharedOptions) { - Formatter = new HtmlDirectoryFormatter(); } /// diff --git a/src/Microsoft.AspNetCore.StaticFiles/FileExtensionContentTypeProvider.cs b/src/Microsoft.AspNetCore.StaticFiles/FileExtensionContentTypeProvider.cs index a41ffe3488..06d694528a 100644 --- a/src/Microsoft.AspNetCore.StaticFiles/FileExtensionContentTypeProvider.cs +++ b/src/Microsoft.AspNetCore.StaticFiles/FileExtensionContentTypeProvider.cs @@ -3,7 +3,6 @@ using System; using System.Collections.Generic; -using System.IO; namespace Microsoft.AspNetCore.StaticFiles { diff --git a/src/Microsoft.AspNetCore.StaticFiles/FileServerOptions.cs b/src/Microsoft.AspNetCore.StaticFiles/FileServerOptions.cs index b533a2a2ef..f46e274cc1 100644 --- a/src/Microsoft.AspNetCore.StaticFiles/FileServerOptions.cs +++ b/src/Microsoft.AspNetCore.StaticFiles/FileServerOptions.cs @@ -8,7 +8,7 @@ namespace Microsoft.AspNetCore.Builder /// /// Options for all of the static file middleware components /// - public class FileServerOptions : SharedOptionsBase + public class FileServerOptions : SharedOptionsBase { /// /// Creates a combined options class for all of the static file middleware components. diff --git a/src/Microsoft.AspNetCore.StaticFiles/Helpers.cs b/src/Microsoft.AspNetCore.StaticFiles/Helpers.cs index 6124d55a3c..720d7ba163 100644 --- a/src/Microsoft.AspNetCore.StaticFiles/Helpers.cs +++ b/src/Microsoft.AspNetCore.StaticFiles/Helpers.cs @@ -2,12 +2,23 @@ // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System; +using Microsoft.AspNetCore.Hosting; using Microsoft.AspNetCore.Http; +using Microsoft.Extensions.FileProviders; namespace Microsoft.AspNetCore.StaticFiles { internal static class Helpers { + internal static IFileProvider ResolveFileProvider(IHostingEnvironment hostingEnv) + { + if (hostingEnv.WebRootFileProvider == null) { + throw new InvalidOperationException("Missing FileProvider."); + } + return hostingEnv.WebRootFileProvider; + } + + internal static bool IsGetOrHeadMethod(string method) { return IsGetMethod(method) || IsHeadMethod(method); diff --git a/src/Microsoft.AspNetCore.StaticFiles/HtmlDirectoryFormatter.cs b/src/Microsoft.AspNetCore.StaticFiles/HtmlDirectoryFormatter.cs index 0064a2600b..75bed5ddae 100644 --- a/src/Microsoft.AspNetCore.StaticFiles/HtmlDirectoryFormatter.cs +++ b/src/Microsoft.AspNetCore.StaticFiles/HtmlDirectoryFormatter.cs @@ -23,6 +23,15 @@ namespace Microsoft.AspNetCore.StaticFiles private HtmlEncoder _htmlEncoder; + public HtmlDirectoryFormatter(HtmlEncoder encoder) + { + if (encoder == null) + { + throw new ArgumentNullException(nameof(encoder)); + } + _htmlEncoder = encoder; + } + /// /// Generates an HTML view for a directory. /// @@ -37,11 +46,6 @@ namespace Microsoft.AspNetCore.StaticFiles throw new ArgumentNullException(nameof(contents)); } - if (_htmlEncoder == null) - { - _htmlEncoder = context.RequestServices.GetRequiredService(); - } - context.Response.ContentType = TextHtmlUtf8; if (Helpers.IsHeadMethod(context.Request.Method)) diff --git a/src/Microsoft.AspNetCore.StaticFiles/Infrastructure/SharedOptionsBase.cs b/src/Microsoft.AspNetCore.StaticFiles/Infrastructure/SharedOptionsBase.cs index abff959072..16900ec6fb 100644 --- a/src/Microsoft.AspNetCore.StaticFiles/Infrastructure/SharedOptionsBase.cs +++ b/src/Microsoft.AspNetCore.StaticFiles/Infrastructure/SharedOptionsBase.cs @@ -2,7 +2,6 @@ // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System; -using Microsoft.AspNetCore.Hosting; using Microsoft.AspNetCore.Http; using Microsoft.Extensions.FileProviders; @@ -11,8 +10,7 @@ namespace Microsoft.AspNetCore.StaticFiles.Infrastructure /// /// Options common to several middleware components /// - /// The type of the subclass - public abstract class SharedOptionsBase + public abstract class SharedOptionsBase { /// /// Creates an new instance of the SharedOptionsBase. @@ -50,17 +48,5 @@ namespace Microsoft.AspNetCore.StaticFiles.Infrastructure get { return SharedOptions.FileProvider; } set { SharedOptions.FileProvider = value; } } - - internal void ResolveFileProvider(IHostingEnvironment hostingEnv) - { - if (FileProvider == null) - { - FileProvider = hostingEnv.WebRootFileProvider; - if (FileProvider == null) - { - throw new InvalidOperationException("Missing FileProvider."); - } - } - } } } diff --git a/src/Microsoft.AspNetCore.StaticFiles/Resources.Designer.cs b/src/Microsoft.AspNetCore.StaticFiles/Resources.Designer.cs index ca7d5222cb..850389c5b1 100644 --- a/src/Microsoft.AspNetCore.StaticFiles/Resources.Designer.cs +++ b/src/Microsoft.AspNetCore.StaticFiles/Resources.Designer.cs @@ -10,22 +10,6 @@ namespace Microsoft.AspNetCore.StaticFiles private static readonly ResourceManager _resourceManager = new ResourceManager("Microsoft.AspNetCore.StaticFiles.Resources", typeof(Resources).GetTypeInfo().Assembly); - /// - /// No IContentTypeProvider was specified. - /// - internal static string Args_NoContentTypeProvider - { - get { return GetString("Args_NoContentTypeProvider"); } - } - - /// - /// No IContentTypeProvider was specified. - /// - internal static string FormatArgs_NoContentTypeProvider() - { - return GetString("Args_NoContentTypeProvider"); - } - /// /// No formatter provided. /// diff --git a/src/Microsoft.AspNetCore.StaticFiles/Resources.resx b/src/Microsoft.AspNetCore.StaticFiles/Resources.resx index d6df934382..73d3ecda10 100644 --- a/src/Microsoft.AspNetCore.StaticFiles/Resources.resx +++ b/src/Microsoft.AspNetCore.StaticFiles/Resources.resx @@ -117,9 +117,6 @@ System.Resources.ResXResourceWriter, System.Windows.Forms, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089 - - No IContentTypeProvider was specified. - No formatter provided. diff --git a/src/Microsoft.AspNetCore.StaticFiles/StaticFileContext.cs b/src/Microsoft.AspNetCore.StaticFiles/StaticFileContext.cs index 5bee24969e..45a0b037a4 100644 --- a/src/Microsoft.AspNetCore.StaticFiles/StaticFileContext.cs +++ b/src/Microsoft.AspNetCore.StaticFiles/StaticFileContext.cs @@ -27,6 +27,8 @@ namespace Microsoft.AspNetCore.StaticFiles private readonly HttpRequest _request; private readonly HttpResponse _response; private readonly ILogger _logger; + private readonly IFileProvider _fileProvider; + private readonly IContentTypeProvider _contentTypeProvider; private string _method; private bool _isGet; private bool _isHead; @@ -47,7 +49,7 @@ namespace Microsoft.AspNetCore.StaticFiles private IList _ranges; - public StaticFileContext(HttpContext context, StaticFileOptions options, PathString matchUrl, ILogger logger) + public StaticFileContext(HttpContext context, StaticFileOptions options, PathString matchUrl, ILogger logger, IFileProvider fileProvider, IContentTypeProvider contentTypeProvider) { _context = context; _options = options; @@ -57,6 +59,8 @@ namespace Microsoft.AspNetCore.StaticFiles _logger = logger; _requestHeaders = _request.GetTypedHeaders(); _responseHeaders = _response.GetTypedHeaders(); + _fileProvider = fileProvider; + _contentTypeProvider = contentTypeProvider; _method = null; _isGet = false; @@ -118,7 +122,7 @@ namespace Microsoft.AspNetCore.StaticFiles public bool LookupContentType() { - if (_options.ContentTypeProvider.TryGetContentType(_subPath.Value, out _contentType)) + if (_contentTypeProvider.TryGetContentType(_subPath.Value, out _contentType)) { return true; } @@ -134,7 +138,7 @@ namespace Microsoft.AspNetCore.StaticFiles public bool LookupFileInfo() { - _fileInfo = _options.FileProvider.GetFileInfo(_subPath.Value); + _fileInfo = _fileProvider.GetFileInfo(_subPath.Value); if (_fileInfo.Exists) { _length = _fileInfo.Length; diff --git a/src/Microsoft.AspNetCore.StaticFiles/StaticFileMiddleware.cs b/src/Microsoft.AspNetCore.StaticFiles/StaticFileMiddleware.cs index 4b5a2da50d..d2cf35ac83 100644 --- a/src/Microsoft.AspNetCore.StaticFiles/StaticFileMiddleware.cs +++ b/src/Microsoft.AspNetCore.StaticFiles/StaticFileMiddleware.cs @@ -7,6 +7,7 @@ using System.Threading.Tasks; using Microsoft.AspNetCore.Builder; using Microsoft.AspNetCore.Hosting; using Microsoft.AspNetCore.Http; +using Microsoft.Extensions.FileProviders; using Microsoft.Extensions.Logging; using Microsoft.Extensions.Options; @@ -21,6 +22,8 @@ namespace Microsoft.AspNetCore.StaticFiles private readonly PathString _matchUrl; private readonly RequestDelegate _next; private readonly ILogger _logger; + private readonly IFileProvider _fileProvider; + private readonly IContentTypeProvider _contentTypeProvider; /// /// Creates a new instance of the StaticFileMiddleware. @@ -51,14 +54,10 @@ namespace Microsoft.AspNetCore.StaticFiles throw new ArgumentNullException(nameof(loggerFactory)); } - if (options.Value.ContentTypeProvider == null) - { - throw new ArgumentException(Resources.Args_NoContentTypeProvider); - } - _next = next; _options = options.Value; - _options.ResolveFileProvider(hostingEnv); + _contentTypeProvider = options.Value.ContentTypeProvider ?? new FileExtensionContentTypeProvider(); + _fileProvider = _options.FileProvider ?? Helpers.ResolveFileProvider(hostingEnv); _matchUrl = _options.RequestPath; _logger = loggerFactory.CreateLogger(); } @@ -70,7 +69,7 @@ namespace Microsoft.AspNetCore.StaticFiles /// public Task Invoke(HttpContext context) { - var fileContext = new StaticFileContext(context, _options, _matchUrl, _logger); + var fileContext = new StaticFileContext(context, _options, _matchUrl, _logger, _fileProvider, _contentTypeProvider); if (!fileContext.ValidateMethod()) { diff --git a/src/Microsoft.AspNetCore.StaticFiles/StaticFileOptions.cs b/src/Microsoft.AspNetCore.StaticFiles/StaticFileOptions.cs index 1dcfa6a783..01cef16b68 100644 --- a/src/Microsoft.AspNetCore.StaticFiles/StaticFileOptions.cs +++ b/src/Microsoft.AspNetCore.StaticFiles/StaticFileOptions.cs @@ -10,7 +10,7 @@ namespace Microsoft.AspNetCore.Builder /// /// Options for serving static files /// - public class StaticFileOptions : SharedOptionsBase + public class StaticFileOptions : SharedOptionsBase { /// /// Defaults to all request paths @@ -25,8 +25,6 @@ namespace Microsoft.AspNetCore.Builder /// public StaticFileOptions(SharedOptions sharedOptions) : base(sharedOptions) { - ContentTypeProvider = new FileExtensionContentTypeProvider(); - OnPrepareResponse = _ => { }; } diff --git a/test/Microsoft.AspNetCore.StaticFiles.Tests/DirectoryBrowserMiddlewareTests.cs b/test/Microsoft.AspNetCore.StaticFiles.Tests/DirectoryBrowserMiddlewareTests.cs index 01be82e080..d5b2c1c384 100644 --- a/test/Microsoft.AspNetCore.StaticFiles.Tests/DirectoryBrowserMiddlewareTests.cs +++ b/test/Microsoft.AspNetCore.StaticFiles.Tests/DirectoryBrowserMiddlewareTests.cs @@ -22,9 +22,10 @@ namespace Microsoft.AspNetCore.StaticFiles [Fact] public async Task NullArguments() { - Assert.Throws(() => StaticFilesTestServer.Create( + // No exception, default provided + StaticFilesTestServer.Create( app => app.UseDirectoryBrowser(new DirectoryBrowserOptions { Formatter = null }), - services => services.AddDirectoryBrowser())); + services => services.AddDirectoryBrowser()); // No exception, default provided StaticFilesTestServer.Create( diff --git a/test/Microsoft.AspNetCore.StaticFiles.Tests/StaticFileContextTest.cs b/test/Microsoft.AspNetCore.StaticFiles.Tests/StaticFileContextTest.cs index ac31dba62a..b522889799 100644 --- a/test/Microsoft.AspNetCore.StaticFiles.Tests/StaticFileContextTest.cs +++ b/test/Microsoft.AspNetCore.StaticFiles.Tests/StaticFileContextTest.cs @@ -21,8 +21,7 @@ namespace Microsoft.AspNetCore.StaticFiles { // Arrange var options = new StaticFileOptions(); - options.FileProvider = new TestFileProvider(); - var context = new StaticFileContext(new DefaultHttpContext(), options, PathString.Empty, NullLogger.Instance); + var context = new StaticFileContext(new DefaultHttpContext(), options, PathString.Empty, NullLogger.Instance, new TestFileProvider(), new FileExtensionContentTypeProvider()); // Act var validateResult = context.ValidatePath(); @@ -43,11 +42,10 @@ namespace Microsoft.AspNetCore.StaticFiles { LastModified = new DateTimeOffset(2014, 1, 2, 3, 4, 5, TimeSpan.Zero) }); - options.FileProvider = fileProvider; var pathString = new PathString("/test"); var httpContext = new DefaultHttpContext(); httpContext.Request.Path = new PathString("/test/foo.txt"); - var context = new StaticFileContext(httpContext, options, pathString, NullLogger.Instance); + var context = new StaticFileContext(httpContext, options, pathString, NullLogger.Instance, fileProvider, new FileExtensionContentTypeProvider()); // Act context.ValidatePath(); diff --git a/test/Microsoft.AspNetCore.StaticFiles.Tests/StaticFileMiddlewareTests.cs b/test/Microsoft.AspNetCore.StaticFiles.Tests/StaticFileMiddlewareTests.cs index d10e854f0d..35d94d8cec 100644 --- a/test/Microsoft.AspNetCore.StaticFiles.Tests/StaticFileMiddlewareTests.cs +++ b/test/Microsoft.AspNetCore.StaticFiles.Tests/StaticFileMiddlewareTests.cs @@ -32,7 +32,8 @@ namespace Microsoft.AspNetCore.StaticFiles [Fact] public async Task NullArguments() { - Assert.Throws(() => StaticFilesTestServer.Create(app => app.UseStaticFiles(new StaticFileOptions { ContentTypeProvider = null }))); + // No exception, default provided + StaticFilesTestServer.Create(app => app.UseStaticFiles(new StaticFileOptions { ContentTypeProvider = null })); // No exception, default provided StaticFilesTestServer.Create(app => app.UseStaticFiles(new StaticFileOptions { FileProvider = null }));