Skip to content

Commit 232c6d0

Browse files
deansheatherdeansheather
committed
Spike comments
1 parent 4bba070 commit 232c6d0

File tree

4 files changed

+113
-56
lines changed

4 files changed

+113
-56
lines changed

‎App/App.xaml.cs‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -91,6 +91,7 @@ public App()
9191
services.AddSingleton<ICoderApiClientFactory, CoderApiClientFactory>();
9292
services.AddSingleton<IAgentApiClientFactory, AgentApiClientFactory>();
9393

94+
services.AddSingleton<IDispatcherQueueManager, AppDispatcherQueueManager>();
9495
services.AddSingleton<ICredentialBackend>(_ =>
9596
new WindowsCredentialBackend(WindowsCredentialBackend.CoderCredentialsTargetName));
9697
services.AddSingleton<ICredentialManager, CredentialManager>();
@@ -291,7 +292,7 @@ public void OnActivated(object? sender, AppActivationArguments args)
291292
public void HandleNotification(AppNotificationManager? sender, AppNotificationActivatedEventArgs args)
292293
{
293294
_logger.LogInformation("handled notification activation: {Argument}", args.Argument);
294-
_userNotifier.HandleActivation(args);
295+
_userNotifier.HandleNotificationActivation(args.Arguments);
295296
}
296297

297298
private static void AddDefaultConfig(IConfigurationBuilder builder)

‎App/Services/DispatcherQueueManager.cs‎

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
using System;
2+
using Microsoft.UI.Dispatching;
3+
using Microsoft.UI.Xaml;
4+
5+
namespace Coder.Desktop.App.Services;
6+
7+
public interface IDispatcherQueueManager
8+
{
9+
public void RunInUiThread(DispatcherQueueHandler action);
10+
}
11+
12+
public class AppDispatcherQueueManager : IDispatcherQueueManager
13+
{
14+
private static DispatcherQueue? DispatcherQueue =>
15+
((App)Application.Current).TrayWindow?.DispatcherQueue;
16+
17+
public void RunInUiThread(DispatcherQueueHandler action)
18+
{
19+
var dispatcherQueue = DispatcherQueue;
20+
if (dispatcherQueue is null)
21+
throw new InvalidOperationException("DispatcherQueue is not available");
22+
if (dispatcherQueue.HasThreadAccess)
23+
{
24+
action();
25+
return;
26+
}
27+
dispatcherQueue.TryEnqueue(action);
28+
}
29+
}

‎App/Services/UpdateController.cs‎

Lines changed: 30 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -40,10 +40,12 @@ public static string ChannelName(this UpdateChannel channel)
4040

4141
public class UpdaterConfig
4242
{
43-
public bool EnableUpdater { get; set; } = true;
44-
[Required] public string UpdateAppCastUrl { get; set; } = "https://releases.coder.com/coder-desktop/windows/appcast.xml";
45-
[Required] public string UpdatePublicKeyBase64 { get; set; } = "NNWN4c+3PmMuAf2G1ERLlu0EwhzHfSiUugOt120hrH8=";
46-
public UpdateChannel? ForcedUpdateChannel { get; set; } = null;
43+
public bool Enable { get; set; } = true;
44+
[Required] public string AppCastUrl { get; set; } = "https://releases.coder.com/coder-desktop/windows/appcast.xml";
45+
[Required] public string PublicKeyBase64 { get; set; } = "NNWN4c+3PmMuAf2G1ERLlu0EwhzHfSiUugOt120hrH8=";
46+
// This preference forces an update channel to be used and prevents the
47+
// user from picking their own channel.
48+
public UpdateChannel? ForcedChannel { get; set; } = null;
4749
}
4850

4951
public interface IUpdateController : IAsyncDisposable
@@ -52,23 +54,29 @@ public interface IUpdateController : IAsyncDisposable
5254
public Task CheckForUpdatesNow();
5355
}
5456

55-
public class SparkleUpdateController : IUpdateController
57+
public class SparkleUpdateController : IUpdateController, INotificationHandler
5658
{
59+
internal const string NotificationHandlerName = "SparkleUpdateNotification";
60+
5761
private static readonly TimeSpan UpdateCheckInterval = TimeSpan.FromHours(24);
5862

5963
private readonly ILogger<SparkleUpdateController> _logger;
6064
private readonly UpdaterConfig _config;
65+
private readonly IUserNotifier _userNotifier;
6166
private readonly IUIFactory _uiFactory;
6267

6368
private readonly SparkleUpdater? _sparkle;
6469

65-
public SparkleUpdateController(ILogger<SparkleUpdateController> logger, IOptions<UpdaterConfig> config, IUIFactory uiFactory)
70+
public SparkleUpdateController(ILogger<SparkleUpdateController> logger, IOptions<UpdaterConfig> config, IUserNotifier userNotifier, IUIFactory uiFactory)
6671
{
6772
_logger = logger;
6873
_config = config.Value;
74+
_userNotifier = userNotifier;
6975
_uiFactory = uiFactory;
7076

71-
if (!_config.EnableUpdater)
77+
_userNotifier.RegisterHandler(NotificationHandlerName, this);
78+
79+
if (!_config.Enable)
7280
{
7381
_logger.LogInformation("updater disabled by policy");
7482
return;
@@ -83,10 +91,10 @@ public SparkleUpdateController(ILogger<SparkleUpdateController> logger, IOptions
8391
// but we use this functionality on Windows for added security against
8492
// malicious release notes.
8593
var checker = new Ed25519Checker(SecurityMode.Strict,
86-
publicKey: _config.UpdatePublicKeyBase64,
94+
publicKey: _config.PublicKeyBase64,
8795
readFileBeingVerifiedInChunks: true);
8896

89-
_sparkle = new SparkleUpdater(_config.UpdateAppCastUrl, checker)
97+
_sparkle = new SparkleUpdater(_config.AppCastUrl, checker)
9098
{
9199
// TODO: custom Configuration for persistence, could just specify
92100
// our own save path with JSONConfiguration TBH
@@ -95,7 +103,7 @@ public SparkleUpdateController(ILogger<SparkleUpdateController> logger, IOptions
95103
// the URL instead.
96104
CheckServerFileName = false,
97105
LogWriter = new CoderSparkleLogger(logger),
98-
AppCastHelper = new CoderSparkleAppCastHelper(_config.ForcedUpdateChannel),
106+
AppCastHelper = new CoderSparkleAppCastHelper(_config.ForcedChannel),
99107
UIFactory = uiFactory,
100108
UseNotificationToast = uiFactory.CanShowToastMessages(),
101109
RelaunchAfterUpdate = true,
@@ -149,6 +157,11 @@ public ValueTask DisposeAsync()
149157
_sparkle?.Dispose();
150158
return ValueTask.CompletedTask;
151159
}
160+
161+
public void HandleNotificationActivation(IDictionary<string, string> args)
162+
{
163+
_ = CheckForUpdatesNow();
164+
}
152165
}
153166

154167
public class CoderSparkleLogger(ILogger<SparkleUpdateController> logger) : SparkleLogger
@@ -268,12 +281,15 @@ bool IUIFactory.CanShowToastMessages()
268281

269282
void IUIFactory.ShowToast(Action clickHandler)
270283
{
271-
userNotifier.ShowActionNotification(
284+
// We disregard the Action passed to us by NetSparkle as it uses cached
285+
// data and does not perform a new update check. The
286+
// INotificationHandler is registered by SparkleUpdateController.
287+
_ = userNotifier.ShowActionNotification(
272288
"Coder Desktop",
273289
"Updates are available, click for more information.",
274-
clickHandler,
275-
CancellationToken.None)
276-
.Wait();
290+
SparkleUpdateController.NotificationHandlerName,
291+
null,
292+
CancellationToken.None);
277293
}
278294

279295
void IUIFactory.Shutdown()

‎App/Services/UserNotifier.cs‎

Lines changed: 52 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -1,94 +1,105 @@
11
using System;
22
using System.Collections.Concurrent;
3+
using System.Collections.Generic;
34
using System.Threading;
45
using System.Threading.Tasks;
56
using Microsoft.Extensions.Logging;
6-
using Microsoft.UI.Xaml;
77
using Microsoft.Windows.AppNotifications;
88
using Microsoft.Windows.AppNotifications.Builder;
99

1010
namespace Coder.Desktop.App.Services;
1111

12-
public interface IUserNotifier : IAsyncDisposable
12+
public interface INotificationHandler
1313
{
14-
public Task ShowErrorNotification(string title, string message, CancellationToken ct = default);
15-
public Task ShowActionNotification(string title, string message, Action action, CancellationToken ct = default);
14+
public void HandleNotificationActivation(IDictionary<string, string> args);
15+
}
16+
17+
public interface IUserNotifier : INotificationHandler, IAsyncDisposable
18+
{
19+
public void RegisterHandler(string name, INotificationHandler handler);
1620

17-
public void HandleActivation(AppNotificationActivatedEventArgs args);
21+
public Task ShowErrorNotification(string title, string message, CancellationToken ct = default);
22+
public Task ShowActionNotification(string title, string message, string handlerName, IDictionary<string, string>? args = null, CancellationToken ct = default);
1823
}
1924

20-
public class UserNotifier(ILogger<UserNotifier> logger) : IUserNotifier
25+
public class UserNotifier(ILogger<UserNotifier> logger, IDispatcherQueueManager dispatcherQueueManager) : IUserNotifier
2126
{
22-
private const string CoderNotificationId = "CoderNotificationId";
27+
private const string CoderNotificationHandler = "CoderNotificationHandler";
2328

2429
private readonly AppNotificationManager _notificationManager = AppNotificationManager.Default;
2530

26-
public ConcurrentDictionary<string, Action> ActionHandlers { get; } = new();
31+
private ConcurrentDictionary<string, INotificationHandler> Handlers { get; } = new();
2732

2833
public ValueTask DisposeAsync()
2934
{
3035
return ValueTask.CompletedTask;
3136
}
3237

38+
public void RegisterHandler(string name, INotificationHandler handler)
39+
{
40+
if (handler is null)
41+
throw new ArgumentNullException(nameof(handler));
42+
if (handler is IUserNotifier)
43+
throw new ArgumentException("Handler cannot be an IUserNotifier", nameof(handler));
44+
if (string.IsNullOrWhiteSpace(name))
45+
throw new ArgumentException("Name cannot be null or whitespace", nameof(name));
46+
if (!Handlers.TryAdd(name, handler))
47+
throw new InvalidOperationException($"A handler with the name '{name}' is already registered.");
48+
}
49+
3350
public Task ShowErrorNotification(string title, string message, CancellationToken ct = default)
3451
{
3552
var builder = new AppNotificationBuilder().AddText(title).AddText(message);
3653
_notificationManager.Show(builder.BuildNotification());
3754
return Task.CompletedTask;
3855
}
3956

40-
public Task ShowActionNotification(string title, string message, Action action, CancellationToken ct = default)
57+
public Task ShowActionNotification(string title, string message, string handlerName, IDictionary<string, string>? args = null, CancellationToken ct = default)
4158
{
42-
var id = Guid.NewGuid().ToString();
43-
var notification = new AppNotificationBuilder()
59+
if (!Handlers.TryGetValue(handlerName, out _))
60+
{
61+
logger.LogWarning("no action handler found for notification with name {HandlerName}, ignoring", handlerName);
62+
return Task.CompletedTask;
63+
}
64+
65+
var builder = new AppNotificationBuilder()
4466
.AddText(title)
4567
.AddText(message)
46-
.AddArgument(CoderNotificationId, id)
47-
.BuildNotification();
48-
ActionHandlers[id] = action;
49-
_notificationManager.Show(notification);
68+
.AddArgument(CoderNotificationHandler, handlerName);
69+
if (args != null)
70+
foreach (var arg in args)
71+
{
72+
if (arg.Key == CoderNotificationHandler)
73+
continue;
74+
builder.AddArgument(arg.Key, arg.Value);
75+
}
76+
77+
_notificationManager.Show(builder.BuildNotification());
5078
return Task.CompletedTask;
5179
}
5280

53-
public void HandleActivation(AppNotificationActivatedEventArgs args)
81+
public void HandleNotificationActivation(IDictionary<string, string> args)
5482
{
55-
// Must not be an Action notification.
56-
if (!args.Arguments.TryGetValue(CoderNotificationId, out var id))
83+
if (!args.TryGetValue(CoderNotificationHandler, out var handlerName))
84+
// Not an action notification, ignore
5785
return;
5886

59-
if (!ActionHandlers.TryRemove(id, out var action))
87+
if (!Handlers.TryGetValue(handlerName, out var handler))
6088
{
61-
logger.LogWarning("no action handler found for notification with ID {NotificationId}, ignoring", id);
89+
logger.LogWarning("no action handler '{HandlerName}' found for notification activation, ignoring", handlerName);
6290
return;
6391
}
6492

65-
var dispatcherQueue = ((App)Application.Current).TrayWindow?.DispatcherQueue;
66-
if (dispatcherQueue == null)
67-
{
68-
logger.LogError("could not acquire DispatcherQueue for notification event handling, is TrayWindow active?");
69-
return;
70-
}
71-
if (!dispatcherQueue.HasThreadAccess)
72-
{
73-
dispatcherQueue.TryEnqueue(RunAction);
74-
return;
75-
}
76-
77-
RunAction();
78-
79-
return;
80-
81-
void RunAction()
93+
dispatcherQueueManager.RunInUiThread(() =>
8294
{
8395
try
8496
{
85-
action();
97+
handler.HandleNotificationActivation(args);
8698
}
8799
catch (Exception ex)
88100
{
89-
logger.LogWarning(ex, "could not handle activation for notification with ID {NotificationId}", id);
101+
logger.LogWarning(ex, "could not handle activation for notification with handler '{HandlerName}", handlerName);
90102
}
91-
}
103+
});
92104
}
93-
94105
}

0 commit comments

Comments
 (0)