Skip to content

[Go] properly set header parameters on api clients#14586

Closed
sokolikp wants to merge 2 commits intoOpenAPITools:masterfrom
sokolikp:sokolik/go-template-fix
Closed

[Go] properly set header parameters on api clients#14586
sokolikp wants to merge 2 commits intoOpenAPITools:masterfrom
sokolikp:sokolik/go-template-fix

Conversation

@sokolikp
Copy link
Copy Markdown
Contributor

@sokolikp sokolikp commented Feb 2, 2023 •
edited
Loading

Context

This PR addresses #14578. Request headers were being improperly added as query params for Go clients. This PR:

  1. Updates the name of the function parameterAddToQuery (added in [Go] report correctly the parameters with the deep object specification #13909) to parameterAddToHeaderOrQuery to indicate that a header or query interface arg can be passed
  2. Correctly passes localVarHeaderParams (instead of localVarQueryParams) to the above function for header params
  3. Regenerates Go samples based on the above. Note that there are some automatic formatting/indentation changes to the samples - I can update those if necessary.

PR checklist

  • Read the contribution guidelines.
  • Pull Request title clearly describes the work in the pull request and Pull Request description provides details about how to validate the work. Missing information here may result in delayed response from the community.
  • Run the following to build the project and update samples: 
    ./mvnw clean package 
./bin/generate-samples.sh
./bin/utils/export_docs_generators.sh
    Commit all changed files.
    This is important, as CI jobs will verify all generator outputs of your HEAD commit as it would merge with master.
    These must match the expectations made by your contribution.
    You may regenerate an individual generator by passing the relevant config(s) as an argument to the script, for example ./bin/generate-samples.sh bin/configs/java*.
    For Windows users, please run the script in Git BASH.
  • File the PR against the correct branch: master (6.3.0) (minor release - breaking changes with fallbacks), 7.0.x (breaking changes without fallbacks)
  • If your PR is targeting a particular programming language, @mention the technical committee members, so they are more likely to review the pull request.

@sokolikp sokolikp force-pushed the sokolik/go-template-fix branch from ccfc692 to 5381ceb Compare February 2, 2023 01:37
@sokolikp sokolikp marked this pull request as ready for review February 2, 2023 02:42
@sokolikp
Copy link
Copy Markdown
Contributor Author

sokolikp commented Feb 2, 2023 •
edited
Loading

cc Go maintainers @antihax @grokify @kemokemo @jirikuncar @ph4r5h4d. I believe I'll have some test failures that I'll need to resolve, but I'm having a difficult time getting the Swagger tests to run locally.

@wing328
Copy link
Copy Markdown
Member

wing328 commented Feb 2, 2023

@sokolikp can you please PM me via Slack when you've time so that I can help you with the test?

https://join.slack.com/t/openapi-generator/shared_invite/zt-12jxxd7p2-XUeQM~4pzsU9x~eGLQqX2g

@sokolikp
Copy link
Copy Markdown
Contributor Author

sokolikp commented Feb 2, 2023 •
edited
Loading

@sokolikp can you please PM me via Slack when you've time so that I can help you with the test?

https://join.slack.com/t/openapi-generator/shared_invite/zt-12jxxd7p2-XUeQM~4pzsU9x~eGLQqX2g

@wing328 thanks, I'll try pinging tomorrow! Logging off for the night.

@sokolikp sokolikp changed the title Sokolik/go template fix [Go] properly set header parameters on api clients Feb 2, 2023
@sokolikp sokolikp force-pushed the sokolik/go-template-fix branch from 5381ceb to 96d6edf Compare February 2, 2023 03:18
wing328
wing328 reviewed Feb 2, 2023
View reviewed changes
Comment thread samples/client/echo_api/java/apache-httpclient/.openapi-generator/VERSION Outdated
@sokolikp sokolikp force-pushed the sokolik/go-template-fix branch from 96d6edf to 607aff1 Compare February 2, 2023 20:34
@wing328 wing328 added this to the 6.4.0 milestone Feb 3, 2023
@sokolikp sokolikp force-pushed the sokolik/go-template-fix branch 6 times, most recently from a3af6e5 to 60254c5 Compare February 3, 2023 03:32
sokolikp
sokolikp commented Feb 3, 2023
View reviewed changes
Comment thread modules/openapi-generator/src/main/resources/go/api.mustache Outdated
Copy link
Copy Markdown
Contributor Author

@sokolikp sokolikp Feb 3, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

L245 and L249 are the most important changes in this PR. This passes localVarHeaderParams instead of localVarQueryParams.

@sokolikp
Copy link
Copy Markdown
Contributor Author

sokolikp commented Feb 4, 2023

@wing328 when you have a chance, could you help me re-kick off the CI tests? Looks like I'm unable.

@sokolikp
Copy link
Copy Markdown
Contributor Author

sokolikp commented Feb 6, 2023

When I run ./mvnw clean package I get the following:

[ERROR] Failures: 
[ERROR]   DefaultGeneratorTest.supportCustomTemplateEngine:318 » Runtime Could not generate api file for 'Ping'
[ERROR]   DefaultGeneratorTest.testProcessUserDefinedTemplatesWithConfig:731 » Runtime Could not generate model 'ApiResponse'
[ERROR]   PythonClientTest.testApiTestsNotGenerated:143 » Runtime Could not generate model 'ApiResponse'
[ERROR]   PythonClientTest.testApisNotGenerated:164 » Runtime Could not generate model 'ApiResponse'
[ERROR]   PythonPriorClientTest.testObjectModelWithRefedAdditionalPropertiesIsGenerated:408 » Runtime Could not generate model 'A'
[ERROR]   StringHelpersTest.startsWithSectionalTest:47->evaluate:28 » Handlebars inline@486b7642:1:4: java.lang.reflect.InaccessibleObjectException: Unable to make field transient java.util.HashMap$Node[] java.util.HashMap.table accessible: module java.base does not "opens java.util" to unnamed module @3b22cdd0
    inline@486b7642:1:4
[ERROR]   StringHelpersTest.startsWithTest:61->evaluate:28 » Handlebars inline@43bd77be:1:2: java.lang.reflect.InaccessibleObjectException: Unable to make field transient java.util.HashMap$Node[] java.util.HashMap.table accessible: module java.base does not "opens java.util" to unnamed module @3b22cdd0
    inline@43bd77be:1:2
[ERROR]   StringHelpersTest.startsWithYesOverrideTest:81->evaluate:28 » Handlebars inline@689e2599:1:2: java.lang.reflect.InaccessibleObjectException: Unable to make field transient java.util.HashMap$Node[] java.util.HashMap.table accessible: module java.base does not "opens java.util" to unnamed module @3b22cdd0
    inline@689e2599:1:2
[INFO] 
[ERROR] Tests run: 1861, Failures: 8, Errors: 0, Skipped: 0

When I run ./bin/generate-samples.sh I get the following:

[pool-1-thread-27] Generation failed for python: (RuntimeException) Could not generate api file for 'usage'
java.lang.RuntimeException: Could not generate api file for 'usage'
	at org.openapitools.codegen.DefaultGenerator.generateApis(DefaultGenerator.java:701)
	at org.openapitools.codegen.DefaultGenerator.generate(DefaultGenerator.java:924)
	at org.openapitools.codegen.cmd.GenerateBatch$GenerationRunner.run(GenerateBatch.java:232)
	at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1136)
	at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:635)
	at java.base/java.lang.Thread.run(Thread.java:833)
