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

feat: dio client interceptor #5

Closed
wants to merge 38 commits into from

Conversation

thorvalld
Copy link
Contributor

  • Added Dio Client interception

@Soap-141 Soap-141 self-requested a review March 19, 2024 13:55
lib/src/client/dio_interceptor.dart Outdated Show resolved Hide resolved
lib/src/client/dio_interceptor.dart Outdated Show resolved Hide resolved
lib/src/client/dio_interceptor.dart Outdated Show resolved Hide resolved
lib/src/client/dio_interceptor.dart Outdated Show resolved Hide resolved
lib/src/client/dio_interceptor.dart Outdated Show resolved Hide resolved
@thorvalld thorvalld requested a review from jeanplevesque March 21, 2024 16:40
@thorvalld thorvalld dismissed jeanplevesque’s stale review March 21, 2024 16:43

resolved, clockskew adjusted and test case added

test/client/dio_interceptor_test.dart Outdated Show resolved Hide resolved
test/client/dio_interceptor_test.dart Outdated Show resolved Hide resolved
test/client/dio_interceptor_test.dart Outdated Show resolved Hide resolved
@thorvalld thorvalld requested a review from jeanplevesque April 9, 2024 06:44
test/client/dio_interceptor_test.dart Outdated Show resolved Hide resolved
test/client/dio_interceptor_test.dart Outdated Show resolved Hide resolved
@Soap-141 Soap-141 self-requested a review April 9, 2024 12:43
test/client/dio_interceptor_test.dart Outdated Show resolved Hide resolved
test/client/dio_interceptor_test.dart Outdated Show resolved Hide resolved
@thorvalld thorvalld requested a review from jeanplevesque April 12, 2024 08:19
test/client/dio_interceptor_test.dart Outdated Show resolved Hide resolved
test/client/dio_interceptor_test.dart Outdated Show resolved Hide resolved
lib/src/client/dio_interceptor.dart Outdated Show resolved Hide resolved
test/client/dio_interceptor_test.dart Outdated Show resolved Hide resolved
test/client/dio_interceptor_test.dart Outdated Show resolved Hide resolved
test/client/dio_interceptor_test.dart Outdated Show resolved Hide resolved
test/client/dio_interceptor_test.dart Outdated Show resolved Hide resolved
test/client/dio_interceptor_test.dart Outdated Show resolved Hide resolved
@thorvalld
Copy link
Contributor Author

@jeanplevesque I still haven't received any change requests yet!

@jeanplevesque
Copy link
Member

@jeanplevesque I still haven't received any change requests yet!

I haven't had time to check yet. I'll try to look soon.


// Register the mock response
dioAdapter.onGet(baseEndpoint, (request) {
options.disableAutoRetryOnClockSkew
Copy link
Member

Choose a reason for hiding this comment

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

Like I said before, the mocked server doesn't know about the configuration of the interceptor so it's weird that you check it. The test server should behave the same no matter the test case.

At high level, the test server must always do the same thing: it returns an unauthorized response when the HMAC signature is invalid and a ok response when the HMAC signature is valid. In order to be valid, the timestamp part of the signature must match the one of the server.

expect(response.requestOptions.headers, contains(headerName));
});

test('Interceptor returns OK with auto-retry tuned to false', () async {
Copy link
Member

Choose a reason for hiding this comment

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

Interceptor returns OK with auto-retry tuned to false

What is this testing?

When the auto-retry is false, it means that the interceptor must not adjust its timestamps using the clock skew. The result of this should be that you ultimately get an unauthorized response from the server.

To get an OK response, you would have to send a valid signature, which implies that you wouldn't need the auto-retry mechanism anyways.


dioAdapter.onGet(baseEndpoint, (request) {
requestCount++;
request.reply(HttpStatus.ok, emptyDataInjection);
Copy link
Member

Choose a reason for hiding this comment

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

Same thing here, it doesn't make sense for you to return an OK if you want to test the auto-retry mechanisms. For the interceptor to consider auto-retrying, you need an unauthorized in the first place.

dio.interceptors.add(interceptor);

dioAdapter.onGet(baseEndpoint, (request) {
request.reply(HttpStatus.ok, {});
Copy link
Member

Choose a reason for hiding this comment

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

Again, if your server always return OK, how are you testing the validity of the timestamp of the signature?

@mergify mergify bot mentioned this pull request Apr 19, 2024
10 tasks
@takla21
Copy link
Contributor

takla21 commented Apr 19, 2024

I'm taking over this feature. I'm closing this PR in favor on this one #7

@takla21 takla21 closed this Apr 19, 2024
@takla21 takla21 deleted the dev/anbe/dio-client-interception branch April 24, 2024 18:18
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.

4 participants