search

criteo / criteo-api-java-sdk

1 stars 5 forks source link

Duplicated OAuth params #15

Closed Solodye closed 5 months ago

Solodye commented 6 months ago

Here OAuth should be only one string: https://github.com/criteo/criteo-api-java-sdk/blob/3400c5bb4d0a636e5a2210e6bd8b4611a8023e9c/sdks/marketingsolutions_preview/src/main/java/com/criteo/api/marketingsolutions/preview/api/CatalogApi.java#L140

Here OAuth also should be only one: https://github.com/criteo/criteo-api-java-sdk/blob/3400c5bb4d0a636e5a2210e6bd8b4611a8023e9c/sdks/marketingsolutions_preview/src/main/java/com/criteo/api/marketingsolutions/preview/ApiClient.java#L98-L112

gturri commented 5 months ago

hi, and thanks for this bug report.

FTR: the code is this repo is auto-generated by https://github.com/criteo/criteo-api-sdk-generator ; which itself relies on OpenApiGenerator.

About the duplicated line authentications.put("oauth", new OAuth());: it is generated by this piece of OpenApiGenerator template file: https://github.com/OpenAPITools/openapi-generator/blob/v6.3.0/modules/openapi-generator/src/main/resources/Java/ApiClient.mustache#L126 which loops on the authMethods variable.

It appears that OpenApiGenerator considers that this variable contains "OAuth" twice because our OpenApi specification has two entries for oauth (clientCredentials and authorizationCode).

So, sure, that part of the generated code does not look great, and we could consider opening a bug report or a pull request to OpenApiGenerator. But it would be a bit tedious (it would not even be sure such pull request would be accepted) and wouldn't be available before a while anyway.

(That's the same explanation for the duplication on String[] localVarAuthNames = new String[] { "oauth", "oauth" };, which is generated by https://github.com/OpenAPITools/openapi-generator/blob/v6.3.0/modules/openapi-generator/src/main/resources/Java/api.mustache#L111 )

So, unless this bothers you (in which case I would appreciate more details on the issue you would be facing) I would suggest to leave it like that (since my understanding is that, beside the beauty of the code, the impact is pretty limited).

I'm hence closing this issue, but feel free to reopen it if you consider this should be fixed nevertheless.