Commit 504cbb27 authored by Thong Kuah's avatar Thong Kuah

Remove un-used inheritance from service

Remove the inheritance from ::BaseService which is causing us to inherit
the initializer that has project as the first arg, as we will not have
access to project with forthcoming group clusters.

Also call install service from create service - 1 less thing to re-use

Extract TestRequest code into a spec helper. Given that we need
different behaviour for Rails 5.0 (and again in Rails 5.1!), it's handy
to have that branching behaviour in one place
parent cc1ccbf8
...@@ -6,12 +6,10 @@ class Projects::Clusters::ApplicationsController < Projects::ApplicationControll ...@@ -6,12 +6,10 @@ class Projects::Clusters::ApplicationsController < Projects::ApplicationControll
before_action :authorize_create_cluster!, only: [:create] before_action :authorize_create_cluster!, only: [:create]
def create def create
application = Clusters::Applications::CreateService Clusters::Applications::CreateService
.new(@cluster, current_user, create_cluster_application_params) .new(@cluster, current_user, create_cluster_application_params)
.execute(request) .execute(request)
Clusters::Applications::ScheduleInstallationService.new(project, current_user).execute(application)
head :no_content head :no_content
rescue Clusters::Applications::CreateService::InvalidApplicationError rescue Clusters::Applications::CreateService::InvalidApplicationError
render_404 render_404
......
...@@ -24,6 +24,8 @@ module Clusters ...@@ -24,6 +24,8 @@ module Clusters
end end
application.save! application.save!
Clusters::Applications::ScheduleInstallationService.new(application).execute
end end
end end
......
...@@ -2,8 +2,14 @@ ...@@ -2,8 +2,14 @@
module Clusters module Clusters
module Applications module Applications
class ScheduleInstallationService < ::BaseService class ScheduleInstallationService
def execute(application) attr_reader :application
def initialize(application)
@application = application
end
def execute
application.make_scheduled! application.make_scheduled!
ClusterInstallAppWorker.perform_async(application.name, application.id) ClusterInstallAppWorker.perform_async(application.name, application.id)
......
# frozen_string_literal: true
require "spec_helper" require "spec_helper"
describe ::Applications::CreateService do describe ::Applications::CreateService do
include TestRequestHelpers
let(:user) { create(:user) } let(:user) { create(:user) }
let(:params) { attributes_for(:application) } let(:params) { attributes_for(:application) }
let(:request) do
if Gitlab.rails5?
ActionController::TestRequest.new({ remote_ip: "127.0.0.1" }, ActionController::TestSession.new)
else
ActionController::TestRequest.new(remote_ip: "127.0.0.1")
end
end
subject { described_class.new(user, params) } subject { described_class.new(user, params) }
it { expect { subject.execute(request) }.to change { Doorkeeper::Application.count }.by(1) } it { expect { subject.execute(test_request) }.to change { Doorkeeper::Application.count }.by(1) }
end end
...@@ -3,21 +3,19 @@ ...@@ -3,21 +3,19 @@
require 'spec_helper' require 'spec_helper'
describe Clusters::Applications::CreateService do describe Clusters::Applications::CreateService do
let(:cluster) { create(:cluster) } include TestRequestHelpers
let(:cluster) { create(:cluster, :project, :provided_by_gcp) }
let(:user) { create(:user) } let(:user) { create(:user) }
let(:params) { { application: 'helm' } } let(:params) { { application: 'helm' } }
let(:service) { described_class.new(cluster, user, params) } let(:service) { described_class.new(cluster, user, params) }
let(:request) do describe '#execute' do
if Gitlab.rails5? before do
ActionController::TestRequest.new({ remote_ip: "127.0.0.1" }, ActionController::TestSession.new) allow(ClusterInstallAppWorker).to receive(:perform_async)
else
ActionController::TestRequest.new(remote_ip: "127.0.0.1")
end end
end
describe '#execute' do subject { service.execute(test_request) }
subject { service.execute(request) }
it 'creates an application' do it 'creates an application' do
expect do expect do
...@@ -27,6 +25,12 @@ describe Clusters::Applications::CreateService do ...@@ -27,6 +25,12 @@ describe Clusters::Applications::CreateService do
end.to change(cluster, :application_helm) end.to change(cluster, :application_helm)
end end
it 'schedules an install via worker' do
expect(ClusterInstallAppWorker).to receive(:perform_async).with('helm', anything).once
subject
end
context 'jupyter application' do context 'jupyter application' do
let(:params) do let(:params) do
{ {
...@@ -35,6 +39,10 @@ describe Clusters::Applications::CreateService do ...@@ -35,6 +39,10 @@ describe Clusters::Applications::CreateService do
} }
end end
before do
allow_any_instance_of(Clusters::Applications::ScheduleInstallationService).to receive(:execute)
end
it 'creates the application' do it 'creates the application' do
expect do expect do
subject subject
......
...@@ -10,14 +10,13 @@ describe Clusters::Applications::ScheduleInstallationService do ...@@ -10,14 +10,13 @@ describe Clusters::Applications::ScheduleInstallationService do
expect(ClusterInstallAppWorker).not_to receive(:perform_async) expect(ClusterInstallAppWorker).not_to receive(:perform_async)
count_before = count_scheduled count_before = count_scheduled
expect { service.execute(application) }.to raise_error(StandardError) expect { service.execute }.to raise_error(StandardError)
expect(count_scheduled).to eq(count_before) expect(count_scheduled).to eq(count_before)
end end
end end
describe '#execute' do describe '#execute' do
let(:project) { double(:project) } let(:service) { described_class.new(application) }
let(:service) { described_class.new(project, nil) }
context 'when application is installable' do context 'when application is installable' do
let(:application) { create(:clusters_applications_helm, :installable) } let(:application) { create(:clusters_applications_helm, :installable) }
...@@ -25,7 +24,7 @@ describe Clusters::Applications::ScheduleInstallationService do ...@@ -25,7 +24,7 @@ describe Clusters::Applications::ScheduleInstallationService do
it 'make the application scheduled' do it 'make the application scheduled' do
expect(ClusterInstallAppWorker).to receive(:perform_async).with(application.name, kind_of(Numeric)).once expect(ClusterInstallAppWorker).to receive(:perform_async).with(application.name, kind_of(Numeric)).once
expect { service.execute(application) }.to change { application.class.with_status(:scheduled).count }.by(1) expect { service.execute }.to change { application.class.with_status(:scheduled).count }.by(1)
end end
end end
......
# frozen_string_literal: true
module TestRequestHelpers
def test_request(remote_ip: '127.0.0.1')
if Gitlab.rails5?
ActionController::TestRequest.new({ remote_ip: remote_ip }, ActionController::TestSession.new)
else
ActionController::TestRequest.new(remote_ip: remote_ip)
end
end
end
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment