diff --git a/src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/Models/JSCommandProviderProxy.cs b/src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/Models/JSCommandProviderProxy.cs index f51d47e1ae..0823b4370d 100644 --- a/src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/Models/JSCommandProviderProxy.cs +++ b/src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/Models/JSCommandProviderProxy.cs @@ -3,6 +3,7 @@ // See the LICENSE file in the project root for more information. using System; +using System.Collections.Concurrent; using System.Collections.Generic; using System.Text.Json; using System.Text.Json.Nodes; @@ -26,7 +27,7 @@ public sealed partial class JSCommandProviderProxy : ICommandProvider, IDisposab private readonly JSExtensionManifest _manifest; private readonly IconInfo _icon; private readonly Dictionary<(string Message, int State), StatusMessage> _shownStatusMessages = new(); - private readonly Dictionary _fallbackAdapters = new(); + private readonly ConcurrentDictionary _fallbackAdapters = new(); private IExtensionHost? _host; private ICommandSettings? _settingsCache; private bool _settingsQueried; diff --git a/src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/Models/JSFallbackCommandItemAdapter.cs b/src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/Models/JSFallbackCommandItemAdapter.cs index dc824cd47e..139e3c112c 100644 --- a/src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/Models/JSFallbackCommandItemAdapter.cs +++ b/src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/Models/JSFallbackCommandItemAdapter.cs @@ -98,7 +98,7 @@ internal sealed partial class JSFallbackCommandItemAdapter : BaseObservable, IFa { try { - _connection.SendNotificationAsync( + _connection.SendRequestAsync( "fallback/updateQuery", new JsonObject { ["commandId"] = _commandId, ["query"] = query }, CancellationToken.None).GetAwaiter().GetResult(); diff --git a/src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/Models/JSListPageProxy.cs b/src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/Models/JSListPageProxy.cs index 00f22bafa0..c2a55b5d27 100644 --- a/src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/Models/JSListPageProxy.cs +++ b/src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/Models/JSListPageProxy.cs @@ -5,6 +5,7 @@ using System; using System.Collections.Concurrent; using System.Collections.Generic; +using System.Runtime.CompilerServices; using System.Text.Json; using System.Text.Json.Nodes; using System.Threading; @@ -24,7 +25,10 @@ namespace Microsoft.CmdPal.UI.ViewModels.Models; /// internal sealed partial class JSListPageProxy : BaseObservable, IListPage { - private static readonly ConcurrentDictionary ProxyRegistry = new(); + // Routing is scoped per connection so that identical page ids from different + // extensions never collide. Proxies are held weakly so they can be collected + // without the registry keeping them alive. + private static readonly ConditionalWeakTable Registries = new(); private readonly string _pageId; private readonly JsonRpcConnection _connection; @@ -36,8 +40,14 @@ internal sealed partial class JSListPageProxy : BaseObservable, IListPage _connection = connection ?? throw new ArgumentNullException(nameof(connection)); _pageData = pageData; - ProxyRegistry[_pageId] = this; - _connection.RegisterNotificationHandler("listPage/itemsChanged", DispatchItemsChanged); + var registry = Registries.GetValue(_connection, static conn => + { + var created = new PageRegistry(); + conn.RegisterNotificationHandler("listPage/itemsChanged", paramsElement => DispatchItemsChanged(created, paramsElement)); + return created; + }); + + registry.Pages[_pageId] = new WeakReference(this); } public event TypedEventHandler? ItemsChanged; @@ -131,7 +141,7 @@ internal sealed partial class JSListPageProxy : BaseObservable, IListPage } } - private static void DispatchItemsChanged(JsonElement paramsElement) + private static void DispatchItemsChanged(PageRegistry registry, JsonElement paramsElement) { try { @@ -142,11 +152,18 @@ internal sealed partial class JSListPageProxy : BaseObservable, IListPage } var pageId = pageProp.GetString(); - if (pageId == null || !ProxyRegistry.TryGetValue(pageId, out var proxy)) + if (pageId == null || !registry.Pages.TryGetValue(pageId, out var weakProxy)) { return; } + if (!weakProxy.TryGetTarget(out var proxy)) + { + // The proxy has been collected; drop the stale entry. + registry.Pages.TryRemove(pageId, out _); + return; + } + var totalItems = -1; if (paramsElement.TryGetProperty("totalItems", out var totalItemsProp) && totalItemsProp.ValueKind == JsonValueKind.Number) @@ -202,4 +219,9 @@ internal sealed partial class JSListPageProxy : BaseObservable, IListPage return items.ToArray(); } + + private sealed class PageRegistry + { + public ConcurrentDictionary> Pages { get; } = new(); + } } diff --git a/src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/Models/JSModelMapper.cs b/src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/Models/JSModelMapper.cs index 58581895c1..77fc0f6868 100644 --- a/src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/Models/JSModelMapper.cs +++ b/src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/Models/JSModelMapper.cs @@ -291,7 +291,7 @@ internal static class JSModelMapper return null; } - var layout = GetString(gridProp, "layout") ?? string.Empty; + var layout = GetString(gridProp, "type") ?? string.Empty; var showTitle = GetBool(gridProp, "showTitle", true); var showSubtitle = GetBool(gridProp, "showSubtitle", true); @@ -316,7 +316,7 @@ internal static class JSModelMapper var filters = new List(); foreach (var element in filtersProp.EnumerateArray()) { - if (GetBool(element, "_isSeparator", false)) + if (GetBool(element, "separator", false)) { filters.Add(new Separator(GetString(element, "title") ?? string.Empty)); continue; diff --git a/src/modules/cmdpal/Tests/Microsoft.CmdPal.UI.ViewModels.UnitTests/JSAdapterProxyTests.cs b/src/modules/cmdpal/Tests/Microsoft.CmdPal.UI.ViewModels.UnitTests/JSAdapterProxyTests.cs index 3053049dc6..0e32bfce29 100644 --- a/src/modules/cmdpal/Tests/Microsoft.CmdPal.UI.ViewModels.UnitTests/JSAdapterProxyTests.cs +++ b/src/modules/cmdpal/Tests/Microsoft.CmdPal.UI.ViewModels.UnitTests/JSAdapterProxyTests.cs @@ -269,6 +269,68 @@ public class JSAdapterProxyTests Assert.IsNotNull(settings!.SettingsPage); } + [TestMethod] + public void ListPage_ExposesFiltersWithSeparatorAndGridType() + { + using var fake = new JSFakeExtension(); + var commandJson = + """ + { + "id": "list-fg", + "pageType": "listPage", + "name": "Filtered", + "gridProperties": { "type": "medium", "showTitle": true }, + "filters": { + "currentFilterId": "all", + "filters": [ + { "id": "all", "name": "All" }, + { "separator": true }, + { "id": "recent", "name": "Recent" } + ] + } + } + """; + fake.OnResult("provider/getCommand", commandJson); + + var provider = CreateProvider(fake); + var page = (IListPage)provider.GetCommand("list-fg")!; + + Assert.IsInstanceOfType(page.GridProperties, typeof(IMediumGridLayout)); + + Assert.IsNotNull(page.Filters); + var filters = page.Filters!.GetFilters(); + Assert.AreEqual(3, filters.Length); + Assert.IsInstanceOfType(filters[0], typeof(IFilter)); + Assert.AreEqual("all", ((IFilter)filters[0]).Id); + Assert.IsInstanceOfType(filters[1], typeof(ISeparatorFilterItem)); + Assert.IsInstanceOfType(filters[2], typeof(IFilter)); + Assert.AreEqual("recent", ((IFilter)filters[2]).Id); + } + + [TestMethod] + public async Task FallbackHandler_SendsUpdateQueryAsRequest() + { + using var fake = new JSFakeExtension(); + fake.OnResult( + "provider/getFallbackCommands", + """[ { "id": "fb-req", "displayTitle": "Initial", "title": "T" } ]"""); + + var received = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + fake.OnRequest("fallback/updateQuery", element => + { + received.TrySetResult(element.GetProperty("query").GetString() ?? string.Empty); + return null; + }); + + var provider = CreateProvider(fake); + var fallback = provider.FallbackCommands()![0]; + + await Task.Run(() => fallback.FallbackHandler.UpdateQuery("typed")); + + var query = await received.Task.WaitAsync(Timeout); + Assert.AreEqual("typed", query); + } + private static void AssertKind(JSFakeExtension fake, IInvokableCommand invokable, string resultJson, CommandResultKind expected) { fake.OnResult("command/invoke", resultJson);