Skip to content

Commit b5ce7e1

Browse files
authored
Merge pull request #486 from GeneralLibrary/fix/oss-update-flow-crash-and-loop
fix: OSS update flow crash & infinite loop in ClientTest/UpgradeTest
2 parents b8a7cef + f6bc318 commit b5ce7e1

7 files changed

Lines changed: 287 additions & 60 deletions

File tree

src/c#/GeneralUpdate.Core/Bootstrap/GeneralUpdateBootstrap.cs

Lines changed: 39 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -53,18 +53,33 @@ namespace GeneralUpdate.Core;
5353
/// components, network policies, and OS strategy.</para>
5454
/// </remarks>
5555
/// <example>
56+
/// <para><b>Standard flow (Client):</b></para>
5657
/// <code>
5758
/// var result = await new GeneralUpdateBootstrap()
58-
/// .SetConfig(new UpdateRequest {
59-
/// UpdateUrl = "https://api.example.com",
60-
/// ClientVersion = "1.0.0",
61-
/// InstallPath = @"C:\MyApp",
62-
/// AppSecretKey = "my-key"
63-
/// })
59+
/// .SetSource("https://api.example.com", "my-key")
6460
/// .SetOption(Option.AppType, AppType.Client)
6561
/// .Hooks&lt;MyCustomHooks&gt;()
6662
/// .LaunchAsync();
6763
/// </code>
64+
/// <para><b>OSS flow (Object Storage Service):</b></para>
65+
/// <code>
66+
/// // OssClient — only secrets in code; identity from generalupdate.manifest.json
67+
/// var result = await new GeneralUpdateBootstrap()
68+
/// .SetSource("https://oss.example.com/versions.json", "my-key")
69+
/// .SetOption(Option.AppType, AppType.OssClient)
70+
/// .Hooks&lt;MyCustomHooks&gt;()
71+
/// .LaunchAsync();
72+
///
73+
/// // OssUpgrade — running from a subdirectory; pass installPath to locate
74+
/// // generalupdate.manifest.json in the parent directory
75+
/// var installPath = Path.GetFullPath(Path.Combine(baseDir, ".."));
76+
/// var result = await new GeneralUpdateBootstrap()
77+
/// .SetSource("https://oss.example.com/versions.json", "my-key",
78+
/// installPath: installPath)
79+
/// .SetOption(Option.AppType, AppType.OssUpgrade)
80+
/// .Hooks&lt;MyCustomHooks&gt;()
81+
/// .LaunchAsync();
82+
/// </code>
6883
/// </example>
6984
public class GeneralUpdateBootstrap : AbstractBootstrap<GeneralUpdateBootstrap, IStrategy>
7085
{
@@ -232,15 +247,29 @@ public GeneralUpdateBootstrap SetConfig(string filePath)
232247
/// <summary>
233248
/// Zero-config entry-point: supply only secrets. Provides sensible identity defaults
234249
/// that pass <see cref="UpdateRequest.Validate"/>; the real identity metadata is
235-
/// discovered later by <see cref="ClientStrategy"/> from <c>generalupdate.manifest.json</c>.
250+
/// discovered later by <see cref="ClientStrategy"/> or <see cref="OssStrategy"/>
251+
/// from <c>generalupdate.manifest.json</c>. Works with all <see cref="AppType"/>
252+
/// modes (<c>Client</c>, <c>Upgrade</c>, <c>OssClient</c>, <c>OssUpgrade</c>).
236253
/// </summary>
254+
/// <param name="updateUrl">The remote endpoint for update validation or version config retrieval.</param>
255+
/// <param name="appSecretKey">The application secret key used for authentication.</param>
256+
/// <param name="reportUrl">Optional URL for reporting update status back to the server.</param>
257+
/// <param name="scheme">Optional URL scheme override (e.g. <c>"https"</c>).</param>
258+
/// <param name="token">Optional authentication token for API requests.</param>
259+
/// <param name="installPath">
260+
/// Optional override for the installation root directory. When <c>null</c>,
261+
/// defaults to <see cref="AppDomain.CurrentDomain.BaseDirectory"/>. Set this when
262+
/// the current process runs from a subdirectory (e.g. the OSS upgrade process in
263+
/// <c>update/</c>) and <c>generalupdate.manifest.json</c> lives in the parent.
264+
/// </param>
237265
/// <returns>This bootstrap instance for chaining.</returns>
238266
public GeneralUpdateBootstrap SetSource(
239267
string updateUrl,
240268
string appSecretKey,
241269
string? reportUrl = null,
242270
string? scheme = null,
243-
string? token = null)
271+
string? token = null,
272+
string? installPath = null)
244273
{
245274
var config = new UpdateRequest
246275
{
@@ -250,6 +279,8 @@ public GeneralUpdateBootstrap SetSource(
250279
Scheme = scheme,
251280
ReportUrl = reportUrl
252281
};
282+
if (installPath != null)
283+
config.InstallPath = installPath;
253284
return SetConfig(config);
254285
}
255286

src/c#/GeneralUpdate.Core/Configuration/UpdateRequest.cs

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -62,17 +62,8 @@ public void Validate()
6262
if (!string.IsNullOrWhiteSpace(UpdateLogUrl) && !Uri.IsWellFormedUriString(UpdateLogUrl, UriKind.Absolute))
6363
throw new ArgumentException("Invalid UpdateLogUrl");
6464

65-
if (string.IsNullOrWhiteSpace(UpdateAppName))
66-
throw new ArgumentException("UpdateAppName cannot be empty");
67-
68-
if (string.IsNullOrWhiteSpace(MainAppName))
69-
throw new ArgumentException("MainAppName cannot be empty");
70-
7165
if (string.IsNullOrWhiteSpace(AppSecretKey))
7266
throw new ArgumentException("AppSecretKey cannot be empty");
73-
74-
if (string.IsNullOrWhiteSpace(ClientVersion))
75-
throw new ArgumentException("ClientVersion cannot be empty");
7667
}
7768
}
7869
}

src/c#/GeneralUpdate.Core/Strategy/OssStrategy.cs

Lines changed: 83 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -150,7 +150,12 @@ public async Task ExecuteAsync()
150150
if (_configInfo == null)
151151
throw new InvalidOperationException("OssStrategy not configured. Call Create() first.");
152152

153-
// Dispatch by role �?no env-var detection needed.
153+
// Fill missing identity fields from generalupdate.manifest.json,
154+
// making the manifest the single source of configuration across
155+
// all update flows (standard ClientStrategy and OssStrategy).
156+
Configuration.AppMetadataDiscoverer.Discover(_configInfo);
157+
158+
// Dispatch by role — no env-var detection needed.
154159
if (_role == AppType.OssUpgrade)
155160
{
156161
await ExecuteUpgradeAsync();
@@ -189,35 +194,63 @@ private async Task ExecuteClientAsync()
189194
var versionFileName = $"{_configInfo.MainAppName ?? _configInfo.UpdateAppName}_versions.json";
190195
var versionsFilePath = Path.Combine(installPath, versionFileName);
191196

197+
GeneralTracer.Info($"[OssClient] InstallPath={installPath}");
198+
GeneralTracer.Info($"[OssClient] VersionFileName={versionFileName}");
199+
GeneralTracer.Info($"[OssClient] ClientVersion={_configInfo.ClientVersion}");
200+
GeneralTracer.Info($"[OssClient] MainAppName={_configInfo.MainAppName}");
201+
GeneralTracer.Info($"[OssClient] UpdateAppName={_configInfo.UpdateAppName}");
202+
192203
if (!string.IsNullOrEmpty(_configInfo.UpdateUrl))
193204
{
205+
GeneralTracer.Info($"[OssClient] Downloading from {_configInfo.UpdateUrl} ...");
194206
await DownloadVersionConfig(_configInfo.UpdateUrl, versionsFilePath).ConfigureAwait(false);
207+
GeneralTracer.Info($"[OssClient] Downloaded -> {versionsFilePath}");
195208
}
196209

197210
if (!File.Exists(versionsFilePath))
198211
{
199-
GeneralTracer.Info("OssStrategy: version config download failed, aborting.");
212+
GeneralTracer.Info("[OssClient] FAIL: version config file not found after download!");
200213
return;
201214
}
202215

203-
var versions = JsonSerializer.Deserialize(
204-
File.ReadAllText(versionsFilePath),
205-
JsonContext.OssVersionRecordJsonContext.Default.ListOssVersionRecord);
216+
var jsonText = File.ReadAllText(versionsFilePath);
217+
GeneralTracer.Info($"[OssClient] JSON downloaded ({jsonText.Length} chars)");
218+
219+
List<OssVersionRecord> versions;
220+
try
221+
{
222+
versions = JsonSerializer.Deserialize(
223+
jsonText,
224+
JsonContext.OssVersionRecordJsonContext.Default.ListOssVersionRecord);
225+
}
226+
catch (Exception ex)
227+
{
228+
GeneralTracer.Error($"[OssClient] JSON deserialize error: {ex.GetType().Name}: {ex.Message}", ex);
229+
throw;
230+
}
231+
206232
if (versions == null || versions.Count == 0)
207233
{
208-
GeneralTracer.Info("OssStrategy: no versions found, aborting.");
234+
GeneralTracer.Info($"[OssClient] FAIL: no versions found (count={versions?.Count.ToString() ?? "null"})");
209235
return;
210236
}
211237

238+
GeneralTracer.Info($"[OssClient] Deserialized {versions.Count} version(s)");
239+
foreach (var v in versions)
240+
GeneralTracer.Info($" - {v.PacketName} v{v.Version} PubTime={v.PubTime:O}");
241+
212242
versions = versions.OrderByDescending(x => x.PubTime).ToList();
213243
var latest = versions.First();
244+
GeneralTracer.Info($"[OssClient] Latest: {latest.Version} (PubTime={latest.PubTime:O})");
214245

215246
if (!IsOssUpgrade(_configInfo.ClientVersion, latest.Version))
216247
{
217-
GeneralTracer.Info("OssStrategy: no upgrade needed.");
248+
GeneralTracer.Info($"[OssClient] No upgrade needed: {_configInfo.ClientVersion} >= {latest.Version}");
218249
return;
219250
}
220251

252+
GeneralTracer.Info($"[OssClient] Upgrade needed: {_configInfo.ClientVersion} -> {latest.Version}");
253+
221254
// Resolve upgrade exe: prefer UpdatePath, fall back to InstallPath
222255
var upgradeDir = !string.IsNullOrWhiteSpace(_configInfo.UpdatePath)
223256
? (Path.IsPathRooted(_configInfo.UpdatePath)
@@ -228,10 +261,28 @@ private async Task ExecuteClientAsync()
228261
? _configInfo.UpdateAppName
229262
: "GeneralUpdate.Upgrade.exe";
230263
var appPath = Path.Combine(upgradeDir, upgradeAppName);
264+
GeneralTracer.Info($"[OssClient] Resolved upgrade path: {appPath}");
265+
266+
// List exe files in the directory to help diagnose missing file issues
267+
try
268+
{
269+
var dirFiles = Directory.GetFiles(upgradeDir, "*.exe").Select(f => Path.GetFileName(f));
270+
GeneralTracer.Info($"[OssClient] *.exe files in {upgradeDir}: [{string.Join(", ", dirFiles)}]");
271+
}
272+
catch (Exception ex)
273+
{
274+
GeneralTracer.Warn($"[OssClient] Could not list directory {upgradeDir}: {ex.Message}");
275+
}
276+
231277
if (!File.Exists(appPath))
278+
{
279+
GeneralTracer.Error($"[OssClient] FAIL: Upgrade app NOT FOUND at {appPath}");
232280
throw new FileNotFoundException($"Upgrade application not found: {appPath}");
281+
}
233282

283+
GeneralTracer.Info($"[OssClient] Launching upgrade: {appPath}");
234284
Process.Start(appPath);
285+
GeneralTracer.Info("[OssClient] Upgrade launched, exiting.");
235286
await GracefulExit.CurrentProcessAsync().ConfigureAwait(false);
236287
}
237288

@@ -273,15 +324,6 @@ private async Task ExecuteUpgradeAsync()
273324
if (!File.Exists(jsonPath) && DownloadSource == null)
274325
throw new FileNotFoundException($"Version config not found: {jsonPath}");
275326

276-
// Hooks: allow cancellation before download
277-
if (!await SafeOnBeforeUpdateAsync(ctx).ConfigureAwait(false))
278-
{
279-
GeneralTracer.Info("OssStrategy (upgrade): cancelled by hook.");
280-
return;
281-
}
282-
283-
await SafeReportUpdateStartedAsync(ctx).ConfigureAwait(false);
284-
285327
// Build download assets from version config or injected source
286328
List<DownloadAsset> assets;
287329
if (DownloadSource != null)
@@ -314,13 +356,38 @@ private async Task ExecuteUpgradeAsync()
314356
if (assets.Count == 0)
315357
throw new InvalidOperationException("No assets to download.");
316358

359+
// Compute LastVersion deterministically via Version comparison
360+
// so hooks see the correct TargetVersion regardless of source ordering.
361+
_configInfo.LastVersion = assets
362+
.Select(a => new Version(a.Version))
363+
.Max()!
364+
.ToString();
365+
ctx = BuildUpdateContext();
366+
367+
// Hooks: allow cancellation before download. Called after assets are
368+
// built and LastVersion is set so OnBeforeUpdateAsync sees the real
369+
// TargetVersion for decision-making.
370+
if (!await SafeOnBeforeUpdateAsync(ctx).ConfigureAwait(false))
371+
{
372+
GeneralTracer.Info("OssStrategy (upgrade): cancelled by hook.");
373+
return;
374+
}
375+
376+
await SafeReportUpdateStartedAsync(ctx).ConfigureAwait(false);
377+
317378
GeneralTracer.Debug($"OssStrategy (upgrade): downloading {assets.Count} asset(s).");
318379
await DownloadAssetsAsync(assets, installPath).ConfigureAwait(false);
319380

320381
GeneralTracer.Debug("OssStrategy (upgrade): decompressing.");
321382
var encoding = Encoding.GetEncoding(_configInfo?.Encoding?.CodePage ?? Encoding.UTF8.CodePage);
322383
DecompressAssets(assets, installPath, encoding);
323384

385+
// Update generalupdate.manifest.json ClientVersion so the client
386+
// reads the correct version on next startup, preventing infinite loops.
387+
Configuration.ManifestInfo.TryUpdateVersion(
388+
installPath,
389+
clientVersion: _configInfo.LastVersion);
390+
324391
await SafeOnDownloadCompletedAsync(ctx).ConfigureAwait(false);
325392
await SafeOnAfterUpdateAsync(ctx).ConfigureAwait(false);
326393
await SafeReportUpdateAppliedAsync(ctx).ConfigureAwait(false);

tests/ClientTest/Program.cs

Lines changed: 71 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,68 @@
66
using GeneralUpdate.Core.Hooks;
77

88
try
9+
{
10+
await RunOssClientAsync();
11+
/*var isOssMode = args.Length > 0 && args[0] == "--oss";
12+
13+
if (isOssMode)
14+
{
15+
await RunOssClientAsync();
16+
}
17+
else
18+
{
19+
await RunStandardClientAsync();
20+
}*/
21+
}
22+
catch (Exception ex)
23+
{
24+
Console.WriteLine($"FATAL: {ex}");
25+
Console.WriteLine("Press Enter to exit...");
26+
Console.ReadLine();
27+
Environment.Exit(1);
28+
}
29+
30+
Console.WriteLine("Press Enter to exit...");
31+
Console.ReadLine();
32+
33+
// ═══════════════════════════════════════════════════════════════════
34+
// OSS Client mode — version JSON download → version compare → launch upgrade
35+
// ═══════════════════════════════════════════════════════════════════
36+
static async Task RunOssClientAsync()
37+
{
38+
Console.WriteLine("=== GeneralUpdate OSS Client Test ===");
39+
Console.WriteLine($"Started at {DateTime.Now}");
40+
Console.WriteLine($"Running from: {AppDomain.CurrentDomain.BaseDirectory}");
41+
42+
// Only secrets are supplied in code. Identity fields (MainAppName,
43+
// ClientVersion, UpdateAppName, UpdatePath) are read from
44+
// generalupdate.manifest.json by OssStrategy via
45+
// AppMetadataDiscoverer.Discover() — same as the standard flow.
46+
var updateUrl = "http://localhost:5000/packages/versions.json";
47+
var appSecretKey = "dfeb5833-975e-4afb-88f1-6278ee9aeff6";
48+
49+
Console.WriteLine($"UpdateUrl: {updateUrl}");
50+
Console.WriteLine();
51+
52+
await new GeneralUpdateBootstrap()
53+
.SetSource(updateUrl, appSecretKey)
54+
.SetOption(Option.AppType, AppType.OssClient)
55+
.Hooks<ClientTestHooks>()
56+
.AddListenerMultiDownloadStatistics(OnDownloadStatistics)
57+
.AddListenerMultiDownloadCompleted(OnDownloadCompleted)
58+
.AddListenerMultiAllDownloadCompleted(OnAllDownloadCompleted)
59+
.AddListenerMultiDownloadError(OnDownloadError)
60+
.AddListenerException(OnException)
61+
.AddListenerUpdateInfo(OnUpdateInfo)
62+
.LaunchAsync();
63+
64+
Console.WriteLine("OSS Client test completed.");
65+
}
66+
67+
// ═══════════════════════════════════════════════════════════════════
68+
// Standard Client mode — silent poll with IPC handoff to Upgrade
69+
// ═══════════════════════════════════════════════════════════════════
70+
static async Task RunStandardClientAsync()
971
{
1072
Console.WriteLine("=== GeneralUpdate Client Test (Silent Mode) ===");
1173
Console.WriteLine($"Started at {DateTime.Now}");
@@ -47,14 +109,12 @@
47109
Console.WriteLine();
48110

49111
// Keep the process alive so the background poll loop can work.
50-
// When the user presses Ctrl+C or Enter, the process exits and
51-
// ProcessExit fires, which triggers the upgrade launch.
52112
var cts = new CancellationTokenSource();
53113
Console.CancelKeyPress += (_, e) =>
54114
{
55115
Console.WriteLine();
56116
Console.WriteLine("[Shutdown] Ctrl+C pressed. Exiting...");
57-
e.Cancel = true; // Prevent immediate kill — let ProcessExit fire
117+
e.Cancel = true;
58118
cts.Cancel();
59119
};
60120

@@ -67,10 +127,6 @@
67127
// Expected on Ctrl+C — graceful shutdown
68128
}
69129

70-
// Explicitly launch the upgrade process before exiting.
71-
// ProcessExit may not fire reliably in all scenarios (e.g. console Ctrl+C),
72-
// so we call TryLaunchUpgrade() directly as the primary launch path.
73-
// If ProcessExit also fires later, the _updaterStarted guard prevents a double-launch.
74130
Console.WriteLine("[Shutdown] Launching upgrade process...");
75131
if (orchestrator != null && orchestrator.HasPreparedUpdate)
76132
{
@@ -86,11 +142,10 @@
86142

87143
Console.WriteLine("[Shutdown] Client test exiting gracefully.");
88144
}
89-
catch (Exception ex)
90-
{
91-
Console.WriteLine($"FATAL: {ex}");
92-
Environment.Exit(1);
93-
}
145+
146+
// ═══════════════════════════════════════════════════════════════════
147+
// Event handlers (shared across both modes)
148+
// ═══════════════════════════════════════════════════════════════════
94149

95150
static void OnDownloadStatistics(object sender, MultiDownloadStatisticsEventArgs e)
96151
{
@@ -136,6 +191,10 @@ static void OnUpdateInfo(object sender, UpdateInfoEventArgs e)
136191
}
137192
}
138193

194+
// ═══════════════════════════════════════════════════════════════════
195+
// Hooks (shared across both modes)
196+
// ═══════════════════════════════════════════════════════════════════
197+
139198
sealed class ClientTestHooks : IUpdateHooks
140199
{
141200
public async Task<bool> OnBeforeUpdateAsync(HookContext ctx)

0 commit comments

Comments
 (0)