Merge branch '2.6.next' of github.com:puppetlabs/puppet into 2.6.next

* '2.6.next' of github.com:puppetlabs/puppet:
  (#6689) Remove extraneous include of Puppet::Util in InventoryActiveRecord
  Remove extra trailing whitespace from lib/puppet/resource.rb
  (#5428) More fully "stub" Puppet::Resource::Reference for use with storedconfigs
  (#6707) Fix typo in rest_authconfig.rb
  (#6689) Make inventory_active_record terminus search quickly
  (#5662) Fixed tests that didnt stub key_attributes
  (#5662) Parsedfile doesnt work with mult keyattr
  (#5661) Creating types dont work with >1 namevar

11 files changed, 67 insertions, 49 deletions

diff --git a/lib/puppet/indirector/facts/inventory_active_record.rb b/lib/puppet/indirector/facts/inventory_active_record.rb
index 89edaf332..db4c63f00 100644
--- a/lib/puppet/indirector/facts/inventory_active_record.rb
+++ b/lib/puppet/indirector/facts/inventory_active_record.rb
@@ -1,3 +1,4 @@
+require 'puppet/rails'
 require 'puppet/rails/inventory_node'
 require 'puppet/rails/inventory_fact'
 require 'puppet/indirector/active_record'
@@ -17,7 +18,7 @@ class Puppet::Node::Facts::InventoryActiveRecord < Puppet::Indirector::ActiveRec
     node.timestamp = facts.timestamp
 
     ActiveRecord::Base.transaction do
-      Puppet::Rails::InventoryFact.delete_all(:inventory_node_id => node.id)
+      Puppet::Rails::InventoryFact.delete_all(:node_id => node.id)
       # We don't want to save internal values as facts, because those are
       # metadata that belong on the node
       facts.values.each do |name,value|
@@ -30,6 +31,7 @@ class Puppet::Node::Facts::InventoryActiveRecord < Puppet::Indirector::ActiveRec
 
   def search(request)
     return [] unless request.options
+    matching_nodes = []
     fact_names = []
     fact_filters = Hash.new {|h,k| h[k] = []}
     meta_filters = Hash.new {|h,k| h[k] = []}
@@ -66,8 +68,11 @@ class Puppet::Node::Facts::InventoryActiveRecord < Puppet::Indirector::ActiveRec
       'le' => '<='
     }.each do |operator_name,operator|
       fact_filters[operator_name].each do |name,value|
-        nodes_with_fact = Puppet::Rails::InventoryNode.has_fact(name)
-        node_sets << nodes_with_fact.select {|h| h.value_for(name).to_f.send(operator, value.to_f)}.map {|node| node.name}
+        facts = Puppet::Rails::InventoryFact.find_by_sql(["SELECT inventory_facts.value, inventory_nodes.name AS node_name
+                                                           FROM inventory_facts INNER JOIN inventory_nodes
+                                                           ON inventory_facts.node_id = inventory_nodes.id
+                                                           WHERE inventory_facts.name = ?", name])
+        node_sets << facts.select {|fact| fact.value.to_f.send(operator, value.to_f)}.map {|fact| fact.node_name}
       end
     end
     node_sets
@@ -84,7 +89,7 @@ class Puppet::Node::Facts::InventoryActiveRecord < Puppet::Indirector::ActiveRec
       'le' => '<='
     }.each do |operator_name,operator|
       meta_filters[operator_name].each do |name,value|
-        node_sets << Puppet::Rails::InventoryNode.find(:all, :conditions => ["timestamp #{operator} ?", value]).map {|node| node.name}
+        node_sets << Puppet::Rails::InventoryNode.find(:all, :select => "name", :conditions => ["timestamp #{operator} ?", value]).map {|node| node.name}
       end
     end
     node_sets
diff --git a/lib/puppet/network/rest_authconfig.rb b/lib/puppet/network/rest_authconfig.rb
index 7a6147a82..e6067612a 100644
--- a/lib/puppet/network/rest_authconfig.rb
+++ b/lib/puppet/network/rest_authconfig.rb
@@ -61,7 +61,7 @@ module Puppet
     def insert_default_acl
       DEFAULT_ACL.each do |acl|
         unless rights[acl[:acl]]
-          Puppet.info "Inserting default '#{acl[:acl]}'(#{acl[:authenticated] ? "auth" : "non-auth"}) acl because #{( !exists? ? "#{Puppet[:rest_authconfig]} doesn't exist" : "none where found in '#{@file}'")}"
+          Puppet.info "Inserting default '#{acl[:acl]}'(#{acl[:authenticated] ? "auth" : "non-auth"}) ACL because #{( !exists? ? "#{Puppet[:rest_authconfig]} doesn't exist" : "none were found in '#{@file}'")}"
           mk_acl(acl)
         end
       end
diff --git a/lib/puppet/provider/parsedfile.rb b/lib/puppet/provider/parsedfile.rb
index ffd36e59f..75a215f4b 100755
--- a/lib/puppet/provider/parsedfile.rb
+++ b/lib/puppet/provider/parsedfile.rb
@@ -334,7 +334,9 @@ class Puppet::Provider::ParsedFile < Puppet::Provider
       @property_hash[:target] = @resource.should(:target) || self.class.default_target
       self.class.modified(@property_hash[:target])
     end
-    @property_hash[:name] ||= @resource.name
+    @resource.class.key_attributes.each do |attr|
+      @property_hash[attr] ||= @resource[attr]
+    end
 
     self.class.flush(@property_hash)
 
diff --git a/lib/puppet/rails/database/004_add_inventory_service_tables.rb b/lib/puppet/rails/database/004_add_inventory_service_tables.rb
index a819cac1a..6e6b28c0c 100644
--- a/lib/puppet/rails/database/004_add_inventory_service_tables.rb
+++ b/lib/puppet/rails/database/004_add_inventory_service_tables.rb
@@ -13,12 +13,12 @@ class AddInventoryServiceTables < ActiveRecord::Migration
 
     unless ActiveRecord::Base.connection.tables.include?("inventory_facts")
       create_table :inventory_facts, :id => false do |t|
-        t.column :inventory_node_id, :integer, :null => false
+        t.column :node_id, :integer, :null => false
         t.column :name, :string, :null => false
         t.column :value, :text, :null => false
       end
 
-      add_index :inventory_facts, [:inventory_node_id, :name], :unique => true
+      add_index :inventory_facts, [:node_id, :name], :unique => true
     end
   end
 
@@ -29,7 +29,7 @@ class AddInventoryServiceTables < ActiveRecord::Migration
     end
 
     if ActiveRecord::Base.connection.tables.include?("inventory_facts")
-      remove_index :inventory_facts, [:inventory_node_id, :name]
+      remove_index :inventory_facts, [:node_id, :name]
       drop_table :inventory_facts
     end
   end
diff --git a/lib/puppet/rails/database/schema.rb b/lib/puppet/rails/database/schema.rb
index 9fd640fe4..7b75f4216 100644
--- a/lib/puppet/rails/database/schema.rb
+++ b/lib/puppet/rails/database/schema.rb
@@ -114,12 +114,12 @@ class Puppet::Rails::Schema
         add_index :inventory_nodes, :name, :unique => true
 
         create_table :inventory_facts, :id => false do |t|
-          t.column :inventory_node_id, :integer, :null => false
+          t.column :node_id, :integer, :null => false
           t.column :name, :string, :null => false
           t.column :value, :text, :null => false
         end
 
-        add_index :inventory_facts, [:inventory_node_id, :name], :unique => true
+        add_index :inventory_facts, [:node_id, :name], :unique => true
       end
     end
   ensure
diff --git a/lib/puppet/rails/inventory_node.rb b/lib/puppet/rails/inventory_node.rb
index b3e321f94..52f8621a4 100644
--- a/lib/puppet/rails/inventory_node.rb
+++ b/lib/puppet/rails/inventory_node.rb
@@ -1,7 +1,7 @@
 require 'puppet/rails/inventory_fact'
 
 class Puppet::Rails::InventoryNode < ::ActiveRecord::Base
-  has_many :facts, :class_name => "Puppet::Rails::InventoryFact", :dependent => :delete_all
+  has_many :facts, :class_name => "Puppet::Rails::InventoryFact", :foreign_key => :node_id, :dependent => :delete_all
 
   named_scope :has_fact_with_value, lambda { |name,value|
     {
@@ -17,18 +17,6 @@ class Puppet::Rails::InventoryNode < ::ActiveRecord::Base
     }
   }
 
-  named_scope :has_fact, lambda { |name|
-    {
-      :conditions => ["inventory_facts.name = ?", name],
-      :joins => :facts
-    }
-  }
-
-  def value_for(fact_name)
-    fact = facts.find_by_name(fact_name)
-    fact ? fact.value : nil
-  end
-
   def facts_to_hash
     facts.inject({}) do |fact_hash,fact|
       fact_hash.merge(fact.name => fact.value)
diff --git a/lib/puppet/resource.rb b/lib/puppet/resource.rb
index a71675e11..214516908 100644
--- a/lib/puppet/resource.rb
+++ b/lib/puppet/resource.rb
@@ -5,6 +5,11 @@ require 'puppet/util/pson'
 # The simplest resource class.  Eventually it will function as the
 # base class for all resource-like behaviour.
 class Puppet::Resource
+  # This stub class is only needed for serialization compatibility with 0.25.x.
+  # Specifically, it exists to provide a compatibility API when using YAML
+  # serialized objects loaded from StoreConfigs.
+  Reference = Puppet::Resource
+
   include Puppet::Util::Tagging
 
   require 'puppet/resource/type_collection_helper'
@@ -87,7 +92,7 @@ class Puppet::Resource
   def yaml_property_munge(x)
     case x
     when Hash
-      x.inject({}) { |h,kv| 
+      x.inject({}) { |h,kv|
         k,v = kv
         h[k] = self.class.value_to_pson_data(v)
         h
@@ -104,7 +109,7 @@ class Puppet::Resource
   # be overridden at some point, but this works for now.
   %w{has_key? keys length delete empty? <<}.each do |method|
     define_method(method) do |*args|
-      @parameters.send(method, *args)
+      parameters.send(method, *args)
     end
   end
 
@@ -112,13 +117,13 @@ class Puppet::Resource
   # to lower-case symbols.
   def []=(param, value)
     validate_parameter(param) if validate_parameters
-    @parameters[parameter_name(param)] = value
+    parameters[parameter_name(param)] = value
   end
 
   # Return a given parameter's value.  Converts all passed names
   # to lower-case symbols.
   def [](param)
-    @parameters[parameter_name(param)]
+    parameters[parameter_name(param)]
   end
 
   def ==(other)
@@ -140,11 +145,11 @@ class Puppet::Resource
 
   # Iterate over each param/value pair, as required for Enumerable.
   def each
-    @parameters.each { |p,v| yield p, v }
+    parameters.each { |p,v| yield p, v }
   end
 
   def include?(parameter)
-    super || @parameters.keys.include?( parameter_name(parameter) )
+    super || parameters.keys.include?( parameter_name(parameter) )
   end
 
   # These two methods are extracted into a Helper
@@ -170,14 +175,6 @@ class Puppet::Resource
     end
   end
 
-  # This stub class is only needed for serialization compatibility with 0.25.x
-  class Reference
-    attr_accessor :type,:title
-    def initialize(type,title)
-      @type,@title = type,title
-    end
-  end
-
   # Create our resource.
   def initialize(type, title = nil, attributes = {})
     @parameters = {}
@@ -204,7 +201,7 @@ class Puppet::Resource
     tag(self.type)
     tag(self.title) if valid_tag?(self.title)
 
-    @reference = Reference.new(@type,@title) # for serialization compatibility with 0.25.x
+    @reference = self # for serialization compatibility with 0.25.x
     if strict? and ! resource_type
       if @type == 'Class'
         raise ArgumentError, "Could not find declared class #{title}"
@@ -234,7 +231,7 @@ class Puppet::Resource
 
   # Produce a simple hash of our parameters.
   def to_hash
-    parse_title.merge @parameters
+    parse_title.merge parameters
   end
 
   def to_s
@@ -256,7 +253,7 @@ class Puppet::Resource
   # Convert our resource to Puppet code.
   def to_manifest
     # Collect list of attributes to align => and move ensure first
-    attr = @parameters.keys
+    attr = parameters.keys
     attr_max = attr.inject(0) { |max,k| k.to_s.length > max ? k.to_s.length : max }
 
     attr.sort!
@@ -266,7 +263,7 @@ class Puppet::Resource
     end
 
     attributes = attr.collect { |k|
-      v = @parameters[k]
+      v = parameters[k]
       if v.is_a? Array
         "  %-#{attr_max}s => %s,\n" % [ k, "[\'#{v.join("', '")}\']" ]
       else
@@ -433,4 +430,10 @@ class Puppet::Resource
       return { :name => title.to_s }
     end
   end
+
+  def parameters
+    # @parameters could have been loaded from YAML, causing it to be nil (by
+    # bypassing initialize).
+    @parameters ||= {}
+  end
 end
diff --git a/lib/puppet/type.rb b/lib/puppet/type.rb
index f70a3ec0b..205d809c1 100644
--- a/lib/puppet/type.rb
+++ b/lib/puppet/type.rb
@@ -382,8 +382,8 @@ class Type
 
     fail("Invalid parameter #{name}(#{name.inspect})") unless self.class.validattr?(name)
 
-    if name == :name
-      name = name_var
+    if name == :name && nv = name_var
+      name = nv
     end
 
     if obj = @parameters[name]
@@ -403,8 +403,8 @@ class Type
 
     fail("Invalid parameter #{name}") unless self.class.validattr?(name)
 
-    if name == :name
-      name = name_var
+    if name == :name && nv = name_var
+      name = nv
     end
     raise Puppet::Error.new("Got nil value for #{name}") if value.nil?
 
diff --git a/spec/unit/indirector/facts/inventory_active_record_spec.rb b/spec/unit/indirector/facts/inventory_active_record_spec.rb
index c29e58400..9558abde2 100644
--- a/spec/unit/indirector/facts/inventory_active_record_spec.rb
+++ b/spec/unit/indirector/facts/inventory_active_record_spec.rb
@@ -68,9 +68,6 @@ describe "Puppet::Node::Facts::InventoryActiveRecord", :if => (Puppet.features.r
       Puppet::Node::Facts.find("foo").should == foo_facts
       Puppet::Rails::InventoryFact.all.map{|f| [f.name,f.value]}.should_not include(["uptime_days", "30"], ["kernel", "Darwin"])
     end
-
-    it "should not replace the node's facts if something goes wrong" do
-    end
   end
 
   describe "#find" do
diff --git a/spec/unit/provider/ssh_authorized_key/parsed_spec.rb b/spec/unit/provider/ssh_authorized_key/parsed_spec.rb
index 11e9233e0..2e5be165a 100755
--- a/spec/unit/provider/ssh_authorized_key/parsed_spec.rb
+++ b/spec/unit/provider/ssh_authorized_key/parsed_spec.rb
@@ -104,6 +104,7 @@ describe provider_class do
   before :each do
     @resource = stub("resource", :name => "foo")
     @resource.stubs(:[]).returns "foo"
+    @resource.class.stubs(:key_attributes).returns( [:name] )
 
     @provider = provider_class.new(@resource)
     provider_class.stubs(:filetype).returns(Puppet::Util::FileType::FileTypeRam)
diff --git a/spec/unit/resource_spec.rb b/spec/unit/resource_spec.rb
index eaa3d5519..345ccd06e 100755
--- a/spec/unit/resource_spec.rb
+++ b/spec/unit/resource_spec.rb
@@ -463,6 +463,28 @@ describe Puppet::Resource do
     end
   end
 
+  describe "when loading 0.25.x storedconfigs YAML" do
+    before :each do
+      @old_storedconfig_yaml = %q{--- !ruby/object:Puppet::Resource::Reference
+builtin_type:
+title: /tmp/bar
+type: File
+}
+    end
+
+    it "should deserialize a Puppet::Resource::Reference without exceptions" do
+      lambda { YAML.load(@old_storedconfig_yaml) }.should_not raise_error
+    end
+
+    it "should deserialize as a Puppet::Resource::Reference as a Puppet::Resource" do
+      YAML.load(@old_storedconfig_yaml).class.should == Puppet::Resource
+    end
+
+    it "should to_hash properly" do
+      YAML.load(@old_storedconfig_yaml).to_hash.should == { :path => "/tmp/bar" }
+    end
+  end
+
   describe "when converting to a RAL resource" do
     it "should use the resource type's :new method to create the resource if the resource is of a builtin type" do
       resource = Puppet::Resource.new("file", @basepath+"/my/file")