Repository navigation
feat: support per-module visible and disable config - #3587
Conversation
Reviewer's GuideIntroduces per-DccObject visible and disabled configuration through new DConfig metadata, build-generated accessors, asynchronous subpath loading, and deferred state application that covers both existing and late-arriving objects. Sequence diagram for asynchronous per-module configuration loadingsequenceDiagram
participant Manager as DccManager
participant Paths as modulepaths DConfig
participant Config as DccModuleConfig
participant Object as DccObject
Manager->>Paths: DConfig::create
Paths-->>Manager: modulePaths
loop each module path
Manager->>Config: DccModuleConfig::create
alt initialization succeeds
Config-->>Manager: configInitializeSucceed
Manager->>Config: visible()
Manager->>Config: disable()
Manager->>Config: deleteLater()
else initialization fails
Config-->>Manager: configInitializeFailed
Manager->>Config: deleteLater()
end
Manager->>Manager: finishModuleConfigItem()
end
Manager->>Manager: refreshModuleConfigState()
Manager->>Object: applyModuleConfigState
Object-->>Manager: hidden or disabled state applied
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey - I've found 2 issues
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="src/dde-control-center/dccmanager.cpp" line_range="1326-1327" />
<code_context>
+ if (!m_moduleConfigReady || !obj)
+ return;
+
+ if (contains(m_moduleHiddenPaths, obj))
+ DccObject::Private::FromObject(obj)->setFlagState(DCC_CONFIG_HIDDEN, true);
+ if (contains(m_moduleDisabledPaths, obj))
+ DccObject::Private::FromObject(obj)->setFlagState(DCC_CONFIG_DISABLED, true);
</code_context>
<issue_to_address>
**Per-module restrictions get cleared**
When an object is restricted by both configuration schemes and the aggregate entry is removed, `updateModuleConfig` clears the shared hidden or disabled flag even though the per-module setting still requires it; `applyModuleConfigState` only sets the flag, so the module becomes visible or enabled.
Compute each shared flag from both configuration sources so removing one source preserves any restriction required by the other.
</issue_to_address>
### Comment 2
<location path="src/dde-control-center/dccmanager.cpp" line_range="1280" />
<code_context>
+ m_moduleHiddenPaths.insert(path);
+ if (config->disable())
+ m_moduleDisabledPaths.insert(path);
+ config->deleteLater();
+ finishModuleConfigItem();
+ });
</code_context>
<issue_to_address>
**Runtime setting changes go unapplied**
When a per-module `visible` or `disable` value changes after initialization, `loadModuleConfigs` reads the initial values and deletes each config without watching for changes, so the running module retains its old visibility or enabled state until the control center restarts.
Keep a change handler for each per-module setting and update the module state when its value changes.
Also at `src/dde-control-center/dccmanager.cpp:1281`.
</issue_to_address>| m_moduleHiddenPaths.insert(path); | ||
| if (config->disable()) | ||
| m_moduleDisabledPaths.insert(path); | ||
| config->deleteLater(); |
There was a problem hiding this comment.
🟡 Medium · Runtime setting changes go unapplied
When a per-module visible or disable value changes after initialization, loadModuleConfigs reads the initial values and deletes each config without watching for changes, so the running module retains its old visibility or enabled state until the control center restarts.
Keep a change handler for each per-module setting and update the module state when its value changes.
Also at src/dde-control-center/dccmanager.cpp:1281.
Prompt for AI agents
In `src/dde-control-center/dccmanager.cpp` at line 1280:
**Runtime setting changes go unapplied**
When a per-module `visible` or `disable` value changes after initialization, `loadModuleConfigs` reads the initial values and deletes each config without watching for changes, so the running module retains its old visibility or enabled state until the control center restarts.
Keep a change handler for each per-module setting and update the module state when its value changes.
Also at `src/dde-control-center/dccmanager.cpp:1281`.fe3f767 to
bd95d86
Compare
| if (!m_moduleConfigReady || !obj) | ||
| return; | ||
|
|
||
| if (contains(m_moduleHiddenPaths, obj)) |
There was a problem hiding this comment.
这里只处理了m_moduleHiddenPaths,没处理m_hideModule,可调用updateConfigState统一处理
1. Add two new DConfig metadata files under misc/configs: - org.deepin.dde.control-center.module.json: defines "visible" and "disable" properties for a single module subpath - org.deepin.dde.control-center.modulepaths.json: lists every DccObject runtime path that should be initialized at startup 2. In CMake, use dconfig2cpp to generate the DccModuleConfig class from the module meta json at build time, and add the binary directory to the include path 3. In DccManager, load the modulePaths list and asynchronously read each subpath through DccModuleConfig::create 4. Accumulate hidden/disabled paths from each subpath's visible/ disable values, and only apply them once all subpaths have returned (m_moduleConfigReady) 5. Call applyModuleConfigState in addObject so late-arriving objects also pick up the final state, and refresh all affected paths once loading is complete 6. Read the cached generated-class properties (config->visible()/ config->disable()) instead of config->config()->value() to avoid synchronous cross-thread round trips 7. Handle initialization failure of individual subpaths by logging and releasing the config instance, then still counting down the pending counter 8. This introduces a per-DccObject configuration scheme (subpath of org.deepin.dde.control-center.module) so individual modules can be hidden/disabled independently, instead of relying only on the aggregate hideModule/disableModule lists Log: Added per-module visible and disable configuration support Influence: 1. Verify control center starts normally with the new config files installed 2. Test setting visible=false on a module path and verify the module is hidden 3. Test setting disable=true on a module path and verify the module is disabled 4. Verify all modules load correctly after config initialization completes 5. Test that late-arriving objects also receive the correct visible/ disable state 6. Verify graceful handling when a module config subpath fails to initialize 7. Confirm the modulepaths list is read correctly and empty list results in no-op behavior 8. Check that no cross-thread blocking or performance regression occurs during startup feat: 支持按模块配置显示与禁用状态 1. 在 misc/configs 下新增两个 DConfig 元数据文件: - org.deepin.dde.control-center.module.json:定义单个模块 subpath 的 visible 与 disable 属性 - org.deepin.dde.control-center.modulepaths.json:列出启动时需要初始 化的所有 DccObject 运行时路径 2. 在 CMake 中使用 dconfig2cpp 在构建时从模块元数据 json 生成 DccModuleConfig 类,并将二进制目录加入 include 路径 3. 在 DccManager 中读取 modulePaths 清单,并通过 DccModuleConfig::create 异步读取每个 subpath 4. 根据每个 subpath 的 visible/disable 值累积隐藏/禁用路径,并等全部返回 后(m_moduleConfigReady)才统一生效 5. 在 addObject 中调用 applyModuleConfigState,使晚到的对象也能命中最终 状态;加载完成后统一刷新受影响路径 6. 使用生成类的缓存属性(config->visible()/config->disable())而非 config->config()->value(),避免跨线程同步阻塞往返 7. 单个 subpath 初始化失败时记录日志并释放实例,同时仍然递减待处理计数 8. 引入针对单个 DccObject 的配置方案(org.deepin.dde.control- center.module 的 subpath),可独立隐藏/禁用某个模块,而不再仅依赖聚合的 hideModule/disableModule 列表 Log: 新增按模块配置显示与禁用功能 Influence: 1. 验证安装新配置文件后控制中心能正常启动 2. 测试对某模块路径设置 visible PMS: TASK-396675
bd95d86 to
86f5632
Compare
deepin pr auto review🤖 AI 代码审查报告📊 总体评价
🔍 详细分析1. 语法逻辑 ✅评价: 语法正确,逻辑清晰 ✅ 通过 潜在问题: 建议: ['在 loadModuleConfigs() 的空列表提前返回分支中补充 m_moduleConfigReady = true,保持状态一致性'] 2. 代码质量 ✅评价: 代码结构清晰,注释完整 ✅ 通过 潜在问题: 建议: ['可考虑为 m_moduleConfigPending 添加超时兜底机制(如 QTimer),在极端情况下(DConfig 内部异常导致信号未发射)确保计数归零,避免单模块配置静默失效'] 3. 代码性能 ✅评价: 性能良好,资源使用合理 ✅ 通过 潜在问题: 建议: [] 4. 代码安全 🔒评价: 存在0个安全漏洞 ✅ 通过
安全漏洞详情: 建议: [] 💡 改进建议代码示例// 修复 loadModuleConfigs() 中空路径列表的状态一致性问题 void DccManager::loadModuleConfigs() {
QScopedPointer<DConfig> pathsConfig(DConfig::create(kAppId, kModulePathsConfigName, QString(), nullptr));
if (!pathsConfig || !pathsConfig->isValid()) {
qCWarning(dccLog) << "Failed to read module paths from" << kModulePathsConfigName;
m_moduleConfigReady = true;
return;
}
const QStringList paths = pathsConfig->value(ModulePathsConfig).toStringList();
m_modulePaths = QSet<QString>(paths.begin(), paths.end());
m_moduleConfigPending = int(m_modulePaths.size());
if (m_moduleConfigPending == 0) {
m_moduleConfigReady = true; // 保持与无效配置分支一致
return;
}
for (const QString &path : std::as_const(m_modulePaths)) {
auto *config = DccModuleConfig::create(kAppId, QLatin1Char('/') + path, this);
if (!config) {
qCWarning(dccLog) << "Failed to create module config for path:" << path;
finishModuleConfigItem();
continue;
}
connect(config, &DccModuleConfig::configInitializeSucceed, this, [this, path, config](DConfig *) {
if (!config->visible())
m_moduleHiddenPaths.insert(path);
if (config->disable())
m_moduleDisabledPaths.insert(path);
config->deleteLater();
finishModuleConfigItem();
});
connect(config, &DccModuleConfig::configInitializeFailed, this, [this, path, config] {
qCWarning(dccLog) << "Failed to init module config:" << path;
config->deleteLater();
finishModuleConfigItem();
});
}
}本报告由 AI 代码审查工具自动生成 |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: caixr23, xujin177 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 |
Log: Added per-module visible and disable configuration support
Influence:
feat: 支持按模块配置显示与禁用状态
hideModule/disableModule 列表
Log: 新增按模块配置显示与禁用功能
Influence:
PMS: TASK-396675
Summary by Sourcery
Support independent visibility and disabled-state control for individual control-center modules.
New Features:
Bug Fixes:
Enhancements:
Build: