Skip to content

feat: add systemd service for login wallpaper change - #27

Merged
deepin-bot[bot] merged 1 commit into
linuxdeepin:masterfrom
mhduiy:wallpaper
Oct 9, 2025
Merged

deepin-bot[bot] merged 1 commit into
linuxdeepin:masterfrom
mhduiy:wallpaper

Conversation

@mhduiy

@mhduiy mhduiy commented Sep 28, 2025 •

Copy link
Copy Markdown
Contributor
  1. Add D-Bus service file and systemd user service for wallpaper slideshow
  2. Implement requestChangeWallpaper D-Bus method to handle login- triggered wallpaper changes
  3. Replace manual session tracking with systemd-based activation for better reliability
  4. Add symbolic link in dde-session-initialized.target.wants for proper service integration
  5. Simplify changeBgAfterLogin method to handle all monitors with login policy

feat: 添加登录壁纸更改的 systemd 服务

  1. 添加 D-Bus 服务文件和 systemd 用户服务用于壁纸幻灯片
  2. 实现 requestChangeWallpaper D-Bus 方法来处理登录触发的壁纸更改
  3. 用基于 systemd 的激活替换手动会话跟踪以提高可靠性
  4. 在 dde-session-initialized.target.wants 中添加符号链接以正确集成服务
  5. 简化 changeBgAfterLogin 方法以处理所有具有登录策略的显示器

pms: BUG-333159

Summary by Sourcery

Enable automated wallpaper slideshow changes on user login by adding D-Bus and systemd user services, removing manual session tracking, and refactoring the login-triggered background change logic.

New Features:

  • Add requestChangeWallpaper D-Bus method to trigger wallpaper changes on login
  • Introduce systemd user service and D-Bus service files for automatic login-time slideshow activation

Enhancements:

  • Replace manual /proc session tracking with systemd-based activation for improved reliability
  • Simplify changeBgAfterLogin logic to iterate through all monitors with login policy

Build:

  • Update CMakeLists to install D-Bus and systemd unit files under share/dbus-1 and lib/systemd

@sourcery-ai

sourcery-ai Bot commented Sep 28, 2025

Copy link
Copy Markdown

Reviewer's Guide

This PR integrates a systemd-based service and D-Bus method to trigger login wallpaper changes, refactors the changeBgAfterLogin logic to remove manual session tracking in favor of JSON-based iteration across monitors, and updates CMake rules to install the new service files.

Class diagram for WallpaperSlideshow and SlideshowManager changes

classDiagram
    class WallpaperSlideshow {
        +void SetWallpaperSlideShow(QString monitorName, QString slideShow)
        +QString GetWallpaperSlideShow(QString monitorName)
        +void requestChangeWallpaper(QString condition)
        +void onPropertyChanged(QString name, QVariant value)
        -SlideshowManager* m_manager
    }
    class SlideshowManager {
        +void setMonitorBackground(QString monitorName, QString imageGile)
        +void handlePrepareForSleep(bool sleep)
        +void changeBgAfterLogin()
        -void autoChangeBg(QString monitorSpace, QDateTime time)
        -QString m_wallpaperSlideShow
    }
    WallpaperSlideshow --> SlideshowManager
Loading

File-Level Changes

Change Details Files
Introduce D-Bus and systemd service files for login-triggered wallpaper changes
  • Add D-Bus service unit under misc/dbus-1/services
  • Add systemd user service and enable it via a wants symlink
  • Update CMakeLists to install D-Bus and systemd directories
src/plugin-qt/wallpaperslideshow/CMakeLists.txt
src/plugin-qt/wallpaperslideshow/misc/dbus-1/services/org.deepin.dde.WallpaperSlideshow.service
src/plugin-qt/wallpaperslideshow/misc/systemd/user/dde-postlogin-wallpaper.service
src/plugin-qt/wallpaperslideshow/misc/systemd/user/dde-session-initialized.target.wants/dde-postlogin-wallpaper.service
Expose requestChangeWallpaper over D-Bus
  • Declare requestChangeWallpaper method in the XML interface
  • Add requestChangeWallpaper slot in wallpaperslideshow.h
  • Implement requestChangeWallpaper in wallpaperslideshow.cpp
src/plugin-qt/wallpaperslideshow/org.deepin.dde.WallpaperSlideshow.xml
src/plugin-qt/wallpaperslideshow/wallpaperslideshow.h
src/plugin-qt/wallpaperslideshow/wallpaperslideshow.cpp
Refactor changeBgAfterLogin to remove manual session tracking
  • Change signature from bool changeBgAfterLogin(QString) to void changeBgAfterLogin()
  • Remove /proc/self/sessionid file operations and runDir checks
  • Use JSON parsing to iterate over all login-policy monitors and call autoChangeBg
src/plugin-qt/wallpaperslideshow/slideshowmanager.h
src/plugin-qt/wallpaperslideshow/slideshowmanager.cpp

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

@deepin-ci-robot

Copy link
Copy Markdown

deepin pr auto review

我来审查这段代码变更,从语法逻辑、代码质量、性能和安全性几个方面进行分析:

语法逻辑

  1. D-Bus服务配置:

    • org.deepin.dde.WallpaperSlideshow.service 中的 Exec=/bin/false 是正确的,因为这是一个D-Bus激活服务,不应该直接执行
    • Systemd服务单元文件配置合理,依赖关系和启动时机设置正确
  2. 代码变更:

    • changeBgAfterLogin 方法从返回bool改为void,简化了调用逻辑
    • 新增的 requestChangeWallpaper 方法正确处理了登录时的壁纸变更请求
    • 移除了不必要的会话ID检查逻辑,简化了代码

代码质量

  1. 改进点:

    • 移除了复杂的会话ID检查逻辑,使代码更简洁
    • 使用D-Bus方式触发壁纸变更,比直接检查文件更符合Linux系统设计原则
    • 添加了适当的日志记录
  2. 需要改进的地方:

    • changeBgAfterLogin 方法中的注释 "// do nothing" 应该更具体地说明原因
    • requestChangeWallpaper 方法中的注释 "// other conditions can be added here" 可以更具体地说明可能的条件

性能

  1. 改进点:

    • 移除了文件I/O操作(检查/proc/self/sessionid和运行时目录文件),提高了性能
    • 直接使用D-Bus调用,减少了不必要的中间步骤
  2. 潜在问题:

    • 在 changeBgAfterLogin 中遍历所有壁纸配置,如果配置项很多可能会有性能影响
    • 没有看到对 m_wallpaperSlideShow 的缓存处理,每次都需要解析JSON

安全性

  1. 改进点:

    • 移除了直接文件操作,减少了潜在的文件系统攻击面
    • 使用系统服务管理机制(systemd)来控制服务生命周期,更安全
  2. 需要注意的地方:

    • D-Bus服务没有看到权限控制,应该确保只有授权用户/服务可以调用
    • requestChangeWallpaper 方法直接接受字符串参数,没有进行输入验证,可能存在安全风险

建议改进

  1. 代码质量:
void WallpaperSlideshow::requestChangeWallpaper(const QString &condition)
{
    // 验证输入参数
    if (condition.isEmpty()) {
        qWarning() << "Empty condition received";
        return;
    }

    if (condition == WSPOLICYLOGIN) {
        m_manager->changeBgAfterLogin();
    } else {
        qWarning() << "Unknown condition:" << condition;
        // 可以考虑在这里扩展支持其他条件
    }
}
  1. 性能优化:
void SlideshowManager::changeBgAfterLogin()
{
    qInfo() << "changeBgAfterLogin";
    
    // 缓存解析结果
    static QJsonDocument cachedDoc;
    static QHash<QString, QString> cachedConfig;
    
    if (cachedDoc.isNull()) {
        cachedDoc = QJsonDocument::fromJson(m_wallpaperSlideShow.toLatin1());
        cachedConfig = cachedDoc.object().toVariantMap();
    }
    
    for (auto it = cachedConfig.begin(); it != cachedConfig.end(); ++it) {
        if (it.value().toString() == WSPOLICYLOGIN)
            autoChangeBg(it.key(), QDateTime::currentDateTimeUtc());
    }
}
  1. 安全性增强:
  • 在D-Bus服务配置中添加权限控制:
[D-BUS Service]
Name=org.deepin.dde.WallpaperSlideshow
Exec=/bin/false
SystemdService=deepin-service-manager.service
User=@USER@
Group=@GROUP@

