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: Use a local cache (storage) for front pictures #6217

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

g123k
Copy link
Collaborator

@g123k g123k commented Jan 16, 2025

Hi everyone!

This PRs introduces cached_network_image as a dependency.
It allows to cache locally images.

For now, only front pictures use this mechanism.
To test:

  • Is-it better?
  • Should we increase the size of the cache?

@codecov-commenter
Copy link

Codecov Report

Attention: Patch coverage is 0% with 2 lines in your changes missing coverage. Please review.

Project coverage is 6.20%. Comparing base (4d9c7fc) to head (cd81c98).
Report is 666 commits behind head on develop.

Files with missing lines Patch % Lines
...ckages/smooth_app/lib/database/transient_file.dart 0.00% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           develop   #6217      +/-   ##
==========================================
- Coverage     9.54%   6.20%   -3.35%     
==========================================
  Files          325     456     +131     
  Lines        16411   26703   +10292     
==========================================
+ Hits          1567    1656      +89     
- Misses       14844   25047   +10203     

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

Copy link
Contributor

@monsieurtanuki monsieurtanuki left a comment

Choose a reason for hiding this comment

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

Hi @g123k!
Good idea!
It would probably be better if we could already have an explicit control on the cache parameters - instead of using blindly the default values.
For instance, our front images never stale.
And we may use different parameters for different kinds of images:

  • most important being front x thumbnails - here we can store a lot
  • front x full - here we may store less as they are much bigger

@g123k
Copy link
Collaborator Author

g123k commented Jan 16, 2025

The reason for my comment to decide later about the cache is:

    1. There are improvements vs the situation today
    1. The cache is not based on the size on the storage, but the number of files.

The lib is based on flutter_cache_manager, where we can only set:

static CacheManager instance = CacheManager(
    Config(
      key,
      stalePeriod: const Duration(days: 7),
      maxNrOfCacheObjects: 20,
      repo: JsonCacheInfoRepository(databaseName: key),
      fileSystem: IOFileSystem(key),
      fileService: HttpFileService(),
    ),
  );

@monsieurtanuki
Copy link
Contributor

@g123k That's my point - using default values may not be suitable in our case:

  • stalePeriod: const Duration(days: 7) - we don't have stale periods
  • maxNrOfCacheObjects: 20 - could be ok, but it will consider both front thumbnails and front full images

If those default values are what you were targetting, that's perfect actually.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

4 participants