swagger-api / swagger-api/swagger-codegen-generators

ApiClient.mustache :: setApiKey() a little unclear and perhaps not future proof

Open
#413 0 comments 0 reactions 0 assignees View on GitHub

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

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.