mirror of
https://github.com/microsoft/PowerToys.git
synced 2026-09-01 19:51:34 +02:00
Fix JsonRpc proxy registration lifetimes
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 813c51f6-19e2-49ee-bf03-10e3187d3728
This commit is contained in:
@@ -18,17 +18,23 @@ namespace Microsoft.CmdPal.JsonRpc.Models;
|
||||
/// </summary>
|
||||
internal sealed partial class JSCommandItemAdapter : JSObservableProxyBase, ICommandItem
|
||||
{
|
||||
private Lazy<ICommand?> _command;
|
||||
private readonly JSLazyCache<ICommand?> _command;
|
||||
private readonly JSLazyCache<IContextItem[]> _moreCommands;
|
||||
|
||||
public JSCommandItemAdapter(JsonElement data, JsonRpcConnection connection)
|
||||
: base(GetNotificationId(data), connection, data)
|
||||
{
|
||||
_command = CreateCommand();
|
||||
_command = new JSLazyCache<ICommand?>(
|
||||
CreateCommand,
|
||||
JSLazyCache<ICommand?>.DisposeValue);
|
||||
_moreCommands = new JSLazyCache<IContextItem[]>(
|
||||
() => JSModelMapper.ParseMoreCommands(Data, Connection),
|
||||
JSModelMapper.DisposeContextItems);
|
||||
}
|
||||
|
||||
public ICommand? Command => Volatile.Read(ref _command).Value;
|
||||
public ICommand? Command => _command.Value;
|
||||
|
||||
public IContextItem[] MoreCommands => JSModelMapper.ParseMoreCommands(Data, Connection);
|
||||
public IContextItem[] MoreCommands => _moreCommands.Value;
|
||||
|
||||
public IIconInfo Icon => JSModelMapper.TryGetIcon(Data, "icon", out var icon)
|
||||
? icon
|
||||
@@ -50,16 +56,25 @@ internal sealed partial class JSCommandItemAdapter : JSObservableProxyBase, ICom
|
||||
{
|
||||
if (propertyName == "command")
|
||||
{
|
||||
Volatile.Write(ref _command, CreateCommand());
|
||||
break;
|
||||
_command.Reset();
|
||||
}
|
||||
else if (propertyName == "moreCommands")
|
||||
{
|
||||
_moreCommands.Reset();
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
private Lazy<ICommand?> CreateCommand()
|
||||
public override void Dispose()
|
||||
{
|
||||
return new Lazy<ICommand?>(
|
||||
() => JSCommandFactory.CreateCommandFromJson(JSModelMapper.GetCommandData(Data), Connection));
|
||||
_moreCommands.Dispose();
|
||||
_command.Dispose();
|
||||
base.Dispose();
|
||||
}
|
||||
|
||||
private ICommand? CreateCommand()
|
||||
{
|
||||
return JSCommandFactory.CreateCommandFromJson(JSModelMapper.GetCommandData(Data), Connection);
|
||||
}
|
||||
|
||||
private static string GetNotificationId(JsonElement data)
|
||||
|
||||
@@ -35,6 +35,7 @@ public sealed partial class JSCommandProviderProxy : ICommandProvider4, IDisposa
|
||||
// Guards _shownStatusMessages. Host status notifications and Dispose can run
|
||||
// on different threads, so reads, writes, and enumeration share one gate.
|
||||
private readonly object _statusLock = new();
|
||||
private readonly object _settingsLock = new();
|
||||
|
||||
// Host notifications can arrive after this proxy subscribes but before
|
||||
// InitializeWithHost attaches the host. Buffer them in arrival order so
|
||||
@@ -44,8 +45,9 @@ public sealed partial class JSCommandProviderProxy : ICommandProvider4, IDisposa
|
||||
private List<BufferedHostNotification>? _preInitNotifications = new();
|
||||
private IExtensionHost? _host;
|
||||
private ICommandSettings? _settingsCache;
|
||||
private bool _settingsLoading;
|
||||
private bool _settingsQueried;
|
||||
private bool _isDisposed;
|
||||
private volatile bool _isDisposed;
|
||||
|
||||
public JSCommandProviderProxy(
|
||||
JsonRpcConnection connection,
|
||||
@@ -84,39 +86,43 @@ public sealed partial class JSCommandProviderProxy : ICommandProvider4, IDisposa
|
||||
{
|
||||
get
|
||||
{
|
||||
if (_settingsQueried)
|
||||
lock (_settingsLock)
|
||||
{
|
||||
return _settingsCache;
|
||||
}
|
||||
while (_settingsLoading && !_isDisposed)
|
||||
{
|
||||
Monitor.Wait(_settingsLock);
|
||||
}
|
||||
|
||||
_settingsQueried = true;
|
||||
if (_isDisposed)
|
||||
{
|
||||
return null;
|
||||
}
|
||||
|
||||
try
|
||||
{
|
||||
var response = _connection.SendRequestAsync(
|
||||
"provider/getSettings",
|
||||
null,
|
||||
CancellationToken.None).GetAwaiter().GetResult();
|
||||
|
||||
if (response.Error != null ||
|
||||
!response.Result.HasValue ||
|
||||
response.Result.Value.ValueKind != JsonValueKind.Object)
|
||||
if (_settingsQueried)
|
||||
{
|
||||
return _settingsCache;
|
||||
}
|
||||
|
||||
var pageId = JSModelMapper.GetString(response.Result.Value, "id") ?? string.Empty;
|
||||
if (!string.IsNullOrEmpty(pageId))
|
||||
{
|
||||
_settingsCache = new JSCommandSettingsProxy(pageId, _connection, response.Result.Value.Clone());
|
||||
}
|
||||
}
|
||||
catch (Exception ex)
|
||||
{
|
||||
Logger.LogDebug($"Failed to get settings for {DisplayName}: {ex.Message}");
|
||||
_settingsLoading = true;
|
||||
}
|
||||
|
||||
return _settingsCache;
|
||||
var settings = LoadSettings();
|
||||
lock (_settingsLock)
|
||||
{
|
||||
_settingsLoading = false;
|
||||
_settingsQueried = true;
|
||||
if (_isDisposed)
|
||||
{
|
||||
(settings as IDisposable)?.Dispose();
|
||||
}
|
||||
else
|
||||
{
|
||||
_settingsCache = settings;
|
||||
}
|
||||
|
||||
Monitor.PulseAll(_settingsLock);
|
||||
return _settingsCache;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -297,6 +303,12 @@ public sealed partial class JSCommandProviderProxy : ICommandProvider4, IDisposa
|
||||
}
|
||||
}
|
||||
|
||||
lock (_settingsLock)
|
||||
{
|
||||
(_settingsCache as IDisposable)?.Dispose();
|
||||
Monitor.PulseAll(_settingsLock);
|
||||
}
|
||||
|
||||
_host = null;
|
||||
}
|
||||
|
||||
@@ -320,6 +332,34 @@ public sealed partial class JSCommandProviderProxy : ICommandProvider4, IDisposa
|
||||
_connection.RegisterNotificationHandler("host/copyText", HandleCopyTextNotification);
|
||||
}
|
||||
|
||||
private ICommandSettings? LoadSettings()
|
||||
{
|
||||
try
|
||||
{
|
||||
var response = _connection.SendRequestAsync(
|
||||
"provider/getSettings",
|
||||
null,
|
||||
CancellationToken.None).GetAwaiter().GetResult();
|
||||
|
||||
if (response.Error != null ||
|
||||
!response.Result.HasValue ||
|
||||
response.Result.Value.ValueKind != JsonValueKind.Object)
|
||||
{
|
||||
return null;
|
||||
}
|
||||
|
||||
var pageId = JSModelMapper.GetString(response.Result.Value, "id") ?? string.Empty;
|
||||
return string.IsNullOrEmpty(pageId)
|
||||
? null
|
||||
: new JSCommandSettingsProxy(pageId, _connection, response.Result.Value.Clone());
|
||||
}
|
||||
catch (Exception ex)
|
||||
{
|
||||
Logger.LogDebug($"Failed to get settings for {DisplayName}: {ex.Message}");
|
||||
return null;
|
||||
}
|
||||
}
|
||||
|
||||
// Buffers a host notification until InitializeWithHost attaches the host.
|
||||
// The params element is cloned because the connection may recycle the source document.
|
||||
private bool TryBufferUntilHostAttached(string method, JsonElement paramsElement)
|
||||
|
||||
@@ -13,18 +13,18 @@ namespace Microsoft.CmdPal.JsonRpc.Models;
|
||||
/// The full settings page payload is kept intact, including title, name, icon,
|
||||
/// details, and commands.
|
||||
/// </summary>
|
||||
internal sealed partial class JSCommandSettingsProxy : ICommandSettings
|
||||
internal sealed partial class JSCommandSettingsProxy : ICommandSettings, IDisposable
|
||||
{
|
||||
private readonly string _settingsPageId;
|
||||
private readonly JsonRpcConnection _connection;
|
||||
private readonly JsonElement _settingsPageData;
|
||||
private readonly JSLazyCache<IContentPage> _settingsPage;
|
||||
|
||||
public JSCommandSettingsProxy(string settingsPageId, JsonRpcConnection connection, JsonElement settingsPageData = default)
|
||||
{
|
||||
_settingsPageId = settingsPageId;
|
||||
_connection = connection;
|
||||
_settingsPageData = settingsPageData;
|
||||
_settingsPage = new JSLazyCache<IContentPage>(
|
||||
() => new JSContentPageProxy(settingsPageId, connection, settingsPageData),
|
||||
JSLazyCache<IContentPage>.DisposeValue);
|
||||
}
|
||||
|
||||
public IContentPage SettingsPage => new JSContentPageProxy(_settingsPageId, _connection, _settingsPageData);
|
||||
public IContentPage SettingsPage => _settingsPage.Value;
|
||||
|
||||
public void Dispose() => _settingsPage.Dispose();
|
||||
}
|
||||
|
||||
@@ -29,11 +29,19 @@ internal sealed partial class JSContentPageProxy : JSObservableProxyBase, IConte
|
||||
|
||||
private readonly string _pageId;
|
||||
private readonly PageRegistry _registry;
|
||||
private readonly JSLazyCache<IDetails?> _details;
|
||||
private readonly JSLazyCache<IContextItem[]> _commands;
|
||||
|
||||
public JSContentPageProxy(string pageId, JsonRpcConnection connection, JsonElement pageData = default)
|
||||
: base(pageId, connection, pageData)
|
||||
{
|
||||
_pageId = pageId ?? throw new ArgumentNullException(nameof(pageId));
|
||||
_details = new JSLazyCache<IDetails?>(
|
||||
() => JSModelMapper.ParseDetails(Data, Connection),
|
||||
JSModelMapper.DisposeDetails);
|
||||
_commands = new JSLazyCache<IContextItem[]>(
|
||||
() => JSModelMapper.ParseContextItems(Data, "commands", Connection),
|
||||
JSModelMapper.DisposeContextItems);
|
||||
|
||||
_registry = Registries.GetValue(Connection, static _ => new PageRegistry());
|
||||
_registry.EnsureSubscribed(Connection);
|
||||
@@ -59,9 +67,9 @@ internal sealed partial class JSContentPageProxy : JSObservableProxyBase, IConte
|
||||
|
||||
public OptionalColor AccentColor => JSModelMapper.ParseColor(Data, "accentColor");
|
||||
|
||||
public IDetails? Details => JSModelMapper.ParseDetails(Data, Connection);
|
||||
public IDetails? Details => _details.Value;
|
||||
|
||||
public IContextItem[] Commands => JSModelMapper.ParseContextItems(Data, "commands", Connection);
|
||||
public IContextItem[] Commands => _commands.Value;
|
||||
|
||||
public IContent[] GetContent()
|
||||
{
|
||||
@@ -89,6 +97,8 @@ internal sealed partial class JSContentPageProxy : JSObservableProxyBase, IConte
|
||||
|
||||
public override void Dispose()
|
||||
{
|
||||
_details.Dispose();
|
||||
_commands.Dispose();
|
||||
if (_registry.Pages.TryGetValue(_pageId, out var pages))
|
||||
{
|
||||
lock (pages)
|
||||
@@ -111,6 +121,21 @@ internal sealed partial class JSContentPageProxy : JSObservableProxyBase, IConte
|
||||
_ => false,
|
||||
};
|
||||
|
||||
protected override void OnPropertyChangesApplied(IReadOnlyList<string> propertyNames)
|
||||
{
|
||||
foreach (var propertyName in propertyNames)
|
||||
{
|
||||
if (propertyName == "details")
|
||||
{
|
||||
_details.Reset();
|
||||
}
|
||||
else if (propertyName == "commands")
|
||||
{
|
||||
_commands.Reset();
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
private static JsonElement? UnwrapContent(JsonElement? result)
|
||||
{
|
||||
if (!result.HasValue)
|
||||
|
||||
@@ -21,18 +21,24 @@ namespace Microsoft.CmdPal.JsonRpc.Models;
|
||||
internal sealed partial class JSFallbackCommandItemAdapter : JSObservableProxyBase, IFallbackCommandItem2
|
||||
{
|
||||
private readonly object _commandStateLock = new();
|
||||
private Lazy<ICommand?> _command;
|
||||
private readonly JSLazyCache<ICommand?> _command;
|
||||
private readonly JSLazyCache<IContextItem[]> _moreCommands;
|
||||
private IFallbackHandler? _fallbackHandler;
|
||||
|
||||
public JSFallbackCommandItemAdapter(JsonElement data, JsonRpcConnection connection)
|
||||
: base(GetNotificationId(data), connection, data)
|
||||
{
|
||||
_command = CreateCommand();
|
||||
_command = new JSLazyCache<ICommand?>(
|
||||
CreateCommand,
|
||||
JSLazyCache<ICommand?>.DisposeValue);
|
||||
_moreCommands = new JSLazyCache<IContextItem[]>(
|
||||
() => JSModelMapper.ParseMoreCommands(Data, Connection),
|
||||
JSModelMapper.DisposeContextItems);
|
||||
}
|
||||
|
||||
public ICommand? Command => Volatile.Read(ref _command).Value;
|
||||
public ICommand? Command => _command.Value;
|
||||
|
||||
public IContextItem[] MoreCommands => JSModelMapper.ParseMoreCommands(Data, Connection);
|
||||
public IContextItem[] MoreCommands => _moreCommands.Value;
|
||||
|
||||
public IIconInfo Icon => JSModelMapper.TryGetIcon(Data, "icon", out var icon)
|
||||
? icon
|
||||
@@ -72,19 +78,31 @@ internal sealed partial class JSFallbackCommandItemAdapter : JSObservableProxyBa
|
||||
{
|
||||
lock (_commandStateLock)
|
||||
{
|
||||
Volatile.Write(ref _command, CreateCommand());
|
||||
_command.Reset();
|
||||
_fallbackHandler = null;
|
||||
}
|
||||
|
||||
break;
|
||||
}
|
||||
else if (propertyName == "moreCommands")
|
||||
{
|
||||
_moreCommands.Reset();
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
private Lazy<ICommand?> CreateCommand()
|
||||
public override void Dispose()
|
||||
{
|
||||
return new Lazy<ICommand?>(
|
||||
() => JSCommandFactory.CreateCommandFromJson(JSModelMapper.GetCommandData(Data), Connection));
|
||||
_moreCommands.Dispose();
|
||||
lock (_commandStateLock)
|
||||
{
|
||||
_command.Dispose();
|
||||
}
|
||||
|
||||
base.Dispose();
|
||||
}
|
||||
|
||||
private ICommand? CreateCommand()
|
||||
{
|
||||
return JSCommandFactory.CreateCommandFromJson(JSModelMapper.GetCommandData(Data), Connection);
|
||||
}
|
||||
|
||||
private static string GetNotificationId(JsonElement data)
|
||||
|
||||
@@ -0,0 +1,87 @@
|
||||
// Copyright (c) Microsoft Corporation
|
||||
// The Microsoft Corporation licenses this file to you under the MIT license.
|
||||
// See the LICENSE file in the project root for more information.
|
||||
|
||||
using System;
|
||||
|
||||
namespace Microsoft.CmdPal.JsonRpc.Models;
|
||||
|
||||
internal sealed partial class JSLazyCache<T> : IDisposable
|
||||
{
|
||||
private readonly object _lock = new();
|
||||
private readonly Func<T> _factory;
|
||||
private readonly Action<T>? _dispose;
|
||||
private T _value = default!;
|
||||
private bool _hasValue;
|
||||
private bool _disposed;
|
||||
|
||||
internal JSLazyCache(Func<T> factory, Action<T>? dispose = null)
|
||||
{
|
||||
_factory = factory;
|
||||
_dispose = dispose;
|
||||
}
|
||||
|
||||
internal T Value
|
||||
{
|
||||
get
|
||||
{
|
||||
lock (_lock)
|
||||
{
|
||||
ObjectDisposedException.ThrowIf(_disposed, this);
|
||||
|
||||
if (!_hasValue)
|
||||
{
|
||||
_value = _factory();
|
||||
_hasValue = true;
|
||||
}
|
||||
|
||||
return _value;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
public void Dispose()
|
||||
{
|
||||
lock (_lock)
|
||||
{
|
||||
if (_disposed)
|
||||
{
|
||||
return;
|
||||
}
|
||||
|
||||
_disposed = true;
|
||||
DisposeCreatedValue();
|
||||
}
|
||||
}
|
||||
|
||||
internal void Reset()
|
||||
{
|
||||
lock (_lock)
|
||||
{
|
||||
if (_disposed)
|
||||
{
|
||||
return;
|
||||
}
|
||||
|
||||
DisposeCreatedValue();
|
||||
_value = default!;
|
||||
_hasValue = false;
|
||||
}
|
||||
}
|
||||
|
||||
internal static void DisposeValue(T value)
|
||||
{
|
||||
if (value is IDisposable disposable)
|
||||
{
|
||||
disposable.Dispose();
|
||||
}
|
||||
}
|
||||
|
||||
private void DisposeCreatedValue()
|
||||
{
|
||||
if (_hasValue)
|
||||
{
|
||||
_dispose?.Invoke(_value);
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -19,17 +19,27 @@ namespace Microsoft.CmdPal.JsonRpc.Models;
|
||||
/// </summary>
|
||||
internal sealed partial class JSListItemAdapter : JSObservableProxyBase, IListItem
|
||||
{
|
||||
private Lazy<ICommand?> _command;
|
||||
private readonly JSLazyCache<ICommand?> _command;
|
||||
private readonly JSLazyCache<IContextItem[]> _moreCommands;
|
||||
private readonly JSLazyCache<IDetails?> _details;
|
||||
|
||||
public JSListItemAdapter(JsonElement data, JsonRpcConnection connection)
|
||||
: base(GetNotificationId(data), connection, data)
|
||||
{
|
||||
_command = CreateCommand();
|
||||
_command = new JSLazyCache<ICommand?>(
|
||||
CreateCommand,
|
||||
JSLazyCache<ICommand?>.DisposeValue);
|
||||
_moreCommands = new JSLazyCache<IContextItem[]>(
|
||||
() => JSModelMapper.ParseMoreCommands(Data, Connection),
|
||||
JSModelMapper.DisposeContextItems);
|
||||
_details = new JSLazyCache<IDetails?>(
|
||||
() => JSModelMapper.ParseDetails(Data, Connection),
|
||||
JSModelMapper.DisposeDetails);
|
||||
}
|
||||
|
||||
public ICommand? Command => Volatile.Read(ref _command).Value;
|
||||
public ICommand? Command => _command.Value;
|
||||
|
||||
public IContextItem[] MoreCommands => JSModelMapper.ParseMoreCommands(Data, Connection);
|
||||
public IContextItem[] MoreCommands => _moreCommands.Value;
|
||||
|
||||
public IIconInfo Icon => JSModelMapper.TryGetIcon(Data, "icon", out var icon)
|
||||
? icon
|
||||
@@ -41,7 +51,7 @@ internal sealed partial class JSListItemAdapter : JSObservableProxyBase, IListIt
|
||||
|
||||
public ITag[] Tags => JSModelMapper.ParseTags(Data);
|
||||
|
||||
public IDetails? Details => JSModelMapper.ParseDetails(Data, Connection);
|
||||
public IDetails? Details => _details.Value;
|
||||
|
||||
public string Section => JSModelMapper.GetString(Data, "section") ?? string.Empty;
|
||||
|
||||
@@ -60,20 +70,32 @@ internal sealed partial class JSListItemAdapter : JSObservableProxyBase, IListIt
|
||||
{
|
||||
if (propertyName == "command")
|
||||
{
|
||||
Volatile.Write(ref _command, CreateCommand());
|
||||
break;
|
||||
_command.Reset();
|
||||
}
|
||||
else if (propertyName == "moreCommands")
|
||||
{
|
||||
_moreCommands.Reset();
|
||||
}
|
||||
else if (propertyName == "details")
|
||||
{
|
||||
_details.Reset();
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
private Lazy<ICommand?> CreateCommand()
|
||||
public override void Dispose()
|
||||
{
|
||||
return new Lazy<ICommand?>(() =>
|
||||
{
|
||||
return JSModelMapper.TryGetCommandData(Data, out var commandData)
|
||||
? JSCommandFactory.CreateCommandFromJson(commandData, Connection)
|
||||
: null;
|
||||
});
|
||||
_moreCommands.Dispose();
|
||||
_details.Dispose();
|
||||
_command.Dispose();
|
||||
base.Dispose();
|
||||
}
|
||||
|
||||
private ICommand? CreateCommand()
|
||||
{
|
||||
return JSModelMapper.TryGetCommandData(Data, out var commandData)
|
||||
? JSCommandFactory.CreateCommandFromJson(commandData, Connection)
|
||||
: null;
|
||||
}
|
||||
|
||||
private static string GetNotificationId(JsonElement data)
|
||||
|
||||
@@ -35,6 +35,8 @@ internal sealed partial class JSListPageProxy : JSObservableProxyBase, IListPage
|
||||
private readonly string _pageId;
|
||||
private readonly PageRegistry _registry;
|
||||
private readonly object _stateLock = new();
|
||||
private readonly JSLazyCache<IFilters?> _filters;
|
||||
private readonly JSLazyCache<ICommandItem?> _emptyContent;
|
||||
private bool? _hasMoreItemsState;
|
||||
private bool _disposed;
|
||||
|
||||
@@ -42,6 +44,10 @@ internal sealed partial class JSListPageProxy : JSObservableProxyBase, IListPage
|
||||
: base(pageId, connection, pageData)
|
||||
{
|
||||
_pageId = pageId ?? throw new ArgumentNullException(nameof(pageId));
|
||||
_filters = new JSLazyCache<IFilters?>(CreateFilters);
|
||||
_emptyContent = new JSLazyCache<ICommandItem?>(
|
||||
CreateEmptyContent,
|
||||
JSLazyCache<ICommandItem?>.DisposeValue);
|
||||
|
||||
// Get the retained registry before subscribing. ConditionalWeakTable may run
|
||||
// the factory on a thread that loses the race and discards its result. If the
|
||||
@@ -77,19 +83,7 @@ internal sealed partial class JSListPageProxy : JSObservableProxyBase, IListPage
|
||||
|
||||
public bool ShowDetails => JSModelMapper.GetBool(Data, "showDetails", false);
|
||||
|
||||
public IFilters? Filters
|
||||
{
|
||||
get
|
||||
{
|
||||
if (JSModelMapper.TryGetProperty(Data, "filters", out var filtersProp) &&
|
||||
filtersProp.ValueKind == JsonValueKind.Object)
|
||||
{
|
||||
return new JSFiltersAdapter(filtersProp, Connection, _pageId);
|
||||
}
|
||||
|
||||
return null;
|
||||
}
|
||||
}
|
||||
public IFilters? Filters => _filters.Value;
|
||||
|
||||
public IGridProperties? GridProperties => JSModelMapper.ParseGridProperties(Data);
|
||||
|
||||
@@ -107,19 +101,7 @@ internal sealed partial class JSListPageProxy : JSObservableProxyBase, IListPage
|
||||
}
|
||||
}
|
||||
|
||||
public ICommandItem? EmptyContent
|
||||
{
|
||||
get
|
||||
{
|
||||
if (JSModelMapper.TryGetProperty(Data, "emptyContent", out var emptyProp) &&
|
||||
emptyProp.ValueKind == JsonValueKind.Object)
|
||||
{
|
||||
return new JSCommandItemAdapter(emptyProp, Connection);
|
||||
}
|
||||
|
||||
return null;
|
||||
}
|
||||
}
|
||||
public ICommandItem? EmptyContent => _emptyContent.Value;
|
||||
|
||||
public IListItem[] GetItems()
|
||||
{
|
||||
@@ -309,6 +291,8 @@ internal sealed partial class JSListPageProxy : JSObservableProxyBase, IListPage
|
||||
|
||||
_disposed = true;
|
||||
|
||||
_filters.Dispose();
|
||||
_emptyContent.Dispose();
|
||||
base.Dispose();
|
||||
|
||||
if (_registry.Pages.TryGetValue(_pageId, out var list))
|
||||
@@ -395,18 +379,44 @@ internal sealed partial class JSListPageProxy : JSObservableProxyBase, IListPage
|
||||
|
||||
protected override void OnPropertyChangesApplied(IReadOnlyList<string> propertyNames)
|
||||
{
|
||||
if (!propertyNames.Contains("hasMoreItems"))
|
||||
foreach (var propertyName in propertyNames)
|
||||
{
|
||||
return;
|
||||
if (propertyName == "filters")
|
||||
{
|
||||
_filters.Reset();
|
||||
}
|
||||
else if (propertyName == "emptyContent")
|
||||
{
|
||||
_emptyContent.Reset();
|
||||
}
|
||||
}
|
||||
|
||||
var value = JSModelMapper.GetBool(Data, "hasMoreItems", false);
|
||||
lock (_stateLock)
|
||||
if (propertyNames.Contains("hasMoreItems"))
|
||||
{
|
||||
_hasMoreItemsState = value;
|
||||
var value = JSModelMapper.GetBool(Data, "hasMoreItems", false);
|
||||
lock (_stateLock)
|
||||
{
|
||||
_hasMoreItemsState = value;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
private IFilters? CreateFilters()
|
||||
{
|
||||
return JSModelMapper.TryGetProperty(Data, "filters", out var filtersProp) &&
|
||||
filtersProp.ValueKind == JsonValueKind.Object
|
||||
? new JSFiltersAdapter(filtersProp, Connection, _pageId)
|
||||
: null;
|
||||
}
|
||||
|
||||
private ICommandItem? CreateEmptyContent()
|
||||
{
|
||||
return JSModelMapper.TryGetProperty(Data, "emptyContent", out var emptyProp) &&
|
||||
emptyProp.ValueKind == JsonValueKind.Object
|
||||
? new JSCommandItemAdapter(emptyProp, Connection)
|
||||
: null;
|
||||
}
|
||||
|
||||
private IListItem[] ParseListItems(JsonElement? result)
|
||||
{
|
||||
if (!result.HasValue)
|
||||
|
||||
@@ -284,6 +284,47 @@ internal static class JSModelMapper
|
||||
return items.ToArray();
|
||||
}
|
||||
|
||||
internal static void DisposeContextItems(IEnumerable<IContextItem> items)
|
||||
{
|
||||
foreach (var item in items)
|
||||
{
|
||||
if (item is not ICommandContextItem commandItem)
|
||||
{
|
||||
continue;
|
||||
}
|
||||
|
||||
DisposeContextItems(commandItem.MoreCommands);
|
||||
if (commandItem.Command is IDisposable disposable)
|
||||
{
|
||||
disposable.Dispose();
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
internal static void DisposeDetails(IDetails? details)
|
||||
{
|
||||
if (details is null)
|
||||
{
|
||||
return;
|
||||
}
|
||||
|
||||
foreach (var element in details.Metadata)
|
||||
{
|
||||
if (element.Data is not IDetailsCommands commands)
|
||||
{
|
||||
continue;
|
||||
}
|
||||
|
||||
foreach (var command in commands.Commands ?? [])
|
||||
{
|
||||
if (command is IDisposable disposable)
|
||||
{
|
||||
disposable.Dispose();
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
internal static ICommandContextItem ParseContextItem(JsonElement element, JsonRpcConnection connection)
|
||||
{
|
||||
var command = JSCommandFactory.CreateCommandFromJson(GetCommandData(element), connection);
|
||||
|
||||
@@ -18,10 +18,26 @@ internal static class JSPropertyChangeRegistry
|
||||
internal static void Register(JsonRpcConnection connection, string commandId, IJSPropertyChangeTarget target)
|
||||
{
|
||||
var registry = Registries.GetValue(connection, static _ => new Registry());
|
||||
var targets = registry.Targets.GetOrAdd(commandId, static _ => []);
|
||||
lock (targets)
|
||||
while (true)
|
||||
{
|
||||
targets.Add(new WeakReference<IJSPropertyChangeTarget>(target));
|
||||
var targets = registry.Targets.GetOrAdd(commandId, static _ => []);
|
||||
lock (targets)
|
||||
{
|
||||
if (!registry.Targets.TryGetValue(commandId, out var currentTargets) ||
|
||||
!ReferenceEquals(targets, currentTargets))
|
||||
{
|
||||
continue;
|
||||
}
|
||||
|
||||
targets.RemoveAll(reference => !reference.TryGetTarget(out _));
|
||||
if (!targets.Exists(reference =>
|
||||
reference.TryGetTarget(out var current) && ReferenceEquals(current, target)))
|
||||
{
|
||||
targets.Add(new WeakReference<IJSPropertyChangeTarget>(target));
|
||||
}
|
||||
|
||||
return;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -39,7 +55,7 @@ internal static class JSPropertyChangeRegistry
|
||||
!reference.TryGetTarget(out var current) || ReferenceEquals(current, target));
|
||||
if (targets.Count == 0)
|
||||
{
|
||||
registry.Targets.TryRemove(commandId, out _);
|
||||
RemoveTargets(registry, commandId, targets);
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -73,6 +89,11 @@ internal static class JSPropertyChangeRegistry
|
||||
liveTargets.Add(target);
|
||||
}
|
||||
}
|
||||
|
||||
if (targets.Count == 0)
|
||||
{
|
||||
RemoveTargets(registry, commandId, targets);
|
||||
}
|
||||
}
|
||||
|
||||
foreach (var target in liveTargets)
|
||||
@@ -81,6 +102,29 @@ internal static class JSPropertyChangeRegistry
|
||||
}
|
||||
}
|
||||
|
||||
internal static int GetRegistrationCount(JsonRpcConnection connection, string commandId)
|
||||
{
|
||||
if (!Registries.TryGetValue(connection, out var registry) ||
|
||||
!registry.Targets.TryGetValue(commandId, out var targets))
|
||||
{
|
||||
return 0;
|
||||
}
|
||||
|
||||
lock (targets)
|
||||
{
|
||||
return targets.Count;
|
||||
}
|
||||
}
|
||||
|
||||
private static void RemoveTargets(
|
||||
Registry registry,
|
||||
string commandId,
|
||||
List<WeakReference<IJSPropertyChangeTarget>> targets)
|
||||
{
|
||||
((ICollection<KeyValuePair<string, List<WeakReference<IJSPropertyChangeTarget>>>>)registry.Targets)
|
||||
.Remove(new KeyValuePair<string, List<WeakReference<IJSPropertyChangeTarget>>>(commandId, targets));
|
||||
}
|
||||
|
||||
private sealed class Registry
|
||||
{
|
||||
internal ConcurrentDictionary<string, List<WeakReference<IJSPropertyChangeTarget>>> Targets { get; } = new();
|
||||
|
||||
@@ -3,10 +3,13 @@
|
||||
// See the LICENSE file in the project root for more information.
|
||||
|
||||
using System;
|
||||
using System.Runtime.CompilerServices;
|
||||
using System.Text.Json;
|
||||
using System.Text.Json.Nodes;
|
||||
using System.Threading;
|
||||
using System.Threading.Tasks;
|
||||
using Microsoft.CmdPal.JsonRpc.Models;
|
||||
using Microsoft.CommandPalette.Extensions;
|
||||
using Microsoft.CommandPalette.Extensions.Toolkit;
|
||||
using Microsoft.VisualStudio.TestTools.UnitTesting;
|
||||
|
||||
@@ -14,6 +17,204 @@ namespace Microsoft.CmdPal.JsonRpc.UnitTests;
|
||||
|
||||
public partial class JSAdapterTests
|
||||
{
|
||||
[TestMethod]
|
||||
public void PropertyChangeRegistry_PrunesDeadTargetsAndDeduplicatesLiveTargets()
|
||||
{
|
||||
using var fake = new JSFakeExtension();
|
||||
const string commandId = "shared-command";
|
||||
var deadTarget = RegisterTemporaryTarget(fake.Connection, commandId);
|
||||
|
||||
GC.Collect();
|
||||
GC.WaitForPendingFinalizers();
|
||||
GC.Collect();
|
||||
|
||||
Assert.IsFalse(deadTarget.TryGetTarget(out _));
|
||||
Assert.AreEqual(1, JSPropertyChangeRegistry.GetRegistrationCount(fake.Connection, commandId));
|
||||
|
||||
var liveTarget = new RecordingPropertyChangeTarget();
|
||||
Parallel.For(
|
||||
0,
|
||||
64,
|
||||
_ => JSPropertyChangeRegistry.Register(fake.Connection, commandId, liveTarget));
|
||||
|
||||
Assert.AreEqual(1, JSPropertyChangeRegistry.GetRegistrationCount(fake.Connection, commandId));
|
||||
|
||||
JSPropertyChangeRegistry.Dispatch(
|
||||
fake.Connection,
|
||||
ParseElement(new JsonObject
|
||||
{
|
||||
["commandId"] = commandId,
|
||||
["properties"] = new JsonObject { ["title"] = "updated" },
|
||||
}));
|
||||
|
||||
Assert.AreEqual(1, liveTarget.ApplyCount);
|
||||
JSPropertyChangeRegistry.Unregister(fake.Connection, commandId, liveTarget);
|
||||
}
|
||||
|
||||
[TestMethod]
|
||||
public void NestedProxyGetters_CacheIdentityAndRegistration()
|
||||
{
|
||||
using var fake = new JSFakeExtension();
|
||||
using var settings = new JSCommandSettingsProxy("settings", fake.Connection);
|
||||
using var listPage = new JSListPageProxy(
|
||||
"list-page",
|
||||
fake.Connection,
|
||||
ParseElement(new JsonObject
|
||||
{
|
||||
["id"] = "list-page",
|
||||
["filters"] = new JsonObject { ["filters"] = new JsonArray() },
|
||||
["emptyContent"] = CommandItem("empty-command"),
|
||||
}));
|
||||
using var contentPage = new JSContentPageProxy(
|
||||
"content-page",
|
||||
fake.Connection,
|
||||
ParseElement(new JsonObject
|
||||
{
|
||||
["id"] = "content-page",
|
||||
["details"] = new JsonObject { ["title"] = "Details" },
|
||||
["commands"] = new JsonArray(ContextItem("content-command")),
|
||||
}));
|
||||
using var commandItem = new JSCommandItemAdapter(
|
||||
ParseElement(new JsonObject
|
||||
{
|
||||
["id"] = "command-item",
|
||||
["title"] = "Command",
|
||||
["moreCommands"] = new JsonArray(ContextItem("command-more")),
|
||||
}),
|
||||
fake.Connection);
|
||||
using var listItem = new JSListItemAdapter(
|
||||
ParseElement(new JsonObject
|
||||
{
|
||||
["id"] = "list-item",
|
||||
["title"] = "List",
|
||||
["details"] = new JsonObject { ["title"] = "Details" },
|
||||
["moreCommands"] = new JsonArray(ContextItem("list-more")),
|
||||
}),
|
||||
fake.Connection);
|
||||
using var fallbackItem = new JSFallbackCommandItemAdapter(
|
||||
ParseElement(new JsonObject
|
||||
{
|
||||
["id"] = "fallback-item",
|
||||
["title"] = "Fallback",
|
||||
["command"] = Command("fallback-command"),
|
||||
["moreCommands"] = new JsonArray(ContextItem("fallback-more")),
|
||||
}),
|
||||
fake.Connection);
|
||||
var concurrentSettingsPages = new IContentPage[64];
|
||||
Parallel.For(0, concurrentSettingsPages.Length, i => concurrentSettingsPages[i] = settings.SettingsPage);
|
||||
|
||||
foreach (var settingsPage in concurrentSettingsPages)
|
||||
{
|
||||
Assert.AreSame(settings.SettingsPage, settingsPage);
|
||||
}
|
||||
|
||||
Assert.AreSame(listPage.Filters, listPage.Filters);
|
||||
Assert.AreSame(listPage.EmptyContent, listPage.EmptyContent);
|
||||
Assert.AreSame(contentPage.Details, contentPage.Details);
|
||||
Assert.AreSame(contentPage.Commands, contentPage.Commands);
|
||||
Assert.AreSame(commandItem.MoreCommands, commandItem.MoreCommands);
|
||||
Assert.AreSame(listItem.MoreCommands, listItem.MoreCommands);
|
||||
Assert.AreSame(listItem.Details, listItem.Details);
|
||||
Assert.AreSame(fallbackItem.MoreCommands, fallbackItem.MoreCommands);
|
||||
|
||||
Assert.AreEqual(1, JSPropertyChangeRegistry.GetRegistrationCount(fake.Connection, "empty-command"));
|
||||
Assert.AreEqual(1, JSPropertyChangeRegistry.GetRegistrationCount(fake.Connection, "content-command"));
|
||||
Assert.AreEqual(1, JSPropertyChangeRegistry.GetRegistrationCount(fake.Connection, "command-more"));
|
||||
Assert.AreEqual(1, JSPropertyChangeRegistry.GetRegistrationCount(fake.Connection, "list-more"));
|
||||
Assert.AreEqual(1, JSPropertyChangeRegistry.GetRegistrationCount(fake.Connection, "fallback-more"));
|
||||
}
|
||||
|
||||
[TestMethod]
|
||||
public void NestedProxyGetters_InvalidateOnlyTheChangedProperty()
|
||||
{
|
||||
using var fake = new JSFakeExtension();
|
||||
using var item = new JSListItemAdapter(
|
||||
ParseElement(new JsonObject
|
||||
{
|
||||
["id"] = "list-item",
|
||||
["title"] = "Before",
|
||||
["details"] = new JsonObject { ["title"] = "Old details" },
|
||||
["moreCommands"] = new JsonArray(ContextItem("old-more")),
|
||||
}),
|
||||
fake.Connection);
|
||||
var originalDetails = item.Details;
|
||||
var originalMoreCommands = item.MoreCommands;
|
||||
|
||||
JSPropertyChangeRegistry.Dispatch(
|
||||
fake.Connection,
|
||||
ParseElement(new JsonObject
|
||||
{
|
||||
["commandId"] = "list-item",
|
||||
["properties"] = new JsonObject { ["title"] = "After" },
|
||||
}));
|
||||
|
||||
Assert.AreEqual("After", item.Title);
|
||||
Assert.AreSame(originalDetails, item.Details);
|
||||
Assert.AreSame(originalMoreCommands, item.MoreCommands);
|
||||
|
||||
JSPropertyChangeRegistry.Dispatch(
|
||||
fake.Connection,
|
||||
ParseElement(new JsonObject
|
||||
{
|
||||
["commandId"] = "list-item",
|
||||
["properties"] = new JsonObject
|
||||
{
|
||||
["details"] = new JsonObject { ["title"] = "New details" },
|
||||
["moreCommands"] = new JsonArray(ContextItem("new-more")),
|
||||
},
|
||||
}));
|
||||
|
||||
Assert.AreEqual("New details", item.Details?.Title);
|
||||
Assert.AreNotSame(originalDetails, item.Details);
|
||||
Assert.AreNotSame(originalMoreCommands, item.MoreCommands);
|
||||
Assert.AreEqual(0, JSPropertyChangeRegistry.GetRegistrationCount(fake.Connection, "old-more"));
|
||||
Assert.AreEqual(1, JSPropertyChangeRegistry.GetRegistrationCount(fake.Connection, "new-more"));
|
||||
}
|
||||
|
||||
[TestMethod]
|
||||
public void ProviderSettings_ConcurrentReadsReturnOneProxy()
|
||||
{
|
||||
using var fake = new JSFakeExtension();
|
||||
var requestCount = 0;
|
||||
fake.OnRequest("provider/getSettings", _ =>
|
||||
{
|
||||
Interlocked.Increment(ref requestCount);
|
||||
return new JsonObject { ["id"] = "settings-page" };
|
||||
});
|
||||
using var provider = CreateProvider(fake);
|
||||
var settings = new ICommandSettings?[64];
|
||||
|
||||
Parallel.For(0, settings.Length, i => settings[i] = provider.Settings);
|
||||
|
||||
Assert.AreEqual(1, requestCount);
|
||||
foreach (var current in settings)
|
||||
{
|
||||
Assert.AreSame(settings[0], current);
|
||||
}
|
||||
}
|
||||
|
||||
[TestMethod]
|
||||
public async Task ProviderSettings_DisposeDuringRequestDoesNotPublishProxy()
|
||||
{
|
||||
using var fake = new JSFakeExtension();
|
||||
using var requestStarted = new ManualResetEventSlim();
|
||||
using var releaseRequest = new ManualResetEventSlim();
|
||||
fake.OnRequest("provider/getSettings", _ =>
|
||||
{
|
||||
requestStarted.Set();
|
||||
releaseRequest.Wait(Timeout);
|
||||
return new JsonObject { ["id"] = "settings-page" };
|
||||
});
|
||||
using var provider = CreateProvider(fake);
|
||||
|
||||
var settings = Task.Run(() => provider.Settings);
|
||||
Assert.IsTrue(requestStarted.Wait(Timeout));
|
||||
provider.Dispose();
|
||||
releaseRequest.Set();
|
||||
|
||||
Assert.IsNull(await settings.WaitAsync(Timeout));
|
||||
}
|
||||
|
||||
// LoadMore folds the loaded page into pagination state and raises
|
||||
// ItemsChanged so the host asks GetItems again and sees the appended items.
|
||||
// It stops once the extension reports the final page.
|
||||
@@ -216,4 +417,43 @@ public partial class JSAdapterTests
|
||||
Assert.IsTrue(host.HiddenCount >= 1);
|
||||
Assert.IsTrue(host.HiddenCount <= host.ShownCount);
|
||||
}
|
||||
|
||||
[MethodImpl(MethodImplOptions.NoInlining)]
|
||||
private static WeakReference<RecordingPropertyChangeTarget> RegisterTemporaryTarget(
|
||||
JsonRpcConnection connection,
|
||||
string commandId)
|
||||
{
|
||||
var target = new RecordingPropertyChangeTarget();
|
||||
JSPropertyChangeRegistry.Register(connection, commandId, target);
|
||||
return new WeakReference<RecordingPropertyChangeTarget>(target);
|
||||
}
|
||||
|
||||
private static JsonObject Command(string id) => new()
|
||||
{
|
||||
["id"] = id,
|
||||
["name"] = id,
|
||||
};
|
||||
|
||||
private static JsonObject CommandItem(string id) => new()
|
||||
{
|
||||
["id"] = id,
|
||||
["title"] = id,
|
||||
["command"] = Command(id),
|
||||
};
|
||||
|
||||
private static JsonObject ContextItem(string id) => new()
|
||||
{
|
||||
["title"] = id,
|
||||
["command"] = Command(id),
|
||||
};
|
||||
|
||||
private sealed class RecordingPropertyChangeTarget : IJSPropertyChangeTarget
|
||||
{
|
||||
public int ApplyCount { get; private set; }
|
||||
|
||||
public void ApplyPropertyChanges(JsonElement properties)
|
||||
{
|
||||
ApplyCount++;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user