Baseflow / flutter_cache_manager

Generic cache manager for flutter
https://baseflow.com
MIT License
749 stars 438 forks source link

removeFile don't remove line into database #268

Open afiocre opened 3 years ago

afiocre commented 3 years ago

🐛 Bug Report

When i call the removeFile() i see that the cached file is deleted into /data/user/0/fr.serialtrip/cache/libCachedImageData But not into the database libCachedImageData.db

When I see the code, i don't understand. Is it a bug ?

Future removeCachedFile(CacheObject cacheObject) async { final provider = await _cacheInfoRepository; final toRemove = []; unawaited(_removeCachedFile(cacheObject, toRemove)); await provider.deleteAll(toRemove); }

With this code unawaited(_removeCachedFile(cacheObject, toRemove)); it's impossible to delete the IDs from database (SQLite in my case) because it's not an await. So the toRemove is empty on the next line.

Configuration

Version: 2.1.1

Platform:

renefloor commented 3 years ago

That part starts with adding the ID to the list, so that doesn't need to be awaited.

  Future<void> _removeCachedFile(
      CacheObject cacheObject, List<int> toRemove) async {
    if (toRemove.contains(cacheObject.id)) return;

    toRemove.add(cacheObject.id);

I don't know what is going wrong here.

afiocre commented 3 years ago

Ok my last test give the way to the bug ...

My function to remove the file from cache is:

  Future<void> _removeCachePhoto(String path) async {
    await DefaultCacheManager().removeFile(path);
  }

And when we call this into a loop FOR it's delete the file but not the ID into de SQLite database.

If i duplicate the removeFile(), it's fix the bug ! But why we need this ? You can reproduce easy the bug into a FOR loop.

  Future<void> _removeCachePhoto(String path) async {
    await DefaultCacheManager().removeFile(path);
    await DefaultCacheManager().removeFile(path);
  }
renefloor commented 3 years ago

What do you mean with a for loop? Removing like 10 different files?

afiocre commented 3 years ago

Yep, i have a Trip who containt X days who contain Y step. And i have a function to remove all photos from cache for cleaning. So i have a loop for steps who each have a different photo.

So the loop try to remove all the photo of steps and there is a bug. If i do each after each it's OK. but the same in the loop it's NOT OK.

easy to reprocude.

renefloor commented 3 years ago

Do you do await _removeCachePhoto(String path) in the loop, or just _removeCachePhoto(String path)?

afiocre commented 3 years ago

I have try both, but same result :(

marcusrohden commented 3 years ago

I have try both, but same result :(

are you using a foreach loop? if yes, it ignores await's. You would need to iterate over a normal indexed for loop to be able to use await properly.

bohdan1krokhmaliuk commented 3 years ago

@renefloor @demos77
Same issue for IOS.

Looks like CacheObject sometimes may have no ID property - that's why toRemove list contains null elements and that's why the record is not deleted from the database. image image

bohdan1krokhmaliuk commented 3 years ago

UPD: by the way if you try to delete object with the same key for the second time - most likely that CacheObject will have an Id and the database will be updated.

From my debugging, I found that when you try to delete it for the first time - it takes cacheObject from _memCache and all the objects in _memCache have no id specified. For the second time if you try to remove file with the same key it will take cacheObject from _futureCache and this one has an id.

image