ocpi-toolkit icon indicating copy to clipboard operation
ocpi-toolkit copied to clipboard

Move responsability to add required headers from clients to the transport client

Open lilgallon opened this issue 2 years ago • 2 comments

this is not really part of the scope of this PR, but adding those parameters to the withRequiredHeaders makes it more obvious: it's tedious and error prone to have all the clients call this method to add the headers. I think we should move the responsibility to the transportClient to add the required headers in the send method. But I tihnk we can move forward with this PR and address this point in another PR, as well as the review of how authentication and tokens are used / named.

Originally posted by @xhanin in https://github.com/IZIVIA/ocpi-toolkit/pull/14#discussion_r1378445840


It concerns

  • .withRequiredHeader()
  • .authenticate()

lilgallon avatar Nov 02 '23 09:11 lilgallon

As this requires an object holding a reference to the PartnerRepo and partnerUrl I can see two simple ways of making this work.

a) making TransportClient abstract, and implement send to add these things before calling a doSend that needs to be implemented by actual implementation. Migration effort would be small, and extensible, as implementation could still override existing methods

b) have a "middleware" that implements send method and wraps the actual transport client

atschabu avatar Jan 11 '24 02:01 atschabu

The a) approach would be nice. It's important to be extensible on this part

lilgallon avatar Jan 11 '24 10:01 lilgallon