Skip to content

feat: add item move support and secret agent improvements #465

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
caixr23 merged 1 commit into from
Dec 30, 2025
Merged

feat: add item move support and secret agent improvements #465

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions debian/deepin-service-plugin-network.install
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
/usr/share/deepin-service-manager/system/plugin-system-network.json
/usr/share/deepin-service-manager/user/plugin-session-network.json
/usr/share/dbus-1/system.d/org.deepin.service.SessionNetwork.conf
/usr/share/dbus-1/system.d/org.deepin.service.SystemNetwork.conf
/usr/share/dbus-1/system.d/*.conf
/usr/lib/*/deepin-service-manager/libnetwork-service.so
/usr/share/deepin-service-manager/network-service/translations
/usr/lib/deepin-daemon/dde-network-secret-dialog
Expand Down
3 changes: 3 additions & 0 deletions net-view/operation/netitem.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,9 @@ public Q_SLOTS:
void childAdded(const NetItem *child);
void childAboutToBeRemoved(const NetItem *parent, int pos);
void childRemoved(const NetItem *child);
// Move过程中，会有对应的Added、Removed，使用model时需要过虑掉
void childAboutToBeMoved(const NetItem *parent, int pos, const NetItem *newParent, int newPos);
void childMoved(const NetItem *child);
void childrenChanged();

protected:
Expand Down
19 changes: 10 additions & 9 deletions net-view/operation/netmanager.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -625,8 +625,8 @@ void NetManagerPrivate::onDataChanged(int dataType, const QString &id, const QVa
item->updateapMode(value.toBool());
} break;
case NetManagerThreadPrivate::AvailableConnectionsChanged: {
NetWirelessDeviceItemPrivate *item = NetItemPrivate::toItem<NetWirelessDeviceItemPrivate>(findItem(id));
if (item) {
NetWirelessDeviceItemPrivate *devItem = NetItemPrivate::toItem<NetWirelessDeviceItemPrivate>(findItem(id));
if (devItem) {
const QStringList &connList = value.toStringList();
NetItemPrivate *mine = findItem(id + ":Mine");
NetItemPrivate *other = findItem(id + ":Other");
Expand All @@ -638,24 +638,25 @@ void NetManagerPrivate::onDataChanged(int dataType, const QString &id, const QVa
if (connList.contains(wirelessItem->id())) {
wirelessItem->updatehasConnection(true);
if (wirelessItem->getParentPrivate() == other) {
other->removeChild(wirelessItem);
}
if (wirelessItem->getParentPrivate() != mine) {
if (!mine->getParent()) {
devItem->addChild(mine);
}
other->moveChild(wirelessItem, mine);
} else if (wirelessItem->getParentPrivate() != mine) {
mine->addChild(wirelessItem);
}
} else {
wirelessItem->updatehasConnection(false);
if (wirelessItem->getParentPrivate() == mine) {
mine->removeChild(wirelessItem);
}
if (wirelessItem->getParentPrivate() != other) {
mine->moveChild(wirelessItem, other);
} else if (wirelessItem->getParentPrivate() != other) {
other->addChild(wirelessItem);
}
}
}
}
if (!mine->getParent() && mine->getChildrenNumber() != 0) {
findItem(id)->addChild(mine);
devItem->addChild(mine);
} else if (mine->getParent() && mine->getChildrenNumber() == 0) {
mine->getParentPrivate()->removeChild(mine);
}
Expand Down
48 changes: 29 additions & 19 deletions net-view/operation/private/netitemprivate.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -105,16 +105,10 @@
return (childPos < getChildrenNumber() ? m_children[childPos] : nullptr);
}

int NetItemPrivate::getChildIndex(const NetItem *child) const

Check warning on line 108 in net-view/operation/private/netitemprivate.cpp

View workflow job for this annotation

GitHub Actions / cppcheck 

The function 'getChildIndex' is never used.
{
int index = 0;
for (auto it = m_children.cbegin(); it != m_children.cend(); it++, index++) {
if (*it == child) {
return index;
}
}

return -1;
auto it = std::find(m_children.begin(), m_children.end(), child);
return it == m_children.end() ? -1 : it - m_children.begin();
}

bool NetItemPrivate::addChild(NetItemPrivate *child, int index)
Expand All @@ -134,19 +128,35 @@
return true;
}

void NetItemPrivate::removeChild(NetItemPrivate *child)
bool NetItemPrivate::removeChild(NetItemPrivate *child)
{
int index = 0;
for (auto it = m_children.begin(); it != m_children.end(); it++, index++) {
if (*it == child->item()) {
Q_EMIT m_item->childAboutToBeRemoved(m_item, index);
m_children.erase(it);
child->m_parent = nullptr;
Q_EMIT m_item->childRemoved(child->item());
Q_EMIT m_item->childrenChanged();
break;
}
auto it = std::find(m_children.begin(), m_children.end(), child->item());
if (it == m_children.end()) {
return false;
}
Q_EMIT m_item->childAboutToBeRemoved(m_item, it - m_children.begin());
m_children.erase(it);
child->m_parent = nullptr;
Q_EMIT m_item->childRemoved(child->item());
Q_EMIT m_item->childrenChanged();
return true;
}

bool NetItemPrivate::moveChild(NetItemPrivate *child, NetItemPrivate *newParent)

Check warning on line 145 in net-view/operation/private/netitemprivate.cpp

View workflow job for this annotation

GitHub Actions / cppcheck 

The function 'moveChild' is never used.
{
if (!child || !newParent || child->m_parent == newParent->item()) {
return false;
}
auto it = std::find(m_children.begin(), m_children.end(), child->item());
if (it == m_children.end()) {
return false;
}

Q_EMIT m_item->childAboutToBeMoved(m_item, it - m_children.begin(), newParent->item(), newParent->getChildrenNumber());
removeChild(child);
newParent->addChild(child);
Comment on lines +155 to +157
Copy link

Copilot AI Dec 29, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The moveChild function calls removeChild(child) and newParent->addChild(child) which will emit their own add/remove signals. This means that in addition to the move signals (childAboutToBeMoved and childMoved), the old parent will also emit childAboutToBeRemoved and childRemoved, while the new parent will emit childAboutToBeAdded and childAdded. According to the comment in netitem.h line 66, these add/remove signals should be filtered out during moves, but this design is error-prone and could lead to incorrect model updates if not handled carefully in all consumers.

Suggested change
Q_EMIT m_item->childAboutToBeMoved(m_item, it - m_children.begin(), newParent->item(), newParent->getChildrenNumber());
removeChild(child);
newParent->addChild(child);
const int oldIndex = static_cast<int>(it - m_children.begin());
const int newIndex = newParent->getChildrenNumber();
Q_EMIT m_item->childAboutToBeMoved(m_item, oldIndex, newParent->item(), newIndex);
// Perform the move without emitting add/remove signals.
m_children.erase(it);
child->m_parent = newParent->item();
newParent->m_children.push_back(child->item());
// Notify both parents that their children collections changed.
Q_EMIT m_item->childrenChanged();
Q_EMIT newParent->item()->childrenChanged();

Copilot uses AI. Check for mistakes.
Q_EMIT m_item->childMoved(child->item());
return true;
}

// UPDATEFUN(NetItem, const QString &, name)
Expand Down
3 changes: 2 additions & 1 deletion net-view/operation/private/netitemprivate.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,8 @@ class NetItemPrivate
public:
virtual ~NetItemPrivate();
virtual bool addChild(NetItemPrivate *child, int index = -1);
void removeChild(NetItemPrivate *child);
bool removeChild(NetItemPrivate *child);
bool moveChild(NetItemPrivate *child, NetItemPrivate *newParent);
void updatename(const QString &name);
void updateid(const QString &id);

Expand Down
8 changes: 4 additions & 4 deletions net-view/operation/private/netmanagerthreadprivate.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,11 @@
#include "netitem.h"
#include "netmanager.h"

#include <NetworkManagerQt/Device>

Check warning on line 10 in net-view/operation/private/netmanagerthreadprivate.h

View workflow job for this annotation

GitHub Actions / cppcheck 

Include file: <NetworkManagerQt/Device> not found. Please note: Cppcheck does not need standard library headers to get proper results.
#include <NetworkManagerQt/WirelessSecuritySetting>

Check warning on line 11 in net-view/operation/private/netmanagerthreadprivate.h

View workflow job for this annotation

GitHub Actions / cppcheck 

Include file: <NetworkManagerQt/WirelessSecuritySetting> not found. Please note: Cppcheck does not need standard library headers to get proper results.

#include <QObject>
#include <QMap>

Check warning on line 13 in net-view/operation/private/netmanagerthreadprivate.h

View workflow job for this annotation

GitHub Actions / cppcheck 

Include file: <QMap> not found. Please note: Cppcheck does not need standard library headers to get proper results.
#include <QObject>

Check warning on line 14 in net-view/operation/private/netmanagerthreadprivate.h

View workflow job for this annotation

GitHub Actions / cppcheck 

Include file: <QObject> not found. Please note: Cppcheck does not need standard library headers to get proper results.

class QTimer;

Expand Down Expand Up @@ -51,9 +51,9 @@
static QVariantMap CheckParamValid(const QVariantMap &param);
static bool CheckPasswordValid(const QString &key, const QString &password);

inline bool NetCheckAvailable() { return m_netCheckAvailable; }
inline bool NetCheckAvailable() const { return m_netCheckAvailable; }

inline bool AirplaneModeEnabled() { return m_airplaneModeEnabled; }
inline bool AirplaneModeEnabled() const { return m_airplaneModeEnabled; }

void setEnabled(bool enabled);
void setAutoScanInterval(int ms);
Expand Down Expand Up @@ -286,7 +286,7 @@
bool m_airplaneModeEnabled;
bool m_isSleeping;
QString m_serverKey;
QMap<NetworkDetails*, QString> m_detailsItemsMap; // 存储 NetworkDetails 指针到唯一ID的映射
QMap<NetworkDetails *, QString> m_detailsItemsMap; // 存储 NetworkDetails 指针到唯一ID的映射
QString m_showPageCmd;
QTimer *m_showPageTimer;
QString m_newVPNuuid;
Expand Down
37 changes: 33 additions & 4 deletions net-view/window/private/netmodel.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ namespace network {
NetModel::NetModel(QObject *parent)
: QAbstractItemModel(parent)
, m_treeRoot(nullptr)
, m_moving(false)
{
}

Expand Down Expand Up @@ -147,10 +148,12 @@ void NetModel::connectObject(const NetItem *obj)
const NetItem *o = objs.takeFirst();

connect(o, &NetItem::dataChanged, this, &NetModel::updateObject);
connect(o, &NetItem::childAboutToBeAdded, this, &NetModel::AboutToAddObject);
connect(o, &NetItem::childAboutToBeAdded, this, &NetModel::aboutToAddObject);
connect(o, &NetItem::childAdded, this, &NetModel::addObject);
connect(o, &NetItem::childAboutToBeRemoved, this, &NetModel::AboutToRemoveObject);
connect(o, &NetItem::childAboutToBeRemoved, this, &NetModel::aboutToRemoveObject);
connect(o, &NetItem::childRemoved, this, &NetModel::removeObject);
connect(o, &NetItem::childAboutToBeMoved, this, &NetModel::aboutToBeMoveObject);
connect(o, &NetItem::childMoved, this, &NetModel::moveObject);
int i = o->getChildrenNumber();
while (i--) {
objs.append(o->getChild(i));
Expand Down Expand Up @@ -181,29 +184,55 @@ void NetModel::updateObject()
}
}

void NetModel::AboutToAddObject(const NetItem *parent, int pos)
void NetModel::aboutToAddObject(const NetItem *parent, int pos)
{
if (m_moving) {
return;
}
QModelIndex i = index(parent);
beginInsertRows(i, pos, pos);
}

void NetModel::addObject(const NetItem *child)
{
if (m_moving) {
return;
}
endInsertRows();
connectObject(child);
}

void NetModel::AboutToRemoveObject(const NetItem *parent, int pos)
void NetModel::aboutToRemoveObject(const NetItem *parent, int pos)
{
if (m_moving) {
return;
}
QModelIndex i = index(parent);
beginRemoveRows(i, pos, pos);
}

void NetModel::removeObject(const NetItem *child)
{
if (m_moving) {
return;
}
endRemoveRows();
disconnectObject(child);
}

void NetModel::aboutToBeMoveObject(const NetItem *parent, int pos, const NetItem *newParent, int newPos)
{
m_moving = true;
QModelIndex i = index(parent);
QModelIndex newI = index(newParent);
beginMoveRows(i, pos, pos, newI, newPos);
}

void NetModel::moveObject(const NetItem *child)
{
endMoveRows();
m_moving = false;
}

} // namespace network
} // namespace dde
9 changes: 6 additions & 3 deletions net-view/window/private/netmodel.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,15 +40,18 @@ class NetModel : public QAbstractItemModel
void connectObject(const NetItem *obj);
void disconnectObject(const NetItem *obj);

public Q_SLOTS:
protected Q_SLOTS:
void updateObject();
void AboutToAddObject(const NetItem *parent, int pos);
void aboutToAddObject(const NetItem *parent, int pos);
void addObject(const NetItem *child);
void AboutToRemoveObject(const NetItem *parent, int pos);
void aboutToRemoveObject(const NetItem *parent, int pos);
void removeObject(const NetItem *child);
void aboutToBeMoveObject(const NetItem *parent, int pos, const NetItem *newParent, int newPos);
void moveObject(const NetItem *child);

private:
NetItem *m_treeRoot;
bool m_moving;
};

} // namespace network
Expand Down
3 changes: 1 addition & 2 deletions network-service-plugin/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,8 +76,7 @@ else()
endif()

install(FILES ${CMAKE_CURRENT_SOURCE_DIR}/src/system/deepin.dde.daemon.conf DESTINATION /etc/NetworkManager/conf.d/)
install(FILES ${CMAKE_CURRENT_SOURCE_DIR}/src/system/org.deepin.service.SystemNetwork.conf DESTINATION share/dbus-1/system.d/)
install(FILES ${CMAKE_CURRENT_SOURCE_DIR}/src/session/org.deepin.service.SessionNetwork.conf DESTINATION share/dbus-1/system.d/)
install(FILES ${CMAKE_CURRENT_SOURCE_DIR}/src/system/org.deepin.dde.Network1.conf DESTINATION share/dbus-1/system.d/)

# dde-network-secret-dialog属于network-service-plugin项目，翻译文件放一起
file(GLOB_RECURSE ALL_SRCS "*.h" "*.cpp")
Expand Down
64 changes: 47 additions & 17 deletions network-service-plugin/src/session/networksecretagent.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,11 @@ NetworkSecretAgent::NetworkSecretAgent(QObject *parent)
: NetworkManager::SecretAgent(QStringLiteral("com.deepin.system.network.SecretAgent"), parent)
, m_callNextId(0)
, m_secretService(new SecretService(this))
, m_waitClientTimer(new QTimer(this))
{
m_waitClientTimer->setSingleShot(true);
m_waitClientTimer->setInterval(5000);
connect(m_waitClientTimer, &QTimer::timeout, this, &NetworkSecretAgent::waitClientTimeOut);
m_server = new QLocalServer(this);
connect(m_server, &QLocalServer::newConnection, this, &NetworkSecretAgent::newConnectionHandler);
m_server->setSocketOptions(QLocalServer::WorldAccessOption);
Expand Down Expand Up @@ -203,24 +207,12 @@ void NetworkSecretAgent::askPasswords(SecretsRequest &request, const QStringList
QString reqJSON = QJsonDocument(req).toJson(QJsonDocument::Compact);
qCDebug(DSM()) << "reqJSON:" << reqJSON;
request.inputCache = reqJSON.toUtf8();
// 无线网密码拉起任务栏网络面板，其他使用密码输入弹窗
if (connType == "802-11-wireless" && !m_clients.isEmpty()) {
for (auto &&client : m_clients) {
client->write("\nrequestSecrets:" + reqJSON.toUtf8() + "\n");
}
if (connType == "802-11-wireless" && m_clients.isEmpty()) {
// 启动定时器，等待
request.status = SecretsRequest::WaitClient;
m_waitClientTimer->start();
Comment on lines +210 to +213
Copy link

@sourcery-ai sourcery-ai bot Dec 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggestion (bug_risk): A single shared wait timer for all wireless requests can lead to surprising behavior when multiple requests are pending

Using a single m_waitClientTimer for all wireless SecretsRequests means a new request can reset the timer while a previous one is still in WaitClient. Since waitClientTimeOut advances only one request at a time, earlier requests can be delayed or starved. Consider either tracking wait state per SecretsRequest (own timer or timestamp) or enforcing that only one wireless request may be in WaitClient at a time and rejecting/queuing additional ones accordingly.

Suggested implementation:

    request.inputCache = reqJSON.toUtf8();
    // For wireless connections, only allow one request to be in WaitClient state at a time.
    // If m_waitClientTimer is already active, we skip the wait and immediately fall back
    // to the normal handling path to avoid starving earlier pending requests.
    if (connType == "802-11-wireless" && m_clients.isEmpty() && !m_waitClientTimer->isActive()) {
        // 启动定时器，等待
        request.status = SecretsRequest::WaitClient;
        m_waitClientTimer->start();
    } else {

If you prefer true queuing (i.e. multiple wireless requests all going through a client-wait phase in order), you will need to:

  1. Track pending SecretsRequests in a queue structure instead of relying solely on a single timer.
  2. Adjust the waitClientTimeOut handler (or equivalent) to pop the next queued wireless request and move it out of WaitClient state, starting the timer again if more remain.
  3. Optionally, add a helper such as bool NetworkSecretAgent::hasWaitingWirelessRequest() const and base the condition on that instead of m_waitClientTimer->isActive().
    The current change implements the "only one wireless request may be in WaitClient at a time" policy with minimal impact: additional wireless requests will follow the existing non-wait path while a wait is already in progress.

Copy link

Copilot AI Dec 29, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The single timer m_waitClientTimer is shared for all wireless secret requests. If multiple wireless secret requests arrive while waiting for clients, calling start() on line 213 for a subsequent request will restart the timer, potentially causing the first request to wait longer than 5 seconds or the wrong request to be processed when waitClientTimeOut() is called. Consider using per-request timers or tracking multiple waiting requests properly.

Suggested change
m_waitClientTimer->start();
if (!m_waitClientTimer->isActive()) {
m_waitClientTimer->start();
}

Copilot uses AI. Check for mistakes.
} else {
// run auth dialog
qCInfo(DSM()) << "run auth dialog:" << NMSecretDialogBin;
QProcess *process = new QProcess(this);
process->setProperty("callId", request.callId);
request.process = process;
connect(process, &QProcess::finished, this, &NetworkSecretAgent::authDialogFinished);
connect(process, &QProcess::readyReadStandardOutput, this, &NetworkSecretAgent::authDialogReadOutput);
connect(process, &QProcess::readyReadStandardError, this, &NetworkSecretAgent::authDialogReadError);
connect(process, &QProcess::errorOccurred, this, &NetworkSecretAgent::authDialogError);
connect(process, &QProcess::started, this, &NetworkSecretAgent::authDialogStarted);
QTimer::singleShot(GET_SECRETS_TIMEOUT, process, &QProcess::kill);
process->start(NMSecretDialogBin);
runAuthDialog(request);
}
}

Expand All @@ -231,6 +223,8 @@ void NetworkSecretAgent::newConnectionHandler()
connect(socket, &QLocalSocket::disconnected, this, &NetworkSecretAgent::disconnectedHandler);
QTimer::singleShot(GET_SECRETS_TIMEOUT, socket, &QLocalSocket::disconnectFromServer);
m_clients.append(socket);
m_waitClientTimer->stop();
waitClientTimeOut();
}

void NetworkSecretAgent::disconnectedHandler()
Expand Down Expand Up @@ -537,6 +531,42 @@ void NetworkSecretAgent::doSecretsResult(QString callId, const QByteArray &data,
m_calls.removeAll(*request);
}

void NetworkSecretAgent::runAuthDialog(SecretsRequest &request)
{
const QString &connType = request.connection.value("connection").value("type").toString();
request.status = SecretsRequest::WaitDialog;
// 无线网密码拉起任务栏网络面板，其他使用密码输入弹窗
if (connType == "802-11-wireless" && !m_clients.isEmpty()) {
for (auto &&client : m_clients) {
client->write("\nrequestSecrets:" + request.inputCache + "\n");
}
} else {
// run auth dialog
qCInfo(DSM()) << "run auth dialog:" << NMSecretDialogBin;
QProcess *process = new QProcess(this);
process->setProperty("callId", request.callId);
request.process = process;
connect(process, &QProcess::finished, this, &NetworkSecretAgent::authDialogFinished);
connect(process, &QProcess::readyReadStandardOutput, this, &NetworkSecretAgent::authDialogReadOutput);
connect(process, &QProcess::readyReadStandardError, this, &NetworkSecretAgent::authDialogReadError);
connect(process, &QProcess::errorOccurred, this, &NetworkSecretAgent::authDialogError);
connect(process, &QProcess::started, this, &NetworkSecretAgent::authDialogStarted);
QTimer::singleShot(GET_SECRETS_TIMEOUT, process, &QProcess::kill);
process->start(NMSecretDialogBin);
}
}

void NetworkSecretAgent::waitClientTimeOut()
{
auto it = std::find_if(m_calls.begin(), m_calls.end(), [](const SecretsRequest &req) {
return req.status == SecretsRequest::WaitClient;
});

if (it != m_calls.end()) {
runAuthDialog(*it);
}
}

QString NetworkSecretAgent::nextId()
{
return QString::number(0xFFFFFFFF & m_callNextId++, 16);
Expand Down
Loading