refactor: make tr_torrent::on_announce_list_changed() private (#6304)

libtransmission/rpcimpl.cc

@@ -669,7 +669,7 @@ namespace make_torrent_field_helpers
     case TR_KEY_status: return st.activity;
     case TR_KEY_torrentFile: return tor.torrent_file();
     case TR_KEY_totalSize: return tor.total_size();
-    case TR_KEY_trackerList: return tor.tracker_list();
+    case TR_KEY_trackerList: return tor.announce_list().to_string();
     case TR_KEY_trackerStats: return make_tracker_stats_vec(tor);
     case TR_KEY_trackers: return make_tracker_vec(tor);
     case TR_KEY_uploadLimit: return tr_torrentGetSpeedLimit_KBps(&tor, TR_UP);
@@ -895,7 +895,8 @@ char const* setFileDLs(tr_torrent* tor, bool wanted, tr_variant* list)
 
 char const* addTrackerUrls(tr_torrent* tor, tr_variant* urls)
 {
-    auto const old_list = tor->tracker_list();
+    auto ann = tor->announce_list();
+    auto const baseline = ann;
 
     for (size_t i = 0, n = tr_variantListSize(urls); i < n; ++i)
     {
@@ -906,23 +907,22 @@ char const* addTrackerUrls(tr_torrent* tor, tr_variant* urls)
             continue;
         }
 
-        tor->announce_list().add(announce);
+        ann.add(announce);
     }
 
-    if (tor->tracker_list() == old_list) // unchanged
+    if (ann == baseline) // unchanged
     {
         return "error setting announce list";
     }
 
-    tor->announce_list().save(tor->torrent_file());
+    tor->set_announce_list(std::move(ann));
-    tor->on_announce_list_changed();
-
     return nullptr;
 }
 
 char const* replaceTrackers(tr_torrent* tor, tr_variant* urls)
 {
-    auto changed = bool{ false };
+    auto ann = tor->announce_list();
+    auto const baseline = ann;
 
     for (size_t i = 0, url_count = tr_variantListSize(urls); i + 1 < url_count; i += 2)
     {
@@ -932,24 +932,23 @@ char const* replaceTrackers(tr_torrent* tor, tr_variant* urls)
         if (tr_variantGetInt(tr_variantListChild(urls, i), &id) &&
             tr_variantGetStrView(tr_variantListChild(urls, i + 1), &newval))
         {
-            changed |= tor->announce_list().replace(static_cast<tr_tracker_id_t>(id), newval);
+            ann.replace(static_cast<tr_tracker_id_t>(id), newval);
         }
     }
 
-    if (!changed)
+    if (ann == baseline) // unchanged
     {
         return "error setting announce list";
     }
 
-    tor->announce_list().save(tor->torrent_file());
+    tor->set_announce_list(std::move(ann));
-    tor->on_announce_list_changed();
-
     return nullptr;
 }
 
 char const* removeTrackers(tr_torrent* tor, tr_variant* ids)
 {
-    auto const old_list = tor->tracker_list();
+    auto ann = tor->announce_list();
+    auto const baseline = ann;
 
     for (size_t i = 0, n = tr_variantListSize(ids); i < n; ++i)
     {
@@ -960,17 +959,15 @@ char const* removeTrackers(tr_torrent* tor, tr_variant* ids)
             continue;
         }
 
-        tor->announce_list().remove(static_cast<tr_tracker_id_t>(id));
+        ann.remove(static_cast<tr_tracker_id_t>(id));
     }
 
-    if (tor->tracker_list() == old_list) // unchanged
+    if (ann == baseline) // unchanged
     {
         return "error setting announce list";
     }
 
-    tor->announce_list().save(tor->torrent_file());
+    tor->set_announce_list(std::move(ann));
-    tor->on_announce_list_changed();
-
     return nullptr;
 }
 
@@ -1106,7 +1103,7 @@ char const* torrentSet(tr_session* session, tr_variant* args_in, tr_variant* /*a
 
         if (std::string_view txt; errmsg == nullptr && tr_variantDictFindStrView(args_in, TR_KEY_trackerList, &txt))
         {
-            if (!tor->set_tracker_list(txt))
+            if (!tor->set_announce_list(txt))
             {
                 errmsg = "Invalid tracker list";
             }

libtransmission/torrent.cc

@@ -1934,58 +1934,69 @@ bool tr_torrent::check_piece(tr_piece_index_t piece)
 
 // ---
 
-bool tr_torrent::set_tracker_list(std::string_view text)
+bool tr_torrent::set_announce_list(std::string_view announce_list_str)
 {
-    auto const lock = this->unique_lock();
+    auto ann = tr_announce_list{};
-
+    return ann.parse(announce_list_str) && set_announce_list(std::move(ann));
-    auto announce_list = tr_announce_list();
-    if (!announce_list.parse(text))
-    {
-        return false;
 }
 
-    auto const has_metadata = this->has_metainfo();
+bool tr_torrent::set_announce_list(tr_announce_list announce_list)
-    if (has_metadata && !announce_list.save(torrent_file()))
 {
-        return false;
+    auto const lock = unique_lock();
-    }
 
-    this->metainfo_.announce_list() = announce_list;
+    auto& tgt = metainfo_.announce_list();
-    this->mark_edited();
 
-    // magnet links
+    tgt = std::move(announce_list);
-    if (!has_metadata)
+
-    {
+    // save the changes
-        auto const magnet_file = this->magnet_file();
-        auto const magnet_link = this->magnet();
     auto save_error = tr_error{};
-        if (!tr_file_save(magnet_file, magnet_link, &save_error))
+    auto filename = std::string{};
+    if (has_metainfo())
     {
-            this->error().set_local_error(fmt::format(
+        filename = torrent_file();
+        tgt.save(filename, &save_error);
+    }
+    else
+    {
+        filename = magnet_file();
+        tr_file_save(filename, magnet(), &save_error);
+    }
+
+    on_announce_list_changed();
+
+    if (save_error.has_value())
+    {
+        error().set_local_error(fmt::format(
             _("Couldn't save '{path}': {error} ({error_code})"),
-                fmt::arg("path", magnet_file),
+            fmt::arg("path", filename),
             fmt::arg("error", save_error.message()),
             fmt::arg("error_code", save_error.code())));
-        }
+        return false;
     }
 
-    /* if we had a tracker-related error on this torrent,
+    return true;
-     * and that tracker's been removed,
+}
-     * then clear the error */
+
+void tr_torrent::on_announce_list_changed()
+{
+    // if we had a tracker-related error on this torrent,
+    // and that tracker's been removed,
+    // then clear the error
     if (auto const& error_url = error_.announce_url(); !std::empty(error_url))
     {
+        auto const& ann = metainfo().announce_list();
         if (std::any_of(
-                std::begin(this->announce_list()),
+                std::begin(ann),
-                std::end(this->announce_list()),
+                std::end(ann),
                 [error_url](auto const& tracker) { return tracker.announce == error_url; }))
         {
             error_.clear();
         }
     }
 
-    on_announce_list_changed();
+    mark_edited();
 
-    return true;
+    session->announcer_->resetTorrent(this);
 }
 
 void tr_torrent::on_tracker_response(tr_tracker_event const* event)
@@ -2027,12 +2038,12 @@ void tr_torrent::on_tracker_response(tr_tracker_event const* event)
 
 bool tr_torrentSetTrackerList(tr_torrent* tor, char const* text)
 {
-    return text != nullptr && tor->set_tracker_list(text);
+    return text != nullptr && tor->set_announce_list(text);
 }
 
 std::string tr_torrentGetTrackerList(tr_torrent const* tor)
 {
-    return tor->tracker_list();
+    return tor->announce_list().to_string();
 }
 
 size_t tr_torrentGetTrackerListToBuf(tr_torrent const* tor, char* buf, size_t buflen)

libtransmission/torrent.h

@@ -504,17 +504,9 @@ public:
         return metainfo_.announce_list();
     }
 
-    [[nodiscard]] constexpr auto& announce_list() noexcept
+    bool set_announce_list(tr_announce_list announce_list);
-    {
-        return metainfo_.announce_list();
-    }
 
-    [[nodiscard]] auto tracker_list() const
+    bool set_announce_list(std::string_view announce_list_str);
-    {
-        return this->announce_list().to_string();
-    }
-
-    bool set_tracker_list(std::string_view text);
 
     /// METAINFO - WEBSEEDS
 
@@ -917,13 +909,6 @@ public:
         return peer_id_;
     }
 
-    // should be called when done modifying the torrent's announce list.
-    void on_announce_list_changed()
-    {
-        mark_edited();
-        session->announcer_->resetTorrent(this);
-    }
-
     void on_block_received(tr_block_index_t block);
 
     [[nodiscard]] constexpr auto& error() noexcept
@@ -1222,6 +1207,9 @@ private:
         return {};
     }
 
+    // must be called after the torrent's announce list changes.
+    void on_announce_list_changed();
+
     // ---
 
     void set_has_piece(tr_piece_index_t piece, bool has)