Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(ocpi-2.2.1): Add Content-Type header on requests (if needed), on all responses & fixed headers case sensitivity #14

Merged

Conversation

lilgallon
Copy link
Member

@lilgallon lilgallon commented Oct 27, 2023

Change notes

Only for ocpi 2.2.1

  • Content-Type: application/json header is now added on requests if the body is not null
  • Content-Type: application/json header is now added on all responses
  • Headers are not case sensitive anymore. It means that X-Request-ID and X-Correlation-ID are properly propagated in responses even if the case does not match. Note: Authorization header was working because it also accepted authorization
  • It is now possible to define how X-Request-ID and X-Correlation-ID are generated. You have to override methods in your TransportClient implementation. By default, the behaviour stays the same, it generates a random UUID.

Migration guide

If you manually added Content-Type header & normalized headers in your Transport implementation, you can remove it.

Otherwise, you do not need to update anything. All the changes are included in the lib.

Notes

For versions 2.1.1 and 2.1.1 Gireve, two issues have been open:

…, & on all responses

- HttpUtils
- OcpiResponseBody
@lilgallon lilgallon requested a review from xhanin October 27, 2023 14:31
@lilgallon lilgallon force-pushed the fix/add-missing-headers-and-fix-post-credentials-response branch from b523bcc to e32977a Compare October 27, 2023 14:41
It fixes debug headers not being sent in response if partner sends them in lowercase
@lilgallon lilgallon force-pushed the fix/add-missing-headers-and-fix-post-credentials-response branch from e32977a to d2728dc Compare October 27, 2023 14:42
@lilgallon lilgallon changed the title fix(ocpi-2.2.1): Add Content-Type header on requests (if needed), on all responses & fixed header case sensitivity fix(ocpi-2.2.1): Add Content-Type header on requests (if needed), on all responses & fixed headers case sensitivity Oct 27, 2023
@lilgallon
Copy link
Member Author

lilgallon commented Oct 27, 2023

I will also add an option to specify explicitely the X-Request-ID and X-Correlation-ID. It will be useful to test that these headers are properly propagated.

It can be useful if the users have a traceId in the coroutine context and want to use it

@lilgallon
Copy link
Member Author

It is now possible to define how X-Request-ID and X-Correlation-ID are generated. Two methods were added to TransportClient interface. By default, the behaviour stays the same, it generates a random UUID.

I also greatly improved Credentials tests by checking every request / response done during the process. This mecanisme can be reused in integration tests for anything else.

@lilgallon lilgallon requested a review from xhanin October 31, 2023 14:17
Copy link

sonarcloud bot commented Oct 31, 2023

Please retry analysis of this Pull-Request directly on SonarCloud.

Copy link
Contributor

@xhanin xhanin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have some feedbacks but that's beyond the scope of this PR. So I approve it like this, but I would like to discuss the approach used for the client and for the tokens before moving on further with more modules implementations.

@lilgallon lilgallon merged commit a4ea337 into main Nov 2, 2023
1 of 2 checks passed
@lilgallon lilgallon deleted the fix/add-missing-headers-and-fix-post-credentials-response branch November 2, 2023 09:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants