From b7047744b21596fafe4cbfdc1064cad1fd8ba76c Mon Sep 17 00:00:00 2001 From: Gregg Tanzillo Date: Mon, 30 Jul 2018 18:02:12 -0400 Subject: [PATCH] Merge pull request #421 from bdunne/request_cancel Request cancel (cherry picked from commit 62de390490c8bc333f27feedfc58455ced1e7483) https://bugzilla.redhat.com/show_bug.cgi?id=1610533 --- .../api/automation_requests_controller.rb | 1 + app/controllers/api/mixins/resource_cancel.rb | 14 +++++++ .../api/provision_requests_controller.rb | 1 + .../api/request_tasks_controller.rb | 1 + app/controllers/api/requests_controller.rb | 1 + .../api/service_requests_controller.rb | 1 + .../api/subcollections/request_tasks.rb | 7 ++++ .../api/subcollections/service_requests.rb | 7 ++++ config/api.yml | 31 ++++++++++++++ spec/requests/automation_requests_spec.rb | 12 ++++++ spec/requests/provision_requests_spec.rb | 12 ++++++ spec/requests/request_tasks_spec.rb | 7 ++++ spec/requests/requests_spec.rb | 12 ++++++ spec/requests/service_orders_spec.rb | 6 +++ spec/requests/service_requests_spec.rb | 12 ++++++ spec/requests/service_templates_spec.rb | 6 +++ spec/support/api/helpers.rb | 8 +++- spec/support/factory_overrides.rb | 23 +++++++++++ .../shared_examples/resource_cancel.rb | 40 +++++++++++++++++++ .../shared_examples/sub_resource_cancel.rb | 38 ++++++++++++++++++ 20 files changed, 239 insertions(+), 1 deletion(-) create mode 100644 app/controllers/api/mixins/resource_cancel.rb create mode 100644 spec/requests/request_tasks_spec.rb create mode 100644 spec/support/factory_overrides.rb create mode 100644 spec/support/shared_examples/resource_cancel.rb create mode 100644 spec/support/shared_examples/sub_resource_cancel.rb diff --git a/app/controllers/api/automation_requests_controller.rb b/app/controllers/api/automation_requests_controller.rb index 15139f0369..fb0fa21cff 100644 --- a/app/controllers/api/automation_requests_controller.rb +++ b/app/controllers/api/automation_requests_controller.rb @@ -1,5 +1,6 @@ module Api class AutomationRequestsController < BaseController + include Api::Mixins::ResourceCancel include Subcollections::RequestTasks def create_resource(type, _id, data) diff --git a/app/controllers/api/mixins/resource_cancel.rb b/app/controllers/api/mixins/resource_cancel.rb new file mode 100644 index 0000000000..52c1b6bf29 --- /dev/null +++ b/app/controllers/api/mixins/resource_cancel.rb @@ -0,0 +1,14 @@ +module Api + module Mixins + module ResourceCancel + def cancel_resource(type, id, _data) + api_action(type, id) do |klass| + resource_search(id, type, klass).cancel + action_result(true, "#{klass.name} #{id} canceled") + end + rescue => err + action_result(false, err.to_s) + end + end + end +end diff --git a/app/controllers/api/provision_requests_controller.rb b/app/controllers/api/provision_requests_controller.rb index c55b0bc29d..19b2891820 100644 --- a/app/controllers/api/provision_requests_controller.rb +++ b/app/controllers/api/provision_requests_controller.rb @@ -1,5 +1,6 @@ module Api class ProvisionRequestsController < BaseController + include Api::Mixins::ResourceCancel include Subcollections::RequestTasks def create_resource(type, _id, data) diff --git a/app/controllers/api/request_tasks_controller.rb b/app/controllers/api/request_tasks_controller.rb index 389e486b8d..1fb6414328 100644 --- a/app/controllers/api/request_tasks_controller.rb +++ b/app/controllers/api/request_tasks_controller.rb @@ -1,4 +1,5 @@ module Api class RequestTasksController < BaseController + include Api::Mixins::ResourceCancel end end diff --git a/app/controllers/api/requests_controller.rb b/app/controllers/api/requests_controller.rb index ac5d7e0e66..0b5b9d33a5 100644 --- a/app/controllers/api/requests_controller.rb +++ b/app/controllers/api/requests_controller.rb @@ -1,5 +1,6 @@ module Api class RequestsController < BaseController + include Api::Mixins::ResourceCancel include Subcollections::RequestTasks def create_resource(type, _id, data) diff --git a/app/controllers/api/service_requests_controller.rb b/app/controllers/api/service_requests_controller.rb index d8711be09b..68796b6a0e 100644 --- a/app/controllers/api/service_requests_controller.rb +++ b/app/controllers/api/service_requests_controller.rb @@ -2,6 +2,7 @@ module Api class ServiceRequestsController < BaseController include Subcollections::RequestTasks include Api::Mixins::Pictures + include Api::Mixins::ResourceCancel alias fetch_service_requests_picture fetch_picture diff --git a/app/controllers/api/subcollections/request_tasks.rb b/app/controllers/api/subcollections/request_tasks.rb index feab64994b..13daf607e6 100644 --- a/app/controllers/api/subcollections/request_tasks.rb +++ b/app/controllers/api/subcollections/request_tasks.rb @@ -15,6 +15,13 @@ def request_tasks_edit_resource(_object, type, id = nil, data = {}) end request_task end + + def request_tasks_cancel_resource(_object, type, id = nil, _data = {}) + resource_search(id, type, collection_class(:request_tasks)).cancel + action_result(true, "RequestTask #{id} canceled") + rescue => err + action_result(false, err.to_s) + end end end end diff --git a/app/controllers/api/subcollections/service_requests.rb b/app/controllers/api/subcollections/service_requests.rb index 62c6df188d..2d51a0dc2e 100644 --- a/app/controllers/api/subcollections/service_requests.rb +++ b/app/controllers/api/subcollections/service_requests.rb @@ -27,6 +27,13 @@ def service_requests_remove_resource(target, type, id, _data) end end + def service_requests_cancel_resource(_target, type, id, _data) + resource_search(id, type, collection_class(:service_requests)).cancel + action_result(true, "Service request #{id} canceled") + rescue => err + action_result(false, err.to_s) + end + private def service_request_ident(service_request) diff --git a/config/api.yml b/config/api.yml index b8298fcfb9..108a6db286 100644 --- a/config/api.yml +++ b/config/api.yml @@ -292,6 +292,8 @@ :identifier: miq_request_approval - :name: deny :identifier: miq_request_approval + - :name: cancel + :identifier: miq_request_control - :name: edit :identifier: miq_request_edit :resource_actions: @@ -303,6 +305,8 @@ :identifier: miq_request_approval - :name: deny :identifier: miq_request_approval + - :name: cancel + :identifier: miq_request_control - :name: edit :identifier: miq_request_edit :availability_zones: @@ -1785,6 +1789,8 @@ :identifier: miq_request_approval - :name: deny :identifier: miq_request_approval + - :name: cancel + :identifier: miq_request_control - :name: edit :identifier: miq_request_edit :resource_actions: @@ -1796,6 +1802,8 @@ :identifier: miq_request_approval - :name: deny :identifier: miq_request_approval + - :name: cancel + :identifier: miq_request_control - :name: edit :identifier: miq_request_edit :quotas: @@ -1921,15 +1929,26 @@ :get: - :name: read :identifier: miq_request_show + :post: + - :name: cancel + :identifier: miq_request_control + :resource_actions: + :post: + - :name: cancel + :identifier: miq_request_control :subcollection_actions: :get: - :name: read :identifier: miq_request_show :post: + - :name: cancel + :identifier: miq_request_control - :name: edit :identifier: miq_request_edit :subresource_actions: :post: + - :name: cancel + :identifier: miq_request_control - :name: edit :identifier: miq_request_edit :requests: @@ -1948,6 +1967,8 @@ :post: - :name: query :identifier: miq_request_show_list + - :name: cancel + :identifier: miq_request_control - :name: create :identifiers: - :klass: AutomationRequest @@ -1976,6 +1997,8 @@ - :name: read :identifier: miq_request_show :post: + - :name: cancel + :identifier: miq_request_control - :name: edit :identifier: miq_request_edit - :name: approve @@ -2298,6 +2321,8 @@ :post: - :name: query :identifier: miq_request_view + - :name: cancel + :identifier: miq_request_control - :name: approve :identifier: miq_request_approval - :name: deny @@ -2317,6 +2342,8 @@ :post: - :name: approve :identifier: miq_request_approval + - :name: cancel + :identifier: miq_request_control - :name: deny :identifier: miq_request_approval - :name: delete @@ -2337,6 +2364,8 @@ :post: - :name: add :identifier: svc_catalog_provision + - :name: cancel + :identifier: miq_request_control - :name: remove :identifier: svc_catalog_provision :subresource_actions: @@ -2344,6 +2373,8 @@ - :name: read :identifier: miq_request_view :post: + - :name: cancel + :identifier: miq_request_control - :name: remove :identifier: svc_catalog_provision :service_templates: diff --git a/spec/requests/automation_requests_spec.rb b/spec/requests/automation_requests_spec.rb index 63e1b1b597..ae1d610187 100644 --- a/spec/requests/automation_requests_spec.rb +++ b/spec/requests/automation_requests_spec.rb @@ -311,5 +311,17 @@ expect(response.parsed_body['options']).to match(hash_including(options)) expect(task.reload.options.keys).to all(be_kind_of(Symbol)) end + + context "SubResource#cancel" do + let(:resource_1_response) { {"success" => false, "message" => "Cancel operation is not supported for AutomationTask"} } + let(:resource_2_response) { {"success" => false, "message" => "Cancel operation is not supported for AutomationTask"} } + include_context "SubResource#cancel", [:automation_request, :request_task], :automation_request, :automation_task + end + end + + context "Resource#cancel" do + let(:resource_1_response) { {"success" => false, "message" => "Cancel operation is not supported for AutomationRequest"} } + let(:resource_2_response) { {"success" => false, "message" => "Cancel operation is not supported for AutomationRequest"} } + include_context "Resource#cancel", "automation_request", :automation_request end end diff --git a/spec/requests/provision_requests_spec.rb b/spec/requests/provision_requests_spec.rb index 00d7f2dc0e..b50203caab 100644 --- a/spec/requests/provision_requests_spec.rb +++ b/spec/requests/provision_requests_spec.rb @@ -501,5 +501,17 @@ expect(response.parsed_body['options']).to match(hash_including(options)) expect(task.reload.options.keys).to all(be_kind_of(Symbol)) end + + context "SubResource#cancel" do + let(:resource_1_response) { {"success" => false, "message" => "Cancel operation is not supported for MiqRequestTask"} } + let(:resource_2_response) { {"success" => false, "message" => "Cancel operation is not supported for MiqRequestTask"} } + include_context "SubResource#cancel", [:provision_request, :request_task], :miq_provision_request, :miq_request_task + end + end + + context "Resource#cancel" do + let(:resource_1_response) { {"success" => false, "message" => "Cancel operation is not supported for MiqProvisionRequest"} } + let(:resource_2_response) { {"success" => false, "message" => "Cancel operation is not supported for MiqProvisionRequest"} } + include_context "Resource#cancel", "provision_request", :miq_provision_request end end diff --git a/spec/requests/request_tasks_spec.rb b/spec/requests/request_tasks_spec.rb new file mode 100644 index 0000000000..2c27f2ebc1 --- /dev/null +++ b/spec/requests/request_tasks_spec.rb @@ -0,0 +1,7 @@ +RSpec.describe "Request Tasks API" do + context "Resource#cancel" do + let(:resource_1_response) { {"success" => false, "message" => "Cancel operation is not supported for MiqRequestTask"} } + let(:resource_2_response) { {"success" => false, "message" => "Cancel operation is not supported for MiqRequestTask"} } + include_context "Resource#cancel", "request_task", :miq_request_task + end +end diff --git a/spec/requests/requests_spec.rb b/spec/requests/requests_spec.rb index 89b203743c..e367cd6935 100644 --- a/spec/requests/requests_spec.rb +++ b/spec/requests/requests_spec.rb @@ -538,5 +538,17 @@ expect(response).to have_http_status(:ok) expect(task.reload.options.keys).to all(be_kind_of(Symbol)) end + + context "SubResource#cancel" do + let(:resource_1_response) { {"success" => false, "message" => "Cancel operation is not supported for MiqRequestTask"} } + let(:resource_2_response) { {"success" => false, "message" => "Cancel operation is not supported for MiqRequestTask"} } + include_context "SubResource#cancel", [:request, :request_task], :service_template_provision_request, :miq_request_task + end + end + + context "Resource#cancel" do + let(:resource_1_response) { {"success" => false, "message" => "Cancel operation is not supported for VmMigrateRequest"} } + let(:resource_2_response) { {"success" => false, "message" => "Cancel operation is not supported for VmMigrateRequest"} } + include_context "Resource#cancel", "request", :vm_migrate_request end end diff --git a/spec/requests/service_orders_spec.rb b/spec/requests/service_orders_spec.rb index 28a12cc833..371d79bab0 100644 --- a/spec/requests/service_orders_spec.rb +++ b/spec/requests/service_orders_spec.rb @@ -613,6 +613,12 @@ expect(shopping_cart.reload.state).to eq(ServiceOrder::STATE_CART) end end + + context "ServiceRequest#cancel" do + let(:resource_1_response) { {"success" => false, "message" => "Cancel operation is not supported for ServiceTemplateProvisionRequest"} } + let(:resource_2_response) { {"success" => false, "message" => "Cancel operation is not supported for ServiceTemplateProvisionRequest"} } + include_context "SubResource#cancel", [:service_order, :service_request], :shopping_cart, :service_template_provision_request + end end context 'Copy Service Order' do diff --git a/spec/requests/service_requests_spec.rb b/spec/requests/service_requests_spec.rb index c24bfe2178..3574a34422 100644 --- a/spec/requests/service_requests_spec.rb +++ b/spec/requests/service_requests_spec.rb @@ -649,5 +649,17 @@ def expect_result_to_have_user_email(email) expect(response.parsed_body['options']).to match(hash_including(options)) expect(task.reload.options.keys).to all(be_kind_of(Symbol)) end + + context "SubResource#cancel" do + let(:resource_1_response) { {"success" => true, "message" => "RequestTask #{resource_1.id} canceled"} } + let(:resource_2_response) { {"success" => true, "message" => "RequestTask #{resource_2.id} canceled"} } + include_context "SubResource#cancel", [:service_request, :request_task], :service_template_transformation_plan_request, :service_template_transformation_plan_task + end + end + + context "Resource#cancel" do + let(:resource_1_response) { {"success" => true, "message" => "ServiceTemplateProvisionRequest #{resource_1.id} canceled", "href" => instance_url(resource_1)} } + let(:resource_2_response) { {"success" => true, "message" => "ServiceTemplateProvisionRequest #{resource_2.id} canceled", "href" => instance_url(resource_2)} } + include_context "Resource#cancel", "service_request", :service_template_transformation_plan_request end end diff --git a/spec/requests/service_templates_spec.rb b/spec/requests/service_templates_spec.rb index a3344a5f33..a3c44245e8 100644 --- a/spec/requests/service_templates_spec.rb +++ b/spec/requests/service_templates_spec.rb @@ -293,6 +293,12 @@ expect(response.parsed_body).to include(expected) expect(response).to have_http_status(:ok) end + + context "ServiceRequest#cancel" do + let(:resource_1_response) { {"success" => true, "message" => "Service request #{resource_1.id} canceled"} } + let(:resource_2_response) { {"success" => true, "message" => "Service request #{resource_2.id} canceled"} } + include_context "SubResource#cancel", [:service_template, :service_request], :service_template, :service_template_transformation_plan_request + end end describe "Service Templates create" do diff --git a/spec/support/api/helpers.rb b/spec/support/api/helpers.rb index 6f6f7c3a81..67081c76f2 100644 --- a/spec/support/api/helpers.rb +++ b/spec/support/api/helpers.rb @@ -171,7 +171,7 @@ def expect_single_resource_query(attr_hash) def expect_single_action_result(options = {}) expect(response).to have_http_status(:ok) - expected = {} + expected = options.slice("href", "message", "success") expected["success"] = options[:success] if options.key?(:success) expected["message"] = a_string_matching(options[:message]) if options[:message] expected["href"] = a_string_matching(options[:href]) if options[:href] @@ -223,6 +223,12 @@ def expect_options_results(type, data = {}) expect(response.headers['Access-Control-Allow-Methods']).to include('OPTIONS') end + def expect_forbidden_request + api_basic_authorize + yield + expect(response).to have_http_status(:forbidden) + end + def select_attributes(attrlist) attrlist.sort.select { |attr| !::Api.encrypted_attribute?(attr) } end diff --git a/spec/support/factory_overrides.rb b/spec/support/factory_overrides.rb new file mode 100644 index 0000000000..c0318d0c25 --- /dev/null +++ b/spec/support/factory_overrides.rb @@ -0,0 +1,23 @@ +FactoryGirl.modify do + factory :miq_request do + trait :with_api_user do + requester { User.find_by(:name=>"API User") } + end + end + + factory :miq_request_task do + trait :with_api_user do + userid { User.find_by(:name=>"API User").userid } + end + end + + factory :service_order do + trait :with_api_user do + user { User.find_by(:name=>"API User") } + end + end + + factory :service_template do + trait :with_api_user + end +end diff --git a/spec/support/shared_examples/resource_cancel.rb b/spec/support/shared_examples/resource_cancel.rb new file mode 100644 index 0000000000..7d72133237 --- /dev/null +++ b/spec/support/shared_examples/resource_cancel.rb @@ -0,0 +1,40 @@ +RSpec.shared_context "Resource#cancel" do |ns, factory| + let(:collection_identifier) { namespace.pluralize.to_sym } + let(:collection_url) { send("api_#{collection_identifier}_url") } + let(:namespace) { ns } # ns is not available from #instance_url method, but namespace is. + let(:resource_1) { FactoryGirl.create(factory, :with_api_user) } + let(:resource_2) { FactoryGirl.create(factory, :with_api_user) } + + def instance_url(instance) + send("api_#{namespace}_url", nil, instance) + end + + context "on a single instance" do + it "unauthorized" do + expect_forbidden_request { post(instance_url(resource_1), :params => gen_request(:cancel)) } + end + + it "authorized" do + api_basic_authorize collection_action_identifier(collection_identifier, :cancel) + + post(instance_url(resource_1), :params => gen_request(:cancel)) + + expect_single_action_result(resource_1_response) + end + end + + context "on multiple instances" do + it "unauthorized" do + expect_forbidden_request { post(collection_url, :params => gen_request(:cancel, [{"href" => instance_url(resource_1)}, {"href" => instance_url(resource_2)}])) } + end + + it "authorized" do + api_basic_authorize collection_action_identifier(collection_identifier, :cancel) + + post(collection_url, :params => gen_request(:cancel, [{"href" => instance_url(resource_1)}, {"href" => instance_url(resource_2)}])) + + expect(response.parsed_body).to include("results" => a_collection_containing_exactly(resource_1_response, resource_2_response)) + expect(response).to have_http_status(:ok) + end + end +end diff --git a/spec/support/shared_examples/sub_resource_cancel.rb b/spec/support/shared_examples/sub_resource_cancel.rb new file mode 100644 index 0000000000..c3d8124d71 --- /dev/null +++ b/spec/support/shared_examples/sub_resource_cancel.rb @@ -0,0 +1,38 @@ +RSpec.shared_context "SubResource#cancel" do |ns, request_factory, factory| + let(:base_namespace) { ns.first.to_s.pluralize.to_sym } + let(:sub_namespace) { ns[1].to_s.pluralize.to_sym } + let(:namespace) { ns.join("_") } + let(:collection_url) { "api_#{namespace.pluralize}_url" } + let(:instance_url) { "api_#{namespace}_url" } + let(:request) { FactoryGirl.create(request_factory, :with_api_user) } + let(:resource_1) { FactoryGirl.create(factory, :with_api_user) } + let(:resource_2) { FactoryGirl.create(factory, :with_api_user) } + + context "single instance cancel" do + it "unauthorized" do + expect_forbidden_request { post(send(instance_url, nil, request, resource_1.id), :params => gen_request(:cancel)) } + end + + it "authorized" do + api_basic_authorize subcollection_action_identifier(base_namespace, sub_namespace, :cancel) + post(send(instance_url, nil, request, resource_1.id), :params => gen_request(:cancel)) + + expect(response).to have_http_status(:ok) + expect(response.parsed_body).to eq(resource_1_response) + end + end + + context "multiple instance cancel" do + it "unauthorized" do + expect_forbidden_request { post(send(collection_url, nil, request), :params => gen_request(:cancel, [{:id => resource_1.id}, {:id => resource_2.id}])) } + end + + it "authorized" do + api_basic_authorize subcollection_action_identifier(base_namespace, sub_namespace, :cancel) + post(send(collection_url, nil, request), :params => gen_request(:cancel, [{:id => resource_1.id}, {:id => resource_2.id}])) + + expect(response).to have_http_status(:ok) + expect(response.parsed_body["results"]).to match_array([resource_1_response, resource_2_response]) + end + end +end