Skip to content

Commit 40a8c9e

Browse files
CopilotJusterZhu
Copilot
and
JusterZhu
authored
[REFACTOR] Merge update-info notification and skip-update into a single unified callback (#172)
* Initial plan * refactor: merge update info notification and skip update into AddListenerUpdatePrecheck Agent-Logs-Url: https://github.com/GeneralLibrary/GeneralUpdate/sessions/5fb26d12-e6f1-482b-bb3b-3dd19e5ccb67 Co-authored-by: JusterZhu <11714536+JusterZhu@users.noreply.github.com> * refactor: remove obsolete SetCustomSkipOption and related legacy skip logic Agent-Logs-Url: https://github.com/GeneralLibrary/GeneralUpdate/sessions/f88841d6-ede2-441d-8617-0b96af0eccaa Co-authored-by: JusterZhu <11714536+JusterZhu@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: JusterZhu <11714536+JusterZhu@users.noreply.github.com>
1 parent 85fb1ba commit 40a8c9e

2 files changed

Lines changed: 116 additions & 32 deletions

File tree

‎src/c#/ClientCoreTest/Bootstrap/GeneralClientBootstrapTests.cs‎

Lines changed: 96 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -72,24 +72,6 @@ public void SetConfig_WithNullConfig_ValidationBehavior()
7272
Assert.NotNull(bootstrap); // Bootstrap instance is valid for testing
7373
}
7474

75-
/// <summary>
76-
/// Tests that SetCustomSkipOption properly sets the skip function.
77-
/// </summary>
78-
[Fact]
79-
public void SetCustomSkipOption_WithValidFunc_ReturnsBootstrap()
80-
{
81-
// Arrange
82-
var bootstrap = new GeneralClientBootstrap();
83-
Func<bool> skipFunc = () => false;
84-
85-
// Act
86-
var result = bootstrap.SetCustomSkipOption(skipFunc);
87-
88-
// Assert
89-
Assert.NotNull(result);
90-
Assert.Same(bootstrap, result); // Fluent interface
91-
}
92-
9375
/// <summary>
9476
/// Tests that AddCustomOption adds custom options correctly.
9577
/// </summary>
@@ -141,6 +123,101 @@ public void AddCustomOption_WithEmptyList_HasAssertionCheck()
141123
Assert.True(exceptionThrown || !exceptionThrown);
142124
}
143125

126+
/// <summary>
127+
/// Tests that AddListenerUpdatePrecheck returns the bootstrap instance for chaining.
128+
/// </summary>
129+
[Fact]
130+
public void AddListenerUpdatePrecheck_WithCallback_ReturnsBootstrap()
131+
{
132+
// Arrange
133+
var bootstrap = new GeneralClientBootstrap();
134+
Func<UpdateInfoEventArgs, bool> precheckFunc = (updateInfo) => false;
135+
136+
// Act
137+
var result = bootstrap.AddListenerUpdatePrecheck(precheckFunc);
138+
139+
// Assert
140+
Assert.NotNull(result);
141+
Assert.Same(bootstrap, result); // Fluent interface
142+
}
143+
144+
/// <summary>
145+
/// Tests that AddListenerUpdatePrecheck callback returning true signals a skip (do not update).
146+
/// </summary>
147+
[Fact]
148+
public void AddListenerUpdatePrecheck_CallbackReturnsTrue_SignalsSkip()
149+
{
150+
// Arrange
151+
var bootstrap = new GeneralClientBootstrap();
152+
var callbackInvoked = false;
153+
Func<UpdateInfoEventArgs, bool> precheckFunc = (updateInfo) =>
154+
{
155+
callbackInvoked = true;
156+
Assert.NotNull(updateInfo); // updateInfo should be provided
157+
return true; // true = skip the update
158+
};
159+
160+
// Act
161+
bootstrap.AddListenerUpdatePrecheck(precheckFunc);
162+
163+
// Assert
164+
Assert.NotNull(bootstrap);
165+
// Callback will be invoked during LaunchAsync when update info is available
166+
Assert.False(callbackInvoked); // Not invoked at registration time
167+
}
168+
169+
/// <summary>
170+
/// Tests that AddListenerUpdatePrecheck callback returning false signals proceed (do update).
171+
/// </summary>
172+
[Fact]
173+
public void AddListenerUpdatePrecheck_CallbackReturnsFalse_SignalsProceed()
174+
{
175+
// Arrange
176+
var bootstrap = new GeneralClientBootstrap();
177+
Func<UpdateInfoEventArgs, bool> precheckFunc = (updateInfo) =>
178+
{
179+
Assert.NotNull(updateInfo);
180+
return false; // false = proceed with the update
181+
};
182+
183+
// Act
184+
var result = bootstrap.AddListenerUpdatePrecheck(precheckFunc);
185+
186+
// Assert
187+
Assert.NotNull(result);
188+
Assert.Same(bootstrap, result);
189+
}
190+
191+
/// <summary>
192+
/// Tests that AddListenerUpdatePrecheck can be chained with other builder methods.
193+
/// </summary>
194+
[Fact]
195+
public void AddListenerUpdatePrecheck_CanBeChained()
196+
{
197+
// Arrange
198+
var bootstrap = new GeneralClientBootstrap();
199+
var config = new Configinfo
200+
{
201+
UpdateUrl = "http://localhost:5000/api/update",
202+
ClientVersion = "1.0.0",
203+
UpgradeClientVersion = "1.0.0",
204+
InstallPath = "/test/path",
205+
AppName = "TestApp.exe",
206+
MainAppName = "TestApp.exe",
207+
AppSecretKey = "test-secret-key"
208+
};
209+
210+
// Act
211+
var result = bootstrap
212+
.SetConfig(config)
213+
.AddListenerUpdatePrecheck((updateInfo) => false)
214+
.AddListenerException((s, e) => { });
215+
216+
// Assert
217+
Assert.NotNull(result);
218+
Assert.Same(bootstrap, result);
219+
}
220+
144221
/// <summary>
145222
/// Tests that event listeners can be added for MultiAllDownloadCompleted.
146223
/// </summary>
@@ -299,7 +376,7 @@ public void FluentInterface_AllowsMethodChaining()
299376
// Act
300377
var result = bootstrap
301378
.SetConfig(config)
302-
.SetCustomSkipOption(() => false)
379+
.AddListenerUpdatePrecheck((updateInfo) => false)
303380
.AddListenerException((s, e) => { });
304381

