From e9bd96e8de4454e25c8261ac5e42f69bf76875e4 Mon Sep 17 00:00:00 2001 From: Brandon Dunne Date: Fri, 15 Jun 2018 12:17:10 -0400 Subject: [PATCH 1/4] Refactor ordering a service template, allow scheduling and queueing --- app/models/service_template.rb | 41 +++++++++++++++- spec/models/service_template_spec.rb | 70 +++++++++++++++++++--------- 2 files changed, 87 insertions(+), 24 deletions(-) diff --git a/app/models/service_template.rb b/app/models/service_template.rb index a4c4f0edf21..f2b4301c8bf 100644 --- a/app/models/service_template.rb +++ b/app/models/service_template.rb @@ -398,8 +398,8 @@ def self.create_from_options(options) private_class_method :create_from_options def provision_request(user, options = nil, request_options = nil) - result = provision_workflow(user, options, request_options).submit_request - raise result[:errors].join(", ") if result[:errors].any? + result = order(user, options, request_options) + raise result[:errors].join(", ") if result[:errors] && result[:errors].any? result[:request] end @@ -408,6 +408,43 @@ def miq_schedules MiqSchedule.where(:towhat => "ServiceTemplate", :id => schedule_ids) end + def queue_order(user_id, options, request_options) + MiqQueue.submit_job( + :class_name => name, + :instance_id => id, + :method_name => "order", + :args => [user_id, options, request_options], + ) + end + + def order(user_or_id, options = nil, request_options = nil, schedule_time = nil) + user = user_or_id.kind_of?(User) ? user_or_id : User.find(user_or_id) + workflow = provision_workflow(user, options, request_options) + if schedule_time + require 'time' + time = Time.parse(schedule_time).utc + + errors = workflow.validate_dialog + return {:errors => errors} unless errors.blank? + + schedule = MiqSchedule.create!( + :name => "Order #{self.class.name} #{id}", + :description => "Order #{self.class.name} #{id}", + :sched_action => {:args => [user.id, options, request_options], :method => "queue_order"}, + :resource_id => id, + :towhat => "ServiceTemplate", + :run_at => { + :interval => {:unit => "once"}, + :start_time => time, + :tz => "UTC", + }, + ) + {:schedule => schedule} + else + workflow.submit_request + end + end + def provision_workflow(user, dialog_options = nil, request_options = nil) dialog_options ||= {} request_options ||= {} diff --git a/spec/models/service_template_spec.rb b/spec/models/service_template_spec.rb index 2c4c6ce9f6b..d7d00130070 100644 --- a/spec/models/service_template_spec.rb +++ b/spec/models/service_template_spec.rb @@ -804,33 +804,59 @@ end end - context "#provision_request" do + context "#order" do let(:user) { FactoryGirl.create(:user, :userid => "barney") } let(:resource_action) { FactoryGirl.create(:resource_action, :action => "Provision") } let(:service_template) { FactoryGirl.create(:service_template, :resource_actions => [resource_action]) } let(:hash) { {:target => service_template, :initiator => 'control', :submit_workflow => nil} } - let(:workflow) { instance_double(ResourceActionWorkflow) } + let(:workflow) { instance_double(ResourceActionWorkflow, :validate_dialog => nil) } let(:miq_request) { FactoryGirl.create(:service_template_provision_request) } - let(:good_result) { { :errors => [], :request => miq_request } } - let(:bad_result) { { :errors => %w(Error1 Error2), :request => miq_request } } - let(:arg1) { {'ordered_by' => 'fred'} } - let(:arg2) { {:initiator => 'control'} } - - it "provisions a service template without errors" do - expect(ResourceActionWorkflow).to(receive(:new) - .with({'ordered_by' => 'fred'}, user, resource_action, hash).and_return(workflow)) - expect(workflow).to receive(:submit_request).and_return(good_result) - expect(workflow).to receive(:request_options=).with(:initiator => 'control') - - expect(service_template.provision_request(user, arg1, arg2)).to eq(miq_request) - end - - it "provisions a service template with errors" do - expect(ResourceActionWorkflow).to(receive(:new) - .with({'ordered_by' => 'fred'}, user, resource_action, hash).and_return(workflow)) - expect(workflow).to receive(:submit_request).and_return(bad_result) - expect(workflow).to receive(:request_options=).with(:initiator => 'control') - expect { service_template.provision_request(user, arg1, arg2) }.to raise_error(RuntimeError) + let(:resource_action_workflow) { ResourceActionWorkflow.new({}, user, resource_action) } + + it "success no optional args" do + expect(ResourceActionWorkflow).to(receive(:new).and_return(resource_action_workflow)) + expect(resource_action_workflow).to receive(:submit_request).and_return(miq_request) + + expect(service_template.order(user)).to eq(miq_request) + end + + it "successfully scheduled" do + EvmSpecHelper.local_miq_server + expect(ResourceActionWorkflow).to(receive(:new).and_return(resource_action_workflow)) + expect(resource_action_workflow).to receive(:validate_dialog).and_return(nil) + + result = service_template.order(user, {}, {}, Time.now.utc.to_s) + + expect(result.keys).to eq([:schedule]) # No errors + expect(result[:schedule]).to have_attributes( + :name => "Order ServiceTemplate #{service_template.id}", + :sched_action => {:args => [user.id, {}, {}], :method => "queue_order"}, + :towhat => "ServiceTemplate", + :resource_id => service_template.id + ) + end + + context "#provision_request" do + let(:arg1) { {'ordered_by' => 'fred'} } + let(:arg2) { {:initiator => 'control'} } + let(:resource_action_workflow) { ResourceActionWorkflow.new(arg1, user, resource_action, hash) } + + it "provisions a service template without errors" do + expect(ResourceActionWorkflow).to(receive(:new).with(arg1, user, resource_action, hash).and_return(resource_action_workflow)) + expect(resource_action_workflow).to receive(:validate_dialog).and_return(nil) + expect(resource_action_workflow).to receive(:make_request).and_return(miq_request) + expect(resource_action_workflow).to receive(:request_options=).with(:initiator => 'control') + + expect(service_template.provision_request(user, arg1, arg2)).to eq(miq_request) + end + + it "provisions a service template with errors" do + expect(ResourceActionWorkflow).to(receive(:new).with(arg1, user, resource_action, hash).and_return(resource_action_workflow)) + expect(resource_action_workflow).to receive(:validate_dialog).and_return(%w(Error1 Error2)) + expect(resource_action_workflow).to receive(:request_options=).with(:initiator => 'control') + + expect { service_template.provision_request(user, arg1, arg2) }.to raise_error(RuntimeError) + end end end From 23d45fad255bc9b40088078d67555ca01604e6d4 Mon Sep 17 00:00:00 2001 From: Nick Carboni Date: Wed, 20 Jun 2018 10:59:17 -0400 Subject: [PATCH 2/4] Remove expect_any_instance_of calls in miq_schedule_spec --- spec/models/miq_schedule_spec.rb | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/spec/models/miq_schedule_spec.rb b/spec/models/miq_schedule_spec.rb index c61ee09b60a..17736ea7ec3 100644 --- a/spec/models/miq_schedule_spec.rb +++ b/spec/models/miq_schedule_spec.rb @@ -722,9 +722,14 @@ end context "resource exists" do + let(:resource) { FactoryGirl.create(:host) } + + before do + allow(Host).to receive(:find_by).with(:id => resource.id).and_return(resource) + end + it "and does not respond to the method" do - resource = FactoryGirl.create(:host) - schedule = FactoryGirl.create(:miq_schedule, :towhat => resource.class.name, :resource_id => resource, :sched_action => {:method => "test_method"}) + schedule = FactoryGirl.create(:miq_schedule, :towhat => resource.class.name, :resource_id => resource.id, :sched_action => {:method => "test_method"}) expect($log).to receive(:warn) do |message| expect(message).to include("no such action: [test_method], aborting schedule") @@ -734,19 +739,17 @@ end it "and responds to the method" do - resource = FactoryGirl.create(:host) - schedule = FactoryGirl.create(:miq_schedule, :towhat => resource.class.name, :resource_id => resource, :sched_action => {:method => "test_method"}) + schedule = FactoryGirl.create(:miq_schedule, :towhat => resource.class.name, :resource_id => resource.id, :sched_action => {:method => "test_method"}) - expect_any_instance_of(Host).to receive("test_method").once + expect(resource).to receive("test_method").once MiqSchedule.queue_scheduled_work(schedule.id, nil, "abc", nil) end it "and responds to the method with arguments" do - resource = FactoryGirl.create(:host) - schedule = FactoryGirl.create(:miq_schedule, :towhat => resource.class.name, :resource_id => resource, :sched_action => {:method => "test_method", :args => ["abc", 123, :a => 1]}) + schedule = FactoryGirl.create(:miq_schedule, :towhat => resource.class.name, :resource_id => resource.id, :sched_action => {:method => "test_method", :args => ["abc", 123, :a => 1]}) - expect_any_instance_of(Host).to receive("test_method").once.with("abc", 123, :a => 1) + expect(resource).to receive("test_method").once.with("abc", 123, :a => 1) MiqSchedule.queue_scheduled_work(schedule.id, nil, "abc", nil) end From 8792f6e7a05692400c8f1c246e7d76586435fe2d Mon Sep 17 00:00:00 2001 From: Nick Carboni Date: Wed, 20 Jun 2018 12:02:01 -0400 Subject: [PATCH 3/4] Remove unused spec variables --- spec/models/service_template_spec.rb | 1 - 1 file changed, 1 deletion(-) diff --git a/spec/models/service_template_spec.rb b/spec/models/service_template_spec.rb index d7d00130070..3542c4bed18 100644 --- a/spec/models/service_template_spec.rb +++ b/spec/models/service_template_spec.rb @@ -809,7 +809,6 @@ let(:resource_action) { FactoryGirl.create(:resource_action, :action => "Provision") } let(:service_template) { FactoryGirl.create(:service_template, :resource_actions => [resource_action]) } let(:hash) { {:target => service_template, :initiator => 'control', :submit_workflow => nil} } - let(:workflow) { instance_double(ResourceActionWorkflow, :validate_dialog => nil) } let(:miq_request) { FactoryGirl.create(:service_template_provision_request) } let(:resource_action_workflow) { ResourceActionWorkflow.new({}, user, resource_action) } From 1597bf89371c9831d3a3b080ff0069be03a9ed42 Mon Sep 17 00:00:00 2001 From: Nick Carboni Date: Wed, 20 Jun 2018 12:04:09 -0400 Subject: [PATCH 4/4] Remove duplicate resource_action_workflow and simplify stubbing The inner variable was shadowing the outer one, `hash` was poorly named and the setup can be moved into one `before` block. --- spec/models/service_template_spec.rb | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/spec/models/service_template_spec.rb b/spec/models/service_template_spec.rb index 3542c4bed18..d4676e843df 100644 --- a/spec/models/service_template_spec.rb +++ b/spec/models/service_template_spec.rb @@ -808,12 +808,15 @@ let(:user) { FactoryGirl.create(:user, :userid => "barney") } let(:resource_action) { FactoryGirl.create(:resource_action, :action => "Provision") } let(:service_template) { FactoryGirl.create(:service_template, :resource_actions => [resource_action]) } - let(:hash) { {:target => service_template, :initiator => 'control', :submit_workflow => nil} } + let(:resource_action_options) { {:target => service_template, :initiator => 'control', :submit_workflow => nil} } let(:miq_request) { FactoryGirl.create(:service_template_provision_request) } - let(:resource_action_workflow) { ResourceActionWorkflow.new({}, user, resource_action) } + let!(:resource_action_workflow) { ResourceActionWorkflow.new({}, user, resource_action, resource_action_options) } + + before do + allow(ResourceActionWorkflow).to(receive(:new).and_return(resource_action_workflow)) + end it "success no optional args" do - expect(ResourceActionWorkflow).to(receive(:new).and_return(resource_action_workflow)) expect(resource_action_workflow).to receive(:submit_request).and_return(miq_request) expect(service_template.order(user)).to eq(miq_request) @@ -821,8 +824,7 @@ it "successfully scheduled" do EvmSpecHelper.local_miq_server - expect(ResourceActionWorkflow).to(receive(:new).and_return(resource_action_workflow)) - expect(resource_action_workflow).to receive(:validate_dialog).and_return(nil) + expect(resource_action_workflow).to receive(:validate_dialog).and_return([]) result = service_template.order(user, {}, {}, Time.now.utc.to_s) @@ -838,11 +840,9 @@ context "#provision_request" do let(:arg1) { {'ordered_by' => 'fred'} } let(:arg2) { {:initiator => 'control'} } - let(:resource_action_workflow) { ResourceActionWorkflow.new(arg1, user, resource_action, hash) } it "provisions a service template without errors" do - expect(ResourceActionWorkflow).to(receive(:new).with(arg1, user, resource_action, hash).and_return(resource_action_workflow)) - expect(resource_action_workflow).to receive(:validate_dialog).and_return(nil) + expect(resource_action_workflow).to receive(:validate_dialog).and_return([]) expect(resource_action_workflow).to receive(:make_request).and_return(miq_request) expect(resource_action_workflow).to receive(:request_options=).with(:initiator => 'control') @@ -850,7 +850,6 @@ end it "provisions a service template with errors" do - expect(ResourceActionWorkflow).to(receive(:new).with(arg1, user, resource_action, hash).and_return(resource_action_workflow)) expect(resource_action_workflow).to receive(:validate_dialog).and_return(%w(Error1 Error2)) expect(resource_action_workflow).to receive(:request_options=).with(:initiator => 'control')