refactor: re-use recalculate_wanted_pieces logic in wishlist constructor (#7946)

* refactor: re-use `recalculate_wanted_pieces` logic in wishlist constructor

* test: does not request pieces that the client already has
This commit is contained in:
Yat Ho
2025-12-20 02:44:57 +08:00
committed by GitHub
parent f61b0213a9
commit 15495640d3
2 changed files with 35 additions and 18 deletions

View File

@@ -318,13 +318,14 @@ private:
// ---
void recalculate_wanted_pieces()
void candidate_list_upkeep()
{
auto n_old_c = std::size(candidates_);
auto salter = tr_salt_shaker<tr_piece_index_t>{};
auto const is_sequential = mediator_.is_sequential_download();
auto const sequential_download_from_piece = mediator_.sequential_download_from_piece();
auto const n_pieces = mediator_.piece_count();
candidates_.reserve(n_pieces);
std::sort(
std::begin(candidates_),
@@ -427,7 +428,7 @@ Wishlist::Impl::Impl(Mediator& mediator_in)
: tags_{ {
// candidates
mediator_in.observe_files_wanted_changed([this](tr_torrent*, tr_file_index_t const*, tr_file_index_t, bool)
{ recalculate_wanted_pieces(); }),
{ candidate_list_upkeep(); }),
// replication, unrequested
mediator_in.observe_peer_disconnect([this](tr_torrent*, tr_bitfield const& b, tr_bitfield const& ar)
{ peer_disconnect(b, ar); }),
@@ -460,22 +461,7 @@ Wishlist::Impl::Impl(Mediator& mediator_in)
} }
, mediator_{ mediator_in }
{
auto salter = tr_salt_shaker<tr_piece_index_t>{};
auto const is_sequential = mediator_.is_sequential_download();
auto const sequential_download_from_piece = mediator_.sequential_download_from_piece();
auto const n_pieces = mediator_.piece_count();
candidates_.reserve(n_pieces);
for (tr_piece_index_t piece = 0U; piece < n_pieces; ++piece)
{
if (mediator_.client_has_piece(piece) || !mediator_.client_wants_piece(piece))
{
continue;
}
auto const salt = get_salt(piece, n_pieces, salter(), is_sequential, sequential_download_from_piece);
candidates_.emplace_back(piece, salt, &mediator_);
}
std::sort(std::begin(candidates_), std::end(candidates_));
candidate_list_upkeep();
}
std::vector<tr_block_span_t> Wishlist::Impl::next(

View File

@@ -219,6 +219,37 @@ TEST_F(PeerMgrWishlistTest, doesNotRequestPiecesThatAreNotWanted)
EXPECT_EQ(mediator.block_span_[0].end, spans[0].end);
}
TEST_F(PeerMgrWishlistTest, doesNotRequestPiecesThatClientHas)
{
auto mediator = MockMediator{ *this };
// setup: three pieces
mediator.piece_count_ = 3;
mediator.block_span_[0] = { 0, 100 };
mediator.block_span_[1] = { 100, 200 };
mediator.block_span_[2] = { 200, 250 };
// we have pieces 0, 1
mediator.client_has_piece_.insert(0);
mediator.client_has_piece_.insert(1);
// peer has all pieces
mediator.piece_replication_[0] = 1;
mediator.piece_replication_[1] = 1;
mediator.piece_replication_[2] = 1;
// we want all three pieces
mediator.client_wants_piece_.insert(0);
mediator.client_wants_piece_.insert(1);
mediator.client_wants_piece_.insert(2);
// we should only get piece 2
auto const spans = Wishlist{ mediator }.next(1000, PeerHasAllPieces);
ASSERT_EQ(1U, std::size(spans));
EXPECT_EQ(mediator.block_span_[2].begin, spans[0].begin);
EXPECT_EQ(mediator.block_span_[2].end, spans[0].end);
}
TEST_F(PeerMgrWishlistTest, onlyRequestBlocksThePeerHas)
{
auto mediator = MockMediator{ *this };