305382
// Assert

‎src/c#/GeneralUpdate.ClientCore/GeneralClientBootstrap.cs‎

Lines changed: 20 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ public class GeneralClientBootstrap : AbstractBootstrap<GeneralClientBootstrap,
3232
/// </summary>
3333
private GlobalConfigInfo? _configInfo;
3434
private IStrategy? _strategy;
35-
private Func<bool>? _customSkipOption;
35+
private Func<UpdateInfoEventArgs, bool>? _updatePrecheck;
3636
private readonly List<Func<bool>> _customOptions = new();
3737

3838
#region Public Methods
@@ -74,17 +74,21 @@ public GeneralClientBootstrap SetConfig(Configinfo configInfo)
7474
}
7575

7676
/// <summary>
77-
/// Let the user decide whether to update in the state of non-mandatory update.
77+
/// Registers a callback that is invoked when update information is available.
78+
/// The callback receives the full <see cref="UpdateInfoEventArgs"/> and returns
79+
/// <c>true</c> to skip the update or <c>false</c> to proceed with the automatic upgrade.
80+
/// Built-in forced-update protection is still applied; if any version is marked as
81+
/// forcibly required the callback return value is ignored and the update proceeds.
7882
/// </summary>
7983
/// <param name="func">
80-
/// Custom function ,Custom actions to let users decide whether to update. true update false do not
81-
/// update .
84+
/// A function that receives <see cref="UpdateInfoEventArgs"/> containing complete update
85+
/// details and returns <c>true</c> to skip, or <c>false</c> to proceed with the update.
8286
/// </param>
83-
/// <returns></returns>
84-
public GeneralClientBootstrap SetCustomSkipOption(Func<bool> func)
87+
/// <returns>The current bootstrap instance for fluent method chaining.</returns>
88+
public GeneralClientBootstrap AddListenerUpdatePrecheck(Func<UpdateInfoEventArgs, bool> func)
8589
{
86-
Debug.Assert(func != null);
87-
_customSkipOption = func;
90+
if (func == null) throw new ArgumentNullException(nameof(func));
91+
_updatePrecheck = func;
8892
return this;
8993
}
9094

@@ -164,11 +168,12 @@ private async Task ExecuteWorkflowAsync()
164168
_configInfo.IsUpgradeUpdate = CheckUpgrade(upgradeResp);
165169
_configInfo.IsMainUpdate = CheckUpgrade(mainResp);
166170

167-
EventManager.Instance.Dispatch(this, new UpdateInfoEventArgs(mainResp));
171+
var updateInfoArgs = new UpdateInfoEventArgs(mainResp);
172+
EventManager.Instance.Dispatch(this, updateInfoArgs);
168173

169174
//If the main program needs to be forced to update, the skip will not take effect.
170175
var isForcibly = CheckForcibly(mainResp.Body) || CheckForcibly(upgradeResp.Body);
171-
if (CanSkip(isForcibly)) return;
176+
if (CanSkip(isForcibly, updateInfoArgs)) return;
172177

173178
//black list initialization.
174179
BlackListManager.Instance?.AddBlackFiles(_configInfo.BlackFiles);
@@ -333,11 +338,13 @@ private bool CheckForcibly(List<VersionInfo>? versions)
333338
/// <summary>
334339
/// User decides if update is required.
335340
/// </summary>
336-
/// <returns>is false to continue execution.</returns>
337-
private bool CanSkip(bool isForcibly)
341+
/// <param name="isForcibly">Whether the update is forcibly required; if true, skip is never allowed.</param>
342+
/// <param name="updateInfo">Update information passed to the precheck callback.</param>
343+
/// <returns>true to skip (abort) the update; false to continue execution.</returns>
344+
private bool CanSkip(bool isForcibly, UpdateInfoEventArgs updateInfo)
338345
{
339346
if (isForcibly) return false;
340-
return _customSkipOption?.Invoke() == true;
347+
return _updatePrecheck?.Invoke(updateInfo) == true;
341348
}
342349

343350
private void CallSmallBowlHome(string processName)

0 commit comments

Comments
 (0)