Commit a83c84dc authored by Bob Van Landuyt's avatar Bob Van Landuyt

Merge branch 'justin_ho-refactor-test-service-to-service-object' into 'master'

Refactor Service#test to use service object

See merge request gitlab-org/gitlab!31854
parents 76e12073 fe694ad2
......@@ -60,11 +60,10 @@ class Projects::ServicesController < Projects::ApplicationController
return { error: true, message: _('Validations failed.'), service_response: @service.errors.full_messages.join(','), test_failed: false }
end
data = @service.test_data(project, current_user)
outcome = @service.test(data)
result = Integrations::Test::ProjectService.new(@service, current_user, params[:event]).execute
unless outcome[:success]
return { error: true, message: _('Test failed.'), service_response: outcome[:result].to_s, test_failed: true }
unless result[:success]
return { error: true, message: _('Test failed.'), service_response: result[:message].to_s, test_failed: true }
end
{}
......
......@@ -215,11 +215,6 @@ class JiraService < IssueTrackerService
{ success: success, result: result }
end
# Jira does not need test data.
def test_data(_, _)
nil
end
override :support_close_issue?
def support_close_issue?
true
......
......@@ -56,12 +56,6 @@ class PipelinesEmailService < Service
project&.ci_pipelines&.any?
end
def test_data(project, user)
data = Gitlab::DataBuilder::Pipeline.build(project.ci_pipelines.last)
data[:user] = user.hook_attrs
data
end
def fields
[
{ type: 'textarea',
......
......@@ -144,10 +144,6 @@ class Service < ApplicationRecord
data_fields.as_json(only: data_fields.class.column_names).except('id', 'service_id')
end
def test_data(project, user)
Gitlab::DataBuilder::Push.build_sample(project, user)
end
def event_channel_names
[]
end
......
# frozen_string_literal: true
module Integrations
module ProjectTestData
private
def push_events_data
Gitlab::DataBuilder::Push.build_sample(project, current_user)
end
def note_events_data
note = project.notes.first
return { error: s_('TestHooks|Ensure the project has notes.') } unless note.present?
Gitlab::DataBuilder::Note.build(note, current_user)
end
def issues_events_data
issue = project.issues.first
return { error: s_('TestHooks|Ensure the project has issues.') } unless issue.present?
issue.to_hook_data(current_user)
end
def merge_requests_events_data
merge_request = project.merge_requests.first
return { error: s_('TestHooks|Ensure the project has merge requests.') } unless merge_request.present?
merge_request.to_hook_data(current_user)
end
def job_events_data
build = project.builds.first
return { error: s_('TestHooks|Ensure the project has CI jobs.') } unless build.present?
Gitlab::DataBuilder::Build.build(build)
end
def pipeline_events_data
pipeline = project.ci_pipelines.newest_first.first
return { error: s_('TestHooks|Ensure the project has CI pipelines.') } unless pipeline.present?
Gitlab::DataBuilder::Pipeline.build(pipeline)
end
def wiki_page_events_data
page = project.wiki.list_pages(limit: 1).first
if !project.wiki_enabled? || page.blank?
return { error: s_('TestHooks|Ensure the wiki is enabled and has pages.') }
end
Gitlab::DataBuilder::WikiPage.build(page, current_user, 'create')
end
def deployment_events_data
deployment = project.deployments.first
return { error: s_('TestHooks|Ensure the project has deployments.') } unless deployment.present?
Gitlab::DataBuilder::Deployment.build(deployment)
end
end
end
# frozen_string_literal: true
module Integrations
module Test
class BaseService
include BaseServiceUtility
attr_accessor :integration, :current_user, :event
# @param integration [Service] The external service that will be called
# @param current_user [User] The user calling the service
# @param event [String/nil] The event that triggered this
def initialize(integration, current_user, event = nil)
@integration = integration
@current_user = current_user
@event = event
end
def execute
if event && (integration.supported_events.exclude?(event) || data.blank?)
return error('Testing not available for this event')
end
return error(data[:error]) if data[:error].present?
integration.test(data)
end
private
def data
raise NotImplementedError
end
end
end
end
# frozen_string_literal: true
module Integrations
module Test
class ProjectService < Integrations::Test::BaseService
include Integrations::ProjectTestData
include Gitlab::Utils::StrongMemoize
def project
strong_memoize(:project) do
integration.project
end
end
private
def data
strong_memoize(:data) do
next pipeline_events_data if integration.is_a?(::PipelinesEmailService)
case event
when 'push', 'tag_push'
push_events_data
when 'note', 'confidential_note'
note_events_data
when 'issue', 'confidential_issue'
issues_events_data
when 'merge_request'
merge_requests_events_data
when 'job'
job_events_data
when 'pipeline'
pipeline_events_data
when 'wiki_page'
wiki_page_events_data
when 'deployment'
deployment_events_data
else
push_events_data
end
end
end
end
end
end
Integrations::Test::ProjectService.prepend_if_ee('::EE::Integrations::Test::ProjectService')
......@@ -2,6 +2,8 @@
module TestHooks
class BaseService
include BaseServiceUtility
attr_accessor :hook, :current_user, :trigger
def initialize(hook, current_user, trigger)
......@@ -12,31 +14,11 @@ module TestHooks
def execute
trigger_key = hook.class.triggers.key(trigger.to_sym)
trigger_data_method = "#{trigger}_data"
if trigger_key.nil? || !self.respond_to?(trigger_data_method, true)
return error('Testing not available for this hook')
end
error_message = catch(:validation_error) do # rubocop:disable Cop/BanCatchThrow
sample_data = self.__send__(trigger_data_method) # rubocop:disable GitlabSecurity/PublicSend
return hook.execute(sample_data, trigger_key) # rubocop:disable Cop/AvoidReturnFromBlocks
end
error(error_message)
end
private
def error(message, http_status = nil)
result = {
message: message,
status: :error
}
return error('Testing not available for this hook') if trigger_key.nil? || data.blank?
return error(data[:error]) if data[:error].present?
result[:http_status] = http_status if http_status
result
hook.execute(data, trigger_key)
end
end
end
......@@ -2,6 +2,9 @@
module TestHooks
class ProjectService < TestHooks::BaseService
include Integrations::ProjectTestData
include Gitlab::Utils::StrongMemoize
attr_writer :project
def project
......@@ -10,58 +13,25 @@ module TestHooks
private
def push_events_data
throw(:validation_error, s_('TestHooks|Ensure the project has at least one commit.')) if project.empty_repo? # rubocop:disable Cop/BanCatchThrow
Gitlab::DataBuilder::Push.build_sample(project, current_user)
end
alias_method :tag_push_events_data, :push_events_data
def note_events_data
note = project.notes.first
throw(:validation_error, s_('TestHooks|Ensure the project has notes.')) unless note.present? # rubocop:disable Cop/BanCatchThrow
Gitlab::DataBuilder::Note.build(note, current_user)
end
def issues_events_data
issue = project.issues.first
throw(:validation_error, s_('TestHooks|Ensure the project has issues.')) unless issue.present? # rubocop:disable Cop/BanCatchThrow
issue.to_hook_data(current_user)
end
alias_method :confidential_issues_events_data, :issues_events_data
def merge_requests_events_data
merge_request = project.merge_requests.first
throw(:validation_error, s_('TestHooks|Ensure the project has merge requests.')) unless merge_request.present? # rubocop:disable Cop/BanCatchThrow
merge_request.to_hook_data(current_user)
end
def job_events_data
build = project.builds.first
throw(:validation_error, s_('TestHooks|Ensure the project has CI jobs.')) unless build.present? # rubocop:disable Cop/BanCatchThrow
Gitlab::DataBuilder::Build.build(build)
end
def pipeline_events_data
pipeline = project.ci_pipelines.first
throw(:validation_error, s_('TestHooks|Ensure the project has CI pipelines.')) unless pipeline.present? # rubocop:disable Cop/BanCatchThrow
Gitlab::DataBuilder::Pipeline.build(pipeline)
end
def wiki_page_events_data
page = project.wiki.list_pages(limit: 1).first
if !project.wiki_enabled? || page.blank?
throw(:validation_error, s_('TestHooks|Ensure the wiki is enabled and has pages.')) # rubocop:disable Cop/BanCatchThrow
def data
strong_memoize(:data) do
case trigger
when 'push_events', 'tag_push_events'
push_events_data
when 'note_events'
note_events_data
when 'issues_events', 'confidential_issues_events'
issues_events_data
when 'merge_requests_events'
merge_requests_events_data
when 'job_events'
job_events_data
when 'pipeline_events'
pipeline_events_data
when 'wiki_page_events'
wiki_page_events_data
end
end
Gitlab::DataBuilder::WikiPage.build(page, current_user, 'create')
end
end
end
......@@ -2,23 +2,26 @@
module TestHooks
class SystemService < TestHooks::BaseService
private
def push_events_data
Gitlab::DataBuilder::Push.sample_data
end
include Gitlab::Utils::StrongMemoize
def tag_push_events_data
Gitlab::DataBuilder::Push.sample_data
end
private
def repository_update_events_data
Gitlab::DataBuilder::Repository.sample_data
def data
strong_memoize(:data) do
case trigger
when 'push_events', 'tag_push_events'
Gitlab::DataBuilder::Push.sample_data
when 'repository_update_events'
Gitlab::DataBuilder::Repository.sample_data
when 'merge_requests_events'
merge_requests_events_data
end
end
end
def merge_requests_events_data
merge_request = MergeRequest.of_projects(current_user.projects.select(:id)).first
throw(:validation_error, s_('TestHooks|Ensure one of your projects has merge requests.')) unless merge_request.present? # rubocop:disable Cop/BanCatchThrow
return { error: s_('TestHooks|Ensure one of your projects has merge requests.') } unless merge_request.present?
merge_request.to_hook_data(current_user)
end
......
......@@ -63,10 +63,6 @@ class GithubService < Service
project&.ci_pipelines&.any?
end
def disabled_title
'Please set up a pipeline on your repository.'
end
def execute(data)
return if disabled? || invalid? || irrelevant_result?(data)
......@@ -75,14 +71,6 @@ class GithubService < Service
update_status(status_message)
end
def test_data(project, user)
pipeline = project.ci_pipelines.newest_first.first
raise disabled_title unless pipeline
Gitlab::DataBuilder::Pipeline.build(pipeline)
end
def test(data)
begin
result = execute(data)
......
# frozen_string_literal: true
module EE
module Integrations
module Test
module ProjectService
extend ::Gitlab::Utils::Override
include ::Gitlab::Utils::StrongMemoize
private
override :data
def data
strong_memoize(:data) do
next pipeline_events_data if integration.is_a?(::GithubService)
super
end
end
end
end
end
end
......@@ -7,13 +7,6 @@ FactoryBot.define do
type { 'GitlabSlackApplicationService' }
end
factory :github_service do
project
active { true }
token { 'github-token' }
type { 'GithubService' }
end
factory :slack_slash_commands_service do
project
active { true }
......
......@@ -54,7 +54,6 @@ RSpec.describe 'User activates GitHub Service' do
)
click_button 'Test settings and save changes'
wait_for_requests
expect(page).to have_content('GitHub activated.')
end
......
......@@ -299,23 +299,6 @@ RSpec.describe GithubService do
end
end
describe '#test_data' do
let(:user) { project.owner }
let(:test_data) { subject.test_data(project, user) }
it 'raises error if no pipeline found' do
project.ci_pipelines.delete_all
expect { test_data }.to raise_error 'Please set up a pipeline on your repository.'
end
it 'generates data for latest pipeline' do
pipeline
expect(test_data[:object_kind]).to eq 'pipeline'
end
end
describe '#test' do
it 'mentions creator in success message' do
dummy_response = { context: "default", creator: { login: "YourUser" } }
......
# frozen_string_literal: true
require 'spec_helper'
describe ::Integrations::Test::ProjectService do
let(:user) { double('user') }
describe '#execute' do
let(:project) { create(:project) }
let(:event) { nil }
let(:sample_data) { { data: 'sample' } }
let(:success_result) { { success: true, result: {} } }
subject { described_class.new(integration, user, event).execute }
context 'without event specified' do
context 'GitHubService' do
let(:integration) { create(:github_service, project: project) }
it_behaves_like 'tests for integration with pipeline data'
end
end
end
end
......@@ -21813,7 +21813,7 @@ msgstr ""
msgid "TestHooks|Ensure the project has CI pipelines."
msgstr ""
msgid "TestHooks|Ensure the project has at least one commit."
msgid "TestHooks|Ensure the project has deployments."
msgstr ""
msgid "TestHooks|Ensure the project has issues."
......
......@@ -165,6 +165,20 @@ FactoryBot.define do
type { 'SlackService' }
end
factory :github_service do
project
type { 'GithubService' }
active { true }
token { 'github-token' }
end
factory :pipelines_email_service do
project
active { true }
type { 'PipelinesEmailService' }
recipients { 'test@example.com' }
end
# this is for testing storing values inside properties, which is deprecated and will be removed in
# https://gitlab.com/gitlab-org/gitlab/issues/29404
trait :without_properties_callback do
......
......@@ -37,22 +37,6 @@ describe PipelinesEmailService, :mailer do
end
end
describe '#test_data' do
let(:build) { create(:ci_build) }
let(:project) { build.project }
let(:user) { create(:user) }
before do
project.add_developer(user)
end
it 'builds test data' do
data = subject.test_data(project, user)
expect(data[:object_kind]).to eq('pipeline')
end
end
shared_examples 'sending email' do |branches_to_be_notified: nil|
before do
subject.recipients = recipients
......
# frozen_string_literal: true
require 'spec_helper'
describe Integrations::Test::ProjectService do
let(:user) { double('user') }
describe '#execute' do
let(:project) { create(:project) }
let(:integration) { create(:slack_service, project: project) }
let(:event) { nil }
let(:sample_data) { { data: 'sample' } }
let(:success_result) { { success: true, result: {} } }
subject { described_class.new(integration, user, event).execute }
context 'without event specified' do
it 'tests the integration with default data' do
allow(Gitlab::DataBuilder::Push).to receive(:build_sample).and_return(sample_data)
expect(integration).to receive(:test).with(sample_data).and_return(success_result)
expect(subject).to eq(success_result)
end
context 'PipelinesEmailService' do
let(:integration) { create(:pipelines_email_service, project: project) }
it_behaves_like 'tests for integration with pipeline data'
end
end
context 'with event specified' do
context 'event not supported by integration' do
let(:integration) { create(:jira_service, project: project) }
let(:event) { 'push' }
it 'returns error message' do
expect(subject).to include({ status: :error, message: 'Testing not available for this event' })
end
end
context 'push' do
let(:event) { 'push' }
it 'executes integration' do
allow(Gitlab::DataBuilder::Push).to receive(:build_sample).and_return(sample_data)
expect(integration).to receive(:test).with(sample_data).and_return(success_result)
expect(subject).to eq(success_result)
end
end
context 'tag_push' do
let(:event) { 'tag_push' }
it 'executes integration' do
allow(Gitlab::DataBuilder::Push).to receive(:build_sample).and_return(sample_data)
expect(integration).to receive(:test).with(sample_data).and_return(success_result)
expect(subject).to eq(success_result)
end
end
context 'note' do
let(:event) { 'note' }
it 'returns error message if not enough data' do
expect(integration).not_to receive(:test)
expect(subject).to include({ status: :error, message: 'Ensure the project has notes.' })
end
it 'executes integration' do
allow(project).to receive(:notes).and_return([Note.new])
allow(Gitlab::DataBuilder::Note).to receive(:build).and_return(sample_data)
expect(integration).to receive(:test).with(sample_data).and_return(success_result)
expect(subject).to eq(success_result)
end
end
context 'issue' do
let(:event) { 'issue' }
let(:issue) { build(:issue) }
it 'returns error message if not enough data' do
expect(integration).not_to receive(:test)
expect(subject).to include({ status: :error, message: 'Ensure the project has issues.' })
end
it 'executes integration' do
allow(project).to receive(:issues).and_return([issue])
allow(issue).to receive(:to_hook_data).and_return(sample_data)
expect(integration).to receive(:test).with(sample_data).and_return(success_result)
expect(subject).to eq(success_result)
end
end
context 'confidential_issue' do
let(:event) { 'confidential_issue' }
let(:issue) { build(:issue) }
it 'returns error message if not enough data' do
expect(integration).not_to receive(:test)
expect(subject).to include({ status: :error, message: 'Ensure the project has issues.' })
end
it 'executes integration' do
allow(project).to receive(:issues).and_return([issue])
allow(issue).to receive(:to_hook_data).and_return(sample_data)
expect(integration).to receive(:test).with(sample_data).and_return(success_result)
expect(subject).to eq(success_result)
end
end
context 'merge_request' do
let(:event) { 'merge_request' }
it 'returns error message if not enough data' do
expect(integration).not_to receive(:test)
expect(subject).to include({ status: :error, message: 'Ensure the project has merge requests.' })
end
it 'executes integration' do
create(:merge_request, source_project: project)
allow_any_instance_of(MergeRequest).to receive(:to_hook_data).and_return(sample_data)
expect(integration).to receive(:test).with(sample_data).and_return(success_result)
expect(subject).to eq(success_result)
end
end
context 'deployment' do
let(:project) { create(:project, :test_repo) }
let(:event) { 'deployment' }
it 'returns error message if not enough data' do
expect(integration).not_to receive(:test)
expect(subject).to include({ status: :error, message: 'Ensure the project has deployments.' })
end
it 'executes integration' do
create(:deployment, project: project)
allow(Gitlab::DataBuilder::Deployment).to receive(:build).and_return(sample_data)
expect(integration).to receive(:test).with(sample_data).and_return(success_result)
expect(subject).to eq(success_result)
end
end
context 'pipeline' do
let(:event) { 'pipeline' }
it 'returns error message if not enough data' do
expect(integration).not_to receive(:test)
expect(subject).to include({ status: :error, message: 'Ensure the project has CI pipelines.' })
end
it 'executes integration' do
create(:ci_empty_pipeline, project: project)
allow(Gitlab::DataBuilder::Pipeline).to receive(:build).and_return(sample_data)
expect(integration).to receive(:test).with(sample_data).and_return(success_result)
expect(subject).to eq(success_result)
end
end
context 'wiki_page' do
let(:project) { create(:project, :wiki_repo) }
let(:event) { 'wiki_page' }
it 'returns error message if wiki disabled' do
allow(project).to receive(:wiki_enabled?).and_return(false)
expect(integration).not_to receive(:test)
expect(subject).to include({ status: :error, message: 'Ensure the wiki is enabled and has pages.' })
end
it 'returns error message if not enough data' do
expect(integration).not_to receive(:test)
expect(subject).to include({ status: :error, message: 'Ensure the wiki is enabled and has pages.' })
end
it 'executes integration' do
create(:wiki_page, wiki: project.wiki)
allow(Gitlab::DataBuilder::WikiPage).to receive(:build).and_return(sample_data)
expect(integration).to receive(:test).with(sample_data).and_return(success_result)
expect(subject).to eq(success_result)
end
end
end
end
end
......@@ -31,15 +31,7 @@ describe TestHooks::ProjectService do
let(:trigger) { 'push_events' }
let(:trigger_key) { :push_hooks }
it 'returns error message if not enough data' do
allow(project).to receive(:empty_repo?).and_return(true)
expect(hook).not_to receive(:execute)
expect(service.execute).to include({ status: :error, message: 'Ensure the project has at least one commit.' })
end
it 'executes hook' do
allow(project).to receive(:empty_repo?).and_return(false)
allow(Gitlab::DataBuilder::Push).to receive(:build_sample).and_return(sample_data)
expect(hook).to receive(:execute).with(sample_data, trigger_key).and_return(success_result)
......@@ -51,15 +43,7 @@ describe TestHooks::ProjectService do
let(:trigger) { 'tag_push_events' }
let(:trigger_key) { :tag_push_hooks }
it 'returns error message if not enough data' do
allow(project).to receive(:empty_repo?).and_return(true)
expect(hook).not_to receive(:execute)
expect(service.execute).to include({ status: :error, message: 'Ensure the project has at least one commit.' })
end
it 'executes hook' do
allow(project).to receive(:empty_repo?).and_return(false)
allow(Gitlab::DataBuilder::Push).to receive(:build_sample).and_return(sample_data)
expect(hook).to receive(:execute).with(sample_data, trigger_key).and_return(success_result)
......
......@@ -29,7 +29,6 @@ describe TestHooks::SystemService do
let(:trigger_key) { :push_hooks }
it 'executes hook' do
allow(project).to receive(:empty_repo?).and_return(false)
expect(Gitlab::DataBuilder::Push).to receive(:sample_data).and_call_original
expect(hook).to receive(:execute).with(Gitlab::DataBuilder::Push::SAMPLE_DATA, trigger_key).and_return(success_result)
......@@ -55,7 +54,6 @@ describe TestHooks::SystemService do
let(:trigger_key) { :repository_update_hooks }
it 'executes hook' do
allow(project).to receive(:empty_repo?).and_return(false)
expect(Gitlab::DataBuilder::Repository).to receive(:sample_data).and_call_original
expect(hook).to receive(:execute).with(Gitlab::DataBuilder::Repository::SAMPLE_DATA, trigger_key).and_return(success_result)
......
# frozen_string_literal: true
RSpec.shared_examples 'tests for integration with pipeline data' do
it 'tests the integration with pipeline data' do
create(:ci_empty_pipeline, project: project)
allow(Gitlab::DataBuilder::Pipeline).to receive(:build).and_return(sample_data)
expect(integration).to receive(:test).with(sample_data).and_return(success_result)
expect(subject).to eq(success_result)
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