Skip to content

Commit

Permalink
feat: refactor, invert ifs, use optimal methods (#1713)
Browse files Browse the repository at this point in the history
  • Loading branch information
@TimothyMakkison
TimothyMakkison committed Jun 10, 2024
1 parent 9c2caf3 commit 51ef445
Showing 1 changed file with 70 additions and 81 deletions.
151 changes: 70 additions & 81 deletions Refit/RequestBuilderImplementation.cs
View file
Original file line number Diff line number Diff line change
@@ -1,16 +1,9 @@
using System;
using System.Collections;
using System.Collections;
using System.Collections.Concurrent;
using System.Collections.Generic;
using System.ComponentModel.Design;
using System.IO;
using System.Linq;
using System.Net.Http;
using System.Reflection;
using System.Text;
using System.Text.RegularExpressions;
using System.Threading;
using System.Threading.Tasks;
using System.Web;

namespace Refit
Expand Down Expand Up @@ -85,51 +78,52 @@ void AddInterfaceHttpMethods(Type interfaceType, Dictionary<string, List<RestMet

RestMethodInfoInternal FindMatchingRestMethodInfo(string key, Type[]? parameterTypes, Type[]? genericArgumentTypes)
{
if (interfaceHttpMethods.TryGetValue(key, out var httpMethods))
if (!interfaceHttpMethods.TryGetValue(key, out var httpMethods))
{
if (parameterTypes == null)
throw new ArgumentException("Method must be defined and have an HTTP Method attribute");
}

if (parameterTypes == null)
{
if (httpMethods.Count > 1)
{
if (httpMethods.Count > 1)
{
throw new ArgumentException($"MethodName exists more than once, '{nameof(parameterTypes)}' mut be defined");
}
return CloseGenericMethodIfNeeded(httpMethods[0], genericArgumentTypes);
throw new ArgumentException(
$"MethodName exists more than once, '{nameof(parameterTypes)}' mut be defined");
}

var isGeneric = genericArgumentTypes?.Length > 0;
return CloseGenericMethodIfNeeded(httpMethods[0], genericArgumentTypes);
}

var possibleMethodsList = httpMethods.Where(method => method.MethodInfo.GetParameters().Length == parameterTypes.Length);
var isGeneric = genericArgumentTypes?.Length > 0;

// If it's a generic method, add that filter
if (isGeneric)
possibleMethodsList = possibleMethodsList.Where(method => method.MethodInfo.IsGenericMethod && method.MethodInfo.GetGenericArguments().Length == genericArgumentTypes!.Length);
else // exclude generic methods
possibleMethodsList = possibleMethodsList.Where(method => !method.MethodInfo.IsGenericMethod);
var possibleMethodsCollection = httpMethods.Where(method =>
method.MethodInfo.GetParameters().Length == parameterTypes.Length);

var possibleMethods = possibleMethodsList.ToList();
// If it's a generic method, add that filter
if (isGeneric)
possibleMethodsCollection = possibleMethodsCollection.Where(method =>
method.MethodInfo.IsGenericMethod && method.MethodInfo.GetGenericArguments().Length ==
genericArgumentTypes!.Length);
else // exclude generic methods
possibleMethodsCollection = possibleMethodsCollection.Where(method => !method.MethodInfo.IsGenericMethod);

if (possibleMethods.Count == 1)
return CloseGenericMethodIfNeeded(possibleMethods[0], genericArgumentTypes);
var possibleMethods = possibleMethodsCollection.ToArray();

var parameterTypesArray = parameterTypes.ToArray();
foreach (var method in possibleMethods)
{
var match = method.MethodInfo.GetParameters()
.Select(p => p.ParameterType)
.SequenceEqual(parameterTypesArray);
if (match)
{
return CloseGenericMethodIfNeeded(method, genericArgumentTypes);
}
}
if (possibleMethods.Length == 1)
return CloseGenericMethodIfNeeded(possibleMethods[0], genericArgumentTypes);

throw new Exception("No suitable Method found...");
}
else
foreach (var method in possibleMethods)
{
throw new ArgumentException("Method must be defined and have an HTTP Method attribute");
var match = method.MethodInfo.GetParameters()
.Select(p => p.ParameterType)
.SequenceEqual(parameterTypes);
if (match)
{
return CloseGenericMethodIfNeeded(method, genericArgumentTypes);
}
}

throw new Exception("No suitable Method found...");
}

RestMethodInfoInternal CloseGenericMethodIfNeeded(RestMethodInfoInternal restMethodInfo, Type[]? genericArgumentTypes)
Expand Down Expand Up @@ -176,7 +170,6 @@ RestMethodInfoInternal CloseGenericMethodIfNeeded(RestMethodInfoInternal restMet

void AddMultipartItem(MultipartFormDataContent multiPartContent, string fileName, string parameterName, object itemValue)
{

if (itemValue is HttpContent content)
{
multiPartContent.Add(content);
Expand Down Expand Up @@ -373,15 +366,12 @@ void AddMultipartItem(MultipartFormDataContent multiPartContent, string fileName
if (obj == null)
continue;

if (parameterInfo != null)
//if we have a parameter info lets check it to make sure it isn't bound to the path
if (parameterInfo is { IsObjectPropertyParameter: true })
{
//if we have a parameter info lets check it to make sure it isn't bound to the path
if (parameterInfo.IsObjectPropertyParameter)
if (parameterInfo.ParameterProperties.Any(x => x.PropertyInfo == propertyInfo))
{
if (parameterInfo.ParameterProperties.Any(x => x.PropertyInfo == propertyInfo))
{
continue;
}
continue;
}
}

Expand All @@ -390,10 +380,9 @@ void AddMultipartItem(MultipartFormDataContent multiPartContent, string fileName
var aliasAttribute = propertyInfo.GetCustomAttribute<AliasAsAttribute>();
key = aliasAttribute?.Name ?? settings.UrlParameterKeyFormatter.Format(key);


// Look to see if the property has a Query attribute, and if so, format it accordingly
var queryAttribute = propertyInfo.GetCustomAttribute<QueryAttribute>();
if (queryAttribute != null && queryAttribute.Format != null)
if (queryAttribute is { Format: not null })
{
obj = settings.FormUrlEncodedParameterFormatter.Format(obj, queryAttribute.Format);
}
Expand Down Expand Up @@ -507,9 +496,9 @@ void AddMultipartItem(MultipartFormDataContent multiPartContent, string fileName
var isParameterMappedToRequest = false;
var param = paramList[i];
// if part of REST resource URL, substitute it in
if (restMethod.ParameterMap.ContainsKey(i))
if (restMethod.ParameterMap.TryGetValue(i, out var parameterMapValue))
{
parameterInfo = restMethod.ParameterMap[i];
parameterInfo = parameterMapValue;
if (parameterInfo.IsObjectPropertyParameter)
{
foreach (var propertyInfo in parameterInfo.ParameterProperties)
Expand All @@ -529,9 +518,9 @@ void AddMultipartItem(MultipartFormDataContent multiPartContent, string fileName
{
string pattern;
string replacement;
if (restMethod.ParameterMap[i].Type == ParameterType.RoundTripping)
if (parameterMapValue.Type == ParameterType.RoundTripping)
{
pattern = $@"{{\*\*{restMethod.ParameterMap[i].Name}}}";
pattern = $@"{{\*\*{parameterMapValue.Name}}}";
var paramValue = (string)param;
replacement = string.Join(
"/",
Expand All @@ -545,7 +534,7 @@ void AddMultipartItem(MultipartFormDataContent multiPartContent, string fileName
}
else
{
pattern = "{" + restMethod.ParameterMap[i].Name + "}";
pattern = "{" + parameterMapValue.Name + "}";
replacement = Uri.EscapeDataString(settings.UrlParameterFormatter
.Format(param, restMethod.ParameterInfoMap[i], restMethod.ParameterInfoMap[i].ParameterType) ?? string.Empty);
}
Expand Down Expand Up @@ -618,9 +607,9 @@ void AddMultipartItem(MultipartFormDataContent multiPartContent, string fileName
}
// if header, add to request headers
if (restMethod.HeaderParameterMap.ContainsKey(i))
if (restMethod.HeaderParameterMap.TryGetValue(i, out var headerParameterValue))
{
headersToAdd[restMethod.HeaderParameterMap[i]] = param?.ToString();
headersToAdd[headerParameterValue] = param?.ToString();
isParameterMappedToRequest = true;
}
Expand All @@ -645,9 +634,9 @@ void AddMultipartItem(MultipartFormDataContent multiPartContent, string fileName
}
//if property, add to populate into HttpRequestMessage.Properties
if (restMethod.PropertyParameterMap.ContainsKey(i))
if (restMethod.PropertyParameterMap.TryGetValue(i, out var propertyParameter))
{
propertiesToAdd[restMethod.PropertyParameterMap[i]] = param;
propertiesToAdd[propertyParameter] = param;
isParameterMappedToRequest = true;
}
Expand Down Expand Up @@ -759,7 +748,7 @@ void AddMultipartItem(MultipartFormDataContent multiPartContent, string fileName
#else
ret.Properties[HttpRequestMessageOptions.InterfaceType] = TargetType;
ret.Properties[HttpRequestMessageOptions.RestMethodInfo] = restMethod.ToRestMethodInfo();
#endif
#endif
// NB: The URI methods in .NET are dumb. Also, we do this
// UriBuilder business so that we preserve any hardcoded query
Expand Down Expand Up @@ -920,33 +909,33 @@ static bool DoNotConvertToQueryMap(object? value)

var type = value.GetType();

bool ShouldReturn() => type == typeof(string) ||
type == typeof(bool) ||
type == typeof(char) ||
typeof(IFormattable).IsAssignableFrom(type) ||
type == typeof(Uri);

// Bail out early & match string
if (ShouldReturn())
if (ShouldReturn(type))
return true;

// Get the element type for enumerables
if (value is IEnumerable enu)
{
var ienu = typeof(IEnumerable<>);
// We don't want to enumerate to get the type, so we'll just look for IEnumerable<T>
var intType = type.GetInterfaces()
.FirstOrDefault(i => i.GetTypeInfo().IsGenericType &&
i.GetGenericTypeDefinition() == ienu);
if (value is not IEnumerable)
return false;

if (intType != null)
{
type = intType.GetGenericArguments()[0];
}
// Get the element type for enumerables
var ienu = typeof(IEnumerable<>);
// We don't want to enumerate to get the type, so we'll just look for IEnumerable<T>
var intType = type.GetInterfaces()
.FirstOrDefault(
i => i.GetTypeInfo().IsGenericType && i.GetGenericTypeDefinition() == ienu
);

if (intType == null)
return false;

}
type = intType.GetGenericArguments()[0];
return ShouldReturn(type);

return ShouldReturn();
static bool ShouldReturn(Type type) =>
type == typeof(string)
|| type == typeof(bool)
|| type == typeof(char)
|| typeof(IFormattable).IsAssignableFrom(type)
|| type == typeof(Uri);
}

static void SetHeader(HttpRequestMessage request, string name, string? value)
Expand Down

0 comments on commit 51ef445

Please sign in to comment.