This patch introduces a language incompatiblity.
Now any collection of the form:
Resource <| optional_query |>
collects virtual resources and catalog resources at the same time.

Signed-off-by: Brice Figureau <[email protected]>
---
 lib/puppet/parser/collector.rb |   36 ++++++++++++-------
 spec/unit/parser/collector.rb  |   77 ++++++++++++++++++++-------------------
 2 files changed, 62 insertions(+), 51 deletions(-)

diff --git a/lib/puppet/parser/collector.rb b/lib/puppet/parser/collector.rb
index 9423db2..1dd1eb9 100644
--- a/lib/puppet/parser/collector.rb
+++ b/lib/puppet/parser/collector.rb
@@ -1,10 +1,12 @@
 # An object that collects stored objects from the central cache and returns
 # them to the current host, yo.
 class Puppet::Parser::Collector
-    attr_accessor :type, :scope, :vquery, :equery, :form, :resources
+    attr_accessor :type, :scope, :vquery, :equery, :form, :resources, 
:collected
 
     # Call the collection method, mark all of the returned objects as 
non-virtual,
-    # and then delete this object from the list of collections to evaluate.
+    # The collector can also delete himself from the compiler if there is no 
more 
+    # resources to collect (valid only for resource fixed-set collector
+    # which get their resources from +collect_resources+ and not from the 
catalog)
     def evaluate
         # Shortcut if we're not using storeconfigs and they're trying to 
collect
         # exported resources.
@@ -12,10 +14,9 @@ class Puppet::Parser::Collector
             Puppet.warning "Not collecting exported resources without 
storeconfigs"
             return false
         end
+
         if self.resources
-            if objects = collect_resources and ! objects.empty?
-                return objects
-            else
+            unless objects = collect_resources and ! objects.empty?
                 return false
             end
         else
@@ -25,15 +26,29 @@ class Puppet::Parser::Collector
             end
             if objects.empty?
                 return false
-            else
-                return objects
             end
         end
+
+        # filter out object that already have been collected by ourself
+        # which can happen if we are called several times and we collect 
catalog
+        # resources
+        objects.reject! { |o| @collected.include?(o.ref) }
+
+        return false if objects.empty?
+
+        # keep an eye on the resources we have collected
+        objects.inject(@collected) { |c,o| c[o.ref]=o; c }
+
+        # return our newly collected resources
+        objects
     end
 
     def initialize(scope, type, equery, vquery, form)
         @scope = scope
 
+        # initialisation
+        @collected = {}
+
         # Canonize the type
         @type = Puppet::Resource::Reference.new(type, "whatever").type
         @equery = equery
@@ -132,13 +147,8 @@ class Puppet::Parser::Collector
 
     # Collect just virtual objects, from our local compiler.
     def collect_virtual(exported = false)
-        if exported
-            method = :exported?
-        else
-            method = :virtual?
-        end
         scope.compiler.resources.find_all do |resource|
-            resource.type == @type and resource.send(method) and 
match?(resource)
+            resource.type == @type and (exported ? resource.exported? : true) 
and match?(resource)
         end
     end
 
diff --git a/spec/unit/parser/collector.rb b/spec/unit/parser/collector.rb
index f92b881..56c684c 100755
--- a/spec/unit/parser/collector.rb
+++ b/spec/unit/parser/collector.rb
@@ -59,8 +59,9 @@ describe Puppet::Parser::Collector, "when collecting specific 
virtual resources"
 
     it "should mark matched resources as non-virtual" do
         @collector.resources = ["File[virtual1]", "File[virtual2]"]
-        one = mock 'one'
+        one = stub_everything 'one'
         one.expects(:virtual=).with(false)
+
         @scope.stubs(:findresource).with("File[virtual1]").returns(one)
         @scope.stubs(:findresource).with("File[virtual2]").returns(nil)
         @collector.evaluate
@@ -68,8 +69,7 @@ describe Puppet::Parser::Collector, "when collecting specific 
virtual resources"
 
     it "should return matched resources" do
         @collector.resources = ["File[virtual1]", "File[virtual2]"]
-        one = mock 'one'
-        one.stubs(:virtual=)
+        one = stub_everything 'one'
         @scope.stubs(:findresource).with("File[virtual1]").returns(one)
         @scope.stubs(:findresource).with("File[virtual2]").returns(nil)
         @collector.evaluate.should == [one]
@@ -77,8 +77,7 @@ describe Puppet::Parser::Collector, "when collecting specific 
virtual resources"
 
     it "should delete itself from the compile's collection list if it has 
found all of its resources" do
         @collector.resources = ["File[virtual1]"]
-        one = mock 'one'
-        one.stubs(:virtual=)
+        one = stub_everything 'one'
         @compiler.expects(:delete_collection).with(@collector)
         @scope.expects(:compiler).returns(@compiler)
         @scope.stubs(:findresource).with("File[virtual1]").returns(one)
@@ -87,15 +86,14 @@ describe Puppet::Parser::Collector, "when collecting 
specific virtual resources"
 
     it "should not delete itself from the compile's collection list if it has 
unfound resources" do
         @collector.resources = ["File[virtual1]"]
-        one = mock 'one'
-        one.stubs(:virtual=)
+        one = stub_everything 'one'
         @compiler.expects(:delete_collection).never
         @scope.stubs(:findresource).with("File[virtual1]").returns(nil)
         @collector.evaluate
     end
 end
 
-describe Puppet::Parser::Collector, "when collecting virtual resources" do
+describe Puppet::Parser::Collector, "when collecting virtual and catalog 
resources" do
     before do
         @scope = mock 'scope'
         @compiler = mock 'compile'
@@ -106,12 +104,18 @@ describe Puppet::Parser::Collector, "when collecting 
virtual resources" do
         @collector = Puppet::Parser::Collector.new(@scope, @resource_type, 
nil, @vquery, :virtual)
     end
 
-    it "should find all resources matching the vquery" do
-        one = stub 'one', :type => "Mytype", :virtual? => true
-        two = stub 'two', :type => "Mytype", :virtual? => true
+    it "should find all virtual resources matching the vquery" do
+        one = stub_everything 'one', :type => "Mytype", :virtual? => true
+        two = stub_everything 'two', :type => "Mytype", :virtual? => true
 
-        one.stubs(:virtual=)
-        two.stubs(:virtual=)
+        @compiler.expects(:resources).returns([one, two])
+
+        @collector.evaluate.should == [one, two]
+    end
+
+    it "should find all non-virtual resources matching the vquery" do
+        one = stub_everything 'one', :type => "Mytype", :virtual? => false
+        two = stub_everything 'two', :type => "Mytype", :virtual? => false
 
         @compiler.expects(:resources).returns([one, two])
 
@@ -119,7 +123,7 @@ describe Puppet::Parser::Collector, "when collecting 
virtual resources" do
     end
 
     it "should mark all matched resources as non-virtual" do
-        one = stub 'one', :type => "Mytype", :virtual? => true
+        one = stub_everything 'one', :type => "Mytype", :virtual? => true
 
         one.expects(:virtual=).with(false)
 
@@ -129,11 +133,8 @@ describe Puppet::Parser::Collector, "when collecting 
virtual resources" do
     end
 
     it "should return matched resources" do
-        one = stub 'one', :type => "Mytype", :virtual? => true
-        two = stub 'two', :type => "Mytype", :virtual? => true
-
-        one.stubs(:virtual=)
-        two.stubs(:virtual=)
+        one = stub_everything 'one', :type => "Mytype", :virtual? => true
+        two = stub_everything 'two', :type => "Mytype", :virtual? => true
 
         @compiler.expects(:resources).returns([one, two])
 
@@ -141,8 +142,8 @@ describe Puppet::Parser::Collector, "when collecting 
virtual resources" do
     end
 
     it "should return all resources of the correct type if there is no virtual 
query" do
-        one = stub 'one', :type => "Mytype", :virtual? => true
-        two = stub 'two', :type => "Mytype", :virtual? => true
+        one = stub_everything 'one', :type => "Mytype", :virtual? => true
+        two = stub_everything 'two', :type => "Mytype", :virtual? => true
 
         one.expects(:virtual=).with(false)
         two.expects(:virtual=).with(false)
@@ -155,8 +156,8 @@ describe Puppet::Parser::Collector, "when collecting 
virtual resources" do
     end
 
     it "should not return or mark resources of a different type" do
-        one = stub 'one', :type => "Mytype", :virtual? => true
-        two = stub 'two', :type => :other, :virtual? => true
+        one = stub_everything 'one', :type => "Mytype", :virtual? => true
+        two = stub_everything 'two', :type => :other, :virtual? => true
 
         one.expects(:virtual=).with(false)
         two.expects(:virtual=).never
@@ -166,14 +167,11 @@ describe Puppet::Parser::Collector, "when collecting 
virtual resources" do
         @collector.evaluate.should == [one]
     end
 
-    it "should not return or mark non-virtual resources" do
-        one = stub 'one', :type => "Mytype", :virtual? => false
-        two = stub 'two', :type => :other, :virtual? => false
+    it "should not return resources that were collected in a previous run of 
this collector" do
+        one = stub_everything 'one', :type => "Mytype", :virtual? => true, 
:title => "test"
+        @compiler.stubs(:resources).returns([one])
 
-        one.expects(:virtual=).never
-        two.expects(:virtual=).never
-
-        @compiler.expects(:resources).returns([one, two])
+        @collector.evaluate
 
         @collector.evaluate.should be_false
     end
@@ -181,8 +179,8 @@ describe Puppet::Parser::Collector, "when collecting 
virtual resources" do
     it "should not return or mark non-matching resources" do
         @collector.vquery = proc { |res| res.name == :one }
 
-        one = stub 'one', :name => :one, :type => "Mytype", :virtual? => true
-        two = stub 'two', :name => :two, :type => "Mytype", :virtual? => true
+        one = stub_everything 'one', :name => :one, :type => "Mytype", 
:virtual? => true
+        two = stub_everything 'two', :name => :two, :type => "Mytype", 
:virtual? => true
 
         one.expects(:virtual=).with(false)
         two.expects(:virtual=).never
@@ -237,8 +235,8 @@ describe Puppet::Parser::Collector, "when collecting 
exported resources" do
     it "should return all matching resources from the current compile and mark 
them non-virtual and non-exported" do
         stub_rails(true)
 
-        one = stub 'one', :type => "Mytype", :virtual? => true, :exported? => 
true
-        two = stub 'two', :type => "Mytype", :virtual? => true, :exported? => 
true
+        one = stub 'one', :type => "Mytype", :virtual? => true, :exported? => 
true, :ref => "one"
+        two = stub 'two', :type => "Mytype", :virtual? => true, :exported? => 
true, :ref => "two"
 
         one.stubs(:exported=)
         one.stubs(:virtual=)
@@ -253,7 +251,7 @@ describe Puppet::Parser::Collector, "when collecting 
exported resources" do
     it "should mark all returned resources as not virtual" do
         stub_rails(true)
 
-        one = stub 'one', :type => "Mytype", :virtual? => true, :exported? => 
true
+        one = stub 'one', :type => "Mytype", :virtual? => true, :exported? => 
true, :ref => "one"
 
         one.stubs(:exported=)
         one.expects(:virtual=).with(false)
@@ -267,13 +265,14 @@ describe Puppet::Parser::Collector, "when collecting 
exported resources" do
         stub_rails()
         Puppet::Rails::Host.stubs(:find_by_name).returns(nil)
 
-        one = stub 'one', :restype => "Mytype", :title => "one", :virtual? => 
true, :exported? => true
+        one = stub 'one', :restype => "Mytype", :title => "one", :virtual? => 
true, :exported? => true, :ref => "one"
         Puppet::Rails::Resource.stubs(:find).returns([one])
 
         resource = mock 'resource'
         one.expects(:to_resource).with(@scope).returns(resource)
         resource.stubs(:exported=)
         resource.stubs(:virtual=)
+        resource.stubs(:ref)
 
         @compiler.stubs(:resources).returns([])
         @scope.stubs(:findresource).returns(nil)
@@ -287,13 +286,14 @@ describe Puppet::Parser::Collector, "when collecting 
exported resources" do
         stub_rails()
         Puppet::Rails::Host.stubs(:find_by_name).returns(nil)
 
-        one = stub 'one', :restype => "Mytype", :title => "one", :virtual? => 
true, :exported? => true
+        one = stub 'one', :restype => "Mytype", :title => "one", :virtual? => 
true, :exported? => true, :ref => "one"
         Puppet::Rails::Resource.stubs(:find).returns([one])
 
         resource = mock 'resource'
         one.expects(:to_resource).with(@scope).returns(resource)
         resource.stubs(:exported=)
         resource.stubs(:virtual=)
+        resource.stubs(:ref)
 
         @compiler.stubs(:resources).returns([])
         @scope.stubs(:findresource).returns(nil)
@@ -308,13 +308,14 @@ describe Puppet::Parser::Collector, "when collecting 
exported resources" do
         stub_rails()
         Puppet::Rails::Host.stubs(:find_by_name).returns(nil)
 
-        one = stub 'one', :restype => "Mytype", :title => "one", :virtual? => 
true, :exported? => true
+        one = stub 'one', :restype => "Mytype", :title => "one", :virtual? => 
true, :exported? => true, :ref => "one"
         Puppet::Rails::Resource.stubs(:find).returns([one])
 
         resource = mock 'resource'
         one.expects(:to_resource).with(@scope).returns(resource)
         resource.expects(:exported=).with(false)
         resource.stubs(:virtual=)
+        resource.stubs(:ref)
 
         @compiler.stubs(:resources).returns([])
         @scope.stubs(:findresource).returns(nil)
-- 
1.6.0.2


--~--~---------~--~----~------------~-------~--~----~
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