From d935d364ed12ec5afe9f1253deb209ad35804e85 Mon Sep 17 00:00:00 2001 From: Charles Kerr Date: Sat, 30 Mar 2024 14:45:00 -0500 Subject: [PATCH] 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 | 8 +++++- libtransmission/completion.h | 18 ++++++------ libtransmission/torrent.h | 6 ++-- tests/libtransmission/completion-test.cc | 36 ++++++++++++------------ tests/libtransmission/lpd-test.cc | 6 ++-- tests/libtransmission/watchdir-test.cc | 2 +- 6 files changed, 40 insertions(+), 36 deletions(-) diff --git a/libtransmission/completion.cc b/libtransmission/completion.cc index d71369711..87afecf8a 100644 --- a/libtransmission/completion.cc +++ b/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); } diff --git a/libtransmission/completion.h b/libtransmission/completion.h index 9897f3005..b2dfded19 100644 --- a/libtransmission/completion.h +++ b/libtransmission/completion.h @@ -10,8 +10,9 @@ #endif #include -#include #include // size_t +#include +#include #include #include @@ -26,21 +27,18 @@ */ struct tr_completion { - struct torrent_view - { - virtual bool piece_is_wanted(tr_piece_index_t piece) const = 0; + using PieceIsWantedFunc = std::function; - virtual ~torrent_view() = default; - }; - - explicit tr_completion(torrent_view const* tor, tr_block_info const* block_info) - : tor_{ tor } + tr_completion(PieceIsWantedFunc&& piece_is_wanted, tr_block_info const* block_info) + : piece_is_wanted_{ std::move(piece_is_wanted) } , 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 }; diff --git a/libtransmission/torrent.h b/libtransmission/torrent.h index 5a04373b0..f9cf0414e 100644 --- a/libtransmission/torrent.h +++ b/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); } diff --git a/tests/libtransmission/completion-test.cc b/tests/libtransmission/completion-test.cc index da3c57c0a..2f53bee80 100644 --- a/tests/libtransmission/completion-test.cc +++ b/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 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>(); 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{}; @@ -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); diff --git a/tests/libtransmission/lpd-test.cc b/tests/libtransmission/lpd-test.cc index 3c7908dce..315dd6e05 100644 --- a/tests/libtransmission/lpd-test.cc +++ b/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_ }; diff --git a/tests/libtransmission/watchdir-test.cc b/tests/libtransmission/watchdir-test.cc index d7f561653..139fee9c8 100644 --- a/tests/libtransmission/watchdir-test.cc +++ b/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();