karlvr/openapi-generator-plus-express-passport

Bug: Response header name string values are changed to camelCase

stevegoossens opened this issue · 6 comments

In openapi-generator-plus the toCodegenHeader function returns each header with a camelCase name, and an unchanged serializedName, e.g.

https://github.com/karlvr/openapi-generator-plus/blob/2e92cfe03829fac5d5f815d541b50d4aa438584f/packages/core/src/process/headers.ts#L65-L66

			name: state.generator.toIdentifier(name),
			serializedName: name,

and
https://github.com/karlvr/openapi-generator-plus/blob/2e92cfe03829fac5d5f815d541b50d4aa438584f/packages/core/src/process/headers.ts#L38-L39

			name: state.generator.toIdentifier(name),
			serializedName: name,

this is fine, except that this repo has the following in the api template:

res.header({{{stringLiteral name}}}, `${response.headers[{{{stringLiteral name}}}]}`)

which means that OpenAPI YAML source like:

      responses:
        '200':
          description: User access grant
          headers:
            Access-Control-Allow-Headers:
              schema:
                type: string
              example: Content-Type, Authorization, Accept, X-Requested-With
            Access-Control-Allow-Methods:
              schema:
                type: string
              example: GET, POST, PUT, LOGIN, DELETE
            Access-Control-Allow-Origin:
              schema:
                type: string
              example: '*'

gets converted via the api.hbs template to:

res.status(200)
res.header('accessControlAllowHeaders', `${response.headers['accessControlAllowHeaders']}`)
res.header('accessControlAllowMethods', `${response.headers['accessControlAllowMethods']}`)
res.header('accessControlAllowOrigin', `${response.headers['accessControlAllowOrigin']}`)

(note the header names have been converted to camelCase)

Solution

As serializedName is just the header name specified in the OpenAPI YAML spec, it can be used in api.hbs like:

res.header({{{stringLiteral serializedName}}}, `${response.headers[{{{stringLiteral name}}}]}`)

which results in the code generation being correct like:

res.status(200)
res.header('Access-Control-Allow-Headers', `${response.headers['accessControlAllowHeaders']}`)
res.header('Access-Control-Allow-Methods', `${response.headers['accessControlAllowMethods']}`)
res.header('Access-Control-Allow-Origin', `${response.headers['accessControlAllowOrigin']}`)
karlvr commented

@stevegoossens I'm just doing some maintenance on this today so I'm taking a look at this. Was the PR not good?

It fixes a bug that would benefit everyone (the bug currently prevents the use of the openapi-specced response headers in the generated code), so it's still good.

I just chose to include the same fix in another branch that provides errors thrown to the NextFunction, as I also needed an error response body and headers.

Ah I see what happened. After I merged my branch with header and error to NextFunction to my forked repo, I deleted the other branches.

I could revert and re-open as I think this bug fix is universally beneficial.

As a side note, I forked this repo to add behavior of passing errors to the Express next function to allow error response bodies and headers rather than the generated no content for 4xx and 5xx errors.

main...stevegoossens:openapi-generator-plus-express-passport:main

I've since realised I can just add:

# Custom templates (overrides)
customTemplates: custom-templates

and add some override templates to achieve the same thing.

I'll probably put my HttpStatusError custom class in the index.hbs and import from there in the overriding templates.

That was somewhat due to this repo not having a readme to show that customTemplates was an option, but I eventually found it in https://github.com/karlvr/openapi-generator-plus-generators/blob/main/packages/typescript-express-example-server/README.md

I think that readme could pretty much be copy-pasted into this repo with just the heading renamed

karlvr commented

@stevegoossens you're quite right, I noticed that yesterday when I was upgrading this module... I've ported the README as you suggested. A tool without documentation is pretty useless, thank you for your perseverance.

karlvr commented

@stevegoossens also if you think the error handling you've implemented makes sense in general, please feel free to make a PR. I've used this template once in a project with a lot of existing express code, so it was tuned for that use-case and my very limited knowledge of express!