(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

@@ -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

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

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}")

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,15 +271,15 @@ 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(
             {
               'success' => false,
               '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