Skip to content

Commit

Permalink
Merge pull request #68 from OmniSharp/fix/routing
Browse files Browse the repository at this point in the history 
Updated to fix an issue where routing wouldn't work correctly
  • Loading branch information
@david-driscoll
david-driscoll authored Feb 2, 2018
2 parents 6960413 + f0b65f1 commit 023ac40
Show file tree
Hide file tree
Showing 6 changed files with 160 additions and 41 deletions.
4 changes: 4 additions & 0 deletions src/Directory.Build.props
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,4 +7,8 @@
<InformationalVersion Condition="'$(GitVersion_InformationalVersion)' != ''">$(GitVersion_InformationalVersion)</InformationalVersion>
<LangVersion>Latest</LangVersion>
</PropertyGroup>
<ItemGroup>
<PackageReference Include="SourceLink.Create.GitHub" Version="2.7.6" PrivateAssets="All" />
<DotNetCliToolReference Include="dotnet-sourcelink-git" Version="2.7.6" />
</ItemGroup>
</Project>
19 changes: 16 additions & 3 deletions src/Server/HandlerCollection.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,12 @@ namespace OmniSharp.Extensions.LanguageServer.Server
class HandlerCollection : IHandlerCollection
{
internal readonly HashSet<HandlerDescriptor> _handlers = new HashSet<HandlerDescriptor>();
internal readonly HashSet<ITextDocumentSyncHandler> _documentSyncHandlers = new HashSet<ITextDocumentSyncHandler>();

public IEnumerable<ITextDocumentSyncHandler> TextDocumentSyncHandlers()
{
return _documentSyncHandlers;
}

public IEnumerator<ILspHandlerDescriptor> GetEnumerator()
{
Expand Down Expand Up @@ -50,9 +56,13 @@ public IDisposable Add(params IJsonRpcHandler[] handlers)
}
}

foreach (var handler in descriptors)
foreach (var descriptor in descriptors)
{
_handlers.Add(handler);
_handlers.Add(descriptor);
if (descriptor.Handler is ITextDocumentSyncHandler documentSyncHandler)
{
_documentSyncHandlers.Add(documentSyncHandler);
}
}

return new ImmutableDisposable(descriptors);
Expand Down Expand Up @@ -87,7 +97,10 @@ private HandlerDescriptor GetDescriptor(string method, Type implementedType, IJs
@params,
registration,
capability,
() => _handlers.RemoveWhere(instance => instance.Handler == handler));
() => {
_handlers.RemoveWhere(instance => instance.Handler == handler);
_documentSyncHandlers.RemoveWhere(instance => instance == handler);
});
}

private Type UnwrapGenericType(Type genericType, Type type)
Expand Down
2 changes: 1 addition & 1 deletion src/Server/LanguageServer.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ internal LanguageServer(
_serializer = serializer;
_handlerMactherCollection = new HandlerMatcherCollection
{
new TextDocumentMatcher(_loggerFactory.CreateLogger<TextDocumentMatcher>()),
new TextDocumentMatcher(_loggerFactory.CreateLogger<TextDocumentMatcher>(), _collection.TextDocumentSyncHandlers),
new ExecuteCommandMatcher(_loggerFactory.CreateLogger<ExecuteCommandMatcher>())
};

Expand Down
18 changes: 8 additions & 10 deletions src/Server/Matchers/TextDocumentMatcher.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,12 @@ namespace OmniSharp.Extensions.LanguageServer.Server.Matchers
public class TextDocumentMatcher : IHandlerMatcher
{
private readonly ILogger _logger;
private readonly Func<IEnumerable<ITextDocumentSyncHandler>> _getSyncHandlers;

public TextDocumentMatcher(ILogger logger)
public TextDocumentMatcher(ILogger logger, Func<IEnumerable<ITextDocumentSyncHandler>> getSyncHandlers)
{
_logger = logger;
_getSyncHandlers = getSyncHandlers;
}

public IEnumerable<ILspHandlerDescriptor> FindHandler(object parameters, IEnumerable<ILspHandlerDescriptor> descriptors)
Expand Down Expand Up @@ -53,11 +55,7 @@ public IEnumerable<ILspHandlerDescriptor> FindHandler(object parameters, IEnumer

private List<TextDocumentAttributes> GetTextDocumentAttributes(IEnumerable<ILspHandlerDescriptor> method, Uri uri)
{
var textDocumentSyncHandlers = method
.Select(x => x.Handler is ITextDocumentSyncHandler r ? r : null)
.Where(x => x != null)
.Distinct();
return textDocumentSyncHandlers
return _getSyncHandlers()
.Select(x => x.GetTextDocumentAttributes(uri))
.Where(x => x != null)
.Distinct()
Expand All @@ -67,7 +65,7 @@ private List<TextDocumentAttributes> GetTextDocumentAttributes(IEnumerable<ILspH
private IEnumerable<ILspHandlerDescriptor> GetHandler(IEnumerable<ILspHandlerDescriptor> method, IEnumerable<TextDocumentAttributes> attributes)
{
return attributes
.SelectMany(x => GetHandler(method, x));
.SelectMany(x => GetHandler(method, x));
}

private IEnumerable<ILspHandlerDescriptor> GetHandler(IEnumerable<ILspHandlerDescriptor> method, TextDocumentAttributes attributes)
Expand All @@ -78,9 +76,9 @@ private IEnumerable<ILspHandlerDescriptor> GetHandler(IEnumerable<ILspHandlerDes
_logger.LogTrace("Checking handler {Method}:{Handler}", method, handler.Handler.GetType().FullName);
var registrationOptions = handler.Registration.RegisterOptions as TextDocumentRegistrationOptions;

_logger.LogTrace("Registration options {OptionsName}", registrationOptions.GetType().FullName);
_logger.LogTrace("Document Selector {DocumentSelector}", registrationOptions.DocumentSelector.ToString());
if (registrationOptions.DocumentSelector == null || registrationOptions.DocumentSelector.IsMatch(attributes))
_logger.LogTrace("Registration options {OptionsName}", registrationOptions?.GetType().FullName);
_logger.LogTrace("Document Selector {DocumentSelector}", registrationOptions?.DocumentSelector.ToString());
if (registrationOptions?.DocumentSelector == null || registrationOptions.DocumentSelector.IsMatch(attributes))
{
_logger.LogTrace("Handler Selected: {Handler} via {DocumentSelector} (targeting {HandlerInterface})", handler.Handler.GetType().FullName, registrationOptions.DocumentSelector.ToString(), handler.HandlerType.FullName);
yield return handler;
Expand Down
93 changes: 74 additions & 19 deletions test/Lsp.Tests/LspRequestRouterTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
using OmniSharp.Extensions.LanguageServer.Server;
using OmniSharp.Extensions.LanguageServer.Server.Abstractions;
using OmniSharp.Extensions.LanguageServer.Server.Handlers;
using OmniSharp.Extensions.LanguageServer.Server.Matchers;
using Xunit;
using Xunit.Abstractions;
using Xunit.Sdk;
Expand All @@ -27,17 +28,11 @@ namespace Lsp.Tests
public class LspRequestRouterTests
{
private readonly TestLoggerFactory _testLoggerFactory;
private readonly IHandlerMatcherCollection _handlerMatcherCollection = new HandlerMatcherCollection();
//private readonly IHandlerMatcherCollection handlerMatcherCollection = new HandlerMatcherCollection();

public LspRequestRouterTests(ITestOutputHelper testOutputHelper)
{
_testLoggerFactory = new TestLoggerFactory(testOutputHelper);
var logger = Substitute.For<ILogger>();
var matcher = Substitute.For<IHandlerMatcher>();
matcher.FindHandler(Arg.Any<object>(), Arg.Any<IEnumerable<ILspHandlerDescriptor>>())
.Returns(new List<HandlerDescriptor>());

_handlerMatcherCollection.Add(matcher);
}

[Fact]
Expand All @@ -47,7 +42,10 @@ public async Task ShouldRouteToCorrect_Notification()
textDocumentSyncHandler.Handle(Arg.Any<DidSaveTextDocumentParams>()).Returns(Task.CompletedTask);

var collection = new HandlerCollection { textDocumentSyncHandler };
var mediator = new LspRequestRouter(collection, _testLoggerFactory, _handlerMatcherCollection, new Serializer());
var handlerMatcherCollection = new HandlerMatcherCollection {
new TextDocumentMatcher(_testLoggerFactory.CreateLogger<TextDocumentMatcher>(), collection.TextDocumentSyncHandlers)
};
var mediator = new LspRequestRouter(collection, _testLoggerFactory, handlerMatcherCollection, new Serializer());

var @params = new DidSaveTextDocumentParams() {
TextDocument = new TextDocumentIdentifier(new Uri("file:///c:/test/123.cs"))
Expand All @@ -69,7 +67,10 @@ public async Task ShouldRouteToCorrect_Notification_WithManyHandlers()
textDocumentSyncHandler2.Handle(Arg.Any<DidSaveTextDocumentParams>()).Returns(Task.CompletedTask);

var collection = new HandlerCollection { textDocumentSyncHandler, textDocumentSyncHandler2 };
var mediator = new LspRequestRouter(collection, _testLoggerFactory, _handlerMatcherCollection, new Serializer());
var handlerMatcherCollection = new HandlerMatcherCollection {
new TextDocumentMatcher(_testLoggerFactory.CreateLogger<TextDocumentMatcher>(), collection.TextDocumentSyncHandlers)
};
var mediator = new LspRequestRouter(collection, _testLoggerFactory, handlerMatcherCollection, new Serializer());

var @params = new DidSaveTextDocumentParams() {
TextDocument = new TextDocumentIdentifier(new Uri("file:///c:/test/123.cake"))
Expand All @@ -79,8 +80,8 @@ public async Task ShouldRouteToCorrect_Notification_WithManyHandlers()

await mediator.RouteNotification(mediator.GetDescriptor(request), request);

await textDocumentSyncHandler.Received(1).Handle(Arg.Any<DidSaveTextDocumentParams>());
await textDocumentSyncHandler2.Received(0).Handle(Arg.Any<DidSaveTextDocumentParams>());
await textDocumentSyncHandler.Received(0).Handle(Arg.Any<DidSaveTextDocumentParams>());
await textDocumentSyncHandler2.Received(1).Handle(Arg.Any<DidSaveTextDocumentParams>());
}

[Fact]
Expand All @@ -96,7 +97,10 @@ public async Task ShouldRouteToCorrect_Request()
.Returns(new CommandContainer());

var collection = new HandlerCollection { textDocumentSyncHandler, codeActionHandler };
var mediator = new LspRequestRouter(collection, _testLoggerFactory, _handlerMatcherCollection, new Serializer());
var handlerMatcherCollection = new HandlerMatcherCollection {
new TextDocumentMatcher(_testLoggerFactory.CreateLogger<TextDocumentMatcher>(), collection.TextDocumentSyncHandlers)
};
var mediator = new LspRequestRouter(collection, _testLoggerFactory, handlerMatcherCollection, new Serializer());

var id = Guid.NewGuid().ToString();
var @params = new DidSaveTextDocumentParams() {
Expand Down Expand Up @@ -131,19 +135,61 @@ public async Task ShouldRouteToCorrect_Request_WithManyHandlers()
.Returns(new CommandContainer());

var collection = new HandlerCollection { textDocumentSyncHandler, textDocumentSyncHandler2, codeActionHandler, codeActionHandler2 };
var mediator = new LspRequestRouter(collection, _testLoggerFactory, _handlerMatcherCollection, new Serializer());
var handlerMatcherCollection = new HandlerMatcherCollection {
new TextDocumentMatcher(_testLoggerFactory.CreateLogger<TextDocumentMatcher>(), collection.TextDocumentSyncHandlers)
};
var mediator = new LspRequestRouter(collection, _testLoggerFactory, handlerMatcherCollection, new Serializer());

var id = Guid.NewGuid().ToString();
var @params = new DidSaveTextDocumentParams() {
var @params = new CodeActionParams() {
TextDocument = new TextDocumentIdentifier(new Uri("file:///c:/test/123.cake"))
};

var request = new Request(id, DocumentNames.CodeAction, JObject.Parse(JsonConvert.SerializeObject(@params, new Serializer(ClientVersion.Lsp3).Settings)));

await mediator.RouteRequest(mediator.GetDescriptor(request), request);

await codeActionHandler.Received(1).Handle(Arg.Any<CodeActionParams>(), Arg.Any<CancellationToken>());
await codeActionHandler2.Received(0).Handle(Arg.Any<CodeActionParams>(), Arg.Any<CancellationToken>());
await codeActionHandler.Received(0).Handle(Arg.Any<CodeActionParams>(), Arg.Any<CancellationToken>());
await codeActionHandler2.Received(1).Handle(Arg.Any<CodeActionParams>(), Arg.Any<CancellationToken>());
}

[Fact]
public async Task ShouldRouteToCorrect_Request_WithManyHandlers_CodeLensHandler()
{
var textDocumentSyncHandler = TextDocumentSyncHandlerExtensions.With(DocumentSelector.ForPattern("**/*.cs"));
var textDocumentSyncHandler2 = TextDocumentSyncHandlerExtensions.With(DocumentSelector.ForPattern("**/*.cake"));
textDocumentSyncHandler.Handle(Arg.Any<DidSaveTextDocumentParams>()).Returns(Task.CompletedTask);
textDocumentSyncHandler2.Handle(Arg.Any<DidSaveTextDocumentParams>()).Returns(Task.CompletedTask);

var codeActionHandler = Substitute.For<ICodeLensHandler>();
codeActionHandler.GetRegistrationOptions().Returns(new CodeLensRegistrationOptions() { DocumentSelector = DocumentSelector.ForPattern("**/*.cs") });
codeActionHandler
.Handle(Arg.Any<CodeLensParams>(), Arg.Any<CancellationToken>())
.Returns(new CodeLensContainer());

var codeActionHandler2 = Substitute.For<ICodeLensHandler>();
codeActionHandler2.GetRegistrationOptions().Returns(new CodeLensRegistrationOptions() { DocumentSelector = DocumentSelector.ForPattern("**/*.cake") });
codeActionHandler2
.Handle(Arg.Any<CodeLensParams>(), Arg.Any<CancellationToken>())
.Returns(new CodeLensContainer());

var collection = new HandlerCollection { textDocumentSyncHandler, textDocumentSyncHandler2, codeActionHandler, codeActionHandler2 };
var handlerMatcherCollection = new HandlerMatcherCollection {
new TextDocumentMatcher(_testLoggerFactory.CreateLogger<TextDocumentMatcher>(), collection.TextDocumentSyncHandlers)
};
var mediator = new LspRequestRouter(collection, _testLoggerFactory, handlerMatcherCollection, new Serializer());

var id = Guid.NewGuid().ToString();
var @params = new CodeLensParams() {
TextDocument = new TextDocumentIdentifier(new Uri("file:///c:/test/123.cs"))
};

var request = new Request(id, DocumentNames.CodeLens, JObject.Parse(JsonConvert.SerializeObject(@params, new Serializer(ClientVersion.Lsp3).Settings)));

await mediator.RouteRequest(mediator.GetDescriptor(request), request);

await codeActionHandler2.Received(0).Handle(Arg.Any<CodeLensParams>(), Arg.Any<CancellationToken>());
await codeActionHandler.Received(1).Handle(Arg.Any<CodeLensParams>(), Arg.Any<CancellationToken>());
}

[Fact]
Expand All @@ -155,7 +201,10 @@ public async Task ShouldRouteTo_CorrectRequestWhenGivenNullParams()
.Returns(Task.CompletedTask);

var collection = new HandlerCollection { handler };
var mediator = new LspRequestRouter(collection, _testLoggerFactory, _handlerMatcherCollection, new Serializer());
var handlerMatcherCollection = new HandlerMatcherCollection {
new TextDocumentMatcher(_testLoggerFactory.CreateLogger<TextDocumentMatcher>(), collection.TextDocumentSyncHandlers)
};
var mediator = new LspRequestRouter(collection, _testLoggerFactory, handlerMatcherCollection, new Serializer());

var id = Guid.NewGuid().ToString();
var request = new Request(id, GeneralNames.Shutdown, new JObject());
Expand All @@ -177,7 +226,10 @@ public async Task ShouldHandle_Request_WithNullParameters()
};

var collection = new HandlerCollection { shutdownHandler };
var mediator = new LspRequestRouter(collection, _testLoggerFactory, _handlerMatcherCollection, new Serializer());
var handlerMatcherCollection = new HandlerMatcherCollection {
new TextDocumentMatcher(_testLoggerFactory.CreateLogger<TextDocumentMatcher>(), collection.TextDocumentSyncHandlers)
};
var mediator = new LspRequestRouter(collection, _testLoggerFactory, handlerMatcherCollection, new Serializer());

JToken @params = JValue.CreateNull(); // If the "params" property present but null, this will be JTokenType.Null.

Expand All @@ -201,7 +253,10 @@ public async Task ShouldHandle_Request_WithMissingParameters()
};

var collection = new HandlerCollection { shutdownHandler };
var mediator = new LspRequestRouter(collection, _testLoggerFactory, _handlerMatcherCollection, new Serializer());
var handlerMatcherCollection = new HandlerMatcherCollection {
new TextDocumentMatcher(_testLoggerFactory.CreateLogger<TextDocumentMatcher>(), collection.TextDocumentSyncHandlers)
};
var mediator = new LspRequestRouter(collection, _testLoggerFactory, handlerMatcherCollection, new Serializer());

JToken @params = null; // If the "params" property was missing entirely, this will be null.

Expand Down
Loading

0 comments on commit 023ac40

Please sign in to comment.