Java target: okhttp-gson-generated classes cleaned up, OAuth supports… #6405

rdolejsi · 2017-08-30T11:39:02Z

… supplying BearerProvider, ApiClient supports RequestDecorator

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 for non-breaking changes and 3.0.0 branch for breaking (non-backward compatible) changes.

Description of the PR

okhttp-gson-generated classes cleaned up, human-readable now
OAuth supports supplying BearerProvider

(details of the change, additional tests that have been done, reference to the issue for tracking, etc)

wing328 · 2017-08-31T14:51:43Z

Thanks for the PR but your commit (as shown in the Commits tab) is not linked to your Github account, which means this PR won't count as your contribution in https://github.com/swagger-api/swagger-codegen/graphs/contributors.

Let me know if you need help fixing it.

Ref: https://github.com/swagger-api/swagger-codegen/wiki/FAQ#how-can-i-update-commits-that-are-not-linked-to-my-github-account

wing328 · 2017-08-31T14:52:29Z

cc @cbornet @ePaul @bbdg @JFCote @sreeshas @jfiala

JFCote · 2017-08-31T16:15:50Z

As far as I'm concerned, the change seems good. The code is clean. But I don't know okhttp at all so others advice might be needed.

wing328 · 2017-09-02T10:32:22Z

cc @cbornet @ePaul as well

cbornet · 2017-09-02T20:06:52Z

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

        if(!"resttemplate".equals(getLibrary())) {
            supportingFiles.add(new SupportingFile("StringUtil.mustache", invokerFolder, "StringUtil.java"));
        }

        supportingFiles.add(new SupportingFile("auth/HttpBasicAuth.mustache", authFolder, "HttpBasicAuth.java"));
        supportingFiles.add(new SupportingFile("auth/ApiKeyAuth.mustache", authFolder, "ApiKeyAuth.java"));
        supportingFiles.add(new SupportingFile("auth/OAuth.mustache", authFolder, "OAuth.java"));
+        supportingFiles.add(new SupportingFile("auth/OAuthBearerProvider.mustache", authFolder, "OAuthBearerProvider.java"));


This file shouldn't be added for retrofit library

I can't make this conditional at the moment as OAuthBearerProvider is required to meaningfully implement OAuth and thus OAuth depends on this interface now - I don't seem to have identified a way to sensibly attach myself within OAuth lifecycle to supply new Bearer - one at the first server hit, a renewed one once the previous one expires.

As this is an interface only and very crucial for our needs, can you please elaborate more on why this can't be included when retrofit library is in place? Maybe I'm overlooking some basic concept..

Retrofit has its own OAuth class that doesn't depend on OAuthBearerProvider so this would be dead code.

Oh, ok. That means it does not have even OAuth itself, right? So the same rules should be applicable as for OAuth class, which is in there already. Or am I missing something?

Double-checked the generation procedures again and got you - multiple OAuth classes are available based on the used library, I'll change the JavaClientCodegen accordingly.

cbornet · 2017-09-02T20:10:35Z

modules/swagger-codegen/src/main/resources/Java/libraries/okhttp-gson/ApiClientHolder.mustache

