Skip to content

Commit 6501260

Browse files
JusterZhuJusterZhu
authored
Merge pull request #482 from GeneralLibrary/refactor/move-manifest-discovery-to-strategy
refactor: move manifest discovery from Bootstrap to ClientStrategy
2 parents 0447f28 + 7d6d195 commit 6501260

6 files changed

Lines changed: 50 additions & 52 deletions

File tree

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -223,8 +223,9 @@ public GeneralUpdateBootstrap SetConfig(string filePath)
223223
}
224224

225225
/// <summary>
226-
/// Zero-config entry-point: supply only secrets. Identity metadata is auto-discovered
227-
/// from <c>generalupdate.manifest.json</c> and hard-coded defaults.
226+
/// Zero-config entry-point: supply only secrets. Provides sensible identity defaults
227+
/// that pass <see cref="UpdateRequest.Validate"/>; the real identity metadata is
228+
/// discovered later by <see cref="ClientStrategy"/> from <c>generalupdate.manifest.json</c>.
228229
/// </summary>
229230
/// <returns>This bootstrap instance for chaining.</returns>
230231
public GeneralUpdateBootstrap SetSource(
@@ -242,7 +243,6 @@ public GeneralUpdateBootstrap SetSource(
242243
Scheme = scheme,
243244
ReportUrl = reportUrl
244245
};
245-
config = AppMetadataDiscoverer.Discover(config);
246246
return SetConfig(config);
247247
}
248248

‎src/c#/GeneralUpdate.Core/Configuration/AppMetadataDiscoverer.cs‎

Lines changed: 27 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -9,42 +9,37 @@ namespace GeneralUpdate.Core.Configuration;
99
public static class AppMetadataDiscoverer
1010
{
1111
/// <summary>
12-
/// Discover and fill every null-or-empty identity field in <paramref name="seed"/>.
12+
/// Discover and fill every null-or-empty identity field in <paramref name="context"/>
13+
/// from <c>generalupdate.manifest.json</c>. Called by <see cref="ClientStrategy"/>
14+
/// during <c>ExecuteStandardWorkflowAsync</c> so that manifest discovery is owned
15+
/// by the strategy rather than the bootstrap.
1316
/// </summary>
14-
public static UpdateRequest Discover(UpdateRequest seed)
17+
public static void Discover(UpdateContext context)
1518
{
16-
if (seed == null)
17-
throw new System.ArgumentNullException(nameof(seed));
19+
if (context == null)
20+
throw new System.ArgumentNullException(nameof(context));
1821

19-
var manifest = ManifestInfo.Load();
22+
var installPath = context.InstallPath;
23+
var manifestPath = System.IO.Path.Combine(installPath, ManifestInfo.FileName);
24+
var manifest = ManifestInfo.Load(manifestPath);
2025

21-
// Identity fields — manifest overrides defaults, not just nulls.
22-
// (UpdateConfiguration pre-fills UpdateAppName with "Update.exe", so simple
23-
// null-coalescing would never pick up the manifest value.)
24-
if (manifest != null)
25-
{
26-
if (!string.IsNullOrWhiteSpace(manifest.MainAppName))
27-
seed.MainAppName = manifest.MainAppName;
28-
if (!string.IsNullOrWhiteSpace(manifest.UpdateAppName))
29-
seed.UpdateAppName = manifest.UpdateAppName;
30-
if (!string.IsNullOrWhiteSpace(manifest.ClientVersion))
31-
seed.ClientVersion = manifest.ClientVersion;
32-
if (!string.IsNullOrWhiteSpace(manifest.UpgradeClientVersion))
33-
seed.UpgradeClientVersion = manifest.UpgradeClientVersion;
34-
if (!string.IsNullOrWhiteSpace(manifest.ProductId))
35-
seed.ProductId = manifest.ProductId;
36-
if (!string.IsNullOrWhiteSpace(manifest.UpdatePath))
37-
seed.UpdatePath = manifest.UpdatePath;
38-
if (!string.IsNullOrWhiteSpace(manifest.AppType)
39-
&& Enum.TryParse<AppType>(manifest.AppType, out var at))
40-
seed.AppType = at;
41-
}
26+
if (manifest == null) return;
4227

43-
// Hard-coded fallbacks only when neither seed nor manifest provided a value.
44-
seed.MainAppName ??= "Client";
45-
seed.UpdateAppName ??= "Update.exe";
46-
seed.InstallPath ??= AppDomain.CurrentDomain.BaseDirectory;
47-
48-
return seed;
28+
// Only fill empty fields — caller-provided values take precedence.
29+
if (string.IsNullOrWhiteSpace(context.MainAppName) && !string.IsNullOrWhiteSpace(manifest.MainAppName))
30+
context.MainAppName = manifest.MainAppName;
31+
if (string.IsNullOrWhiteSpace(context.UpdateAppName) && !string.IsNullOrWhiteSpace(manifest.UpdateAppName))
32+
context.UpdateAppName = manifest.UpdateAppName;
33+
if (string.IsNullOrWhiteSpace(context.ClientVersion) && !string.IsNullOrWhiteSpace(manifest.ClientVersion))
34+
context.ClientVersion = manifest.ClientVersion;
35+
if (string.IsNullOrWhiteSpace(context.UpgradeClientVersion) && !string.IsNullOrWhiteSpace(manifest.UpgradeClientVersion))
36+
context.UpgradeClientVersion = manifest.UpgradeClientVersion;
37+
if (string.IsNullOrWhiteSpace(context.ProductId) && !string.IsNullOrWhiteSpace(manifest.ProductId))
38+
context.ProductId = manifest.ProductId;
39+
if (string.IsNullOrWhiteSpace(context.UpdatePath) && !string.IsNullOrWhiteSpace(manifest.UpdatePath))
40+
context.UpdatePath = manifest.UpdatePath;
41+
if (context.AppType == null && !string.IsNullOrWhiteSpace(manifest.AppType)
42+
&& Enum.TryParse<AppType>(manifest.AppType, out var at))
43+
context.AppType = at;
4944
}
5045
}

‎src/c#/GeneralUpdate.Core/Configuration/UpdateConfiguration.cs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ public abstract class UpdateConfiguration
4949
/// In <see cref="ConfigurationMapper.MapToProcessContract" />, this value is mapped to the
5050
/// <see cref="ProcessContract.AppName" /> property.
5151
/// </remarks>
52-
public string MainAppName { get; set; }
52+
public string MainAppName { get; set; } = "Client";
5353

5454
/// <summary>
5555
/// The installation path of the application files.
@@ -108,7 +108,7 @@ public abstract class UpdateConfiguration
108108
/// Comparing <c>ClientVersion</c> against the latest version from the server determines whether the
109109
/// main application needs updating (<see cref="UpdateContext.IsMainUpdate" />).
110110
/// </remarks>
111-
public string ClientVersion { get; set; }
111+
public string ClientVersion { get; set; } = "1.0.0.0";
112112

113113
/// <summary>
114114
/// A list of specific files to exclude from the update process.

‎src/c#/GeneralUpdate.Core/Strategy/ClientStrategy.cs‎

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -398,6 +398,14 @@ private async Task ExecuteStandardWorkflowAsync()
398398
GeneralTracer.Info(
399399
$"ClientStrategy: validating client={_configInfo!.ClientVersion}, upgrade={_configInfo.UpgradeClientVersion}");
400400

401+
// Discover identity metadata from the manifest in InstallPath BEFORE
402+
// constructing HttpDownloadSource so the server call sees real values.
403+
// UpdateStrategy writes versions back to InstallPath after each update;
404+
// using the parameterless Load() would read from BaseDirectory instead
405+
// and could pick up a stale manifest when InstallPath is customized.
406+
// Only fills empty fields — caller-provided values take precedence.
407+
AppMetadataDiscoverer.Discover(_configInfo!);
408+
401409
// Use injected DownloadSource (Hub/HTTP), or default to HttpDownloadSource
402410
var downloadSource = DownloadSource ?? new Download.Sources.HttpDownloadSource(
403411
_configInfo.UpdateUrl,
@@ -412,15 +420,8 @@ private async Task ExecuteStandardWorkflowAsync()
412420
// Call server validation — returns assets from the two Validate calls
413421
var sourceResult = await downloadSource.ListAsync().ConfigureAwait(false);
414422

415-
// Read local manifest from the install path, not BaseDirectory.
416-
// UpdateStrategy writes versions back to InstallPath after each update;
417-
// using the parameterless Load() would read from BaseDirectory instead
418-
// and could pick up a stale manifest when InstallPath is customized.
419-
// Caller's explicit value takes precedence; manifest is a fallback.
420-
var installPath = _configInfo!.InstallPath;
421-
var manifest = ManifestInfo.Load(Path.Combine(installPath, ManifestInfo.FileName));
422-
var localClientVersion = _configInfo.ClientVersion ?? manifest?.ClientVersion;
423-
var localUpgradeVersion = _configInfo.UpgradeClientVersion ?? manifest?.UpgradeClientVersion;
423+
var localClientVersion = _configInfo!.ClientVersion;
424+
var localUpgradeVersion = _configInfo.UpgradeClientVersion ?? localClientVersion;
424425

425426
// Pre-resolve upgrade version with client-version fallback so that
426427
// HasUpdate and Build agree on the same version. Build internally

‎tests/CoreTest/Bootstrap/ParameterMatrixAndEventTests.cs‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -196,7 +196,8 @@ public void Configinfo_Validate_MissingMainAppName_Throws()
196196
{
197197
UpdateUrl = "https://api.example.com",
198198
ClientVersion = "1.0.0",
199-
AppSecretKey = "key"
199+
AppSecretKey = "key",
200+
MainAppName = null!
200201
};
201202

202203
Assert.Throws<ArgumentException>(() => config.Validate());
@@ -209,7 +210,8 @@ public void Configinfo_Validate_MissingClientVersion_Throws()
209210
{
210211
UpdateUrl = "https://api.example.com",
211212
MainAppName = "MyApp.exe",
212-
AppSecretKey = "key"
213+
AppSecretKey = "key",
214+
ClientVersion = null!
213215
};
214216

215217
Assert.Throws<ArgumentException>(() => config.Validate());

‎tests/CoreTest/Configuration/UpdateConfigurationTests.cs‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -18,10 +18,10 @@ public void Ctor_AppName_DefaultsToUpdateExe()
1818
}
1919

2020
[Fact]
21-
public void Ctor_MainAppName_DefaultsToNull()
21+
public void Ctor_MainAppName_DefaultsToClient()
2222
{
2323
var config = new TestableConfig();
24-
Assert.Null(config.MainAppName);
24+
Assert.Equal("Client", config.MainAppName);
2525
}
2626

2727
[Fact]
@@ -47,10 +47,10 @@ public void Ctor_AppSecretKey_DefaultsToNull()
4747
}
4848

4949
[Fact]
50-
public void Ctor_ClientVersion_DefaultsToNull()
50+
public void Ctor_ClientVersion_DefaultsToVersion()
5151
{
5252
var config = new TestableConfig();
53-
Assert.Null(config.ClientVersion);
53+
Assert.Equal("1.0.0.0", config.ClientVersion);
5454
}
5555

5656
[Fact]

0 commit comments

Comments
 (0)