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

Support @Tag #586

Merged
merged 1 commit into from Mar 20, 2024
Merged

Support @Tag #586

merged 1 commit into from Mar 20, 2024

Conversation

lwj1994
Copy link
Contributor

@lwj1994 lwj1994 commented Mar 18, 2024

Add tag

@Tag parameter annotation for setting tag on the underlying Chopper Request object. These can be read
in Converters or Interceptors for tracing, analytics, varying behavior, and more.

if want to filter null value or empty String for some url. we can make an IncludeBodyNullOrEmptyTag Object as Tag.

class IncludeBodyNullOrEmptyTag {
  bool includeNull = false;
  bool includeEmpty = false;

  IncludeBodyNullOrEmptyTag(this.includeNull, this.includeEmpty);
}

@get(path: '/include')
Future<Response> includeBodyNullOrEmptyTag(
    {@Tag()
    IncludeBodyNullOrEmptyTag tag = const IncludeBodyNullOrEmptyTag()});

get tag via request.tag in Converter or Interceptor:

class TagConverter extends JsonConverter {
  FutureOr<Request> convertRequest(Request request) {
    final tag = request.tag;
    if (tag is IncludeBodyNullOrEmptyTag) {
      if (request.body is Map) {
        final Map body = request.body as Map;
        final Map bodyCopy = {};
        for (final MapEntry entry in body.entries) {
          if (!tag.includeNull && entry.value == null) continue;
          if (!tag.includeEmpty && entry.value == "") continue;
          bodyCopy[entry.key] = entry.value;
        }
        request = request.copyWith(body: bodyCopy);
      }
    }
  }
}

Copy link

codecov bot commented Mar 18, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 94.17%. Comparing base (8ff4ecd) to head (c5ac0bd).

Additional details and impacted files
@@           Coverage Diff            @@
##           develop     #586   +/-   ##
========================================
  Coverage    94.17%   94.17%           
========================================
  Files           11       11           
  Lines          481      481           
========================================
  Hits           453      453           
  Misses          28       28           

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

@techouse
Copy link
Collaborator

techouse commented Mar 18, 2024

Hey,

Thanx for the PR, however, I'm not sure I understand what this @Tag annotation should be used for.

Can you add some concrete examples and/or tests please?

@techouse techouse self-assigned this Mar 18, 2024
@techouse techouse added the enhancement New feature or request label Mar 18, 2024
@lwj1994
Copy link
Contributor Author

lwj1994 commented Mar 18, 2024

Hey,

Thanx for the PR, however, I'm not sure I understand what this @Tag annotation should be used for.

Can you add some concrete examples and/or tests please?

ok. i will add it later.

Let me start with a small example .

some path need check token to trigger refresh token when token is out of date.

we can determine the path in interceptor:

class  TokenInterceptor extends RequestInterceptor{
   request( ) {
      if (path.startWith("/xxx/xx")  
        || path.startWith("/xxx/xx")
        || path.startWith("/xxx/xx")
        || path.startWith("/xxx/xx")
       // ...... more
){
           // wait to refresh token 
          await ensureTokenIsValid();
          request.headers.add["Auth":  token];
      }
   }
}

If those token-dependent requests are tagged, then no judgment is required。

if (request.tags["needToken"] != null){
           // wait to refresh token 
          await ensureTokenIsValid();
          request.headers.add["Auth":  token];
      }

@techouse
Copy link
Collaborator

techouse commented Mar 18, 2024

Why don't you simply catch an expired token, refresh it and repeat the call once more?

(request) --> (error 401: token expired) --> (auth interceptor refresh token) --> (repeat request) --> (ok 200)

@techouse techouse added question Further information is requested and removed enhancement New feature or request labels Mar 18, 2024
@lwj1994
Copy link
Contributor Author

lwj1994 commented Mar 18, 2024

Why don't you simply catch an expired token, refresh it and repeat the call once more?

(request) --> (error 401: token expired) --> (auth interceptor refresh token) --> (repeat request) --> (ok 200)

thx, it looks more simple,but the BodyType will lose type, just as dynamic, how to get origin response's BodyType

class AuthInterceptor implements chopper.ResponseInterceptor {

  @override
  FutureOr<chopper.Response<dynamic>> onResponse(chopper.Response<dynamic> response) async{
    if(response.statusCode == 401){
      await refreshToken();
      // resend
      response = await chopperClient.send<BodyType>(response.base.request as chopper.Request);
    }
    return response;
  }
}

i noticed that 👍🏻 after #547 interceptor can get type.

@techouse
Copy link
Collaborator

i noticed that 👍🏻 after #547 interceptor can get type.

Correct! Can you please test #547 and let @Guldem know if everything works for you. It's a bigger/breaking PR so we'd be grateful for any real-world tests. 😇

@Guldem
Copy link
Contributor

Guldem commented Mar 19, 2024

It's nice to add meta information to a request but I personally never needed something like this. Like @techouse says in most cases it can be solved with a interceptor. But on the other side it can't really hurt 😄

requests.md Outdated Show resolved Hide resolved
requests.md Outdated Show resolved Hide resolved
@techouse techouse added enhancement New feature or request and removed question Further information is requested labels Mar 19, 2024
@lwj1994
Copy link
Contributor Author

lwj1994 commented Mar 20, 2024

hi guys, i refactored tag, make tag as a Object instead of Map<String, dynamic>. and i add my showcase in chopper/example/tag.dart. Maybe @tag not very useful, but some cases are used as a means of tracking requests.

Add tag

@Tag parameter annotation for setting tag on the underlying Chopper Request object. These can be read
in Converters or Interceptors for tracing, analytics, varying behavior, and more.

if want to filter null value or empty String for some url. we can make an IncludeBodyNullOrEmptyTag Object as Tag.

class IncludeBodyNullOrEmptyTag {
  bool includeNull = false;
  bool includeEmpty = false;

  IncludeBodyNullOrEmptyTag(this.includeNull, this.includeEmpty);
}

@get(path: '/include')
Future<Response> includeBodyNullOrEmptyTag(
    {@Tag()
    IncludeBodyNullOrEmptyTag tag = const IncludeBodyNullOrEmptyTag()});

get tag via request.tag in Converter or Interceptor:

class TagConverter extends JsonConverter {
  FutureOr<Request> convertRequest(Request request) {
    final tag = request.tag;
    if (tag is IncludeBodyNullOrEmptyTag) {
      if (request.body is Map) {
        final Map body = request.body as Map;
        final Map bodyCopy = {};
        for (final MapEntry entry in body.entries) {
          if (!tag.includeNull && entry.value == null) continue;
          if (!tag.includeEmpty && entry.value == "") continue;
          bodyCopy[entry.key] = entry.value;
        }
        request = request.copyWith(body: bodyCopy);
      }
    }
  }
}

Copy link
Contributor

@Guldem Guldem left a comment

Choose a reason for hiding this comment

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

LGTM

@JEuler
Copy link
Collaborator

JEuler commented Mar 20, 2024

LGTM

@techouse techouse merged commit 7c65a25 into lejard-h:develop Mar 20, 2024
6 checks passed
@techouse techouse added this to the 7.3.0 milestone Mar 21, 2024
techouse added a commit that referenced this pull request Mar 23, 2024
# chopper

## 7.3.0

- Add support for `@Tag` annotation ([#586](#586))

# chopper_generator

## 7.3.0

- Add support for `@Tag` annotation ([#586](#586))
@techouse techouse mentioned this pull request Mar 23, 2024
techouse added a commit that referenced this pull request Apr 3, 2024
* 🔖 release v7.3.0

# chopper

## 7.3.0

- Add support for `@Tag` annotation ([#586](#586))

# chopper_generator

## 7.3.0

- Add support for `@Tag` annotation ([#586](#586))

---------

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: wenchieh <alwjlola@gmail.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.

None yet

4 participants