diff --git a/.rubocop.yml b/.rubocop.yml index 631ea78a2..1dcf17632 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -80,6 +80,7 @@ Metrics/ClassLength: - 'lib/framework/core/options/option_store.rb' - 'lib/framework/cli/cli.rb' - 'lib/resolvers/networking_linux_resolver.rb' + - 'lib/framework/core/cache_manager.rb' Naming/AccessorMethodName: Exclude: diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index d0f27d646..fe431c0d5 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -1,6 +1,6 @@ # This configuration was generated by # `rubocop --auto-gen-config --exclude-limit 1000` -# on 2020-05-11 09:20:37 +0300 using RuboCop version 0.74.0. +# on 2020-06-10 16:06:37 +0300 using RuboCop version 0.81.0. # The point is for the user to remove these configuration records # one by one as the offenses are removed from the code base. # Note that changes in the inspected code, or installation of new @@ -110,7 +110,7 @@ RSpec/LeakyConstantDeclaration: - 'spec/facter/resolvers/macosx/mountpoints_resolver_spec.rb' - 'spec/facter/resolvers/utils/windows/network_utils_spec.rb' -# Offense count: 99 +# Offense count: 96 # Configuration parameters: EnforcedStyle. # SupportedStyles: have_received, receive RSpec/MessageSpies: @@ -177,7 +177,7 @@ RSpec/SubjectStub: - 'spec/custom_facts/util/fact_spec.rb' - 'spec/custom_facts/util/resolution_spec.rb' -# Offense count: 131 +# Offense count: 128 # Configuration parameters: IgnoreNameless, IgnoreSymbolicNames. RSpec/VerifiedDoubles: Exclude: diff --git a/lib/custom_facts/util/collection.rb b/lib/custom_facts/util/collection.rb index 52eaee1e3..014a36d7a 100644 --- a/lib/custom_facts/util/collection.rb +++ b/lib/custom_facts/util/collection.rb @@ -95,8 +95,7 @@ def external_facts return @external_facts unless @external_facts.nil? load_external_facts - external_facts = @facts.select { |_k, v| v.options[:fact_type] == :external } - @external_facts = Facter::Utils.deep_copy(external_facts.keys) + @external_facts = @facts.select { |_k, v| v.options[:fact_type] == :external } end def invalidate_custom_facts @@ -148,11 +147,7 @@ def to_hash def value(name) fact = fact(name) - fact_value = fact&.value - return Facter.core_value(name) if fact_value.nil? - - core_value = Facter.core_value(name) if fact.used_resolution_weight <= 0 - core_value.nil? ? fact_value : core_value + fact&.value end private diff --git a/lib/custom_facts/util/config.rb b/lib/custom_facts/util/config.rb index bc3e4cf5a..eb4b9bca5 100644 --- a/lib/custom_facts/util/config.rb +++ b/lib/custom_facts/util/config.rb @@ -8,7 +8,7 @@ module LegacyFacter module Util module Config def self.ext_fact_loader - @ext_fact_loader || LegacyFacter::Util::DirectoryLoader.default_loader + @ext_fact_loader || LegacyFacter::Util::DirectoryLoader.new end def self.ext_fact_loader=(loader) diff --git a/lib/custom_facts/util/directory_loader.rb b/lib/custom_facts/util/directory_loader.rb index 9ae83bbc5..a8bdb8a3a 100644 --- a/lib/custom_facts/util/directory_loader.rb +++ b/lib/custom_facts/util/directory_loader.rb @@ -30,55 +30,82 @@ class NoSuchDirectoryError < RuntimeError EXTERNAL_FACT_WEIGHT = 10_000 # Directory for fact loading - attr_reader :directory + attr_reader :directories - def initialize(dir, weight = nil) - @directory = dir - @weight = weight || EXTERNAL_FACT_WEIGHT + def initialize(dir = LegacyFacter::Util::Config.external_facts_dirs, weight = EXTERNAL_FACT_WEIGHT) + @directories = [dir].flatten + @weight = weight end - def self.loader_for(dir) - raise NoSuchDirectoryError unless File.directory?(dir) + # Load facts from files in fact directory using the relevant parser classes to + # parse them. + def load(collection) + weight = @weight + + searched_facts, cached_facts = load_directory_entries(collection) + + load_cached_facts(collection, cached_facts, weight) - LegacyFacter::Util::DirectoryLoader.new(dir) + load_searched_facts(collection, searched_facts, weight) end - def self.default_loader - loaders = LegacyFacter::Util::Config.external_facts_dirs.collect do |dir| - LegacyFacter::Util::DirectoryLoader.new(dir) + private + + def load_directory_entries(_collection) + cm = Facter::CacheManager.new + facts = [] + entries.each do |file| + basename = File.basename(file) + if facts.find { |f| f.name == basename } && cm.group_cached?(basename) + Facter.log_exception(Exception.new("Caching is enabled for group \"#{basename}\" while "\ + 'there are at least two external facts files with the same filename')) + else + searched_fact = Facter::SearchedFact.new(basename, nil, [], nil, :file) + searched_fact.file = file + facts << searched_fact + end end - LegacyFacter::Util::CompositeLoader.new(loaders) + + cm.resolve_facts(facts) end - # Load facts from files in fact directory using the relevant parser classes to - # parse them. - def load(collection) - weight = @weight - entries.each do |file| - parser = LegacyFacter::Util::Parser.parser_for(file) + def load_cached_facts(collection, cached_facts, weight) + cached_facts.each do |cached_fact| + collection.add(cached_fact.name, value: cached_fact.value, fact_type: :external, + file: cached_fact.file) { has_weight(weight) } + end + end + + def load_searched_facts(collection, searched_facts, weight) + searched_facts.each do |fact| + parser = LegacyFacter::Util::Parser.parser_for(fact.file) next if parser.nil? data = parser.results if data == false - LegacyFacter.warn "Could not interpret fact file #{file}" + LegacyFacter.warn "Could not interpret fact file #{fact.file}" elsif (data == {}) || data.nil? - LegacyFacter.warn "Fact file #{file} was parsed but returned an empty data set" + LegacyFacter.warn "Fact file #{fact.file} was parsed but returned an empty data set" else - data.each { |p, v| collection.add(p, value: v, fact_type: :external) { has_weight(weight) } } + data.each do |p, v| + collection.add(p, value: v, fact_type: :external, + file: fact.file) { has_weight(weight) } + end end end end - private - def entries - Dir.entries(directory).find_all { |f| should_parse?(f) }.sort.map { |f| File.join(directory, f) } + dirs = @directories.select { |directory| File.directory?(directory) }.map do |directory| + Dir.entries(directory).map { |directory_entry| File.join(directory, directory_entry) } + end + dirs.flatten.select { |f| should_parse?(f) } rescue Errno::ENOENT [] end def should_parse?(file) - file !~ /^\./ + File.basename(file) !~ /^\./ end end end diff --git a/lib/custom_facts/util/fact.rb b/lib/custom_facts/util/fact.rb index cb88358e3..3689ef137 100644 --- a/lib/custom_facts/util/fact.rb +++ b/lib/custom_facts/util/fact.rb @@ -123,8 +123,10 @@ def value announce_when_no_suitable_resolution(suitable_resolutions) announce_when_no_value_found(@value) - @value + @value = resolve_value end + + @value end # @api private @@ -138,6 +140,13 @@ def extract_ldapname_option!(options) private + def resolve_value + return Facter.core_value(name) if @value.nil? + + core_value = Facter.core_value(name) if @used_resolution_weight <= 0 + core_value.nil? ? @value : core_value + end + # Are we in the midst of a search? def searching? @searching diff --git a/lib/custom_facts/util/resolution.rb b/lib/custom_facts/util/resolution.rb index 2646f9300..2a42e0d06 100644 --- a/lib/custom_facts/util/resolution.rb +++ b/lib/custom_facts/util/resolution.rb @@ -86,7 +86,7 @@ def evaluate(&block) end def options(options) - accepted_options = %i[name value timeout weight fact_type] + accepted_options = %i[name value timeout weight fact_type file] accepted_options.each do |option_name| instance_variable_set("@#{option_name}", options.delete(option_name)) if options.key?(option_name) diff --git a/lib/framework/core/cache_manager.rb b/lib/framework/core/cache_manager.rb index fbc085405..92170bc7a 100644 --- a/lib/framework/core/cache_manager.rb +++ b/lib/framework/core/cache_manager.rb @@ -13,14 +13,17 @@ def resolve_facts(searched_facts) return searched_facts, [] if !File.directory?(@cache_dir) || !Options[:cache] facts = [] - searched_facts.each do |fact| + searched_facts.delete_if do |fact| res = resolve_fact(fact) - facts << res if res + if res + facts << res + true + else + false + end end - facts.each do |fact| - searched_facts.delete_if { |f| f.name == fact.name } - end - [searched_facts, facts] + + [searched_facts, facts.flatten] end def cache_facts(resolved_facts) @@ -37,10 +40,21 @@ def cache_facts(resolved_facts) end end + def group_cached?(group_name) + cached = @fact_groups.get_group_ttls(group_name) ? true : false + delete_cache(group_name) unless cached + cached + end + private def resolve_fact(searched_fact) - group_name = @fact_groups.get_fact_group(searched_fact.name) + group_name = if searched_fact.file + searched_fact.name + else + @fact_groups.get_fact_group(searched_fact.name) + end + return unless group_name return unless group_cached?(group_name) @@ -51,16 +65,32 @@ def resolve_fact(searched_fact) return unless data @log.debug("loading cached values for #{group_name} facts") - create_fact(searched_fact, data[searched_fact.name]) + + create_facts(searched_fact, data) end - def create_fact(searched_fact, value) - Facter::ResolvedFact.new(searched_fact.name, value, searched_fact.type, - searched_fact.user_query, searched_fact.filter_tokens) + def create_facts(searched_fact, data) + if searched_fact.type == :file + facts = [] + data.each do |fact_name, fact_value| + fact = Facter::ResolvedFact.new(fact_name, fact_value, searched_fact.type, + searched_fact.user_query, searched_fact.filter_tokens) + fact.file = searched_fact.file + facts << fact + end + facts + else + [Facter::ResolvedFact.new(searched_fact.name, data[searched_fact.name], searched_fact.type, + searched_fact.user_query, searched_fact.filter_tokens)] + end end def cache_fact(fact) - group_name = @fact_groups.get_fact_group(fact.name) + group_name = if fact.file + File.basename(fact.file) + else + @fact_groups.get_fact_group(fact.name) + end return if !group_name || fact.value.nil? return unless group_cached?(group_name) @@ -98,12 +128,6 @@ def read_group_json(group_name) @groups[group_name] = data end - def group_cached?(group_name) - cached = @fact_groups.get_group_ttls(group_name) ? true : false - delete_cache(group_name) unless cached - cached - end - def check_ttls?(group_name) ttls = @fact_groups.get_group_ttls(group_name) return false unless ttls diff --git a/lib/framework/core/fact/external/external_fact_manager.rb b/lib/framework/core/fact/external/external_fact_manager.rb index d40967d33..f696dbf81 100644 --- a/lib/framework/core/fact/external/external_fact_manager.rb +++ b/lib/framework/core/fact/external/external_fact_manager.rb @@ -17,10 +17,11 @@ def external_facts(custom_facts) resolved_custom_facts = [] custom_facts.each do |custom_fact| - fact_value = LegacyFacter.value(custom_fact.name) - resolved_fact = ResolvedFact.new(custom_fact.name, fact_value, :custom) + fact = LegacyFacter[custom_fact.name] + resolved_fact = ResolvedFact.new(custom_fact.name, fact.value, :custom) resolved_fact.filter_tokens = [] resolved_fact.user_query = custom_fact.user_query + resolved_fact.file = fact.options[:file] resolved_custom_facts << resolved_fact end diff --git a/lib/framework/core/fact_loaders/external_fact_loader.rb b/lib/framework/core/fact_loaders/external_fact_loader.rb index 443c58f02..765a79609 100644 --- a/lib/framework/core/fact_loaders/external_fact_loader.rb +++ b/lib/framework/core/fact_loaders/external_fact_loader.rb @@ -34,8 +34,9 @@ def load_external_facts external_facts_to_load = LegacyFacter.collection.external_facts - external_facts_to_load&.each do |external_fact_name| - loaded_fact = LoadedFact.new(external_fact_name.to_s, nil, :external) + external_facts_to_load&.each do |k, v| + loaded_fact = LoadedFact.new(k.to_s, nil, :external) + loaded_fact.file = v.options[:file] external_facts << loaded_fact end diff --git a/lib/framework/parsers/query_parser.rb b/lib/framework/parsers/query_parser.rb index ed925b6b9..44f5afcc2 100644 --- a/lib/framework/parsers/query_parser.rb +++ b/lib/framework/parsers/query_parser.rb @@ -26,7 +26,8 @@ def parse(query_list, loaded_fact) matched_facts = [] @log.debug "User query is: #{query_list}" @query_list = query_list - query_list = loaded_fact.map(&:name) unless query_list.any? + + return no_user_query(loaded_fact) unless query_list.any? query_list.each do |query| @log.debug "Query is #{query}" @@ -37,6 +38,14 @@ def parse(query_list, loaded_fact) matched_facts.flatten(1) end + def no_user_query(loaded_facts) + searched_facts = [] + loaded_facts.each do |loaded_fact| + searched_facts << SearchedFact.new(loaded_fact.name, loaded_fact.klass, [], '', loaded_fact.type) + end + searched_facts + end + def search_for_facts(query, loaded_fact_hash) resolvable_fact_list = [] query = query.to_s @@ -90,7 +99,10 @@ def construct_loaded_fact(query_tokens, query_token_range, loaded_fact) fact_name = loaded_fact.name.to_s klass_name = loaded_fact.klass type = loaded_fact.type - SearchedFact.new(fact_name, klass_name, filter_tokens, user_query, type) + sf = SearchedFact.new(fact_name, klass_name, filter_tokens, user_query, type) + sf.file = loaded_fact.file + + sf end def construct_filter_tokens(query_tokens, query_token_range) diff --git a/lib/models/loaded_fact.rb b/lib/models/loaded_fact.rb index 497a97f1e..59e74abd0 100644 --- a/lib/models/loaded_fact.rb +++ b/lib/models/loaded_fact.rb @@ -3,11 +3,13 @@ module Facter class LoadedFact attr_reader :name, :klass, :type + attr_accessor :file - def initialize(name, klass, type = nil) + def initialize(name, klass, type = nil, file = nil) @name = name @klass = klass @type = type.nil? ? :core : type + @file = file end end end diff --git a/lib/models/resolved_fact.rb b/lib/models/resolved_fact.rb index 535a54ea8..1d2597f47 100644 --- a/lib/models/resolved_fact.rb +++ b/lib/models/resolved_fact.rb @@ -3,7 +3,7 @@ module Facter class ResolvedFact attr_reader :name, :type - attr_accessor :user_query, :filter_tokens, :value + attr_accessor :user_query, :filter_tokens, :value, :file def initialize(name, value = '', type = :core, user_query = nil, filter_tokens = []) @name = name diff --git a/lib/models/searched_fact.rb b/lib/models/searched_fact.rb index af71a4d01..40374f215 100644 --- a/lib/models/searched_fact.rb +++ b/lib/models/searched_fact.rb @@ -3,6 +3,7 @@ module Facter class SearchedFact attr_reader :name, :fact_class, :filter_tokens, :user_query, :type + attr_accessor :file def initialize(fact_name, fact_class, filter_tokens, user_query, type) @name = fact_name diff --git a/spec/custom_facts/util/collection_spec.rb b/spec/custom_facts/util/collection_spec.rb index ce1d28401..d7ebccbb2 100755 --- a/spec/custom_facts/util/collection_spec.rb +++ b/spec/custom_facts/util/collection_spec.rb @@ -136,17 +136,17 @@ describe 'when the weight of the resolution is 0' do before do - allow(Facter).to receive(:core_value).with('yayness').and_return('core_result') - allow(Facter).to receive(:core_value).with('my_fact').and_return(nil) - allow(Facter).to receive(:core_value).with('non_existing_fact') - allow(Facter).to receive(:core_value).with('nil_core_value_custom').and_return(nil) + allow(Facter).to receive(:core_value).with(:yayness).and_return('core_result') + allow(Facter).to receive(:core_value).with(:my_fact).and_return(nil) + allow(Facter).to receive(:core_value).with(:non_existing_fact) + allow(Facter).to receive(:core_value).with(:nil_core_value_custom).and_return(nil) end context 'when there is a custom fact with the name in collection' do it 'calls Facter.core_value' do collection.value('yayness') - expect(Facter).to have_received(:core_value).with('yayness') + expect(Facter).to have_received(:core_value).with(:yayness) end it 'returns core facts value' do @@ -158,7 +158,7 @@ it 'calls Facter.core_value' do collection.value('non_existing_fact') - expect(Facter).to have_received(:core_value).with('non_existing_fact') + expect(Facter).not_to have_received(:core_value).with(:non_existing_fact) end it 'returns custom facts value' do @@ -178,10 +178,10 @@ collection.add('100_weight_fact', value: 'my_weight_fact_value', weight: 100) collection.add('100_weight_nil_fact', value: nil, weight: 100) - allow(Facter).to receive(:core_value).with('100_weight_fact').and_return('core_result') - allow(Facter).to receive(:core_value).with('100_weight_nil_fact').and_return('core_100_weight_nil_fact_value') - allow(Facter).to receive(:core_value).with('core_fact_only').and_return('core_fact_only_value') - allow(Facter).to receive(:core_value).with('no_fact').and_return(nil) + allow(Facter).to receive(:core_value).with(:'100_weight_fact').and_return('core_result') + allow(Facter).to receive(:core_value).with(:'100_weight_nil_fact').and_return('core_100_weight_nil_fact_value') + allow(Facter).to receive(:core_value).with(:core_fact_only).and_return('core_fact_only_value') + allow(Facter).to receive(:core_value).with(:no_fact).and_return(nil) end context 'when there is a custom fact with the name in collection' do @@ -196,12 +196,6 @@ end end - context 'when no custom fact and one core fact with the name' do - it 'returns the core fact value' do - expect(collection.value('core_fact_only')).to eq('core_fact_only_value') - end - end - context 'when no custom fact and no core fact with the name' do it 'returns nil' do expect(collection.value('no_fact')).to be_nil @@ -419,7 +413,7 @@ end it 'returns external fact' do - expect(collection.external_facts.first).to eq(:my_external_fact) + expect(collection.external_facts.first[0]).to eq(:my_external_fact) end end diff --git a/spec/custom_facts/util/directory_loader_spec.rb b/spec/custom_facts/util/directory_loader_spec.rb index eef8dfe11..d4c1226e0 100755 --- a/spec/custom_facts/util/directory_loader_spec.rb +++ b/spec/custom_facts/util/directory_loader_spec.rb @@ -12,24 +12,13 @@ let(:collection_double) { instance_spy(LegacyFacter::Util::Collection) } it 'makes the directory available' do - expect(dir_loader.directory).to be_instance_of(String) - end - - # it "can be created with a given directory" do - # expect(Facter::Util::DirectoryLoader.loader_for("lib").directory).to eq "../lib" - # end - - it 'raises an error when the directory does not exist' do - missing_dir = 'missing' - allow(File).to receive(:directory?).with(missing_dir).and_return(false) - expect { LegacyFacter::Util::DirectoryLoader.loader_for(missing_dir) } - .to raise_error LegacyFacter::Util::DirectoryLoader::NoSuchDirectoryError + expect(dir_loader.directories).to be_instance_of(Array) end it "does nothing bad when dir doesn't exist" do fakepath = '/foobar/path' my_loader = LegacyFacter::Util::DirectoryLoader.new(fakepath) - allow(FileTest).to receive(:exists?).with(my_loader.directory).and_return(false) + allow(FileTest).to receive(:exists?).with(my_loader.directories[0]).and_return(false) expect { my_loader.load(collection) }.not_to raise_error end @@ -48,8 +37,9 @@ write_to_file('data.yaml', YAML.dump(data)) dir_loader.load(collection_double) + file = File.join(dir_loader.directories[0], 'data.yaml') - expect(collection_double).to have_received(:add).with('f1', value: 'one', fact_type: :external) + expect(collection_double).to have_received(:add).with('f1', value: 'one', fact_type: :external, file: file) end it "ignores files that begin with '.'" do @@ -107,7 +97,7 @@ end def write_to_file(file_name, to_write) - file = File.join(dir_loader.directory, file_name) + file = File.join(dir_loader.directories[0], file_name) File.open(file, 'w') { |f| f.print to_write } end end diff --git a/spec/facter/cache_manager_spec.rb b/spec/facter/cache_manager_spec.rb index eb45cb4f4..2b12c737e 100644 --- a/spec/facter/cache_manager_spec.rb +++ b/spec/facter/cache_manager_spec.rb @@ -6,14 +6,19 @@ let(:cache_dir) { '/etc/facter/cache' } let(:searched_core_fact) do instance_spy(Facter::SearchedFact, name: 'os', fact_class: instance_spy(Facts::Linux::Os::Name), - filter_tokens: [], user_query: '', type: :core) + filter_tokens: [], user_query: '', type: :core, file: nil) end let(:searched_custom_fact) do instance_spy(Facter::SearchedFact, name: 'my_custom_fact', fact_class: nil, filter_tokens: [], - user_query: '', type: :custom) + user_query: '', type: :custom, file: nil) end - let(:searched_facts) { [searched_core_fact, searched_custom_fact] } + let(:searched_external_fact) do + instance_spy(Facter::SearchedFact, name: 'ext_file.txt', fact_class: nil, filter_tokens: [], + user_query: '', type: :file, file: '/tmp/ext_file.txt') + end + let(:searched_facts) { [searched_core_fact, searched_custom_fact, searched_external_fact] } let(:cached_core_fact) { "{\n \"os\": \"Ubuntu\"\n}" } + let(:cached_external_fact) { "{\n \"my_external_fact\": \"ext_fact\"\n}" } let(:resolved_core_fact) { mock_resolved_fact('os', 'Ubuntu', '', []) } let(:resolved_facts) { [resolved_core_fact] } @@ -67,6 +72,7 @@ allow(File).to receive(:directory?).with(cache_dir).and_return(true) allow(fact_groups).to receive(:get_fact_group).with('os').and_return(group_name) allow(fact_groups).to receive(:get_fact_group).with('my_custom_fact').and_return(nil) + allow(fact_groups).to receive(:get_group_ttls).with('ext_file.txt').and_return(nil) allow(File).to receive(:readable?).with(cache_file_name).and_return(true) allow(File).to receive(:mtime).with(cache_file_name).and_return(Time.now) allow(Facter::Util::FileHelper).to receive(:safe_read).with(cache_file_name).and_return(cached_core_fact) @@ -75,6 +81,8 @@ it 'returns cached fact' do allow(fact_groups).to receive(:get_group_ttls).with(group_name).and_return(1000) + allow(File).to receive(:readable?).with(cache_file_name).and_return(true) + allow(File).to receive(:readable?).with(File.join(cache_dir, 'ext_file.txt')).and_return(false) _, cached_facts = cache_manager.resolve_facts(searched_facts) expect(cached_facts).to be_an_instance_of(Array).and contain_exactly( @@ -84,20 +92,41 @@ it 'returns searched fact' do allow(fact_groups).to receive(:get_group_ttls).with(group_name).and_return(1000) + allow(File).to receive(:readable?).with(cache_file_name).and_return(true) + allow(File).to receive(:readable?).with(File.join(cache_dir, 'ext_file.txt')).and_return(false) sf, _cf = cache_manager.resolve_facts(searched_facts) expect(sf).to be_an_instance_of(Array).and contain_exactly( - an_object_having_attributes(name: 'my_custom_fact', type: :custom) + an_object_having_attributes(name: 'my_custom_fact', type: :custom), + an_object_having_attributes(name: 'ext_file.txt', type: :file) ) end it 'deletes cache file' do allow(fact_groups).to receive(:get_group_ttls).with(group_name).and_return(nil) + allow(File).to receive(:readable?).with(cache_file_name).and_return(true) allow(File).to receive(:delete).with(cache_file_name) + allow(File).to receive(:readable?).with(File.join(cache_dir, 'ext_file.txt')).and_return(false) cache_manager.resolve_facts(searched_facts) expect(File).to have_received(:delete).with(cache_file_name) end + + it 'returns cached external facts' do + allow(fact_groups).to receive(:get_group_ttls).with('ext_file.txt').and_return(1000) + allow(fact_groups).to receive(:get_group_ttls).with(group_name).and_return(nil) + allow(File).to receive(:readable?).with(cache_file_name).and_return(false) + allow(Facter::Util::FileHelper).to receive(:safe_read).with(File.join(cache_dir, 'ext_file.txt')) + .and_return(cached_external_fact) + allow(File).to receive(:readable?).with(File.join(cache_dir, 'ext_file.txt')).and_return(true) + allow(File).to receive(:mtime).with(File.join(cache_dir, 'ext_file.txt')).and_return(Time.now) + + _, cached_facts = cache_manager.resolve_facts(searched_facts) + expect(cached_facts).to be_an_instance_of(Array).and contain_exactly( + an_instance_of(Facter::ResolvedFact).and(having_attributes(name: 'my_external_fact', value: 'ext_fact', + type: :file)) + ) + end end end @@ -123,6 +152,7 @@ allow(File).to receive(:directory?).with(cache_dir).and_return(true) allow(fact_groups).to receive(:get_fact_group).with('os').and_return(group_name) allow(fact_groups).to receive(:get_fact_group).with('my_custom_fact').and_return(nil) + allow(fact_groups).to receive(:get_fact_group).with('my_external_fact').and_return(nil) allow(fact_groups).to receive(:get_group_ttls).with(group_name).and_return(1000) allow(File).to receive(:readable?).with(cache_file_name).and_return(false) allow(File).to receive(:write).with(cache_file_name, cached_core_fact) @@ -135,4 +165,38 @@ end end end + + describe '#group_cached?' do + context 'with ttls' do + before do + allow(fact_groups).to receive(:get_group_ttls).with(group_name).and_return(1000) + allow(File).to receive(:readable?).with(cache_file_name).and_return(false) + end + + it 'returns true' do + result = cache_manager.group_cached?(group_name) + expect(result).to be true + end + end + + context 'without ttls' do + before do + allow(fact_groups).to receive(:get_group_ttls).with(group_name).and_return(nil) + allow(Facter::Options).to receive(:[]).with(:cache).and_return(true) + allow(File).to receive(:delete).with(cache_file_name) + end + + it 'returns false' do + allow(File).to receive(:readable?).with(cache_file_name).and_return(false) + result = cache_manager.group_cached?(group_name) + expect(result).to be false + end + + it 'deletes invalid cache file' do + allow(File).to receive(:readable?).with(cache_file_name).and_return(true) + cache_manager.group_cached?(group_name) + expect(File).to have_received(:delete).with(cache_file_name) + end + end + end end diff --git a/spec/facter/query_parser_spec.rb b/spec/facter/query_parser_spec.rb index 64c6895b0..be4152863 100644 --- a/spec/facter/query_parser_spec.rb +++ b/spec/facter/query_parser_spec.rb @@ -8,8 +8,9 @@ os_name_class = 'Facter::Ubuntu::OsName' os_family_class = 'Facter::Ubuntu::OsFamily' - loaded_fact_os_name = double(Facter::LoadedFact, name: 'os.name', klass: os_name_class, type: :core) - loaded_fact_os_family = double(Facter::LoadedFact, name: 'os.family', klass: os_family_class, type: :core) + loaded_fact_os_name = double(Facter::LoadedFact, name: 'os.name', klass: os_name_class, type: :core, file: nil) + loaded_fact_os_family = double(Facter::LoadedFact, name: 'os.family', klass: os_family_class, type: :core, + file: nil) loaded_facts = [loaded_fact_os_name, loaded_fact_os_family] matched_facts = Facter::QueryParser.parse(query_list, loaded_facts) @@ -23,7 +24,7 @@ os_name_class = 'Facter::Ubuntu::OsName' - loaded_fact_os_name = double(Facter::LoadedFact, name: 'os.release', klass: os_name_class, type: :core) + loaded_fact_os_name = double(Facter::LoadedFact, name: 'os.release', klass: os_name_class, type: :core, file: nil) loaded_facts = [loaded_fact_os_name] matched_facts = Facter::QueryParser.parse(query_list, loaded_facts) @@ -37,8 +38,10 @@ networking_class = 'Facter::Ubuntu::NetworkInterface' os_family_class = 'Facter::Ubuntu::OsFamily' - loaded_fact_networking = double(Facter::LoadedFact, name: 'ipaddress_.*', klass: networking_class, type: :legacy) - loaded_fact_os_family = double(Facter::LoadedFact, name: 'os.family', klass: os_family_class, type: :core) + loaded_fact_networking = double(Facter::LoadedFact, name: 'ipaddress_.*', klass: networking_class, type: :legacy, + file: nil) + loaded_fact_os_family = double(Facter::LoadedFact, name: 'os.family', klass: os_family_class, type: :core, + file: nil) loaded_facts = [loaded_fact_networking, loaded_fact_os_family] matched_facts = Facter::QueryParser.parse(query_list, loaded_facts) @@ -67,8 +70,8 @@ query_list = ['custom_fact'] os_name_class = 'Facter::Ubuntu::OsName' - loaded_fact_os_name = double(Facter::LoadedFact, name: 'os.name', klass: os_name_class, type: :core) - loaded_fact_custom_fact = double(Facter::LoadedFact, name: 'custom_fact', klass: nil, type: :custom) + loaded_fact_os_name = double(Facter::LoadedFact, name: 'os.name', klass: os_name_class, type: :core, file: nil) + loaded_fact_custom_fact = double(Facter::LoadedFact, name: 'custom_fact', klass: nil, type: :custom, file: nil) loaded_facts = [loaded_fact_os_name, loaded_fact_custom_fact] matched_facts = Facter::QueryParser.parse(query_list, loaded_facts) @@ -80,7 +83,7 @@ it 'queries if param is symbol' do query_list = [:path] path_class = 'Facter::Ubuntu::Path' - loaded_fact_path = double(Facter::LoadedFact, name: 'path', klass: path_class, type: :core) + loaded_fact_path = double(Facter::LoadedFact, name: 'path', klass: path_class, type: :core, file: nil) loaded_facts = [loaded_fact_path] matched_facts = Facter::QueryParser.parse(query_list, loaded_facts) diff --git a/spec/framework/core/fact/external/external_fact_manager_spec.rb b/spec/framework/core/fact/external/external_fact_manager_spec.rb index a64776103..5ba401baf 100644 --- a/spec/framework/core/fact/external/external_fact_manager_spec.rb +++ b/spec/framework/core/fact/external/external_fact_manager_spec.rb @@ -4,11 +4,13 @@ describe '#resolve' do let(:custom_fact_name) { 'my_custom_fact' } let(:custom_fact_value) { 'custom_fact_value' } + let(:custom_fact) { Facter::Util::Fact.new(custom_fact_name) } let(:searched_fact) { Facter::SearchedFact.new(custom_fact_name, nil, [], '', :custom) } let(:custom_fact_manager) { Facter::ExternalFactManager.new } before do - allow(LegacyFacter).to receive(:value).with(custom_fact_name).and_return(custom_fact_value) + allow(LegacyFacter).to receive(:[]).with(custom_fact_name).and_return(custom_fact) + allow(custom_fact).to receive(:value).and_return(custom_fact_value) end it 'resolves one custom fact' do diff --git a/spec/framework/core/fact_loaders/external_fact_loader_spec.rb b/spec/framework/core/fact_loaders/external_fact_loader_spec.rb index c92b94916..b2c5624fe 100644 --- a/spec/framework/core/fact_loaders/external_fact_loader_spec.rb +++ b/spec/framework/core/fact_loaders/external_fact_loader_spec.rb @@ -2,6 +2,7 @@ describe Facter::ExternalFactLoader do let(:collection) { double(LegacyFacter::Util::Collection) } + let(:external_fact) { Facter::Util::Fact.new('external_fact', options: { fact_type: :external }) } before do allow(LegacyFacter).to receive(:collection).and_return(collection) @@ -43,7 +44,7 @@ describe '#external_facts' do context 'when loads one external fact' do before do - allow(collection).to receive(:external_facts).and_return(['external_fact']) + allow(collection).to receive(:external_facts).and_return({ 'external_fact' => external_fact }) allow(Facter::Options).to receive(:external_dir?).and_return(true) allow(Facter::Options).to receive(:external_dir).and_return(['external_fact_dir']) end diff --git a/spec/mocks/util.rb b/spec/mocks/util.rb index 445dffd03..23041fab8 100644 --- a/spec/mocks/util.rb +++ b/spec/mocks/util.rb @@ -46,7 +46,7 @@ def mock_query_parser(user_query, loaded_fact_hash) def mock_resolved_fact(fact_name, fact_value, user_query = nil, filter_tokens = [], type = :core) resolved_fact_mock = double(Facter::ResolvedFact, name: fact_name, value: fact_value, user_query: user_query, filter_tokens: filter_tokens, type: type, - legacy?: type == :legacy, core?: type == :core) + legacy?: type == :legacy, core?: type == :core, file: nil) allow_attr_change(resolved_fact_mock, fact_name, fact_value) resolved_fact_mock