More predictable Downloader::startDownload()

[veloman-yunkan]

Fixes kiwix/kiwix-desktop#1022 (as well as the other case described in #1050 without fixing the latter)

Before this change Downloader::startDownload() might avoid starting a new download when a download with the specified URI was already present in its cache. This might be confusing for the following reasons:

1. uri is not the only parameter of Downloader::startDownload() - a target download directory may also be specified through the second options parameter. Thus calling Downloader::startDownload() twice with the same URI but different download directories would not save files into the second directory.

2. Files of a completed download may be removed, whereupon downloading the same files again won't be a no-op. However in such a situation Downloader refuses to actually repeat a previous download.

This change is sufficient to fix the bugs described in #1050 without any changes in kiwix-desktop. However I am not sure that it is the right way to do that - maybe adding a removeDownload() function to kiwix::Downloader (and thus fixing #1050) is justified in any case.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 39.42%. Comparing base (b2ae1d6) to head (ebf0fe8).
Report is 119 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1052      +/-   ##
==========================================
+ Coverage   39.36%   39.42%   +0.05%     
==========================================
  Files          58       58              
  Lines        4075     4069       -6     
  Branches     2245     2243       -2     
==========================================
  Hits         1604     1604              
+ Misses       1091     1085       -6     
  Partials     1380     1380              

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

[mgautierfr]

This change is sufficient to fix the bugs described in #1050 without any changes in kiwix-desktop. However I am not sure that it is the right way to do that - maybe adding a removeDownload() function to kiwix::Downloader (and thus fixing #1050) is justified in any case.

How aria2's addUri behaves if we had two downloads with the same uri/options ?
addUri documentation tells nothing about that, but we need to know. I think it is necessary to remove you doubts (and mines) about this solution.

I feel a bit unconfident about simply remove this check (even if I agree with you description).

[kelson42]

@veloman-yunkan What is the status of this PR? Does last comment of @mgautierfr has triggered a difficulty?

[veloman-yunkan]

@veloman-yunkan What is the status of this PR? Does last comment of @mgautierfr has triggered a difficulty?

@kelson42 No additional technical difficulties. As I wrote in the description I was not sure that the proposed change was a good idea and @mgautierfr's feedback was along the same lines. Yet I will keep it open for a while until I decide how to proceed.

[veloman-yunkan]

I started working on an alternative solution based on introducing a new method Downloader::forgetDownload() but didn't like it early in the process and ended up with #1066. Now we have to decide which way to go.

[mgautierfr]

I started working on an alternative solution based on introducing a new method Downloader::forgetDownload() but didn't like it early in the process and ended up with #1066. Now we have to decide which way to go.

I like better #1066. At least we are "sure" that aria2 will not misbehave when we have two downloads for the same file/option.

[kelson42]

@veloman-yunkan What is the status of this PR? Does last comment of @mgautierfr has triggered a difficulty?

@kelson42 No additional technical difficulties. As I wrote in the description I was not sure that the proposed change was a good idea and @mgautierfr's feedback was along the same lines. Yet I will keep it open for a while until I decide how to proceed.

OK, it seems an alternative has been chosen, but the two issues this PR was going to fix are still open:

• Cancelling a download breaks the option to start it again kiwix-desktop#1022
• Impossible to drop entries from the kiwix::Downloader's cache #1050 without fixing the latter)

So not sure if this PR should be revamped, a new PR started or just the two issue to be closed...