fix: Required field does not take effect

[MakDon]

Fix #2717

[MakDon]

The pipeline is complaining because the field required is switched from false to true but i can not figure out why x-optionalDataType: "String" has been removed.

[johanbrandhorst]

Can you try just regenerating the files by following the instructions in CONTRIBUTING.md?

[MakDon]

I've regenerated the files. But errors occurs when generating files, and it will stop the generating on Macos:

swagger-codegen generate -i examples/internal/proto/examplepb/a_bit_of_everything.swagger.json \
        -l go -o examples/internal/clients/abe --additional-properties packageName=abe
/usr/local/bin/swagger-codegen: 1: -e: not found

Following the error info, I found that there's a -e at the begginning of the /usr/local/bin/swagger-codegen:

makdon-System-Product-Name% 
makdon-System-Product-Name% docker run -v $(pwd):/grpc-gateway -w /grpc-gateway --rm -it ghcr.io/grpc-ecosystem/grpc-gateway/build-env:1.17 "/bin/bash"
vscode ➜ /grpc-gateway (makdon-master) $ 
vscode ➜ /grpc-gateway (makdon-master) $ 
vscode ➜ /grpc-gateway (makdon-master) $ pwd
/grpc-gateway
vscode ➜ /grpc-gateway (makdon-master) $ which swagger-codegen 
/usr/local/bin/swagger-codegen
vscode ➜ /grpc-gateway (makdon-master) $ cat /usr/local/bin/swagger-codegen
-e #!/bin/bash
java -jar /usr/local/bin/swagger-codegen-cli.jar "$@"
vscode ➜ /grpc-gateway (makdon-master) $ swagger-codegen 
/usr/local/bin/swagger-codegen: line 1: -e: command not found
Available languages: [ada, ada-server, akka-scala, android, apache2, apex, aspnetcore, bash, csharp, clojure, cwiki, cpprest, csharp-dotnet2, dart, dart-jaguar, elixir, elm, eiffel, erlang-client, erlang-server, finch, flash, python-flask, go, go-server, groovy, haskell-http-client, haskell, jmeter, jaxrs-cxf-client, jaxrs-cxf, java, inflector, jaxrs-cxf-cdi, jaxrs-spec, jaxrs, msf4j, java-pkmst, java-play-framework, jaxrs-resteasy-eap, jaxrs-resteasy, javascript, javascript-closure-angular, java-vertx, kotlin, lua, lumen, nancyfx, nodejs-server, objc, perl, php, powershell, pistache-server, python, qt5cpp, r, rails5, restbed, ruby, rust, rust-server, scala, scala-gatling, scala-lagom-server, scalatra, scalaz, php-silex, sinatra, slim, spring, dynamic-html, html2, html, swagger, swagger-yaml, swift4, swift3, swift, php-symfony, tizen, typescript-aurelia, typescript-angular, typescript-inversify, typescript-angularjs, typescript-fetch, typescript-jquery, typescript-node, undertow, ze-ph, kotlin-server]
vscode ➜ /grpc-gateway (makdon-master) $

When I am using Macos, this error would stop the make and return an error. But when using Ubuntu, it reports the error and continues generating files. What is this -e used for? Should we remove it to avoid it breaks the make generate?

[johanbrandhorst]

I've regenerated the files. But errors occurs when generating files, and it will stop the generating on Macos:

swagger-codegen generate -i examples/internal/proto/examplepb/a_bit_of_everything.swagger.json \
        -l go -o examples/internal/clients/abe --additional-properties packageName=abe
/usr/local/bin/swagger-codegen: 1: -e: not found

Following the error info, I found that there's a -e at the begginning of the file:

makdon-System-Product-Name% 
makdon-System-Product-Name% docker run -v $(pwd):/grpc-gateway -w /grpc-gateway --rm -it ghcr.io/grpc-ecosystem/grpc-gateway/build-env:1.17 "/bin/bash"
vscode ➜ /grpc-gateway (makdon-master) $ 
vscode ➜ /grpc-gateway (makdon-master) $ 
vscode ➜ /grpc-gateway (makdon-master) $ pwd
/grpc-gateway
vscode ➜ /grpc-gateway (makdon-master) $ which swagger-codegen 
/usr/local/bin/swagger-codegen
vscode ➜ /grpc-gateway (makdon-master) $ cat /usr/local/bin/swagger-codegen
-e #!/bin/bash
java -jar /usr/local/bin/swagger-codegen-cli.jar "$@"
vscode ➜ /grpc-gateway (makdon-master) $ swagger-codegen 
/usr/local/bin/swagger-codegen: line 1: -e: command not found
Available languages: [ada, ada-server, akka-scala, android, apache2, apex, aspnetcore, bash, csharp, clojure, cwiki, cpprest, csharp-dotnet2, dart, dart-jaguar, elixir, elm, eiffel, erlang-client, erlang-server, finch, flash, python-flask, go, go-server, groovy, haskell-http-client, haskell, jmeter, jaxrs-cxf-client, jaxrs-cxf, java, inflector, jaxrs-cxf-cdi, jaxrs-spec, jaxrs, msf4j, java-pkmst, java-play-framework, jaxrs-resteasy-eap, jaxrs-resteasy, javascript, javascript-closure-angular, java-vertx, kotlin, lua, lumen, nancyfx, nodejs-server, objc, perl, php, powershell, pistache-server, python, qt5cpp, r, rails5, restbed, ruby, rust, rust-server, scala, scala-gatling, scala-lagom-server, scalatra, scalaz, php-silex, sinatra, slim, spring, dynamic-html, html2, html, swagger, swagger-yaml, swift4, swift3, swift, php-symfony, tizen, typescript-aurelia, typescript-angular, typescript-inversify, typescript-angularjs, typescript-fetch, typescript-jquery, typescript-node, undertow, ze-ph, kotlin-server]
vscode ➜ /grpc-gateway (makdon-master) $