+
+package {{invokerPackage}};
+
+public interface ApiClientHolder {


~~This doesn't seem to be used anywhere. What is it for ?~~
Sorry, didn't see the full diff...

Just to elaborate a bit more on this - the client Api's generated by swagger do not implement any interface. ApiClientHolder is what it describes - a holder of ApiClient. It acts for us both as a way to reach ApiClient within generated Api as well as a way to hold more generated Api's and organize them meaningfully (better than using plain Object).

If I think from the top of my head, the other possibility would be to introduce something like SwaggerClient or SwaggerApi which would characterize the generated Api and allow manipulations with it. It would expose ApiClient within any such Api - just like ApiClientHolder does now.

What do you think?

cbornet · 2017-09-02T20:44:44Z

@rdolejsi could you explain what the ApiClientRequestDecorator brings compared to an Interceptor ?

rdolejsi · 2017-09-04T08:40:05Z

RequestDecorator is indeed used to intercept outgoing request builder to decorate the request (with security features, extra headers, etc.). As there are no generic interceptors (just one HttpLoggingInterceptor), I chose the name to denote the pattern it's doing - decorating.

On a second thought, we might as well introduce OutgoingRequestInterceptor to suit wider needs - what do you think?

cbornet · 2017-09-04T09:13:02Z

Okhttp has a generic Interceptor interface so how about just providing an addInterceptor(Interceptor interceptor) method ?

rdolejsi · 2017-09-04T10:12:50Z

Looks like Interceptor is called only after the request is built, thus allows to work with quite immutable objects. The current RequestDecorator (or be it OutgoingRequestInterceptor) allows you to work with Request.Builder, thus gaining access to the request structure before the actual request is created.

rdolejsi · 2017-09-04T11:54:12Z

New code version for this PR: OAuthBearerProvider related to base OAuth only, ApiClientHolder.mustache moved to correct scope - tests no longer fail.

… supplying BearerProvider, ApiClient supports RequestDecorator

cbornet · 2017-09-19T06:04:30Z

@rdolejsi can you generate the java client samples ?

wing328 · 2017-09-23T14:56:09Z

@rdolejsi I got some errors after updating Java okhttp-gson samples:

[INFO] ------------------------------------------------------------------------
[ERROR] Failed to execute goal org.apache.maven.plugins:maven-compiler-plugin:3.1:compile (default-compile) on project swagger-petstore-okhttp-gson: Compilation failure: Compilation failure:
[ERROR] /private/tmp/swagger-codegen/samples/client/petstore/java/okhttp-gson/src/main/java/io/swagger/client/api/UserApi.java:[377,57] ')' expected
[ERROR] /private/tmp/swagger-codegen/samples/client/petstore/java/okhttp-gson/src/main/java/io/swagger/client/api/UserApi.java:[377,70] not a statement
[ERROR] /private/tmp/swagger-codegen/samples/client/petstore/java/okhttp-gson/src/main/java/io/swagger/client/api/UserApi.java:[377,76] ';' expected
[ERROR] /private/tmp/swagger-codegen/samples/client/petstore/java/okhttp-gson/src/main/java/io/swagger/client/api/UserApi.java:[377,97] not a statement
[ERROR] /private/tmp/swagger-codegen/samples/client/petstore/java/okhttp-gson/src/main/java/io/swagger/client/api/UserApi.java:[377,113] ';' expected
[ERROR] /private/tmp/swagger-codegen/samples/client/petstore/java/okhttp-gson/src/main/java/io/swagger/client/api/UserApi.java:[377,128] not a statement
[ERROR] /private/tmp/swagger-codegen/samples/client/petstore/java/okhttp-gson/src/main/java/io/swagger/client/api/UserApi.java:[377,144] ';' expected

Please take a look when you've time.

wing328 · 2017-10-03T13:38:34Z

@rdolejsi may I know if you need help fixing the error? Seems like it's missing a ) or ;

…ct-specific type adapters

…lasses for query-based calls

…n serializer options

wing328 added Client: Java Enhancement: Feature labels Aug 31, 2017

wing328 added this to the v2.3.0 milestone Aug 31, 2017

cbornet reviewed Sep 2, 2017

View reviewed changes

rdolejsi force-pushed the java-okhttp-gson-cleanup branch from 3243dfb to 5d078b7 Compare September 4, 2017 11:47

Java target: okhttp-gson-generated classes cleaned up, OAuth supports…

bf99a2c

… supplying BearerProvider, ApiClient supports RequestDecorator

rdolejsi force-pushed the java-okhttp-gson-cleanup branch from 5d078b7 to bf99a2c Compare September 4, 2017 12:16

Roman Dolejsi and others added 3 commits October 24, 2017 17:11

Java target: okhttp-gson-generated classes: JSON supports adding obje…

609b436

…ct-specific type adapters

Java target: okhttp-gson-generated classes: Typo fixes in generated c…

9039757

…lasses for query-based calls

Java target: okhttp-gson-generated classes: JSON supports setting Gso…

3c8a9a6

…n serializer options

wing328 modified the milestones: v2.3.0, v2.4.0 Dec 18, 2017

frantuma added the Language: java label Jun 27, 2018

frantuma force-pushed the master branch from 4ba5164 to 949f0e6 Compare May 20, 2025 09:13


		package {{invokerPackage}};

		public interface ApiClientHolder {

Java target: okhttp-gson-generated classes cleaned up, OAuth supports… #6405

Are you sure you want to change the base?

Java target: okhttp-gson-generated classes cleaned up, OAuth supports… #6405

Uh oh!

Conversation

rdolejsi commented Aug 30, 2017

PR checklist

Description of the PR

Uh oh!

wing328 commented Aug 31, 2017

Uh oh!

wing328 commented Aug 31, 2017

Uh oh!

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

Uh oh!

Uh oh!

wing328 commented Sep 2, 2017

Uh oh!

cbornet Sep 2, 2017

Choose a reason for hiding this comment

Uh oh!

rdolejsi Sep 4, 2017

Choose a reason for hiding this comment

Uh oh!

cbornet Sep 4, 2017

Choose a reason for hiding this comment

Uh oh!

rdolejsi Sep 4, 2017

Choose a reason for hiding this comment

Uh oh!

rdolejsi Sep 4, 2017

Choose a reason for hiding this comment

Uh oh!

cbornet Sep 2, 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!

rdolejsi Sep 4, 2017

Choose a reason for hiding this comment

Uh oh!

cbornet commented Sep 2, 2017

Uh oh!

rdolejsi commented Sep 4, 2017

Uh oh!

cbornet commented Sep 4, 2017

Uh oh!

rdolejsi commented Sep 4, 2017

Uh oh!

rdolejsi commented Sep 4, 2017

Uh oh!

cbornet commented Sep 19, 2017

Uh oh!

wing328 commented Sep 23, 2017

Uh oh!

wing328 commented Oct 3, 2017

Uh oh!

Uh oh!

JFCote commented Aug 31, 2017 •

edited

Loading

cbornet Sep 2, 2017 •

edited

Loading