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

sokolikp · 2023-02-02T01:34:21Z

Context

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

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
Correctly passes localVarHeaderParams (instead of localVarQueryParams) to the above function for header params
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 · 2023-02-02T02:44:30Z

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 · 2023-02-02T02:45:47Z

@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 · 2023-02-02T02:50:53Z

@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.

samples/client/echo_api/java/apache-httpclient/.openapi-generator/VERSION

sokolikp · 2023-02-03T03:35:45Z

modules/openapi-generator/src/main/resources/go/api.mustache

 	{{/required}}
 	{{^required}}
 	if r.{{paramName}} != nil {
-		parameterAddToQuery(localVarQueryParams, "{{baseName}}", r.{{paramName}}, "{{collectionFormat}}")
+		parameterAddToHeaderOrQuery(localVarHeaderParams, "{{baseName}}", r.{{paramName}}, "{{collectionFormat}}")


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

sokolikp · 2023-02-04T19:14:24Z

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

sokolikp · 2023-02-06T20:15:15Z

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 · 2023-02-06T20:18:13Z

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 · 2023-02-06T20:32:08Z

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)

modules/openapi-generator/src/main/resources/go/client.mustache

wing328 · 2023-02-07T13:45:46Z

Closed via #14637

sokolikp force-pushed the sokolik/go-template-fix branch from ccfc692 to 5381ceb Compare February 2, 2023 01:37

sokolikp marked this pull request as ready for review February 2, 2023 02:42

wing328 added Client: Go Issue: Bug labels Feb 2, 2023

sokolikp changed the title ~~Sokolik/go template fix~~ [Go] properly set header parameters on api clients Feb 2, 2023

sokolikp force-pushed the sokolik/go-template-fix branch from 5381ceb to 96d6edf Compare February 2, 2023 03:18

wing328 reviewed Feb 2, 2023

View reviewed changes

samples/client/echo_api/java/apache-httpclient/.openapi-generator/VERSION Outdated Show resolved Hide resolved

sokolikp force-pushed the sokolik/go-template-fix branch from 96d6edf to 607aff1 Compare February 2, 2023 20:34

wing328 added this to the 6.4.0 milestone Feb 3, 2023

sokolikp force-pushed the sokolik/go-template-fix branch 6 times, most recently from a3af6e5 to 60254c5 Compare February 3, 2023 03:32

sokolikp commented Feb 3, 2023

View reviewed changes

sokolikp force-pushed the sokolik/go-template-fix branch from c272452 to df02377 Compare February 7, 2023 00:32

sokolikp closed this Feb 7, 2023

sokolikp reopened this Feb 7, 2023

wing328 reviewed Feb 7, 2023

View reviewed changes

modules/openapi-generator/src/main/resources/go/client.mustache Outdated Show resolved Hide resolved

sokolikp closed this Feb 7, 2023

sokolikp reopened this Feb 7, 2023

[Go] properly set header parameters on api clients

877c9eb

sokolikp force-pushed the sokolik/go-template-fix branch from df02377 to 877c9eb Compare February 7, 2023 01:55

wip

e9f4208

wing328 mentioned this pull request Feb 7, 2023

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

Merged

6 tasks

wing328 closed this Feb 7, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

sokolikp commented Feb 2, 2023 •

edited

Loading

sokolikp commented Feb 2, 2023 •

edited

Loading

wing328 commented Feb 2, 2023

sokolikp commented Feb 2, 2023 •

edited

Loading

sokolikp Feb 3, 2023

sokolikp commented Feb 4, 2023

sokolikp commented Feb 6, 2023

sokolikp commented Feb 6, 2023

sokolikp commented Feb 6, 2023 •

edited

Loading

wing328 commented Feb 7, 2023

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

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

Conversation

sokolikp commented Feb 2, 2023 • edited Loading

Context

PR checklist

sokolikp commented Feb 2, 2023 • edited Loading

wing328 commented Feb 2, 2023

sokolikp commented Feb 2, 2023 • edited Loading

sokolikp Feb 3, 2023

Choose a reason for hiding this comment

sokolikp commented Feb 4, 2023

sokolikp commented Feb 6, 2023

sokolikp commented Feb 6, 2023

sokolikp commented Feb 6, 2023 • edited Loading

wing328 commented Feb 7, 2023

sokolikp commented Feb 2, 2023 •

edited

Loading

sokolikp commented Feb 2, 2023 •

edited

Loading

sokolikp commented Feb 2, 2023 •

edited

Loading

sokolikp commented Feb 6, 2023 •

edited

Loading