+1 On Mar 7, 2009, at 10:07 AM, Brice Figureau wrote:
> > 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 > > > > -- The most overlooked advantage to owning a computer is that if they foul up there's no law against wacking them around a little. -- Joe Martin --------------------------------------------------------------------- Luke Kanies | http://reductivelabs.com | http://madstop.com --~--~---------~--~----~------------~-------~--~----~ 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 -~----------~----~----~----~------~----~------~--~---
