feat: calculate path substitutions in RestMethodInfo (#1897)

Co-authored-by: Chris Pulman <[email protected]>

Author: TimothyMakkison

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<IApiBindPathToObject>("http://foo", settings);
+
+        await fixture.GetFooBars(
+            new PathBoundObject() { SomeProperty = 12345, SomeProperty2 = longPathString }
+        );
+        mockHttp.VerifyNoOutstandingExpectation();
+    }
+
     [Fact]
     public async Task GetWithPathBoundObjectDifferentCasing()
     {

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<TApi>(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<string, List<RestMethodInfoInternal>> interfaceHttpMethods;
@@ -645,8 +646,6 @@ bool paramsContainsCancellationToken
                     ret.Content = multiPartContent;
                 }
 
-                var urlTarget =
-                    (basePath == "/" ? string.Empty : basePath) + restMethod.RelativePath;
                 var queryParamsToAdd = new List<KeyValuePair<string, string?>>();
                 var headersToAdd = restMethod.Headers.Count > 0 ?
                     new Dictionary<string, string?>(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<KeyValuePair<string, string?>> 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)