fix: preserve legacy DConfig cache with prefixed cache - #582
Conversation
1. Add libdtk6core-bin dependency to libdtk6core-dev package (DTK6) 2. Add libdtkcore5-bin dependency to libdtkcore-dev package (DTK5) 3. Ensure development packages pull in matching version tools packages 4. Maintain consistent version alignment between dev libraries and tools Log: Enhanced DTK development packages to include matching tools dependencies Influence: 1. Verify dev package installation includes corresponding tools packages 2. Confirm version alignment between dev and tools packages 3. Test installation with versioned dependencies in offline environments 4. Validate compatibility with existing build systems using DTK dev packages chore: 为 DTK 开发包添加配套工具包依赖 1. 为 libdtk6core-dev 包(DTK6)添加 libdtk6core-bin 依赖 2. 为 libdtkcore-dev 包(DTK5)添加 libdtkcore5-bin 依赖 3. 确保开发包安装时自动引入对应版本的工具包 4. 保持开发库与工具包之间版本一致性 Log: 增强 DTK 开发包,新增配套工具包依赖 Influence: 1. 验证开发包安装时是否包含对应的工具包 2. 确认开发包与工具包之间版本一致性 3. 在离线环境中测试带版本依赖的安装过程 4. 验证现有使用 DTK 开发包的构建系统兼容性
添加管控类型方法调用的返回值 Log: 添加管控类型 Influence: 管控进程. Task: https://pms.uniontech.com/task-view-260335.html Task: https://pms.uniontech.com/task-view-260333.html Change-Id: If91612d7b3e824b26081f169d5acc108cd1cef98 (cherry picked from commit b1e3ac4)
1. Refactor DConfigCacheImpl::load to support loading cache from both legacy path and newly configured path prefix 2. Implement loadCache helper returning detailed status (NotFound/ Loaded/Invalid) and tracking loaded keys 3. When cache prefix is set, load legacy cache first then merge with prefixed cache so legacy-only values are not lost 4. Track cache changes when legacy keys are missing from new overlay cache 5. Add comprehensive tests covering user cache overlay, global cache overlay, and no-appid/subpath scenarios 6. Preserve save behavior which always persists merged state to the prefix path 7. Update documentation for setCachePathPrefix explaining the new merge behavior Log: Cache path prefix now preserves legacy configuration values during migration Influence: 1. Verify setCachePathPrefix with no prior cache creates a new prefixed cache correctly 2. Test migration scenario where legacy cache exists with different values than new prefixed cache 3. Check global config applies prefix overlay behavior correctly 4. Validate configs without appid and with subpath work with cache overlay 5. Confirm saving after merged load persists legacy-only values into the new prefix 6. Ensure files without cache prefix still load exactly as before 7. Test invalid cache files in legacy path do not block new cache loading fix: 在加载时将前缀缓存叠加到传统缓存之上 1. 重构 DConfigCacheImpl::load,支持同时从传统路径和新设置的路径前缀加载 缓存 2. 实现 loadCache 辅助函数,返回详细状态(NotFound/Loaded/Invalid)并跟 踪已加载的键 3. 设置缓存前缀时,先加载传统缓存,再合并前缀缓存,避免丢失仅存在于传统 缓存中的值 4. 当新覆盖缓存缺少传统键时,跟踪缓存变化状态 5. 添加全面测试,覆盖用户缓存覆盖、全局缓存覆盖、无应用ID和子路径场景 6. 保持保存行为不变,始终将合并后的状态写入前缀路径 7. 更新 setCachePathPrefix 文档,说明新的合并行为 Log: 缓存路径前缀在迁移过程中现在可以保留传统配置值 Influence: 1. 验证使用 setCachePathPrefix 且无历史缓存时能正确创建新前缀缓存 2. 测试传统缓存与新前缀缓存存在不同值时的迁移场景 3. 检查全局配置是否正确应用前缀覆盖行为 4. 验证无应用ID和带子路径的配置能正常工作于缓存覆盖 5. 确认合并加载后的保存操作能够将传统独有值持久化到新前缀 6. 确保未设置缓存前缀时的文件加载行为完全不变 7. 测试传统路径中存在无效缓存文件时不会阻止新缓存加载
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: 18202781743 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 GuideThe PR preserves backward compatibility when a cache-path prefix is introduced by loading the legacy cache first, overlaying prefixed values, and using the existing dirty/save flow to migrate legacy-only keys; it also adds authorized permission parsing and broad regression coverage for user, global, no-appid, and subpath configurations. Sequence diagram for prefixed DConfig cache migrationsequenceDiagram
participant Caller
participant DConfigCacheImpl
participant LegacyCache
participant PrefixedCache
participant SavePath
Caller->>DConfigCacheImpl: load(localPrefix)
DConfigCacheImpl->>LegacyCache: loadCache(legacyCacheDir, legacyKeys)
DConfigCacheImpl->>PrefixedCache: loadCache(newCacheDir, newKeys)
PrefixedCache-->>DConfigCacheImpl: overlay prefixed values
DConfigCacheImpl->>DConfigCacheImpl: mark cacheChanged for legacy-only keys
Caller->>DConfigCacheImpl: save(localPrefix)
DConfigCacheImpl->>SavePath: save merged values under prefixed cache path
Flow diagram for legacy and prefixed cache resolutionflowchart TD
A[load cache] --> B{cache prefix set?}
B -->|No| C[load prefixed path]
B -->|Yes| D[load legacy path first]
D --> E{legacy cache valid?}
E -->|Invalid| F[continue with prefixed cache]
E -->|Loaded| G[retain legacy values]
F --> H[load prefixed path]
G --> H
H --> I{prefixed key exists?}
I -->|Yes| J[use prefixed value]
I -->|No| K[keep legacy value and mark changed]
J --> L[save merged values to prefixed path]
K --> L
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 1 issue
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="src/dconfigfile.cpp" line_range="393-394" />
<code_context>
+ if (tmp == QLatin1String("readwrite")) {
p = DConfigFile::ReadWrite;
+ } else if (tmp == QLatin1String("authorizedreadonly")) {
+ p = DConfigFile::AuthorizedReadOnly;
+ } else if (tmp == QLatin1String("authorizedreadwrite")) {
+ p = DConfigFile::AuthorizedReadWrite;
+ }
</code_context>
<issue_to_address>
**🚨 issue (security):** Metadata entries with `permissions: "authorizedreadonly"` are parsed as `AuthorizedReadOnly`, but `cacheValue()` only rejects exactly `ReadOnly`; cached values therefore override the default for authorized-read-only settings just like writable settings. The public cache access path has no authorization check, so unauthorized callers can use the cache to override these settings.
**Triggers:** When a configuration item uses `authorizedreadonly` metadata and a cache contains a value for it.
**Suggested fix:** Handle `AuthorizedReadOnly` separately in `cacheValue()` and enforce the intended authorization policy before accepting a cached override.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| p = DConfigFile::AuthorizedReadOnly; | ||
| } else if (tmp == QLatin1String("authorizedreadwrite")) { |
There was a problem hiding this comment.
🚨 issue (security): Metadata entries with permissions: "authorizedreadonly" are parsed as AuthorizedReadOnly, but cacheValue() only rejects exactly ReadOnly; cached values therefore override the default for authorized-read-only settings just like writable settings. The public cache access path has no authorization check, so unauthorized callers can use the cache to override these settings.
Triggers: When a configuration item uses authorizedreadonly metadata and a cache contains a value for it.
Suggested fix: Handle AuthorizedReadOnly separately in cacheValue() and enforce the intended authorization policy before accepting a cached override.
Summary
Validation
-j6)-j6)No migration state file or claimed-key tracking is introduced.
Summary by Sourcery
Preserve backward compatibility for legacy DConfig caches while adding authorized permission parsing and comprehensive prefixed-cache coverage.
New Features:
Bug Fixes:
Enhancements:
Documentation:
Tests:
Chores: