Skip to content
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.

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

feat: use ValueStringBuilder adding the query parameters #1719

Merged
merged 3 commits into from
Jun 24, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
45 changes: 36 additions & 9 deletions Refit/RequestBuilderImplementation.cs
Original file line number Diff line number Diff line change
Expand Up @@ -994,15 +994,7 @@ param as IDictionary<string, string>

if (queryParamsToAdd.Count != 0)
{
var pairs = queryParamsToAdd
.Where(x => x.Key != null && x.Value != null)
.Select(
x =>
Uri.EscapeDataString(x.Key)
+ "="
+ Uri.EscapeDataString(x.Value ?? string.Empty)
);
uri.Query = string.Join("&", pairs);
uri.Query = CreateQueryString(queryParamsToAdd);;
}
else
{
Expand Down Expand Up @@ -1108,6 +1100,41 @@ var value in ParseEnumerableQueryParameterValue(
}
}

static string CreateQueryString(List<KeyValuePair<string, string?>> queryParamsToAdd)
{
// Suppress warning as ValueStringBuilder.ToString calls Dispose()
#pragma warning disable CA2000
var vsb = new ValueStringBuilder(stackalloc char[512]);
#pragma warning restore CA2000
var firstQuery = true;
foreach (var queryParam in queryParamsToAdd)
{
if(queryParam is not { Key: not null, Value: not null })
continue;
if(!firstQuery)
{
// for all items after the first we add a & symbol
vsb.Append('&');
}
var key = Uri.EscapeDataString(queryParam.Key);
#if NET6_0_OR_GREATER
// if first query does not start with ? then prepend it
if (vsb.Length == 0 && key.Length > 0 && key[0] != '?')
{
// query starts with ?
vsb.Append('?');
}
#endif
vsb.Append(key);
vsb.Append('=');
vsb.Append(Uri.EscapeDataString(queryParam.Value ?? string.Empty));
if (firstQuery)
firstQuery = false;
}

return vsb.ToString();
}

Func<HttpClient, object[], IObservable<T?>> BuildRxFuncForMethod<T, TBody>(
RestMethodInfoInternal restMethod
)
Expand Down
312 changes: 312 additions & 0 deletions Refit/ValueStringBuilder.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,312 @@
using System.Buffers;
using System.Diagnostics;
using System.Runtime.CompilerServices;
using System.Runtime.InteropServices;

namespace Refit;

// From https://github/dotnet/runtime/blob/main/src/libraries/Common/src/System/Text/ValueStringBuilder.cs
internal ref struct ValueStringBuilder
{
private char[]? _arrayToReturnToPool;
private Span<char> _chars;
private int _pos;

public ValueStringBuilder(Span<char> initialBuffer)
{
_arrayToReturnToPool = null;
_chars = initialBuffer;
_pos = 0;
}

public ValueStringBuilder(int initialCapacity)
{
_arrayToReturnToPool = ArrayPool<char>.Shared.Rent(initialCapacity);
_chars = _arrayToReturnToPool;
_pos = 0;
}

Check warning on line 27 in Refit/ValueStringBuilder.cs

View check run for this annotation

Codecov / codecov/patch

Refit/ValueStringBuilder.cs#L24-L27

Added lines #L24 - L27 were not covered by tests

public int Length
{
get => _pos;
set
{
Debug.Assert(value >= 0);
Debug.Assert(value <= _chars.Length);
_pos = value;
}

Check warning on line 37 in Refit/ValueStringBuilder.cs

View check run for this annotation

Codecov / codecov/patch

Refit/ValueStringBuilder.cs#L36-L37

Added lines #L36 - L37 were not covered by tests
}

public int Capacity => _chars.Length;

Check warning on line 40 in Refit/ValueStringBuilder.cs

View check run for this annotation

Codecov / codecov/patch

Refit/ValueStringBuilder.cs#L40

Added line #L40 was not covered by tests

public void EnsureCapacity(int capacity)
{
// This is not expected to be called this with negative capacity
Debug.Assert(capacity >= 0);

// If the caller has a bug and calls this with negative capacity, make sure to call Grow to throw an exception.
if ((uint)capacity > (uint)_chars.Length)
Grow(capacity - _pos);
}

Check warning on line 50 in Refit/ValueStringBuilder.cs

View check run for this annotation

Codecov / codecov/patch

Refit/ValueStringBuilder.cs#L49-L50

Added lines #L49 - L50 were not covered by tests

/// <summary>
/// Get a pinnable reference to the builder.
/// Does not ensure there is a null char after <see cref="Length"/>
/// This overload is pattern matched in the C# 7.3+ compiler so you can omit
/// the explicit method call, and write eg "fixed (char* c = builder)"
/// </summary>
public ref char GetPinnableReference()
{
return ref MemoryMarshal.GetReference(_chars);

Check warning on line 60 in Refit/ValueStringBuilder.cs

View check run for this annotation

Codecov / codecov/patch

Refit/ValueStringBuilder.cs#L60

Added line #L60 was not covered by tests
}

/// <summary>
/// Get a pinnable reference to the builder.
/// </summary>
/// <param name="terminate">Ensures that the builder has a null char after <see cref="Length"/></param>
public ref char GetPinnableReference(bool terminate)
{
if (terminate)
{
EnsureCapacity(Length + 1);
_chars[Length] = '\0';

Check warning on line 72 in Refit/ValueStringBuilder.cs

View check run for this annotation

Codecov / codecov/patch

Refit/ValueStringBuilder.cs#L71-L72

Added lines #L71 - L72 were not covered by tests
}
return ref MemoryMarshal.GetReference(_chars);

Check warning on line 74 in Refit/ValueStringBuilder.cs

View check run for this annotation

Codecov / codecov/patch

Refit/ValueStringBuilder.cs#L74

Added line #L74 was not covered by tests
}

public ref char this[int index]
{
get
{
Debug.Assert(index < _pos);
return ref _chars[index];

Check warning on line 82 in Refit/ValueStringBuilder.cs

View check run for this annotation

Codecov / codecov/patch

Refit/ValueStringBuilder.cs#L82

Added line #L82 was not covered by tests
}
}

public override string ToString()
{
var s = _chars.Slice(0, _pos).ToString();
Dispose();
return s;
}

/// <summary>Returns the underlying storage of the builder.</summary>
public Span<char> RawChars => _chars;

Check warning on line 94 in Refit/ValueStringBuilder.cs

View check run for this annotation

Codecov / codecov/patch

Refit/ValueStringBuilder.cs#L94

Added line #L94 was not covered by tests

/// <summary>
/// Returns a span around the contents of the builder.
/// </summary>
/// <param name="terminate">Ensures that the builder has a null char after <see cref="Length"/></param>
public ReadOnlySpan<char> AsSpan(bool terminate)
{
if (terminate)
{
EnsureCapacity(Length + 1);
_chars[Length] = '\0';

Check warning on line 105 in Refit/ValueStringBuilder.cs

View check run for this annotation

Codecov / codecov/patch

Refit/ValueStringBuilder.cs#L104-L105

Added lines #L104 - L105 were not covered by tests
}
return _chars.Slice(0, _pos);

Check warning on line 107 in Refit/ValueStringBuilder.cs

View check run for this annotation

Codecov / codecov/patch

Refit/ValueStringBuilder.cs#L107

Added line #L107 was not covered by tests
}

public ReadOnlySpan<char> AsSpan() => _chars.Slice(0, _pos);
public ReadOnlySpan<char> AsSpan(int start) => _chars.Slice(start, _pos - start);
public ReadOnlySpan<char> AsSpan(int start, int length) => _chars.Slice(start, length);

Check warning on line 112 in Refit/ValueStringBuilder.cs

View check run for this annotation

Codecov / codecov/patch

Refit/ValueStringBuilder.cs#L110-L112

Added lines #L110 - L112 were not covered by tests

public bool TryCopyTo(Span<char> destination, out int charsWritten)
{
if (_chars.Slice(0, _pos).TryCopyTo(destination))
{
charsWritten = _pos;
Dispose();
return true;

Check warning on line 120 in Refit/ValueStringBuilder.cs

View check run for this annotation

Codecov / codecov/patch

Refit/ValueStringBuilder.cs#L118-L120

Added lines #L118 - L120 were not covered by tests
}
else
{
charsWritten = 0;
Dispose();
return false;

Check warning on line 126 in Refit/ValueStringBuilder.cs

View check run for this annotation

Codecov / codecov/patch

Refit/ValueStringBuilder.cs#L124-L126

Added lines #L124 - L126 were not covered by tests
}
}

public void Insert(int index, char value, int count)
{
if (_pos > _chars.Length - count)
{
Grow(count);

Check warning on line 134 in Refit/ValueStringBuilder.cs

View check run for this annotation

Codecov / codecov/patch

Refit/ValueStringBuilder.cs#L134

Added line #L134 was not covered by tests
}

var remaining = _pos - index;
_chars.Slice(index, remaining).CopyTo(_chars.Slice(index + count));
_chars.Slice(index, count).Fill(value);
_pos += count;
}

Check warning on line 141 in Refit/ValueStringBuilder.cs

View check run for this annotation

Codecov / codecov/patch

Refit/ValueStringBuilder.cs#L137-L141

Added lines #L137 - L141 were not covered by tests

public void Insert(int index, string? s)
{
if (s == null)
{
return;

Check warning on line 147 in Refit/ValueStringBuilder.cs

View check run for this annotation

Codecov / codecov/patch

Refit/ValueStringBuilder.cs#L147

Added line #L147 was not covered by tests
}

var count = s.Length;

Check warning on line 150 in Refit/ValueStringBuilder.cs

View check run for this annotation

Codecov / codecov/patch

Refit/ValueStringBuilder.cs#L150

Added line #L150 was not covered by tests

if (_pos > (_chars.Length - count))
{
Grow(count);

Check warning on line 154 in Refit/ValueStringBuilder.cs

View check run for this annotation

Codecov / codecov/patch

Refit/ValueStringBuilder.cs#L154

Added line #L154 was not covered by tests
}

var remaining = _pos - index;
_chars.Slice(index, remaining).CopyTo(_chars.Slice(index + count));
s
#if !NETCOREAPP
.AsSpan()
#endif
.CopyTo(_chars.Slice(index));
_pos += count;
}

Check warning on line 165 in Refit/ValueStringBuilder.cs

View check run for this annotation

Codecov / codecov/patch

Refit/ValueStringBuilder.cs#L157-L165

Added lines #L157 - L165 were not covered by tests

[MethodImpl(MethodImplOptions.AggressiveInlining)]
public void Append(char c)
{
var pos = _pos;
var chars = _chars;
if ((uint)pos < (uint)chars.Length)
{
chars[pos] = c;
_pos = pos + 1;
}
else
{
GrowAndAppend(c);

Check warning on line 179 in Refit/ValueStringBuilder.cs

View check run for this annotation

Codecov / codecov/patch

Refit/ValueStringBuilder.cs#L179

Added line #L179 was not covered by tests
}
}

Check warning on line 181 in Refit/ValueStringBuilder.cs

View check run for this annotation

Codecov / codecov/patch

Refit/ValueStringBuilder.cs#L181

Added line #L181 was not covered by tests

[MethodImpl(MethodImplOptions.AggressiveInlining)]
public void Append(string? s)
{
if (s == null)
{
return;

Check warning on line 188 in Refit/ValueStringBuilder.cs

View check run for this annotation

Codecov / codecov/patch

Refit/ValueStringBuilder.cs#L188

Added line #L188 was not covered by tests
}

var pos = _pos;
if (s.Length == 1 && (uint)pos < (uint)_chars.Length) // very common case, e.g. appending strings from NumberFormatInfo like separators, percent symbols, etc.
{
_chars[pos] = s[0];
_pos = pos + 1;
}
else
{
AppendSlow(s);
}
}

private void AppendSlow(string s)
{
var pos = _pos;
if (pos > _chars.Length - s.Length)
{
Grow(s.Length);

Check warning on line 208 in Refit/ValueStringBuilder.cs

View check run for this annotation

Codecov / codecov/patch

Refit/ValueStringBuilder.cs#L208

Added line #L208 was not covered by tests
}

s
#if !NETCOREAPP
.AsSpan()
#endif
.CopyTo(_chars.Slice(pos));
_pos += s.Length;
}

public void Append(char c, int count)
{
if (_pos > _chars.Length - count)
{
Grow(count);

Check warning on line 223 in Refit/ValueStringBuilder.cs

View check run for this annotation

Codecov / codecov/patch

Refit/ValueStringBuilder.cs#L223

Added line #L223 was not covered by tests
}

var dst = _chars.Slice(_pos, count);

Check warning on line 226 in Refit/ValueStringBuilder.cs

View check run for this annotation

Codecov / codecov/patch

Refit/ValueStringBuilder.cs#L226

Added line #L226 was not covered by tests
for (var i = 0; i < dst.Length; i++)
{
dst[i] = c;

Check warning on line 229 in Refit/ValueStringBuilder.cs

View check run for this annotation

Codecov / codecov/patch

Refit/ValueStringBuilder.cs#L229

Added line #L229 was not covered by tests
}
_pos += count;
}

Check warning on line 232 in Refit/ValueStringBuilder.cs

View check run for this annotation

Codecov / codecov/patch

Refit/ValueStringBuilder.cs#L231-L232

Added lines #L231 - L232 were not covered by tests

public void Append(ReadOnlySpan<char> value)
{
var pos = _pos;

Check warning on line 236 in Refit/ValueStringBuilder.cs

View check run for this annotation

Codecov / codecov/patch

Refit/ValueStringBuilder.cs#L236

Added line #L236 was not covered by tests
if (pos > _chars.Length - value.Length)
{
Grow(value.Length);

Check warning on line 239 in Refit/ValueStringBuilder.cs

View check run for this annotation

Codecov / codecov/patch

Refit/ValueStringBuilder.cs#L239

Added line #L239 was not covered by tests
}

value.CopyTo(_chars.Slice(_pos));
_pos += value.Length;
}

Check warning on line 244 in Refit/ValueStringBuilder.cs

View check run for this annotation

Codecov / codecov/patch

Refit/ValueStringBuilder.cs#L242-L244

Added lines #L242 - L244 were not covered by tests

[MethodImpl(MethodImplOptions.AggressiveInlining)]
public Span<char> AppendSpan(int length)
{
var origPos = _pos;

Check warning on line 249 in Refit/ValueStringBuilder.cs

View check run for this annotation

Codecov / codecov/patch

Refit/ValueStringBuilder.cs#L249

Added line #L249 was not covered by tests
if (origPos > _chars.Length - length)
{
Grow(length);

Check warning on line 252 in Refit/ValueStringBuilder.cs

View check run for this annotation

Codecov / codecov/patch

Refit/ValueStringBuilder.cs#L252

Added line #L252 was not covered by tests
}

_pos = origPos + length;
return _chars.Slice(origPos, length);

Check warning on line 256 in Refit/ValueStringBuilder.cs

View check run for this annotation

Codecov / codecov/patch

Refit/ValueStringBuilder.cs#L255-L256

Added lines #L255 - L256 were not covered by tests
}

[MethodImpl(MethodImplOptions.NoInlining)]
private void GrowAndAppend(char c)
{
Grow(1);
Append(c);
}

Check warning on line 264 in Refit/ValueStringBuilder.cs

View check run for this annotation

Codecov / codecov/patch

Refit/ValueStringBuilder.cs#L262-L264

Added lines #L262 - L264 were not covered by tests

/// <summary>
/// Resize the internal buffer either by doubling current buffer size or
/// by adding <paramref name="additionalCapacityBeyondPos"/> to
/// <see cref="_pos"/> whichever is greater.
/// </summary>
/// <param name="additionalCapacityBeyondPos">
/// Number of chars requested beyond current position.
/// </param>
[MethodImpl(MethodImplOptions.NoInlining)]
private void Grow(int additionalCapacityBeyondPos)
{
Debug.Assert(additionalCapacityBeyondPos > 0);
Debug.Assert(_pos > _chars.Length - additionalCapacityBeyondPos, "Grow called incorrectly, no resize is needed.");

const uint ArrayMaxLength = 0x7FFFFFC7; // same as Array.MaxLength

// Increase to at least the required size (_pos + additionalCapacityBeyondPos), but try
// to double the size if possible, bounding the doubling to not go beyond the max array length.
var newCapacity = (int)Math.Max(
(uint)(_pos + additionalCapacityBeyondPos),
Math.Min((uint)_chars.Length * 2, ArrayMaxLength));

Check warning on line 286 in Refit/ValueStringBuilder.cs

View check run for this annotation

Codecov / codecov/patch

Refit/ValueStringBuilder.cs#L284-L286

Added lines #L284 - L286 were not covered by tests

// Make sure to let Rent throw an exception if the caller has a bug and the desired capacity is negative.
// This could also go negative if the actual required length wraps around.
var poolArray = ArrayPool<char>.Shared.Rent(newCapacity);

Check warning on line 290 in Refit/ValueStringBuilder.cs

View check run for this annotation

Codecov / codecov/patch

Refit/ValueStringBuilder.cs#L290

Added line #L290 was not covered by tests

_chars.Slice(0, _pos).CopyTo(poolArray);

Check warning on line 292 in Refit/ValueStringBuilder.cs

View check run for this annotation

Codecov / codecov/patch

Refit/ValueStringBuilder.cs#L292

Added line #L292 was not covered by tests

var toReturn = _arrayToReturnToPool;
_chars = _arrayToReturnToPool = poolArray;

Check warning on line 295 in Refit/ValueStringBuilder.cs

View check run for this annotation

Codecov / codecov/patch

Refit/ValueStringBuilder.cs#L294-L295

Added lines #L294 - L295 were not covered by tests
if (toReturn != null)
{
ArrayPool<char>.Shared.Return(toReturn);

Check warning on line 298 in Refit/ValueStringBuilder.cs

View check run for this annotation

Codecov / codecov/patch

Refit/ValueStringBuilder.cs#L298

Added line #L298 was not covered by tests
}
}

Check warning on line 300 in Refit/ValueStringBuilder.cs

View check run for this annotation

Codecov / codecov/patch

Refit/ValueStringBuilder.cs#L300

Added line #L300 was not covered by tests

[MethodImpl(MethodImplOptions.AggressiveInlining)]
public void Dispose()
{
var toReturn = _arrayToReturnToPool;
this = default; // for safety, to avoid using pooled array if this instance is erroneously appended to again
if (toReturn != null)
{
ArrayPool<char>.Shared.Return(toReturn);

Check warning on line 309 in Refit/ValueStringBuilder.cs

View check run for this annotation

Codecov / codecov/patch

Refit/ValueStringBuilder.cs#L309

Added line #L309 was not covered by tests
}
}
}
Loading