diff --git a/docs/requirements/Component-ExternalSystemGateway.md b/docs/requirements/Component-ExternalSystemGateway.md index 1540528a..5b22d754 100644 --- a/docs/requirements/Component-ExternalSystemGateway.md +++ b/docs/requirements/Component-ExternalSystemGateway.md @@ -40,7 +40,7 @@ Each external system definition includes: - **Method Definitions**: List of available API methods, each with: - Method name. - **HTTP method**: GET, POST, PUT, PATCH, or DELETE. - - **Path**: Relative path appended to the base URL (e.g., `/recipes/{id}`). + - **Path**: Relative path appended to the base URL (e.g., `/recipes/{id}`). A path may contain `{param}` placeholders (identifier-named — letter/underscore start, then letters/digits/underscores). At invocation each placeholder is replaced with the URL-escaped value of the like-named parameter, and that parameter is **consumed**: it is removed from both the GET/DELETE query string and the POST/PUT/PATCH JSON body so it is never duplicated. A placeholder with no matching parameter, or one whose value is null, is an authoring error and raises a clear `ArgumentException` naming the placeholder. - Parameter definitions (name, type). Supports the extended type system (Boolean, Integer, Float, String, Object, List). - Return type definition. Supports the extended type system for complex response structures. @@ -72,7 +72,7 @@ Each database connection definition includes: All external system calls are **HTTP/REST** with **JSON** serialization: - The ESG acts as an HTTP client. The external system definition provides the base URL; each method definition specifies the HTTP method and relative path. -- Request parameters are serialized as JSON in the request body (POST/PUT/PATCH) or as query parameters (GET/DELETE). +- Request parameters are serialized as JSON in the request body (POST/PUT/PATCH) or as query parameters (GET/DELETE). Parameters consumed by `{param}` path-template substitution (see Method Definitions → Path) are carried in the URL path and excluded from both the body and the query string. - Response bodies are deserialized from JSON into the method's defined return type. - Credentials (API key header or Basic Auth header) are attached to every request per the system's authentication configuration. diff --git a/src/ZB.MOM.WW.ScadaBridge.ExternalSystemGateway/ExternalSystemClient.cs b/src/ZB.MOM.WW.ScadaBridge.ExternalSystemGateway/ExternalSystemClient.cs index 1b1bfcc0..a08bfd67 100644 --- a/src/ZB.MOM.WW.ScadaBridge.ExternalSystemGateway/ExternalSystemClient.cs +++ b/src/ZB.MOM.WW.ScadaBridge.ExternalSystemGateway/ExternalSystemClient.cs @@ -2,6 +2,7 @@ using System.Net; using System.Net.Http.Headers; using System.Text; using System.Text.Json; +using System.Text.RegularExpressions; using Microsoft.Extensions.Logging; using Microsoft.Extensions.Options; using ZB.MOM.WW.ScadaBridge.Commons.Entities.ExternalSystems; @@ -274,7 +275,7 @@ public class ExternalSystemClient : IExternalSystemClient // Timeout property is per-instance. client.Timeout = Timeout.InfiniteTimeSpan; - var url = BuildUrl(system.EndpointUrl, method.Path, parameters, method.HttpMethod); + var url = BuildUrl(system.EndpointUrl, method.Path, parameters, method.HttpMethod, out var consumedParams); // The request and response own IDisposable resources (StringContent, the // response content stream). Dispose both, including on the exception paths. @@ -288,10 +289,18 @@ public class ExternalSystemClient : IExternalSystemClient method.HttpMethod.Equals("PUT", StringComparison.OrdinalIgnoreCase) || method.HttpMethod.Equals("PATCH", StringComparison.OrdinalIgnoreCase)) { - if (parameters != null && parameters.Count > 0) + // Parameters consumed by `{param}` path-template substitution are + // already carried in the URL — exclude them from the JSON body so a + // path parameter is not duplicated as a body field. + var bodyParameters = consumedParams.Count == 0 + ? parameters + : parameters?.Where(p => !consumedParams.Contains(p.Key)) + .ToDictionary(p => p.Key, p => p.Value); + + if (bodyParameters != null && bodyParameters.Count > 0) { request.Content = new StringContent( - JsonSerializer.Serialize(parameters), + JsonSerializer.Serialize(bodyParameters), Encoding.UTF8, "application/json"); } @@ -432,14 +441,56 @@ public class ExternalSystemClient : IExternalSystemClient return value.Substring(0, maxChars) + $"… [truncated, {value.Length} chars total]"; } - private static string BuildUrl(string baseUrl, string path, IReadOnlyDictionary? parameters, string httpMethod) + /// + /// Matches a `{param}` path-template placeholder. The name must be a valid + /// identifier (letter/underscore start, then letters/digits/underscores) so the + /// pattern cannot accidentally swallow JSON-ish braces in a hand-authored path. + /// + private static readonly Regex PathParamRegex = new( + @"\{([A-Za-z_][A-Za-z0-9_]*)\}", RegexOptions.Compiled); + + private static string BuildUrl( + string baseUrl, + string path, + IReadOnlyDictionary? parameters, + string httpMethod, + out ISet consumedParams) { + consumedParams = new HashSet(StringComparer.Ordinal); + // A method that targets the base URL itself has an empty (or "/") path. // Appending a trailing "/" in that case yields ".../api/" which some // servers treat as a distinct resource — only append a segment when the // method actually defines a non-empty relative path. var trimmedBase = baseUrl.TrimEnd('/'); var trimmedPath = path.Trim().TrimStart('/'); + + // Substitute `{param}` placeholders in the path with escaped parameter + // values. Each substituted name is recorded so it is excluded from BOTH + // the query string (GET/DELETE) and the JSON body (POST/PUT/PATCH) — a + // path parameter must not be duplicated. A placeholder with no matching + // parameter (or a null value) is an authoring error: throw a clear + // ArgumentException naming it, mirroring ValidateHttpMethod. + if (trimmedPath.Length > 0 && trimmedPath.Contains('{')) + { + var consumed = consumedParams; + trimmedPath = PathParamRegex.Replace(trimmedPath, match => + { + var name = match.Groups[1].Value; + if (parameters == null || + !parameters.TryGetValue(name, out var value) || + value == null) + { + throw new ArgumentException( + $"Path template parameter '{{{name}}}' has no value — supply a non-null '{name}' parameter for this method.", + nameof(path)); + } + + consumed.Add(name); + return Uri.EscapeDataString(value.ToString() ?? string.Empty); + }); + } + var url = string.IsNullOrEmpty(trimmedPath) ? trimmedBase : trimmedBase + "/" + trimmedPath; @@ -449,8 +500,9 @@ public class ExternalSystemClient : IExternalSystemClient httpMethod.Equals("DELETE", StringComparison.OrdinalIgnoreCase)) && parameters != null && parameters.Count > 0) { + var consumed = consumedParams; var queryString = string.Join("&", - parameters.Where(p => p.Value != null) + parameters.Where(p => p.Value != null && !consumed.Contains(p.Key)) .Select(p => $"{Uri.EscapeDataString(p.Key)}={Uri.EscapeDataString(p.Value?.ToString() ?? "")}")); // Only append "?" when the effective query string is non-empty — a method diff --git a/tests/ZB.MOM.WW.ScadaBridge.ExternalSystemGateway.Tests/ExternalSystemClientTests.cs b/tests/ZB.MOM.WW.ScadaBridge.ExternalSystemGateway.Tests/ExternalSystemClientTests.cs index 9d165506..c2913966 100644 --- a/tests/ZB.MOM.WW.ScadaBridge.ExternalSystemGateway.Tests/ExternalSystemClientTests.cs +++ b/tests/ZB.MOM.WW.ScadaBridge.ExternalSystemGateway.Tests/ExternalSystemClientTests.cs @@ -1114,4 +1114,79 @@ public class ExternalSystemClientTests Assert.True(result.Success); } + + // ── ExternalSystemGateway: {param} path-template substitution ── + + [Fact] + public async Task PathTemplate_SubstitutesAndConsumesParameter() + { + // A `{id}` placeholder in the method path is replaced with the escaped + // parameter value and the consumed name is removed from the query string. + var system = new ExternalSystemDefinition("TestAPI", "https://api.example.com", "none") { Id = 1 }; + var method = new ExternalSystemMethod("getRecipe", "GET", "/recipes/{id}") { Id = 1, ExternalSystemDefinitionId = 1 }; + StubResolution(system, method); + + var handler = new RequestCapturingHandler(HttpStatusCode.OK, "{}"); + _httpClientFactory.CreateClient(Arg.Any()).Returns(new HttpClient(handler)); + + var client = new ExternalSystemClient( + _httpClientFactory, _repository, NullLogger.Instance); + + await client.CallAsync("TestAPI", "getRecipe", new Dictionary + { + ["id"] = "R 1", + ["filter"] = "x", + }); + + var uri = handler.LastUri!.AbsoluteUri; + // The value is substituted into the path (escaped) and consumed — it must + // NOT reappear in the query string; the remaining parameter still does. + Assert.Equal("https://api.example.com/recipes/R%201?filter=x", uri); + Assert.DoesNotContain("id=", uri); + } + + [Fact] + public async Task PathTemplate_Post_ConsumedParamExcludedFromBody() + { + // A path-template parameter consumed by a POST path must not also be + // serialized into the JSON body. + var system = new ExternalSystemDefinition("TestAPI", "https://api.example.com", "none") { Id = 1 }; + var method = new ExternalSystemMethod("addLine", "POST", "/orders/{orderId}/lines") { Id = 1, ExternalSystemDefinitionId = 1 }; + StubResolution(system, method); + + var handler = new RequestCapturingHandler(HttpStatusCode.OK, "{}"); + _httpClientFactory.CreateClient(Arg.Any()).Returns(new HttpClient(handler)); + + var client = new ExternalSystemClient( + _httpClientFactory, _repository, NullLogger.Instance); + + await client.CallAsync("TestAPI", "addLine", new Dictionary + { + ["orderId"] = 5, + ["qty"] = 2, + }); + + Assert.Equal("https://api.example.com/orders/5/lines", handler.LastUri!.ToString()); + Assert.Contains("\"qty\":2", handler.LastBody); + Assert.DoesNotContain("orderId", handler.LastBody); + } + + [Fact] + public async Task PathTemplate_MissingParameter_ThrowsClearError() + { + // A `{id}` placeholder with no matching parameter is an authoring error — + // reject it with a clear ArgumentException naming the placeholder. + var system = new ExternalSystemDefinition("TestAPI", "https://api.example.com", "none") { Id = 1 }; + var method = new ExternalSystemMethod("getRecipe", "GET", "/recipes/{id}") { Id = 1, ExternalSystemDefinitionId = 1 }; + StubResolution(system, method); + + _httpClientFactory.CreateClient(Arg.Any()).Returns(new HttpClient(new RequestCapturingHandler(HttpStatusCode.OK, "{}"))); + + var client = new ExternalSystemClient( + _httpClientFactory, _repository, NullLogger.Instance); + + var ex = await Assert.ThrowsAsync( + () => client.CallAsync("TestAPI", "getRecipe")); + Assert.Contains("id", ex.Message); + } }