Generated Java client API not thread safe

[fabiankessler]

The codegen generates a DefaultApi class and an ApiClient class.
Both classes have zero Javadoc.

Intuitively, I would assume that an API is a thread safe service API. That's not the case here.
DefaultApi looks fine, but it uses the ApiClient which a) is mutable and b) stores state in class fields, for example the responseHeaders.

Either document how it's supposed to be used, and that it's not thread safe, or refactor.

[wing328]

@fabiankessler thanks for the feedback.

To ensure thread-safety, please create an instance of ApiClient per thread and let us know if you still experience any thread-safety issue.

Agreed with you that the documentation can be improved and we'll work on that.

[fabiankessler]

The generated ApiClient uses a com.sun.jersey.api.client.Client and that says in the Javadoc:

Methods to create instances of {@link WebResource} are thread-safe. Methods
that modify configuration and or filters are not guaranteed to be
thread-safe.
The creation of a Client instance is an expensive operation and
the instance may make use of and retain many resources. It is therefore
recommended that a Client instance is reused for the creation of
{@link WebResource} instances that require the same configuration settings.

You're right, with the current code of the generated ApiClient it's best to create (and keep) one per Thread. One for all is not safe. One per use is terribly slow (because the Client underneath gets thrown away).

For my use case, where I stick the ApiClient into a singleton service, I have refactored the code to be thread safe. I don't want to use ThreadLocal, and prefer having just one Client to share.

My changes:

• made ApiClient immutable: created an ApiClient.Builder for the configuration options with defaultHeaderMap, debugging, basePath, json and so on
• removed the authentications stuff (don't use it)
• removed responseHeaders and statusCode. instead the invokeAPI() method returns a Response object containing the ClientResponse, plus the result (generic type T).
• hostMap is static and a ConcurrentHashMap

Here it is: https://gist.github.com/fabiankessler/a6cb51e247a8c450d4aa

But I think for the general use case, with authentication capabilities, the one-per-thread is better. Still, you could use the Builder, it looks more cleaned up than having setters all over the place.

[wing328]

@fabiankessler besides jersey 1.x, you can also use other Java HTTP library (e.g. Jersey 2.x, okhttp) in the Java API client. Please run java -jar modules/swagger-codegen-cli/target/swagger-codegen-cli.jar config-help -l java for more information.

[fabiankessler]

The generated code for Jersey2 creates a new Client on every call to invokeAPI(). See https://github.com/swagger-api/swagger-codegen/blob/master/samples/client/petstore/java/jersey2/src/main/java/io/swagger/client/ApiClient.java

Also, the Client is never closed.

The implementation is org.glassfish.jersey.client.JerseyClient. The interface is javax.ws.rs.client.Client (2.0) and says:

Clients are heavy-weight objects that manage the client-side communication
infrastructure. Initialization as well as disposal of a {@code Client} instance
may be a rather expensive operation. It is therefore advised to construct only
a small number of {@code Client} instances in the application. Client instances
must be {@link #close() properly closed} before being disposed to avoid leaking
resources.

Why is this ApiClient not creating just one and re-using it?

A test on my localhost with client/server using this generated code for 1000 "hello world" service requests takes 11 seconds. With re-use of the Client it takes 2.7 seconds. (A similar SOAP service with generated client code takes 2.9 seconds.)

[fabiankessler]

Thanks for the hint on Jersey 2. I was not aware of that. It would be really cool if the editor at http://editor.swagger.io/ could do that. If it would bring up a window with options.
The existing is cool already, but unfortunately it hides all the additional coolness.

[wing328]

@fabiankessler thanks for the feedback. We'll update the Jersey2 API client with your suggestion above - creating just one and re-using it.

[wing328]

@fabiankessler the PR to improve Jersey2's ApiClient has been merged into master. Please pull the latest and let us know if you've further feedback.

[wing328]

@fabiankessler better readme has been added to the Java API client. Please let us know if you've further feedback. Closing this task for now.