From 52daca7056218b9180d942798597f6bc1c7bb932 Mon Sep 17 00:00:00 2001 From: Pranav K Date: Sat, 3 Apr 2021 07:44:49 -0700 Subject: [PATCH 1/2] Clean up some MVC code --- .../ControllerBinderDelegateProvider.cs | 14 +++++++++----- .../src/TagHelpers/UrlResolutionTagHelper.cs | 10 +--------- src/Mvc/Mvc.TagHelpers/src/AttributeMatcher.cs | 7 ++----- .../src/DefaultDisplayTemplates.cs | 2 +- .../Mvc.ViewFeatures/src/DefaultEditorTemplates.cs | 12 ++++++++++-- src/Mvc/Mvc.ViewFeatures/src/ModelExplorer.cs | 4 ++-- .../src/ModelExplorerExtensions.cs | 8 ++++---- src/Mvc/MvcNoDeps.slnf | 2 +- .../ChunkingCookieManager/ChunkingCookieManager.cs | 4 +--- 9 files changed, 31 insertions(+), 32 deletions(-) diff --git a/src/Mvc/Mvc.Core/src/Controllers/ControllerBinderDelegateProvider.cs b/src/Mvc/Mvc.Core/src/Controllers/ControllerBinderDelegateProvider.cs index b8c5788afb6d..84164a688e51 100644 --- a/src/Mvc/Mvc.Core/src/Controllers/ControllerBinderDelegateProvider.cs +++ b/src/Mvc/Mvc.Core/src/Controllers/ControllerBinderDelegateProvider.cs @@ -4,7 +4,9 @@ using System; using System.Collections.Generic; using System.Diagnostics; +using System.Linq; using System.Threading.Tasks; +using Microsoft.AspNetCore.Mvc.Abstractions; using Microsoft.AspNetCore.Mvc.ModelBinding; namespace Microsoft.AspNetCore.Mvc.Controllers @@ -55,6 +57,9 @@ internal static class ControllerBinderDelegateProvider return null; } + var parameters = actionDescriptor.Parameters as List ?? actionDescriptor.Parameters.ToList(); + var properties = actionDescriptor.BoundProperties as List ?? actionDescriptor.BoundProperties.ToList(); + return Bind; async Task Bind(ControllerContext controllerContext, object controller, Dictionary arguments) @@ -67,9 +72,8 @@ async Task Bind(ControllerContext controllerContext, object controller, Dictiona Debug.Assert(valueProvider is not null); - var parameters = actionDescriptor.Parameters; - - for (var i = 0; i < parameters.Count; i++) + var parameterCount = parameters.Count; + for (var i = 0; i < parameterCount; i++) { var parameter = parameters[i]; var bindingInfo = parameterBindingInfo![i]; @@ -95,8 +99,8 @@ async Task Bind(ControllerContext controllerContext, object controller, Dictiona } } - var properties = actionDescriptor.BoundProperties; - for (var i = 0; i < properties.Count; i++) + var propertyCount = properties.Count; + for (var i = 0; i < propertyCount; i++) { var property = properties[i]; var bindingInfo = propertyBindingInfo![i]; diff --git a/src/Mvc/Mvc.Razor/src/TagHelpers/UrlResolutionTagHelper.cs b/src/Mvc/Mvc.Razor/src/TagHelpers/UrlResolutionTagHelper.cs index b3cbce85787b..29fcacc77061 100644 --- a/src/Mvc/Mvc.Razor/src/TagHelpers/UrlResolutionTagHelper.cs +++ b/src/Mvc/Mvc.Razor/src/TagHelpers/UrlResolutionTagHelper.cs @@ -343,15 +343,7 @@ private static string CreateTrimmedString(string input, int start) private static bool IsCharWhitespace(char ch) { - for (var i = 0; i < ValidAttributeWhitespaceChars.Length; i++) - { - if (ValidAttributeWhitespaceChars[i] == ch) - { - return true; - } - } - // the character is not white space - return false; + return ValidAttributeWhitespaceChars.AsSpan().IndexOf(ch) != -1; } private class EncodeFirstSegmentContent : IHtmlContent diff --git a/src/Mvc/Mvc.TagHelpers/src/AttributeMatcher.cs b/src/Mvc/Mvc.TagHelpers/src/AttributeMatcher.cs index 55584fa94f93..b52d98041707 100644 --- a/src/Mvc/Mvc.TagHelpers/src/AttributeMatcher.cs +++ b/src/Mvc/Mvc.TagHelpers/src/AttributeMatcher.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 System.Collections.Generic; using Microsoft.AspNetCore.Razor.TagHelpers; namespace Microsoft.AspNetCore.Mvc.TagHelpers @@ -24,7 +23,7 @@ internal static class AttributeMatcher /// true if a mode was determined, otherwise false. public static bool TryDetermineMode( TagHelperContext context, - IReadOnlyList> modeInfos, + ModeAttributes[] modeInfos, Func compare, out TMode result) { @@ -47,13 +46,11 @@ public static bool TryDetermineMode( result = default; // Perf: Avoid allocating enumerator - var modeInfosCount = modeInfos.Count; var allAttributes = context.AllAttributes; // Read interface .Count once rather than per iteration var allAttributesCount = allAttributes.Count; - for (var i = 0; i < modeInfosCount; i++) + foreach (var modeInfo in modeInfos.AsSpan()) { - var modeInfo = modeInfos[i]; var requiredAttributes = modeInfo.Attributes; // If there are fewer attributes present than required, one or more of them must be missing. if (allAttributesCount >= requiredAttributes.Length && diff --git a/src/Mvc/Mvc.ViewFeatures/src/DefaultDisplayTemplates.cs b/src/Mvc/Mvc.ViewFeatures/src/DefaultDisplayTemplates.cs index d1671a248e85..ac3b51999b67 100644 --- a/src/Mvc/Mvc.ViewFeatures/src/DefaultDisplayTemplates.cs +++ b/src/Mvc/Mvc.ViewFeatures/src/DefaultDisplayTemplates.cs @@ -212,7 +212,7 @@ public static IHtmlContent ObjectTemplate(IHtmlHelper htmlHelper) var viewBufferScope = serviceProvider.GetRequiredService(); var content = new HtmlContentBuilder(modelExplorer.Metadata.Properties.Count); - foreach (var propertyExplorer in modelExplorer.Properties) + foreach (var propertyExplorer in modelExplorer.PropertiesInternal.AsSpan()) { var propertyMetadata = propertyExplorer.Metadata; if (!ShouldShow(propertyExplorer, templateInfo)) diff --git a/src/Mvc/Mvc.ViewFeatures/src/DefaultEditorTemplates.cs b/src/Mvc/Mvc.ViewFeatures/src/DefaultEditorTemplates.cs index 847bd5d34e5a..b0676b1450db 100644 --- a/src/Mvc/Mvc.ViewFeatures/src/DefaultEditorTemplates.cs +++ b/src/Mvc/Mvc.ViewFeatures/src/DefaultEditorTemplates.cs @@ -255,7 +255,7 @@ public static IHtmlContent ObjectTemplate(IHtmlHelper htmlHelper) var viewBufferScope = serviceProvider.GetRequiredService(); var content = new HtmlContentBuilder(modelExplorer.Metadata.Properties.Count); - foreach (var propertyExplorer in modelExplorer.Properties) + foreach (var propertyExplorer in modelExplorer.PropertiesInternal.AsSpan()) { var propertyMetadata = propertyExplorer.Metadata; if (!ShouldShow(propertyExplorer, templateInfo)) @@ -482,6 +482,14 @@ public override void Write(char[] buffer, int index, int count) } } + public override void Write(ReadOnlySpan buffer) + { + if (!buffer.IsEmpty) + { + HasContent = true; + } + } + public override void Write(string value) { if (!string.IsNullOrEmpty(value)) @@ -540,7 +548,7 @@ public override void Encode(TextWriter output, string value, int startIndex, int return; } - output.Write(value.Substring(startIndex, characterCount)); + output.Write(value.AsSpan(startIndex, characterCount)); } public override unsafe int FindFirstCharacterToEncode(char* text, int textLength) diff --git a/src/Mvc/Mvc.ViewFeatures/src/ModelExplorer.cs b/src/Mvc/Mvc.ViewFeatures/src/ModelExplorer.cs index 9b38ea9d4ac9..77254c093b0c 100644 --- a/src/Mvc/Mvc.ViewFeatures/src/ModelExplorer.cs +++ b/src/Mvc/Mvc.ViewFeatures/src/ModelExplorer.cs @@ -199,7 +199,7 @@ public Type ModelType /// public IEnumerable Properties => PropertiesInternal; - private ModelExplorer[] PropertiesInternal + internal ModelExplorer[] PropertiesInternal { get { @@ -472,4 +472,4 @@ private ModelExplorer CreateExplorerForProperty( return new ModelExplorer(_metadataProvider, this, propertyMetadata, modelAccessor); } } -} \ No newline at end of file +} diff --git a/src/Mvc/Mvc.ViewFeatures/src/ModelExplorerExtensions.cs b/src/Mvc/Mvc.ViewFeatures/src/ModelExplorerExtensions.cs index c1eaa04c99ad..32e067f180db 100644 --- a/src/Mvc/Mvc.ViewFeatures/src/ModelExplorerExtensions.cs +++ b/src/Mvc/Mvc.ViewFeatures/src/ModelExplorerExtensions.cs @@ -4,7 +4,6 @@ using System; using System.Diagnostics; using System.Globalization; -using System.Linq; namespace Microsoft.AspNetCore.Mvc.ViewFeatures { @@ -68,12 +67,13 @@ public static string GetSimpleDisplayText(this ModelExplorer modelExplorer) return stringResult; } - var firstProperty = modelExplorer.Properties.FirstOrDefault(); - if (firstProperty == null) + if (modelExplorer.PropertiesInternal.Length == 0) { return string.Empty; } + var firstProperty = modelExplorer.PropertiesInternal[0]; + if (firstProperty.Model == null) { return firstProperty.Metadata.NullDisplayText; @@ -82,4 +82,4 @@ public static string GetSimpleDisplayText(this ModelExplorer modelExplorer) return Convert.ToString(firstProperty.Model, CultureInfo.CurrentCulture); } } -} \ No newline at end of file +} diff --git a/src/Mvc/MvcNoDeps.slnf b/src/Mvc/MvcNoDeps.slnf index 9438fbf3bab4..b393e2b59c4f 100644 --- a/src/Mvc/MvcNoDeps.slnf +++ b/src/Mvc/MvcNoDeps.slnf @@ -67,7 +67,7 @@ "src\\Mvc\\test\\WebSites\\SimpleWebSite\\SimpleWebSite.csproj", "src\\Mvc\\test\\WebSites\\TagHelpersWebSite\\TagHelpersWebSite.csproj", "src\\Mvc\\test\\WebSites\\VersioningWebSite\\VersioningWebSite.csproj", - "src\\Mvc\\test\\WebSites\\XmlFormattersWebSite\\XmlFormattersWebSite.csproj", + "src\\Mvc\\test\\WebSites\\XmlFormattersWebSite\\XmlFormattersWebSite.csproj" ] } } \ No newline at end of file diff --git a/src/Shared/ChunkingCookieManager/ChunkingCookieManager.cs b/src/Shared/ChunkingCookieManager/ChunkingCookieManager.cs index 9511580b8afd..05f51180b093 100644 --- a/src/Shared/ChunkingCookieManager/ChunkingCookieManager.cs +++ b/src/Shared/ChunkingCookieManager/ChunkingCookieManager.cs @@ -70,9 +70,7 @@ private static int ParseChunksCount(string? value) { if (value != null && value.StartsWith(ChunkCountPrefix, StringComparison.Ordinal)) { - var chunksCountString = value.Substring(ChunkCountPrefix.Length); - int chunksCount; - if (int.TryParse(chunksCountString, NumberStyles.None, CultureInfo.InvariantCulture, out chunksCount)) + if (int.TryParse(value.AsSpan(ChunkCountPrefix.Length), NumberStyles.None, CultureInfo.InvariantCulture, out var chunksCount)) { return chunksCount; } From 8877dbc9da90b2146fc78c0a223a976a7fcbf62c Mon Sep 17 00:00:00 2001 From: Pranav K Date: Sat, 3 Apr 2021 10:22:21 -0700 Subject: [PATCH 2/2] PR feedback --- .../ControllerBinderDelegateProvider.cs | 19 +++++++++++++------ .../Mvc.TagHelpers/src/AttributeMatcher.cs | 4 ++-- .../src/DefaultDisplayTemplates.cs | 2 +- .../src/DefaultEditorTemplates.cs | 17 ++++------------- 4 files changed, 20 insertions(+), 22 deletions(-) diff --git a/src/Mvc/Mvc.Core/src/Controllers/ControllerBinderDelegateProvider.cs b/src/Mvc/Mvc.Core/src/Controllers/ControllerBinderDelegateProvider.cs index 84164a688e51..3d1b0743e32d 100644 --- a/src/Mvc/Mvc.Core/src/Controllers/ControllerBinderDelegateProvider.cs +++ b/src/Mvc/Mvc.Core/src/Controllers/ControllerBinderDelegateProvider.cs @@ -57,8 +57,17 @@ internal static class ControllerBinderDelegateProvider return null; } - var parameters = actionDescriptor.Parameters as List ?? actionDescriptor.Parameters.ToList(); - var properties = actionDescriptor.BoundProperties as List ?? actionDescriptor.BoundProperties.ToList(); + var parameters = actionDescriptor.Parameters switch + { + List list => list.ToArray(), + _ => actionDescriptor.Parameters.ToArray() + }; + + var properties = actionDescriptor.BoundProperties switch + { + List list => list.ToArray(), + _ => actionDescriptor.BoundProperties.ToArray() + }; return Bind; @@ -72,8 +81,7 @@ async Task Bind(ControllerContext controllerContext, object controller, Dictiona Debug.Assert(valueProvider is not null); - var parameterCount = parameters.Count; - for (var i = 0; i < parameterCount; i++) + for (var i = 0; i < parameters.Length; i++) { var parameter = parameters[i]; var bindingInfo = parameterBindingInfo![i]; @@ -99,8 +107,7 @@ async Task Bind(ControllerContext controllerContext, object controller, Dictiona } } - var propertyCount = properties.Count; - for (var i = 0; i < propertyCount; i++) + for (var i = 0; i < properties.Length; i++) { var property = properties[i]; var bindingInfo = propertyBindingInfo![i]; diff --git a/src/Mvc/Mvc.TagHelpers/src/AttributeMatcher.cs b/src/Mvc/Mvc.TagHelpers/src/AttributeMatcher.cs index b52d98041707..a15f0895b073 100644 --- a/src/Mvc/Mvc.TagHelpers/src/AttributeMatcher.cs +++ b/src/Mvc/Mvc.TagHelpers/src/AttributeMatcher.cs @@ -49,11 +49,11 @@ public static bool TryDetermineMode( var allAttributes = context.AllAttributes; // Read interface .Count once rather than per iteration var allAttributesCount = allAttributes.Count; - foreach (var modeInfo in modeInfos.AsSpan()) + foreach (var modeInfo in modeInfos) { var requiredAttributes = modeInfo.Attributes; // If there are fewer attributes present than required, one or more of them must be missing. - if (allAttributesCount >= requiredAttributes.Length && + if (allAttributesCount >= requiredAttributes.Length && !HasMissingAttributes(allAttributes, requiredAttributes) && compare(result, modeInfo.Mode) <= 0) { diff --git a/src/Mvc/Mvc.ViewFeatures/src/DefaultDisplayTemplates.cs b/src/Mvc/Mvc.ViewFeatures/src/DefaultDisplayTemplates.cs index ac3b51999b67..1e7e0dbbd217 100644 --- a/src/Mvc/Mvc.ViewFeatures/src/DefaultDisplayTemplates.cs +++ b/src/Mvc/Mvc.ViewFeatures/src/DefaultDisplayTemplates.cs @@ -212,7 +212,7 @@ public static IHtmlContent ObjectTemplate(IHtmlHelper htmlHelper) var viewBufferScope = serviceProvider.GetRequiredService(); var content = new HtmlContentBuilder(modelExplorer.Metadata.Properties.Count); - foreach (var propertyExplorer in modelExplorer.PropertiesInternal.AsSpan()) + foreach (var propertyExplorer in modelExplorer.PropertiesInternal) { var propertyMetadata = propertyExplorer.Metadata; if (!ShouldShow(propertyExplorer, templateInfo)) diff --git a/src/Mvc/Mvc.ViewFeatures/src/DefaultEditorTemplates.cs b/src/Mvc/Mvc.ViewFeatures/src/DefaultEditorTemplates.cs index b0676b1450db..e9e695fb1d9c 100644 --- a/src/Mvc/Mvc.ViewFeatures/src/DefaultEditorTemplates.cs +++ b/src/Mvc/Mvc.ViewFeatures/src/DefaultEditorTemplates.cs @@ -255,7 +255,7 @@ public static IHtmlContent ObjectTemplate(IHtmlHelper htmlHelper) var viewBufferScope = serviceProvider.GetRequiredService(); var content = new HtmlContentBuilder(modelExplorer.Metadata.Properties.Count); - foreach (var propertyExplorer in modelExplorer.PropertiesInternal.AsSpan()) + foreach (var propertyExplorer in modelExplorer.PropertiesInternal) { var propertyMetadata = propertyExplorer.Metadata; if (!ShouldShow(propertyExplorer, templateInfo)) @@ -476,26 +476,17 @@ public override void Write(char value) public override void Write(char[] buffer, int index, int count) { - if (count > 0) - { - HasContent = true; - } + HasContent |= count > 0; } public override void Write(ReadOnlySpan buffer) { - if (!buffer.IsEmpty) - { - HasContent = true; - } + HasContent |= buffer.IsEmpty; } public override void Write(string value) { - if (!string.IsNullOrEmpty(value)) - { - HasContent = true; - } + HasContent |= !string.IsNullOrEmpty(value); } }