[Golang][client] fix undefined: localVarFile #382

grokify · 2018-06-22T15:30:48Z

PR checklist

Read the contribution guidelines.
Ran the shell script under ./bin/ to update Petstore sample so that CIs can verify the change. (For instance, only need to run ./bin/{LANG}-petstore.sh and ./bin/security/{LANG}-petstore.sh if updating the {LANG} (e.g. php, ruby, python, etc) code generator or {LANG} client's mustache templates). Windows batch files can be found in .\bin\windows\.
Filed the PR against the correct branch: master, 3.1.x, 4.0.x. Default: master.
Copied the technical committee to review the pull request if your PR is targeting a particular programming language.

Description of the PR

When building a Go client with a required formData parameter of file type, a undefined: localVarFile error is countered when compiling the client.

This happens because localVarFile is currently only defined when a file is optional. This fix does the following:

defines localVarFile when a file is required
sets localVarFile to the variable name as defined in the function call

Known Limitations

This approach can only handle a single file, but this is also a limitation of the current Go generator can as APIClient.prepareRequest() takes a single pair of localVarFileName and localVarFileBytes parameters. A future enhancement could support multiple files, either with the same multipart/form-data name or different ones.

Tests

mvn package was successfully run in the samples/client/petstore/go directory.

Additional Petstore tests were not added for this use case yet because an endpoint with a required formData file is not present in the Petstore spec. Adding this endpoint path to the Petstore spec will be a future project.

This has been tested locally with the following Swagger spec with a successful required file upload.

https://github.com/grokify/go-ringcentral/blob/master/codegen/swagger_spec.yaml

References

antihax · 2018-06-22T15:48:47Z

I don't see actual code changes? is this not in scope of the test file?

grokify · 2018-06-22T15:53:10Z

I don't see actual code changes? is this not in scope of the test file?

That's right. Code changes exist in api.mustache, but no code is changed in the generated go-petstore client because the Petstore spec does not include a required formData file parameter. At the moment, it only has an optional required: false formData file request parameter which is not affected by this issue.

Adding new endpoints to the Petstore API via petstore-with-fake-endpoints-models-for-testing.yaml would let us see changes like this reflected in the generated clients. Let me look into adding a POST /pet/{petId}/uploadImageWithRequiredFile endpoint with required: true.

…ined-localVarFile

grokify · 2018-06-23T06:50:31Z

Updates:

Added POST /pet/{petId}/uploadImageWithRequiredFile endpoint to 2.0 and 3.0 petstore-with-fake-endpoints-models-for-testing.yaml
Built new go-petstore with APIClientPetApi.UploadFileWithRequiredFile function
Added new test TestUploadFileRequired with early return until supported in server
Streamlined api.mustache update

wing328 · 2018-06-25T04:01:02Z

@grokify thanks for the fix and the additional test cases to cover the issue moving forward.

Given that there are new test cases, I wonder if you can run the following to update other Petstore samples as well so that the Shippable CI tests will pass:

./bin/ruby-petstore.sh > /dev/null 2>&1
./bin/java-petstore-all.sh > /dev/null 2>&1
./bin/java-jaxrs-petstore-server-all.sh > /dev/null 2>&1
./bin/spring-all-pestore.sh > /dev/null 2>&1
./bin/kotlin-client-petstore.sh > /dev/null 2>&1
./bin/kotlin-client-string.sh > /dev/null 2>&1
./bin/kotlin-client-threetenbp.sh > /dev/null 2>&1
./bin/kotlin-server-petstore.shl> /dev/null 2>&1
./bin/php-petstore.sh > /dev/null 2>&1
./bin/php-silex-petstore-server.shj> /dev/null 2>&1
./bin/php-symfony-petstore.sh > /dev/null 2>&1
./bin/php-lumen-petstore-server.sh > /dev/null 2>&1
./bin/php-slim-petstore-server.sh > /dev/null 2>&1
./bin/php-ze-ph-petstore-server.sh > /dev/null 2>&1
./bin/openapi3/php-petstore.sh > /dev/null 2>&1

…ined-localVarFile

grokify · 2018-06-26T14:30:25Z

Updated the samples and the Shippable CI tests now succeed but others are now failing. Any assistance to resolve these would be appreciated.

CircleCI

[ERROR] COMPILATION ERROR :
[ERROR] /home/circleci/OpenAPITools/openapi-generator/samples/server/petstore/jaxrs-cxf/src/main/java/org/openapitools/api/impl/PetApiServiceImpl.java:[27,8] org.openapitools.api.impl.PetApiServiceImpl is not abstract and does not override abstract method uploadFileWithRequiredFile(java.lang.Long,org.apache.cxf.jaxrs.ext.multipart.Attachment,java.lang.String) in org.openapitools.api.PetApi
[ERROR] Failed to execute goal org.apache.maven.plugins:maven-compiler-plugin:3.1:compile (default-compile) on project openapi-cxf-server: Compilation failure
[ERROR] /home/circleci/OpenAPITools/openapi-generator/samples/server/petstore/jaxrs-cxf/src/main/java/org/openapitools/api/impl/PetApiServiceImpl.java:[27,8] org.openapitools.api.impl.PetApiServiceImpl is not abstract and does not override abstract method uploadFileWithRequiredFile(java.lang.Long,org.apache.cxf.jaxrs.ext.multipart.Attachment,java.lang.String) in org.openapitools.api.PetApi

It seems samples/server/petstore/jaxrs-cxf is built by bin/jaxrs-cxf-petstore-server.sh but that did not resolve these CircleCI errors.

Travis CI

$ sudo apt-get install -qq bats
WARNING: The following packages cannot be authenticated!
bats
E: There were unauthenticated packages and -y was used without --allow-unauthenticated

This particular issue doesn't appear to be an issue on the project master branch or my fork fix branch:

antihax · 2018-06-27T13:34:44Z

@wing328 I ran into similar issues to this before where we wanted to add a test case but doing so broke other clients as they did not support part of the standard.

What would be the best way to proceed in these cases?

grokify · 2018-06-27T15:23:03Z

CircleCI Error: PetApiServiceImpl is not abstract... uploadFileWithRequiredFile

The previous CircleCI error:

PetApiServiceImpl is not abstract and does not override abstract method uploadFileWithRequiredFile

is due to bin/jaxrs-cxf-petstore-server.sh not overwriting / updating existing files including samples/server/petstore/jaxrs-cxf/src/main/java/org/openapitools/api/impl/PetApiServiceImpl.java. Because of this, PetApiServiceImpl.java would not get the new method uploadFileWithRequiredFile.

Deleting the samples/server/petstore/jaxrs-cxf/ directory before running bin/jaxrs-cxf-petstore-server.sh resolved this. I have a "test" branch that I did get to successfully build on CircleCI with PetApiServiceImpl uploadFileWithRequiredFile as seen here:

https://circleci.com/gh/grokify/openapi-generator/7

Full error line:

[ERROR] /home/circleci/OpenAPITools/openapi-generator/samples/server/petstore/jaxrs-cxf/src/main/java/org/openapitools/api/impl/PetApiServiceImpl.java:[27,8] org.openapitools.api.impl.PetApiServiceImpl is not abstract and does not override abstract method uploadFileWithRequiredFile(java.lang.Long,org.apache.cxf.jaxrs.ext.multipart.Attachment,java.lang.String) in org.openapitools.api.PetApi

Building Samples Not Cleaning Output Directories

On another note, it seems bin/go-petstore.sh will create new files for samples/client/petstore/go/go-petstore but will not remove old files that are no longer generated.

In both cases, it seems like having bin/jaxrs-cxf-petstore-server.sh and bin/go-petstore.sh remove the target directory would be cleaner. Here's a summary:

builder	output directory	notes
`bin/jaxrs-cxf-petstore-server.sh`	`samples/server/petstore/jaxrs-cxf/`	Does not overwrite / update existing files
`bin/go-petstore.sh`	`samples/client/petstore/go/go-petstore`	Does not remove old files that are no longer generated

Latest Build Errors

bin/run-all-petstore was run to update all the samples.

CircleCI: The latest CircleCI errors are due to a number of download issues with scalaz-petstore-client as listed in the logs.
Shippable: successful rebuild on fork: https://app.shippable.com/github/grokify/openapi-generator/runs/15/1/console
AppVeyor: investigation underway

grokify · 2018-06-28T02:44:06Z

CircleCI ✔️

CircleCI is fixed in grokify/openapi-generator/14. Issue is some sample files like samples/client/petstore/clojure/pom.xml are excluded in .gitignore but must be preserved.

Shippable ❌

Shippable is failing with what looks like a race condition: Failure: ShutdownMonitorThread already started.

Link: https://app.shippable.com/github/OpenAPITools/openapi-generator/runs/1078/1/console

Error:

[ERROR] Failed to execute goal org.eclipse.jetty:jetty-maven-plugin:9.2.15.v20160210:start (start-jetty) on project spring-mvc-j8-localdatetime: Failure: ShutdownMonitorThread already started -> [Help 1]
[ERROR] 
[ERROR] To see the full stack trace of the errors, re-run Maven with the -e switch.
[ERROR] Re-run Maven using the -X switch to enable full debug logging.
[ERROR] 
[ERROR] For more information about the errors and possible solutions, please read the following articles:
[ERROR] [Help 1] http://cwiki.apache.org/confluence/display/MAVEN/MojoExecutionException
[ERROR] 
[ERROR] After correcting the problems, you can resume the build with the command
[ERROR]   mvn <goals> -rf :spring-mvc-j8-localdatetime

Rebuild on fork succeeds:

https://app.shippable.com/github/grokify/openapi-generator/runs/22/1/console

Local tests succeed via:

$ bin/spring-mvc-petstore-j8-localdatetime.sh
$ cd samples/server/petstore/spring-mvc-j8-localdatetime/
$ mvn verify -Psamples

grokify · 2018-06-28T04:44:11Z

@antihax @bvwells @wing328

~~All tests are passing with no conflicts eb1e146.~~ Now addressing merge conflicts due to 9b909df

Primary changes are:

api.mustache: https://github.com/OpenAPITools/openapi-generator/pull/382/files#diff-e70c0e4856318b336d243d2d9a54232f
pet_api_test.go: https://github.com/OpenAPITools/openapi-generator/pull/382/files#diff-7782decde9080f5a41d4c37aeb579302
2_0/petstore-with-fake-endpoints-models-for-testing.yaml: https://github.com/OpenAPITools/openapi-generator/pull/382/files#diff-306628117f125fc0db5f1ad40d7b625c
3_0/petstore-with-fake-endpoints-models-for-testing.yaml: https://github.com/OpenAPITools/openapi-generator/pull/382/files#diff-d9b221b1064638087f63507cc10c34c0

Verify in generated client:

api_pet.go: https://github.com/OpenAPITools/openapi-generator/pull/382/files#diff-997ad6babb705701e5384eaf2ee634fc

jmini · 2018-06-28T05:18:05Z

samples/client/petstore/eiffel/api_client.ecf

@@ -1,5 +1,9 @@
 <?xml version="1.0" encoding="ISO-8859-1"?>
+<<<<<<< HEAD


Can you please solve this merge conflict?

jmini · 2018-06-28T05:18:25Z

samples/client/petstore/eiffel/test/api_test.ecf

@@ -1,5 +1,9 @@
 <?xml version="1.0" encoding="ISO-8859-1"?>
+<<<<<<< HEAD


Can you please solve this merge conflict?

jmini · 2018-06-28T05:24:16Z

Shippable — Run 1081 status is FAILED.

Now that we have merged 3.1.x to master to work on 3.1.0-SNAPSHOT, can you please merge master again in your branch?
This way your version will also be 3.1.0-SNAPSHOT and not the 3.0.3 release version.

Then please run:

mvn verify
bin/utils/ensure-up-to-date.sh

And commit the results. This should fix the Shippable Build.

grokify · 2018-06-28T05:27:42Z

Shippable — Run 1081 status is FAILED.

Shippable — Run 1079 and other CIs were successful on eb1e146, but a number of merge conflicts were introduced after 9b909df. 1081 was an initial failed effort to resolve these.

grokify · 2018-06-28T08:04:59Z

After merging master at 9b909df, and successfully running mvn verify and bin/utils/ensure-up-to-date, Shippable has timed out twice. Will try again later.

Here's some info on the two runs:

Shippable — Run 1085

Log stopped after JSON dump following below:

generator/samples/server/petstore/kotlin-server/ktor/src/main/kotlin/org/openapitools/server/apis/UserApi.kt
[main] INFO  o.o.codegen.DefaultGenerator - ############ Supporting file info ############

Shippable — Run 1084

Log stopped after the following:

[main] INFO  o.o.codegen.AbstractGenerator - writing file /root/src/github.com/OpenAPITools/openapi-generator/samples/client/petstore/perl/deep_module_test/lib/Something/Deep/Object/AdditionalPropertiesClass.pm
[main] INFO  o.o.codegen.AbstractGenerator - writing file /root/src/github.com/OpenAPITools/openapi-generator/samples/client/petstore/perl/deep_module_test/t/AdditionalPropertiesClassTest.t
[main] INFO  o.o.codegen.AbstractGenerator - writing file /root/src/github.com/OpenAPITools/openapi-generator/samples/client/petstore/perl/deep_module_test/docs/AdditionalPropertiesClass.md
[main] INFO  o.o.codegen.AbstractGenerator - writing file /root/src/github.com/OpenAPITools/openapi-generator/samples/client/petstore/perl/deep_module_test/lib/Something/Deep/Object/Animal.pm

grokify · 2018-06-28T15:08:21Z

master has been merged into branch.

All tests are passing with no conflicts e72745d.

No changes were necessary to resolve Shippable timeouts and race conditions. These are not deterministic so a few more test runs were able to resolve them.

wing328 · 2018-07-02T03:33:01Z

FYI. Moved the new test under /fake and make some minor code improvements to Go generator via #430

jmini · 2018-07-02T06:44:10Z

Added milestone here

grokify · 2018-07-02T12:55:21Z

I ran into similar issues to this before where we wanted to add a test case but doing so broke other clients as they did not support part of the standard.

What would be the best way to proceed in these cases?

@antihax Rebuilding the samples should solve these issues, but it may require some debugging. In this particular case, some files that needed to get updated were not getting updated. This required following the log entries which I'll look into providing a PR for.

If you let me know what test case you want to add, I can look into it ensuring tests for other clients pass. I'm generally interested in increasing the number of test cases for better coverage.

* fix undefined: localVarFile * add required formData file endpoints to 2.0 and 3.0 specs * streamline api.mustache update * update sampels * update samples * update samples * update samples bin/jaxrs-cxf-client-petstore.sh * update samples * update samples * update samples run-all-petstore * update samples * update samples * Trigger CI due to race condition * update samples * update samples * Trigger CI due to previous timeout * Trigger CI due to previous Shippable timeout * Trigger CI due to previous Shippable race condition

fix undefined: localVarFile

6194093

grokify changed the title ~~fix undefined: localVarFile~~ [Golang][client] fix undefined: localVarFile Jun 22, 2018

grokify added 3 commits June 22, 2018 11:06

add required formData file endpoints to 2.0 and 3.0 specs

bf4aedb

streamline api.mustache update

53caca1

Merge branch 'master' into go-client/fix/formdata_required-file_undef…

dc9c683

…ined-localVarFile

wing328 added Client: Go Issue: Bug labels Jun 25, 2018

grokify added 5 commits June 25, 2018 22:28

update sampels

c7611aa

update samples

a543d57

Merge branch 'master' into go-client/fix/formdata_required-file_undef…

e6b0338

…ined-localVarFile

update samples

118404a

Merge branch 'master' into go-client/fix/formdata_required-file_undef…

5708f85

…ined-localVarFile

grokify added 5 commits June 26, 2018 07:49

update samples bin/jaxrs-cxf-client-petstore.sh

f72472d

Merge branch 'master' into go-client/fix/testing

10b9419

update samples

87e240b

update samples

3b15c3d

update samples run-all-petstore

2c6791c

grokify added 4 commits June 27, 2018 11:30

Merge branch 'master' into go-client/fix/testing

61f76d9

update samples

83874b1

update samples

cf42bc8

update samples

13fd826

Trigger CI due to race condition

eb1e146

merge master 9b909df

89bfda5

jmini reviewed Jun 28, 2018

View reviewed changes

update samples

2684c0c

grokify added 2 commits June 27, 2018 22:48

update samples

3439754

Trigger CI due to previous timeout

f5cc397

grokify added 2 commits June 28, 2018 06:50

Trigger CI due to previous Shippable timeout

027091e

Trigger CI due to previous Shippable race condition

e72745d

wy-z mentioned this pull request Jun 29, 2018

[go][api] Fix error: 'undeclared name: localVarFile' #416

Closed

4 tasks

wing328 merged commit e960fe9 into OpenAPITools:master Jul 1, 2018

grokify deleted the go-client/fix/formdata_required-file_undefined-localVarFile branch July 2, 2018 06:17

jmini added this to the 3.1.0 milestone Jul 2, 2018

grokify mentioned this pull request Jul 4, 2018

[jaxrs-cfx][server] bin/jaxrs-cxf-petstore-server.sh does not generate required files #451

Closed

karismann mentioned this pull request May 11, 2019

[Go] Bug: Error "undefined: localVarFile" when specifying POST on file swagger-api/swagger-codegen#9431

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Golang][client] fix undefined: localVarFile #382

[Golang][client] fix undefined: localVarFile #382

grokify commented Jun 22, 2018 •

edited

Loading

antihax commented Jun 22, 2018

grokify commented Jun 22, 2018 •

edited

Loading

grokify commented Jun 23, 2018 •

edited

Loading

wing328 commented Jun 25, 2018

grokify commented Jun 26, 2018 •

edited

Loading

antihax commented Jun 27, 2018

grokify commented Jun 27, 2018 •

edited

Loading

grokify commented Jun 28, 2018 •

edited

Loading

grokify commented Jun 28, 2018 •

edited

Loading

jmini Jun 28, 2018

jmini Jun 28, 2018

jmini commented Jun 28, 2018

grokify commented Jun 28, 2018 •

edited

Loading

grokify commented Jun 28, 2018 •

edited

Loading

grokify commented Jun 28, 2018 •

edited

Loading

wing328 commented Jul 2, 2018

jmini commented Jul 2, 2018

grokify commented Jul 2, 2018 •

edited

Loading

		@@ -1,5 +1,9 @@
		<?xml version="1.0" encoding="ISO-8859-1"?>
		<<<<<<< HEAD

[Golang][client] fix undefined: localVarFile #382

[Golang][client] fix undefined: localVarFile #382

Conversation

grokify commented Jun 22, 2018 • edited Loading

PR checklist

Description of the PR

Known Limitations

Tests

References

antihax commented Jun 22, 2018

grokify commented Jun 22, 2018 • edited Loading

grokify commented Jun 23, 2018 • edited Loading

wing328 commented Jun 25, 2018

grokify commented Jun 26, 2018 • edited Loading

antihax commented Jun 27, 2018

grokify commented Jun 27, 2018 • edited Loading

grokify commented Jun 28, 2018 • edited Loading

grokify commented Jun 28, 2018 • edited Loading

jmini Jun 28, 2018

Choose a reason for hiding this comment

jmini Jun 28, 2018

Choose a reason for hiding this comment

jmini commented Jun 28, 2018

grokify commented Jun 28, 2018 • edited Loading

grokify commented Jun 28, 2018 • edited Loading

grokify commented Jun 28, 2018 • edited Loading

wing328 commented Jul 2, 2018

jmini commented Jul 2, 2018

grokify commented Jul 2, 2018 • edited Loading

grokify commented Jun 22, 2018 •

edited

Loading

grokify commented Jun 22, 2018 •

edited

Loading

grokify commented Jun 23, 2018 •

edited

Loading

grokify commented Jun 26, 2018 •

edited

Loading

grokify commented Jun 27, 2018 •

edited

Loading

grokify commented Jun 28, 2018 •

edited

Loading

grokify commented Jun 28, 2018 •

edited

Loading

grokify commented Jun 28, 2018 •

edited

Loading

grokify commented Jun 28, 2018 •

edited

Loading

grokify commented Jun 28, 2018 •

edited

Loading

grokify commented Jul 2, 2018 •

edited

Loading