From 7a3cabfe12f0bdbc821bf2e27e63b686c88a424d Mon Sep 17 00:00:00 2001 From: arvidn Date: Mon, 13 Jul 2015 22:19:55 -0400 Subject: [PATCH] fixed some resume issues, specifically around file priorities and override resume data --- include/libtorrent/add_torrent_params.hpp | 2 + include/libtorrent/entry.hpp | 2 +- src/torrent.cpp | 34 +-- test/test_resume.cpp | 257 +++++++++++++++++++--- 4 files changed, 249 insertions(+), 46 deletions(-) diff --git a/include/libtorrent/add_torrent_params.hpp b/include/libtorrent/add_torrent_params.hpp index 754099e88..e47ff6620 100644 --- a/include/libtorrent/add_torrent_params.hpp +++ b/include/libtorrent/add_torrent_params.hpp @@ -153,6 +153,8 @@ namespace libtorrent // add_torrent_params configuring the torrent override the corresponding // configuration from the resume file, with the one exception of save // resume data, which has its own flag (for historic reasons). + // If this flag is set, but file_priorities is empty, file priorities + // are still loaded from the resume data, if present. flag_override_resume_data = 0x002, // If ``flag_upload_mode`` is set, the torrent will be initialized in diff --git a/include/libtorrent/entry.hpp b/include/libtorrent/entry.hpp index ce49c70df..5c3181739 100644 --- a/include/libtorrent/entry.hpp +++ b/include/libtorrent/entry.hpp @@ -146,7 +146,7 @@ namespace libtorrent // hidden bool operator==(entry const& e) const; bool operator!=(entry const& e) const { return !(*this == e); } - + // copies the structure of the right hand side into this // entry. #ifndef TORRENT_NO_DEPRECATE diff --git a/src/torrent.cpp b/src/torrent.cpp index 5ce5d60f5..f2e281903 100644 --- a/src/torrent.cpp +++ b/src/torrent.cpp @@ -1860,7 +1860,9 @@ namespace libtorrent construct_storage(); - if (!m_seed_mode && m_resume_data) + // if we've already loaded file priorities, don't load piece priorities, + // they will interfere. + if (!m_seed_mode && m_resume_data && m_file_priority.empty()) { bdecode_node piece_priority = m_resume_data->node .dict_find_string("piece_priority"); @@ -5437,7 +5439,7 @@ namespace libtorrent } update_piece_priorities(); } - + int torrent::file_priority(int index) const { // this call is only valid on torrents with metadata @@ -6608,7 +6610,7 @@ namespace libtorrent #endif return; } - + p->set_country(j->name); } #endif @@ -6742,7 +6744,9 @@ namespace libtorrent if (m_completed_time != 0 && m_completed_time < m_added_time) m_completed_time = m_added_time; - if (!m_seed_mode && !m_override_resume_data) + // load file priorities except if the add_torrent_param file was set to + // override resume data + if (!m_seed_mode && (!m_override_resume_data || m_file_priority.empty())) { bdecode_node file_priority = rd.dict_find_list("file_priority"); if (file_priority && file_priority.list_size() @@ -7157,10 +7161,20 @@ namespace libtorrent ret["announce_to_lsd"] = m_announce_to_lsd; ret["auto_managed"] = m_auto_managed; - // write piece priorities - // but only if they are not set to the default - if (has_picker()) + // piece priorities and file priorities are mutually exclusive. If there + // are file priorities set, don't save piece priorities. + if (!m_file_priority.empty()) { + // write file priorities + entry::list_type& file_priority = ret["file_priority"].list(); + file_priority.clear(); + for (int i = 0, end(m_file_priority.size()); i < end; ++i) + file_priority.push_back(m_file_priority[i]); + } + else if (has_picker()) + { + // write piece priorities + // but only if they are not set to the default bool default_prio = true; for (int i = 0, end(m_torrent_file->num_pieces()); i < end; ++i) { @@ -7178,12 +7192,6 @@ namespace libtorrent piece_priority[i] = m_picker->piece_priority(i); } } - - // write file priorities - entry::list_type& file_priority = ret["file_priority"].list(); - file_priority.clear(); - for (int i = 0, end(m_file_priority.size()); i < end; ++i) - file_priority.push_back(m_file_priority[i]); } void torrent::get_full_peer_list(std::vector& v) const diff --git a/test/test_resume.cpp b/test/test_resume.cpp index 918537834..795ebd7f3 100644 --- a/test/test_resume.cpp +++ b/test/test_resume.cpp @@ -35,6 +35,8 @@ POSSIBILITY OF SUCH DAMAGE. #include "libtorrent/torrent_info.hpp" #include "libtorrent/random.hpp" #include "libtorrent/create_torrent.hpp" +#include "libtorrent/alert_types.hpp" +#include "libtorrent/entry.hpp" #include @@ -46,7 +48,9 @@ using namespace libtorrent; boost::shared_ptr generate_torrent() { file_storage fs; - fs.add_file("test_resume/tmp1", 128 * 1024 * 10); + fs.add_file("test_resume/tmp1", 128 * 1024 * 8); + fs.add_file("test_resume/tmp2", 128 * 1024); + fs.add_file("test_resume/tmp3", 128 * 1024); libtorrent::create_torrent t(fs, 128 * 1024, 6); t.add_tracker("http://torrent_file_tracker.com/announce"); @@ -65,7 +69,8 @@ boost::shared_ptr generate_torrent() return boost::make_shared(&buf[0], buf.size()); } -std::vector generate_resume_data(torrent_info* ti) +std::vector generate_resume_data(torrent_info* ti + , char const* file_priorities = "") { entry rd; @@ -93,8 +98,12 @@ std::vector generate_resume_data(torrent_info* ti) rd["last_download"] = 1350; rd["last_upload"] = 1351; rd["finished_time"] = 1352; - entry::list_type& file_prio = rd["file_priority"].list(); - file_prio.push_back(entry(1)); + if (file_priorities && file_priorities[0]) + { + entry::list_type& file_prio = rd["file_priority"].list(); + for (int i = 0; file_priorities[i]; ++i) + file_prio.push_back(entry(file_priorities[i] - '0')); + } rd["piece_priority"] = std::string(ti->num_pieces(), '\x01'); rd["auto_managed"] = 0; @@ -121,10 +130,9 @@ std::vector generate_resume_data(torrent_info* ti) return ret; } -torrent_status test_resume_flags(int flags) +torrent_handle test_resume_flags(libtorrent::session& ses, int flags + , char const* file_priorities = "1111", char const* resume_file_prio = "") { - libtorrent::session ses; - boost::shared_ptr ti = generate_torrent(); add_torrent_params p; @@ -139,19 +147,24 @@ torrent_status test_resume_flags(int flags) p.trackers.push_back("http://add_torrent_params_tracker.com/announce"); p.url_seeds.push_back("http://add_torrent_params_url_seed.com"); - std::vector rd = generate_resume_data(ti.get()); + std::vector rd = generate_resume_data(ti.get(), resume_file_prio); p.resume_data.swap(rd); p.max_uploads = 1; p.max_connections = 2; p.upload_limit = 3; p.download_limit = 4; - p.file_priorities.push_back(2); + + std::vector priorities_vector; + for (int i = 0; file_priorities[i]; ++i) + priorities_vector.push_back(file_priorities[i] - '0'); + + p.file_priorities = priorities_vector; torrent_handle h = ses.add_torrent(p); torrent_status s = h.status(); TEST_EQUAL(s.info_hash, ti->info_hash()); - return s; + return h; } void default_tests(torrent_status const& s) @@ -175,12 +188,169 @@ void default_tests(torrent_status const& s) TEST_EQUAL(s.completed_time, 1348); } -TORRENT_TEST(resume) -{ - torrent_status s; +// TODO: test what happens when loading a resume file with both piece priorities +// and file priorities (file prio should take presedence) - fprintf(stderr, "flags: 0\n"); - s = test_resume_flags(0); +// TODO: make sure a resume file only ever contain file priorities OR piece +// priorities. Never both. + +// TODO: generally save + +TORRENT_TEST(file_priorities_default) +{ + libtorrent::session ses; + std::vector file_priorities = test_resume_flags(ses, 0, "", "").file_priorities(); + + TEST_EQUAL(file_priorities.size(), 3); + TEST_EQUAL(file_priorities[0], 1); + TEST_EQUAL(file_priorities[1], 1); + TEST_EQUAL(file_priorities[2], 1); +} + +TORRENT_TEST(file_priorities_resume_seed_mode) +{ + // in share mode file priorities should always be 0 + libtorrent::session ses; + std::vector file_priorities = test_resume_flags(ses, + add_torrent_params::flag_share_mode, "", "123").file_priorities(); + + TEST_EQUAL(file_priorities.size(), 3); + TEST_EQUAL(file_priorities[0], 0); + TEST_EQUAL(file_priorities[1], 0); + TEST_EQUAL(file_priorities[2], 0); +} + +TORRENT_TEST(file_priorities_seed_mode) +{ + // in share mode file priorities should always be 0 + libtorrent::session ses; + std::vector file_priorities = test_resume_flags(ses, + add_torrent_params::flag_share_mode, "123", "").file_priorities(); + + TEST_EQUAL(file_priorities.size(), 3); + TEST_EQUAL(file_priorities[0], 0); + TEST_EQUAL(file_priorities[1], 0); + TEST_EQUAL(file_priorities[2], 0); +} + +TORRENT_TEST(resume_save_load) +{ + libtorrent::session ses; + torrent_handle h = test_resume_flags(ses, 0, "123", ""); + + h.save_resume_data(); + + save_resume_data_alert const* a = alert_cast( + wait_for_alert(ses, save_resume_data_alert::alert_type + , "resume_save_load")); + + TEST_CHECK(a); + if (a == NULL) return; + + TEST_CHECK(a->resume_data); + + entry& e = *a->resume_data.get(); + entry::list_type& l = e["file_priority"].list(); + entry::list_type::iterator i = l.begin(); + + TEST_EQUAL(l.size(), 3); + TEST_EQUAL(*i++, 1); + TEST_EQUAL(*i++, 2); + TEST_EQUAL(*i++, 3); +} + +TORRENT_TEST(resume_save_load_resume) +{ + libtorrent::session ses; + torrent_handle h = test_resume_flags(ses, 0, "", "123"); + + h.save_resume_data(); + + save_resume_data_alert const* a = alert_cast( + wait_for_alert(ses, save_resume_data_alert::alert_type + , "resume_save_load")); + + TEST_CHECK(a); + if (a == NULL) return; + + TEST_CHECK(a->resume_data); + + entry& e = *a->resume_data.get(); + entry::list_type& l = e["file_priority"].list(); + entry::list_type::iterator i = l.begin(); + + TEST_EQUAL(l.size(), 3); + TEST_EQUAL(*i++, 1); + TEST_EQUAL(*i++, 2); + TEST_EQUAL(*i++, 3); +} + +TORRENT_TEST(file_priorities_resume_override) +{ + // make sure that an empty file_priorities vector in add_torrent_params won't + // override the resume data file priorities, even when override resume data + // flag is set. + libtorrent::session ses; + std::vector file_priorities = test_resume_flags(ses, + add_torrent_params::flag_override_resume_data, "", "123").file_priorities(); + + TEST_EQUAL(file_priorities.size(), 3); + TEST_EQUAL(file_priorities[0], 1); + TEST_EQUAL(file_priorities[1], 2); + TEST_EQUAL(file_priorities[2], 3); +} + +TORRENT_TEST(file_priorities_resume) +{ + libtorrent::session ses; + std::vector file_priorities = test_resume_flags(ses, 0, "", "123").file_priorities(); + + TEST_EQUAL(file_priorities.size(), 3); + TEST_EQUAL(file_priorities[0], 1); + TEST_EQUAL(file_priorities[1], 2); + TEST_EQUAL(file_priorities[2], 3); +} + +TORRENT_TEST(file_priorities1) +{ + libtorrent::session ses; + std::vector file_priorities = test_resume_flags(ses, 0, "010").file_priorities(); + + TEST_EQUAL(file_priorities.size(), 3); + TEST_EQUAL(file_priorities[0], 0); + TEST_EQUAL(file_priorities[1], 1); + TEST_EQUAL(file_priorities[2], 0); + +//#error save resume data and assert the file priorities are preserved +} + +TORRENT_TEST(file_priorities2) +{ + libtorrent::session ses; + std::vector file_priorities = test_resume_flags(ses, 0, "123").file_priorities(); + + TEST_EQUAL(file_priorities.size(), 3); + TEST_EQUAL(file_priorities[0], 1); + TEST_EQUAL(file_priorities[1], 2); + TEST_EQUAL(file_priorities[2], 3); +} + +TORRENT_TEST(file_priorities3) +{ + libtorrent::session ses; + std::vector file_priorities = test_resume_flags(ses, 0, "4321").file_priorities(); + + TEST_EQUAL(file_priorities.size(), 3); + TEST_EQUAL(file_priorities[0], 4); + TEST_EQUAL(file_priorities[1], 3); + TEST_EQUAL(file_priorities[2], 2); +} + +TORRENT_TEST(plain) +{ + libtorrent::session ses; + + torrent_status s = test_resume_flags(ses, 0).status(); default_tests(s); #ifdef TORRENT_WINDOWS TEST_EQUAL(s.save_path, "c:\\add_torrent_params save_path"); @@ -197,9 +367,12 @@ TORRENT_TEST(resume) TEST_EQUAL(s.ip_filter_applies, false); TEST_EQUAL(s.connections_limit, 1345); TEST_EQUAL(s.uploads_limit, 1346); +} - fprintf(stderr, "flags: use_resume_save_path\n"); - s = test_resume_flags(add_torrent_params::flag_use_resume_save_path); +TORRENT_TEST(use_resume_save_path) +{ + libtorrent::session ses; + torrent_status s = test_resume_flags(ses, add_torrent_params::flag_use_resume_save_path).status(); default_tests(s); #ifdef TORRENT_WINDOWS TEST_EQUAL(s.save_path, "c:\\resume_data save_path"); @@ -216,10 +389,14 @@ TORRENT_TEST(resume) TEST_EQUAL(s.ip_filter_applies, false); TEST_EQUAL(s.connections_limit, 1345); TEST_EQUAL(s.uploads_limit, 1346); +} - fprintf(stderr, "flags: override_resume_data\n"); - s = test_resume_flags(add_torrent_params::flag_override_resume_data - | add_torrent_params::flag_paused); +TORRENT_TEST(override_resume_data) +{ + libtorrent::session ses; + torrent_status s = test_resume_flags(ses + , add_torrent_params::flag_override_resume_data + | add_torrent_params::flag_paused).status(); default_tests(s); #ifdef TORRENT_WINDOWS @@ -237,10 +414,13 @@ TORRENT_TEST(resume) TEST_EQUAL(s.ip_filter_applies, false); TEST_EQUAL(s.connections_limit, 2); TEST_EQUAL(s.uploads_limit, 1); +} - fprintf(stderr, "flags: seed_mode\n"); - s = test_resume_flags(add_torrent_params::flag_override_resume_data - | add_torrent_params::flag_seed_mode); +TORRENT_TEST(seed_mode) +{ + libtorrent::session ses; + torrent_status s = test_resume_flags(ses, add_torrent_params::flag_override_resume_data + | add_torrent_params::flag_seed_mode).status(); default_tests(s); #ifdef TORRENT_WINDOWS TEST_EQUAL(s.save_path, "c:\\add_torrent_params save_path"); @@ -257,9 +437,12 @@ TORRENT_TEST(resume) TEST_EQUAL(s.ip_filter_applies, false); TEST_EQUAL(s.connections_limit, 2); TEST_EQUAL(s.uploads_limit, 1); +} - fprintf(stderr, "flags: upload_mode\n"); - s = test_resume_flags(add_torrent_params::flag_upload_mode); +TORRENT_TEST(upload_mode) +{ + libtorrent::session ses; + torrent_status s = test_resume_flags(ses, add_torrent_params::flag_upload_mode).status(); default_tests(s); #ifdef TORRENT_WINDOWS TEST_EQUAL(s.save_path, "c:\\add_torrent_params save_path"); @@ -276,10 +459,14 @@ TORRENT_TEST(resume) TEST_EQUAL(s.ip_filter_applies, false); TEST_EQUAL(s.connections_limit, 1345); TEST_EQUAL(s.uploads_limit, 1346); +} - fprintf(stderr, "flags: share_mode\n"); - s = test_resume_flags(add_torrent_params::flag_override_resume_data - | add_torrent_params::flag_share_mode); +TORRENT_TEST(share_mode) +{ + libtorrent::session ses; + torrent_status s = test_resume_flags(ses + , add_torrent_params::flag_override_resume_data + | add_torrent_params::flag_share_mode).status(); default_tests(s); #ifdef TORRENT_WINDOWS TEST_EQUAL(s.save_path, "c:\\add_torrent_params save_path"); @@ -296,10 +483,13 @@ TORRENT_TEST(resume) TEST_EQUAL(s.ip_filter_applies, false); TEST_EQUAL(s.connections_limit, 2); TEST_EQUAL(s.uploads_limit, 1); +} +TORRENT_TEST(auto_managed) +{ + libtorrent::session ses; // resume data overrides the auto-managed flag - fprintf(stderr, "flags: auto_managed\n"); - s = test_resume_flags(add_torrent_params::flag_auto_managed); + torrent_status s = test_resume_flags(ses, add_torrent_params::flag_auto_managed).status(); default_tests(s); #ifdef TORRENT_WINDOWS TEST_EQUAL(s.save_path, "c:\\add_torrent_params save_path"); @@ -316,10 +506,13 @@ TORRENT_TEST(resume) TEST_EQUAL(s.ip_filter_applies, false); TEST_EQUAL(s.connections_limit, 1345); TEST_EQUAL(s.uploads_limit, 1346); +} +TORRENT_TEST(paused) +{ + libtorrent::session ses; // resume data overrides the paused flag - fprintf(stderr, "flags: paused\n"); - s = test_resume_flags(add_torrent_params::flag_paused); + torrent_status s = test_resume_flags(ses, add_torrent_params::flag_paused).status(); default_tests(s); #ifdef TORRENT_WINDOWS TEST_EQUAL(s.save_path, "c:\\add_torrent_params save_path");