From 1d3d9df8234e954ff58f7f6043014715e8bce59d Mon Sep 17 00:00:00 2001 From: Dean Sheather Date: Mon, 5 May 2025 15:57:50 +1000 Subject: [PATCH 01/11] feat: add workspace app icons to tray window - Adds AgentAppViewModel to handle each button - Adds collapsible control components to handle the collapsing section - Adds Uuid type to work around issues with the built-in Guid type - Adds ModelMerge utility for merging lists with minimal updates to work around constant flashing in the UI --- App/App.csproj | 5 +- App/App.xaml.cs | 6 + App/Controls/ExpandChevron.xaml | 31 +++ App/Controls/ExpandChevron.xaml.cs | 21 ++ App/Controls/ExpandContent.xaml | 61 +++++ App/Controls/ExpandContent.xaml.cs | 39 +++ App/Converters/DependencyObjectSelector.cs | 13 + App/Models/CredentialModel.cs | 8 +- App/Services/CredentialManager.cs | 37 ++- App/Services/RpcController.cs | 2 +- App/{ => Utils}/DisplayScale.cs | 2 +- App/Utils/ModelMerge.cs | 85 ++++++ App/ViewModels/AgentAppViewModel.cs | 150 +++++++++++ App/ViewModels/AgentViewModel.cs | 272 ++++++++++++++++++- App/ViewModels/TrayWindowViewModel.cs | 109 ++++---- App/Views/DirectoryPickerWindow.xaml.cs | 1 + App/Views/Pages/TrayWindowMainPage.xaml | 284 +++++++++++++------- App/Views/SignInWindow.xaml.cs | 1 + App/Views/TrayWindow.xaml.cs | 1 + App/packages.lock.json | 19 +- CoderSdk/Coder/CoderApiClient.cs | 31 +++ CoderSdk/Coder/WorkspaceAgents.cs | 29 ++ Tests.App/Services/CredentialManagerTest.cs | 6 +- Tests.Vpn.Proto/UuidTest.cs | 139 ++++++++++ Vpn.Proto/Uuid.cs | 175 ++++++++++++ 25 files changed, 1333 insertions(+), 194 deletions(-) create mode 100644 App/Controls/ExpandChevron.xaml create mode 100644 App/Controls/ExpandChevron.xaml.cs create mode 100644 App/Controls/ExpandContent.xaml create mode 100644 App/Controls/ExpandContent.xaml.cs rename App/{ => Utils}/DisplayScale.cs (94%) create mode 100644 App/Utils/ModelMerge.cs create mode 100644 App/ViewModels/AgentAppViewModel.cs create mode 100644 CoderSdk/Coder/WorkspaceAgents.cs create mode 100644 Tests.Vpn.Proto/UuidTest.cs create mode 100644 Vpn.Proto/Uuid.cs diff --git a/App/App.csproj b/App/App.csproj index 982612f..fcfb92f 100644 --- a/App/App.csproj +++ b/App/App.csproj @@ -1,4 +1,4 @@ - + WinExe net8.0-windows10.0.19041.0 @@ -16,7 +16,7 @@ preview - DISABLE_XAML_GENERATED_MAIN + DISABLE_XAML_GENERATED_MAIN;DISABLE_XAML_GENERATED_BREAK_ON_UNHANDLED_EXCEPTION Coder Desktop coder.ico @@ -57,6 +57,7 @@ + all runtime; build; native; contentfiles; analyzers; buildtransitive diff --git a/App/App.xaml.cs b/App/App.xaml.cs index 2c7e87e..d7aea0b 100644 --- a/App/App.xaml.cs +++ b/App/App.xaml.cs @@ -11,6 +11,7 @@ using Coder.Desktop.App.Views; using Coder.Desktop.App.Views.Pages; using Coder.Desktop.CoderSdk.Agent; +using Coder.Desktop.CoderSdk.Coder; using Coder.Desktop.Vpn; using Microsoft.Extensions.Configuration; using Microsoft.Extensions.DependencyInjection; @@ -62,8 +63,11 @@ public App() loggerConfig.ReadFrom.Configuration(builder.Configuration); }); + services.AddSingleton(); services.AddSingleton(); + services.AddSingleton(_ => + new WindowsCredentialBackend(WindowsCredentialBackend.CoderCredentialsTargetName)); services.AddSingleton(); services.AddSingleton(); @@ -90,6 +94,8 @@ public App() services.AddTransient(); services.AddTransient(); services.AddTransient(); + services.AddSingleton(); + services.AddSingleton(); services.AddTransient(); services.AddTransient(); services.AddTransient(); diff --git a/App/Controls/ExpandChevron.xaml b/App/Controls/ExpandChevron.xaml new file mode 100644 index 0000000..75281e7 --- /dev/null +++ b/App/Controls/ExpandChevron.xaml @@ -0,0 +1,31 @@ + + + + + + + + + + + + + + diff --git a/App/Controls/ExpandChevron.xaml.cs b/App/Controls/ExpandChevron.xaml.cs new file mode 100644 index 0000000..0dbd68e --- /dev/null +++ b/App/Controls/ExpandChevron.xaml.cs @@ -0,0 +1,21 @@ +using DependencyPropertyGenerator; +using Microsoft.UI.Xaml.Controls; +using Microsoft.UI.Xaml.Media; + +namespace Coder.Desktop.App.Controls; + +[DependencyProperty("IsOpen", DefaultValue = false)] +[DependencyProperty("Foreground")] +public sealed partial class ExpandChevron : UserControl +{ + public ExpandChevron() + { + InitializeComponent(); + } + + partial void OnIsOpenChanged(bool oldValue, bool newValue) + { + var newState = newValue ? "NormalOn" : "NormalOff"; + AnimatedIcon.SetState(ChevronIcon, newState); + } +} diff --git a/App/Controls/ExpandContent.xaml b/App/Controls/ExpandContent.xaml new file mode 100644 index 0000000..a53ad3a --- /dev/null +++ b/App/Controls/ExpandContent.xaml @@ -0,0 +1,61 @@ + + + + + + + + + + + + + + + + + + Visible + + + + + + + + + + + + + + + + + + diff --git a/App/Controls/ExpandContent.xaml.cs b/App/Controls/ExpandContent.xaml.cs new file mode 100644 index 0000000..1cd5d2f --- /dev/null +++ b/App/Controls/ExpandContent.xaml.cs @@ -0,0 +1,39 @@ +using DependencyPropertyGenerator; +using Microsoft.UI.Xaml; +using Microsoft.UI.Xaml.Controls; +using Microsoft.UI.Xaml.Markup; + +namespace Coder.Desktop.App.Controls; + +[ContentProperty(Name = nameof(Children))] +[DependencyProperty("IsOpen", DefaultValue = false)] +public sealed partial class ExpandContent : UserControl +{ + public UIElementCollection Children => CollapsiblePanel.Children; + + public ExpandContent() + { + InitializeComponent(); + } + + public void CollapseAnimation_Completed(object? sender, object args) + { + // Hide the panel completely when the collapse animation is done. This + // cannot be done with keyframes for some reason. + // + // Without this, the space will still be reserved for the panel. + CollapsiblePanel.Visibility = Visibility.Collapsed; + } + + partial void OnIsOpenChanged(bool oldValue, bool newValue) + { + var newState = newValue ? "ExpandedState" : "CollapsedState"; + + // The animation can't set visibility when starting or ending the + // animation. + if (newValue) + CollapsiblePanel.Visibility = Visibility.Visible; + + VisualStateManager.GoToState(this, newState, true); + } +} diff --git a/App/Converters/DependencyObjectSelector.cs b/App/Converters/DependencyObjectSelector.cs index a31c33b..ec586d0 100644 --- a/App/Converters/DependencyObjectSelector.cs +++ b/App/Converters/DependencyObjectSelector.cs @@ -156,6 +156,15 @@ private void UpdateSelectedObject() ClearValue(SelectedObjectProperty); } + private static void VerifyReferencesProperty(IObservableVector references) + { + // Ensure unique keys and that the values are DependencyObjectSelectorItem. + var items = references.OfType>().ToArray(); + var keys = items.Select(i => i.Key).Distinct().ToArray(); + if (keys.Length != references.Count) + throw new ArgumentException("ObservableCollection Keys must be unique."); + } + // Called when the References property is replaced. private static void ReferencesPropertyChanged(DependencyObject obj, DependencyPropertyChangedEventArgs args) { @@ -166,12 +175,16 @@ private static void ReferencesPropertyChanged(DependencyObject obj, DependencyPr oldValue.VectorChanged -= self.OnVectorChangedReferences; var newValue = args.NewValue as DependencyObjectCollection; if (newValue != null) + { + VerifyReferencesProperty(newValue); newValue.VectorChanged += self.OnVectorChangedReferences; + } } // Called when the References collection changes without being replaced. private void OnVectorChangedReferences(IObservableVector sender, IVectorChangedEventArgs args) { + VerifyReferencesProperty(sender); UpdateSelectedObject(); } diff --git a/App/Models/CredentialModel.cs b/App/Models/CredentialModel.cs index 542c1c0..d30f894 100644 --- a/App/Models/CredentialModel.cs +++ b/App/Models/CredentialModel.cs @@ -1,3 +1,5 @@ +using System; + namespace Coder.Desktop.App.Models; public enum CredentialState @@ -5,10 +7,10 @@ public enum CredentialState // Unknown means "we haven't checked yet" Unknown, - // Invalid means "we checked and there's either no saved credentials or they are not valid" + // Invalid means "we checked and there's either no saved credentials, or they are not valid" Invalid, - // Valid means "we checked and there are saved credentials and they are valid" + // Valid means "we checked and there are saved credentials, and they are valid" Valid, } @@ -16,7 +18,7 @@ public class CredentialModel { public CredentialState State { get; init; } = CredentialState.Unknown; - public string? CoderUrl { get; init; } + public Uri? CoderUrl { get; init; } public string? ApiToken { get; init; } public string? Username { get; init; } diff --git a/App/Services/CredentialManager.cs b/App/Services/CredentialManager.cs index a2f6567..bf90fcc 100644 --- a/App/Services/CredentialManager.cs +++ b/App/Services/CredentialManager.cs @@ -21,7 +21,7 @@ public class RawCredentials [JsonSerializable(typeof(RawCredentials))] public partial class RawCredentialsJsonContext : JsonSerializerContext; -public interface ICredentialManager +public interface ICredentialManager : ICoderApiClientCredentialProvider { public event EventHandler CredentialsChanged; @@ -59,7 +59,8 @@ public interface ICredentialBackend /// public class CredentialManager : ICredentialManager { - private const string CredentialsTargetName = "Coder.Desktop.App.Credentials"; + private readonly ICredentialBackend Backend; + private readonly ICoderApiClientFactory CoderApiClientFactory; // _opLock is held for the full duration of SetCredentials, and partially // during LoadCredentials. _opLock protects _inFlightLoad, _loadCts, and @@ -79,14 +80,6 @@ public class CredentialManager : ICredentialManager // immediate). private volatile CredentialModel? _latestCredentials; - private ICredentialBackend Backend { get; } = new WindowsCredentialBackend(CredentialsTargetName); - - private ICoderApiClientFactory CoderApiClientFactory { get; } = new CoderApiClientFactory(); - - public CredentialManager() - { - } - public CredentialManager(ICredentialBackend backend, ICoderApiClientFactory coderApiClientFactory) { Backend = backend; @@ -108,6 +101,20 @@ public CredentialModel GetCachedCredentials() }; } + // Implements ICoderApiClientCredentialProvider + public CoderApiClientCredential? GetCoderApiClientCredential() + { + var latestCreds = _latestCredentials; + if (latestCreds is not { State: CredentialState.Valid }) + return null; + + return new CoderApiClientCredential + { + CoderUrl = latestCreds.CoderUrl, + ApiToken = latestCreds.ApiToken ?? "", + }; + } + public async Task GetSignInUri() { try @@ -253,6 +260,12 @@ private async Task PopulateModel(RawCredentials? credentials, C State = CredentialState.Invalid, }; + if (!Uri.TryCreate(credentials.CoderUrl, UriKind.Absolute, out var uri)) + return new CredentialModel + { + State = CredentialState.Invalid, + }; + BuildInfo buildInfo; User me; try @@ -279,7 +292,7 @@ private async Task PopulateModel(RawCredentials? credentials, C return new CredentialModel { State = CredentialState.Valid, - CoderUrl = credentials.CoderUrl, + CoderUrl = uri, ApiToken = credentials.ApiToken, Username = me.Username, }; @@ -298,6 +311,8 @@ private void UpdateState(CredentialModel newModel) public class WindowsCredentialBackend : ICredentialBackend { + public const string CoderCredentialsTargetName = "Coder.Desktop.App.Credentials"; + private readonly string _credentialsTargetName; public WindowsCredentialBackend(string credentialsTargetName) diff --git a/App/Services/RpcController.cs b/App/Services/RpcController.cs index 17d3ccb..70dfe9f 100644 --- a/App/Services/RpcController.cs +++ b/App/Services/RpcController.cs @@ -170,7 +170,7 @@ public async Task StartVpn(CancellationToken ct = default) { Start = new StartRequest { - CoderUrl = credentials.CoderUrl, + CoderUrl = credentials.CoderUrl?.ToString(), ApiToken = credentials.ApiToken, }, }, ct); diff --git a/App/DisplayScale.cs b/App/Utils/DisplayScale.cs similarity index 94% rename from App/DisplayScale.cs rename to App/Utils/DisplayScale.cs index cd5101c..7cc79d6 100644 --- a/App/DisplayScale.cs +++ b/App/Utils/DisplayScale.cs @@ -3,7 +3,7 @@ using Microsoft.UI.Xaml; using WinRT.Interop; -namespace Coder.Desktop.App; +namespace Coder.Desktop.App.Utils; /// /// A static utility class to house methods related to the visual scale of the display monitor. diff --git a/App/Utils/ModelMerge.cs b/App/Utils/ModelMerge.cs new file mode 100644 index 0000000..d5a19bc --- /dev/null +++ b/App/Utils/ModelMerge.cs @@ -0,0 +1,85 @@ +using System; +using System.Collections.Generic; +using System.Linq; + +namespace Coder.Desktop.App.Utils; + +public interface IModelMergeable +{ + /// + /// Applies a merge of obj to `this`. + /// + /// + /// True if the two objects represent the same item (e.g. the ID/name + /// matches) and the merge was performed. + /// + public bool ApplyMerge(T obj); +} + +/// +/// A static utility class providing methods for merging model updates with +/// as little UI updates as possible. +/// The main goal of the utilities in this class is to prevent redraws in +/// ItemsRepeater items when nothing has changed. +/// +public static class ModelMerge +{ + /// + /// Merges two observable lists with as little operations as possible + /// to avoid excessive/unncessary UI updates. + /// It's assumed that the target list is already sorted. + /// + public static void MergeLists(IList target, IList update, Comparison sorter) + where T : IModelMergeable + { + var newItems = update.ToList(); + + // Update and remove existing items. We use index-based for loops here + // because we remove items, and removing items while using the list as + // an IEnumerable will throw an exception. + for (var i = 0; i < target.Count; i++) + { + // Even though we're removing items before a "break", we still use + // index-based for loops here to avoid exceptions. + for (var j = 0; j < newItems.Count; j++) + { + if (!target[i].ApplyMerge(newItems[j])) continue; + + // Prevent it from being added below, or checked again. We + // don't need to decrement `j` here because we're breaking + // out of this inner loop. + newItems.RemoveAt(j); + goto OuterLoopEnd; // continue outer loop + } + + // A merge couldn't occur, so we need to remove the old item and + // decrement `i` for the next iteration. + target.Remove(target[i]); + i--; + + OuterLoopEnd: ; + } + + // Add any items that were missing into their correct sorted place. + // It's assumed the list is already sorted. + foreach (var item in newItems) + { + // Perform a binary search. List has BinarySearch(), but + // IList does not. + // + // Inserts after existing equal elements. + var low = 0; + var high = target.Count; + while (low < high) + { + var mid = (low + high) / 2; + if (sorter(item, target[mid]) < 0) + high = mid; + else + low = mid + 1; + } + + target.Insert(low, item); + } + } +} diff --git a/App/ViewModels/AgentAppViewModel.cs b/App/ViewModels/AgentAppViewModel.cs new file mode 100644 index 0000000..25e695e --- /dev/null +++ b/App/ViewModels/AgentAppViewModel.cs @@ -0,0 +1,150 @@ +using System; +using System.Linq; +using Windows.System; +using Coder.Desktop.App.Utils; +using Coder.Desktop.Vpn.Proto; +using CommunityToolkit.Mvvm.ComponentModel; +using CommunityToolkit.Mvvm.Input; +using Microsoft.Extensions.Logging; +using Microsoft.UI.Xaml; +using Microsoft.UI.Xaml.Media; +using Microsoft.UI.Xaml.Media.Imaging; + +namespace Coder.Desktop.App.ViewModels; + +public interface IAgentAppViewModelFactory +{ + public AgentAppViewModel Create(Uuid id, string name, Uri appUri, Uri? iconUrl); +} + +public class AgentAppViewModelFactory : IAgentAppViewModelFactory +{ + private readonly ILogger _childLogger; + + public AgentAppViewModelFactory(ILogger childLogger) + { + _childLogger = childLogger; + } + + public AgentAppViewModel Create(Uuid id, string name, Uri appUri, Uri? iconUrl) + { + return new AgentAppViewModel(_childLogger) + { + Id = id, + Name = name, + AppUri = appUri, + IconUrl = iconUrl, + }; + } +} + +public partial class AgentAppViewModel : ObservableObject, IModelMergeable +{ + // HACK: We need to set the icon size for SVGs otherwise they might get cut + // off. These must be kept in sync with the XAML code. + public const int IconWidth = 20; + public const int IconHeight = 20; + + private readonly ILogger _logger; + + public required Uuid Id { get; init; } + + public required string Name { get; set; } + + [ObservableProperty] + [NotifyPropertyChangedFor(nameof(Details))] + public required partial Uri AppUri { get; set; } + + [ObservableProperty] + [NotifyPropertyChangedFor(nameof(ImageSource))] + public partial Uri? IconUrl { get; set; } + + [ObservableProperty] public partial bool UseFallbackIcon { get; set; } = true; + + public string Details => + (string.IsNullOrWhiteSpace(Name) ? "(no name)" : Name) + ":\n\n" + AppUri; + + public ImageSource ImageSource + { + get + { + if (IconUrl is null || (IconUrl.Scheme != "http" && IconUrl.Scheme != "https")) + { + UseFallbackIcon = true; + return new BitmapImage(); + } + + // Determine what image source to use based on extension, use a + // BitmapImage as last resort. + var ext = IconUrl.AbsolutePath.Split('/').LastOrDefault()?.Split('.').LastOrDefault(); + // TODO: this is definitely a hack, URLs shouldn't need to end in .svg + if (ext is "svg") + { + // TODO: Some SVGs like `/icon/cursor.svg` contain PNG data and + // don't render at all. + var svg = new SvgImageSource(IconUrl) + { + RasterizePixelWidth = IconWidth, + RasterizePixelHeight = IconHeight, + }; + svg.Opened += (_, _) => _logger.LogDebug("app icon opened (svg): {uri}", IconUrl); + svg.OpenFailed += (_, args) => + _logger.LogDebug("app icon failed to open (svg): {uri}: {Status}", IconUrl, args.Status); + return svg; + } + + var bitmap = new BitmapImage(IconUrl); + bitmap.ImageOpened += (_, _) => _logger.LogDebug("app icon opened (bitmap): {uri}", IconUrl); + bitmap.ImageFailed += (_, args) => + _logger.LogDebug("app icon failed to open (bitmap): {uri}: {ErrorMessage}", IconUrl, args.ErrorMessage); + return bitmap; + } + } + + public AgentAppViewModel(ILogger logger) + { + _logger = logger; + } + + public bool ApplyMerge(AgentAppViewModel obj) + { + if (Id != obj.Id) return false; + + // To avoid spurious UI updates which cause flashing, don't actually + // write to values unless they've changed. + if (Name != obj.Name) + Name = obj.Name; + if (AppUri != obj.AppUri) + AppUri = obj.AppUri; + if (IconUrl != obj.IconUrl) + { + UseFallbackIcon = true; + IconUrl = obj.IconUrl; + } + + return true; + } + + public void OnImageOpened(object? sender, RoutedEventArgs e) + { + UseFallbackIcon = false; + } + + public void OnImageFailed(object? sender, RoutedEventArgs e) + { + UseFallbackIcon = true; + } + + [RelayCommand] + private void OpenApp() + { + try + { + _ = Launcher.LaunchUriAsync(AppUri); + } + catch + { + // TODO: log/notify + } + } +} diff --git a/App/ViewModels/AgentViewModel.cs b/App/ViewModels/AgentViewModel.cs index f5b5e0e..7f99b07 100644 --- a/App/ViewModels/AgentViewModel.cs +++ b/App/ViewModels/AgentViewModel.cs @@ -1,11 +1,62 @@ +using System; +using System.Collections.Generic; +using System.Collections.ObjectModel; +using System.ComponentModel; +using System.Linq; +using System.Threading; +using System.Threading.Tasks; using Windows.ApplicationModel.DataTransfer; +using Coder.Desktop.App.Services; +using Coder.Desktop.App.Utils; +using Coder.Desktop.CoderSdk.Coder; +using Coder.Desktop.Vpn.Proto; +using CommunityToolkit.Mvvm.ComponentModel; using CommunityToolkit.Mvvm.Input; +using Microsoft.Extensions.Logging; +using Microsoft.UI.Dispatching; using Microsoft.UI.Xaml; using Microsoft.UI.Xaml.Controls; using Microsoft.UI.Xaml.Controls.Primitives; namespace Coder.Desktop.App.ViewModels; +public interface IAgentViewModelFactory +{ + public AgentViewModel Create(Uuid id, string hostname, string hostnameSuffix, + AgentConnectionStatus connectionStatus, Uri dashboardBaseUrl, string? workspaceName); +} + +public class AgentViewModelFactory : IAgentViewModelFactory +{ + private readonly ILogger _childLogger; + private readonly ICoderApiClientFactory _coderApiClientFactory; + private readonly ICredentialManager _credentialManager; + private readonly IAgentAppViewModelFactory _agentAppViewModelFactory; + + public AgentViewModelFactory(ILogger childLogger, ICoderApiClientFactory coderApiClientFactory, + ICredentialManager credentialManager, IAgentAppViewModelFactory agentAppViewModelFactory) + { + _childLogger = childLogger; + _coderApiClientFactory = coderApiClientFactory; + _credentialManager = credentialManager; + _agentAppViewModelFactory = agentAppViewModelFactory; + } + + public AgentViewModel Create(Uuid id, string hostname, string hostnameSuffix, + AgentConnectionStatus connectionStatus, Uri dashboardBaseUrl, string? workspaceName) + { + return new AgentViewModel(_childLogger, _coderApiClientFactory, _credentialManager, _agentAppViewModelFactory) + { + Id = id, + Hostname = hostname, + HostnameSuffix = hostnameSuffix, + ConnectionStatus = connectionStatus, + DashboardBaseUrl = dashboardBaseUrl, + WorkspaceName = workspaceName, + }; + } +} + public enum AgentConnectionStatus { Green, @@ -14,17 +65,228 @@ public enum AgentConnectionStatus Gray, } -public partial class AgentViewModel +public partial class AgentViewModel : ObservableObject, IModelMergeable { - public required string Hostname { get; set; } + private const string DefaultDashboardUrl = "https://coder.com"; + private const int MaxAppsPerRow = 6; + + private readonly ILogger _logger; + private readonly ICoderApiClientFactory _coderApiClientFactory; + private readonly ICredentialManager _credentialManager; + private readonly IAgentAppViewModelFactory _agentAppViewModelFactory; - public required string HostnameSuffix { get; set; } // including leading dot + // The AgentViewModel only gets created on the UI thread. + private readonly DispatcherQueue _dispatcherQueue = + DispatcherQueue.GetForCurrentThread(); - public required AgentConnectionStatus ConnectionStatus { get; set; } + // This isn't an ObservableProperty because the property itself never + // changes. We add an event listener for the collection changing in the + // constructor. + public readonly ObservableCollection Apps = []; + + public required Uuid Id { get; init; } + + [ObservableProperty] + [NotifyPropertyChangedFor(nameof(FullHostname))] + public required partial string Hostname { get; set; } + + [ObservableProperty] + [NotifyPropertyChangedFor(nameof(FullHostname))] + public required partial string HostnameSuffix { get; set; } // including leading dot public string FullHostname => Hostname + HostnameSuffix; - public required string DashboardUrl { get; set; } + [ObservableProperty] + [NotifyPropertyChangedFor(nameof(ShowExpandAppsMessage))] + [NotifyPropertyChangedFor(nameof(ExpandAppsMessage))] + public required partial AgentConnectionStatus ConnectionStatus { get; set; } + + [ObservableProperty] + [NotifyPropertyChangedFor(nameof(DashboardUrl))] + public required partial Uri DashboardBaseUrl { get; set; } + + [ObservableProperty] + [NotifyPropertyChangedFor(nameof(DashboardUrl))] + public required partial string? WorkspaceName { get; set; } + + [ObservableProperty] public partial bool IsExpanded { get; set; } = false; + + [ObservableProperty] + [NotifyPropertyChangedFor(nameof(ShowExpandAppsMessage))] + [NotifyPropertyChangedFor(nameof(ExpandAppsMessage))] + public partial bool FetchingApps { get; set; } = false; + + [ObservableProperty] + [NotifyPropertyChangedFor(nameof(ShowExpandAppsMessage))] + [NotifyPropertyChangedFor(nameof(ExpandAppsMessage))] + public partial bool AppFetchErrored { get; set; } = false; + + // We only show 6 apps max, which fills the entire width of the tray + // window. + public IEnumerable VisibleApps => Apps.Count > MaxAppsPerRow ? Apps.Take(MaxAppsPerRow) : Apps; + + public bool ShowExpandAppsMessage => ExpandAppsMessage != null; + + public string? ExpandAppsMessage + { + get + { + if (ConnectionStatus == AgentConnectionStatus.Gray) + return "Your workspace is offline."; + if (FetchingApps && Apps.Count == 0) + // Don't show this message if we have any apps already. When + // they finish loading, we'll just update the screen with any + // changes. + return "Fetching workspace apps..."; + if (AppFetchErrored && Apps.Count == 0) + // There's very limited screen real estate here so we don't + // show the actual error message. + return "Could not fetch workspace apps."; + if (Apps.Count == 0) + return "No apps to show."; + return null; + } + } + + public string DashboardUrl + { + get + { + if (string.IsNullOrWhiteSpace(WorkspaceName)) return DashboardBaseUrl.ToString(); + try + { + return new Uri(DashboardBaseUrl, $"/@me/{WorkspaceName}").ToString(); + } + catch + { + return DefaultDashboardUrl; + } + } + } + + public AgentViewModel(ILogger logger, ICoderApiClientFactory coderApiClientFactory, + ICredentialManager credentialManager, IAgentAppViewModelFactory agentAppViewModelFactory) + { + _logger = logger; + _coderApiClientFactory = coderApiClientFactory; + _credentialManager = credentialManager; + _agentAppViewModelFactory = agentAppViewModelFactory; + + // Since the property value itself never changes, we add event + // listeners for the underlying collection changing instead. + Apps.CollectionChanged += (_, _) => + { + OnPropertyChanged(new PropertyChangedEventArgs(nameof(VisibleApps))); + OnPropertyChanged(new PropertyChangedEventArgs(nameof(ShowExpandAppsMessage))); + OnPropertyChanged(new PropertyChangedEventArgs(nameof(ExpandAppsMessage))); + }; + } + + public bool ApplyMerge(AgentViewModel model) + { + if (Id != model.Id) return false; + + // To avoid spurious UI updates which cause flashing, don't actually + // write to values unless they've changed. + if (Hostname != model.Hostname) + Hostname = model.Hostname; + if (HostnameSuffix != model.HostnameSuffix) + HostnameSuffix = model.HostnameSuffix; + if (ConnectionStatus != model.ConnectionStatus) + ConnectionStatus = model.ConnectionStatus; + if (DashboardBaseUrl != model.DashboardBaseUrl) + DashboardBaseUrl = model.DashboardBaseUrl; + if (WorkspaceName != model.WorkspaceName) + WorkspaceName = model.WorkspaceName; + + // Apps are not set externally. + + return true; + } + + [RelayCommand] + private void ToggleExpanded() + { + // TODO: this should bubble to every other agent in the list so only + // one can be active at a time. + SetExpanded(!IsExpanded); + } + + public void SetExpanded(bool expanded) + { + IsExpanded = expanded; + + // Every time the drawer is expanded, re-fetch all apps. + if (expanded && !FetchingApps) + FetchApps(); + } + + partial void OnConnectionStatusChanged(AgentConnectionStatus oldValue, AgentConnectionStatus newValue) + { + if (IsExpanded && newValue is not AgentConnectionStatus.Gray) FetchApps(); + } + + private void FetchApps() + { + if (FetchingApps) return; + FetchingApps = true; + + var client = _coderApiClientFactory.Create(_credentialManager); + var cts = new CancellationTokenSource(TimeSpan.FromSeconds(15)); + client.GetWorkspaceAgent(Id.ToString(), cts.Token).ContinueWith(t => + { + cts.Dispose(); + ContinueFetchApps(t); + }, CancellationToken.None); + } + + private void ContinueFetchApps(Task task) + { + // Ensure we're on the UI thread. + if (!_dispatcherQueue.HasThreadAccess) + { + _dispatcherQueue.TryEnqueue(() => ContinueFetchApps(task)); + return; + } + + FetchingApps = false; + AppFetchErrored = !task.IsCompletedSuccessfully; + if (!task.IsCompletedSuccessfully) + { + _logger.LogWarning(task.Exception, "Could not fetch workspace agent"); + return; + } + + var workspaceAgent = task.Result; + var apps = new List(); + foreach (var app in workspaceAgent.Apps) + { + if (!app.External || !string.IsNullOrEmpty(app.Command)) continue; + + if (!Uuid.TryParse(app.Id, out var uuid)) + { + _logger.LogWarning("Could not parse app UUID '{Id}' for '{DisplayName}', app will not appear in list", + app.Id, app.DisplayName); + continue; + } + + if (!Uri.TryCreate(app.Url, UriKind.Absolute, out var appUri)) + { + _logger.LogWarning("Could not parse app URI '{Url}' for '{DisplayName}', app will not appear in list", + app.Url, app.DisplayName); + continue; + } + + // Icon parse failures are not fatal, we will just use the fallback + // icon. + _ = Uri.TryCreate(DashboardBaseUrl, app.Icon, out var iconUrl); + + apps.Add(_agentAppViewModelFactory.Create(uuid, app.DisplayName, appUri, iconUrl)); + } + + // Sort by name. + ModelMerge.MergeLists(Apps, apps, (a, b) => string.Compare(a.Name, b.Name, StringComparison.Ordinal)); + } [RelayCommand] private void CopyHostname(object parameter) diff --git a/App/ViewModels/TrayWindowViewModel.cs b/App/ViewModels/TrayWindowViewModel.cs index f845521..acc2a44 100644 --- a/App/ViewModels/TrayWindowViewModel.cs +++ b/App/ViewModels/TrayWindowViewModel.cs @@ -1,9 +1,12 @@ using System; using System.Collections.Generic; +using System.Collections.ObjectModel; +using System.ComponentModel; using System.Linq; using System.Threading.Tasks; using Coder.Desktop.App.Models; using Coder.Desktop.App.Services; +using Coder.Desktop.App.Utils; using Coder.Desktop.App.Views; using Coder.Desktop.Vpn.Proto; using CommunityToolkit.Mvvm.ComponentModel; @@ -25,11 +28,17 @@ public partial class TrayWindowViewModel : ObservableObject private readonly IServiceProvider _services; private readonly IRpcController _rpcController; private readonly ICredentialManager _credentialManager; + private readonly IAgentViewModelFactory _agentViewModelFactory; private FileSyncListWindow? _fileSyncListWindow; private DispatcherQueue? _dispatcherQueue; + // This isn't an ObservableProperty because the property itself never + // changes. We add an event listener for the collection changing in the + // constructor. + public readonly ObservableCollection Agents = []; + [ObservableProperty] [NotifyPropertyChangedFor(nameof(ShowEnableSection))] [NotifyPropertyChangedFor(nameof(ShowWorkspacesHeader))] @@ -49,13 +58,6 @@ public partial class TrayWindowViewModel : ObservableObject [NotifyPropertyChangedFor(nameof(ShowFailedSection))] public partial string? VpnFailedMessage { get; set; } = null; - [ObservableProperty] - [NotifyPropertyChangedFor(nameof(VisibleAgents))] - [NotifyPropertyChangedFor(nameof(ShowNoAgentsSection))] - [NotifyPropertyChangedFor(nameof(ShowAgentsSection))] - [NotifyPropertyChangedFor(nameof(ShowAgentOverflowButton))] - public partial List Agents { get; set; } = []; - public bool ShowEnableSection => VpnFailedMessage is null && VpnLifecycle is not VpnLifecycle.Started; public bool ShowWorkspacesHeader => VpnFailedMessage is null && VpnLifecycle is VpnLifecycle.Started; @@ -79,11 +81,22 @@ public partial class TrayWindowViewModel : ObservableObject [ObservableProperty] public partial string DashboardUrl { get; set; } = "https://coder.com"; public TrayWindowViewModel(IServiceProvider services, IRpcController rpcController, - ICredentialManager credentialManager) + ICredentialManager credentialManager, IAgentViewModelFactory agentViewModelFactory) { _services = services; _rpcController = rpcController; _credentialManager = credentialManager; + _agentViewModelFactory = agentViewModelFactory; + + // Since the property value itself never changes, we add event + // listeners for the underlying collection changing instead. + Agents.CollectionChanged += (_, _) => + { + OnPropertyChanged(new PropertyChangedEventArgs(nameof(VisibleAgents))); + OnPropertyChanged(new PropertyChangedEventArgs(nameof(ShowNoAgentsSection))); + OnPropertyChanged(new PropertyChangedEventArgs(nameof(ShowAgentsSection))); + OnPropertyChanged(new PropertyChangedEventArgs(nameof(ShowAgentOverflowButton))); + }; } public void Initialize(DispatcherQueue dispatcherQueue) @@ -107,32 +120,22 @@ private void UpdateFromRpcModel(RpcModel rpcModel) return; } - // As a failsafe, if RPC is disconnected we disable the switch. The - // Window should not show the current Page if the RPC is disconnected. - if (rpcModel.RpcLifecycle is RpcLifecycle.Disconnected) + // As a failsafe, if RPC is disconnected (or we're not signed in) we + // disable the switch. The Window should not show the current Page if + // the RPC is disconnected. + var credentialModel = _credentialManager.GetCachedCredentials(); + if (rpcModel.RpcLifecycle is RpcLifecycle.Disconnected || credentialModel.State is not CredentialState.Valid || + credentialModel.CoderUrl == null) { VpnLifecycle = VpnLifecycle.Unknown; VpnSwitchActive = false; - Agents = []; + Agents.Clear(); return; } VpnLifecycle = rpcModel.VpnLifecycle; VpnSwitchActive = rpcModel.VpnLifecycle is VpnLifecycle.Starting or VpnLifecycle.Started; - // Get the current dashboard URL. - var credentialModel = _credentialManager.GetCachedCredentials(); - Uri? coderUri = null; - if (credentialModel.State == CredentialState.Valid && !string.IsNullOrWhiteSpace(credentialModel.CoderUrl)) - try - { - coderUri = new Uri(credentialModel.CoderUrl, UriKind.Absolute); - } - catch - { - // Ignore - } - // Add every known agent. HashSet workspacesWithAgents = []; List agents = []; @@ -156,60 +159,48 @@ private void UpdateFromRpcModel(RpcModel rpcModel) } var lastHandshakeAgo = DateTime.UtcNow.Subtract(agent.LastHandshake.ToDateTime()); + var connectionStatus = lastHandshakeAgo < TimeSpan.FromMinutes(5) + ? AgentConnectionStatus.Green + : AgentConnectionStatus.Yellow; workspacesWithAgents.Add(agent.WorkspaceId); var workspace = rpcModel.Workspaces.FirstOrDefault(w => w.Id == agent.WorkspaceId); - agents.Add(new AgentViewModel - { - Hostname = fqdnPrefix, - HostnameSuffix = fqdnSuffix, - ConnectionStatus = lastHandshakeAgo < TimeSpan.FromMinutes(5) - ? AgentConnectionStatus.Green - : AgentConnectionStatus.Yellow, - DashboardUrl = WorkspaceUri(coderUri, workspace?.Name), - }); + agents.Add(_agentViewModelFactory.Create( + agent.ParseId(), + fqdnPrefix, + fqdnSuffix, + connectionStatus, + credentialModel.CoderUrl, + workspace?.Name)); } // For every stopped workspace that doesn't have any agents, add a // dummy agent row. foreach (var workspace in rpcModel.Workspaces.Where(w => w.Status == Workspace.Types.Status.Stopped && !workspacesWithAgents.Contains(w.Id))) - agents.Add(new AgentViewModel - { - // We just assume that it's a single-agent workspace. - Hostname = workspace.Name, + agents.Add(_agentViewModelFactory.Create( + // Workspace ID is fine as a stand-in here, it shouldn't + // conflict with any agent IDs. + workspace.ParseId(), + // We assume that it's a single-agent workspace. + workspace.Name, // TODO: this needs to get the suffix from the server - HostnameSuffix = ".coder", - ConnectionStatus = AgentConnectionStatus.Gray, - DashboardUrl = WorkspaceUri(coderUri, workspace.Name), - }); + ".coder", + AgentConnectionStatus.Gray, + credentialModel.CoderUrl, + workspace.Name)); // Sort by status green, red, gray, then by hostname. - agents.Sort((a, b) => + ModelMerge.MergeLists(Agents, agents, (a, b) => { if (a.ConnectionStatus != b.ConnectionStatus) return a.ConnectionStatus.CompareTo(b.ConnectionStatus); return string.Compare(a.FullHostname, b.FullHostname, StringComparison.Ordinal); }); - Agents = agents; if (Agents.Count < MaxAgents) ShowAllAgents = false; } - private string WorkspaceUri(Uri? baseUri, string? workspaceName) - { - if (baseUri == null) return DefaultDashboardUrl; - if (string.IsNullOrWhiteSpace(workspaceName)) return baseUri.ToString(); - try - { - return new Uri(baseUri, $"/@me/{workspaceName}").ToString(); - } - catch - { - return DefaultDashboardUrl; - } - } - private void UpdateFromCredentialsModel(CredentialModel credentialModel) { // Ensure we're on the UI thread. @@ -224,7 +215,7 @@ private void UpdateFromCredentialsModel(CredentialModel credentialModel) // or this URI is invalid. CredentialModel.CoderUrl should never be // null while the Page is active as the Page is only displayed when // CredentialModel.Status == Valid. - DashboardUrl = credentialModel.CoderUrl ?? DefaultDashboardUrl; + DashboardUrl = credentialModel.CoderUrl?.ToString() ?? DefaultDashboardUrl; } public void VpnSwitch_Toggled(object sender, RoutedEventArgs e) diff --git a/App/Views/DirectoryPickerWindow.xaml.cs b/App/Views/DirectoryPickerWindow.xaml.cs index 6ed5f43..499e417 100644 --- a/App/Views/DirectoryPickerWindow.xaml.cs +++ b/App/Views/DirectoryPickerWindow.xaml.cs @@ -1,6 +1,7 @@ using System; using System.Runtime.InteropServices; using Windows.Graphics; +using Coder.Desktop.App.Utils; using Coder.Desktop.App.ViewModels; using Coder.Desktop.App.Views.Pages; using Microsoft.UI.Windowing; diff --git a/App/Views/Pages/TrayWindowMainPage.xaml b/App/Views/Pages/TrayWindowMainPage.xaml index 42a9abd..d6d27fe 100644 --- a/App/Views/Pages/TrayWindowMainPage.xaml +++ b/App/Views/Pages/TrayWindowMainPage.xaml @@ -97,108 +97,192 @@ - - - - - - - - - + + + + + + + + + - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + - - - - - - - - - - - - - - - - - + HorizontalTextAlignment="Left" + HorizontalAlignment="Stretch" + TextTrimming="CharacterEllipsis" + TextWrapping="NoWrap"> + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/App/Views/SignInWindow.xaml.cs b/App/Views/SignInWindow.xaml.cs index 3fe4b5c..c8ff8df 100644 --- a/App/Views/SignInWindow.xaml.cs +++ b/App/Views/SignInWindow.xaml.cs @@ -1,6 +1,7 @@ using System; using Windows.Graphics; using Coder.Desktop.App.Controls; +using Coder.Desktop.App.Utils; using Coder.Desktop.App.ViewModels; using Coder.Desktop.App.Views.Pages; using Microsoft.UI.Windowing; diff --git a/App/Views/TrayWindow.xaml.cs b/App/Views/TrayWindow.xaml.cs index eac24e8..3475104 100644 --- a/App/Views/TrayWindow.xaml.cs +++ b/App/Views/TrayWindow.xaml.cs @@ -6,6 +6,7 @@ using Coder.Desktop.App.Controls; using Coder.Desktop.App.Models; using Coder.Desktop.App.Services; +using Coder.Desktop.App.Utils; using Coder.Desktop.App.Views.Pages; using CommunityToolkit.Mvvm.Input; using Microsoft.UI; diff --git a/App/packages.lock.json b/App/packages.lock.json index 1541d01..a47908a 100644 --- a/App/packages.lock.json +++ b/App/packages.lock.json @@ -18,6 +18,16 @@ "Microsoft.WindowsAppSDK": "1.6.250108002" } }, + "CommunityToolkit.WinUI.Extensions": { + "type": "Direct", + "requested": "[8.2.250402, )", + "resolved": "8.2.250402", + "contentHash": "rAOYzNX6kdUeeE1ejGd6Q8B+xmyZvOrWFUbqCgOtP8OQsOL66en9ZQTtzxAlaaFC4qleLvnKcn8FJFBezujOlw==", + "dependencies": { + "CommunityToolkit.Common": "8.2.1", + "Microsoft.WindowsAppSDK": "1.6.250108002" + } + }, "DependencyPropertyGenerator": { "type": "Direct", "requested": "[1.5.0, )", @@ -142,15 +152,6 @@ "resolved": "8.2.1", "contentHash": "LWuhy8cQKJ/MYcy3XafJ916U3gPH/YDvYoNGWyQWN11aiEKCZszzPOTJAOvBjP9yG8vHmIcCyPUt4L82OK47Iw==" }, - "CommunityToolkit.WinUI.Extensions": { - "type": "Transitive", - "resolved": "8.2.250402", - "contentHash": "rAOYzNX6kdUeeE1ejGd6Q8B+xmyZvOrWFUbqCgOtP8OQsOL66en9ZQTtzxAlaaFC4qleLvnKcn8FJFBezujOlw==", - "dependencies": { - "CommunityToolkit.Common": "8.2.1", - "Microsoft.WindowsAppSDK": "1.6.250108002" - } - }, "Google.Protobuf": { "type": "Transitive", "resolved": "3.29.3", diff --git a/CoderSdk/Coder/CoderApiClient.cs b/CoderSdk/Coder/CoderApiClient.cs index 79c5c2f..15845bb 100644 --- a/CoderSdk/Coder/CoderApiClient.cs +++ b/CoderSdk/Coder/CoderApiClient.cs @@ -5,6 +5,18 @@ namespace Coder.Desktop.CoderSdk.Coder; public interface ICoderApiClientFactory { public ICoderApiClient Create(string baseUrl); + public ICoderApiClient Create(ICoderApiClientCredentialProvider provider); +} + +public class CoderApiClientCredential +{ + public required Uri CoderUrl { get; set; } + public required string ApiToken { get; set; } +} + +public interface ICoderApiClientCredentialProvider +{ + public CoderApiClientCredential? GetCoderApiClientCredential(); } public class CoderApiClientFactory : ICoderApiClientFactory @@ -13,6 +25,23 @@ public ICoderApiClient Create(string baseUrl) { return new CoderApiClient(baseUrl); } + + public ICoderApiClient Create(ICoderApiClientCredentialProvider provider) + { + var cred = provider.GetCoderApiClientCredential(); + if (cred == null) + throw new InvalidOperationException( + "Cannot create Coder API client with invalid credential provider: credential is null"); + + var client = Create(cred.CoderUrl); + client.SetSessionToken(cred.ApiToken); + return client; + } + + public ICoderApiClient Create(Uri baseUrl) + { + return new CoderApiClient(baseUrl); + } } public partial interface ICoderApiClient @@ -24,6 +53,8 @@ public partial interface ICoderApiClient [JsonSerializable(typeof(Response))] [JsonSerializable(typeof(User))] [JsonSerializable(typeof(ValidationError))] +[JsonSerializable(typeof(WorkspaceAgent))] +[JsonSerializable(typeof(WorkspaceApp))] public partial class CoderApiJsonContext : JsonSerializerContext; /// diff --git a/CoderSdk/Coder/WorkspaceAgents.cs b/CoderSdk/Coder/WorkspaceAgents.cs new file mode 100644 index 0000000..01e93dd --- /dev/null +++ b/CoderSdk/Coder/WorkspaceAgents.cs @@ -0,0 +1,29 @@ +namespace Coder.Desktop.CoderSdk.Coder; + +public partial interface ICoderApiClient +{ + public Task GetWorkspaceAgent(string id, CancellationToken ct = default); +} + +public class WorkspaceAgent +{ + public WorkspaceApp[] Apps { get; set; } = []; +} + +public class WorkspaceApp +{ + public string Id { get; set; } = string.Empty; + public string Url { get; set; } = string.Empty; + public bool External { get; set; } = false; + public string DisplayName { get; set; } = string.Empty; + public string Command { get; set; } = string.Empty; + public string Icon { get; set; } = string.Empty; +} + +public partial class CoderApiClient +{ + public Task GetWorkspaceAgent(string id, CancellationToken ct = default) + { + return SendRequestNoBodyAsync(HttpMethod.Get, "/api/v2/workspaceagents/" + id, ct); + } +} diff --git a/Tests.App/Services/CredentialManagerTest.cs b/Tests.App/Services/CredentialManagerTest.cs index 9d00cf2..9f1b0df 100644 --- a/Tests.App/Services/CredentialManagerTest.cs +++ b/Tests.App/Services/CredentialManagerTest.cs @@ -9,7 +9,7 @@ namespace Coder.Desktop.Tests.App.Services; [TestFixture] public class CredentialManagerTest { - private const string TestServerUrl = "https://dev.coder.com"; + private const string TestServerUrl = "https://dev.coder.com/"; private const string TestApiToken = "abcdef1234-abcdef1234567890ABCDEF"; private const string TestUsername = "dean"; @@ -50,7 +50,7 @@ public async Task EndToEnd(CancellationToken ct) // Cached credential should be valid. cred = manager1.GetCachedCredentials(); Assert.That(cred.State, Is.EqualTo(CredentialState.Valid)); - Assert.That(cred.CoderUrl, Is.EqualTo(TestServerUrl)); + Assert.That(cred.CoderUrl?.ToString(), Is.EqualTo(TestServerUrl)); Assert.That(cred.ApiToken, Is.EqualTo(TestApiToken)); Assert.That(cred.Username, Is.EqualTo(TestUsername)); @@ -62,7 +62,7 @@ public async Task EndToEnd(CancellationToken ct) var manager2 = new CredentialManager(credentialBackend, apiClientFactory.Object); cred = await manager2.LoadCredentials(ct).WaitAsync(ct); Assert.That(cred.State, Is.EqualTo(CredentialState.Valid)); - Assert.That(cred.CoderUrl, Is.EqualTo(TestServerUrl)); + Assert.That(cred.CoderUrl?.ToString(), Is.EqualTo(TestServerUrl)); Assert.That(cred.ApiToken, Is.EqualTo(TestApiToken)); Assert.That(cred.Username, Is.EqualTo(TestUsername)); diff --git a/Tests.Vpn.Proto/UuidTest.cs b/Tests.Vpn.Proto/UuidTest.cs new file mode 100644 index 0000000..90d8f73 --- /dev/null +++ b/Tests.Vpn.Proto/UuidTest.cs @@ -0,0 +1,139 @@ +using Coder.Desktop.Vpn.Proto; + +namespace Coder.Desktop.Tests.Vpn.Proto; + +[TestFixture] +public class UuidTest +{ + private const string UuidStr = "df762f71-898c-44a2-84c6-8add83704266"; + + private static readonly byte[] UuidBytes = + [0xdf, 0x76, 0x2f, 0x71, 0x89, 0x8c, 0x44, 0xa2, 0x84, 0xc6, 0x8a, 0xdd, 0x83, 0x70, 0x42, 0x66]; + + [Test(Description = "Convert UUID bytes => Uuid")] + public void BytesToUuid() + { + var uuid = new Uuid(UuidBytes); + Assert.That(uuid.ToString(), Is.EqualTo(UuidStr)); + Assert.That(uuid.Bytes.ToArray(), Is.EqualTo(UuidBytes)); + } + + [Test(Description = "Convert UUID string => Uuid")] + public void StringToUuid() + { + var uuid = new Uuid(UuidStr); + Assert.That(uuid.ToString(), Is.EqualTo(UuidStr)); + Assert.That(uuid.Bytes.ToArray(), Is.EqualTo(UuidBytes)); + } + + [Test(Description = "Convert capitalized UUID string => Uuid")] + public void CapitalizedStringToUuid() + { + var uuid = new Uuid(UuidStr.ToUpper()); + // The capitalized string should be discarded after parsing. + Assert.That(uuid.ToString(), Is.EqualTo(UuidStr)); + Assert.That(uuid.Bytes.ToArray(), Is.EqualTo(UuidBytes)); + } + + [Test(Description = "Invalid length")] + public void InvalidLength() + { + var ex = Assert.Throws(() => _ = new Uuid([])); + Assert.That(ex.Message, Does.Contain("UUID must be 16 bytes, but was 0 bytes")); + ex = Assert.Throws(() => _ = new Uuid(UuidBytes.AsSpan(..^1))); + Assert.That(ex.Message, Does.Contain("UUID must be 16 bytes, but was 15 bytes")); + var longerBytes = UuidBytes.Append((byte)0x0).ToArray(); + ex = Assert.Throws(() => _ = new Uuid(longerBytes)); + Assert.That(ex.Message, Does.Contain("UUID must be 16 bytes, but was 17 bytes")); + + ex = Assert.Throws(() => _ = new Uuid("")); + Assert.That(ex.Message, Does.Contain("UUID string must be 36 characters, but was 0 characters")); + ex = Assert.Throws(() => _ = new Uuid(UuidStr[..^1])); + Assert.That(ex.Message, Does.Contain("UUID string must be 36 characters, but was 35 characters")); + ex = Assert.Throws(() => _ = new Uuid(UuidStr + "0")); + Assert.That(ex.Message, Does.Contain("UUID string must be 36 characters, but was 37 characters")); + } + + [Test(Description = "Invalid version")] + public void InvalidVersion() + { + var invalidVersionBytes = new byte[16]; + Array.Copy(UuidBytes, invalidVersionBytes, UuidBytes.Length); + invalidVersionBytes[6] = (byte)(invalidVersionBytes[6] & 0x0f); // clear version nibble + var ex = Assert.Throws(() => _ = new Uuid(invalidVersionBytes)); + Assert.That(ex.Message, Does.Contain("ID does not seem like a valid UUIDv4")); + + var invalidVersionChars = UuidStr.ToCharArray(); + invalidVersionChars[14] = '0'; // clear version nibble + ex = Assert.Throws(() => _ = new Uuid(new string(invalidVersionChars))); + Assert.That(ex.Message, Does.Contain("ID does not seem like a valid UUIDv4")); + } + + [Test(Description = "Invalid string")] + public void InvalidString() + { + var hyphenMissing = UuidStr.Replace("-", "0"); + var ex = Assert.Throws(() => _ = new Uuid(hyphenMissing)); + Assert.That(ex.Message, Does.Contain("UUID string must have dashes at positions 8, 13, 18, and 23")); + + var invalidHex = UuidStr.ToCharArray(); + invalidHex[2] = 'g'; // invalid hex digit + ex = Assert.Throws(() => _ = new Uuid(new string(invalidHex))); + Assert.That(ex.Message, Does.Contain("UUID string has invalid hex digits at position 2")); + } + + [Test(Description = "Try methods")] + public void Try() + { + Assert.That(Uuid.TryFrom(UuidBytes, out var uuid1), Is.True); + Assert.That(uuid1.Bytes.ToArray(), Is.EqualTo(UuidBytes)); + + Assert.That(Uuid.TryFrom([], out var uuid2), Is.False); + Assert.That(uuid2, Is.EqualTo(Uuid.Zero)); + + Assert.That(Uuid.TryParse(UuidStr, out var uuid3), Is.True); + Assert.That(uuid3.ToString(), Is.EqualTo(UuidStr)); + + Assert.That(Uuid.TryParse("", out var uuid4), Is.False); + Assert.That(uuid4, Is.EqualTo(Uuid.Zero)); + } + + [Test(Description = "Equality")] + public void Equality() + { + var differentUuidBytes = new byte[16]; + Array.Copy(UuidBytes, differentUuidBytes, UuidBytes.Length); + differentUuidBytes[0] = (byte)(differentUuidBytes[0] + 1); + + var uuid1 = new Uuid(UuidBytes); + var uuid2 = new Uuid(UuidBytes); + var uuid3 = new Uuid(differentUuidBytes); + +#pragma warning disable CS1718 // Comparison made to same variable +#pragma warning disable NUnit2010 // Use Is.EqualTo constraint instead of direct comparison + Assert.That(uuid1 == uuid1, Is.True); + Assert.That(uuid1 != uuid1, Is.False); + Assert.That(Uuid.Zero == Uuid.Zero, Is.True); + Assert.That(Uuid.Zero != Uuid.Zero, Is.False); +#pragma warning restore NUnit2010 +#pragma warning restore CS1718 + + Assert.That(uuid1 == uuid2, Is.True); + Assert.That(uuid2 == uuid1, Is.True); + Assert.That(uuid1 != uuid2, Is.False); + Assert.That(uuid2 != uuid1, Is.False); + + Assert.That(uuid1 == uuid3, Is.False); + Assert.That(uuid3 == uuid1, Is.False); + Assert.That(uuid1 != uuid3, Is.True); + Assert.That(uuid3 != uuid1, Is.True); + + Assert.That(uuid1.Equals(uuid2), Is.True); + Assert.That(uuid2.Equals(uuid1), Is.True); + Assert.That(uuid1.Equals(uuid3), Is.False); + Assert.That(uuid3.Equals(uuid1), Is.False); + + Assert.That(uuid1.Equals(null!), Is.False); + Assert.That(uuid1.Equals(new object()), Is.False); + } +} diff --git a/Vpn.Proto/Uuid.cs b/Vpn.Proto/Uuid.cs new file mode 100644 index 0000000..0f921dc --- /dev/null +++ b/Vpn.Proto/Uuid.cs @@ -0,0 +1,175 @@ +using System.Globalization; +using System.Text; +using Google.Protobuf; + +namespace Coder.Desktop.Vpn.Proto; + +/// +/// A simplistic UUIDv4 class that wraps a 16-byte array. We don't use the +/// native Guid class because it has some weird reordering behavior due to +/// legacy Windows stuff. This class is not guaranteed to provide full RFC +/// 4122 compliance, but it should provide enough coverage for Coder +/// Desktop. +/// +public class Uuid +{ + private readonly byte[] _bytes; + public static Uuid Zero { get; } = new(); + + public ReadOnlySpan Bytes => _bytes; + + public Uuid() + { + _bytes = new byte[16]; + } + + public Uuid(ReadOnlySpan bytes) + { + if (bytes.Length != 16) + throw new ArgumentException($"UUID must be 16 bytes, but was {bytes.Length} bytes", nameof(bytes)); + if (bytes[6] >> 4 != 0x4) + throw new ArgumentException("ID does not seem like a valid UUIDv4", nameof(bytes)); + _bytes = bytes.ToArray(); + } + + public Uuid(string str) + { + if (str.Length != 36) + throw new ArgumentException($"UUID string must be 36 characters, but was {str.Length} characters", + nameof(str)); + + var currentIndex = 0; + _bytes = new byte[16]; + + for (var i = 0; i < 36; i++) + { + if (i is 8 or 13 or 18 or 23) + { + if (str[i] != '-') + throw new ArgumentException("UUID string must have dashes at positions 8, 13, 18, and 23", + nameof(str)); + continue; + } + + // Take two hex digits and convert them to a byte. + var hex = str[i..(i + 2)]; + if (!byte.TryParse(hex, NumberStyles.HexNumber, null, out var b)) + throw new ArgumentException($"UUID string has invalid hex digits at position {i}", nameof(str)); + _bytes[currentIndex] = b; + currentIndex++; + + // Advance the loop index by 1 as we processed two characters. + i++; + } + + if (currentIndex != 16) + throw new ArgumentException($"UUID string must have 16 bytes, but was {currentIndex} bytes", nameof(str)); + if (_bytes[6] >> 4 != 0x4) + throw new ArgumentException("ID does not seem like a valid UUIDv4", nameof(str)); + } + + public static Uuid FromByteString(ByteString byteString) + { + return new Uuid(byteString.Span); + } + + public static bool TryFrom(ReadOnlySpan bytes, out Uuid uuid) + { + try + { + uuid = new Uuid(bytes); + return true; + } + catch + { + uuid = Zero; + return false; + } + } + + public static bool TryFrom(ByteString byteString, out Uuid uuid) + { + return TryFrom(byteString.Span, out uuid); + } + + public static bool TryParse(string str, out Uuid uuid) + { + try + { + uuid = new Uuid(str); + return true; + } + catch + { + uuid = Zero; + return false; + } + } + + public override string ToString() + { + if (_bytes.Length != 16) + throw new ArgumentException($"UUID must be 16 bytes, but was {_bytes.Length} bytes", nameof(_bytes)); + + // Print every byte as hex, with dashes in the right places. + var sb = new StringBuilder(36); + for (var i = 0; i < 16; i++) + { + if (i is 4 or 6 or 8 or 10) + sb.Append('-'); + sb.Append(_bytes[i].ToString("x2")); + } + + return sb.ToString(); + } + + #region Uuid equality + + public override bool Equals(object? obj) + { + return obj is Uuid other && Equals(other); + } + + public bool Equals(Uuid? other) + { + return other is not null && _bytes.SequenceEqual(other._bytes); + } + + public override int GetHashCode() + { + return _bytes.GetHashCode(); + } + + public static bool operator ==(Uuid left, Uuid right) + { + return left.Equals(right); + } + + public static bool operator !=(Uuid left, Uuid right) + { + return !left.Equals(right); + } + + #endregion +} + +public partial class Workspace +{ + public Uuid ParseId() + { + return Uuid.FromByteString(Id); + } +} + +public partial class Agent +{ + public Uuid ParseId() + { + return Uuid.FromByteString(Id); + } + + public Uuid ParseWorkspaceId() + { + return Uuid.FromByteString(WorkspaceId); + } +} From c6d45ea257cf959a983e72322b0fe239534f38ff Mon Sep 17 00:00:00 2001 From: Dean Sheather Date: Tue, 6 May 2025 16:28:39 +1000 Subject: [PATCH 02/11] fix various warnings --- App/Controls/ExpandChevron.xaml | 2 +- App/Controls/ExpandChevron.xaml.cs | 2 -- App/Services/CredentialManager.cs | 2 +- App/ViewModels/FileSyncListViewModel.cs | 3 ++- App/Views/Pages/SignInTokenPage.xaml | 4 ++-- App/Views/Pages/SignInUrlPage.xaml | 2 +- 6 files changed, 7 insertions(+), 8 deletions(-) diff --git a/App/Controls/ExpandChevron.xaml b/App/Controls/ExpandChevron.xaml index 75281e7..0b68d4d 100644 --- a/App/Controls/ExpandChevron.xaml +++ b/App/Controls/ExpandChevron.xaml @@ -17,7 +17,7 @@ Height="16" Margin="0,0,8,0" RenderTransformOrigin="0.5, 0.5" - Foreground="{x:Bind Foreground, Mode=Oneway}" + Foreground="{x:Bind Foreground, Mode=OneWay}" HorizontalAlignment="Center" VerticalAlignment="Center" AnimatedIcon.State="NormalOff"> diff --git a/App/Controls/ExpandChevron.xaml.cs b/App/Controls/ExpandChevron.xaml.cs index 0dbd68e..45aa6c4 100644 --- a/App/Controls/ExpandChevron.xaml.cs +++ b/App/Controls/ExpandChevron.xaml.cs @@ -1,11 +1,9 @@ using DependencyPropertyGenerator; using Microsoft.UI.Xaml.Controls; -using Microsoft.UI.Xaml.Media; namespace Coder.Desktop.App.Controls; [DependencyProperty("IsOpen", DefaultValue = false)] -[DependencyProperty("Foreground")] public sealed partial class ExpandChevron : UserControl { public ExpandChevron() diff --git a/App/Services/CredentialManager.cs b/App/Services/CredentialManager.cs index bf90fcc..35b2262 100644 --- a/App/Services/CredentialManager.cs +++ b/App/Services/CredentialManager.cs @@ -105,7 +105,7 @@ public CredentialModel GetCachedCredentials() public CoderApiClientCredential? GetCoderApiClientCredential() { var latestCreds = _latestCredentials; - if (latestCreds is not { State: CredentialState.Valid }) + if (latestCreds is not { State: CredentialState.Valid } || latestCreds.CoderUrl is null) return null; return new CoderApiClientCredential diff --git a/App/ViewModels/FileSyncListViewModel.cs b/App/ViewModels/FileSyncListViewModel.cs index 9235141..da40e5c 100644 --- a/App/ViewModels/FileSyncListViewModel.cs +++ b/App/ViewModels/FileSyncListViewModel.cs @@ -339,7 +339,8 @@ public void OpenRemotePathSelectDialog() pickerViewModel.PathSelected += OnRemotePathSelected; _remotePickerWindow = new DirectoryPickerWindow(pickerViewModel); - _remotePickerWindow.SetParent(_window); + if (_window is not null) + _remotePickerWindow.SetParent(_window); _remotePickerWindow.Closed += (_, _) => { _remotePickerWindow = null; diff --git a/App/Views/Pages/SignInTokenPage.xaml b/App/Views/Pages/SignInTokenPage.xaml index 8613f19..1994848 100644 --- a/App/Views/Pages/SignInTokenPage.xaml +++ b/App/Views/Pages/SignInTokenPage.xaml @@ -86,14 +86,14 @@ - public static void MergeLists(IList target, IList update, Comparison sorter) + public static void MergeLists(IList target, IEnumerable update, Comparison sorter) where T : IModelMergeable { var newItems = update.ToList(); @@ -54,7 +54,7 @@ public static void MergeLists(IList target, IList update, Comparison // A merge couldn't occur, so we need to remove the old item and // decrement `i` for the next iteration. - target.Remove(target[i]); + target.RemoveAt(i); i--; OuterLoopEnd: ; diff --git a/App/ViewModels/AgentAppViewModel.cs b/App/ViewModels/AgentAppViewModel.cs index 25e695e..6e89070 100644 --- a/App/ViewModels/AgentAppViewModel.cs +++ b/App/ViewModels/AgentAppViewModel.cs @@ -40,16 +40,11 @@ public AgentAppViewModel Create(Uuid id, string name, Uri appUri, Uri? iconUrl) public partial class AgentAppViewModel : ObservableObject, IModelMergeable { - // HACK: We need to set the icon size for SVGs otherwise they might get cut - // off. These must be kept in sync with the XAML code. - public const int IconWidth = 20; - public const int IconHeight = 20; - private readonly ILogger _logger; public required Uuid Id { get; init; } - public required string Name { get; set; } + [ObservableProperty] public required partial string Name { get; set; } [ObservableProperty] [NotifyPropertyChangedFor(nameof(Details))] @@ -82,11 +77,7 @@ public ImageSource ImageSource { // TODO: Some SVGs like `/icon/cursor.svg` contain PNG data and // don't render at all. - var svg = new SvgImageSource(IconUrl) - { - RasterizePixelWidth = IconWidth, - RasterizePixelHeight = IconHeight, - }; + var svg = new SvgImageSource(IconUrl); svg.Opened += (_, _) => _logger.LogDebug("app icon opened (svg): {uri}", IconUrl); svg.OpenFailed += (_, args) => _logger.LogDebug("app icon failed to open (svg): {uri}: {Status}", IconUrl, args.Status); diff --git a/App/Views/Pages/TrayWindowMainPage.xaml b/App/Views/Pages/TrayWindowMainPage.xaml index d6d27fe..f18cd3e 100644 --- a/App/Views/Pages/TrayWindowMainPage.xaml +++ b/App/Views/Pages/TrayWindowMainPage.xaml @@ -259,7 +259,6 @@ ToolTipService.ToolTip="{x:Bind Details}"> - +{ + public List AttemptedMerges = []; + public int Id { get; } + + public MergeableItem(int id) + { + Id = id; + } + + public bool ApplyMerge(MergeableItem obj) + { + AttemptedMerges.Add(obj.Id); + return Id == obj.Id; + } + + public override string ToString() + { + return $"MergeableItem {Id}"; + } + + #region MergeableItem equality + + public override bool Equals(object? obj) + { + return obj is MergeableItem other && Equals(other); + } + + public bool Equals(MergeableItem? other) + { + return other is not null && Id == other.Id; + } + + public override int GetHashCode() + { + return Id.GetHashCode(); + } + + public static bool operator ==(MergeableItem left, MergeableItem right) + { + return left.Equals(right); + } + + public static bool operator !=(MergeableItem left, MergeableItem right) + { + return !left.Equals(right); + } + + #endregion +} + +/// +/// A wrapper around list that tracks Insert and RemoveAt operations. +/// +public class TrackableList : IList +{ + public List Items = []; + public List> Operations = []; + + public void Insert(int index, T item) + { + Items.Insert(index, item); + Operations.Add(new ListOperation + { + Type = ListOperation.OperationType.Insert, + Index = index, + Item = item, + }); + } + + public void RemoveAt(int index) + { + var item = Items[index]; + Items.RemoveAt(index); + Operations.Add(new ListOperation + { + Type = ListOperation.OperationType.RemoveAt, + Index = index, + Item = item, + }); + } + + public T this[int index] + { + get => Items[index]; + // We don't expect this to be called in the test. + set => throw new NotImplementedException(); + } + + public int IndexOf(T item) + { + // We don't expect this to be called in the test. + throw new NotImplementedException(); + } + + public IEnumerator GetEnumerator() + { + // We don't expect this to be called in the test. + throw new NotImplementedException(); + } + + IEnumerator IEnumerable.GetEnumerator() + { + // We don't expect this to be called in the test. + throw new NotImplementedException(); + } + + public void Add(T item) + { + // We don't expect this to be called in the test. + throw new NotImplementedException(); + } + + public void Clear() + { + // We don't expect this to be called in the test. + throw new NotImplementedException(); + } + + public bool Contains(T item) + { + // We don't expect this to be called in the test. + throw new NotImplementedException(); + } + + public void CopyTo(T[] array, int arrayIndex) + { + // We don't expect this to be called in the test. + throw new NotImplementedException(); + } + + public bool Remove(T item) + { + // We don't expect this to be called in the test. + throw new NotImplementedException(); + } + + public int Count => Items.Count; + + public bool IsReadOnly => false; +} + +public class ListOperation +{ + public enum OperationType + { + Insert, + RemoveAt, + } + + public required OperationType Type { get; init; } + public required int Index { get; init; } + public required TO Item { get; init; } + + public override string ToString() + { + return $"ListOperation {Type} {Index} {Item}"; + } + + #region ListOperation equality + + public override bool Equals(object? obj) + { + return obj is ListOperation other && Equals(other); + } + + public bool Equals(ListOperation? other) + { + return other is not null && Type == other.Type && Index == other.Index && Item.Equals(other.Item); + } + + public override int GetHashCode() + { + return HashCode.Combine(Type, Index, Item); + } + + public static bool operator ==(ListOperation left, ListOperation right) + { + return left.Equals(right); + } + + public static bool operator !=(ListOperation left, ListOperation right) + { + return !left.Equals(right); + } + + #endregion +} + +#endregion + +[TestFixture] +public class ModelMergeTest +{ + [Test(Description = "Full merge test with merged, removed and added items")] + public void Full() + { + var original1 = new MergeableItem(1); + var original3 = new MergeableItem(3); + var original4 = new MergeableItem(4); + var update2 = new MergeableItem(2); + var update1 = new MergeableItem(1); + var update4 = new MergeableItem(4); + var target = new TrackableList + { + Items = + [ + original1, + original3, + original4, + ], + }; + var update = new List + { + update2, + update1, + update4, + }; + + ModelMerge.MergeLists( + target, + update, + (a, b) => a.Id - b.Id); + + // Compare directly rather than using `Is.EquivalentTo` because we want + // to ensure the references are what we expect (rather than just + // equality). + Assert.That(target.Items.Count, Is.EqualTo(3)); + Assert.That(target.Items[0], Is.SameAs(original1)); + Assert.That(target.Items[1], Is.SameAs(update2)); + Assert.That(target.Items[2], Is.SameAs(original4)); + + // All the original items should have attempted to merge. + // original1: update2 (X), update1 (O) // update1 will be ignored now + Assert.That(original1.AttemptedMerges, Is.EquivalentTo([2, 1])); + // original3: update2 (X), update4 (X) + Assert.That(original3.AttemptedMerges, Is.EquivalentTo([2, 4])); + // original4: update2 (X), update4 (O) // update4 will be ignored now + Assert.That(original4.AttemptedMerges, Is.EquivalentTo([2, 4])); + + // We should've only performed two list writes operations. Removes are + // processed first, then inserts. + Assert.That(target.Operations, Is.EquivalentTo(new List> + { + // RemoveAt(1) => original3 => [original1, original4] + new() + { + Type = ListOperation.OperationType.RemoveAt, + Index = 1, + Item = original3, + }, + // Insert(1, update2) => [original1, update2, original4] + new() + { + Type = ListOperation.OperationType.Insert, + Index = 1, + Item = update2, + }, + })); + } + + [Test(Description = "Sorts when inserting")] + public void Sorts() + { + var target = new TrackableList(); + var update = new List + { + new(3), + new(2), + new(5), + new(0), + new(4), + new(1), + new(6), + new(8), + new(7), + new(9), + }; + ModelMerge.MergeLists( + target, + update, + (a, b) => a.Id - b.Id); + + // Ensure it inserted with correct sorting. + Assert.That(target.Items.Count, Is.EqualTo(10)); + var ids = target.Items.Select(i => i.Id).ToList(); + Assert.That(ids, Is.EquivalentTo([0, 1, 2, 3, 4, 5, 6, 7, 8, 9])); + + // Ensure it performed the correct operations. + Assert.That(target.Operations.Count, Is.EqualTo(10)); + Assert.That(target.Operations, Is.EquivalentTo(new List> + { + // Insert(0, 3) => [3] + new() + { + Type = ListOperation.OperationType.Insert, + Index = 0, + Item = new MergeableItem(3), + }, + // Insert(0, 2) => [2, 3] + new() + { + Type = ListOperation.OperationType.Insert, + Index = 0, + Item = new MergeableItem(2), + }, + // Insert(2, 5) => [2, 3, 5] + new() + { + Type = ListOperation.OperationType.Insert, + Index = 2, + Item = new MergeableItem(5), + }, + // Insert(0, 0) => [0, 2, 3, 5] + new() + { + Type = ListOperation.OperationType.Insert, + Index = 0, + Item = new MergeableItem(0), + }, + // Insert(3, 4) => [0, 2, 3, 4, 5] + new() + { + Type = ListOperation.OperationType.Insert, + Index = 3, + Item = new MergeableItem(4), + }, + // Insert11, 1) => [0, 1, 2, 3, 4, 5] + new() + { + Type = ListOperation.OperationType.Insert, + Index = 1, + Item = new MergeableItem(1), + }, + // Insert(6, 6) => [0, 1, 2, 3, 4, 5, 6] + new() + { + Type = ListOperation.OperationType.Insert, + Index = 6, + Item = new MergeableItem(6), + }, + // Insert(7, 8) => [0, 1, 2, 3, 4, 5, 6, 8] + new() + { + Type = ListOperation.OperationType.Insert, + Index = 7, + Item = new MergeableItem(8), + }, + // Insert(7, 7) => [0, 1, 2, 3, 4, 5, 6, 7, 8] + new() + { + Type = ListOperation.OperationType.Insert, + Index = 7, + Item = new MergeableItem(7), + }, + // Insert(9, 9) => [0, 1, 2, 3, 4, 5, 6, 7, 8, 9] + new() + { + Type = ListOperation.OperationType.Insert, + Index = 9, + Item = new MergeableItem(9), + }, + })); + } + + [Test(Description = "Sorts AFTER when inserting with matching sort order")] + public void SortsAfter() + { + var target = new List + { + new(1), + new(3), + new(4), + }; + var update = new List + { + new(4), + new(2), + new(3), + new(1), + }; + + ModelMerge.MergeLists( + target, + update, + // Sort 2 and 3 as equal, so that 2 is inserted after 3. + (a, b) => + { + if (a.Id is 2 or 3) return 0; + return a.Id - b.Id; + }); + + // Ensure it inserted with correct sorting. + Assert.That(target.Count, Is.EqualTo(4)); + var ids = target.Select(i => i.Id).ToList(); + Assert.That(ids, Is.EquivalentTo([1, 3, 2, 4])); + } +} From 0fefc8c3349ad5efe2898363911507d60e33dab4 Mon Sep 17 00:00:00 2001 From: Dean Sheather Date: Thu, 8 May 2025 12:48:18 +1000 Subject: [PATCH 04/11] ModelMerge comments --- App/Utils/ModelMerge.cs | 85 ------------ App/Utils/ModelUpdate.cs | 97 ++++++++++++++ App/ViewModels/AgentAppViewModel.cs | 4 +- App/ViewModels/AgentViewModel.cs | 6 +- App/ViewModels/TrayWindowViewModel.cs | 2 +- .../{ModelMergeTest.cs => ModelUpdateTest.cs} | 122 ++++++++++-------- 6 files changed, 168 insertions(+), 148 deletions(-) delete mode 100644 App/Utils/ModelMerge.cs create mode 100644 App/Utils/ModelUpdate.cs rename Tests.App/Utils/{ModelMergeTest.cs => ModelUpdateTest.cs} (74%) diff --git a/App/Utils/ModelMerge.cs b/App/Utils/ModelMerge.cs deleted file mode 100644 index 42e1577..0000000 --- a/App/Utils/ModelMerge.cs +++ /dev/null @@ -1,85 +0,0 @@ -using System; -using System.Collections.Generic; -using System.Linq; - -namespace Coder.Desktop.App.Utils; - -public interface IModelMergeable -{ - /// - /// Applies a merge of obj to `this`. - /// - /// - /// True if the two objects represent the same item (e.g. the ID/name - /// matches) and the merge was performed. - /// - public bool ApplyMerge(T obj); -} - -/// -/// A static utility class providing methods for merging model updates with -/// as little UI updates as possible. -/// The main goal of the utilities in this class is to prevent redraws in -/// ItemsRepeater items when nothing has changed. -/// -public static class ModelMerge -{ - /// - /// Merges two observable lists with as little operations as possible - /// to avoid excessive/unncessary UI updates. - /// It's assumed that the target list is already sorted. - /// - public static void MergeLists(IList target, IEnumerable update, Comparison sorter) - where T : IModelMergeable - { - var newItems = update.ToList(); - - // Update and remove existing items. We use index-based for loops here - // because we remove items, and removing items while using the list as - // an IEnumerable will throw an exception. - for (var i = 0; i < target.Count; i++) - { - // Even though we're removing items before a "break", we still use - // index-based for loops here to avoid exceptions. - for (var j = 0; j < newItems.Count; j++) - { - if (!target[i].ApplyMerge(newItems[j])) continue; - - // Prevent it from being added below, or checked again. We - // don't need to decrement `j` here because we're breaking - // out of this inner loop. - newItems.RemoveAt(j); - goto OuterLoopEnd; // continue outer loop - } - - // A merge couldn't occur, so we need to remove the old item and - // decrement `i` for the next iteration. - target.RemoveAt(i); - i--; - - OuterLoopEnd: ; - } - - // Add any items that were missing into their correct sorted place. - // It's assumed the list is already sorted. - foreach (var item in newItems) - { - // Perform a binary search. List has BinarySearch(), but - // IList does not. - // - // Inserts after existing equal elements. - var low = 0; - var high = target.Count; - while (low < high) - { - var mid = (low + high) / 2; - if (sorter(item, target[mid]) < 0) - high = mid; - else - low = mid + 1; - } - - target.Insert(low, item); - } - } -} diff --git a/App/Utils/ModelUpdate.cs b/App/Utils/ModelUpdate.cs new file mode 100644 index 0000000..3e37b0e --- /dev/null +++ b/App/Utils/ModelUpdate.cs @@ -0,0 +1,97 @@ +using System; +using System.Collections.Generic; +using System.Linq; + +namespace Coder.Desktop.App.Utils; + +public interface IModelUpdateable +{ + /// + /// Applies changes from obj to `this` if they represent the same + /// object based on some identifier like an ID or fixed name. + /// + /// + /// True if the two objects represent the same item and the changes + /// were applied. + /// + public bool TryApplyChanges(T obj); +} + +/// +/// A static utility class providing methods for applying model updates +/// with as little UI updates as possible. +/// The main goal of the utilities in this class is to prevent redraws in +/// ItemsRepeater items when nothing has changed. +/// +public static class ModelUpdate +{ + /// + /// Takes all items in `update` and either applies them to existing + /// items in `target`, or adds them to `target` if there are no + /// matching items. + /// Any items in `target` that don't have a corresponding item in + /// `update` will be removed from `target`. + /// Items are inserted in their correct sort position according to + /// `sorter`. It's assumed that the target list is already sorted by + /// `sorter`. + /// + /// Target list to be updated + /// Incoming list to apply to `target` + /// + /// Comparison to use for sorting. Note that the sort order does not + /// need to be the ID/name field used in the IModelUpdateable + /// implementation, and can be by any order. + /// New items will be sorted after existing items. + /// + public static void ApplyLists(IList target, IEnumerable update, Comparison sorter) + where T : IModelUpdateable + { + var newItems = update.ToList(); + + // Update and remove existing items. We use index-based for loops here + // because we remove items, and removing items while using the list as + // an IEnumerable will throw an exception. + for (var i = 0; i < target.Count; i++) + { + // Even though we're removing items before a "break", we still use + // index-based for loops here to avoid exceptions. + for (var j = 0; j < newItems.Count; j++) + { + if (!target[i].TryApplyChanges(newItems[j])) continue; + + // Prevent it from being added below, or checked again. We + // don't need to decrement `j` here because we're breaking + // out of this inner loop. + newItems.RemoveAt(j); + goto OuterLoopEnd; // continue outer loop + } + + // A merge couldn't occur, so we need to remove the old item and + // decrement `i` for the next iteration. + target.RemoveAt(i); + i--; + + OuterLoopEnd: ; + } + + // Add any items that were missing into their correct sorted place. + // It's assumed the list is already sorted. + foreach (var newItem in newItems) + { + for (var i = 0; i < target.Count; i++) + // If the new item sorts before the current item, insert it + // after. + if (sorter(newItem, target[i]) < 0) + { + target.Insert(i, newItem); + goto OuterLoopEnd; + } + + // Handle the case where target is empty or the new item is + // equal to or after every other item. + target.Add(newItem); + + OuterLoopEnd: ; + } + } +} diff --git a/App/ViewModels/AgentAppViewModel.cs b/App/ViewModels/AgentAppViewModel.cs index 6e89070..fd2ed3b 100644 --- a/App/ViewModels/AgentAppViewModel.cs +++ b/App/ViewModels/AgentAppViewModel.cs @@ -38,7 +38,7 @@ public AgentAppViewModel Create(Uuid id, string name, Uri appUri, Uri? iconUrl) } } -public partial class AgentAppViewModel : ObservableObject, IModelMergeable +public partial class AgentAppViewModel : ObservableObject, IModelUpdateable { private readonly ILogger _logger; @@ -97,7 +97,7 @@ public AgentAppViewModel(ILogger logger) _logger = logger; } - public bool ApplyMerge(AgentAppViewModel obj) + public bool TryApplyChanges(AgentAppViewModel obj) { if (Id != obj.Id) return false; diff --git a/App/ViewModels/AgentViewModel.cs b/App/ViewModels/AgentViewModel.cs index 7f99b07..0d6ebd2 100644 --- a/App/ViewModels/AgentViewModel.cs +++ b/App/ViewModels/AgentViewModel.cs @@ -65,7 +65,7 @@ public enum AgentConnectionStatus Gray, } -public partial class AgentViewModel : ObservableObject, IModelMergeable +public partial class AgentViewModel : ObservableObject, IModelUpdateable { private const string DefaultDashboardUrl = "https://coder.com"; private const int MaxAppsPerRow = 6; @@ -182,7 +182,7 @@ public AgentViewModel(ILogger logger, ICoderApiClientFactory cod }; } - public bool ApplyMerge(AgentViewModel model) + public bool TryApplyChanges(AgentViewModel model) { if (Id != model.Id) return false; @@ -285,7 +285,7 @@ private void ContinueFetchApps(Task task) } // Sort by name. - ModelMerge.MergeLists(Apps, apps, (a, b) => string.Compare(a.Name, b.Name, StringComparison.Ordinal)); + ModelUpdate.ApplyLists(Apps, apps, (a, b) => string.Compare(a.Name, b.Name, StringComparison.Ordinal)); } [RelayCommand] diff --git a/App/ViewModels/TrayWindowViewModel.cs b/App/ViewModels/TrayWindowViewModel.cs index acc2a44..7aef055 100644 --- a/App/ViewModels/TrayWindowViewModel.cs +++ b/App/ViewModels/TrayWindowViewModel.cs @@ -191,7 +191,7 @@ private void UpdateFromRpcModel(RpcModel rpcModel) workspace.Name)); // Sort by status green, red, gray, then by hostname. - ModelMerge.MergeLists(Agents, agents, (a, b) => + ModelUpdate.ApplyLists(Agents, agents, (a, b) => { if (a.ConnectionStatus != b.ConnectionStatus) return a.ConnectionStatus.CompareTo(b.ConnectionStatus); diff --git a/Tests.App/Utils/ModelMergeTest.cs b/Tests.App/Utils/ModelUpdateTest.cs similarity index 74% rename from Tests.App/Utils/ModelMergeTest.cs rename to Tests.App/Utils/ModelUpdateTest.cs index 48ab05a..ef381f6 100644 --- a/Tests.App/Utils/ModelMergeTest.cs +++ b/Tests.App/Utils/ModelUpdateTest.cs @@ -5,17 +5,17 @@ namespace Coder.Desktop.Tests.App.Utils; #region ModelMerge test classes -public class MergeableItem : IModelMergeable +public class UpdateableItem : IModelUpdateable { public List AttemptedMerges = []; public int Id { get; } - public MergeableItem(int id) + public UpdateableItem(int id) { Id = id; } - public bool ApplyMerge(MergeableItem obj) + public bool TryApplyChanges(UpdateableItem obj) { AttemptedMerges.Add(obj.Id); return Id == obj.Id; @@ -30,10 +30,10 @@ public override string ToString() public override bool Equals(object? obj) { - return obj is MergeableItem other && Equals(other); + return obj is UpdateableItem other && Equals(other); } - public bool Equals(MergeableItem? other) + public bool Equals(UpdateableItem? other) { return other is not null && Id == other.Id; } @@ -43,12 +43,12 @@ public override int GetHashCode() return Id.GetHashCode(); } - public static bool operator ==(MergeableItem left, MergeableItem right) + public static bool operator ==(UpdateableItem left, UpdateableItem right) { return left.Equals(right); } - public static bool operator !=(MergeableItem left, MergeableItem right) + public static bool operator !=(UpdateableItem left, UpdateableItem right) { return !left.Equals(right); } @@ -75,6 +75,17 @@ public void Insert(int index, T item) }); } + public void Add(T item) + { + Items.Add(item); + Operations.Add(new ListOperation + { + Type = ListOperation.OperationType.Insert, + Index = Items.Count - 1, + Item = item, + }); + } + public void RemoveAt(int index) { var item = Items[index]; @@ -112,12 +123,6 @@ IEnumerator IEnumerable.GetEnumerator() throw new NotImplementedException(); } - public void Add(T item) - { - // We don't expect this to be called in the test. - throw new NotImplementedException(); - } - public void Clear() { // We don't expect this to be called in the test. @@ -173,7 +178,7 @@ public override bool Equals(object? obj) public bool Equals(ListOperation? other) { - return other is not null && Type == other.Type && Index == other.Index && Item.Equals(other.Item); + return other is not null && Type == other.Type && Index == other.Index && Item!.Equals(other.Item); } public override int GetHashCode() @@ -197,18 +202,18 @@ public override int GetHashCode() #endregion [TestFixture] -public class ModelMergeTest +public class ModelUpdateTest { [Test(Description = "Full merge test with merged, removed and added items")] public void Full() { - var original1 = new MergeableItem(1); - var original3 = new MergeableItem(3); - var original4 = new MergeableItem(4); - var update2 = new MergeableItem(2); - var update1 = new MergeableItem(1); - var update4 = new MergeableItem(4); - var target = new TrackableList + var original1 = new UpdateableItem(1); + var original3 = new UpdateableItem(3); + var original4 = new UpdateableItem(4); + var update2 = new UpdateableItem(2); + var update1 = new UpdateableItem(1); + var update4 = new UpdateableItem(4); + var target = new TrackableList { Items = [ @@ -217,14 +222,14 @@ public void Full() original4, ], }; - var update = new List + var update = new List { update2, update1, update4, }; - ModelMerge.MergeLists( + ModelUpdate.ApplyLists( target, update, (a, b) => a.Id - b.Id); @@ -247,19 +252,19 @@ public void Full() // We should've only performed two list writes operations. Removes are // processed first, then inserts. - Assert.That(target.Operations, Is.EquivalentTo(new List> + Assert.That(target.Operations, Is.EquivalentTo(new List> { // RemoveAt(1) => original3 => [original1, original4] new() { - Type = ListOperation.OperationType.RemoveAt, + Type = ListOperation.OperationType.RemoveAt, Index = 1, Item = original3, }, // Insert(1, update2) => [original1, update2, original4] new() { - Type = ListOperation.OperationType.Insert, + Type = ListOperation.OperationType.Insert, Index = 1, Item = update2, }, @@ -269,8 +274,8 @@ public void Full() [Test(Description = "Sorts when inserting")] public void Sorts() { - var target = new TrackableList(); - var update = new List + var target = new TrackableList(); + var update = new List { new(3), new(2), @@ -283,7 +288,7 @@ public void Sorts() new(7), new(9), }; - ModelMerge.MergeLists( + ModelUpdate.ApplyLists( target, update, (a, b) => a.Id - b.Id); @@ -295,77 +300,77 @@ public void Sorts() // Ensure it performed the correct operations. Assert.That(target.Operations.Count, Is.EqualTo(10)); - Assert.That(target.Operations, Is.EquivalentTo(new List> + Assert.That(target.Operations, Is.EquivalentTo(new List> { // Insert(0, 3) => [3] new() { - Type = ListOperation.OperationType.Insert, + Type = ListOperation.OperationType.Insert, Index = 0, - Item = new MergeableItem(3), + Item = new UpdateableItem(3), }, // Insert(0, 2) => [2, 3] new() { - Type = ListOperation.OperationType.Insert, + Type = ListOperation.OperationType.Insert, Index = 0, - Item = new MergeableItem(2), + Item = new UpdateableItem(2), }, // Insert(2, 5) => [2, 3, 5] new() { - Type = ListOperation.OperationType.Insert, + Type = ListOperation.OperationType.Insert, Index = 2, - Item = new MergeableItem(5), + Item = new UpdateableItem(5), }, // Insert(0, 0) => [0, 2, 3, 5] new() { - Type = ListOperation.OperationType.Insert, + Type = ListOperation.OperationType.Insert, Index = 0, - Item = new MergeableItem(0), + Item = new UpdateableItem(0), }, // Insert(3, 4) => [0, 2, 3, 4, 5] new() { - Type = ListOperation.OperationType.Insert, + Type = ListOperation.OperationType.Insert, Index = 3, - Item = new MergeableItem(4), + Item = new UpdateableItem(4), }, // Insert11, 1) => [0, 1, 2, 3, 4, 5] new() { - Type = ListOperation.OperationType.Insert, + Type = ListOperation.OperationType.Insert, Index = 1, - Item = new MergeableItem(1), + Item = new UpdateableItem(1), }, // Insert(6, 6) => [0, 1, 2, 3, 4, 5, 6] new() { - Type = ListOperation.OperationType.Insert, + Type = ListOperation.OperationType.Insert, Index = 6, - Item = new MergeableItem(6), + Item = new UpdateableItem(6), }, // Insert(7, 8) => [0, 1, 2, 3, 4, 5, 6, 8] new() { - Type = ListOperation.OperationType.Insert, + Type = ListOperation.OperationType.Insert, Index = 7, - Item = new MergeableItem(8), + Item = new UpdateableItem(8), }, // Insert(7, 7) => [0, 1, 2, 3, 4, 5, 6, 7, 8] new() { - Type = ListOperation.OperationType.Insert, + Type = ListOperation.OperationType.Insert, Index = 7, - Item = new MergeableItem(7), + Item = new UpdateableItem(7), }, // Insert(9, 9) => [0, 1, 2, 3, 4, 5, 6, 7, 8, 9] new() { - Type = ListOperation.OperationType.Insert, + Type = ListOperation.OperationType.Insert, Index = 9, - Item = new MergeableItem(9), + Item = new UpdateableItem(9), }, })); } @@ -373,24 +378,27 @@ public void Sorts() [Test(Description = "Sorts AFTER when inserting with matching sort order")] public void SortsAfter() { - var target = new List + var target = new List { new(1), new(3), + new(3), new(4), }; - var update = new List + var update = new List { new(4), new(2), new(3), + new(3), new(1), }; - ModelMerge.MergeLists( + ModelUpdate.ApplyLists( target, update, - // Sort 2 and 3 as equal, so that 2 is inserted after 3. + // Sort 2 and 3 as equal, so that 2 is inserted after both of the + // 3s. (a, b) => { if (a.Id is 2 or 3) return 0; @@ -398,8 +406,8 @@ public void SortsAfter() }); // Ensure it inserted with correct sorting. - Assert.That(target.Count, Is.EqualTo(4)); + Assert.That(target.Count, Is.EqualTo(5)); var ids = target.Select(i => i.Id).ToList(); - Assert.That(ids, Is.EquivalentTo([1, 3, 2, 4])); + Assert.That(ids, Is.EquivalentTo([1, 3, 3, 2, 4])); } } From 8879191469788002625e0e77f43e98c25866f324 Mon Sep 17 00:00:00 2001 From: Dean Sheather Date: Thu, 8 May 2025 13:26:37 +1000 Subject: [PATCH 05/11] redo of IconUrl and ImageSource --- App/ViewModels/AgentAppViewModel.cs | 143 +++++++++++++++--------- App/ViewModels/AgentViewModel.cs | 30 ++--- App/Views/Pages/TrayWindowMainPage.xaml | 4 +- 3 files changed, 103 insertions(+), 74 deletions(-) diff --git a/App/ViewModels/AgentAppViewModel.cs b/App/ViewModels/AgentAppViewModel.cs index fd2ed3b..55942bc 100644 --- a/App/ViewModels/AgentAppViewModel.cs +++ b/App/ViewModels/AgentAppViewModel.cs @@ -1,12 +1,16 @@ using System; using System.Linq; using Windows.System; +using Coder.Desktop.App.Models; +using Coder.Desktop.App.Services; using Coder.Desktop.App.Utils; using Coder.Desktop.Vpn.Proto; using CommunityToolkit.Mvvm.ComponentModel; using CommunityToolkit.Mvvm.Input; using Microsoft.Extensions.Logging; using Microsoft.UI.Xaml; +using Microsoft.UI.Xaml.Controls; +using Microsoft.UI.Xaml.Controls.Primitives; using Microsoft.UI.Xaml.Media; using Microsoft.UI.Xaml.Media.Imaging; @@ -14,21 +18,15 @@ namespace Coder.Desktop.App.ViewModels; public interface IAgentAppViewModelFactory { - public AgentAppViewModel Create(Uuid id, string name, Uri appUri, Uri? iconUrl); + public AgentAppViewModel Create(Uuid id, string name, string appUri, Uri? iconUrl); } -public class AgentAppViewModelFactory : IAgentAppViewModelFactory +public class AgentAppViewModelFactory(ILogger childLogger, ICredentialManager credentialManager) + : IAgentAppViewModelFactory { - private readonly ILogger _childLogger; - - public AgentAppViewModelFactory(ILogger childLogger) - { - _childLogger = childLogger; - } - - public AgentAppViewModel Create(Uuid id, string name, Uri appUri, Uri? iconUrl) + public AgentAppViewModel Create(Uuid id, string name, string appUri, Uri? iconUrl) { - return new AgentAppViewModel(_childLogger) + return new AgentAppViewModel(childLogger, credentialManager) { Id = id, Name = name, @@ -40,7 +38,10 @@ public AgentAppViewModel Create(Uuid id, string name, Uri appUri, Uri? iconUrl) public partial class AgentAppViewModel : ObservableObject, IModelUpdateable { + private const string SessionTokenUriVar = "$SESSION_TOKEN"; + private readonly ILogger _logger; + private readonly ICredentialManager _credentialManager; public required Uuid Id { get; init; } @@ -48,53 +49,29 @@ public partial class AgentAppViewModel : ObservableObject, IModelUpdateable (string.IsNullOrWhiteSpace(Name) ? "(no name)" : Name) + ":\n\n" + AppUri; - public ImageSource ImageSource - { - get - { - if (IconUrl is null || (IconUrl.Scheme != "http" && IconUrl.Scheme != "https")) - { - UseFallbackIcon = true; - return new BitmapImage(); - } - - // Determine what image source to use based on extension, use a - // BitmapImage as last resort. - var ext = IconUrl.AbsolutePath.Split('/').LastOrDefault()?.Split('.').LastOrDefault(); - // TODO: this is definitely a hack, URLs shouldn't need to end in .svg - if (ext is "svg") - { - // TODO: Some SVGs like `/icon/cursor.svg` contain PNG data and - // don't render at all. - var svg = new SvgImageSource(IconUrl); - svg.Opened += (_, _) => _logger.LogDebug("app icon opened (svg): {uri}", IconUrl); - svg.OpenFailed += (_, args) => - _logger.LogDebug("app icon failed to open (svg): {uri}: {Status}", IconUrl, args.Status); - return svg; - } - - var bitmap = new BitmapImage(IconUrl); - bitmap.ImageOpened += (_, _) => _logger.LogDebug("app icon opened (bitmap): {uri}", IconUrl); - bitmap.ImageFailed += (_, args) => - _logger.LogDebug("app icon failed to open (bitmap): {uri}: {ErrorMessage}", IconUrl, args.ErrorMessage); - return bitmap; - } - } - - public AgentAppViewModel(ILogger logger) + public AgentAppViewModel(ILogger logger, ICredentialManager credentialManager) { _logger = logger; + _credentialManager = credentialManager; + + // Apply the icon URL to the icon image source when it is updated. + IconImageSource = UpdateIcon(); + PropertyChanged += (_, args) => + { + if (args.PropertyName == nameof(IconUrl)) + IconImageSource = UpdateIcon(); + }; } public bool TryApplyChanges(AgentAppViewModel obj) @@ -116,6 +93,36 @@ public bool TryApplyChanges(AgentAppViewModel obj) return true; } + private ImageSource UpdateIcon() + { + if (IconUrl is null || (IconUrl.Scheme != "http" && IconUrl.Scheme != "https")) + { + UseFallbackIcon = true; + return new BitmapImage(); + } + + // Determine what image source to use based on extension, use a + // BitmapImage as last resort. + var ext = IconUrl.AbsolutePath.Split('/').LastOrDefault()?.Split('.').LastOrDefault(); + // TODO: this is definitely a hack, URLs shouldn't need to end in .svg + if (ext is "svg") + { + // TODO: Some SVGs like `/icon/cursor.svg` contain PNG data and + // don't render at all. + var svg = new SvgImageSource(IconUrl); + svg.Opened += (_, _) => _logger.LogDebug("app icon opened (svg): {uri}", IconUrl); + svg.OpenFailed += (_, args) => + _logger.LogDebug("app icon failed to open (svg): {uri}: {Status}", IconUrl, args.Status); + return svg; + } + + var bitmap = new BitmapImage(IconUrl); + bitmap.ImageOpened += (_, _) => _logger.LogDebug("app icon opened (bitmap): {uri}", IconUrl); + bitmap.ImageFailed += (_, args) => + _logger.LogDebug("app icon failed to open (bitmap): {uri}: {ErrorMessage}", IconUrl, args.ErrorMessage); + return bitmap; + } + public void OnImageOpened(object? sender, RoutedEventArgs e) { UseFallbackIcon = false; @@ -127,15 +134,45 @@ public void OnImageFailed(object? sender, RoutedEventArgs e) } [RelayCommand] - private void OpenApp() + private void OpenApp(object parameter) { try { - _ = Launcher.LaunchUriAsync(AppUri); + var uriString = AppUri; + var cred = _credentialManager.GetCachedCredentials(); + if (cred.State is CredentialState.Valid && cred.ApiToken is not null) + uriString = uriString.Replace(SessionTokenUriVar, cred.ApiToken); + uriString += SessionTokenUriVar; + if (uriString.Contains(SessionTokenUriVar)) + throw new Exception($"URI contains {SessionTokenUriVar} variable but could not be replaced"); + + var uri = new Uri(uriString); + _ = Launcher.LaunchUriAsync(uri); } - catch + catch (Exception e) { - // TODO: log/notify + _logger.LogWarning(e, "could not parse or launch app"); + + if (parameter is not FrameworkElement frameworkElement) return; + var flyout = new Flyout + { + Content = new TextBlock + { + Text = $"Could not open app: {e.Message}", + Margin = new Thickness(4), + TextWrapping = TextWrapping.Wrap, + }, + FlyoutPresenterStyle = new Style(typeof(FlyoutPresenter)) + { + Setters = + { + new Setter(ScrollViewer.HorizontalScrollModeProperty, ScrollMode.Disabled), + new Setter(ScrollViewer.HorizontalScrollBarVisibilityProperty, ScrollBarVisibility.Disabled), + }, + }, + }; + FlyoutBase.SetAttachedFlyout(frameworkElement, flyout); + FlyoutBase.ShowAttachedFlyout(frameworkElement); } } } diff --git a/App/ViewModels/AgentViewModel.cs b/App/ViewModels/AgentViewModel.cs index 0d6ebd2..f72bc97 100644 --- a/App/ViewModels/AgentViewModel.cs +++ b/App/ViewModels/AgentViewModel.cs @@ -26,26 +26,16 @@ public AgentViewModel Create(Uuid id, string hostname, string hostnameSuffix, AgentConnectionStatus connectionStatus, Uri dashboardBaseUrl, string? workspaceName); } -public class AgentViewModelFactory : IAgentViewModelFactory +public class AgentViewModelFactory( + ILogger childLogger, + ICoderApiClientFactory coderApiClientFactory, + ICredentialManager credentialManager, + IAgentAppViewModelFactory agentAppViewModelFactory) : IAgentViewModelFactory { - private readonly ILogger _childLogger; - private readonly ICoderApiClientFactory _coderApiClientFactory; - private readonly ICredentialManager _credentialManager; - private readonly IAgentAppViewModelFactory _agentAppViewModelFactory; - - public AgentViewModelFactory(ILogger childLogger, ICoderApiClientFactory coderApiClientFactory, - ICredentialManager credentialManager, IAgentAppViewModelFactory agentAppViewModelFactory) - { - _childLogger = childLogger; - _coderApiClientFactory = coderApiClientFactory; - _credentialManager = credentialManager; - _agentAppViewModelFactory = agentAppViewModelFactory; - } - public AgentViewModel Create(Uuid id, string hostname, string hostnameSuffix, AgentConnectionStatus connectionStatus, Uri dashboardBaseUrl, string? workspaceName) { - return new AgentViewModel(_childLogger, _coderApiClientFactory, _credentialManager, _agentAppViewModelFactory) + return new AgentViewModel(childLogger, coderApiClientFactory, credentialManager, agentAppViewModelFactory) { Id = id, Hostname = hostname, @@ -270,10 +260,10 @@ private void ContinueFetchApps(Task task) continue; } - if (!Uri.TryCreate(app.Url, UriKind.Absolute, out var appUri)) + if (string.IsNullOrEmpty(app.Url)) { - _logger.LogWarning("Could not parse app URI '{Url}' for '{DisplayName}', app will not appear in list", - app.Url, app.DisplayName); + _logger.LogWarning("App URI '{Url}' for '{DisplayName}' is empty, app will not appear in list", app.Url, + app.DisplayName); continue; } @@ -281,7 +271,7 @@ private void ContinueFetchApps(Task task) // icon. _ = Uri.TryCreate(DashboardBaseUrl, app.Icon, out var iconUrl); - apps.Add(_agentAppViewModelFactory.Create(uuid, app.DisplayName, appUri, iconUrl)); + apps.Add(_agentAppViewModelFactory.Create(uuid, app.DisplayName, app.Url, iconUrl)); } // Sort by name. diff --git a/App/Views/Pages/TrayWindowMainPage.xaml b/App/Views/Pages/TrayWindowMainPage.xaml index f18cd3e..5ab9ddb 100644 --- a/App/Views/Pages/TrayWindowMainPage.xaml +++ b/App/Views/Pages/TrayWindowMainPage.xaml @@ -251,16 +251,18 @@ Date: Thu, 8 May 2025 15:28:44 +1000 Subject: [PATCH 06/11] display apps --- App/ViewModels/AgentAppViewModel.cs | 1 - App/ViewModels/AgentViewModel.cs | 31 +++++++++++++++++++++++++++++ CoderSdk/Coder/WorkspaceAgents.cs | 9 +++++++++ 3 files changed, 40 insertions(+), 1 deletion(-) diff --git a/App/ViewModels/AgentAppViewModel.cs b/App/ViewModels/AgentAppViewModel.cs index 55942bc..fae81f3 100644 --- a/App/ViewModels/AgentAppViewModel.cs +++ b/App/ViewModels/AgentAppViewModel.cs @@ -142,7 +142,6 @@ private void OpenApp(object parameter) var cred = _credentialManager.GetCachedCredentials(); if (cred.State is CredentialState.Valid && cred.ApiToken is not null) uriString = uriString.Replace(SessionTokenUriVar, cred.ApiToken); - uriString += SessionTokenUriVar; if (uriString.Contains(SessionTokenUriVar)) throw new Exception($"URI contains {SessionTokenUriVar} variable but could not be replaced"); diff --git a/App/ViewModels/AgentViewModel.cs b/App/ViewModels/AgentViewModel.cs index f72bc97..c0b3bda 100644 --- a/App/ViewModels/AgentViewModel.cs +++ b/App/ViewModels/AgentViewModel.cs @@ -60,6 +60,11 @@ public partial class AgentViewModel : ObservableObject, IModelUpdateable _logger; private readonly ICoderApiClientFactory _coderApiClientFactory; private readonly ICredentialManager _credentialManager; @@ -274,6 +279,32 @@ private void ContinueFetchApps(Task task) apps.Add(_agentAppViewModelFactory.Create(uuid, app.DisplayName, app.Url, iconUrl)); } + foreach (var displayApp in workspaceAgent.DisplayApps) + { + if (displayApp is not WorkspaceAgent.DisplayAppVscode and not WorkspaceAgent.DisplayAppVscodeInsiders) + continue; + + var id = VscodeAppUuid; + var displayName = "VS Code"; + var icon = "/icon/code.svg"; + var scheme = "vscode"; + if (displayApp is WorkspaceAgent.DisplayAppVscodeInsiders) + { + id = VscodeInsidersAppUuid; + displayName = "VS Code Insiders"; + icon = "/icon/code-insiders.svg"; + scheme = "vscode-insiders"; + } + + var appUri = $"{scheme}://vscode-remote/ssh-remote+{FullHostname}/{workspaceAgent.ExpandedDirectory}"; + + // Icon parse failures are not fatal, we will just use the fallback + // icon. + _ = Uri.TryCreate(DashboardBaseUrl, icon, out var iconUrl); + + apps.Add(_agentAppViewModelFactory.Create(id, displayName, appUri, iconUrl)); + } + // Sort by name. ModelUpdate.ApplyLists(Apps, apps, (a, b) => string.Compare(a.Name, b.Name, StringComparison.Ordinal)); } diff --git a/CoderSdk/Coder/WorkspaceAgents.cs b/CoderSdk/Coder/WorkspaceAgents.cs index 01e93dd..5a5cb57 100644 --- a/CoderSdk/Coder/WorkspaceAgents.cs +++ b/CoderSdk/Coder/WorkspaceAgents.cs @@ -7,7 +7,16 @@ public partial interface ICoderApiClient public class WorkspaceAgent { + public const string DisplayAppVscode = "vscode"; + public const string DisplayAppVscodeInsiders = "vscode_insiders"; + + public string ExpandedDirectory { get; set; } = ""; + public WorkspaceApp[] Apps { get; set; } = []; + + // This isn't an enum to avoid future display apps breaking the desktop + // app. + public string[] DisplayApps { get; set; } = []; } public class WorkspaceApp From c5f360a050f1f0d3ac887f28a42aa702f95ca223 Mon Sep 17 00:00:00 2001 From: Dean Sheather Date: Thu, 8 May 2025 16:28:06 +1000 Subject: [PATCH 07/11] only one expanded at a time --- App/Controls/ExpandContent.xaml | 10 ----- App/ViewModels/TrayWindowViewModel.cs | 10 +++++ App/Views/Pages/TrayWindowMainPage.xaml | 18 ++++++--- App/Views/TrayWindow.xaml.cs | 53 +++++++++++++++---------- 4 files changed, 54 insertions(+), 37 deletions(-) diff --git a/App/Controls/ExpandContent.xaml b/App/Controls/ExpandContent.xaml index a53ad3a..d36170d 100644 --- a/App/Controls/ExpandContent.xaml +++ b/App/Controls/ExpandContent.xaml @@ -18,16 +18,6 @@ - - - - - Visible - - - + { + // When an agent is expanded: + if (args.PropertyName == nameof(AgentViewModel.IsExpanded) && agent.IsExpanded) + // Collapse every other agent. + foreach (var otherAgent in Agents.Where(a => a.Id != agent.Id && a.IsExpanded)) + otherAgent.IsExpanded = false; + }; + // Sort by status green, red, gray, then by hostname. ModelUpdate.ApplyLists(Agents, agents, (a, b) => { diff --git a/App/Views/Pages/TrayWindowMainPage.xaml b/App/Views/Pages/TrayWindowMainPage.xaml index 5ab9ddb..050fd68 100644 --- a/App/Views/Pages/TrayWindowMainPage.xaml +++ b/App/Views/Pages/TrayWindowMainPage.xaml @@ -232,16 +232,22 @@ - + + + + diff --git a/App/Views/TrayWindow.xaml.cs b/App/Views/TrayWindow.xaml.cs index 3475104..5d1755c 100644 --- a/App/Views/TrayWindow.xaml.cs +++ b/App/Views/TrayWindow.xaml.cs @@ -25,6 +25,7 @@ public sealed partial class TrayWindow : Window private const int WIDTH = 300; private NativeApi.POINT? _lastActivatePosition; + private int _maxHeightSinceLastActivation; private readonly IRpcController _rpcController; private readonly ICredentialManager _credentialManager; @@ -139,30 +140,22 @@ public void SetRootFrame(Page page) private void RootFrame_SizeChanged(object sender, SizedFrameEventArgs e) { - ResizeWindow(e.NewSize.Height); - MoveWindow(); + MoveAndResize(e.NewSize.Height); } - private void ResizeWindow() + private void MoveAndResize(double height) { - ResizeWindow(RootFrame.GetContentSize().Height); - } - - private void ResizeWindow(double height) - { - if (height <= 0) height = 100; // will be resolved next frame typically - - var scale = DisplayScale.WindowScale(this); - var newWidth = (int)(WIDTH * scale); - var newHeight = (int)(height * scale); - AppWindow.Resize(new SizeInt32(newWidth, newHeight)); + var size = CalculateWindowSize(height); + var pos = CalculateWindowPosition(size); + var rect = new RectInt32(pos.X, pos.Y, size.Width, size.Height); + AppWindow.MoveAndResize(rect); } private void MoveResizeAndActivate() { SaveCursorPos(); - ResizeWindow(); - MoveWindow(); + _maxHeightSinceLastActivation = 0; + MoveAndResize(RootFrame.GetContentSize().Height); AppWindow.Show(); NativeApi.SetForegroundWindow(WindowNative.GetWindowHandle(this)); } @@ -179,15 +172,33 @@ private void SaveCursorPos() _lastActivatePosition = null; } - private void MoveWindow() + private SizeInt32 CalculateWindowSize(double height) { - AppWindow.Move(GetWindowPosition()); + if (height <= 0) height = 100; // will be resolved next frame typically + + var scale = DisplayScale.WindowScale(this); + var newWidth = (int)(WIDTH * scale); + var newHeight = (int)(height * scale); + // Store the maximum height we've seen for positioning purposes. + if (newHeight > _maxHeightSinceLastActivation) + _maxHeightSinceLastActivation = newHeight; + + return new SizeInt32(newWidth, newHeight); } - private PointInt32 GetWindowPosition() + private PointInt32 CalculateWindowPosition(SizeInt32 size) { - var height = AppWindow.Size.Height; - var width = AppWindow.Size.Width; + var width = size.Width; + var height = size.Height; + // For positioning purposes, pretend the window is the maximum size it + // has been since it was last activated. This has the affect of + // allowing the window to move up to accomodate more content, but + // prevents it from moving back down when the window shrinks again. + // + // Prevents a lot of jittery behavior with app drawers. + if (height < _maxHeightSinceLastActivation) + height = _maxHeightSinceLastActivation; + var cursorPosition = _lastActivatePosition; if (cursorPosition is null) { From b215ba0f6afc69deeade50cb8a6a50243ed1d563 Mon Sep 17 00:00:00 2001 From: Dean Sheather Date: Thu, 8 May 2025 16:50:48 +1000 Subject: [PATCH 08/11] fix startup lock bug --- App/ViewModels/TrayWindowViewModel.cs | 25 ++++++++++++++++++++----- App/Views/Pages/TrayWindowMainPage.xaml | 2 +- 2 files changed, 21 insertions(+), 6 deletions(-) diff --git a/App/ViewModels/TrayWindowViewModel.cs b/App/ViewModels/TrayWindowViewModel.cs index 11b7f39..6a32d7b 100644 --- a/App/ViewModels/TrayWindowViewModel.cs +++ b/App/ViewModels/TrayWindowViewModel.cs @@ -16,7 +16,6 @@ using Microsoft.UI.Dispatching; using Microsoft.UI.Xaml; using Microsoft.UI.Xaml.Controls; -using Exception = System.Exception; namespace Coder.Desktop.App.ViewModels; @@ -106,8 +105,8 @@ public void Initialize(DispatcherQueue dispatcherQueue) _rpcController.StateChanged += (_, rpcModel) => UpdateFromRpcModel(rpcModel); UpdateFromRpcModel(_rpcController.GetState()); - _credentialManager.CredentialsChanged += (_, credentialModel) => UpdateFromCredentialsModel(credentialModel); - UpdateFromCredentialsModel(_credentialManager.GetCachedCredentials()); + _credentialManager.CredentialsChanged += (_, credentialModel) => UpdateFromCredentialModel(credentialModel); + UpdateFromCredentialModel(_credentialManager.GetCachedCredentials()); } private void UpdateFromRpcModel(RpcModel rpcModel) @@ -211,16 +210,32 @@ private void UpdateFromRpcModel(RpcModel rpcModel) if (Agents.Count < MaxAgents) ShowAllAgents = false; } - private void UpdateFromCredentialsModel(CredentialModel credentialModel) + private void UpdateFromCredentialModel(CredentialModel credentialModel) { // Ensure we're on the UI thread. if (_dispatcherQueue == null) return; if (!_dispatcherQueue.HasThreadAccess) { - _dispatcherQueue.TryEnqueue(() => UpdateFromCredentialsModel(credentialModel)); + _dispatcherQueue.TryEnqueue(() => UpdateFromCredentialModel(credentialModel)); return; } + // CredentialModel updates trigger RpcStateModel updates first. This + // resolves an issue on startup where the window would be locked for 5 + // seconds, even if all startup preconditions have been met: + // + // 1. RPC state updates, but credentials are invalid so the window + // enters the invalid loading state to prevent interaction. + // 2. Credential model finally becomes valid after reaching out to the + // server to check credentials. + // 3. UpdateFromCredentialModel previously did not re-trigger RpcModel + // update. + // 4. Five seconds after step 1, a new RPC state update would come in + // and finally unlock the window. + // + // Calling UpdateFromRpcModel at step 3 resolves this issue. + UpdateFromRpcModel(_rpcController.GetState()); + // HACK: the HyperlinkButton crashes the whole app if the initial URI // or this URI is invalid. CredentialModel.CoderUrl should never be // null while the Page is active as the Page is only displayed when diff --git a/App/Views/Pages/TrayWindowMainPage.xaml b/App/Views/Pages/TrayWindowMainPage.xaml index 050fd68..a277c12 100644 --- a/App/Views/Pages/TrayWindowMainPage.xaml +++ b/App/Views/Pages/TrayWindowMainPage.xaml @@ -241,7 +241,7 @@ Foreground="{ThemeResource SystemControlForegroundBaseMediumBrush}" TextAlignment="Center" VerticalAlignment="Center" - Margin="0,-1,0,0" /> + Margin="0,-3,0,0" /> Date: Thu, 8 May 2025 17:22:22 +1000 Subject: [PATCH 09/11] auto expand first agent --- App/ViewModels/TrayWindowViewModel.cs | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/App/ViewModels/TrayWindowViewModel.cs b/App/ViewModels/TrayWindowViewModel.cs index 6a32d7b..19ba3df 100644 --- a/App/ViewModels/TrayWindowViewModel.cs +++ b/App/ViewModels/TrayWindowViewModel.cs @@ -33,6 +33,11 @@ public partial class TrayWindowViewModel : ObservableObject private DispatcherQueue? _dispatcherQueue; + // When we transition from 0 online workspaces to >0 online workspaces, the + // first agent will be expanded. This bool tracks whether this has occurred + // yet (or if the user has expanded something themselves). + private bool _hasExpandedAgent; + // This isn't an ObservableProperty because the property itself never // changes. We add an event listener for the collection changing in the // constructor. @@ -194,9 +199,12 @@ private void UpdateFromRpcModel(RpcModel rpcModel) { // When an agent is expanded: if (args.PropertyName == nameof(AgentViewModel.IsExpanded) && agent.IsExpanded) + { + _hasExpandedAgent = true; // Collapse every other agent. foreach (var otherAgent in Agents.Where(a => a.Id != agent.Id && a.IsExpanded)) otherAgent.IsExpanded = false; + } }; // Sort by status green, red, gray, then by hostname. @@ -208,6 +216,15 @@ private void UpdateFromRpcModel(RpcModel rpcModel) }); if (Agents.Count < MaxAgents) ShowAllAgents = false; + + var firstOnlineAgent = agents.FirstOrDefault(a => a.ConnectionStatus != AgentConnectionStatus.Gray); + if (firstOnlineAgent is null) + _hasExpandedAgent = false; + if (!_hasExpandedAgent && firstOnlineAgent is not null) + { + firstOnlineAgent.SetExpanded(true); + _hasExpandedAgent = true; + } } private void UpdateFromCredentialModel(CredentialModel credentialModel) From b3c2eaaaee6200e668ba861c0aa7045d2d93374d Mon Sep 17 00:00:00 2001 From: Dean Sheather Date: Fri, 9 May 2025 16:32:02 +1000 Subject: [PATCH 10/11] rework agent expand bubbling --- App/App.xaml.cs | 3 +- App/Program.cs | 2 - App/Services/UserNotifier.cs | 1 - App/Utils/TitleBarIcon.cs | 17 ++--- App/ViewModels/AgentAppViewModel.cs | 34 ++++++---- App/ViewModels/AgentViewModel.cs | 89 ++++++++++++++++++++----- App/ViewModels/TrayWindowViewModel.cs | 48 ++++++++----- App/Views/DirectoryPickerWindow.xaml.cs | 1 - App/Views/FileSyncListWindow.xaml.cs | 3 +- App/Views/SignInWindow.xaml.cs | 1 - 10 files changed, 134 insertions(+), 65 deletions(-) diff --git a/App/App.xaml.cs b/App/App.xaml.cs index 2b747a6..c875a06 100644 --- a/App/App.xaml.cs +++ b/App/App.xaml.cs @@ -20,9 +20,9 @@ using Microsoft.UI.Xaml; using Microsoft.Win32; using Microsoft.Windows.AppLifecycle; +using Microsoft.Windows.AppNotifications; using Serilog; using LaunchActivatedEventArgs = Microsoft.UI.Xaml.LaunchActivatedEventArgs; -using Microsoft.Windows.AppNotifications; namespace Coder.Desktop.App; @@ -196,6 +196,7 @@ public void OnActivated(object? sender, AppActivationArguments args) _logger.LogWarning("URI activation with null data"); return; } + HandleURIActivation(protoArgs.Uri); break; diff --git a/App/Program.cs b/App/Program.cs index 3749c3b..bf4f16e 100644 --- a/App/Program.cs +++ b/App/Program.cs @@ -63,11 +63,9 @@ private static void Main(string[] args) notificationManager.NotificationInvoked += app.HandleNotification; notificationManager.Register(); if (activationArgs.Kind != ExtendedActivationKind.Launch) - { // this means we were activated without having already launched, so handle // the activation as well. app.OnActivated(null, activationArgs); - } }); } catch (Exception e) diff --git a/App/Services/UserNotifier.cs b/App/Services/UserNotifier.cs index 9cdf6c1..ca9ad2d 100644 --- a/App/Services/UserNotifier.cs +++ b/App/Services/UserNotifier.cs @@ -26,4 +26,3 @@ public Task ShowErrorNotification(string title, string message) return Task.CompletedTask; } } - diff --git a/App/Utils/TitleBarIcon.cs b/App/Utils/TitleBarIcon.cs index 3efc81d..283453d 100644 --- a/App/Utils/TitleBarIcon.cs +++ b/App/Utils/TitleBarIcon.cs @@ -1,19 +1,16 @@ -using System; using Microsoft.UI; using Microsoft.UI.Windowing; using Microsoft.UI.Xaml; -using Microsoft.UI.Xaml.Controls.Primitives; using WinRT.Interop; -namespace Coder.Desktop.App.Utils +namespace Coder.Desktop.App.Utils; + +public static class TitleBarIcon { - public static class TitleBarIcon + public static void SetTitlebarIcon(Window window) { - public static void SetTitlebarIcon(Window window) - { - var hwnd = WindowNative.GetWindowHandle(window); - var windowId = Win32Interop.GetWindowIdFromWindow(hwnd); - AppWindow.GetFromWindowId(windowId).SetIcon("coder.ico"); - } + var hwnd = WindowNative.GetWindowHandle(window); + var windowId = Win32Interop.GetWindowIdFromWindow(hwnd); + AppWindow.GetFromWindowId(windowId).SetIcon("coder.ico"); } } diff --git a/App/ViewModels/AgentAppViewModel.cs b/App/ViewModels/AgentAppViewModel.cs index fae81f3..c9bc0e6 100644 --- a/App/ViewModels/AgentAppViewModel.cs +++ b/App/ViewModels/AgentAppViewModel.cs @@ -18,13 +18,13 @@ namespace Coder.Desktop.App.ViewModels; public interface IAgentAppViewModelFactory { - public AgentAppViewModel Create(Uuid id, string name, string appUri, Uri? iconUrl); + public AgentAppViewModel Create(Uuid id, string name, Uri appUri, Uri? iconUrl); } public class AgentAppViewModelFactory(ILogger childLogger, ICredentialManager credentialManager) : IAgentAppViewModelFactory { - public AgentAppViewModel Create(Uuid id, string name, string appUri, Uri? iconUrl) + public AgentAppViewModel Create(Uuid id, string name, Uri appUri, Uri? iconUrl) { return new AgentAppViewModel(childLogger, credentialManager) { @@ -45,11 +45,13 @@ public partial class AgentAppViewModel : ObservableObject, IModelUpdateable Apps = []; - public required Uuid Id { get; init; } + public readonly Uuid Id; [ObservableProperty] [NotifyPropertyChangedFor(nameof(FullHostname))] @@ -160,12 +162,28 @@ public string DashboardUrl } public AgentViewModel(ILogger logger, ICoderApiClientFactory coderApiClientFactory, - ICredentialManager credentialManager, IAgentAppViewModelFactory agentAppViewModelFactory) + ICredentialManager credentialManager, IAgentAppViewModelFactory agentAppViewModelFactory, + IAgentExpanderHost expanderHost, Uuid id) { _logger = logger; _coderApiClientFactory = coderApiClientFactory; _credentialManager = credentialManager; _agentAppViewModelFactory = agentAppViewModelFactory; + _expanderHost = expanderHost; + + Id = id; + + PropertyChanged += (_, args) => + { + if (args.PropertyName == nameof(IsExpanded)) + { + _expanderHost.HandleAgentExpanded(Id, IsExpanded); + + // Every time the drawer is expanded, re-fetch all apps. + if (IsExpanded && !FetchingApps) + FetchApps(); + } + }; // Since the property value itself never changes, we add event // listeners for the underlying collection changing instead. @@ -202,18 +220,15 @@ public bool TryApplyChanges(AgentViewModel model) [RelayCommand] private void ToggleExpanded() { - // TODO: this should bubble to every other agent in the list so only - // one can be active at a time. SetExpanded(!IsExpanded); } public void SetExpanded(bool expanded) { + if (IsExpanded == expanded) return; + // This will bubble up to the TrayWindowViewModel because of the + // PropertyChanged handler. IsExpanded = expanded; - - // Every time the drawer is expanded, re-fetch all apps. - if (expanded && !FetchingApps) - FetchApps(); } partial void OnConnectionStatusChanged(AgentConnectionStatus oldValue, AgentConnectionStatus newValue) @@ -226,7 +241,26 @@ private void FetchApps() if (FetchingApps) return; FetchingApps = true; - var client = _coderApiClientFactory.Create(_credentialManager); + // If the workspace is off, then there's no agent and there's no apps. + if (ConnectionStatus == AgentConnectionStatus.Gray) + { + FetchingApps = false; + Apps.Clear(); + return; + } + + // API client creation could fail, which would leave FetchingApps true. + ICoderApiClient client; + try + { + client = _coderApiClientFactory.Create(_credentialManager); + } + catch + { + FetchingApps = false; + throw; + } + var cts = new CancellationTokenSource(TimeSpan.FromSeconds(15)); client.GetWorkspaceAgent(Id.ToString(), cts.Token).ContinueWith(t => { @@ -265,18 +299,24 @@ private void ContinueFetchApps(Task task) continue; } - if (string.IsNullOrEmpty(app.Url)) + if (!Uri.TryCreate(app.Url, UriKind.Absolute, out var appUri)) { - _logger.LogWarning("App URI '{Url}' for '{DisplayName}' is empty, app will not appear in list", app.Url, + _logger.LogWarning("Could not parse app URI '{Url}' for '{DisplayName}', app will not appear in list", + app.Url, app.DisplayName); continue; } + // HTTP or HTTPS external apps are usually things like + // wikis/documentation, which clutters up the app. + if (appUri.Scheme is "http" or "https") + continue; + // Icon parse failures are not fatal, we will just use the fallback // icon. _ = Uri.TryCreate(DashboardBaseUrl, app.Icon, out var iconUrl); - apps.Add(_agentAppViewModelFactory.Create(uuid, app.DisplayName, app.Url, iconUrl)); + apps.Add(_agentAppViewModelFactory.Create(uuid, app.DisplayName, appUri, iconUrl)); } foreach (var displayApp in workspaceAgent.DisplayApps) @@ -296,7 +336,22 @@ private void ContinueFetchApps(Task task) scheme = "vscode-insiders"; } - var appUri = $"{scheme}://vscode-remote/ssh-remote+{FullHostname}/{workspaceAgent.ExpandedDirectory}"; + Uri appUri; + try + { + appUri = new UriBuilder + { + Scheme = scheme, + Host = "vscode-remote", + Path = $"/ssh-remote+{FullHostname}/{workspaceAgent.ExpandedDirectory}", + }.Uri; + } + catch (Exception e) + { + _logger.LogWarning("Could not craft app URI for display app {displayApp}, app will not appear in list", + displayApp); + continue; + } // Icon parse failures are not fatal, we will just use the fallback // icon. diff --git a/App/ViewModels/TrayWindowViewModel.cs b/App/ViewModels/TrayWindowViewModel.cs index 19ba3df..35f8df1 100644 --- a/App/ViewModels/TrayWindowViewModel.cs +++ b/App/ViewModels/TrayWindowViewModel.cs @@ -19,7 +19,12 @@ namespace Coder.Desktop.App.ViewModels; -public partial class TrayWindowViewModel : ObservableObject +public interface IAgentExpanderHost +{ + public void HandleAgentExpanded(Uuid id, bool expanded); +} + +public partial class TrayWindowViewModel : ObservableObject, IAgentExpanderHost { private const int MaxAgents = 5; private const string DefaultDashboardUrl = "https://coder.com"; @@ -82,7 +87,7 @@ public partial class TrayWindowViewModel : ObservableObject public IEnumerable VisibleAgents => ShowAllAgents ? Agents : Agents.Take(MaxAgents); - [ObservableProperty] public partial string DashboardUrl { get; set; } = "https://coder.com"; + [ObservableProperty] public partial string DashboardUrl { get; set; } = DefaultDashboardUrl; public TrayWindowViewModel(IServiceProvider services, IRpcController rpcController, ICredentialManager credentialManager, IAgentViewModelFactory agentViewModelFactory) @@ -103,6 +108,24 @@ public TrayWindowViewModel(IServiceProvider services, IRpcController rpcControll }; } + // Implements IAgentExpanderHost + public void HandleAgentExpanded(Uuid id, bool expanded) + { + // Ensure we're on the UI thread. + if (_dispatcherQueue == null) return; + if (!_dispatcherQueue.HasThreadAccess) + { + _dispatcherQueue.TryEnqueue(() => HandleAgentExpanded(id, expanded)); + return; + } + + if (!expanded) return; + _hasExpandedAgent = true; + // Collapse every other agent. + foreach (var otherAgent in Agents.Where(a => a.Id != id)) + otherAgent.SetExpanded(false); + } + public void Initialize(DispatcherQueue dispatcherQueue) { _dispatcherQueue = dispatcherQueue; @@ -170,6 +193,7 @@ private void UpdateFromRpcModel(RpcModel rpcModel) var workspace = rpcModel.Workspaces.FirstOrDefault(w => w.Id == agent.WorkspaceId); agents.Add(_agentViewModelFactory.Create( + this, agent.ParseId(), fqdnPrefix, fqdnSuffix, @@ -183,6 +207,7 @@ private void UpdateFromRpcModel(RpcModel rpcModel) foreach (var workspace in rpcModel.Workspaces.Where(w => w.Status == Workspace.Types.Status.Stopped && !workspacesWithAgents.Contains(w.Id))) agents.Add(_agentViewModelFactory.Create( + this, // Workspace ID is fine as a stand-in here, it shouldn't // conflict with any agent IDs. workspace.ParseId(), @@ -194,19 +219,6 @@ private void UpdateFromRpcModel(RpcModel rpcModel) credentialModel.CoderUrl, workspace.Name)); - foreach (var agent in agents) - agent.PropertyChanged += (_, args) => - { - // When an agent is expanded: - if (args.PropertyName == nameof(AgentViewModel.IsExpanded) && agent.IsExpanded) - { - _hasExpandedAgent = true; - // Collapse every other agent. - foreach (var otherAgent in Agents.Where(a => a.Id != agent.Id && a.IsExpanded)) - otherAgent.IsExpanded = false; - } - }; - // Sort by status green, red, gray, then by hostname. ModelUpdate.ApplyLists(Agents, agents, (a, b) => { @@ -306,13 +318,13 @@ private static string MaybeUnwrapTunnelError(Exception e) } [RelayCommand] - public void ToggleShowAllAgents() + private void ToggleShowAllAgents() { ShowAllAgents = !ShowAllAgents; } [RelayCommand] - public void ShowFileSyncListWindow() + private void ShowFileSyncListWindow() { // This is safe against concurrent access since it all happens in the // UI thread. @@ -328,7 +340,7 @@ public void ShowFileSyncListWindow() } [RelayCommand] - public void SignOut() + private void SignOut() { if (VpnLifecycle is not VpnLifecycle.Stopped) return; diff --git a/App/Views/DirectoryPickerWindow.xaml.cs b/App/Views/DirectoryPickerWindow.xaml.cs index a84e535..7af6db3 100644 --- a/App/Views/DirectoryPickerWindow.xaml.cs +++ b/App/Views/DirectoryPickerWindow.xaml.cs @@ -9,7 +9,6 @@ using Microsoft.UI.Xaml.Media; using WinRT.Interop; using WinUIEx; -using Coder.Desktop.App.Utils; namespace Coder.Desktop.App.Views; diff --git a/App/Views/FileSyncListWindow.xaml.cs b/App/Views/FileSyncListWindow.xaml.cs index fb899cc..ccd2452 100644 --- a/App/Views/FileSyncListWindow.xaml.cs +++ b/App/Views/FileSyncListWindow.xaml.cs @@ -1,8 +1,8 @@ +using Coder.Desktop.App.Utils; using Coder.Desktop.App.ViewModels; using Coder.Desktop.App.Views.Pages; using Microsoft.UI.Xaml.Media; using WinUIEx; -using Coder.Desktop.App.Utils; namespace Coder.Desktop.App.Views; @@ -23,5 +23,4 @@ public FileSyncListWindow(FileSyncListViewModel viewModel) this.CenterOnScreen(); } - } diff --git a/App/Views/SignInWindow.xaml.cs b/App/Views/SignInWindow.xaml.cs index 7e17911..2acd0a5 100644 --- a/App/Views/SignInWindow.xaml.cs +++ b/App/Views/SignInWindow.xaml.cs @@ -7,7 +7,6 @@ using Microsoft.UI.Windowing; using Microsoft.UI.Xaml; using Microsoft.UI.Xaml.Media; -using Coder.Desktop.App.Utils; namespace Coder.Desktop.App.Views; From b31dd2ebb9bbbf51b652a8c6e130d9bf997d681e Mon Sep 17 00:00:00 2001 From: Dean Sheather Date: Fri, 9 May 2025 17:07:48 +1000 Subject: [PATCH 11/11] fix fmt --- App/Utils/ModelUpdate.cs | 12 ++++++++++-- Coder.Desktop.sln.DotSettings | 1 + 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/App/Utils/ModelUpdate.cs b/App/Utils/ModelUpdate.cs index 3e37b0e..010c510 100644 --- a/App/Utils/ModelUpdate.cs +++ b/App/Utils/ModelUpdate.cs @@ -71,7 +71,11 @@ public static void ApplyLists(IList target, IEnumerable update, Compari target.RemoveAt(i); i--; - OuterLoopEnd: ; + // Rider fights `dotnet format` about whether there should be a + // space before the semicolon or not. +#pragma warning disable format + OuterLoopEnd: ; +#pragma warning enable format } // Add any items that were missing into their correct sorted place. @@ -91,7 +95,11 @@ public static void ApplyLists(IList target, IEnumerable update, Compari // equal to or after every other item. target.Add(newItem); - OuterLoopEnd: ; + // Rider fights `dotnet format` about whether there should be a + // space before the semicolon or not. +#pragma warning disable format + OuterLoopEnd: ; +#pragma warning enable format } } } diff --git a/Coder.Desktop.sln.DotSettings b/Coder.Desktop.sln.DotSettings index bf138c2..f524684 100644 --- a/Coder.Desktop.sln.DotSettings +++ b/Coder.Desktop.sln.DotSettings @@ -5,6 +5,7 @@ True True True + True <Patterns xmlns="urn:schemas-jetbrains-com:member-reordering-patterns"> <TypePattern DisplayName="Non-reorderable types" Priority="99999999"> <TypePattern.Match>