This terminus behaves the same on all supported DB platforms, by performing a
limited portion of its query in SQL, and the rest of the comparison in Ruby.
Its results are consistent with the YAML terminus.

Paired-With: Jesse Wolfe

Signed-off-by: Nick Lewis <[email protected]>
---
 .../indirector/facts/inventory_active_record.rb    |   14 ++++++++++----
 .../database/004_add_inventory_service_tables.rb   |    6 +++---
 lib/puppet/rails/database/schema.rb                |    4 ++--
 lib/puppet/rails/inventory_node.rb                 |   14 +-------------
 .../facts/inventory_active_record_spec.rb          |    3 ---
 5 files changed, 16 insertions(+), 25 deletions(-)

diff --git a/lib/puppet/indirector/facts/inventory_active_record.rb 
b/lib/puppet/indirector/facts/inventory_active_record.rb
index 89edaf3..5d130ea 100644
--- a/lib/puppet/indirector/facts/inventory_active_record.rb
+++ b/lib/puppet/indirector/facts/inventory_active_record.rb
@@ -1,8 +1,10 @@
+require 'puppet/rails'
 require 'puppet/rails/inventory_node'
 require 'puppet/rails/inventory_fact'
 require 'puppet/indirector/active_record'
 
 class Puppet::Node::Facts::InventoryActiveRecord < 
Puppet::Indirector::ActiveRecord
+  include Puppet::Util
   def find(request)
     node = Puppet::Rails::InventoryNode.find_by_name(request.key)
     return nil unless node
@@ -17,7 +19,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 +32,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 +69,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 +90,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/rails/database/004_add_inventory_service_tables.rb 
b/lib/puppet/rails/database/004_add_inventory_service_tables.rb
index a819cac..6e6b28c 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 9fd640f..7b75f42 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 b3e321f..52f8621 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/spec/unit/indirector/facts/inventory_active_record_spec.rb 
b/spec/unit/indirector/facts/inventory_active_record_spec.rb
index c29e584..9558abd 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
-- 
1.7.4.1

-- 
You received this message because you are subscribed to the Google Groups 
"Puppet Developers" group.
To post to this group, send email to [email protected].
To unsubscribe from this group, send email to 
[email protected].
For more options, visit this group at 
http://groups.google.com/group/puppet-dev?hl=en.

Reply via email to