Conversation
When removing an anonymous share, the directory permissions were not being restored to their original state. This fix implements a permission management system that records original permissions when setting up anonymous shares and restores them when shares are removed. Key changes: 1. Added AnonymousPermissionManager class to handle permission recording and restoration 2. Records original permissions when enabling anonymous sharing 3. Restores permissions when removing anonymous shares or switching to non-anonymous 4. Handles home directory permissions for anonymous access 5. Prevents restoration if user manually modified permissions 6. Uses JSON configuration file to persist permission records across sessions Log: Fixed directory permission restoration after removing anonymous shares Influence: 1. Test anonymous share creation and verify permissions are set correctly 2. Test removing anonymous shares and verify permissions are restored 3. Test switching from anonymous to non-anonymous sharing 4. Verify home directory permissions are handled correctly 5. Test permission restoration when user manually modifies permissions 6. Verify permission records persist after application restart fix: 修复移除匿名共享后目录权限未恢复的问题 移除匿名共享时，目录权限未能恢复到原始状态。此修复实现了权限管理系统，在 设置匿名共享时记录原始权限，并在移除共享时恢复权限。 主要变更： 1. 新增 AnonymousPermissionManager 类处理权限记录和恢复 2. 启用匿名共享时记录原始权限 3. 移除匿名共享或切换到非匿名时恢复权限 4. 处理匿名访问所需的用户主目录权限 5. 防止在用户手动修改权限后进行恢复 6. 使用 JSON 配置文件持久化权限记录 Log: 修复移除匿名共享后目录权限恢复问题 Influence: 1. 测试匿名共享创建并验证权限设置正确 2. 测试移除匿名共享并验证权限恢复 3. 测试从匿名切换到非匿名共享 4. 验证用户主目录权限正确处理 5. 测试用户手动修改权限后的恢复行为 6. 验证权限记录在应用重启后持久化 Bug: https://pms.uniontech.com/bug-view-350345.html
Added kCleanTrashType to the condition that handles progress notifications for move to trash and restore operations. This ensures that when cleaning trash, the progress calculation uses the correct metric (number of source URLs) instead of the default file count, providing accurate progress reporting for trash cleaning operations. Influence: 1. Test trash cleaning operations to verify progress bar updates correctly 2. Compare progress reporting between trash cleaning and other file operations 3. Verify that trash cleaning completes without progress calculation errors fix: 将清理回收站类型添加到进度通知逻辑中 在移动至回收站和恢复操作的处理条件中添加了 kCleanTrashType 类型。这确保 在清理回收站时，进度计算使用正确的指标（源 URL 数量）而不是默认的文件计 数，为回收站清理操作提供准确的进度报告。 Influence: 1. 测试回收站清理操作，验证进度条正确更新 2. 比较回收站清理与其他文件操作的进度报告 3. 验证回收站清理完成时没有进度计算错误
6.5.121 Log:
Contributor
|
[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. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
TAG Bot TAG: 6.5.121 |
Reviewer's GuideIntroduces an AnonymousPermissionManager to centrally track and safely restore filesystem permissions for anonymous directory shares, updates dirshare UI and menu flows to use it (including handling transitions between anonymous and non-anonymous shares and unsharing), and fixes file operation progress notifications for trash cleaning jobs. Sequence diagram for updating a share from anonymous to non-anonymoussequenceDiagram
actor User
participant ShareControlWidget
participant UserShareHelper
participant AnonymousPermissionManager
User ->> ShareControlWidget: toggle share options
activate ShareControlWidget
ShareControlWidget->>ShareControlWidget: filePath = url.toLocalFile()
ShareControlWidget->>UserShareHelper: shareInfoByPath(filePath)
UserShareHelper-->>ShareControlWidget: oldShareInfo
ShareControlWidget->>ShareControlWidget: wasAnonymous = oldShareInfo[kAnonymous]
ShareControlWidget->>ShareControlWidget: shareFolder()
alt shareFolder fails and not isShared(filePath)
ShareControlWidget-->>ShareControlWidget: disable share UI
ShareControlWidget-->>User: share disabled
else share still exists
ShareControlWidget->>ShareControlWidget: isNowAnonymous = (shareAnonymousSelector index == 1)
alt wasAnonymous and not isNowAnonymous
ShareControlWidget->>AnonymousPermissionManager: restoreDirectoryPermissions(filePath)
ShareControlWidget->>AnonymousPermissionManager: restoreHomeDirectoryIfNoAnonymousShares()
else other transitions
Note over ShareControlWidget,AnonymousPermissionManager: Non-anonymous to anonymous handled inside shareFolder
end
end
deactivate ShareControlWidget
Sequence diagram for unsharing an anonymous directory via menu or widgetsequenceDiagram
actor User
participant UI as ShareControlWidget_or_DirShareMenuScene
participant UserShareHelper
participant AnonymousPermissionManager
User ->> UI: unshare directory
activate UI
UI->>UI: filePath = url.toLocalFile() or u.path()
UI->>UserShareHelper: shareInfoByPath(filePath)
UserShareHelper-->>UI: shareInfo
UI->>UI: wasAnonymous = shareInfo[kAnonymous]
UI->>UserShareHelper: removeShareByPath(filePath)
UserShareHelper-->>UI: success flag
alt success and wasAnonymous
UI->>AnonymousPermissionManager: restoreDirectoryPermissions(filePath)
UI->>AnonymousPermissionManager: restoreHomeDirectoryIfNoAnonymousShares()
else not anonymous or failure
UI-->>User: unshare failed or no permission changes
end
deactivate UI
Class diagram for AnonymousPermissionManager and updated dirshare componentsclassDiagram
direction TB
class AnonymousPermissionManager {
<<singleton>>
+static char kJsonVersion
+static char kJsonShares
+static char kJsonHomeDirectory
+static char kJsonPath
+static char kJsonOriginal
+static char kJsonExpected
+static AnonymousPermissionManager instance()
+bool setAnonymousPermissions(QString filePath, DirectoryType type, bool writable)
+bool restoreDirectoryPermissions(QString filePath)
+bool restoreHomeDirectoryIfNoAnonymousShares()
+void cleanRecord(QString filePath)
-AnonymousPermissionManager(QObject parent)
-bool loadFromFile()
-bool saveToFile()
-QFile~Permissions~ getExpectedPermissions(QFile~Permissions~ original)
-QFile~Permissions~ getHomeExpectedPermissions(QFile~Permissions~ original)
-int getCurrentAnonymousShareCount() const
-QString configFilePath
-QMap~QString, PermissionRecord~ shareRecords
-HomeDirectoryRecord homeRecord
}
class DirectoryType {
<<enumeration>>
kSharedDirectory
kHomeDirectory
}
class PermissionRecord {
QFile~Permissions~ original
QFile~Permissions~ expected
}
class HomeDirectoryRecord {
QString path
QFile~Permissions~ original
QFile~Permissions~ expected
}
class ShareControlWidget {
+void updateShare()
+bool shareFolder()
+bool unshareFolder()
-QUrl url
-QComboBox sharePermissionSelector
-QComboBox shareAnonymousSelector
}
class DirShareMenuScene {
+bool triggered(QAction action)
}
class UserShareHelper {
+QVariantMap shareInfoByPath(QString filePath)
+bool isShared(QString filePath)
+bool removeShareByPath(QString filePath)
}
AnonymousPermissionManager --> DirectoryType
AnonymousPermissionManager --> PermissionRecord
AnonymousPermissionManager --> HomeDirectoryRecord
ShareControlWidget ..> AnonymousPermissionManager : uses
ShareControlWidget ..> UserShareHelper : uses
DirShareMenuScene ..> AnonymousPermissionManager : uses
DirShareMenuScene ..> UserShareHelper : uses
Flow diagram for AnonymousPermissionManager permission lifecycleflowchart TB
start([Start anonymous share setup])
checkType{Directory type?}
sharedPath([Shared directory])
homePath([Home directory])
start --> checkType
checkType -->|kSharedDirectory| sharedPath
checkType -->|kHomeDirectory| homePath
sharedPath --> recordShared[Record PermissionRecord
original = current
expected = getExpectedPermissions or add ExeGroup ExeOther]
recordShared --> setSharedPerms{"setPermissions(expected) success?"}
setSharedPerms -->|no| failShared[[Fail and remove record]]
setSharedPerms -->|yes| saveShared[Save shareRecords to JSON]
saveShared --> endShared([Anonymous shared directory active])
homePath --> recordHome{homeRecord.path empty?}
recordHome -->|yes| setHomeRecord[Store path, original, expected = getHomeExpectedPermissions]
recordHome -->|no| skipHomeRecord[Use existing homeRecord]
setHomeRecord --> setHomePerms
skipHomeRecord --> setHomePerms
setHomePerms[Set home directory permissions to expected] --> homePermsOk{setPermissions success?}
homePermsOk -->|no| failHome[[Fail to set home permissions]]
homePermsOk -->|yes| saveHome[Save homeRecord to JSON]
saveHome --> endHome([Home directory prepared for anonymous shares])
subgraph Restore_directory_after_unshare
restoreStart([Unshare anonymous directory]) --> hasRecord{shareRecords contains path?}
hasRecord -->|no| restoreEndSkip([Skip restore])
hasRecord -->|yes| comparePerms{current == expected?}
comparePerms -->|no| userModified[[User modified perms, cleanRecord]]
comparePerms -->|yes| setOriginal[Set permissions to original]
setOriginal --> restored{setPermissions success?}
restored -->|yes| cleanRecordNode[cleanRecord and save]
restored -->|no| restoreFail[[Failed to restore directory]]
cleanRecordNode --> restoreEnd([Directory restored])
end
subgraph Restore_home_when_no_anonymous_shares
homeRestoreStart([Check home restore]) --> hasHome{homeRecord.path empty?}
hasHome -->|yes| homeRestoreSkip([Nothing to restore])
hasHome -->|no| countShares[getCurrentAnonymousShareCount]
countShares --> hasShares{count > 0?}
hasShares -->|yes| keepHome([Keep home permissions])
hasShares -->|no| compareHomePerms{current == expected?}
compareHomePerms -->|no| clearHomeRecord[[User modified home perms, clear homeRecord]]
compareHomePerms -->|yes| restoreHomePerms[Set home permissions to original]
restoreHomePerms --> homeRestored{setPermissions success?}
homeRestored -->|yes| clearHomeRecord2[Clear homeRecord and save]
homeRestored -->|no| homeRestoreFail[[Failed to restore home directory]]
end
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Contributor
deepin pr auto review这份代码审查主要针对 1. 语法与逻辑审查
|
![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 left some high level feedback:
- In
restoreHomeDirectoryIfNoAnonymousShares,getCurrentAnonymousShareCount()only uses the persistedshareRecordsmap, so stale entries (e.g., shares removed outside this code path or after an app restart) will block home-permission restoration; consider reconciling against actual current shares or pruning non-existent paths when loading. restoreDirectoryPermissionsassumes the directory still exists and callsQFileInfo(filePath).permissions()andQFile::setPermissionsunconditionally; it might be safer to explicitly handle non-existent paths (e.g., log and clean the record) to avoid leaving orphaned records in the JSON.- The comment on
getCurrentAnonymousShareCount()says it gets the count fromUserShareHelper, but the implementation just returnsshareRecords.size(); either update the comment or wire this through to the actual share source to avoid confusion about what the count represents.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `restoreHomeDirectoryIfNoAnonymousShares`, `getCurrentAnonymousShareCount()` only uses the persisted `shareRecords` map, so stale entries (e.g., shares removed outside this code path or after an app restart) will block home-permission restoration; consider reconciling against actual current shares or pruning non-existent paths when loading.
- `restoreDirectoryPermissions` assumes the directory still exists and calls `QFileInfo(filePath).permissions()` and `QFile::setPermissions` unconditionally; it might be safer to explicitly handle non-existent paths (e.g., log and clean the record) to avoid leaving orphaned records in the JSON.
- The comment on `getCurrentAnonymousShareCount()` says it gets the count from `UserShareHelper`, but the implementation just returns `shareRecords.size()`; either update the comment or wire this through to the actual share source to avoid confusion about what the count represents.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Contributor
Author
|
/forcemerge |
Contributor
|
This pr force merged! (status: blocked) |
Contributor
|
TAG Bot ✅ Tag created successfully 📋 Tag Details
|
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.
Summary by Sourcery
Introduce centralized management of filesystem permissions for anonymous directory shares and ensure permissions are restored when anonymous sharing is disabled or removed.
New Features:
Bug Fixes:
Enhancements: