From 4167b9b9a278ffebd41f41ecb6a999cb3fcedea6 Mon Sep 17 00:00:00 2001 From: Glenn Sarti Date: Sat, 16 Sep 2017 20:47:27 -0700 Subject: [PATCH] (GH-136) Add loading information to the Puppet Version command Previously the Language client had no way to know whether the langauge server had completed loading of facts, types and functions. This caused requests to be blocked because of mutex locks while this information is being loaded. This commit; - Adds three properties which the client can use to determine the loading status of various parts of the Language Server, with unit tests - The puppet and facter helpers were modified to expose a *_loaded? method - The puppet and facter helpers will now return empty datasets if they in the process of being loaded on another thread. This speeds up responses to the client --- lib/languageserver/puppet_version.rb | 14 ++++++++--- lib/puppet-languageserver/facter_helper.rb | 8 +++++++ lib/puppet-languageserver/message_router.rb | 8 +++++-- lib/puppet-languageserver/puppet_helper.rb | 23 ++++++++++++++++++- .../message_router_spec.rb | 15 ++++++++++++ 5 files changed, 62 insertions(+), 6 deletions(-) diff --git a/lib/languageserver/puppet_version.rb b/lib/languageserver/puppet_version.rb index 9d03e112..938293dd 100644 --- a/lib/languageserver/puppet_version.rb +++ b/lib/languageserver/puppet_version.rb @@ -1,9 +1,12 @@ module LanguageServer module PuppetVersion # export interface PuppetVersionDetails { - # puppetVersion: string; - # facterVersion: string; - # languageServerVersion: string; + # puppetVersion: string; + # facterVersion: string; + # languageServerVersion: string; + # factsLoaded: boolean; + # functionsLoaded: boolean; + # typesLoaded: boolean; # } def self.create(options) @@ -13,6 +16,11 @@ def self.create(options) result['puppetVersion'] = options['puppetVersion'] result['facterVersion'] = options['facterVersion'] + + result['factsLoaded'] = options['factsLoaded'] unless options['factsLoaded'].nil? + result['functionsLoaded'] = options['functionsLoaded'] unless options['functionsLoaded'].nil? + result['typesLoaded'] = options['typesLoaded'] unless options['typesLoaded'].nil? + result['languageServerVersion'] = PuppetLanguageServer.version result diff --git a/lib/puppet-languageserver/facter_helper.rb b/lib/puppet-languageserver/facter_helper.rb index be38320f..294e21c6 100644 --- a/lib/puppet-languageserver/facter_helper.rb +++ b/lib/puppet-languageserver/facter_helper.rb @@ -1,6 +1,7 @@ module PuppetLanguageServer module FacterHelper @ops_lock = Mutex.new + @facts_loaded = nil def self.reset @ops_lock.synchronize do @@ -14,6 +15,10 @@ def self.load_facts_async end end + def self.facts_loaded? + @facts_loaded.nil? ? false : @facts_loaded + end + def self.load_facts @ops_lock.synchronize do _load_facts @@ -21,6 +26,7 @@ def self.load_facts end def self.facts + return {} if @facts_loaded == false @ops_lock.synchronize do _load_facts if @fact_hash.nil? @fact_hash.clone @@ -30,6 +36,7 @@ def self.facts # DO NOT ops_lock on any of these methods # deadlocks will ensue! def self._reset + @facts_loaded = nil Facter.reset @fact_hash = nil end @@ -40,6 +47,7 @@ def self._load_facts Facter.loadfacts @fact_hash = Facter.to_hash PuppetLanguageServer.log_message(:debug, "[FacterHelper::_load_facts] Finished loading #{@fact_hash.keys.count} facts") + @facts_loaded = true end private_class_method :_load_facts end diff --git a/lib/puppet-languageserver/message_router.rb b/lib/puppet-languageserver/message_router.rb index 2af6a0f4..3b90b282 100644 --- a/lib/puppet-languageserver/message_router.rb +++ b/lib/puppet-languageserver/message_router.rb @@ -43,8 +43,12 @@ def receive_request(request) request.reply_result(nil) when 'puppet/getVersion' - request.reply_result(LanguageServer::PuppetVersion.create('puppetVersion' => Puppet.version, - 'facterVersion' => Facter.version)) + request.reply_result(LanguageServer::PuppetVersion.create('puppetVersion' => Puppet.version, + 'facterVersion' => Facter.version, + 'factsLoaded' => PuppetLanguageServer::FacterHelper.facts_loaded?, + 'functionsLoaded' => PuppetLanguageServer::PuppetHelper.functions_loaded?, + 'typesLoaded' => PuppetLanguageServer::PuppetHelper.types_loaded? + )) when 'puppet/getResource' type_name = request.params['typename'] diff --git a/lib/puppet-languageserver/puppet_helper.rb b/lib/puppet-languageserver/puppet_helper.rb index ed1f017b..c74ccf87 100644 --- a/lib/puppet-languageserver/puppet_helper.rb +++ b/lib/puppet-languageserver/puppet_helper.rb @@ -8,7 +8,9 @@ module PuppetHelper @ops_lock_funcs = Mutex.new @types_hash = nil @function_module = nil - + @types_loaded = nil + @functions_loaded = nil + def self.reset @ops_lock_types.synchronize do @ops_lock_funcs.synchronize do @@ -35,6 +37,10 @@ def self.load_types_async end end + def self.types_loaded? + @types_loaded.nil? ? false : @types_loaded + end + def self.load_types @ops_lock_types.synchronize do _load_types @@ -43,6 +49,7 @@ def self.load_types def self.get_type(name) result = nil + return result if @types_loaded == false @ops_lock_types.synchronize do _load_types if @types_hash.nil? result = @types_hash[name.intern] @@ -52,6 +59,7 @@ def self.get_type(name) def self.type_names result = [] + return result if @types_loaded == false @ops_lock_types.synchronize do _load_types if @types_hash.nil? result = @types_hash.keys.map(&:to_s) @@ -60,6 +68,10 @@ def self.type_names end # Functions + def self.functions_loaded? + @functions_loaded.nil? ? false : @functions_loaded + end + def self.load_functions @ops_lock_funcs.synchronize do _load_functions if @function_module.nil? @@ -74,6 +86,7 @@ def self.load_functions_async def self.functions result = [] + return result if @functions_loaded == false @ops_lock_funcs.synchronize do _load_functions if @function_module.nil? result = @function_module.all_function_info.dup @@ -83,6 +96,7 @@ def self.functions def self.function(name) result = nil + return result if @functions_loaded == false @ops_lock_funcs.synchronize do _load_functions if @function_module.nil? result = @function_module.all_function_info[name.intern] @@ -92,6 +106,7 @@ def self.function(name) def self.function_names result = [] + return result if @functions_loaded == false @ops_lock_funcs.synchronize do _load_functions if @function_module.nil? result = @function_module.all_function_info.keys.map(&:to_s) @@ -104,6 +119,8 @@ def self.function_names def self._reset @types_hash = nil @function_module = nil + @types_loaded = nil + @functions_loaded = nil end private_class_method :_reset @@ -118,6 +135,7 @@ def self.prune_resource_parameters(resources) private_class_method :prune_resource_parameters def self._load_types + @types_loaded = false @types_hash = {} # This is an expensive call # From https://github.com/puppetlabs/puppet/blob/ebd96213cab43bb2a8071b7ac0206c3ed0be8e58/lib/puppet/metatype/manager.rb#L182-L189 @@ -143,11 +161,13 @@ def self._load_types type_count = @types_hash.count PuppetLanguageServer.log_message(:debug, "[PuppetHelper::_load_types] Finished loading #{type_count} types") + @types_loaded = true nil end private_class_method :_load_types def self._load_functions + @functions_loaded = false autoloader = Puppet::Parser::Functions.autoloader # This is an expensive call @@ -165,6 +185,7 @@ def self._load_functions function_count = @function_module.all_function_info.keys.map(&:to_s).count PuppetLanguageServer.log_message(:debug, "[PuppetHelper::_load_functions] Finished loading #{function_count} functions") + @functions_loaded = true nil end private_class_method :_load_functions diff --git a/spec/unit/puppet-languageserver/message_router_spec.rb b/spec/unit/puppet-languageserver/message_router_spec.rb index 331182f7..33914db1 100644 --- a/spec/unit/puppet-languageserver/message_router_spec.rb +++ b/spec/unit/puppet-languageserver/message_router_spec.rb @@ -57,6 +57,21 @@ it 'should reply with the Language Server version' do expect(request).to receive(:reply_result).with(hash_including('languageServerVersion')) + subject.receive_request(request) + end + it 'should reply with whether the facts are loaded' do + expect(request).to receive(:reply_result).with(hash_including('factsLoaded')) + + subject.receive_request(request) + end + it 'should reply with whether the functions are loaded' do + expect(request).to receive(:reply_result).with(hash_including('functionsLoaded')) + + subject.receive_request(request) + end + it 'should reply with whether the types are loaded' do + expect(request).to receive(:reply_result).with(hash_including('typesLoaded')) + subject.receive_request(request) end end