refactor: Unify OSS update path through OSSUpdateStrategy#414
Merged
Conversation
Move client-side OSS logic (version config download, upgrade check, process launch) from LaunchOssAsync() into OSSUpdateStrategy. The strategy now handles both client and upgrade OSS roles internally. - OSSUpdateStrategy.ExecuteAsync(): detects role via GlobalConfigInfoOSS env var, dispatches to ExecuteClientAsync() or ExecuteUpgradeAsync() accordingly - GeneralUpdateBootstrap: AppType.OSS now goes through LaunchWithStrategy(new OSSUpdateStrategy()) — consistent with Client and Upgrade paths - Deleted LaunchOssAsync(), DownloadOssFile(), IsOssUpgrade() from GeneralUpdateBootstrap Closes #409
Contributor
There was a problem hiding this comment.
Pull request overview
This PR refactors the OSS update workflow so both OSS “client-side” and “upgrade-side” execution is routed through OSSUpdateStrategy, removing the separate OSS path previously implemented in GeneralUpdateBootstrap. This aligns OSS behavior with the existing strategy-based dispatch used for Client and Upgrade, including consistent hook/reporter wiring via LaunchWithStrategy.
Changes:
- Moved the client-side OSS flow (download versions config → version check → launch upgrader → exit) into
OSSUpdateStrategy. - Updated
OSSUpdateStrategy.ExecuteAsync()to detect role viaGlobalConfigInfoOSSIPC and dispatch to client vs upgrade execution. - Updated
GeneralUpdateBootstrapsoAppType.OSSusesLaunchWithStrategy(new OSSUpdateStrategy())and removed the legacy OSS methods.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
src/c#/GeneralUpdate.Core/Strategy/OSSUpdateStrategy.cs |
Adds client-side OSS execution and role dispatch within the strategy, consolidating OSS flows. |
src/c#/GeneralUpdate.Core/Bootstrap/GeneralUpdateBootstrap.cs |
Routes AppType.OSS through LaunchWithStrategy and deletes the duplicated OSS bootstrap code path. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
54
to
+70
| if (_configInfo == null) | ||
| throw new InvalidOperationException("OSSUpdateStrategy not configured. Call Create() first."); | ||
|
|
||
| // Upgrade side: GlobalConfigInfoOSS env var was set by the client, | ||
| // so we download packages, decompress, and start the main app. | ||
| var ossJson = Environments.GetEnvironmentVariable("GlobalConfigInfoOSS"); | ||
| if (!string.IsNullOrWhiteSpace(ossJson)) | ||
| { | ||
| await ExecuteUpgradeAsync(); | ||
| return; | ||
| } | ||
|
|
||
| // Client side: download version config, check for new version, | ||
| // start the upgrade process, and exit. | ||
| await ExecuteClientAsync(); | ||
| } | ||
|
|
Comment on lines
+88
to
+90
| var versions = JsonSerializer.Deserialize( | ||
| File.ReadAllText(versionsFilePath), | ||
| JsonContext.VersionOSSJsonContext.Default.ListVersionOSS); |
Comment on lines
+131
to
+140
| private static void DownloadOssVersionFile(string url, string path) | ||
| { | ||
| if (File.Exists(path)) | ||
| { | ||
| File.SetAttributes(path, FileAttributes.Normal); | ||
| File.Delete(path); | ||
| } | ||
| using var httpClient = new HttpClient(); | ||
| var bytes = httpClient.GetByteArrayAsync(url).GetAwaiter().GetResult(); | ||
| File.WriteAllBytes(path, bytes); |
| using System.IO; | ||
| using System.Linq; | ||
| using System.Net.Http; | ||
| using System.Runtime.InteropServices; |
…split OSS mode no longer needs a separate upgrade process. The strategy downloads version config, downloads packages from OSS, decompresses, starts the main app, and exits — all in one process. - Removed client/upgrade env-var-based dispatch from OSSUpdateStrategy - Single ExecuteAsync() flow: download config -> download packages -> decompress -> start app -> exit - Removed GlobalConfigInfoOSS env var bridging (no longer needed) Related #409
OSS mode uses a separate upgrade process to download packages and decompress, but does NOT check UpgradeClientVersion (the upgrade process itself never needs upgrading — differs from standard flow). Client side (main app): Download version config → check update → start upgrade process → exit Upgrade side (GeneralUpdate.Upgrade.exe): Read version config → download OSS packages → decompress → start main app Related #409
Replace AppType.OSS with explicit OSSClient/OSSUpgrade to match the Client/Upgrade pattern. OSSUpdateStrategy now accepts AppType via constructor instead of detecting role via GlobalConfigInfoOSS env var. - AppType enum: OSS = 3 -> OSSClient = 3, OSSUpgrade = 4 - Bootstrap dispatch: OSSClient/OSSUpgrade through LaunchWithStrategy - OSSUpdateStrategy: role dispatched by constructor AppType parameter - Updated tests for new enum values Closes #409
The upgrade side (ExecuteUpgradeAsync) reads version config directly from the local JSON file — it never reads GlobalConfigInfoOSS env var. Writing it in ExecuteClientAsync was dead code. Removed ossConfig + Environments.SetEnvironmentVariable block. Related #409
DownloadVersionConfig calls HttpClient which throws InvalidOperationException when UpdateUrl is empty/null. Skip the download when UpdateUrl is not configured, then check for local version config file existence as before. Fixes OssIntegrationTests.OSSUpdateStrategy_RequiresConfig test. Related #409
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
Removes the dual OSS code path by merging \LaunchOssAsync()\ from \GeneralUpdateBootstrap\ into \OSSUpdateStrategy, making the dispatch consistent across all \AppType\ values.
Changes
OSSUpdateStrategy
GeneralUpdateBootstrap
Benefits
Closes #409