diff options
42 files changed, 394 insertions, 148 deletions
diff --git a/app/models/concerns/protected_ref.rb b/app/models/concerns/protected_ref.rb index ef95d6b0f98..454374121f3 100644 --- a/app/models/concerns/protected_ref.rb +++ b/app/models/concerns/protected_ref.rb @@ -23,7 +23,7 @@ module ProtectedRef # If we don't `protected_branch` or `protected_tag` would be empty and # `project` cannot be delegated to it, which in turn would cause validations # to fail. - has_many :"#{type}_access_levels", dependent: :destroy, inverse_of: self.model_name.singular # rubocop:disable Cop/ActiveRecordDependent + has_many :"#{type}_access_levels", inverse_of: self.model_name.singular # rubocop:disable Cop/ActiveRecordDependent validates :"#{type}_access_levels", length: { is: 1, message: "are restricted to a single instance per #{self.model_name.human}." } diff --git a/app/models/notification_recipient.rb b/app/models/notification_recipient.rb index dc862565a71..183e098d819 100644 --- a/app/models/notification_recipient.rb +++ b/app/models/notification_recipient.rb @@ -27,46 +27,45 @@ class NotificationRecipient @notification_setting ||= find_notification_setting end - def raw_notification_level - notification_setting&.level&.to_sym - end - def notification_level - # custom is treated the same as watch if it's enabled - otherwise it's - # set to :custom, meaning to send exactly when our type is :participating - # or :mention. - @notification_level ||= - case raw_notification_level - when :custom - if @custom_action && notification_setting&.event_enabled?(@custom_action) - :watch - else - :custom - end - else - raw_notification_level - end + @notification_level ||= notification_setting&.level&.to_sym end def notifiable? return false unless has_access? return false if own_activity? - return true if @type == :subscription - - return false if notification_level.nil? || notification_level == :disabled - - return %i[participating mention].include?(@type) if notification_level == :custom + # even users with :disabled notifications receive manual subscriptions + return !unsubscribed? if @type == :subscription - return false if %i[watch participating].include?(notification_level) && excluded_watcher_action? - - return false unless NotificationSetting.levels[notification_level] <= NotificationSetting.levels[@type] + return false unless suitable_notification_level? + # check this last because it's expensive + # nobody should receive notifications if they've specifically unsubscribed return false if unsubscribed? true end + def suitable_notification_level? + case notification_level + when :disabled, nil + false + when :custom + custom_enabled? || %i[participating mention].include?(@type) + when :watch, :participating + !excluded_watcher_action? + when :mention + @type == :mention + else + false + end + end + + def custom_enabled? + @custom_action && notification_setting&.event_enabled?(@custom_action) + end + def unsubscribed? return false unless @target return false unless @target.respond_to?(:subscriptions) @@ -98,7 +97,7 @@ class NotificationRecipient def excluded_watcher_action? return false unless @custom_action - return false if raw_notification_level == :custom + return false if notification_level == :custom NotificationSetting::EXCLUDED_WATCHER_EVENTS.include?(@custom_action) end diff --git a/app/services/concerns/users/new_user_notifier.rb b/app/services/concerns/users/new_user_notifier.rb new file mode 100644 index 00000000000..231693ce7a9 --- /dev/null +++ b/app/services/concerns/users/new_user_notifier.rb @@ -0,0 +1,9 @@ +module Users + module NewUserNotifier + def notify_new_user(user, reset_token) + log_info("User \"#{user.name}\" (#{user.email}) was created") + notification_service.new_user(user, reset_token) if reset_token + system_hook_service.execute_hooks_for(user, :create) + end + end +end diff --git a/app/services/notification_recipient_service.rb b/app/services/notification_recipient_service.rb index 21c9c314a2a..c9f07c140f7 100644 --- a/app/services/notification_recipient_service.rb +++ b/app/services/notification_recipient_service.rb @@ -95,7 +95,7 @@ module NotificationRecipientService def add_participants(user) return unless target.respond_to?(:participants) - self << [target.participants(user), :watch] + self << [target.participants(user), :participating] end # Get project/group users with CUSTOM notification level diff --git a/app/services/test_hooks/system_service.rb b/app/services/test_hooks/system_service.rb index 76c3c19bd74..67552edefc9 100644 --- a/app/services/test_hooks/system_service.rb +++ b/app/services/test_hooks/system_service.rb @@ -2,47 +2,16 @@ module TestHooks class SystemService < TestHooks::BaseService private - def project - @project ||= begin - project = Project.first - - throw(:validation_error, 'Ensure that at least one project exists.') unless project - - project - end - end - def push_events_data - if project.empty_repo? - throw(:validation_error, "Ensure project \"#{project.human_name}\" has commits.") - end - - Gitlab::DataBuilder::Push.build_sample(project, current_user) + Gitlab::DataBuilder::Push.sample_data end def tag_push_events_data - if project.repository.tags.empty? - throw(:validation_error, "Ensure project \"#{project.human_name}\" has tags.") - end - - Gitlab::DataBuilder::Push.build_sample(project, current_user) + Gitlab::DataBuilder::Push.sample_data end def repository_update_events_data - commit = project.commit - ref = "#{Gitlab::Git::BRANCH_REF_PREFIX}#{project.default_branch}" - - unless commit - throw(:validation_error, "Ensure project \"#{project.human_name}\" has commits.") - end - - change = Gitlab::DataBuilder::Repository.single_change( - commit.parent_id || Gitlab::Git::BLANK_SHA, - commit.id, - ref - ) - - Gitlab::DataBuilder::Repository.update(project, current_user, [change], [ref]) + Gitlab::DataBuilder::Repository.sample_data end end end diff --git a/app/services/users/create_service.rb b/app/services/users/create_service.rb index 74abc017cea..c8a3c461d60 100644 --- a/app/services/users/create_service.rb +++ b/app/services/users/create_service.rb @@ -1,5 +1,7 @@ module Users class CreateService < BaseService + include NewUserNotifier + def initialize(current_user, params = {}) @current_user = current_user @params = params.dup @@ -10,11 +12,7 @@ module Users @reset_token = user.generate_reset_token if user.recently_sent_password_reset? - if user.save - log_info("User \"#{user.name}\" (#{user.email}) was created") - notification_service.new_user(user, @reset_token) if @reset_token - system_hook_service.execute_hooks_for(user, :create) - end + notify_new_user(user, @reset_token) if user.save user end diff --git a/app/services/users/update_service.rb b/app/services/users/update_service.rb index dfbd6016c3f..2f9855273dc 100644 --- a/app/services/users/update_service.rb +++ b/app/services/users/update_service.rb @@ -1,5 +1,7 @@ module Users class UpdateService < BaseService + include NewUserNotifier + def initialize(user, params = {}) @user = user @params = params.dup @@ -10,7 +12,11 @@ module Users assign_attributes(&block) + user_exists = @user.persisted? + if @user.save(validate: validate) + notify_new_user(@user, nil) unless user_exists + success else error(@user.errors.full_messages.uniq.join('. ')) diff --git a/app/workers/build_coverage_worker.rb b/app/workers/build_coverage_worker.rb index f7ae996bb17..cd4af85d047 100644 --- a/app/workers/build_coverage_worker.rb +++ b/app/workers/build_coverage_worker.rb @@ -1,6 +1,6 @@ class BuildCoverageWorker include Sidekiq::Worker - include BuildQueue + include PipelineQueue def perform(build_id) Ci::Build.find_by(id: build_id)&.update_coverage diff --git a/app/workers/build_finished_worker.rb b/app/workers/build_finished_worker.rb index 466410bf08c..e2a1b3dcc41 100644 --- a/app/workers/build_finished_worker.rb +++ b/app/workers/build_finished_worker.rb @@ -1,6 +1,8 @@ class BuildFinishedWorker include Sidekiq::Worker - include BuildQueue + include PipelineQueue + + enqueue_in group: :processing def perform(build_id) Ci::Build.find_by(id: build_id).try do |build| diff --git a/app/workers/build_hooks_worker.rb b/app/workers/build_hooks_worker.rb index 9965af935d4..dedaf2835e6 100644 --- a/app/workers/build_hooks_worker.rb +++ b/app/workers/build_hooks_worker.rb @@ -1,6 +1,8 @@ class BuildHooksWorker include Sidekiq::Worker - include BuildQueue + include PipelineQueue + + enqueue_in group: :hooks def perform(build_id) Ci::Build.find_by(id: build_id) diff --git a/app/workers/build_queue_worker.rb b/app/workers/build_queue_worker.rb index fa9e097e40a..e5ceb9ef715 100644 --- a/app/workers/build_queue_worker.rb +++ b/app/workers/build_queue_worker.rb @@ -1,6 +1,8 @@ class BuildQueueWorker include Sidekiq::Worker - include BuildQueue + include PipelineQueue + + enqueue_in group: :processing def perform(build_id) Ci::Build.find_by(id: build_id).try do |build| diff --git a/app/workers/build_success_worker.rb b/app/workers/build_success_worker.rb index bf009dfab0f..20ec24bd18a 100644 --- a/app/workers/build_success_worker.rb +++ b/app/workers/build_success_worker.rb @@ -1,6 +1,8 @@ class BuildSuccessWorker include Sidekiq::Worker - include BuildQueue + include PipelineQueue + + enqueue_in group: :processing def perform(build_id) Ci::Build.find_by(id: build_id).try do |build| diff --git a/app/workers/concerns/build_queue.rb b/app/workers/concerns/build_queue.rb deleted file mode 100644 index cf0ead40a8b..00000000000 --- a/app/workers/concerns/build_queue.rb +++ /dev/null @@ -1,8 +0,0 @@ -# Concern for setting Sidekiq settings for the various CI build workers. -module BuildQueue - extend ActiveSupport::Concern - - included do - sidekiq_options queue: :build - end -end diff --git a/app/workers/concerns/pipeline_queue.rb b/app/workers/concerns/pipeline_queue.rb index ca3860e1d38..ddf45b91345 100644 --- a/app/workers/concerns/pipeline_queue.rb +++ b/app/workers/concerns/pipeline_queue.rb @@ -1,8 +1,18 @@ +## # Concern for setting Sidekiq settings for the various CI pipeline workers. +# module PipelineQueue extend ActiveSupport::Concern included do - sidekiq_options queue: :pipeline + sidekiq_options queue: 'pipeline_default' + end + + class_methods do + def enqueue_in(group:) + raise ArgumentError, 'Unspecified queue group!' if group.empty? + + sidekiq_options queue: "pipeline_#{group}" + end end end diff --git a/app/workers/expire_job_cache_worker.rb b/app/workers/expire_job_cache_worker.rb index e383202260d..98a7500bffe 100644 --- a/app/workers/expire_job_cache_worker.rb +++ b/app/workers/expire_job_cache_worker.rb @@ -1,6 +1,8 @@ class ExpireJobCacheWorker include Sidekiq::Worker - include BuildQueue + include PipelineQueue + + enqueue_in group: :cache def perform(job_id) job = CommitStatus.joins(:pipeline, :project).find_by(id: job_id) diff --git a/app/workers/expire_pipeline_cache_worker.rb b/app/workers/expire_pipeline_cache_worker.rb index 7c02d6cf892..1a0e7f92875 100644 --- a/app/workers/expire_pipeline_cache_worker.rb +++ b/app/workers/expire_pipeline_cache_worker.rb @@ -2,6 +2,8 @@ class ExpirePipelineCacheWorker include Sidekiq::Worker include PipelineQueue + enqueue_in group: :cache + def perform(pipeline_id) pipeline = Ci::Pipeline.find_by(id: pipeline_id) return unless pipeline diff --git a/app/workers/pipeline_hooks_worker.rb b/app/workers/pipeline_hooks_worker.rb index 7e36eacebf8..30a75ec8435 100644 --- a/app/workers/pipeline_hooks_worker.rb +++ b/app/workers/pipeline_hooks_worker.rb @@ -2,6 +2,8 @@ class PipelineHooksWorker include Sidekiq::Worker include PipelineQueue + enqueue_in group: :hooks + def perform(pipeline_id) Ci::Pipeline.find_by(id: pipeline_id) .try(:execute_hooks) diff --git a/app/workers/pipeline_process_worker.rb b/app/workers/pipeline_process_worker.rb index 357e4a9a1c3..8c067d05081 100644 --- a/app/workers/pipeline_process_worker.rb +++ b/app/workers/pipeline_process_worker.rb @@ -2,6 +2,8 @@ class PipelineProcessWorker include Sidekiq::Worker include PipelineQueue + enqueue_in group: :processing + def perform(pipeline_id) Ci::Pipeline.find_by(id: pipeline_id) .try(:process!) diff --git a/app/workers/pipeline_success_worker.rb b/app/workers/pipeline_success_worker.rb index cc0eb708cf9..cb8bb2ffe75 100644 --- a/app/workers/pipeline_success_worker.rb +++ b/app/workers/pipeline_success_worker.rb @@ -2,6 +2,8 @@ class PipelineSuccessWorker include Sidekiq::Worker include PipelineQueue + enqueue_in group: :processing + def perform(pipeline_id) Ci::Pipeline.find_by(id: pipeline_id).try do |pipeline| MergeRequests::MergeWhenPipelineSucceedsService diff --git a/app/workers/pipeline_update_worker.rb b/app/workers/pipeline_update_worker.rb index 96c4152c674..5fa399dff4c 100644 --- a/app/workers/pipeline_update_worker.rb +++ b/app/workers/pipeline_update_worker.rb @@ -2,6 +2,8 @@ class PipelineUpdateWorker include Sidekiq::Worker include PipelineQueue + enqueue_in group: :processing + def perform(pipeline_id) Ci::Pipeline.find_by(id: pipeline_id) .try(:update_status) diff --git a/app/workers/stage_update_worker.rb b/app/workers/stage_update_worker.rb index eef0b11e70b..c301cea5ad6 100644 --- a/app/workers/stage_update_worker.rb +++ b/app/workers/stage_update_worker.rb @@ -2,6 +2,8 @@ class StageUpdateWorker include Sidekiq::Worker include PipelineQueue + enqueue_in group: :processing + def perform(stage_id) Ci::Stage.find_by(id: stage_id).try do |stage| stage.update_status diff --git a/changelogs/unreleased/bugfix-notify-custom-participants.yml b/changelogs/unreleased/bugfix-notify-custom-participants.yml new file mode 100644 index 00000000000..04fcb95e18a --- /dev/null +++ b/changelogs/unreleased/bugfix-notify-custom-participants.yml @@ -0,0 +1,5 @@ +--- +title: Fixed: Notifications weren't sending to participating users with a `Custom` notification setting. +merge_request: 13680 +author: jneen +type: fixed diff --git a/changelogs/unreleased/docs-update-ci-docker-using-docker-images.yml b/changelogs/unreleased/docs-update-ci-docker-using-docker-images.yml new file mode 100644 index 00000000000..d8a5073f110 --- /dev/null +++ b/changelogs/unreleased/docs-update-ci-docker-using-docker-images.yml @@ -0,0 +1,5 @@ +--- +title: Update 'Using Docker images' documentation +merge_request: 13848 +author: +type: other diff --git a/changelogs/unreleased/sh-system-hooks-ldap-users.yml b/changelogs/unreleased/sh-system-hooks-ldap-users.yml new file mode 100644 index 00000000000..87514ec00ea --- /dev/null +++ b/changelogs/unreleased/sh-system-hooks-ldap-users.yml @@ -0,0 +1,5 @@ +--- +title: Fire system hooks when a user is created via LDAP +merge_request: +author: +type: fixed diff --git a/config/sidekiq_queues.yml b/config/sidekiq_queues.yml index 83abc83c9f0..24c001362c6 100644 --- a/config/sidekiq_queues.yml +++ b/config/sidekiq_queues.yml @@ -27,6 +27,10 @@ - [new_merge_request, 2] - [build, 2] - [pipeline, 2] + - [pipeline_processing, 5] + - [pipeline_default, 3] + - [pipeline_cache, 3] + - [pipeline_hooks, 2] - [gitlab_shell, 2] - [email_receiver, 2] - [emails_on_push, 2] diff --git a/db/post_migrate/20170822101017_migrate_pipeline_sidekiq_queues.rb b/db/post_migrate/20170822101017_migrate_pipeline_sidekiq_queues.rb new file mode 100644 index 00000000000..8441cfe7968 --- /dev/null +++ b/db/post_migrate/20170822101017_migrate_pipeline_sidekiq_queues.rb @@ -0,0 +1,17 @@ +class MigratePipelineSidekiqQueues < ActiveRecord::Migration + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + + def up + sidekiq_queue_migrate 'build', to: 'pipeline_default' + sidekiq_queue_migrate 'pipeline', to: 'pipeline_default' + end + + def down + sidekiq_queue_migrate 'pipeline_default', to: 'pipeline' + sidekiq_queue_migrate 'pipeline_processing', to: 'pipeline' + sidekiq_queue_migrate 'pipeline_hooks', to: 'pipeline' + sidekiq_queue_migrate 'pipeline_cache', to: 'pipeline' + end +end diff --git a/doc/ci/docker/using_docker_images.md b/doc/ci/docker/using_docker_images.md index dc5313c5597..6e8beceb6fe 100644 --- a/doc/ci/docker/using_docker_images.md +++ b/doc/ci/docker/using_docker_images.md @@ -388,15 +388,40 @@ that runner. As an example, let's assume that you want to use the `registry.example.com/private/image:latest` image which is private and requires you to login into a private container registry. + +Let's also assume that these are the login credentials: + +| Key | Value | +|----------|----------------------| +| registry | registry.example.com | +| username | my_username | +| password | my_password | + To configure access for `registry.example.com`, follow these steps: -1. Do a `docker login` on your computer: +1. Find what the value of `DOCKER_AUTH_CONFIG` should be. There are two ways to + accomplish this: + - **First way -** Do a `docker login` on your local machine: - ```bash - docker login registry.example.com --username my_username --password my_password - ``` + ```bash + docker login registry.example.com --username my_username --password my_password + ``` + + Then copy the content of `~/.docker/config.json`. + - **Second way -** In some setups, it's possible that Docker client will use + the available system keystore to store the result of `docker login`. In + that case, it's impossible to read `~/.docker/config.json`, so you will + need to prepare the required base64-encoded version of + `${username}:${password}` manually. Open a terminal and execute the + following command: + + ```bash + echo -n "my_username:my_password" | base64 + + # Example output to copy + bXlfdXNlcm5hbWU6bXlfcGFzc3dvcmQ= + ``` -1. Copy the content of `~/.docker/config.json` 1. Create a [secret variable] `DOCKER_AUTH_CONFIG` with the content of the Docker configuration file as the value: @@ -410,7 +435,8 @@ To configure access for `registry.example.com`, follow these steps: } ``` -1. Do a `docker logout` on your computer if you don't need access to the +1. Optionally,if you followed the first way of finding the `DOCKER_AUTH_CONFIG` + value, do a `docker logout` on your computer if you don't need access to the registry from it: ```bash @@ -418,7 +444,7 @@ To configure access for `registry.example.com`, follow these steps: ``` 1. You can now use any private image from `registry.example.com` defined in - `image` and/or `services` in your [`.gitlab-ci.yml` file][yaml-priv-reg]: + `image` and/or `services` in your `.gitlab-ci.yml` file: ```yaml image: my.registry.tld:5000/namespace/image:tag diff --git a/lib/gitlab/data_builder/push.rb b/lib/gitlab/data_builder/push.rb index 5c5f507d44d..4ab5b3455a5 100644 --- a/lib/gitlab/data_builder/push.rb +++ b/lib/gitlab/data_builder/push.rb @@ -3,6 +3,35 @@ module Gitlab module Push extend self + SAMPLE_DATA = + { + object_kind: "push", + event_name: "push", + before: "95790bf891e76fee5e1747ab589903a6a1f80f22", + after: "da1560886d4f094c3e6c9ef40349f7d38b5d27d7", + ref: "refs/heads/master", + checkout_sha: "da1560886d4f094c3e6c9ef40349f7d38b5d27d7", + message: "Hello World", + user_id: 4, + user_name: "John Smith", + user_email: "john@example.com", + user_avatar: "https://s.gravatar.com/avatar/d4c74594d841139328695756648b6bd6?s=8://s.gravatar.com/avatar/d4c74594d841139328695756648b6bd6?s=80", + project_id: 15, + commits: [ + { + id: "c5feabde2d8cd023215af4d2ceeb7a64839fc428", + message: "Add simple search to projects in public area", + timestamp: "2013-05-13T18:18:08+00:00", + url: "https://test.example.com/gitlab/gitlabhq/commit/c5feabde2d8cd023215af4d2ceeb7a64839fc428", + author: { + name: "Test User", + email: "test@example.com" + } + } + ], + total_commits_count: 1 + }.freeze + # Produce a hash of post-receive data # # data = { @@ -74,6 +103,10 @@ module Gitlab build(project, user, commits.last&.id, commits.first&.id, ref, commits) end + def sample_data + SAMPLE_DATA + end + private def checkout_sha(repository, newrev, ref) diff --git a/lib/gitlab/data_builder/repository.rb b/lib/gitlab/data_builder/repository.rb index b42dc052949..c9c13ec6487 100644 --- a/lib/gitlab/data_builder/repository.rb +++ b/lib/gitlab/data_builder/repository.rb @@ -3,6 +3,23 @@ module Gitlab module Repository extend self + SAMPLE_DATA = { + event_name: 'repository_update', + user_id: 10, + user_name: 'john.doe', + user_email: 'test@example.com', + user_avatar: 'http://example.com/avatar/user.png', + project_id: 40, + changes: [ + { + before: "8205ea8d81ce0c6b90fbe8280d118cc9fdad6130", + after: "4045ea7a3df38697b3730a20fb73c8bed8a3e69e", + ref: "refs/heads/master" + } + ], + "refs": ["refs/heads/master"] + }.freeze + # Produce a hash of post-receive data def update(project, user, changes, refs) { @@ -30,6 +47,10 @@ module Gitlab ref: ref } end + + def sample_data + SAMPLE_DATA + end end end end diff --git a/lib/gitlab/database/migration_helpers.rb b/lib/gitlab/database/migration_helpers.rb index b83e633c7ed..5e2c6cc5cad 100644 --- a/lib/gitlab/database/migration_helpers.rb +++ b/lib/gitlab/database/migration_helpers.rb @@ -611,6 +611,20 @@ module Gitlab remove_foreign_key(*args) rescue ArgumentError end + + def sidekiq_queue_migrate(queue_from, to:) + while sidekiq_queue_length(queue_from) > 0 + Sidekiq.redis do |conn| + conn.rpoplpush "queue:#{queue_from}", "queue:#{to}" + end + end + end + + def sidekiq_queue_length(queue_name) + Sidekiq.redis do |conn| + conn.llen("queue:#{queue_name}") + end + end end end end diff --git a/spec/factories/ci/triggers.rb b/spec/factories/ci/triggers.rb index 40c4663c6d8..3734c7040c0 100644 --- a/spec/factories/ci/triggers.rb +++ b/spec/factories/ci/triggers.rb @@ -1,5 +1,7 @@ FactoryGirl.define do factory :ci_trigger_without_token, class: Ci::Trigger do + owner + factory :ci_trigger do sequence(:token) { |n| "token#{n}" } end diff --git a/spec/lib/gitlab/database/migration_helpers_spec.rb b/spec/lib/gitlab/database/migration_helpers_spec.rb index ec2274a70aa..c25fd459dd7 100644 --- a/spec/lib/gitlab/database/migration_helpers_spec.rb +++ b/spec/lib/gitlab/database/migration_helpers_spec.rb @@ -2,9 +2,7 @@ require 'spec_helper' describe Gitlab::Database::MigrationHelpers do let(:model) do - ActiveRecord::Migration.new.extend( - described_class - ) + ActiveRecord::Migration.new.extend(described_class) end before do @@ -845,4 +843,51 @@ describe Gitlab::Database::MigrationHelpers do end end end + + describe 'sidekiq migration helpers', :sidekiq, :redis do + let(:worker) do + Class.new do + include Sidekiq::Worker + sidekiq_options queue: 'test' + end + end + + describe '#sidekiq_queue_length' do + context 'when queue is empty' do + it 'returns zero' do + Sidekiq::Testing.disable! do + expect(model.sidekiq_queue_length('test')).to eq 0 + end + end + end + + context 'when queue contains jobs' do + it 'returns correct size of the queue' do + Sidekiq::Testing.disable! do + worker.perform_async('Something', [1]) + worker.perform_async('Something', [2]) + + expect(model.sidekiq_queue_length('test')).to eq 2 + end + end + end + end + + describe '#migrate_sidekiq_queue' do + it 'migrates jobs from one sidekiq queue to another' do + Sidekiq::Testing.disable! do + worker.perform_async('Something', [1]) + worker.perform_async('Something', [2]) + + expect(model.sidekiq_queue_length('test')).to eq 2 + expect(model.sidekiq_queue_length('new_test')).to eq 0 + + model.sidekiq_queue_migrate('test', to: 'new_test') + + expect(model.sidekiq_queue_length('test')).to eq 0 + expect(model.sidekiq_queue_length('new_test')).to eq 2 + end + end + end + end end diff --git a/spec/migrations/migrate_pipeline_sidekiq_queues_spec.rb b/spec/migrations/migrate_pipeline_sidekiq_queues_spec.rb new file mode 100644 index 00000000000..e02bcd2f4da --- /dev/null +++ b/spec/migrations/migrate_pipeline_sidekiq_queues_spec.rb @@ -0,0 +1,55 @@ +require 'spec_helper' +require Rails.root.join('db', 'post_migrate', '20170822101017_migrate_pipeline_sidekiq_queues.rb') + +describe MigratePipelineSidekiqQueues, :sidekiq, :redis do + include Gitlab::Database::MigrationHelpers + + context 'when there are jobs in the queues' do + it 'correctly migrates queue when migrating up' do + Sidekiq::Testing.disable! do + stubbed_worker(queue: :pipeline).perform_async('Something', [1]) + stubbed_worker(queue: :build).perform_async('Something', [1]) + + described_class.new.up + + expect(sidekiq_queue_length('pipeline')).to eq 0 + expect(sidekiq_queue_length('build')).to eq 0 + expect(sidekiq_queue_length('pipeline_default')).to eq 2 + end + end + + it 'correctly migrates queue when migrating down' do + Sidekiq::Testing.disable! do + stubbed_worker(queue: :pipeline_default).perform_async('Class', [1]) + stubbed_worker(queue: :pipeline_processing).perform_async('Class', [2]) + stubbed_worker(queue: :pipeline_hooks).perform_async('Class', [3]) + stubbed_worker(queue: :pipeline_cache).perform_async('Class', [4]) + + described_class.new.down + + expect(sidekiq_queue_length('pipeline')).to eq 4 + expect(sidekiq_queue_length('pipeline_default')).to eq 0 + expect(sidekiq_queue_length('pipeline_processing')).to eq 0 + expect(sidekiq_queue_length('pipeline_hooks')).to eq 0 + expect(sidekiq_queue_length('pipeline_cache')).to eq 0 + end + end + end + + context 'when there are no jobs in the queues' do + it 'does not raise error when migrating up' do + expect { described_class.new.up }.not_to raise_error + end + + it 'does not raise error when migrating down' do + expect { described_class.new.down }.not_to raise_error + end + end + + def stubbed_worker(queue:) + Class.new do + include Sidekiq::Worker + sidekiq_options queue: queue + end + end +end diff --git a/spec/requests/api/triggers_spec.rb b/spec/requests/api/triggers_spec.rb index 572e9a0fd07..1e206fd2a9e 100644 --- a/spec/requests/api/triggers_spec.rb +++ b/spec/requests/api/triggers_spec.rb @@ -8,8 +8,8 @@ describe API::Triggers do let!(:project) { create(:project, :repository, creator: user) } let!(:master) { create(:project_member, :master, user: user, project: project) } let!(:developer) { create(:project_member, :developer, user: user2, project: project) } - let!(:trigger) { create(:ci_trigger, project: project, token: trigger_token) } - let!(:trigger2) { create(:ci_trigger, project: project, token: trigger_token_2) } + let!(:trigger) { create(:ci_trigger, project: project, token: trigger_token, owner: user) } + let!(:trigger2) { create(:ci_trigger, project: project, token: trigger_token_2, owner: user2) } let!(:trigger_request) { create(:ci_trigger_request, trigger: trigger, created_at: '2015-01-01 12:13:14') } describe 'POST /projects/:project_id/trigger/pipeline' do @@ -22,7 +22,6 @@ describe API::Triggers do before do stub_ci_pipeline_to_return_yaml_file - trigger.update(owner: user) end context 'Handles errors' do @@ -254,8 +253,6 @@ describe API::Triggers do describe 'POST /projects/:id/triggers/:trigger_id/take_ownership' do context 'authenticated user with valid permissions' do it 'updates owner' do - expect(trigger.owner).to be_nil - post api("/projects/#{project.id}/triggers/#{trigger.id}/take_ownership", user) expect(response).to have_http_status(200) diff --git a/spec/requests/api/v3/triggers_spec.rb b/spec/requests/api/v3/triggers_spec.rb index 075de2c0cba..d4648136841 100644 --- a/spec/requests/api/v3/triggers_spec.rb +++ b/spec/requests/api/v3/triggers_spec.rb @@ -7,7 +7,10 @@ describe API::V3::Triggers do let!(:project) { create(:project, :repository, creator: user) } let!(:master) { create(:project_member, :master, user: user, project: project) } let!(:developer) { create(:project_member, :developer, user: user2, project: project) } - let!(:trigger) { create(:ci_trigger, project: project, token: trigger_token) } + + let!(:trigger) do + create(:ci_trigger, project: project, token: trigger_token, owner: user) + end describe 'POST /projects/:project_id/trigger' do let!(:project2) { create(:project) } diff --git a/spec/services/ci/create_pipeline_service_spec.rb b/spec/services/ci/create_pipeline_service_spec.rb index 8465a6f99bd..fdd0cea4f3b 100644 --- a/spec/services/ci/create_pipeline_service_spec.rb +++ b/spec/services/ci/create_pipeline_service_spec.rb @@ -470,7 +470,8 @@ describe Ci::CreatePipelineService do context 'when ref is not protected' do context 'when trigger belongs to no one' do let(:user) {} - let(:trigger_request) { create(:ci_trigger_request) } + let(:trigger) { create(:ci_trigger, owner: nil) } + let(:trigger_request) { create(:ci_trigger_request, trigger: trigger) } it 'creates a pipeline' do expect(execute_service(trigger_request: trigger_request)) diff --git a/spec/services/notification_service_spec.rb b/spec/services/notification_service_spec.rb index 44b2d28d1d4..5b349017c8b 100644 --- a/spec/services/notification_service_spec.rb +++ b/spec/services/notification_service_spec.rb @@ -130,7 +130,18 @@ describe NotificationService, :mailer do project.add_master(issue.author) project.add_master(assignee) project.add_master(note.author) - create(:note_on_issue, noteable: issue, project_id: issue.project_id, note: '@subscribed_participant cc this guy') + + @u_custom_off = create_user_with_notification(:custom, 'custom_off') + project.add_guest(@u_custom_off) + + create( + :note_on_issue, + author: @u_custom_off, + noteable: issue, + project_id: issue.project_id, + note: 'i think @subscribed_participant should see this' + ) + update_custom_notification(:new_note, @u_guest_custom, resource: project) update_custom_notification(:new_note, @u_custom_global) end @@ -140,8 +151,7 @@ describe NotificationService, :mailer do add_users_with_subscription(note.project, issue) reset_delivered_emails! - # Ensure create SentNotification by noteable = issue 6 times, not noteable = note - expect(SentNotification).to receive(:record).with(issue, any_args).exactly(8).times + expect(SentNotification).to receive(:record).with(issue, any_args).exactly(9).times notification.new_note(note) @@ -153,6 +163,7 @@ describe NotificationService, :mailer do should_email(@subscriber) should_email(@watcher_and_subscriber) should_email(@subscribed_participant) + should_email(@u_custom_off) should_not_email(@u_guest_custom) should_not_email(@u_guest_watcher) should_not_email(note.author) diff --git a/spec/services/test_hooks/system_service_spec.rb b/spec/services/test_hooks/system_service_spec.rb index 00d89924766..a15708bf82f 100644 --- a/spec/services/test_hooks/system_service_spec.rb +++ b/spec/services/test_hooks/system_service_spec.rb @@ -7,7 +7,6 @@ describe TestHooks::SystemService do let(:project) { create(:project, :repository) } let(:hook) { create(:system_hook) } let(:service) { described_class.new(hook, current_user, trigger) } - let(:sample_data) { { data: 'sample' }} let(:success_result) { { status: :success, http_status: 200, message: 'ok' } } before do @@ -26,18 +25,11 @@ describe TestHooks::SystemService do context 'push_events' do let(:trigger) { 'push_events' } - 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 project \"#{project.human_name}\" has commits." }) - 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(Gitlab::DataBuilder::Push).to receive(:sample_data).and_call_original - expect(hook).to receive(:execute).with(sample_data, trigger).and_return(success_result) + expect(hook).to receive(:execute).with(Gitlab::DataBuilder::Push::SAMPLE_DATA, trigger).and_return(success_result) expect(service.execute).to include(success_result) end end @@ -45,18 +37,11 @@ describe TestHooks::SystemService do context 'tag_push_events' do let(:trigger) { 'tag_push_events' } - it 'returns error message if not enough data' do - allow(project.repository).to receive(:tags).and_return([]) - - expect(hook).not_to receive(:execute) - expect(service.execute).to include({ status: :error, message: "Ensure project \"#{project.human_name}\" has tags." }) - end - it 'executes hook' do allow(project.repository).to receive(:tags).and_return(['tag']) - allow(Gitlab::DataBuilder::Push).to receive(:build_sample).and_return(sample_data) + expect(Gitlab::DataBuilder::Push).to receive(:sample_data).and_call_original - expect(hook).to receive(:execute).with(sample_data, trigger).and_return(success_result) + expect(hook).to receive(:execute).with(Gitlab::DataBuilder::Push::SAMPLE_DATA, trigger).and_return(success_result) expect(service.execute).to include(success_result) end end @@ -64,17 +49,11 @@ describe TestHooks::SystemService do context 'repository_update_events' do let(:trigger) { 'repository_update_events' } - it 'returns error message if not enough data' do - allow(project).to receive(:commit).and_return(nil) - expect(hook).not_to receive(:execute) - expect(service.execute).to include({ status: :error, message: "Ensure project \"#{project.human_name}\" has commits." }) - end - it 'executes hook' do allow(project).to receive(:empty_repo?).and_return(false) - allow(Gitlab::DataBuilder::Repository).to receive(:update).and_return(sample_data) + expect(Gitlab::DataBuilder::Repository).to receive(:sample_data).and_call_original - expect(hook).to receive(:execute).with(sample_data, trigger).and_return(success_result) + expect(hook).to receive(:execute).with(Gitlab::DataBuilder::Repository::SAMPLE_DATA, trigger).and_return(success_result) expect(service.execute).to include(success_result) end end diff --git a/spec/services/users/update_service_spec.rb b/spec/services/users/update_service_spec.rb index 985f6d94876..6ee35a33b2d 100644 --- a/spec/services/users/update_service_spec.rb +++ b/spec/services/users/update_service_spec.rb @@ -37,7 +37,10 @@ describe Users::UpdateService do describe '#execute!' do it 'updates the name' do - result = update_user(user, name: 'New Name') + service = described_class.new(user, name: 'New Name') + expect(service).not_to receive(:notify_new_user) + + result = service.execute! expect(result).to be true expect(user.name).to eq('New Name') @@ -49,6 +52,18 @@ describe Users::UpdateService do end.to raise_error(ActiveRecord::RecordInvalid) end + it 'fires system hooks when a new user is saved' do + system_hook_service = spy(:system_hook_service) + user = build(:user) + service = described_class.new(user, name: 'John Doe') + expect(service).to receive(:notify_new_user).and_call_original + expect(service).to receive(:system_hook_service).and_return(system_hook_service) + + service.execute + + expect(system_hook_service).to have_received(:execute_hooks_for).with(user, :create) + end + def update_user(user, opts) described_class.new(user, opts).execute! end diff --git a/spec/workers/concerns/build_queue_spec.rb b/spec/workers/concerns/build_queue_spec.rb deleted file mode 100644 index 6bf955e0be2..00000000000 --- a/spec/workers/concerns/build_queue_spec.rb +++ /dev/null @@ -1,14 +0,0 @@ -require 'spec_helper' - -describe BuildQueue do - let(:worker) do - Class.new do - include Sidekiq::Worker - include BuildQueue - end - end - - it 'sets the queue name of a worker' do - expect(worker.sidekiq_options['queue'].to_s).to eq('build') - end -end diff --git a/spec/workers/concerns/pipeline_queue_spec.rb b/spec/workers/concerns/pipeline_queue_spec.rb index 40794d0e42a..eac5a770e5f 100644 --- a/spec/workers/concerns/pipeline_queue_spec.rb +++ b/spec/workers/concerns/pipeline_queue_spec.rb @@ -8,7 +8,17 @@ describe PipelineQueue do end end - it 'sets the queue name of a worker' do - expect(worker.sidekiq_options['queue'].to_s).to eq('pipeline') + it 'sets a default pipelines queue automatically' do + expect(worker.sidekiq_options['queue']) + .to eq 'pipeline_default' + end + + describe '.enqueue_in' do + it 'sets a custom sidekiq queue with prefix and group' do + worker.enqueue_in(group: :processing) + + expect(worker.sidekiq_options['queue']) + .to eq 'pipeline_processing' + end end end diff --git a/spec/workers/pipeline_metrics_worker_spec.rb b/spec/workers/pipeline_metrics_worker_spec.rb index ef71125c0b6..896f9e6e7f2 100644 --- a/spec/workers/pipeline_metrics_worker_spec.rb +++ b/spec/workers/pipeline_metrics_worker_spec.rb @@ -2,7 +2,12 @@ require 'spec_helper' describe PipelineMetricsWorker do let(:project) { create(:project, :repository) } - let!(:merge_request) { create(:merge_request, source_project: project, source_branch: pipeline.ref, head_pipeline: pipeline) } + + let!(:merge_request) do + create(:merge_request, source_project: project, + source_branch: pipeline.ref, + head_pipeline: pipeline) + end let(:pipeline) do create(:ci_empty_pipeline, @@ -14,6 +19,8 @@ describe PipelineMetricsWorker do finished_at: Time.now) end + let(:status) { 'pending' } + describe '#perform' do before do described_class.new.perform(pipeline.id) |