Skip to content

Commit 58a1197

Browse files
committed
fix: address PR review — parameter order and Discover() scope
1. Fix SetSource parameter order: updateUrl first, appSecretKey second (was swapped by earlier linter edit, causing confusion with docs). 2. Move AppMetadataDiscoverer.Discover() from SetConfig() into SetSource(). SetConfig is for explicit user-provided config — manifest must not override user values. SetSource is the zero-config path where manifest fills identity gaps. 3. Align ClientTest call site to new parameter order.
1 parent ec016d9 commit 58a1197

2 files changed

Lines changed: 15 additions & 16 deletions

File tree

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

Lines changed: 14 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -214,18 +214,16 @@ private async Task<GeneralUpdateBootstrap> LaunchWithStrategy(IStrategy roleStra
214214
}
215215

216216
/// <summary>
217-
/// Applies the primary configuration object. Missing identity fields are
218-
/// auto-discovered from <c>generalupdate.manifest.json</c> or defaults.
219-
/// Validates required fields, maps to the internal <see cref="GlobalConfigInfo"/>,
220-
/// and initialises the blacklist matcher for file exclusion during update operations.
217+
/// Applies the primary configuration object. Validates required fields, maps to
218+
/// the internal <see cref="GlobalConfigInfo"/>, and initialises the blacklist
219+
/// matcher for file exclusion during update operations.
221220
/// </summary>
222-
/// <param name="configInfo">User-facing configuration. Secrets (<c>UpdateUrl</c>,
223-
/// <c>AppSecretKey</c>, <c>Token</c>) must be set. Identity fields are auto-filled
224-
/// when omitted.</param>
221+
/// <param name="configInfo">User-facing configuration. All required fields must
222+
/// be set explicitly — no auto-discovery is performed. Use
223+
/// <see cref="SetSource"/> for the zero-config path.</param>
225224
/// <returns>This bootstrap instance for chaining.</returns>
226225
public GeneralUpdateBootstrap SetConfig(Configinfo configInfo)
227226
{
228-
configInfo = AppMetadataDiscoverer.Discover(configInfo);
229227
configInfo.Validate();
230228
_configInfo = ConfigurationMapper.MapToGlobalConfigInfo(configInfo);
231229

@@ -280,25 +278,27 @@ public GeneralUpdateBootstrap SetConfig(string filePath)
280278
}
281279

282280
/// <summary>
283-
/// Minimal entry-point: supply only secrets. Identity metadata is auto-discovered
281+
/// Zero-config entry-point: supply only secrets. Identity metadata is auto-discovered
284282
/// from <c>generalupdate.manifest.json</c> and hard-coded defaults.
285283
/// </summary>
286284
/// <returns>This bootstrap instance for chaining.</returns>
287285
public GeneralUpdateBootstrap SetSource(
288-
string appSecretKey,
289286
string updateUrl,
290-
string? reportUrl = null,
287+
string appSecretKey,
291288
string? token = null,
292-
string? scheme = null)
289+
string? scheme = null,
290+
string? reportUrl = null)
293291
{
294-
return SetConfig(new Configinfo
292+
var config = new Configinfo
295293
{
296294
UpdateUrl = updateUrl,
297295
AppSecretKey = appSecretKey,
298296
Token = token,
299297
Scheme = scheme,
300298
ReportUrl = reportUrl
301-
});
299+
};
300+
config = AppMetadataDiscoverer.Discover(config);
301+
return SetConfig(config);
302302
}
303303

304304
/// <summary>

tests/ClientTest/Program.cs

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,9 +20,8 @@
2020

2121
// Identity metadata (MainAppName, ClientVersion, UpdateAppName, UpdatePath, …)
2222
// is auto-discovered from generalupdate.manifest.json (generated by GeneralUpdate.Tools).
23-
// SetSource params: (appSecretKey, updateUrl, reportUrl?, token?, scheme?)
2423
await new GeneralUpdateBootstrap()
25-
.SetSource(appSecretKey, updateUrl, reportUrl)
24+
.SetSource(updateUrl, appSecretKey, reportUrl: reportUrl)
2625
.Option(UpdateOptions.AppType, AppType.Client)
2726
.Hooks<ClientTestHooks>()
2827
.AddListenerMultiDownloadStatistics(OnDownloadStatistics)

0 commit comments

Comments
 (0)