Merge pull request #848 from FolkCoder/handle-query-inner-collections

handle serialization for query objects with IEnumerable properties
reactiveui · Mar 19, 2020 · eb49e2b · eb49e2b
2 parents c87284e + 466854b
commit eb49e2b
Show file tree

Hide file tree

Showing 2 changed files with 58 additions and 33 deletions.
diff --git a/Refit.Tests/RequestBuilder.cs b/Refit.Tests/RequestBuilder.cs
@@ -129,6 +129,8 @@ public class ComplexQueryObject
         public string TestAlias1 {get; set;}
 
         public string TestAlias2 {get; set;}
+
+        public IEnumerable<int> TestCollection { get; set; }
     }
 
 
@@ -221,6 +223,19 @@ public void PostWithObjectQueryParameterHasCorrectQuerystring()
             Assert.Equal("/foo?test-query-alias=one&TestAlias2=two", uri.PathAndQuery);
         }
 
+        [Fact]
+        public void ObjectQueryParameterWithInnerCollectionHasCorrectQuerystring()
+        {
+            var fixture = new RequestBuilderImplementation<IDummyHttpApi>();
+            var factory = fixture.BuildRequestFactoryForMethod(nameof(IDummyHttpApi.ComplexTypeQueryWithInnerCollection));
+
+            var param = new ComplexQueryObject { TestCollection = new[] { 1, 2, 3 } };
+            var output = factory(new object[] { param });
+            var uri = new Uri(new Uri("http://api"), output.RequestUri);
+
+            Assert.Equal("/foo?TestCollection=1%2C2%2C3", uri.PathAndQuery);
+        }
+
         [Fact]
         public void MultipleQueryAttributesWithNulls()
         {
@@ -709,6 +724,9 @@ Task<ApiResponse<object>> UploadFile(int companyId,
         [Post("/foo")]
         Task PostWithComplexTypeQuery([Query]ComplexQueryObject queryParams);
 
+        [Get("/foo")]
+        Task ComplexTypeQueryWithInnerCollection([Query]ComplexQueryObject queryParams);
+
         [Get("/api/{obj.someProperty}")]
         Task QueryWithOptionalParametersPathBoundObject(PathBoundObject obj, [Query]string text = null, [Query]int? optionalId = null, [Query(CollectionFormat = CollectionFormat.Multi)]string[] filters = null);
 

diff --git a/Refit/RequestBuilderImplementation.cs b/Refit/RequestBuilderImplementation.cs
@@ -592,45 +592,14 @@ Func<object[], Task<HttpRequestMessage>> BuildRequestFactoryForMethod(RestMethod
                         var attr = queryAttribute ?? new QueryAttribute();
                         if (DoNotConvertToQueryMap(param))
                         {
-                            if (!(param is string) && param is IEnumerable paramValues)
-                            {
-                                switch (attr.CollectionFormat)
-                                {
-                                    case CollectionFormat.Multi:
-                                        foreach (var paramValue in paramValues)
-                                        {
-                                            queryParamsToAdd.Add(new KeyValuePair<string, string>(
-                                                restMethod.QueryParameterMap[i],
-                                                settings.UrlParameterFormatter.Format(paramValue, restMethod.ParameterInfoMap[i], restMethod.ParameterInfoMap[i].ParameterType)));
-                                        }
-                                        continue;
-                                    default:
-                                        var delimiter = attr.CollectionFormat == CollectionFormat.Ssv ? " "
-                                            : attr.CollectionFormat == CollectionFormat.Tsv ? "\t"
-                                            : attr.CollectionFormat == CollectionFormat.Pipes ? "|"
-                                            : ",";
-
-                                        // Missing a "default" clause was preventing the collection from serializing at all, as it was hitting "continue" thus causing an off-by-one error
-
-                                        var formattedValues = paramValues
-                                            .Cast<object>()
-                                            .Select(v => settings.UrlParameterFormatter.Format(v, restMethod.ParameterInfoMap[i], restMethod.ParameterInfoMap[i].ParameterType));
-
-                                        queryParamsToAdd.Add(new KeyValuePair<string, string>(
-                                            restMethod.QueryParameterMap[i],
-                                            string.Join(delimiter, formattedValues)));
-                                        continue;
-                                }
-                            }
-
-                            queryParamsToAdd.Add(new KeyValuePair<string, string>(restMethod.QueryParameterMap[i], settings.UrlParameterFormatter.Format(param, restMethod.ParameterInfoMap[i], restMethod.ParameterInfoMap[i].ParameterType)));
+                            queryParamsToAdd.AddRange(ParseQueryParameter(param, restMethod.ParameterInfoMap[i], restMethod.QueryParameterMap[i], attr));
                         }
                         else
                         {
                             foreach (var kvp in BuildQueryMap(param, attr.Delimiter, parameterInfo))
                             {
                                 var path = !string.IsNullOrWhiteSpace(attr.Prefix) ? $"{attr.Prefix}{attr.Delimiter}{kvp.Key}" : kvp.Key;
-                                queryParamsToAdd.Add(new KeyValuePair<string, string>(path, settings.UrlParameterFormatter.Format(kvp.Value, restMethod.ParameterInfoMap[i], restMethod.ParameterInfoMap[i].ParameterType)));
+                                queryParamsToAdd.AddRange(ParseQueryParameter(kvp.Value, restMethod.ParameterInfoMap[i], path, attr));
                             }
                         }
 
@@ -714,6 +683,44 @@ Func<object[], Task<HttpRequestMessage>> BuildRequestFactoryForMethod(RestMethod
             };
         }
 
+        IEnumerable<KeyValuePair<string, string>> ParseQueryParameter(object param, ParameterInfo parameterInfo, string queryPath, QueryAttribute queryAttribute)
+        {
+            if (!(param is string) && param is IEnumerable paramValues)
+            {
+                switch (queryAttribute.CollectionFormat)
+                {
+                    case CollectionFormat.Multi:
+                        foreach (var paramValue in paramValues)
+                        {
+                            yield return new KeyValuePair<string, string>(
+                                queryPath,
+                                settings.UrlParameterFormatter.Format(paramValue, parameterInfo, parameterInfo.ParameterType));
+                        }
+
+                        break;
+
+                    default:
+                        var delimiter = queryAttribute.CollectionFormat == CollectionFormat.Ssv ? " "
+                            : queryAttribute.CollectionFormat == CollectionFormat.Tsv ? "\t"
+                            : queryAttribute.CollectionFormat == CollectionFormat.Pipes ? "|"
+                            : ",";
+
+                        // Missing a "default" clause was preventing the collection from serializing at all, as it was hitting "continue" thus causing an off-by-one error
+                        var formattedValues = paramValues
+                            .Cast<object>()
+                            .Select(v => settings.UrlParameterFormatter.Format(v, parameterInfo, parameterInfo.ParameterType));
+
+                        yield return new KeyValuePair<string, string>(queryPath, string.Join(delimiter, formattedValues));
+
+                        break;
+                }
+            }
+            else
+            {
+                yield return new KeyValuePair<string, string>(queryPath, settings.UrlParameterFormatter.Format(param, parameterInfo, parameterInfo.ParameterType));
+            }
+        }
+
         Func<HttpClient, object[], IObservable<T>> BuildRxFuncForMethod<T, TBody>(RestMethodInfo restMethod)
         {
             var taskFunc = BuildCancellableTaskFuncForMethod<T, TBody>(restMethod);