Fixing #703, mostly. You still cannot do multi-condition queries, but you can at least query against any parameter, and matching any value is sufficient for a match, so the tags work fine.

git-svn-id: https://reductivelabs.com/svn/puppet/trunk@2705 980ebf18-57e1-0310-9a29-db15c13687c0

7 files changed, 32 insertions, 22 deletions

diff --git a/CHANGELOG b/CHANGELOG
index a8de66598..eec54b4c9 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -1,3 +1,7 @@
+    You can now do simple queries in a collection of
+    exported resources.  You still cannot do multi-condition queries,
+    though. (#703)
+
     puppetca now exits with a non-zero code if it cannot
     find any host certificates to clean. (Patch by Dean
     Wilson.)
diff --git a/lib/puppet/parser/ast/collexpr.rb b/lib/puppet/parser/ast/collexpr.rb
index f69b2e92e..e9f2e63e0 100644
--- a/lib/puppet/parser/ast/collexpr.rb
+++ b/lib/puppet/parser/ast/collexpr.rb
@@ -43,7 +43,9 @@ class CollExpr < AST::Branch
         end
 
         case @oper
-        when "and", "or": oper = @oper.upcase
+        when "and", "or":
+            raise Puppet::ParseError, "Puppet does not currently support collecting exported resources with more than one condition"
+            #oper = @oper.upcase
         when "==": oper = "="
         else
             oper = @oper
@@ -54,17 +56,13 @@ class CollExpr < AST::Branch
             if str1 == "title"
                 str = "title #{oper} '#{str2}'"
             else
-                unless self.form == :virtual or str1 == "title"
-                    parsefail "Collection from the database only supports " +
-                        "title matching currently"
-                end
-                str = "rails_parameters.name = '#{str1}' and " +
-                    "rails_parameters.value #{oper} '#{str2}'"
+                str = "param_values.value #{oper} '#{str2}' and " +
+                    "param_names.name = '#{str1}'"
             end
         else
-            str = [str1, oper, str2].join(" ")
+            str = "(%s) %s (%s)" % [str1, oper, str2]
         end
-
+        
         return str, code
     end
 
diff --git a/lib/puppet/parser/collector.rb b/lib/puppet/parser/collector.rb
index 868946c8c..62eafdd65 100644
--- a/lib/puppet/parser/collector.rb
+++ b/lib/puppet/parser/collector.rb
@@ -22,9 +22,13 @@ class Puppet::Parser::Collector
         end
         host = Puppet::Rails::Host.find_by_name(@scope.host)
 
-        args = {}
+        args = {:include => {:param_values => :param_name}}
+        args[:conditions] = "restype = '%s'" % [@type]
+        if @equery
+            args[:conditions] += " AND (%s)" % [@equery]
+        end
         if host
-            args[:conditions] = "host_id != #{host.id}"
+            args[:conditions] = "host_id != %s AND %s" % [host.id, args[:conditions]]
         else
             #Puppet.info "Host %s is uninitialized" % @scope.host
         end
@@ -33,7 +37,7 @@ class Puppet::Parser::Collector
         # and such, we'll need to vary the conditions, but this works with no
         # attributes, anyway.
         time = Puppet::Util.thinmark do
-            Puppet::Rails::Resource.find_all_by_restype_and_exported(@type, true,
+            Puppet::Rails::Resource.find(:all, @type, true,
                 args
             ).each do |obj|
                 if resource = export_resource(obj)
diff --git a/lib/puppet/rails/resource.rb b/lib/puppet/rails/resource.rb
index 233982ed9..19aeb9205 100644
--- a/lib/puppet/rails/resource.rb
+++ b/lib/puppet/rails/resource.rb
@@ -106,7 +106,12 @@ class Puppet::Rails::Resource < ActiveRecord::Base
         hash[:source] = scope.source
         obj = Puppet::Parser::Resource.new(hash)
 
+        names = []
         self.param_names.each do |pname|
+            # We can get the same name multiple times because of how the
+            # db layout works.
+            next if names.include?(pname.name)
+            names << pname.name
             obj.set(pname.to_resourceparam(self, scope.source))
         end
 
diff --git a/test/lib/puppettest/support/collection.rb b/test/lib/puppettest/support/collection.rb
index 5dbb7a223..69feb5077 100644
--- a/test/lib/puppettest/support/collection.rb
+++ b/test/lib/puppettest/support/collection.rb
@@ -1,7 +1,7 @@
 
 module PuppetTest::Support::Collection
     def run_collection_queries(form)
-        {true => [%{title == "/tmp/testing"}, %{(title == "/tmp/testing")},
+        {true => [%{title == "/tmp/testing"}, %{(title == "/tmp/testing")}, %{group == bin},
             %{title == "/tmp/testing" and group == bin}, %{title == bin or group == bin},
             %{title == "/tmp/testing" or title == bin}, %{title == "/tmp/testing"},
             %{(title == "/tmp/testing" or title == bin) and group == bin}],
diff --git a/test/rails/ast.rb b/test/rails/ast.rb
index 0493237f5..b205aa0d5 100755
--- a/test/rails/ast.rb
+++ b/test/rails/ast.rb
@@ -30,15 +30,14 @@ class TestRailsAST < PuppetTest::TestCase
             code = nil
             str = nil
 
-            # We don't support anything but the title in rails right now
+            # We don't support more than one search criteria at the moment.
             retval = nil
             bad = false
             # Figure out if the search is for anything rails will ignore
-            string.scan(/(\w+) [!=]= \w+/) do |s|
-                unless s[0] == "title"
-                    bad = true
-                    break
-                end
+            if string =~ /\band\b|\bor\b/
+                bad = true
+            else
+                bad = false
             end
 
             # And if it is, make sure we throw an error.
@@ -54,7 +53,7 @@ class TestRailsAST < PuppetTest::TestCase
             end
             assert_nothing_raised("Could not find resource") do
                 retval = Puppet::Rails::Resource.find(:all,
-                    :include => :param_values,
+                    :include => {:param_values => :param_name},
                     :conditions => str) 
             end
 
diff --git a/test/rails/railsresource.rb b/test/rails/railsresource.rb
index a7b6bbc02..32408db21 100755
--- a/test/rails/railsresource.rb
+++ b/test/rails/railsresource.rb
@@ -194,7 +194,7 @@ class TestExportedResources < PuppetTest::TestCase
         res = mkresource :type => "file", :title => "/tmp/testing",
             :source => @source, :scope => @scope,
             :params => {:owner => "root", :source => ["/tmp/A", "/tmp/B"],
-                :mode => "755", :require => [ref1, ref2]}
+                :mode => "755", :require => [ref1, ref2], :subscribe => ref1}
 
         res.line = 50
 
@@ -214,7 +214,7 @@ class TestExportedResources < PuppetTest::TestCase
         res = mkresource :type => "file", :title => "/tmp/testing",
             :source => @source, :scope => @scope,
             :params => {:owner => "bin", :source => ["/tmp/A", "/tmp/C"],
-            :check => "checksum",  :require => [ref1, ref2]}
+            :check => "checksum",  :require => [ref1, ref2], :subscribe => ref2}
 
         res.line = 75
         res.exported = true