Merge pull request puppetlabs#212 from hunner/handle_refreshonly

Handle refreshonly correctly

Author: Ashley Penney

lib/puppet/provider/postgresql_psql/ruby.rb

@@ -2,7 +2,7 @@
 
   def command()
     if ((! resource[:unless]) or (resource[:unless].empty?))
-      if (resource[:refreshonly])
+      if (resource.refreshonly?)
         # So, if there's no 'unless', and we're in "refreshonly" mode,
         # we need to return the target command here.  If we don't,
         # then Puppet will generate an event indicating that this

lib/puppet/type/postgresql_psql.rb

@@ -24,7 +24,7 @@ def sync(refreshing = false)
       # method, and then inside of the body of 'sync' we can tell
       # whether or not we're refreshing.
 
-      if ((@resource[:refreshonly] == :false) || refreshing)
+      if (!@resource.refreshonly? || refreshing)
         # If we're not in 'refreshonly' mode, or we're not currently
         # refreshing, then we just call the parent method.
         super()
@@ -69,10 +69,11 @@ def sync(refreshing = false)
     defaultto("/tmp")
   end
 
-  newparam(:refreshonly) do
+  newparam(:refreshonly, :boolean => true) do
     desc "If 'true', then the SQL will only be executed via a notify/subscribe event."
 
     defaultto(:false)
+    newvalues(:true, :false)
   end
 
   def refresh()

spec/spec_helper.rb

@@ -1,6 +1,9 @@
 require 'puppetlabs_spec_helper/module_spec_helper'
 
 RSpec.configure do |c|
+  c.mock_with :rspec do |mock|
+    mock.syntax = [:expect, :should]
+  end
   c.include PuppetlabsSpec::Files
 
   c.before :each do

spec/unit/puppet/provider/postgresql_psql/ruby_spec.rb

@@ -0,0 +1,113 @@
+require 'spec_helper'
+
+describe Puppet::Type.type(:postgresql_psql).provider(:ruby) do
+  let(:name) { 'rspec psql test' }
+  let(:resource) do
+    Puppet::Type.type(:postgresql_psql).new({ :name => name, :provider => :ruby }.merge attributes)
+  end
+
+  let(:provider) { resource.provider }
+
+  context("#run_sql_command") do
+    describe "with default attributes" do
+      let(:attributes) do { :db => 'spec_db' } end
+
+      it "executes with the given psql_path on the given DB" do
+        expect(Puppet::Util::SUIDManager).to receive(:run_and_capture).with(
+          ['psql', '-d', attributes[:db], '-t', '-c', 'SELECT something'],
+          'postgres',
+          'postgres'
+        )
+        provider.run_sql_command("SELECT something")
+      end
+    end
+    describe "with psql_path and db" do
+      let(:attributes) do {
+        :psql_path  => '/opt/postgres/psql',
+        :psql_user  => 'spec_user',
+        :psql_group => 'spec_group',
+        :cwd        => '/spec',
+        :db         => 'spec_db'
+      } end
+
+      it "executes with the given psql_path on the given DB" do
+        expect(Dir).to receive(:chdir).with(attributes[:cwd]).and_yield
+        expect(Puppet::Util::SUIDManager).to receive(:run_and_capture).with(
+          [attributes[:psql_path], '-d', attributes[:db], '-t', '-c', 'SELECT something'],
+          attributes[:psql_user],
+          attributes[:psql_group]
+        )
+        provider.run_sql_command("SELECT something")
+      end
+    end
+  end
+
+  context("#command") do
+    context "when unless is specified" do
+      [:true, :false, true, false].each do |refresh|
+        context "and refreshonly is #{refresh}" do
+          let(:attributes) { {
+            :command     => 'SELECT something',
+            :db          => 'spec_db',
+            :unless      => 'SELECT something',
+            :refreshonly => refresh
+          } }
+
+          it "does not fail when the status is successful" do
+            expect(provider).to receive(:run_unless_sql_command).and_return ["1 row returned", 0]
+            provider.command
+          end
+
+          it "returns the given command when rows are returned" do
+            expect(provider).to receive(:run_unless_sql_command).and_return ["1 row returned", 0]
+            expect(provider.command).to eq("SELECT something")
+          end
+
+          it "does not return the given command when no rows are returned" do
+            expect(provider).to receive(:run_unless_sql_command).and_return ["0 rows returned", 0]
+            expect(provider.command).to_not eq("SELECT something")
+          end
+
+          it "raises an error when the sql command fails" do
+            allow(provider).to receive(:run_unless_sql_command).and_return ["Something went wrong", 1]
+            expect { provider.command }.to raise_error(Puppet::Error, /Something went wrong/)
+          end
+        end
+      end
+    end
+
+    context "when unless is not specified" do
+      context "and refreshonly is true" do
+        let(:attributes) do {
+          :command     => 'SELECT something',
+          :db          => 'spec_db',
+          :refreshonly => :true
+        } end
+        it "does not run unless sql command" do
+          expect(provider).to_not receive(:run_unless_sql_command)
+          provider.command
+        end
+
+        it "returns the given command do disable sync" do
+          expect(provider.command).to eq("SELECT something")
+        end
+      end
+
+      context "and refreshonly is false" do
+        let(:attributes) do {
+          :command     => 'SELECT something',
+          :db          => 'spec_db',
+          :refreshonly => :false
+        } end
+        it "does not run unless sql command" do
+          expect(provider).to_not receive(:run_unless_sql_command)
+          provider.command
+        end
+
+        it "does not return the command so as to enable sync" do
+          expect(provider.command).to_not eq("SELECT something")
+        end
+      end
+    end
+  end
+end

