[FIX] Fix client generation when custom header are defined

[gquadrati]

Description

Two different problems have been detected:

1. if an header parameter is optional, its generated name contains "?" at the end of the name, so the generated code does not compile

2. if an header parameter is defined at path level, generated code does not compile:
   Example:

// Request type definition
export type TestHeaderParametersAtPathLevelT = r.IGetApiRequestType<
  { readonly "x-header-param": string },
  "x-header-param",
  // ^^^^^^^        ERROR: Type '"x-header-param"' does not satisfy 'RequestHeaderKey'.
  never,
  | r.IResponseType<200, undefined, never>
  | r.IResponseType<500, undefined, never>
>;

This because IGetApiRequestType from @pagopa/ts-commons is defined as

export interface IGetApiRequestType<P, KH extends RequestHeaderKey, Q extends string, R> extends IBaseApiRequestType<"get", P, KH, Q, R> {
    readonly method: "get";
}

where RequestHeaderKey is

export declare type RequestHeaderKey = "Accept-Encoding" | "Authorization" | "Content-Type" | "Host" | "If-None-Match" | "Ocp-Apim-Subscription-Key" | "X-Functions-Key";

So we expect KH type to be a subset of RequestHeaderKey.

The workaround is, just like what happen for method header parameters, to ignore headers other that the expected ones in request type definition.

Two failing tests have been added.

Motivation and Context

This PR is intended to fix issue #269

How Has This Been Tested?

• added unit test and e2e tests that break the current code
• Generated the openapi referenced in issue and built pagopa-api-config-fe project corrctly.

node dist/commands/gen-api-models/cli --api-spec https://raw.githubusercontent.com/pagopa/pagopa-api-config/main/openapi/swagger.json --no-strict  --out-dir ../pagopa-api-config-fe/generated/api  --request-types --response-decoders --client

Screenshots (if appropriate):

Types of changes

• Chore (improvement with no change in the behaviour)
• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to not work as expected)

Checklist:

• My change requires a change to the documentation.
• I have updated the documentation accordingly.

[pagopa-github-bot]

	Warnings
⚠️	Please include a Pivotal story at the beginning of the PR title (see below).

Example of PR titles that include pivotal stories:

• single story: [#123456] my PR title
• multiple stories: [#123456,#123457,#123458] my PR title

Generated by 🚫 dangerJS

[balanza]

If I get it right, this strips off all non-standard headers. My concerns are this is done silently, so the user will never know about it. Am I right?

Which makes me think about... do we need this concept of "standard headers"? What's that for?

[gquadrati]

If I get it right, this strips off all non-standard headers. My concerns are this is done silently, so the user will never know about it. Am I right?

You're right, but please notice that somehow it's already happening when the header is defined at method level.
You can see the openapi and the related snapshot.

Which makes me think about... do we need this concept of "standard headers"? What's that for?

Standard headers are the ones defined in @pagopa/ts-commons, but I don't know why this restriction was setup in the first place.
Maybe we can think about upgrading that type to a more inclusive one.

[balanza]

What if we just accept any string, and let the "validation" to happen on the input specification? I mean: an invalid header should not be included in the spec in the first place.

Maybe @gunzip remembers the need for such a constraint.

[gunzip]

no as it was set up by @cloudify

[balanza]

@gquadrati to summarize, what are the changes expected from this PR?

When...	Before	After
...a custom header is specified	Code is generated, but then fails to build	Code is generated but custom headers are removed silently

[gquadrati]

Yes, and actually there is another change:

When...	Before	After
...an optional header is specified	Code is generated, but then fails to build	Code is generated considering the header parameter as optional

[balanza]

@gquadrati about custom headers: I see there will be no real-life changes to actual users, still we're producing a result which silently ignores users' input. It's a red-flag for me as it may lead to very unexpected issues, I'd rather fail the code generation.
Anyway I'm fine with the proposed changes, too, so we can merge if you think it's ok. Whatever you decide, I advice to place a good comment on that very piece of code to explain why we're doing that.

Let's discuss the need for restrict which header a user can define on another PR. I see no value nowadays, we may decide is a useless constraint.

[gquadrati]

I don't like the idea to make the generation fail as it's not a user problem, he has done everything right.
I don't like this solution neither, but I suppose changing @pagopa/ts-commons types will require a bit of attention.

We can propose the user to put headers as method parameter and not path param, and just fix the optional header issue.
@balanza What do you think about it?

[balanza]

I think we can merge code as is and then we think about remove the constraint on standard headers.

Rationale: users should be able to provide any OpenAPI spec as long as it's valid, it's not this module's responsibility to argue on what the user puts in the spec.

[gquadrati]

fine for me.

[balanza]

Maybe this is no longer a need after #286 @gquadrati

[gquadrati]

No more useful