Empty array query parameter bug fix

[Boz0r]

Fixed a bug where passing an empty array to WithQueryParameter would result in the server receiving an array of length 1 containing an empty string.

[egil]

Now that I look at the code again, MessageRequestBuilder should probably just have a couple of overloads of WithPathParameter instead of a single one that uses reflection, e.g.:

       public IMessageRequestBuilder WithPathParameter(string name, IEnumerable<object?> values)
       {
            if (string.IsNullOrWhiteSpace(name))
            {
                throw new ArgumentException($"'{nameof(name)}' cannot be null or whitespace", nameof(name));
            }

            if (values is null)
            {
                return this;
            }

            var sb = new StringBuilder();
            foreach(var value in values.OfType<object>())
            {
                    sb.Append(sb.Length == 0
                        ? Uri.EscapeDataString(objects[i].ToString())
                        : $"&{name}={Uri.EscapeDataString(objects[i].ToString())}");
                }

            if(sb.Length > 0)
            {
                queryMapper["#" + name] = sb.ToString();
            }

            return this;
       }
       
       public IMessageRequestBuilder WithPathParameter(string name, object? value)
       {
            if (string.IsNullOrWhiteSpace(name))
            {
                throw new ArgumentException($"'{nameof(name)}' cannot be null or whitespace", nameof(name));
            }

            if (value is null)
            {
                return this;
            }

            queryMapper[name] = value.ToString();

            return this;
        }

(Typed this out on my phone while putting the kids in bed, so code probably needs a slight tweak, but hope it's clear what I mean).

[Boz0r]

I split up the function with an extra overload

[egil]

OK I could not push to your branch, but here is my suggestion, that replaces all WithQueryParameter methods:

        public IMessageRequestBuilder WithQueryParameter(string name, string? value)
        {
            if (string.IsNullOrWhiteSpace(name))
            {
                throw new ArgumentException($"'{nameof(name)}' cannot be null or whitespace", nameof(name));
            }

            if (value is not null)
            {
                queryMapper[name] = value;
            }

            return this;
        }

        public IMessageRequestBuilder WithQueryParameter(string name, IEnumerable values)
        {
            if (string.IsNullOrWhiteSpace(name))
            {
                throw new ArgumentException($"'{nameof(name)}' cannot be null or whitespace", nameof(name));
            }

            if (values is null)
            {
                return this;
            }

            var sb = new StringBuilder();
            foreach (var value in values.OfType<object>())
            {
                sb.Append(sb.Length == 0
                    ? Uri.EscapeDataString(value.ToString())
                    : $"&{name}={Uri.EscapeDataString(value.ToString())}");
            }

            if (sb.Length > 0)
            {
                queryMapper["#" + name] = sb.ToString();
            }

            return this;
        }

        public IMessageRequestBuilder WithQueryParameter(string name, object? value)
            => WithQueryParameter(name, value?.ToString());

For the tests, I would add this replace the "Array" and "List" tests with this:

        [Theory]
        [InlineAutoNSubstituteData("/api")]
        public void Should_Omit_Query_Parameters_With_Empty_Collection(
            string template)
        {
            var sut = CreateSut(template);

            sut.WithQueryParameter("foo", Array.Empty<string>());
            var message = sut.Build(HttpMethod.Get);

            message!
                .RequestUri!
                .ToString()
                .Should()
                .Be($"/api");
        }

and remove the odd Vector2 test.

[perkops]

@egil or @Boz0r whats is the status on this PR?