diff --git a/include/downloader.h b/include/downloader.h index f385823c9..4cc7b7730 100644 --- a/include/downloader.h +++ b/include/downloader.h @@ -168,7 +168,10 @@ class Download { */ class Downloader { - public: + public: // types + typedef std::vector> Options; + + public: // functions Downloader(); virtual ~Downloader(); @@ -177,14 +180,21 @@ class Downloader /** * Start a new download. * - * This method is thread safe and return a pointer to a newly created `Download`. + * This method is thread safe and returns a pointer to a newly created + * `Download` or an existing one with a matching URI. In the latter case + * the options parameter is ignored, which can lead to surprising results. + * For example, if the old and new download requests (sharing the same URI) + * have different values for the download directory or output file name + * options, after the download is reported to be complete the downloaded file + * will be present only at the location specified for the first request. + * * User should call `update` on the returned `Download` to have an accurate status. * * @param uri: The uri of the thing to download. * @param options: A series of pair to pass to aria. * @return: The newly created Download. */ - std::shared_ptr startDownload(const std::string& uri, const std::vector>& options = {}); + std::shared_ptr startDownload(const std::string& uri, const Options& options = {}); /** * Get a download corrsponding to a download id (did) @@ -206,7 +216,7 @@ class Downloader */ std::vector getDownloadIds() const; - private: + private: // data mutable std::mutex m_lock; std::map> m_knownDownloads; std::shared_ptr mp_aria; diff --git a/src/downloader.cpp b/src/downloader.cpp index d874ad899..25dfc1e95 100644 --- a/src/downloader.cpp +++ b/src/downloader.cpp @@ -18,6 +18,7 @@ */ #include "downloader.h" +#include "tools.h" #include "tools/pathTools.h" #include "tools/stringTools.h" @@ -166,13 +167,45 @@ std::vector Downloader::getDownloadIds() const { return ret; } -std::shared_ptr Downloader::startDownload(const std::string& uri, const std::vector>& options) +namespace +{ + +bool downloadCanBeReused(const Download& d, + const std::string& uri, + const Downloader::Options& /*options*/) +{ + const auto& uris = d.getUris(); + const bool sameURI = std::find(uris.begin(), uris.end(), uri) != uris.end(); + + if ( !sameURI ) + return false; + + switch ( d.getStatus() ) { + case Download::K_ERROR: + case Download::K_UNKNOWN: + case Download::K_REMOVED: + return false; + + case Download::K_ACTIVE: + case Download::K_WAITING: + case Download::K_PAUSED: + return true; // XXX: what if options are different? + + case Download::K_COMPLETE: + return fileExists(d.getPath()); // XXX: what if options are different? + } + + return false; +} + +} // unnamed namespace + +std::shared_ptr Downloader::startDownload(const std::string& uri, const Options& options) { std::unique_lock lock(m_lock); for (auto& p: m_knownDownloads) { auto& d = p.second; - auto& uris = d->getUris(); - if (std::find(uris.begin(), uris.end(), uri) != uris.end()) + if ( downloadCanBeReused(*d, uri, options) ) return d; } std::vector uris = {uri};