(GH-177) Add unregistering of capabilities

Previously client capabilities could be dynamically registered.  This commit
adds the logic and track to dynamically unregister a capability;
* Adds the unregister_capability and parse_unregister_capability_response!
  methods
* Adds the client/unregisterCapability notification handler
* Adds tests for unregistering

Author: glennsarti

lib/puppet-languageserver/language_client.rb

@@ -44,7 +44,7 @@ def parse_lsp_configuration_settings!(settings = [{}])
     end
 
     def capability_registrations(method)
-      return [{ :registered => false, :state => :complete }] if @registrations[method].nil?
+      return [{ :registered => false, :state => :complete }] if @registrations[method].nil? || @registrations[method].empty?
       @registrations[method].dup
     end
 
@@ -56,7 +56,7 @@ def register_capability(method, options = {})
       if @registrations[method] && @registrations[method].select { |i| i[:state] == :pending }.count > 0
         # The protocol doesn't specify whether this is allowed and is probably per client specific. For the moment we will allow
         # the registration to be sent but log a message that something may be wrong.
-        PuppetLanguageServer.log_message(:warn, "A dynamic registration for the #{method} method is already in progress")
+        PuppetLanguageServer.log_message(:warn, "A dynamic registration/deregistration for the #{method} method is already in progress")
       end
 
       params = LSP::RegistrationParams.new.from_h!('registrations' => [])
@@ -70,6 +70,31 @@ def register_capability(method, options = {})
       true
     end
 
+    def unregister_capability(method)
+      if @registrations[method].nil?
+        PuppetLanguageServer.log_message(:debug, "No registrations to deregister for the #{method}")
+        return true
+      end
+
+      params = LSP::UnregistrationParams.new.from_h!('unregisterations' => [])
+      @registrations[method].each do |reg|
+        next if reg[:id].nil?
+        PuppetLanguageServer.log_message(:warn, "A dynamic registration/deregistration for the #{method} method, with id #{reg[:id]} is already in progress") if reg[:state] == :pending
+        # Ignore registrations that don't need to be unregistered
+        next if reg[:state] == :complete && !reg[:registered]
+        params.unregisterations << LSP::Unregistration.new.from_h!('id' => reg[:id], 'method' => method)
+        reg[:state] = :pending
+      end
+
+      if params.unregisterations.count.zero?
+        PuppetLanguageServer.log_message(:debug, "Nothing to deregister for the #{method} method")
+        return true
+      end
+
+      message_router.json_rpc_handler.send_client_request('client/unregisterCapability', params)
+      true
+    end
+
     def parse_register_capability_response!(response, original_request)
       raise 'Response is not from client/registerCapability request' unless original_request['method'] == 'client/registerCapability'
 
@@ -97,6 +122,30 @@ def parse_register_capability_response!(response, original_request)
       true
     end
 
+    def parse_unregister_capability_response!(response, original_request)
+      raise 'Response is not from client/unregisterCapability request' unless original_request['method'] == 'client/unregisterCapability'
+
+      unless response.key?('result')
+        original_request['params'].unregisterations.each do |reg|
+          # Mark the registration as completed and failed
+          @registrations[reg.method__lsp] = [] if @registrations[reg.method__lsp].nil?
+          @registrations[reg.method__lsp].select { |i| i[:id] == reg.id && i[:registered] }.each { |i| i[:state] = :complete }
+          @registrations[reg.method__lsp].delete_if { |i| i[:id] == reg.id && !i[:registered] }
+        end
+        return true
+      end
+
+      original_request['params'].unregisterations.each do |reg|
+        PuppetLanguageServer.log_message(:info, "Succesfully dynamically unregistered the #{reg.method__lsp} method")
+
+        # Remove registrations
+        @registrations[reg.method__lsp] = [] if @registrations[reg.method__lsp].nil?
+        @registrations[reg.method__lsp].delete_if { |i| i[:id] == reg.id }
+      end
+
+      true
+    end
+
     private
 
     def new_request_id

lib/puppet-languageserver/message_router.rb

@@ -321,6 +321,8 @@ def receive_response(response, original_request)
       case original_request['method']
       when 'client/registerCapability'
         client.parse_register_capability_response!(response, original_request)
+      when 'client/unregisterCapability'
+        client.parse_unregister_capability_response!(response, original_request)
       when 'workspace/configuration'
         return unless receive_response_succesful?(response)
         client.parse_lsp_configuration_settings!(response['result'])

spec/languageserver/unit/puppet-languageserver/language_client_spec.rb

@@ -331,6 +331,72 @@
     end
   end
 
