Skip to content

Commit 7ea046b

Browse files
authored
Include route params in RpcHttp instance when http proxying is enabled. (#9997)
* Include route params in RpcHttp instance when http proxying is enabled. * Fixes to address PR comments.
1 parent 4981eb8 commit 7ea046b

File tree

3 files changed

+96
-23
lines changed

3 files changed

+96
-23
lines changed

src/WebJobs.Script.Grpc/MessageExtensions/GrpcMessageConversionExtensions.cs

Lines changed: 44 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -101,9 +101,26 @@ internal static TypedData ToModelBindingDataArray(this ParameterBindingData[] da
101101

102102
internal static async Task<TypedData> ToRpcHttp(this HttpRequest request, ILogger logger, GrpcCapabilities capabilities)
103103
{
104+
bool requiresRouteParams = !string.IsNullOrEmpty(capabilities.GetCapabilityState(RpcWorkerConstants.RequiresRouteParameters));
105+
bool isHttpProxying = !string.IsNullOrEmpty(capabilities.GetCapabilityState(RpcWorkerConstants.HttpUri));
106+
bool shouldUseNullableValueDictionary = ShouldUseNullableValueDictionary(capabilities);
107+
108+
// If proxying the http request to the worker and
109+
// worker requesting route params, send an empty rpc http object with only params populated.
110+
if (isHttpProxying && requiresRouteParams)
111+
{
112+
var typedDataWithRouteParams = new TypedData
113+
{
114+
Http = new RpcHttp()
115+
};
116+
117+
PopulateHttpRouteDataAsParams(request, typedDataWithRouteParams.Http, shouldUseNullableValueDictionary);
118+
119+
return typedDataWithRouteParams;
120+
}
121+
104122
// If proxying the http request to the worker, keep the grpc message minimal
105-
bool skipHttpInputs = !string.IsNullOrEmpty(capabilities.GetCapabilityState(RpcWorkerConstants.HttpUri));
106-
if (skipHttpInputs)
123+
if (isHttpProxying)
107124
{
108125
return EmptyRpcHttp;
109126
}
@@ -119,7 +136,6 @@ internal static async Task<TypedData> ToRpcHttp(this HttpRequest request, ILogge
119136
Http = http
120137
};
121138

122-
var shouldUseNullableValueDictionary = ShouldUseNullableValueDictionary(capabilities);
123139
foreach (var pair in request.Query)
124140
{
125141
var value = pair.Value.ToString();
@@ -153,24 +169,7 @@ internal static async Task<TypedData> ToRpcHttp(this HttpRequest request, ILogge
153169
}
154170
}
155171

156-
if (request.HttpContext.Items.TryGetValue(HttpExtensionConstants.AzureWebJobsHttpRouteDataKey, out object routeData))
157-
{
158-
Dictionary<string, object> parameters = (Dictionary<string, object>)routeData;
159-
foreach (var pair in parameters)
160-
{
161-
if (pair.Value != null)
162-
{
163-
if (shouldUseNullableValueDictionary)
164-
{
165-
http.NullableParams.Add(pair.Key, new NullableString { Value = pair.Value.ToString() });
166-
}
167-
else
168-
{
169-
http.Params.Add(pair.Key, pair.Value.ToString());
170-
}
171-
}
172-
}
173-
}
172+
PopulateHttpRouteDataAsParams(request, http, shouldUseNullableValueDictionary);
174173

175174
// parse ClaimsPrincipal if exists
176175
if (request.HttpContext?.User?.Identities != null)
@@ -222,6 +221,30 @@ internal static async Task<TypedData> ToRpcHttp(this HttpRequest request, ILogge
222221
return typedData;
223222
}
224223

224+
private static void PopulateHttpRouteDataAsParams(HttpRequest request, RpcHttp http, bool shouldUseNullableValueDictionary)
225+
{
226+
if (!request.HttpContext.Items.TryGetValue(HttpExtensionConstants.AzureWebJobsHttpRouteDataKey, out var routeData)
227+
|| routeData is not Dictionary<string, object> parameters)
228+
{
229+
return;
230+
}
231+
232+
foreach (var pair in parameters)
233+
{
234+
if (pair.Value != null)
235+
{
236+
if (shouldUseNullableValueDictionary)
237+
{
238+
http.NullableParams.Add(pair.Key, new NullableString { Value = pair.Value.ToString() });
239+
}
240+
else
241+
{
242+
http.Params.Add(pair.Key, pair.Value.ToString());
243+
}
244+
}
245+
}
246+
}
247+
225248
private static async Task PopulateBody(HttpRequest request, RpcHttp http, GrpcCapabilities capabilities, ILogger logger)
226249
{
227250
object body = null;

src/WebJobs.Script/Workers/Rpc/RpcWorkerConstants.cs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,11 @@ public static class RpcWorkerConstants
5757
public const string HttpUri = "HttpUri";
5858
public const string WorkerOpenTelemetryEnabled = nameof(WorkerOpenTelemetryEnabled);
5959

60+
/// <summary>
61+
/// Indicates whether the RpcHttp request should include the route parameters when the request is being proxied to an HTTP worker.
62+
/// </summary>
63+
public const string RequiresRouteParameters = "RequiresRouteParameters";
64+
6065
/// <summary>
6166
/// Indicates whether empty entries in the trigger message should be included when sending RpcInvocation data to OOP workers.
6267
/// </summary>

test/WebJobs.Script.Tests/Workers/ScriptInvocationContextExtensionsTests.cs

Lines changed: 47 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
using System.Threading.Tasks;
1111
using Google.Protobuf.Collections;
1212
using Microsoft.AspNetCore.Http;
13+
using Microsoft.Azure.WebJobs.Extensions.Http;
1314
using Microsoft.Azure.WebJobs.Script.Description;
1415
using Microsoft.Azure.WebJobs.Script.Grpc;
1516
using Microsoft.Azure.WebJobs.Script.Grpc.Messages;
@@ -332,16 +333,52 @@ public async Task ToRpc_Http()
332333
public async Task ToRpc_Http_WithProxy()
333334
{
334335
// Specify that we're using proxies.
335-
var rpcHttp = await CreateTestRpcHttp(new Dictionary<string, string>() { { "HttpUri", "something" } });
336+
var rpcHttp = await CreateTestRpcHttp(new Dictionary<string, string>() { { RpcWorkerConstants.HttpUri, "something" } });
336337

337338
// everything should come back empty
338339
Assert.Empty(rpcHttp.Url);
339340
Assert.Empty(rpcHttp.Headers);
340341
Assert.Empty(rpcHttp.Query);
341342
Assert.Null(rpcHttp.Body);
343+
Assert.Empty(rpcHttp.Params);
344+
Assert.Empty(rpcHttp.NullableParams);
342345
}
343346

344-
private async Task<RpcHttp> CreateTestRpcHttp(IDictionary<string, string> capabilities = null)
347+
[Fact]
348+
public async Task ToRpc_Http_WithProxy_HandleRouteParams_Capability()
349+
{
350+
// Specify that we're using proxies & worker can handle route params.
351+
var workerCapabilities = new Dictionary<string, string>()
352+
{
353+
{ RpcWorkerConstants.HttpUri, "http://localhost:1234" },
354+
{ RpcWorkerConstants.RequiresRouteParameters, bool.TrueString }
355+
};
356+
357+
var routeDataValues = new Dictionary<string, object>
358+
{
359+
{ "category", "computer" },
360+
{ "brand", "microsoft" }
361+
};
362+
var httpContextItems = new Dictionary<object, object>
363+
{
364+
[HttpExtensionConstants.AzureWebJobsHttpRouteDataKey] = routeDataValues
365+
};
366+
367+
var rpcHttp = await CreateTestRpcHttp(workerCapabilities, httpContextItems);
368+
369+
// Ensure the response includes route params
370+
Assert.Equal(2, rpcHttp.Params.Count);
371+
Assert.Equal("computer", rpcHttp.Params["category"]);
372+
Assert.Equal("microsoft", rpcHttp.Params["brand"]);
373+
374+
// everything else should come back empty
375+
Assert.Empty(rpcHttp.Url);
376+
Assert.Empty(rpcHttp.Headers);
377+
Assert.Empty(rpcHttp.Query);
378+
Assert.Null(rpcHttp.Body);
379+
}
380+
381+
private async Task<RpcHttp> CreateTestRpcHttp(IDictionary<string, string> capabilities = null, IDictionary<object, object> httpRequestContextItems = null)
345382
{
346383
var logger = new TestLogger("test");
347384
GrpcCapabilities grpcCapabilities = new GrpcCapabilities(logger);
@@ -354,6 +391,14 @@ private async Task<RpcHttp> CreateTestRpcHttp(IDictionary<string, string> capabi
354391
headers.Add("test-header", "test value");
355392
var request = HttpTestHelpers.CreateHttpRequest("POST", "http://local/test?a=b", headers: headers, body: "test body");
356393

394+
if (httpRequestContextItems is not null)
395+
{
396+
foreach (var item in httpRequestContextItems)
397+
{
398+
request.HttpContext.Items.Add(item.Key, item.Value);
399+
}
400+
}
401+
357402
var bindingData = new Dictionary<string, object>
358403
{
359404
{ "req", request },

0 commit comments

Comments
 (0)