Merge pull request #191 from hlindberg/HI-239-cherry-pick-speedup-from-master

(HI-239) Cherry pick speedup from master (from #22142)

Author: Kylo Ginsberg

lib/hiera/backend.rb

@@ -34,15 +34,19 @@ def datadir(backend, scope)
       #
       # If the file is not found nil is returned
       def datafile(backend, scope, source, extension)
-        file = File.join([datadir(backend, scope), "#{source}.#{extension}"])
+        datafile_in(datadir(backend, scope), source, extension)
+      end
 
-        unless File.exist?(file)
-          Hiera.debug("Cannot find datafile #{file}, skipping")
+      # @api private
+      def datafile_in(datadir, source, extension)
+        file = File.join(datadir, "#{source}.#{extension}")
 
-          return nil
+        if File.exist?(file)
+          file
+        else
+          Hiera.debug("Cannot find datafile #{file}, skipping")
+          nil
         end
-
-        return file
       end
 
       # Constructs a list of data sources to search
@@ -73,6 +77,35 @@ def datasources(scope, override=nil, hierarchy=nil)
         end
       end
 
+      # Constructs a list of data files to search
+      #
+      # If you give it a specific hierarchy it will just use that
+      # else it will use the global configured one, failing that
+      # it will just look in the 'common' data source.
+      #
+      # An override can be supplied that will be pre-pended to the
+      # hierarchy.
+      #
+      # The source names will be subject to variable expansion based
+      # on scope
+      #
+      # Only files that exist will be returned. If the file is missing, then
+      # the block will not receive the file.
+      #
+      # @yield [String, String] the source string and the name of the resulting file
+      # @api public
+      def datasourcefiles(backend, scope, extension, override=nil, hierarchy=nil)
+        datadir = Backend.datadir(backend, scope)
+        Backend.datasources(scope, override, hierarchy) do |source|
+          Hiera.debug("Looking for data source #{source}")
+          file = datafile_in(datadir, source, extension)
+
+          if file
+            yield source, file
+          end
+        end
+      end
+
       # Parse a string like <code>'%{foo}'</code> against a supplied
       # scope and additional scope.  If either scope or
       # extra_scope includes the variable 'foo', then it will

lib/hiera/backend/yaml_backend.rb

@@ -13,12 +13,7 @@ def lookup(key, scope, order_override, resolution_type)
 
         Hiera.debug("Looking up #{key} in YAML backend")
 
-        Backend.datasources(scope, order_override) do |source|
-          Hiera.debug("Looking for data source #{source}")
-          yamlfile = Backend.datafile(:yaml, scope, source, "yaml") || next
-
-          next unless file_exists?(yamlfile)
-
+        Backend.datasourcefiles(:yaml, scope, "yaml", order_override) do |source, yamlfile|
           data = @cache.read_file(yamlfile, Hash) do |data|
             YAML.load(data) || {}
           end

spec/unit/backend/yaml_backend_spec.rb

@@ -37,38 +37,16 @@ def read_file(path, expected_type, &block)
       end
 
       describe "#lookup" do
-        it "should look for data in all sources" do
-          Backend.expects(:datasources).multiple_yields(["one"], ["two"])
-          Backend.expects(:datafile).with(:yaml, {}, "one", "yaml").returns(nil)
-          Backend.expects(:datafile).with(:yaml, {}, "two", "yaml").returns(nil)
-
-          @backend.lookup("key", {}, nil, :priority)
-        end
-
         it "should pick data earliest source that has it for priority searches" do
-          Backend.expects(:datasources).multiple_yields(["one"], ["two"])
-          Backend.expects(:datafile).with(:yaml, {}, "one", "yaml").returns("/nonexisting/one.yaml")
-          Backend.expects(:datafile).with(:yaml, {}, "two", "yaml").returns(nil).never
-
-          @backend.stubs(:file_exists?).with("/nonexisting/one.yaml").returns(true)
+          Backend.expects(:datasourcefiles).with(:yaml, {}, "yaml", nil).yields(["one", "/nonexisting/one.yaml"])
           @cache.value = "---\nkey: answer"
 
           @backend.lookup("key", {}, nil, :priority).should == "answer"
         end
 
-        it "should not look up missing data files" do
-          Backend.expects(:datasources).multiple_yields(["one"])
-          Backend.expects(:datafile).with(:yaml, {}, "one", "yaml").returns(nil)
-          YAML.expects(:load_file).never
-
-          @backend.lookup("key", {}, nil, :priority)
-        end
-
         describe "handling unexpected YAML values" do
           before do
-            Backend.expects(:datasources).multiple_yields(["one"])
-            Backend.expects(:datafile).with(:yaml, {}, "one", "yaml").returns("/nonexisting/one.yaml")
-            @backend.stubs(:file_exists?).with("/nonexisting/one.yaml").returns(true)
+            Backend.expects(:datasourcefiles).with(:yaml, {}, "yaml", nil).yields(["one", "/nonexisting/one.yaml"])
           end
 
           it "returns nil when the YAML value is nil" do
@@ -88,24 +66,15 @@ def read_file(path, expected_type, &block)
         end
 
         it "should build an array of all data sources for array searches" do
-          Backend.expects(:datasources).multiple_yields(["one"], ["two"])
-          Backend.expects(:datafile).with(:yaml, {}, "one", "yaml").returns("/nonexisting/one.yaml")
-          Backend.expects(:datafile).with(:yaml, {}, "two", "yaml").returns("/nonexisting/two.yaml")
-          File.stubs(:exist?).with("/nonexisting/one.yaml").returns(true)
-          File.stubs(:exist?).with("/nonexisting/two.yaml").returns(true)
-
+          Backend.expects(:datasourcefiles).with(:yaml, {}, "yaml", nil).multiple_yields(["one", "/nonexisting/one.yaml"], ["two", "/nonexisting/two.yaml"])
           @cache.expects(:read_file).with("/nonexisting/one.yaml", Hash).returns({"key"=>"answer"})
           @cache.expects(:read_file).with("/nonexisting/two.yaml", Hash).returns({"key"=>"answer"})
 
           @backend.lookup("key", {}, nil, :array).should == ["answer", "answer"]
         end
 
         it "should ignore empty hash of data sources for hash searches" do
-          Backend.expects(:datasources).multiple_yields(["one"], ["two"])
-          Backend.expects(:datafile).with(:yaml, {}, "one", "yaml").returns("/nonexisting/one.yaml")
-          Backend.expects(:datafile).with(:yaml, {}, "two", "yaml").returns("/nonexisting/two.yaml")
-          File.stubs(:exist?).with("/nonexisting/one.yaml").returns(true)
-          File.stubs(:exist?).with("/nonexisting/two.yaml").returns(true)
+          Backend.expects(:datasourcefiles).with(:yaml, {}, "yaml", nil).multiple_yields(["one", "/nonexisting/one.yaml"], ["two", "/nonexisting/two.yaml"])
 
           @cache.expects(:read_file).with("/nonexisting/one.yaml", Hash).returns({})
           @cache.expects(:read_file).with("/nonexisting/two.yaml", Hash).returns({"key"=>{"a"=>"answer"}})
@@ -114,11 +83,7 @@ def read_file(path, expected_type, &block)
         end
 
         it "should build a merged hash of data sources for hash searches" do
-          Backend.expects(:datasources).multiple_yields(["one"], ["two"])
-          Backend.expects(:datafile).with(:yaml, {}, "one", "yaml").returns("/nonexisting/one.yaml")
-          Backend.expects(:datafile).with(:yaml, {}, "two", "yaml").returns("/nonexisting/two.yaml")
-          File.stubs(:exist?).with("/nonexisting/one.yaml").returns(true)
-          File.stubs(:exist?).with("/nonexisting/two.yaml").returns(true)
+          Backend.expects(:datasourcefiles).with(:yaml, {}, "yaml", nil).multiple_yields(["one", "/nonexisting/one.yaml"], ["two", "/nonexisting/two.yaml"])
 
           @cache.expects(:read_file).with("/nonexisting/one.yaml", Hash).returns({"key"=>{"a"=>"answer"}})
           @cache.expects(:read_file).with("/nonexisting/two.yaml", Hash).returns({"key"=>{"b"=>"answer", "a"=>"wrong"}})
@@ -127,11 +92,7 @@ def read_file(path, expected_type, &block)
         end
 
         it "should fail when trying to << a Hash" do
-          Backend.expects(:datasources).multiple_yields(["one"], ["two"])
-          Backend.expects(:datafile).with(:yaml, {}, "one", "yaml").returns("/nonexisting/one.yaml")
-          Backend.expects(:datafile).with(:yaml, {}, "two", "yaml").returns("/nonexisting/two.yaml")
-          File.stubs(:exist?).with("/nonexisting/one.yaml").returns(true)
-          File.stubs(:exist?).with("/nonexisting/two.yaml").returns(true)
+          Backend.expects(:datasourcefiles).with(:yaml, {}, "yaml", nil).multiple_yields(["one", "/nonexisting/one.yaml"], ["two", "/nonexisting/two.yaml"])
 
           @cache.expects(:read_file).with("/nonexisting/one.yaml", Hash).returns({"key"=>["a", "answer"]})
           @cache.expects(:read_file).with("/nonexisting/two.yaml", Hash).returns({"key"=>{"a"=>"answer"}})
@@ -140,11 +101,7 @@ def read_file(path, expected_type, &block)
         end
 
         it "should fail when trying to merge an Array" do
-          Backend.expects(:datasources).multiple_yields(["one"], ["two"])
-          Backend.expects(:datafile).with(:yaml, {}, "one", "yaml").returns("/nonexisting/one.yaml")
-          Backend.expects(:datafile).with(:yaml, {}, "two", "yaml").returns("/nonexisting/two.yaml")
-          File.stubs(:exist?).with("/nonexisting/one.yaml").returns(true)
-          File.stubs(:exist?).with("/nonexisting/two.yaml").returns(true)
+          Backend.expects(:datasourcefiles).with(:yaml, {}, "yaml", nil).multiple_yields(["one", "/nonexisting/one.yaml"], ["two", "/nonexisting/two.yaml"])
 
           @cache.expects(:read_file).with("/nonexisting/one.yaml", Hash).returns({"key"=>{"a"=>"answer"}})
           @cache.expects(:read_file).with("/nonexisting/two.yaml", Hash).returns({"key"=>["a", "wrong"]})
@@ -153,19 +110,15 @@ def read_file(path, expected_type, &block)
         end
 
         it "should parse the answer for scope variables" do
-          Backend.expects(:datasources).yields("one")
-          Backend.expects(:datafile).with(:yaml, {"rspec" => "test"}, "one", "yaml").returns("/nonexisting/one.yaml")
-          File.stubs(:exist?).with("/nonexisting/one.yaml").returns(true)
+          Backend.expects(:datasourcefiles).with(:yaml, {"rspec" => "test"}, "yaml", nil).multiple_yields(["one", "/nonexisting/one.yaml"])
 
           @cache.expects(:read_file).with("/nonexisting/one.yaml", Hash).returns({"key"=>"test_%{rspec}"})
 
           @backend.lookup("key", {"rspec" => "test"}, nil, :priority).should == "test_test"
         end
 
         it "should retain datatypes found in yaml files" do
-          Backend.expects(:datasources).yields("one").times(3)
-          Backend.expects(:datafile).with(:yaml, {}, "one", "yaml").returns("/nonexisting/one.yaml").times(3)
-          File.stubs(:exist?).with("/nonexisting/one.yaml").returns(true)
+          Backend.expects(:datasourcefiles).with(:yaml, {}, "yaml", nil).multiple_yields(["one", "/nonexisting/one.yaml"]).times(3)
 
 
           @cache.value = "---\nstringval: 'string'\nboolval: true\nnumericval: 1"