auth0-java icon indicating copy to clipboard operation
auth0-java copied to clipboard

Avoid unnecessary http client creation

Open jlannoy opened this issue 1 year ago • 3 comments

Checklist

  • [X] I have looked into the Readme and Examples, and have not found a suitable solution or answer.
  • [X] I have looked into the API documentation and have not found a suitable solution or answer.
  • [X] I have searched the issues and have not found a suitable solution or answer.
  • [X] I have searched the Auth0 Community forums and have not found a suitable solution or answer.
  • [X] I agree to the terms within the Auth0 Code of Conduct.

Describe the problem you'd like to have solved

Similar to the issue #480, the ManagmentAPI builder set the value of the httpClient to an instantiated http client, which could then be overridden by callers.

Describe the ideal solution

Objects.nonNull(httpClient) ? httpClient : DefaultHttpClient.newBuilder().build() should be used in the build() method.

Alternatives and current workarounds

None.

Additional context

No response

jlannoy avatar Feb 20 '24 21:02 jlannoy

Btw, if you could think to an other way to keep the base url in private final HttpUrl baseUrl;(inside AuthAPI and ManagementAPI) it would then be possible to exclude OkHttp3 + Okio dependencies when using an other http client.

jlannoy avatar Feb 20 '24 21:02 jlannoy

Hi @jlannoy

Thank you so much for bringing this up! We really appreciate your input. Could you please share a bit more detail or an example to help us better understand the issue? I'm not sure, but this change might potentially introduce a breaking change.

Thanks again for your insights!

tanya732 avatar Jan 31 '25 07:01 tanya732

Hello.

In the ManagementAPI class, the Builder at line 405 is instantiating a new client (private Auth0HttpClient httpClient = DefaultHttpClient.newBuilder().build();) even when it's not needed.

Example : if you call new ManagementAPI.Builder("myDomain","myToken").withHttpClient(myOwnClient).build(), a default client has been created for nothing.

The build method should be:

public ManagementAPI build() {
        return new AuthAPI(domain, clientId, clientSecret,
                Objects.nonNull(httpClient) ? httpClient : DefaultHttpClient.newBuilder().build());
}

with the httpClient null by default in the builder, instead of current:

public ManagementAPI build() {
        return new ManagementAPI(domain, SimpleTokenProvider.create(apiToken), httpClient);
}

In fact, it's the exact same change as the issue #480 except that it's for the ManagementAPI class.

jlannoy avatar Jan 31 '25 08:01 jlannoy