Previously if uploading a report that caused an exception you got a message saying it couldn't render the proper temlate. Now it just renders text telling you what was wrong with the YAML upload.
Paired-with: Nick Lewis Signed-off-by: Matt Robinson <[email protected]> --- Local-branch: ticket/next/5543 app/controllers/reports_controller.rb | 20 ++---- db/schema.rb | 6 +- spec/controllers/reports_controller_spec.rb | 107 +++++++++++++-------------- 3 files changed, 58 insertions(+), 75 deletions(-) diff --git a/app/controllers/reports_controller.rb b/app/controllers/reports_controller.rb index 5a58eb6..9da508b 100644 --- a/app/controllers/reports_controller.rb +++ b/app/controllers/reports_controller.rb @@ -13,22 +13,14 @@ class ReportsController < InheritedResources::Base end def upload - if params[:report][:report].blank? - render :status => 406 - return - end - begin - report = Report.create_from_yaml(params[:report][:report]) - unless report.valid? - Rails.logger.debug "WARNING! ReportsController#create failed:" - report.errors.full_messages.each { |msg| Rails.logger.debug msg } - render :status => 406 - end - rescue ArgumentError => e - Rails.logger.debug "WARNING! ReportsController#create failed:" + Report.create_from_yaml(params[:report][:report]) + render :text => "Report successfully uploaded" + rescue => e + error_text = "ERROR! ReportsController#upload failed:" + Rails.logger.debug error_text Rails.logger.debug e.message - render :status => 406 + render :text => "#{error_text} #{e.message}", :status => 406 end end diff --git a/db/schema.rb b/db/schema.rb index c6c4d24..34b87d8 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -11,10 +11,6 @@ ActiveRecord::Schema.define(:version => 20101206225510) do - create_table "Times", :id => false, :force => true do |t| - t.datetime "time" - end - create_table "assignments", :force => true do |t| t.integer "node_id" t.integer "service_id" @@ -85,7 +81,7 @@ ActiveRecord::Schema.define(:version => 20101206225510) do create_table "old_reports", :force => true do |t| t.integer "node_id" - t.text "report", :limit => 2147483647 + t.text "report", :limit => 16777215 t.datetime "created_at" t.datetime "updated_at" t.string "host" diff --git a/spec/controllers/reports_controller_spec.rb b/spec/controllers/reports_controller_spec.rb index 41a4046..d990a73 100644 --- a/spec/controllers/reports_controller_spec.rb +++ b/spec/controllers/reports_controller_spec.rb @@ -3,84 +3,79 @@ require File.expand_path(File.dirname(__FILE__) + '/../spec_helper') require 'shared_behaviors/controller_mixins' describe ReportsController do + before :each do + @yaml = File.read(Rails.root.join('spec', 'fixtures', 'sample_report.yml')) + end + def model; Report end it_should_behave_like "without JSON pagination" - %w(create upload).each do |method| - describe "creating a report via #{method}" do - before :each do - @yaml = File.read(Rails.root.join('spec', 'fixtures', 'sample_report.yml')) - end + describe "#upload" do + describe "correctly formatted POST", :shared => true do + it { should_not raise_error } + it { should change(Report, :count).by(1) } + it { should change{ Node.find_by_name("sample_node")}.from(nil) } + end - describe "correctly formatted POST", :shared => true do - it { should_not raise_error } - it { should change(Report, :count).by(1) } - it { should change{ Node.find_by_name("sample_node")}.from(nil) } + describe "with a POST from Puppet 2.6.x" do + subject do + lambda { post_with_body('upload', @yaml, :content_type => 'application/x-yaml') } end - describe "with a POST from Puppet 2.6.x" do - subject do - lambda { post_with_body(method, @yaml, :content_type => 'application/x-yaml') } - end + it_should_behave_like "correctly formatted POST" + end - it_should_behave_like "correctly formatted POST" + describe "with a POST from Puppet 0.25.x" do + subject do + lambda { post('upload', :report => @yaml) } end - describe "with a POST from Puppet 0.25.x" do - subject do - lambda { post(method, :report => @yaml) } - end + it_should_behave_like "correctly formatted POST" + end - it_should_behave_like "correctly formatted POST" + describe "with a POST with a report inside the report parameter" do + subject do + lambda { post('upload', :report => { :report => @yaml }) } end - describe "with a POST with a report inside the report parameter" do - subject do - lambda { post(method, :report => { :report => @yaml }) } - end + it_should_behave_like "correctly formatted POST" + end - it_should_behave_like "correctly formatted POST" + describe "with a POST without a report, the response code" do + before :each do + post('upload', :report => "" ) end - describe "with a POST without a report, the response code" do - before :each do - post(method, :report => "" ) - end - - subject { response.code } - - it { should == "406" } + it "should return a 406 response and the error text" do + response.code.should == '406' + response.body.should == "ERROR! ReportsController#upload failed: The supplied report is in invalid format 'FalseClass', expected 'Puppet::Transaction::Report'" end + end - describe "with a POST with invalid report data, the response code" do - before :each do - post(method, :report => "foo bar baz bad data invalid") - end - - subject { response.code } + describe "with a POST with invalid report data, the response code" do + before :each do + post('upload', :report => "foo bar baz bad data invalid") + end - it { should == "406" } + it "should return a 406 response and the error text" do + response.code.should == '406' + response.body.should == "ERROR! ReportsController#upload failed: The supplied report is in invalid format 'String', expected 'Puppet::Transaction::Report'" end + end + end - describe "when disable_legacy_report_upload_url is set to true" do - before :each do - SETTINGS.stubs(:disable_legacy_report_upload_url).returns(true) - end - - if method == "create" - it "should fail with a 403 error" do - response = post_with_body(method, @yaml, :content_type => 'application/x-yaml') - response.status.should == "403 Forbidden" - end - else - it "should succeed" do - response = post_with_body(method, @yaml, :content_type => 'application/x-yaml') - response.status.should == "200 OK" - end - end + describe "#create" do + it "should fail with a 403 error when disable_legacy_report_upload_url is true" do + SETTINGS.stubs(:disable_legacy_report_upload_url).returns(true) + response = post_with_body('create', @yaml, :content_type => 'application/x-yaml') + response.status.should == "403 Forbidden" + end - end + it "should succeed when disable_legacy_report_upload_url is false" do + SETTINGS.stubs(:disable_legacy_report_upload_url).returns(false) + response = post_with_body('create', @yaml, :content_type => 'application/x-yaml') + response.status.should == "200 OK" end end -- 1.7.3.1 -- 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.
