From d1596f8659afa0ee162c613d57cbc51ec4f8085f Mon Sep 17 00:00:00 2001 From: Benjamin Duffield Date: Tue, 23 Jul 2019 23:27:20 -0400 Subject: [PATCH 1/5] add failing test --- .../__tests__/serviceGeneratorTest.ts | 39 +++++++++++++++++++ 1 file changed, 39 insertions(+) diff --git a/src/commands/generate/__tests__/serviceGeneratorTest.ts b/src/commands/generate/__tests__/serviceGeneratorTest.ts index 6df4c59b..925441f9 100644 --- a/src/commands/generate/__tests__/serviceGeneratorTest.ts +++ b/src/commands/generate/__tests__/serviceGeneratorTest.ts @@ -261,6 +261,45 @@ describe("serviceGenerator", () => { assertOutputAndExpectedAreEqual(outDir, expectedDir, "services/paramTypeService.ts"); }); + it("handles out of order path params", async () => { + await generateService( + { + endpoints: [ + { + args: [ + { + argName: "param1", + markers: [], + paramType: { + path: {}, + type: "path", + }, + type: stringType, + }, + { + argName: "param2", + markers: [], + paramType: { + path: {}, + type: "path", + }, + type: stringType, + }, + ], + endpointName: "foo", + httpMethod: HttpMethod.GET, + httpPath: "/{param2}/{param1}", + markers: [], + }, + ], + serviceName: { name: "OutOfOrderPathService", package: "com.palantir.services" }, + }, + new Map(), + simpleAst, + ); + assertOutputAndExpectedAreEqual(outDir, expectedDir, "services/outOfOrderPathService.ts"); + }); + it("handles header auth-type", async () => { await generateService( { From 91f11c9929bd4d8e7261899cb89da0ebeb5155ff Mon Sep 17 00:00:00 2001 From: Benjamin Duffield Date: Tue, 23 Jul 2019 23:27:45 -0400 Subject: [PATCH 2/5] generate for failing test + manually edit to match desired output --- .../services/outOfOrderPathService.ts | 30 +++++++++++++++++++ 1 file changed, 30 insertions(+) create mode 100644 src/commands/generate/__tests__/resources/services/outOfOrderPathService.ts diff --git a/src/commands/generate/__tests__/resources/services/outOfOrderPathService.ts b/src/commands/generate/__tests__/resources/services/outOfOrderPathService.ts new file mode 100644 index 00000000..2f528609 --- /dev/null +++ b/src/commands/generate/__tests__/resources/services/outOfOrderPathService.ts @@ -0,0 +1,30 @@ +import { IHttpApiBridge, MediaType } from "conjure-client"; + +export interface IOutOfOrderPathService { + foo(param1: string, param2: string): Promise; +} + +export class OutOfOrderPathService { + constructor(private bridge: IHttpApiBridge) { + } + + public foo(param1: string, param2: string): Promise { + return this.bridge.callEndpoint({ + data: undefined, + endpointName: "foo", + endpointPath: "/{param2}/{param1}", + headers: { + }, + method: "GET", + pathArguments: [ + param2, + + param1, + ], + queryArguments: { + }, + requestMediaType: MediaType.APPLICATION_JSON, + responseMediaType: MediaType.APPLICATION_JSON, + }); + } +} From 79ab7c77d54d7c0b89643339b7e8715106c0e456 Mon Sep 17 00:00:00 2001 From: Benjamin Duffield Date: Tue, 23 Jul 2019 23:27:55 -0400 Subject: [PATCH 3/5] fix path param ordering --- src/commands/generate/serviceGenerator.ts | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/src/commands/generate/serviceGenerator.ts b/src/commands/generate/serviceGenerator.ts index 34be738a..117e6371 100644 --- a/src/commands/generate/serviceGenerator.ts +++ b/src/commands/generate/serviceGenerator.ts @@ -165,6 +165,13 @@ function generateEndpointBody( } }); + const pathParamsFromPath = parsePathParamsFromPath(endpointDefinition.httpPath); + pathParamsFromPath.forEach(pathParam => { + if (pathArgNames.indexOf(pathParam) === -1) { + throw new Error("path parameter present in path template but not provided as endpoint param: " + pathParam); + } + }); + if (bodyArgs.length > 1) { throw Error("endpoint cannot have more than one body arg, found: " + bodyArgs.length); } @@ -201,7 +208,7 @@ function generateEndpointBody( writer.writeLine(`method: "${endpointDefinition.httpMethod}",`); writer.write("pathArguments: ["); - pathArgNames.forEach(pathArgName => writer.indent().writeLine(pathArgName + ",")); + pathParamsFromPath.forEach(pathArgName => writer.indent().writeLine(pathArgName + ",")); writer.writeLine("],"); writer.write("queryArguments: {"); @@ -222,3 +229,13 @@ function mediaType(conjureType?: IType) { } return "MediaType.APPLICATION_JSON"; } + +function parsePathParamsFromPath(httpPath: string): string[] { + // first fix up the path to remove any ':.+' stuff in path params + const fixedPath = httpPath.replace(/{(.*):[^}]*}/, "{$1}"); + // follow-up by just pulling out any path segment with a starting '{' and trailing '}' + return fixedPath + .split("/") + .filter(segment => segment.startsWith("{") && segment.endsWith("}")) + .map(segment => segment.slice(1, -1)); +} From d1c37162225c85a435e5b8c57da898f8443bdd3b Mon Sep 17 00:00:00 2001 From: Benjamin Duffield Date: Thu, 25 Jul 2019 10:28:40 -0400 Subject: [PATCH 4/5] remove unnecessary validation --- src/commands/generate/serviceGenerator.ts | 5 ----- 1 file changed, 5 deletions(-) diff --git a/src/commands/generate/serviceGenerator.ts b/src/commands/generate/serviceGenerator.ts index 117e6371..285dab3e 100644 --- a/src/commands/generate/serviceGenerator.ts +++ b/src/commands/generate/serviceGenerator.ts @@ -166,11 +166,6 @@ function generateEndpointBody( }); const pathParamsFromPath = parsePathParamsFromPath(endpointDefinition.httpPath); - pathParamsFromPath.forEach(pathParam => { - if (pathArgNames.indexOf(pathParam) === -1) { - throw new Error("path parameter present in path template but not provided as endpoint param: " + pathParam); - } - }); if (bodyArgs.length > 1) { throw Error("endpoint cannot have more than one body arg, found: " + bodyArgs.length); From 7473419bec7741e441a72a27a6d8c3c2d3d0d4d5 Mon Sep 17 00:00:00 2001 From: Benjamin Duffield Date: Fri, 26 Jul 2019 11:47:29 -0400 Subject: [PATCH 5/5] manual changelog --- changelog/@unreleased/path-param-ordering.v2.yml | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 changelog/@unreleased/path-param-ordering.v2.yml diff --git a/changelog/@unreleased/path-param-ordering.v2.yml b/changelog/@unreleased/path-param-ordering.v2.yml new file mode 100644 index 00000000..e8750e7b --- /dev/null +++ b/changelog/@unreleased/path-param-ordering.v2.yml @@ -0,0 +1,5 @@ +type: fix +fix: + description: Fix a problem with the generated code in the case where path params show up in different orders in the path template and the args block. + links: + - https://github.com/palantir/conjure-typescript/pull/100