From 7f4e058133a0aa6b07bd3402cf01009818066d32 Mon Sep 17 00:00:00 2001
From: Nick Lewis <nick@puppetlabs.com>
Date: Mon, 13 Dec 2010 17:07:33 -0800
Subject: [PATCH] (#4487) Fix environment column in hosts table

An entire environment object was being stored in a string field,
causing the ZAML form of the environment to be stored. This was
over-ridden to return just the ZAML serialized version of the name.
Since the hosts model didn't know how to interpret a serialized
value, it just returned the ZAML string as the environment. This
patch stringifies the environment before putting it in the hosts
table, which stores it properly.

This patch also introduces a new method of testing using Tableless
ActiveRecord models, which emulate their database schema. This
helps to eliminate some stubbing, but it is still impossible to
fully and accurately test all ActiveRecord interactions without a
real database.

Paired-With: Matt Robinson
---
 .../indirector/catalog/active_record.rb       |  2 +-
 .../indirector/catalog/active_record_spec.rb  | 37 +++++++++++++------
 2 files changed, 27 insertions(+), 12 deletions(-)

diff --git a/lib/puppet/indirector/catalog/active_record.rb b/lib/puppet/indirector/catalog/active_record.rb
index fabb08eb98b..f814f4aff78 100644
--- a/lib/puppet/indirector/catalog/active_record.rb
+++ b/lib/puppet/indirector/catalog/active_record.rb
@@ -32,7 +32,7 @@ def save(request)
 
       if node = Puppet::Node.find(catalog.name)
         host.ip = node.parameters["ipaddress"]
-        host.environment = node.environment
+        host.environment = node.environment.to_s
       end
 
       host.save
diff --git a/spec/unit/indirector/catalog/active_record_spec.rb b/spec/unit/indirector/catalog/active_record_spec.rb
index 4e9d049a110..df61d59d7ce 100755
--- a/spec/unit/indirector/catalog/active_record_spec.rb
+++ b/spec/unit/indirector/catalog/active_record_spec.rb
@@ -6,6 +6,23 @@
 describe "Puppet::Resource::Catalog::ActiveRecord" do
   confine "Missing Rails" => Puppet.features.rails?
 
+  require 'puppet/rails'
+  class Tableless < ActiveRecord::Base
+    def self.columns
+      @columns ||= []
+    end
+    def self.column(name, sql_type=nil, default=nil, null=true)
+      columns << ActiveRecord::ConnectionAdapters::Column.new(name.to_s, default, sql_type.to_s, null)
+    end
+  end
+
+  class Host < Tableless
+    column :name, :string, :null => false
+    column :ip, :string
+    column :environment, :string
+    column :last_compile, :datetime
+  end
+
   before do
     require 'puppet/indirector/catalog/active_record'
     Puppet.features.stubs(:rails?).returns true
@@ -76,15 +93,17 @@
 
   describe "when saving an instance" do
     before do
-      @host = stub 'host', :name => "foo", :save => nil, :merge_resources => nil, :last_compile= => nil, :ip= => nil, :environment= => nil
+      @host = Host.new(:name => "foo")
+      @host.stubs(:merge_resources)
+      @host.stubs(:save)
       @host.stubs(:railsmark).yields
 
-      @node = stub_everything 'node', :parameters => {}
-      Puppet::Node.stubs(:find).returns(@node)
+      @node = Puppet::Node.new("foo", :environment => "environment")
+      Puppet::Node.indirection.stubs(:find).with("foo").returns(@node)
 
       Puppet::Rails::Host.stubs(:find_by_name).returns @host
       @catalog = Puppet::Resource::Catalog.new("foo")
-      @request = stub 'request', :key => "foo", :instance => @catalog
+      @request = Puppet::Indirector::Request.new(:active_record, :save, @catalog)
     end
 
     it "should find the Rails host with the same name" do
@@ -111,25 +130,21 @@
     it "should set host ip if we could find a matching node" do
       @node.stubs(:parameters).returns({"ipaddress" => "192.168.0.1"})
 
-      @host.expects(:ip=).with '192.168.0.1'
-
       @terminus.save(@request)
+      @host.ip.should == '192.168.0.1'
     end
 
     it "should set host environment if we could find a matching node" do
-      @node.stubs(:environment).returns("myenv")
-
-      @host.expects(:environment=).with 'myenv'
-
       @terminus.save(@request)
+      @host.environment.should == "environment"
     end
 
     it "should set the last compile time on the host" do
       now = Time.now
       Time.expects(:now).returns now
-      @host.expects(:last_compile=).with now
 
       @terminus.save(@request)
+      @host.last_compile.should == now
     end
 
     it "should save the Rails host instance" do