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[bot]]

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Comparison is base (b2ae1d6) 39.36% compared to head (ebf0fe8) 39.42%. Report is 1 commits behind head on main.

Additional details and impacted files

```diff @@ 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 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

[mgautierfr]

This change is sufficient to fix the bugs described in https://github.com/kiwix/libkiwix/issues/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 https://github.com/kiwix/libkiwix/issues/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 https://github.com/kiwix/libkiwix/pull/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:

• https://github.com/kiwix/kiwix-desktop/issues/1022
• https://github.com/kiwix/libkiwix/issues/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...