adyen-java-api-library icon indicating copy to clipboard operation
adyen-java-api-library copied to clipboard

Allow reuse and customization of HttpClient

Open vpavic opened this issue 2 years ago • 7 comments

Java version: 11 Library version: 17.2.0

Is your feature request related to a problem? Please describe. At present, a new instance of HttpClient is created for every request as can be seen here: https://github.com/Adyen/adyen-java-api-library/blob/21ae8bf5eb16a2375333da405f3b79cdbd431fa1/src/main/java/com/adyen/httpclient/AdyenHttpClient.java#L107-L120

This is sub-optimal for a couple of reasons:

  • it presents a significant overhead for systems that frequently interact with Adyen's API
  • it makes customization of the underlying client very difficult

Describe the solution you'd like Ideally, AdyenHttpClient should reuse the same HttpClient and make it possible for users to supply their own client.

Describe alternatives you've considered ~Subclassing AdyenHttpClient to override the AdyenHttpClient#request(String, String, Config, boolean, RequestOptions, HttpMethod, Map<String,String>) but that far from being a clean solution.~

This is not possible due to AdyenHttpClient#createRequest being private and AdyenResponse being package private.

Additional context n/a

I'm willing to provide a PR to address this if that would speed up delivery of this improvement.

vpavic avatar Nov 11 '21 23:11 vpavic

Hello @vpavic,

Thank you for opening this issue. You can create your own custom client and pass it here
If you would like to extend it and not to create a new client with every call, feel free to open a pr and we are going to review it.

regards, Alexandros Adyen

AlexandrosMor avatar Dec 02 '21 14:12 AlexandrosMor

Was this implemented in the meanwhile or is there some other reason why the issue was closed?

I planned to take a look at this in January and hopefully prepare the PR.

vpavic avatar Dec 23 '21 14:12 vpavic

Hey @vpavic,

I closed the issue due to 3 weeks of inactivity and no response. I was unaware you were still planning to work on this and hence wanted the issue to remain open. I've reopened it for your convenience!

Kind Regards, Wouter Adyen

wboereboom avatar Dec 23 '21 14:12 wboereboom

Sorry about my lack of response and thanks for reopening the issue. :+1:

vpavic avatar Dec 24 '21 08:12 vpavic

Hi @AlexandrosMor , my problem is kinda related so i'll just ask here.

You can create your own custom client and pass it here

We'd like to set interceptors on the HttpClient to log the requests for testing. I'd prefer to extend AdyenHttpClient and overwrite createCloseableHttpClient(config) to add the interceptors to the client when its created. But all methods of AdyenHttpClient are private, except the request()-Methods from the ClientInterface. Could we make these Methods protected so that i could actually reuse the code from AdyenHttpClient? Alternatively i could make a pull request to add interceptors to AdyenHttpClient through the Config-Object or something like that.

kind regards, Lukas

LukasDrP avatar Jan 19 '22 14:01 LukasDrP

Hi @LukasDrP

I can create a PR for it. But before It. Whenever you just need to log requests only why do we need an interceptor on the config function? RequestHooks option might be better for your logging case. Isn't it?

Hi, @LukasDrP @wboereboom @AlexandrosMor I am new at this repo. A quick question just to learn the use case. I am confused about setConfig option who needs to change the configuration(Api-key, truststore etc...) at runtime by Client.setConfig. If we need a client with a different configuration why don't create the other client than using the same client with different configurations? While creating PR. It makes the maintainability very hard. And also ClientInterface too, Why we don't make it as config stateful rather than make it as config stateless by not passing config as a parameter to each function?

Without breaking backward compatibility. I will try to create a PR for adding hooks and reuse of the HttpClient

kind regards, Yusuf

ykalayy avatar Jul 19 '22 00:07 ykalayy

Hi all, I opened PR for this a while ago. Could you please review it? @AlexandrosMor @wboereboom @Morerice @michaelpaul @jillingk I am open to other possible solutions. I just only need a feedback

Thanks, Yusuf

ykalayy avatar Aug 09 '22 20:08 ykalayy