[CHEF-2992] remove proxy object for evaluating attributes

Although having the DSL go through a purpose-specific object is a better
design, the risk introduced by that change isn't worth it right now.

7 files changed, 50 insertions, 141 deletions

diff --git a/chef/lib/chef/dsl/attribute.rb b/chef/lib/chef/dsl/attribute.rb
deleted file mode 100644
index aeeed4fa33..0000000000
--- a/chef/lib/chef/dsl/attribute.rb
+++ /dev/null
@@ -1,74 +0,0 @@
-#
-# Author:: Daniel DeLeo (<dan@opscode.com>)
-# Copyright:: Copyright (c) 2012 Opscode, Inc.
-# License:: Apache License, Version 2.0
-#
-# Licensed under the Apache License, Version 2.0 (the "License");
-# you may not use this file except in compliance with the License.
-# You may obtain a copy of the License at
-#
-#     http://www.apache.org/licenses/LICENSE-2.0
-#
-# Unless required by applicable law or agreed to in writing, software
-# distributed under the License is distributed on an "AS IS" BASIS,
-# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-# See the License for the specific language governing permissions and
-# limitations under the License.
-
-require 'chef/exceptions'
-require 'chef/mixin/from_file'
-require 'chef/dsl/include_attribute'
-
-class Chef
-  module DSL
-
-    # == Chef::DSL::Attribute
-    # Chef::DSL::Attribute is a wrapper around evaluating attributes files. It
-    # is composed of a run_context and node, and implements attribute file
-    # loading.
-    #
-    # Chef::DSL::Attribute is _mostly_ a proxy around the Node object, and
-    # unknown methods are unconditionally forwarded to the node.
-    class Attribute
-
-      attr_reader :node
-      attr_reader :run_context
-
-      include Chef::Mixin::FromFile
-
-      def initialize(node, run_context)
-        @node = node
-        @run_context = run_context
-      end
-
-      def cookbook_collection
-        run_context.cookbook_collection
-      end
-
-      # Loads the attribute file specified by the short name of the
-      # file, e.g., loads specified cookbook's
-      #   "attributes/mailservers.rb"
-      # if passed
-      #   "mailservers"
-      def eval_attribute(cookbook_name, attr_file_name)
-        cookbook = cookbook_collection[cookbook_name]
-        unless cookbook
-          raise Chef::Exceptions::CookbookNotFound, "could not find cookbook #{cookbook_name} trying to load attribute #{cookbook_name}::#{attr_file_name}" 
-        end
-
-        attribute_filename = cookbook.attribute_filenames_by_short_filename[attr_file_name]
-        unless attribute_filename
-          raise Chef::Exceptions::AttributeNotFound, "could not find attribute file #{cookbook_name}::#{attr_file_name}"
-        end
-
-        self.from_file(attribute_filename)
-        self
-      end
-
-      def method_missing(method_name, *args, &block)
-        node.send(method_name, *args, &block)
-      end
-
-    end
-  end
-end
diff --git a/chef/lib/chef/dsl/include_attribute.rb b/chef/lib/chef/dsl/include_attribute.rb
index 45d12ddee2..d8342af6a7 100644
--- a/chef/lib/chef/dsl/include_attribute.rb
+++ b/chef/lib/chef/dsl/include_attribute.rb
@@ -29,13 +29,14 @@ class Chef
       #   "mailservers"
       def include_attribute(*attr_file_specs)
         attr_file_specs.flatten.each do |attr_file_spec|
-          cookbook, attr_file = parse_attribute_file_spec(attr_file_spec)
-          if run_context.loaded_fully_qualified_attribute?(cookbook, attr_file)
-            Chef::Log.debug("I am not loading attribute file #{cookbook}::#{attr_file}, because I have already seen it.")
+          cookbook_name, attr_file = parse_attribute_file_spec(attr_file_spec)
+          if run_context.loaded_fully_qualified_attribute?(cookbook_name, attr_file)
+            Chef::Log.debug("I am not loading attribute file #{cookbook_name}::#{attr_file}, because I have already seen it.")
           else
-            Chef::Log.debug("Loading Attribute #{cookbook}::#{attr_file}")
-            run_context.loaded_attribute(cookbook, attr_file)
-            Chef::DSL::Attribute.new(node, run_context)
+            Chef::Log.debug("Loading Attribute #{cookbook_name}::#{attr_file}")
+            run_context.loaded_attribute(cookbook_name, attr_file)
+            attr_file_path = run_context.resolve_attribute(cookbook_name, attr_file)
+            node.from_file(attr_file_path)
           end
         end
         true
diff --git a/chef/lib/chef/node.rb b/chef/lib/chef/node.rb
index f1468706b1..1229e0db28 100644
--- a/chef/lib/chef/node.rb
+++ b/chef/lib/chef/node.rb
@@ -26,6 +26,7 @@ require 'chef/mixin/check_helper'
 require 'chef/mixin/params_validate'
 require 'chef/mixin/from_file'
 require 'chef/mixin/deep_merge'
+require 'chef/dsl/include_attribute'
 require 'chef/environment'
 require 'chef/couchdb'
 require 'chef/rest'
@@ -46,7 +47,10 @@ class Chef
     attr_accessor :recipe_list, :couchdb, :couchdb_rev, :run_state, :run_list
     attr_reader :couchdb_id
 
+    attr_accessor :run_context
+
     include Chef::Mixin::FromFile
+    include Chef::DSL::IncludeAttribute
 
     include Chef::Mixin::CheckHelper
     include Chef::Mixin::ParamsValidate
diff --git a/chef/lib/chef/run_context.rb b/chef/lib/chef/run_context.rb
index 9d343c772a..625a993941 100644
--- a/chef/lib/chef/run_context.rb
+++ b/chef/lib/chef/run_context.rb
@@ -21,7 +21,6 @@ require 'chef/resource_collection'
 require 'chef/node'
 require 'chef/role'
 require 'chef/log'
-require 'chef/dsl/attribute'
 
 class Chef
   # == Chef::RunContext
@@ -55,6 +54,7 @@ class Chef
       @loaded_attributes = {}
       @events = events
 
+      @node.run_context = self
     end
 
     def load(run_list_expansion)
@@ -92,6 +92,16 @@ class Chef
       cookbook.recipe_filenames_by_name[recipe_short_name]
     end
 
+    def resolve_attribute(cookbook_name, attr_file_name)
+      cookbook = cookbook_collection[cookbook_name]
+      raise Chef::Exceptions::CookbookNotFound, "could not find cookbook #{cookbook_name} while loading attribute #{name}" unless cookbook
+
+      attribute_filename = cookbook.attribute_filenames_by_short_filename[attr_file_name]
+      raise Chef::Exceptions::AttributeNotFound, "could not find filename for attribute #{attr_file_name} in cookbook #{cookbook_name}" unless attribute_filename
+
+      attribute_filename
+    end
+
     def notifies_immediately(notification)
       nr = notification.notifying_resource
       if nr.instance_of?(Chef::Resource)
@@ -165,7 +175,7 @@ class Chef
     end
 
     def loaded_attribute(cookbook, attribute_file)
-      @loaded_attributes["#{cookbook}::#{recipe}"] = true
+      @loaded_attributes["#{cookbook}::#{attribute_file}"] = true
     end
 
     private
@@ -233,8 +243,7 @@ class Chef
         begin
           Chef::Log.debug("Node #{@node.name} loading cookbook #{cookbook_name}'s attribute file #{filename}")
           attr_file_basename = ::File.basename(filename, ".rb")
-          attribute_dsl = Chef::DSL::Attribute.new(node, self)
-          attribute_dsl.eval_attribute(cookbook_name, attr_file_basename)
+          @node.include_attribute("#{cookbook_name}::#{attr_file_basename}")
         rescue Exception => e
           @events.attribute_file_load_failed(filename, e)
           raise
diff --git a/chef/spec/unit/dsl/attribute_spec.rb b/chef/spec/unit/dsl/attribute_spec.rb
deleted file mode 100644
index c2983eb187..0000000000
--- a/chef/spec/unit/dsl/attribute_spec.rb
+++ /dev/null
@@ -1,32 +0,0 @@
-
-
-require 'spec_helper'
-require 'chef/dsl/attribute'
-
-describe Chef::DSL::Attribute do
-
-  before do
-    @node = Chef::Node.new
-
-    @cookbook_repo = File.expand_path(File.join(CHEF_SPEC_DATA, "cookbooks"))
-    @cookbook_loader = Chef::CookbookLoader.new(@cookbook_repo)
-    @cookbook_loader.load_cookbooks
-
-    @cookbook_collection = Chef::CookbookCollection.new(@cookbook_loader.cookbooks_by_name)
-
-    @events = Chef::EventDispatch::Dispatcher.new
-    @run_context = Chef::RunContext.new(@node, @cookbook_collection, @events)
-
-    @attribute = Chef::DSL::Attribute.new(@node, @run_context)
-    @attribute.eval_attribute("openldap", "default")
-    @attribute.eval_attribute("openldap", "smokey")
-  end
-
-  it "should eval attributes files in cookbooks" do
-    @node.ldap_server.should eql("ops1prod")
-    @node.ldap_basedn.should eql("dc=hjksolutions,dc=com")
-    @node.ldap_replication_password.should eql("forsure")
-    @node.smokey.should eql("robinson")
-  end
-end
-
diff --git a/chef/spec/unit/node_spec.rb b/chef/spec/unit/node_spec.rb
index 43866a63ad..9741f94bfb 100644
--- a/chef/spec/unit/node_spec.rb
+++ b/chef/spec/unit/node_spec.rb
@@ -21,7 +21,6 @@ require 'ostruct'
 
 describe Chef::Node do
   before(:each) do
-    Chef::Config.node_path(File.expand_path(File.join(CHEF_SPEC_DATA, "nodes")))
     @node = Chef::Node.new()
   end
 
@@ -425,28 +424,30 @@ describe Chef::Node do
     end
   end
 
-  # TODO: timh, cw: 2010-5-19: Node.recipe? deprecated. See node.rb
-  # describe "recipes" do
-  #   it "should have a RunList of recipes that should be applied" do
-  #     @node.recipes.should be_a_kind_of(Chef::RunList)
-  #   end
-  #
-  #   it "should allow you to query whether or not it has a recipe applied with recipe?" do
-  #     @node.recipes << "sunrise"
-  #     @node.recipe?("sunrise").should eql(true)
-  #     @node.recipe?("not at home").should eql(false)
-  #   end
-  #
-  #   it "should return false if a recipe has not been seen" do
-  #     @node.recipe?("snakes").should eql(false)
-  #   end
-  #
-  #   it "should allow you to set recipes with arguments" do
-  #     @node.recipes "one", "two"
-  #     @node.recipe?("one").should eql(true)
-  #     @node.recipe?("two").should eql(true)
-  #   end
-  # end
+  describe "when evaluating attributes files" do
+    before do
+      @node = Chef::Node.new
+
+      @cookbook_repo = File.expand_path(File.join(CHEF_SPEC_DATA, "cookbooks"))
+      @cookbook_loader = Chef::CookbookLoader.new(@cookbook_repo)
+      @cookbook_loader.load_cookbooks
+
+      @cookbook_collection = Chef::CookbookCollection.new(@cookbook_loader.cookbooks_by_name)
+
+      @events = Chef::EventDispatch::Dispatcher.new
+      @run_context = Chef::RunContext.new(@node, @cookbook_collection, @events)
+
+      @node.include_attribute("openldap::default")
+      @node.include_attribute("openldap::smokey")
+    end
+
+    it "sets attributes from the files" do
+      @node.ldap_server.should eql("ops1prod")
+      @node.ldap_basedn.should eql("dc=hjksolutions,dc=com")
+      @node.ldap_replication_password.should eql("forsure")
+      @node.smokey.should eql("robinson")
+    end
+  end
 
   describe "roles" do
     it "should allow you to query whether or not it has a recipe applied with role?" do
diff --git a/chef/spec/unit/run_context_spec.rb b/chef/spec/unit/run_context_spec.rb
index c24dd352a8..f90acd15a5 100644
--- a/chef/spec/unit/run_context_spec.rb
+++ b/chef/spec/unit/run_context_spec.rb
@@ -24,12 +24,12 @@ Chef::Log.level = :debug
 
 describe Chef::RunContext do
   before(:each) do
-    Chef::Config.node_path(File.expand_path(File.join(CHEF_SPEC_DATA, "run_context", "nodes")))
     @chef_repo_path = File.expand_path(File.join(CHEF_SPEC_DATA, "run_context", "cookbooks"))
     cl = Chef::CookbookLoader.new(@chef_repo_path)
     cl.load_cookbooks
     @cookbook_collection = Chef::CookbookCollection.new(cl)
     @node = Chef::Node.new
+    @node.run_list << "test" << "test::one" << "test::two"
     @events = Chef::EventDispatch::Dispatcher.new
     @run_context = Chef::RunContext.new(@node, @cookbook_collection, @events)
   end
@@ -54,7 +54,7 @@ describe Chef::RunContext do
     end
 
     it "should load all the recipes specified for this node" do
-      @run_context.resource_collection[0].to_s.should == "cat[einstein]"  
+      @run_context.resource_collection[0].to_s.should == "cat[einstein]"
       @run_context.resource_collection[1].to_s.should == "cat[loulou]"
       @run_context.resource_collection[2].to_s.should == "cat[birthday]"
       @run_context.resource_collection[3].to_s.should == "cat[peanut]"