Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

(GH-245) Use object cache for fact data #246

Merged
merged 3 commits into from
Apr 29, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions lib/puppet-languageserver/facter_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
2 changes: 1 addition & 1 deletion lib/puppet-languageserver/message_handler.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
9 changes: 0 additions & 9 deletions lib/puppet-languageserver/puppet_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
10 changes: 0 additions & 10 deletions lib/puppet-languageserver/sidecar_queue.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down
9 changes: 0 additions & 9 deletions lib/puppet_languageserver_sidecar.rb
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,6 @@ def self.require_gems(options)
workspace_functions
workspace_types
facts
facts_all
].freeze

class CommandLineParser
Expand Down Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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') }
Expand Down
18 changes: 18 additions & 0 deletions spec/languageserver/acceptance/end_to_end_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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 | - | - |
Expand Down Expand Up @@ -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'))
Expand Down
9 changes: 9 additions & 0 deletions spec/languageserver/spec_editor_client.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down