From 7f692d717c9eef5f2604d797e2e37c9bfa504218 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 | 39 ++++++++++++++- spec/models/service_template_spec.rb | 73 +++++++++++++++++++--------- 2 files changed, 88 insertions(+), 24 deletions(-) diff --git a/app/models/service_template.rb b/app/models/service_template.rb index 3c188fac207..e88bcbe2188 100644 --- a/app/models/service_template.rb +++ b/app/models/service_template.rb @@ -392,11 +392,48 @@ 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 + result = order(user, options, request_options) raise result[:errors].join(", ") if result[:errors].any? result[:request] 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 eab9edb719a..53e003f4e93 100644 --- a/spec/models/service_template_spec.rb +++ b/spec/models/service_template_spec.rb @@ -810,35 +810,62 @@ 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'} } - 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 "provision's a service template without errors" do - expect(ResourceActionWorkflow).to(receive(:new) - .with({}, user, resource_action, hash).and_return(workflow)) - expect(workflow).to receive(:submit_request).and_return(good_result) - expect(workflow).to receive(:set_value).with('ordered_by', 'fred') - expect(workflow).to receive(:request_options=).with(:initiator => 'control') - - expect(service_template.provision_request(user, arg1, arg2)).to eq(miq_request) - end - - it "provision's a service template with errors" do - expect(ResourceActionWorkflow).to(receive(:new) - .with({}, user, resource_action, hash).and_return(workflow)) - expect(workflow).to receive(:submit_request).and_return(bad_result) - expect(workflow).to receive(:set_value).with('ordered_by', 'fred') - 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({}, user, resource_action, hash) } + + it "provision's a service template without errors" do + expect(ResourceActionWorkflow).to(receive(:new).with({}, 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(:set_value).with('ordered_by', 'fred') + 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 "provision's a service template with errors" do + expect(ResourceActionWorkflow).to(receive(:new).with({}, 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(:set_value).with('ordered_by', 'fred') + 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 f246d59edc327eb04335dfdbe4663f5cec1cc779 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 1db8e558673..a0eec07d289 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 08bf0f37d5bfaf306b0e526e510ad348e168c1c2 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 | 2 -- 1 file changed, 2 deletions(-) diff --git a/spec/models/service_template_spec.rb b/spec/models/service_template_spec.rb index 53e003f4e93..beaa79b385c 100644 --- a/spec/models/service_template_spec.rb +++ b/spec/models/service_template_spec.rb @@ -815,9 +815,7 @@ 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'} } - 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(:resource_action_workflow) { ResourceActionWorkflow.new({}, user, resource_action) } it "success no optional args" do From 7a64f1699054b4a7632d942e9ab6c7ee94b46e4e 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 beaa79b385c..4b95a2a3f86 100644 --- a/spec/models/service_template_spec.rb +++ b/spec/models/service_template_spec.rb @@ -814,12 +814,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'} } + let(:resource_action_options) { {:target => service_template, :initiator => 'control'} } 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) @@ -827,8 +830,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) @@ -844,11 +846,9 @@ context "#provision_request" do let(:arg1) { {'ordered_by' => 'fred'} } let(:arg2) { {:initiator => 'control'} } - let(:resource_action_workflow) { ResourceActionWorkflow.new({}, user, resource_action, hash) } it "provision's a service template without errors" do - expect(ResourceActionWorkflow).to(receive(:new).with({}, 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(:set_value).with('ordered_by', 'fred') expect(resource_action_workflow).to receive(:request_options=).with(:initiator => 'control') @@ -857,7 +857,6 @@ end it "provision's a service template with errors" do - expect(ResourceActionWorkflow).to(receive(:new).with({}, 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(:set_value).with('ordered_by', 'fred') expect(resource_action_workflow).to receive(:request_options=).with(:initiator => 'control')