Migrate to nullsatefy and Dio 4

[erickok]

Migrates to Dart 2.12 and enabled nullsafety. This also upgrades to Dio 4's new interceptors with handlers.

The example was updated for Android to use v3 embedding. Updated iOS example app Podfile.

--

Similar to #81 but compatible with Dio 4.

[alr2413]

ping @hurshi.
please merge it if there is no issue

[slavap]

@erickok
Please make DioCacheManager._buildResponse() non-private and async (make it protected), with changed to RequestInterceptorHandler it is currently too hard to adjust cached data additionally in DioCacheManager descendants.

@protected
Future<Response> buildResponse(CacheObj obj, int statusCode, RequestOptions options) async {
}

And add await in two places of DioCacheManager where buildResponse() called.

[erickok]

@slavap I am not the maintainer of the library so I am not sure I want to introduce such changes in this PR. Maybe you can submit a separate PR instead (branched from this nullsafety PR)?

Otherwise if you explain a bit more what you want to do, maybe there is a different way - the new handler-bsed interceptors are exactly build with a use-case in mind where you want to amend the default behaviour.

[slavap]

@erickok it is proper change for new handler based structure. Example: there are some data in cache and they are still valid, but some headers and cookies are changed, so I need just slightly adjust Response without discarding the cache. So I have descendant which manages this after data returned from the cache.

[erickok]

@slavap Fair enough but in the end it's not really my decision to make.

[slavap]

@erickok As you see author of this repo is very slowly reacting. My change is absolutely minor and does not affect anything in existing codebase. But fits properly into new dio logic.

[erickok]

@slavap I understand. Especially because the owner is so slow, I suggest you fork the library yourself, apply this pr and your preferred change, and just use that as ref in your flutter project dependencies. I don't think i can be of much help to you.

[slavap]

@erickok ok, thanks. Of course I’m using my own version of this lib, but it is plainly wrong.

[mozaffari]

Hey maintainer, merge this as soon as possible.
I had to download @erickok 's version and use it as local.

 dio_http_cache: 
   path: "./local_packages/dio-http-cache-feature-nullsafety-dio4"

[aldychris]

Hey maintainer, merge this as soon as possible.
I had to download @erickok 's version and use it as local.

dio_http_cache:
path: "./local_packages/dio-http-cache-feature-nullsafety-dio4"

I do this while waiting @hurshi to review and merge it

dio_http_cache: 
    git:
      url: https://github.com/vrtdev/dio-http-cache.git
      ref: feature/nullsafety-dio4

[mozaffari]

Hey maintainer, merge this as soon as possible.
I had to download @erickok 's version and use it as local.
dio_http_cache:
path: "./local_packages/dio-http-cache-feature-nullsafety-dio4"

I do this while waiting @hurshi to review and merge it

dio_http_cache: 
    git:
      url: https://github.com/vrtdev/dio-http-cache.git
      ref: feature/nullsafety-dio4

Thanks, this is way better. I will use this syntax

[tneotia]

Hi @erickok is there a reason you made baseUrl required? As per the docs it doesn't need to be required and I want to make sure that omitting it in my production app doesn't cause any issues.

[erickok]

Hmm you might be right. I though it was a strict requirement (as there were non-null azssertions in _getUriByPath) but reading through it again indeed it allows not setting a baseUrl if the full path is provided when deleting from cache. I will update this.

[erickok]

Fixed - thanks for the feedback.

[franticn]

@erickok Thank you very much for your PR.
We will support nullsafety and Flutter 2.0 through new branches in the future, but before that, we need to do some cache features and bug fixes.

[slavap]

@franticn
Please also add changes described here: #85
after merging this branch,

[renntbenrennt]

@slavap hey, is your change going to break the original stuff? Or is it necessary to add/change? Otherwise I will just make the dep link to @erickok 's fork version until this package is update, which I hope it will be out before the climate change take over the civilization...

Ps, I don't directly use the package, just the package I have to use use this... and that package is outdated as well... and the updating work lead me to here...
PPs, there's a holiday in China coming up, but you guys have to be aware that the working situation in China is not heaven at all, with those 996 life style, I kind of surprised by the fact that this package is not one of kpi product, so kudos to the owner and pray that he has time back to his stuff instead of draining by the work(assuming he is occupied by the work...)

[slavap]

@SeasonLeee

1. No, won't break anything.
2. It is nice to have change considering Dio 4 changes, it will seriously simplify implementation of descendants.

[renntbenrennt]

@slavap yeah... sure... thanks... but I'm not the user of this package nor that dio, so no comment on here😅

It just one of the package I need to use depend on this.... 😩... either way... I'm just going to use @erickok 's fork... because I got job to finish😄

[mozaffari]

@erickok you have to handler.resolve(response); _onError if maxStale is set.

Otherwise, neither error happens or the request completes if maxStale is not expired and there is a prev success request.

this happens when there is an error and the cache age is expired but maxStale is set to some future date so inside _onRequest _pullFromCacheBeforeMaxAge(options) is null and if (null != responseDataFromCache) is false, so _onRequest can not resolve the response.

now _onError the _pullFromCacheBeforeMaxStale(e.requestOptions); can return a previous success response because there is an old cached request available
but inside of if (null != responseDataFromCache) {...} block of code instead of resolving the response we return the response directly and that can not resolve the response and handler.next(e) also can not be called because to the return _buildResponse(responseDataFromCache, responseDataFromCache.statusCode, e.requestOptions);

Bellow is my full _onError method version of your PR

  _onError(DioError e, ErrorInterceptorHandler handler) async {
    if ((e.requestOptions.extra[DIO_CACHE_KEY_TRY_CACHE] ?? false) == true) {
      var responseDataFromCache =
          await _pullFromCacheBeforeMaxStale(e.requestOptions);
      if (null != responseDataFromCache) {
        var response = _buildResponse(responseDataFromCache,
            responseDataFromCache.statusCode, e.requestOptions);

        return handler.resolve(response);
      }
    }
    return handler.next(e);
  }

[erickok]

@mozaffari Thank you for finding and reporting this! I updated my code

[franticn]

@erickok @mozaffari Thanks for your pr.

[Ted-chiptech]

Could this be Duration? maxAge instead?
The documentation said

maxAge: set the cache time. If the value is null or not setted, it will try to get maxAge and maxStale from response headers.

It doesn't match the code anymore, and the code below is still checking ```if (null != maxAge)` { ... }``

[erickok]

Sure, that sounds right. This PR is already merged though so you can create a new PR maybe, which makes the parameter nullable?

[haroonkhan9426]

Following the same issue.