Skip to content

Commit 673624c

Browse files
vbilopavvbilopav
committed
fix(tsclient): correct generated client for @body_parameter_name endpoints
The TypeScript client generator was broken for any endpoint using @body_parameter_name: - It emitted "body: request.<name>?" — the TS optional "?" suffix (added for parameters with a default or a custom type) leaked into the runtime property name, a syntax error. The query-exclusion key was likewise ["<name>?"], so the body parameter was not stripped from the query string. Now the bare converted name is used for both the body expression and the exclusion key. - It emitted a fetch body even for GET, which fetch forbids. A body is now only emitted for methods that can carry one (not GET); a GET body parameter (e.g. a server-filled HTTP Custom Type field forwarded by a proxy POST) is simply excluded from the query and not sent. - It matched @body_parameter_name only against the converted and actual names, so the expanded HTTP-type field name (e.g. _response_body) was ignored — the parameter leaked into the query and no body was emitted, even though the server resolved it. The generator now also matches the ExpandedName alias, consistent with the server-side body-parameter resolution. Tests: NpgsqlRestTests/TsClientTests/BodyParamGetTests.cs covers a GET endpoint (converted name) and a POST HTTP-Custom-Type endpoint targeted by the expanded name _response_body. Full suite green (2293). See changelog/v3.18.2.md. version.txt / npm not bumped yet.
1 parent 62ef5cf commit 673624c

3 files changed

Lines changed: 229 additions & 5 deletions

File tree

‎NpgsqlRestTests/TsClientTests/BodyParamGetTests.cs‎

Lines changed: 202 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,202 @@
1+
namespace NpgsqlRestTests
2+
{
3+
public static partial class Database
4+
{
5+
public static void TsClientBodyParamGetTests()
6+
{
7+
// GET endpoint with @body_parameter_name targeting a defaulted parameter (so it carries the
8+
// TS optional "?" suffix in the interface). Regression for three TsClient bugs:
9+
// 1) the body expression emitted "request.payload?" (trailing "?" — a syntax error),
10+
// 2) the query-exclusion key was ["payload?"] (with "?") so the param was NOT stripped,
11+
// 3) a fetch body was emitted for a GET (fetch forbids a body on GET).
12+
script.Append("""
13+
create schema if not exists tsclient_test;
14+
create function tsclient_test.bodyparam_get(_keyword text, _payload text default null)
15+
returns text
16+
language sql
17+
as $$
18+
select coalesce(_keyword, '') || coalesce(_payload, '');
19+
$$;
20+
comment on function tsclient_test.bodyparam_get(text, text) is '
21+
HTTP GET
22+
tsclient_module=bodyparam_get
23+
body_parameter_name payload';
24+
25+
-- POST endpoint with an HTTP Custom Type whose body field is targeted by @body_parameter_name using
26+
-- its EXPANDED signature name (_response_body). The generator must match that name (via ExpandedName)
27+
-- the same way the server does, emit the body with the bare converted name (request.responseBody),
28+
-- and exclude it from the query string. Regression for the generator only matching converted/actual
29+
-- names. The HTTP type points at an unused port; it never fires during code generation.
30+
create type tsclient_test.tsc_http_probe as (
31+
body text,
32+
status_code int,
33+
success boolean,
34+
error_message text
35+
);
36+
comment on type tsclient_test.tsc_http_probe is 'GET http://localhost:1/tsc-dummy';
37+
38+
create function tsclient_test.bodyparam_expanded(_response tsclient_test.tsc_http_probe default null)
39+
returns text
40+
language plpgsql
41+
as $$ begin return ''; end; $$;
42+
comment on function tsclient_test.bodyparam_expanded(tsclient_test.tsc_http_probe) is '
43+
HTTP POST
44+
tsclient_module=bodyparam_expanded
45+
body_parameter_name _response_body';
46+
""");
47+
}
48+
}
49+
}
50+
51+
namespace NpgsqlRestTests.TsClientTests
52+
{
53+
[Collection("TestFixture")]
54+
public class BodyParamGetTests
55+
{
56+
private const string Expected = """
57+
const baseUrl = "";
58+
const parseQuery = (query: Record<any, any>) => "?" + Object.keys(query ? query : {})
59+
.map(key => {
60+
const value = (query[key] != null ? query[key] : "") as string;
61+
if (Array.isArray(value)) {
62+
return value.map((s: string) => s ? `${key}=${encodeURIComponent(s)}` : `${key}=`).join("&");
63+
}
64+
return `${key}=${encodeURIComponent(value)}`;
65+
})
66+
.join("&");
67+
68+
interface ITsclientTestBodyparamGetRequest {
69+
keyword: string | null;
70+
payload?: string | null;
71+
}
72+
73+
74+
/**
75+
* function tsclient_test.bodyparam_get(
76+
* _keyword text,
77+
* _payload text DEFAULT NULL::text
78+
* )
79+
* returns text
80+
*
81+
* @remarks
82+
* comment on function tsclient_test.bodyparam_get is 'HTTP GET
83+
* tsclient_module=bodyparam_get
84+
* body_parameter_name payload';
85+
*
86+
* @param request - Object containing request parameters.
87+
* @returns {string}
88+
*
89+
* @see FUNCTION tsclient_test.bodyparam_get
90+
*/
91+
export async function tsclientTestBodyparamGet(
92+
request: ITsclientTestBodyparamGetRequest
93+
) : Promise<string> {
94+
const response = await fetch(baseUrl + "/api/tsclient-test/bodyparam-get" + parseQuery((({ ["payload"]: _1, ...rest }) => rest)(request)), {
95+
method: "GET",
96+
});
97+
return await response.text();
98+
}
99+
100+
101+
""";
102+
103+
private const string ExpectedExpanded = """
104+
const baseUrl = "";
105+
const parseQuery = (query: Record<any, any>) => "?" + Object.keys(query ? query : {})
106+
.map(key => {
107+
const value = (query[key] != null ? query[key] : "") as string;
108+
if (Array.isArray(value)) {
109+
return value.map((s: string) => s ? `${key}=${encodeURIComponent(s)}` : `${key}=`).join("&");
110+
}
111+
return `${key}=${encodeURIComponent(value)}`;
112+
})
113+
.join("&");
114+
115+
interface ITsclientTestBodyparamExpandedRequest {
116+
responseBody?: string | null;
117+
responseStatusCode?: number | null;
118+
responseSuccess?: boolean | null;
119+
responseErrorMessage?: string | null;
120+
}
121+
122+
123+
/**
124+
* function tsclient_test.bodyparam_expanded(
125+
* _response_body text DEFAULT NULL::tsclient_test.tsc_http_probe,
126+
* _response_status_code integer,
127+
* _response_success boolean,
128+
* _response_error_message text
129+
* )
130+
* returns text
131+
*
132+
* @remarks
133+
* comment on function tsclient_test.bodyparam_expanded is 'HTTP POST
134+
* tsclient_module=bodyparam_expanded
135+
* body_parameter_name _response_body';
136+
*
137+
* @param request - Object containing request parameters.
138+
* @returns {string}
139+
*
140+
* @see FUNCTION tsclient_test.bodyparam_expanded
141+
*/
142+
export async function tsclientTestBodyparamExpanded(
143+
request: ITsclientTestBodyparamExpandedRequest
144+
) : Promise<string> {
145+
const response = await fetch(baseUrl + "/api/tsclient-test/bodyparam-expanded" + parseQuery((({ ["responseBody"]: _1, ...rest }) => rest)(request)), {
146+
method: "POST",
147+
body: request.responseBody
148+
});
149+
return await response.text();
150+
}
151+
152+
153+
""";
154+
155+
[Fact]
156+
public void Test_BodyParamGet_GeneratedFile()
157+
{
158+
var filePath = Path.Combine(Setup.Program.TsClientOutputPath, "bodyparam_get.ts");
159+
File.Exists(filePath).Should().BeTrue($"Expected file at {filePath}");
160+
161+
var content = File.ReadAllText(filePath);
162+
163+
// The fixed generator must NOT emit the optional "?" suffix in the runtime name, and must
164+
// not attach a body to a GET request.
165+
content.Should().NotContain("request.payload?", "the body expression must not carry the TS optional suffix");
166+
content.Should().NotContain("[\"payload?\"]", "the query-exclusion key must be the bare property name");
167+
content.Should().NotContain("body:", "a GET request must not emit a fetch body");
168+
169+
// Full-content match. Normalize per-line trailing whitespace and line endings: the generator
170+
// emits trailing spaces on blank comment lines, which are easily mangled when transcribed.
171+
Normalize(content).Should().Be(Normalize(Expected));
172+
}
173+
174+
[Fact]
175+
public void Test_BodyParamExpanded_GeneratedFile()
176+
{
177+
var filePath = Path.Combine(Setup.Program.TsClientOutputPath, "bodyparam_expanded.ts");
178+
File.Exists(filePath).Should().BeTrue($"Expected file at {filePath}");
179+
180+
var content = File.ReadAllText(filePath);
181+
182+
// @body_parameter_name targeted the expanded signature name (_response_body); the generator
183+
// must resolve it to the body field and emit the body with the bare converted name.
184+
content.Should().Contain("body: request.responseBody", "the expanded name must resolve to the body field, emitted by its converted name");
185+
content.Should().Contain("[\"responseBody\"]", "the body parameter must be excluded from the query string");
186+
content.Should().NotContain("parseQuery(request)", "the body parameter must not be left in the unfiltered query");
187+
content.Should().NotContain("request.responseBody?", "no trailing optional suffix in the runtime name");
188+
189+
Normalize(content).Should().Be(Normalize(ExpectedExpanded));
190+
}
191+
192+
private static string Normalize(string s)
193+
{
194+
var lines = s.Replace("\r\n", "\n").Split('\n');
195+
for (var i = 0; i < lines.Length; i++)
196+
{
197+
lines[i] = lines[i].TrimEnd();
198+
}
199+
return string.Join("\n", lines);
200+
}
201+
}
202+
}

‎changelog/v3.18.2.md‎

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,9 +24,19 @@ Previously, an HTTP Custom Type whose `body` field held a large payload (e.g. a
2424

2525
Previously the annotation value was force-lowercased and compared case-sensitively, so the camelCase converted name never matched, and the expanded signature name (`_response_body`) matched nothing at all — it is stored as neither the actual nor the converted name. This made it impossible to redirect a single expanded HTTP-type field (such as the response body) into the proxy request body.
2626

27+
### 3. TypeScript client generation for `@body_parameter_name`
28+
29+
The generated TypeScript client was broken for an endpoint with `@body_parameter_name`:
30+
31+
- the body expression was emitted as `request.responseBody?` — a syntax error (the TS optional `?` suffix leaked into the runtime property name);
32+
- the query-string exclusion key was `["responseBody?"]`, so the body parameter was **not** stripped from the query string;
33+
- a `body` was emitted even for a `GET` request, which `fetch` forbids.
34+
35+
The generator now uses the parameter's bare name for the body expression and the exclusion key, only emits a `fetch` body for methods that can carry one (not `GET`), and — like the server — matches `@body_parameter_name` against the converted, actual, or expanded signature name of an HTTP Custom Type field (e.g. `responseBody`, `_response`, or `_response_body`).
36+
2737
### Why these go together
2838

29-
The pattern "fetch with an HTTP Custom Type, then `@proxy` to an upstream" now works cleanly: redirect the (large) body field into the upstream request body with `@body_parameter_name`, while the remaining small fields travel on the query string under the new length guard.
39+
The pattern "fetch with an HTTP Custom Type, then `@proxy` to an upstream" now works cleanly end to end — server **and** generated client: redirect the (large) body field into the upstream request body with `@body_parameter_name`, while the remaining small fields travel on the query string under the new length guard.
3040

3141
## Notes
3242

@@ -35,4 +45,4 @@ The pattern "fetch with an HTTP Custom Type, then `@proxy` to an upstream" now w
3545

3646
## Tests
3747

38-
`NpgsqlRestTests/ProxyTests/ProxyHttpTypeProbeTest.cs` adds three cases (WireMock proxy target echoing the received URL / body): body redirect by converted name (`responseBody`), body redirect by expanded signature name (`_response_body`), and an oversized HTTP-type body field skipped from the proxy query string. Full suite green (2291).
48+
`NpgsqlRestTests/ProxyTests/ProxyHttpTypeProbeTest.cs` adds three cases (WireMock proxy target echoing the received URL / body): body redirect by converted name (`responseBody`), body redirect by expanded signature name (`_response_body`), and an oversized HTTP-type body field skipped from the proxy query string. `NpgsqlRestTests/TsClientTests/BodyParamGetTests.cs` covers the generated client for `@body_parameter_name` endpoints: a `GET` case (no `?`-suffixed name, parameter excluded from the query, no `fetch` body on `GET`) and a `POST` HTTP-Custom-Type case targeted by the expanded name `_response_body` (body emitted as `request.responseBody`, excluded from the query). Full suite green (2293).

‎plugins/NpgsqlRest.TsClient/TsClient.cs‎

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -473,9 +473,17 @@ bool Handle(RoutineEndpoint endpoint)
473473
var nameSuffix = (descriptor.HasDefault || descriptor.CustomType is not null) ? "?" : "";
474474
paramNames[i] = QuoteJavaScriptVariableName($"{parameter.ConvertedName}{nameSuffix}");
475475
if (string.Equals(endpoint.BodyParameterName, parameter.ConvertedName, StringComparison.OrdinalIgnoreCase) ||
476-
string.Equals(endpoint.BodyParameterName, parameter.ActualName, StringComparison.OrdinalIgnoreCase))
476+
string.Equals(endpoint.BodyParameterName, parameter.ActualName, StringComparison.OrdinalIgnoreCase) ||
477+
(parameter.ExpandedName is not null && string.Equals(endpoint.BodyParameterName, parameter.ExpandedName, StringComparison.OrdinalIgnoreCase)))
477478
{
478-
bodyParameterName = paramNames[i];
479+
// Match the same three names the server-side body-parameter resolution accepts:
480+
// converted (responseBody), actual (the shared composite base, _response), and the
481+
// expanded per-field signature name (_response_body, via ExpandedName).
482+
// Use the bare converted name for emission — NOT paramNames[i], which carries the TS
483+
// optional "?" suffix (e.g. "responseBody?"). That suffix is only for the interface
484+
// property declaration; it must not leak into the runtime property name used for the
485+
// body expression (request.responseBody) or the query-exclusion key (["responseBody"]).
486+
bodyParameterName = parameter.ConvertedName;
479487
}
480488
}
481489
string requestDesc = "";
@@ -1137,7 +1145,11 @@ string NewLine(string? input, int ident) =>
11371145
lastContentHeaderWasUrl = false;
11381146
}
11391147

1140-
if (body is null && bodyParameterName is not null)
1148+
// Emit the request body for a designated body parameter only when the method can carry one.
1149+
// fetch() forbids a body on GET, so a GET endpoint with @body_parameter_name still excludes
1150+
// that parameter from the query string (above) but does not send it as a body — e.g. when it
1151+
// is server-filled (an HTTP Custom Type field) and forwarded by a proxy POST upstream.
1152+
if (body is null && bodyParameterName is not null && endpoint.Method != Method.GET)
11411153
{
11421154
body = $"body: request.{bodyParameterName}";
11431155
}

0 commit comments

Comments
 (0)