fix: sdk exploded syntax - take into account request plugin options

[sdo-1A]

Proposed change

When we previously implemented the exploded syntax for SDK parameter serialization, we removed the possibility of taking into account the query parameters that can be updated in the Request Plugins - which needs to be fixed. Here is a possible fix, which prepares the URL using the request plugins options (which could update the query parameters).

Also, the parameter queryParamSerialization has been added to RequestOptionsParameters, which can be used in order to serialize the query parameter modifications that are potentially made. In another PR, I will add a method allowing to deserialize the parameters, which facilitates the modifications in the Request Plugins.

Related issues

- No issue associated -

[nx-cloud]

View your CI Pipeline Execution ↗ for commit f00d17a.

Command	Status	Duration	Result
nx run-many --target=test-e2e	❌ Failed	9m 56s	View ↗
nx run-many --target=test-int	✅ Succeeded	20m 54s	View ↗
nx run-many --target=build --projects=eslint-pl...	✅ Succeeded	1s	View ↗
nx run-many --target=publish --nx-bail --userco...	✅ Succeeded	31s	View ↗
nx run-many --target=prepare-publish --exclude-...	✅ Succeeded	1m 26s	View ↗
nx run-many --target=build,build-swagger	✅ Succeeded	6m 8s	View ↗
nx affected --target=test --collectCoverage	✅ Succeeded	19s	View ↗
nx run ama-sdk-schematics:build-swagger	✅ Succeeded	3s	View ↗
Additional runs (2)	✅ Succeeded	...	View ↗

---

☁️ Nx Cloud last updated this comment at 2025-04-09 15:13:07 UTC

[codecov]

Codecov Report

Attention: Patch coverage is 91.48936% with 4 lines in your changes missing coverage. Please review.

Project coverage is 69.98%. Comparing base (c894748) to head (f00d17a).
Report is 2 commits behind head on release/12.1.

✅ All tests successful. No failed tests found.

Files with missing lines	Patch %	Lines
...kages/@ama-sdk/core/src/fwk/param-serialization.ts	89.47%	0 Missing and 2 partials ⚠️
...ges/@ama-sdk/core/src/clients/api-beacon-client.ts	0.00%	1 Missing ⚠️
...ages/@ama-sdk/core/src/clients/api-fetch-client.ts	0.00%	1 Missing ⚠️

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

🚀 New features to boost your workflow:

• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[cpaulve-1A]

We should probably consider also if the character is not encoded or at least document it

[cpaulve-1A]

Suggested change

	obj[paramSerialization.style === 'deepObject' ? key.split(OPENING_SQUARE_BRACKET_URL_CODE)[1].split(CLOSING_SQUARE_BRACKET_URL_CODE)[0] : key] = value;
	obj[paramSerialization.style === 'deepObject' ? key.match(`^${OPENING_SQUARE_BRACKET_URL_CODE})([^${CLOSING_SQUARE_BRACKET_URL_CODE}]+)`)[1]` : key] = value;

[sdo-1A]

According to the OpenAPI documentation, special characters should be encoded since this is required by RFC6570 and RFC3986, but I can document this yes

[cpaulve-1A]

Please document it just in case

[cpaulve-1A]

You should log something if the combination exploded/style is not supported

[sdo-1A]

I previously added logs but we had decided these would saturate the flow, but we can rediscuss this if you find it beneficial

[sdo-1A]

After discussion, we decided to add these logs in another PR if necessary

[fpaul-1A]

since this is used several times we could extract the code in a dedicated function

export function objectFromPairwiseArray<T>(inputArray: T[]): Record<string, T> {
  return inputArray.reduce((obj: { [key: string]: T}, currentValue, index, array) => {
    if (index % 2 === 0) {
      obj[currentValue] = array[index + 1];
    }
    return obj;
  }, []);
}

Suggested change

	return splitObject.reduce((obj: { [key: string]: PrimitiveType }, currentValue, index, array) => {
	if (index % 2 === 0) {
	obj[currentValue] = array[index + 1];
	}
	return obj;
	}, {} as { [key: string]: PrimitiveType });
	return objectFromPairwiseArray(splitObject);

or something like that

[sdo-1A]

this is used only twice (deserialization of object query and path parameters), I am not sure if it is worth creating another function in the code but if it makes it easier to read/understand then yes I can make the update

[cpaulve-1A]

Why do you use the ArrayPathParams method to deserialize an object?

[cpaulve-1A]

Even if this is a similar logic, it seems confusing when reading the code.

[sdo-1A]

Since the delimiter of the serialized object parameter is the same as the delimiter of array parameters:
https://swagger.io/specification/#style-examples
I did this to avoid code duplicates

[sdo-1A]

@cpaulve-1A I added a comment to explain, hoping this makes it a bit clearer, what do you think?

[cpaulve-1A]

I would rename the method to avoid confusion or duplicate code.

[cpaulve-1A]

You could rename it something like splitParameterElements

[cpaulve-1A]

We could have a comment here to explain the logic between the alternative key/value pairs

[sdo-1A]

Same as the comment below, I added the link to the OpenAPI documentation in the function description but I can add description comments if this is not enough

[cpaulve-1A]

   myParam.matchAll(`${outerDelimiter}?[^${innerDelimiter}]+${innerDelimiter}[^${outerDelimiter}]+${outerDelimiter}?`)
.reduce(obj, serializedProperty => {
   const splitProperty = serializedProperty.match('^${outerDelimiter}?([^${innerDelimiter}]+)${innerDelimiter}([^${outerDelimiter}]+)${outerDelimiter}?
});
  obj[splitProperty[1]] = splitProperty[2];

But I am not sure it is clearer (it works for all the use case I think if you remove the prefix)

[sdo-1A]

@cpaulve-1A let me know if the added comments make it a bit easier to understand the logic

[sdo-1A]

After reflection, we have decided to implement the deserialization of the parameters in a separate PR since these functions are beneficial but non-essential. This will allow further time for discussions on the implementation.
Also, this deserialization feature does not need to target release/12.1 necessarily so this new PR will target the main branch.

UPDATE: The separate has been created: #3116

did not realize the breaking for our current user

[fpaul-1A]

should we log/throw an error in case both data.paramSerializationOptions?.enableParameterSerialization and isParamValueRecord(queryParams) are false?
I assume here the params are silently ignored

[sdo-1A]

I could throw an error yes if you find this helpful, I was not sure when to add error logs or not

[sdo-1A]

updated

[sdo-1A]

To facilitate the review of this file, I will explain my logic here (small tip: the file is easier to read in unified mode):

• All of the query / path parameter logic is grouped together since these params are not needed for the headers or body
• queryParams will always be created and initialized as an empty object {}
• paramSerializationOptions will always be created with the value of this.client.options.enableParameterSerialization since this is passed to the request options
• If there are path parameters, we will create the if statement to allow path parameter serialization
  • Within this if statement, we check if there are query params as well to add their serialization logic if needed
• If there are no path parameters, the variables basePath and tokenizedUrl are initialized as they were before
  • We check if there are query parameters to add the same logic as before (an if statement with the serialization logic)