From 54f4b8e98c3ceba81edc541667204711a3af9a4e Mon Sep 17 00:00:00 2001 From: Glenn Sarti Date: Tue, 17 Apr 2018 20:16:06 +0800 Subject: [PATCH] (GH-11) Refactor the transport layers to loosen object coupling Previously the different transport types, TCP and STDIO, were very closely coupled which made debugging and maintaining them difficult. This commit changes the relationships between the server, client connection, message handler and message router classes/objects to be much looser and can therefore be defined/instantiated easier at run time; New object model - (Transport Server) creates a (Client Connection object) per incoming connection - Each (Client Connection object) creates a (Handler object) - Each (Handler object) creates a (Message Router object) Therefore the Message Router can create messages which traverse back down the object chain and is eventually emitted by the Transport Server. This model should apply whether it is a STDIO or TCP transport * The JSON Handler no longer inherits the TCP Server Connection, but a generic SimpleServerConnectionHandler which has some base methods. The message router object is now assigned via the message_router methods, which can be passed in via the initializer, or defaults to an instance of the PuppetDebugServer::MessageRouter class. The client connection object is expected to implement the SimpleServerConnectionBase "interface" and passedv in via the Handler initializer. * The Message Router no longer inherits from the JSON handler, but has a json_handler method. It is assumed the Handler will set this when it creates an instance of the Router * A new SimpleSTDIOServerConnection object has been created which implements the base SimpleServerConnectionBase object. This object is the concrete implementation of connection over the STDIO transport * The TCP Server Connection now inherits from the SimpleServerConnectionBase object and tracks the originating TCP Server object and socket via the initializer * PuppetLanguageServer was modified to use the new STDIO connection type and use the JSON Handler instead of the MessageRouter * The document_type method was moved from the MessageRouter code to the DocumentStore module. This made more sense and means that we don't need to create an entire Message Router object to do a simple static document type check * The validation queue was modified with the new namespace of the document_type static method * Tests were modified for the new object hierarchy --- CHANGELOG.md | 2 + lib/puppet-debugserver.rb | 2 +- lib/puppet-debugserver/json_handler.rb | 36 ++++++--- lib/puppet-debugserver/message_router.rb | 73 ++++++++++--------- lib/puppet-editor-services.rb | 2 +- lib/puppet-editor-services/simple_base.rb | 49 +++++++++++++ .../simple_stdio_server.rb | 26 +++++++ .../simple_tcp_server.rb | 36 +++------ lib/puppet-languageserver.rb | 7 +- lib/puppet-languageserver/json_rpc_handler.rb | 61 ++++++++++------ lib/puppet-languageserver/message_router.rb | 51 ++++++------- lib/puppet-languageserver/validation_queue.rb | 4 +- ...ge_router_spec.rb => json_handler_spec.rb} | 22 +++--- .../message_router_spec.rb | 10 ++- .../integration/puppet-languageserver_spec.rb | 8 +- spec/languageserver/spec_helper.rb | 27 ++----- .../message_router_spec.rb | 18 +++-- 17 files changed, 261 insertions(+), 173 deletions(-) create mode 100644 lib/puppet-editor-services/simple_base.rb create mode 100644 lib/puppet-editor-services/simple_stdio_server.rb rename spec/debugserver/integration/puppet-debugserver/{message_router_spec.rb => json_handler_spec.rb} (84%) diff --git a/CHANGELOG.md b/CHANGELOG.md index f49d5646..30a67515 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,8 @@ Check [Keep a Changelog](http://keepachangelog.com/) for recommendations on how ## Unreleased +- ([GH-11](https://github.com/lingua-pupuli/puppet-editor-services/issues/11)) Refactor the transport layers to loosen object coupling + ## 0.10.0 - 2018-03-29 - ([GH-218](https://github.com/jpogran/puppet-vscode/issues/218)) Validate EPP files diff --git a/lib/puppet-debugserver.rb b/lib/puppet-debugserver.rb index e0ee10bd..116a4b68 100644 --- a/lib/puppet-debugserver.rb +++ b/lib/puppet-debugserver.rb @@ -83,7 +83,7 @@ def self.rpc_server(options) server.add_service(options[:ipaddress], options[:port]) trap('INT') { server.stop_services(true) } - server.start(PuppetDebugServer::MessageRouter, options, 2) + server.start(PuppetDebugServer::JSONHandler, options, 2) log_message(:info, 'Debug Server exited.') end diff --git a/lib/puppet-debugserver/json_handler.rb b/lib/puppet-debugserver/json_handler.rb index 1325dab1..bb993f09 100644 --- a/lib/puppet-debugserver/json_handler.rb +++ b/lib/puppet-debugserver/json_handler.rb @@ -4,17 +4,31 @@ # https://github.com/Microsoft/vscode-debugadapter-node/blob/master/protocol/src/debugProtocol.ts module PuppetDebugServer - class JSONHandler < PuppetEditorServices::SimpleTCPServerConnection - def initialize(*_options) + class JSONHandler < PuppetEditorServices::SimpleServerConnectionHandler + attr_accessor :message_router + + def initialize(options = {}) + options = {} if options.nil? + @state = :data @buffer = [] @response_sequence = 1 + + @client_connection = options[:connection] + if options[:message_router].nil? + @message_router = PuppetDebugServer::MessageRouter.new(options) + else + @message_router = options[:message_router] + end + @message_router.json_handler = self end + # From PuppetEditorServices::SimpleServerConnectionHandler def post_init PuppetDebugServer.log_message(:info, 'Client has connected to the debug server') end + # From PuppetEditorServices::SimpleServerConnectionHandler def unbind PuppetDebugServer.log_message(:info, 'Client has disconnected from the debug server') end @@ -35,6 +49,7 @@ def extract_headers(raw_header) header end + # From PuppetEditorServices::SimpleServerConnectionHandler def receive_data(data) # Inspired by https://github.com/PowerShell/PowerShellEditorServices/blob/dba65155c38d3d9eeffae5f0358b5a3ad0215fac/src/PowerShellEditorServices.Protocol/MessageProtocol/MessageReader.cs return if data.empty? @@ -82,7 +97,7 @@ def send_response(response) PuppetDebugServer.log_message(:debug, "--- OUTBOUND\n#{response_json}\n---") size = response_json.bytesize - send_data "Content-Length: #{size}\r\n\r\n" + response_json + @client_connection.send_data "Content-Length: #{size}\r\n\r\n" + response_json end def send_event(response) @@ -95,7 +110,7 @@ def send_event(response) PuppetDebugServer.log_message(:debug, "--- OUTBOUND\n#{response_json}\n---") size = response_json.bytesize - send_data "Content-Length: #{size}\r\n\r\n" + response_json + @client_connection.send_data "Content-Length: #{size}\r\n\r\n" + response_json end def parse_data(data) @@ -115,7 +130,7 @@ def received_parsed_object(obj) # NOTE: Not implemented as it doesn't make sense using JSON RPC over pure TCP / UnixSocket. else PuppetDebugServer.log_message(:error, 'Closing connection as request is not a Hash') - close_connection_after_writing + @client_connection.close_connection_after_writing @state = :ignore end end @@ -124,17 +139,12 @@ def process(obj) message = PuppetDebugServer::Protocol::ProtocolMessage.create(obj) case message['type'] when 'request' - receive_request(PuppetDebugServer::Protocol::Request.create(obj), obj) + message_router.receive_request(PuppetDebugServer::Protocol::Request.create(obj), obj) else PuppetDebugServer.log_message(:error, "Unknown protocol message type #{message['type']}") end end - # This method must be overriden in the user's inherited class. - def receive_request(request, _request_json) - PuppetDebugServer.log_message(:debug, "request received:\n#{request.inspect}") - end - def encode_json(data) JSON.generate(data) end @@ -147,6 +157,10 @@ def reply_error(request, message, body) send_response response end + def close_connection + @client_connection.close_connection unless @client_connection.nil? + end + # This method could be overriden in the user's inherited class. def parsing_error(_data, exception) PuppetDebugServer.log_message(:error, "parsing error:\n#{exception.message}") diff --git a/lib/puppet-debugserver/message_router.rb b/lib/puppet-debugserver/message_router.rb index dd1f5342..a20c358c 100644 --- a/lib/puppet-debugserver/message_router.rb +++ b/lib/puppet-debugserver/message_router.rb @@ -1,33 +1,34 @@ module PuppetDebugServer - class MessageRouter < JSONHandler - def initialize(*options) - super(*options) + class MessageRouter + attr_accessor :json_handler + + def initialize(*_options) end def send_termination_event obj = PuppetDebugServer::Protocol::TerminatedEvent.create({}) - send_event obj + @json_handler.send_event obj end def send_exited_event(exitcode) obj = PuppetDebugServer::Protocol::ExitedEvent.create('exitCode' => exitcode) - send_event obj + @json_handler.send_event obj end def send_output_event(options) obj = PuppetDebugServer::Protocol::OutputEvent.create(options) - send_event obj + @json_handler.send_event obj end def send_stopped_event(reason, options = {}) options['reason'] = reason obj = PuppetDebugServer::Protocol::StoppedEvent.create(options) - send_event obj + @json_handler.send_event obj end def send_thread_event(reason, thread_id) obj = PuppetDebugServer::Protocol::ThreadEvent.create('reason' => reason, 'threadId' => thread_id) - send_event obj + @json_handler.send_event obj end def receive_request(request, original_json) @@ -59,12 +60,12 @@ def receive_request(request, original_json) 'success' => true }, request ) - send_response response + @json_handler.send_response response # Send a message that we are initialized # This must happen _after_ the capabilites are sent sleep(0.5) # Sleep for a small amount of time to give the client time to process the capabilites response - send_event PuppetDebugServer::Protocol::InitializedEvent.create + @json_handler.send_event PuppetDebugServer::Protocol::InitializedEvent.create when 'configurationDone' PuppetDebugServer.log_message(:debug, 'Received configurationDone request.') @@ -75,7 +76,7 @@ def receive_request(request, original_json) 'success' => true }, request ) - send_response response + @json_handler.send_response response # Start the debug session if the session is not already running and, setup and configuration have completed PuppetDebugServer::PuppetDebugSession.start if !PuppetDebugServer::PuppetDebugSession.session_active? && PuppetDebugServer::PuppetDebugSession.setup? @@ -92,7 +93,7 @@ def receive_request(request, original_json) 'success' => 'true' }, request ) - send_response response + @json_handler.send_response response when 'setFunctionBreakpoints' PuppetDebugServer.log_message(:debug, 'Received setFunctionBreakpoints request.') @@ -112,7 +113,7 @@ def receive_request(request, original_json) 'success' => 'true' }, request ) - send_response response + @json_handler.send_response response when 'launch' PuppetDebugServer.log_message(:debug, 'Received launch request.') @@ -124,7 +125,7 @@ def receive_request(request, original_json) 'success' => true }, request ) - send_response response + @json_handler.send_response response # Start the debug session PuppetDebugServer::PuppetDebugSession.setup(self, original_json['arguments']) @@ -148,7 +149,7 @@ def receive_request(request, original_json) }, request ) end - send_response response + @json_handler.send_response response when 'stackTrace' PuppetDebugServer.log_message(:debug, 'Received stackTrace request.') @@ -160,7 +161,7 @@ def receive_request(request, original_json) 'success' => false }, request ) - send_response response + @json_handler.send_response response return end @@ -171,7 +172,7 @@ def receive_request(request, original_json) 'stackFrames' => frames }, request ) - send_response response + @json_handler.send_response response when 'scopes' PuppetDebugServer.log_message(:debug, 'Received scopes request.') @@ -183,7 +184,7 @@ def receive_request(request, original_json) 'success' => false }, request ) - send_response response + @json_handler.send_response response return end @@ -196,7 +197,7 @@ def receive_request(request, original_json) 'scopes' => [] }, request ) - send_response response + @json_handler.send_response response return end @@ -207,7 +208,7 @@ def receive_request(request, original_json) 'scopes' => scopes }, request ) - send_response response + @json_handler.send_response response when 'variables' PuppetDebugServer.log_message(:debug, 'Received variables request.') @@ -219,7 +220,7 @@ def receive_request(request, original_json) 'success' => false }, request ) - send_response response + @json_handler.send_response response return end @@ -230,7 +231,7 @@ def receive_request(request, original_json) 'variables' => variables }, request ) - send_response response + @json_handler.send_response response when 'evaluate' PuppetDebugServer.log_message(:debug, 'Received evaluate request.') @@ -242,7 +243,7 @@ def receive_request(request, original_json) 'success' => false }, request ) - send_response response + @json_handler.send_response response return end @@ -254,7 +255,7 @@ def receive_request(request, original_json) 'success' => true }, request ) - send_response response + @json_handler.send_response response return end @@ -270,7 +271,7 @@ def receive_request(request, original_json) 'variablesReference' => 0 }, request ) - send_response response + @json_handler.send_response response rescue => exception # rubocop:disable Style/RescueStandardError response = PuppetDebugServer::Protocol::Response.create_from_request( { @@ -278,7 +279,7 @@ def receive_request(request, original_json) 'message' => exception.to_s }, request ) - send_response response + @json_handler.send_response response end when 'continue' @@ -293,7 +294,7 @@ def receive_request(request, original_json) 'allThreadsContinued' => true }, request ) - send_response response + @json_handler.send_response response when 'stepIn' PuppetDebugServer.log_message(:debug, 'Received stepIn request.') @@ -305,14 +306,14 @@ def receive_request(request, original_json) 'success' => false }, request ) - send_response response + @json_handler.send_response response return end # Stepin the debug session PuppetDebugServer::PuppetDebugSession.continue_stepin_session - send_response PuppetDebugServer::Protocol::StepInResponse.create_from_request({ 'success' => true }, request) + @json_handler.send_response PuppetDebugServer::Protocol::StepInResponse.create_from_request({ 'success' => true }, request) when 'stepOut' PuppetDebugServer.log_message(:debug, 'Received stepOut request.') @@ -324,14 +325,14 @@ def receive_request(request, original_json) 'success' => false }, request ) - send_response response + @json_handler.send_response response return end # Next the debug session PuppetDebugServer::PuppetDebugSession.continue_stepout_session - send_response PuppetDebugServer::Protocol::StepOutResponse.create_from_request({ 'success' => true }, request) + @json_handler.send_response PuppetDebugServer::Protocol::StepOutResponse.create_from_request({ 'success' => true }, request) when 'next' PuppetDebugServer.log_message(:debug, 'Received next request.') @@ -343,20 +344,20 @@ def receive_request(request, original_json) 'success' => false }, request ) - send_response response + @json_handler.send_response response return end # Next the debug session PuppetDebugServer::PuppetDebugSession.continue_next_session - send_response PuppetDebugServer::Protocol::NextResponse.create_from_request({ 'success' => true }, request) + @json_handler.send_response PuppetDebugServer::Protocol::NextResponse.create_from_request({ 'success' => true }, request) when 'disconnect' # Don't really care about the arguments - Kill everything PuppetDebugServer.log_message(:info, 'Received disconnect request. Closing connection to client...') - close_connection - + @json_handler.close_connection + # TODO: client isn't disconnecting properly.... else PuppetDebugServer.log_message(:error, "Unknown request command #{request['command']}") @@ -366,7 +367,7 @@ def receive_request(request, original_json) 'message' => "This feature is not supported - Request #{request['command']}" }, request ) - send_response response + @json_handler.send_response response end end end diff --git a/lib/puppet-editor-services.rb b/lib/puppet-editor-services.rb index b85c1ffe..b65549df 100644 --- a/lib/puppet-editor-services.rb +++ b/lib/puppet-editor-services.rb @@ -1,4 +1,4 @@ -%w[logging version simple_tcp_server].each do |lib| +%w[logging version simple_base simple_tcp_server simple_stdio_server].each do |lib| begin require "puppet-editor-services/#{lib}" rescue LoadError diff --git a/lib/puppet-editor-services/simple_base.rb b/lib/puppet-editor-services/simple_base.rb new file mode 100644 index 00000000..4fbfeef1 --- /dev/null +++ b/lib/puppet-editor-services/simple_base.rb @@ -0,0 +1,49 @@ +module PuppetEditorServices + class SimpleServerConnectionBase + # Override this method + # @api public + def error? + false + end + + # Override this method + # @api public + def send_data(_data) + false + end + + # Override this method + # @api public + def close_connection_after_writing + true + end + + # Override this method + # @api public + def close_connection + true + end + end + + class SimpleServerConnectionHandler + attr_accessor :client_connection + + # Override this method + # @api public + def receive_data(_data) + false + end + + # Override this method + # @api public + def post_init + true + end + + # Override this method + # @api public + def unbind + true + end + end +end diff --git a/lib/puppet-editor-services/simple_stdio_server.rb b/lib/puppet-editor-services/simple_stdio_server.rb new file mode 100644 index 00000000..3406ef97 --- /dev/null +++ b/lib/puppet-editor-services/simple_stdio_server.rb @@ -0,0 +1,26 @@ +module PuppetEditorServices + class SimpleSTDIOServerConnection < SimpleServerConnectionBase + attr_accessor :socket + + def initialize(socket) + @socket = socket + end + + def send_data(data) + return false if socket.nil? + socket.write(data) + true + end + + def close_connection_after_writing + socket.flush unless socket.nil? + simple_tcp_server.remove_connection_async(socket) + true + end + + def close_connection + simple_tcp_server.remove_connection_async(socket) + true + end + end +end diff --git a/lib/puppet-editor-services/simple_tcp_server.rb b/lib/puppet-editor-services/simple_tcp_server.rb index 5c27a6ec..393947c4 100644 --- a/lib/puppet-editor-services/simple_tcp_server.rb +++ b/lib/puppet-editor-services/simple_tcp_server.rb @@ -4,47 +4,30 @@ # http://stackoverflow.com/questions/29858113/unable-to-make-socket-accept-non-blocking-ruby-2-2 module PuppetEditorServices - class SimpleTCPServerConnection + class SimpleTCPServerConnection < SimpleServerConnectionBase attr_accessor :socket attr_accessor :simple_tcp_server - # Methods to override - def post_init - # Override this to recieve events after a client is connected - PuppetEditorServices.log_message(:debug, 'TCPSRV: Client has connected') + def initialize(simple_tcp_server, socket) + @simple_tcp_server = simple_tcp_server + @socket = socket end - def unbind - # Override this to recieve events after a client is disconnected - PuppetEditorServices.log_message(:debug, 'TCPSRV: Client has disconnected') - end - - def receive_data(data) - # Override this to recieve data - PuppetEditorServices.log_message(:debug, "TCPSRV: Received #{data.length} characters") - end - - # @api public - def error? - false - end - - # @api public def send_data(data) return false if socket.nil? socket.write(data) true end - # @api public def close_connection_after_writing socket.flush unless socket.nil? simple_tcp_server.remove_connection_async(socket) + true end - # @api public def close_connection simple_tcp_server.remove_connection_async(socket) + true end end @@ -77,7 +60,6 @@ def log(message) def get_data(io, connection_data) data = io.recv_nonblock(1048576) # with maximum number of bytes to read at a time... raise 'Received a 0byte payload' if data.length.zero? - # We're already in a callback so no need to invoke as a callback connection_data[:handler].receive_data(data) rescue StandardError => e @@ -239,7 +221,7 @@ def io_review begin callback(self, :add_connection, io.accept_nonblock, self.class.services[io]) rescue Errno::EWOULDBLOCK => _ # rubocop:disable Lint/HandleExceptions - # There's nothing too handle. Swallow the error + # There's nothing to handle. Swallow the error rescue StandardError => e log(e.message) end @@ -332,8 +314,8 @@ def stop_connections # @api private def add_connection(io, service_object) handler = @handler_klass.new(@handler_start_options) - handler.socket = io - handler.simple_tcp_server = self + conn = SimpleTCPServerConnection.new(self, io) + handler.client_connection = conn if io self.class.c_locker.synchronize do self.class.io_connection_dic[io] = { handler: handler, service: service_object } diff --git a/lib/puppet-languageserver.rb b/lib/puppet-languageserver.rb index 14ad75ea..3a10400f 100644 --- a/lib/puppet-languageserver.rb +++ b/lib/puppet-languageserver.rb @@ -155,8 +155,9 @@ def self.rpc_server(options) $stdin.sync = true $stdout.sync = true - handler = PuppetLanguageServer::MessageRouter.new - handler.socket = $stdout + handler = PuppetLanguageServer::JSONRPCHandler.new(options) + client_connection = PuppetEditorServices::SimpleSTDIOServerConnection.new($stdout) + handler.client_connection = client_connection handler.post_init loop do @@ -172,7 +173,7 @@ def self.rpc_server(options) server.add_service(options[:ipaddress], options[:port]) trap('INT') { server.stop_services(true) } - server.start(PuppetLanguageServer::MessageRouter, options, 2) + server.start(PuppetLanguageServer::JSONRPCHandler, options, 2) end log_message(:info, 'Language Server exited.') diff --git a/lib/puppet-languageserver/json_rpc_handler.rb b/lib/puppet-languageserver/json_rpc_handler.rb index 85d52bc4..98683994 100644 --- a/lib/puppet-languageserver/json_rpc_handler.rb +++ b/lib/puppet-languageserver/json_rpc_handler.rb @@ -37,8 +37,11 @@ module PuppetLanguageServer KEY_CODE = 'code'.freeze KEY_MESSAGE = 'message'.freeze - class JSONRPCHandler < PuppetEditorServices::SimpleTCPServerConnection - def initialize(*_options) + class JSONRPCHandler < PuppetEditorServices::SimpleServerConnectionHandler + attr_accessor :message_router + + def initialize(options = {}) + options = {} if options.nil? @key_jsonrpc = KEY_JSONRPC @key_id = KEY_ID @key_method = KEY_METHOD @@ -46,12 +49,22 @@ def initialize(*_options) @state = :data @buffer = [] + + @client_connection = options[:connection] + if options[:message_router].nil? + @message_router = PuppetLanguageServer::MessageRouter.new(options) + else + @message_router = options[:message_router] + end + @message_router.json_rpc_handler = self end + # From PuppetEditorServices::SimpleServerConnectionHandler def post_init PuppetLanguageServer.log_message(:info, 'Client has connected to the language server') end + # From PuppetEditorServices::SimpleServerConnectionHandler def unbind PuppetLanguageServer.log_message(:info, 'Client has disconnected from the language server') end @@ -72,6 +85,7 @@ def extract_headers(raw_header) header end + # From PuppetEditorServices::SimpleServerConnectionHandler def receive_data(data) # Inspired by https://github.com/PowerShell/PowerShellEditorServices/blob/dba65155c38d3d9eeffae5f0358b5a3ad0215fac/src/PowerShellEditorServices.Protocol/MessageProtocol/MessageReader.cs return if data.empty? @@ -113,7 +127,7 @@ def send_response(response) PuppetEditorServices.log_message(:debug, "--- OUTBOUND\n#{response}\n---") size = response.bytesize if response.respond_to?(:bytesize) - send_data "Content-Length: #{size}\r\n\r\n" + response + @client_connection.send_data "Content-Length: #{size}\r\n\r\n" + response end def parse_data(data) @@ -171,20 +185,19 @@ def process(obj) end if is_request - receive_request Request.new(self, id, method, params) + @message_router.receive_request Request.new(self, id, method, params) else - receive_notification method, params + @message_router.receive_notification method, params end end - # This method must be overriden in the user's inherited class. - def receive_request(request) - PuppetEditorServices.log_message(:debug, "request received:\n#{request.inspect}") + def close_connection + @client_connection.close_connection unless @client_connection.nil? end - # This method must be overriden in the user's inherited class. - def receive_notification(method, params) - PuppetEditorServices.log_message(:debug, "notification received (method: #{method.inspect}, params: #{params.inspect})") + def connection_error? + return false if @client_connection.nil? + @client_connection.error? end def encode_json(data) @@ -201,7 +214,7 @@ def reply_error(id, code, message) end def reply_diagnostics(uri, diagnostics) - return nil if error? + return nil if connection_error? response = { KEY_JSONRPC => VALUE_VERSION, @@ -242,15 +255,15 @@ def invalid_request(_obj, code, message = nil) class Request attr_reader :rpc_method, :params, :id - def initialize(conn, id, rpc_method, params) - @conn = conn + def initialize(json_rpc_handler, id, rpc_method, params) + @json_rpc_handler = json_rpc_handler @id = id @rpc_method = rpc_method @params = params end def reply_result(result) - return nil if @conn.error? + return nil if @json_rpc_handler.connection_error? response = { KEY_JSONRPC => VALUE_VERSION, @@ -258,31 +271,31 @@ def reply_result(result) KEY_RESULT => result } - @conn.send_response(@conn.encode_json(response)) + @json_rpc_handler.send_response(@json_rpc_handler.encode_json(response)) true end def reply_internal_error(message = nil) - return nil if @conn.error? - @conn.reply_error(@id, CODE_INTERNAL_ERROR, message || MSG_INTERNAL_ERROR) + return nil if @json_rpc_handler.error? + @json_rpc_handler.reply_error(@id, CODE_INTERNAL_ERROR, message || MSG_INTERNAL_ERROR) end def reply_method_not_found(message = nil) - return nil if @conn.error? - @conn.reply_error(@id, CODE_METHOD_NOT_FOUND, message || MSG_METHOD_NOT_FOUND) + return nil if @json_rpc_handler.connection_error? + @json_rpc_handler.reply_error(@id, CODE_METHOD_NOT_FOUND, message || MSG_METHOD_NOT_FOUND) end def reply_invalid_params(message = nil) - return nil if @conn.error? - @conn.reply_error(@id, CODE_INVALID_PARAMS, message || MSG_INVALID_PARAMS) + return nil if @json_rpc_handler.connection_error? + @json_rpc_handler.reply_error(@id, CODE_INVALID_PARAMS, message || MSG_INVALID_PARAMS) end def reply_custom_error(code, message) - return nil if @conn.error? + return nil if @json_rpc_handler.connection_error? unless code.is_a?(Integer) && (-32099..-32000).cover?(code) raise ArgumentError, 'code must be an integer between -32099 and -32000' end - @conn.reply_error(@id, code, message) + @json_rpc_handler.reply_error(@id, code, message) end end end diff --git a/lib/puppet-languageserver/message_router.rb b/lib/puppet-languageserver/message_router.rb index aef24e22..0c55a598 100644 --- a/lib/puppet-languageserver/message_router.rb +++ b/lib/puppet-languageserver/message_router.rb @@ -38,6 +38,19 @@ def self.document_version(uri) def self.document_uris @doc_mutex.synchronize { @documents.keys.dup } end + + def self.document_type(uri) + case uri + when /\/Puppetfile$/i + :puppetfile + when /\.pp$/i + :manifest + when /\.epp$/i + :epp + else + :unknown + end + end end module CrashDump @@ -90,30 +103,18 @@ def self.write_crash_file(err, filename = nil, additional = {}) nil end - class MessageRouter < JSONRPCHandler - def initialize(*options) - super(*options) + class MessageRouter + attr_accessor :json_rpc_handler - @workspace = options.first[:workspace] unless options.compact.empty? + def initialize(options = {}) + options = {} if options.nil? + @workspace = options[:workspace] end def documents PuppetLanguageServer::DocumentStore end - def document_type(uri) - case uri - when /\/Puppetfile$/i - :puppetfile - when /\.pp$/i - :manifest - when /\.epp$/i - :epp - else - :unknown - end - end - def receive_request(request) case request.rpc_method when 'initialize' @@ -157,7 +158,7 @@ def receive_request(request) when 'puppet/compileNodeGraph' file_uri = request.params['external'] - unless document_type(file_uri) == :manifest + unless documents.document_type(file_uri) == :manifest request.reply_result(LanguageServer::PuppetCompilation.create('error' => 'Files of this type can not be used to create a node graph.')) return end @@ -191,7 +192,7 @@ def receive_request(request) file_uri = formatted_request['documentUri'] content = documents.document(file_uri) - case document_type(file_uri) + case documents.document_type(file_uri) when :manifest changes, new_content = PuppetLanguageServer::DocumentValidator.fix_validate_errors(content, @workspace) else @@ -220,7 +221,7 @@ def receive_request(request) char_num = request.params['position']['character'] content = documents.document(file_uri) begin - case document_type(file_uri) + case documents.document_type(file_uri) when :manifest request.reply_result(PuppetLanguageServer::CompletionProvider.complete(content, line_num, char_num)) else @@ -246,7 +247,7 @@ def receive_request(request) char_num = request.params['position']['character'] content = documents.document(file_uri) begin - case document_type(file_uri) + case documents.document_type(file_uri) when :manifest request.reply_result(PuppetLanguageServer::HoverProvider.resolve(content, line_num, char_num)) else @@ -263,7 +264,7 @@ def receive_request(request) char_num = request.params['position']['character'] content = documents.document(file_uri) begin - case document_type(file_uri) + case documents.document_type(file_uri) when :manifest request.reply_result(PuppetLanguageServer::DefinitionProvider.find_definition(content, line_num, char_num)) else @@ -289,7 +290,7 @@ def receive_notification(method, params) when 'exit' PuppetLanguageServer.log_message(:info, 'Received exit notification. Closing connection to client...') - close_connection + @json_rpc_handler.close_connection when 'textDocument/didOpen' PuppetLanguageServer.log_message(:info, 'Received textDocument/didOpen notification.') @@ -297,7 +298,7 @@ def receive_notification(method, params) content = params['textDocument']['text'] doc_version = params['textDocument']['version'] documents.set_document(file_uri, content, doc_version) - PuppetLanguageServer::ValidationQueue.enqueue(file_uri, doc_version, @workspace, self) + PuppetLanguageServer::ValidationQueue.enqueue(file_uri, doc_version, @workspace, @json_rpc_handler) when 'textDocument/didClose' PuppetLanguageServer.log_message(:info, 'Received textDocument/didClose notification.') @@ -310,7 +311,7 @@ def receive_notification(method, params) content = params['contentChanges'][0]['text'] # TODO: Bad hardcoding zero doc_version = params['textDocument']['version'] documents.set_document(file_uri, content, doc_version) - PuppetLanguageServer::ValidationQueue.enqueue(file_uri, doc_version, @workspace, self) + PuppetLanguageServer::ValidationQueue.enqueue(file_uri, doc_version, @workspace, @json_rpc_handler) when 'textDocument/didSave' PuppetLanguageServer.log_message(:info, 'Received textDocument/didSave notification.') diff --git a/lib/puppet-languageserver/validation_queue.rb b/lib/puppet-languageserver/validation_queue.rb index 7b08dfcc..72f50f0d 100644 --- a/lib/puppet-languageserver/validation_queue.rb +++ b/lib/puppet-languageserver/validation_queue.rb @@ -12,7 +12,7 @@ module ValidationQueue # Enqueue a file to be validated def self.enqueue(file_uri, doc_version, workspace, connection_object) - document_type = connection_object.document_type(file_uri) + document_type = PuppetLanguageServer::DocumentStore.document_type(file_uri) unless %i[manifest epp].include?(document_type) # Can't validate these types so just emit an empty validation result @@ -48,7 +48,7 @@ def self.enqueue(file_uri, doc_version, workspace, connection_object) # Synchronously validate a file def self.validate_sync(file_uri, doc_version, workspace, connection_object) - document_type = connection_object.document_type(file_uri) + document_type = PuppetLanguageServer::DocumentStore.document_type(file_uri) content = documents.document(file_uri, doc_version) return nil if content.nil? result = validate(document_type, content, workspace) diff --git a/spec/debugserver/integration/puppet-debugserver/message_router_spec.rb b/spec/debugserver/integration/puppet-debugserver/json_handler_spec.rb similarity index 84% rename from spec/debugserver/integration/puppet-debugserver/message_router_spec.rb rename to spec/debugserver/integration/puppet-debugserver/json_handler_spec.rb index 3bc9f7d4..d75dda60 100644 --- a/spec/debugserver/integration/puppet-debugserver/message_router_spec.rb +++ b/spec/debugserver/integration/puppet-debugserver/json_handler_spec.rb @@ -1,12 +1,10 @@ require 'spec_debug_helper' require 'json' -# A stubbed JSON Handler which is used to remember data sent to the client -class StubbedMessageRouter < PuppetDebugServer::MessageRouter +class StubbedSimpleServerConnection < PuppetEditorServices::SimpleServerConnectionBase attr_reader :data_stream - def initialize(*options) - super(*options) + def initialize() @data_stream = [] end @@ -32,8 +30,11 @@ def next_seq_id @tx_seq_id += 1 end -describe 'PuppetDebugServer::MessageRouter' do - let(:subject) { StubbedMessageRouter.new({}) } +describe 'PuppetDebugServer::JSONHandler' do + let(:subject_options) {{ + connection: StubbedSimpleServerConnection.new + }} + let(:subject) { PuppetDebugServer::JSONHandler.new(subject_options) } before(:each) { allow(subject).to receive(:close_connection_after_writing).and_return(true) @@ -47,8 +48,7 @@ def next_seq_id it 'should respond with the correct capabilities' do subject.parse_data(initialize_request) - response = data_from_request_seq_id(subject, 1) - + response = data_from_request_seq_id(subject.client_connection, 1) expect(response['body']['supportsConfigurationDoneRequest']).to be true expect(response['body']['supportsFunctionBreakpoints']).to be true expect(response['body']['supportsRestartRequest']).to be false @@ -61,7 +61,7 @@ def next_seq_id it 'should respond with an Initialized event' do subject.parse_data(initialize_request) - response = data_from_event_name(subject, 'initialized') + response = data_from_event_name(subject.client_connection, 'initialized') expect(response).to_not be nil end @@ -86,7 +86,7 @@ def next_seq_id 8 => 'stepOut', 9 => 'next', }.each do |seq_id, command| - response = data_from_request_seq_id(subject, seq_id) + response = data_from_request_seq_id(subject.client_connection, seq_id) expect(response).to_not be nil expect(response['success']).to be false expect(response['command']).to eq(command) @@ -101,7 +101,7 @@ def next_seq_id last_seq_id = nil [10, 20, 30].each do |req_seq_id| - response = data_from_request_seq_id(subject, req_seq_id) + response = data_from_request_seq_id(subject.client_connection, req_seq_id) expect(response).to_not be nil expect(response['success']).to be false unless last_seq_id.nil? diff --git a/spec/languageserver/integration/puppet-languageserver/message_router_spec.rb b/spec/languageserver/integration/puppet-languageserver/message_router_spec.rb index b8270ad0..9f2165e5 100644 --- a/spec/languageserver/integration/puppet-languageserver/message_router_spec.rb +++ b/spec/languageserver/integration/puppet-languageserver/message_router_spec.rb @@ -1,8 +1,12 @@ require 'spec_helper' describe 'message_router' do - let(:subject_options) { nil } - let(:subject) { PuppetLanguageServer::MessageRouter.new(subject_options) } + let(:subject_options) {} + let(:subject) do + result = PuppetLanguageServer::MessageRouter.new(subject_options) + result.json_rpc_handler = MockJSONRPCHandler.new + result + end describe '#receive_request' do let(:documents) {{ @@ -104,7 +108,7 @@ @default_crash_file = PuppetLanguageServer::CrashDump.default_crash_file File.delete(@default_crash_file) if File.exists?(@default_crash_file) - expect(subject).to receive(:close_connection).and_raise('MockError') + expect(subject.json_rpc_handler).to receive(:close_connection).and_raise('MockError') end it 'should create a default crash dump file' do diff --git a/spec/languageserver/integration/puppet-languageserver_spec.rb b/spec/languageserver/integration/puppet-languageserver_spec.rb index 711d4cc2..6e90fd5a 100644 --- a/spec/languageserver/integration/puppet-languageserver_spec.rb +++ b/spec/languageserver/integration/puppet-languageserver_spec.rb @@ -3,11 +3,11 @@ require 'socket' SERVER_TCP_PORT = 8081 -SERVER_HOST = '127.0.0.1' +SERVER_HOST = '127.0.0.1'.freeze -def start_tcp_server(start_options = ['--no-preload','--timeout=5']) +def start_tcp_server(start_options = ['--timeout=5']) cmd = "ruby puppet-languageserver #{start_options.join(' ')} --port=#{SERVER_TCP_PORT} --ip=0.0.0.0" - + stdin, stdout, stderr, wait_thr = Open3.popen3(cmd) # Wait for the Language Server to indicate it started line = nil @@ -20,7 +20,7 @@ def start_tcp_server(start_options = ['--no-preload','--timeout=5']) wait_thr end -def start_stdio_server(start_options = ['--no-preload','--timeout=5']) +def start_stdio_server(start_options = ['--timeout=5']) cmd = "ruby puppet-languageserver #{start_options.join(' ')} --stdio" stdin, stdout, stderr, wait_thr = Open3.popen3(cmd) diff --git a/spec/languageserver/spec_helper.rb b/spec/languageserver/spec_helper.rb index bef3e9a3..7ea9a38f 100644 --- a/spec/languageserver/spec_helper.rb +++ b/spec/languageserver/spec_helper.rb @@ -46,31 +46,20 @@ def wait_for_puppet_loading end # Mock ojects -class MockJSONRPCHandler < PuppetLanguageServer::JSONRPCHandler - attr_accessor :socket - attr_accessor :simple_tcp_server - - def post_init - end - - def unbind - end - - def receive_data(data) - end - - def error? - false - end - +class MockConnection < PuppetEditorServices::SimpleServerConnectionBase def send_data(data) true end +end - def close_connection_after_writing +class MockJSONRPCHandler < PuppetLanguageServer::JSONRPCHandler + def initialize(options = {}) + super(options) + + @client_connection = MockConnection.new end - def close_connection + def receive_data(data) end end diff --git a/spec/languageserver/unit/puppet-languageserver/message_router_spec.rb b/spec/languageserver/unit/puppet-languageserver/message_router_spec.rb index 39a31f02..933ac811 100644 --- a/spec/languageserver/unit/puppet-languageserver/message_router_spec.rb +++ b/spec/languageserver/unit/puppet-languageserver/message_router_spec.rb @@ -7,8 +7,12 @@ UNKNOWN_FILENAME = 'file:///I_do_not_work.exe' ERROR_CAUSING_FILE_CONTENT = "file_content which causes errros\n <%- Wee!\n class 'foo' {'" - let(:subject_options) { nil } - let(:subject) { PuppetLanguageServer::MessageRouter.new(subject_options) } + let(:subject_options) {} + let(:subject) do + result = PuppetLanguageServer::MessageRouter.new(subject_options) + result.json_rpc_handler = MockJSONRPCHandler.new + result + end describe '#documents' do it 'should respond to documents method' do @@ -21,8 +25,10 @@ let(:request_rpc_method) { nil } let(:request_params) { {} } let(:request_id) { 0 } - let(:request) { PuppetLanguageServer::JSONRPCHandler::Request.new( - request_connection,request_id,request_rpc_method,request_params) } + let(:request) do + PuppetLanguageServer::JSONRPCHandler::Request.new( + request_connection, request_id, request_rpc_method, request_params) + end before(:each) do allow(PuppetLanguageServer).to receive(:log_message) @@ -467,7 +473,7 @@ context 'given a notification that raises an error' do let(:notification_method) { 'exit' } before(:each) do - expect(subject).to receive(:close_connection).and_raise('MockError') + expect(subject.json_rpc_handler).to receive(:close_connection).and_raise('MockError') allow(PuppetLanguageServer::CrashDump).to receive(:write_crash_file) end @@ -507,7 +513,7 @@ end it 'should close the connection' do - expect(subject).to receive(:close_connection) + expect(subject.json_rpc_handler).to receive(:close_connection) subject.receive_notification(notification_method, notification_params) end