diff options
49 files changed, 429 insertions, 93 deletions
diff --git a/CHANGELOG b/CHANGELOG index 28834c1129a..21beb3a488b 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -67,6 +67,7 @@ v 8.11.0 (unreleased) - The overhead of instrumented method calls has been reduced - Remove `search_id` of labels dropdown filter to fix 'Missleading URI for labels in Merge Requests and Issues view'. !5368 (Scott Le) - Load project invited groups and members eagerly in `ProjectTeam#fetch_members` + - Add pipeline events hook - Bump gitlab_git to speedup DiffCollection iterations - Rewrite description of a blocked user in admin settings. (Elias Werberich) - Make branches sortable without push permission !5462 (winniehell) diff --git a/app/controllers/concerns/service_params.rb b/app/controllers/concerns/service_params.rb index 471d15af913..a69877edfd4 100644 --- a/app/controllers/concerns/service_params.rb +++ b/app/controllers/concerns/service_params.rb @@ -7,11 +7,16 @@ module ServiceParams :build_key, :server, :teamcity_url, :drone_url, :build_type, :description, :issues_url, :new_issue_url, :restrict_to_branch, :channel, :colorize_messages, :channels, - :push_events, :issues_events, :merge_requests_events, :tag_push_events, - :note_events, :build_events, :wiki_page_events, - :notify_only_broken_builds, :add_pusher, - :send_from_committer_email, :disable_diffs, :external_wiki_url, - :notify, :color, + # We're using `issues_events` and `merge_requests_events` + # in the view so we still need to explicitly state them + # here. `Service#event_names` would only give + # `issue_events` and `merge_request_events` (singular!) + # See app/helpers/services_helper.rb for how we + # make those event names plural as special case. + :issues_events, :merge_requests_events, + :notify_only_broken_builds, :notify_only_broken_pipelines, + :add_pusher, :send_from_committer_email, :disable_diffs, + :external_wiki_url, :notify, :color, :server_host, :server_port, :default_irc_uri, :enable_ssl_verification, :jira_issue_transition_id] @@ -19,9 +24,7 @@ module ServiceParams FILTER_BLANK_PARAMS = [:password] def service_params - dynamic_params = [] - dynamic_params.concat(@service.event_channel_names) - + dynamic_params = @service.event_channel_names + @service.event_names service_params = params.permit(:id, service: ALLOWED_PARAMS + dynamic_params) if service_params[:service].is_a?(Hash) diff --git a/app/controllers/projects/hooks_controller.rb b/app/controllers/projects/hooks_controller.rb index a60027ff477..b5624046387 100644 --- a/app/controllers/projects/hooks_controller.rb +++ b/app/controllers/projects/hooks_controller.rb @@ -56,6 +56,7 @@ class Projects::HooksController < Projects::ApplicationController def hook_params params.require(:hook).permit( :build_events, + :pipeline_events, :enable_ssl_verification, :issues_events, :merge_requests_events, diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb index 92dad9377c9..ca2fd4f7409 100644 --- a/app/models/ci/build.rb +++ b/app/models/ci/build.rb @@ -344,7 +344,7 @@ module Ci def execute_hooks return unless project - build_data = Gitlab::BuildDataBuilder.build(self) + build_data = Gitlab::DataBuilder::Build.build(self) project.execute_hooks(build_data.dup, :build_hooks) project.execute_services(build_data.dup, :build_hooks) project.running_or_pending_build_count(force: true) diff --git a/app/models/ci/pipeline.rb b/app/models/ci/pipeline.rb index 92fae78fe4e..40615097804 100644 --- a/app/models/ci/pipeline.rb +++ b/app/models/ci/pipeline.rb @@ -19,6 +19,8 @@ module Ci after_save :keep_around_commits + delegate :stages, to: :statuses + state_machine :status, initial: :created do event :queue do transition :created => :pending @@ -56,6 +58,10 @@ module Ci before_transition do |pipeline| pipeline.update_duration end + + after_transition do |pipeline, transition| + pipeline.execute_hooks unless transition.loopback? + end end # ref can't be HEAD or SHA, can only be branch/tag name @@ -251,6 +257,15 @@ module Ci private + def execute_hooks + project.execute_hooks(pipeline_data, :pipeline_hooks) + project.execute_services(pipeline_data, :pipeline_hooks) + end + + def pipeline_data + Gitlab::DataBuilder::Pipeline.build(self) + end + def latest_builds_status return 'failed' unless yaml_errors.blank? diff --git a/app/models/hooks/project_hook.rb b/app/models/hooks/project_hook.rb index ba42a8eeb70..836a75b0608 100644 --- a/app/models/hooks/project_hook.rb +++ b/app/models/hooks/project_hook.rb @@ -5,5 +5,6 @@ class ProjectHook < WebHook scope :note_hooks, -> { where(note_events: true) } scope :merge_request_hooks, -> { where(merge_requests_events: true) } scope :build_hooks, -> { where(build_events: true) } + scope :pipeline_hooks, -> { where(pipeline_events: true) } scope :wiki_page_hooks, -> { where(wiki_page_events: true) } end diff --git a/app/models/hooks/web_hook.rb b/app/models/hooks/web_hook.rb index 8b87b6c3d64..f365dee3141 100644 --- a/app/models/hooks/web_hook.rb +++ b/app/models/hooks/web_hook.rb @@ -8,6 +8,7 @@ class WebHook < ActiveRecord::Base default_value_for :merge_requests_events, false default_value_for :tag_push_events, false default_value_for :build_events, false + default_value_for :pipeline_events, false default_value_for :enable_ssl_verification, true scope :push_hooks, -> { where(push_events: true) } diff --git a/app/models/project_services/builds_email_service.rb b/app/models/project_services/builds_email_service.rb index 5e166471077..fa66e5864b8 100644 --- a/app/models/project_services/builds_email_service.rb +++ b/app/models/project_services/builds_email_service.rb @@ -51,8 +51,7 @@ class BuildsEmailService < Service end def test_data(project = nil, user = nil) - build = project.builds.last - Gitlab::BuildDataBuilder.build(build) + Gitlab::DataBuilder::Build.build(project.builds.last) end def fields diff --git a/app/models/service.rb b/app/models/service.rb index 40cd9b861f0..09b4717a523 100644 --- a/app/models/service.rb +++ b/app/models/service.rb @@ -36,6 +36,7 @@ class Service < ActiveRecord::Base scope :merge_request_hooks, -> { where(merge_requests_events: true, active: true) } scope :note_hooks, -> { where(note_events: true, active: true) } scope :build_hooks, -> { where(build_events: true, active: true) } + scope :pipeline_hooks, -> { where(pipeline_events: true, active: true) } scope :wiki_page_hooks, -> { where(wiki_page_events: true, active: true) } scope :external_issue_trackers, -> { issue_trackers.active.without_defaults } @@ -79,13 +80,17 @@ class Service < ActiveRecord::Base end def test_data(project, user) - Gitlab::PushDataBuilder.build_sample(project, user) + Gitlab::DataBuilder::Push.build_sample(project, user) end def event_channel_names [] end + def event_names + supported_events.map { |event| "#{event}_events" } + end + def event_field(event) nil end diff --git a/app/services/delete_branch_service.rb b/app/services/delete_branch_service.rb index 87f066edb6f..918eddaa53a 100644 --- a/app/services/delete_branch_service.rb +++ b/app/services/delete_branch_service.rb @@ -39,7 +39,12 @@ class DeleteBranchService < BaseService end def build_push_data(branch) - Gitlab::PushDataBuilder - .build(project, current_user, branch.target.sha, Gitlab::Git::BLANK_SHA, "#{Gitlab::Git::BRANCH_REF_PREFIX}#{branch.name}", []) + Gitlab::DataBuilder::Push.build( + project, + current_user, + branch.target.sha, + Gitlab::Git::BLANK_SHA, + "#{Gitlab::Git::BRANCH_REF_PREFIX}#{branch.name}", + []) end end diff --git a/app/services/delete_tag_service.rb b/app/services/delete_tag_service.rb index 32e0eed6b63..d0cb151a010 100644 --- a/app/services/delete_tag_service.rb +++ b/app/services/delete_tag_service.rb @@ -33,7 +33,12 @@ class DeleteTagService < BaseService end def build_push_data(tag) - Gitlab::PushDataBuilder - .build(project, current_user, tag.target.sha, Gitlab::Git::BLANK_SHA, "#{Gitlab::Git::TAG_REF_PREFIX}#{tag.name}", []) + Gitlab::DataBuilder::Push.build( + project, + current_user, + tag.target.sha, + Gitlab::Git::BLANK_SHA, + "#{Gitlab::Git::TAG_REF_PREFIX}#{tag.name}", + []) end end diff --git a/app/services/git_push_service.rb b/app/services/git_push_service.rb index 6f521462cf3..e3f25ff1597 100644 --- a/app/services/git_push_service.rb +++ b/app/services/git_push_service.rb @@ -138,13 +138,23 @@ class GitPushService < BaseService end def build_push_data - @push_data ||= Gitlab::PushDataBuilder. - build(@project, current_user, params[:oldrev], params[:newrev], params[:ref], push_commits) + @push_data ||= Gitlab::DataBuilder::Push.build( + @project, + current_user, + params[:oldrev], + params[:newrev], + params[:ref], + push_commits) end def build_push_data_system_hook - @push_data_system ||= Gitlab::PushDataBuilder. - build(@project, current_user, params[:oldrev], params[:newrev], params[:ref], []) + @push_data_system ||= Gitlab::DataBuilder::Push.build( + @project, + current_user, + params[:oldrev], + params[:newrev], + params[:ref], + []) end def push_to_existing_branch? diff --git a/app/services/git_tag_push_service.rb b/app/services/git_tag_push_service.rb index d2b52f16fa8..e6002b03b93 100644 --- a/app/services/git_tag_push_service.rb +++ b/app/services/git_tag_push_service.rb @@ -34,12 +34,24 @@ class GitTagPushService < BaseService end end - Gitlab::PushDataBuilder. - build(project, current_user, params[:oldrev], params[:newrev], params[:ref], commits, message) + Gitlab::DataBuilder::Push.build( + project, + current_user, + params[:oldrev], + params[:newrev], + params[:ref], + commits, + message) end def build_system_push_data - Gitlab::PushDataBuilder. - build(project, current_user, params[:oldrev], params[:newrev], params[:ref], [], '') + Gitlab::DataBuilder::Push.build( + project, + current_user, + params[:oldrev], + params[:newrev], + params[:ref], + [], + '') end end diff --git a/app/services/notes/post_process_service.rb b/app/services/notes/post_process_service.rb index 534c48aefff..e4cd3fc7833 100644 --- a/app/services/notes/post_process_service.rb +++ b/app/services/notes/post_process_service.rb @@ -16,7 +16,7 @@ module Notes end def hook_data - Gitlab::NoteDataBuilder.build(@note, @note.author) + Gitlab::DataBuilder::Note.build(@note, @note.author) end def execute_note_hooks diff --git a/app/services/test_hook_service.rb b/app/services/test_hook_service.rb index e85e58751e7..280c81f7d2d 100644 --- a/app/services/test_hook_service.rb +++ b/app/services/test_hook_service.rb @@ -1,6 +1,6 @@ class TestHookService def execute(hook, current_user) - data = Gitlab::PushDataBuilder.build_sample(hook.project, current_user) + data = Gitlab::DataBuilder::Push.build_sample(hook.project, current_user) hook.execute(data, 'push_hooks') end end diff --git a/app/views/projects/hooks/_project_hook.html.haml b/app/views/projects/hooks/_project_hook.html.haml index 8151187d499..3fcf1692e09 100644 --- a/app/views/projects/hooks/_project_hook.html.haml +++ b/app/views/projects/hooks/_project_hook.html.haml @@ -3,7 +3,7 @@ .col-md-8.col-lg-7 %strong.light-header= hook.url %div - - %w(push_events tag_push_events issues_events note_events merge_requests_events build_events wiki_page_events).each do |trigger| + - %w(push_events tag_push_events issues_events note_events merge_requests_events build_events pipeline_events wiki_page_events).each do |trigger| - if hook.send(trigger) %span.label.label-gray.deploy-project-label= trigger.titleize .col-md-4.col-lg-5.text-right-lg.prepend-top-5 diff --git a/app/views/shared/web_hooks/_form.html.haml b/app/views/shared/web_hooks/_form.html.haml index 470dac6d75b..a672c28de39 100644 --- a/app/views/shared/web_hooks/_form.html.haml +++ b/app/views/shared/web_hooks/_form.html.haml @@ -66,6 +66,13 @@ %p.light This url will be triggered when the build status changes %li + = f.check_box :pipeline_events, class: 'pull-left' + .prepend-left-20 + = f.label :pipeline_events, class: 'list-label' do + %strong Pipeline events + %p.light + This url will be triggered when the pipeline status changes + %li = f.check_box :wiki_page_events, class: 'pull-left' .prepend-left-20 = f.label :wiki_page_events, class: 'list-label' do diff --git a/db/migrate/20160728081025_add_pipeline_events_to_web_hooks.rb b/db/migrate/20160728081025_add_pipeline_events_to_web_hooks.rb new file mode 100644 index 00000000000..b800e6d7283 --- /dev/null +++ b/db/migrate/20160728081025_add_pipeline_events_to_web_hooks.rb @@ -0,0 +1,16 @@ +class AddPipelineEventsToWebHooks < ActiveRecord::Migration + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + + disable_ddl_transaction! + + def up + add_column_with_default(:web_hooks, :pipeline_events, :boolean, + default: false, allow_null: false) + end + + def down + remove_column(:web_hooks, :pipeline_events) + end +end diff --git a/db/migrate/20160728103734_add_pipeline_events_to_services.rb b/db/migrate/20160728103734_add_pipeline_events_to_services.rb new file mode 100644 index 00000000000..bcd24fe1566 --- /dev/null +++ b/db/migrate/20160728103734_add_pipeline_events_to_services.rb @@ -0,0 +1,16 @@ +class AddPipelineEventsToServices < ActiveRecord::Migration + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + + disable_ddl_transaction! + + def up + add_column_with_default(:services, :pipeline_events, :boolean, + default: false, allow_null: false) + end + + def down + remove_column(:services, :pipeline_events) + end +end diff --git a/db/schema.rb b/db/schema.rb index 6c85e1e9dba..5b901b17265 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -895,6 +895,7 @@ ActiveRecord::Schema.define(version: 20160810142633) do t.string "category", default: "common", null: false t.boolean "default", default: false t.boolean "wiki_page_events", default: true + t.boolean "pipeline_events", default: false, null: false end add_index "services", ["project_id"], name: "index_services_on_project_id", using: :btree @@ -1098,6 +1099,7 @@ ActiveRecord::Schema.define(version: 20160810142633) do t.boolean "build_events", default: false, null: false t.boolean "wiki_page_events", default: false, null: false t.string "token" + t.boolean "pipeline_events", default: false, null: false end add_index "web_hooks", ["project_id"], name: "index_web_hooks_on_project_id", using: :btree diff --git a/lib/api/entities.rb b/lib/api/entities.rb index ae74d14a4bb..e8a3a6a9df0 100644 --- a/lib/api/entities.rb +++ b/lib/api/entities.rb @@ -48,7 +48,8 @@ module API class ProjectHook < Hook expose :project_id, :push_events - expose :issues_events, :merge_requests_events, :tag_push_events, :note_events, :build_events + expose :issues_events, :merge_requests_events, :tag_push_events + expose :note_events, :build_events, :pipeline_events expose :enable_ssl_verification end @@ -344,7 +345,8 @@ module API class ProjectService < Grape::Entity expose :id, :title, :created_at, :updated_at, :active - expose :push_events, :issues_events, :merge_requests_events, :tag_push_events, :note_events, :build_events + expose :push_events, :issues_events, :merge_requests_events + expose :tag_push_events, :note_events, :build_events, :pipeline_events # Expose serialized properties expose :properties do |service, options| field_names = service.fields. diff --git a/lib/api/project_hooks.rb b/lib/api/project_hooks.rb index 6bb70bc8bc3..3f63cd678e8 100644 --- a/lib/api/project_hooks.rb +++ b/lib/api/project_hooks.rb @@ -45,6 +45,7 @@ module API :tag_push_events, :note_events, :build_events, + :pipeline_events, :enable_ssl_verification ] @hook = user_project.hooks.new(attrs) @@ -78,6 +79,7 @@ module API :tag_push_events, :note_events, :build_events, + :pipeline_events, :enable_ssl_verification ] diff --git a/lib/gitlab/build_data_builder.rb b/lib/gitlab/data_builder/build.rb index 9f45aefda0f..6548e6475c6 100644 --- a/lib/gitlab/build_data_builder.rb +++ b/lib/gitlab/data_builder/build.rb @@ -1,6 +1,8 @@ module Gitlab - class BuildDataBuilder - class << self + module DataBuilder + module Build + extend self + def build(build) project = build.project commit = build.pipeline diff --git a/lib/gitlab/note_data_builder.rb b/lib/gitlab/data_builder/note.rb index 8bdc89a7751..50fea1232af 100644 --- a/lib/gitlab/note_data_builder.rb +++ b/lib/gitlab/data_builder/note.rb @@ -1,6 +1,8 @@ module Gitlab - class NoteDataBuilder - class << self + module DataBuilder + module Note + extend self + # Produce a hash of post-receive data # # For all notes: diff --git a/lib/gitlab/data_builder/pipeline.rb b/lib/gitlab/data_builder/pipeline.rb new file mode 100644 index 00000000000..06a783ebc1c --- /dev/null +++ b/lib/gitlab/data_builder/pipeline.rb @@ -0,0 +1,62 @@ +module Gitlab + module DataBuilder + module Pipeline + extend self + + def build(pipeline) + { + object_kind: 'pipeline', + object_attributes: hook_attrs(pipeline), + user: pipeline.user.try(:hook_attrs), + project: pipeline.project.hook_attrs(backward: false), + commit: pipeline.commit.try(:hook_attrs), + builds: pipeline.builds.map(&method(:build_hook_attrs)) + } + end + + def hook_attrs(pipeline) + { + id: pipeline.id, + ref: pipeline.ref, + tag: pipeline.tag, + sha: pipeline.sha, + before_sha: pipeline.before_sha, + status: pipeline.status, + stages: pipeline.stages, + created_at: pipeline.created_at, + finished_at: pipeline.finished_at, + duration: pipeline.duration + } + end + + def build_hook_attrs(build) + { + id: build.id, + stage: build.stage, + name: build.name, + status: build.status, + created_at: build.created_at, + started_at: build.started_at, + finished_at: build.finished_at, + when: build.when, + manual: build.manual?, + user: build.user.try(:hook_attrs), + runner: build.runner && runner_hook_attrs(build.runner), + artifacts_file: { + filename: build.artifacts_file.filename, + size: build.artifacts_size + } + } + end + + def runner_hook_attrs(runner) + { + id: runner.id, + description: runner.description, + active: runner.active?, + is_shared: runner.is_shared? + } + end + end + end +end diff --git a/lib/gitlab/push_data_builder.rb b/lib/gitlab/data_builder/push.rb index c8f12577112..4f81863da35 100644 --- a/lib/gitlab/push_data_builder.rb +++ b/lib/gitlab/data_builder/push.rb @@ -1,6 +1,8 @@ module Gitlab - class PushDataBuilder - class << self + module DataBuilder + module Push + extend self + # Produce a hash of post-receive data # # data = { diff --git a/spec/factories/project_hooks.rb b/spec/factories/project_hooks.rb index 3195fb3ddcc..c709432c865 100644 --- a/spec/factories/project_hooks.rb +++ b/spec/factories/project_hooks.rb @@ -5,5 +5,17 @@ FactoryGirl.define do trait :token do token { SecureRandom.hex(10) } end + + trait :all_events_enabled do + %w[push_events + merge_requests_events + tag_push_events + issues_events + note_events + build_events + pipeline_events].each do |event| + send(event, true) + end + end end end diff --git a/spec/helpers/notes_helper_spec.rb b/spec/helpers/notes_helper_spec.rb index 153f1864ceb..853b8b2f7f7 100644 --- a/spec/helpers/notes_helper_spec.rb +++ b/spec/helpers/notes_helper_spec.rb @@ -38,6 +38,12 @@ describe NotesHelper do end describe '#preload_max_access_for_authors' do + before do + # #preload_max_access_for_authors would read cache from RequestStore, + # so we should make sure it's clean. + RequestStore.clear! + end + it 'loads multiple users' do expected_access = { owner.id => Gitlab::Access::OWNER, diff --git a/spec/lib/gitlab/build_data_builder_spec.rb b/spec/lib/gitlab/data_builder/build_spec.rb index 23ae5cfacc4..6c71e98066b 100644 --- a/spec/lib/gitlab/build_data_builder_spec.rb +++ b/spec/lib/gitlab/data_builder/build_spec.rb @@ -1,11 +1,11 @@ require 'spec_helper' -describe 'Gitlab::BuildDataBuilder' do +describe Gitlab::DataBuilder::Build do let(:build) { create(:ci_build) } describe '.build' do let(:data) do - Gitlab::BuildDataBuilder.build(build) + described_class.build(build) end it { expect(data).to be_a(Hash) } diff --git a/spec/lib/gitlab/note_data_builder_spec.rb b/spec/lib/gitlab/data_builder/note_spec.rb index 3d6bcdfd873..9a4dec91e56 100644 --- a/spec/lib/gitlab/note_data_builder_spec.rb +++ b/spec/lib/gitlab/data_builder/note_spec.rb @@ -1,9 +1,9 @@ require 'spec_helper' -describe 'Gitlab::NoteDataBuilder', lib: true do +describe Gitlab::DataBuilder::Note, lib: true do let(:project) { create(:project) } let(:user) { create(:user) } - let(:data) { Gitlab::NoteDataBuilder.build(note, user) } + let(:data) { described_class.build(note, user) } let(:fixed_time) { Time.at(1425600000) } # Avoid time precision errors before(:each) do diff --git a/spec/lib/gitlab/data_builder/pipeline_spec.rb b/spec/lib/gitlab/data_builder/pipeline_spec.rb new file mode 100644 index 00000000000..a68f5943a6a --- /dev/null +++ b/spec/lib/gitlab/data_builder/pipeline_spec.rb @@ -0,0 +1,36 @@ +require 'spec_helper' + +describe Gitlab::DataBuilder::Pipeline do + let(:user) { create(:user) } + let(:project) { create(:project) } + + let(:pipeline) do + create(:ci_pipeline, + project: project, + status: 'success', + sha: project.commit.sha, + ref: project.default_branch) + end + + let!(:build) { create(:ci_build, pipeline: pipeline) } + + describe '.build' do + let(:data) { described_class.build(pipeline) } + let(:attributes) { data[:object_attributes] } + let(:build_data) { data[:builds].first } + let(:project_data) { data[:project] } + + it { expect(attributes).to be_a(Hash) } + it { expect(attributes[:ref]).to eq(pipeline.ref) } + it { expect(attributes[:sha]).to eq(pipeline.sha) } + it { expect(attributes[:tag]).to eq(pipeline.tag) } + it { expect(attributes[:id]).to eq(pipeline.id) } + it { expect(attributes[:status]).to eq(pipeline.status) } + + it { expect(build_data).to be_a(Hash) } + it { expect(build_data[:id]).to eq(build.id) } + it { expect(build_data[:status]).to eq(build.status) } + + it { expect(project_data).to eq(project.hook_attrs(backward: false)) } + end +end diff --git a/spec/lib/gitlab/push_data_builder_spec.rb b/spec/lib/gitlab/data_builder/push_spec.rb index 6bd7393aaa7..b73434e8dd7 100644 --- a/spec/lib/gitlab/push_data_builder_spec.rb +++ b/spec/lib/gitlab/data_builder/push_spec.rb @@ -1,6 +1,6 @@ require 'spec_helper' -describe Gitlab::PushDataBuilder, lib: true do +describe Gitlab::DataBuilder::Push, lib: true do let(:project) { create(:project) } let(:user) { create(:user) } diff --git a/spec/models/build_spec.rb b/spec/models/build_spec.rb index 60a221eba50..3d66ccf3f28 100644 --- a/spec/models/build_spec.rb +++ b/spec/models/build_spec.rb @@ -42,7 +42,7 @@ describe Ci::Build, models: true do describe '#ignored?' do subject { build.ignored? } - context 'if build is not allowed to fail' do + context 'when build is not allowed to fail' do before do build.allow_failure = false end @@ -64,7 +64,7 @@ describe Ci::Build, models: true do end end - context 'if build is allowed to fail' do + context 'when build is allowed to fail' do before do build.allow_failure = true end @@ -92,7 +92,7 @@ describe Ci::Build, models: true do it { is_expected.to be_empty } - context 'if build.trace contains text' do + context 'when build.trace contains text' do let(:text) { 'example output' } before do build.trace = text @@ -102,7 +102,7 @@ describe Ci::Build, models: true do it { expect(subject.length).to be >= text.length } end - context 'if build.trace hides token' do + context 'when build.trace hides token' do let(:token) { 'my_secret_token' } before do @@ -275,7 +275,8 @@ describe Ci::Build, models: true do context 'when yaml_variables are undefined' do before do - build.yaml_variables = nil + build.update(yaml_variables: nil) + build.reload # reload pipeline so that it resets config_processor end context 'use from gitlab-ci.yml' do @@ -283,13 +284,13 @@ describe Ci::Build, models: true do stub_ci_pipeline_yaml_file(config) end - context 'if config is not found' do + context 'when config is not found' do let(:config) { nil } it { is_expected.to eq(predefined_variables) } end - context 'if config does not have a questioned job' do + context 'when config does not have a questioned job' do let(:config) do YAML.dump({ test_other: { @@ -301,7 +302,7 @@ describe Ci::Build, models: true do it { is_expected.to eq(predefined_variables) } end - context 'if config has variables' do + context 'when config has variables' do let(:config) do YAML.dump({ test: { @@ -393,7 +394,7 @@ describe Ci::Build, models: true do it { is_expected.to be_falsey } end - context 'if there are runner' do + context 'when there are runners' do let(:runner) { create(:ci_runner) } before do @@ -423,8 +424,8 @@ describe Ci::Build, models: true do describe '#stuck?' do subject { build.stuck? } - %w(pending).each do |state| - context "if commit_status.status is #{state}" do + %w[pending].each do |state| + context "when commit_status.status is #{state}" do before do build.status = state end @@ -444,8 +445,8 @@ describe Ci::Build, models: true do end end - %w(success failed canceled running).each do |state| - context "if commit_status.status is #{state}" do + %w[success failed canceled running].each do |state| + context "when commit_status.status is #{state}" do before do build.status = state end @@ -767,7 +768,7 @@ describe Ci::Build, models: true do describe '#when' do subject { build.when } - context 'if is undefined' do + context 'when `when` is undefined' do before do build.when = nil end @@ -777,13 +778,13 @@ describe Ci::Build, models: true do stub_ci_pipeline_yaml_file(config) end - context 'if config is not found' do + context 'when config is not found' do let(:config) { nil } it { is_expected.to eq('on_success') } end - context 'if config does not have a questioned job' do + context 'when config does not have a questioned job' do let(:config) do YAML.dump({ test_other: { @@ -795,7 +796,7 @@ describe Ci::Build, models: true do it { is_expected.to eq('on_success') } end - context 'if config has when' do + context 'when config has `when`' do let(:config) do YAML.dump({ test: { @@ -881,7 +882,7 @@ describe Ci::Build, models: true do subject { build.play } - it 'enques a build' do + it 'enqueues a build' do is_expected.to be_pending is_expected.to eq(build) end @@ -899,9 +900,10 @@ describe Ci::Build, models: true do describe '#when' do subject { build.when } - context 'if is undefined' do + context 'when `when` is undefined' do before do - build.when = nil + build.update(when: nil) + build.reload # reload pipeline so that it resets config_processor end context 'use from gitlab-ci.yml' do @@ -909,13 +911,13 @@ describe Ci::Build, models: true do stub_ci_pipeline_yaml_file(config) end - context 'if config is not found' do + context 'when config is not found' do let(:config) { nil } it { is_expected.to eq('on_success') } end - context 'if config does not have a questioned job' do + context 'when config does not have a questioned job' do let(:config) do YAML.dump({ test_other: { @@ -927,7 +929,7 @@ describe Ci::Build, models: true do it { is_expected.to eq('on_success') } end - context 'if config has when' do + context 'when config has when' do let(:config) do YAML.dump({ test: { diff --git a/spec/models/ci/pipeline_spec.rb b/spec/models/ci/pipeline_spec.rb index 28d07f67b26..9c5d56fe5bb 100644 --- a/spec/models/ci/pipeline_spec.rb +++ b/spec/models/ci/pipeline_spec.rb @@ -18,6 +18,8 @@ describe Ci::Pipeline, models: true do it { is_expected.to respond_to :git_author_email } it { is_expected.to respond_to :short_sha } + it { is_expected.to delegate_method(:stages).to(:statuses) } + describe '#valid_commit_sha' do context 'commit.sha can not start with 00000000' do before do @@ -295,4 +297,87 @@ describe Ci::Pipeline, models: true do it { is_expected.to eq('canceled') } end end + + describe '#execute_hooks' do + let!(:build_a) { create_build('a') } + let!(:build_b) { create_build('b') } + + let!(:hook) do + create(:project_hook, project: project, pipeline_events: enabled) + end + + before do + ProjectWebHookWorker.drain + end + + context 'with pipeline hooks enabled' do + let(:enabled) { true } + + before do + WebMock.stub_request(:post, hook.url) + end + + context 'with multiple builds' do + context 'when build is queued' do + before do + build_a.queue + build_b.queue + end + + it 'receive a pending event once' do + expect(WebMock).to have_requested_pipeline_hook('pending').once + end + end + + context 'when build is run' do + before do + build_a.queue + build_a.run + build_b.queue + build_b.run + end + + it 'receive a running event once' do + expect(WebMock).to have_requested_pipeline_hook('running').once + end + end + + context 'when all builds succeed' do + before do + build_a.success + build_b.success + end + + it 'receive a success event once' do + expect(WebMock).to have_requested_pipeline_hook('success').once + end + end + + def have_requested_pipeline_hook(status) + have_requested(:post, hook.url).with do |req| + json_body = JSON.parse(req.body) + json_body['object_attributes']['status'] == status && + json_body['builds'].length == 2 + end + end + end + end + + context 'with pipeline hooks disabled' do + let(:enabled) { false } + + before do + build_a.queue + build_b.queue + end + + it 'did not execute pipeline_hook after touched' do + expect(WebMock).not_to have_requested(:post, hook.url) + end + end + + def create_build(name) + create(:ci_build, :created, pipeline: pipeline, name: name) + end + end end diff --git a/spec/models/project_services/assembla_service_spec.rb b/spec/models/project_services/assembla_service_spec.rb index 00c4e0fb64c..d672d80156c 100644 --- a/spec/models/project_services/assembla_service_spec.rb +++ b/spec/models/project_services/assembla_service_spec.rb @@ -39,7 +39,7 @@ describe AssemblaService, models: true do token: 'verySecret', subdomain: 'project_name' ) - @sample_data = Gitlab::PushDataBuilder.build_sample(project, user) + @sample_data = Gitlab::DataBuilder::Push.build_sample(project, user) @api_url = 'https://atlas.assembla.com/spaces/project_name/github_tool?secret_key=verySecret' WebMock.stub_request(:post, @api_url) end diff --git a/spec/models/project_services/builds_email_service_spec.rb b/spec/models/project_services/builds_email_service_spec.rb index ca2cd8aa551..0194f9e2563 100644 --- a/spec/models/project_services/builds_email_service_spec.rb +++ b/spec/models/project_services/builds_email_service_spec.rb @@ -1,7 +1,9 @@ require 'spec_helper' describe BuildsEmailService do - let(:data) { Gitlab::BuildDataBuilder.build(create(:ci_build)) } + let(:data) do + Gitlab::DataBuilder::Build.build(create(:ci_build)) + end describe 'Validations' do context 'when service is active' do @@ -39,7 +41,7 @@ describe BuildsEmailService do describe '#test' do it 'sends email' do - data = Gitlab::BuildDataBuilder.build(create(:ci_build)) + data = Gitlab::DataBuilder::Build.build(create(:ci_build)) subject.recipients = 'test@gitlab.com' expect(BuildEmailWorker).to receive(:perform_async) @@ -49,7 +51,7 @@ describe BuildsEmailService do context 'notify only failed builds is true' do it 'sends email' do - data = Gitlab::BuildDataBuilder.build(create(:ci_build)) + data = Gitlab::DataBuilder::Build.build(create(:ci_build)) data[:build_status] = "success" subject.recipients = 'test@gitlab.com' diff --git a/spec/models/project_services/campfire_service_spec.rb b/spec/models/project_services/campfire_service_spec.rb index 1adf93258f3..c76ae21421b 100644 --- a/spec/models/project_services/campfire_service_spec.rb +++ b/spec/models/project_services/campfire_service_spec.rb @@ -54,7 +54,7 @@ describe CampfireService, models: true do subdomain: 'project-name', room: 'test-room' ) - @sample_data = Gitlab::PushDataBuilder.build_sample(project, user) + @sample_data = Gitlab::DataBuilder::Push.build_sample(project, user) @rooms_url = 'https://verySecret:X@project-name.campfirenow.com/rooms.json' @headers = { 'Content-Type' => 'application/json; charset=utf-8' } end diff --git a/spec/models/project_services/drone_ci_service_spec.rb b/spec/models/project_services/drone_ci_service_spec.rb index 3a8e67438fc..8ef892259f2 100644 --- a/spec/models/project_services/drone_ci_service_spec.rb +++ b/spec/models/project_services/drone_ci_service_spec.rb @@ -84,7 +84,9 @@ describe DroneCiService, models: true do include_context :drone_ci_service let(:user) { create(:user, username: 'username') } - let(:push_sample_data) { Gitlab::PushDataBuilder.build_sample(project, user) } + let(:push_sample_data) do + Gitlab::DataBuilder::Push.build_sample(project, user) + end it do service_hook = double diff --git a/spec/models/project_services/flowdock_service_spec.rb b/spec/models/project_services/flowdock_service_spec.rb index 6518098ceea..d2557019756 100644 --- a/spec/models/project_services/flowdock_service_spec.rb +++ b/spec/models/project_services/flowdock_service_spec.rb @@ -52,7 +52,7 @@ describe FlowdockService, models: true do service_hook: true, token: 'verySecret' ) - @sample_data = Gitlab::PushDataBuilder.build_sample(project, user) + @sample_data = Gitlab::DataBuilder::Push.build_sample(project, user) @api_url = 'https://api.flowdock.com/v1/messages' WebMock.stub_request(:post, @api_url) end diff --git a/spec/models/project_services/gemnasium_service_spec.rb b/spec/models/project_services/gemnasium_service_spec.rb index 2c5583bdaa2..3d0b6c9816b 100644 --- a/spec/models/project_services/gemnasium_service_spec.rb +++ b/spec/models/project_services/gemnasium_service_spec.rb @@ -55,7 +55,7 @@ describe GemnasiumService, models: true do token: 'verySecret', api_key: 'GemnasiumUserApiKey' ) - @sample_data = Gitlab::PushDataBuilder.build_sample(project, user) + @sample_data = Gitlab::DataBuilder::Push.build_sample(project, user) end it "calls Gemnasium service" do expect(Gemnasium::GitlabService).to receive(:execute).with(an_instance_of(Hash)).once diff --git a/spec/models/project_services/hipchat_service_spec.rb b/spec/models/project_services/hipchat_service_spec.rb index 1b383219eb9..34eafbe555d 100644 --- a/spec/models/project_services/hipchat_service_spec.rb +++ b/spec/models/project_services/hipchat_service_spec.rb @@ -48,7 +48,9 @@ describe HipchatService, models: true do let(:project_name) { project.name_with_namespace.gsub(/\s/, '') } let(:token) { 'verySecret' } let(:server_url) { 'https://hipchat.example.com'} - let(:push_sample_data) { Gitlab::PushDataBuilder.build_sample(project, user) } + let(:push_sample_data) do + Gitlab::DataBuilder::Push.build_sample(project, user) + end before(:each) do allow(hipchat).to receive_messages( @@ -108,7 +110,15 @@ describe HipchatService, models: true do end context 'tag_push events' do - let(:push_sample_data) { Gitlab::PushDataBuilder.build(project, user, Gitlab::Git::BLANK_SHA, '1' * 40, 'refs/tags/test', []) } + let(:push_sample_data) do + Gitlab::DataBuilder::Push.build( + project, + user, + Gitlab::Git::BLANK_SHA, + '1' * 40, + 'refs/tags/test', + []) + end it "calls Hipchat API for tag push events" do hipchat.execute(push_sample_data) @@ -185,7 +195,7 @@ describe HipchatService, models: true do end it "calls Hipchat API for commit comment events" do - data = Gitlab::NoteDataBuilder.build(commit_note, user) + data = Gitlab::DataBuilder::Note.build(commit_note, user) hipchat.execute(data) expect(WebMock).to have_requested(:post, api_url).once @@ -217,7 +227,7 @@ describe HipchatService, models: true do end it "calls Hipchat API for merge request comment events" do - data = Gitlab::NoteDataBuilder.build(merge_request_note, user) + data = Gitlab::DataBuilder::Note.build(merge_request_note, user) hipchat.execute(data) expect(WebMock).to have_requested(:post, api_url).once @@ -244,7 +254,7 @@ describe HipchatService, models: true do end it "calls Hipchat API for issue comment events" do - data = Gitlab::NoteDataBuilder.build(issue_note, user) + data = Gitlab::DataBuilder::Note.build(issue_note, user) hipchat.execute(data) message = hipchat.send(:create_message, data) @@ -270,7 +280,7 @@ describe HipchatService, models: true do end it "calls Hipchat API for snippet comment events" do - data = Gitlab::NoteDataBuilder.build(snippet_note, user) + data = Gitlab::DataBuilder::Note.build(snippet_note, user) hipchat.execute(data) expect(WebMock).to have_requested(:post, api_url).once @@ -293,7 +303,7 @@ describe HipchatService, models: true do context 'build events' do let(:pipeline) { create(:ci_empty_pipeline) } let(:build) { create(:ci_build, pipeline: pipeline) } - let(:data) { Gitlab::BuildDataBuilder.build(build) } + let(:data) { Gitlab::DataBuilder::Build.build(build) } context 'for failed' do before { build.drop } diff --git a/spec/models/project_services/irker_service_spec.rb b/spec/models/project_services/irker_service_spec.rb index b528baaf15c..3bed0e6093f 100644 --- a/spec/models/project_services/irker_service_spec.rb +++ b/spec/models/project_services/irker_service_spec.rb @@ -46,7 +46,9 @@ describe IrkerService, models: true do let(:irker) { IrkerService.new } let(:user) { create(:user) } let(:project) { create(:project) } - let(:sample_data) { Gitlab::PushDataBuilder.build_sample(project, user) } + let(:sample_data) do + Gitlab::DataBuilder::Push.build_sample(project, user) + end let(:recipients) { '#commits irc://test.net/#test ftp://bad' } let(:colorize_messages) { '1' } diff --git a/spec/models/project_services/jira_service_spec.rb b/spec/models/project_services/jira_service_spec.rb index 342403f6354..9037ca5cc20 100644 --- a/spec/models/project_services/jira_service_spec.rb +++ b/spec/models/project_services/jira_service_spec.rb @@ -66,7 +66,7 @@ describe JiraService, models: true do password: 'gitlab_jira_password' ) @jira_service.save # will build API URL, as api_url was not specified above - @sample_data = Gitlab::PushDataBuilder.build_sample(project, user) + @sample_data = Gitlab::DataBuilder::Push.build_sample(project, user) # https://github.com/bblimke/webmock#request-with-basic-authentication @api_url = 'http://gitlab_jira_username:gitlab_jira_password@jira.example.com/rest/api/2/issue/JIRA-123/transitions' @comment_url = 'http://gitlab_jira_username:gitlab_jira_password@jira.example.com/rest/api/2/issue/JIRA-123/comment' diff --git a/spec/models/project_services/pushover_service_spec.rb b/spec/models/project_services/pushover_service_spec.rb index 19c0270a493..5959c81577d 100644 --- a/spec/models/project_services/pushover_service_spec.rb +++ b/spec/models/project_services/pushover_service_spec.rb @@ -48,7 +48,9 @@ describe PushoverService, models: true do let(:pushover) { PushoverService.new } let(:user) { create(:user) } let(:project) { create(:project) } - let(:sample_data) { Gitlab::PushDataBuilder.build_sample(project, user) } + let(:sample_data) do + Gitlab::DataBuilder::Push.build_sample(project, user) + end let(:api_key) { 'verySecret' } let(:user_key) { 'verySecret' } diff --git a/spec/models/project_services/slack_service_spec.rb b/spec/models/project_services/slack_service_spec.rb index 45a5f4ef12a..28af68d13b4 100644 --- a/spec/models/project_services/slack_service_spec.rb +++ b/spec/models/project_services/slack_service_spec.rb @@ -45,7 +45,9 @@ describe SlackService, models: true do let(:slack) { SlackService.new } let(:user) { create(:user) } let(:project) { create(:project) } - let(:push_sample_data) { Gitlab::PushDataBuilder.build_sample(project, user) } + let(:push_sample_data) do + Gitlab::DataBuilder::Push.build_sample(project, user) + end let(:webhook_url) { 'https://hooks.slack.com/services/SVRWFV0VVAR97N/B02R25XN3/ZBqu7xMupaEEICInN685' } let(:username) { 'slack_username' } let(:channel) { 'slack_channel' } @@ -195,7 +197,7 @@ describe SlackService, models: true do it "uses the right channel" do slack.update_attributes(note_channel: "random") - note_data = Gitlab::NoteDataBuilder.build(issue_note, user) + note_data = Gitlab::DataBuilder::Note.build(issue_note, user) expect(Slack::Notifier).to receive(:new). with(webhook_url, channel: "random"). @@ -235,7 +237,7 @@ describe SlackService, models: true do end it "calls Slack API for commit comment events" do - data = Gitlab::NoteDataBuilder.build(commit_note, user) + data = Gitlab::DataBuilder::Note.build(commit_note, user) slack.execute(data) expect(WebMock).to have_requested(:post, webhook_url).once @@ -249,7 +251,7 @@ describe SlackService, models: true do end it "calls Slack API for merge request comment events" do - data = Gitlab::NoteDataBuilder.build(merge_request_note, user) + data = Gitlab::DataBuilder::Note.build(merge_request_note, user) slack.execute(data) expect(WebMock).to have_requested(:post, webhook_url).once @@ -262,7 +264,7 @@ describe SlackService, models: true do end it "calls Slack API for issue comment events" do - data = Gitlab::NoteDataBuilder.build(issue_note, user) + data = Gitlab::DataBuilder::Note.build(issue_note, user) slack.execute(data) expect(WebMock).to have_requested(:post, webhook_url).once @@ -276,7 +278,7 @@ describe SlackService, models: true do end it "calls Slack API for snippet comment events" do - data = Gitlab::NoteDataBuilder.build(snippet_note, user) + data = Gitlab::DataBuilder::Note.build(snippet_note, user) slack.execute(data) expect(WebMock).to have_requested(:post, webhook_url).once diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index f67acbbef37..54505f6b822 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -895,7 +895,9 @@ describe User, models: true do subject { create(:user) } let!(:project1) { create(:project) } let!(:project2) { create(:project, forked_from_project: project1) } - let!(:push_data) { Gitlab::PushDataBuilder.build_sample(project2, subject) } + let!(:push_data) do + Gitlab::DataBuilder::Push.build_sample(project2, subject) + end let!(:push_event) { create(:event, action: Event::PUSHED, project: project2, target: project1, author: subject, data: push_data) } before do diff --git a/spec/requests/api/project_hooks_spec.rb b/spec/requests/api/project_hooks_spec.rb index 34fac297923..914e88c9487 100644 --- a/spec/requests/api/project_hooks_spec.rb +++ b/spec/requests/api/project_hooks_spec.rb @@ -7,9 +7,9 @@ describe API::API, 'ProjectHooks', api: true do let!(:project) { create(:project, creator_id: user.id, namespace: user.namespace) } let!(:hook) do create(:project_hook, - project: project, url: "http://example.com", - push_events: true, merge_requests_events: true, tag_push_events: true, - issues_events: true, note_events: true, build_events: true, + :all_events_enabled, + project: project, + url: 'http://example.com', enable_ssl_verification: true) end @@ -33,6 +33,7 @@ describe API::API, 'ProjectHooks', api: true do expect(json_response.first['tag_push_events']).to eq(true) expect(json_response.first['note_events']).to eq(true) expect(json_response.first['build_events']).to eq(true) + expect(json_response.first['pipeline_events']).to eq(true) expect(json_response.first['enable_ssl_verification']).to eq(true) end end @@ -91,6 +92,7 @@ describe API::API, 'ProjectHooks', api: true do expect(json_response['tag_push_events']).to eq(false) expect(json_response['note_events']).to eq(false) expect(json_response['build_events']).to eq(false) + expect(json_response['pipeline_events']).to eq(false) expect(json_response['enable_ssl_verification']).to eq(true) end diff --git a/spec/workers/build_email_worker_spec.rb b/spec/workers/build_email_worker_spec.rb index 98deae0a588..788b92c1b84 100644 --- a/spec/workers/build_email_worker_spec.rb +++ b/spec/workers/build_email_worker_spec.rb @@ -5,7 +5,7 @@ describe BuildEmailWorker do let(:build) { create(:ci_build) } let(:user) { create(:user) } - let(:data) { Gitlab::BuildDataBuilder.build(build) } + let(:data) { Gitlab::DataBuilder::Build.build(build) } subject { BuildEmailWorker.new } diff --git a/spec/workers/emails_on_push_worker_spec.rb b/spec/workers/emails_on_push_worker_spec.rb index 796751efe8d..eecc32875a5 100644 --- a/spec/workers/emails_on_push_worker_spec.rb +++ b/spec/workers/emails_on_push_worker_spec.rb @@ -5,7 +5,7 @@ describe EmailsOnPushWorker do let(:project) { create(:project) } let(:user) { create(:user) } - let(:data) { Gitlab::PushDataBuilder.build_sample(project, user) } + let(:data) { Gitlab::DataBuilder::Push.build_sample(project, user) } let(:recipients) { user.email } let(:perform) { subject.perform(project.id, recipients, data.stringify_keys) } |