总体来说,这次代码变更改进了代码的简洁性和性能,移除了不必要的文件操作,改用更现代的D-Bus通信方式。但在安全性和错误处理方面还有提升空间。

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Hey there - I've reviewed your changes and they look great!

Prompt for AI Agents
Please address the comments from this code review:

## Individual Comments

### Comment 1
<location> `src/plugin-qt/wallpaperslideshow/slideshowmanager.cpp:252-253` </location>
<code_context>
-        }
-    }
+    qInfo() << "changeBgAfterLogin";
+    QJsonDocument doc = QJsonDocument::fromJson(m_wallpaperSlideShow.toLatin1());
+    QVariantMap tempMap = doc.object().toVariantMap();

-    if (needChangeBg) {
</code_context>

<issue_to_address>
**issue (bug_risk):** No error handling for invalid JSON in m_wallpaperSlideShow.

Add a validity check for QJsonDocument and log a warning if parsing fails to improve reliability and aid debugging.
</issue_to_address>

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

@18202781743
18202781743 requested a review from Copilot September 29, 2025 06:10

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull Request Overview

Adds systemd service integration for automated wallpaper changes on user login, replacing manual session tracking with D-Bus method calls triggered by systemd services.

  • Introduces requestChangeWallpaper D-Bus method to handle login-triggered wallpaper changes
  • Replaces manual /proc/self/sessionid tracking with systemd-based activation for improved reliability
  • Simplifies login wallpaper logic to iterate through all monitors with login policy

Reviewed Changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
wallpaperslideshow.h Adds new D-Bus method declaration for wallpaper change requests
wallpaperslideshow.cpp Implements requestChangeWallpaper method to trigger login wallpaper changes
slideshowmanager.h Changes changeBgAfterLogin signature from returning bool to void
slideshowmanager.cpp Replaces session file tracking with simplified monitor iteration logic
org.deepin.dde.WallpaperSlideshow.xml Adds D-Bus interface definition for new requestChangeWallpaper method
dde-postlogin-wallpaper.service (symlink) Creates systemd service symlink for target integration
dde-postlogin-wallpaper.service Defines systemd user service to call D-Bus method on login
org.deepin.dde.WallpaperSlideshow.service Adds D-Bus service file for automatic service activation
CMakeLists.txt Adds installation rules for D-Bus and systemd service files

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Comment thread src/plugin-qt/wallpaperslideshow/wallpaperslideshow.cpp Outdated
Comment thread src/plugin-qt/wallpaperslideshow/slideshowmanager.cpp Outdated
18202781743
18202781743 previously approved these changes Sep 30, 2025
1. Replace reading session ID from /proc/self/sessionid with D-Bus call
to org.deepin.dde.SessionManager1.GetSessionPath
2. Improve file handling by using QFileInfo and ensuring directory
creation
3. Enhance session tracking by comparing D-Bus session paths instead of
session IDs
4. Add proper error handling for D-Bus calls and file operations
5. Use QIODevice::ReadWrite mode for better file management

The changes provide more reliable session detection using D-Bus services
and improve the robustness of wallpaper slideshow management during
login sessions.

refactor: 使用 D-Bus 会话路径替换会话 ID 进行登录检测

1. 将读取 /proc/self/sessionid 替换为调用
org.deepin.dde.SessionManager1.GetSessionPath 的 D-Bus 方法
2. 使用 QFileInfo 改进文件处理并确保目录创建
3. 通过比较 D-Bus 会话路径而非会话 ID 来增强会话跟踪
4. 为 D-Bus 调用和文件操作添加适当的错误处理
5. 使用 QIODevice::ReadWrite 模式改进文件管理

这些更改通过使用 D-Bus 服务提供了更可靠的会话检测,并提高了登录会话期间
壁纸幻灯片管理的鲁棒性。

pms: BUG-333159
@deepin-ci-robot

Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: fly602, mhduiy

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

@mhduiy

mhduiy commented Oct 9, 2025

Copy link
Copy Markdown
Contributor Author

/forcemerge

@deepin-bot

deepin-bot Bot commented Oct 9, 2025

Copy link
Copy Markdown

This pr force merged! (status: blocked)

@deepin-bot
deepin-bot Bot merged commit 9d9b43b into linuxdeepin:master Oct 9, 2025
6 of 8 checks passed
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.

5 participants