When I am using Macos, this error would stop the make and return an error. But when using Ubuntu, it reports the error and continues generating files. What is this -e used for? Should we remove it to avoid it breaks the make generate?

I've seen this error a lot as well, and I've ignored it because it hasn't caused any issues. If it's causing a problem on MacOS we should definitely fix it, but would you mind making a separate PR? Thanks!

[MakDon]

Of course. I will make another PR for this after a second confirm. But I can not find out where the dockerfile for ghcr.io/grpc-ecosystem/grpc-gateway/build-env:1.17 locates. And I do not have the permission to update the image on GitHub Packages

[MakDon]

After generating the files, more test breaks because the required field is added into the stubs. I will work on it these days.

[johanbrandhorst]

I think it's this file: https://github.com/grpc-ecosystem/grpc-gateway/blob/master/.github/Dockerfile. I've been pushing it manually (which is why the tag hasn't been updated for 1.18 yet 😬). Would be great if we had a Github actions job for it now that we've migrated away from CircleCI, but don't consider that a requirement - I can do a manual push once we have a PR. Thanks for your work on this!

[MakDon]

The pipeline is complaining because it generates the files using codes from this PR, but compare it with the origin repo. It can not be fixed by regenerate the files and commit into my branch. Maybe we can just ignore it.

[johanbrandhorst]

The pipeline is complaining because it generates the files using codes from this PR, but compare it with the origin repo. It can not be fixed by regenerate the files and commit into my branch. Maybe we can just ignore it.

Do you mind creating a separate PR that switches us to use the 1.19 container version for our generate job? Just edit this and this and run the generation script again.

[johanbrandhorst]

I'm confused as to why the CI generation would complain, it shouldn't compare against main, it should compare against this PR's contents. Looks to me like we just need to rerun generation again?

[MakDon]

I'm confused as to why the CI generation would complain, it shouldn't compare against main, it should compare against this PR's contents. Looks to me like we just need to rerun generation again?

You're right. Sorry for my mistake. I am trying to figure out the reason because I've tried to regenerate the files in this and this commit

[johanbrandhorst]

I tried generating this locally as well and can't reproduce the diff, I'm very confused. Any chance you could rebase this on main and pull in the new 1.19 tag in the CI job?

[MakDon]

I closed this PR accidentally... I've rebased and force pushed, and going to reopen and run the pipelines.

[MakDon]

The golangci is broken, I tried to reproduce it, and got the following message(which is not caused by this PR):

➜  grpc-gateway git:(makdon-master) golangci-lint version                                           
golangci-lint has version 1.48.0 built from 2d8fea81 on 2022-08-04T18:44:38Z
➜  grpc-gateway git:(makdon-master) golangci-lint run --enable goimports 
protoc-gen-grpc-gateway/main.go:6: File is not `goimports`-ed (goimports)
//   protoc --grpc-gateway_out=output_directory path/to/input.proto
protoc-gen-openapiv2/internal/genopenapi/generator.go:80: File is not `goimports`-ed (goimports)
//    JSON, we only want to add fields (see below, extensionMarshalJSON).
//    An infinite recursion would happen if we'd call json.Marshal on the struct
//    that has swaggerObject as an embedded field. To avoid that, we'll create
//    type aliases, and those don't have the custom MarshalJSON methods defined
//    on them. See http://choly.ca/post/go-json-marshalling/ (or, if it ever
//    goes away, use
//    https://web.archive.org/web/20190806073003/http://choly.ca/post/go-json-marshalling/.
protoc-gen-openapiv2/internal/genopenapi/template.go:32: File is not `goimports`-ed (goimports)
//  The OpenAPI specification does not allow for more than one endpoint with the same HTTP method and path.
//  This prevents multiple gRPC service methods from sharing the same stripped version of the path and method.
//  For example: `GET /v1/{name=organizations/*}/roles` and `GET /v1/{name=users/*}/roles` both get stripped to `GET /v1/{name}/roles`.
//  We must make the URL unique by adding a suffix and an incrementing index to each path parameter
//  to differentiate the endpoints.
//  Since path parameter names do not affect the request contents (i.e. they're replaced in the path)
//  this will be hidden from the real grpc gateway consumer.
protoc-gen-openapiv2/internal/genopenapi/template.go:7:2: SA1019: "io/ioutil" has been deprecated since Go 1.16: As of Go 1.16, the same functionality is now provided by package io or package os, and those implementations should be preferred in new code. See the specific function documentation for details. (staticcheck)
        "io/ioutil"
        ^
utilities/readerfactory.go:6:2: SA1019: "io/ioutil" has been deprecated since Go 1.16: As of Go 1.16, the same functionality is now provided by package io or package os, and those implementations should be preferred in new code. See the specific function documentation for details. (staticcheck)
        "io/ioutil"
        ^
internal/codegenerator/parse_req.go:6:2: SA1019: "io/ioutil" has been deprecated since Go 1.16: As of Go 1.16, the same functionality is now provided by package io or package os, and those implementations should be preferred in new code. See the specific function documentation for details. (staticcheck)
        "io/ioutil"
        ^

So I ran golangci-lint run --enable goimports --fix that produces this commit But I wonder if we should make another separate PR to do the formatting, and then rebase on it.

[johanbrandhorst]

Thanks for your contribution and for your patience!