fix: fix the language of the first connection which create#544
Merged
deepin-bot[bot] merged 1 commit intolinuxdeepin:masterfrom Apr 9, 2026
Merged
fix: fix the language of the first connection which create#544deepin-bot[bot] merged 1 commit intolinuxdeepin:masterfrom
deepin-bot[bot] merged 1 commit intolinuxdeepin:masterfrom
Conversation
fix the language of the first connection which create PMS: BUG-354915
Reviewer's GuideRefactors network initialization to better integrate with Deepin Accounts and LockService, ensures wired connections are created with the correct user or system language (including delayed translation updates), simplifies device handling, and cleans up connection management for wired and wireless devices. Sequence diagram for wired connection creation and language updatesequenceDiagram
actor User
participant SystemContainer
participant NetworkInitialization
participant AccountsService as com.deepin.daemon.Accounts
participant NetworkManager as org.freedesktop.NetworkManager
SystemContainer->>NetworkInitialization: constructor(parent)
activate NetworkInitialization
NetworkInitialization->>NetworkInitialization: initDeviceInfo()
NetworkInitialization->>NetworkInitialization: initConnection()
NetworkInitialization->>AccountsService: checkAccountStatus() via accountInterface(UserList/Locale)
AccountsService-->>NetworkInitialization: locale or error
alt locale found
NetworkInitialization->>NetworkInitialization: installUserTranslator(json)
NetworkInitialization->>NetworkInitialization: installLaunguage(locale)
NetworkInitialization->>NetworkInitialization: m_initilized = true
else no locale
NetworkInitialization->>NetworkInitialization: m_initilized remains false
end
Note over NetworkInitialization,NetworkManager: After 3s QTimer
NetworkInitialization->>NetworkInitialization: addFirstConnection()
NetworkInitialization->>NetworkManager: networkInterfaces()
NetworkManager-->>NetworkInitialization: List<Device>
loop each Ethernet device
NetworkInitialization->>NetworkInitialization: hasConnection(device, unSaveConnections)
alt device unmanaged or down or no carrier
NetworkInitialization->>NetworkManager: remove unsaved connections
NetworkManager-->>NetworkInitialization: removed
else no existing saved connection
NetworkInitialization->>NetworkInitialization: connectionMatchName(device)
alt server system
NetworkInitialization->>NetworkInitialization: use interfaceName
else desktop system
NetworkInitialization->>NetworkInitialization: generate tr("Wired Connection"[ N])
end
NetworkInitialization->>NetworkManager: addConnection(connMap)
NetworkManager-->>NetworkInitialization: new connection path and uuid
alt m_initilized is false and suffix >= 0
NetworkInitialization->>NetworkInitialization: cache uuid -> suffix in m_untranslactionConnections
end
NetworkInitialization->>NetworkInitialization: m_newConnectionNames[path] = name
end
end
deactivate NetworkInitialization
Note over User,NetworkInitialization: Later when user login or created
AccountsService-->>NetworkInitialization: UserAdded(userPath) signal
activate NetworkInitialization
NetworkInitialization->>NetworkInitialization: onUserAdded(userPath)
NetworkInitialization->>NetworkInitialization: updateConnectionLaunguage(userPath)
NetworkInitialization->>AccountsService: accountInterface(userPath, Locale)
AccountsService-->>NetworkInitialization: locale
NetworkInitialization->>NetworkInitialization: installLaunguage(locale)
NetworkInitialization->>NetworkInitialization: m_initilized = true
NetworkInitialization->>NetworkInitialization: updateConnectionLaunguage()
loop for each cached uuid in m_untranslactionConnections
NetworkInitialization->>NetworkManager: findConnectionByUuid(uuid)
NetworkManager-->>NetworkInitialization: Connection
NetworkInitialization->>NetworkManager: update or updateUnsaved(new localized id)
NetworkManager-->>NetworkInitialization: ack
NetworkInitialization->>NetworkInitialization: m_newConnectionNames[path] = localizedName
end
NetworkInitialization->>NetworkInitialization: clear m_untranslactionConnections
deactivate NetworkInitialization
Class diagram for updated network initialization and system containerclassDiagram
namespace systemservice {
class SystemContainer {
-SystemIPConflict* m_ipConflictHandler
-ConnectivityProcesser* m_connectivityHelper
-NetworkInitialization* m_initializator
+SystemContainer(QObject* parent)
+~SystemContainer()
}
class NetworkInitialization {
-QMap~QString, QString~ m_newConnectionNames
-bool m_initilized
-bool m_accountServiceRegister
-bool m_hasAddFirstConnection
-QMap~QString, QDateTime~ m_lastCreateTime
-QMap~QString, int~ m_untranslactionConnections
+NetworkInitialization(QObject* parent)
+~NetworkInitialization()
+void updateLanguage(QString locale)
-void initDeviceInfo()
-void initConnection()
-void addFirstConnection()
-void addFirstConnection(NetworkManager::WiredDevice* device)
-bool hasConnection(NetworkManager::WiredDevice* device, QList~QSharedPointer~NetworkManager::Connection~~ & unSaveDevices)
-QPair~int, QString~ connectionMatchName(NetworkManager::WiredDevice* device) const
-QVariant accountInterface(QString path, QString key, bool isUser) const
-bool installUserTranslator(QString json)
-void installLaunguage(QString locale)
-void hideWirelessDevice(QSharedPointer~NetworkManager::Device~ device, bool disableNetwork)
-void initDeviceConnection(QSharedPointer~NetworkManager::WiredDevice~ device)
-void checkAccountStatus()
-void updateConnectionLaunguage(QString account)
-void updateConnectionLaunguage()
-bool isServerSystem() const
<<slots>>
-void onUserChanged(QString json)
-void onUserAdded(QString json)
-void onInitDeviceConnection()
-void onWiredDevicePropertyChanged()
-void onDeviceAdded(QString uni)
-void onAvailableConnectionDisappeared(QString connectionUni)
}
}
SystemContainer --> NetworkInitialization : owns
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 2 issues, and left some high level feedback:
- In onUserAdded() you now call updateConnectionLaunguage(user) before installUserTranslator(user), but the signal argument may still be a JSON payload rather than a D-Bus path; consider reusing installUserTranslator’s logic (or its parsed path/locale) so that updateConnectionLaunguage() is only invoked with a verified account object path to avoid invalid D-Bus property lookups and loading an empty-locale translator.
- hideWirelessDevice() now connects a lambda to managedChanged without Qt::UniqueConnection, which can lead to multiple duplicate connections if hideWirelessDevice() is called repeatedly for the same device; it would be safer to add Qt::UniqueConnection or guard against reconnecting.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In onUserAdded() you now call updateConnectionLaunguage(user) before installUserTranslator(user), but the signal argument may still be a JSON payload rather than a D-Bus path; consider reusing installUserTranslator’s logic (or its parsed path/locale) so that updateConnectionLaunguage() is only invoked with a verified account object path to avoid invalid D-Bus property lookups and loading an empty-locale translator.
- hideWirelessDevice() now connects a lambda to managedChanged without Qt::UniqueConnection, which can lead to multiple duplicate connections if hideWirelessDevice() is called repeatedly for the same device; it would be safer to add Qt::UniqueConnection or guard against reconnecting.
## Individual Comments
### Comment 1
<location path="network-service-plugin/src/system/networkinitialization.cpp" line_range="367-374" />
<code_context>
+void NetworkInitialization::installLaunguage(const QString &locale)
</code_context>
<issue_to_address>
**suggestion (bug_risk):** installLaunguage ignores the result of translator.load(), reducing error visibility compared to the previous implementation.
Previously, translator.load(...) was checked and both success and failure (with locale) were logged. Now installLaunguage() always installs the translator and only logs the qm path, even if load() failed (e.g., missing file or wrong locale). Please restore a check on the boolean result of load() and log a warning on failure, as before.
Suggested implementation:
```cpp
void NetworkInitialization::installLaunguage(const QString &locale)
{
// Build the QM file path for the requested locale using the same logic
// that was previously used at the call site.
const QString qmPath = /* existing logic that was previously used to compute the qmPath from locale */;
const bool loaded = m_translator.load(qmPath);
if (loaded) {
qApp->installTranslator(&m_translator);
qInfo() << "Loaded translation for locale" << locale << "from" << qmPath;
} else {
qWarning() << "Failed to load translation for locale" << locale << "from" << qmPath;
}
}
```
To fully implement the requested behavior without reducing error visibility, you should:
1. Replace the `qmPath` construction comment with the exact logic that was previously inlined at the call site (before the refactor to `installLaunguage`). This ensures the same file path and locale handling as before.
2. Replace `m_translator` with the actual `QTranslator` instance used in this class/file (for example, if it was previously a local `QTranslator translator;`, either:
- make it a member `QTranslator m_translator;` and reuse it here, or
- keep it local inside this function and adjust the `qApp->installTranslator(...)` call accordingly).
3. If the original code logged more detailed information (e.g. separate `qInfo` on success and `qWarning` including `locale` and the full path on failure), mirror that exact logging format here.
4. Make sure `qApp` (or `QCoreApplication::instance()`) is available in this translation unit by including the appropriate Qt headers if they were not already included.
</issue_to_address>
### Comment 2
<location path="network-service-plugin/src/system/networkinitialization.cpp" line_range="388-386" />
<code_context>
dbusInter.setProperty("Managed", managed);
}
- connect(device.data(), &NetworkManager::Device::managedChanged, this, &NetworkInitialization::onManagedChanged, Qt::UniqueConnection);
+ NetworkManager::Device *devicePtr = device.data();
+ connect(devicePtr, &NetworkManager::Device::managedChanged, this, [ devicePtr, managed ] {
+ if (devicePtr->managed() == managed)
+ return ;
+
+ qCDebug(DSM) << "device" << devicePtr->interfaceName() << "managed changed" << devicePtr->managed() << ", will set it managed" << managed;
+ QDBusInterface dbusInter("org.freedesktop.NetworkManager", devicePtr->uni(), "org.freedesktop.NetworkManager.Device", QDBusConnection::systemBus());
+ dbusInter.setProperty("Managed", managed);
+ });
}
</code_context>
<issue_to_address>
**nitpick (bug_risk):** Using a lambda without Qt::UniqueConnection in hideWirelessDevice may attach duplicate slots on repeated calls.
The previous connection used Qt::UniqueConnection to avoid duplicate connections; the new lambda connection does not. If hideWirelessDevice is called repeatedly for the same device, this lambda will be connected multiple times and will run on every managedChanged emission, causing redundant DBus calls and more complex signal/slot state. Please add Qt::UniqueConnection or otherwise ensure the connection is only created once per device.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
+367
to
+374
| void NetworkInitialization::installLaunguage(const QString &locale) | ||
| { | ||
| QFile file(filePath); | ||
| if (!file.open(QFile::ReadOnly | QFile::Text)) { | ||
| qCWarning(DSM) << "Failed to open locale file:" << filePath << "-" << file.errorString(); | ||
| return QString(); | ||
| } | ||
|
|
||
| QMap<QString, QString> localeMap; | ||
| QTextStream in(&file); | ||
| while (!in.atEnd()) { | ||
| QString line = in.readLine(); | ||
| if (!keywords.isEmpty() && !line.contains(keywords)) { | ||
| continue; | ||
| } | ||
| QStringList pair = line.split(splitKey); | ||
| if (pair.size() == 2) { | ||
| localeMap.insert(pair.at(0).trimmed(), pair.at(1).trimmed()); | ||
| } | ||
| } | ||
| for (const auto &key : keys) { | ||
| if (localeMap.contains(key)) | ||
| return localeMap.value(key).split('.').first(); | ||
| } | ||
| return QString(); | ||
| } | ||
|
|
||
| bool NetworkInitialization::installSystemTranslator() | ||
| { | ||
| QString locale = getLocaleValue("/etc/locale.conf", { "LANGUAGE", "LANG" }, "=", "LANG"); | ||
| if (locale.isEmpty()) { | ||
| locale = getLocaleValue("/etc/deepin-installer/deepin-installer.conf", { "DI_LOCALE", "LIVE_LOCALES" }, "=", "LOCALE"); | ||
| } | ||
| if (!locale.isEmpty()) { | ||
| qCInfo(DSM) << "Install system language:" << locale; | ||
| return installTranslator(locale); | ||
| } | ||
| return false; | ||
| static QTranslator translator; | ||
| QCoreApplication::removeTranslator(&translator); | ||
| const QString qmFile = QString("%1/network-service-plugin_%2.qm").arg(QM_FILES_DIR).arg(locale); | ||
| translator.load(qmFile); | ||
| QCoreApplication::installTranslator(&translator); | ||
| qCDebug(DSM) << "install translation file" << qmFile; |
There was a problem hiding this comment.
suggestion (bug_risk): installLaunguage ignores the result of translator.load(), reducing error visibility compared to the previous implementation.
Previously, translator.load(...) was checked and both success and failure (with locale) were logged. Now installLaunguage() always installs the translator and only logs the qm path, even if load() failed (e.g., missing file or wrong locale). Please restore a check on the boolean result of load() and log a warning on failure, as before.
Suggested implementation:
void NetworkInitialization::installLaunguage(const QString &locale)
{
// Build the QM file path for the requested locale using the same logic
// that was previously used at the call site.
const QString qmPath = /* existing logic that was previously used to compute the qmPath from locale */;
const bool loaded = m_translator.load(qmPath);
if (loaded) {
qApp->installTranslator(&m_translator);
qInfo() << "Loaded translation for locale" << locale << "from" << qmPath;
} else {
qWarning() << "Failed to load translation for locale" << locale << "from" << qmPath;
}
}
To fully implement the requested behavior without reducing error visibility, you should:
- Replace the
qmPathconstruction comment with the exact logic that was previously inlined at the call site (before the refactor toinstallLaunguage). This ensures the same file path and locale handling as before. - Replace
m_translatorwith the actualQTranslatorinstance used in this class/file (for example, if it was previously a localQTranslator translator;, either:- make it a member
QTranslator m_translator;and reuse it here, or - keep it local inside this function and adjust the
qApp->installTranslator(...)call accordingly).
- make it a member
- If the original code logged more detailed information (e.g. separate
qInfoon success andqWarningincludinglocaleand the full path on failure), mirror that exact logging format here. - Make sure
qApp(orQCoreApplication::instance()) is available in this translation unit by including the appropriate Qt headers if they were not already included.
| @@ -450,14 +385,25 @@ void NetworkInitialization::hideWirelessDevice(const QSharedPointer<NetworkManag | |||
| QDBusInterface dbusInter("org.freedesktop.NetworkManager", device->uni(), "org.freedesktop.NetworkManager.Device", QDBusConnection::systemBus()); | |||
| dbusInter.setProperty("Managed", managed); | |||
There was a problem hiding this comment.
nitpick (bug_risk): Using a lambda without Qt::UniqueConnection in hideWirelessDevice may attach duplicate slots on repeated calls.
The previous connection used Qt::UniqueConnection to avoid duplicate connections; the new lambda connection does not. If hideWirelessDevice is called repeatedly for the same device, this lambda will be connected multiple times and will run on every managedChanged emission, causing redundant DBus calls and more complex signal/slot state. Please add Qt::UniqueConnection or otherwise ensure the connection is only created once per device.
caixr23
approved these changes
Apr 9, 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
|
/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.
fix the language of the first connection which create
PMS: BUG-354915
Summary by Sourcery
Improve initialization and naming of wired network connections to respect the correct user/system language and updated D-Bus services.
Bug Fixes:
Enhancements: