[Pkg-puppet-devel] [facter] 240/352: [FACT-239] Cleanly evaluate blocks in resolution context

[Stig Sandbeck Mathisen]

This is an automated email from the git hooks/post-receive script.

ssm pushed a commit to branch master
in repository facter.

commit 7a581bbb73c2d0f7290047a0e19217085bd78a9b
Author: Adrien Thebo <git at somethingsinistral.net>
Date:   Thu Jan 30 14:32:33 2014 -0800

    [FACT-239] Cleanly evaluate blocks in resolution context
    
    doing an `#instance_eval` on another object is a little bit evil and
    entangles the calling object and object being evaluated. This commit
    defines an `#evaluate` method on resolutions so that resolutions can
    define the behavior of evaluating a block.
---
 lib/facter/core/aggregate.rb      |  4 ++++
 lib/facter/util/fact.rb           |  6 +++---
 lib/facter/util/resolution.rb     | 24 ++++++++++++++++++++++++
 spec/unit/core/aggregate_spec.rb  |  7 +++++++
 spec/unit/util/fact_spec.rb       | 11 -----------
 spec/unit/util/resolution_spec.rb | 16 ++++++++++++++++
 6 files changed, 54 insertions(+), 14 deletions(-)

diff --git a/lib/facter/core/aggregate.rb b/lib/facter/core/aggregate.rb
index fa98dff..5637f94 100644
--- a/lib/facter/core/aggregate.rb
+++ b/lib/facter/core/aggregate.rb
@@ -69,6 +69,10 @@ class Facter::Core::Aggregate
     end
   end
 
+  def evaluate(&block)
+    instance_eval(&block)
+  end
+
   # Define a new chunk for the given aggregate
   #
   # @api public
diff --git a/lib/facter/util/fact.rb b/lib/facter/util/fact.rb
index 75f6580..b4a3e58 100644
--- a/lib/facter/util/fact.rb
+++ b/lib/facter/util/fact.rb
@@ -47,7 +47,7 @@ class Facter::Util::Fact
     begin
       resolve = Facter::Util::Resolution.new(@name, self)
 
-      resolve.instance_eval(&block) if block
+      resolve.evaluate(&block) if block
       @resolves << resolve
 
       resolve
@@ -69,10 +69,10 @@ class Facter::Util::Fact
 
     if resolve.nil?
       resolve = Facter::Util::Resolution.new(resolve_name, self)
-      resolve.instance_eval(&block) if block
+      resolve.evaluate(&block) if block
       @resolves << resolve
     else
-      resolve.instance_eval(&block) if block
+      resolve.evaluate(&block) if block
     end
 
   rescue => e
diff --git a/lib/facter/util/resolution.rb b/lib/facter/util/resolution.rb
index af0c7a3..007a4b8 100644
--- a/lib/facter/util/resolution.rb
+++ b/lib/facter/util/resolution.rb
@@ -62,6 +62,30 @@ class Facter::Util::Resolution
     @weight = nil
   end
 
+  # Evaluate the given block in the context of this resolution. If a block has
+  # already been evaluated emit a warning to that effect.
+  #
+  # @return [void]
+  def evaluate(&block)
+    if @last_evaluated
+      msg = "Already evaluated #{@name}"
+      msg << " at #{@last_evaluated}" if msg.is_a? String
+      msg << ", reevaluating anyways"
+      Facter.warn msg
+    end
+
+    instance_eval(&block)
+
+    # Ruby 1.9+ provides the source location of procs which can provide useful
+    # debugging information if a resolution is being evaluated twice. Since 1.8
+    # doesn't support this we opportunistically provide this information.
+    if block.respond_to? :source_location
+      @last_evaluated = block.source_location.join(':')
+    else
+      @last_evaluated = true
+    end
+  end
+
   def set_options(options)
     if options[:name]
       @name = options.delete(:name)
diff --git a/spec/unit/core/aggregate_spec.rb b/spec/unit/core/aggregate_spec.rb
index 875734b..fd4bcb6 100644
--- a/spec/unit/core/aggregate_spec.rb
+++ b/spec/unit/core/aggregate_spec.rb
@@ -140,4 +140,11 @@ describe Facter::Core::Aggregate do
       subject.value
     end
   end
+
+  describe "evaluating" do
+    it "evaluates the block in the context of the aggregate" do
+      subject.expects(:has_weight).with(5)
+      subject.evaluate { has_weight(5) }
+    end
+  end
 end
diff --git a/spec/unit/util/fact_spec.rb b/spec/unit/util/fact_spec.rb
index e1b2ce0..46624a3 100755
--- a/spec/unit/util/fact_spec.rb
+++ b/spec/unit/util/fact_spec.rb
@@ -23,12 +23,6 @@ describe Facter::Util::Fact do
   end
 
   describe "when adding resolution mechanisms" do
-    it "can create a new resolution instance with a block" do
-      Facter::Util::Resolution.expects(:new).at_least_once.returns resolution
-
-      fact.add { }
-    end
-
     it "instance_evals the passed block on the new resolution" do
       fact.add {
         setcode { "foo" }
@@ -52,7 +46,6 @@ describe Facter::Util::Fact do
   end
 
   describe "adding resolution mechanisms by name" do
-    subject(:fact) { described_class.new('yay') }
 
     it "creates a new resolution if no such resolution exists" do
       res = stub 'resolution', :name => 'named'
@@ -74,10 +67,6 @@ describe Facter::Util::Fact do
     end
   end
 
-  it "can able to return a value" do
-    Facter::Util::Fact.new("yay").should respond_to(:value)
-  end
-
   describe "when returning a value" do
     before do
       fact = Facter::Util::Fact.new("yay")
diff --git a/spec/unit/util/resolution_spec.rb b/spec/unit/util/resolution_spec.rb
index bdbe032..2d7c119 100755
--- a/spec/unit/util/resolution_spec.rb
+++ b/spec/unit/util/resolution_spec.rb
@@ -134,4 +134,20 @@ describe Facter::Util::Resolution do
       end.to raise_error(ArgumentError, /Invalid resolution options.*foo/)
     end
   end
+
+  describe "evaluating" do
+    it "evaluates the block in the context of the given resolution" do
+      subject.expects(:has_weight).with(5)
+      subject.evaluate { has_weight(5) }
+    end
+
+    it "raises a warning if the resolution is evaluated twice" do
+      Facter.expects(:warn).with do |msg|
+        expect(msg).to match /Already evaluated foo at.*reevaluating anyways/
+      end
+
+      subject.evaluate { }
+      subject.evaluate { }
+    end
+  end
 end

-- 
Alioth's /usr/local/bin/git-commit-notice on /srv/git.debian.org/git/pkg-puppet/facter.git