refactor: remove torrent_view virtual class (#6738)

* refactor: remove torrent_view virtual class

* chore: workaround for google-readability-todo warnings

* chore: fix completion tests

libtransmission/completion.cc

@@ -16,6 +16,12 @@
 #include "libtransmission/block-info.h"
 #include "libtransmission/completion.h"
 #include "libtransmission/tr-assert.h"
+#include "libtransmission/torrent.h"
+
+tr_completion::tr_completion(tr_torrent const* tor, tr_block_info const* block_info)
+    : tr_completion{ [tor](tr_piece_index_t const piece) { return tor->piece_is_wanted(piece); }, block_info }
+{
+}
 
 uint64_t tr_completion::compute_has_valid() const
 {
@@ -55,7 +61,7 @@ uint64_t tr_completion::compute_size_when_done() const
     auto size = uint64_t{ 0 };
     for (tr_piece_index_t piece = 0, n_pieces = block_info_->piece_count(); piece < n_pieces; ++piece)
     {
-        if (tor_->piece_is_wanted(piece))
+        if (piece_is_wanted_(piece))
         {
             size += block_info_->piece_size(piece);
         }

libtransmission/completion.h

@@ -10,8 +10,9 @@
 #endif
 
 #include <algorithm>
-#include <cstdint>
 #include <cstddef> // size_t
+#include <cstdint>
+#include <functional>
 #include <optional>
 #include <vector>
 
@@ -26,21 +27,18 @@
  */
 struct tr_completion
 {
-    struct torrent_view
+    using PieceIsWantedFunc = std::function<bool(tr_piece_index_t piece)>;
-    {
-        virtual bool piece_is_wanted(tr_piece_index_t piece) const = 0;
 
-        virtual ~torrent_view() = default;
+    tr_completion(PieceIsWantedFunc&& piece_is_wanted, tr_block_info const* block_info)
-    };
+        : piece_is_wanted_{ std::move(piece_is_wanted) }
-
-    explicit tr_completion(torrent_view const* tor, tr_block_info const* block_info)
-        : tor_{ tor }
         , block_info_{ block_info }
         , blocks_{ block_info_->block_count() }
     {
         blocks_.set_has_none();
     }
 
+    tr_completion(tr_torrent const* tor, tr_block_info const* block_info);
+
     [[nodiscard]] constexpr tr_bitfield const& blocks() const noexcept
     {
         return blocks_;
@@ -175,7 +173,7 @@ private:
 
     void remove_block(tr_block_index_t block);
 
-    torrent_view const* tor_;
+    PieceIsWantedFunc piece_is_wanted_;
     tr_block_info const* block_info_;
 
     tr_bitfield blocks_{ 0 };

libtransmission/torrent.h

@@ -63,7 +63,7 @@ class RenameTest_singleFilenameTorrent_Test;
 } // namespace libtransmission::test
 
 /** @brief Torrent object */
-struct tr_torrent final : public tr_completion::torrent_view
+struct tr_torrent
 {
     using Speed = libtransmission::Values::Speed;
 
@@ -174,7 +174,7 @@ struct tr_torrent final : public tr_completion::torrent_view
 
     explicit tr_torrent(tr_torrent_metainfo&& tm)
         : metainfo_{ std::move(tm) }
-        , completion_{ this, &this->metainfo_.block_info() }
+        , completion_{ this, &metainfo_.block_info() }
     {
     }
 
@@ -389,7 +389,7 @@ struct tr_torrent final : public tr_completion::torrent_view
 
     /// WANTED
 
-    [[nodiscard]] bool piece_is_wanted(tr_piece_index_t piece) const final
+    [[nodiscard]] bool piece_is_wanted(tr_piece_index_t piece) const
     {
         return files_wanted_.piece_wanted(piece);
     }

tests/libtransmission/completion-test.cc

@@ -23,13 +23,13 @@ using CompletionTest = ::testing::Test;
 namespace
 {
 
-struct TestTorrent : public tr_completion::torrent_view
+struct TestTorrent
 {
     std::set<tr_piece_index_t> dnd_pieces;
 
-    [[nodiscard]] bool piece_is_wanted(tr_piece_index_t piece) const final
+    [[nodiscard]] tr_completion makeCompletion(tr_block_info const& block_info) const
     {
-        return dnd_pieces.count(piece) == 0;
+        return { [this](tr_piece_index_t const piece) { return dnd_pieces.count(piece) == 0; }, &block_info };
     }
 };
 
@@ -41,7 +41,7 @@ TEST_F(CompletionTest, MagnetLink)
 {
     auto torrent = TestTorrent{};
     auto block_info = tr_block_info{};
-    auto completion = tr_completion(&torrent, &block_info);
+    auto completion = torrent.makeCompletion(block_info);
 
     EXPECT_FALSE(completion.has_all());
     EXPECT_TRUE(completion.has_none());
@@ -64,7 +64,7 @@ TEST_F(CompletionTest, setBlocks)
 
     auto torrent = TestTorrent{};
     auto const block_info = tr_block_info{ TotalSize, PieceSize };
-    auto completion = tr_completion(&torrent, &block_info);
+    auto completion = torrent.makeCompletion(block_info);
     EXPECT_FALSE(completion.blocks().has_all());
     EXPECT_FALSE(completion.has_all());
     EXPECT_EQ(0, completion.has_total());
@@ -85,7 +85,7 @@ TEST_F(CompletionTest, hasBlock)
     auto constexpr TotalSize = uint64_t{ BlockSize * 4096 };
     auto constexpr PieceSize = uint64_t{ BlockSize * 64 };
     auto const block_info = tr_block_info{ TotalSize, PieceSize };
-    auto completion = tr_completion(&torrent, &block_info);
+    auto completion = torrent.makeCompletion(block_info);
 
     EXPECT_FALSE(completion.has_block(0));
     EXPECT_FALSE(completion.has_block(1));
@@ -106,7 +106,7 @@ TEST_F(CompletionTest, hasBlocks)
     auto constexpr PieceSize = uint64_t{ BlockSize * 64 };
     auto const block_info = tr_block_info{ TotalSize, PieceSize };
 
-    auto completion = tr_completion(&torrent, &block_info);
+    auto completion = torrent.makeCompletion(block_info);
     EXPECT_FALSE(completion.has_blocks({ 0, 1 }));
     EXPECT_FALSE(completion.has_blocks({ 0, 2 }));
 
@@ -122,7 +122,7 @@ TEST_F(CompletionTest, hasNone)
     auto constexpr PieceSize = uint64_t{ BlockSize * 64 };
     auto const block_info = tr_block_info{ TotalSize, PieceSize };
 
-    auto completion = tr_completion(&torrent, &block_info);
+    auto completion = torrent.makeCompletion(block_info);
     EXPECT_TRUE(completion.has_none());
 
     completion.add_block(0);
@@ -137,7 +137,7 @@ TEST_F(CompletionTest, hasPiece)
     auto const block_info = tr_block_info{ TotalSize, PieceSize };
 
     // check that the initial state does not have it
-    auto completion = tr_completion(&torrent, &block_info);
+    auto completion = torrent.makeCompletion(block_info);
     EXPECT_FALSE(completion.has_piece(0));
     EXPECT_FALSE(completion.has_piece(1));
     EXPECT_EQ(0, completion.has_valid());
@@ -174,7 +174,7 @@ TEST_F(CompletionTest, percentCompleteAndDone)
     auto const block_info = tr_block_info{ TotalSize, PieceSize };
 
     // check that in blank-slate initial state, isDone() is false
-    auto completion = tr_completion(&torrent, &block_info);
+    auto completion = torrent.makeCompletion(block_info);
     EXPECT_DOUBLE_EQ(0.0, completion.percent_complete());
     EXPECT_DOUBLE_EQ(0.0, completion.percent_done());
 
@@ -215,7 +215,7 @@ TEST_F(CompletionTest, hasTotalAndValid)
     auto const block_info = tr_block_info{ TotalSize, PieceSize };
 
     // check that the initial blank-slate state has nothing
-    auto completion = tr_completion(&torrent, &block_info);
+    auto completion = torrent.makeCompletion(block_info);
     EXPECT_EQ(0, completion.has_total());
     EXPECT_EQ(completion.has_valid(), completion.has_total());
 
@@ -253,7 +253,7 @@ TEST_F(CompletionTest, leftUntilDone)
     auto const block_info = tr_block_info{ TotalSize, PieceSize };
 
     // check that the initial blank-slate state has nothing
-    auto completion = tr_completion(&torrent, &block_info);
+    auto completion = torrent.makeCompletion(block_info);
     EXPECT_EQ(block_info.total_size(), completion.left_until_done());
 
     // check that adding the final piece adjusts by block_info.final_piece_size
@@ -301,7 +301,7 @@ TEST_F(CompletionTest, sizeWhenDone)
     auto const block_info = tr_block_info{ TotalSize, PieceSize };
 
     // check that adding or removing blocks or pieces does not affect sizeWhenDone
-    auto completion = tr_completion(&torrent, &block_info);
+    auto completion = torrent.makeCompletion(block_info);
     EXPECT_EQ(block_info.total_size(), completion.size_when_done());
     completion.add_block(0);
     EXPECT_EQ(block_info.total_size(), completion.size_when_done());
@@ -336,7 +336,7 @@ TEST_F(CompletionTest, createPieceBitfield)
     auto const block_info = tr_block_info{ TotalSize, PieceSize };
 
     // make a completion object that has a random assortment of pieces
-    auto completion = tr_completion(&torrent, &block_info);
+    auto completion = torrent.makeCompletion(block_info);
     auto buf = tr_rand_obj<std::array<char, 65>>();
     ASSERT_EQ(std::size(buf), block_info.piece_count());
     for (uint64_t i = 0; i < block_info.piece_count(); ++i)
@@ -368,7 +368,7 @@ TEST_F(CompletionTest, countMissingBytesInPiece)
     auto constexpr TotalSize = uint64_t{ BlockSize * 4096 } + 1;
     auto constexpr PieceSize = uint64_t{ BlockSize * 64 };
     auto const block_info = tr_block_info{ TotalSize, PieceSize };
-    auto completion = tr_completion(&torrent, &block_info);
+    auto completion = torrent.makeCompletion(block_info);
 
     EXPECT_EQ(block_info.piece_size(0), completion.count_missing_bytes_in_piece(0));
     completion.add_block(0);
@@ -391,7 +391,7 @@ TEST_F(CompletionTest, amountDone)
     auto constexpr TotalSize = uint64_t{ BlockSize * 4096 } + 1;
     auto constexpr PieceSize = uint64_t{ BlockSize * 64 };
     auto const block_info = tr_block_info{ TotalSize, PieceSize };
-    auto completion = tr_completion(&torrent, &block_info);
+    auto completion = torrent.makeCompletion(block_info);
 
     // make bins s.t. each bin is a single piece
     auto bins = std::array<float, TotalSize / PieceSize>{};
@@ -435,7 +435,7 @@ TEST_F(CompletionTest, countHasBytesInSpan)
     auto constexpr TotalSize = uint64_t{ BlockSize * 4096 } + 1;
     auto constexpr PieceSize = uint64_t{ BlockSize * 64 };
     auto const block_info = tr_block_info{ TotalSize, PieceSize };
-    auto completion = tr_completion(&torrent, &block_info);
+    auto completion = torrent.makeCompletion(block_info);
 
     // torrent is complete
     auto blocks = tr_bitfield{ block_info.block_count() };
@@ -479,7 +479,7 @@ TEST_F(CompletionTest, wantNone)
     auto constexpr TotalSize = uint64_t{ BlockSize * 4096 };
     auto constexpr PieceSize = uint64_t{ BlockSize * 64 };
     auto const block_info = tr_block_info{ TotalSize, PieceSize };
-    auto completion = tr_completion(&torrent, &block_info);
+    auto completion = torrent.makeCompletion(block_info);
 
     // we have some data
     completion.add_block(0);

tests/libtransmission/lpd-test.cc

@@ -112,7 +112,7 @@ TEST_F(LpdTest, HelloWorld)
     EXPECT_EQ(0U, std::size(mediator.found_));
 }
 
-// TODO: flaky test should be fixed instead of disabled
+// TODO(anyone): flaky test should be fixed instead of disabled
 TEST_F(LpdTest, DISABLED_CanAnnounceAndRead)
 {
     auto mediator_a = MyMediator{ *session_ };
@@ -135,7 +135,7 @@ TEST_F(LpdTest, DISABLED_CanAnnounceAndRead)
     EXPECT_EQ(0U, mediator_b.found_.count(info_hash_str));
 }
 
-// TODO: flaky test should be fixed instead of disabled
+// TODO(anyone): flaky test should be fixed instead of disabled
 TEST_F(LpdTest, DISABLED_canMultiAnnounce)
 {
     auto mediator_a = MyMediator{ *session_ };
@@ -172,7 +172,7 @@ TEST_F(LpdTest, DISABLED_canMultiAnnounce)
     }
 }
 
-// TODO: flaky test should be fixed instead of disabled
+// TODO(anyone): flaky test should be fixed instead of disabled
 TEST_F(LpdTest, DISABLED_DoesNotReannounceTooSoon)
 {
     auto mediator_a = MyMediator{ *session_ };

tests/libtransmission/watchdir-test.cc

@@ -226,7 +226,7 @@ TEST_P(WatchDirTest, watch)
     EXPECT_TRUE(std::empty(names));
 }
 
-// TODO: flaky test should be fixed instead of disabled
+// TODO(ckerr): flaky test should be fixed instead of disabled
 TEST_P(WatchDirTest, DISABLED_retry)
 {
     auto const path = sandboxDir();