Skip to content

feat(app): 全新的公告系统#2786

Open
LinQingYuu wants to merge 6 commits intodevfrom
feat/announcement
Open

feat(app): 全新的公告系统#2786
LinQingYuu wants to merge 6 commits intodevfrom
feat/announcement

Conversation

@LinQingYuu
Copy link
Copy Markdown
Member

@LinQingYuu LinQingYuu commented May 3, 2026

重做了整套公告系统,使其和更新服务解耦

并且扩展支持了按时间和版本进行展示

目前可能还需要修一点 bug,以及配套设施还没落地

Summary by Sourcery

引入一个专用的公告服务,用于根据时间和版本约束获取并展示远程公告,并与更新系统解耦。

新功能:

  • 添加一个由生命周期管理的 AnnouncementService,用于从已配置的服务器获取公告,并通过带主题样式的消息框进行展示。
  • 支持按公告配置客户端版本范围和有效时间窗口的过滤,以控制可见性。
  • 允许为每条公告配置操作,包括打开已验证的 URL,以及永久隐藏特定公告。

增强内容:

  • 定义强类型的公告模型,包括公告级别、跳过条件以及按按钮配置的操作,以标准化公告弹窗的远程配置方式。
Original summary in English

Summary by Sourcery

Introduce a dedicated announcement service that fetches and displays remote announcements based on time and version constraints, decoupled from the update system.

New Features:

  • Add a lifecycle-managed AnnouncementService that retrieves announcements from configured servers and displays them via themed message boxes.
  • Support per-announcement filtering by client version range and valid time window to control visibility.
  • Allow per-announcement actions including opening validated URLs and permanently hiding specific announcements.

Enhancements:

  • Define strongly typed models for announcements, including levels, skip conditions, and per-button operations to standardize remote configuration of announcement dialogs.

@pcl-ce-automation pcl-ce-automation Bot added 🛠️ 等待审查 Pull Request 已完善,等待维护者或负责人进行代码审查 size: L PR 大小评估:大型 labels May 3, 2026
@sourcery-ai
Copy link
Copy Markdown

sourcery-ai Bot commented May 3, 2026

审阅者指南

引入了一个新的、解耦的公告系统,用于从远程服务器获取公告,根据时间和应用版本进行筛选,并以可配置的严重级别和按钮操作显示公告,同时持久化保存用户已忽略的公告。

获取和显示公告的时序图

sequenceDiagram
    actor User
    participant AppLifecycle
    participant AnnouncementService
    participant HttpRequest
    participant AnnouncementServer
    participant MsgBoxWrapper
    participant Browser

    User ->> AppLifecycle: start_application
    AppLifecycle ->> AnnouncementService: _Start()
    loop each_source_in_AnnouncementServerList
        AnnouncementService ->> HttpRequest: GetJsonAsync<List<AnnouncementDetails>>(source)
        HttpRequest ->> AnnouncementServer: HTTP_GET_announcements
        AnnouncementServer -->> HttpRequest: List<AnnouncementDetails>
        HttpRequest -->> AnnouncementService: List<AnnouncementDetails>
        AnnouncementService ->> AnnouncementService: filter_by_time_and_version
        AnnouncementService ->> AnnouncementService: cleanup_invalid_ignored_ids
        loop each_filtered_announcement
            AnnouncementService ->> MsgBoxWrapper: ShowWithCustomButtons(details,title,theme,buttons)
            MsgBoxWrapper -->> User: show_announcement_dialog
            User ->> MsgBoxWrapper: click_button
            MsgBoxWrapper ->> AnnouncementService: invoke_callback(operation,argument)
            alt operation_is_OpenWebSite
                AnnouncementService ->> Browser: Process.Start(uri)
            else operation_is_StopShow
                AnnouncementService ->> AnnouncementService: add_id_to_ignored
            else other_operation
                AnnouncementService ->> AnnouncementService: no_op
            end
        end
    end
    AppLifecycle ->> AnnouncementService: state_change_to_Closing
    AnnouncementService ->> AnnouncementService: serialize_ignored_to_Config
Loading

公告系统的更新类图

