Merge pull request #246 from glennsarti/gh245refactorfacts

(GH-245) Use object cache for fact data

Author: James Pogran

lib/puppet-languageserver/facter_helper.rb

@@ -46,5 +46,11 @@ def self.fact_names
       return [] if @facts_loaded == false
       cache.object_names_by_section(:fact).map(&:to_s)
     end
+
+    def self.facts_to_hash
+      fact_hash = {}
+      cache.objects_by_section(:fact) { |factname, fact| fact_hash[factname.to_s] = fact.value }
+      fact_hash
+    end
   end
 end

lib/puppet-languageserver/message_handler.rb

@@ -46,7 +46,7 @@ def request_puppet_getversion(_, _json_rpc_message)
     end
 
     def request_puppet_getfacts(_, _json_rpc_message)
-      results = PuppetLanguageServer::PuppetHelper.get_all_facts(documents.store_root_path)
+      results = PuppetLanguageServer::FacterHelper.facts_to_hash
       LSP::PuppetFactResponse.new('facts' => results)
     end
 

lib/puppet-languageserver/puppet_helper.rb

@@ -54,15 +54,6 @@ def self.get_node_graph(content, local_workspace)
       end
     end
 
-    def self.get_all_facts(local_workspace)
-      ap = PuppetLanguageServer::Sidecar::Protocol::ActionParams.new
-
-      args = ['--action-parameters=' + ap.to_json]
-      args << "--local-workspace=#{local_workspace}" unless local_workspace.nil?
-
-      sidecar_queue.execute_sync('facts_all', args, false)
-    end
-
     def self.get_puppet_resource(typename, title, local_workspace)
       ap = PuppetLanguageServer::Sidecar::Protocol::ActionParams.new
       ap['typename'] = typename

lib/puppet-languageserver/sidecar_queue.rb

@@ -118,16 +118,6 @@ def execute_sync(action, additional_args, handle_errors = false)
 
         PuppetLanguageServer::FacterHelper.assert_facts_loaded
 
-      when 'facts_all'
-        list = {}
-        PuppetLanguageServer::Sidecar::Protocol::FactList
-          .new
-          .from_json!(result)
-          .map { |element| list[element.key] = element.value }
-        PuppetLanguageServer.log_message(:debug, "SidecarQueue Thread: facts returned #{list.count} items")
-
-        return list
-
       when 'node_graph'
         return PuppetLanguageServer::Sidecar::Protocol::PuppetNodeGraph.new.from_json!(result)
 

lib/puppet_languageserver_sidecar.rb

@@ -119,7 +119,6 @@ def self.require_gems(options)
     workspace_functions
     workspace_types
     facts
-    facts_all
   ].freeze
 
   class CommandLineParser
@@ -400,14 +399,6 @@ def self.execute(options)
       inject_workspace_as_environment unless injected
       PuppetLanguageServerSidecar::FacterHelper.retrieve_facts(cache)
 
-    when 'facts_all'
-      # Can't cache for facts
-      cache = PuppetLanguageServerSidecar::Cache::Null.new
-      # Inject the workspace etc. if present
-      injected = inject_workspace_as_module
-      inject_workspace_as_environment unless injected
-      PuppetLanguageServerSidecar::FacterHelper.retrieve_facts(cache)
-
     else
       log_message(:error, "Unknown action #{options[:action]}. Expected one of #{ACTION_LIST}")
     end

spec/languageserver-sidecar/integration/puppet-languageserver-sidecar/facter_helper_spec.rb

@@ -51,23 +51,6 @@ def run_sidecar(cmd_options)
     end
   end
 
-  describe 'when running facts_all action' do
-    let (:cmd_options) { ['--action', 'facts_all'] }
-
-    it 'should return a deserializable facts object with all default facts' do
-      result = run_sidecar(cmd_options)
-      deserial = PuppetLanguageServer::Sidecar::Protocol::FactList.new
-      expect { deserial.from_json!(result) }.to_not raise_error
-      default_fact_names.each do |name|
-        expect(deserial).to contain_child_with_key(name)
-      end
-
-      module_fact_names.each do |name|
-        expect(deserial).to_not contain_child_with_key(name)
-      end
-    end
-  end
-
   context 'given a workspace containing a module' do
     # Test fixtures used is fixtures/valid_module_workspace
     let(:workspace) { File.join($fixtures_dir, 'valid_module_workspace') }

spec/languageserver/acceptance/end_to_end_spec.rb

@@ -14,6 +14,7 @@
 # Diagnostics response       |      X      |        |              |
 # Hover (Class)              |      X      |        |              |
 # Puppet resource            |      X      |        |              |
+# Puppet facts               |      X      |        |              |
 # Node graph preview         |      X      |        |              |
 # Completion (Typing)        |      X      |    -   |       -      |
 # Completion (Invoked)       |      X      |    -   |       -      |
@@ -127,6 +128,23 @@ def path_to_uri(path)
       expect(result['result']['contents']).not_to be_nil
       expect(result['result']['contents']).not_to be_empty
 
+      # Puppet Facts request
+      @client.clear_messages!
+      @client.send_data(@client.puppet_getfacts_request(@client.next_seq_id))
+      expect(@client).to receive_message_with_request_id_within_timeout([@client.current_seq_id, 15])
+      result = @client.data_from_request_seq_id(@client.current_seq_id)
+      #   Expect there to be some facts
+      expect(result['result']['facts']).not_to be_nil
+      expect(result['result']['facts']).not_to be_empty
+      #   Expect core facts. Ref https://puppet.com/docs/facter/latest/core_facts.html
+      %w[facterversion kernel os system_uptime].each do |factname|
+        expect(result['result']['facts'][factname]).not_to be_nil
+        expect(result['result']['facts'][factname]).not_to be_empty
+      end
+      #   Expect nested core facts. Ref https://puppet.com/docs/facter/latest/core_facts.html
+      expect(result['result']['facts']['os']['release']).not_to be_nil
+      expect(result['result']['facts']['os']['release']).not_to be_empty
+
       # Puppet Resource request
       @client.clear_messages!
       @client.send_data(@client.puppet_getresource_request(@client.next_seq_id, 'user'))

spec/languageserver/spec_editor_client.rb

@@ -191,6 +191,15 @@ def puppet_getversion_request(seq_id)
     })
   end
 
+  def puppet_getfacts_request(seq_id)
+    ::JSON.generate({
+      'jsonrpc' => '2.0',
+      'id'      => seq_id,
+      'method'  => 'puppet/getFacts',
+      'params'  => {}
+    })
+  end
+
   def puppet_getresource_request(seq_id, type_name)
     ::JSON.generate({
       'jsonrpc' => '2.0',