Caused by: com.github.jknack.handlebars.HandlebarsException: endpoint.handlebars:3:3: java.lang.reflect.InaccessibleObjectException: Unable to make field transient java.util.HashMap$Node[] java.util.HashMap.table accessible: module java.base does not "opens java.util" to unnamed module @41a962cf
    endpoint.handlebars:3:3
	at java.base/java.lang.reflect.AccessibleObject.checkCanSetAccessible(AccessibleObject.java:354)
	at java.base/java.lang.reflect.AccessibleObject.checkCanSetAccessible(AccessibleObject.java:297)
	at java.base/java.lang.reflect.Field.checkCanSetAccessible(Field.java:178)
	at java.base/java.lang.reflect.Field.setAccessible(Field.java:172)
	at com.github.jknack.handlebars.context.FieldValueResolver$FieldMember.setAccessible(FieldValueResolver.java:150)
	at com.github.jknack.handlebars.context.MemberValueResolver.cache(MemberValueResolver.java:82)
	at com.github.jknack.handlebars.context.MemberValueResolver.resolve(MemberValueResolver.java:54)
	at com.github.jknack.handlebars.Context$CompositeValueResolver.resolve(Context.java:199)
	at com.github.jknack.handlebars.internal.path.PropertyPath.eval(PropertyPath.java:52)
	at com.github.jknack.handlebars.Context$PathExpressionChain.next(Context.java:361)
	at com.github.jknack.handlebars.Context$PathExpressionChain.eval(Context.java:381)
	at com.github.jknack.handlebars.Context.get(Context.java:618)
	at com.github.jknack.handlebars.internal.Partial.override(Partial.java:253)
	at com.github.jknack.handlebars.internal.Partial.merge(Partial.java:226)
	at com.github.jknack.handlebars.internal.BaseTemplate.apply(BaseTemplate.java:125)
	at com.github.jknack.handlebars.internal.TemplateList.merge(TemplateList.java:95)
	at com.github.jknack.handlebars.internal.BaseTemplate.apply(BaseTemplate.java:125)
	at com.github.jknack.handlebars.internal.BaseTemplate.apply(BaseTemplate.java:113)
	at org.openapitools.codegen.templating.HandlebarsEngineAdapter.compileTemplate(HandlebarsEngineAdapter.java:92)
	at org.openapitools.codegen.TemplateManager.write(TemplateManager.java:163)
	at org.openapitools.codegen.languages.PythonClientCodegen.processTemplateToFile(PythonClientCodegen.java:482)
	at org.openapitools.codegen.languages.PythonClientCodegen.generateFiles(PythonClientCodegen.java:505)
	at org.openapitools.codegen.languages.PythonClientCodegen.generateEndpoints(PythonClientCodegen.java:671)
	at org.openapitools.codegen.languages.PythonClientCodegen.postProcessOperationsWithModels(PythonClientCodegen.java:916)
	at org.openapitools.codegen.DefaultGenerator.processOperations(DefaultGenerator.java:1246)
	at org.openapitools.codegen.DefaultGenerator.generateApis(DefaultGenerator.java:605)
	... 5 more
Caused by: java.lang.reflect.InaccessibleObjectException: Unable to make field transient java.util.HashMap$Node[] java.util.HashMap.table accessible: module java.base does not "opens java.util" to unnamed module @41a962cf
	... 31 more

I'm not sure whether either of these are important given when I'm trying to update. @wing328 let me know if you have ideas on how to fix these.

@sokolikp
Copy link
Copy Markdown
Contributor Author

sokolikp commented Feb 6, 2023

Note that the updated /samples files are still generated (e.g. for python), but it's possible the script fails before the Java samples are updated.

@sokolikp
Copy link
Copy Markdown
Contributor Author

sokolikp commented Feb 6, 2023 •
edited
Loading

Ah I found this thread where it indicates to run with:

_JAVA_OPTIONS="--add-opens=java.base/java.lang=ALL-UNNAMED --add-opens=java.base/java.util=ALL-UNNAMED"

This resolves more errors for me! It works for on on all scripts except one: ./mvnw clean package:

[ERROR] Failures: 
[ERROR]   DefaultGeneratorTest.supportCustomTemplateEngine:318 » Runtime Could not generate api file for 'Ping'
[INFO] 
[ERROR] Tests run: 1861, Failures: 1, Errors: 0, Skipped: 0

To recap:

  • [ FAILS ] ./mvnw clean package (run w/ above Java options - a single command fails with above error "Runtime Could not generate api file for 'Ping'")
  • [ SUCCEEDS ] ./bin/generate-samples.sh (run w/ above Java options)
  • [ SUCCEEDS ] ./mvnw clean package (run w/ above Java options)

@sokolikp sokolikp force-pushed the sokolik/go-template-fix branch from c272452 to df02377 Compare February 7, 2023 00:32
@sokolikp sokolikp closed this Feb 7, 2023
@sokolikp sokolikp reopened this Feb 7, 2023
wing328
wing328 reviewed Feb 7, 2023
View reviewed changes
Comment thread modules/openapi-generator/src/main/resources/go/client.mustache Outdated
@sokolikp sokolikp closed this Feb 7, 2023
@sokolikp sokolikp reopened this Feb 7, 2023
@sokolikp sokolikp force-pushed the sokolik/go-template-fix branch from df02377 to 877c9eb Compare February 7, 2023 01:55
@wing328 wing328 mentioned this pull request Feb 7, 2023
Merged
6 tasks
@wing328
Copy link
Copy Markdown
Member

wing328 commented Feb 7, 2023

Closed via #14637

@wing328 wing328 closed this Feb 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@wing328 wing328 wing328 left review comments

Assignees

No one assigned

Labels

Client: Go Issue: Bug

Projects

None yet

Milestone

6.4.0

Development

Successfully merging this pull request may close these issues.

2 participants

@sokolikp @wing328