From 3768a983b7de973e22c7f070c315e13c955b6e07 Mon Sep 17 00:00:00 2001 From: Whit Waldo Date: Wed, 3 Jul 2024 12:47:54 -0500 Subject: [PATCH] Added overload to support SDK supplying query string on invoked URL (#1310) * Refactored extensions and their tests into separate directories Signed-off-by: Whit Waldo * Added overload to method invocation to allow query string parameters to be passed in via the SDK instead of being uncermoniously added to the end of the produced HttpRequestMessage URI Signed-off-by: Whit Waldo * Added unit tests to support implementation Signed-off-by: Whit Waldo * Marking HttpExtensions as internal to prevent external usage and updating to work against Uri instead of HttpRequestMessage. Signed-off-by: Whit Waldo * Updated unit tests to match new extension purpose Signed-off-by: Whit Waldo * Resolved an ambiguous method invocation wherein it was taking the query string and passing it as the payload for a request. Removed the offending method and reworked the remaining configurations so there's no API impact. Signed-off-by: Whit Waldo --------- Signed-off-by: Whit Waldo --- src/Dapr.Client/DaprClient.cs | 40 ++++++++++-- src/Dapr.Client/DaprClientGrpc.cs | 46 +++++++++++++- .../{ => Extensions}/EnumExtensions.cs | 5 +- src/Dapr.Client/Extensions/HttpExtensions.cs | 51 +++++++++++++++ .../DaprClientTest.InvokeMethodAsync.cs | 40 ++++++++++++ .../{ => Extensions}/EnumExtensionTest.cs | 6 +- .../Extensions/HttpExtensionTest.cs | 63 +++++++++++++++++++ 7 files changed, 238 insertions(+), 13 deletions(-) rename src/Dapr.Client/{ => Extensions}/EnumExtensions.cs (88%) create mode 100644 src/Dapr.Client/Extensions/HttpExtensions.cs rename test/Dapr.Client.Test/{ => Extensions}/EnumExtensionTest.cs (87%) create mode 100644 test/Dapr.Client.Test/Extensions/HttpExtensionTest.cs diff --git a/src/Dapr.Client/DaprClient.cs b/src/Dapr.Client/DaprClient.cs index 21777105b..4f89d8668 100644 --- a/src/Dapr.Client/DaprClient.cs +++ b/src/Dapr.Client/DaprClient.cs @@ -306,6 +306,20 @@ public HttpRequestMessage CreateInvokeMethodRequest(string appId, string methodN return CreateInvokeMethodRequest(HttpMethod.Post, appId, methodName); } + /// + /// Creates an that can be used to perform service invocation for the + /// application identified by and invokes the method specified by + /// with the POST HTTP method. + /// + /// The Dapr application id to invoke the method on. + /// The name of the method to invoke. + /// A collection of key/value pairs to populate the query string from. + /// An for use with SendInvokeMethodRequestAsync. + public HttpRequestMessage CreateInvokeMethodRequest(string appId, string methodName, IReadOnlyCollection> queryStringParameters) + { + return CreateInvokeMethodRequest(HttpMethod.Post, appId, methodName, queryStringParameters); + } + /// /// Creates an that can be used to perform service invocation for the /// application identified by and invokes the method specified by @@ -317,6 +331,19 @@ public HttpRequestMessage CreateInvokeMethodRequest(string appId, string methodN /// An for use with SendInvokeMethodRequestAsync. public abstract HttpRequestMessage CreateInvokeMethodRequest(HttpMethod httpMethod, string appId, string methodName); + /// + /// Creates an that can be used to perform service invocation for the + /// application identified by and invokes the method specified by + /// with the HTTP method specified by . + /// + /// The to use for the invocation request. + /// The Dapr application id to invoke the method on. + /// The name of the method to invoke. + /// A collection of key/value pairs to populate the query string from. + /// An for use with SendInvokeMethodRequestAsync. + public abstract HttpRequestMessage CreateInvokeMethodRequest(HttpMethod httpMethod, string appId, + string methodName, IReadOnlyCollection> queryStringParameters); + /// /// Creates an that can be used to perform service invocation for the /// application identified by and invokes the method specified by @@ -329,9 +356,9 @@ public HttpRequestMessage CreateInvokeMethodRequest(string appId, string methodN /// An for use with SendInvokeMethodRequestAsync. public HttpRequestMessage CreateInvokeMethodRequest(string appId, string methodName, TRequest data) { - return CreateInvokeMethodRequest(HttpMethod.Post, appId, methodName, data); + return CreateInvokeMethodRequest(HttpMethod.Post, appId, methodName, new List>(), data); } - + /// /// Creates an that can be used to perform service invocation for the /// application identified by and invokes the method specified by @@ -343,9 +370,10 @@ public HttpRequestMessage CreateInvokeMethodRequest(string appId, stri /// The Dapr application id to invoke the method on. /// The name of the method to invoke. /// The data that will be JSON serialized and provided as the request body. + /// A collection of key/value pairs to populate the query string from. /// An for use with SendInvokeMethodRequestAsync. - public abstract HttpRequestMessage CreateInvokeMethodRequest(HttpMethod httpMethod, string appId, string methodName, TRequest data); - + public abstract HttpRequestMessage CreateInvokeMethodRequest(HttpMethod httpMethod, string appId, string methodName, IReadOnlyCollection> queryStringParameters, TRequest data); + /// /// Perform health-check of Dapr sidecar. Return 'true' if sidecar is healthy. Otherwise 'false'. /// CheckHealthAsync handle and will return 'false' if error will occur on transport level @@ -526,7 +554,7 @@ public Task InvokeMethodAsync( TRequest data, CancellationToken cancellationToken = default) { - var request = CreateInvokeMethodRequest(httpMethod, appId, methodName, data); + var request = CreateInvokeMethodRequest(httpMethod, appId, methodName, new List>(), data); return InvokeMethodAsync(request, cancellationToken); } @@ -620,7 +648,7 @@ public Task InvokeMethodAsync( TRequest data, CancellationToken cancellationToken = default) { - var request = CreateInvokeMethodRequest(httpMethod, appId, methodName, data); + var request = CreateInvokeMethodRequest(httpMethod, appId, methodName, new List>(), data); return InvokeMethodAsync(request, cancellationToken); } diff --git a/src/Dapr.Client/DaprClientGrpc.cs b/src/Dapr.Client/DaprClientGrpc.cs index 3cd7de526..af245afc3 100644 --- a/src/Dapr.Client/DaprClientGrpc.cs +++ b/src/Dapr.Client/DaprClientGrpc.cs @@ -345,7 +345,32 @@ public override async Task InvokeBindingAsync(BindingRequest re #region InvokeMethod Apis + /// + /// Creates an that can be used to perform service invocation for the + /// application identified by and invokes the method specified by + /// with the HTTP method specified by . + /// + /// The to use for the invocation request. + /// The Dapr application id to invoke the method on. + /// The name of the method to invoke. + /// An for use with SendInvokeMethodRequestAsync. public override HttpRequestMessage CreateInvokeMethodRequest(HttpMethod httpMethod, string appId, string methodName) + { + return CreateInvokeMethodRequest(httpMethod, appId, methodName, new List>()); + } + + /// + /// Creates an that can be used to perform service invocation for the + /// application identified by and invokes the method specified by + /// with the HTTP method specified by . + /// + /// The to use for the invocation request. + /// The Dapr application id to invoke the method on. + /// The name of the method to invoke. + /// A collection of key/value pairs to populate the query string from. + /// An for use with SendInvokeMethodRequestAsync. + public override HttpRequestMessage CreateInvokeMethodRequest(HttpMethod httpMethod, string appId, string methodName, + IReadOnlyCollection> queryStringParameters) { ArgumentVerifier.ThrowIfNull(httpMethod, nameof(httpMethod)); ArgumentVerifier.ThrowIfNullOrEmpty(appId, nameof(appId)); @@ -356,7 +381,8 @@ public override HttpRequestMessage CreateInvokeMethodRequest(HttpMethod httpMeth // // This approach avoids some common pitfalls that could lead to undesired encoding. var path = $"/v1.0/invoke/{appId}/method/{methodName.TrimStart('/')}"; - var request = new HttpRequestMessage(httpMethod, new Uri(this.httpEndpoint, path)); + var requestUri = new Uri(this.httpEndpoint, path).AddQueryParameters(queryStringParameters); + var request = new HttpRequestMessage(httpMethod, requestUri); request.Options.Set(new HttpRequestOptionsKey(AppIdKey), appId); request.Options.Set(new HttpRequestOptionsKey(MethodNameKey), methodName); @@ -369,13 +395,27 @@ public override HttpRequestMessage CreateInvokeMethodRequest(HttpMethod httpMeth return request; } - public override HttpRequestMessage CreateInvokeMethodRequest(HttpMethod httpMethod, string appId, string methodName, TRequest data) + /// + /// Creates an that can be used to perform service invocation for the + /// application identified by and invokes the method specified by + /// with the HTTP method specified by and a JSON serialized request body specified by + /// . + /// + /// The type of the data that will be JSON serialized and provided as the request body. + /// The to use for the invocation request. + /// The Dapr application id to invoke the method on. + /// The name of the method to invoke. + /// The data that will be JSON serialized and provided as the request body. + /// A collection of key/value pairs to populate the query string from. + /// An for use with SendInvokeMethodRequestAsync. + public override HttpRequestMessage CreateInvokeMethodRequest(HttpMethod httpMethod, string appId, string methodName, + IReadOnlyCollection> queryStringParameters, TRequest data) { ArgumentVerifier.ThrowIfNull(httpMethod, nameof(httpMethod)); ArgumentVerifier.ThrowIfNullOrEmpty(appId, nameof(appId)); ArgumentVerifier.ThrowIfNull(methodName, nameof(methodName)); - var request = CreateInvokeMethodRequest(httpMethod, appId, methodName); + var request = CreateInvokeMethodRequest(httpMethod, appId, methodName, queryStringParameters); request.Content = JsonContent.Create(data, options: this.JsonSerializerOptions); return request; } diff --git a/src/Dapr.Client/EnumExtensions.cs b/src/Dapr.Client/Extensions/EnumExtensions.cs similarity index 88% rename from src/Dapr.Client/EnumExtensions.cs rename to src/Dapr.Client/Extensions/EnumExtensions.cs index 6b058ca77..df9c9ad33 100644 --- a/src/Dapr.Client/EnumExtensions.cs +++ b/src/Dapr.Client/Extensions/EnumExtensions.cs @@ -11,6 +11,7 @@ // limitations under the License. // ------------------------------------------------------------------------ +#nullable enable using System; using System.Reflection; using System.Runtime.Serialization; @@ -27,12 +28,14 @@ internal static class EnumExtensions /// public static string GetValueFromEnumMember(this T value) where T : Enum { + ArgumentNullException.ThrowIfNull(value, nameof(value)); + var memberInfo = typeof(T).GetMember(value.ToString(), BindingFlags.Static | BindingFlags.Public | BindingFlags.DeclaredOnly); if (memberInfo.Length <= 0) return value.ToString(); var attributes = memberInfo[0].GetCustomAttributes(typeof(EnumMemberAttribute), false); - return attributes.Length > 0 ? ((EnumMemberAttribute)attributes[0]).Value : value.ToString(); + return (attributes.Length > 0 ? ((EnumMemberAttribute)attributes[0]).Value : value.ToString()) ?? value.ToString(); } } } diff --git a/src/Dapr.Client/Extensions/HttpExtensions.cs b/src/Dapr.Client/Extensions/HttpExtensions.cs new file mode 100644 index 000000000..259d2747d --- /dev/null +++ b/src/Dapr.Client/Extensions/HttpExtensions.cs @@ -0,0 +1,51 @@ +// ------------------------------------------------------------------------ +// Copyright 2024 The Dapr Authors +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// http://www.apache.org/licenses/LICENSE-2.0 +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// ------------------------------------------------------------------------ + +#nullable enable +using System; +using System.Collections.Generic; +using System.Text; + +namespace Dapr.Client +{ + /// + /// Provides extensions specific to HTTP types. + /// + internal static class HttpExtensions + { + /// + /// Appends key/value pairs to the query string on an HttpRequestMessage. + /// + /// The uri to append the query string parameters to. + /// The key/value pairs to populate the query string with. + public static Uri AddQueryParameters(this Uri? uri, + IReadOnlyCollection>? queryStringParameters) + { + ArgumentNullException.ThrowIfNull(uri, nameof(uri)); + if (queryStringParameters is null) + return uri; + + var uriBuilder = new UriBuilder(uri); + var qsBuilder = new StringBuilder(uriBuilder.Query); + foreach (var kvParam in queryStringParameters) + { + if (qsBuilder.Length > 0) + qsBuilder.Append('&'); + qsBuilder.Append($"{Uri.EscapeDataString(kvParam.Key)}={Uri.EscapeDataString(kvParam.Value)}"); + } + + uriBuilder.Query = qsBuilder.ToString(); + return uriBuilder.Uri; + } + } +} diff --git a/test/Dapr.Client.Test/DaprClientTest.InvokeMethodAsync.cs b/test/Dapr.Client.Test/DaprClientTest.InvokeMethodAsync.cs index 5d46000a1..484f327d0 100644 --- a/test/Dapr.Client.Test/DaprClientTest.InvokeMethodAsync.cs +++ b/test/Dapr.Client.Test/DaprClientTest.InvokeMethodAsync.cs @@ -518,6 +518,18 @@ public async Task CreateInvokeMethodRequest_TransformsUrlCorrectly(string method Assert.Equal(new Uri(expected).AbsoluteUri, request.RequestUri.AbsoluteUri); } + [Fact] + public async Task CreateInvokeMethodRequest_AppendQueryStringValuesCorrectly() + { + await using var client = TestClient.CreateForDaprClient(c => + { + c.UseGrpcEndpoint("http://localhost").UseHttpEndpoint("https://test-endpoint:3501").UseJsonSerializationOptions(this.jsonSerializerOptions); + }); + + var request = client.InnerClient.CreateInvokeMethodRequest("test-app", "mymethod", (IReadOnlyCollection>)new List> { new("a", "0"), new("b", "1") }); + Assert.Equal(new Uri("https://test-endpoint:3501/v1.0/invoke/test-app/method/mymethod?a=0&b=1").AbsoluteUri, request.RequestUri.AbsoluteUri); + } + [Fact] public async Task CreateInvokeMethodRequest_WithoutApiToken_CreatesHttpRequestWithoutApiTokenHeader() { @@ -617,6 +629,34 @@ public async Task CreateInvokeMethodRequest_WithData_CreatesJsonContent() Assert.Equal(data.Color, actual.Color); } + [Fact] + public async Task CreateInvokeMethodRequest_WithData_CreatesJsonContentWithQueryString() + { + await using var client = TestClient.CreateForDaprClient(c => + { + c.UseGrpcEndpoint("http://localhost").UseHttpEndpoint("https://test-endpoint:3501").UseJsonSerializationOptions(this.jsonSerializerOptions); + }); + + var data = new Widget + { + Color = "red", + }; + + var request = client.InnerClient.CreateInvokeMethodRequest(HttpMethod.Post, "test-app", "test", new List> { new("a", "0"), new("b", "1") }, data); + + Assert.Equal(new Uri("https://test-endpoint:3501/v1.0/invoke/test-app/method/test?a=0&b=1").AbsoluteUri, request.RequestUri.AbsoluteUri); + + var content = Assert.IsType(request.Content); + Assert.Equal(typeof(Widget), content.ObjectType); + Assert.Same(data, content.Value); + + // the best way to verify the usage of the correct settings object + var actual = await content.ReadFromJsonAsync(this.jsonSerializerOptions); + Assert.Equal(data.Color, actual.Color); + } + + + [Fact] public async Task InvokeMethodWithResponseAsync_ReturnsMessageWithoutCheckingStatus() { diff --git a/test/Dapr.Client.Test/EnumExtensionTest.cs b/test/Dapr.Client.Test/Extensions/EnumExtensionTest.cs similarity index 87% rename from test/Dapr.Client.Test/EnumExtensionTest.cs rename to test/Dapr.Client.Test/Extensions/EnumExtensionTest.cs index be78c3861..83c4354f9 100644 --- a/test/Dapr.Client.Test/EnumExtensionTest.cs +++ b/test/Dapr.Client.Test/Extensions/EnumExtensionTest.cs @@ -1,7 +1,7 @@ using System.Runtime.Serialization; using Xunit; -namespace Dapr.Client.Test +namespace Dapr.Client.Test.Extensions { public class EnumExtensionTest { @@ -29,9 +29,9 @@ public void GetValueFromEnumMember_BlueResolvesAsExpected() public enum TestEnum { - [EnumMember(Value="red")] + [EnumMember(Value = "red")] Red, - [EnumMember(Value="YELLOW")] + [EnumMember(Value = "YELLOW")] Yellow, Blue } diff --git a/test/Dapr.Client.Test/Extensions/HttpExtensionTest.cs b/test/Dapr.Client.Test/Extensions/HttpExtensionTest.cs new file mode 100644 index 000000000..7b93c1c91 --- /dev/null +++ b/test/Dapr.Client.Test/Extensions/HttpExtensionTest.cs @@ -0,0 +1,63 @@ +using System.Collections.Generic; +using System.Net.Http; +using Xunit; + +namespace Dapr.Client.Test.Extensions +{ + public class HttpExtensionTest + { + [Fact] + public void AddQueryParameters_ReturnsEmptyQueryStringWithNullParameters() + { + const string uri = "https://localhost/mypath"; + var httpRq = new HttpRequestMessage(HttpMethod.Get, uri); + var updatedUri = httpRq.RequestUri.AddQueryParameters(null); + Assert.Equal(uri, updatedUri.AbsoluteUri); + } + + [Fact] + public void AddQueryParameters_ReturnsOriginalQueryStringWithNullParameters() + { + const string uri = "https://localhost/mypath?a=0&b=1"; + var httpRq = new HttpRequestMessage(HttpMethod.Get, uri); + var updatedUri = httpRq.RequestUri.AddQueryParameters(null); + Assert.Equal(uri, updatedUri.AbsoluteUri); + } + + [Fact] + public void AddQueryParameters_BuildsQueryString() + { + var httpRq = new HttpRequestMessage(HttpMethod.Get, "https://localhost/mypath?a=0"); + var updatedUri = httpRq.RequestUri.AddQueryParameters(new List> + { + new("test", "value") + }); + Assert.Equal("https://localhost/mypath?a=0&test=value", updatedUri.AbsoluteUri); + } + + [Fact] + public void AddQueryParameters_BuildQueryStringWithDuplicateKeys() + { + var httpRq = new HttpRequestMessage(HttpMethod.Get, "https://localhost/mypath"); + var updatedUri = httpRq.RequestUri.AddQueryParameters(new List> + { + new("test", "1"), + new("test", "2"), + new("test", "3") + }); + Assert.Equal("https://localhost/mypath?test=1&test=2&test=3", updatedUri.AbsoluteUri); + } + + [Fact] + public void AddQueryParameters_EscapeSpacesInValues() + { + var httpRq = new HttpRequestMessage(HttpMethod.Get, "https://localhost/mypath"); + var updatedUri = httpRq.RequestUri.AddQueryParameters(new List> + { + new("name1", "John Doe"), + new("name2", "Jane Doe") + }); + Assert.Equal("https://localhost/mypath?name1=John%20Doe&name2=Jane%20Doe", updatedUri.AbsoluteUri); + } + } +}