Batch 10: OSS switch to new download + delete old files#380
Merged
Conversation
- OSSUpdateStrategy: replace old DownloadManager with DefaultDownloadOrchestrator - Delete SilentUpdateMode.cs — replaced by SilentPollOrchestrator - Delete DownloadManager.cs — replaced by IDownloadOrchestrator - Delete DownloadTask.cs — replaced by DownloadAsset + orchestrator - Clean up stale comments in ClientUpdateStrategy Closes #379
Contributor
There was a problem hiding this comment.
Pull request overview
This PR completes the OSS update-path migration to the newer download abstractions by switching OSSUpdateStrategy from the legacy DownloadManager/DownloadTask implementation to DefaultDownloadOrchestrator, and removes the remaining legacy download/silent-mode implementation files.
Changes:
- Updated
OSSUpdateStrategyto buildDownloadAsset/DownloadPlanand execute downloads viaDefaultDownloadOrchestrator. - Removed legacy silent update implementation (
SilentUpdateMode) and legacy download implementation (DownloadManager,DownloadTask). - Cleaned up outdated comments referencing the removed silent-mode path.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/c#/GeneralUpdate.Core/Strategy/OSSUpdateStrategy.cs | Switch OSS package download flow to DownloadPlan + DefaultDownloadOrchestrator. |
| src/c#/GeneralUpdate.Core/Strategy/ClientUpdateStrategy.cs | Removes stale comment referencing legacy silent update mode. |
| src/c#/GeneralUpdate.Core/Silent/SilentUpdateMode.cs | Deleted legacy silent update mode implementation. |
| src/c#/GeneralUpdate.Core/Download/DownloadTask.cs | Deleted legacy download task implementation. |
| src/c#/GeneralUpdate.Core/Download/DownloadManager.cs | Deleted legacy download manager implementation. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+120
to
+128
| var assets = versions.Select(v => new Download.Models.DownloadAsset( | ||
| Name: v.PacketName ?? v.Version ?? "unknown", | ||
| Url: v.Url ?? string.Empty, | ||
| Size: 0, | ||
| SHA256: v.Hash, | ||
| Version: v.Version ?? "0.0.0" | ||
| )).ToList(); | ||
|
|
||
| var plan = new Download.Models.DownloadPlan(assets, false); |
Comment on lines
+120
to
+127
| var assets = versions.Select(v => new Download.Models.DownloadAsset( | ||
| Name: v.PacketName ?? v.Version ?? "unknown", | ||
| Url: v.Url ?? string.Empty, | ||
| Size: 0, | ||
| SHA256: v.Hash, | ||
| Version: v.Version ?? "0.0.0" | ||
| )).ToList(); | ||
|
|
Comment on lines
+128
to
135
| var plan = new Download.Models.DownloadPlan(assets, false); | ||
|
|
||
| var httpClient = new System.Net.Http.HttpClient(); | ||
| try | ||
| { | ||
| var version = new VersionInfo | ||
| { | ||
| Name = versionInfo.PacketName, | ||
| Version = versionInfo.Version, | ||
| Url = versionInfo.Url, | ||
| Format = Format.ZIP, | ||
| Hash = versionInfo.Hash | ||
| }; | ||
| manager.Add(new DownloadTask(manager, version)); | ||
| var orchestrator = new Download.Orchestrators.DefaultDownloadOrchestrator(httpClient); | ||
| await orchestrator.ExecuteAsync(plan, _appPath).ConfigureAwait(false); | ||
| } |
This was referenced May 24, 2026
JusterZhu
added a commit
that referenced
this pull request
May 24, 2026
Fixes from PR #368 (Batch 4): 1. UpdateInfoEventArgs: make VersionRespDTO nullable to avoid null ref 2. ClientUpdateStrategy: build VersionInfo list from DownloadAssets instead of passing empty list to ProcessInfo constructor 3. ClientUpdateStrategy: use injected _orchestrator if available, fall back to creating one with HttpClient 5. DefaultDownloadOrchestrator: ensure destDir exists (Directory.CreateDirectory) and dispose SemaphoreSlim (using) 6. DownloadPlanBuilder: guard against invalid currentVersion string Fixes from PR #380 (Batch 10): 8. OSSUpdateStrategy: set zip filename to match Decompress() expectations 9. OSSUpdateStrategy: throw clear exception when URL is null/empty 10. OSSUpdateStrategy: set HttpClient.Timeout to 60s to match old behavior Closes #386
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.
Summary
Final cleanup: switches OSS update to the new download abstractions and removes all legacy code.
Changes
OSSUpdateStrategy
Deleted files (627 lines removed)
Build
Closes #379