refactor: Phase 1 — role-based OSS strategy, built into unified Bootstrap (#352)#353
Merged
Merged
Conversation
- Added AppType.OSSApp = 3 for OSS update mode - Created OSSUpdateStrategy (implements IStrategy), absorbs old OSSStrategy logic - Bootstrap now dispatches to OSS on AppType.OSSApp: client-side: download version config, compare, start upgrade process upgrade-side: read GlobalConfigInfoOSS from env, execute OSSUpdateStrategy - Removed legacy files: OSSStrategy.cs, GeneralUpdateOSS.cs, GeneralClientOSS.cs - OSS mode is now fully built-in — no separate entry classes needed Closes #352
Contributor
There was a problem hiding this comment.
Pull request overview
Phase 1 of the “role-based strategy” refactor aiming to consolidate OSS update support into the unified GeneralUpdateBootstrap, replacing the legacy OSS bootstrap/strategy types.
Changes:
- Added
AppType.OSSApp = 3as an OSS update mode identifier. - Introduced
OSSUpdateStrategy : IStrategyto replace the legacyOSSStrategy/ OSS bootstraps. - Removed obsolete OSS entrypoints (
GeneralUpdateOSS,GeneralClientOSS) and legacyOSSStrategy.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| src/c#/GeneralUpdate.Core/Strategy/OSSUpdateStrategy.cs | Adds a new OSS strategy implementation for downloading/decompressing OSS packages and starting the app. |
| src/c#/GeneralUpdate.Core/Strategy/OSSStrategy.cs | Removes the legacy OSS strategy class. |
| src/c#/GeneralUpdate.Core/Configuration/AppType.cs | Adds OSSApp constant for OSS mode. |
| src/c#/GeneralUpdate.Core/Bootstrap/GeneralUpdateOSS.cs | Removes obsolete OSS upgrade bootstrap entrypoint. |
| src/c#/GeneralUpdate.Core/Bootstrap/GeneralUpdateBootstrap.cs | Minimal diff shown (BOM), but file is expected by PR description to host OSS dispatch. |
| src/c#/GeneralUpdate.Core/Bootstrap/GeneralClientOSS.cs | Removes obsolete OSS client bootstrap entrypoint. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+27
to
+34
| private GlobalConfigInfo? _configInfo; | ||
| private readonly string _appPath = AppDomain.CurrentDomain.BaseDirectory; | ||
| private const int TimeOut = 60; | ||
|
|
||
| public void Create(GlobalConfigInfo parameter) | ||
| { | ||
| _configInfo = parameter ?? throw new ArgumentNullException(nameof(parameter)); | ||
| } |
Comment on lines
+43
to
+49
| var versionFileName = $"{_configInfo.MainAppName ?? _configInfo.AppName}_versions.json"; | ||
|
|
||
| GeneralTracer.Debug("OSSUpdateStrategy: 1. Reading version configuration file."); | ||
| var jsonPath = Path.Combine(_appPath, versionFileName); | ||
| if (!File.Exists(jsonPath)) | ||
| throw new FileNotFoundException(jsonPath); | ||
|
|
Comment on lines
+79
to
+90
| public void StartApp() | ||
| { | ||
| var appName = _configInfo?.MainAppName ?? _configInfo?.AppName; | ||
| if (string.IsNullOrEmpty(appName)) return; | ||
|
|
||
| var appPath = Path.Combine(_appPath, appName); | ||
| if (!File.Exists(appPath)) | ||
| throw new FileNotFoundException($"Application not found: {appPath}"); | ||
|
|
||
| Process.Start(appPath); | ||
| GeneralTracer.Debug("OSSUpdateStrategy: application started."); | ||
| } |
Comment on lines
+1
to
4
| using System; | ||
| using System.Collections.Generic; | ||
| using System.Diagnostics; | ||
| using System.IO; |
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
Phase 1 of the v2 role-based strategy refactoring. Built-in OSS mode through the unified Bootstrap.
Changes
Added
AppType.OSSApp = 3— OSS update mode identifierOSSUpdateStrategy : IStrategy— absorbs old OSSStrategy, implements standard strategy interfaceUpdated
GeneralUpdateBootstrap.LaunchAsync()— new AppType.OSSApp dispatch:GlobalConfigInfoOSSfrom environment, executesOSSUpdateStrategyDownloadOssFile(),IsOssUpgrade()Removed (obsolete → absorbed)
OSSStrategy.cs— replaced byOSSUpdateStrategyGeneralUpdateOSS.cs— functionality moved toLaunchOssAsync()GeneralClientOSS.cs— functionality moved toLaunchOssAsync()OSS usage after this PR
Verification
Up next (Phase 1b)
Extract ClientUpdateStrategy and UpgradeUpdateStrategy from Bootstrap's inline methods.
Closes #352