diff --git a/lib/puppet/indirector/resource/active_record.rb b/lib/puppet/indirector/resource/active_record.rb index 361ca8ec0..e886e0eff 100644 --- a/lib/puppet/indirector/resource/active_record.rb +++ b/lib/puppet/indirector/resource/active_record.rb @@ -1,100 +1,93 @@ require 'puppet/indirector/active_record' class Puppet::Resource::ActiveRecord < Puppet::Indirector::ActiveRecord def search(request) type = request_to_type_name(request) host = request.options[:host] filter = request.options[:filter] if filter and filter[1] =~ /^(and|or)$/i then raise Puppet::Error, "Complex search on StoreConfigs resources is not supported" end query = build_active_record_query(type, host, filter) Puppet::Rails::Resource.find(:all, query) end private def request_to_type_name(request) - name = request.key.split('/', 2)[0] - type = Puppet::Type.type(name) or raise Puppet::Error, "Could not find type #{name}" - type.name + request.key.split('/', 2)[0] or + raise "No key found in the request, failing: #{request.inspect}" end def filter_to_active_record(filter) # Don't call me if you don't have a filter, please. filter.is_a?(Array) or raise ArgumentError, "active record filters must be arrays" a, op, b = filter case op when /^(and|or)$/i then extra = [] first, args = filter_to_active_record a extra += args second, args = filter_to_active_record b extra += args return "(#{first}) #{op.upcase} (#{second})", extra when "==", "!=" then op = '=' if op == '==' # SQL, yayz! case a when "title" then return "title #{op} ?", [b] when "tag" then return "puppet_tags.name #{op} ?", [b] else return "param_names.name = ? AND param_values.value #{op} ?", [a, b] end else raise ArgumentError, "unknown operator #{op.inspect} in #{filter.inspect}" end end def build_active_record_query(type, host, filter) raise Puppet::DevError, "Cannot collect resources for a nil host" unless host search = "(exported=? AND restype=?)" - # Some versions of ActiveRecord just to_s a symbol, which our type is, but - # others preserve the symbol-nature, which causes our storage (string) and - # query (symbol) to mismatch. So, manually stringify. --daniel 2011-09-08 - # - # Also, PostgreSQL is case sensitive; we need to make sure our type name - # here matches what we inject. --daniel 2011-09-30 - arguments = [true, type.to_s.capitalize] + arguments = [true, type] if filter then sql, values = filter_to_active_record(filter) search += " AND #{sql}" arguments += values end # note: we're not eagerly including any relations here because it can # create large numbers of objects that we will just throw out later. We # used to eagerly include param_names/values but the way the search filter # is built ruined those efforts and we were eagerly loading only the # searched parameter and not the other ones. query = {} case search when /puppet_tags/ query = { :joins => { :resource_tags => :puppet_tag } } when /param_name/ query = { :joins => { :param_values => :param_name } } end # We're going to collect objects from rails, but we don't want any # objects from this host. if host = Puppet::Rails::Host.find_by_name(host) search += " AND (host_id != ?)" arguments << host.id end query[:conditions] = [search, *arguments] query end end diff --git a/spec/unit/indirector/resource/active_record_spec.rb b/spec/unit/indirector/resource/active_record_spec.rb index 919ec364a..c923f7910 100755 --- a/spec/unit/indirector/resource/active_record_spec.rb +++ b/spec/unit/indirector/resource/active_record_spec.rb @@ -1,197 +1,191 @@ #!/usr/bin/env rspec require 'spec_helper' begin require 'sqlite3' rescue LoadError end require 'puppet/rails' require 'puppet/node/facts' describe "Puppet::Resource::ActiveRecord", :if => (Puppet.features.rails? and defined? SQLite3) do include PuppetSpec::Files before :each do dir = Pathname(tmpdir('puppet-var')) Puppet[:vardir] = dir.to_s Puppet[:dbadapter] = 'sqlite3' Puppet[:dblocation] = (dir + 'storeconfigs.sqlite').to_s Puppet[:storeconfigs] = true end after :each do ActiveRecord::Base.remove_connection end subject { require 'puppet/indirector/resource/active_record' Puppet::Resource.indirection.terminus(:active_record) } it "should automatically initialize Rails" do # Other tests in the suite may have established the connection, which will # linger; the assertion is just to enforce our assumption about the call, # not because I *really* want to test ActiveRecord works. Better to have # an early failure than wonder why the test overall doesn't DTRT. ActiveRecord::Base.remove_connection ActiveRecord::Base.should_not be_connected subject.should be ActiveRecord::Base.should be_connected end describe "#search" do before :each do Puppet::Rails.init end def search(type, host = 'default.local', filter = nil) args = { :host => host, :filter => filter } subject.search(Puppet::Resource.indirection.request(:search, type, args)) end - it "should fail if the type is not known to Puppet" do - expect { search("banana") }.to raise_error Puppet::Error, /Could not find type/ - end - it "should return an empty array if no resources match" do - search("exec").should == [] + search("Exec").should == [] end # Assert that this is a case-insensitive rule, too. %w{and or AND OR And Or anD oR}.each do |op| it "should fail if asked to search with #{op.inspect}" do filter = [%w{tag == foo}, op, %w{title == bar}] - expect { search("notify", 'localhost', filter) }. + expect { search("Notify", 'localhost', filter) }. to raise_error Puppet::Error, /not supported/ end end context "with a matching resource" do before :each do host = Puppet::Rails::Host.create!(:name => 'one.local') Puppet::Rails::Resource. create!(:host => host, :restype => 'Exec', :title => 'whammo', :exported => true) end it "should return something responding to `to_resource` if a resource matches" do - found = search("exec") + found = search("Exec") found.length.should == 1 found.map do |item| item.should respond_to :to_resource item.restype.should == "Exec" end end it "should not filter resources that have been found before" do search("Exec").should == search("Exec") end end end describe "#build_active_record_query" do before :each do Puppet::Rails.init end - let :type do - Puppet::Type.type('notify').name - end + let :type do 'Notify' end def query(type, host, filter = nil) subject.send :build_active_record_query, type, host, filter end it "should exclude all database resources from the host" do host = Puppet::Rails::Host.create! :name => 'one.local' got = query(type, host.name) got.keys.should =~ [:conditions] got[:conditions][0] =~ /\(host_id != \?\)/ got[:conditions].last.should == host.id end it "should join appropriately when filtering on parameters" do filter = %w{propname == propval} got = query(type, 'whatever', filter) got.keys.should =~ [:conditions, :joins] got[:joins].should == { :param_values => :param_name } got[:conditions][0].should =~ /param_names\.name = \?/ got[:conditions][0].should =~ /param_values\.value = \?/ got[:conditions].should be_include filter.first got[:conditions].should be_include filter.last end it "should join appropriately when filtering on tags" do filter = %w{tag == test} got = query(type, 'whatever', filter) got.keys.should =~ [:conditions, :joins] got[:joins].should == {:resource_tags => :puppet_tag} got[:conditions].first.should =~ /puppet_tags/ got[:conditions].should_not be_include filter.first got[:conditions].should be_include filter.last end it "should only search for exported resources with the matching type" do got = query(type, 'whatever') got.keys.should =~ [:conditions] got[:conditions][0].should be_include "(exported=? AND restype=?)" got[:conditions][1].should == true got[:conditions][2].should == type.to_s.capitalize end it "should capitalize the type, since PGSQL is case sensitive" do got = query(type, 'whatever') got[:conditions][2].should == 'Notify' end end describe "#filter_to_active_record" do def filter_to_active_record(input) subject.send :filter_to_active_record, input end [nil, '', 'whatever', 12].each do |input| it "should fail if filter is not an array (with #{input.inspect})" do expect { filter_to_active_record(input) }. to raise_error ArgumentError, /must be arrays/ end end # Not exhaustive, just indicative. ['=', '<>', '=~', '+', '-', '!'].each do |input| it "should fail with unexpected comparison operators (with #{input.inspect})" do expect { filter_to_active_record(["one", input, "two"]) }. to raise_error ArgumentError, /unknown operator/ end end { ["title", "==", "whatever"] => ["title = ?", ["whatever"]], ["title", "!=", "whatever"] => ["title != ?", ["whatever"]], # Technically, these are not supported by Puppet yet, but as we pay # approximately zero cost other than a few minutes writing the tests, # and it would be *harder* to fail on them, nested queries. [["title", "==", "foo"], "or", ["title", "==", "bar"]] => ["(title = ?) OR (title = ?)", ["foo", "bar"]], [["title", "==", "foo"], "or", ["tag", "==", "bar"]] => ["(title = ?) OR (puppet_tags.name = ?)", ["foo", "bar"]], [["title", "==", "foo"], "or", ["param", "==", "bar"]] => ["(title = ?) OR (param_names.name = ? AND param_values.value = ?)", ["foo", "param", "bar"]], [[["title","==","foo"],"or",["tag", "==", "bar"]],"and",["param","!=","baz"]] => ["((title = ?) OR (puppet_tags.name = ?)) AND "+ "(param_names.name = ? AND param_values.value != ?)", ["foo", "bar", "param", "baz"]] }.each do |input, expect| it "should map #{input.inspect} to #{expect.inspect}" do filter_to_active_record(input).should == expect end end end end