feat: use ValueStringBuilder adding the query parameters (#1719)

* feat: use `ValueStringBuilder` adding the query parameters

* chore: merge, suppress warning and extract method

---------

Co-authored-by: Chris Pulman <chris.pulman@yahoo.com>

Refit/RequestBuilderImplementation.cs

@@ -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
                 {
@@ -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
         )

Refit/ValueStringBuilder.cs

@@ -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;
+    }
+
+    public int Length
+    {
+        get => _pos;
+        set
+        {
+            Debug.Assert(value >= 0);
+            Debug.Assert(value <= _chars.Length);
+            _pos = value;
+        }
+    }
+
+    public int Capacity => _chars.Length;
+
+    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);
+    }
+
+    /// <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);
+    }
+
+    /// <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';
+        }
+        return ref MemoryMarshal.GetReference(_chars);
+    }
+
+    public ref char this[int index]
+    {
+        get
+        {
+            Debug.Assert(index < _pos);
+            return ref _chars[index];
+        }
+    }
+
+    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;
+
+    /// <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';
+        }
+        return _chars.Slice(0, _pos);
+    }
+
+    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);
+
+    public bool TryCopyTo(Span<char> destination, out int charsWritten)
+    {
+        if (_chars.Slice(0, _pos).TryCopyTo(destination))
+        {
+            charsWritten = _pos;
+            Dispose();
+            return true;
+        }
+        else
+        {
+            charsWritten = 0;
+            Dispose();
+            return false;
+        }
+    }
+
+    public void Insert(int index, char value, int count)
+    {
+        if (_pos > _chars.Length - count)
+        {
+            Grow(count);
+        }
+
+        var remaining = _pos - index;
+        _chars.Slice(index, remaining).CopyTo(_chars.Slice(index + count));
+        _chars.Slice(index, count).Fill(value);
+        _pos += count;
+    }
+
+    public void Insert(int index, string? s)
+    {
+        if (s == null)
+        {
+            return;
+        }
+
+        var count = s.Length;
+
+        if (_pos > (_chars.Length - count))
+        {
+            Grow(count);
+        }
+
+        var remaining = _pos - index;
+        _chars.Slice(index, remaining).CopyTo(_chars.Slice(index + count));
+        s
+#if !NETCOREAPP
+            .AsSpan()
+#endif
+            .CopyTo(_chars.Slice(index));
+        _pos += count;
+    }
+
+    [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);
+        }
+    }
+
+    [MethodImpl(MethodImplOptions.AggressiveInlining)]
+    public void Append(string? s)
+    {
+        if (s == null)
+        {
+            return;
+        }
+
+        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);
+        }
+
+        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);
+        }
+
+        var dst = _chars.Slice(_pos, count);
+        for (var i = 0; i < dst.Length; i++)
+        {
+            dst[i] = c;
+        }
+        _pos += count;
+    }
+
+    public void Append(ReadOnlySpan<char> value)
+    {
+        var pos = _pos;
+        if (pos > _chars.Length - value.Length)
+        {
+            Grow(value.Length);
+        }
+
+        value.CopyTo(_chars.Slice(_pos));
+        _pos += value.Length;
+    }
+
+    [MethodImpl(MethodImplOptions.AggressiveInlining)]
+    public Span<char> AppendSpan(int length)
+    {
+        var origPos = _pos;
+        if (origPos > _chars.Length - length)
+        {
+            Grow(length);
+        }
+
+        _pos = origPos + length;
+        return _chars.Slice(origPos, length);
+    }
+
+    [MethodImpl(MethodImplOptions.NoInlining)]
+    private void GrowAndAppend(char c)
+    {
+        Grow(1);
+        Append(c);
+    }
+
+    /// <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));
+
+        // 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);
+
+        _chars.Slice(0, _pos).CopyTo(poolArray);
+
+        var toReturn = _arrayToReturnToPool;
+        _chars = _arrayToReturnToPool = poolArray;
+        if (toReturn != null)
+        {
+            ArrayPool<char>.Shared.Return(toReturn);
+        }
+    }
+
+    [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);
+        }
+    }
+}