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

✨ Restructure interceptors #547

Merged
merged 34 commits into from
Apr 5, 2024

Conversation

Guldem
Copy link
Contributor

@Guldem Guldem commented Jan 5, 2024

Restructuring interceptors

This PR/Draft contains the current progress/experimenting on restructuring interceptors. See #544 for more information on the proposal. Any feedback/help is welcome!

Description

Refactor interceptors into a chain of interceptors. This will reduce the amount of different type interceptor interfaces to 1. A Interceptor will be responsible for both the Request and the Response if needed.

Specifying a interceptor would look like this:

class MyInterceptor implements Interceptor {
 @override
 FutureOr<Response<BodyType>> intercept<BodyType>(Chain<BodyType> chain) async {
    final request = chain.request;
    final updatedRequest = // Do something with intercepted request here

    final response = await chain.proceed(updatedRequest);
    final updatedResponse =  // Do something with intercepted response.

    return updatedResponse;
  }
}

All Interceptor are chained so the Request will first go down the chain of interceptors and in the last step the network call is make. The Response will then go back through the chain of interceptors.

Besides user created interceptors some of the internal logic have been refactor into InternalInterceptor.

InternalInterceptors:

  • RequestConverterInterceptor: Wrapper interceptor that wraps request converters
  • ResponseConverterInterceptor: Wrapper interceptor that wraps response converters
  • HttpCallInterceptor: Interceptor that makes the actual http call
  • AuthenticatorInterceptor: Wrapper interceptor that wraps the authenticator

Progress

  • Setup the actual chain
  • Include converters in the chain
  • Include authenticator in the chain
  • Refactor HeadersInterceptor, CurlInterceptor and HttpLoggingInterceptor to use new interface.
  • Update HttpLoggingInterceptor to track call time.
  • Check/Fix exception handling
  • Update existing tests
  • Add new tests
  • Add dart doc to code
  • Add documentation

Breaking changes

  • RequestInterceptor is removed
  • Function RequestInterceptors are removed
  • ResponseInterceptor is removed
  • Function ResponseInterceptors are removed

Copy link

codecov bot commented Jan 5, 2024

Codecov Report

Attention: Patch coverage is 96.27329% with 6 lines in your changes are missing coverage. Please review.

Project coverage is 92.85%. Comparing base (84471e4) to head (25f6d71).

Files Patch % Lines
chopper/lib/src/chopper_exception.dart 0.00% 3 Missing ⚠️
chopper/lib/src/chain/interceptor_chain.dart 95.45% 1 Missing ⚠️
...er/lib/src/interceptors/http_call_interceptor.dart 90.00% 1 Missing ⚠️
...lib/src/interceptors/http_logging_interceptor.dart 96.87% 1 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##           develop     #547      +/-   ##
===========================================
- Coverage    93.52%   92.85%   -0.67%     
===========================================
  Files           12       22      +10     
  Lines          463      490      +27     
===========================================
+ Hits           433      455      +22     
- Misses          30       35       +5     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@techouse techouse assigned techouse and Guldem and unassigned techouse Mar 2, 2024
@techouse techouse added the enhancement New feature or request label Mar 2, 2024
@lwj1994 lwj1994 mentioned this pull request Mar 18, 2024
@techouse
Copy link
Collaborator

techouse commented Apr 4, 2024

@Guldem you'll have to resolve some merge conflicts that got generated by #592 it seems ❤️‍🩹

@Guldem
Copy link
Contributor Author

Guldem commented Apr 5, 2024

@Guldem you'll have to resolve some merge conflicts that got generated by #592 it seems ❤️‍🩹

Oooh no! 😱 Thank will take a look at it.

…structure_interceptors

# Conflicts:
#	chopper/lib/chopper.dart
@techouse techouse merged commit ab5a80f into lejard-h:develop Apr 5, 2024
6 checks passed
@techouse
Copy link
Collaborator

techouse commented Apr 5, 2024

@Guldem I'll prep a pre-release PR for this baby

techouse added a commit that referenced this pull request Apr 5, 2024
# chopper

## 8.0.0-rc.1

- Restructure interceptors ([#547](#547))

# chopper_generator

## 8.0.0-rc.1

- Restructure interceptors ([#547](#547))
@techouse techouse linked an issue Apr 5, 2024 that may be closed by this pull request
techouse added a commit that referenced this pull request Apr 6, 2024
# chopper

## 8.0.0-rc.1

- Restructure interceptors ([#547](#547))

# chopper_generator

## 8.0.0-rc.1

- Restructure interceptors ([#547](#547))

---------

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: Job Guldemeester <jobguldemeester@gmail.com>
@techouse
Copy link
Collaborator

techouse commented Apr 6, 2024

@Guldem this has now been published as a pre-release v8.0.0-rc.1

Thank you soooo much for your hard work :)

CC/ @JEuler

@Guldem
Copy link
Contributor Author

Guldem commented Apr 6, 2024

@Guldem this has now been published as a pre-release v8.0.0-rc.1

Thank you soooo much for your hard work :)

CC/ @JEuler

Cool! Thanks for the feedback and support :)

@Guldem Guldem deleted the feature/restructure_interceptors branch April 6, 2024 18:40
techouse added a commit that referenced this pull request May 3, 2024
# chopper

## 8.0.0

- Restructure interceptors ([#547](#547))
- Add per-request timeout ([#604](#604))

# chopper_generator

## 8.0.0

- Restructure interceptors ([#547](#547))
- Add per-request timeout ([#604](#604))

## 3.0.0

- Require Chopper ^8.0.0
@techouse techouse mentioned this pull request May 3, 2024
@techouse
Copy link
Collaborator

techouse commented May 5, 2024

@Guldem I've been using this PR in a large scale app for a week now and I just can't reiterate on how amazing this piece of work is. Thanx 🙏

@Guldem
Copy link
Contributor Author

Guldem commented May 5, 2024

@Guldem I've been using this PR in a large scale app for a week now and I just can't reiterate on how amazing this piece of work is. Thanx 🙏

Glad to hear, thanks!

techouse added a commit that referenced this pull request May 9, 2024
# chopper

## 8.0.0

- Restructure interceptors ([#547](#547))
- Add per-request timeout ([#604](#604))

# chopper_generator

## 8.0.0

- Restructure interceptors ([#547](#547))
- Add per-request timeout ([#604](#604))

## 3.0.0

- Require Chopper ^8.0.0

---------

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: Job Guldemeester <job@dutchcodingcompany.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Improve interceptor structure/base structure [Feature Request] Allow to add interceptors after constructor
4 participants