Skip to content

feat: use TryGetSingle instead of collection enumerable to lists. #1738

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Jun 25, 2024
Merged

feat: use TryGetSingle instead of collection enumerable to lists. #1738

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
94a72b4
feat: use `TryGetSingle` instead of collection enumerable to lists.
TimothyMakkison Jun 25, 2024
4f86827
Merge branch 'main' into improve_linq
ChrisPulman Jun 25, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
feat: use TryGetSingle instead of collection enumerable to lists.
  • Loading branch information
@TimothyMakkison
TimothyMakkison committed Jun 25, 2024
commit 94a72b4e4b2f3f75deb3bb6d953701579e78fe06
2 changes: 1 addition & 1 deletion Refit.Benchmarks/Program.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,5 +9,5 @@
{
BenchmarkRunner.Run<EndToEndBenchmark>();
// BenchmarkRunner.Run<StartupBenchmark>();
// BenchmarkRunner.Run<PerformanceBenchmarks>();
// BenchmarkRunner.Run<PerformanceBenchmark>();
}
27 changes: 27 additions & 0 deletions Refit/EnumerableExtensions.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
namespace Refit;

internal static class EnumerableExtensions
{
internal static EnumerablePeek TryGetSingle<T>(this IEnumerable<T> enumerable, out T? value)
{
value = default(T);
using var enumerator = enumerable.GetEnumerator();
var hasFirst = enumerator.MoveNext();
if (!hasFirst)
return EnumerablePeek.Empty;

value = enumerator.Current;
if (!enumerator.MoveNext())
return EnumerablePeek.Single;

value = default(T);
return EnumerablePeek.Many;
}
}

internal enum EnumerablePeek
{
Empty,
Single,
Many
}
136 changes: 66 additions & 70 deletions Refit/RestMethodInfo.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,29 +85,29 @@
DetermineIfResponseMustBeDisposed();

// Exclude cancellation token parameters from this list
var parameterList = methodInfo
var parameterArray = methodInfo
.GetParameters()
.Where(p => p.ParameterType != typeof(CancellationToken))
.ToList();
ParameterInfoMap = parameterList
.Where(static p => p.ParameterType != typeof(CancellationToken))
.ToArray();
ParameterInfoMap = parameterArray
.Select((parameter, index) => new { index, parameter })
.ToDictionary(x => x.index, x => x.parameter);
ParameterMap = BuildParameterMap(RelativePath, parameterList);
BodyParameterInfo = FindBodyParameter(parameterList, IsMultipart, hma.Method);
AuthorizeParameterInfo = FindAuthorizationParameter(parameterList);
ParameterMap = BuildParameterMap(RelativePath, parameterArray);
BodyParameterInfo = FindBodyParameter(parameterArray, IsMultipart, hma.Method);
AuthorizeParameterInfo = FindAuthorizationParameter(parameterArray);

Headers = ParseHeaders(methodInfo);
HeaderParameterMap = BuildHeaderParameterMap(parameterList);
HeaderParameterMap = BuildHeaderParameterMap(parameterArray);
HeaderCollectionParameterMap = RestMethodInfoInternal.BuildHeaderCollectionParameterMap(
parameterList
parameterArray
);
PropertyParameterMap = BuildRequestPropertyMap(parameterList);
PropertyParameterMap = BuildRequestPropertyMap(parameterArray);

// get names for multipart attachments
AttachmentNameMap = [];
if (IsMultipart)
{
for (var i = 0; i < parameterList.Count; i++)
for (var i = 0; i < parameterArray.Length; i++)
{
if (
ParameterMap.ContainsKey(i)
Expand All @@ -119,19 +119,19 @@
continue;
}

var attachmentName = GetAttachmentNameForParameter(parameterList[i]);
var attachmentName = GetAttachmentNameForParameter(parameterArray[i]);
if (attachmentName == null)
continue;

AttachmentNameMap[i] = Tuple.Create(
attachmentName,
GetUrlNameForParameter(parameterList[i])
GetUrlNameForParameter(parameterArray[i])
);
}
}

QueryParameterMap = [];
for (var i = 0; i < parameterList.Count; i++)
for (var i = 0; i < parameterArray.Length; i++)
{
if (
ParameterMap.ContainsKey(i)
Expand All @@ -145,21 +145,21 @@
continue;
}

QueryParameterMap.Add(i, GetUrlNameForParameter(parameterList[i]));
QueryParameterMap.Add(i, GetUrlNameForParameter(parameterArray[i]));
}

var ctParams = methodInfo
var ctParamEnumerable = methodInfo
.GetParameters()
.Where(p => p.ParameterType == typeof(CancellationToken))
.ToList();
if (ctParams.Count > 1)
.TryGetSingle(out var ctParam);
if (ctParamEnumerable == EnumerablePeek.Many)
{
throw new ArgumentException(
$"Argument list to method \"{methodInfo.Name}\" can only contain a single CancellationToken"
);
}

CancellationToken = ctParams.FirstOrDefault();
CancellationToken = ctParam;

IsApiResponse =
ReturnResultType!.GetTypeInfo().IsGenericType
Expand All @@ -170,31 +170,30 @@
|| ReturnResultType == typeof(IApiResponse);
}

static HashSet<int> BuildHeaderCollectionParameterMap(List<ParameterInfo> parameterList)
static HashSet<int> BuildHeaderCollectionParameterMap(ParameterInfo[] parameterArray)
{
var headerCollectionMap = new HashSet<int>();

for (var i = 0; i < parameterList.Count; i++)
for (var i = 0; i < parameterArray.Length; i++)
{
var param = parameterList[i];
var param = parameterArray[i];
var headerCollection = param
.GetCustomAttributes(true)
.OfType<HeaderCollectionAttribute>()
.FirstOrDefault();

if (headerCollection != null)
if (headerCollection == null) continue;

//opted for IDictionary<string, string> semantics here as opposed to the looser IEnumerable<KeyValuePair<string, string>> because IDictionary will enforce uniqueness of keys
if (param.ParameterType.IsAssignableFrom(typeof(IDictionary<string, string>)))
{
//opted for IDictionary<string, string> semantics here as opposed to the looser IEnumerable<KeyValuePair<string, string>> because IDictionary will enforce uniqueness of keys
if (param.ParameterType.IsAssignableFrom(typeof(IDictionary<string, string>)))
{
headerCollectionMap.Add(i);
}
else
{
throw new ArgumentException(
$"HeaderCollection parameter of type {param.ParameterType.Name} is not assignable from IDictionary<string, string>"
);
}
headerCollectionMap.Add(i);
}
else
{
throw new ArgumentException(
$"HeaderCollection parameter of type {param.ParameterType.Name} is not assignable from IDictionary<string, string>"
);
}
}

Expand All @@ -209,13 +208,13 @@
public RestMethodInfo ToRestMethodInfo() =>
new(Name, Type, MethodInfo, RelativePath, ReturnType);

static Dictionary<int, string> BuildRequestPropertyMap(List<ParameterInfo> parameterList)
static Dictionary<int, string> BuildRequestPropertyMap(ParameterInfo[] parameterArray)
{
var propertyMap = new Dictionary<int, string>();

for (var i = 0; i < parameterList.Count; i++)
for (var i = 0; i < parameterArray.Length; i++)
{
var param = parameterList[i];
var param = parameterArray[i];
var propertyAttribute = param
.GetCustomAttributes(true)
.OfType<PropertyAttribute>()
Expand All @@ -233,12 +232,11 @@
return propertyMap;
}

static PropertyInfo[] GetParameterProperties(ParameterInfo parameter)
static IEnumerable<PropertyInfo> GetParameterProperties(ParameterInfo parameter)
{
return parameter
.ParameterType.GetProperties(BindingFlags.Public | BindingFlags.Instance)
.Where(p => p.CanRead && p.GetMethod?.IsPublic == true)
.ToArray();
.Where(static p => p.CanRead && p.GetMethod?.IsPublic == true);
}

static void VerifyUrlPathIsSane(string relativePath)
Expand All @@ -254,7 +252,7 @@

static Dictionary<int, RestMethodParameterInfo> BuildParameterMap(
string relativePath,
List<ParameterInfo> parameterInfo
ParameterInfo[] parameterInfo
)
{
var ret = new Dictionary<int, RestMethodParameterInfo>();
Expand Down Expand Up @@ -311,11 +309,11 @@
};
#if NET6_0_OR_GREATER
ret.TryAdd(
parameterInfo.IndexOf(restMethodParameterInfo.ParameterInfo),
Array.IndexOf(parameterInfo, restMethodParameterInfo.ParameterInfo),
restMethodParameterInfo
);
#else
var idx = parameterInfo.IndexOf(restMethodParameterInfo.ParameterInfo);
var idx = Array.IndexOf(parameterInfo, restMethodParameterInfo.ParameterInfo);

Check warning on line 316 in Refit/RestMethodInfo.cs

View check run for this annotation

Codecov / codecov/patch

Refit/RestMethodInfo.cs#L316 

Added line #L316 was not covered by tests
if (!ret.ContainsKey(idx))
{
ret.Add(idx, restMethodParameterInfo);
Expand All @@ -329,7 +327,7 @@
)
{
var property = value1;
var parameterIndex = parameterInfo.IndexOf(property.Item1);
var parameterIndex = Array.IndexOf(parameterInfo, property.Item1);
//If we already have this parameter, add additional ParameterProperty
if (ret.TryGetValue(parameterIndex, out var value2))
{
Expand All @@ -355,12 +353,12 @@
);
#if NET6_0_OR_GREATER
ret.TryAdd(
parameterInfo.IndexOf(restMethodParameterInfo.ParameterInfo),
Array.IndexOf(parameterInfo, restMethodParameterInfo.ParameterInfo),
restMethodParameterInfo
);
#else
// Do the contains check
var idx = parameterInfo.IndexOf(restMethodParameterInfo.ParameterInfo);
var idx = Array.IndexOf(parameterInfo, restMethodParameterInfo.ParameterInfo);

Check warning on line 361 in Refit/RestMethodInfo.cs

View check run for this annotation

Codecov / codecov/patch

Refit/RestMethodInfo.cs#L361 

Added line #L361 was not covered by tests
if (!ret.ContainsKey(idx))
{
ret.Add(idx, restMethodParameterInfo);
Expand Down Expand Up @@ -421,7 +419,7 @@
// 2) POST/PUT/PATCH: Reference type other than string
// 3) If there are two reference types other than string, without the body attribute, throw

var bodyParams = parameterList
var bodyParamEnumerable = parameterList
.Select(
x =>
new
Expand All @@ -433,12 +431,12 @@
}
)
.Where(x => x.BodyAttribute != null)
.ToList();
.TryGetSingle(out var bodyParam);

// multipart requests may not contain a body, implicit or explicit
if (isMultipart)
{
if (bodyParams.Count > 0)
if (bodyParamEnumerable != EnumerablePeek.Empty)
{
throw new ArgumentException(
"Multipart requests may not contain a Body parameter"
Expand All @@ -447,19 +445,18 @@
return null;
}

if (bodyParams.Count > 1)
if (bodyParamEnumerable == EnumerablePeek.Many)
{
throw new ArgumentException("Only one parameter can be a Body parameter");
}

// #1, body attribute wins
if (bodyParams.Count == 1)
if (bodyParamEnumerable == EnumerablePeek.Single)
{
var ret = bodyParams[0];
return Tuple.Create(
ret.BodyAttribute!.SerializationMethod,
ret.BodyAttribute.Buffered ?? RefitSettings.Buffered,
parameterList.IndexOf(ret.Parameter)
bodyParam!.BodyAttribute!.SerializationMethod,
bodyParam.BodyAttribute.Buffered ?? RefitSettings.Buffered,
parameterList.IndexOf(bodyParam.Parameter)
);
}

Expand All @@ -475,7 +472,7 @@

// see if we're a post/put/patch
// explicitly skip [Query], [HeaderCollection], and [Property]-denoted params
var refParams = parameterList
var refParamEnumerable = parameterList
.Where(
pi =>
!pi.ParameterType.GetTypeInfo().IsValueType
Expand All @@ -484,22 +481,22 @@
&& pi.GetCustomAttribute<HeaderCollectionAttribute>() == null
&& pi.GetCustomAttribute<PropertyAttribute>() == null
)
.ToList();
.TryGetSingle(out var refParam);

// Check for rule #3
if (refParams.Count > 1)
if (refParamEnumerable == EnumerablePeek.Many)
{
throw new ArgumentException(
"Multiple complex types found. Specify one parameter as the body using BodyAttribute"
);
}

if (refParams.Count == 1)
if (refParamEnumerable == EnumerablePeek.Single)
{
return Tuple.Create(
BodySerializationMethod.Serialized,
RefitSettings.Buffered,
parameterList.IndexOf(refParams[0])
parameterList.IndexOf(refParam!)
);
}

Expand All @@ -508,7 +505,7 @@

static Tuple<string, int>? FindAuthorizationParameter(IList<ParameterInfo> parameterList)
{
var authorizeParams = parameterList
var authorizeParamsEnumerable = parameterList
.Select(
x =>
new
Expand All @@ -520,19 +517,18 @@
}
)
.Where(x => x.AuthorizeAttribute != null)
.ToList();
.TryGetSingle(out var authorizeParam);

if (authorizeParams.Count > 1)
if (authorizeParamsEnumerable == EnumerablePeek.Many)
{
throw new ArgumentException("Only one parameter can be an Authorize parameter");
}

if (authorizeParams.Count == 1)
if (authorizeParamsEnumerable == EnumerablePeek.Single)
{
var ret = authorizeParams[0];
return Tuple.Create(
ret.AuthorizeAttribute!.Scheme,
parameterList.IndexOf(ret.Parameter)
authorizeParam!.AuthorizeAttribute!.Scheme,
parameterList.IndexOf(authorizeParam.Parameter)
);
}

Expand Down Expand Up @@ -582,13 +578,13 @@
return ret;
}

static Dictionary<int, string> BuildHeaderParameterMap(List<ParameterInfo> parameterList)
static Dictionary<int, string> BuildHeaderParameterMap(ParameterInfo[] parameterArray)
{
var ret = new Dictionary<int, string>();

for (var i = 0; i < parameterList.Count; i++)
for (var i = 0; i < parameterArray.Length; i++)
{
var header = parameterList[i]
var header = parameterArray[i]
.GetCustomAttributes(true)
.OfType<HeaderAttribute>()
.Select(ha => ha.Header)
Expand Down
Loading