+  describe '#unregister_capability' do
+    let(:method_name) { 'mockMethod' }
+
+    before(:each) do
+      # Mock an already succesful registration
+      subject.instance_variable_set(:@registrations, {
+        method_name => [{ :id => 'id001', :state => :complete, :registered => true }]
+      })
+    end
+
+    it 'should send a client request and return true' do
+      expect(json_rpc_handler).to receive(:send_client_request).with('client/unregisterCapability', Object)
+      expect(subject.unregister_capability(method_name)).to eq(true)
+    end
+
+    it 'should include the method to register' do
+      subject.unregister_capability(method_name)
+      expect(json_rpc_handler.connection.buffer).to include("\"method\":\"#{method_name}\"")
+    end
+
+    it 'should log a message if a registration is already in progress' do
+      allow(json_rpc_handler).to receive(:send_client_request)
+      expect(PuppetLanguageServer).to receive(:log_message).with(:warn, /#{method_name}/)
+
+      subject.unregister_capability(method_name)
+      subject.unregister_capability(method_name)
+    end
+
+    it 'should not log a message if a previous registration completed' do
+      req_method_name = nil
+      req_method_params = nil
+      # Remember the registration so we can fake a response later
+      allow(json_rpc_handler).to receive(:send_client_request) do |n, p|
+        req_method_name = n
+        req_method_params = p
+      end
+
+      expect(PuppetLanguageServer).to_not receive(:log_message).with(:warn, /#{method_name}/)
+      # Send as registration request
+      subject.unregister_capability(method_name)
+      # Mock a valid response
+      response = { 'jsonrpc'=>'2.0', 'id'=> 0, 'result' => nil }
+      original_request = { 'jsonrpc'=>'2.0', 'id' => 0, 'method' => req_method_name, 'params' => req_method_params }
+
+      subject.parse_unregister_capability_response!(response, original_request)
+
+      subject.unregister_capability(method_name)
+    end
+
+    it 'should not deregister methods that have not been registerd' do
+      expect(json_rpc_handler).to_not receive(:send_client_request)
+
+      subject.unregister_capability('unknown')
+    end
+
+    it 'should not deregister methods that are no longer registerd' do
+      expect(json_rpc_handler).to_not receive(:send_client_request)
+
+      subject.instance_variable_set(:@registrations, {
+        method_name => [{ :id => 'id001', :state => :complete, :registered => false }]
+      })
+
+      subject.unregister_capability(method_name)
+    end
+  end
+
   describe '#parse_register_capability_response!' do
     let(:request_id) { 0 }
     let(:response_result) { nil }
@@ -390,4 +456,84 @@
       end
     end
   end
+
+  describe '#parse_unregister_capability_response!' do
+    let(:request_id) { 0 }
+    let(:response_result) { nil }
+    let(:response) { {'jsonrpc'=>'2.0', 'id'=> request_id, 'result' => response_result } }
+    let(:original_request) { {'jsonrpc'=>'2.0', 'id'=> request_id, 'method' => request_method, 'params' => request_params} }
+    let(:method_name) { 'validMethod' }
+    let(:initial_registration) { true }
+
+    before(:each) do
+      # Mock an already succesful registration
+      subject.instance_variable_set(:@registrations, {
+        method_name => [{ :id => 'id001', :state => :complete, :registered => initial_registration }]
+      })
+    end
+
+    context 'Given an original request that is not an unregistration' do
+      let(:request_method) { 'mockMethod' }
+      let(:request_params) { {} }
+
+      it 'should raise an error if the original request was not a registration' do
+        expect{ subject.parse_unregister_capability_response!(response, original_request) }.to raise_error(/client\/unregisterCapability/)
+      end
+    end
+
+    context 'Given a valid original request' do
+      let(:request_method) { 'client/unregisterCapability' }
+      let(:request_params) do
+        params = LSP::UnregistrationParams.new.from_h!('unregisterations' => [])
+        params.unregisterations << LSP::Unregistration.new.from_h!('id' => 'id001', 'method' => method_name)
+        params
+      end
+
+      before(:each) do
+        # Mimic an unregistration that is in progress
+        subject.instance_variable_set(:@registrations, {
+          method_name => [{ :id => 'id001', :state => :pending, :registered => initial_registration }]
+        })
+      end
+
+      context 'that failed' do
+        before(:each) do
+          response.delete('result') if response.key?('result')
+          response['error'] = { 'code' => -1, 'message' => 'mock message' }
+        end
+
+        context 'and was previously registered' do
+          it 'should retain that it is registered' do
+            subject.parse_unregister_capability_response!(response, original_request)
+
+            expect(subject.capability_registrations(method_name)).to eq([{:id=>"id001", :registered=>true, :state=>:complete}])
+          end
+        end
+
+        context 'and was not previously registered' do
+          let(:initial_registration) { false }
+
+          it 'should no longer be in the registration list' do
+            subject.parse_unregister_capability_response!(response, original_request)
+
+            expect(subject.capability_registrations(method_name)).to eq([{ :registered => false, :state => :complete }])
+          end
+        end
+      end
+
+      context 'that succeeded' do
+        it 'should log the registration' do
+          expect(PuppetLanguageServer).to receive(:log_message).with(:info, /validMethod/)
+
+          subject.parse_unregister_capability_response!(response, original_request)
+        end
+
+        it 'should no longer be in the registration list' do
+          subject.parse_unregister_capability_response!(response, original_request)
+
+          expect(subject.capability_registrations(method_name)).to eq([{ :registered => false, :state => :complete }])
+        end
+      end
+    end
+  end
 end