fix(esg): JSON-serialise non-scalar GET/DELETE query parameters (review 2026-10-21 05/O2)
This commit is contained in:
@@ -614,7 +614,7 @@ public class ExternalSystemClient : IExternalSystemClient
|
|||||||
var consumed = consumedParams;
|
var consumed = consumedParams;
|
||||||
var queryString = string.Join("&",
|
var queryString = string.Join("&",
|
||||||
parameters.Where(p => p.Value != null && !consumed.Contains(p.Key))
|
parameters.Where(p => p.Value != null && !consumed.Contains(p.Key))
|
||||||
.Select(p => $"{Uri.EscapeDataString(p.Key)}={Uri.EscapeDataString(p.Value?.ToString() ?? "")}"));
|
.Select(p => $"{Uri.EscapeDataString(p.Key)}={Uri.EscapeDataString(FormatQueryValue(p.Value))}"));
|
||||||
|
|
||||||
// Only append "?" when the effective query string is non-empty — a method
|
// Only append "?" when the effective query string is non-empty — a method
|
||||||
// whose parameter values are all null produces no query string, and the
|
// whose parameter values are all null produces no query string, and the
|
||||||
@@ -629,6 +629,31 @@ public class ExternalSystemClient : IExternalSystemClient
|
|||||||
return url;
|
return url;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// <summary>
|
||||||
|
/// Renders a GET/DELETE query-string value. Scalars use <c>ToString()</c>; collections,
|
||||||
|
/// dictionaries and other complex objects are JSON-serialised (as the POST/PUT/PATCH
|
||||||
|
/// branch already does) instead of degrading to a CLR type name such as
|
||||||
|
/// <c>System.Int32[]</c> (review 2026-10-21 05/O2).
|
||||||
|
/// </summary>
|
||||||
|
internal static string FormatQueryValue(object? value)
|
||||||
|
{
|
||||||
|
switch (value)
|
||||||
|
{
|
||||||
|
case null:
|
||||||
|
return string.Empty;
|
||||||
|
case string or char or bool or Guid or Uri or DateTime or DateTimeOffset or TimeSpan or Enum or decimal:
|
||||||
|
return value.ToString() ?? string.Empty;
|
||||||
|
case JsonElement { ValueKind: JsonValueKind.Object or JsonValueKind.Array } json:
|
||||||
|
return json.GetRawText();
|
||||||
|
case JsonElement scalar:
|
||||||
|
return scalar.ToString();
|
||||||
|
}
|
||||||
|
|
||||||
|
return value.GetType().IsPrimitive
|
||||||
|
? value.ToString() ?? string.Empty
|
||||||
|
: JsonSerializer.Serialize(value);
|
||||||
|
}
|
||||||
|
|
||||||
private void ApplyAuth(HttpRequestMessage request, ExternalSystemDefinition system)
|
private void ApplyAuth(HttpRequestMessage request, ExternalSystemDefinition system)
|
||||||
{
|
{
|
||||||
// Distinguish "intentionally unauthenticated" (AuthType = none)
|
// Distinguish "intentionally unauthenticated" (AuthType = none)
|
||||||
|
|||||||
@@ -782,6 +782,35 @@ public class ExternalSystemClientTests : IDisposable
|
|||||||
Assert.Contains("page=2", uri);
|
Assert.Contains("page=2", uri);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// <summary>Verifies that non-scalar GET parameters are JSON-serialised rather than rendered as CLR type names.</summary>
|
||||||
|
/// <returns>A task that represents the asynchronous operation.</returns>
|
||||||
|
[Fact]
|
||||||
|
public async Task Call_GetWithNonScalarParameters_JsonSerialisesThem()
|
||||||
|
{
|
||||||
|
// Review 2026-10-21 05/O2: new[]{1,2,3}.ToString() is "System.Int32[]".
|
||||||
|
var system = new ExternalSystemDefinition("TestAPI", "https://api.example.com", "none") { Id = 1 };
|
||||||
|
var method = new ExternalSystemMethod("search", "GET", "/search") { Id = 1, ExternalSystemDefinitionId = 1 };
|
||||||
|
StubResolution(system, method);
|
||||||
|
|
||||||
|
var handler = new RequestCapturingHandler(HttpStatusCode.OK, "{}");
|
||||||
|
_httpClientFactory.CreateClient(Arg.Any<string>()).Returns(new HttpClient(handler));
|
||||||
|
var client = new ExternalSystemClient(
|
||||||
|
_httpClientFactory, _repository, NullLogger<ExternalSystemClient>.Instance);
|
||||||
|
|
||||||
|
await client.CallAsync("TestAPI", "search", new Dictionary<string, object?>
|
||||||
|
{
|
||||||
|
["ids"] = new[] { 1, 2, 3 },
|
||||||
|
["f"] = new Dictionary<string, object?> { ["a"] = 1 },
|
||||||
|
["n"] = 7,
|
||||||
|
});
|
||||||
|
|
||||||
|
var uri = handler.LastUri!.AbsoluteUri;
|
||||||
|
Assert.Contains("ids=" + Uri.EscapeDataString("[1,2,3]"), uri);
|
||||||
|
Assert.Contains("f=" + Uri.EscapeDataString("{\"a\":1}"), uri);
|
||||||
|
Assert.Contains("n=7", uri);
|
||||||
|
Assert.DoesNotContain("System.", uri);
|
||||||
|
}
|
||||||
|
|
||||||
/// <summary>Verifies that a GET call whose parameters are all null does not append a trailing question mark.</summary>
|
/// <summary>Verifies that a GET call whose parameters are all null does not append a trailing question mark.</summary>
|
||||||
/// <returns>A task that represents the asynchronous operation.</returns>
|
/// <returns>A task that represents the asynchronous operation.</returns>
|
||||||
[Fact]
|
[Fact]
|
||||||
|
|||||||
Reference in New Issue
Block a user