This change replaces calls to <model object>.save with calls to <model class>.indirection.save(<model object>). This makes the use of the indirector explicit rather than implicit so that it will be easier to search for all indirector call sites using grep. This is an intermediate refactor on the way towards allowing indirector calls to be explicitly routed to multiple termini.
This patch affects production code. Signed-off-by: Paul Berry <[email protected]> --- Local-branch: maint/next/remove_save_delegation lib/puppet/application/kick.rb | 2 +- lib/puppet/application/queue.rb | 4 ++-- lib/puppet/application/resource.rb | 2 +- lib/puppet/configurer.rb | 2 +- lib/puppet/file_bucket/dipper.rb | 2 +- lib/puppet/indirector.rb | 7 ------- lib/puppet/indirector/catalog/compiler.rb | 2 +- lib/puppet/network/handler/filebucket.rb | 2 +- lib/puppet/network/handler/master.rb | 2 +- lib/puppet/network/http/handler.rb | 2 +- lib/puppet/ssl/certificate_authority.rb | 4 ++-- lib/puppet/ssl/certificate_revocation_list.rb | 2 +- lib/puppet/ssl/host.rb | 4 ++-- lib/puppet/transaction.rb | 2 +- spec/unit/application/queue_spec.rb | 2 +- spec/unit/application/resource_spec.rb | 4 ++-- spec/unit/configurer_spec.rb | 4 ++-- spec/unit/indirector/catalog/compiler_spec.rb | 2 +- spec/unit/network/http/handler_spec.rb | 6 +++--- spec/unit/ssl/certificate_authority_spec.rb | 4 ++-- spec/unit/ssl/host_spec.rb | 2 +- test/network/handler/master.rb | 6 +++--- 22 files changed, 31 insertions(+), 38 deletions(-) diff --git a/lib/puppet/application/kick.rb b/lib/puppet/application/kick.rb index c6c6fd7..12dad65 100644 --- a/lib/puppet/application/kick.rb +++ b/lib/puppet/application/kick.rb @@ -120,7 +120,7 @@ class Puppet::Application::Kick < Puppet::Application :background => ! options[:foreground], :ignoreschedules => options[:ignoreschedules] } - run = Puppet::Run.new( run_options ).save( url ) + run = Puppet::Run.indirection.save(Puppet::Run.new( run_options ), url) puts "Getting status" result = run.status puts "status is #{result}" diff --git a/lib/puppet/application/queue.rb b/lib/puppet/application/queue.rb index c7e0ce8..b9e8ca4 100644 --- a/lib/puppet/application/queue.rb +++ b/lib/puppet/application/queue.rb @@ -41,12 +41,12 @@ class Puppet::Application::Queue < Puppet::Application require 'puppet/indirector/catalog/queue' # provides Puppet::Indirector::Queue.subscribe Puppet.notice "Starting puppetqd #{Puppet.version}" Puppet::Resource::Catalog::Queue.subscribe do |catalog| - # Once you have a Puppet::Resource::Catalog instance, calling save on it should suffice + # Once you have a Puppet::Resource::Catalog instance, passing it to save should suffice # to put it through to the database via its active_record indirector (which is determined # by the terminus_class = :active_record setting above) Puppet::Util.benchmark(:notice, "Processing queued catalog for #{catalog.name}") do begin - catalog.save + Puppet::Resource::Catalog.indirection.save(catalog) rescue => detail puts detail.backtrace if Puppet[:trace] Puppet.err "Could not save queued catalog for #{catalog.name}: #{detail}" diff --git a/lib/puppet/application/resource.rb b/lib/puppet/application/resource.rb index 3a7fdcc..c7c1c28 100644 --- a/lib/puppet/application/resource.rb +++ b/lib/puppet/application/resource.rb @@ -78,7 +78,7 @@ class Puppet::Application::Resource < Puppet::Application if params.empty? [ Puppet::Resource.indirection.find( key ) ] else - [ Puppet::Resource.new( type, name, :parameters => params ).save( key ) ] + [ Puppet::Resource.indirection.save(Puppet::Resource.new( type, name, :parameters => params ), key) ] end else Puppet::Resource.indirection.search( key, {} ) diff --git a/lib/puppet/configurer.rb b/lib/puppet/configurer.rb index 77a9065..50aaa0d 100644 --- a/lib/puppet/configurer.rb +++ b/lib/puppet/configurer.rb @@ -176,7 +176,7 @@ class Puppet::Configurer puts report.summary if Puppet[:summarize] save_last_run_summary(report) if Puppet[:report] - report.save + Puppet::Transaction::Report.indirection.save(report) end rescue => detail puts detail.backtrace if Puppet[:trace] diff --git a/lib/puppet/file_bucket/dipper.rb b/lib/puppet/file_bucket/dipper.rb index 8d69389..367d6bf 100644 --- a/lib/puppet/file_bucket/dipper.rb +++ b/lib/puppet/file_bucket/dipper.rb @@ -36,7 +36,7 @@ class Puppet::FileBucket::Dipper file_bucket_file = Puppet::FileBucket::File.new(contents, :bucket_path => @local_path, :path => absolutize_path(file) ) dest_path = "#...@rest_path}#{file_bucket_file.name}" - file_bucket_file.save(dest_path) + Puppet::FileBucket::File.indirection.save(file_bucket_file, dest_path) return file_bucket_file.checksum_data rescue => detail puts detail.backtrace if Puppet[:trace] diff --git a/lib/puppet/indirector.rb b/lib/puppet/indirector.rb index d3455c0..9effc5c 100644 --- a/lib/puppet/indirector.rb +++ b/lib/puppet/indirector.rb @@ -21,7 +21,6 @@ module Puppet::Indirector raise(ArgumentError, "Already handling indirection for #[email protected]}; cannot also handle #{indirection}") if @indirection # populate this class with the various new methods extend ClassMethods - include InstanceMethods include Puppet::Indirector::Envelope extend Puppet::Network::FormatHandler @@ -33,10 +32,4 @@ module Puppet::Indirector module ClassMethods attr_reader :indirection end - - module InstanceMethods - def save(key = nil) - self.class.indirection.save self, key - end - end end diff --git a/lib/puppet/indirector/catalog/compiler.rb b/lib/puppet/indirector/catalog/compiler.rb index b80143a..f4c4081 100644 --- a/lib/puppet/indirector/catalog/compiler.rb +++ b/lib/puppet/indirector/catalog/compiler.rb @@ -23,7 +23,7 @@ class Puppet::Resource::Catalog::Compiler < Puppet::Indirector::Code facts = Puppet::Node::Facts.convert_from(format, text_facts) end facts.add_timestamp - facts.save + Puppet::Node::Facts.indirection.save(facts) end # Compile a node's catalog. diff --git a/lib/puppet/network/handler/filebucket.rb b/lib/puppet/network/handler/filebucket.rb index ee10dbd..55028ee 100755 --- a/lib/puppet/network/handler/filebucket.rb +++ b/lib/puppet/network/handler/filebucket.rb @@ -28,7 +28,7 @@ class Puppet::Network::Handler # :nodoc: def addfile(contents, path, client = nil, clientip = nil) contents = Base64.decode64(contents) if client bucket = Puppet::FileBucket::File.new(contents) - bucket.save + Puppet::FileBucket::File.indirection.save(bucket) end # Return the contents associated with a given md5 sum. diff --git a/lib/puppet/network/handler/master.rb b/lib/puppet/network/handler/master.rb index 5b9dd33..62aab53 100644 --- a/lib/puppet/network/handler/master.rb +++ b/lib/puppet/network/handler/master.rb @@ -47,7 +47,7 @@ class Puppet::Network::Handler client ||= facts["hostname"] # Pass the facts to the fact handler - Puppet::Node::Facts.new(client, facts).save unless local? + Puppet::Node::Facts.indirection.save(Puppet::Node::Facts.new(client, facts)) unless local? catalog = Puppet::Resource::Catalog.indirection.find(client) diff --git a/lib/puppet/network/http/handler.rb b/lib/puppet/network/http/handler.rb index fbd7d19..916f02c 100644 --- a/lib/puppet/network/http/handler.rb +++ b/lib/puppet/network/http/handler.rb @@ -146,7 +146,7 @@ module Puppet::Network::HTTP::Handler format = request_format(request) obj = model(indirection_name).convert_from(format, data) - result = obj.save(key) + result = model(indirection_name).indirection.save(obj, key) return_yaml_response(response, result) end diff --git a/lib/puppet/ssl/certificate_authority.rb b/lib/puppet/ssl/certificate_authority.rb index 5725be4..d65067c 100644 --- a/lib/puppet/ssl/certificate_authority.rb +++ b/lib/puppet/ssl/certificate_authority.rb @@ -96,7 +96,7 @@ class Puppet::SSL::CertificateAuthority unless @crl = Puppet::SSL::CertificateRevocationList.indirection.find(Puppet::SSL::CA_NAME) @crl = Puppet::SSL::CertificateRevocationList.new(Puppet::SSL::CA_NAME) @crl.generate(host.certificate.content, host.key.content) - @crl.save + Puppet::SSL::CertificateRevocationList.indirection.save(@crl) end end @crl @@ -248,7 +248,7 @@ class Puppet::SSL::CertificateAuthority # Save the now-signed cert. This should get routed correctly depending # on the certificate type. - cert.save + Puppet::SSL::Certificate.indirection.save(cert) # And remove the CSR if this wasn't self signed. Puppet::SSL::CertificateRequest.indirection.destroy(csr.name) unless self_signing_csr diff --git a/lib/puppet/ssl/certificate_revocation_list.rb b/lib/puppet/ssl/certificate_revocation_list.rb index 44e0a9e..293f4b8 100644 --- a/lib/puppet/ssl/certificate_revocation_list.rb +++ b/lib/puppet/ssl/certificate_revocation_list.rb @@ -79,6 +79,6 @@ class Puppet::SSL::CertificateRevocationList < Puppet::SSL::Base @content.sign(cakey, OpenSSL::Digest::SHA1.new) - save + Puppet::SSL::CertificateRevocationList.indirection.save(self) end end diff --git a/lib/puppet/ssl/host.rb b/lib/puppet/ssl/host.rb index 6539b05..7f71ced 100644 --- a/lib/puppet/ssl/host.rb +++ b/lib/puppet/ssl/host.rb @@ -126,7 +126,7 @@ class Puppet::SSL::Host @key = Key.new(name) @key.generate begin - @key.save + Key.indirection.save(@key) rescue @key = nil raise @@ -144,7 +144,7 @@ class Puppet::SSL::Host @certificate_request = CertificateRequest.new(name) @certificate_request.generate(key.content) begin - @certificate_request.save + CertificateRequest.indirection.save(@certificate_request) rescue @certificate_request = nil raise diff --git a/lib/puppet/transaction.rb b/lib/puppet/transaction.rb index dcd9aad..4c0ea9a 100644 --- a/lib/puppet/transaction.rb +++ b/lib/puppet/transaction.rb @@ -298,7 +298,7 @@ class Puppet::Transaction if Puppet[:report] begin - report.save + Puppet::Transaction::Report.indirection.save(report) rescue => detail Puppet.err "Reporting failed: #{detail}" end diff --git a/spec/unit/application/queue_spec.rb b/spec/unit/application/queue_spec.rb index 619f94e..1c65b55 100755 --- a/spec/unit/application/queue_spec.rb +++ b/spec/unit/application/queue_spec.rb @@ -172,7 +172,7 @@ describe Puppet::Application::Queue do it "should log and save each catalog passed by the queue" do catalog = Puppet::Resource::Catalog.new('eh') - Puppet::Resource::Catalog.indirection.expects(:save).with(catalog, nil) + Puppet::Resource::Catalog.indirection.expects(:save).with(catalog) Puppet::Resource::Catalog::Queue.expects(:subscribe).yields(catalog) Puppet.expects(:notice).times(2) diff --git a/spec/unit/application/resource_spec.rb b/spec/unit/application/resource_spec.rb index 6ab9994..df479ac 100755 --- a/spec/unit/application/resource_spec.rb +++ b/spec/unit/application/resource_spec.rb @@ -184,7 +184,7 @@ describe Puppet::Application::Resource do @resource.command_line.stubs(:args).returns(['type','name','param=temp']) res = stub "resource" - res.expects(:save).with('https://host:8139/production/resources/type/name').returns(res) + Puppet::Resource.indirection.expects(:save).with(res, 'https://host:8139/production/resources/type/name').returns(res) res.expects(:collect) res.expects(:to_manifest) Puppet::Resource.expects(:new).with('type', 'name', :parameters => {'param' => 'temp'}).returns(res) @@ -220,7 +220,7 @@ describe Puppet::Application::Resource do @resource.command_line.stubs(:args).returns(['type','name','param=temp']) res = stub "resource" - res.expects(:save).with('type/name').returns(res) + Puppet::Resource.indirection.expects(:save).with(res, 'type/name').returns(res) res.expects(:collect) res.expects(:to_manifest) Puppet::Resource.expects(:new).with('type', 'name', :parameters => {'param' => 'temp'}).returns(res) diff --git a/spec/unit/configurer_spec.rb b/spec/unit/configurer_spec.rb index 0f65d1d..fc718fa 100755 --- a/spec/unit/configurer_spec.rb +++ b/spec/unit/configurer_spec.rb @@ -272,7 +272,7 @@ describe Puppet::Configurer, "when sending a report" do it "should save the report if reporting is enabled" do Puppet.settings[:report] = true - Puppet::Transaction::Report.indirection.expects(:save).with(@report, nil) + Puppet::Transaction::Report.indirection.expects(:save).with(@report) @configurer.send_report(@report) end @@ -300,7 +300,7 @@ describe Puppet::Configurer, "when sending a report" do it "should log but not fail if saving the report fails" do Puppet.settings[:report] = true - Puppet::Transaction::Report.indirection.expects(:save).with(@report, nil).raises "whatever" + Puppet::Transaction::Report.indirection.expects(:save).with(@report).raises "whatever" Puppet.expects(:err) lambda { @configurer.send_report(@report) }.should_not raise_error diff --git a/spec/unit/indirector/catalog/compiler_spec.rb b/spec/unit/indirector/catalog/compiler_spec.rb index c58fbbd..c8c53ae 100755 --- a/spec/unit/indirector/catalog/compiler_spec.rb +++ b/spec/unit/indirector/catalog/compiler_spec.rb @@ -194,7 +194,7 @@ describe Puppet::Resource::Catalog::Compiler do @request.options[:facts] = "bar" Puppet::Node::Facts.expects(:convert_from).returns @facts - Puppet::Node::Facts.indirection.expects(:save).with(@facts, nil) + Puppet::Node::Facts.indirection.expects(:save).with(@facts) @compiler.extract_facts_from_request(@request) end diff --git a/spec/unit/network/http/handler_spec.rb b/spec/unit/network/http/handler_spec.rb index 8f9388a..e14b573 100755 --- a/spec/unit/network/http/handler_spec.rb +++ b/spec/unit/network/http/handler_spec.rb @@ -354,7 +354,7 @@ describe Puppet::Network::HTTP::Handler do @model_instance = stub('indirected model instance') @model_class.stubs(:convert_from).returns(@model_instance) - @model_instance.stubs(:save) + @indirection.stubs(:save) @format = stub 'format', :suitable? => true, :name => "format", :mime => "text/format" Puppet::Network::FormatHandler.stubs(:format).returns @format @@ -380,7 +380,7 @@ describe Puppet::Network::HTTP::Handler do end it "should use a common method for determining the request parameters" do - @model_instance.expects(:save).with('key').once + @indirection.expects(:save).with(@model_instance, 'key').once @handler.do_save("my_handler", "key", {}, @request, @response) end @@ -390,7 +390,7 @@ describe Puppet::Network::HTTP::Handler do end it "should return the yaml-serialized result when a model save call succeeds" do - @model_instance.stubs(:save).returns(@model_instance) + @indirection.stubs(:save).returns(@model_instance) @model_instance.expects(:to_yaml).returns('foo') @handler.do_save("my_handler", "my_result", {}, @request, @response) end diff --git a/spec/unit/ssl/certificate_authority_spec.rb b/spec/unit/ssl/certificate_authority_spec.rb index a350c0f..7198e33 100755 --- a/spec/unit/ssl/certificate_authority_spec.rb +++ b/spec/unit/ssl/certificate_authority_spec.rb @@ -149,7 +149,7 @@ describe Puppet::SSL::CertificateAuthority do Puppet::SSL::CertificateRevocationList.expects(:new).returns crl crl.expects(:generate).with(@ca.host.certificate.content, @ca.host.key.content) - Puppet::SSL::CertificateRevocationList.indirection.expects(:save).with(crl, nil) + Puppet::SSL::CertificateRevocationList.indirection.expects(:save).with(crl) @ca.crl.should equal(crl) end @@ -330,7 +330,7 @@ describe Puppet::SSL::CertificateAuthority do end it "should save the resulting certificate" do - Puppet::SSL::Certificate.indirection.expects(:save).with(@cert, nil) + Puppet::SSL::Certificate.indirection.expects(:save).with(@cert) @ca.sign(@name, :ca, @request) end diff --git a/spec/unit/ssl/host_spec.rb b/spec/unit/ssl/host_spec.rb index 0523943..7791109 100755 --- a/spec/unit/ssl/host_spec.rb +++ b/spec/unit/ssl/host_spec.rb @@ -380,7 +380,7 @@ describe Puppet::SSL::Host do key = stub 'key', :public_key => mock("public_key"), :content => "mycontent" @host.stubs(:key).returns(key) @request.expects(:generate).with("mycontent") - Puppet::SSL::CertificateRequest.indirection.expects(:save).with(@request, nil) + Puppet::SSL::CertificateRequest.indirection.expects(:save).with(@request) @host.generate_certificate_request.should be_true @host.certificate_request.should equal(@request) diff --git a/test/network/handler/master.rb b/test/network/handler/master.rb index 56d1991..f4bad70 100755 --- a/test/network/handler/master.rb +++ b/test/network/handler/master.rb @@ -33,7 +33,7 @@ class TestMaster < Test::Unit::TestCase def test_hostname_is_used_if_client_is_missing @master.expects(:decode_facts).returns("hostname" => "yay") facts = Puppet::Node::Facts.new("the_facts") - Puppet::Node::Facts.indirection.stubs(:save).with(facts, nil) + Puppet::Node::Facts.indirection.stubs(:save).with(facts) Puppet::Node::Facts.expects(:new).with { |name, facts| name == "yay" }.returns(facts) @master.getconfig("facts") @@ -42,7 +42,7 @@ class TestMaster < Test::Unit::TestCase def test_facts_are_saved facts = Puppet::Node::Facts.new("the_facts") Puppet::Node::Facts.expects(:new).returns(facts) - Puppet::Node::Facts.indirection.expects(:save).with(facts, nil) + Puppet::Node::Facts.indirection.expects(:save).with(facts) @master.stubs(:decode_facts) @@ -51,7 +51,7 @@ class TestMaster < Test::Unit::TestCase def test_catalog_is_used_for_compiling facts = Puppet::Node::Facts.new("the_facts") - Puppet::Node::Facts.indirection.stubs(:save).with(facts, nil) + Puppet::Node::Facts.indirection.stubs(:save).with(facts) Puppet::Node::Facts.stubs(:new).returns(facts) @master.stubs(:decode_facts) -- 1.7.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.