classDiagram
    class AnnouncementService {
        <<partial>>
        -string[] _AllowScheme
        -string[] _AnnouncementServerList
        -List~string~ _ignored
        +Task _Start()
        -Action _GetSelectCallback(string operation, string arguments)
        -MsgBoxTheme _GetSelectTheme(AnnouncementLevel level)
    }

    class AnnouncementDetails {
        +string Title
        +string Details
        +int Priority
        +string Id
        +AnnouncementLevel Level
        +string ReleaseDate
        +AnnouncementSkipCondition SkipOn
        +IEnumerable~AnnouncementOperation~ Buttons
    }

    class AnnouncementOperation {
        +string ButtonText
        +string Operation
        +string Argument
    }

    class AnnouncementSkipCondition {
        +string MinVersion
        +string MaxVersion
        +string NotAfter
        +string NotBefore
    }

    class AnnouncementLevel {
        <<enumeration>>
        Lowest
        Medium
        Highest
    }

    AnnouncementService o-- AnnouncementDetails : uses
    AnnouncementDetails o-- AnnouncementSkipCondition : uses
    AnnouncementDetails o-- AnnouncementOperation : uses
    AnnouncementDetails --> AnnouncementLevel : uses
Loading

文件级改动

变更 详情 文件
添加一个由生命周期管理的公告服务,从配置的服务器加载、筛选并显示公告。
  • 将 AnnouncementService 注册为生命周期服务,在应用进入运行状态时启动。
  • 启动时,使用 HttpRequest.GetJsonAsync 从配置的远程服务器获取公告列表。
  • 在 Config.System.HiddenAnnouncement 中维护并持久化已忽略公告 ID 列表,并清理服务器上已不存在的 ID。
  • 按优先级顺序、时间窗口(notBefore/notAfter)以及基于 Basics.VersionName 的应用版本范围筛选公告。
  • 使用 MsgBoxWrapper 显示每一个符合条件的公告,其主题根据公告严重级别推导,并为按钮动态创建与操作绑定的回调。
PCL.Core/App/Essentials/Announcement/AnnouncementService.cs
添加数据模型,用于描述公告内容、严重级别、跳过条件以及按钮操作,并映射为服务器响应的 JSON。
  • 定义 AnnouncementDetails 记录类型,包含标题、详情、优先级、ID、级别、发布日期、跳过条件和按钮列表,并通过 JsonPropertyName 注解用于反序列化。
  • 引入 AnnouncementOperation 记录类型,用于表示来自 JSON 的按钮文本、操作键和参数负载。
  • 引入 AnnouncementLevel 枚举,用于表示严重级别并映射到对话框主题。
  • 定义 AnnouncementSkipCondition 类,用于表示控制公告展示时机的版本和时间条件。
PCL.Core/App/Essentials/Announcement/Models/AnnouncementDetails.cs
PCL.Core/App/Essentials/Announcement/Models/AnnouncementOperation.cs
PCL.Core/App/Essentials/Announcement/Models/AnnouncementLevel.cs
PCL.Core/App/Essentials/Announcement/Models/AnnouncementSkipCondition.cs

提示与命令

与 Sourcery 交互

  • 触发新代码审查: 在 Pull Request 中评论 @sourcery-ai review
  • 继续讨论: 直接回复 Sourcery 的审查评论。
  • 从审查评论生成 GitHub Issue: 通过回复审查评论来让 Sourcery 基于该评论创建一个 issue。你也可以回复审查评论 @sourcery-ai issue 来从该评论创建一个 issue。
  • 生成 Pull Request 标题: 在 Pull Request 标题的任意位置写上 @sourcery-ai,即可随时生成标题。你也可以在 Pull Request 中评论 @sourcery-ai title 来(重新)生成标题。
  • 生成 Pull Request 摘要: 在 Pull Request 正文任意位置写上 @sourcery-ai summary,即可在你想要的位置生成 PR 摘要。你也可以在 Pull Request 中评论 @sourcery-ai summary 来(重新)生成摘要。
  • 生成审阅者指南: 在 Pull Request 中评论 @sourcery-ai guide,即可随时(重新)生成审阅者指南。
  • 解决所有 Sourcery 评论: 在 Pull Request 中评论 @sourcery-ai resolve 来解决所有 Sourcery 评论。如果你已经处理完所有评论且不想再看到它们,这会很有用。
  • 撤销所有 Sourcery 审查: 在 Pull Request 中评论 @sourcery-ai dismiss 来撤销所有现有的 Sourcery 审查。尤其适用于你希望从头开始新一轮审查——别忘了再评论 @sourcery-ai review 来触发新的审查!

自定义你的体验

通过访问你的 控制面板 可以:

  • 启用或禁用审查功能,例如 Sourcery 生成的 Pull Request 摘要、审阅者指南等。
  • 修改审查语言。
  • 添加、删除或编辑自定义审查说明。
  • 调整其他审查设置。

获取帮助

Original review guide in English

Reviewer's Guide

Introduces a new, decoupled announcement system that fetches announcements from remote servers, filters them by time and app version, and displays them with configurable severity and button actions, while persisting user-ignored announcements.

Sequence diagram for fetching and displaying announcements

sequenceDiagram
    actor User
    participant AppLifecycle
    participant AnnouncementService
    participant HttpRequest
    participant AnnouncementServer
    participant MsgBoxWrapper
    participant Browser

    User ->> AppLifecycle: start_application
    AppLifecycle ->> AnnouncementService: _Start()
    loop each_source_in_AnnouncementServerList
        AnnouncementService ->> HttpRequest: GetJsonAsync<List<AnnouncementDetails>>(source)
        HttpRequest ->> AnnouncementServer: HTTP_GET_announcements
        AnnouncementServer -->> HttpRequest: List<AnnouncementDetails>
        HttpRequest -->> AnnouncementService: List<AnnouncementDetails>
        AnnouncementService ->> AnnouncementService: filter_by_time_and_version
        AnnouncementService ->> AnnouncementService: cleanup_invalid_ignored_ids
        loop each_filtered_announcement
            AnnouncementService ->> MsgBoxWrapper: ShowWithCustomButtons(details,title,theme,buttons)
            MsgBoxWrapper -->> User: show_announcement_dialog
            User ->> MsgBoxWrapper: click_button
            MsgBoxWrapper ->> AnnouncementService: invoke_callback(operation,argument)
            alt operation_is_OpenWebSite
                AnnouncementService ->> Browser: Process.Start(uri)
            else operation_is_StopShow
                AnnouncementService ->> AnnouncementService: add_id_to_ignored
            else other_operation
                AnnouncementService ->> AnnouncementService: no_op
            end
        end
    end
    AppLifecycle ->> AnnouncementService: state_change_to_Closing
    AnnouncementService ->> AnnouncementService: serialize_ignored_to_Config
Loading

Updated class diagram for the announcement system

classDiagram
    class AnnouncementService {
        <<partial>>
        -string[] _AllowScheme
        -string[] _AnnouncementServerList
        -List~string~ _ignored
        +Task _Start()
        -Action _GetSelectCallback(string operation, string arguments)
        -MsgBoxTheme _GetSelectTheme(AnnouncementLevel level)
    }

    class AnnouncementDetails {
        +string Title
        +string Details
        +int Priority
        +string Id
        +AnnouncementLevel Level
        +string ReleaseDate
        +AnnouncementSkipCondition SkipOn
        +IEnumerable~AnnouncementOperation~ Buttons
    }

    class AnnouncementOperation {
        +string ButtonText
        +string Operation
        +string Argument
    }

    class AnnouncementSkipCondition {
        +string MinVersion
        +string MaxVersion
        +string NotAfter
        +string NotBefore
    }

    class AnnouncementLevel {
        <<enumeration>>
        Lowest
        Medium
        Highest
    }

    AnnouncementService o-- AnnouncementDetails : uses
    AnnouncementDetails o-- AnnouncementSkipCondition : uses
    AnnouncementDetails o-- AnnouncementOperation : uses
    AnnouncementDetails --> AnnouncementLevel : uses
Loading

File-Level Changes

Change Details Files
Add lifecycle-managed announcement service that loads, filters, and displays announcements from configured servers.
  • Register AnnouncementService as a lifecycle service that starts on app running state.
  • On start, fetch announcement lists from configured remote servers using HttpRequest.GetJsonAsync.
  • Maintain and persist a list of ignored announcement IDs in Config.System.HiddenAnnouncement, cleaning up IDs that no longer exist on the server.
  • Filter announcements by priority ordering, time window (notBefore/notAfter), and app version range derived from Basics.VersionName.
  • Display each eligible announcement using MsgBoxWrapper with a theme derived from announcement severity and dynamically created buttons bound to operations.
PCL.Core/App/Essentials/Announcement/AnnouncementService.cs
Add data models describing announcement content, severity, skip conditions, and button operations, mapped to JSON for server responses.
  • Define AnnouncementDetails record with title, details, priority, id, level, release date, skip conditions, and button list, annotated with JsonPropertyName for deserialization.
  • Introduce AnnouncementOperation record for button text, operation key, and argument payload from JSON.
  • Introduce AnnouncementLevel enum to represent severity levels mapped to dialog themes.
  • Define AnnouncementSkipCondition class to represent version and time conditions controlling when announcements are shown.
PCL.Core/App/Essentials/Announcement/Models/AnnouncementDetails.cs
PCL.Core/App/Essentials/Announcement/Models/AnnouncementOperation.cs
PCL.Core/App/Essentials/Announcement/Models/AnnouncementLevel.cs
PCL.Core/App/Essentials/Announcement/Models/AnnouncementSkipCondition.cs

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

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.

Hey - 我发现了 3 个问题,并给出了一些总体反馈:

  • AnnouncementSkipCondition 中,MaxVersion 的 JSON 属性名是 "Max",而 MinVersion"min";如果后端使用统一的小写命名(例如 max),这会在绑定时默默失败,并可能破坏版本区间逻辑——建议将大小写与实际有效负载对齐。
  • AnnouncementDetails.Priority 上的注释写的是“值越大优先级越高”,但查询使用的是 OrderBy(a => a.Priority),这会让值更小的优先显示;请改用 OrderByDescending,或者澄清该字段的含义,以避免出现反向行为。
  • 版本和 URI 的解析路径(new Version(Basics.VersionName.Split("-")[0])new Uri(arguments))假设数据始终有效,如果服务器返回异常值会抛出异常;建议使用 Version.TryParse/Uri.TryCreate 并提供优雅的降级方案,避免因为单个错误值导致整个公告流程失败。
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
-`AnnouncementSkipCondition` 中,`MaxVersion` 的 JSON 属性名是 `"Max"`,而 `MinVersion``"min"`;如果后端使用统一的小写命名(例如 `max`),这会在绑定时默默失败,并可能破坏版本区间逻辑——建议将大小写与实际有效负载对齐。
- `AnnouncementDetails.Priority` 上的注释写的是“值越大优先级越高”,但查询使用的是 `OrderBy(a => a.Priority)`,这会让值更小的优先显示;请改用 `OrderByDescending`,或者澄清该字段的含义,以避免出现反向行为。
- 版本和 URI 的解析路径(`new Version(Basics.VersionName.Split("-")[0])``new Uri(arguments)`)假设数据始终有效,如果服务器返回异常值会抛出异常;建议使用 `Version.TryParse`/`Uri.TryCreate` 并提供优雅的降级方案,避免因为单个错误值导致整个公告流程失败。

## Individual Comments

### Comment 1
<location path="PCL.Core/App/Essentials/Announcement/AnnouncementService.cs" line_range="48-57" />
<code_context>
+                var invalid = _ignored.Except(response.Select(a => a.Id)).ToList();
+                _ignored.RemoveAll(invalid.Contains);
+                
+                var announcements = response.OrderBy(a => a.Priority).Where(a =>
+                {
+                    var isNotAfterValid = DateTimeOffset.TryParse(a.SkipOn.NotAfter, out var notAfter);
+                    var isNotBeforeValid = DateTimeOffset.TryParse(a.SkipOn.NotBefore, out var notBefore);
+                    var localTime = DateTimeOffset.Now;
+                    if (isNotAfterValid && localTime > notAfter) return false;
+                    if (isNotBeforeValid && localTime < notBefore) return false;
+                    var currentVersion = new Version(Basics.VersionName.Split("-")[0]);
+                    var max = new Version(a.SkipOn.MaxVersion ?? "999.999.999");
+                    var min = new Version(a.SkipOn.MinVersion ?? "0.0.0");
+                    
+                    // [min,max]
</code_context>
<issue_to_address>
**issue (bug_risk):** 版本解析在 `MinVersion`/`MaxVersion` 包含无效或空值时可能会抛出异常。

`new Version(a.SkipOn.MaxVersion ?? "999.999.999")`(以及 `MinVersion` 的对应代码)如果服务器返回空字符串或格式错误的版本字符串,就会抛出异常。建议使用 `Version.TryParse` 并提供默认值,或者在反序列化阶段把空字符串规范化为 `null`,然后再应用这些默认值,以避免整个公告处理因为一个条目出错而中断。
</issue_to_address>

### Comment 2
<location path="PCL.Core/App/Essentials/Announcement/AnnouncementService.cs" line_range="65-68" />
<code_context>
+                });
+                foreach (var detail in announcements)
+                {
+                    Context.Debug(MsgBoxWrapper.ShowWithCustomButtons(
+                        detail.Details, $"{detail.Title} ({detail.ReleaseDate})", _GetSelectTheme(detail.Level),
+                        false,
+                        detail.Buttons.Select(operation => new MsgBoxButtonInfo(operation.ButtonText,
+                            OnClick: _GetSelectCallback(operation.Operation, operation.Argument))).ToArray()).ToString());
+                }
</code_context>
<issue_to_address>
**suggestion:** 某一个公告服务器失败会中止后续服务器的处理。

因为 `foreach (var source in _AnnouncementServerList)` 循环被包裹在一个统一的 `try`/`catch (HttpRequestException)` 中,只要任意一个服务器抛出异常,就会阻止后续服务器被处理。请针对每个服务器单独处理 `HttpRequestException`(例如,在每次迭代外包一层,或者在 `GetJsonAsync` 内部捕获),以避免单个失败的端点阻塞其它端点。

建议实现方式:

```csharp
                foreach (var detail in announcements)
                {
                    Context.Debug(MsgBoxWrapper.ShowWithCustomButtons(
                        detail.Details, $"{detail.Title} ({detail.ReleaseDate})", _GetSelectTheme(detail.Level),
                        false,
                        detail.Buttons.Select(operation => new MsgBoxButtonInfo(operation.ButtonText,
                            OnClick: _GetSelectCallback(operation.Operation, operation.Argument))).ToArray()).ToString());
                }
            }
        }
    }

    private static Action _GetSelectCallback(string operation, string arguments) => operation switch

```

要完整实现你在审查意见中建议的“按服务器分别处理错误”,可以:
1. 在该方法前面找到 `foreach (var source in _AnnouncementServerList)` 循环。
2. 将每个服务器的 HTTP 调用(例如获取该 `source` 公告的 `GetJsonAsync`/`HttpClient` 调用)包裹在各自的 `try`/`catch (HttpRequestException ex)` 块中。
3. 在每个服务器自身的 `catch` 中,使用 `Context.Error("加载公告失败", ex, ActionLevel.HintErr);`(或带上服务器/来源标识的变体)记录失败日志,然后使用 `continue;` 继续处理下一个服务器,而不是中止整个循环。
4. 移除或避免任何仍然包裹整个 `_AnnouncementServerList` 循环的外层 `try`/`catch (HttpRequestException)`,这样单个失败的端点就不会阻止后续端点被处理。
</issue_to_address>

### Comment 3
<location path="PCL.Core/App/Essentials/Announcement/Models/AnnouncementSkipCondition.cs" line_range="9-10" />
<code_context>
+{
+    [JsonPropertyName("min")]
+    public string? MinVersion { get; init; }
+    [JsonPropertyName("Max")]
+    public string? MaxVersion { get; init; }
+    [JsonPropertyName("notAfter")]
+    public string? NotAfter { get; init; }
</code_context>
<issue_to_address>
**issue (bug_risk):** `Max` 与 `min`/`notAfter`/`notBefore` 在 JSON 属性大小写上的不一致可能导致 API 不匹配。

`MaxVersion` 标注为 `[JsonPropertyName("Max")]`,而其他属性使用的是小写名称。如果实际 API 返回的是 `max`(以便与 `min`/`notAfter` 保持一致),`MaxVersion` 将会反序列化为 null。请确认预期的 JSON 大小写形式,并在需要时更新该属性(例如改为 `"max"`),以保证一致性与正确的反序列化行为。
</issue_to_address>

Sourcery 对开源项目免费——如果你觉得我们的评审有帮助,欢迎分享 ✨
帮我变得更有用!请在每条评论上点 👍 或 👎,我会根据你的反馈改进评审质量。
Original comment in English

Hey - I've found 3 issues, and left some high level feedback:

  • In AnnouncementSkipCondition, the JSON property name for MaxVersion is "Max" while MinVersion is "min"; if the backend uses a consistent lower‑case naming (e.g. max), this will silently fail to bind and may break the version range logic—consider aligning the casing with the actual payload.
  • The comment on AnnouncementDetails.Priority says a higher value means higher priority, but the query uses OrderBy(a => a.Priority) which will show lower values first; either switch to OrderByDescending or clarify the meaning of the field to avoid inverted behavior.
  • The version and URI parsing paths (new Version(Basics.VersionName.Split("-")[0]), new Uri(arguments)) assume always-valid data and will throw on unexpected values from the server; consider using Version.TryParse/Uri.TryCreate with graceful fallback to avoid the whole announcement flow failing on a single bad value.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- In `AnnouncementSkipCondition`, the JSON property name for `MaxVersion` is `"Max"` while `MinVersion` is `"min"`; if the backend uses a consistent lower‑case naming (e.g. `max`), this will silently fail to bind and may break the version range logic—consider aligning the casing with the actual payload.
- The comment on `AnnouncementDetails.Priority` says a higher value means higher priority, but the query uses `OrderBy(a => a.Priority)` which will show lower values first; either switch to `OrderByDescending` or clarify the meaning of the field to avoid inverted behavior.
- The version and URI parsing paths (`new Version(Basics.VersionName.Split("-")[0])`, `new Uri(arguments)`) assume always-valid data and will throw on unexpected values from the server; consider using `Version.TryParse`/`Uri.TryCreate` with graceful fallback to avoid the whole announcement flow failing on a single bad value.

## Individual Comments

### Comment 1
<location path="PCL.Core/App/Essentials/Announcement/AnnouncementService.cs" line_range="48-57" />
<code_context>
+                var invalid = _ignored.Except(response.Select(a => a.Id)).ToList();
+                _ignored.RemoveAll(invalid.Contains);
+                
+                var announcements = response.OrderBy(a => a.Priority).Where(a =>
+                {
+                    var isNotAfterValid = DateTimeOffset.TryParse(a.SkipOn.NotAfter, out var notAfter);
+                    var isNotBeforeValid = DateTimeOffset.TryParse(a.SkipOn.NotBefore, out var notBefore);
+                    var localTime = DateTimeOffset.Now;
+                    if (isNotAfterValid && localTime > notAfter) return false;
+                    if (isNotBeforeValid && localTime < notBefore) return false;
+                    var currentVersion = new Version(Basics.VersionName.Split("-")[0]);
+                    var max = new Version(a.SkipOn.MaxVersion ?? "999.999.999");
+                    var min = new Version(a.SkipOn.MinVersion ?? "0.0.0");
+                    
+                    // [min,max]
</code_context>
<issue_to_address>
**issue (bug_risk):** Version parsing can throw if `MinVersion`/`MaxVersion` contain invalid or empty values.

`new Version(a.SkipOn.MaxVersion ?? "999.999.999")` (and the `MinVersion` equivalent) will throw if the server sends an empty or malformed version string. Consider using `Version.TryParse` with a default value, or normalizing empty strings to `null` during deserialization before applying these defaults, to avoid aborting announcement processing.
</issue_to_address>

### Comment 2
<location path="PCL.Core/App/Essentials/Announcement/AnnouncementService.cs" line_range="65-68" />
<code_context>
+                });
+                foreach (var detail in announcements)
+                {
+                    Context.Debug(MsgBoxWrapper.ShowWithCustomButtons(
+                        detail.Details, $"{detail.Title} ({detail.ReleaseDate})", _GetSelectTheme(detail.Level),
+                        false,
+                        detail.Buttons.Select(operation => new MsgBoxButtonInfo(operation.ButtonText,
+                            OnClick: _GetSelectCallback(operation.Operation, operation.Argument))).ToArray()).ToString());
+                }
</code_context>
<issue_to_address>
**suggestion:** A failure on one announcement server aborts processing of subsequent servers.

Because the `foreach (var source in _AnnouncementServerList)` loop is inside a single `try`/`catch (HttpRequestException)`, an exception from any server will prevent later servers from being processed. Please handle `HttpRequestException` per server (e.g., around each iteration or inside `GetJsonAsync`) so one failing endpoint doesn’t block the rest.

Suggested implementation:

```csharp
                foreach (var detail in announcements)
                {
                    Context.Debug(MsgBoxWrapper.ShowWithCustomButtons(
                        detail.Details, $"{detail.Title} ({detail.ReleaseDate})", _GetSelectTheme(detail.Level),
                        false,
                        detail.Buttons.Select(operation => new MsgBoxButtonInfo(operation.ButtonText,
                            OnClick: _GetSelectCallback(operation.Operation, operation.Argument))).ToArray()).ToString());
                }
            }
        }
    }

    private static Action _GetSelectCallback(string operation, string arguments) => operation switch

```

To fully implement per-server error handling as suggested in your review comment, you should:
1. Locate the `foreach (var source in _AnnouncementServerList)` loop earlier in this method.
2. Wrap the HTTP call for each server (e.g., the `GetJsonAsync`/`HttpClient` call that fetches announcements for that `source`) in its own `try`/`catch (HttpRequestException ex)` block.
3. Inside that per-server `catch`, log the failure using `Context.Error("加载公告失败", ex, ActionLevel.HintErr);` (or a variant that includes the server/source identifier) and then `continue;` to proceed to the next server instead of aborting the loop.
4. Remove or avoid any remaining outer `try`/`catch (HttpRequestException)` that wraps the entire `_AnnouncementServerList` loop, so that one failing endpoint does not prevent subsequent servers from being processed.
</issue_to_address>

### Comment 3
<location path="PCL.Core/App/Essentials/Announcement/Models/AnnouncementSkipCondition.cs" line_range="9-10" />
<code_context>
+{
+    [JsonPropertyName("min")]
+    public string? MinVersion { get; init; }
+    [JsonPropertyName("Max")]
+    public string? MaxVersion { get; init; }
+    [JsonPropertyName("notAfter")]
+    public string? NotAfter { get; init; }
</code_context>
<issue_to_address>
**issue (bug_risk):** Inconsistent JSON property casing for `Max` vs `min`/`notAfter`/`notBefore` may cause API mismatch.

`MaxVersion` is annotated with `[JsonPropertyName("Max")]` while the others use lowercase names. If the API actually returns `max` (to match `min`/`notAfter`), `MaxVersion` will deserialize as null. Please confirm the expected JSON casing and update this property (e.g., to `"max"`) if needed for consistency and correct deserialization.
</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.

Comment on lines +48 to +57
var announcements = response.OrderBy(a => a.Priority).Where(a =>
{
var isNotAfterValid = DateTimeOffset.TryParse(a.SkipOn.NotAfter, out var notAfter);
var isNotBeforeValid = DateTimeOffset.TryParse(a.SkipOn.NotBefore, out var notBefore);
var localTime = DateTimeOffset.Now;
if (isNotAfterValid && localTime > notAfter) return false;
if (isNotBeforeValid && localTime < notBefore) return false;
var currentVersion = new Version(Basics.VersionName.Split("-")[0]);
var max = new Version(a.SkipOn.MaxVersion ?? "999.999.999");
var min = new Version(a.SkipOn.MinVersion ?? "0.0.0");
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

issue (bug_risk): 版本解析在 MinVersion/MaxVersion 包含无效或空值时可能会抛出异常。

new Version(a.SkipOn.MaxVersion ?? "999.999.999")(以及 MinVersion 的对应代码)如果服务器返回空字符串或格式错误的版本字符串,就会抛出异常。建议使用 Version.TryParse 并提供默认值,或者在反序列化阶段把空字符串规范化为 null,然后再应用这些默认值,以避免公告处理因为一个条目出错而中断。

Original comment in English

issue (bug_risk): Version parsing can throw if MinVersion/MaxVersion contain invalid or empty values.

new Version(a.SkipOn.MaxVersion ?? "999.999.999") (and the MinVersion equivalent) will throw if the server sends an empty or malformed version string. Consider using Version.TryParse with a default value, or normalizing empty strings to null during deserialization before applying these defaults, to avoid aborting announcement processing.

Comment on lines +65 to +68
Context.Debug(MsgBoxWrapper.ShowWithCustomButtons(
detail.Details, $"{detail.Title} ({detail.ReleaseDate})", _GetSelectTheme(detail.Level),
false,
detail.Buttons.Select(operation => new MsgBoxButtonInfo(operation.ButtonText,
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

suggestion: 某一个公告服务器失败会中止后续服务器的处理。

因为 foreach (var source in _AnnouncementServerList) 循环被包裹在一个统一的 try/catch (HttpRequestException) 中,只要任意一个服务器抛出异常,就会阻止后续服务器被处理。请针对每个服务器单独处理 HttpRequestException(例如,在每次迭代外包一层,或者在 GetJsonAsync 内部捕获),以避免单个失败的端点阻塞其它端点。

建议实现方式:

                foreach (var detail in announcements)
                {
                    Context.Debug(MsgBoxWrapper.ShowWithCustomButtons(
                        detail.Details, $"{detail.Title} ({detail.ReleaseDate})", _GetSelectTheme(detail.Level),
                        false,
                        detail.Buttons.Select(operation => new MsgBoxButtonInfo(operation.ButtonText,
                            OnClick: _GetSelectCallback(operation.Operation, operation.Argument))).ToArray()).ToString());
                }
            }
        }
    }

    private static Action _GetSelectCallback(string operation, string arguments) => operation switch

要完整实现你在审查意见中建议的“按服务器分别处理错误”,可以:

  1. 在该方法前面找到 foreach (var source in _AnnouncementServerList) 循环。
  2. 将每个服务器的 HTTP 调用(例如获取该 source 公告的 GetJsonAsync/HttpClient 调用)包裹在各自的 try/catch (HttpRequestException ex) 块中。
  3. 在每个服务器自身的 catch 中,使用 Context.Error("加载公告失败", ex, ActionLevel.HintErr);(或带上服务器/来源标识的变体)记录失败日志,然后使用 continue; 继续处理下一个服务器,而不是中止整个循环。
  4. 移除或避免任何仍然包裹整个 _AnnouncementServerList 循环的外层 try/catch (HttpRequestException),这样单个失败的端点就不会阻止后续端点被处理。
Original comment in English

suggestion: A failure on one announcement server aborts processing of subsequent servers.

Because the foreach (var source in _AnnouncementServerList) loop is inside a single try/catch (HttpRequestException), an exception from any server will prevent later servers from being processed. Please handle HttpRequestException per server (e.g., around each iteration or inside GetJsonAsync) so one failing endpoint doesn’t block the rest.

Suggested implementation:

                foreach (var detail in announcements)
                {
                    Context.Debug(MsgBoxWrapper.ShowWithCustomButtons(
                        detail.Details, $"{detail.Title} ({detail.ReleaseDate})", _GetSelectTheme(detail.Level),
                        false,
                        detail.Buttons.Select(operation => new MsgBoxButtonInfo(operation.ButtonText,
                            OnClick: _GetSelectCallback(operation.Operation, operation.Argument))).ToArray()).ToString());
                }
            }
        }
    }

    private static Action _GetSelectCallback(string operation, string arguments) => operation switch

To fully implement per-server error handling as suggested in your review comment, you should:

  1. Locate the foreach (var source in _AnnouncementServerList) loop earlier in this method.
  2. Wrap the HTTP call for each server (e.g., the GetJsonAsync/HttpClient call that fetches announcements for that source) in its own try/catch (HttpRequestException ex) block.
  3. Inside that per-server catch, log the failure using Context.Error("加载公告失败", ex, ActionLevel.HintErr); (or a variant that includes the server/source identifier) and then continue; to proceed to the next server instead of aborting the loop.
  4. Remove or avoid any remaining outer try/catch (HttpRequestException) that wraps the entire _AnnouncementServerList loop, so that one failing endpoint does not prevent subsequent servers from being processed.

Comment thread PCL.Core/App/Essentials/Announcement/Models/AnnouncementSkipCondition.cs Outdated
@wtommy932

This comment was marked as off-topic.

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

Labels

size: L PR 大小评估:大型 🛠️ 等待审查 Pull Request 已完善,等待维护者或负责人进行代码审查

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants