[DO NOT REVIEW] feat: support exploded syntax in sdk #2865

sdo-1A · 2025-02-18T16:17:08Z

Proposed change

Support of exploded syntax for query and path parameters: https://swagger.io/docs/specification/v3_0/serialization/

Related issues

- No issue associated -

nx-cloud · 2025-02-18T16:18:06Z

View your CI Pipeline Execution ↗ for commit 0dbb519.

Command	Status	Duration	Result
`nx run-many --target=test-int`	✅ Succeeded	24m 23s	View ↗
`nx run-many --target=test-e2e`	✅ Succeeded	10m 21s	View ↗
`nx run-many --target=build --projects=eslint-pl...`	✅ Succeeded	2s	View ↗
`nx run-many --target=publish --nx-bail --userco...`	✅ Succeeded	33s	View ↗
`nx run-many --target=prepare-publish --exclude-...`	✅ Succeeded	1m 15s	View ↗
`nx run-many --target=build,build-swagger`	✅ Succeeded	5m 48s	View ↗
`nx affected --target=test --collectCoverage`	✅ Succeeded	1m 1s	View ↗
`nx affected --target=lint`	✅ Succeeded	2m 36s	View ↗
`Additional runs (3)`	✅ Succeeded	...	View ↗

☁️ Nx Cloud last updated this comment at 2025-02-18 17:02:16 UTC

codecov · 2025-02-18T16:23:49Z

Codecov Report

Attention: Patch coverage is 66.05505% with 37 lines in your changes missing coverage. Please review.

Project coverage is 71.47%. Comparing base (d032a4d) to head (0dbb519).

✅ All tests successful. No failed tests found.

Files with missing lines	Patch %	Lines
packages/@ama-sdk/core/src/fwk/api.helpers.ts	76.25%	16 Missing and 3 partials ⚠️
...ges/@ama-sdk/core/src/clients/api-beacon-client.ts	0.00%	7 Missing ⚠️
...ages/@ama-sdk/core/src/clients/api-fetch-client.ts	0.00%	7 Missing ⚠️
...-sdk/core/src/plugins/si-token/si-token.request.ts	57.14%	0 Missing and 3 partials ⚠️
packages/@ama-sdk/core/src/utils/generic-api.ts	0.00%	1 Missing ⚠️

Additional details and impacted files

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

fpaul-1A · 2025-02-19T13:46:09Z

...cript/core/openapi-codegen-typescript/src/main/java/com/amadeus/codegen/ts/LambdaHelper.java

@@ -67,7 +67,7 @@ public UrlParamReplacerLambda() {}

      @Override
      public String formatFragment(String fragment) {
-          return fragment.replaceAll("\\{([\\w_-]+)\\}", "\\${data['$1']}");
+          return fragment.replaceAll("\\{([\\w_-]+)\\}", "\\${pathParameters['$1']}");


the doc of the class should be updated as well (same for the other one bellow)

fpaul-1A · 2025-02-19T13:48:10Z

packages/@ama-sdk/core/src/fwk/api.helpers.ts

+    ? Object.keys(object)
+      .filter((objectKey) => !!object[objectKey])
+      .reduce<{ [key: string]: ParamSerialization }>((acc, objectKey) => {
+        acc[objectKey] = object[objectKey] as ParamSerialization;
+        return acc;
+      }, {})


Suggested change

? Object.keys(object)

.filter((objectKey) => !!object[objectKey])

.reduce<{ [key: string]: ParamSerialization }>((acc, objectKey) => {

acc[objectKey] = object[objectKey] as ParamSerialization;

return acc;

}, {})

? Object.fromEntries(Object.entries(object).filter(([_key, value]) => typeof value !== 'undefined'))

(you can even remove _key:

Suggested change

? Object.keys(object)

.filter((objectKey) => !!object[objectKey])

.reduce<{ [key: string]: ParamSerialization }>((acc, objectKey) => {

acc[objectKey] = object[objectKey] as ParamSerialization;

return acc;

}, {})

? Object.fromEntries(Object.entries(object).filter(([, value]) => typeof value !== 'undefined'))

)

fpaul-1A · 2025-02-19T13:55:39Z

packages/@ama-sdk/core/src/fwk/api.helpers.ts

+  if (isPartialRecordTypeString(queryParameters)) {
+    const queryPart = Object.keys(queryParameters)
+      .filter((name) => typeof queryParameters[name] !== 'undefined')
+      .map((name) => `${name}=${queryParameters[name]!}`)
+      .join('&');
+
+    return url + (queryPart ? paramsPrefix + queryPart : '');
+  }
+
+  const filteredQueryParams = filterUndefinedQueryParams(queryParameters);


Suggested change

if (isPartialRecordTypeString(queryParameters)) {

const queryPart = Object.keys(queryParameters)

.filter((name) => typeof queryParameters[name] !== 'undefined')

.map((name) => `${name}=${queryParameters[name]!}`)

.join('&');

return url + (queryPart ? paramsPrefix + queryPart : '');

}

const filteredQueryParams = filterUndefinedQueryParams(queryParameters);

const filteredQueryParams = filterUndefinedQueryParams(queryParameters);

if (isRecordTypeString(filteredQueryParams)) {

const queryPart = Object.entries(filteredQueryParams)

.map(([name, value]) => `${name}=${value}`)

.join('&');

return url + (queryPart ? paramsPrefix + queryPart : '');

}

kpanot · 2025-02-19T14:19:17Z

packages/@ama-sdk/client-angular/src/api-angular-client.ts

@@ -75,7 +88,8 @@ export class ApiAngularClient implements ApiClient {
    let opts: RequestOptions = {
      ...requestOptionsParameters,
      headers: new Headers(filterUndefinedValues(requestOptionsParameters.headers)),
-      queryParams: filterUndefinedValues(requestOptionsParameters.queryParams)
+      queryParams: filterUndefinedValues(requestOptionsParameters.queryParams),


I think you have a breaking change here for the plugins:
Regarding your template update, you always pass {} as value for queryParams.
Which means that the external plugins implemented to treat queryParams will not have any effect anymore.

There is also another point to handle: even if the correct queryParams are provided to the plugin, the update of it by a plugin will be ignore (as queryParameters has the priority).
A suggestion (not the best one :S) can be to raise an error if the ref to the queryParams is different that the original one and if !!queryParameters

kpanot · 2025-02-19T14:22:45Z

packages/@ama-sdk/client-angular/src/api-angular-client.ts

+    if (Array.isArray(names)) {
+      return extractQueryParams(data, names);
+    }
    return extractQueryParams(data, names);


Why is it different to

Suggested change

if (Array.isArray(names)) {

return extractQueryParams(data, names);

}

return extractQueryParams(data, names);

return extractQueryParams(data, names);

?

I did this in order to target the correct function since this function has been overloaded, I check the type of the parameter to target either the deprecated version or the new version.

But it should not change the return type of extractQueryParams.
Did you face type issue without the Array.isArray?
(I was actually triggered because adding JS process to handle TypeScript type issue is generally not the good practice :S)

kpanot · 2025-02-19T14:26:55Z

packages/@ama-sdk/core/src/fwk/api.helpers.ts

+ * @param obj
+ */
+export function isRecordTypeString(obj: any): obj is { [key: string]: string } {
+  return Object.values(obj).every((item) => typeof item === 'string');


This is true for any type obj !== 'object' maybe you should check:

Suggested change

return Object.values(obj).every((item) => typeof item === 'string');

return typeof obj === 'object' && Object.values(obj).every((item) => typeof item === 'string');

kpanot · 2025-02-19T14:33:47Z

packages/@ama-sdk/core/src/fwk/api.helpers.ts

+    ? Object.keys(object)
+      .filter((objectKey) => !!object[objectKey])
+      .reduce<{ [key: string]: ParamSerialization }>((acc, objectKey) => {
+        acc[objectKey] = object[objectKey] as ParamSerialization;


Suggested change

acc[objectKey] = object[objectKey] as ParamSerialization;

acc[objectKey] = object[objectKey]!;

?

kpanot · 2025-02-19T14:39:01Z

packages/@ama-sdk/client-fetch/src/api-fetch-client.ts

    return extractQueryParams(data, names);
  }

  /** @inheritdoc */
-  public tokenizeRequestOptions(url: string, queryParameters: { [key: string]: string }, piiParamTokens: { [key: string]: string }, data: any): TokenizedOptions | undefined {
+  public tokenizeRequestOptions(


As other function you implemented, maybe an overload of this one to deprecate { [key: string]: string } can be valuable?

kpanot · 2025-02-19T14:41:44Z

You may want to add a deprecate: .... commit message to list your commit in deprecated warnings in release not

feat: support exploded syntax in sdk

0dbb519

github-actions bot added enhancement New feature or request project:@ama-sdk/core project:@ama-sdk/schematics project:@ama-sdk/client-fetch project:@ama-sdk/client-angular project:@ama-sdk/client-beacon labels Feb 18, 2025

sdo-1A marked this pull request as ready for review February 19, 2025 09:56

sdo-1A requested a review from a team as a code owner February 19, 2025 09:56

fpaul-1A reviewed Feb 19, 2025

View reviewed changes

kpanot reviewed Feb 19, 2025

View reviewed changes

sdo-1A changed the title ~~feat: support exploded syntax in sdk~~ [DO NOT REVIEW] feat: support exploded syntax in sdk Feb 20, 2025

kpanot marked this pull request as draft February 22, 2025 08:53

sdo-1A mentioned this pull request Feb 24, 2025

feat: support exploded syntax in sdk #2896

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[DO NOT REVIEW] feat: support exploded syntax in sdk #2865

[DO NOT REVIEW] feat: support exploded syntax in sdk #2865

sdo-1A commented Feb 18, 2025

nx-cloud bot commented Feb 18, 2025 •

edited

Loading

codecov bot commented Feb 18, 2025 •

edited

Loading

fpaul-1A Feb 19, 2025

fpaul-1A Feb 19, 2025

kpanot Feb 19, 2025

fpaul-1A Feb 19, 2025

kpanot Feb 19, 2025

kpanot Feb 19, 2025

sdo-1A Feb 20, 2025

kpanot Feb 20, 2025

kpanot Feb 19, 2025

kpanot Feb 19, 2025

kpanot Feb 19, 2025

kpanot commented Feb 19, 2025

	return Object.values(obj).every((item) => typeof item === 'string');
	return typeof obj === 'object' && Object.values(obj).every((item) => typeof item === 'string');

	acc[objectKey] = object[objectKey] as ParamSerialization;
	acc[objectKey] = object[objectKey]!;

[DO NOT REVIEW] feat: support exploded syntax in sdk #2865

Are you sure you want to change the base?

[DO NOT REVIEW] feat: support exploded syntax in sdk #2865

Conversation

sdo-1A commented Feb 18, 2025

Proposed change

Related issues

nx-cloud bot commented Feb 18, 2025 • edited Loading

codecov bot commented Feb 18, 2025 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kpanot commented Feb 19, 2025

nx-cloud bot commented Feb 18, 2025 •

edited

Loading

codecov bot commented Feb 18, 2025 •

edited

Loading