Skip to content

Updated to fix an issue where routing wouldn't work correctly #68

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Feb 2, 2018
Merged

Updated to fix an issue where routing wouldn't work correctly #68

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions src/Directory.Build.props
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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