(FACT-2499) Facts with aliases are resolved only once

.rubocop_todo.yml

@@ -329,7 +329,6 @@ RSpec/VerifiedDoubles:
     - 'spec/facter/resolvers/windows/virtualization_resolver_spec.rb'
     - 'spec/facter/resolvers/windows/win_os_description_resolver_spec.rb'
     - 'spec/framework/config/block_list_spec.rb'
-    - 'spec/framework/core/fact/internal/internal_fact_manager_spec.rb'
     - 'spec/framework/core/fact_loaders/external_fact_loader_spec.rb'
     - 'spec/framework/core/fact_loaders/fact_loader_spec.rb'
     - 'spec/framework/core/fact_manager_spec.rb'

lib/framework/core/fact/internal/internal_fact_manager.rb

@@ -19,7 +19,10 @@ def filter_internal_facts(searched_facts)
     def start_threads(searched_facts)
       threads = []
 
-      searched_facts.reject { |elem| elem.fact_class.nil? }.each do |searched_fact|
+      # only resolve a fact once, even if multiple search facts depend on that fact
+      searched_facts
+        .uniq { |searched_fact| searched_fact.fact_class.name }
+        .each do |searched_fact|
         threads << Thread.new do
           fact = CoreFact.new(searched_fact)
           fact.create

spec/facter/facter_spec.rb

@@ -10,6 +10,7 @@
   let(:logger) { instance_spy(Facter::Log) }
   let(:fact_manager_spy) { instance_spy(Facter::FactManager) }
   let(:fact_collection_spy) { instance_spy(Facter::FactCollection) }
+  let(:key_error) { KeyError.new('key error') }
 
   before do
     Facter.instance_variable_set(:@logger, logger)
@@ -173,7 +174,8 @@
         .to receive(:build_fact_collection!)
         .with([])
         .and_return(fact_collection_spy)
-      allow(fact_collection_spy).to receive(:value).with('os', 'name').and_raise(KeyError)
+
+      allow(fact_collection_spy).to receive(:value).with('os', 'name').and_raise(key_error)
 
       result = Facter.fact(user_query)
       expect(result).to be_nil
@@ -203,7 +205,7 @@
         .to receive(:build_fact_collection!)
         .with([])
         .and_return(fact_collection_spy)
-      allow(fact_collection_spy).to receive(:value).with('os', 'name').and_raise(KeyError)
+      allow(fact_collection_spy).to receive(:value).with('os', 'name').and_raise(key_error)
 
       result = Facter[user_query]
       expect(result).to be_nil

spec/framework/core/fact/internal/internal_fact_manager_spec.rb

@@ -1,39 +1,62 @@
 # frozen_string_literal: true
 
 describe Facter::InternalFactManager do
+  let(:internal_fact_manager) { Facter::InternalFactManager.new }
+  let(:os_name_class_spy) { class_spy(Facts::Debian::Os::Name) }
+  let(:os_name_instance_spy) { instance_spy(Facts::Debian::Os::Name) }
+
   describe '#resolve_facts' do
     it 'resolved one core fact' do
-      ubuntu_os_name = double(Facts::Debian::Os::Name)
-
       resolved_fact = mock_resolved_fact('os', 'Ubuntu', nil, [])
 
-      allow(ubuntu_os_name).to receive(:new).and_return(ubuntu_os_name)
-      allow(ubuntu_os_name).to receive(:call_the_resolver).and_return(resolved_fact)
+      allow(os_name_class_spy).to receive(:new).and_return(os_name_instance_spy)
+      allow(os_name_instance_spy).to receive(:call_the_resolver).and_return(resolved_fact)
 
-      searched_fact = double(Facter::SearchedFact, name: 'os', fact_class: ubuntu_os_name, filter_tokens: [],
-                                                   user_query: '', type: :core)
+      searched_fact = instance_spy(Facter::SearchedFact, name: 'os', fact_class: os_name_class_spy, filter_tokens: [],
+                                                         user_query: '', type: :core)
 
-      core_fact_manager = Facter::InternalFactManager.new
-      resolved_facts = core_fact_manager.resolve_facts([searched_fact])
+      resolved_facts = internal_fact_manager.resolve_facts([searched_fact])
 
       expect(resolved_facts).to eq([resolved_fact])
     end
 
     it 'resolved one legacy fact' do
-      windows_networking_interface = double(Facts::Windows::NetworkInterfaces)
+      networking_interface_class_spy = class_spy(Facts::Windows::NetworkInterfaces)
+      windows_networking_interface = instance_spy(Facts::Windows::NetworkInterfaces)
 
       resolved_fact = mock_resolved_fact('network_Ethernet0', '192.168.5.121', nil, [], :legacy)
 
-      allow(windows_networking_interface).to receive(:new).and_return(windows_networking_interface)
+      allow(networking_interface_class_spy).to receive(:new).and_return(windows_networking_interface)
       allow(windows_networking_interface).to receive(:call_the_resolver).and_return(resolved_fact)
 
-      searched_fact = double(Facter::SearchedFact, name: 'network_.*', fact_class: windows_networking_interface,
-                                                   filter_tokens: [], user_query: '', type: :core)
+      searched_fact = instance_spy(Facter::SearchedFact, name: 'network_.*', fact_class: networking_interface_class_spy,
+                                                         filter_tokens: [], user_query: '', type: :core)
 
-      core_fact_manager = Facter::InternalFactManager.new
-      resolved_facts = core_fact_manager.resolve_facts([searched_fact])
+      resolved_facts = internal_fact_manager.resolve_facts([searched_fact])
 
       expect(resolved_facts).to eq([resolved_fact])
     end
+
+    context 'when there are multiple search facts pointing to the same fact' do
+      before do
+        resolved_fact = mock_resolved_fact('os', 'Ubuntu', nil, [])
+
+        allow(os_name_class_spy).to receive(:new).and_return(os_name_instance_spy)
+        allow(os_name_instance_spy).to receive(:call_the_resolver).and_return(resolved_fact)
+
+        searched_fact = instance_spy(Facter::SearchedFact, name: 'os.name', fact_class: os_name_class_spy,
+                                                           filter_tokens: [], user_query: '', type: :core)
+
+        searched_fact_with_alias = instance_spy(Facter::SearchedFact, name: 'operatingsystem',
+                                                                      fact_class: os_name_class_spy, filter_tokens: [],
+                                                                      user_query: '', type: :core)
+
+        internal_fact_manager.resolve_facts([searched_fact, searched_fact_with_alias])
+      end
+
+      it 'resolves the fact only once' do
+        expect(os_name_instance_spy).to have_received(:call_the_resolver).once
+      end
+    end
   end
 end

spec/framework/core/options/options_validator_spec.rb

@@ -4,6 +4,10 @@
   describe '#validate' do
     let(:logger) { instance_spy(Facter::Log) }
 
+    before do
+      allow(Facter::Log).to receive(:new).and_return(logger)
+    end
+
     context 'when options are invalid pairs' do
       let(:options) { ['--puppet', '--no-ruby'] }
       let(:error_code) { 1 }