Skip to content

Commit 2311cbb

Browse files
IHandlersManager and Handler linking improvments (#334)
* Added extension method to get all the unique handlers contained in the IHandlersManager * renamed link parameter names to better reflect their meaning. Also added exceptions to help users diagnose issues * Added unit tests * Added dap tests
1 parent 085ef9c commit 2311cbb

18 files changed

+209
-29
lines changed

src/Client/LanguageClientOptions.cs

+2-2
Original file line numberDiff line numberDiff line change
@@ -64,8 +64,8 @@ ILanguageClientRegistry IJsonRpcHandlerRegistry<ILanguageClientRegistry>.AddHand
6464
ILanguageClientRegistry IJsonRpcHandlerRegistry<ILanguageClientRegistry>.AddHandler(string method, Type type, JsonRpcHandlerOptions options) =>
6565
AddHandler(method, type, options);
6666

67-
ILanguageClientRegistry IJsonRpcHandlerRegistry<ILanguageClientRegistry>.AddHandlerLink(string sourceMethod, string destinationMethod) =>
68-
AddHandlerLink(sourceMethod, destinationMethod);
67+
ILanguageClientRegistry IJsonRpcHandlerRegistry<ILanguageClientRegistry>.AddHandlerLink(string fromMethod, string toMethod) =>
68+
AddHandlerLink(fromMethod, toMethod);
6969

7070
ILanguageClientRegistry IJsonRpcHandlerRegistry<ILanguageClientRegistry>.OnJsonRequest(string method, Func<JToken, Task<JToken>> handler, JsonRpcHandlerOptions options) =>
7171
OnJsonRequest(method, handler, options);

src/Dap.Client/DebugAdapterClientOptions.cs

+2-2
Original file line numberDiff line numberDiff line change
@@ -54,8 +54,8 @@ IDebugAdapterClientRegistry IJsonRpcHandlerRegistry<IDebugAdapterClientRegistry>
5454
IDebugAdapterClientRegistry IJsonRpcHandlerRegistry<IDebugAdapterClientRegistry>.AddHandler(string method, Type type, JsonRpcHandlerOptions options) =>
5555
AddHandler(method, type, options);
5656

57-
IDebugAdapterClientRegistry IJsonRpcHandlerRegistry<IDebugAdapterClientRegistry>.AddHandlerLink(string sourceMethod, string destinationMethod) =>
58-
AddHandlerLink(sourceMethod, destinationMethod);
57+
IDebugAdapterClientRegistry IJsonRpcHandlerRegistry<IDebugAdapterClientRegistry>.AddHandlerLink(string fromMethod, string toMethod) =>
58+
AddHandlerLink(fromMethod, toMethod);
5959

6060
IDebugAdapterClientRegistry IJsonRpcHandlerRegistry<IDebugAdapterClientRegistry>.OnJsonRequest(
6161
string method, Func<JToken, Task<JToken>> handler, JsonRpcHandlerOptions options

src/Dap.Server/DebugAdapterServerOptions.cs

+2-2
Original file line numberDiff line numberDiff line change
@@ -42,8 +42,8 @@ IDebugAdapterServerRegistry IJsonRpcHandlerRegistry<IDebugAdapterServerRegistry>
4242
IDebugAdapterServerRegistry IJsonRpcHandlerRegistry<IDebugAdapterServerRegistry>.AddHandler(string method, Type type, JsonRpcHandlerOptions options) =>
4343
AddHandler(method, type, options);
4444

45-
IDebugAdapterServerRegistry IJsonRpcHandlerRegistry<IDebugAdapterServerRegistry>.AddHandlerLink(string sourceMethod, string destinationMethod) =>
46-
AddHandlerLink(sourceMethod, destinationMethod);
45+
IDebugAdapterServerRegistry IJsonRpcHandlerRegistry<IDebugAdapterServerRegistry>.AddHandlerLink(string fromMethod, string toMethod) =>
46+
AddHandlerLink(fromMethod, toMethod);
4747

4848
IDebugAdapterServerRegistry IJsonRpcHandlerRegistry<IDebugAdapterServerRegistry>.OnJsonRequest(
4949
string method, Func<JToken, Task<JToken>> handler, JsonRpcHandlerOptions options

src/Dap.Shared/DebugAdapterHandlerCollection.cs

+17-3
Original file line numberDiff line numberDiff line change
@@ -43,12 +43,26 @@ public IDisposable Add(string method, Type handlerType, JsonRpcHandlerOptions op
4343
method, ActivatorUtilities.CreateInstance(_serviceProvider, handlerType) as IJsonRpcHandler, options
4444
);
4545

46-
IDisposable IHandlersManager.AddLink(string sourceMethod, string destinationMethod)
46+
IDisposable IHandlersManager.AddLink(string fromMethod, string toMethod)
4747
{
48-
var source = _descriptors.First(z => z.Method == sourceMethod);
48+
var source = _descriptors.FirstOrDefault(z => z.Method == fromMethod);
49+
if (source == null)
50+
{
51+
if (_descriptors.Any(z => z.Method == toMethod))
52+
{
53+
throw new ArgumentException(
54+
$"Could not find descriptor for '{fromMethod}', but I did find one for '{toMethod}'. Did you mean to link '{toMethod}' to '{fromMethod}' instead?", fromMethod
55+
);
56+
}
57+
58+
throw new ArgumentException(
59+
$"Could not find descriptor for '{fromMethod}', has it been registered yet? Descriptors must be registered before links can be created!", nameof(fromMethod)
60+
);
61+
}
62+
4963
HandlerDescriptor descriptor = null;
5064
descriptor = GetDescriptor(
51-
destinationMethod,
65+
toMethod,
5266
source.HandlerType,
5367
source.Handler,
5468
source.RequestProcessType.HasValue ? new JsonRpcHandlerOptions { RequestProcessType = source.RequestProcessType.Value } : null,

src/JsonRpc/CompositeHandlersManager.cs

+2-2
Original file line numberDiff line numberDiff line change
@@ -55,9 +55,9 @@ public IDisposable Add(string method, Type handlerType, JsonRpcHandlerOptions op
5555
return result;
5656
}
5757

58-
public IDisposable AddLink(string sourceMethod, string destinationMethod)
58+
public IDisposable AddLink(string fromMethod, string toMethod)
5959
{
60-
var result = _parent.AddLink(sourceMethod,destinationMethod);
60+
var result = _parent.AddLink(fromMethod,toMethod);
6161
_compositeDisposable.Add(result);
6262
return result;
6363
}

src/JsonRpc/HandlerCollection.cs

+17-3
Original file line numberDiff line numberDiff line change
@@ -91,10 +91,24 @@ public IDisposable Add(string method, Type handlerType, JsonRpcHandlerOptions op
9191
method, ActivatorUtilities.CreateInstance(_serviceProvider, handlerType) as IJsonRpcHandler, options
9292
);
9393

94-
public IDisposable AddLink(string sourceMethod, string destinationMethod)
94+
public IDisposable AddLink(string fromMethod, string toMethod)
9595
{
96-
var source = _descriptors.FirstOrDefault(z => z.Method == sourceMethod);
97-
var descriptor = new LinkedHandler(destinationMethod, source, () => _descriptors.RemoveAll(z => z.Method == destinationMethod));
96+
var source = _descriptors.FirstOrDefault(z => z.Method == fromMethod);
97+
if (source == null)
98+
{
99+
if (_descriptors.Any(z => z.Method == toMethod))
100+
{
101+
throw new ArgumentException(
102+
$"Could not find descriptor for '{fromMethod}', but I did find one for '{toMethod}'. Did you mean to link '{toMethod}' to '{fromMethod}' instead?", fromMethod
103+
);
104+
}
105+
106+
throw new ArgumentException(
107+
$"Could not find descriptor for '{fromMethod}', has it been registered yet? Descriptors must be registered before links can be created!", nameof(fromMethod)
108+
);
109+
}
110+
111+
var descriptor = new LinkedHandler(toMethod, source, () => _descriptors.RemoveAll(z => z.Method == toMethod));
98112
ImmutableInterlocked.InterlockedExchange(ref _descriptors, _descriptors.Add(descriptor));
99113
return descriptor;
100114
}
+18
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
using System.Collections.Generic;
2+
using System.Linq;
3+
4+
namespace OmniSharp.Extensions.JsonRpc
5+
{
6+
public static class HandlersManagerExtensions
7+
{
8+
/// <summary>
9+
/// Gets all the unique handlers currently registered with the manager
10+
/// </summary>
11+
/// <param name="handlersManager"></param>
12+
/// <returns></returns>
13+
public static IEnumerable<IJsonRpcHandler> GetHandlers(this IHandlersManager handlersManager)
14+
{
15+
return handlersManager.Descriptors.Select(z => z.Handler).Distinct();
16+
}
17+
}
18+
}

src/JsonRpc/IHandlersManager.cs

+1-1
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ public interface IHandlersManager
1111
IDisposable Add(string method, JsonRpcHandlerFactory factory, JsonRpcHandlerOptions options);
1212
IDisposable Add(Type handlerType, JsonRpcHandlerOptions options);
1313
IDisposable Add(string method, Type handlerType, JsonRpcHandlerOptions options);
14-
IDisposable AddLink(string sourceMethod, string destinationMethod);
14+
IDisposable AddLink(string fromMethod, string toMethod);
1515
IEnumerable<IHandlerDescriptor> Descriptors { get; }
1616
}
1717
}

src/JsonRpc/IJsonRpcHandlerRegistry.cs

+1-1
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ public interface IJsonRpcHandlerRegistry<out T> : IJsonRpcHandlerRegistry where
2525
T AddHandler<TTHandler>(string method, JsonRpcHandlerOptions options = null) where TTHandler : IJsonRpcHandler;
2626
T AddHandler(Type type, JsonRpcHandlerOptions options = null);
2727
T AddHandler(string method, Type type, JsonRpcHandlerOptions options = null);
28-
T AddHandlerLink(string sourceMethod, string destinationMethod);
28+
T AddHandlerLink(string fromMethod, string toMethod);
2929

3030
T OnJsonRequest(string method, Func<JToken, Task<JToken>> handler, JsonRpcHandlerOptions options = null);
3131
T OnJsonRequest(string method, Func<JToken, CancellationToken, Task<JToken>> handler, JsonRpcHandlerOptions options = null);

src/JsonRpc/InterimJsonRpcServerRegistry.cs

+2-2
Original file line numberDiff line numberDiff line change
@@ -58,9 +58,9 @@ public sealed override T AddHandler(string method, Type type, JsonRpcHandlerOpti
5858
return (T) (object) this;
5959
}
6060

61-
public sealed override T AddHandlerLink(string sourceMethod, string destinationMethod)
61+
public sealed override T AddHandlerLink(string fromMethod, string toMethod)
6262
{
63-
_handlersManager.AddLink(sourceMethod, destinationMethod);
63+
_handlersManager.AddLink(fromMethod, toMethod);
6464
return (T) (object) this;
6565
}
6666
}

src/JsonRpc/JsonRpcCommonMethodsBase.cs

+1-1
Original file line numberDiff line numberDiff line change
@@ -112,7 +112,7 @@ public T OnNotification(string method, Func<CancellationToken, Task> handler, Js
112112
public abstract T AddHandler<THandler>(string method, JsonRpcHandlerOptions options = null) where THandler : IJsonRpcHandler;
113113
public abstract T AddHandler(Type type, JsonRpcHandlerOptions options = null);
114114
public abstract T AddHandler(string method, Type type, JsonRpcHandlerOptions options = null);
115-
public abstract T AddHandlerLink(string sourceMethod, string destinationMethod);
115+
public abstract T AddHandlerLink(string fromMethod, string toMethod);
116116

117117
#endregion
118118
}

src/JsonRpc/JsonRpcOptionsRegistryBase.cs

+2-2
Original file line numberDiff line numberDiff line change
@@ -69,9 +69,9 @@ public sealed override T AddHandler(string method, Type type, JsonRpcHandlerOpti
6969
return (T) (object) this;
7070
}
7171

72-
public sealed override T AddHandlerLink(string sourceMethod, string destinationMethod)
72+
public sealed override T AddHandlerLink(string fromMethod, string toMethod)
7373
{
74-
Handlers.Add(JsonRpcHandlerDescription.Link(sourceMethod, destinationMethod));
74+
Handlers.Add(JsonRpcHandlerDescription.Link(fromMethod, toMethod));
7575
return (T) (object) this;
7676
}
7777

src/JsonRpc/JsonRpcServerOptionsBase.cs

+2-2
Original file line numberDiff line numberDiff line change
@@ -129,9 +129,9 @@ public T WithMaximumRequestTimeout(TimeSpan maximumRequestTimeout)
129129
return (T) (object) this;
130130
}
131131

132-
public T WithLink(string source, string destination)
132+
public T WithLink(string fromMethod, string toMethod)
133133
{
134-
Handlers.Add(JsonRpcHandlerDescription.Link(source, destination));
134+
Handlers.Add(JsonRpcHandlerDescription.Link(fromMethod, toMethod));
135135
return (T) (object) this;
136136
}
137137
}

src/Server/LanguageServerOptions.cs

+2-2
Original file line numberDiff line numberDiff line change
@@ -41,8 +41,8 @@ ILanguageServerRegistry IJsonRpcHandlerRegistry<ILanguageServerRegistry>.AddHand
4141
ILanguageServerRegistry IJsonRpcHandlerRegistry<ILanguageServerRegistry>.AddHandler(string method, Type type, JsonRpcHandlerOptions options) =>
4242
AddHandler(method, type, options);
4343

44-
ILanguageServerRegistry IJsonRpcHandlerRegistry<ILanguageServerRegistry>.AddHandlerLink(string sourceMethod, string destinationMethod) =>
45-
AddHandlerLink(sourceMethod, destinationMethod);
44+
ILanguageServerRegistry IJsonRpcHandlerRegistry<ILanguageServerRegistry>.AddHandlerLink(string fromMethod, string toMethod) =>
45+
AddHandlerLink(fromMethod, toMethod);
4646

4747
ILanguageServerRegistry IJsonRpcHandlerRegistry<ILanguageServerRegistry>.OnJsonRequest(string method, Func<JToken, Task<JToken>> handler, JsonRpcHandlerOptions options) =>
4848
OnJsonRequest(method, handler, options);

src/Shared/SharedHandlerCollection.cs

+17-3
Original file line numberDiff line numberDiff line change
@@ -60,12 +60,26 @@ IDisposable IHandlersManager.Add(Type handlerType, JsonRpcHandlerOptions options
6060
IDisposable IHandlersManager.Add(string method, Type handlerType, JsonRpcHandlerOptions options) =>
6161
Add(method, ActivatorUtilities.CreateInstance(_serviceProvider, handlerType) as IJsonRpcHandler, options);
6262

63-
IDisposable IHandlersManager.AddLink(string sourceMethod, string destinationMethod)
63+
IDisposable IHandlersManager.AddLink(string fromMethod, string toMethod)
6464
{
65-
var source = _descriptors.First(z => z.Method == sourceMethod);
65+
var source = _descriptors.FirstOrDefault(z => z.Method == fromMethod);
66+
if (source == null)
67+
{
68+
if (_descriptors.Any(z => z.Method == toMethod))
69+
{
70+
throw new ArgumentException(
71+
$"Could not find descriptor for '{fromMethod}', but I did find one for '{toMethod}'. Did you mean to link '{toMethod}' to '{fromMethod}' instead?", fromMethod
72+
);
73+
}
74+
75+
throw new ArgumentException(
76+
$"Could not find descriptor for '{fromMethod}', has it been registered yet? Descriptors must be registered before links can be created!", nameof(fromMethod)
77+
);
78+
}
79+
6680
LspHandlerDescriptor descriptor = null;
6781
descriptor = GetDescriptor(
68-
destinationMethod,
82+
toMethod,
6983
source.HandlerType,
7084
source.Handler,
7185
source.RequestProcessType.HasValue ? new JsonRpcHandlerOptions { RequestProcessType = source.RequestProcessType.Value } : null,
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,54 @@
1+
using System;
2+
using System.Threading.Tasks;
3+
using FluentAssertions;
4+
using Microsoft.Extensions.DependencyInjection;
5+
using NSubstitute;
6+
using OmniSharp.Extensions.DebugAdapter.Protocol.Requests;
7+
using OmniSharp.Extensions.DebugAdapter.Testing;
8+
using OmniSharp.Extensions.JsonRpc;
9+
using OmniSharp.Extensions.JsonRpc.Testing;
10+
using Xunit;
11+
using Xunit.Abstractions;
12+
13+
namespace Dap.Tests.Integration
14+
{
15+
public class HandlersManagerIntegrationTests : DebugAdapterProtocolTestBase
16+
{
17+
public HandlersManagerIntegrationTests(ITestOutputHelper testOutputHelper) : base(new JsonRpcTestOptions().ConfigureForXUnit(testOutputHelper))
18+
{
19+
}
20+
21+
[Fact]
22+
public async Task Should_Return_Default_Handlers()
23+
{
24+
var (client, server) = await Initialize(options => {}, options => {});
25+
26+
var handlersManager = server.GetRequiredService<IHandlersManager>();
27+
handlersManager.Descriptors.Should().HaveCount(2);
28+
handlersManager.GetHandlers().Should().HaveCount(2);
29+
}
30+
31+
[Fact]
32+
public async Task Link_Should_Fail_If_No_Handler_Is_Defined()
33+
{
34+
var (client, server) = await Initialize(options => {}, options => {});
35+
36+
var handlersManager = server.GetRequiredService<IHandlersManager>();
37+
38+
Action a = () => handlersManager.AddLink(RequestNames.Completions, "my/completions");
39+
a.Should().Throw<ArgumentException>().Which.Message.Should().Contain("Descriptors must be registered before links can be created");
40+
}
41+
42+
[Fact]
43+
public async Task Link_Should_Fail_If_Link_Is_On_The_Wrong_Side()
44+
{
45+
var (client, server) = await Initialize(options => {}, options => {});
46+
47+
var handlersManager = server.GetRequiredService<IHandlersManager>();
48+
handlersManager.Add(Substitute.For(new Type[] { typeof(ICompletionsHandler) }, Array.Empty<object>()) as IJsonRpcHandler, new JsonRpcHandlerOptions());
49+
50+
Action a = () => handlersManager.AddLink("my/completions", RequestNames.Completions);
51+
a.Should().Throw<ArgumentException>().Which.Message.Should().Contain($"Did you mean to link '{RequestNames.Completions}' to 'my/completions' instead");
52+
}
53+
}
54+
}

test/JsonRpc.Tests/TestLanguageServerRegistry.cs

+1-1
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ public override IJsonRpcServerRegistry AddHandler(string method, IJsonRpcHandler
2828
public override IJsonRpcServerRegistry AddHandler(Type type, JsonRpcHandlerOptions options = null) => throw new NotImplementedException();
2929

3030
public override IJsonRpcServerRegistry AddHandler(string method, Type type, JsonRpcHandlerOptions options = null) => throw new NotImplementedException();
31-
public override IJsonRpcServerRegistry AddHandlerLink(string sourceMethod, string destinationMethod) => throw new NotImplementedException();
31+
public override IJsonRpcServerRegistry AddHandlerLink(string fromMethod, string toMethod) => throw new NotImplementedException();
3232

3333
public override IJsonRpcServerRegistry AddHandler(string method, JsonRpcHandlerFactory handlerFunc, JsonRpcHandlerOptions options = null)
3434
{
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,66 @@
1+
using System;
2+
using System.Threading.Tasks;
3+
using FluentAssertions;
4+
using Microsoft.Extensions.DependencyInjection;
5+
using NSubstitute;
6+
using OmniSharp.Extensions.JsonRpc;
7+
using OmniSharp.Extensions.JsonRpc.Testing;
8+
using OmniSharp.Extensions.LanguageProtocol.Testing;
9+
using OmniSharp.Extensions.LanguageServer.Protocol;
10+
using OmniSharp.Extensions.LanguageServer.Protocol.Document;
11+
using Xunit;
12+
using Xunit.Abstractions;
13+
14+
namespace Lsp.Tests.Integration
15+
{
16+
public class HandlersManagerIntegrationTests : LanguageProtocolTestBase
17+
{
18+
public HandlersManagerIntegrationTests(ITestOutputHelper testOutputHelper) : base(new JsonRpcTestOptions().ConfigureForXUnit(testOutputHelper))
19+
{
20+
}
21+
22+
[Fact]
23+
public async Task Should_Return_Default_Handlers()
24+
{
25+
var (client, server) = await Initialize(options => {}, options => {});
26+
27+
var handlersManager = server.GetRequiredService<IHandlersManager>();
28+
handlersManager.Descriptors.Should().HaveCount(8);
29+
handlersManager.GetHandlers().Should().HaveCount(5);
30+
}
31+
32+
[Fact]
33+
public async Task Should_Return_Additional_Handlers()
34+
{
35+
var (client, server) = await Initialize(options => {}, options => {});
36+
37+
server.Register(o => o.AddHandler(Substitute.For(new Type[] { typeof (ICompletionHandler), typeof(ICompletionResolveHandler) }, Array.Empty<object>()) as IJsonRpcHandler));
38+
var handlersManager = server.GetRequiredService<IHandlersManager>();
39+
handlersManager.Descriptors.Should().HaveCount(10);
40+
handlersManager.GetHandlers().Should().HaveCount(6);
41+
}
42+
43+
[Fact]
44+
public async Task Link_Should_Fail_If_No_Handler_Is_Defined()
45+
{
46+
var (client, server) = await Initialize(options => {}, options => {});
47+
48+
var handlersManager = server.GetRequiredService<IHandlersManager>();
49+
50+
Action a = () => handlersManager.AddLink(TextDocumentNames.Completion, "my/completion");
51+
a.Should().Throw<ArgumentException>().Which.Message.Should().Contain("Descriptors must be registered before links can be created");
52+
}
53+
54+
[Fact]
55+
public async Task Link_Should_Fail_If_Link_Is_On_The_Wrong_Side()
56+
{
57+
var (client, server) = await Initialize(options => {}, options => {});
58+
59+
server.Register(o => o.AddHandler(Substitute.For(new Type[] { typeof (ICompletionHandler), typeof(ICompletionResolveHandler) }, Array.Empty<object>()) as IJsonRpcHandler));
60+
var handlersManager = server.GetRequiredService<IHandlersManager>();
61+
62+
Action a = () => handlersManager.AddLink("my/completion", TextDocumentNames.Completion);
63+
a.Should().Throw<ArgumentException>().Which.Message.Should().Contain($"Did you mean to link '{TextDocumentNames.Completion}' to 'my/completion' instead");
64+
}
65+
}
66+
}

0 commit comments

Comments
 (0)