microsoft / kiota

OpenAPI based HTTP Client code generator
https://aka.ms/kiota/docs
MIT License
2.84k stars 194 forks source link

[Typescript] Explode option on QueryParam serialization is not taken into account #4487

Closed LaurentVandenbosch closed 4 months ago

LaurentVandenbosch commented 5 months ago

I have a query param defined as follows : image

The request sent by the typescript client generated serializes it as non-exploded : image

baywet commented 5 months ago

Hi @LaurentVandenbosch Thanks for using kiota and for reaching out. Can you share the URI template that was generated for that description please? ( should be in an index.ts file under a /api/catalog/item/fr-be/article-groups directory more or less) Thanks!

LaurentVandenbosch commented 5 months ago

Hi @baywet thank you for the response and sorry for the delay on my part. Here is what is being generated :

/**
 * Uri template for the request builder.
 */
export const ProductsRequestBuilderUriTemplate = "{+baseurl}/api/catalog/{contextId}/{languageCode}/products?articleGroupIds={articleGroupIds}";
baywet commented 5 months ago

Thanks for the additional information. This is caused because of the combination of required and explode.

Here are the different path templates that will be generated as of today depending on there parameters

required explode result
y n {+baseurl}/api/catalog/{contextId}/{languageCode}/products?articleGroupIds={articleGroupIds}
y y {+baseurl}/api/catalog/{contextId}/{languageCode}/products?articleGroupIds={articleGroupIds}
n n {+baseurl}/api/catalog/{contextId}/{languageCode}/products{?articleGroupIds}
n y {+baseurl}/api/catalog/{contextId}/{languageCode}/products{?articleGroupIds*}

Note: generating {+baseurl}/api/catalog/{contextId}/{languageCode}/products?articleGroupIds={articleGroupIds*} is strictly identical to {+baseurl}/api/catalog/{contextId}/{languageCode}/products?articleGroupIds={articleGroupIds*} from a RFC 6570 perspective since no operator is provided for the variable. And adding query style operator (? or &) results in an invalid URI.

The solution would be to "force" the parameter to be exploded even when it's required in this block

Is this something you'd be interested in submitting a pull request for?

ahusseini commented 4 months ago

@baywet seeing the same in c#

The solution would be to "force" the parameter to be exploded even when it's required in this block

What should the result look like if this were changed?

Something like this?

if (parameters.Length != 0)
{
    var queryParameters = string.Join(",", parameters.Select(static x => x.Name.SanitizeParameterNameForUrlTemplate() + (x.Explode ? "*" : string.Empty)));
    queryStringParameters = $"{{?{optionalParameters}}}";
}

Is the differentiation between "required" and "optional" needed in the way the template is being generated to preserve some sort of handling by Std.UriTemplate.Expand?

ex. would these be handled differently?

/resource{?requiredParam*,optionalParam} /resource?requiredParam={requiredParam}{?optionalParam}

baywet commented 4 months ago

@ahusseini we originally didn't have any differentiation for required parameters, but made the change at the request of @darrelmiller and community members so there'd be additional "validation" before the request goes out.

TBH I think this is ultimately a limitation of the RFC that doesn't provide a way to say something is both required and exploded.

Hence I suggested we made a small change to the template building logic to NOT use the required syntax when something is exploded.