refactor: remove dconfig and optimize permission management#3048
Open
mhduiy wants to merge 1 commit intolinuxdeepin:masterfrom
Open
refactor: remove dconfig and optimize permission management#3048mhduiy wants to merge 1 commit intolinuxdeepin:masterfrom
mhduiy wants to merge 1 commit intolinuxdeepin:masterfrom
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: mhduiy 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 |
Reviewer's GuideRefactors the privacy/permissions subsystem to remove dconfig-based blacklist caching and file-path-based blacklists, introduces package-name-based permission management backed by desktop file source paths, and centralizes dpkg querying and executable resolution logic in PrivacySecurityWorker. Sequence diagram for package-based permission initializationsequenceDiagram
participant DdeAppItemModel
participant PrivacySecurityWorker
participant ApplicationItem
participant QtConcurrent
participant DpkgQuery
participant PrivacySecurityModel
PrivacySecurityWorker->>DdeAppItemModel: data(index, DesktopSourcePathRole)
DdeAppItemModel-->>PrivacySecurityWorker: desktopSourcePath
PrivacySecurityWorker->>ApplicationItem: onDesktopPathChanged(desktopSourcePath)
PrivacySecurityWorker->>QtConcurrent: run(updateAppPath with execs, item)
QtConcurrent->>PrivacySecurityWorker: getAppPath(execs)
QtConcurrent-->>PrivacySecurityWorker: appPath
alt item.desktopPath is empty
QtConcurrent->>PrivacySecurityWorker: getPackageNameByPath(appPath)
else item.desktopPath is set
QtConcurrent->>PrivacySecurityWorker: getPackageNameByPath(item.desktopPath)
end
PrivacySecurityWorker->>DpkgQuery: dpkg-query -S path
DpkgQuery-->>PrivacySecurityWorker: packageName
QtConcurrent-->>PrivacySecurityWorker: invokeMethod(this, path, packageName)
PrivacySecurityWorker->>ApplicationItem: onAppPathChanged(path)
PrivacySecurityWorker->>ApplicationItem: onPackageChanged(packageName)
PrivacySecurityWorker->>PrivacySecurityModel: updatePermission(item)
PrivacySecurityWorker->>PrivacySecurityModel: emitAppDataChanged(item)
Class diagram for updated privacy permission managementclassDiagram
class ApplicationItem {
+QString id
+QString name
+QString appPath
+QString package()
+QString icon() const
+QString sortField() const
+QString desktopPath() const
+bool isPremissionEnabled(int premission) const
+void setPremissionEnabled(int premission, bool enabled)
+void onAppPathChanged(QString path)
+void onPackageChanged(QString package)
+void onExecsChanged(QMap<QString,QString> execs)
+void onExecutablePathsChanged(QStringList paths)
+void onIconChanged(QString icon)
+bool onPremissionEnabledChanged(int premission, bool enabled)
+void onDesktopPathChanged(QString desktopPath)
}
class PrivacySecurityModel {
+QMap<int,int> m_premissionMap
+QMap<QString,QSet<QString>> m_blacklistByPackage
+unsigned m_uniqueID
+bool m_updating
+void updatePermission()
+bool updatePermission(ApplicationItem *item)
+const QString premissiontoPath(int premission) const
+int pathtoPremission(const QString &path, bool mainPremission) const
+bool isPremissionEnabled(int premission) const
+void setBlackListByPackage(QMap<QString,QSet<QString>> blacklistByPackage)
+unsigned createUniqueID()
+bool addApplictionItem(ApplicationItem *item)
+void removeApplictionItem(const QString &id)
}
class PrivacySecurityWorker {
-PrivacySecurityModel *m_model
-QAbstractItemModel *m_ddeAmModel
-PrivacySecurityDataProxy *m_dataProxy
-QMap<QString,QSet<QString>> m_blacklistByPackage
+PrivacySecurityWorker(PrivacySecurityModel *appsModel, QObject *parent)
+void init()
+void initApp()
+void updateAllPermission()
+ApplicationItem *addAppItem(int dataIndex)
+void updateAppPath(ApplicationItem *item)
+void setAppPermissionEnableByCheck(bool ok)
+QStringList getExecutable(const QString &packageName)
+QString getPackageNameByPath(const QString &path)
}
class PrivacySecurityDataProxy {
-bool m_serviceExists
-DConfig *m_dconfig
+PrivacySecurityDataProxy(QObject *parent)
+bool existsService() const
+QMap<QString,QSet<QString>> getCacheBlacklist()
+void setCacheBlacklist(const QMap<QString,QSet<QString>> &cacheBlacklist)
}
class AppItemModel {
<<enum Roles>>
+int IdRole
+int XCreatedByRole
+int ExecsRole
+int CategoriesRole
+int DesktopSourcePathRole
}
PrivacySecurityWorker --> PrivacySecurityModel : manages
PrivacySecurityWorker --> PrivacySecurityDataProxy : uses
PrivacySecurityModel --> ApplicationItem : owns items
PrivacySecurityWorker --> ApplicationItem : populates and updates
PrivacySecurityWorker --> AppItemModel : reads roles
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:
- PrivacySecurityDataProxy still declares getCacheBlacklist() in the header but its implementation has been removed from the .cpp, which will cause a linker error; either restore the implementation or drop the declaration (and any remaining uses) completely.
- In PrivacySecurityWorker::addAppItem the variable name
desktopSoucePathis misspelled; consider renaming it todesktopSourcePathfor consistency and readability. - The helper methods getExecutable() and getPackageNameByPath() in PrivacySecurityWorker are declared as private slots but are only used as internal helpers; making them regular private methods instead of slots would clarify their intent and avoid unnecessary exposure to the signal/slot system.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- PrivacySecurityDataProxy still declares getCacheBlacklist() in the header but its implementation has been removed from the .cpp, which will cause a linker error; either restore the implementation or drop the declaration (and any remaining uses) completely.
- In PrivacySecurityWorker::addAppItem the variable name `desktopSoucePath` is misspelled; consider renaming it to `desktopSourcePath` for consistency and readability.
- The helper methods getExecutable() and getPackageNameByPath() in PrivacySecurityWorker are declared as private slots but are only used as internal helpers; making them regular private methods instead of slots would clarify their intent and avoid unnecessary exposure to the signal/slot system.
## Individual Comments
### Comment 1
<location path="src/plugin-privacy/operation/privacysecurityworker.cpp" line_range="43-52" />
<code_context>
- QProcess pro;
- pro.start("/usr/bin/dpkg", QStringList() << "--print-architecture");
- pro.waitForFinished(2000);
- if (pro.exitCode() != 0) {
- qCWarning(DCC_PRIVACY) << "Failed to get dpkg architecture, dpkg error: " << pro.readAllStandardError().simplified()
- << ", fallback to QSysInfo architecture:" << arch;
</code_context>
<issue_to_address>
**issue (bug_risk):** qDebug(DCC_PRIVACY) is not a valid Qt logging call and will fail to compile
Use a categorized logging macro here, e.g. `qCWarning(DCC_PRIVACY)` (to match other error paths in this module) or `qCDebug(DCC_PRIVACY)` if you prefer a debug-level message.
</issue_to_address>
### Comment 2
<location path="src/plugin-privacy/operation/privacysecurityworker.h" line_range="56-57" />
<code_context>
void init();
void initApp();
+ QStringList getExecutable(const QString &packageName);
+ QString getPackageNameByPath(const QString &path);
+
private:
</code_context>
<issue_to_address>
**suggestion:** Helper methods are declared as slots even though they are not used as signals/slots
`getExecutable` and `getPackageNameByPath` are currently declared as slots but only used as private helpers and not connected to any signals. This is slightly misleading and adds unnecessary meta-object overhead. Please move them to the `private:` section unless you intend to use them as slots.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
1. Removed org.deepin.dde.control-center.privacy.json dconfig file as permission blacklist caching is no longer needed 2. Added desktopPath property to ApplicationItem to track desktop file source path 3. Added new roles to dde-apps.h for categories and desktop source path 4. Moved getExecutable() and getPackageNameByPath() functions from PrivacySecurityDataProxy to PrivacySecurityWorker 5. Changed permission blacklist storage from app paths to package names for better consistency 6. Simplified permission checking logic by using package-based blacklist instead of file-based 7. Updated copyright years from 2025 to 2025-2026 across multiple files The changes optimize permission management by: - Removing dependency on dconfig for caching, simplifying configuration - Using package names instead of file paths for blacklist, making permission management more reliable - Centralizing package and executable path resolution logic in PrivacySecurityWorker - Adding desktop path tracking to better handle application identification refactor: 移除 dconfig 并优化权限管理 1. 移除 org.deepin.dde.control-center.privacy.json dconfig 文件，不再需 要权限黑名单缓存 2. 为 ApplicationItem 添加 desktopPath 属性以跟踪桌面文件源路径 3. 在 dde-apps.h 中添加新的角色用于分类和桌面源路径 4. 将 getExecutable() 和 getPackageNameByPath() 函数从 PrivacySecurityDataProxy 移动到 PrivacySecurityWorker 5. 将权限黑名单存储从应用路径改为包名，提高一致性 6. 通过使用基于包的黑名单而非基于文件的黑名单简化权限检查逻辑 7. 更新多个文件的版权年份从 2025 到 2025-2026 这些更改通过以下方式优化权限管理： - 移除对 dconfig 缓存的依赖，简化配置 - 使用包名而非文件路径作为黑名单，使权限管理更可靠 - 在 PrivacySecurityWorker 中集中处理包和可执行路径解析逻辑 - 添加桌面路径跟踪以更好地处理应用识别 PMS: BUG-351621
deepin pr auto reviewGit Diff 代码审查报告总体评估本次代码变更主要涉及隐私权限模块的重构，从基于文件路径的权限管理转向基于包名的权限管理。主要改动包括删除配置文件、重构数据模型和代理类、添加桌面路径支持等。整体代码质量良好，但存在一些可以改进的地方。 详细审查1. 语法与逻辑问题
2. 代码质量问题
3. 性能问题
4. 安全问题
改进建议
总结本次代码变更实现了从基于文件路径到基于包名的权限管理转换，整体方向正确。主要需要关注的是代码重复、命名一致性和安全防护等方面的问题。建议在合并前进行适当的重构和测试。 |
Contributor
Author
|
am 的 desktopsourcepath 是被override过的path,不一定是安装包内部的，使用这个来判断包名并不靠谱 |
|
TAG Bot New tag: 6.1.75 |
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
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.
The changes optimize permission management by:
refactor: 移除 dconfig 并优化权限管理
这些更改通过以下方式优化权限管理：
PMS: BUG-351621
Summary by Sourcery
Refine privacy plugin permission handling by moving package lookup logic into the worker, switching permission blacklisting to be package-based instead of path-based, and wiring additional app metadata such as desktop file paths from the app model into application items.
New Features:
Bug Fixes:
Enhancements:
Build:
Chores: