Refactor #3296
Merged
Refactor #3296
+584
−452
Conversation
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
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: Johnson-zs The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
Warning
详情 {
"export": {
"debian/rules": {
"a": [
"export QT_SELECT=5"
],
"b": [
"export QT_SELECT=6"
]
}
}
} |
Reviewer's GuideThis PR introduces a newly centralized SystemServiceManager to standardize systemd service operations, refactors UserShareHelper and SMB browser utilities to replace ad-hoc D-Bus/QProcess calls with this manager, removes legacy QGSettings modules and conditional code from desktop plugins, and updates tests to align with the refactored service management. Sequence diagram for service enablement via SystemServiceManagersequenceDiagram
participant Plugin as "Plugin (UserShareHelper/SmbBrowserUtils)"
participant SSM as "SystemServiceManager"
participant Systemd as "systemd D-Bus API"
Plugin->>SSM: enableServiceNow("smbd.service")
SSM->>Systemd: pkexec systemctl enable --now smbd.service
Systemd-->>SSM: Service enabled and started
SSM-->>Plugin: success/failure
Class diagram for the new SystemServiceManager and refactored service managementclassDiagram
class SystemServiceManager {
+static instance() : SystemServiceManager
+isServiceRunning(serviceName: QString) : bool
+startService(serviceName: QString) : bool
+enableServiceNow(serviceName: QString) : bool
-unitPathFromName(serviceName: QString) : QString
}
class UserShareHelper {
+isSambaServiceRunning() : bool
+startSmbService() : QPair<bool, QString>
}
class SmbBrowserUtils {
+isServiceRuning(service: QString) : bool
+enableServiceNow(service: QString) : bool
+checkAndEnableService(service: QString) : bool
}
UserShareHelper --> SystemServiceManager : uses
SmbBrowserUtils --> SystemServiceManager : uses
Class diagram for removed legacy QGSettings logic in desktop pluginsclassDiagram
class InnerDesktopAppFilter {
+refreshModel()
+acceptInsert(url: QUrl) : bool
+acceptReset(urls: QList<QUrl>) : QList<QUrl>
+acceptRename(oldUrl: QUrl, newUrl: QUrl) : bool
-keys: QMap<QString, QUrl>
-hidden: QMap<QString, bool>
}
class CanvasModelFilter {
+refreshModel()
+resetFilter(urls: QList<QUrl>) : bool
+insertFilter(url: QUrl) : bool
+renameFilter(oldUrl: QUrl, newUrl: QUrl) : bool
-keys: QMap<QString, QUrl>
-hidden: QMap<QString, bool>
}
InnerDesktopAppFilter --|> CanvasModelFilter : inherits
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 there - I've reviewed your changes - here's some feedback:
- Please add unit tests for SystemServiceManager to verify both successful and failure paths for isServiceRunning, startService, and enableServiceNow.
- The synchronous QProcess::execute call in enableServiceNow can block the calling thread; consider providing an asynchronous API or adding explicit timeouts to prevent UI stalls when pkexec prompts for authentication.
- Consider caching the D-Bus interfaces in SystemServiceManager (e.g., the Manager and Unit interfaces) instead of recreating them on each method call to reduce overhead and improve performance.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Please add unit tests for SystemServiceManager to verify both successful and failure paths for isServiceRunning, startService, and enableServiceNow.
- The synchronous QProcess::execute call in enableServiceNow can block the calling thread; consider providing an asynchronous API or adding explicit timeouts to prevent UI stalls when pkexec prompts for authentication.
- Consider caching the D-Bus interfaces in SystemServiceManager (e.g., the Manager and Unit interfaces) instead of recreating them on each method call to reduce overhead and improve performance.
## Individual Comments
### Comment 1
<location> `src/plugins/filemanager/dfmplugin-smbbrowser/utils/smbbrowserutils.cpp:92` </location>
<code_context>
- }
- return false;
+ // 使用 SystemServiceManager 检查服务状态
+ QString serviceName = QString("%1d.service").arg(service);
+ return dfmbase::SystemServiceManager::instance().isServiceRunning(serviceName);
}
</code_context>
<issue_to_address>
**issue (bug_risk):** Service name construction may be incorrect for 'smb' and 'nmb'.
If the input is 'smbd' or 'nmbd', this logic will generate 'smbdd.service' or 'nmbdd.service', which is likely incorrect. Please ensure the input is normalized to prevent this issue.
</issue_to_address>
### Comment 2
<location> `src/dfm-base/utils/systemservicemanager.cpp:132-143` </location>
<code_context>
+ // 分析执行结果
+ bool success = (exitCode == 0);
+
+ if (success) {
+ qCInfo(logDFMBase) << "SystemServiceManager: Successfully enabled service" << serviceName;
+ // 启用服务后，systemd 会自动重载，但显式调用 Reload 以确保状态同步
+ // 这一步也可以省略，因为 --now 已经启动了服务，enable 也已持久化
+ QDBusInterface managerIface(
+ kSystemdService,
+ kSystemdManagerPath,
+ kSystemdManagerInterface,
+ QDBusConnection::systemBus());
+ if (managerIface.isValid()) {
+ managerIface.call("Reload");
+ }
</code_context>
<issue_to_address>
**suggestion:** Reloading systemd after enabling service may be redundant.
Explicitly calling Reload after 'systemctl enable --now' is usually unnecessary, as systemd reloads units automatically. Removing this call can help avoid minor delays unless a forced reload is required for a specific reason.
```suggestion
if (success) {
qCInfo(logDFMBase) << "SystemServiceManager: Successfully enabled service" << serviceName;
// systemd 会自动重载，无需显式调用 Reload
```
</issue_to_address>
### Comment 3
<location> `src/plugins/filemanager/dfmplugin-smbbrowser/utils/smbbrowserutils.cpp:98` </location>
<code_context>
-bool startService(const QString &service)
+bool enableServiceNow(const QString &service)
{
if (service.isEmpty() || (service != "smb" && service != "nmb")) {
- fmWarning() << "Invalid service name for start operation:" << service;
+ fmWarning() << "Invalid service name for enable operation:" << service;
</code_context>
<issue_to_address>
**suggestion:** Service name validation is strict and may limit extensibility.
Currently, only 'smb' and 'nmb' are allowed. Making this validation configurable would simplify future service additions.
</issue_to_address>
### Comment 4
<location> `src/plugins/common/dfmplugin-dirshare/utils/usersharehelper.cpp:620-626` </location>
<code_context>
- return { true, "" };
+ QString errMsg;
+ // 使用 SystemServiceManager 统一管理服务启动和启用
+ bool smbdOk = SystemServiceManager::instance().enableServiceNow("smbd.service");
+ if (!smbdOk) {
+ errMsg = "Failed to start and enable smbd.service";
+ return { false, errMsg };
+ }
+
+ bool nmbdOk = SystemServiceManager::instance().enableServiceNow("nmbd.service");
+ if (!smbdOk || !nmbdOk) {
+ if (!smbdOk) {
</code_context>
<issue_to_address>
**nitpick:** Redundant check for smbdOk after already returning on failure.
The second 'if (!smbdOk || !nmbdOk)' can be simplified to only check 'nmbdOk', since 'smbdOk' has already been handled.
</issue_to_address>
### Comment 5
<location> `autotests/plugins/dfmplugin-smbbrowser/test_smbbrowserutils.cpp:116-121` </location>
<code_context>
TEST_F(UT_SmbBrowserUtils, StartService)
{
- EXPECT_FALSE(smb_browser_utils::startService(""));
- EXPECT_FALSE(smb_browser_utils::startService("hello"));
- EXPECT_FALSE(smb_browser_utils::startService("xxx..."));
</code_context>
<issue_to_address>
**suggestion (testing):** Tests for startService and enableServiceAsync have been removed, but enableServiceNow is not directly tested.
Please add unit tests for enableServiceNow, covering valid, invalid, and edge-case service names, as well as error handling.
Suggested implementation:
```cpp
TEST_F(UT_SmbBrowserUtils, StartService)
{
#if QT_VERSION < QT_VERSION_CHECK(6, 0, 0)
// stub.set_lamda(&QDBusAbstractInterface::asyncCall, [] { __DBG_STUB_INVOKE__ return QDBusPendingCall::fromError(QDBusError()); });
typedef QDBusPendingCall (QDBusAbstractInterface::*AsyncCall)(const QString &method,
stub.set_lamda(&QDBusPendingCall::waitForFinished, [] { __DBG_STUB_INVOKE__ });
stub.set_lamda(&QDBusPendingCall::isValid, [] { __DBG_STUB_INVOKE__ return true; });
#endif
}
// Unit tests for enableServiceNow
TEST_F(UT_SmbBrowserUtils, EnableServiceNow)
{
// Valid service name
EXPECT_TRUE(smb_browser_utils::enableServiceNow("smb"));
// Invalid service names
EXPECT_FALSE(smb_browser_utils::enableServiceNow(""));
EXPECT_FALSE(smb_browser_utils::enableServiceNow("invalid_service"));
EXPECT_FALSE(smb_browser_utils::enableServiceNow("123!@#"));
// Edge-case: whitespace only
EXPECT_FALSE(smb_browser_utils::enableServiceNow(" "));
// Edge-case: very long service name
QString longServiceName(1024, 'a');
EXPECT_FALSE(smb_browser_utils::enableServiceNow(longServiceName));
// Simulate DBus error (stub should be set up to return error)
// This assumes you have a stub or mock for DBus error simulation
stub.set_lamda(&QDBusPendingCall::isValid, [] { __DBG_STUB_INVOKE__ return false; });
EXPECT_FALSE(smb_browser_utils::enableServiceNow("smb"));
// Reset stub for other tests
stub.set_lamda(&QDBusPendingCall::isValid, [] { __DBG_STUB_INVOKE__ return true; });
}
```
- If `smb_browser_utils::enableServiceNow` requires more setup or teardown, you may need to adjust the test fixture or add additional stubbing/mocking.
- If the stub object is not available in this scope, you will need to ensure it is accessible for simulating DBus errors.
- If the function signature or return type of `enableServiceNow` is different, adjust the test expectations accordingly.
</issue_to_address>
### Comment 6
<location> `autotests/plugins/dfmplugin-smbbrowser/test_smbbrowserutils.cpp:124-125` </location>
<code_context>
- bool startServiceResult = true;
- stub.set_lamda(smb_browser_utils::startService, [&] { __DBG_STUB_INVOKE__ return startServiceResult; });
- stub.set_lamda(smb_browser_utils::enableServiceAsync, [] { __DBG_STUB_INVOKE__ });
+ stub.set_lamda(smb_browser_utils::enableServiceNow, [] { return true; });
EXPECT_TRUE(smb_browser_utils::checkAndEnableService("smb"));
- startServiceResult = false;
</code_context>
<issue_to_address>
**suggestion (testing):** CheckAndEnableService test now stubs enableServiceNow, but does not verify error handling or failure cases.
Please add tests for failure cases, such as enableServiceNow returning false, and verify that error handling and messaging behave correctly.
Suggested implementation:
```cpp
serviceRunning = false;
stub.set_lamda(smb_browser_utils::enableServiceNow, [] { return true; });
EXPECT_TRUE(smb_browser_utils::checkAndEnableService("smb"));
// Test failure case: enableServiceNow returns false
stub.set_lamda(smb_browser_utils::enableServiceNow, [] { return false; });
EXPECT_FALSE(smb_browser_utils::checkAndEnableService("smb"));
EXPECT_FALSE(smb_browser_utils::checkAndEnableService("sb"));
}
```
If `checkAndEnableService` produces error messages or logs, you may need to add assertions to verify the correct error output. This will require access to the error reporting mechanism (e.g., a mock logger or error string).
</issue_to_address>
### Comment 7
<location> `src/dfm-base/utils/systemservicemanager.cpp:23` </location>
<code_context>
+static constexpr char kSystemdManagerInterface[] { "org.freedesktop.systemd1.Manager" };
+static constexpr char kSystemdUnitInterface[] { "org.freedesktop.systemd1.Unit" };
+
+SystemServiceManager &SystemServiceManager::instance()
+{
+ static SystemServiceManager ins;
</code_context>
<issue_to_address>
**issue (complexity):** Consider replacing the current class and D-Bus logic with two simple free functions that shell out to systemctl and pkexec for equivalent service management.
Here’s a much slimmer implementation that preserves exactly the same behavior but drops all of the low-level D-Bus plumbing and hard-to‐test abstractions. You can just shell out to `systemctl`/`pkexec` (or even `systemctl` alone if you’re already running as root):
```cpp
// Replace your entire SystemServiceManager.cpp/.h with two free functions:
bool isServiceRunning(const QString &svc) {
if (svc.isEmpty()) return false;
// returns 0 if active/running, >0 otherwise
return QProcess::execute(QStringLiteral("systemctl"),
{ QStringLiteral("is-active"),
QStringLiteral("--quiet"),
svc }) == 0;
}
bool enableServiceNow(const QString &svc) {
if (svc.isEmpty()) return false;
// pkexec will pop up PolicyKit; change to "systemctl" only if you already have privileges
QString prog = QStringLiteral("pkexec");
QStringList args = { QStringLiteral("systemctl"),
QStringLiteral("enable"),
QStringLiteral("--now"),
svc };
int exitCode = QProcess::execute(prog, args);
if (exitCode == 0) {
qCInfo(logDFMBase) << "Enabled and started" << svc;
return true;
}
qCWarning(logDFMBase)
<< "Failed to enable" << svc << "pkexec exit code" << exitCode;
return false;
}
```
• isServiceRunning() now simply shells out to `systemctl is-active --quiet`
• enableServiceNow() calls `pkexec systemctl enable --now` directly
• Completely removes:
– QDBusInterface usage
– unit-path lookup, ManagerInterface, SubState checks
– Singleton boilerplate
This preserves exactly the “check & enable/start” behavior with ~25 LOC instead of 250.
</issue_to_address>
### Comment 8
<location> `src/dfm-base/utils/systemservicemanager.h:40` </location>
<code_context>
+ /**
+ * @brief 服务操作类型
+ */
+ enum ServiceAction {
+ Start, // 启动服务（临时）
+ Stop, // 停止服务
</code_context>
<issue_to_address>
**issue (complexity):** Consider removing the unused ServiceAction enum from the public header to simplify the API surface.
Here’s one quick win to shrink your public API without affecting any existing functionality—remove the unused `ServiceAction` enum (it isn’t referenced by any of your public methods). If you really need a generic “perform arbitrary action” internally, you can keep that enum in your .cpp or a private helper.
Before (in SystemServiceManager.h):
```cpp
public:
/**
* @brief 服务操作类型
*/
enum ServiceAction {
Start,
Stop,
Restart,
Enable,
Disable
};
```
After:
```cpp
public:
// …remove the enum entirely…
static SystemServiceManager &instance();
bool isServiceRunning(const QString &serviceName);
bool startService(const QString &serviceName);
bool enableServiceNow(const QString &serviceName);
```
If you still need to DRY-up your implementation, you can add a private helper in the .cpp:
```cpp
// In SystemServiceManager.cpp (or an anonymous namespace)
enum class ServiceAction { Start, Stop, Restart, Enable, Disable };
bool SystemServiceManager::performAction(ServiceAction action, const QString &svc) {
const auto path = unitPathFromName(svc);
// dispatch to the correct systemd call based on `action`
…
}
// Then implement:
bool SystemServiceManager::startService(const QString &svc) {
return performAction(ServiceAction::Start, svc);
}
```
This trims your public header down to exactly what callers need, while preserving all of your implementation-side flexibility.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| bool startService(const QString &service) | ||
| bool enableServiceNow(const QString &service) | ||
| { | ||
| if (service.isEmpty() || (service != "smb" && service != "nmb")) { |
There was a problem hiding this comment.
suggestion: Service name validation is strict and may limit extensibility.
Currently, only 'smb' and 'nmb' are allowed. Making this validation configurable would simplify future service additions.
|
Note
详情{
"debian/control": [
{
"line": "Homepage: http://www.deepin.org",
"line_number": 54,
"rule": "S35",
"reason": "Url link | 6fe814dfb7"
}
]
} |
|
Warning
详情 {
"export": {
"debian/rules": {
"a": [
"export QT_SELECT=5"
],
"b": [
"export QT_SELECT=6"
]
}
}
} |
|
Note
详情{
"debian/control": [
{
"line": "Homepage: http://www.deepin.org",
"line_number": 54,
"rule": "S35",
"reason": "Url link | 6fe814dfb7"
}
]
} |
Johnson-zs
force-pushed
the
refactor_safety
branch
from
6fbdc7f to
c76b31a
Compare
|
Warning
详情 {
"export": {
"debian/rules": {
"a": [
"export QT_SELECT=5"
],
"b": [
"export QT_SELECT=6"
]
}
}
} |
|
Note
详情{
"debian/control": [
{
"line": "Homepage: http://www.deepin.org",
"line_number": 54,
"rule": "S35",
"reason": "Url link | 6fe814dfb7"
}
]
} |
Johnson-zs
force-pushed
the
refactor_safety
branch
from
c76b31a to
b462ea6
Compare
|
Warning
详情 {
"export": {
"debian/rules": {
"a": [
"export QT_SELECT=5"
],
"b": [
"export QT_SELECT=6"
]
}
}
} |
|
Note
详情{
"debian/control": [
{
"line": "Homepage: http://www.deepin.org",
"line_number": 54,
"rule": "S35",
"reason": "Url link | 6fe814dfb7"
}
]
} |
Johnson-zs
force-pushed
the
refactor_safety
branch
from
b462ea6 to
d74bf9e
Compare
|
Warning
详情 {
"export": {
"debian/rules": {
"a": [
"export QT_SELECT=5"
],
"b": [
"export QT_SELECT=6"
]
}
}
} |
|
Note
详情{
"debian/control": [
{
"line": "Homepage: http://www.deepin.org",
"line_number": 54,
"rule": "S35",
"reason": "Url link | 6fe814dfb7"
}
]
} |
|
Warning
详情 {
"export": {
"debian/rules": {
"a": [
"export QT_SELECT=5"
],
"b": [
"export QT_SELECT=6"
]
}
}
} |
|
Note
详情{
"debian/control": [
{
"line": "Homepage: http://www.deepin.org",
"line_number": 54,
"rule": "S35",
"reason": "Url link | 6fe814dfb7"
}
]
} |
Updated Debian packaging configuration files to modernize the build system and clean up file installations: 1. Removed debian/compat file as debhelper-compat is now preferred (using version 12) 2. Updated Standards-Version from 3.9.8 to 4.5.0 3. Changed QT_SELECT from 5 to 6 in debian/rules 4. Simplified dbus service file installations using wildcards 5. Removed explicit MemoryLimit from daemon service file 6. Reorganized installation paths for better maintainability 7. Added explicit dependency on pkexec 8. Cleaned up Build-Depends list by removing redundant packages These changes bring the package up to date with current Debian packaging standards and improve maintainability while maintaining functional equivalence. Influence: 1. Verify package builds successfully with new dependencies 2. Check dbus services are properly installed and functional 3. Ensure daemon service runs without previous memory limits 4. Confirm all UI features remain available after installation chore: 更新 Debian 打包配置 更新 Debian 打包配置文件以现代化构建系统并清理文件安装： 1. 移除 debian/compat 文件，现使用 debhelper-compat（版本12） 2. 将 Standards-Version 从 3.9.8 更新至 4.5.0 3. 将 debian/rules 中的 QT_SELECT 从 5 改为 6 4. 使用通配符简化 dbus 服务文件安装 5. 从守护程序服务文件中移除显式内存限制 6. 重新组织安装路径以提高可维护性 7. 添加对 pkexec 的显式依赖 8. 清理 Build-Depends 列表中的冗余包 这些变化使软件包符合当前 Debian 打包标准，并在保持功能等价的同时提高可维 护性。 Influence: 1. 验证软件包能否使用新依赖成功构建 2. 检查 dbus 服务是否正确安装并正常工作 3. 确保守护程序服务运行时不受先前内存限制影响 4. 确认安装后所有 UI 功能保持可用
Improved the security measures in dde-file-manager-pkexec by: 1. Using XDG_RUNTIME_DIR instead of /tmp for temporary files to comply with FHS standards 2. Implementing secure random temporary file creation with mktemp 3. Setting strict file permissions (600) for the temporary file 4. Adding proper error handling for temp file creation 5. Removing temporary files securely with forced deletion 6. Storing and passing uid securely These changes prevent symlink attacks and race conditions while maintaining functionality during privilege escalation operations. Log: Improved security measures in file manager privilege escalation Influence: 1. Verify privilege escalation still works properly 2. Test with different XDG_RUNTIME_DIR configurations 3. Attempt symlink attacks to verify security improvements 4. Check temporary file permissions post-creation 5. Verify temporary files are properly cleaned up 6. Test with users having different UIDs refactor: 增强 pkexec 提权操作的安全性 对 dde-file-manager-pkexec 进行了安全增强： 1. 使用 XDG_RUNTIME_DIR 代替 /tmp 用于临时文件，符合 FHS 标准 2. 使用 mktemp 实现安全随机临时文件创建 3. 为临时文件设置严格的权限 (600) 4. 添加了临时文件创建的适当错误处理 5. 安全删除临时文件并强制删除 6. 安全存储和传递用户 UID 这些更改防止了符号链接攻击和竞态条件，同时在提权操作中保持功能正常。 Log: 改进了文件管理器提权操作的安全措施 Influence: 1. 验证提权操作是否仍然正常工作 2. 使用不同 XDG_RUNTIME_DIR 配置进行测试 3. 尝试符号链接攻击以验证安全改进 4. 检查临时文件创建后的权限 5. 验证临时文件是否被正确清理 6. 使用不同 UID 的用户进行测试
Removed all QGSettings related code guarded by QT_VERSION < QT_VERSION_CHECK(6, 0, 0) conditions across multiple files including menu scenes, desktop canvas, and organizer components. This cleanup targets code that was maintained for Qt5 backward compatibility. With the transition to Qt6 complete, these legacy implementations are no longer needed. The changes affect: 1. Action icon visibility handling in menus 2. Desktop context menu visibility control 3. Desktop app filtering logic 4. Watermark display settings 5. Organizer desktop app filtering All functionality now falls back to alternative configuration mechanisms like Application::appObtuselySetting() where appropriate. Influence: 1. Verify menu items still show/hide correctly without icon settings 2. Test desktop context menu behavior 3. Confirm desktop apps filtering works properly 4. Check watermark display functionality 5. Validate organizer app filtering operations refactor: 移除遗留的QGSettings代码 删除了多处文件中由QT_VERSION < QT_VERSION_CHECK(6, 0, 0)条件保护的 QGSettings相关代码，涉及菜单场景、桌面画布和组织器等组件。此次清理针对 的是为Qt5向后兼容而保留的旧有实现。随着向Qt6过渡完成，这些遗留实现不再 需要。 变更内容包括： 1. 菜单中操作图标可见性处理 2. 桌面右键菜单可见性控制 3. 桌面应用过滤逻辑 4. 水印显示设置 5. 组织器桌面应用过滤 所有功能现在都回退到适当的替代配置机制，如 Application::appObtuselySetting()等。 Influence: 1. 验证菜单项在无图标设置情况下仍能正确显示/隐藏 2. 测试桌面右键菜单行为 3. 确认桌面应用过滤功能正常工作 4. 检查水印显示功能 5. 验证组织器应用过滤操作
1. Added new SystemServiceManager class in dfm-base/utils for unified systemd service management 2. Removed redundant EnableSmbServices D-Bus method and outdated service control implementations 3. Migrated smb service management logic to use SystemServiceManager consistently 4. Implemented proper service monitoring and control via systemd D-Bus API with PolicyKit integration 5. Added support for both temporary starts and persistent enablements 6. Improved error handling and logging throughout service operations Log: Unified SMB service management with systemd D-Bus interface Influence: 1. Test SMB sharing functionality end-to-end 2. Verify service auto-start after reboot 3. Check error handling when service operations fail 4. Test concurrent service operations 5. Verify permission requirements for service management refactor: 使用SystemServiceManager统一管理服务 1. 在dfm-base/utils中添加新的SystemServiceManager类用于统一管理系统服务 2. 移除了冗余的EnableSmbServices D-Bus方法和过时的服务控制实现 3. 将smb服务管理逻辑统一迁移到使用SystemServiceManager 4. 通过systemd D-Bus API集成PolicyKit实现正确的服务监控和控制 5. 添加了对临时启动和持久启用的支持 6. 改进了服务操作过程中的错误处理和日志记录 Log: 使用systemd D-Bus接口统一SMB服务管理 Influence: 1. 测试端到端的SMB共享功能 2. 验证系统重启后服务自动启动 3. 检查服务操作失败时的错误处理 4. 测试并发服务操作 5. 验证服务管理所需的权限要求
1. Added debian/control.in template file for dynamic dependency
generation
2. Modified rules to detect Professional edition and add conditional
dependencies
3. Removed hardcoded dependencies ("hello" and others) from control file
4. Implemented logic to insert Professional-specific package
dependencies only when needed
Log: Updated package dependencies to support dynamic Professional
edition detection
Influence:
1. Verify package builds correctly on both Professional and Community
editions
2. Test installation with all dependencies satisfied
3. Check Professional-specific packages are properly included when
needed
4. Verify no missing dependencies exist in Community edition builds
5. Ensure the templating system correctly handles special characters
and newlines
chore: 更新 Debian 包依赖关系
1. 添加 debian/control.in 模板文件用于动态生成依赖关系
2. 修改 rules 文件以检测专业版并添加条件依赖
3. 从 control 文件中移除硬编码的依赖项("hello"等)
4. 实现逻辑仅在需要时插入专业版特定包依赖
Log: 更新包依赖关系以支持动态检测专业版
Influence:
1. 验证在专业版和社区版上都能正确构建包
2. 测试安装时所有依赖都能被满足
3. 检查专业版特定包在需要时被正确包含
4. 确保社区版构建中没有缺失依赖
5. 验证模板系统能正确处理特殊字符和换行符
1. Removed redundant test cases for smbd auto start since functionality is now handled differently 2. Eliminated tests for obsolete service management methods (startService, enableServiceAsync) 3. Simplified service checking logic tests to use enableServiceNow directly 4. Cleaned up test cases that were testing implementation details rather than behavior 5. Removed EnableSmbServices tests as this functionality has been deprecated 6. Various whitespace and formatting fixes in test files The changes reflect architectural improvements where: 1. Service management has been consolidated into simpler, more reliable methods 2. Obsolete D-Bus interactions have been removed 3. Implementation details were over-tested compared to behavior 4. The codebase has moved away from direct SMB service manipulation refactor: 移除过时的SMB服务测试 1. 移除了smbd自动启动的冗余测试用例，该功能现在以不同方式处理 2. 删除了过时服务管理方法(startService, enableServiceAsync)的测试 3. 简化了服务检查逻辑测试，直接使用enableServiceNow方法 4. 清理了测试实现细节而非行为的测试用例 5. 移除了EnableSmbServices测试，该功能已被弃用 6. 测试文件中各种空格和格式修正 这些变更反映了架构改进： 1. 服务管理已整合为更简单可靠的方法 2. 过时的D-Bus交互已被移除 3. 之前过度测试了实现细节而非行为 4. 代码库已转向不再直接操作SMB服务
Updated Debian package control files to handle compatibility issues with older package versions. Changes include: 1. Modified Replaces field to specify version constraints for dde- desktop and added dde-file-manager-services-plugins 2. Added Breaks field to explicitly declare incompatibility with older versions of dde-desktop and dde-file-manager-services-plugins 3. Updated version requirements to 6.5.101 across all relevant packages 4. Applied changes to both debian/control and debian/control.in files for consistency These changes ensure proper package upgrade paths and prevent installation conflicts with incompatible older package versions. Influence: 1. Verify package installation on systems with older versions of dde- desktop and related packages 2. Test upgrade scenarios from previous package versions 3. Check for any conflicts during package removal or replacement 4. Validate dependency resolution during fresh installations chore: 更新包依赖版本 更新了Debian包控制文件以处理旧包版本的兼容性问题。更改包括： 1. 修改Replaces字段以指定dde-desktop的版本约束并添加dde-file-manager- services-plugins 2. 添加Breaks字段以明确声明与旧版本dde-desktop和dde-file-manager- services-plugins的不兼容性 3. 将所有相关包的版本要求更新为6.5.101 4. 将更改同时应用于debian/control和debian/control.in文件以确保一致性 这些变更确保正确的包升级路径，并防止与不兼容的旧包版本产生安装冲突。 Influence: 1. 在使用旧版本dde-desktop和相关包的系统上验证包安装 2. 测试从先前包版本的升级场景 3. 检查包移除或替换过程中的任何冲突 4. 验证新安装过程中的依赖项解析
Removed the explicit systemd Reload call after enabling services since systemd automatically handles reload when using --now option. The call was redundant and not necessary for correct operation. Also simplified the error handling in UserShareHelper by removing unnecessary smbd service check since only nmbd service is critical for the operation. The changes: 1. Removed manual systemd Reload call from SystemServiceManager::enableServiceNow 2. Simplified UserShareHelper error handling to only check nmbd service status 3. Removed redundant error message concatenation logic These changes make the code cleaner and more focused on essential operations while maintaining the same functionality. Influence: 1. Verify that services still start correctly after enabling 2. Test SMB sharing functionality to ensure it works without the explicit Reload 3. Check error messages when service enabling fails 4. Confirm nmbd service is the only critical one for share operations refactor: 移除多余的服务重载和 smbd 检查 删除了启用服务后显式调用 systemd 的 Reload 操作，因为使用 --now 选项时 systemd 会自动处理重载。该调用是多余的且对正确运行并非必要。同时简化了 UserShareHelper 的错误处理，移除了不必要的 smbd 服务检查，因为只有 nmbd 服务对该操作是关键性的。 更改内容： 1. 从 SystemServiceManager::enableServiceNow 中删除了手动 systemd Reload 调用 2. 简化 UserShareHelper 错误处理仅检查 nmbd 服务状态 3. 移除了多余的错误消息拼接逻辑 这些改动使代码更简洁，更专注于基本操作，同时保持原有功能不变。 Influence: 1. 验证启用后服务是否能正常启动 2. 测试 SMB 共享功能确保在没有显式 Reload 时仍能工作 3. 检查服务启用失败时的错误消息 4. 确认 nmbd 服务是共享操作中唯一关键的服务
1. Deleted the outdated polkit encryption configuration file from installation 2. This file was related to deprecated encryption functionality 3. The removal ensures cleaner package installation without unused legacy files 4. Part of ongoing system security cleanup efforts Log: Removed obsolete encryption policy file from installation Influence: 1. Verify system encryption features still work without this file 2. Check package installation completes successfully 3. Test file manager operations involving encryption 4. Ensure no polkit authorization prompts are missing chore: 移除过时的polkit加密规则文件 1. 从安装列表中删除了过时的polkit加密配置文件 2. 该文件与已弃用的加密功能相关 3. 移除操作确保更干净的软件包安装，不含未使用的遗留文件 4. 系统安全清理工作的一部分 Log: 从安装中移除了过时的加密策略文件 Influence: 1. 验证系统加密功能在该文件移除后仍能正常工作 2. 检查软件包安装是否能成功完成 3. 测试文件管理器中涉及加密的操作 4. 确保没有缺少必要的polkit授权提示
1. Removed canonicalUrlList utility function from SystemPathUtil as it was no longer being used 2. Updated trash file operations to directly use source URLs without canonical processing 3. Cleaned up related code and improved consistency in file operations logic 4. The function was redundant as URL processing is now handled more efficiently elsewhere Log: Removed unused URL canonicalization handling in trash operations Influence: 1. Test all trash operations (move to/restore from/clean/copy from) 2. Verify behavior with symlinks in trash operations 3. Check error handling for invalid URLs 4. Test undo/redo operations for trash fix: 移除未使用的canonicalUrlList函数 1. 从SystemPathUtil中移除了不再使用的canonicalUrlList工具函数 2. 更新回收站文件操作直接使用源URL而不再进行标准化处理 3. 清理了相关代码并改进了文件操作逻辑的一致性 4. 该函数已冗余，URL处理现在在其他地方更高效地完成 Log: 移除了回收站操作中未使用的URL标准化处理 Influence: 1. 测试所有回收站操作(移动到/恢复到/清空/从回收站复制) 2. 验证包含符号链接的回收站操作行为 3. 检查对无效URL的错误处理 4. 测试回收站操作的撤销/重做功能 Bug: https://pms.uniontech.com/bug-view-339887.html
Changed the network check logic for CIFS mounts from checking only first mount point to checking all available mount points when determining trash state. Added new method checkAllCIFSBusy() in NetworkUtils to handle this. Previously, when network disconnection occurred while accessing CIFS shares, the system would only check the first mount point, potentially causing the main thread to hang if that mount point was unresponsive. This fix checks all mount points properly and prevents UI freezes. Log: Fixed thread freeze issue when network disconnection occurs during CIFS access Influence: 1. Test trash operations while connected to CIFS/SMB shares 2. Verify application doesn't freeze when network disconnects 3. Check trash state updates correctly after operations 4. Monitor system responsiveness during network interruptions 5. Test with multiple CIFS mount points simultaneously fix: 解决断网导致主线程卡死问题 修改了检查CIFS挂载的网络逻辑，从仅检查第一个挂载点改为检查所有可用挂载点 来确定回收站状态。在NetworkUtils中添加了新的checkAllCIFSBusy()方法来处理 这种情况。 之前当访问CIFS共享时发生网络断开，系统只会检查第一个挂载点，如果该挂载点 无响应可能导致主线程卡死。此修复会正确检查所有挂载点并防止UI冻结。 Log: 修复了访问CIFS时断网导致应用卡死的问题 Influence: 1. 测试连接CIFS/SMB共享时的回收站操作 2. 验证网络断开时应用不会冻结 3. 检查操作后回收站状态是否正确更新 4. 监控网络中断期间系统的响应性 5. 测试同时使用多个CIFS挂载点的情况
Added checkRetry() call in doMoveToTrash() function to properly handle retry logic during trash file operations. This ensures consistent behavior with the rest of the file operations workflow where retry attempts are properly evaluated before continuing with subsequent actions. Log: Fixed missing retry check in trash file operations Influence: 1. Test moving files to trash with insufficient permissions 2. Verify retry mechanism works properly during trash operations 3. Check skip and cancel operations still function correctly 4. Test with various error scenarios that would trigger retry fix: 添加回收站文件操作中缺失的重试检查 在doMoveToTrash()函数中添加了checkRetry()调用，以正确处理回收站文件操作 中的重试逻辑。这确保了与文件操作流程其他部分的一致性，即在继续后续操作之 前正确评估重试尝试。 Log: 修复回收站文件操作中缺失的重试检查 Influence: 1. 测试在权限不足时移动文件到回收站的情况 2. 验证回收站操作中重试机制正常工作 3. 检查跳过和取消操作仍能正常运行 4. 测试会触发重试的各种错误场景
Johnson-zs
force-pushed
the
refactor_safety
branch
from
cb232bd to
f7e3e48
Compare
|
Warning
详情 {
"export": {
"debian/rules": {
"a": [
"export QT_SELECT=5"
],
"b": [
"export QT_SELECT=6"
]
}
}
} |
|
Note
详情{
"debian/control": [
{
"line": "Homepage: http://www.deepin.org",
"line_number": 54,
"rule": "S35",
"reason": "Url link | 6fe814dfb7"
}
]
} |
|
Warning
详情 {
"export": {
"debian/rules": {
"a": [
"export QT_SELECT=5"
],
"b": [
"export QT_SELECT=6"
]
}
}
} |
|
TAG Bot TAG: 6.5.102 |
6.5.102 Log:
Johnson-zs
force-pushed
the
refactor_safety
branch
from
c237ef1 to
aab340a
Compare
|
Warning
详情 {
"export": {
"debian/rules": {
"a": [
"export QT_SELECT=5"
],
"b": [
"export QT_SELECT=6"
]
}
}
} |
deepin pr auto review这是一个比较复杂的代码变更，涉及多个模块的改动。我将从以下几个方面进行审查：
建议和潜在问题：
总体来说，这次改动提高了代码的可维护性、安全性和性能，是一个积极的改进。建议在部署前进行充分的测试，特别是服务管理相关的功能。 |
|
Note
详情{
"debian/control": [
{
"line": "Homepage: http://www.deepin.org",
"line_number": 54,
"rule": "S35",
"reason": "Url link | 6fe814dfb7"
}
]
} |
|
/forcemerge |
|
This pr force merged! (status: blocked) |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 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.
Summary by Sourcery
Unify service management by introducing SystemServiceManager and refactor existing code to use it, while removing legacy QGSettings logic and updating affected tests
New Features:
Enhancements:
Tests: