From 11c11f2122f0be3e79c90fce29b81b528e9f104d Mon Sep 17 00:00:00 2001 From: forozco Date: Tue, 13 Feb 2018 15:35:15 +0000 Subject: [PATCH 01/19] WIP --- pyls/dispatcher.py | 30 ------- pyls/language_server.py | 149 +++++++++++++++++++++++++++++------ pyls/python_ls.py | 60 +++++++++----- pyls/rpc_manager.py | 106 +++++++++++++++++++++++++ pyls/server.py | 127 ----------------------------- pyls/workspace.py | 6 +- setup.py | 1 + test/fixtures.py | 15 +++- test/test_dispatcher.py | 21 ----- test/test_language_server.py | 42 +++++----- test/test_rpc_manager.py | 22 ++++++ 11 files changed, 327 insertions(+), 252 deletions(-) delete mode 100644 pyls/dispatcher.py create mode 100644 pyls/rpc_manager.py delete mode 100644 pyls/server.py delete mode 100644 test/test_dispatcher.py create mode 100644 test/test_rpc_manager.py diff --git a/pyls/dispatcher.py b/pyls/dispatcher.py deleted file mode 100644 index b6a1c376..00000000 --- a/pyls/dispatcher.py +++ /dev/null @@ -1,30 +0,0 @@ -# Copyright 2017 Palantir Technologies, Inc. -import re - -_RE_FIRST_CAP = re.compile('(.)([A-Z][a-z]+)') -_RE_ALL_CAP = re.compile('([a-z0-9])([A-Z])') - - -class JSONRPCMethodDispatcher(object): - """JSON RPC method dispatcher that calls methods on itself with params.""" - - def __getitem__(self, item): - """The jsonrpc dispatcher uses getitem to retrieve the RPC method implementation.""" - method_name = "m_" + _method_to_string(item) - if not hasattr(self, method_name): - raise KeyError("Cannot find method %s" % method_name) - func = getattr(self, method_name) - - def wrapped(*args, **kwargs): - return func(*args, **kwargs) - - return wrapped - - -def _method_to_string(method): - return _camel_to_underscore(method.replace("/", "__").replace("$", "")) - - -def _camel_to_underscore(string): - s1 = _RE_FIRST_CAP.sub(r'\1_\2', string) - return _RE_ALL_CAP.sub(r'\1_\2', s1).lower() diff --git a/pyls/language_server.py b/pyls/language_server.py index f3101820..aea73852 100644 --- a/pyls/language_server.py +++ b/pyls/language_server.py @@ -1,8 +1,14 @@ # Copyright 2017 Palantir Technologies, Inc. import logging import socketserver -from . import dispatcher, uris -from .server import JSONRPCServer +from uuid import uuid1 + +from concurrent.futures import ThreadPoolExecutor, Future +from jsonrpc.jsonrpc2 import JSONRPC20Response, JSONRPC20Request +from jsonrpc.exceptions import JSONRPCMethodNotFound + +from . import uris +from .rpc_manager import JSONRPCManager log = logging.getLogger(__name__) @@ -22,50 +28,138 @@ def handle(self): def start_tcp_lang_server(bind_addr, port, handler_class): - if not issubclass(handler_class, JSONRPCServer): - raise ValueError("Handler class must be a subclass of JSONRPCServer") + if not issubclass(handler_class, LanguageServer): + raise ValueError('Handler class must be a subclass of JSONRPCServer') # Construct a custom wrapper class around the user's handler_class wrapper_class = type( - handler_class.__name__ + "Handler", + handler_class.__name__ + 'Handler', (_StreamHandlerWrapper,), {'DELEGATE_CLASS': handler_class} ) - server = socketserver.ThreadingTCPServer((bind_addr, port), wrapper_class) + server = socketserver.TCPServer((bind_addr, port), wrapper_class) try: - log.info("Serving %s on (%s, %s)", handler_class.__name__, bind_addr, port) + log.info('Serving %s on (%s, %s)', handler_class.__name__, bind_addr, port) server.serve_forever() finally: - log.info("Shutting down") + log.info('Shutting down') server.server_close() def start_io_lang_server(rfile, wfile, handler_class): - if not issubclass(handler_class, JSONRPCServer): - raise ValueError("Handler class must be a subclass of JSONRPCServer") - log.info("Starting %s IO language server", handler_class.__name__) + if not issubclass(handler_class, LanguageServer): + raise ValueError('Handler class must be a subclass of JSONRPCServer') + log.info('Starting %s IO language server', handler_class.__name__) server = handler_class(rfile, wfile) - server.handle() + server.start() -class LanguageServer(dispatcher.JSONRPCMethodDispatcher, JSONRPCServer): +class JSONRPCManager(object): """ Implementation of the Microsoft VSCode Language Server Protocol https://github.com/Microsoft/language-server-protocol/blob/master/versions/protocol-1-x.md """ - process_id = None - root_uri = None - init_opts = None + def __init__(self, rx, tx): + self._message_manager = JSONRPCManager(rx, tx) + self._sent_requests = {} + self._received_requests = {} + self.executor_service = ThreadPoolExecutor() + self.process_id = None + self.root_uri = None + self.init_opts = None + + def start(self): + self.consume_requests() + + def call(self, method, params=None): + log.debug('Calling %s %s', method, params) + request = JSONRPC20Request(_id=str(uuid1()), method=method, params=params) + request_future = Future() + self._sent_requests[request._id] = request_future + self._message_manager.write_message(request.data) + return request_future + + def notify(self, method, params=None): + log.debug('Notify %s %s', method, params) + notification = JSONRPC20Request(method=method, params=params) + self._message_manager.write_message(notification.data) + + + def consume_requests(self): + """ Infinite loop watching for messages from the client""" + for message in self._message_manager.get_messages(): + log.debug('Received message %s', message if isinstance(message, dict) else message.data) + if isinstance(message, JSONRPC20Response): + self._handle_response(message) + elif isinstance(message, JSONRPC20Request): + if message.is_notification: + self.handle_notification(message.method, message.params) + else: + self._handle_request(message) + else: + # TODO(forozco): do something with rpc errors + pass + + def _handle_request(self, request): + handler = self.get_request_handler(request.method) + if handler is None: + self._message_manager.write_message(JSONRPCMethodNotFound().data) + return + elif request._id in self._received_requests: + log.error('Received request %s with duplicate id', request.data) + return + + future = self.executor_service.submit(handler, **request.params) + self._received_requests[request._id] = future + def did_finish(completed_future): + if completed_future.cancelled(): + log.debug('Cleared cancelled request %d', request._id) + del self._received_requests[request._id] + return + + error, trace = completed_future.exception_info() + response = None + if error is not None: + if isinstance(error, dict): + response = JSONRPC20Response(_id=request._id, error=error) + log.error("responded to %s with %s", request.data, response.data) + else: + log.error('request %d failed %s %s', request._id, error, trace) + return + else: + log.debug('Sending response %s', completed_future.result()) + response = JSONRPC20Response(_id=request._id, result=completed_future.result()) + self._message_manager.write_message(response._data) + del self._received_requests[request._id] + + future.add_done_callback(did_finish) + + def _handle_response(self, response): + try: + request = self._sent_requests[response._id] + def cleanup(): + del self._sent_requests[response._id] + request.add_done_callback(cleanup) + request.set_result(response.result if response.result is not None else response.error) + + except KeyError: + log.error('Received unexpected response %s', response.data) + + def handle_notification(self, method, params): + pass - def capabilities(self): # pylint: disable=no-self-use - return {} + def get_request_handler(self, method): + pass def initialize(self, root_uri, init_opts, process_id): pass + def capabilities(self): # pylint: disable=no-self-use + return {} + def m_initialize(self, **kwargs): - log.debug("Language server initialized with %s", kwargs) + log.debug('Language server initialized with %s', kwargs) if 'rootUri' in kwargs: self.root_uri = kwargs['rootUri'] elif 'rootPath' in kwargs: @@ -82,14 +176,17 @@ def m_initialize(self, **kwargs): return {'capabilities': self.capabilities()} def m___cancel_request(self, **kwargs): - # TODO: We could I suppose launch tasks in their own threads and kill - # them on cancel, but is it really worth the effort given most methods - # are reasonably quick? - # This tends to happen when cancelling a hover request - pass + request_id = kwargs['id'] + log.debug('Cancel request %d', request_id) + try: + # Request will only be cancelled if it has not begun execution + self._received_requests[request_id].cancel() + except KeyError: + log.error('Received cancel for finished/nonexistent request %d', request_id) def m_shutdown(self, **_kwargs): - self.shutdown() + self.m_exit() def m_exit(self, **_kwargs): - self.exit() + self.executor_service.shutdown() + self._message_manager.exit() diff --git a/pyls/python_ls.py b/pyls/python_ls.py index a966156c..0da4080d 100644 --- a/pyls/python_ls.py +++ b/pyls/python_ls.py @@ -1,5 +1,6 @@ # Copyright 2017 Palantir Technologies, Inc. import logging +import re from . import lsp, _utils from .config import config from .language_server import LanguageServer @@ -7,38 +8,45 @@ log = logging.getLogger(__name__) +_RE_FIRST_CAP = re.compile('(.)([A-Z][a-z]+)') +_RE_ALL_CAP = re.compile('([a-z0-9])([A-Z])') + LINT_DEBOUNCE_S = 0.5 # 500 ms class PythonLanguageServer(LanguageServer): # pylint: disable=too-many-public-methods,redefined-builtin - workspace = None - config = None - - # Set of method dispatchers to query - _dispatchers = [] - - def __getitem__(self, item): - """Override the method dispatcher to farm out any unknown messages to our plugins.""" - try: - return super(PythonLanguageServer, self).__getitem__(item) - except KeyError: - log.debug("Checking dispatchers for %s: %s", item, self._dispatchers) + def __init__(self, rx, tx): + super(PythonLanguageServer, self).__init__(rx, tx) + self.workspace = None + self.config = None + self._dispatchers = [] + + def handle_notification(self, method, params): + handler = self.get_request_handler(method) + if handler is None: + log.error('could not find notification handler for %s', method) + else: + handler(**params) + + def get_request_handler(self, method): + method_call = 'm_{}'.format(_method_to_string(method)) + if hasattr(self, method_call): + return getattr(self, method_call) + elif self._dispatchers: for dispatcher in self._dispatchers: try: - return dispatcher.__getitem__(item) + return dispatcher.__getitem__(method_call) except KeyError: pass - raise KeyError("Unknown item %s" % item) - - def _hook_caller(self, hook_name): - return self.config.plugin_manager.subset_hook_caller(hook_name, self.config.disabled_plugins) + return None def _hook(self, hook_name, doc_uri=None, **kwargs): + """Calls hook_name and returns a list of results from all registered handlers""" doc = self.workspace.get_document(doc_uri) if doc_uri else None - hook = self.config.plugin_manager.subset_hook_caller(hook_name, self.config.disabled_plugins) - return hook(config=self.config, workspace=self.workspace, document=doc, **kwargs) + hook_handlers = self.config.plugin_manager.subset_hook_caller(hook_name, self.config.disabled_plugins) + return hook_handlers(config=self.config, workspace=self.workspace, document=doc, **kwargs) def capabilities(self): server_capabilities = { @@ -66,7 +74,7 @@ def capabilities(self): 'textDocumentSync': lsp.TextDocumentSyncKind.INCREMENTAL, 'experimental': merge(self._hook('pyls_experimental_capabilities')) } - log.info("Server capabilities: %s", server_capabilities) + log.info('Server capabilities: %s', server_capabilities) return server_capabilities def initialize(self, root_uri, init_opts, _process_id): @@ -92,7 +100,8 @@ def definitions(self, doc_uri, position): return flatten(self._hook('pyls_definitions', doc_uri, position=position)) def document_symbols(self, doc_uri): - return flatten(self._hook('pyls_document_symbols', doc_uri)) + def wrapper(): + return flatten(self._hook('pyls_document_symbols', doc_uri)) def execute_command(self, command, arguments): return self._hook('pyls_execute_command', command=command, arguments=arguments) @@ -194,6 +203,15 @@ def m_workspace__execute_command(self, command=None, arguments=None): return self.execute_command(command, arguments) +def _method_to_string(method): + return _camel_to_underscore(method.replace("/", "__").replace("$", "")) + + +def _camel_to_underscore(string): + s1 = _RE_FIRST_CAP.sub(r'\1_\2', string) + return _RE_ALL_CAP.sub(r'\1_\2', s1).lower() + + def flatten(list_of_lists): return [item for lst in list_of_lists for item in lst] diff --git a/pyls/rpc_manager.py b/pyls/rpc_manager.py new file mode 100644 index 00000000..270a3ee1 --- /dev/null +++ b/pyls/rpc_manager.py @@ -0,0 +1,106 @@ +# Copyright 2017 Palantir Technologies, Inc. +import json +import logging + +from jsonrpc.jsonrpc2 import JSONRPC20Response +from jsonrpc.jsonrpc import JSONRPCRequest +from jsonrpc.exceptions import ( + JSONRPCError, + JSONRPCInvalidRequest, + JSONRPCInvalidRequestException, + JSONRPCParseError, +) + + +log = logging.getLogger(__name__) + + +class JSONRPCManager(object): + """ Read/Write JSON RPC messages """ + + def __init__(self, rfile, wfile): + self.batch_messages = {} + self.rfile = rfile + self.wfile = wfile + + def exit(self): + # Exit causes a complete exit of the server + self.rfile.close() + self.wfile.close() + + def get_messages(self): + """Generator that produces well structured JSON RPC message + :return JSONRPCRequest request: + """ + while True: + request_str = self._read_message() + + if request_str is None: + self.write_message(JSONRPCParseError()._data) + continue + if isinstance(request_str, bytes): + request_str = request_str.decode("utf-8") + + try: + message = JSONRPCRequest.from_json(request_str) + except (TypeError, ValueError, JSONRPCInvalidRequestException): + try: + message = JSONRPC20Response.from_json(request_str) + except (KeyError, ValueError): + try: + message = JSONRPCError.from_json(request_str) + except (ValueError, TypeError): + self.write_message(JSONRPCInvalidRequest()._data) + continue + + yield message + + def write_message(self, message): + """ Write message to out file descriptor + + :param any message: response blob + """ + body = json.dumps(message, separators=(",", ":")) + content_length = len(body) + response = ( + "Content-Length: {}\r\n" + "Content-Type: application/vscode-jsonrpc; charset=utf8\r\n\r\n" + "{}".format(content_length, body) + ) + self.wfile.write(response.encode('utf-8')) + self.wfile.flush() + + def _read_message(self): + """Reads the contents of a message + + :return body of message if parsable else None + """ + line = self.rfile.readline() + + if not line: + return None + + content_length = _content_length(line) + + # Blindly consume all header lines + while line and line.strip(): + line = self.rfile.readline() + + if not line: + return None + + # Grab the body + return self.rfile.read(content_length) + + +def _content_length(line): + """Extract the content length from an input line.""" + if line.startswith(b'Content-Length: '): + _, value = line.split(b'Content-Length: ') + value = value.strip() + try: + return int(value) + except ValueError: + raise ValueError("Invalid Content-Length header: {}".format(value)) + + return None diff --git a/pyls/server.py b/pyls/server.py deleted file mode 100644 index fb7069c5..00000000 --- a/pyls/server.py +++ /dev/null @@ -1,127 +0,0 @@ -# Copyright 2017 Palantir Technologies, Inc. -import json -import logging -import uuid - -from jsonrpc import jsonrpc2, JSONRPCResponseManager - -log = logging.getLogger(__name__) - - -class JSONRPCServer(object): - """ Read/Write JSON RPC messages """ - - def __init__(self, rfile, wfile): - self.rfile = rfile - self.wfile = wfile - - self._callbacks = {} - self._shutdown = False - - def exit(self): - # Exit causes a complete exit of the server - self.rfile.close() - self.wfile.close() - - def shutdown(self): - # Shutdown signals the server to stop, but not exit - self._shutdown = True - log.debug("Server shut down, awaiting exit notification") - - def handle(self): - while True: - try: - data = self._read_message() - log.debug("Got message: %s", data) - - if self._shutdown: - # Handle only the exit notification when we're shut down - JSONRPCResponseManager.handle(data, {'exit': self.exit}) - break - - if isinstance(data, bytes): - data = data.decode("utf-8") - - msg = json.loads(data) - if 'method' in msg: - # It's a notification or request - # Dispatch to the thread pool for handling - response = JSONRPCResponseManager.handle(data, self) - if response is not None: - self._write_message(response.data) - else: - # Otherwise, it's a response message - on_result, on_error = self._callbacks.pop(msg['id']) - if 'result' in msg and on_result: - on_result(msg['result']) - elif 'error' in msg and on_error: - on_error(msg['error']) - except: # pylint: disable=bare-except - log.exception("Language server exiting due to uncaught exception") - break - - def call(self, method, params=None, on_result=None, on_error=None): - """Call a method on the client.""" - msg_id = str(uuid.uuid4()) - log.debug("Sending request %s: %s: %s", msg_id, method, params) - req = jsonrpc2.JSONRPC20Request(method=method, params=params) - req._id = msg_id - - def _default_on_error(error): - log.error("Call to %s failed with %s", method, error) - - if not on_error: - on_error = _default_on_error - - self._callbacks[msg_id] = (on_result, on_error) - self._write_message(req.data) - - def notify(self, method, params=None): - """ Send a notification to the client, expects no response. """ - log.debug("Sending notification %s: %s", method, params) - req = jsonrpc2.JSONRPC20Request( - method=method, params=params, is_notification=True - ) - self._write_message(req.data) - - def _read_message(self): - line = self.rfile.readline() - - if not line: - raise EOFError() - - content_length = _content_length(line) - - # Blindly consume all header lines - while line and line.strip(): - line = self.rfile.readline() - - if not line: - raise EOFError() - - # Grab the body - return self.rfile.read(content_length) - - def _write_message(self, msg): - body = json.dumps(msg, separators=(",", ":")) - content_length = len(body) - response = ( - "Content-Length: {}\r\n" - "Content-Type: application/vscode-jsonrpc; charset=utf8\r\n\r\n" - "{}".format(content_length, body) - ) - self.wfile.write(response.encode('utf-8')) - self.wfile.flush() - - -def _content_length(line): - """Extract the content length from an input line.""" - if line.startswith(b'Content-Length: '): - _, value = line.split(b'Content-Length: ') - value = value.strip() - try: - return int(value) - except ValueError: - raise ValueError("Invalid Content-Length header: {}".format(value)) - - return None diff --git a/pyls/workspace.py b/pyls/workspace.py index fd9374da..11b67e1e 100644 --- a/pyls/workspace.py +++ b/pyls/workspace.py @@ -130,12 +130,10 @@ def apply_edit(self, edit, on_result=None, on_error=None): ) def publish_diagnostics(self, doc_uri, diagnostics): - params = {'uri': doc_uri, 'diagnostics': diagnostics} - self._lang_server.notify(self.M_PUBLISH_DIAGNOSTICS, params) + self._lang_server.notify(self.M_PUBLISH_DIAGNOSTICS, params={'uri': doc_uri, 'diagnostics': diagnostics}) def show_message(self, message, msg_type=lsp.MessageType.Info): - params = {'type': msg_type, 'message': message} - self._lang_server.notify(self.M_SHOW_MESSAGE, params) + self._lang_server.notify(self.M_SHOW_MESSAGE, params={'type': msg_type, 'message': message}) def source_roots(self, document_path): """Return the source roots for the given document.""" diff --git a/setup.py b/setup.py index 194d3d3f..b4d9101b 100755 --- a/setup.py +++ b/setup.py @@ -34,6 +34,7 @@ install_requires=[ 'configparser', 'future>=0.14.0', + 'futures; python_version == "2.7"', 'jedi>=0.10', 'json-rpc', 'mccabe', diff --git a/test/fixtures.py b/test/fixtures.py index d25f191f..1e4ba043 100644 --- a/test/fixtures.py +++ b/test/fixtures.py @@ -1,10 +1,23 @@ # Copyright 2017 Palantir Technologies, Inc. import pytest +import os from pyls import uris +from pyls.rpc_manager import JSONRPCManager from pyls.config.config import Config from pyls.python_ls import PythonLanguageServer from pyls.workspace import Workspace -from io import StringIO +from StringIO import StringIO + + +@pytest.fixture +def rpc_manager(tmpdir): + # JSONRPCManager rx + manager_rx, tester_tx = os.pipe() + # Server to client pipe + tester_rx, manager_tx = os.pipe() + + yield JSONRPCManager(os.fdopen(manager_rx, tester_tx), tester_tx, tester_rx + @pytest.fixture diff --git a/test/test_dispatcher.py b/test/test_dispatcher.py deleted file mode 100644 index 12bd1d73..00000000 --- a/test/test_dispatcher.py +++ /dev/null @@ -1,21 +0,0 @@ -# Copyright 2017 Palantir Technologies, Inc. -import pytest -from pyls import dispatcher - - -class TestDispatcher(dispatcher.JSONRPCMethodDispatcher): - - def m_test__method(self, **params): - return params - - -def test_method_dispatcher(): - td = TestDispatcher() - params = {'hello': 'world'} - assert td['test/method'](**params) == params - - -def test_method_dispatcher_missing_method(): - td = TestDispatcher() - with pytest.raises(KeyError): - td['test/noMethod']('hello') diff --git a/test/test_language_server.py b/test/test_language_server.py index e1641dff..c22a5e64 100644 --- a/test/test_language_server.py +++ b/test/test_language_server.py @@ -1,22 +1,23 @@ # Copyright 2017 Palantir Technologies, Inc. -import json import os from threading import Thread import jsonrpc +from jsonrpc.exceptions import JSONRPCMethodNotFound import pytest -from pyls.server import JSONRPCServer from pyls.language_server import start_io_lang_server from pyls.python_ls import PythonLanguageServer -class JSONRPCClient(JSONRPCServer): +class JSONRPCClient(PythonLanguageServer): """ This is a weird way of testing.. but we're going to have two JSONRPCServers talking to each other. One pretending to be a 'VSCode'-like client, the other is our language server """ pass +def start_client(client): + client.start() @pytest.fixture def client_server(): @@ -34,57 +35,54 @@ def client_server(): server.start() client = JSONRPCClient(os.fdopen(scr, 'rb'), os.fdopen(csw, 'wb')) + Thread(target=start_client, args=client) yield client, server - client.call('shutdown') - response = _get_response(client) - assert response['result'] is None + def check(completed_future): + assert completed_future.result() is None + client.call('shutdown').add_done_callback(check) client.notify('exit') def test_initialize(client_server): client, server = client_server + def check(completed_future): + assert 'capabilities' in completed_future.result() client.call('initialize', { 'processId': 1234, 'rootPath': os.path.dirname(__file__), 'initializationOptions': {} - }) - response = _get_response(client) - - assert 'capabilities' in response['result'] + }).add_done_callback(check) def test_missing_message(client_server): client, server = client_server - client.call('unknown_method') - response = _get_response(client) - assert response['error']['code'] == -32601 # Method not implemented error + def check(result): + assert result['code'] == JSONRPCMethodNotFound.CODE + client.call('unknown_method').add_done_callback(check) def test_linting(client_server): client, server = client_server # Initialize + def check(result): + assert 'capabilities' in result client.call('initialize', { 'processId': 1234, 'rootPath': os.path.dirname(__file__), 'initializationOptions': {} - }) - response = _get_response(client) - - assert 'capabilities' in response['result'] + }).add_done_callback(check) # didOpen client.notify('textDocument/didOpen', { 'textDocument': {'uri': 'file:///test', 'text': 'import sys'} }) - response = _get_notification(client) - - assert response['method'] == 'textDocument/publishDiagnostics' - assert len(response['params']['diagnostics']) > 0 + # assert response['method'] == 'textDocument/publishDiagnostics' + # assert len(response['params']['diagnostics']) > 0 def _get_notification(client): @@ -94,4 +92,4 @@ def _get_notification(client): def _get_response(client): - return json.loads(client._read_message().decode('utf-8')) + return client._message_manager.get_messages().next().data diff --git a/test/test_rpc_manager.py b/test/test_rpc_manager.py new file mode 100644 index 00000000..c66f826e --- /dev/null +++ b/test/test_rpc_manager.py @@ -0,0 +1,22 @@ +# Copyright 2018 Palantir Technologies, Inc. +from jsonrpc.jsonrpc2 import JSONRPC20Request + +CONTENT_HEADER='Content-Length: {}\r\nContent-Type: application/vscode-jsonrpc; charset=utf8\r\n\r\n' +SIMPLE_MESSAGE='{"jsonrpc":"2.0","id":0,"method":"initialize","params":{}}\n' + + +def test_get_parse_message(rpc_manager): + manager, tx_pipe, rx_pipe = rpc_manager + tx_pipe.write(CONTENT_HEADER.format(len(SIMPLE_MESSAGE)).encode('utf-8')) + tx_pipe.write(SIMPLE_MESSAGE.encode('utf-8')) + messages = rpc_manager.get_messages() + assert isinstance(messages.next(), JSONRPC20Request) + +# +# def test_fail_to_parse(rpc_manager): +# wfile = rpc_manager.rfile +# rfile = rpc_manager.wfile +# wfile.write(unicode("test")) +# wfile.write(unicode('"jsonrpc": "2.0", "method": "textDocument/didOpen", "params": {}')) +# messages = rpc_manager.get_messages() +# assert messages.next() == "test" From 51b11ab8efd9294a864f8181412880cb5501b566 Mon Sep 17 00:00:00 2001 From: forozco Date: Wed, 14 Feb 2018 14:01:57 +0000 Subject: [PATCH 02/19] cleanup --- pyls/__main__.py | 7 +- pyls/language_server.py | 192 ---------------------------- pyls/message_manager.py | 116 +++++++++++++++++ pyls/python_ls.py | 121 +++++++++++++++--- pyls/rpc_manager.py | 239 ++++++++++++++++++++++------------- pyls/workspace.py | 10 +- test/fixtures.py | 12 +- test/test_language_server.py | 76 +++-------- test/test_message_manager.py | 13 ++ test/test_rpc_manager.py | 10 -- 10 files changed, 415 insertions(+), 381 deletions(-) delete mode 100644 pyls/language_server.py create mode 100644 pyls/message_manager.py create mode 100644 test/test_message_manager.py diff --git a/pyls/__main__.py b/pyls/__main__.py index 1671b67e..a74ead94 100644 --- a/pyls/__main__.py +++ b/pyls/__main__.py @@ -4,8 +4,7 @@ import logging import logging.config import sys -from . import language_server -from .python_ls import PythonLanguageServer +from .python_ls import start_io_lang_server, start_tcp_lang_server, PythonLanguageServer LOG_FORMAT = "%(asctime)s UTC - %(levelname)s - %(name)s - %(message)s" @@ -65,10 +64,10 @@ def main(): logging.getLogger().setLevel(level) if args.tcp: - language_server.start_tcp_lang_server(args.host, args.port, PythonLanguageServer) + start_tcp_lang_server(args.host, args.port, PythonLanguageServer) else: stdin, stdout = _binary_stdio() - language_server.start_io_lang_server(stdin, stdout, PythonLanguageServer) + start_io_lang_server(stdin, stdout, PythonLanguageServer) def _binary_stdio(): diff --git a/pyls/language_server.py b/pyls/language_server.py deleted file mode 100644 index aea73852..00000000 --- a/pyls/language_server.py +++ /dev/null @@ -1,192 +0,0 @@ -# Copyright 2017 Palantir Technologies, Inc. -import logging -import socketserver -from uuid import uuid1 - -from concurrent.futures import ThreadPoolExecutor, Future -from jsonrpc.jsonrpc2 import JSONRPC20Response, JSONRPC20Request -from jsonrpc.exceptions import JSONRPCMethodNotFound - -from . import uris -from .rpc_manager import JSONRPCManager - -log = logging.getLogger(__name__) - - -class _StreamHandlerWrapper(socketserver.StreamRequestHandler, object): - """A wrapper class that is used to construct a custom handler class.""" - - delegate = None - - def setup(self): - super(_StreamHandlerWrapper, self).setup() - # pylint: disable=no-member - self.delegate = self.DELEGATE_CLASS(self.rfile, self.wfile) - - def handle(self): - self.delegate.handle() - - -def start_tcp_lang_server(bind_addr, port, handler_class): - if not issubclass(handler_class, LanguageServer): - raise ValueError('Handler class must be a subclass of JSONRPCServer') - - # Construct a custom wrapper class around the user's handler_class - wrapper_class = type( - handler_class.__name__ + 'Handler', - (_StreamHandlerWrapper,), - {'DELEGATE_CLASS': handler_class} - ) - - server = socketserver.TCPServer((bind_addr, port), wrapper_class) - try: - log.info('Serving %s on (%s, %s)', handler_class.__name__, bind_addr, port) - server.serve_forever() - finally: - log.info('Shutting down') - server.server_close() - - -def start_io_lang_server(rfile, wfile, handler_class): - if not issubclass(handler_class, LanguageServer): - raise ValueError('Handler class must be a subclass of JSONRPCServer') - log.info('Starting %s IO language server', handler_class.__name__) - server = handler_class(rfile, wfile) - server.start() - - -class JSONRPCManager(object): - """ Implementation of the Microsoft VSCode Language Server Protocol - https://github.com/Microsoft/language-server-protocol/blob/master/versions/protocol-1-x.md - """ - - def __init__(self, rx, tx): - self._message_manager = JSONRPCManager(rx, tx) - self._sent_requests = {} - self._received_requests = {} - self.executor_service = ThreadPoolExecutor() - self.process_id = None - self.root_uri = None - self.init_opts = None - - def start(self): - self.consume_requests() - - def call(self, method, params=None): - log.debug('Calling %s %s', method, params) - request = JSONRPC20Request(_id=str(uuid1()), method=method, params=params) - request_future = Future() - self._sent_requests[request._id] = request_future - self._message_manager.write_message(request.data) - return request_future - - def notify(self, method, params=None): - log.debug('Notify %s %s', method, params) - notification = JSONRPC20Request(method=method, params=params) - self._message_manager.write_message(notification.data) - - - def consume_requests(self): - """ Infinite loop watching for messages from the client""" - for message in self._message_manager.get_messages(): - log.debug('Received message %s', message if isinstance(message, dict) else message.data) - if isinstance(message, JSONRPC20Response): - self._handle_response(message) - elif isinstance(message, JSONRPC20Request): - if message.is_notification: - self.handle_notification(message.method, message.params) - else: - self._handle_request(message) - else: - # TODO(forozco): do something with rpc errors - pass - - def _handle_request(self, request): - handler = self.get_request_handler(request.method) - if handler is None: - self._message_manager.write_message(JSONRPCMethodNotFound().data) - return - elif request._id in self._received_requests: - log.error('Received request %s with duplicate id', request.data) - return - - future = self.executor_service.submit(handler, **request.params) - self._received_requests[request._id] = future - def did_finish(completed_future): - if completed_future.cancelled(): - log.debug('Cleared cancelled request %d', request._id) - del self._received_requests[request._id] - return - - error, trace = completed_future.exception_info() - response = None - if error is not None: - if isinstance(error, dict): - response = JSONRPC20Response(_id=request._id, error=error) - log.error("responded to %s with %s", request.data, response.data) - else: - log.error('request %d failed %s %s', request._id, error, trace) - return - else: - log.debug('Sending response %s', completed_future.result()) - response = JSONRPC20Response(_id=request._id, result=completed_future.result()) - self._message_manager.write_message(response._data) - del self._received_requests[request._id] - - future.add_done_callback(did_finish) - - def _handle_response(self, response): - try: - request = self._sent_requests[response._id] - def cleanup(): - del self._sent_requests[response._id] - request.add_done_callback(cleanup) - request.set_result(response.result if response.result is not None else response.error) - - except KeyError: - log.error('Received unexpected response %s', response.data) - - def handle_notification(self, method, params): - pass - - def get_request_handler(self, method): - pass - - def initialize(self, root_uri, init_opts, process_id): - pass - - def capabilities(self): # pylint: disable=no-self-use - return {} - - def m_initialize(self, **kwargs): - log.debug('Language server initialized with %s', kwargs) - if 'rootUri' in kwargs: - self.root_uri = kwargs['rootUri'] - elif 'rootPath' in kwargs: - root_path = kwargs['rootPath'] - self.root_uri = uris.from_fs_path(root_path) - else: - self.root_uri = '' - self.init_opts = kwargs.get('initializationOptions') - self.process_id = kwargs.get('processId') - - self.initialize(self.root_uri, self.init_opts, self.process_id) - - # Get our capabilities - return {'capabilities': self.capabilities()} - - def m___cancel_request(self, **kwargs): - request_id = kwargs['id'] - log.debug('Cancel request %d', request_id) - try: - # Request will only be cancelled if it has not begun execution - self._received_requests[request_id].cancel() - except KeyError: - log.error('Received cancel for finished/nonexistent request %d', request_id) - - def m_shutdown(self, **_kwargs): - self.m_exit() - - def m_exit(self, **_kwargs): - self.executor_service.shutdown() - self._message_manager.exit() diff --git a/pyls/message_manager.py b/pyls/message_manager.py new file mode 100644 index 00000000..8d7735a8 --- /dev/null +++ b/pyls/message_manager.py @@ -0,0 +1,116 @@ +# Copyright 2017 Palantir Technologies, Inc. +import json +import logging +import threading + +from jsonrpc.jsonrpc2 import JSONRPC20Response +from jsonrpc.jsonrpc import JSONRPCRequest +from jsonrpc.exceptions import ( + JSONRPCInvalidRequestException, +) + + +log = logging.getLogger(__name__) + + +class MessageManager(object): + """ Read/Write JSON RPC messages """ + + def __init__(self, rfile, wfile): + self.batch_messages = {} + self.rfile = rfile + self.wfile = wfile + self.write_lock = threading.Lock() + + def close(self): + with self.write_lock: + self.wfile.close() + self.rfile.close() + + def get_messages(self): + """ + Generator that produces well structured JSON RPC message. + + Returns: + message: received message + + Note: + This method is not thread safe and should only invoked from a single thread + """ + while not self.rfile.closed: + request_str = self._read_message() + + if request_str is None: + # log.error("failed to read message") + continue + if isinstance(request_str, bytes): + request_str = request_str.decode("utf-8") + + try: + try: + message_blob = json.loads(request_str) + message = JSONRPCRequest.from_data(message_blob) + except JSONRPCInvalidRequestException: + # work around where JSONRPC20Reponse expects _id key + message_blob['_id'] = message_blob['id'] + message = JSONRPC20Response(**message_blob) + except (KeyError, ValueError): + log.error("Could not parse message %s", request_str) + continue + + yield message + + def write_message(self, message): + """ Write message to out file descriptor + + Args: + message (dict): body of the message to send + """ + with self.write_lock: + if self.wfile.closed: + return + log.debug("Sending %s", message) + body = json.dumps(message, separators=(",", ":")) + content_length = len(body) + response = ( + "Content-Length: {}\r\n" + "Content-Type: application/vscode-jsonrpc; charset=utf8\r\n\r\n" + "{}".format(content_length, body) + ) + self.wfile.write(response.encode('utf-8')) + self.wfile.flush() + + def _read_message(self): + """Reads the contents of a message + + :return body of message if parsable else None + """ + line = self.rfile.readline() + + if not line: + return None + + content_length = _content_length(line) + + # Blindly consume all header lines + while line and line.strip(): + line = self.rfile.readline() + + if not line: + return None + + # Grab the body + return self.rfile.read(content_length) + + +def _content_length(line): + """Extract the content length from an input line.""" + if line.startswith(b'Content-Length: '): + _, value = line.split(b'Content-Length: ') + value = value.strip() + try: + return int(value) + except ValueError: + raise ValueError("Invalid Content-Length header: {}".format(value)) + + return None diff --git a/pyls/python_ls.py b/pyls/python_ls.py index 0da4080d..f7533c92 100644 --- a/pyls/python_ls.py +++ b/pyls/python_ls.py @@ -1,9 +1,11 @@ # Copyright 2017 Palantir Technologies, Inc. import logging import re -from . import lsp, _utils +import socketserver + +from . import lsp, _utils, uris from .config import config -from .language_server import LanguageServer +from .rpc_manager import JSONRPCManager from .workspace import Workspace log = logging.getLogger(__name__) @@ -14,33 +16,83 @@ LINT_DEBOUNCE_S = 0.5 # 500 ms -class PythonLanguageServer(LanguageServer): +class _StreamHandlerWrapper(socketserver.StreamRequestHandler, object): + """A wrapper class that is used to construct a custom handler class.""" + + delegate = None + + def setup(self): + super(_StreamHandlerWrapper, self).setup() + # pylint: disable=no-member + self.delegate = self.DELEGATE_CLASS(self.rfile, self.wfile) + + def handle(self): + self.delegate.handle() + + +def start_tcp_lang_server(bind_addr, port, handler_class): + if not issubclass(handler_class, ): + raise ValueError('Handler class must be a subclass of JSONRPCServer') + + # Construct a custom wrapper class around the user's handler_class + wrapper_class = type( + handler_class.__name__ + 'Handler', + (_StreamHandlerWrapper,), + {'DELEGATE_CLASS': handler_class} + ) + + server = socketserver.TCPServer((bind_addr, port), wrapper_class) + try: + log.info('Serving %s on (%s, %s)', handler_class.__name__, bind_addr, port) + server.serve_forever() + finally: + log.info('Shutting down') + server.server_close() + + +def start_io_lang_server(rfile, wfile, handler_class): + if not issubclass(handler_class, PythonLanguageServer): + raise ValueError('Handler class must be a subclass of JSONRPCServer') + log.info('Starting %s IO language server', handler_class.__name__) + server = handler_class(rfile, wfile) + server.start() + + +class PythonLanguageServer(object): # pylint: disable=too-many-public-methods,redefined-builtin def __init__(self, rx, tx): - super(PythonLanguageServer, self).__init__(rx, tx) + self.rpc_manager = JSONRPCManager(rx, tx, self.handle_request) self.workspace = None self.config = None self._dispatchers = [] - def handle_notification(self, method, params): - handler = self.get_request_handler(method) - if handler is None: - log.error('could not find notification handler for %s', method) - else: - handler(**params) + def start(self): + """Entry point for the server""" + self.rpc_manager.consume_requests() + + def handle_request(self, method, params): + """Provides calllables to handle message requests + + Args: + method (str): + + Returns: + Callable if method is to be handled + + Raises: + KeyError: + """ - def get_request_handler(self, method): method_call = 'm_{}'.format(_method_to_string(method)) if hasattr(self, method_call): - return getattr(self, method_call) + return getattr(self, method_call)(**params) elif self._dispatchers: for dispatcher in self._dispatchers: - try: - return dispatcher.__getitem__(method_call) - except KeyError: - pass - return None + if method_call in dispatcher: + return dispatcher[method_call](**params) + + raise KeyError def _hook(self, hook_name, doc_uri=None, **kwargs): """Calls hook_name and returns a list of results from all registered handlers""" @@ -77,12 +129,37 @@ def capabilities(self): log.info('Server capabilities: %s', server_capabilities) return server_capabilities - def initialize(self, root_uri, init_opts, _process_id): - self.workspace = Workspace(root_uri, lang_server=self) + def m_initialize(self, **kwargs): + log.debug('Language server initialized with %s', kwargs) + if 'rootUri' in kwargs: + root_uri = kwargs['rootUri'] + elif 'rootPath' in kwargs: + root_path = kwargs['rootPath'] + root_uri = uris.from_fs_path(root_path) + else: + root_uri = '' + init_opts = kwargs.get('initializationOptions') + + self.workspace = Workspace(root_uri, rpc_manager=self.rpc_manager) self.config = config.Config(root_uri, init_opts) self._dispatchers = self._hook('pyls_dispatchers') self._hook('pyls_initialize') + # Get our capabilities + return {'capabilities': self.capabilities()} + + def m___cancel_request(self, **kwargs): + def handler(): + self.rpc_manager.cancel(kwargs['id']) + return handler + + def m_shutdown(self, **_kwargs): + self.rpc_manager.shutdown() + return None + + def m_exit(self, **_kwargs): + self.rpc_manager.exit() + def code_actions(self, doc_uri, range, context): return flatten(self._hook('pyls_code_actions', doc_uri, range=range, context=context)) @@ -100,8 +177,7 @@ def definitions(self, doc_uri, position): return flatten(self._hook('pyls_definitions', doc_uri, position=position)) def document_symbols(self, doc_uri): - def wrapper(): - return flatten(self._hook('pyls_document_symbols', doc_uri)) + return flatten(self._hook('pyls_document_symbols', doc_uri)) def execute_command(self, command, arguments): return self._hook('pyls_execute_command', command=command, arguments=arguments) @@ -133,6 +209,9 @@ def rename(self, doc_uri, position, new_name): def signature_help(self, doc_uri, position): return self._hook('pyls_signature_help', doc_uri, position=position) + def m__cancel_request(self, **kwargs): + self.rpc_manager.cancel(kwargs['id']) + def m_text_document__did_close(self, textDocument=None, **_kwargs): self.workspace.rm_document(textDocument['uri']) diff --git a/pyls/rpc_manager.py b/pyls/rpc_manager.py index 270a3ee1..3635e5d1 100644 --- a/pyls/rpc_manager.py +++ b/pyls/rpc_manager.py @@ -1,106 +1,173 @@ # Copyright 2017 Palantir Technologies, Inc. -import json import logging +from uuid import uuid1 -from jsonrpc.jsonrpc2 import JSONRPC20Response -from jsonrpc.jsonrpc import JSONRPCRequest -from jsonrpc.exceptions import ( - JSONRPCError, - JSONRPCInvalidRequest, - JSONRPCInvalidRequestException, - JSONRPCParseError, -) +from concurrent.futures import ThreadPoolExecutor, Future +from jsonrpc.base import JSONRPCBaseResponse +from jsonrpc.jsonrpc1 import JSONRPC10Response +from jsonrpc.jsonrpc2 import JSONRPC20Response, JSONRPC20Request +from jsonrpc.exceptions import JSONRPCMethodNotFound, JSONRPCInternalError +from .message_manager import MessageManager log = logging.getLogger(__name__) +RESPONSE_CLASS_MAP = { + "1.0": JSONRPC10Response, + "2.0": JSONRPC20Response +} -class JSONRPCManager(object): - """ Read/Write JSON RPC messages """ - def __init__(self, rfile, wfile): - self.batch_messages = {} - self.rfile = rfile - self.wfile = wfile +class JSONRPCManager(object): + """ Implementation of the Microsoft VSCode Language Server Protocol + https://github.com/Microsoft/language-server-protocol/blob/master/versions/protocol-1-x.md + """ + + def __init__(self, rx, tx, message_handler): + self._message_manager = MessageManager(rx, tx) + self._message_handler = message_handler + self._shutdown = False + self._sent_requests = {} + self._received_requests = {} + self._executor_service = ThreadPoolExecutor() + + def start(self): + """Start reading JSONRPC messages off of rx""" + self.consume_requests() + + def shutdown(self): + """Set flag to ignore all non exit messages""" + self._shutdown = True def exit(self): - # Exit causes a complete exit of the server - self.rfile.close() - self.wfile.close() + """Stop listening for new message""" + self._executor_service.shutdown() + self._message_manager.close() - def get_messages(self): - """Generator that produces well structured JSON RPC message - :return JSONRPCRequest request: - """ - while True: - request_str = self._read_message() - - if request_str is None: - self.write_message(JSONRPCParseError()._data) - continue - if isinstance(request_str, bytes): - request_str = request_str.decode("utf-8") - - try: - message = JSONRPCRequest.from_json(request_str) - except (TypeError, ValueError, JSONRPCInvalidRequestException): - try: - message = JSONRPC20Response.from_json(request_str) - except (KeyError, ValueError): - try: - message = JSONRPCError.from_json(request_str) - except (ValueError, TypeError): - self.write_message(JSONRPCInvalidRequest()._data) - continue - - yield message - - def write_message(self, message): - """ Write message to out file descriptor - - :param any message: response blob - """ - body = json.dumps(message, separators=(",", ":")) - content_length = len(body) - response = ( - "Content-Length: {}\r\n" - "Content-Type: application/vscode-jsonrpc; charset=utf8\r\n\r\n" - "{}".format(content_length, body) - ) - self.wfile.write(response.encode('utf-8')) - self.wfile.flush() - - def _read_message(self): - """Reads the contents of a message - - :return body of message if parsable else None - """ - line = self.rfile.readline() + def call(self, method, params=None): + """Send a JSONRPC message with an expected response. - if not line: - return None + Args: + method (str): The method name of the message to send + params (dict): The payload of the message - content_length = _content_length(line) + Returns: + Future that will resolve once a response has been recieved - # Blindly consume all header lines - while line and line.strip(): - line = self.rfile.readline() + """ + log.debug('Calling %s %s', method, params) + request = JSONRPC20Request(_id=uuid1().int, method=method, params=params) + request_future = Future() + self._sent_requests[request._id] = request_future + self._message_manager.write_message(request.data) + return request_future + + def notify(self, method, params=None): + """Send a JSONRPC notification. + + Args: + method (str): The method name of the notification to send + params (dict): The payload of the notification + """ + log.debug('Notify %s %s', method, params) + notification = JSONRPC20Request(method=method, params=params) + self._message_manager.write_message(notification.data) + + def cancel(self, request_id): + """Cancel pending request handler. + + Args: + request_id (string | number): The id of the original request + + Note: + Request will only be cancelled if it has not begun execution. + """ + log.debug('Cancel request %d', request_id) + try: + self._received_requests[request_id].cancel() + except KeyError: + log.error('Received cancel for finished/nonexistent request %d', request_id) + + def consume_requests(self): + """ Infinite loop watching for messages from the client.""" + for message in self._message_manager.get_messages(): + if isinstance(message, JSONRPCBaseResponse): + self._handle_response(message) + else: + self._handle_request(message) + + def _handle_request(self, request): + """Execute corresponding handler for the recieved request + + Args: + request (JSONRPCBaseRequest): request to act upon + + Note: + requests are handled asynchronously if the handler returns a callable, otherwise they are handle + synchronously by the main thread + """ + if self._shutdown and request.method != 'exit': + return - if not line: - return None + params = request.params if request.params is not None else {} + try: + maybe_handler = self._message_handler(request.method, params) + except KeyError: + log.debug("No handler found for %s", request.method) + self._message_manager.write_message( + JSONRPC20Response(_id=request._id, error=JSONRPCMethodNotFound()._data).data) + return + + if request._id in self._received_requests: + log.error('Received request %s with duplicate id', request.data) + elif callable(maybe_handler): + self._handle_async_request(request, maybe_handler) + elif not request.is_notification: + log.debug("sync request %s", request._id) + response = _make_response(request, result=maybe_handler) + self._message_manager.write_message(response.data) + + def _handle_async_request(self, request, handler): + log.debug("async request %s", request._id) + future = self._executor_service.submit(handler) + + if request.is_notification: + return + + self._received_requests[request._id] = future + + def did_finish_callback(completed_future): + if completed_future.cancelled(): + log.debug('Cleared cancelled request %d', request._id) + del self._received_requests[request._id] + return + + error, trace = completed_future.exception_info() + del self._received_requests[request._id] + if error is not None: + log.error("Failed to handle request %s with error %s %s", request._id, error, trace) + # TODO(forozco): add more descriptive error + response = _make_response(request, errror=JSONRPCInternalError()._data) + else: + response = _make_response(request, result=completed_future.result()) + self._message_manager.write_message(response.data) + future.add_done_callback(did_finish_callback) + + def _handle_response(self, response): + try: + request = self._sent_requests[response._id] + log.debug("Received response %s", response.data) - # Grab the body - return self.rfile.read(content_length) + def cleanup(_): + del self._sent_requests[response._id] + request.add_done_callback(cleanup) + request.set_result(response.data) + except KeyError: + log.error('Received unexpected response %s', response.data) -def _content_length(line): - """Extract the content length from an input line.""" - if line.startswith(b'Content-Length: '): - _, value = line.split(b'Content-Length: ') - value = value.strip() - try: - return int(value) - except ValueError: - raise ValueError("Invalid Content-Length header: {}".format(value)) - return None +def _make_response(request, **kwargs): + response = RESPONSE_CLASS_MAP[request.JSONRPC_VERSION](_id=request._id, **kwargs) + response.request = request + return response diff --git a/pyls/workspace.py b/pyls/workspace.py index 11b67e1e..4f2a893b 100644 --- a/pyls/workspace.py +++ b/pyls/workspace.py @@ -74,12 +74,12 @@ class Workspace(object): M_SHOW_MESSAGE = 'window/showMessage' PRELOADED_MODULES = get_preferred_submodules() - def __init__(self, root_uri, lang_server=None): + def __init__(self, root_uri, rpc_manager=None): self._root_uri = root_uri + self._rpc_manager = rpc_manager self._root_uri_scheme = uris.urlparse(self._root_uri)[0] self._root_path = uris.to_fs_path(self._root_uri) self._docs = {} - self._lang_server = lang_server # Whilst incubating, keep private self.__rope = Project(self._root_path) @@ -124,16 +124,16 @@ def update_document(self, doc_uri, change, version=None): self._docs[doc_uri].version = version def apply_edit(self, edit, on_result=None, on_error=None): - return self._lang_server.call( + return self._rpc_manager.call( self.M_APPLY_EDIT, {'edit': edit}, on_result=on_result, on_error=on_error ) def publish_diagnostics(self, doc_uri, diagnostics): - self._lang_server.notify(self.M_PUBLISH_DIAGNOSTICS, params={'uri': doc_uri, 'diagnostics': diagnostics}) + self._rpc_manager.notify(self.M_PUBLISH_DIAGNOSTICS, params={'uri': doc_uri, 'diagnostics': diagnostics}) def show_message(self, message, msg_type=lsp.MessageType.Info): - self._lang_server.notify(self.M_SHOW_MESSAGE, params={'type': msg_type, 'message': message}) + self._rpc_manager.notify(self.M_SHOW_MESSAGE, params={'type': msg_type, 'message': message}) def source_roots(self, document_path): """Return the source roots for the given document.""" diff --git a/test/fixtures.py b/test/fixtures.py index 1e4ba043..b3ed545b 100644 --- a/test/fixtures.py +++ b/test/fixtures.py @@ -2,7 +2,7 @@ import pytest import os from pyls import uris -from pyls.rpc_manager import JSONRPCManager +from pyls.message_manager import MessageManager from pyls.config.config import Config from pyls.python_ls import PythonLanguageServer from pyls.workspace import Workspace @@ -10,14 +10,16 @@ @pytest.fixture -def rpc_manager(tmpdir): - # JSONRPCManager rx +def message_manager(tmpdir): manager_rx, tester_tx = os.pipe() - # Server to client pipe tester_rx, manager_tx = os.pipe() - yield JSONRPCManager(os.fdopen(manager_rx, tester_tx), tester_tx, tester_rx + rx, tx, = os.fdopen(tester_rx, 'rb'), os.fdopen(tester_tx, 'wb') + yield MessageManager(os.fdopen(manager_rx, 'rb'), os.fdopen(tester_tx, 'wb')), rx, tx + + rx.close() + tx.close() @pytest.fixture diff --git a/test/test_language_server.py b/test/test_language_server.py index c22a5e64..ed559065 100644 --- a/test/test_language_server.py +++ b/test/test_language_server.py @@ -6,15 +6,9 @@ from jsonrpc.exceptions import JSONRPCMethodNotFound import pytest -from pyls.language_server import start_io_lang_server -from pyls.python_ls import PythonLanguageServer +from pyls.python_ls import start_io_lang_server ,PythonLanguageServer - -class JSONRPCClient(PythonLanguageServer): - """ This is a weird way of testing.. but we're going to have two JSONRPCServers - talking to each other. One pretending to be a 'VSCode'-like client, the other is - our language server """ - pass +CALL_TIMEOUT = 2 def start_client(client): client.start() @@ -28,68 +22,34 @@ def client_server(): # Server to client pipe scr, scw = os.pipe() - server = Thread(target=start_io_lang_server, args=( + server_thread = Thread(target=start_io_lang_server, args=( os.fdopen(csr, 'rb'), os.fdopen(scw, 'wb'), PythonLanguageServer )) - server.daemon = True - server.start() + server_thread.daemon = True + server_thread.start() - client = JSONRPCClient(os.fdopen(scr, 'rb'), os.fdopen(csw, 'wb')) - Thread(target=start_client, args=client) + client = PythonLanguageServer(os.fdopen(scr, 'rb'), os.fdopen(csw, 'wb')) + client_thread = Thread(target=start_client, args=[client]) + client_thread.daemon = True + client_thread.start() - yield client, server + yield client - def check(completed_future): - assert completed_future.result() is None - client.call('shutdown').add_done_callback(check) - client.notify('exit') + shutdown_response = client.rpc_manager.call('shutdown').result(timeout=CALL_TIMEOUT) + assert shutdown_response['result'] is None + client.rpc_manager.notify('exit') def test_initialize(client_server): - client, server = client_server - - def check(completed_future): - assert 'capabilities' in completed_future.result() - client.call('initialize', { + response = client_server.rpc_manager.call('initialize', { 'processId': 1234, 'rootPath': os.path.dirname(__file__), 'initializationOptions': {} - }).add_done_callback(check) + }).result(timeout=CALL_TIMEOUT) + assert 'capabilities' in response['result'] def test_missing_message(client_server): - client, server = client_server - - def check(result): - assert result['code'] == JSONRPCMethodNotFound.CODE - client.call('unknown_method').add_done_callback(check) - - -def test_linting(client_server): - client, server = client_server - - # Initialize - def check(result): - assert 'capabilities' in result - client.call('initialize', { - 'processId': 1234, - 'rootPath': os.path.dirname(__file__), - 'initializationOptions': {} - }).add_done_callback(check) - - # didOpen - client.notify('textDocument/didOpen', { - 'textDocument': {'uri': 'file:///test', 'text': 'import sys'} - }) - # assert response['method'] == 'textDocument/publishDiagnostics' - # assert len(response['params']['diagnostics']) > 0 - - -def _get_notification(client): - request = jsonrpc.jsonrpc.JSONRPCRequest.from_json(client._read_message().decode('utf-8')) - assert request.is_notification - return request.data - + response = client_server.rpc_manager.call('unknown_method').result(timeout=CALL_TIMEOUT) + assert response['error']['code'] == JSONRPCMethodNotFound.CODE -def _get_response(client): - return client._message_manager.get_messages().next().data diff --git a/test/test_message_manager.py b/test/test_message_manager.py new file mode 100644 index 00000000..0dc45671 --- /dev/null +++ b/test/test_message_manager.py @@ -0,0 +1,13 @@ +# Copyright 2018 Palantir Technologies, Inc. +from jsonrpc.jsonrpc2 import JSONRPC20Request + +CONTENT_HEADER='Content-Length: {}\r\nContent-Type: application/vscode-jsonrpc; charset=utf8\r\n\r\n' +SIMPLE_MESSAGE='{"jsonrpc":"2.0","id":0,"method":"initialize","params":{}}\n' + + +# def test_recieve_message(message_manager): +# manager, rx, tx_pipe = message_manager +# tx_pipe.write(CONTENT_HEADER.format(len(SIMPLE_MESSAGE)).encode('utf-8')) +# tx_pipe.write(SIMPLE_MESSAGE.encode('utf-8')) +# messages = manager.get_messages() +# assert isinstance(messages.next(), JSONRPC20Request) diff --git a/test/test_rpc_manager.py b/test/test_rpc_manager.py index c66f826e..dcee31b3 100644 --- a/test/test_rpc_manager.py +++ b/test/test_rpc_manager.py @@ -1,16 +1,6 @@ # Copyright 2018 Palantir Technologies, Inc. from jsonrpc.jsonrpc2 import JSONRPC20Request -CONTENT_HEADER='Content-Length: {}\r\nContent-Type: application/vscode-jsonrpc; charset=utf8\r\n\r\n' -SIMPLE_MESSAGE='{"jsonrpc":"2.0","id":0,"method":"initialize","params":{}}\n' - - -def test_get_parse_message(rpc_manager): - manager, tx_pipe, rx_pipe = rpc_manager - tx_pipe.write(CONTENT_HEADER.format(len(SIMPLE_MESSAGE)).encode('utf-8')) - tx_pipe.write(SIMPLE_MESSAGE.encode('utf-8')) - messages = rpc_manager.get_messages() - assert isinstance(messages.next(), JSONRPC20Request) # # def test_fail_to_parse(rpc_manager): From 2e746a5997456eb4790e1e969a287eecb4deb567 Mon Sep 17 00:00:00 2001 From: forozco Date: Wed, 14 Feb 2018 16:17:24 +0000 Subject: [PATCH 03/19] more tests --- pyls/python_ls.py | 3 +- pyls/rpc_manager.py | 6 +- setup.py | 2 +- test/fixtures.py | 14 ---- test/test_message_manager.py | 65 +++++++++++++++--- test/test_rpc_manager.py | 129 ++++++++++++++++++++++++++++++++--- 6 files changed, 181 insertions(+), 38 deletions(-) diff --git a/pyls/python_ls.py b/pyls/python_ls.py index f7533c92..2862d2c4 100644 --- a/pyls/python_ls.py +++ b/pyls/python_ls.py @@ -5,6 +5,7 @@ from . import lsp, _utils, uris from .config import config +from .message_manager import MessageManager from .rpc_manager import JSONRPCManager from .workspace import Workspace @@ -62,7 +63,7 @@ class PythonLanguageServer(object): # pylint: disable=too-many-public-methods,redefined-builtin def __init__(self, rx, tx): - self.rpc_manager = JSONRPCManager(rx, tx, self.handle_request) + self.rpc_manager = JSONRPCManager(MessageManager(rx, tx), self.handle_request) self.workspace = None self.config = None self._dispatchers = [] diff --git a/pyls/rpc_manager.py b/pyls/rpc_manager.py index 3635e5d1..6852c859 100644 --- a/pyls/rpc_manager.py +++ b/pyls/rpc_manager.py @@ -8,8 +8,6 @@ from jsonrpc.jsonrpc2 import JSONRPC20Response, JSONRPC20Request from jsonrpc.exceptions import JSONRPCMethodNotFound, JSONRPCInternalError -from .message_manager import MessageManager - log = logging.getLogger(__name__) RESPONSE_CLASS_MAP = { @@ -23,8 +21,8 @@ class JSONRPCManager(object): https://github.com/Microsoft/language-server-protocol/blob/master/versions/protocol-1-x.md """ - def __init__(self, rx, tx, message_handler): - self._message_manager = MessageManager(rx, tx) + def __init__(self, message_manager, message_handler): + self._message_manager = message_manager self._message_handler = message_handler self._shutdown = False self._sent_requests = {} diff --git a/setup.py b/setup.py index 00fe97db..e3860de6 100755 --- a/setup.py +++ b/setup.py @@ -51,7 +51,7 @@ # for example: # $ pip install -e .[test] extras_require={ - 'test': ['tox', 'versioneer', 'pytest', 'pytest-cov', 'coverage'], + 'test': ['tox', 'versioneer', 'pytest', 'mock', 'pytest-cov', 'coverage'], }, # To provide executable scripts, use entry points in preference to the diff --git a/test/fixtures.py b/test/fixtures.py index b3ed545b..a51c2ca4 100644 --- a/test/fixtures.py +++ b/test/fixtures.py @@ -2,26 +2,12 @@ import pytest import os from pyls import uris -from pyls.message_manager import MessageManager from pyls.config.config import Config from pyls.python_ls import PythonLanguageServer from pyls.workspace import Workspace from StringIO import StringIO -@pytest.fixture -def message_manager(tmpdir): - manager_rx, tester_tx = os.pipe() - tester_rx, manager_tx = os.pipe() - - rx, tx, = os.fdopen(tester_rx, 'rb'), os.fdopen(tester_tx, 'wb') - - yield MessageManager(os.fdopen(manager_rx, 'rb'), os.fdopen(tester_tx, 'wb')), rx, tx - - rx.close() - tx.close() - - @pytest.fixture def pyls(tmpdir): """ Return an initialized python LS """ diff --git a/test/test_message_manager.py b/test/test_message_manager.py index 0dc45671..9d0e4bad 100644 --- a/test/test_message_manager.py +++ b/test/test_message_manager.py @@ -1,13 +1,60 @@ # Copyright 2018 Palantir Technologies, Inc. -from jsonrpc.jsonrpc2 import JSONRPC20Request +import pytest +import os +from jsonrpc.jsonrpc2 import JSONRPC20Request, JSONRPC20Response +from pyls.message_manager import MessageManager -CONTENT_HEADER='Content-Length: {}\r\nContent-Type: application/vscode-jsonrpc; charset=utf8\r\n\r\n' -SIMPLE_MESSAGE='{"jsonrpc":"2.0","id":0,"method":"initialize","params":{}}\n' +@pytest.fixture +def message_manager(tmpdir): + manager_rx, tester_tx = os.pipe() + tester_rx, manager_tx = os.pipe() + client = MessageManager(os.fdopen(manager_rx, 'rb'), os.fdopen(manager_tx, 'wb')) + server = MessageManager(os.fdopen(tester_rx, 'rb'), os.fdopen(tester_tx, 'wb')) -# def test_recieve_message(message_manager): -# manager, rx, tx_pipe = message_manager -# tx_pipe.write(CONTENT_HEADER.format(len(SIMPLE_MESSAGE)).encode('utf-8')) -# tx_pipe.write(SIMPLE_MESSAGE.encode('utf-8')) -# messages = manager.get_messages() -# assert isinstance(messages.next(), JSONRPC20Request) + yield client, server + + client.close() + server.close() + + +def test_receive_request(message_manager): + client, server = message_manager + request = {'jsonrpc': '2.0', 'id': 0, 'method': 'initialize', 'params': {}} + client.write_message(request) + message = server.get_messages().next() + assert isinstance(message, JSONRPC20Request) + assert request == message.data + assert not message.is_notification + + +def test_receive_notification(message_manager): + client, server = message_manager + notification = {'jsonrpc': '2.0', 'method': 'notify', 'params': {}} + client.write_message(notification) + message = server.get_messages().next() + assert isinstance(message, JSONRPC20Request) + assert notification == message.data + assert message.is_notification + + +def test_receive_response(message_manager): + client, server = message_manager + response = {'jsonrpc': '2.0', 'id': 0, 'result': {}} + client.write_message(response) + message = server.get_messages().next() + assert isinstance(message, JSONRPC20Response ) + assert response == message.data + + +def test_drop_bad_message(message_manager): + client, server = message_manager + response = {'jsonrpc': '2.0', 'id': 0, 'result': {}} + client.write_message(response) + server.close() + try: + server.get_messages().next() + except StopIteration: + pass + else: + assert False diff --git a/test/test_rpc_manager.py b/test/test_rpc_manager.py index dcee31b3..6371f75d 100644 --- a/test/test_rpc_manager.py +++ b/test/test_rpc_manager.py @@ -1,12 +1,123 @@ # Copyright 2018 Palantir Technologies, Inc. -from jsonrpc.jsonrpc2 import JSONRPC20Request +import pytest +import time +from StringIO import StringIO +from jsonrpc.jsonrpc2 import JSONRPC20Request, JSONRPC20Response +from pyls.message_manager import MessageManager +from pyls.rpc_manager import JSONRPCManager +from mock import Mock -# -# def test_fail_to_parse(rpc_manager): -# wfile = rpc_manager.rfile -# rfile = rpc_manager.wfile -# wfile.write(unicode("test")) -# wfile.write(unicode('"jsonrpc": "2.0", "method": "textDocument/didOpen", "params": {}')) -# messages = rpc_manager.get_messages() -# assert messages.next() == "test" +BASE_HANDLED_RESPONSE_CONTENT = 'handled' +BASE_HANDLED_RESPONSE = JSONRPC20Response(_id=1, result=BASE_HANDLED_RESPONSE_CONTENT) + +@pytest.fixture +def rpc_management(): + message_manager = MessageManager(StringIO(), StringIO()) + message_manager.get_messages = Mock(return_value=[JSONRPC20Request(_id=1, method='test', params={})]) + message_manager.write_message = Mock() + message_handler = Mock(return_value=BASE_HANDLED_RESPONSE_CONTENT) + rpc_manager = JSONRPCManager(message_manager, message_handler) + + yield rpc_manager, message_manager, message_handler, + + rpc_manager.exit() + + +def test_handle_request_sync(rpc_management): + rpc_manager, message_manager, message_handler = rpc_management + + rpc_manager.start() + message_manager.get_messages.assert_any_call() + message_manager.write_message.assert_called_once_with(BASE_HANDLED_RESPONSE.data) + message_handler.assert_called_once_with('test', {}) + + +def test_handle_request_async(rpc_management): + rpc_manager, message_manager, message_handler = rpc_management + response = JSONRPC20Response(_id=1, result="async") + + def wrapper(): + return 'async' + message_handler.configure_mock(return_value=wrapper) + + rpc_manager.start() + message_manager.get_messages.assert_any_call() + message_handler.assert_called_once_with('test', {}) + time.sleep(1) + message_manager.write_message.assert_called_once_with(response.data) + + +def test_handle_notification_sync(rpc_management): + rpc_manager, message_manager, message_handler = rpc_management + notification = JSONRPC20Request(method='notification', params={}, is_notification=True) + message_manager.get_messages.configure_mock(return_value=[notification]) + + rpc_manager.start() + message_manager.get_messages.assert_any_call() + message_handler.assert_called_once_with('notification', {}) + message_manager.write_message.assert_not_called() + + +def test_handle_notification_sync_empty(rpc_management): + rpc_manager, message_manager, message_handler = rpc_management + notification = JSONRPC20Request(method='notification', params=None, is_notification=True) + message_manager.get_messages.configure_mock(return_value=[notification]) + + rpc_manager.start() + message_manager.get_messages.assert_any_call() + message_handler.assert_called_once_with('notification', {}) + message_manager.write_message.assert_not_called() + + +def test_handle_notification_async(rpc_management): + rpc_manager, message_manager, message_handler = rpc_management + notification = JSONRPC20Request(method='notification', params={}, is_notification=True) + def wrapper(): + pass + message_handler.configure_mock(return_value=wrapper) + message_manager.get_messages.configure_mock(return_value=[notification]) + + rpc_manager.start() + message_manager.get_messages.assert_any_call() + message_handler.assert_called_once_with('notification', {}) + message_manager.write_message.assert_not_called() + + +def test_handle_notification_async_empty(rpc_management): + rpc_manager, message_manager, message_handler = rpc_management + notification = JSONRPC20Request(method='notification', params=None, is_notification=True) + def wrapper(): + pass + message_handler.configure_mock(return_value=wrapper) + message_manager.get_messages.configure_mock(return_value=[notification]) + + rpc_manager.start() + message_manager.get_messages.assert_any_call() + message_handler.assert_called_once_with('notification', {}) + message_manager.write_message.assert_not_called() + + +def test_send_request(rpc_management): + rpc_manager, message_manager, message_handler = rpc_management + + response_future = rpc_manager.call('request', {}) + message_manager.write_message.assert_called_once() + assert len(rpc_manager._sent_requests) == 1 + request_id = rpc_manager._sent_requests.keys()[0] + + response = JSONRPC20Response(_id=request_id, result={}) + message_manager.get_messages.configure_mock(return_value=[response]) + + rpc_manager.start() + message_manager.get_messages.assert_any_call() + assert not rpc_manager._sent_requests + assert response_future.result() == response.data + + +def test_send_notification(rpc_management): + rpc_manager, message_manager, message_handler = rpc_management + + rpc_manager.notify('notify', {}) + message_manager.write_message.assert_called_once_with(JSONRPC20Request(method='notify', params={}).data) + From b554a86faeb1c7d914d8790909173766ebaf150c Mon Sep 17 00:00:00 2001 From: forozco Date: Wed, 14 Feb 2018 22:14:26 +0000 Subject: [PATCH 04/19] address comments --- ...{message_manager.py => json_rpc_server.py} | 11 ++-- pyls/python_ls.py | 52 +++++++++---------- pyls/rpc_manager.py | 17 +++--- ...age_manager.py => test_json_rpc_server.py} | 24 ++++----- test/test_rpc_manager.py | 10 ++-- tox.ini | 1 + 6 files changed, 54 insertions(+), 61 deletions(-) rename pyls/{message_manager.py => json_rpc_server.py} (92%) rename test/{test_message_manager.py => test_json_rpc_server.py} (69%) diff --git a/pyls/message_manager.py b/pyls/json_rpc_server.py similarity index 92% rename from pyls/message_manager.py rename to pyls/json_rpc_server.py index 8d7735a8..ac27edb8 100644 --- a/pyls/message_manager.py +++ b/pyls/json_rpc_server.py @@ -13,7 +13,7 @@ log = logging.getLogger(__name__) -class MessageManager(object): +class JSONRPCServer(object): """ Read/Write JSON RPC messages """ def __init__(self, rfile, wfile): @@ -28,8 +28,7 @@ def close(self): self.rfile.close() def get_messages(self): - """ - Generator that produces well structured JSON RPC message. + """Generator that produces well structured JSON RPC message. Returns: message: received message @@ -41,8 +40,7 @@ def get_messages(self): request_str = self._read_message() if request_str is None: - # log.error("failed to read message") - continue + break if isinstance(request_str, bytes): request_str = request_str.decode("utf-8") @@ -83,7 +81,8 @@ def write_message(self, message): def _read_message(self): """Reads the contents of a message - :return body of message if parsable else None + Returns: + body of message if parsable else None """ line = self.rfile.readline() diff --git a/pyls/python_ls.py b/pyls/python_ls.py index 2862d2c4..80d1ffd8 100644 --- a/pyls/python_ls.py +++ b/pyls/python_ls.py @@ -1,11 +1,11 @@ # Copyright 2017 Palantir Technologies, Inc. import logging -import re import socketserver +import re from . import lsp, _utils, uris from .config import config -from .message_manager import MessageManager +from .json_rpc_server import JSONRPCServer from .rpc_manager import JSONRPCManager from .workspace import Workspace @@ -32,8 +32,8 @@ def handle(self): def start_tcp_lang_server(bind_addr, port, handler_class): - if not issubclass(handler_class, ): - raise ValueError('Handler class must be a subclass of JSONRPCServer') + if not isinstance(handler_class, PythonLanguageServer): + raise ValueError('Handler class must be an instance of PythonLanguageServer') # Construct a custom wrapper class around the user's handler_class wrapper_class = type( @@ -53,36 +53,41 @@ def start_tcp_lang_server(bind_addr, port, handler_class): def start_io_lang_server(rfile, wfile, handler_class): if not issubclass(handler_class, PythonLanguageServer): - raise ValueError('Handler class must be a subclass of JSONRPCServer') + raise ValueError('Handler class must be an instance of PythonLanguageServer') log.info('Starting %s IO language server', handler_class.__name__) server = handler_class(rfile, wfile) server.start() class PythonLanguageServer(object): + """ Implementation of the Microsoft VSCode Language Server Protocol + https://github.com/Microsoft/language-server-protocol/blob/master/versions/protocol-1-x.md + """ + # pylint: disable=too-many-public-methods,redefined-builtin def __init__(self, rx, tx): - self.rpc_manager = JSONRPCManager(MessageManager(rx, tx), self.handle_request) + self.rpc_manager = JSONRPCManager(JSONRPCServer(rx, tx), self.handle_request) self.workspace = None self.config = None self._dispatchers = [] def start(self): """Entry point for the server""" - self.rpc_manager.consume_requests() + self.rpc_manager.start() def handle_request(self, method, params): - """Provides calllables to handle message requests + """Provides callables to handle requests or responses to those reqeuests Args: - method (str): + method (str): name of the message + params (dict): body of the message Returns: Callable if method is to be handled Raises: - KeyError: + KeyError: Handler for method is not found """ method_call = 'm_{}'.format(_method_to_string(method)) @@ -93,7 +98,7 @@ def handle_request(self, method, params): if method_call in dispatcher: return dispatcher[method_call](**params) - raise KeyError + raise KeyError('Handler for method {} not found'.format(method)) def _hook(self, hook_name, doc_uri=None, **kwargs): """Calls hook_name and returns a list of results from all registered handlers""" @@ -130,26 +135,20 @@ def capabilities(self): log.info('Server capabilities: %s', server_capabilities) return server_capabilities - def m_initialize(self, **kwargs): - log.debug('Language server initialized with %s', kwargs) - if 'rootUri' in kwargs: - root_uri = kwargs['rootUri'] - elif 'rootPath' in kwargs: - root_path = kwargs['rootPath'] - root_uri = uris.from_fs_path(root_path) - else: - root_uri = '' - init_opts = kwargs.get('initializationOptions') - - self.workspace = Workspace(root_uri, rpc_manager=self.rpc_manager) - self.config = config.Config(root_uri, init_opts) + def m_initialize(self, processId=None, rootUri=None, rootPath=None, initializationOptions={}): # pylint: disable=dangerous-default-value + log.debug('Language server initialized with %s %s %s %s', processId, rootUri, rootPath, initializationOptions) + if rootUri is None: + rootUri = uris.from_fs_path(rootPath) if rootPath is not None else '' + + self.workspace = Workspace(rootUri, rpc_manager=self.rpc_manager) + self.config = config.Config(rootUri, initializationOptions) self._dispatchers = self._hook('pyls_dispatchers') self._hook('pyls_initialize') # Get our capabilities return {'capabilities': self.capabilities()} - def m___cancel_request(self, **kwargs): + def m__cancel_request(self, **kwargs): def handler(): self.rpc_manager.cancel(kwargs['id']) return handler @@ -210,9 +209,6 @@ def rename(self, doc_uri, position, new_name): def signature_help(self, doc_uri, position): return self._hook('pyls_signature_help', doc_uri, position=position) - def m__cancel_request(self, **kwargs): - self.rpc_manager.cancel(kwargs['id']) - def m_text_document__did_close(self, textDocument=None, **_kwargs): self.workspace.rm_document(textDocument['uri']) diff --git a/pyls/rpc_manager.py b/pyls/rpc_manager.py index 6852c859..0adecfec 100644 --- a/pyls/rpc_manager.py +++ b/pyls/rpc_manager.py @@ -17,9 +17,6 @@ class JSONRPCManager(object): - """ Implementation of the Microsoft VSCode Language Server Protocol - https://github.com/Microsoft/language-server-protocol/blob/master/versions/protocol-1-x.md - """ def __init__(self, message_manager, message_handler): self._message_manager = message_manager @@ -121,34 +118,32 @@ def _handle_request(self, request): elif callable(maybe_handler): self._handle_async_request(request, maybe_handler) elif not request.is_notification: - log.debug("sync request %s", request._id) + log.debug('Sync request %s', request._id) response = _make_response(request, result=maybe_handler) self._message_manager.write_message(response.data) def _handle_async_request(self, request, handler): - log.debug("async request %s", request._id) + log.debug('Async request %s', request._id) future = self._executor_service.submit(handler) if request.is_notification: return - self._received_requests[request._id] = future - def did_finish_callback(completed_future): + del self._received_requests[request._id] if completed_future.cancelled(): log.debug('Cleared cancelled request %d', request._id) - del self._received_requests[request._id] return error, trace = completed_future.exception_info() - del self._received_requests[request._id] if error is not None: - log.error("Failed to handle request %s with error %s %s", request._id, error, trace) + log.error('Failed to handle request %s with error %s %s', request._id, error, trace) # TODO(forozco): add more descriptive error - response = _make_response(request, errror=JSONRPCInternalError()._data) + response = _make_response(request, error=JSONRPCInternalError()._data) else: response = _make_response(request, result=completed_future.result()) self._message_manager.write_message(response.data) + self._received_requests[request._id] = future future.add_done_callback(did_finish_callback) def _handle_response(self, response): diff --git a/test/test_message_manager.py b/test/test_json_rpc_server.py similarity index 69% rename from test/test_message_manager.py rename to test/test_json_rpc_server.py index 9d0e4bad..234a7132 100644 --- a/test/test_message_manager.py +++ b/test/test_json_rpc_server.py @@ -2,15 +2,15 @@ import pytest import os from jsonrpc.jsonrpc2 import JSONRPC20Request, JSONRPC20Response -from pyls.message_manager import MessageManager +from pyls.json_rpc_server import JSONRPCServer @pytest.fixture -def message_manager(tmpdir): +def json_rpc_server(tmpdir): manager_rx, tester_tx = os.pipe() tester_rx, manager_tx = os.pipe() - client = MessageManager(os.fdopen(manager_rx, 'rb'), os.fdopen(manager_tx, 'wb')) - server = MessageManager(os.fdopen(tester_rx, 'rb'), os.fdopen(tester_tx, 'wb')) + client = JSONRPCServer(os.fdopen(manager_rx, 'rb'), os.fdopen(manager_tx, 'wb')) + server = JSONRPCServer(os.fdopen(tester_rx, 'rb'), os.fdopen(tester_tx, 'wb')) yield client, server @@ -18,8 +18,8 @@ def message_manager(tmpdir): server.close() -def test_receive_request(message_manager): - client, server = message_manager +def test_receive_request(json_rpc_server): + client, server = json_rpc_server request = {'jsonrpc': '2.0', 'id': 0, 'method': 'initialize', 'params': {}} client.write_message(request) message = server.get_messages().next() @@ -28,8 +28,8 @@ def test_receive_request(message_manager): assert not message.is_notification -def test_receive_notification(message_manager): - client, server = message_manager +def test_receive_notification(json_rpc_server): + client, server = json_rpc_server notification = {'jsonrpc': '2.0', 'method': 'notify', 'params': {}} client.write_message(notification) message = server.get_messages().next() @@ -38,8 +38,8 @@ def test_receive_notification(message_manager): assert message.is_notification -def test_receive_response(message_manager): - client, server = message_manager +def test_receive_response(json_rpc_server): + client, server = json_rpc_server response = {'jsonrpc': '2.0', 'id': 0, 'result': {}} client.write_message(response) message = server.get_messages().next() @@ -47,8 +47,8 @@ def test_receive_response(message_manager): assert response == message.data -def test_drop_bad_message(message_manager): - client, server = message_manager +def test_drop_bad_message(json_rpc_server): + client, server = json_rpc_server response = {'jsonrpc': '2.0', 'id': 0, 'result': {}} client.write_message(response) server.close() diff --git a/test/test_rpc_manager.py b/test/test_rpc_manager.py index 6371f75d..e3da3203 100644 --- a/test/test_rpc_manager.py +++ b/test/test_rpc_manager.py @@ -3,7 +3,7 @@ import time from StringIO import StringIO from jsonrpc.jsonrpc2 import JSONRPC20Request, JSONRPC20Response -from pyls.message_manager import MessageManager +from pyls.json_rpc_server import JSONRPCServer from pyls.rpc_manager import JSONRPCManager from mock import Mock @@ -13,7 +13,7 @@ @pytest.fixture def rpc_management(): - message_manager = MessageManager(StringIO(), StringIO()) + message_manager = JSONRPCServer(StringIO(), StringIO()) message_manager.get_messages = Mock(return_value=[JSONRPC20Request(_id=1, method='test', params={})]) message_manager.write_message = Mock() message_handler = Mock(return_value=BASE_HANDLED_RESPONSE_CONTENT) @@ -44,8 +44,10 @@ def wrapper(): rpc_manager.start() message_manager.get_messages.assert_any_call() message_handler.assert_called_once_with('test', {}) - time.sleep(1) - message_manager.write_message.assert_called_once_with(response.data) + + if rpc_manager._sent_requests: + rpc_manager._sent_requests.values()[0].result(timeout=1) + message_manager.write_message.assert_called_once_with(response.data) def test_handle_notification_sync(rpc_management): diff --git a/tox.ini b/tox.ini index 28a64c69..3fcd443b 100644 --- a/tox.ini +++ b/tox.ini @@ -22,6 +22,7 @@ commands = deps = pytest coverage + mock pytest-cov pylint From bef8d3fac2837718ba65665bd69f1a8815867477 Mon Sep 17 00:00:00 2001 From: forozco Date: Thu, 15 Feb 2018 14:18:38 +0000 Subject: [PATCH 05/19] lint --- pyls/python_ls.py | 4 +-- test/fixtures.py | 55 +++++++++++++++++++++++++++++--- test/plugins/test_completion.py | 4 +-- test/plugins/test_definitions.py | 8 ++--- test/plugins/test_format.py | 6 ++-- test/plugins/test_references.py | 2 +- test/plugins/test_signature.py | 2 +- test/test_document.py | 15 +-------- test/test_json_rpc_server.py | 18 +---------- test/test_language_server.py | 12 +++---- test/test_rpc_manager.py | 30 +++-------------- test/test_uris.py | 2 +- tox.ini | 6 ++-- 13 files changed, 80 insertions(+), 84 deletions(-) diff --git a/pyls/python_ls.py b/pyls/python_ls.py index 80d1ffd8..8d12ba0f 100644 --- a/pyls/python_ls.py +++ b/pyls/python_ls.py @@ -135,13 +135,13 @@ def capabilities(self): log.info('Server capabilities: %s', server_capabilities) return server_capabilities - def m_initialize(self, processId=None, rootUri=None, rootPath=None, initializationOptions={}): # pylint: disable=dangerous-default-value + def m_initialize(self, processId=None, rootUri=None, rootPath=None, initializationOptions=None): log.debug('Language server initialized with %s %s %s %s', processId, rootUri, rootPath, initializationOptions) if rootUri is None: rootUri = uris.from_fs_path(rootPath) if rootPath is not None else '' self.workspace = Workspace(rootUri, rpc_manager=self.rpc_manager) - self.config = config.Config(rootUri, initializationOptions) + self.config = config.Config(rootUri, initializationOptions or {}) self._dispatchers = self._hook('pyls_dispatchers') self._hook('pyls_initialize') diff --git a/test/fixtures.py b/test/fixtures.py index a51c2ca4..63d9fb95 100644 --- a/test/fixtures.py +++ b/test/fixtures.py @@ -1,11 +1,26 @@ # Copyright 2017 Palantir Technologies, Inc. -import pytest import os +from StringIO import StringIO +from mock import Mock +import pytest +from jsonrpc.jsonrpc2 import JSONRPC20Response, JSONRPC20Request + from pyls import uris from pyls.config.config import Config from pyls.python_ls import PythonLanguageServer -from pyls.workspace import Workspace -from StringIO import StringIO +from pyls.rpc_manager import JSONRPCManager +from pyls.workspace import Workspace, Document +from pyls.json_rpc_server import JSONRPCServer + +BASE_HANDLED_RESPONSE_CONTENT = 'handled' +BASE_HANDLED_RESPONSE = JSONRPC20Response(_id=1, result=BASE_HANDLED_RESPONSE_CONTENT) + +DOC_URI = uris.from_fs_path(__file__) +DOC = """import sys + +def main(): + print sys.stdin.read() +""" @pytest.fixture @@ -31,6 +46,38 @@ def workspace(tmpdir): @pytest.fixture -def config(workspace): +def rpc_management(): + message_manager = JSONRPCServer(StringIO(), StringIO()) + message_manager.get_messages = Mock(return_value=[JSONRPC20Request(_id=1, method='test', params={})]) + message_manager.write_message = Mock() + message_handler = Mock(return_value=BASE_HANDLED_RESPONSE_CONTENT) + rpc_manager = JSONRPCManager(message_manager, message_handler) + + yield rpc_manager, message_manager, message_handler, + + rpc_manager.exit() + + +@pytest.fixture +def json_rpc_server(): + manager_rx, tester_tx = os.pipe() + tester_rx, manager_tx = os.pipe() + + client = JSONRPCServer(os.fdopen(manager_rx, 'rb'), os.fdopen(manager_tx, 'wb')) + server = JSONRPCServer(os.fdopen(tester_rx, 'rb'), os.fdopen(tester_tx, 'wb')) + + yield client, server + + client.close() + server.close() + + +@pytest.fixture +def config(workspace): # pylint: disable=redefined-outer-name """Return a config object.""" return Config(workspace.root_uri, {}) + + +@pytest.fixture +def doc(): + return Document(DOC_URI, DOC) diff --git a/test/plugins/test_completion.py b/test/plugins/test_completion.py index eb716e96..2e5c764b 100644 --- a/test/plugins/test_completion.py +++ b/test/plugins/test_completion.py @@ -37,7 +37,7 @@ def test_jedi_completion(): doc = Document(DOC_URI, DOC) items = pyls_jedi_completions(doc, com_position) - assert len(items) > 0 + assert items assert items[0]['label'] == 'isabs(s)' # Test we don't throw with big character @@ -52,7 +52,7 @@ def test_rope_completion(): doc = Document(DOC_URI, DOC, rope=rope) items = pyls_rope_completions(doc, com_position) - assert len(items) > 0 + assert items assert items[0]['label'] == 'isabs' diff --git a/test/plugins/test_definitions.py b/test/plugins/test_definitions.py index 4c937ad2..c360ac44 100644 --- a/test/plugins/test_definitions.py +++ b/test/plugins/test_definitions.py @@ -24,13 +24,13 @@ def test_definitions(): cursor_pos = {'line': 3, 'character': 6} # The definition of 'a' - range = { + def_range = { 'start': {'line': 0, 'character': 4}, 'end': {'line': 0, 'character': 5} } doc = Document(DOC_URI, DOC) - assert [{'uri': DOC_URI, 'range': range}] == pyls_definitions(doc, cursor_pos) + assert [{'uri': DOC_URI, 'range': def_range}] == pyls_definitions(doc, cursor_pos) def test_builtin_definition(): @@ -47,10 +47,10 @@ def test_assignment(): cursor_pos = {'line': 11, 'character': 19} # The assignment of 'self.members' - range = { + def_range = { 'start': {'line': 8, 'character': 13}, 'end': {'line': 8, 'character': 20} } doc = Document(DOC_URI, DOC) - assert [{'uri': DOC_URI, 'range': range}] == pyls_definitions(doc, cursor_pos) + assert [{'uri': DOC_URI, 'range': def_range}] == pyls_definitions(doc, cursor_pos) diff --git a/test/plugins/test_format.py b/test/plugins/test_format.py index d944b512..b73953a2 100644 --- a/test/plugins/test_format.py +++ b/test/plugins/test_format.py @@ -30,11 +30,11 @@ def test_format(): def test_range_format(): doc = Document(DOC_URI, DOC) - range = { + def_range = { 'start': {'line': 0, 'character': 0}, 'end': {'line': 4, 'character': 10} } - res = pyls_format_range(doc, range) + res = pyls_format_range(doc, def_range) assert len(res) == 1 @@ -44,4 +44,4 @@ def test_range_format(): def test_no_change(): doc = Document(DOC_URI, GOOD_DOC) - assert len(pyls_format_document(doc)) == 0 + assert not pyls_format_document(doc) diff --git a/test/plugins/test_references.py b/test/plugins/test_references.py index afe9247e..c8cc2742 100644 --- a/test/plugins/test_references.py +++ b/test/plugins/test_references.py @@ -33,7 +33,7 @@ def create_file(name, content): return workspace -def test_references(tmp_workspace): +def test_references(tmp_workspace): # pylint: disable=redefined-outer-name # Over 'Test1' in class Test1(): position = {'line': 0, 'character': 8} DOC1_URI = uris.from_fs_path(os.path.join(tmp_workspace.root_path, DOC1_NAME)) diff --git a/test/plugins/test_signature.py b/test/plugins/test_signature.py index e0a5c74f..34cb77c5 100644 --- a/test/plugins/test_signature.py +++ b/test/plugins/test_signature.py @@ -25,7 +25,7 @@ def test_no_signature(): doc = Document(DOC_URI, DOC) sigs = signature.pyls_signature_help(doc, sig_position)['signatures'] - assert len(sigs) == 0 + assert not sigs def test_signature(): diff --git a/test/test_document.py b/test/test_document.py index 5c6439c8..8565ca81 100644 --- a/test/test_document.py +++ b/test/test_document.py @@ -1,21 +1,8 @@ # Copyright 2017 Palantir Technologies, Inc. import sys -import pytest -from pyls import uris +from test.fixtures import DOC_URI, DOC from pyls.workspace import Document -DOC_URI = uris.from_fs_path(__file__) -DOC = """import sys - -def main(): - print sys.stdin.read() -""" - - -@pytest.fixture -def doc(): - return Document(DOC_URI, DOC) - def test_document_props(doc): assert doc.uri == DOC_URI diff --git a/test/test_json_rpc_server.py b/test/test_json_rpc_server.py index 234a7132..f284aa5c 100644 --- a/test/test_json_rpc_server.py +++ b/test/test_json_rpc_server.py @@ -1,21 +1,5 @@ # Copyright 2018 Palantir Technologies, Inc. -import pytest -import os from jsonrpc.jsonrpc2 import JSONRPC20Request, JSONRPC20Response -from pyls.json_rpc_server import JSONRPCServer - -@pytest.fixture -def json_rpc_server(tmpdir): - manager_rx, tester_tx = os.pipe() - tester_rx, manager_tx = os.pipe() - - client = JSONRPCServer(os.fdopen(manager_rx, 'rb'), os.fdopen(manager_tx, 'wb')) - server = JSONRPCServer(os.fdopen(tester_rx, 'rb'), os.fdopen(tester_tx, 'wb')) - - yield client, server - - client.close() - server.close() def test_receive_request(json_rpc_server): @@ -43,7 +27,7 @@ def test_receive_response(json_rpc_server): response = {'jsonrpc': '2.0', 'id': 0, 'result': {}} client.write_message(response) message = server.get_messages().next() - assert isinstance(message, JSONRPC20Response ) + assert isinstance(message, JSONRPC20Response) assert response == message.data diff --git a/test/test_language_server.py b/test/test_language_server.py index ed559065..a8c59acd 100644 --- a/test/test_language_server.py +++ b/test/test_language_server.py @@ -1,18 +1,17 @@ # Copyright 2017 Palantir Technologies, Inc. import os from threading import Thread - -import jsonrpc from jsonrpc.exceptions import JSONRPCMethodNotFound import pytest - -from pyls.python_ls import start_io_lang_server ,PythonLanguageServer +from pyls.python_ls import start_io_lang_server, PythonLanguageServer CALL_TIMEOUT = 2 + def start_client(client): client.start() + @pytest.fixture def client_server(): """ A fixture to setup a client/server """ @@ -40,7 +39,7 @@ def client_server(): client.rpc_manager.notify('exit') -def test_initialize(client_server): +def test_initialize(client_server): # pylint: disable=redefined-outer-name response = client_server.rpc_manager.call('initialize', { 'processId': 1234, 'rootPath': os.path.dirname(__file__), @@ -49,7 +48,6 @@ def test_initialize(client_server): assert 'capabilities' in response['result'] -def test_missing_message(client_server): +def test_missing_message(client_server): # pylint: disable=redefined-outer-name response = client_server.rpc_manager.call('unknown_method').result(timeout=CALL_TIMEOUT) assert response['error']['code'] == JSONRPCMethodNotFound.CODE - diff --git a/test/test_rpc_manager.py b/test/test_rpc_manager.py index e3da3203..3b7c312b 100644 --- a/test/test_rpc_manager.py +++ b/test/test_rpc_manager.py @@ -1,27 +1,6 @@ # Copyright 2018 Palantir Technologies, Inc. -import pytest -import time -from StringIO import StringIO +from test.fixtures import BASE_HANDLED_RESPONSE from jsonrpc.jsonrpc2 import JSONRPC20Request, JSONRPC20Response -from pyls.json_rpc_server import JSONRPCServer -from pyls.rpc_manager import JSONRPCManager -from mock import Mock - - -BASE_HANDLED_RESPONSE_CONTENT = 'handled' -BASE_HANDLED_RESPONSE = JSONRPC20Response(_id=1, result=BASE_HANDLED_RESPONSE_CONTENT) - -@pytest.fixture -def rpc_management(): - message_manager = JSONRPCServer(StringIO(), StringIO()) - message_manager.get_messages = Mock(return_value=[JSONRPC20Request(_id=1, method='test', params={})]) - message_manager.write_message = Mock() - message_handler = Mock(return_value=BASE_HANDLED_RESPONSE_CONTENT) - rpc_manager = JSONRPCManager(message_manager, message_handler) - - yield rpc_manager, message_manager, message_handler, - - rpc_manager.exit() def test_handle_request_sync(rpc_management): @@ -75,6 +54,7 @@ def test_handle_notification_sync_empty(rpc_management): def test_handle_notification_async(rpc_management): rpc_manager, message_manager, message_handler = rpc_management notification = JSONRPC20Request(method='notification', params={}, is_notification=True) + def wrapper(): pass message_handler.configure_mock(return_value=wrapper) @@ -89,6 +69,7 @@ def wrapper(): def test_handle_notification_async_empty(rpc_management): rpc_manager, message_manager, message_handler = rpc_management notification = JSONRPC20Request(method='notification', params=None, is_notification=True) + def wrapper(): pass message_handler.configure_mock(return_value=wrapper) @@ -101,7 +82,7 @@ def wrapper(): def test_send_request(rpc_management): - rpc_manager, message_manager, message_handler = rpc_management + rpc_manager, message_manager, _ = rpc_management response_future = rpc_manager.call('request', {}) message_manager.write_message.assert_called_once() @@ -118,8 +99,7 @@ def test_send_request(rpc_management): def test_send_notification(rpc_management): - rpc_manager, message_manager, message_handler = rpc_management + rpc_manager, message_manager, _ = rpc_management rpc_manager.notify('notify', {}) message_manager.write_message.assert_called_once_with(JSONRPC20Request(method='notify', params={}).data) - diff --git a/test/test_uris.py b/test/test_uris.py index 37dae33e..d4e177e6 100644 --- a/test/test_uris.py +++ b/test/test_uris.py @@ -1,7 +1,7 @@ # Copyright 2017 Palantir Technologies, Inc. +from test import unix_only, windows_only import pytest from pyls import uris -from test import unix_only, windows_only @unix_only diff --git a/tox.ini b/tox.ini index 3fcd443b..3cd16bdc 100644 --- a/tox.ini +++ b/tox.ini @@ -28,6 +28,6 @@ deps = [testenv:lint] commands = - pylint pyls - pycodestyle pyls - pyflakes pyls + pylint pyls test + pycodestyle pyls test --exclude=test/plugins/.ropeproject,test/.ropeproject + pyflakes pyls test From 509a9c5acac8cf604a8c092c06c048ab49e09d71 Mon Sep 17 00:00:00 2001 From: forozco Date: Thu, 15 Feb 2018 14:24:42 +0000 Subject: [PATCH 06/19] fix stringIO --- test/fixtures.py | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/test/fixtures.py b/test/fixtures.py index 63d9fb95..991f4e4e 100644 --- a/test/fixtures.py +++ b/test/fixtures.py @@ -1,6 +1,6 @@ # Copyright 2017 Palantir Technologies, Inc. import os -from StringIO import StringIO +import sys from mock import Mock import pytest from jsonrpc.jsonrpc2 import JSONRPC20Response, JSONRPC20Request @@ -12,6 +12,11 @@ from pyls.workspace import Workspace, Document from pyls.json_rpc_server import JSONRPCServer +if sys.version_info[0] < 3: + from StringIO import StringIO +else: + from io import StringIO + BASE_HANDLED_RESPONSE_CONTENT = 'handled' BASE_HANDLED_RESPONSE = JSONRPC20Response(_id=1, result=BASE_HANDLED_RESPONSE_CONTENT) @@ -26,9 +31,7 @@ def main(): @pytest.fixture def pyls(tmpdir): """ Return an initialized python LS """ - rfile = StringIO() - wfile = StringIO() - ls = PythonLanguageServer(rfile, wfile) + ls = PythonLanguageServer(StringIO, StringIO) ls.m_initialize( processId=1, From b0c21abaffec2de2a8e9516018579469d6dacc4f Mon Sep 17 00:00:00 2001 From: forozco Date: Thu, 15 Feb 2018 14:29:00 +0000 Subject: [PATCH 07/19] py3 next --- test/test_json_rpc_server.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/test/test_json_rpc_server.py b/test/test_json_rpc_server.py index f284aa5c..608a909d 100644 --- a/test/test_json_rpc_server.py +++ b/test/test_json_rpc_server.py @@ -6,7 +6,7 @@ def test_receive_request(json_rpc_server): client, server = json_rpc_server request = {'jsonrpc': '2.0', 'id': 0, 'method': 'initialize', 'params': {}} client.write_message(request) - message = server.get_messages().next() + message = next(server.get_messages()) assert isinstance(message, JSONRPC20Request) assert request == message.data assert not message.is_notification @@ -16,7 +16,7 @@ def test_receive_notification(json_rpc_server): client, server = json_rpc_server notification = {'jsonrpc': '2.0', 'method': 'notify', 'params': {}} client.write_message(notification) - message = server.get_messages().next() + message = next(server.get_messages()) assert isinstance(message, JSONRPC20Request) assert notification == message.data assert message.is_notification @@ -26,7 +26,7 @@ def test_receive_response(json_rpc_server): client, server = json_rpc_server response = {'jsonrpc': '2.0', 'id': 0, 'result': {}} client.write_message(response) - message = server.get_messages().next() + message = next(server.get_messages()) assert isinstance(message, JSONRPC20Response) assert response == message.data @@ -37,7 +37,7 @@ def test_drop_bad_message(json_rpc_server): client.write_message(response) server.close() try: - server.get_messages().next() + next(server.get_messages()) except StopIteration: pass else: From 7859c5111cb7a776cdbe7968278fd46cc3069b1b Mon Sep 17 00:00:00 2001 From: forozco Date: Thu, 15 Feb 2018 14:57:47 +0000 Subject: [PATCH 08/19] more py3 --- pyls/rpc_manager.py | 6 +++--- test/test_rpc_manager.py | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/pyls/rpc_manager.py b/pyls/rpc_manager.py index 0adecfec..3b5cc615 100644 --- a/pyls/rpc_manager.py +++ b/pyls/rpc_manager.py @@ -24,7 +24,7 @@ def __init__(self, message_manager, message_handler): self._shutdown = False self._sent_requests = {} self._received_requests = {} - self._executor_service = ThreadPoolExecutor() + self._executor_service = ThreadPoolExecutor(max_workers=5) def start(self): """Start reading JSONRPC messages off of rx""" @@ -135,9 +135,9 @@ def did_finish_callback(completed_future): log.debug('Cleared cancelled request %d', request._id) return - error, trace = completed_future.exception_info() + error = completed_future.exception() if error is not None: - log.error('Failed to handle request %s with error %s %s', request._id, error, trace) + log.error('Failed to handle request %s with error %s', request._id, error) # TODO(forozco): add more descriptive error response = _make_response(request, error=JSONRPCInternalError()._data) else: diff --git a/test/test_rpc_manager.py b/test/test_rpc_manager.py index 3b7c312b..c06d6d29 100644 --- a/test/test_rpc_manager.py +++ b/test/test_rpc_manager.py @@ -87,7 +87,7 @@ def test_send_request(rpc_management): response_future = rpc_manager.call('request', {}) message_manager.write_message.assert_called_once() assert len(rpc_manager._sent_requests) == 1 - request_id = rpc_manager._sent_requests.keys()[0] + request_id = list(rpc_manager._sent_requests.keys())[0] response = JSONRPC20Response(_id=request_id, result={}) message_manager.get_messages.configure_mock(return_value=[response]) From 9b4592d288ab2e96414ebe000593c1e0e1c5203b Mon Sep 17 00:00:00 2001 From: forozco Date: Fri, 16 Feb 2018 11:37:26 +0000 Subject: [PATCH 09/19] cleanup --- pyls/python_ls.py | 2 +- pyls/rpc_manager.py | 4 ++-- vscode-client/src/extension.ts | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/pyls/python_ls.py b/pyls/python_ls.py index 8d12ba0f..f43f4a8a 100644 --- a/pyls/python_ls.py +++ b/pyls/python_ls.py @@ -135,7 +135,7 @@ def capabilities(self): log.info('Server capabilities: %s', server_capabilities) return server_capabilities - def m_initialize(self, processId=None, rootUri=None, rootPath=None, initializationOptions=None): + def m_initialize(self, processId=None, rootUri=None, rootPath=None, initializationOptions=None, **kwargs): log.debug('Language server initialized with %s %s %s %s', processId, rootUri, rootPath, initializationOptions) if rootUri is None: rootUri = uris.from_fs_path(rootPath) if rootPath is not None else '' diff --git a/pyls/rpc_manager.py b/pyls/rpc_manager.py index 3b5cc615..08cbc0c6 100644 --- a/pyls/rpc_manager.py +++ b/pyls/rpc_manager.py @@ -1,6 +1,6 @@ # Copyright 2017 Palantir Technologies, Inc. import logging -from uuid import uuid1 +from uuid import uuid4 from concurrent.futures import ThreadPoolExecutor, Future from jsonrpc.base import JSONRPCBaseResponse @@ -51,7 +51,7 @@ def call(self, method, params=None): """ log.debug('Calling %s %s', method, params) - request = JSONRPC20Request(_id=uuid1().int, method=method, params=params) + request = JSONRPC20Request(_id=uuid4().int, method=method, params=params) request_future = Future() self._sent_requests[request._id] = request_future self._message_manager.write_message(request.data) diff --git a/vscode-client/src/extension.ts b/vscode-client/src/extension.ts index 005299d1..3eea77e3 100644 --- a/vscode-client/src/extension.ts +++ b/vscode-client/src/extension.ts @@ -44,7 +44,7 @@ function startLangServerTCP(addr: number, documentSelector: string[]): Disposabl export function activate(context: ExtensionContext) { context.subscriptions.push(startLangServer("pyls", ["-vv"], ["python"])); - // For TCP + // For TCP server needs to be started seperately // context.subscriptions.push(startLangServerTCP(2087, ["python"])); } From 4f622c215d75e8837b56f526700a7749646b0c54 Mon Sep 17 00:00:00 2001 From: forozco Date: Fri, 16 Feb 2018 13:54:13 +0000 Subject: [PATCH 10/19] add support for batch requests --- pyls/json_rpc_server.py | 40 +++++++++++++++++++++++------ pyls/python_ls.py | 2 +- pyls/rpc_manager.py | 12 ++++----- test/test_json_rpc_server.py | 49 ++++++++++++++++++++++++++++++------ test/test_rpc_manager.py | 9 ++++--- 5 files changed, 86 insertions(+), 26 deletions(-) diff --git a/pyls/json_rpc_server.py b/pyls/json_rpc_server.py index ac27edb8..e28e7be8 100644 --- a/pyls/json_rpc_server.py +++ b/pyls/json_rpc_server.py @@ -3,7 +3,7 @@ import logging import threading -from jsonrpc.jsonrpc2 import JSONRPC20Response +from jsonrpc.jsonrpc2 import JSONRPC20Response, JSONRPC20BatchRequest, JSONRPC20BatchResponse from jsonrpc.jsonrpc import JSONRPCRequest from jsonrpc.exceptions import ( JSONRPCInvalidRequestException, @@ -17,7 +17,7 @@ class JSONRPCServer(object): """ Read/Write JSON RPC messages """ def __init__(self, rfile, wfile): - self.batch_messages = {} + self.pending_request = {} self.rfile = rfile self.wfile = wfile self.write_lock = threading.Lock() @@ -47,28 +47,40 @@ def get_messages(self): try: try: message_blob = json.loads(request_str) - message = JSONRPCRequest.from_data(message_blob) + request = JSONRPCRequest.from_data(message_blob) + if isinstance(request, JSONRPC20BatchRequest): + self._add_batch_request(request) + messages = request + else: + messages = [request] except JSONRPCInvalidRequestException: # work around where JSONRPC20Reponse expects _id key message_blob['_id'] = message_blob['id'] - message = JSONRPC20Response(**message_blob) + # we do not send out batch requests so no need to support batch responses + messages = [JSONRPC20Response(**message_blob)] except (KeyError, ValueError): log.error("Could not parse message %s", request_str) continue - yield message + for message in messages: + yield message def write_message(self, message): """ Write message to out file descriptor Args: - message (dict): body of the message to send + message (JSONRPCRequest, JSONRPCResponse): body of the message to send """ with self.write_lock: if self.wfile.closed: return + elif isinstance(message, JSONRPC20Response) and message._id in self.pending_request: + batch_response = self.pending_request[message._id](message) + if batch_response is not None: + message = batch_response + log.debug("Sending %s", message) - body = json.dumps(message, separators=(",", ":")) + body = message.json content_length = len(body) response = ( "Content-Length: {}\r\n" @@ -101,6 +113,20 @@ def _read_message(self): # Grab the body return self.rfile.read(content_length) + def _add_batch_request(self, requests): + pending_requests = [request for request in requests if not request.is_notification] + if not pending_requests: + return + + batch_request = {'pending': len(pending_requests), 'resolved': []} + for request in pending_requests: + def cleanup_message(response): + batch_request['pending'] -= 1 + batch_request['resolved'].append(response) + del self.pending_request[request._id] + return JSONRPC20BatchResponse(batch_request['resolved']) if batch_request['pending'] == 0 else None + self.pending_request[request._id] = cleanup_message + def _content_length(line): """Extract the content length from an input line.""" diff --git a/pyls/python_ls.py b/pyls/python_ls.py index f43f4a8a..f2157214 100644 --- a/pyls/python_ls.py +++ b/pyls/python_ls.py @@ -135,7 +135,7 @@ def capabilities(self): log.info('Server capabilities: %s', server_capabilities) return server_capabilities - def m_initialize(self, processId=None, rootUri=None, rootPath=None, initializationOptions=None, **kwargs): + def m_initialize(self, processId=None, rootUri=None, rootPath=None, initializationOptions=None, **_kwargs): log.debug('Language server initialized with %s %s %s %s', processId, rootUri, rootPath, initializationOptions) if rootUri is None: rootUri = uris.from_fs_path(rootPath) if rootPath is not None else '' diff --git a/pyls/rpc_manager.py b/pyls/rpc_manager.py index 08cbc0c6..8734ed77 100644 --- a/pyls/rpc_manager.py +++ b/pyls/rpc_manager.py @@ -54,7 +54,7 @@ def call(self, method, params=None): request = JSONRPC20Request(_id=uuid4().int, method=method, params=params) request_future = Future() self._sent_requests[request._id] = request_future - self._message_manager.write_message(request.data) + self._message_manager.write_message(request) return request_future def notify(self, method, params=None): @@ -66,7 +66,7 @@ def notify(self, method, params=None): """ log.debug('Notify %s %s', method, params) notification = JSONRPC20Request(method=method, params=params) - self._message_manager.write_message(notification.data) + self._message_manager.write_message(notification) def cancel(self, request_id): """Cancel pending request handler. @@ -109,8 +109,7 @@ def _handle_request(self, request): maybe_handler = self._message_handler(request.method, params) except KeyError: log.debug("No handler found for %s", request.method) - self._message_manager.write_message( - JSONRPC20Response(_id=request._id, error=JSONRPCMethodNotFound()._data).data) + self._message_manager.write_message(JSONRPC20Response(_id=request._id, error=JSONRPCMethodNotFound()._data)) return if request._id in self._received_requests: @@ -119,8 +118,7 @@ def _handle_request(self, request): self._handle_async_request(request, maybe_handler) elif not request.is_notification: log.debug('Sync request %s', request._id) - response = _make_response(request, result=maybe_handler) - self._message_manager.write_message(response.data) + self._message_manager.write_message(_make_response(request, result=maybe_handler)) def _handle_async_request(self, request, handler): log.debug('Async request %s', request._id) @@ -142,7 +140,7 @@ def did_finish_callback(completed_future): response = _make_response(request, error=JSONRPCInternalError()._data) else: response = _make_response(request, result=completed_future.result()) - self._message_manager.write_message(response.data) + self._message_manager.write_message(response) self._received_requests[request._id] = future future.add_done_callback(did_finish_callback) diff --git a/test/test_json_rpc_server.py b/test/test_json_rpc_server.py index 608a909d..587f43f9 100644 --- a/test/test_json_rpc_server.py +++ b/test/test_json_rpc_server.py @@ -1,39 +1,39 @@ # Copyright 2018 Palantir Technologies, Inc. -from jsonrpc.jsonrpc2 import JSONRPC20Request, JSONRPC20Response +from jsonrpc.jsonrpc2 import JSONRPC20Request, JSONRPC20Response, JSONRPC20BatchRequest def test_receive_request(json_rpc_server): client, server = json_rpc_server - request = {'jsonrpc': '2.0', 'id': 0, 'method': 'initialize', 'params': {}} + request = JSONRPC20Request(_id=0, method='initialize', params={}) client.write_message(request) message = next(server.get_messages()) assert isinstance(message, JSONRPC20Request) - assert request == message.data + assert request.data == message.data assert not message.is_notification def test_receive_notification(json_rpc_server): client, server = json_rpc_server - notification = {'jsonrpc': '2.0', 'method': 'notify', 'params': {}} + notification = JSONRPC20Request(method='initialize', params={}, is_notification=True) client.write_message(notification) message = next(server.get_messages()) assert isinstance(message, JSONRPC20Request) - assert notification == message.data + assert notification.data == message.data assert message.is_notification def test_receive_response(json_rpc_server): client, server = json_rpc_server - response = {'jsonrpc': '2.0', 'id': 0, 'result': {}} + response = JSONRPC20Response(_id=0, result={}) client.write_message(response) message = next(server.get_messages()) assert isinstance(message, JSONRPC20Response) - assert response == message.data + assert response.data == message.data def test_drop_bad_message(json_rpc_server): client, server = json_rpc_server - response = {'jsonrpc': '2.0', 'id': 0, 'result': {}} + response = JSONRPC20Response(_id=0, result={}) client.write_message(response) server.close() try: @@ -42,3 +42,36 @@ def test_drop_bad_message(json_rpc_server): pass else: assert False + + +def test_recieve_batch_request(json_rpc_server): + client, server = json_rpc_server + request_1 = JSONRPC20Request(_id=1, method='test_2', params={}) + request_2 = JSONRPC20Request(_id=2, method='test_2', params={}) + request = JSONRPC20BatchRequest(request_1, request_2) + client.write_message(request) + + messages = server.get_messages() + message_1 = next(messages) + message_2 = next(messages) + assert isinstance(message_1, JSONRPC20Request) + assert request_1.data == message_1.data + assert isinstance(message_2, JSONRPC20Request) + assert request_2.data == message_2.data + + +def test_send_batch_request_notification(json_rpc_server): + client, server = json_rpc_server + request_1 = JSONRPC20Request(_id=1, method='test_1', params={}) + request_2 = JSONRPC20Request(method='test_2', params={}) + request = JSONRPC20BatchRequest(request_1, request_2) + client.write_message(request) + + # load batch request + next(server.get_messages()) + + response_1 = JSONRPC20Response(_id=1, result='response_1') + server.write_message(response_1) + + response = next(client.get_messages()) + assert response.data == response_1.data diff --git a/test/test_rpc_manager.py b/test/test_rpc_manager.py index c06d6d29..4d7f7789 100644 --- a/test/test_rpc_manager.py +++ b/test/test_rpc_manager.py @@ -7,9 +7,10 @@ def test_handle_request_sync(rpc_management): rpc_manager, message_manager, message_handler = rpc_management rpc_manager.start() - message_manager.get_messages.assert_any_call() - message_manager.write_message.assert_called_once_with(BASE_HANDLED_RESPONSE.data) + message_manager.write_message.assert_called_once() message_handler.assert_called_once_with('test', {}) + (sent_message, ), _ = message_manager.write_message.call_args + assert sent_message.data == BASE_HANDLED_RESPONSE.data def test_handle_request_async(rpc_management): @@ -102,4 +103,6 @@ def test_send_notification(rpc_management): rpc_manager, message_manager, _ = rpc_management rpc_manager.notify('notify', {}) - message_manager.write_message.assert_called_once_with(JSONRPC20Request(method='notify', params={}).data) + message_manager.write_message.assert_called_once() + (sent_message, ), _ = message_manager.write_message.call_args + assert sent_message.data == (JSONRPC20Request(_id=None, method='notify', params={})).data From 9f079180621fca9850423d060980ddec2f2ae964 Mon Sep 17 00:00:00 2001 From: forozco Date: Sat, 17 Feb 2018 15:49:53 +0000 Subject: [PATCH 11/19] send notifications without an Id field --- pyls/rpc_manager.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pyls/rpc_manager.py b/pyls/rpc_manager.py index 8734ed77..b19513e6 100644 --- a/pyls/rpc_manager.py +++ b/pyls/rpc_manager.py @@ -65,7 +65,7 @@ def notify(self, method, params=None): params (dict): The payload of the notification """ log.debug('Notify %s %s', method, params) - notification = JSONRPC20Request(method=method, params=params) + notification = JSONRPC20Request(method=method, params=params, is_notification=True) self._message_manager.write_message(notification) def cancel(self, request_id): From c86963e0a44fef18f2b4f5e536d2e2d394362a1f Mon Sep 17 00:00:00 2001 From: forozco Date: Sat, 17 Feb 2018 18:37:55 +0000 Subject: [PATCH 12/19] fix tests --- test/test_rpc_manager.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/test_rpc_manager.py b/test/test_rpc_manager.py index 4d7f7789..f87b4fc1 100644 --- a/test/test_rpc_manager.py +++ b/test/test_rpc_manager.py @@ -105,4 +105,4 @@ def test_send_notification(rpc_management): rpc_manager.notify('notify', {}) message_manager.write_message.assert_called_once() (sent_message, ), _ = message_manager.write_message.call_args - assert sent_message.data == (JSONRPC20Request(_id=None, method='notify', params={})).data + assert sent_message.data == (JSONRPC20Request(method='notify', params={}, is_notification=True)).data From eaacb4a7b2f59f39ada52c87f3272c9304cd6aab Mon Sep 17 00:00:00 2001 From: forozco Date: Mon, 19 Feb 2018 10:00:24 +0000 Subject: [PATCH 13/19] only respond with unhandled exceptions to requests --- pyls/rpc_manager.py | 5 ++++- test/test_rpc_manager.py | 24 ++++++++++++++++++++++++ 2 files changed, 28 insertions(+), 1 deletion(-) diff --git a/pyls/rpc_manager.py b/pyls/rpc_manager.py index b19513e6..d3165c73 100644 --- a/pyls/rpc_manager.py +++ b/pyls/rpc_manager.py @@ -109,7 +109,10 @@ def _handle_request(self, request): maybe_handler = self._message_handler(request.method, params) except KeyError: log.debug("No handler found for %s", request.method) - self._message_manager.write_message(JSONRPC20Response(_id=request._id, error=JSONRPCMethodNotFound()._data)) + # Do not need to notify client of failure with notifications + if not request.is_notification: + self._message_manager.write_message( + JSONRPC20Response(_id=request._id, error=JSONRPCMethodNotFound()._data)) return if request._id in self._received_requests: diff --git a/test/test_rpc_manager.py b/test/test_rpc_manager.py index f87b4fc1..2d9ff15b 100644 --- a/test/test_rpc_manager.py +++ b/test/test_rpc_manager.py @@ -1,6 +1,7 @@ # Copyright 2018 Palantir Technologies, Inc. from test.fixtures import BASE_HANDLED_RESPONSE from jsonrpc.jsonrpc2 import JSONRPC20Request, JSONRPC20Response +from jsonrpc.exceptions import JSONRPCMethodNotFound def test_handle_request_sync(rpc_management): @@ -30,6 +31,17 @@ def wrapper(): message_manager.write_message.assert_called_once_with(response.data) +def test_handle_request_unknown_method(rpc_management): + rpc_manager, message_manager, message_handler = rpc_management + message_handler.configure_mock(side_effect=KeyError) + + rpc_manager.start() + message_manager.get_messages.assert_any_call() + message_handler.assert_called_once_with('test', {}) + (sent_message, ), _ = message_manager.write_message.call_args + assert sent_message.data == JSONRPC20Response(_id=1, error=JSONRPCMethodNotFound()._data).data + + def test_handle_notification_sync(rpc_management): rpc_manager, message_manager, message_handler = rpc_management notification = JSONRPC20Request(method='notification', params={}, is_notification=True) @@ -82,6 +94,18 @@ def wrapper(): message_manager.write_message.assert_not_called() +def test_handle_notification_unknown_method(rpc_management): + rpc_manager, message_manager, message_handler = rpc_management + notification = JSONRPC20Request(method='notification', params=None, is_notification=True) + message_manager.get_messages.configure_mock(return_value=[notification]) + message_handler.configure_mock(side_effect=KeyError) + + rpc_manager.start() + message_manager.get_messages.assert_any_call() + message_handler.assert_called_once_with('notification', {}) + message_manager.write_message.assert_not_called() + + def test_send_request(rpc_management): rpc_manager, message_manager, _ = rpc_management From 2e0764cab49ed86f7c3568083d6a82f5e508ea6d Mon Sep 17 00:00:00 2001 From: forozco Date: Mon, 19 Feb 2018 21:02:20 +0000 Subject: [PATCH 14/19] cleanup message handling, make async api nicer --- pyls/python_ls.py | 3 +- pyls/rpc_manager.py | 90 +++++++++++++++++++++++------------- test/fixtures.py | 4 +- test/test_language_server.py | 14 ++++-- test/test_rpc_manager.py | 52 +++++++++++++++++++-- tox.ini | 6 --- 6 files changed, 116 insertions(+), 53 deletions(-) diff --git a/pyls/python_ls.py b/pyls/python_ls.py index f2157214..314aab5e 100644 --- a/pyls/python_ls.py +++ b/pyls/python_ls.py @@ -8,6 +8,7 @@ from .json_rpc_server import JSONRPCServer from .rpc_manager import JSONRPCManager from .workspace import Workspace +from .rpc_manager import MissingMethodException log = logging.getLogger(__name__) @@ -98,7 +99,7 @@ def handle_request(self, method, params): if method_call in dispatcher: return dispatcher[method_call](**params) - raise KeyError('Handler for method {} not found'.format(method)) + raise MissingMethodException('No handler for for method {}'.format(method)) def _hook(self, hook_name, doc_uri=None, **kwargs): """Calls hook_name and returns a list of results from all registered handlers""" diff --git a/pyls/rpc_manager.py b/pyls/rpc_manager.py index d3165c73..0a5ecb89 100644 --- a/pyls/rpc_manager.py +++ b/pyls/rpc_manager.py @@ -6,7 +6,7 @@ from jsonrpc.base import JSONRPCBaseResponse from jsonrpc.jsonrpc1 import JSONRPC10Response from jsonrpc.jsonrpc2 import JSONRPC20Response, JSONRPC20Request -from jsonrpc.exceptions import JSONRPCMethodNotFound, JSONRPCInternalError +from jsonrpc.exceptions import JSONRPCMethodNotFound, JSONRPCDispatchException, JSONRPCServerError log = logging.getLogger(__name__) @@ -16,6 +16,10 @@ } +class MissingMethodException(Exception): + pass + + class JSONRPCManager(object): def __init__(self, message_manager, message_handler): @@ -40,14 +44,14 @@ def exit(self): self._message_manager.close() def call(self, method, params=None): - """Send a JSONRPC message with an expected response. + """Send a JSONRPC request. Args: method (str): The method name of the message to send params (dict): The payload of the message Returns: - Future that will resolve once a response has been recieved + Future that will resolve once a response has been received """ log.debug('Calling %s %s', method, params) @@ -95,33 +99,39 @@ def _handle_request(self, request): """Execute corresponding handler for the recieved request Args: - request (JSONRPCBaseRequest): request to act upon + request (JSONRPCBaseRequest): Request to act upon Note: - requests are handled asynchronously if the handler returns a callable, otherwise they are handle + Requests are handled asynchronously if the handler returns a callable, otherwise they are handle synchronously by the main thread """ if self._shutdown and request.method != 'exit': return - params = request.params if request.params is not None else {} + output = None try: - maybe_handler = self._message_handler(request.method, params) - except KeyError: - log.debug("No handler found for %s", request.method) + maybe_handler = self._message_handler(request.method, request.params if request.params is not None else {}) + except MissingMethodException as e: + log.debug(e.message) # Do not need to notify client of failure with notifications - if not request.is_notification: - self._message_manager.write_message( - JSONRPC20Response(_id=request._id, error=JSONRPCMethodNotFound()._data)) - return - - if request._id in self._received_requests: - log.error('Received request %s with duplicate id', request.data) - elif callable(maybe_handler): - self._handle_async_request(request, maybe_handler) - elif not request.is_notification: - log.debug('Sync request %s', request._id) - self._message_manager.write_message(_make_response(request, result=maybe_handler)) + output = JSONRPC20Response(_id=request._id, error=JSONRPCMethodNotFound()._data) + except JSONRPCDispatchException as e: + output = _make_response(request, error=e.error._data) + except Exception as e: # pylint: disable=broad-except + log.error('endpoint exception %s %s %s', e.__class__.__name__, e.args, str(e)) + output = _make_response(request, error=JSONRPCServerError()._data) + else: + if request._id in self._received_requests: + log.error('Received request %s with duplicate id', request.data) + elif callable(maybe_handler): + log.debug('Async request %s', request._id) + self._handle_async_request(request, maybe_handler) + else: + output = _make_response(request, result=maybe_handler) + finally: + if not request.is_notification and output is not None: + log.debug('Sync request %s', request._id) + self._message_manager.write_message(output) def _handle_async_request(self, request, handler): log.debug('Async request %s', request._id) @@ -134,31 +144,45 @@ def did_finish_callback(completed_future): del self._received_requests[request._id] if completed_future.cancelled(): log.debug('Cleared cancelled request %d', request._id) - return - - error = completed_future.exception() - if error is not None: - log.error('Failed to handle request %s with error %s', request._id, error) - # TODO(forozco): add more descriptive error - response = _make_response(request, error=JSONRPCInternalError()._data) else: - response = _make_response(request, result=completed_future.result()) - self._message_manager.write_message(response) + try: + result = completed_future.result() + except JSONRPCDispatchException as e: + output = _make_response(request, error=e.error._data) + except Exception as e: # pylint: disable=broad-except + # TODO(forozco): add more descriptive error + log.error('endpoint exception %s %s %s', e.__class__.__name__, e.args, str(e)) + output = _make_response(request, error=JSONRPCServerError()._data) + else: + output = _make_response(request, result=result) + finally: + self._message_manager.write_message(output) + self._received_requests[request._id] = future future.add_done_callback(did_finish_callback) def _handle_response(self, response): + """Handle the response to requests sent from the server to the client. + + Args: + response: (JSONRPC20Response): Received response + + """ try: request = self._sent_requests[response._id] + except KeyError: + log.error('Received unexpected response %s', response.data) + else: log.debug("Received response %s", response.data) def cleanup(_): del self._sent_requests[response._id] request.add_done_callback(cleanup) - request.set_result(response.data) - except KeyError: - log.error('Received unexpected response %s', response.data) + if 'result' in response.data: + request.set_result(response.result) + else: + request.set_exception(JSONRPCDispatchException(**response.error)) def _make_response(request, **kwargs): diff --git a/test/fixtures.py b/test/fixtures.py index 991f4e4e..60e31344 100644 --- a/test/fixtures.py +++ b/test/fixtures.py @@ -50,9 +50,7 @@ def workspace(tmpdir): @pytest.fixture def rpc_management(): - message_manager = JSONRPCServer(StringIO(), StringIO()) - message_manager.get_messages = Mock(return_value=[JSONRPC20Request(_id=1, method='test', params={})]) - message_manager.write_message = Mock() + message_manager = Mock(**{'get_messages.return_value': [JSONRPC20Request(_id=1, method='test', params={})]}) message_handler = Mock(return_value=BASE_HANDLED_RESPONSE_CONTENT) rpc_manager = JSONRPCManager(message_manager, message_handler) diff --git a/test/test_language_server.py b/test/test_language_server.py index a8c59acd..13b5cff7 100644 --- a/test/test_language_server.py +++ b/test/test_language_server.py @@ -1,7 +1,7 @@ # Copyright 2017 Palantir Technologies, Inc. import os from threading import Thread -from jsonrpc.exceptions import JSONRPCMethodNotFound +from jsonrpc.exceptions import JSONRPCMethodNotFound, JSONRPCDispatchException import pytest from pyls.python_ls import start_io_lang_server, PythonLanguageServer @@ -35,7 +35,7 @@ def client_server(): yield client shutdown_response = client.rpc_manager.call('shutdown').result(timeout=CALL_TIMEOUT) - assert shutdown_response['result'] is None + assert shutdown_response is None client.rpc_manager.notify('exit') @@ -45,9 +45,13 @@ def test_initialize(client_server): # pylint: disable=redefined-outer-name 'rootPath': os.path.dirname(__file__), 'initializationOptions': {} }).result(timeout=CALL_TIMEOUT) - assert 'capabilities' in response['result'] + assert 'capabilities' in response def test_missing_message(client_server): # pylint: disable=redefined-outer-name - response = client_server.rpc_manager.call('unknown_method').result(timeout=CALL_TIMEOUT) - assert response['error']['code'] == JSONRPCMethodNotFound.CODE + try: + client_server.rpc_manager.call('unknown_method').result(timeout=CALL_TIMEOUT) + except JSONRPCDispatchException as e: + assert e.error.code == JSONRPCMethodNotFound.CODE + else: + assert False diff --git a/test/test_rpc_manager.py b/test/test_rpc_manager.py index 2d9ff15b..44f181c9 100644 --- a/test/test_rpc_manager.py +++ b/test/test_rpc_manager.py @@ -1,7 +1,8 @@ # Copyright 2018 Palantir Technologies, Inc. from test.fixtures import BASE_HANDLED_RESPONSE from jsonrpc.jsonrpc2 import JSONRPC20Request, JSONRPC20Response -from jsonrpc.exceptions import JSONRPCMethodNotFound +from jsonrpc.exceptions import JSONRPCMethodNotFound, JSONRPCServerError, JSONRPCDispatchException +from pyls.rpc_manager import MissingMethodException def test_handle_request_sync(rpc_management): @@ -16,7 +17,6 @@ def test_handle_request_sync(rpc_management): def test_handle_request_async(rpc_management): rpc_manager, message_manager, message_handler = rpc_management - response = JSONRPC20Response(_id=1, result="async") def wrapper(): return 'async' @@ -26,14 +26,56 @@ def wrapper(): message_manager.get_messages.assert_any_call() message_handler.assert_called_once_with('test', {}) + # block until request has been handled if rpc_manager._sent_requests: rpc_manager._sent_requests.values()[0].result(timeout=1) - message_manager.write_message.assert_called_once_with(response.data) + message_manager.write_message.assert_called_once() + (sent_message, ), _ = message_manager.write_message.call_args + assert sent_message.data == JSONRPC20Response(_id=1, result="async").data + + +def test_handle_request_async_exception(rpc_management): + rpc_manager, message_manager, message_handler = rpc_management + + def wrapper(): + raise RuntimeError("something bad happened") + message_handler.configure_mock(return_value=wrapper) + + rpc_manager.start() + message_manager.get_messages.assert_any_call() + message_handler.assert_called_once_with('test', {}) + + # block until request has been handled + if rpc_manager._sent_requests: + rpc_manager._sent_requests.values()[0].result(timeout=1) + message_manager.write_message.assert_called_once() + (sent_message, ), _ = message_manager.write_message.call_args + assert sent_message.data == JSONRPC20Response(_id=1, error=JSONRPCServerError()._data).data + + +def test_handle_request_async_error(rpc_management): + rpc_manager, message_manager, message_handler = rpc_management + error_response = JSONRPCDispatchException(code=123, message="something bad happened", data={}) + + def wrapper(): + raise error_response + message_handler.configure_mock(return_value=wrapper) + + rpc_manager.start() + message_manager.get_messages.assert_any_call() + message_handler.assert_called_once_with('test', {}) + + # block until request has been handled + if rpc_manager._sent_requests: + rpc_manager._sent_requests.values()[0].result(timeout=1) + message_manager.write_message.assert_called_once() + (sent_message, ), _ = message_manager.write_message.call_args + assert sent_message.error == error_response.error._data def test_handle_request_unknown_method(rpc_management): rpc_manager, message_manager, message_handler = rpc_management - message_handler.configure_mock(side_effect=KeyError) + message_handler.configure_mock(side_effect=MissingMethodException) rpc_manager.start() message_manager.get_messages.assert_any_call() @@ -120,7 +162,7 @@ def test_send_request(rpc_management): rpc_manager.start() message_manager.get_messages.assert_any_call() assert not rpc_manager._sent_requests - assert response_future.result() == response.data + assert response_future.result() == {} def test_send_notification(rpc_management): diff --git a/tox.ini b/tox.ini index 3cd16bdc..3b5e11f6 100644 --- a/tox.ini +++ b/tox.ini @@ -10,12 +10,6 @@ envlist = py27,py34,lint ignore = E226, E722 max-line-length = 120 -[pytest] -testpaths = test -addopts = - --cov-report html --cov-report term --junitxml=pytest.xml - --cov pyls --cov test - [testenv] commands = py.test {posargs:test/} From 1f332eb839aa34ab1b49eba608d3b51e61b32a7c Mon Sep 17 00:00:00 2001 From: forozco Date: Mon, 19 Feb 2018 21:21:58 +0000 Subject: [PATCH 15/19] ugh py3 --- pyls/rpc_manager.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/pyls/rpc_manager.py b/pyls/rpc_manager.py index 0a5ecb89..9dc792e0 100644 --- a/pyls/rpc_manager.py +++ b/pyls/rpc_manager.py @@ -112,7 +112,7 @@ def _handle_request(self, request): try: maybe_handler = self._message_handler(request.method, request.params if request.params is not None else {}) except MissingMethodException as e: - log.debug(e.message) + log.debug(e) # Do not need to notify client of failure with notifications output = JSONRPC20Response(_id=request._id, error=JSONRPCMethodNotFound()._data) except JSONRPCDispatchException as e: @@ -134,7 +134,6 @@ def _handle_request(self, request): self._message_manager.write_message(output) def _handle_async_request(self, request, handler): - log.debug('Async request %s', request._id) future = self._executor_service.submit(handler) if request.is_notification: From 01ee433cabf21cfa3f91e1b35298604238691a08 Mon Sep 17 00:00:00 2001 From: forozco Date: Mon, 19 Feb 2018 21:52:42 +0000 Subject: [PATCH 16/19] fix flake --- test/test_rpc_manager.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/test/test_rpc_manager.py b/test/test_rpc_manager.py index 44f181c9..6e21a6f5 100644 --- a/test/test_rpc_manager.py +++ b/test/test_rpc_manager.py @@ -1,4 +1,5 @@ # Copyright 2018 Palantir Technologies, Inc. +from time import sleep from test.fixtures import BASE_HANDLED_RESPONSE from jsonrpc.jsonrpc2 import JSONRPC20Request, JSONRPC20Response from jsonrpc.exceptions import JSONRPCMethodNotFound, JSONRPCServerError, JSONRPCDispatchException @@ -27,6 +28,7 @@ def wrapper(): message_handler.assert_called_once_with('test', {}) # block until request has been handled + sleep(0) if rpc_manager._sent_requests: rpc_manager._sent_requests.values()[0].result(timeout=1) message_manager.write_message.assert_called_once() @@ -46,6 +48,7 @@ def wrapper(): message_handler.assert_called_once_with('test', {}) # block until request has been handled + sleep(0) if rpc_manager._sent_requests: rpc_manager._sent_requests.values()[0].result(timeout=1) message_manager.write_message.assert_called_once() @@ -66,6 +69,7 @@ def wrapper(): message_handler.assert_called_once_with('test', {}) # block until request has been handled + sleep(0) if rpc_manager._sent_requests: rpc_manager._sent_requests.values()[0].result(timeout=1) message_manager.write_message.assert_called_once() From e178f0668ed51df7958e6ff75ee5414c120751f3 Mon Sep 17 00:00:00 2001 From: forozco Date: Mon, 19 Feb 2018 21:56:10 +0000 Subject: [PATCH 17/19] . --- test/test_rpc_manager.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/test/test_rpc_manager.py b/test/test_rpc_manager.py index 6e21a6f5..f0b9b4a7 100644 --- a/test/test_rpc_manager.py +++ b/test/test_rpc_manager.py @@ -28,7 +28,7 @@ def wrapper(): message_handler.assert_called_once_with('test', {}) # block until request has been handled - sleep(0) + sleep(0.25) if rpc_manager._sent_requests: rpc_manager._sent_requests.values()[0].result(timeout=1) message_manager.write_message.assert_called_once() @@ -48,7 +48,7 @@ def wrapper(): message_handler.assert_called_once_with('test', {}) # block until request has been handled - sleep(0) + sleep(0.25) if rpc_manager._sent_requests: rpc_manager._sent_requests.values()[0].result(timeout=1) message_manager.write_message.assert_called_once() @@ -69,7 +69,7 @@ def wrapper(): message_handler.assert_called_once_with('test', {}) # block until request has been handled - sleep(0) + sleep(0.25) if rpc_manager._sent_requests: rpc_manager._sent_requests.values()[0].result(timeout=1) message_manager.write_message.assert_called_once() From 6ff7e7d9ee1f2703d11d347813237f59862fd18e Mon Sep 17 00:00:00 2001 From: forozco Date: Tue, 20 Feb 2018 11:03:28 +0000 Subject: [PATCH 18/19] addressed comments --- pyls/json_rpc_server.py | 13 +++++-------- pyls/python_ls.py | 5 ++--- pyls/workspace.py | 2 +- test/fixtures.py | 2 +- test/test_language_server.py | 2 +- tox.ini | 9 ++++++++- 6 files changed, 18 insertions(+), 15 deletions(-) diff --git a/pyls/json_rpc_server.py b/pyls/json_rpc_server.py index e28e7be8..8c0ad25f 100644 --- a/pyls/json_rpc_server.py +++ b/pyls/json_rpc_server.py @@ -5,10 +5,7 @@ from jsonrpc.jsonrpc2 import JSONRPC20Response, JSONRPC20BatchRequest, JSONRPC20BatchResponse from jsonrpc.jsonrpc import JSONRPCRequest -from jsonrpc.exceptions import ( - JSONRPCInvalidRequestException, -) - +from jsonrpc.exceptions import JSONRPCInvalidRequestException log = logging.getLogger(__name__) @@ -30,7 +27,7 @@ def close(self): def get_messages(self): """Generator that produces well structured JSON RPC message. - Returns: + Yields: message: received message Note: @@ -59,14 +56,14 @@ def get_messages(self): # we do not send out batch requests so no need to support batch responses messages = [JSONRPC20Response(**message_blob)] except (KeyError, ValueError): - log.error("Could not parse message %s", request_str) + log.exception("Could not parse message %s", request_str) continue for message in messages: yield message def write_message(self, message): - """ Write message to out file descriptor + """ Write message to out file descriptor. Args: message (JSONRPCRequest, JSONRPCResponse): body of the message to send @@ -91,7 +88,7 @@ def write_message(self, message): self.wfile.flush() def _read_message(self): - """Reads the contents of a message + """Reads the contents of a message. Returns: body of message if parsable else None diff --git a/pyls/python_ls.py b/pyls/python_ls.py index 314aab5e..9204fce6 100644 --- a/pyls/python_ls.py +++ b/pyls/python_ls.py @@ -6,9 +6,8 @@ from . import lsp, _utils, uris from .config import config from .json_rpc_server import JSONRPCServer -from .rpc_manager import JSONRPCManager +from .rpc_manager import JSONRPCManager, MissingMethodException from .workspace import Workspace -from .rpc_manager import MissingMethodException log = logging.getLogger(__name__) @@ -141,7 +140,7 @@ def m_initialize(self, processId=None, rootUri=None, rootPath=None, initializati if rootUri is None: rootUri = uris.from_fs_path(rootPath) if rootPath is not None else '' - self.workspace = Workspace(rootUri, rpc_manager=self.rpc_manager) + self.workspace = Workspace(rootUri, self.rpc_manager) self.config = config.Config(rootUri, initializationOptions or {}) self._dispatchers = self._hook('pyls_dispatchers') self._hook('pyls_initialize') diff --git a/pyls/workspace.py b/pyls/workspace.py index 4f2a893b..992c9b63 100644 --- a/pyls/workspace.py +++ b/pyls/workspace.py @@ -74,7 +74,7 @@ class Workspace(object): M_SHOW_MESSAGE = 'window/showMessage' PRELOADED_MODULES = get_preferred_submodules() - def __init__(self, root_uri, rpc_manager=None): + def __init__(self, root_uri, rpc_manager): self._root_uri = root_uri self._rpc_manager = rpc_manager self._root_uri_scheme = uris.urlparse(self._root_uri)[0] diff --git a/test/fixtures.py b/test/fixtures.py index 60e31344..5a43de0d 100644 --- a/test/fixtures.py +++ b/test/fixtures.py @@ -45,7 +45,7 @@ def pyls(tmpdir): @pytest.fixture def workspace(tmpdir): """Return a workspace.""" - return Workspace(uris.from_fs_path(str(tmpdir))) + return Workspace(uris.from_fs_path(str(tmpdir)), Mock()) @pytest.fixture diff --git a/test/test_language_server.py b/test/test_language_server.py index 13b5cff7..dffb9811 100644 --- a/test/test_language_server.py +++ b/test/test_language_server.py @@ -54,4 +54,4 @@ def test_missing_message(client_server): # pylint: disable=redefined-outer-name except JSONRPCDispatchException as e: assert e.error.code == JSONRPCMethodNotFound.CODE else: - assert False + assert False, "expected JSONRPCDispatchException" diff --git a/tox.ini b/tox.ini index 3b5e11f6..8bf29a6e 100644 --- a/tox.ini +++ b/tox.ini @@ -9,6 +9,13 @@ envlist = py27,py34,lint [pycodestyle] ignore = E226, E722 max-line-length = 120 +exclude = test/plugins/.ropeproject,test/.ropeproject + +[pytest] +testpaths = test +addopts = + --cov-report html --cov-report term --junitxml=pytest.xml + --cov pyls --cov test [testenv] commands = @@ -23,5 +30,5 @@ deps = [testenv:lint] commands = pylint pyls test - pycodestyle pyls test --exclude=test/plugins/.ropeproject,test/.ropeproject + pycodestyle pyls test pyflakes pyls test From 2a75bd9dea994298e06e77fcf75475da083216ef Mon Sep 17 00:00:00 2001 From: forozco Date: Tue, 20 Feb 2018 11:47:04 +0000 Subject: [PATCH 19/19] more comments --- pyls/python_ls.py | 22 ++++++++++------------ pyls/rpc_manager.py | 9 ++++----- 2 files changed, 14 insertions(+), 17 deletions(-) diff --git a/pyls/python_ls.py b/pyls/python_ls.py index 9204fce6..61de3569 100644 --- a/pyls/python_ls.py +++ b/pyls/python_ls.py @@ -100,6 +100,16 @@ def handle_request(self, method, params): raise MissingMethodException('No handler for for method {}'.format(method)) + def m__cancel_request(self, **kwargs): + self.rpc_manager.cancel(kwargs['id']) + + def m_shutdown(self, **_kwargs): + self.rpc_manager.shutdown() + return None + + def m_exit(self, **_kwargs): + self.rpc_manager.exit() + def _hook(self, hook_name, doc_uri=None, **kwargs): """Calls hook_name and returns a list of results from all registered handlers""" doc = self.workspace.get_document(doc_uri) if doc_uri else None @@ -148,18 +158,6 @@ def m_initialize(self, processId=None, rootUri=None, rootPath=None, initializati # Get our capabilities return {'capabilities': self.capabilities()} - def m__cancel_request(self, **kwargs): - def handler(): - self.rpc_manager.cancel(kwargs['id']) - return handler - - def m_shutdown(self, **_kwargs): - self.rpc_manager.shutdown() - return None - - def m_exit(self, **_kwargs): - self.rpc_manager.exit() - def code_actions(self, doc_uri, range, context): return flatten(self._hook('pyls_code_actions', doc_uri, range=range, context=context)) diff --git a/pyls/rpc_manager.py b/pyls/rpc_manager.py index 9dc792e0..62211e7e 100644 --- a/pyls/rpc_manager.py +++ b/pyls/rpc_manager.py @@ -28,7 +28,7 @@ def __init__(self, message_manager, message_handler): self._shutdown = False self._sent_requests = {} self._received_requests = {} - self._executor_service = ThreadPoolExecutor(max_workers=5) + self._executor_service = ThreadPoolExecutor(max_workers=5) # arbitrary pool size def start(self): """Start reading JSONRPC messages off of rx""" @@ -52,7 +52,6 @@ def call(self, method, params=None): Returns: Future that will resolve once a response has been received - """ log.debug('Calling %s %s', method, params) request = JSONRPC20Request(_id=uuid4().int, method=method, params=params) @@ -85,7 +84,7 @@ def cancel(self, request_id): try: self._received_requests[request_id].cancel() except KeyError: - log.error('Received cancel for finished/nonexistent request %d', request_id) + log.debug('Received cancel for finished/nonexistent request %d', request_id) def consume_requests(self): """ Infinite loop watching for messages from the client.""" @@ -118,7 +117,7 @@ def _handle_request(self, request): except JSONRPCDispatchException as e: output = _make_response(request, error=e.error._data) except Exception as e: # pylint: disable=broad-except - log.error('endpoint exception %s %s %s', e.__class__.__name__, e.args, str(e)) + log.exception('synchronous method handler exception') output = _make_response(request, error=JSONRPCServerError()._data) else: if request._id in self._received_requests: @@ -150,7 +149,7 @@ def did_finish_callback(completed_future): output = _make_response(request, error=e.error._data) except Exception as e: # pylint: disable=broad-except # TODO(forozco): add more descriptive error - log.error('endpoint exception %s %s %s', e.__class__.__name__, e.args, str(e)) + log.exception('asynchronous method handler exception') output = _make_response(request, error=JSONRPCServerError()._data) else: output = _make_response(request, result=result)