refactor: remove UpgradeMode pass-through field from Core#509
Merged
Conversation
…egration testing - Replace OSS/deprecated silent mode with a single mode-agnostic ClientTest entry point - Remove CLI argument branching (chain/cross-version); the SDK's DownloadPlanBuilder selects CVP vs chain automatically based on server response (IsCrossVersion + FromVersion) - Fix MSBuild CopyUpgradeTest target: stop excluding *.json so UpgradeTest.runtimeconfig.json is bundled as Update.exe, fixing 'hostpolicy.dll not found' launch error - Improve events logging (IsCrossVersion/AppType/FromVersion display in OnUpdateInfo) - Update UpgradeTest with clearer IPC flow documentation Co-Authored-By: Claude <noreply@anthropic.com>
- Replace hard-coded '%TEMP%/...' IPC path with Path.GetTempPath() at runtime - Use OS-agnostic wording for strategy comments (WindowsStrategy → OS strategy) - Use switch expression for AppType display to handle null/unknown values
UpgradeMode was only a pass-through field in GeneralUpdate.Core with no consuming logic. The server-side UpgradeService now always uses the unified response flow (CVP first + chain fallback) instead of three-way branching on UpgradeMode. Changes: - Remove UpgradeMode property from VersionEntry, DownloadAsset - Remove UpgradeMode mapping in HttpDownloadSource and ClientStrategy Co-Authored-By: Claude <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Removes the unused UpgradeMode pass-through field from GeneralUpdate.Core data models and their mappings, and updates the test console apps to exercise the standard/core update flows.
Changes:
- Removed
UpgradeModefromVersionEntryandDownloadAsset, and removed related mapping inHttpDownloadSourceandClientStrategy. - Simplified
ClientTestandUpgradeTestprograms to focus on the standard (non-OSS) workflows and updated console output. - Adjusted
ClientTest.csprojbuild output copying forUpgradeTestartifacts.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/UpgradeTest/Program.cs | Removes OSS-mode path and clarifies standard upgrade/IPC behavior in the test harness. |
| tests/ClientTest/Program.cs | Refactors the client test harness into a single “core update test” flow and updates event output. |
| tests/ClientTest/ClientTest.csproj | Changes which UpgradeTest build artifacts are copied into ClientTest output. |
| src/c#/GeneralUpdate.Core/Strategy/ClientStrategy.cs | Removes UpgradeMode propagation when creating VersionEntry event payloads. |
| src/c#/GeneralUpdate.Core/Download/Sources/HttpDownloadSource.cs | Removes UpgradeMode mapping when converting VersionEntry → DownloadAsset. |
| src/c#/GeneralUpdate.Core/Download/Models/DownloadAsset.cs | Removes UpgradeMode from the public DownloadAsset record. |
| src/c#/GeneralUpdate.Core/Configuration/VersionEntry.cs | Removes the UpgradeMode JSON property from the public VersionEntry DTO. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
8
to
11
| try | ||
| { | ||
| await RunOssClientAsync(); | ||
| /*var isOssMode = args.Length > 0 && args[0] == "--oss"; | ||
|
|
||
| if (isOssMode) | ||
| { | ||
| await RunOssClientAsync(); | ||
| } | ||
| else | ||
| { | ||
| await RunStandardClientAsync(); | ||
| }*/ | ||
| await RunUpdateTestAsync(); | ||
| } |
Comment on lines
+43
to
+47
| var updateUrl = "http://localhost:7391/Upgrade/Verification"; | ||
| var reportUrl = "http://localhost:7391/Upgrade/Report"; | ||
| var appSecretKey = | ||
| Environment.GetEnvironmentVariable("APP_SECRET_KEY") | ||
| ?? "dfeb5833-975e-4afb-88f1-6278ee9aeff6"; |
Comment on lines
164
to
168
| [JsonPropertyName("urlExpireTimeUtc")] | ||
| public DateTime? UrlExpireTimeUtc { get; set; } | ||
|
|
||
| /// <summary> | ||
| /// The upgrade mode: 1 = VersionChain (sequential version upgrades), 2 = CrossVersion (cross-version upgrade). | ||
| /// </summary> | ||
| /// <remarks> | ||
| /// <para> | ||
| /// <list type="bullet"> | ||
| /// <item> | ||
| /// <description><c>1 (VersionChain)</c>: Upgrades through versions sequentially, without skipping intermediate versions.</description> | ||
| /// </item> | ||
| /// <item> | ||
| /// <description><c>2 (CrossVersion)</c>: Directly upgrades from an old version to any newer version.</description> | ||
| /// </item> | ||
| /// </list> | ||
| /// </para> | ||
| /// </remarks> | ||
| [JsonPropertyName("upgradeMode")] | ||
| public int? UpgradeMode { get; set; } | ||
|
|
||
| /// <summary> | ||
| /// Whether this is a cross-version upgrade package. |
Comment on lines
18
to
22
| bool IsForcibly = false, | ||
| bool IsFreeze = false, | ||
| int RecordId = 0, | ||
| int? UpgradeMode = null, | ||
| int? AppType = null, | ||
| string? AuthScheme = null, |
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
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.
Closes #508
UpgradeModeinGeneralUpdate.Coreis a pure pass-through field with zero consuming logic.Changes:
UpgradeModeproperty fromVersionEntry,DownloadAssetHttpDownloadSource.MapVersionEntryandClientStrategyconstructorImpact: None — no consumer in Core.