From fa3a57b84d3ffa90c744ca51fb8f6154b76ac245 Mon Sep 17 00:00:00 2001 From: Tim M <49349513+TimothyMakkison@users.noreply.github.com> Date: Sun, 3 Nov 2024 04:03:56 +0000 Subject: [PATCH] feat: calculate path substitutions in `RestMethodInfo` (#1897) Co-authored-by: Chris Pulman --- Refit.Tests/RestService.cs | 19 +++ Refit/RequestBuilderImplementation.cs | 157 ++++++++++-------- Refit/RestMethodInfo.cs | 219 +++++++++++++++----------- 3 files changed, 235 insertions(+), 160 deletions(-) diff --git a/Refit.Tests/RestService.cs b/Refit.Tests/RestService.cs index 27361ae5e..9d81b1185 100644 --- a/Refit.Tests/RestService.cs +++ b/Refit.Tests/RestService.cs @@ -450,6 +450,25 @@ await fixture.GetFooBars( mockHttp.VerifyNoOutstandingExpectation(); } + [Fact] + public async Task GetWithLongPathBoundObject() + { + var mockHttp = new MockHttpMessageHandler(); + var longPathString = string.Concat(Enumerable.Repeat("barNone", 1000)); + mockHttp + .Expect(HttpMethod.Get, $"http://foo/foos/12345/bar/{longPathString}") + .WithExactQueryString("") + .Respond("application/json", "Ok"); + + var settings = new RefitSettings { HttpMessageHandlerFactory = () => mockHttp }; + var fixture = RestService.For("http://foo", settings); + + await fixture.GetFooBars( + new PathBoundObject() { SomeProperty = 12345, SomeProperty2 = longPathString } + ); + mockHttp.VerifyNoOutstandingExpectation(); + } + [Fact] public async Task GetWithPathBoundObjectDifferentCasing() { diff --git a/Refit/RequestBuilderImplementation.cs b/Refit/RequestBuilderImplementation.cs index 99258a8e2..c24422b61 100644 --- a/Refit/RequestBuilderImplementation.cs +++ b/Refit/RequestBuilderImplementation.cs @@ -1,9 +1,9 @@ using System.Collections; using System.Collections.Concurrent; +using System.Diagnostics; using System.Net.Http; using System.Reflection; using System.Text; -using System.Text.RegularExpressions; using System.Web; namespace Refit @@ -14,6 +14,7 @@ class RequestBuilderImplementation(RefitSettings? refitSettings = null) partial class RequestBuilderImplementation : IRequestBuilder { + private const int StackallocThreshold = 512; static readonly QueryAttribute DefaultQueryAttribute = new (); static readonly Uri BaseUri = new ("http://api"); readonly Dictionary> interfaceHttpMethods; @@ -645,8 +646,6 @@ bool paramsContainsCancellationToken ret.Content = multiPartContent; } - var urlTarget = - (basePath == "/" ? string.Empty : basePath) + restMethod.RelativePath; var queryParamsToAdd = new List>(); var headersToAdd = restMethod.Headers.Count > 0 ? new Dictionary(restMethod.Headers) @@ -662,14 +661,10 @@ bool paramsContainsCancellationToken if (restMethod.ParameterMap.TryGetValue(i, out var parameterMapValue)) { parameterInfo = parameterMapValue; - if (parameterInfo.IsObjectPropertyParameter) + if (!parameterInfo.IsObjectPropertyParameter) { - urlTarget = AddObjectParametersToUrl(parameterInfo, param, urlTarget); - //don't continue here as we want it to fall through so any parameters on this object not bound here get passed as query parameters - } - else - { - urlTarget = AddValueParameterToUrl(restMethod, parameterMapValue, param, i, urlTarget); + // mark parameter mapped if not an object + // we want objects to fall through so any parameters on this object not bound here get passed as query parameters isParameterMappedToRequest = true; } } @@ -758,6 +753,8 @@ bool paramsContainsCancellationToken // NB: The URI methods in .NET are dumb. Also, we do this // UriBuilder business so that we preserve any hardcoded query // parameters as well as add the parameterized ones. + var urlTarget = BuildRelativePath(basePath, restMethod, paramList); + var uri = new UriBuilder(new Uri(BaseUri, urlTarget)); ParseExistingQueryString(uri, queryParamsToAdd); @@ -778,72 +775,102 @@ bool paramsContainsCancellationToken }; } - string AddObjectParametersToUrl(RestMethodParameterInfo parameterInfo, object param, string urlTarget) + string BuildRelativePath(string basePath, RestMethodInfoInternal restMethod, object[] paramList) { - foreach (var propertyInfo in parameterInfo.ParameterProperties) + basePath = basePath == "/" ? string.Empty : basePath; + var pathFragments = restMethod.FragmentPath; + if (pathFragments.Count == 0) { - var propertyObject = propertyInfo.PropertyInfo.GetValue(param); - urlTarget = Regex.Replace( - urlTarget, - "{" + propertyInfo.Name + "}", - Uri.EscapeDataString( - settings.UrlParameterFormatter.Format( - propertyObject, - propertyInfo.PropertyInfo, - propertyInfo.PropertyInfo.PropertyType - ) ?? string.Empty - ), - RegexOptions.IgnoreCase | RegexOptions.CultureInvariant - ); + return basePath; + } + if (string.IsNullOrEmpty(basePath) && pathFragments.Count == 1) + { + Debug.Assert(pathFragments[0].IsConstant); + return pathFragments[0].Value!; } - return urlTarget; +#pragma warning disable CA2000 + var vsb = new ValueStringBuilder(stackalloc char[StackallocThreshold]); +#pragma warning restore CA2000 + vsb.Append(basePath); + + foreach (var fragment in pathFragments) + { + AppendPathFragmentValue(ref vsb, restMethod, paramList, fragment); + } + + return vsb.ToString(); } - string AddValueParameterToUrl(RestMethodInfoInternal restMethod, RestMethodParameterInfo parameterMapValue, - object param, int i, string urlTarget) + void AppendPathFragmentValue(ref ValueStringBuilder vsb, RestMethodInfoInternal restMethod, object[] paramList, + ParameterFragment fragment) { - string pattern; - string replacement; - if (parameterMapValue.Type == ParameterType.RoundTripping) + if (fragment.IsConstant) { - pattern = $@"{{\*\*{parameterMapValue.Name}}}"; - var paramValue = (string)param; - replacement = string.Join( - "/", - paramValue - .Split('/') - .Select( - s => - Uri.EscapeDataString( - settings.UrlParameterFormatter.Format( - s, - restMethod.ParameterInfoArray[i], - restMethod.ParameterInfoArray[i].ParameterType - ) ?? string.Empty - ) - ) - ); + vsb.Append(fragment.Value!); + return; } - else + + var contains = restMethod.ParameterMap.TryGetValue(fragment.ArgumentIndex, out var parameterMapValue); + if (!contains || parameterMapValue is null) + throw new InvalidOperationException($"{restMethod.ParameterMap} should contain parameter."); + + if (fragment.IsObjectProperty) { - pattern = "{" + parameterMapValue.Name + "}"; - replacement = Uri.EscapeDataString( - settings.UrlParameterFormatter.Format( - param, - restMethod.ParameterInfoArray[i], - restMethod.ParameterInfoArray[i].ParameterType - ) ?? string.Empty - ); + var param = paramList[fragment.ArgumentIndex]; + var property = parameterMapValue.ParameterProperties[fragment.PropertyIndex]; + var propertyObject = property.PropertyInfo.GetValue(param); + + vsb.Append(Uri.EscapeDataString(settings.UrlParameterFormatter.Format( + propertyObject, + property.PropertyInfo, + property.PropertyInfo.PropertyType + ) ?? string.Empty)); + return; } - urlTarget = Regex.Replace( - urlTarget, - pattern, - replacement, - RegexOptions.IgnoreCase | RegexOptions.CultureInvariant - ); - return urlTarget; + if (fragment.IsDynamicRoute) + { + var param = paramList[fragment.ArgumentIndex]; + + if (parameterMapValue.Type == ParameterType.Normal) + { + vsb.Append(Uri.EscapeDataString( + settings.UrlParameterFormatter.Format( + param, + restMethod.ParameterInfoArray[fragment.ArgumentIndex], + restMethod.ParameterInfoArray[fragment.ArgumentIndex].ParameterType + ) ?? string.Empty + )); + return; + } + + // If round tripping, split string up, format each segment and append to vsb. + Debug.Assert(parameterMapValue.Type == ParameterType.RoundTripping); + var paramValue = (string)param; + var split = paramValue.Split('/'); + + var firstSection = true; + foreach (var section in split) + { + if(!firstSection) + vsb.Append('/'); + + vsb.Append( + Uri.EscapeDataString( + settings.UrlParameterFormatter.Format( + section, + restMethod.ParameterInfoArray[fragment.ArgumentIndex], + restMethod.ParameterInfoArray[fragment.ArgumentIndex].ParameterType + ) ?? string.Empty + )); + firstSection = false; + } + + return; + } + + throw new ArgumentException($"{nameof(ParameterFragment)} is in an invalid form."); } void AddBodyToRequest(RestMethodInfoInternal restMethod, object param, HttpRequestMessage ret) @@ -1168,7 +1195,7 @@ static string CreateQueryString(List> queryParamsT { // Suppress warning as ValueStringBuilder.ToString calls Dispose() #pragma warning disable CA2000 - var vsb = new ValueStringBuilder(stackalloc char[512]); + var vsb = new ValueStringBuilder(stackalloc char[StackallocThreshold]); #pragma warning restore CA2000 var firstQuery = true; foreach (var queryParam in queryParamsToAdd) diff --git a/Refit/RestMethodInfo.cs b/Refit/RestMethodInfo.cs index 4e8eb095a..539f23a73 100644 --- a/Refit/RestMethodInfo.cs +++ b/Refit/RestMethodInfo.cs @@ -46,6 +46,7 @@ internal class RestMethodInfoInternal public Dictionary> AttachmentNameMap { get; set; } public ParameterInfo[] ParameterInfoArray { get; set; } public Dictionary ParameterMap { get; set; } + public List FragmentPath { get ; set ; } public Type ReturnType { get; set; } public Type ReturnResultType { get; set; } public Type DeserializedResultType { get; set; } @@ -53,7 +54,7 @@ internal class RestMethodInfoInternal public bool IsApiResponse { get; } public bool ShouldDisposeResponse { get; private set; } - static readonly Regex ParameterRegex = new(@"{(.*?)}"); + static readonly Regex ParameterRegex = new("{(([^/?\r\n])*?)}"); static readonly HttpMethod PatchMethod = new("PATCH"); #pragma warning disable CS8618 // Non-nullable field must contain a non-null value when exiting constructor. Consider declaring as nullable. @@ -90,7 +91,7 @@ public RestMethodInfoInternal( .GetParameters() .Where(static p => p.ParameterType != typeof(CancellationToken)) .ToArray(); - ParameterMap = BuildParameterMap(RelativePath, ParameterInfoArray); + (ParameterMap, FragmentPath) = BuildParameterMap(RelativePath, ParameterInfoArray); BodyParameterInfo = FindBodyParameter(ParameterInfoArray, IsMultipart, hma.Method); AuthorizeParameterInfo = FindAuthorizationParameter(ParameterInfoArray); @@ -267,7 +268,7 @@ static void VerifyUrlPathIsSane(string relativePath) ); } - static Dictionary BuildParameterMap( + static (Dictionary ret, List fragmentList) BuildParameterMap( string relativePath, ParameterInfo[] parameterInfo ) @@ -275,123 +276,140 @@ ParameterInfo[] parameterInfo var ret = new Dictionary(); // This section handles pattern matching in the URL. We also need it to add parameter key/values for any attribute with a [Query] - var parameterizedParts = relativePath - .Split('/', '?') - .SelectMany(x => ParameterRegex.Matches(x).Cast()) - .ToList(); + var parameterizedParts = ParameterRegex.Matches(relativePath).Cast().ToArray(); - if (parameterizedParts.Count > 0) + if (parameterizedParts.Length == 0) { - var paramValidationDict = parameterInfo.ToDictionary( - k => GetUrlNameForParameter(k).ToLowerInvariant(), - v => v - ); - //if the param is an lets make a dictionary for all it's potential parameters - var objectParamValidationDict = parameterInfo - .Where(x => x.ParameterType.GetTypeInfo().IsClass) - .SelectMany(x => GetParameterProperties(x).Select(p => Tuple.Create(x, p))) - .GroupBy( - i => $"{i.Item1.Name}.{GetUrlNameForProperty(i.Item2)}".ToLowerInvariant() - ) - .ToDictionary(k => k.Key, v => v.First()); - foreach (var match in parameterizedParts) + if(string.IsNullOrEmpty(relativePath)) + return (ret, []); + + return (ret, [ParameterFragment.Constant(relativePath)]); + } + + var paramValidationDict = parameterInfo.ToDictionary( + k => GetUrlNameForParameter(k).ToLowerInvariant(), + v => v + ); + //if the param is an lets make a dictionary for all it's potential parameters + var objectParamValidationDict = parameterInfo + .Where(x => x.ParameterType.GetTypeInfo().IsClass) + .SelectMany(x => GetParameterProperties(x).Select(p => Tuple.Create(x, p))) + .GroupBy( + i => $"{i.Item1.Name}.{GetUrlNameForProperty(i.Item2)}".ToLowerInvariant() + ) + .ToDictionary(k => k.Key, v => v.First()); + + var fragmentList = new List(); + var index = 0; + + foreach (var match in parameterizedParts) + { + // Add constant value from given http path + if (match.Index != index) + { + fragmentList.Add(ParameterFragment.Constant(relativePath.Substring(index, match.Index - index))); + } + index = match.Index + match.Length; + + var rawName = match.Groups[1].Value.ToLowerInvariant(); + var isRoundTripping = rawName.StartsWith("**"); + var name = isRoundTripping ? rawName.Substring(2) : rawName; + + if (paramValidationDict.TryGetValue(name, out var value)) //if it's a standard parameter { - var rawName = match.Groups[1].Value.ToLowerInvariant(); - var isRoundTripping = rawName.StartsWith("**"); - string name; - if (isRoundTripping) + var paramType = value.ParameterType; + if (isRoundTripping && paramType != typeof(string)) { - name = rawName.Substring(2); + throw new ArgumentException( + $"URL {relativePath} has round-tripping parameter {rawName}, but the type of matched method parameter is {paramType.FullName}. It must be a string." + ); } - else + var parameterType = isRoundTripping + ? ParameterType.RoundTripping + : ParameterType.Normal; + var restMethodParameterInfo = new RestMethodParameterInfo(name, value) { - name = rawName; - } + Type = parameterType + }; - if (paramValidationDict.TryGetValue(name, out var value)) //if it's a standard parameter + var parameterIndex = Array.IndexOf(parameterInfo, restMethodParameterInfo.ParameterInfo); + fragmentList.Add(ParameterFragment.Dynamic(parameterIndex)); +#if NET6_0_OR_GREATER + ret.TryAdd( + parameterIndex, + restMethodParameterInfo + ); +#else + if (!ret.ContainsKey(parameterIndex)) + { + ret.Add(parameterIndex, restMethodParameterInfo); + } +#endif + } + //else if it's a property on a object parameter + else if ( + objectParamValidationDict.TryGetValue(name, out var value1) + && !isRoundTripping + ) + { + var property = value1; + var parameterIndex = Array.IndexOf(parameterInfo, property.Item1); + //If we already have this parameter, add additional ParameterProperty + if (ret.TryGetValue(parameterIndex, out var value2)) { - var paramType = value.ParameterType; - if (isRoundTripping && paramType != typeof(string)) + if (!value2.IsObjectPropertyParameter) { throw new ArgumentException( - $"URL {relativePath} has round-tripping parameter {rawName}, but the type of matched method parameter is {paramType.FullName}. It must be a string." + $"Parameter {property.Item1.Name} matches both a parameter and nested parameter on a parameter object" ); } - var parameterType = isRoundTripping - ? ParameterType.RoundTripping - : ParameterType.Normal; - var restMethodParameterInfo = new RestMethodParameterInfo(name, value) - { - Type = parameterType - }; + + value2.ParameterProperties.Add( + new RestMethodParameterProperty(name, property.Item2) + ); + fragmentList.Add(ParameterFragment.DynamicObject(parameterIndex, value2.ParameterProperties.Count - 1)); + } + else + { + var restMethodParameterInfo = new RestMethodParameterInfo( + true, + property.Item1 + ); + restMethodParameterInfo.ParameterProperties.Add( + new RestMethodParameterProperty(name, property.Item2) + ); + + var idx = Array.IndexOf(parameterInfo, restMethodParameterInfo.ParameterInfo); + fragmentList.Add(ParameterFragment.DynamicObject(idx, 0)); #if NET6_0_OR_GREATER ret.TryAdd( - Array.IndexOf(parameterInfo, restMethodParameterInfo.ParameterInfo), + idx, restMethodParameterInfo ); #else - var idx = Array.IndexOf(parameterInfo, restMethodParameterInfo.ParameterInfo); + // Do the contains check if (!ret.ContainsKey(idx)) { ret.Add(idx, restMethodParameterInfo); } #endif } - //else if it's a property on a object parameter - else if ( - objectParamValidationDict.TryGetValue(name, out var value1) - && !isRoundTripping - ) - { - var property = value1; - var parameterIndex = Array.IndexOf(parameterInfo, property.Item1); - //If we already have this parameter, add additional ParameterProperty - if (ret.TryGetValue(parameterIndex, out var value2)) - { - if (!value2.IsObjectPropertyParameter) - { - throw new ArgumentException( - $"Parameter {property.Item1.Name} matches both a parameter and nested parameter on a parameter object" - ); - } - - value2.ParameterProperties.Add( - new RestMethodParameterProperty(name, property.Item2) - ); - } - else - { - var restMethodParameterInfo = new RestMethodParameterInfo( - true, - property.Item1 - ); - restMethodParameterInfo.ParameterProperties.Add( - new RestMethodParameterProperty(name, property.Item2) - ); -#if NET6_0_OR_GREATER - ret.TryAdd( - Array.IndexOf(parameterInfo, restMethodParameterInfo.ParameterInfo), - restMethodParameterInfo - ); -#else - // Do the contains check - var idx = Array.IndexOf(parameterInfo, restMethodParameterInfo.ParameterInfo); - if (!ret.ContainsKey(idx)) - { - ret.Add(idx, restMethodParameterInfo); - } -#endif - } - } - else - { - throw new ArgumentException( - $"URL {relativePath} has parameter {rawName}, but no method parameter matches" - ); - } } + else + { + throw new ArgumentException( + $"URL {relativePath} has parameter {rawName}, but no method parameter matches" + ); + } + } + + // add trailing string + if (index < relativePath.Length - 1) + { + var trailingConstant = relativePath.Substring(index, relativePath.Length - index); + fragmentList.Add(ParameterFragment.Constant(trailingConstant)); } - return ret; + return (ret, fragmentList); } static string GetUrlNameForParameter(ParameterInfo paramInfo) @@ -670,4 +688,15 @@ void DetermineIfResponseMustBeDisposed() && DeserializedResultType != typeof(Stream); } } + + internal record struct ParameterFragment(string? Value, int ArgumentIndex, int PropertyIndex) + { + public bool IsConstant => Value != null; + public bool IsDynamicRoute => ArgumentIndex >= 0 && PropertyIndex < 0; + public bool IsObjectProperty => ArgumentIndex >= 0 && PropertyIndex >= 0; + + public static ParameterFragment Constant(string value) => new (value, -1, -1); + public static ParameterFragment Dynamic(int index) => new (null, index, -1); + public static ParameterFragment DynamicObject(int index, int propertyIndex) => new (null, index, propertyIndex); + } }