swagger-api / swagger-api/swagger-codegen-generators
ApiClient.mustache :: setApiKey() a little unclear and perhaps not future proof
Nobody has claimed this yet.
- Dominant language
- Mustache
- Stars
- 299
- Forks
- 439
- PR merge metrics
- No merged PRs in 30d
Description
Hi there
Awesome project thanks! Saves an absolute tonne of work for me :-)
This issue is possibly just my lack of familiarity with the project, but I was confused with setApiKey method on ApiClient.mustache for java.
Unclear Part
It is a helper method to
set API key value for the first API key authentication.
This is the unclear part for me. Should the name of the method not reflect the intent a little clearer. e.g. setFirstApiKey or similar?
Just that I didn't actually realise this until diving into the code. Again, that's on me, but lazily using the client for 1 auth header works fine, but for 2 breaks ... sometimes, depending on the alphabetical order of the key names.
Future Proof
I believe the code does actually bind the user's code to the "hoped" iteration value - i.e. an unintended coupling to the internals of HashMap iterator order. Currently, and likely for ever given it's java, that will remain the same, but the spec says it can't be relied on. Also new unmodifiable HashMap created by Map.copyOf(...) in java 10 explicitly randomise the order for each JVM start up. So, this ApiClient could conceivably use a new Map implementation which does the same and breaks users' code.
I'm not entirely sure what I'd propose here - but basically control of the sorted order anyway.
I'd be happy to create a PR etc, but I thought I'd run this by you to check if it's worth looking at at all as it's partially an philosophical question.
Multiple Auth Headers
The general use case for multiple Authentication types - in my case multiple ApiKeyAuth. In this case I should just "get" the authentications map and modify directly? I think this was the part that confused me as it seemed like I was breaking the encapsulation provided by setApiKey
Obviously, if you agree is worth a look, then it applies to the other helper methods also. And I haven't checked widely throughout other generated code, but maybe beyond.
Anyway, thanks again.
Cheers,
Andy
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Review ApiClient.mustache for Java and trace the generated ApiClient.setApiKey helper together with its authentication map handling. Clarify the intended behavior for multiple ApiKeyAuth entries, naming, and iteration order with maintainers; done requires an agreed scope and corresponding generated-client coverage.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- java
- Domain
- api, authentication
- Issue type
- Feature
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Stale
- Clarity
- Needs clarification
- Newbie friendliness
- 25/100