Skip to content

Commit

Permalink
housekeeping: invert ifs, use TryGetValue, remove unneeded `ToArr…
Browse files Browse the repository at this point in the history 
…ay` (#1619)
  • Loading branch information
@TimothyMakkison
TimothyMakkison committed Nov 26, 2023
1 parent 2dff048 commit 3cbe67a
Show file tree
Hide file tree
Showing 2 changed files with 125 additions and 135 deletions.
118 changes: 58 additions & 60 deletions Refit/FormValueMultimap.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -48,86 +48,84 @@ public FormValueMultimap(object source, RefitSettings settings)

lock (PropertyCache)
{
if (!PropertyCache.ContainsKey(type))
if (!PropertyCache.TryGetValue(type, out var properties))
{
PropertyCache[type] = GetProperties(type);
properties = GetProperties(type);
PropertyCache[type] = properties;
}

foreach (var property in PropertyCache[type])
foreach (var property in properties)
{
var value = property.GetValue(source, null);
if (value != null)
if (value == null)
continue;

var fieldName = GetFieldNameForProperty(property);

// see if there's a query attribute
var attrib = property.GetCustomAttribute<QueryAttribute>(true);

if (value is not IEnumerable enumerable)
{
var fieldName = GetFieldNameForProperty(property);
Add(
fieldName,
settings.FormUrlEncodedParameterFormatter.Format(value, attrib?.Format)
);
continue;
}

// see if there's a query attribute
var attrib = property.GetCustomAttribute<QueryAttribute>(true);
var collectionFormat =
attrib != null && attrib.IsCollectionFormatSpecified
? attrib.CollectionFormat
: settings.CollectionFormat;

if (value is IEnumerable enumerable)
{
var collectionFormat =
attrib != null && attrib.IsCollectionFormatSpecified
? attrib.CollectionFormat
: settings.CollectionFormat;
switch (collectionFormat)
{
case CollectionFormat.Multi:
foreach (var item in enumerable)
{
Add(
fieldName,
settings.FormUrlEncodedParameterFormatter.Format(
item,
attrib?.Format
)
);
}

switch (collectionFormat)
break;
case CollectionFormat.Csv:
case CollectionFormat.Ssv:
case CollectionFormat.Tsv:
case CollectionFormat.Pipes:
var delimiter = collectionFormat switch
{
case CollectionFormat.Multi:
foreach (var item in enumerable)
{
Add(
fieldName,
settings.FormUrlEncodedParameterFormatter.Format(
item,
attrib?.Format
)
);
}
break;
case CollectionFormat.Csv:
case CollectionFormat.Ssv:
case CollectionFormat.Tsv:
case CollectionFormat.Pipes:
var delimiter = collectionFormat switch
{
CollectionFormat.Csv => ",",
CollectionFormat.Ssv => " ",
CollectionFormat.Tsv => "\t",
_ => "|"
};

var formattedValues = enumerable
.Cast<object>()
.Select(
v =>
settings.FormUrlEncodedParameterFormatter.Format(
v,
attrib?.Format
)
);
Add(fieldName, string.Join(delimiter, formattedValues));
break;
default:
Add(
fieldName,
CollectionFormat.Csv => ",",
CollectionFormat.Ssv => " ",
CollectionFormat.Tsv => "\t",
_ => "|"
};

var formattedValues = enumerable
.Cast<object>()
.Select(
v =>
settings.FormUrlEncodedParameterFormatter.Format(
value,
v,
attrib?.Format
)
);
break;
}
}
else
{
);
Add(fieldName, string.Join(delimiter, formattedValues));
break;
default:
Add(
fieldName,
settings.FormUrlEncodedParameterFormatter.Format(
value,
attrib?.Format
)
);
}
break;
}
}
}
Expand Down
142 changes: 67 additions & 75 deletions Refit/RequestBuilderImplementation.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ public RequestBuilderImplementation(RefitSettings? refitSettings = null)

partial class RequestBuilderImplementation : IRequestBuilder
{
static readonly ISet<HttpMethod> BodylessMethods = new HashSet<HttpMethod>
static readonly HashSet<HttpMethod> BodylessMethods = new HashSet<HttpMethod>
{
HttpMethod.Get,
HttpMethod.Head
Expand Down Expand Up @@ -74,16 +74,17 @@ partial class RequestBuilderImplementation : IRequestBuilder
{
var attrs = methodInfo.GetCustomAttributes(true);
var hasHttpMethod = attrs.OfType<HttpMethodAttribute>().Any();
if (hasHttpMethod)
{
if (!methods.ContainsKey(methodInfo.Name))
{
methods.Add(methodInfo.Name, new List<RestMethodInfoInternal>());
}
if (!hasHttpMethod)
continue;

var restinfo = new RestMethodInfoInternal(interfaceType, methodInfo, settings);
methods[methodInfo.Name].Add(restinfo);
if (!methods.TryGetValue(methodInfo.Name, out var methodInfoInternals))
{
methodInfoInternals = new List<RestMethodInfoInternal>();
methods.Add(methodInfo.Name, methodInfoInternals);
}

var restinfo = new RestMethodInfoInternal(interfaceType, methodInfo, settings);
methodInfoInternals.Add(restinfo);
}
}

Expand All @@ -93,64 +94,62 @@ partial class RequestBuilderImplementation : IRequestBuilder
Type[]? genericArgumentTypes
)
{
if (interfaceHttpMethods.TryGetValue(key, out var httpMethods))
if (!interfaceHttpMethods.TryGetValue(key, out var httpMethods))
{
throw new ArgumentException(
"Method must be defined and have an HTTP Method attribute"
);
}

if (parameterTypes == null)
{
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 possibleMethodsList = httpMethods.Where(
method => method.MethodInfo.GetParameters().Length == parameterTypes.Length
);

var possibleMethods = possibleMethodsList.ToList();
// 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
);

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

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.Count == 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(
Expand Down Expand Up @@ -487,19 +486,12 @@ CancellationToken cancellationToken
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 @@ -511,7 +503,7 @@ CancellationToken cancellationToken

// 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,
Expand Down Expand Up @@ -657,9 +649,9 @@ bool paramsContainsCancellationToken
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 @@ -684,9 +676,9 @@ bool paramsContainsCancellationToken
{
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 @@ -706,7 +698,7 @@ bool paramsContainsCancellationToken
}
else
{
pattern = "{" + restMethod.ParameterMap[i].Name + "}";
pattern = "{" + parameterMapValue.Name + "}";
replacement = Uri.EscapeDataString(
settings.UrlParameterFormatter.Format(
param,
Expand Down Expand Up @@ -802,9 +794,9 @@ await content
}
// 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 Down Expand Up @@ -1000,7 +992,7 @@ await content
}
}
if (queryParamsToAdd.Any())
if (queryParamsToAdd.Count != 0)
{
var pairs = queryParamsToAdd
.Where(x => x.Key != null && x.Value != null)
Expand Down

0 comments on commit 3cbe67a

Please sign in to comment.