fix: Add automatic VPN connection after network state monitoring#522
Merged
deepin-bot[bot] merged 1 commit intolinuxdeepin:masterfrom Mar 19, 2026
Merged
fix: Add automatic VPN connection after network state monitoring#522deepin-bot[bot] merged 1 commit intolinuxdeepin:masterfrom
deepin-bot[bot] merged 1 commit intolinuxdeepin:masterfrom
Conversation
Reviewer's GuideAdds automatic VPN activation when NetworkManager reports a successful network connection, wiring status change notifications into NetworkThread and selecting appropriate autoconnect VPN profiles to start via D-Bus. Sequence diagram for automatic VPN activation on network status changesequenceDiagram
participant NetworkManagerNotifier as NetworkManager_Notifier
participant NetworkThread as NetworkThread
participant NetworkManager as NetworkManager
participant DBus as DBus
NetworkManagerNotifier->>NetworkThread: statusChanged(status)
activate NetworkThread
NetworkThread->>NetworkThread: onStatusChanged(status)
alt vpnEnabled is false
NetworkThread-->>NetworkManagerNotifier: return
else status is not Connected
NetworkThread-->>NetworkManagerNotifier: return
else status is Connected and vpnEnabled is true
NetworkThread->>NetworkManager: activeConnections()
NetworkManager-->>NetworkThread: ActiveConnection_List
NetworkThread->>NetworkThread: build actConnections
NetworkThread->>NetworkManager: listConnections()
NetworkManager-->>NetworkThread: Connection_List
NetworkThread->>NetworkThread: filter autoconnect VPN connections
NetworkThread->>NetworkThread: select latest per serviceType
loop for each selected VPN connection
NetworkThread->>NetworkManager: activateConnection(connectionPath, rootPath, rootPath)
NetworkManager-->>DBus: org.freedesktop.NetworkManager.ActivateConnection
DBus-->>NetworkThread: QDBusPendingReply
alt activation error
NetworkThread->>NetworkThread: log warning
else activation success
NetworkThread->>NetworkThread: log debug
end
end
end
deactivate NetworkThread
Updated class diagram for NetworkThread with onStatusChangedclassDiagram
class NetworkThread {
- NetworkEnabledConfig* m_networkConfig
- QList~NetworkManager_Device_Ptr~ m_devices
+ void init()
+ QString enableDevice(NetworkManager_Device_Ptr device)
+ QString disableDevice(NetworkManager_Device_Ptr device)
+ void onStatusChanged(NetworkManager_Status status)
- void disableVpn()
- bool airplaneWifiEnabled()
}
class NetworkEnabledConfig {
+ bool vpnEnabled()
}
class NetworkManager_Notifier {
+ void deviceAdded()
+ void deviceRemoved()
+ void statusChanged(NetworkManager_Status status)
}
class NetworkManager {
+ static NetworkManager_Notifier* notifier()
+ static NetworkManager_ActiveConnection_List activeConnections()
+ static NetworkManager_Connection_List listConnections()
+ static QDBusPendingReply activateConnection(QString connectionPath, QString devicePath, QString specificObjectPath)
}
class NetworkManager_ActiveConnection {
+ NetworkManager_ActiveConnection_State state()
+ NetworkManager_Connection_Ptr connection()
}
class NetworkManager_Connection {
+ QString path()
+ QString name()
+ NetworkManager_ConnectionSettings_Ptr settings()
}
class NetworkManager_ConnectionSettings {
+ bool autoconnect()
+ NetworkManager_ConnectionSettings_ConnectionType connectionType()
+ quint64 timestamp()
+ NetworkManager_VpnSetting_Ptr setting(NetworkManager_Setting_SettingType type)
}
class NetworkManager_VpnSetting {
+ QString serviceType()
}
NetworkManager_Notifier --> NetworkThread : emits_statusChanged
NetworkThread --> NetworkEnabledConfig : uses
NetworkThread --> NetworkManager : uses
NetworkManager --> NetworkManager_Notifier : returns
NetworkManager --> NetworkManager_ActiveConnection : manages
NetworkManager --> NetworkManager_Connection : manages
NetworkManager_ActiveConnection --> NetworkManager_Connection : refers
NetworkManager_Connection --> NetworkManager_ConnectionSettings : has
NetworkManager_ConnectionSettings --> NetworkManager_VpnSetting : returns
Flow diagram for NetworkThread onStatusChanged VPN selection logicflowchart TD
A[onStatusChanged status] --> B{vpnEnabled}
B -- false --> Z[Return]
B -- true --> C{status is Connected}
C -- false --> Z
C -- true --> D[Get activeConnections]
D --> E[Build actConnections list of non activating and non activated connections]
E --> F[Get all connections via listConnections]
F --> G[Iterate connections]
G --> H{connection path in actConnections}
H -- true --> G
H -- false --> I{settings autoconnect is true}
I -- false --> G
I -- true --> J{connectionType is Vpn}
J -- false --> G
J -- true --> K[Get VpnSetting and serviceType]
K --> L{serviceType in vpnContainer}
L -- false --> M[Add connection to vpnContainer]
L -- true --> N{timestamp newer than stored}
N -- true --> O[Replace stored connection]
N -- false --> G
M --> G
O --> G
G -->|loop done| P[Iterate vpnContainer]
P --> Q[For each selected VPN connection call activateConnection via DBus]
Q --> R{reply is error}
R -- true --> S[Log warning]
R -- false --> T[Log success]
S --> U[End]
T --> U[End]
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- In
onStatusChanged, the logic buildingactConnectionsappears inverted: you add connections whose state is notActivated/Activatingand then skip any connection whose path is inactConnections, which contradicts the comment// 过滤掉正在连接的and likely ends up skipping inactive connections instead of active/activating ones. - Calling
onStatusChanged(NetworkManager::status())directly frominit()assumes thatm_networkConfigand the NetworkManager state are already fully initialized; if there are code paths where this is not true, you may want to guard this call or defer it until initialization is guaranteed complete.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `onStatusChanged`, the logic building `actConnections` appears inverted: you add connections whose state is *not* `Activated`/`Activating` and then skip any connection whose path is in `actConnections`, which contradicts the comment `// 过滤掉正在连接的` and likely ends up skipping inactive connections instead of active/activating ones.
- Calling `onStatusChanged(NetworkManager::status())` directly from `init()` assumes that `m_networkConfig` and the NetworkManager state are already fully initialized; if there are code paths where this is not true, you may want to guard this call or defer it until initialization is guaranteed complete.
## Individual Comments
### Comment 1
<location path="network-service-plugin/src/system/networkthread.cpp" line_range="389-390" />
<code_context>
+
+ QStringList actConnections;
+ NetworkManager::ActiveConnection::List allactiveConnections = NetworkManager::activeConnections();
+ for (const NetworkManager::ActiveConnection::Ptr &activeConnection : allactiveConnections) {
+ if (activeConnection->state() != NetworkManager::ActiveConnection::State::Activated
+ && activeConnection->state() != NetworkManager::ActiveConnection::State::Activating)
+ actConnections << activeConnection->connection()->path();
</code_context>
<issue_to_address>
**issue (bug_risk):** The filter for `actConnections` looks inverted and will skip the wrong connections.
This condition collects connections whose state is neither `Activated` nor `Activating`. Later, `actConnections.contains(conn->path())` is used to skip connections that are “currently connecting”, but with this logic you instead skip those that are *not* active/activating, which inverts the intended behavior (`// 过滤掉正在连接的`). Please flip the condition so only `Activated`/`Activating` connections are added to `actConnections` (e.g. use `==` with `||`).
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
ut003640
force-pushed
the
master
branch
2 times, most recently
from
b10947f to
1a6b332
Compare
caixr23
previously approved these changes
Mar 19, 2026
VPN auto-connection is not started by NetworkManager, but by external network services. This adds the VPN startup functionality. Log: Fix VPN not auto-connecting PMS: BUG-287121 fix: 增加监听网络状态后自动连接VPN VPN自动连接并非NetworkManager启动，而是由外部网络服务启动，这里加上启动VPN的功能 Log: 修复VPN没有自动连接
deepin pr auto review这段代码主要实现了一个功能：在网络状态变为已连接后，自动查找并连接设置了"自动连接"属性的 VPN。以下是对这段 1. 语法逻辑
2. 代码质量
3. 代码性能
4. 代码安全
总结与修改建议代码以下是结合上述建议优化后的代码片段（主要优化了数据结构以提高性能）： void NetworkThread::onStatusChanged(NetworkManager::Status status)
{
// 防御性检查：确保VPN功能已开启且网络已连接
if (!m_networkConfig->vpnEnabled())
return;
if (status != NetworkManager::Status::Connected)
return;
// 使用 QSet 提高查找效率
QSet<QString> activeConnectionPaths;
NetworkManager::ActiveConnection::List allActiveConnections = NetworkManager::activeConnections();
for (const NetworkManager::ActiveConnection::Ptr &activeConnection : allActiveConnections) {
if (activeConnection->connection().isNull())
continue;
if (activeConnection->state() == NetworkManager::ActiveConnection::State::Activated
|| activeConnection->state() == NetworkManager::ActiveConnection::State::Activating) {
activeConnectionPaths.insert(activeConnection->connection()->path());
}
}
QMap<QString, NetworkManager::Connection::Ptr> candidateVpns;
NetworkManager::Connection::List allConnections = NetworkManager::listConnections();
for (const NetworkManager::Connection::Ptr &conn : allConnections) {
// 过滤掉正在连接的或者已经连接成功的
if (activeConnectionPaths.contains(conn->path()))
continue;
// 这里只查找设置了自动连接的
if (!conn->settings()->autoconnect())
continue;
// 查找VPN的连接方式
NetworkManager::ConnectionSettings::ConnectionType type = conn->settings()->connectionType();
if (type == NetworkManager::ConnectionSettings::ConnectionType::Vpn) {
NetworkManager::VpnSetting::Ptr setting = conn->settings()->setting(NetworkManager::Setting::SettingType::Vpn).dynamicCast<NetworkManager::VpnSetting>();
if (setting.isNull())
continue;
QString serviceType = setting->serviceType();
if (candidateVpns.contains(serviceType)) {
// 优先使用最后一次连接成功的连接
const NetworkManager::Connection::Ptr &lastSetting = candidateVpns[serviceType];
if (lastSetting->settings()->timestamp() < conn->settings()->timestamp())
candidateVpns[serviceType] = conn;
} else {
candidateVpns[serviceType] = conn;
}
}
}
for (auto it = candidateVpns.begin(); it != candidateVpns.end(); ++it) {
const NetworkManager::Connection::Ptr &connection = it.value();
// 再次检查当前网络状态，防止在网络快速切换时发起无效连接
if (NetworkManager::status() != NetworkManager::Status::Connected) {
qWarning() << "Network status changed, aborting VPN connection attempt for:" << connection->name();
break;
}
QDBusPendingReply<QDBusObjectPath> reply = NetworkManager::activateConnection(connection->path(), "/", "/");
QDBusPendingCallWatcher *watcher = new QDBusPendingCallWatcher(reply, this);
QString connectionName = connection->name();
connect(watcher, &QDBusPendingCallWatcher::finished, watcher, &QDBusPendingCallWatcher::deleteLater);
connect(watcher, &QDBusPendingCallWatcher::finished, this, [this, connectionName](QDBusPendingCallWatcher *watcher) {
QDBusPendingReply<QDBusObjectPath> reply = *watcher;
if (reply.isError()) {
qWarning() << "Failed to connect VPN:" << connectionName << "Reason:" << reply.error().message();
} else {
qDebug() << "VPN connection initiated successfully:" << connectionName;
}
});
}
}主要变更点：
|
caixr23
approved these changes
Mar 19, 2026
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: caixr23, ut003640 The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
Contributor
Author
|
/merge |
Contributor
|
This pr cannot be merged! (status: unstable) |
Contributor
Author
|
/forcemerge |
Contributor
|
This pr force merged! (status: unstable) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
VPN auto-connection is not started by NetworkManager, but by external network services. This adds the VPN startup functionality.
Log: Fix VPN not auto-connecting
PMS: BUG-287121
fix: 增加监听网络状态后自动连接VPN
VPN自动连接并非NetworkManager启动，而是由外部网络服务启动，这里加上启动VPN的功能
Log: 修复VPN没有自动连接
Summary by Sourcery
Add automatic VPN activation when the network service detects a successful network connection.
New Features:
Enhancements: