Skip to content

feat(factory): upgrade DccFactory to v2.0 with lifecycle and DBus data channel#3228

Open
caixr23 wants to merge 1 commit into
linuxdeepin:masterfrom
caixr23:dcc-v2
Open

feat(factory): upgrade DccFactory to v2.0 with lifecycle and DBus data channel#3228
caixr23 wants to merge 1 commit into
linuxdeepin:masterfrom
caixr23:dcc-v2

Conversation

@caixr23
Copy link
Copy Markdown
Contributor

@caixr23 caixr23 commented May 13, 2026

  1. Rename DccFactory to DccFactory_20, bump IID to v2.0
  2. Add registerType() for QML type registration in main thread
  3. Add active() for post-creation activation in main thread
  4. Add get/set virtual methods for DBus-triggered data access in worker thread
  5. Add DCC_FULL_FACTORY_CLASS macro with singleton create and full lifecycle delegation to classname::registerType/active/get/set
  6. Preserve v1.0 interface in dccfactoryold.h for backward compatibility
  7. Wire DBus get/set slots in ControlCenterDBusAdaptor and DccManager (async dispatch pending)
  8. Migrate DefAppModel plugin as first v2.0 adopter, move qmlRegisterType into static registerType()

Influence:

  1. Verify existing v1.0 plugins still load via old interface
  2. Verify DefAppModel QML types register correctly on startup

feat(factory): 升级 DccFactory 至 v2.0,支持生命周期管理与 DBus 数据通道

  1. 将 DccFactory 重命名为 DccFactory_20,IID 升级为 v2.0
  2. 新增 registerType() 用于在主线程中注册 QML 类型
  3. 新增 active() 用于在主线程中执行创建后激活
  4. 新增 get/set 虚函数用于在子线程中响应 DBus 数据请求
  5. 新增 DCC_FULL_FACTORY_CLASS 宏,支持单例创建及完整生命周期 委托至 classname::registerType/active/get/set
  6. 在 dccfactoryold.h 中保留 v1.0 接口以向后兼容
  7. 在 ControlCenterDBusAdaptor 和 DccManager 中接入 DBus get/set (异步分发待实现)
  8. 迁移 DefAppModel 插件作为首个 v2.0 适配示例,将 qmlRegisterType 移入静态 registerType()

Influence:

  1. 验证已有的 v1.0 插件仍能通过旧接口正常加载
  2. 验证 DefAppModel 的 QML 类型在启动时正确注册

@deepin-ci-robot
Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: caixr23

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

Copy link
Copy Markdown

@sourcery-ai sourcery-ai Bot left a comment

Choose a reason for hiding this comment

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

Sorry @caixr23, you have reached your weekly rate limit of 500000 diff characters.

Please try again later or upgrade to continue using Sourcery

Comment thread include/dccfactory.h
class DccObject;

class DccFactory : public QObject
class DccFactory_20 : public QObject
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

保留DccFactory名,旧插件处理会不会冲突

Comment thread include/dccfactory.h
}; \
}

#define DCC_FULL_FACTORY_CLASS(classname) \
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

命名带版本号可以不冲突 DCC_FACTORY_V20_CLASS

Comment thread include/dccfactory.h
Q_SIGNALS:
// 模块中有设置项变化时,发出该信号
// 命名valuesChanged() 待定
void propertiesChanged(const QVariantMap &properties);
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

信号命名问题,数据类型用QString(json)还是QVariantMap,get/set的类型要不要统一?
信号是否有依赖插件是否实现

@deepin-bot
Copy link
Copy Markdown

deepin-bot Bot commented May 16, 2026

TAG Bot

New tag: 6.1.86
DISTRIBUTION: unstable
Suggest: synchronizing this PR through rebase #3238

@deepin-bot
Copy link
Copy Markdown

deepin-bot Bot commented May 20, 2026

TAG Bot

New tag: 6.1.87
DISTRIBUTION: unstable
Suggest: synchronizing this PR through rebase #3245

@deepin-bot
Copy link
Copy Markdown

deepin-bot Bot commented May 22, 2026

TAG Bot

New tag: 6.1.88
DISTRIBUTION: unstable
Suggest: synchronizing this PR through rebase #3251

@deepin-bot
Copy link
Copy Markdown

deepin-bot Bot commented May 25, 2026

TAG Bot

New tag: 6.1.89
DISTRIBUTION: unstable
Suggest: synchronizing this PR through rebase #3253

…a channel

1. Rename DccFactory to DccFactory_20, bump IID to v2.0
2. Add registerType() for QML type registration in main thread
3. Add active() for post-creation activation in main thread
4. Add get/set virtual methods for DBus-triggered data access in
   worker thread
5. Add DCC_FULL_FACTORY_CLASS macro with singleton create and full
   lifecycle delegation to classname::registerType/active/get/set
6. Preserve v1.0 interface in dccfactoryold.h for backward
   compatibility
7. Wire DBus get/set slots in ControlCenterDBusAdaptor and DccManager
   (async dispatch pending)
8. Migrate DefAppModel plugin as first v2.0 adopter, move
   qmlRegisterType into static registerType()

Influence:
1. Verify existing v1.0 plugins still load via old interface
2. Verify DefAppModel QML types register correctly on startup

feat(factory): 升级 DccFactory 至 v2.0,支持生命周期管理与 DBus 数据通道

1. 将 DccFactory 重命名为 DccFactory_20,IID 升级为 v2.0
2. 新增 registerType() 用于在主线程中注册 QML 类型
3. 新增 active() 用于在主线程中执行创建后激活
4. 新增 get/set 虚函数用于在子线程中响应 DBus 数据请求
5. 新增 DCC_FULL_FACTORY_CLASS 宏,支持单例创建及完整生命周期
   委托至 classname::registerType/active/get/set
6. 在 dccfactoryold.h 中保留 v1.0 接口以向后兼容
7. 在 ControlCenterDBusAdaptor 和 DccManager 中接入 DBus get/set
   (异步分发待实现)
8. 迁移 DefAppModel 插件作为首个 v2.0 适配示例,将
   qmlRegisterType 移入静态 registerType()

Influence:
1. 验证已有的 v1.0 插件仍能通过旧接口正常加载
2. 验证 DefAppModel 的 QML 类型在启动时正确注册
@deepin-ci-robot
Copy link
Copy Markdown

deepin pr auto review

你好!我是CodeGeeX。我仔细审查了你提供的Git Diff,本次修改主要是将DDE控制中心的插件工厂接口从v1.0升级到v2.0,增加了生命周期管理(注册、激活)和跨进程数据交互(通过DBus的Get/Set)的能力。

整体设计思路清晰,但代码在语法逻辑、代码质量、代码性能和代码安全方面存在一些需要重点关注的问题,特别是多线程并发和内存管理方面的隐患。

以下是详细的审查意见和改进建议:

一、 语法与逻辑错误

1. DCC_FULL_FACTORY_CLASS 宏中存在严重的内存泄漏和逻辑错误
dccfactory.hDCC_FULL_FACTORY_CLASS 宏中,create 函数的实现有逻辑缺陷:

QObject *create(QObject *parent = nullptr) override {
    if (!m_object) {
        m_object = new classname(parent); // [1] 内存泄漏风险
        connect(m_object, &classname::propertiesChanged, this, &classname##DccFactory::propertiesChanged);
    }
    return m_object;
}
  • 问题1(内存泄漏):如果多次调用 create 且传入了不同的 parent,由于 !m_objectfalse,传入的 parent 会被直接丢弃,且如果未来有逻辑销毁了 m_object 但未重置指针,会导致悬空指针。
  • 问题2(逻辑混乱)create 的语义通常是“创建一个新实例”,而此处的实现是“单例模式”。如果设计意图是单例,那么不应该有 parent 参数(单例的生命周期通常由自身或全局管理,而非 QObject 树);如果设计意图是工厂多例,则不应缓存 m_object
  • 改进建议:如果确需单例,建议移除 parent 参数,并处理对象销毁时的指针重置;或者将 m_object 的创建移到 registerTypeactive 中,让 create 仅返回实例。

2. DccManager::getset 的异步逻辑未完成且存在逻辑断裂
dccmanager.cpp 中:

QString DccManager::get(const QString &module, const QString &param) {
    auto message = this->message();
    setDelayedReply(true);
    // TODO:异步调用对应module的Factory的get函数
    return QString(); // [致命错误]
}
  • 问题:调用了 setDelayedReply(true) 告诉 DBus “我会异步回复”,但函数最后却直接 return QString()。Qt DBus 适配器会将这个空字符串作为响应发回客户端,异步回复机制完全失效,且客户端永远只能收到空字符串。
  • 改进建议:在异步任务完成时,必须显式调用 QDBusMessage::reply() 并通过 QDBusConnection::send() 发送响应。如果暂时未实现异步逻辑,请先删除 setDelayedReply(true),让函数同步返回结果,否则会导致客户端调用阻塞或超时。

3. DccFactory_20 虚函数签名中的 const 返回值无意义

virtual const QString get(const QString &param) { return QString(); }
virtual const QString set(const QString &param) { return QString(); }
  • 问题:函数返回非引用/非指针的 const QString 在 C++ 中没有实际意义,因为返回的是右值(临时对象),编译器会忽略顶层的 const 限定符。
  • 改进建议:移除返回值的 const,改为 virtual QString get(...)virtual QString set(...)

二、 代码质量

1. 宏 DCC_FULL_FACTORY_CLASS 过于臃肿且脆弱

  • 问题:该宏包含了大量的逻辑实现(单例控制、信号连接、函数转发)。宏无法进行单步调试,且在编译期的错误提示极难阅读。
  • 改进建议:强烈建议将 DccFactory_20 的实现提取为一个模板类,替代宏。例如:
    template<typename T>
    class DccFactoryImpl : public dccV25::DccFactory_20 {
    public:
        void registerType() override { T::registerType(); }
        QObject *create(QObject *parent = nullptr) override { /* ... */ }
        // ... 其他转发逻辑
    };
    #define DCC_FULL_FACTORY_CLASS(classname) \
        namespace { class Q_DECL_EXPORT classname##DccFactory : public DccFactoryImpl<classname> { Q_OBJECT Q_PLUGIN_METADATA(IID DccFactory_iid) Q_INTERFACES(dccV25::DccFactory_20) public: using DccFactoryImpl<classname>::DccFactoryImpl; }; }

2. 版权年份修改 (2027 -> 2026)

  • 问题:Diff 中多处将 2024 - 2027 改为 2024 - 2026。如果是新代码,起始年份应该是当前年份(如 2024 - 20262024 - present)。但如果只是修改已有文件,随意缩短结束年份可能存在法务风险,请确认是否符合项目的版权规范。

3. 命名规范不一致

  • 问题DccFactory_20 这种带版本号下划线的命名不够优雅,且与旧版 DccFactory 容易混淆。dccfactoryold.h 中的类名依然是 DccFactory
  • 改进建议:建议使用更具语义的命名,如 DccFactoryV2,或者如果旧接口仅用于兼容,可考虑放入 dccV1 命名空间下。

三、 代码性能

1. GetSet 的参数序列化性能

  • 问题get(const QString &param)set(const QString &param) 的注释中提到“建议支持批量获取/设置”。这意味着 param 很可能是一个 JSON 字符串。在子线程(DBus触发)中频繁进行 JSON 的序列化与反序列化,性能开销较大。
  • 改进建议:如果数据结构固定,建议使用 QVariantMap 或自定义 DBus 数据类型(通过 Q_DECLARE_METATYPE 注册)代替 JSON 字符串,这样 Qt DBus 可以直接进行底层 marshalling,避免每次调用时的字符串解析开销。

2. DccFactory_20::active() 中的隐藏开销

  • 问题active() 在主线程调用,且注释说明初始化流程按顺序调用。如果各个插件的 active() 包含大量耗时 I/O 或同步 DBus 调用,会严重阻塞控制中心主线程的启动。
  • 改进建议:确保 active() 仅做轻量级的 UI 初始化,耗时的数据加载应放在 create (子线程) 或异步加载中。

四、 代码安全

1. 多线程数据竞争

  • 问题:注释中明确指出 getset 是在子线程中调用,而 activecreate 是在主线程中调用。DCC_FULL_FACTORY_CLASS 宏中持有 m_object 指针,主线程可能正在调用 m_object->active(),而子线程同时调用 m_object->get(),如果 m_object 内部状态未加锁,将导致灾难性的数据竞争和随机崩溃。
  • 改进建议
    1. DccFactory_20 的文档注释中强烈强调:实现类必须保证 get/set 相对于 active/其他方法 的线程安全性
    2. DCC_FULL_FACTORY_CLASS 宏(或模板类)中,对 m_object 的访问(特别是 create 中的判空和赋值)应当使用 QMutexstd::atomic<QObject*> 保护,防止主线程和子线程同时初始化导致的 double-free 或野指针。

2. DBus 接口的安全校验

  • 问题:新增了 Set 方法,允许外部通过 DBus 修改控制中心内部模块的状态。如果状态涉及敏感信息(如管理员密码、网络配置等),可能被恶意进程篡改。
  • 改进建议:在 ControlCenterDBusAdaptor::SetDccManager::set 中,增加 DBus 调用者的权限校验(例如通过 QDBusMessage::service() 获取 PID/UID,或使用 PolicyKit 进行鉴权)。

3. 模块名注入风险

  • 问题DccManager::get(const QString &module, ...) 中的 module 参数直接来自 DBus 外部输入。如果内部使用 Map 查找插件,需防止外部传入特殊构造的 module 名导致越界或异常。
  • 改进建议:对 module 参数进行严格的格式校验(如正则匹配 ^[a-zA-Z0-9_-]+$),拒绝包含特殊字符的输入。

总结与核心修改建议代码示例

针对最致命的 create 逻辑和多线程问题,建议将宏中的实现调整如下(或重构为模板类):

// 在 DCC_FULL_FACTORY_CLASS 宏中,修改 create 和 active 的逻辑
public:
    using dccV25::DccFactory_20::DccFactory_20;
    
    void registerType() override {
        classname::registerType();
    }
    
    QObject *create(QObject *parent = nullptr) override {
        // 注意:如果设计为单例,parent不应由外部传入,或者不使用单例模式
        // 这里假设设计意图是单例,忽略parent
        if (!m_object) {
            m_object = new classname(); 
            connect(m_object, &classname::propertiesChanged, this, &classname##DccFactory::propertiesChanged);
        }
        return m_object;
    }
    
    void active() override {
        if (m_object) { // 必须判空,因为 create 可能还没被调用
            m_object->active();
        }
    }
    
    const QString get(const QString &param) override {
        // TODO: 此处存在多线程并发访问 m_object 的风险,需加锁或确保 m_object 在 active 前已完全初始化且不再析构
        if (m_object) {
            return m_object->get(param);
        }
        return QString();
    }
    
    const QString set(const QString &param) override {
        if (m_object) {
            return m_object->set(param);
        }
        return QString("Object not initialized");
    }

同时,请务必修复 dccmanager.cpp 中的 setDelayedReply 问题,避免 DBus 通信彻底失效。

@deepin-bot
Copy link
Copy Markdown

deepin-bot Bot commented Jun 4, 2026

TAG Bot

New tag: 6.1.90
DISTRIBUTION: unstable
Suggest: synchronizing this PR through rebase #3279

@deepin-bot
Copy link
Copy Markdown

deepin-bot Bot commented Jun 4, 2026

TAG Bot

New tag: 6.1.91
DISTRIBUTION: unstable
Suggest: synchronizing this PR through rebase #3281

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants