feat: do separate IPv4 and IPv6 port checks in Qt and GTK Client (#6525)

* fix: specify `port-test` ip protocol in response when possible

* feat: IPv4 and IPv6 port test in Qt Client

* feat: shorten timeout of `port-test`

* feat: IPv4 and IPv6 port test in Gtk Client

* chore: housekeeping

* refactor: remove IP protocol error message

* code review: mikedld gtk

* feat: return tag in qt rpc response

* code review: mikedld qt

* feat: move port test button up alongside spin button

* fixup! code review: mikedld gtk

* fixup! code review: mikedld qt

* code review: port status initial text

* feat: decouple ipv4 and ipv6 status updates (GTK)

* feat: decouple ipv4 and ipv6 status updates (Qt)

* code review: unknown protocols are non-pending

Co-authored-by: Mike Gelfand <mikedld@users.noreply.github.com>

* code review: simplify status text when the statuses are the same

* Revert "feat: return tag in qt rpc response"

This reverts commit 2a022c2bb0ee7ddad81f8176839cf0d043422368.

* code review: add translation context for status text (GTK)

* code review: move `port_test_pending_` to `Impl` (GTK)

* fixup! code review: move `port_test_pending_` to `Impl` (GTK)

---------

Co-authored-by: Mike Gelfand <mikedld@users.noreply.github.com>
This commit is contained in:
Yat Ho 2024-01-22 06:50:26 +08:00 committed by GitHub
parent 29a566664a
commit 32ef92e7a7
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
12 changed files with 337 additions and 116 deletions

View File

@ -673,7 +673,7 @@ Response arguments:
| Key | Value Type | Description | Key | Value Type | Description
| :-- | :-- | :-- | :-- | :-- | :--
| `port-is-open` | boolean | true if port is open, false if port is closed | `port-is-open` | boolean | true if port is open, false if port is closed
| `ipProtocol` | string | `ipv4` if the test was carried out on IPv4, `ipv6` if the test was carried out on IPv6, unset if an error occured | `ipProtocol` | string | `ipv4` if the test was carried out on IPv4, `ipv6` if the test was carried out on IPv6, unset if it cannot be determined
### 4.5 Session shutdown ### 4.5 Session shutdown
This method tells the transmission session to shut down. This method tells the transmission session to shut down.

View File

@ -42,11 +42,14 @@
#include <fmt/core.h> #include <fmt/core.h>
#include <array>
#include <limits> #include <limits>
#include <map> #include <map>
#include <memory> #include <memory>
#include <optional>
#include <sstream> #include <sstream>
#include <string> #include <string>
#include <string_view>
using namespace libtransmission::Values; using namespace libtransmission::Values;
@ -878,10 +881,23 @@ public:
TR_DISABLE_COPY_MOVE(NetworkPage) TR_DISABLE_COPY_MOVE(NetworkPage)
private: private:
enum PortTestStatus : uint8_t
{
PORT_TEST_UNKNOWN = 0U,
PORT_TEST_CHECKING,
PORT_TEST_OPEN,
PORT_TEST_CLOSED,
PORT_TEST_ERROR
};
void portTestSetSensitive();
void updatePortStatusText();
void onCorePrefsChanged(tr_quark key); void onCorePrefsChanged(tr_quark key);
void onPortTested(bool isOpen); void onPortTested(std::optional<bool> result, Session::PortTestIpProtocol ip_protocol);
void onPortTest(); void onPortTest();
static std::string_view getPortStatusText(PortTestStatus status) noexcept;
private: private:
Glib::RefPtr<Session> core_; Glib::RefPtr<Session> core_;
@ -891,15 +907,18 @@ private:
sigc::connection portTag_; sigc::connection portTag_;
sigc::connection prefsTag_; sigc::connection prefsTag_;
std::array<PortTestStatus, Session::NUM_PORT_TEST_IP_PROTOCOL> portTestStatus_ = {};
}; };
void NetworkPage::onCorePrefsChanged(tr_quark const key) void NetworkPage::onCorePrefsChanged(tr_quark const key)
{ {
if (key == TR_KEY_peer_port) if (key == TR_KEY_peer_port)
{ {
gtr_label_set_text(*portLabel_, _("Status unknown")); portTestStatus_[Session::PORT_TEST_IPV4] = PORT_TEST_UNKNOWN;
portButton_->set_sensitive(true); portTestStatus_[Session::PORT_TEST_IPV6] = PORT_TEST_UNKNOWN;
portSpin_->set_sensitive(true); updatePortStatusText();
portTestSetSensitive();
} }
} }
@ -909,28 +928,79 @@ NetworkPage::~NetworkPage()
portTag_.disconnect(); portTag_.disconnect();
} }
void NetworkPage::onPortTested(bool isOpen) std::string_view NetworkPage::getPortStatusText(PortTestStatus const status) noexcept
{ {
portLabel_->set_markup(fmt::format( switch (status)
isOpen ? _("Port is {markup_begin}open{markup_end}") : _("Port is {markup_begin}closed{markup_end}"), {
fmt::arg("markup_begin", "<b>"), case PORT_TEST_UNKNOWN:
fmt::arg("markup_end", "</b>"))); return C_("Port test status", "unknown");
portButton_->set_sensitive(true); case PORT_TEST_CHECKING:
portSpin_->set_sensitive(true); return C_("Port test status", "checking…");
case PORT_TEST_OPEN:
return C_("Port test status", "open");
case PORT_TEST_CLOSED:
return C_("Port test status", "closed");
case PORT_TEST_ERROR:
return C_("Port test status", "error");
default:
return {};
}
}
void NetworkPage::updatePortStatusText()
{
auto const status_ipv4 = getPortStatusText(portTestStatus_[Session::PORT_TEST_IPV4]);
auto const status_ipv6 = getPortStatusText(portTestStatus_[Session::PORT_TEST_IPV6]);
portLabel_->set_markup(
portTestStatus_[Session::PORT_TEST_IPV4] == portTestStatus_[Session::PORT_TEST_IPV6] ?
fmt::format(_("Status: <b>{status}</b>"), fmt::arg("status", status_ipv4)) :
fmt::format(
_("Status: <b>{status_ipv4}</b> (IPv4), <b>{status_ipv6}</b> (IPv6)"),
fmt::arg("status_ipv4", status_ipv4),
fmt::arg("status_ipv6", status_ipv6)));
}
void NetworkPage::portTestSetSensitive()
{
// Depend on the RPC call status instead of the UI status, so that the widgets
// won't be enabled even if the port peer port changed while we have port-test
// RPC call(s) in-flight.
auto const sensitive = !core_->port_test_pending(Session::PORT_TEST_IPV4) &&
!core_->port_test_pending(Session::PORT_TEST_IPV6);
portButton_->set_sensitive(sensitive);
portSpin_->set_sensitive(sensitive);
}
void NetworkPage::onPortTested(std::optional<bool> const result, Session::PortTestIpProtocol const ip_protocol)
{
// Only update the UI if the current status is "checking", so that
// we won't show the port test results for the old peer port if it
// changed while we have port-test RPC call(s) in-flight.
if (portTestStatus_[ip_protocol] == PORT_TEST_CHECKING)
{
portTestStatus_[ip_protocol] = result ? (*result ? PORT_TEST_OPEN : PORT_TEST_CLOSED) : PORT_TEST_ERROR;
updatePortStatusText();
}
portTestSetSensitive();
} }
void NetworkPage::onPortTest() void NetworkPage::onPortTest()
{ {
portButton_->set_sensitive(false); portTestStatus_[Session::PORT_TEST_IPV4] = PORT_TEST_CHECKING;
portSpin_->set_sensitive(false); portTestStatus_[Session::PORT_TEST_IPV6] = PORT_TEST_CHECKING;
portLabel_->set_text(_("Testing TCP port…")); updatePortStatusText();
if (!portTag_.connected()) if (!portTag_.connected())
{ {
portTag_ = core_->signal_port_tested().connect([this](bool is_open) { onPortTested(is_open); }); portTag_ = core_->signal_port_tested().connect(
[this](std::optional<bool> status, Session::PortTestIpProtocol ip_protocol) { onPortTested(status, ip_protocol); });
} }
core_->port_test(); core_->port_test(Session::PORT_TEST_IPV4);
core_->port_test(Session::PORT_TEST_IPV6);
portTestSetSensitive();
} }
NetworkPage::NetworkPage( NetworkPage::NetworkPage(
@ -944,6 +1014,7 @@ NetworkPage::NetworkPage(
, portSpin_(init_spin_button("listening_port_spin", TR_KEY_peer_port, 1, std::numeric_limits<uint16_t>::max(), 1)) , portSpin_(init_spin_button("listening_port_spin", TR_KEY_peer_port, 1, std::numeric_limits<uint16_t>::max(), 1))
{ {
portButton_->signal_clicked().connect([this]() { onPortTest(); }); portButton_->signal_clicked().connect([this]() { onPortTest(); });
updatePortStatusText();
prefsTag_ = core_->signal_prefs_changed().connect([this](auto key) { onCorePrefsChanged(key); }); prefsTag_ = core_->signal_prefs_changed().connect([this](auto key) { onCorePrefsChanged(key); });

View File

@ -44,12 +44,14 @@
#include <fmt/core.h> #include <fmt/core.h>
#include <algorithm> #include <algorithm>
#include <array>
#include <cinttypes> // PRId64 #include <cinttypes> // PRId64
#include <cstring> // strstr #include <cstring> // strstr
#include <functional> #include <functional>
#include <iostream> #include <iostream>
#include <map> #include <map>
#include <memory> #include <memory>
#include <optional>
#include <string> #include <string>
#include <string_view> #include <string_view>
#include <utility> #include <utility>
@ -74,6 +76,9 @@ public:
size_t get_active_torrent_count() const; size_t get_active_torrent_count() const;
bool get_port_test_pending(PortTestIpProtocol ip_protocol);
void set_port_test_pending(bool pending, PortTestIpProtocol ip_protocol);
void update(); void update();
void torrents_added(); void torrents_added();
@ -169,7 +174,7 @@ private:
sigc::signal<void(bool)> signal_blocklist_updated_; sigc::signal<void(bool)> signal_blocklist_updated_;
sigc::signal<void(bool)> signal_busy_; sigc::signal<void(bool)> signal_busy_;
sigc::signal<void(tr_quark)> signal_prefs_changed_; sigc::signal<void(tr_quark)> signal_prefs_changed_;
sigc::signal<void(bool)> signal_port_tested_; sigc::signal<void(std::optional<bool>, PortTestIpProtocol)> signal_port_tested_;
sigc::signal<void(std::unordered_set<tr_torrent_id_t> const&, Torrent::ChangeFlags)> signal_torrents_changed_; sigc::signal<void(std::unordered_set<tr_torrent_id_t> const&, Torrent::ChangeFlags)> signal_torrents_changed_;
Glib::RefPtr<Gio::FileMonitor> monitor_; Glib::RefPtr<Gio::FileMonitor> monitor_;
@ -182,6 +187,7 @@ private:
bool inhibit_allowed_ = false; bool inhibit_allowed_ = false;
bool have_inhibit_cookie_ = false; bool have_inhibit_cookie_ = false;
bool dbus_error_ = false; bool dbus_error_ = false;
std::array<bool, NUM_PORT_TEST_IP_PROTOCOL> port_test_pending_ = {};
guint inhibit_cookie_ = 0; guint inhibit_cookie_ = 0;
gint busy_count_ = 0; gint busy_count_ = 0;
Glib::RefPtr<Gio::ListStore<Torrent>> raw_model_; Glib::RefPtr<Gio::ListStore<Torrent>> raw_model_;
@ -1220,33 +1226,67 @@ void Session::Impl::send_rpc_request(
**** Sending a test-port request via RPC **** Sending a test-port request via RPC
***/ ***/
void Session::port_test() void Session::port_test(PortTestIpProtocol const ip_protocol)
{ {
auto const tag = nextTag; static auto constexpr IpStr = std::array{ "ipv4"sv, "ipv6"sv };
++nextTag;
if (port_test_pending(ip_protocol))
{
return;
}
impl_->set_port_test_pending(true, ip_protocol);
auto const tag = nextTag++;
auto arguments_map = tr_variant::Map{ 1U };
arguments_map.try_emplace(TR_KEY_ipProtocol, tr_variant::unmanaged_string(IpStr[ip_protocol]));
auto request_map = tr_variant::Map{ 3U };
request_map.try_emplace(TR_KEY_method, tr_variant::unmanaged_string("port-test"sv));
request_map.try_emplace(TR_KEY_tag, tag);
request_map.try_emplace(TR_KEY_arguments, std::move(arguments_map));
tr_variant request;
tr_variantInitDict(&request, 2);
tr_variantDictAddStrView(&request, TR_KEY_method, "port-test");
tr_variantDictAddInt(&request, TR_KEY_tag, tag);
impl_->send_rpc_request( impl_->send_rpc_request(
request, tr_variant{ std::move(request_map) },
tag, tag,
[this](auto& response) [this, ip_protocol](tr_variant& response)
{ {
tr_variant* args = nullptr; impl_->set_port_test_pending(false, ip_protocol);
bool is_open = false;
if (!tr_variantDictFindDict(&response, TR_KEY_arguments, &args) || auto status = std::optional<bool>{};
!tr_variantDictFindBool(args, TR_KEY_port_is_open, &is_open)) if (tr_variant* args = nullptr; tr_variantDictFindDict(&response, TR_KEY_arguments, &args))
{ {
is_open = false; if (auto result = bool{}; tr_variantDictFindBool(args, TR_KEY_port_is_open, &result))
{
status = result;
}
} }
impl_->signal_port_tested().emit(is_open); // If for whatever reason the status optional is empty here,
// then something must have gone wrong with the port test,
// so the UI should show the "error" state
impl_->signal_port_tested().emit(status, ip_protocol);
}); });
} }
bool Session::port_test_pending(Session::PortTestIpProtocol ip_protocol) const noexcept
{
return impl_->get_port_test_pending(ip_protocol);
}
bool Session::Impl::get_port_test_pending(Session::PortTestIpProtocol ip_protocol)
{
return ip_protocol < NUM_PORT_TEST_IP_PROTOCOL && port_test_pending_[ip_protocol];
}
void Session::Impl::set_port_test_pending(bool pending, Session::PortTestIpProtocol ip_protocol)
{
if (ip_protocol < NUM_PORT_TEST_IP_PROTOCOL)
{
port_test_pending_[ip_protocol] = pending;
}
}
/*** /***
**** Updating a blocklist via RPC **** Updating a blocklist via RPC
***/ ***/
@ -1386,7 +1426,7 @@ sigc::signal<void(tr_quark)>& Session::signal_prefs_changed()
return impl_->signal_prefs_changed(); return impl_->signal_prefs_changed();
} }
sigc::signal<void(bool)>& Session::signal_port_tested() sigc::signal<void(std::optional<bool>, Session::PortTestIpProtocol)>& Session::signal_port_tested()
{ {
return impl_->signal_port_tested(); return impl_->signal_port_tested();
} }

View File

@ -23,6 +23,7 @@
#include <cstddef> #include <cstddef>
#include <cstdint> #include <cstdint>
#include <memory> #include <memory>
#include <optional>
#include <string> #include <string>
#include <unordered_set> #include <unordered_set>
#include <vector> #include <vector>
@ -37,6 +38,13 @@ public:
ERR_NO_MORE_TORRENTS = 1000 /* finished adding a batch */ ERR_NO_MORE_TORRENTS = 1000 /* finished adding a batch */
}; };
enum PortTestIpProtocol : uint8_t
{
PORT_TEST_IPV4,
PORT_TEST_IPV6,
NUM_PORT_TEST_IP_PROTOCOL // Must always be the last value
};
using Model = IF_GTKMM4(Gio::ListModel, Gtk::TreeModel); using Model = IF_GTKMM4(Gio::ListModel, Gtk::TreeModel);
public: public:
@ -128,7 +136,8 @@ public:
*** ***
**/ **/
void port_test(); void port_test(PortTestIpProtocol ip_protocol);
bool port_test_pending(PortTestIpProtocol ip_protocol) const noexcept;
void blocklist_update(); void blocklist_update();
@ -141,7 +150,7 @@ public:
sigc::signal<void(bool)>& signal_blocklist_updated(); sigc::signal<void(bool)>& signal_blocklist_updated();
sigc::signal<void(bool)>& signal_busy(); sigc::signal<void(bool)>& signal_busy();
sigc::signal<void(tr_quark)>& signal_prefs_changed(); sigc::signal<void(tr_quark)>& signal_prefs_changed();
sigc::signal<void(bool)>& signal_port_tested(); sigc::signal<void(std::optional<bool>, PortTestIpProtocol)>& signal_port_tested();
sigc::signal<void(std::unordered_set<tr_torrent_id_t> const&, Torrent::ChangeFlags)>& signal_torrents_changed(); sigc::signal<void(std::unordered_set<tr_torrent_id_t> const&, Torrent::ChangeFlags)>& signal_torrents_changed();
protected: protected:

View File

@ -1245,14 +1245,18 @@
</packing> </packing>
</child> </child>
<child> <child>
<object class="GtkSpinButton" id="listening_port_spin"> <object class="GtkLabel" id="listening_port_status_label">
<property name="visible">True</property> <property name="visible">True</property>
<property name="can-focus">True</property> <property name="can-focus">False</property>
<property name="hexpand">True</property> <property name="label">...</property>
<property name="xalign">0</property>
<attributes>
<attribute name="style" value="italic"/>
</attributes>
</object> </object>
<packing> <packing>
<property name="left-attach">1</property> <property name="left-attach">1</property>
<property name="top-attach">0</property> <property name="top-attach">1</property>
</packing> </packing>
</child> </child>
<child> <child>
@ -1262,14 +1266,10 @@
<property name="hexpand">True</property> <property name="hexpand">True</property>
<property name="spacing">12</property> <property name="spacing">12</property>
<child> <child>
<object class="GtkLabel" id="listening_port_status_label"> <object class="GtkSpinButton" id="listening_port_spin">
<property name="visible">True</property> <property name="visible">True</property>
<property name="can-focus">False</property> <property name="can-focus">True</property>
<property name="label" translatable="yes">Status unknown</property> <property name="hexpand">True</property>
<property name="xalign">0</property>
<attributes>
<attribute name="style" value="italic"/>
</attributes>
</object> </object>
<packing> <packing>
<property name="expand">True</property> <property name="expand">True</property>
@ -1295,7 +1295,7 @@
</object> </object>
<packing> <packing>
<property name="left-attach">1</property> <property name="left-attach">1</property>
<property name="top-attach">1</property> <property name="top-attach">0</property>
</packing> </packing>
</child> </child>
</object> </object>

View File

@ -868,12 +868,16 @@
</object> </object>
</child> </child>
<child> <child>
<object class="GtkSpinButton" id="listening_port_spin"> <object class="GtkLabel" id="listening_port_status_label">
<property name="focusable">1</property>
<property name="hexpand">1</property> <property name="hexpand">1</property>
<property name="label">...</property>
<property name="xalign">0</property>
<attributes>
<attribute name="style" value="italic"/>
</attributes>
<layout> <layout>
<property name="column">1</property> <property name="column">1</property>
<property name="row">0</property> <property name="row">1</property>
</layout> </layout>
</object> </object>
</child> </child>
@ -882,13 +886,13 @@
<property name="hexpand">1</property> <property name="hexpand">1</property>
<property name="spacing">12</property> <property name="spacing">12</property>
<child> <child>
<object class="GtkLabel" id="listening_port_status_label"> <object class="GtkSpinButton" id="listening_port_spin">
<property name="focusable">1</property>
<property name="hexpand">1</property> <property name="hexpand">1</property>
<property name="label" translatable="1">Status unknown</property> <layout>
<property name="xalign">0</property> <property name="column">1</property>
<attributes> <property name="row">0</property>
<attribute name="style" value="italic"></attribute> </layout>
</attributes>
</object> </object>
</child> </child>
<child> <child>
@ -901,7 +905,7 @@
</child> </child>
<layout> <layout>
<property name="column">1</property> <property name="column">1</property>
<property name="row">1</property> <property name="row">0</property>
</layout> </layout>
</object> </object>
</child> </child>

View File

@ -1144,6 +1144,12 @@ void onPortTested(tr_web::FetchResponse const& web_response)
auto const& [status, body, primary_ip, did_connect, did_timeout, user_data] = web_response; auto const& [status, body, primary_ip, did_connect, did_timeout, user_data] = web_response;
auto* data = static_cast<tr_rpc_idle_data*>(user_data); auto* data = static_cast<tr_rpc_idle_data*>(user_data);
if (auto const addr = tr_address::from_string(primary_ip);
data->args_out.find_if<std::string_view>(TR_KEY_ipProtocol) == nullptr && addr && addr->is_valid())
{
data->args_out.try_emplace(TR_KEY_ipProtocol, addr->is_ipv4() ? "ipv4"sv : "ipv6"sv);
}
if (status != 200) if (status != 200)
{ {
tr_idle_function_done( tr_idle_function_done(
@ -1155,30 +1161,30 @@ void onPortTested(tr_web::FetchResponse const& web_response)
return; return;
} }
auto const addr = tr_address::from_string(primary_ip);
if (!addr || !addr->is_valid())
{
tr_idle_function_done(data, "Unknown error, please file a bug report to us");
return;
}
data->args_out.try_emplace(TR_KEY_port_is_open, tr_strv_starts_with(body, '1')); data->args_out.try_emplace(TR_KEY_port_is_open, tr_strv_starts_with(body, '1'));
data->args_out.try_emplace(TR_KEY_ipProtocol, addr->is_ipv4() ? "ipv4"sv : "ipv6"sv);
tr_idle_function_done(data, SuccessResult); tr_idle_function_done(data, SuccessResult);
} }
char const* portTest(tr_session* session, tr_variant::Map const& args_in, struct tr_rpc_idle_data* idle_data) char const* portTest(tr_session* session, tr_variant::Map const& args_in, struct tr_rpc_idle_data* idle_data)
{ {
auto ip_proto = tr_web::FetchOptions::IPProtocol::ANY; static auto constexpr TimeoutSecs = 20s;
auto const port = session->advertisedPeerPort();
auto const url = fmt::format("https://portcheck.transmissionbt.com/{:d}", port.host());
auto options = tr_web::FetchOptions{ url, onPortTested, idle_data };
options.timeout_secs = TimeoutSecs;
if (auto const* val = args_in.find_if<std::string_view>(TR_KEY_ipProtocol); val != nullptr) if (auto const* val = args_in.find_if<std::string_view>(TR_KEY_ipProtocol); val != nullptr)
{ {
if (*val == "ipv4"sv) if (*val == "ipv4"sv)
{ {
ip_proto = tr_web::FetchOptions::IPProtocol::V4; options.ip_proto = tr_web::FetchOptions::IPProtocol::V4;
idle_data->args_out.try_emplace(TR_KEY_ipProtocol, "ipv4"sv);
} }
else if (*val == "ipv6"sv) else if (*val == "ipv6"sv)
{ {
ip_proto = tr_web::FetchOptions::IPProtocol::V6; options.ip_proto = tr_web::FetchOptions::IPProtocol::V6;
idle_data->args_out.try_emplace(TR_KEY_ipProtocol, "ipv6"sv);
} }
else else
{ {
@ -1186,10 +1192,6 @@ char const* portTest(tr_session* session, tr_variant::Map const& args_in, struct
} }
} }
auto const port = session->advertisedPeerPort();
auto const url = fmt::format("https://portcheck.transmissionbt.com/{:d}", port.host());
auto options = tr_web::FetchOptions{ url, onPortTested, idle_data };
options.ip_proto = ip_proto;
session->fetch(std::move(options)); session->fetch(std::move(options));
return nullptr; return nullptr;
} }

View File

@ -6,6 +6,7 @@
#include "PrefsDialog.h" #include "PrefsDialog.h"
#include <cassert> #include <cassert>
#include <optional>
#include <QCheckBox> #include <QCheckBox>
#include <QComboBox> #include <QComboBox>
@ -426,19 +427,70 @@ void PrefsDialog::initDesktopTab()
// --- // ---
void PrefsDialog::onPortTested(bool isOpen) QString PrefsDialog::getPortStatusText(PrefsDialog::PortTestStatus status) noexcept
{ {
ui_.testPeerPortButton->setEnabled(true); switch (status)
widgets_[Prefs::PEER_PORT]->setEnabled(true); {
ui_.peerPortStatusLabel->setText(isOpen ? tr("Port is <b>open</b>") : tr("Port is <b>closed</b>")); case PORT_TEST_UNKNOWN:
return tr("unknown");
case PORT_TEST_CHECKING:
return tr("checking…");
case PORT_TEST_OPEN:
return tr("open");
case PORT_TEST_CLOSED:
return tr("closed");
case PORT_TEST_ERROR:
return tr("error");
default:
return {};
}
}
void PrefsDialog::updatePortStatusLabel()
{
auto const status_ipv4 = getPortStatusText(port_test_status_[Session::PORT_TEST_IPV4]);
auto const status_ipv6 = getPortStatusText(port_test_status_[Session::PORT_TEST_IPV6]);
ui_.peerPortStatusLabel->setText(
port_test_status_[Session::PORT_TEST_IPV4] == port_test_status_[Session::PORT_TEST_IPV6] ?
tr("Status: <b>%1</b>").arg(status_ipv4) :
tr("Status: <b>%1</b> (IPv4), <b>%2</b> (IPv6)").arg(status_ipv4).arg(status_ipv6));
}
void PrefsDialog::portTestSetEnabled()
{
// Depend on the RPC call status instead of the UI status, so that the widgets
// won't be enabled even if the port peer port changed while we have port-test
// RPC call(s) in-flight.
auto const sensitive = !session_.portTestPending(Session::PORT_TEST_IPV4) &&
!session_.portTestPending(Session::PORT_TEST_IPV6);
ui_.testPeerPortButton->setEnabled(sensitive);
widgets_[Prefs::PEER_PORT]->setEnabled(sensitive);
}
void PrefsDialog::onPortTested(std::optional<bool> result, Session::PortTestIpProtocol ip_protocol)
{
// Only update the UI if the current status is "checking", so that
// we won't show the port test results for the old peer port if it
// changed while we have port-test RPC call(s) in-flight.
if (port_test_status_[ip_protocol] == PORT_TEST_CHECKING)
{
port_test_status_[ip_protocol] = result ? (*result ? PORT_TEST_OPEN : PORT_TEST_CLOSED) : PORT_TEST_ERROR;
updatePortStatusLabel();
}
portTestSetEnabled();
} }
void PrefsDialog::onPortTest() void PrefsDialog::onPortTest()
{ {
ui_.peerPortStatusLabel->setText(tr("Testing TCP Port…")); port_test_status_[Session::PORT_TEST_IPV4] = PORT_TEST_CHECKING;
ui_.testPeerPortButton->setEnabled(false); port_test_status_[Session::PORT_TEST_IPV6] = PORT_TEST_CHECKING;
widgets_[Prefs::PEER_PORT]->setEnabled(false); updatePortStatusLabel();
session_.portTest();
session_.portTest(Session::PORT_TEST_IPV4);
session_.portTest(Session::PORT_TEST_IPV6);
portTestSetEnabled();
} }
void PrefsDialog::initNetworkTab() void PrefsDialog::initNetworkTab()
@ -464,6 +516,8 @@ void PrefsDialog::initNetworkTab()
connect(ui_.testPeerPortButton, &QAbstractButton::clicked, this, &PrefsDialog::onPortTest); connect(ui_.testPeerPortButton, &QAbstractButton::clicked, this, &PrefsDialog::onPortTest);
connect(&session_, &Session::portTested, this, &PrefsDialog::onPortTested); connect(&session_, &Session::portTested, this, &PrefsDialog::onPortTested);
updatePortStatusLabel();
} }
// --- // ---
@ -781,8 +835,10 @@ void PrefsDialog::refreshPref(int key)
} }
case Prefs::PEER_PORT: case Prefs::PEER_PORT:
ui_.peerPortStatusLabel->setText(tr("Status unknown")); port_test_status_[Session::PORT_TEST_IPV4] = PORT_TEST_UNKNOWN;
ui_.testPeerPortButton->setEnabled(true); port_test_status_[Session::PORT_TEST_IPV6] = PORT_TEST_UNKNOWN;
updatePortStatusLabel();
portTestSetEnabled();
break; break;
default: default:

View File

@ -5,21 +5,21 @@
#pragma once #pragma once
#include <array>
#include <map> #include <map>
#include <optional>
#include <libtransmission/tr-macros.h> #include <libtransmission/tr-macros.h>
#include "BaseDialog.h" #include "BaseDialog.h"
#include "Prefs.h" #include "Prefs.h"
#include "Session.h"
#include "ui_PrefsDialog.h" #include "ui_PrefsDialog.h"
class QHttp; class QHttp;
class QMessageBox; class QMessageBox;
class QString; class QString;
class Prefs;
class Session;
class PrefsDialog : public BaseDialog class PrefsDialog : public BaseDialog
{ {
Q_OBJECT Q_OBJECT
@ -39,7 +39,7 @@ private slots:
void encryptionEdited(int); void encryptionEdited(int);
void altSpeedDaysEdited(int); void altSpeedDaysEdited(int);
void sessionUpdated(); void sessionUpdated();
void onPortTested(bool); void onPortTested(std::optional<bool>, Session::PortTestIpProtocol);
void onPortTest(); void onPortTest();
void onIdleLimitChanged(); void onIdleLimitChanged();
void onQueueStalledMinutesChanged(); void onQueueStalledMinutesChanged();
@ -52,11 +52,23 @@ private slots:
private: private:
using key2widget_t = std::map<int, QWidget*>; using key2widget_t = std::map<int, QWidget*>;
enum PortTestStatus : uint8_t
{
PORT_TEST_UNKNOWN = 0U,
PORT_TEST_CHECKING,
PORT_TEST_OPEN,
PORT_TEST_CLOSED,
PORT_TEST_ERROR
};
bool updateWidgetValue(QWidget* widget, int pref_key) const; bool updateWidgetValue(QWidget* widget, int pref_key) const;
void portTestSetEnabled();
void linkWidgetToPref(QWidget* widget, int pref_key); void linkWidgetToPref(QWidget* widget, int pref_key);
void updateBlocklistLabel(); void updateBlocklistLabel();
void updateDownloadingWidgetsLocality(); void updateDownloadingWidgetsLocality();
void updatePortStatusLabel();
void updateSeedingWidgetsLocality(); void updateSeedingWidgetsLocality();
static QString getPortStatusText(PortTestStatus status) noexcept;
void setPref(int key, QVariant const& v); void setPref(int key, QVariant const& v);
@ -75,6 +87,7 @@ private:
bool const is_server_; bool const is_server_;
bool is_local_ = {}; bool is_local_ = {};
std::array<PortTestStatus, Session::NUM_PORT_TEST_IP_PROTOCOL> port_test_status_ = {};
key2widget_t widgets_; key2widget_t widgets_;
QWidgetList web_widgets_; QWidgetList web_widgets_;

View File

@ -650,7 +650,7 @@
</property> </property>
</widget> </widget>
</item> </item>
<item row="0" column="1" colspan="2"> <item row="0" column="1">
<widget class="QSpinBox" name="peerPortSpin"> <widget class="QSpinBox" name="peerPortSpin">
<property name="minimum"> <property name="minimum">
<number>1</number> <number>1</number>
@ -660,14 +660,7 @@
</property> </property>
</widget> </widget>
</item> </item>
<item row="1" column="1"> <item row="0" column="2">
<widget class="QLabel" name="peerPortStatusLabel">
<property name="text">
<string>Status unknown</string>
</property>
</widget>
</item>
<item row="1" column="2">
<widget class="QPushButton" name="testPeerPortButton"> <widget class="QPushButton" name="testPeerPortButton">
<property name="text"> <property name="text">
<string>Te&amp;st Port</string> <string>Te&amp;st Port</string>
@ -677,6 +670,13 @@
</property> </property>
</widget> </widget>
</item> </item>
<item row="1" column="1" colspan="2">
<widget class="QLabel" name="peerPortStatusLabel">
<property name="text">
<string notr="true">...</string>
</property>
</widget>
</item>
<item row="2" column="0" colspan="3"> <item row="2" column="0" colspan="3">
<widget class="QCheckBox" name="randomPeerPortCheck"> <widget class="QCheckBox" name="randomPeerPortCheck">
<property name="text"> <property name="text">

View File

@ -6,6 +6,7 @@
#include <algorithm> #include <algorithm>
#include <array> #include <array>
#include <cassert> #include <cassert>
#include <optional>
#include <string_view> #include <string_view>
#include <utility> #include <utility>
@ -39,6 +40,8 @@
#include "Torrent.h" #include "Torrent.h"
#include "VariantHelpers.h" #include "VariantHelpers.h"
using namespace std::literals;
using ::trqt::variant_helpers::dictAdd; using ::trqt::variant_helpers::dictAdd;
using ::trqt::variant_helpers::dictFind; using ::trqt::variant_helpers::dictFind;
using ::trqt::variant_helpers::getValue; using ::trqt::variant_helpers::getValue;
@ -81,32 +84,43 @@ void Session::sessionSet(tr_quark const key, QVariant const& value)
exec("session-set", &args); exec("session-set", &args);
} }
void Session::portTest() void Session::portTest(Session::PortTestIpProtocol const ip_protocol)
{ {
static auto constexpr IpStr = std::array{ "ipv4"sv, "ipv6"sv };
if (portTestPending(ip_protocol))
{
return;
}
port_test_pending_[ip_protocol] = true;
auto args = tr_variant::make_map(1U);
tr_variantDictAddStrView(&args, TR_KEY_ipProtocol, IpStr[ip_protocol]);
auto const response_func = [this, ip_protocol](RpcResponse const& r)
{
port_test_pending_[ip_protocol] = false;
// If for whatever reason the status optional is empty here,
// then something must have gone wrong with the port test,
// so the UI should show the "error" state
emit portTested(dictFind<bool>(r.args.get(), TR_KEY_port_is_open), ip_protocol);
};
auto* q = new RpcQueue{}; auto* q = new RpcQueue{};
q->add([this]() { return exec("port-test", nullptr); }); q->add([this, &args]() { return exec("port-test", &args); }, response_func);
q->add( q->add(response_func);
[this](RpcResponse const& r)
{
bool is_open = false;
if (r.success)
{
auto const value = dictFind<bool>(r.args.get(), TR_KEY_port_is_open);
if (value)
{
is_open = *value;
}
}
emit portTested(is_open);
});
q->run(); q->run();
} }
bool Session::portTestPending(Session::PortTestIpProtocol const ip_protocol) const noexcept
{
return ip_protocol < NUM_PORT_TEST_IP_PROTOCOL && port_test_pending_[ip_protocol];
}
void Session::copyMagnetLinkToClipboard(int torrent_id) void Session::copyMagnetLinkToClipboard(int torrent_id)
{ {
auto constexpr MagnetLinkKey = std::string_view{ "magnetLink" }; auto constexpr MagnetLinkKey = std::string_view{ "magnetLink" };

View File

@ -5,8 +5,10 @@
#pragma once #pragma once
#include <array>
#include <cstdint> // int64_t #include <cstdint> // int64_t
#include <map> #include <map>
#include <optional>
#include <string_view> #include <string_view>
#include <vector> #include <vector>
@ -69,11 +71,20 @@ public:
return blocklist_size_; return blocklist_size_;
} }
enum PortTestIpProtocol : uint8_t
{
PORT_TEST_IPV4,
PORT_TEST_IPV6,
NUM_PORT_TEST_IP_PROTOCOL
};
void setBlocklistSize(int64_t i); void setBlocklistSize(int64_t i);
void updateBlocklist(); void updateBlocklist();
void portTest(); void portTest(PortTestIpProtocol ip_protocol);
void copyMagnetLinkToClipboard(int torrent_id); void copyMagnetLinkToClipboard(int torrent_id);
bool portTestPending(PortTestIpProtocol ip_protocol) const noexcept;
/** returns true if the transmission session is being run inside this client */ /** returns true if the transmission session is being run inside this client */
bool isServer() const; bool isServer() const;
@ -130,7 +141,7 @@ public slots:
signals: signals:
void sourceChanged(); void sourceChanged();
void portTested(bool is_open); void portTested(std::optional<bool> status, PortTestIpProtocol ip_protocol);
void statsUpdated(); void statsUpdated();
void sessionUpdated(); void sessionUpdated();
void blocklistUpdated(int); void blocklistUpdated(int);
@ -168,6 +179,7 @@ private:
std::map<TorrentProperties, std::vector<std::string_view>> names_; std::map<TorrentProperties, std::vector<std::string_view>> names_;
int64_t blocklist_size_ = -1; int64_t blocklist_size_ = -1;
std::array<bool, NUM_PORT_TEST_IP_PROTOCOL> port_test_pending_ = {};
tr_session* session_ = {}; tr_session* session_ = {};
QStringList idle_json_; QStringList idle_json_;
tr_session_stats stats_ = EmptyStats; tr_session_stats stats_ = EmptyStats;