From 246d2add6fd49afdaf76bb5694f00d126a85212b Mon Sep 17 00:00:00 2001 From: Taylor Southwick Date: Tue, 7 Feb 2023 10:41:42 -0800 Subject: [PATCH] Refactor HttpBrowserCapability to support this[] --- .../SystemWebAdaptersExtensions.cs | 2 +- .../Adapters/IBrowserCapabilitiesFactory.cs | 11 +++ .../Adapters/IHttpBrowserCapabilityFeature.cs | 11 +++ .../BrowserCapabilitiesFactory.cs | 59 ++++++++++++- .../Configuration/HttpCapabilitiesBase.cs | 84 ++++++++++++++++--- .../Configuration/ParsedBrowserResult.cs | 62 -------------- .../Generated/ExcludedApis.txt | 3 + .../Generated/Ref.Standard.cs | 3 + .../HttpBrowserCapabilities.cs | 4 +- .../HttpRequest.cs | 20 +---- .../BrowserCapabilitiesFactoryTests.cs | 2 +- 11 files changed, 162 insertions(+), 99 deletions(-) create mode 100644 src/Microsoft.AspNetCore.SystemWebAdapters/Adapters/IBrowserCapabilitiesFactory.cs create mode 100644 src/Microsoft.AspNetCore.SystemWebAdapters/Adapters/IHttpBrowserCapabilityFeature.cs delete mode 100644 src/Microsoft.AspNetCore.SystemWebAdapters/Configuration/ParsedBrowserResult.cs diff --git a/src/Microsoft.AspNetCore.SystemWebAdapters.CoreServices/SystemWebAdaptersExtensions.cs b/src/Microsoft.AspNetCore.SystemWebAdapters.CoreServices/SystemWebAdaptersExtensions.cs index 7b564e8976..7990d2fc99 100644 --- a/src/Microsoft.AspNetCore.SystemWebAdapters.CoreServices/SystemWebAdaptersExtensions.cs +++ b/src/Microsoft.AspNetCore.SystemWebAdapters.CoreServices/SystemWebAdaptersExtensions.cs @@ -18,7 +18,7 @@ public static ISystemWebAdapterBuilder AddSystemWebAdapters(this IServiceCollect services.AddHttpContextAccessor(); services.AddSingleton(_ => HttpRuntimeFactory.Create()); services.AddSingleton(); - services.AddSingleton(); + services.AddSingleton(); services.AddTransient(); return new SystemWebAdapterBuilder(services) diff --git a/src/Microsoft.AspNetCore.SystemWebAdapters/Adapters/IBrowserCapabilitiesFactory.cs b/src/Microsoft.AspNetCore.SystemWebAdapters/Adapters/IBrowserCapabilitiesFactory.cs new file mode 100644 index 0000000000..1453d2a387 --- /dev/null +++ b/src/Microsoft.AspNetCore.SystemWebAdapters/Adapters/IBrowserCapabilitiesFactory.cs @@ -0,0 +1,11 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +namespace Microsoft.AspNetCore.SystemWebAdapters; + +#if NET6_0_OR_GREATER +public interface IBrowserCapabilitiesFactory +{ + IHttpBrowserCapabilityFeature Create(HttpRequestCore request); +} +#endif diff --git a/src/Microsoft.AspNetCore.SystemWebAdapters/Adapters/IHttpBrowserCapabilityFeature.cs b/src/Microsoft.AspNetCore.SystemWebAdapters/Adapters/IHttpBrowserCapabilityFeature.cs new file mode 100644 index 0000000000..af23530907 --- /dev/null +++ b/src/Microsoft.AspNetCore.SystemWebAdapters/Adapters/IHttpBrowserCapabilityFeature.cs @@ -0,0 +1,11 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +namespace Microsoft.AspNetCore.SystemWebAdapters; + +#if NET6_0_OR_GREATER +public interface IHttpBrowserCapabilityFeature +{ + string? this[string key] { get; } +} +#endif diff --git a/src/Microsoft.AspNetCore.SystemWebAdapters/Configuration/BrowserCapabilitiesFactory.cs b/src/Microsoft.AspNetCore.SystemWebAdapters/Configuration/BrowserCapabilitiesFactory.cs index b199b01d13..ce15cb8781 100644 --- a/src/Microsoft.AspNetCore.SystemWebAdapters/Configuration/BrowserCapabilitiesFactory.cs +++ b/src/Microsoft.AspNetCore.SystemWebAdapters/Configuration/BrowserCapabilitiesFactory.cs @@ -1,7 +1,12 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. +using System.Collections.Generic; +using System.Diagnostics; using System.Text.RegularExpressions; +using Microsoft.AspNetCore.SystemWebAdapters; +using Microsoft.Extensions.Caching.Memory; +using Microsoft.Extensions.DependencyInjection; namespace System.Web.Configuration; @@ -18,7 +23,7 @@ namespace System.Web.Configuration; /// to do custom stuff. These are not implemented in the framework and are potentially added by uses. For now, we don't /// support that pattern. /// -internal class BrowserCapabilitiesFactory +internal sealed class BrowserCapabilitiesFactory : IBrowserCapabilitiesFactory { // Func will return true if we want to end the processing of further funcs private readonly Func[] _processList; @@ -40,7 +45,30 @@ public BrowserCapabilitiesFactory() }; } - public ParsedBrowserResult Process(string userAgent) + IHttpBrowserCapabilityFeature IBrowserCapabilitiesFactory.Create(HttpRequestCore request) + { + var userAgent = request.Headers.UserAgent; + + if (string.IsNullOrWhiteSpace(userAgent)) + { + return EmptyBrowserFeatures.Instance; + } + else if (request.HttpContext.RequestServices.GetService() is { } cache) + { + return cache.GetOrCreate(userAgent, entry => + { + entry.SlidingExpiration = TimeSpan.FromMinutes(2); + + return Parse(userAgent); + }); + } + else + { + return Parse(userAgent); + } + } + + public IHttpBrowserCapabilityFeature Parse(string userAgent) { var dictionary = new ParsedBrowserResult(); @@ -621,7 +649,23 @@ private bool UcbrowserProcess(string userAgent, ParsedBrowserResult dictionary) return false; } - private struct RegexResult + private sealed class ParsedBrowserResult : Dictionary, IHttpBrowserCapabilityFeature + { + public ParsedBrowserResult() + : base(StringComparer.OrdinalIgnoreCase) + { + } + + [Conditional("NotNeededYet")] + [Diagnostics.CodeAnalysis.SuppressMessage("Performance", "CA1822:Mark members as static", Justification = "Retained in case we need the data later")] + public void AddBrowser(string browser) + { + } + + string? IHttpBrowserCapabilityFeature.this[string key] => TryGetValue(key, out var value) ? value : null; + } + + private readonly struct RegexResult { private readonly Match? _match; private readonly Match? _match2; @@ -655,7 +699,7 @@ public RegexResult(Match? match, Match? match2) } } - private class RegexWorker + private sealed class RegexWorker { private readonly Regex _regex; private readonly Regex? _regex2; @@ -673,4 +717,11 @@ public RegexWorker(string pattern, string? pattern2 = null) public RegexResult Process(string userAgent) => new(_regex?.Match(userAgent), _regex2?.Match(userAgent)); } + + private sealed class EmptyBrowserFeatures : IHttpBrowserCapabilityFeature + { + public static IHttpBrowserCapabilityFeature Instance { get; } = new EmptyBrowserFeatures(); + + public string? this[string key] => null; + } } diff --git a/src/Microsoft.AspNetCore.SystemWebAdapters/Configuration/HttpCapabilitiesBase.cs b/src/Microsoft.AspNetCore.SystemWebAdapters/Configuration/HttpCapabilitiesBase.cs index 2e648067c8..572d067a92 100644 --- a/src/Microsoft.AspNetCore.SystemWebAdapters/Configuration/HttpCapabilitiesBase.cs +++ b/src/Microsoft.AspNetCore.SystemWebAdapters/Configuration/HttpCapabilitiesBase.cs @@ -1,28 +1,92 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. +using System.Globalization; +using Microsoft.AspNetCore.Http.Features; +using Microsoft.AspNetCore.SystemWebAdapters; +using Microsoft.Extensions.DependencyInjection; + namespace System.Web.Configuration; public class HttpCapabilitiesBase { - private readonly ParsedBrowserResult _data; + private readonly HttpContextCore _context; + + private FeatureReference _capability; + + private protected HttpCapabilitiesBase(HttpContextCore context) + { + _context = context; + _capability = FeatureReference.Default; + } - private protected HttpCapabilitiesBase(BrowserCapabilitiesFactory factory, string userAgent) + private IHttpBrowserCapabilityFeature Capability { - _data = factory.Process(userAgent); + get + { + if (_capability.Fetch(_context.Features) is { } existing) + { + return existing; + } + + return _capability.Update(_context.Features, _context.RequestServices.GetRequiredService().Create(_context.Request)); + } } - public string? Browser => _data["browser"]; + public string? Browser => Capability["browser"]; + + public string? Version => Capability["version"]; + + public int MajorVersion => GetInt("majorversion"); + + public double MinorVersion => GetDouble("minorversion"); + + public string? Platform => Capability["platform"]; - public string? Version => _data["version"]; + public bool Crawler => GetBoolean("crawler"); - public int MajorVersion => _data.GetInt("majorversion"); + public string? Type => Capability["type"]; - public double MinorVersion => _data.GetDouble("minorversion"); + public string? PreferredRequestEncoding => Capability["preferredRequestEncoding"]; - public string? Platform => _data["platform"]; + public string? this[string key] => Capability[key]; - public bool Crawler => _data.GetBoolean("crawler"); + public bool IsMobileDevice => GetBoolean("isMobileDevice"); - public bool IsMobileDevice => _data.GetBoolean("isMobileDevice"); + private int GetInt(string key) => int.TryParse(Capability[key], NumberStyles.Integer, CultureInfo.InvariantCulture, out var result) ? result : throw new HttpUnhandledException($"Invalid string from browser capabilities '{key}'"); + + private bool GetBoolean(string key) => bool.TryParse(Capability[key], out var result) && result; + + private double GetDouble(string key) + { + const NumberStyles Style = NumberStyles.Float | NumberStyles.AllowDecimalPoint; + + var value = Capability[key]; + + if (value is not null) + { + if (double.TryParse(value, Style, CultureInfo.InvariantCulture, out var result)) + { + return result; + } + + // Handle if there's more than one decimal i.e. .4.1 -> .4 + var firstDecimal = value.IndexOf('.', StringComparison.Ordinal); + + if (firstDecimal != -1) + { + var nextDecimal = value.IndexOf('.', firstDecimal + 1); + + if (nextDecimal != -1) + { + if (double.TryParse(value.AsSpan()[..nextDecimal], Style, CultureInfo.InvariantCulture, out var result2)) + { + return result2; + } + } + } + } + + throw new HttpUnhandledException($"Invalid string from browser capabilities '{key}'"); + } } diff --git a/src/Microsoft.AspNetCore.SystemWebAdapters/Configuration/ParsedBrowserResult.cs b/src/Microsoft.AspNetCore.SystemWebAdapters/Configuration/ParsedBrowserResult.cs deleted file mode 100644 index 90b6f93e68..0000000000 --- a/src/Microsoft.AspNetCore.SystemWebAdapters/Configuration/ParsedBrowserResult.cs +++ /dev/null @@ -1,62 +0,0 @@ -// Licensed to the .NET Foundation under one or more agreements. -// The .NET Foundation licenses this file to you under the MIT license. - -using System.Collections.Generic; -using System.Diagnostics; -using System.Globalization; - -namespace System.Web.Configuration; - -internal class ParsedBrowserResult -{ - private readonly Dictionary _data = new(StringComparer.OrdinalIgnoreCase); - - [Conditional("NotNeededYet")] - [Diagnostics.CodeAnalysis.SuppressMessage("Performance", "CA1822:Mark members as static", Justification = "Retained in case we need the data later")] - public void AddBrowser(string browser) - { - } - - public string? this[string key] - { - get => _data.TryGetValue(key, out var value) ? value : null; - set => _data[key] = value; - } - - public int GetInt(string key) => int.TryParse(this[key], NumberStyles.Integer, CultureInfo.InvariantCulture, out var result) ? result : throw new HttpUnhandledException($"Invalid string from browser capabilities '{key}'"); - - public bool GetBoolean(string key) => bool.TryParse(this[key], out var result) && result; - - public double GetDouble(string key) - { - const NumberStyles Style = NumberStyles.Float | NumberStyles.AllowDecimalPoint; - - var value = this[key]; - - if (value is not null) - { - if (double.TryParse(value, Style, CultureInfo.InvariantCulture, out var result)) - { - return result; - } - - // Handle if there's more than one decimal i.e. .4.1 -> .4 - var firstDecimal = value.IndexOf('.', StringComparison.Ordinal); - - if (firstDecimal != -1) - { - var nextDecimal = value.IndexOf('.', firstDecimal + 1); - - if (nextDecimal != -1) - { - if (double.TryParse(value.AsSpan()[..nextDecimal], Style, CultureInfo.InvariantCulture, out var result2)) - { - return result2; - } - } - } - } - - throw new HttpUnhandledException($"Invalid string from browser capabilities '{key}'"); - } -} diff --git a/src/Microsoft.AspNetCore.SystemWebAdapters/Generated/ExcludedApis.txt b/src/Microsoft.AspNetCore.SystemWebAdapters/Generated/ExcludedApis.txt index 04825f6616..3f67613e8a 100644 --- a/src/Microsoft.AspNetCore.SystemWebAdapters/Generated/ExcludedApis.txt +++ b/src/Microsoft.AspNetCore.SystemWebAdapters/Generated/ExcludedApis.txt @@ -14,3 +14,6 @@ T:Microsoft.AspNetCore.SystemWebAdapters.SessionState.ISessionState # We manually type forward this only for .NET 4.7.2+ T:System.Web.SameSiteMode + +T:Microsoft.AspNetCore.SystemWebAdapters.IBrowserCapabilitiesFactory +T:Microsoft.AspNetCore.SystemWebAdapters.IHttpBrowserCapabilityFeature diff --git a/src/Microsoft.AspNetCore.SystemWebAdapters/Generated/Ref.Standard.cs b/src/Microsoft.AspNetCore.SystemWebAdapters/Generated/Ref.Standard.cs index 2363bc54d9..04333f3476 100644 --- a/src/Microsoft.AspNetCore.SystemWebAdapters/Generated/Ref.Standard.cs +++ b/src/Microsoft.AspNetCore.SystemWebAdapters/Generated/Ref.Standard.cs @@ -525,9 +525,12 @@ internal HttpCapabilitiesBase() { } public string Browser { get { throw new System.PlatformNotSupportedException("Only supported when running on ASP.NET Core or System.Web");} } public bool Crawler { get { throw new System.PlatformNotSupportedException("Only supported when running on ASP.NET Core or System.Web");} } public bool IsMobileDevice { get { throw new System.PlatformNotSupportedException("Only supported when running on ASP.NET Core or System.Web");} } + public string this[string key] { get { throw new System.PlatformNotSupportedException("Only supported when running on ASP.NET Core or System.Web");} } public int MajorVersion { get { throw new System.PlatformNotSupportedException("Only supported when running on ASP.NET Core or System.Web");} } public double MinorVersion { get { throw new System.PlatformNotSupportedException("Only supported when running on ASP.NET Core or System.Web");} } public string Platform { get { throw new System.PlatformNotSupportedException("Only supported when running on ASP.NET Core or System.Web");} } + public string PreferredRequestEncoding { get { throw new System.PlatformNotSupportedException("Only supported when running on ASP.NET Core or System.Web");} } + public string Type { get { throw new System.PlatformNotSupportedException("Only supported when running on ASP.NET Core or System.Web");} } public string Version { get { throw new System.PlatformNotSupportedException("Only supported when running on ASP.NET Core or System.Web");} } } } diff --git a/src/Microsoft.AspNetCore.SystemWebAdapters/HttpBrowserCapabilities.cs b/src/Microsoft.AspNetCore.SystemWebAdapters/HttpBrowserCapabilities.cs index 9247825484..04805c3b21 100644 --- a/src/Microsoft.AspNetCore.SystemWebAdapters/HttpBrowserCapabilities.cs +++ b/src/Microsoft.AspNetCore.SystemWebAdapters/HttpBrowserCapabilities.cs @@ -7,8 +7,8 @@ namespace System.Web; public class HttpBrowserCapabilities : HttpCapabilitiesBase { - internal HttpBrowserCapabilities(BrowserCapabilitiesFactory factory, string userAgent) - : base(factory, userAgent) + internal HttpBrowserCapabilities(HttpContextCore core) + : base(core) { } } diff --git a/src/Microsoft.AspNetCore.SystemWebAdapters/HttpRequest.cs b/src/Microsoft.AspNetCore.SystemWebAdapters/HttpRequest.cs index 6d32196086..94498af325 100644 --- a/src/Microsoft.AspNetCore.SystemWebAdapters/HttpRequest.cs +++ b/src/Microsoft.AspNetCore.SystemWebAdapters/HttpRequest.cs @@ -196,25 +196,7 @@ public bool IsLocal public string? UserHostName => _request.HttpContext.Connection.RemoteIpAddress?.ToString(); - public HttpBrowserCapabilities Browser - { - get - { - if (_browser is null) - { - var factory = _request.HttpContext.RequestServices.GetService(); - - if (factory is null) - { - throw new InvalidOperationException("Browser capabilities requires AddSystemWebAdapters() to be called on service collection"); - } - - _browser = new(factory, _request.Headers.UserAgent); - } - - return _browser; - } - } + public HttpBrowserCapabilities Browser => _browser ??= new(_request.HttpContext); public string? this[string key] => Params[key]; diff --git a/test/Microsoft.AspNetCore.SystemWebAdapters.Tests/Configuration/BrowserCapabilitiesFactoryTests.cs b/test/Microsoft.AspNetCore.SystemWebAdapters.Tests/Configuration/BrowserCapabilitiesFactoryTests.cs index 2327d7ff29..615c40f502 100644 --- a/test/Microsoft.AspNetCore.SystemWebAdapters.Tests/Configuration/BrowserCapabilitiesFactoryTests.cs +++ b/test/Microsoft.AspNetCore.SystemWebAdapters.Tests/Configuration/BrowserCapabilitiesFactoryTests.cs @@ -20,7 +20,7 @@ public void VerifyCapabilities(TestData data) var browserFactory = new BrowserCapabilitiesFactory(); // Act - var result = browserFactory.Process(data.UserAgent); + var result = browserFactory.Parse(data.UserAgent); // Assert Assert.Equal(data.Browser, result["browser"]);