spec/unit/puppet/type/postgresql_psql_spec.rb

@@ -0,0 +1,91 @@
+require 'spec_helper'
+
+describe Puppet::Type.type(:postgresql_psql), "when validating attributes" do
+  [:name, :unless, :db, :psql_path, :psql_user, :psql_group].each do |attr|
+    it "should have a #{attr} parameter" do
+      expect(Puppet::Type.type(:postgresql_psql).attrtype(attr)).to eq(:param)
+    end
+  end
+
+  [:command].each do |attr|
+    it "should have a #{attr} property" do
+      expect(Puppet::Type.type(:postgresql_psql).attrtype(attr)).to eq(:property)
+    end
+  end
+end
+
+describe Puppet::Type.type(:postgresql_psql), :unless => Puppet.features.microsoft_windows? do
+  subject do
+    Puppet::Type.type(:postgresql_psql).new({:name => 'rspec'}.merge attributes)
+  end
+
+  describe "available attributes" do
+    {
+      :name        => "rspec",
+      :command     => "SELECT stuff",
+      :unless      => "SELECT other,stuff",
+      :db          => "postgres",
+      :psql_path   => "/bin/false",
+      :psql_user   => "postgres",
+      :psql_group  => "postgres",
+      :cwd         => "/var/lib",
+      :refreshonly => :true,
+    }.each do |attr, value|
+      context attr do
+        let(:attributes) do { attr => value } end
+        its([attr]) { should == value }
+      end
+    end
+
+    context "default values" do
+      let(:attributes) do {} end
+      its([:psql_path]) { should eq("psql") }
+      its([:psql_user]) { should eq("postgres") }
+      its([:psql_group]) { should eq("postgres") }
+      its([:cwd]) { should eq("/tmp") }
+      its(:refreshonly?) { should be_false }
+    end
+  end
+
+  describe "#refreshonly" do
+    [true, :true].each do |refreshonly|
+      context "=> #{refreshonly.inspect}" do
+        let(:attributes) do { :refreshonly => refreshonly } end
+        it "has a value of true" do
+          expect(subject.refreshonly?).to be_true
+        end
+        it "will not enforce command on sync because refresh() will be called" do
+          expect(subject.provider).to_not receive(:command=)
+          subject.property(:command).sync
+        end
+      end
+    end
+
+    [false, :false].each do |refreshonly|
+      context "=> #{refreshonly.inspect}" do
+        let(:attributes) do { :refreshonly => refreshonly } end
+        it "has a value of false" do
+          expect(subject.refreshonly?).to be_false
+        end
+        it "will enforce command on sync because refresh() will not be called" do
+          expect(subject.provider).to receive(:command=)
+          subject.property(:command).sync
+        end
+      end
+    end
+  end
+
+  ## If we refresh the resource, the command should always be run regardless of
+  ## refreshonly
+  describe "when responding to refresh" do
+    [true, :true, false, :false].each do |refreshonly|
+      context "with refreshonly => #{refreshonly.inspect}" do
+        let(:attributes) do { :refreshonly => refreshonly } end
+        it "will enforce command on sync" do
+          expect(subject.provider).to receive(:command=)
+          subject.refresh
+        end
+      end
+    end
+  end
+end