[Java] Templates to support google-api-client library by charlescapps · Pull Request #6838 · swagger-api/swagger-codegen

charlescapps · 2017-10-27T23:08:07Z

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: 3.0.0 branch for changes related to OpenAPI spec 3.0. Default: master.
Copied the technical committee to review the pull request if your PR is targeting a particular programming langauge.

@bbdouglas (2017/07) @JFCote (2017/08) @sreeshas (2017/08) @jfiala (2017/08) @lukoyanov (2017/09) @cbornet (2017/09)

Description of the PR

See issue #6825
This PR adds support for generating client code for the Google APIs Client for Java. This HTTP client is not just for Google's APIs, as you can read in the description of their page:

The Google APIs Client Library for Java is a flexible, efficient, and powerful Java client library for accessing any HTTP-based API on the web, not just Google APIs.

Motivation

The google-api-client is really standard in GCP for making HTTP requests. It works great with authentication in GCP (or outside GCP), by automatically handling refresh tokens with the google Credential class. Furthermore, this provides a clear upgrade path for anyone on Google Cloud Endpoints who wants to migrate to a better REST framework, since Cloud Endpoints uses the google-api-client.

Changes in this PR

Add a new library to the Java client codegen called google-api-client
Templates under modules/swagger-codegen/src/main/resources/Java/libraries/google-api-client
A minor tweak to DefaultCodegen to have a list of required/body params, so the client can generate overloaded methods that use a Map<String, Object> for query params.
Generated the samples for google-api-client, and verified they compile, etc.

Features of the generated Google ApiClient

Nice builder-style pattern e.g. apiClient.myFooApi().doSomeRequest(params...)
Generates overloaded API methods to specify query params as a Map<String, Object>, and methods ending in forHttpResponse that return the raw HttpResponse rather than deserializing as a POJO.
Configurable basePath, HttpTransport, HttpRequestInitializer (used for auth), and ObjectMapper.
Uses a standard Jackson ObjectMapper Instead of using the problematic Google-custom-Jackson (e.g.com.google.api.client.json.jackson2.JacksonFactory)
Does not generate auth-related classes under the auth package, because I'm not sure what that would gain us; it's really straightforward to use an HttpRequestInitializer to provide and access token, e.g. you can use com.google.api.client.auth.oauth2.Credential and it can also do token refreshes automatically (or just use an access token if that's all you have).

cc: @drd

…generated module

…ing API instances

…ject>

JFCote

I did a quick check. Look goods to me. I just have some points that needs clarification. Thanks for the PR.

JFCote · 2017-10-30T12:26:13Z

modules/swagger-codegen/src/main/java/io/swagger/codegen/languages/JavaClientCodegen.java

-        supportingFiles.add(new SupportingFile("auth/OAuth.mustache", authFolder, "OAuth.java"));
-        supportingFiles.add(new SupportingFile("auth/OAuthFlow.mustache", authFolder, "OAuthFlow.java"));
+        // google-api-client doesn't use the Swagger auth, because it uses Google Credential directly (HttpRequestInitializer)
+        if (!"google-api-client".equals(getLibrary())) {


I think this should go in the GoogleApiClientCodegen.java since it's only useful for your generator.

Okay, I'll do that right now, sounds like a plan.

FYI this is following the existing pattern. There actually aren't any subclasses of JavaClientCodegen; it uses if-statements on the library throughout to determine the configuration of each library. I'd have to figure out how to support a new subclass, it looks like it uses reflection to instantiate the JavaClientCodegen with the classes listed in modules/swagger-codegen/src/main/resources/META-INF/services/io.swagger.codegen.CodegenConfig

JFCote · 2017-10-30T12:30:34Z

modules/swagger-codegen/src/main/java/io/swagger/codegen/languages/JavaClientCodegen.java

        }

-        if (!("feign".equals(getLibrary()) || "resttemplate".equals(getLibrary()) || usesAnyRetrofitLibrary())) {
+        if (!("feign".equals(getLibrary()) || "resttemplate".equals(getLibrary()) || usesAnyRetrofitLibrary() || "google-api-client".equals(getLibrary()))) {


oh I see other library has done it too... @wing328, is this normal? Since we are using objects, we should'nt have any "if" in the JavaClientCodegen... It should all be in specific file of each generator...
Or there is a good reason for this ??

Yeah, it appears there are no subclasses of JavaClientCodegen, see above comment, @wing328 probably has a more complete explanation.

Ok. It's weird because for example, for the server generator "java play framework" which I maintain, there is:
JavaPlayFrameworkCodegen -> AbstractJavaCodegen -> DefaultCodegen.
This prevent having "if" everywhere.

Same thing for Typescript jQuery which I maintain too:
TypeScriptJqueryClientCodegen -> AbstractTypeScriptClientCodegen -> DefaultCodegen

I wonder why the JavaClientCodegen are all mixed in one file.
For the moment, don't change anything. We will see with @wing328 answers :)

That makes sense. Yeah, a refactor into subclasses probably belongs in a separate pull request, but I'm game to do whatever you suggest.

This is ok for now and belong in another pull request like you said. :)

When we added support for other HTTP libraries, we didn't foresee we can support so many libraries (9 HTTP libraries support so far thanks to the awesome community). I agree a separate PR to refactor into subclasses would be ideal.

That makes sense. Is this waiting on any additional reviews? Let me know if any more changes are needed here. Thanks! This will really help out our company in our migration to use Swagger clients for all of our service-to-service communication. (And possibly we could use this for our Android app, too).

@charlescapps I'll take another look later today and merge accordingly.

JFCote · 2017-10-30T12:32:27Z

modules/swagger-codegen/src/main/java/io/swagger/codegen/CodegenOperation.java

    public List<CodegenParameter> queryParams = new ArrayList<CodegenParameter>();
    public List<CodegenParameter> headerParams = new ArrayList<CodegenParameter>();
    public List<CodegenParameter> formParams = new ArrayList<CodegenParameter>();
+    public List<CodegenParameter> requiredParams = new ArrayList<>();


I think you need to put CodegenParameter in the ArrayList to be able to compile swagger-codegen in Java < 1.7

Will do right now! Yes, makes sense to support 1.6.

JFCote · 2017-10-30T12:33:35Z

modules/swagger-codegen/src/main/java/io/swagger/codegen/CodegenOperation.java

 public class CodegenOperation {
    public final List<CodegenProperty> responseHeaders = new ArrayList<CodegenProperty>();
-    public boolean hasAuthMethods, hasConsumes, hasProduces, hasParams, hasOptionalParams,
+    public boolean hasAuthMethods, hasConsumes, hasProduces, hasParams, hasOptionalParams, hasRequiredParams,


I'm surprised this is needed. Most generator use the params array and loop with "isRequired" to build almost everything. Is there any specific reason to add this to the "generic code"?

Yeah, you can check {{isRequired}} but that's not sufficient, because we use {{hasNext}} to see if there's a subsequent element, and that isn't aware of isRequired, so it was generating code with 2 commas in a row.

Ok makes sense!

…le code.

…lace.

… statements

wing328 · 2017-11-03T10:46:00Z

@charlescapps thanks for the PR, which looks good to me and my test results are good.

cc @lwander

wing328 · 2017-11-03T12:48:21Z

UPDATE: Added the new Java API client to CircleCI via cf5eba8

charlescapps · 2017-11-03T18:26:39Z

Thanks for being so responsive. I'm excited to use this new client, now we won't need to build from a fork.

wing328 · 2017-11-04T16:12:24Z

@charlescapps FYI. I've filed #6885 to skip the check for body parameter, which can be optional

Ref: #6878

wing328 · 2017-11-09T13:17:47Z

@charlescapps looks like #6885 introduces an issue. In the latest master, I ran ./bin/java-petstore-google-api-client.sh to update the Java (google-api-client) Petstore sample and notice some changes. Here is the diff for one of them:

         return apiClient.getHttpRequestFactory().buildRequest(HttpMethods.POST, genericUrl, content).execute();
     }
 
-    public HttpResponse fakeOuterBooleanSerializeForHttpResponse(Boolean body, Map<String, Object> params) throws IOException {
+    public HttpResponse fakeOuterBooleanSerializeForHttpResponse(Map<String, Object> params) throws IOException {
         Object postBody = body;
         
         UriBuilder uriBuilder = UriBuilder.fromUri(apiClient.getBasePath() + "/fake/outer/boolean");```

May I know if you've time to take a look?

charlescapps added 9 commits October 27, 2017 15:36

Add support for the google-api-client

c65e75b

When there's no return type, just return (no parsing the response)

4e1ac5d

Update the deserialization to use a Jackson typeref.

482b21a

Delete the auth mustache files as we don't need them

8445a59

Stop generating unneeded auth files; get SBT/gradle working with the …

43ed38b

…generated module

Add builder-style methods to the google-api-client ApiClient for gett…

bc1d4ea

…ing API instances

Update the README to reflect the new client library option

bc86f90

Generated overloaded methods to send query params as a Map<String, Ob…

76a157f

…ject>

Add files for Sample codes

023fd4c

JFCote reviewed Oct 30, 2017

View reviewed changes

charlescapps added 5 commits October 30, 2017 09:11

Add type in angle brackets to support Java 6

0c9d862

Required query params must be added to the Map of params. Update samp…

a118b0e

…le code.

Use explicit types instead of diamonds to support Java6 in one more p…

88ee6fc

…lace.

Clean up javadoc warnings in generated code / remove pointless return…

db0e2b7

… statements

Clean up extra newlines in generated code

ecd6d21

wing328 added Client: Java Enhancement: Feature labels Nov 2, 2017

wing328 added this to the v2.3.0 milestone Nov 2, 2017

wing328 merged commit c2eb53a into swagger-api:master Nov 3, 2017

charlescapps mentioned this pull request Mar 7, 2018

[Java][google-api-client] Empty POST requests don't send content type; List type query params #7786

Closed

Conversation

charlescapps commented Oct 27, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR checklist

Description of the PR

Motivation

Changes in this PR

Features of the generated Google ApiClient

Uh oh!

JFCote left a comment

Choose a reason for hiding this comment

Uh oh!

JFCote Oct 30, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

charlescapps Oct 30, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

charlescapps Oct 30, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

wing328 commented Nov 3, 2017

Uh oh!

wing328 commented Nov 3, 2017

Uh oh!

charlescapps commented Nov 3, 2017

Uh oh!

wing328 commented Nov 4, 2017

Uh oh!

wing328 commented Nov 9, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

charlescapps commented Oct 27, 2017 •

edited

Loading

JFCote Oct 30, 2017 •

edited

Loading

charlescapps Oct 30, 2017 •

edited

Loading

charlescapps Oct 30, 2017 •

edited

Loading