Migrate to NNBD

[Jjagg]

This migrates the library to NNBD. For now this only migrates the flutter_cache_manager package and its tests. There's 2 test failures I have yet to resolve, so I'm submitting as a draft for now.

Heads up that build_runner needs to be run before running tests because of changes to Mockito to support NNBD.

Fixes #253.

:thinking: Checklist before submitting

• [ ] All projects build
• [ ] Follows style guide lines (code style guide)
• [ ] Relevant documentation was updated
• [ ] Rebased onto current develop

[hoc081098]

Dart beta has been already release, please publish it. https://medium.com/dartlang/preparing-the-dart-and-flutter-ecosystem-for-null-safety-e550ce72c010

[Jjagg]

Thanks for reviewing @hoc081098, I made some changes.

[renefloor]

@Jjagg I wanted to start with this today, but found your PR. I think we should have way more non-optionals and throw an exception if something is not found. For example

Future<FileInfo?> getFile(String key, {bool ignoreMemCache = false})

This should just be Future<FileInfo?> imo. What do you prefer? Shall I comment on your PR, push changes to your branch or start my own branch?

I don't mind any breaking changes, as long as it is documented. That's why this will be a new major version.

[Jjagg]

IMO a caching library should not throw an exception when an item is retrieved that is not in the cache. But I don't have too strong of an opinion on it and I don't use this library directly, so it doesn't matter much to me.

I've given you access to my fork. Feel free to commit directly to this branch or branch off and set up your own pull request/commit directly to the main branch. Either would be a little faster than you commenting and me making the changes :)

[renefloor]

I'm also not 100% sure about the best way, I think both null and an exception are fine if documented well.

[ryanheise]

From Effective Dart:

Dart uses exceptions when an error occurs in your program.

And from the Language Tour:

Exceptions are errors indicating that something unexpected happened

I agree that for a caching library, a cache miss is not an error but something to be expected.

[acoutts]

Hi- the force unwrap here is causing a runtime error for me: https://github.com/Jjagg/flutter_cache_manager/blob/nnbd/flutter_cache_manager/lib/src/storage/file_system/file_system_io.dart#L16

../../.pub-cache/git/flutter_cache_manager-439af02c4c059986f46101a003ac8ebd20658ef7/flutter_cache_manager/lib/src/storage/file_system/file_system_io.dart:16:23: Warning: Operand of null-aware operation '!' has type 'Directory' which excludes null.
 - 'Directory' is from 'dart:io'.
    var path = p.join(baseDir!.path, key);
                      ^

../../.pub-cache/git/flutter_cache_manager-439af02c4c059986f46101a003ac8ebd20658ef7/flutter_cache_manager/lib/src/storage/cache_info_repositories/cache_object_provider.dart:196:20: Warning: Operand of null-aware operation '!' has type 'Directory' which excludes null.
 - 'Directory' is from 'dart:io'.

      directory = (await getApplicationSupportDirectory())!;
                  ^

[acoutts]

[Jjagg]

@acoutts Looks like they changed behavior in path_provider for the the stable 2.0.0 release: flutter/plugins#3582. Are you overriding that dependency by any chance?

I'll update dependencies to the stable version for packages where it's already available and fix that issue.

[acoutts]

I'm not overriding it no- i'm on 2.0.0-nullsafety.1.

[acoutts]

I was able to silence the error with this override:

dependency_overrides:
  path_provider: 2.0.0-nullsafety.1

[renefloor]

In the latest stable of path_provider they changed the return type from Future<Directory?> to Future https://github.com/flutter/plugins/commit/bb3fc5ad22aca210d617d5a5c8f958bc3c629d62#diff-78e86d7326b605de0b626343cc28963ef7bff3587995640f6c1dddadbd7333ed

I upgraded the dependencies that have a stable version now.

[kevmoo]

Who has the power to merge and publish?

[Jjagg]

That would be @renefloor :)

[renefloor]

@kevmoo it is already published as preview version: https://pub.dev/packages/flutter_cache_manager/versions/3.0.0-nullsafety.0

I try to migrate CachedNetworkImage this week and if I don't get any issues I'll merge and publish as stable.

[ryanheise]

In NonStoringObjectProvider, it defines:

  @override
  Future<CacheObject> get(String url) {
    return Future.value(null);
  }

But CacheObject is non-nullable.

The analyser didn't detect any problem statically (which is unfortunate since that should be the goal of null safety) but the documentation for Future.value, indicates that you will get a runtime exception:

If [value] is omitted or null, it is converted to FutureOr<T> by value as FutureOr<T>. If T is not nullable, then a non-null [value] must be provided, otherwise the construction throws.

[PcolBP]

After upgrading sqflite to 2.0.0+3 , now getDatabasesPath returns non-nullable String no need to add null-aware operand.

In case when sqflite is upgraded and cacheManager stays at nullsafety.1 branch error occur:

[Jjagg]

I'm only getting a warning, so they might have changed the analyzer behavior. Nevertheless, doesn't hurt to upgrade sqflite. EDIT: Oh, I thought you got an error because of the red text 😅

[davidmartos96]

@Jjagg There is this issue as well in the NNBD version. https://github.com/Baseflow/flutter_cache_manager/issues/297 The id of the cache object can be null when reaching the removeFile function, so the unconditional bang fails.

[codecov[bot]]

Codecov Report

Merging #278 (c6c6fb6) into develop (d9061bd) will decrease coverage by 1.37%. The diff coverage is 72.26%.

@@             Coverage Diff             @@
##           develop     #278      +/-   ##
===========================================
- Coverage    76.11%   74.73%   -1.38%     
===========================================
  Files           21       21              
  Lines          674      669       -5     
===========================================
- Hits           513      500      -13     
- Misses         161      169       +8     

Impacted Files	Coverage Δ
.../lib/src/cache_managers/default_cache_manager.dart	0.00% <0.00%> (ø)
...ter_cache_manager/lib/src/compat/file_fetcher.dart	77.77% <ø> (-12.23%)	:arrow_down:
...utter_cache_manager/lib/src/config/_config_io.dart	12.50% <ø> (ø)
...cache_info_repositories/cache_object_provider.dart	0.00% <0.00%> (ø)
...info_repositories/non_storing_object_provider.dart	0.00% <ø> (ø)
...er/lib/src/storage/file_system/file_system_io.dart	0.00% <0.00%> (ø)
flutter_cache_manager/lib/src/web/queue_item.dart	100.00% <ø> (ø)
..._info_repositories/json_cache_info_repository.dart	90.62% <75.86%> (-2.16%)	:arrow_down:
...lutter_cache_manager/lib/src/web/file_service.dart	87.87% <81.81%> (-6.57%)	:arrow_down:
flutter_cache_manager/lib/src/cache_manager.dart	88.60% <83.33%> (+1.42%)	:arrow_up:
... and 11 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update c869918...c6c6fb6. Read the comment docs.