Re-download images

[ngosang]

wallabag-app master branch

I have the option "Cache images locally" checked in the app (not in wallabag web). Sometimes there is a problem downloading the images and I can't see it in some articles. Related https://github.com/wallabag/android-app/issues/444#issuecomment-283505478 If I delete the article and add it again usually works, so there is no problem with the article.

I propose to implement 3 solutions: 1) When you update the article list try to re-download missing images (I mean partial, I don't know if full update tries to redownload all images but I think this is to much overhead) 2) If you enter in one article and images are not stored locally, try to show the online version, and then save them locally. 3) Add a button to refetch the article (download both, text and images).

[Strubbl]

When you update the article list try to re-download missing images (I mean partial, I don't know if full update tries to redownload all images but I think this is to much overhead)

i should already have this behaviour. it looks for the image being in the file system and if not tries to download it. @di72nn how about the status of the image being downloaded on any http error? Does it mark as "image cached"? What if only one image out of three of one article is fetched and the others are not. What is the status in the db?

If you enter in one article and images are not stored locally, try to show the online version, and then save them locally.

should also be the current behaviour as in the app. it is implemented like this except for the part to save them locally if the online version is available (only for article display, on feed update it fetches online images). see https://github.com/wallabag/android-app/blob/master/app/src/main/java/fr/gaulupeau/apps/Poche/network/ImageCacheUtils.java#L50

Add a button to refetch the article (download both, text and images).

good idea, i would like to have that, too.

[di72nn]

Here's how the image cache currently works:

• Stored article content (HTML code, basically) is not changed by the image cache.
• Every article has a boolean flag "images downloaded".
• When the "image fetching" operation performs, it iterates over all the articles with "images downloaded" flag set to false.
  1. For every image URL found in the article, the app tries to download an image file. Download errors are ignored.
  2. Regardless of the result (all of the images downloaded / some of the images downloaded / none of the images downloaded) the "images downloaded" flag for the article is set true.
• When articles are updated from server, the "images downloaded" flag may be set to false only if article content was changed.
• When an article is opened for reading and the image caching is enabled, a temporary copy (it is not stored) of the article content is modified:
  1. For every image URL found in the article, the app replaces the URL with a URL pointing to a local file. If there is no local file for a specific URL (it was deleted, or wasn't downloaded in the first place), the original URL remains intact, so the app tries to access the online version of the image. The online version is not cached at this point since these are separate processes.
• If you disable image caching, the cached images are not deleted, but they are not used either. So you can enable it again without re-downloading every image.

Current implementation is not perfect, but it is good to have this feature anyway.

Sometimes there is a problem downloading the images and I can't see it in some articles. If I delete the article and add it again usually works, so there is no problem with the article.

This is theoretically possible (if the download is interrupted, the partially downloaded image won't be removed or re-downloaded in the future; this should be fixed, BTW), but if the image was inaccessible at the moment of the "image fetching", the online version is used.

When you update the article list try to re-download missing images (I mean partial, I don't know if full update tries to redownload all images but I think this is to much overhead)

As I said, currently the unit of caching is article: either we have its content cached or not. Not the separate images. We need to introduce more advanced caching in order to implement it. That would be a good feature, but it is not a priority for me.

If you enter in one article and images are not stored locally, try to show the online version, and then save them locally.

In such cases the online version is shown, but not cached.

Add a button to refetch the article (download both, text and images).

I'm ok for a button to refetch article images (basically, set the "images downloaded" flag to false and run the "image fetching" operation), but not the content (the content is never changed locally, use fast update to get server-side changes).

[ngosang]

Regardless of the result (all of the images downloaded / some of the images downloaded / none of the images downloaded) the "images downloaded" flag for the article is set true.

Could you change that? if any image download fails set it to false so next time will be retried. This could be problematic in some cases but...

If you enter in one article and images are not stored locally, try to show the online version, and then save them locally. In such cases the online version is shown, but not cached.

Could you test that? I can't replicate the issue now but I think this is not the current behaviour. Take a look at the first comment screenshot, that images exists but wasn't displayed. I deleted the article and added it again and everything worked.

[di72nn]

Could you change that? if any image download fails set it to false so next time will be retried. This could be problematic in some cases but...

I just realized I've never thought about errors related to connection (flaky connection, etc.; I've thought only about server-side issues). That should be done for the client-side errors.

Could you test that? I can't replicate the issue now but I think this is not the current behaviour. Take a look at the first comment screenshot, that images exists but wasn't displayed. I deleted the article and added it again and everything worked.

I don't know how to test it. Next time you encounter anything like that, we at least need logs of opening the article for reading (it should contain some info regarding URL replacements).

[ngosang]

I did a small change to skip HTTP 500 errors in my build. Maybe it helps you in some way.

---
 app/src/main/java/fr/gaulupeau/apps/Poche/service/MainService.java | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/app/src/main/java/fr/gaulupeau/apps/Poche/service/MainService.java b/app/src/main/java/fr/gaulupeau/apps/Poche/service/MainService.java
index 5ade058..f38eba8 100644
--- a/app/src/main/java/fr/gaulupeau/apps/Poche/service/MainService.java
+++ b/app/src/main/java/fr/gaulupeau/apps/Poche/service/MainService.java
@@ -275,6 +275,12 @@ public class MainService extends IntentServiceBase {
                 itemResult = null;
             }

+            if(itemResult != null && !itemResult.isSuccess()
+                    && itemResult.getErrorType() == ActionResult.ErrorType.UNKNOWN) {
+                Log.i(TAG, "syncOfflineQueue() ignoring UNKNOWN");
+                itemResult = null;
+            }
+
             if(itemResult == null || itemResult.isSuccess()) {
                 completedQueueItems.add(item);
             } else if(itemResult.getErrorType() != null) {
-- 

[Radexw]

Hi guys, I just subscribed to your project on codeTriage. This is my first time when I join an open-source project :D so please be patient with me . I read your comments here regarding fetching the images. What do you say about downloading files via Android's download manager? In this way you have the status for downloaded files and also a broadcast to know when file was downloaded successfully.

[di72nn]

Hi @Radexw I can't say whether using download manager is an acceptable solution right now. Anyway, it is of somewhat low priority, since it mostly works and there are other things that are not implemented at all.