From a7ed2e9da0379ba76773a6ff7151579e2f177a70 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oliver=20G=C3=BCnther?= Date: Wed, 9 Oct 2019 15:38:24 +0200 Subject: [PATCH] Rewrite jobs to use ActiveJob syntax --- app/controllers/copy_projects_controller.rb | 18 +- app/controllers/sys_controller.rb | 2 +- app/models/attachment.rb | 14 +- app/models/journal_notification_mailer.rb | 13 +- app/models/repository.rb | 2 +- app/models/watcher_notification_mailer.rb | 3 +- .../projects/schedule_deletion_service.rb | 3 +- .../scm/create_managed_repository_service.rb | 4 +- .../scm/delete_managed_repository_service.rb | 4 +- app/services/users/delete_service.rb | 2 +- app/workers/application_job.rb | 8 + .../attachments/cleanup_uncontainered_job.rb | 2 + app/workers/copy_project_job.rb | 16 +- app/workers/delete_user_job.rb | 12 +- app/workers/deliver_invitation_job.rb | 14 +- app/workers/deliver_notification_job.rb | 4 +- .../deliver_watcher_notification_job.rb | 2 +- .../deliver_work_package_notification_job.rb | 4 +- .../enqueue_work_package_notification_job.rb | 11 +- app/workers/extract_fulltext_job.rb | 8 +- app/workers/projects/delete_project_job.rb | 5 +- app/workers/rake_job.rb | 4 +- .../scm/create_local_repository_job.rb | 11 +- .../scm/create_remote_repository_job.rb | 4 +- .../scm/delete_remote_repository_job.rb | 4 +- app/workers/scm/relocate_repository_job.rb | 4 +- app/workers/scm/remote_repository_job.rb | 17 +- app/workers/scm/storage_updater_job.rb | 14 +- config/initializers/doorkeeper.rb | 2 +- config/initializers/user_invitation.rb | 4 +- lib/open_project/scm/manageable_repository.rb | 5 +- .../app/controllers/documents_controller.rb | 2 +- .../app/workers/work_package_webhook_job.rb | 4 +- .../webhooks/event_resources/work_package.rb | 2 +- spec/controllers/sys_controller_spec.rb | 277 +++++++++--------- .../table/queries/filter_spec.rb | 4 +- spec/models/attachment_spec.rb | 12 +- spec/models/copy_project_job_spec.rb | 112 ++++--- .../journal_notification_mailer_spec.rb | 33 +-- .../watcher_notification_mailer_spec.rb | 14 +- spec/requests/api/v3/project_resource_spec.rb | 3 + .../api/v3/user/user_resource_spec.rb | 39 +-- .../api/v3/work_package_resource_spec.rb | 89 +++--- .../work_packages_by_project_resource_spec.rb | 6 +- .../schedule_deletion_service_spec.rb | 9 +- .../create_managed_repository_service_spec.rb | 13 +- .../delete_managed_repository_service_spec.rb | 11 +- spec/support/scm/countable_repository.rb | 16 +- spec/support/scm/relocate_repository.rb | 8 +- .../deliver_watcher_notification_job_spec.rb | 6 +- ...iver_work_package_notification_job_spec.rb | 25 +- ...ueue_work_package_notification_job_spec.rb | 127 ++++---- .../scm/create_local_repository_job_spec.rb | 17 +- 53 files changed, 486 insertions(+), 563 deletions(-) diff --git a/app/controllers/copy_projects_controller.rb b/app/controllers/copy_projects_controller.rb index 802d071c5ab..20a52b79a02 100644 --- a/app/controllers/copy_projects_controller.rb +++ b/app/controllers/copy_projects_controller.rb @@ -81,21 +81,23 @@ class CopyProjectsController < ApplicationController end def enqueue_copy_job - copy_project_job = CopyProjectJob.new(user_id: User.current.id, - source_project_id: @project.id, - target_project_params: target_project_params, - associations_to_copy: params[:only], - send_mails: params[:notifications] == '1') - - Delayed::Job.enqueue copy_project_job, priority: ::ApplicationJob.priority_number(:low) + CopyProjectJob.perform_later(user_id: User.current.id, + source_project_id: @project.id, + target_project_params: target_project_params, + associations_to_copy: params[:only].reject(&:blank?), + send_mails: params[:notifications] == '1') end + ## + # Returns the target project params for + # the project to be copied. Stringifies id keys of custom field values + # due to serialization def target_project_params @copy_project .attributes .compact .with_indifferent_access - .merge(custom_field_values: @copy_project.custom_value_attributes) + .merge(custom_field_values: @copy_project.custom_value_attributes.transform_keys(&:to_s)) end def copy_started_notice diff --git a/app/controllers/sys_controller.rb b/app/controllers/sys_controller.rb index 47780b17306..60f921dbaf2 100644 --- a/app/controllers/sys_controller.rb +++ b/app/controllers/sys_controller.rb @@ -105,7 +105,7 @@ class SysController < ActionController::Base def update_storage_information(repository, force = false) if force - Delayed::Job.enqueue ::Scm::StorageUpdaterJob.new(repository), priority: ::ApplicationJob.priority_number(:low) + ::Scm::StorageUpdaterJob.perform_later(repository) true else repository.update_required_storage diff --git a/app/models/attachment.rb b/app/models/attachment.rb index e714b58789b..9d5e40029bd 100644 --- a/app/models/attachment.rb +++ b/app/models/attachment.rb @@ -200,8 +200,7 @@ class Attachment < ActiveRecord::Base def extract_fulltext return unless OpenProject::Database.allows_tsv? - job = ExtractFulltextJob.new(id) - Delayed::Job.enqueue job, priority: ::ApplicationJob.priority_number(:low) + ExtractFulltextJob.perform_later(id) end # Extract the fulltext of any attachments where fulltext is still nil. @@ -213,12 +212,11 @@ class Attachment < ActiveRecord::Base .where(fulltext: nil) .pluck(:id) .each do |id| - job = ExtractFulltextJob.new(id) if run_now - job.perform + ExtractFulltextJob.perform_now(id) else - Delayed::Job.enqueue job, priority: ::ApplicationJob.priority_number(:low) + ExtractFulltextJob.perform_later(id) end end end @@ -227,16 +225,14 @@ class Attachment < ActiveRecord::Base return unless OpenProject::Database.allows_tsv? Attachment.pluck(:id).each do |id| - job = ExtractFulltextJob.new(id) - job.perform + ExtractFulltextJob.perform_now(id) end end private def schedule_cleanup_uncontainered_job - Delayed::Job.enqueue Attachments::CleanupUncontaineredJob.new, - priority: ::ApplicationJob.priority_number(:low) + Attachments::CleanupUncontaineredJob.perform_later end def filesize_below_allowed_maximum diff --git a/app/models/journal_notification_mailer.rb b/app/models/journal_notification_mailer.rb index a2c033735af..08ceee69e3a 100644 --- a/app/models/journal_notification_mailer.rb +++ b/app/models/journal_notification_mailer.rb @@ -47,17 +47,14 @@ class JournalNotificationMailer if Journal::AggregatedJournal.hides_notifications?(aggregated, aggregated.predecessor) work_package = aggregated.predecessor.journable notification_receivers(work_package).each do |recipient| - job = DeliverWorkPackageNotificationJob.new(aggregated.predecessor.id, - recipient.id, - User.current.id) - Delayed::Job.enqueue job, priority: ::ApplicationJob.priority_number(:notification) + DeliverWorkPackageNotificationJob + .perform_later(aggregated.predecessor.id, recipient.id, User.current.id) end end - job = EnqueueWorkPackageNotificationJob.new(journal.id, User.current.id) - Delayed::Job.enqueue job, - run_at: delivery_time, - priority: ::ApplicationJob.priority_number(:notification) + EnqueueWorkPackageNotificationJob + .set(wait_until: delivery_time) + .perform_later(journal.id, User.current.id) end def send_notification?(journal) diff --git a/app/models/repository.rb b/app/models/repository.rb index 7a200d97fd1..6e799291a5c 100644 --- a/app/models/repository.rb +++ b/app/models/repository.rb @@ -205,7 +205,7 @@ class Repository < ActiveRecord::Base if storage_updated_at.nil? || storage_updated_at < oldest_cachable_time - Delayed::Job.enqueue ::Scm::StorageUpdaterJob.new(self), priority: ::ApplicationJob.priority_number(:low) + ::Scm::StorageUpdaterJob.perform_later(self) return true end end diff --git a/app/models/watcher_notification_mailer.rb b/app/models/watcher_notification_mailer.rb index 71a9b609809..64df749f223 100644 --- a/app/models/watcher_notification_mailer.rb +++ b/app/models/watcher_notification_mailer.rb @@ -35,8 +35,7 @@ class WatcherNotificationMailer return unless notify_about_watcher_added?(watcher, watcher_setter) unless other_jobs_queued?(watcher.watchable) - job = DeliverWatcherNotificationJob.new(watcher.id, watcher.user.id, watcher_setter.id) - Delayed::Job.enqueue job, priority: ::ApplicationJob.priority_number(:notification) + DeliverWatcherNotificationJob.perform_later(watcher.id, watcher.user.id, watcher_setter.id) end end diff --git a/app/services/projects/schedule_deletion_service.rb b/app/services/projects/schedule_deletion_service.rb index a26a848cd7e..4090786fb40 100644 --- a/app/services/projects/schedule_deletion_service.rb +++ b/app/services/projects/schedule_deletion_service.rb @@ -50,8 +50,7 @@ module Projects end def persist(call) - Delayed::Job.enqueue DeleteProjectJob.new(user_id: user.id, project_id: model.id), - priority: ::ApplicationJob.priority_number(:low) + DeleteProjectJob.perform_later(user_id: user.id, project_id: model.id) call end end diff --git a/app/services/scm/create_managed_repository_service.rb b/app/services/scm/create_managed_repository_service.rb index 4a0cb5ab951..4a20b414bcc 100644 --- a/app/services/scm/create_managed_repository_service.rb +++ b/app/services/scm/create_managed_repository_service.rb @@ -46,9 +46,9 @@ class Scm::CreateManagedRepositoryService < Scm::BaseRepositoryService # creating and deleting repositories, which provides transactional DB access # as well as filesystem access. if repository.class.manages_remote? - Scm::CreateRemoteRepositoryJob.new(repository, perform_now: true).perform + Scm::CreateRemoteRepositoryJob.perform_now(repository) else - Scm::CreateLocalRepositoryJob.new(repository).perform + Scm::CreateLocalRepositoryJob.perform_later(repository) end return true end diff --git a/app/services/scm/delete_managed_repository_service.rb b/app/services/scm/delete_managed_repository_service.rb index c839727db97..6e4fee819f0 100644 --- a/app/services/scm/delete_managed_repository_service.rb +++ b/app/services/scm/delete_managed_repository_service.rb @@ -38,7 +38,7 @@ class Scm::DeleteManagedRepositoryService < Scm::BaseRepositoryService return false unless repository.managed? if repository.class.manages_remote? - Scm::DeleteRemoteRepositoryJob.new(repository, perform_now: true).perform + Scm::DeleteRemoteRepositoryJob.perform_now(repository) true else delete_local_repository @@ -61,7 +61,7 @@ class Scm::DeleteManagedRepositoryService < Scm::BaseRepositoryService # Instead, this will be refactored into a single service wrapper for # creating and deleting repositories, which provides transactional DB access # as well as filesystem access. - Scm::DeleteLocalRepositoryJob.new(managed_path).perform + Scm::DeleteLocalRepositoryJob.perform_now(managed_path) end true diff --git a/app/services/users/delete_service.rb b/app/services/users/delete_service.rb index 0046e2b23cb..083694eaba9 100644 --- a/app/services/users/delete_service.rb +++ b/app/services/users/delete_service.rb @@ -48,7 +48,7 @@ module Users # as destroying users is a lengthy process we handle it in the background # and lock the account now so that no action can be performed with it user.lock! - Delayed::Job.enqueue DeleteUserJob.new(user.id), priority: ::ApplicationJob.priority_number(:low) + DeleteUserJob.perform_later(user) logout! if self_delete? diff --git a/app/workers/application_job.rb b/app/workers/application_job.rb index fd9b058b932..56ec4a0c806 100644 --- a/app/workers/application_job.rb +++ b/app/workers/application_job.rb @@ -45,6 +45,14 @@ class ApplicationJob < ::ActiveJob::Base end end + def self.queue_with_priority(value = :default) + if value.is_a?(Symbol) + super priority_number(value) + else + super value + end + end + def self.inherited(child) child.prepend Setup end diff --git a/app/workers/attachments/cleanup_uncontainered_job.rb b/app/workers/attachments/cleanup_uncontainered_job.rb index 3a9bf6a1f7e..c81c78aa4a9 100644 --- a/app/workers/attachments/cleanup_uncontainered_job.rb +++ b/app/workers/attachments/cleanup_uncontainered_job.rb @@ -29,6 +29,8 @@ #++ class Attachments::CleanupUncontaineredJob < ApplicationJob + queue_with_priority :low + def perform Attachment .where(container: nil) diff --git a/app/workers/copy_project_job.rb b/app/workers/copy_project_job.rb index 2243c9405e9..b81dca88e99 100644 --- a/app/workers/copy_project_job.rb +++ b/app/workers/copy_project_job.rb @@ -28,28 +28,32 @@ #++ class CopyProjectJob < ApplicationJob + queue_with_priority :low include OpenProject::LocaleHelper attr_reader :user_id, :source_project_id, :target_project_params, + :target_project_name, + :target_project, + :errors, :associations_to_copy, :send_mails - def initialize(user_id:, source_project_id:, target_project_params:, + def perform(user_id:, source_project_id:, target_project_params:, associations_to_copy:, send_mails: false) + # Needs refactoring after moving to activejob + @user_id = user_id @source_project_id = source_project_id @target_project_params = target_project_params.with_indifferent_access @associations_to_copy = associations_to_copy @send_mails = send_mails - end - def perform User.current = user - target_project_name = target_project_params[:name] + @target_project_name = target_project_params[:name] - target_project, errors = with_locale_for(user) do + @target_project, @errors = with_locale_for(user) do create_project_copy(source_project, target_project_params, associations_to_copy, @@ -128,6 +132,6 @@ class CopyProjectJob < ApplicationJob end def logger - Delayed::Worker.logger + Rails.logger end end diff --git a/app/workers/delete_user_job.rb b/app/workers/delete_user_job.rb index eab3020ef0f..4b910624c6f 100644 --- a/app/workers/delete_user_job.rb +++ b/app/workers/delete_user_job.rb @@ -28,17 +28,9 @@ #++ class DeleteUserJob < ApplicationJob - def initialize(user_id) - @user_id = user_id - end + queue_with_priority :low - def perform + def perform(user) user.destroy end - - private - - def user - @user ||= User.find @user_id - end end diff --git a/app/workers/deliver_invitation_job.rb b/app/workers/deliver_invitation_job.rb index ca424584ff4..aef04125824 100644 --- a/app/workers/deliver_invitation_job.rb +++ b/app/workers/deliver_invitation_job.rb @@ -28,21 +28,11 @@ #++ class DeliverInvitationJob < ApplicationJob - attr_reader :token_id - - def initialize(token_id) - @token_id = token_id - end - - def perform + def perform(token) if token UserMailer.user_signed_up(token).deliver_later else - Rails.logger.warn "Can't deliver invitation. The token is missing: #{token_id}" + Rails.logger.warn "Can't deliver invitation. The token is missing." end end - - def token - @token ||= Token::Invitation.find_by(id: @token_id) - end end diff --git a/app/workers/deliver_notification_job.rb b/app/workers/deliver_notification_job.rb index b990a380d7a..53e6a832bda 100644 --- a/app/workers/deliver_notification_job.rb +++ b/app/workers/deliver_notification_job.rb @@ -28,12 +28,10 @@ #++ class DeliverNotificationJob < ApplicationJob - def initialize(recipient_id, sender_id) + def perform(recipient_id, sender_id) @recipient_id = recipient_id @sender_id = sender_id - end - def perform # nothing to do if recipient was deleted in the meantime return unless recipient diff --git a/app/workers/deliver_watcher_notification_job.rb b/app/workers/deliver_watcher_notification_job.rb index 7aa998875fb..b06e5dc6087 100644 --- a/app/workers/deliver_watcher_notification_job.rb +++ b/app/workers/deliver_watcher_notification_job.rb @@ -29,7 +29,7 @@ class DeliverWatcherNotificationJob < DeliverNotificationJob - def initialize(watcher_id, recipient_id, watcher_setter_id) + def perform(watcher_id, recipient_id, watcher_setter_id) @watcher_id = watcher_id super(recipient_id, watcher_setter_id) diff --git a/app/workers/deliver_work_package_notification_job.rb b/app/workers/deliver_work_package_notification_job.rb index 8f0258d396a..bdb34aebc49 100644 --- a/app/workers/deliver_work_package_notification_job.rb +++ b/app/workers/deliver_work_package_notification_job.rb @@ -28,7 +28,9 @@ #++ class DeliverWorkPackageNotificationJob < DeliverNotificationJob - def initialize(journal_id, recipient_id, author_id) + queue_with_priority :notification + + def perform(journal_id, recipient_id, author_id) @journal_id = journal_id super(recipient_id, author_id) end diff --git a/app/workers/enqueue_work_package_notification_job.rb b/app/workers/enqueue_work_package_notification_job.rb index 6a439c44fe8..d527d2b2fb8 100644 --- a/app/workers/enqueue_work_package_notification_job.rb +++ b/app/workers/enqueue_work_package_notification_job.rb @@ -29,12 +29,12 @@ # Enqueues class EnqueueWorkPackageNotificationJob < ApplicationJob - def initialize(journal_id, author_id) + + def perform(journal_id, author_id) + # This is caused by a DJ job running as ActiveJob @journal_id = journal_id @author_id = author_id - end - def perform # if the WP has been deleted the unaggregated journal will have been deleted too # and our job here is done return nil unless raw_journal @@ -46,7 +46,7 @@ class EnqueueWorkPackageNotificationJob < ApplicationJob # sending the notification. Our job here is done. return nil unless @journal - @author = User.find_by(id: @author_id) || DeletedUser.first + @author = User.find_by(id: author_id) || DeletedUser.first # Do not deliver notifications if a follow-up journal will already have sent a notification # on behalf of this job. @@ -64,8 +64,7 @@ class EnqueueWorkPackageNotificationJob < ApplicationJob def deliver_notifications_for(journal) notification_receivers(work_package).each do |recipient| - job = DeliverWorkPackageNotificationJob.new(journal.id, recipient.id, @author_id) - Delayed::Job.enqueue job, priority: ::ApplicationJob.priority_number(:notification) + DeliverWorkPackageNotificationJob.perform_later(journal.id, recipient.id, @author_id) end OpenProject::Notifications.send( diff --git a/app/workers/extract_fulltext_job.rb b/app/workers/extract_fulltext_job.rb index e140bf39fe7..f795fb99439 100644 --- a/app/workers/extract_fulltext_job.rb +++ b/app/workers/extract_fulltext_job.rb @@ -29,18 +29,18 @@ #++ class ExtractFulltextJob < ApplicationJob - def initialize(attachment_id) + queue_with_priority :low + + def perform(attachment_id) @attachment_id = attachment_id @attachment = nil @text = nil @file = nil @filename = nil @language = OpenProject::Configuration.main_content_language - end - def perform return unless OpenProject::Database.allows_tsv? - return unless @attachment = find_attachment(@attachment_id) + return unless @attachment = find_attachment(attachment_id) init update diff --git a/app/workers/projects/delete_project_job.rb b/app/workers/projects/delete_project_job.rb index c4f96834c44..5391f9058d0 100644 --- a/app/workers/projects/delete_project_job.rb +++ b/app/workers/projects/delete_project_job.rb @@ -30,17 +30,16 @@ module Projects class DeleteProjectJob < ApplicationJob + queue_with_priority :low include OpenProject::LocaleHelper attr_reader :user_id, :project_id - def initialize(user_id:, project_id:) + def perform(user_id:, project_id:) @user_id = user_id @project_id = project_id - end - def perform service_call = delete_project if service_call.failure? diff --git a/app/workers/rake_job.rb b/app/workers/rake_job.rb index cccd55dfaf5..84d65cd810a 100644 --- a/app/workers/rake_job.rb +++ b/app/workers/rake_job.rb @@ -33,11 +33,9 @@ require 'rake' class RakeJob < ApplicationJob attr_reader :task_name - def initialize(task_name) + def perform(task_name) @task_name = task_name - end - def perform Rails.logger.info { "Invoking Rake task #{task_name}." } invoke end diff --git a/app/workers/scm/create_local_repository_job.rb b/app/workers/scm/create_local_repository_job.rb index 81eb7cab866..1a2b07bc2b8 100644 --- a/app/workers/scm/create_local_repository_job.rb +++ b/app/workers/scm/create_local_repository_job.rb @@ -35,7 +35,9 @@ # creation and deletion of repositories BOTH on the database and filesystem. # Until then, a synchronous process is more failsafe. class Scm::CreateLocalRepositoryJob < ApplicationJob - def initialize(repository) + def perform(repository) + @repository = repository + # Cowardly refusing to override existing local repository if File.directory?(repository.root_url) raise OpenProject::Scm::Exceptions::ScmError.new( @@ -43,13 +45,6 @@ class Scm::CreateLocalRepositoryJob < ApplicationJob ) end - # TODO currently uses the full repository object, - # as the Job is performed synchronously. - # Change this to serialize the ID once its turned to process asynchronously. - @repository = repository - end - - def perform # Create the repository locally. mode = (config[:mode] || default_mode) diff --git a/app/workers/scm/create_remote_repository_job.rb b/app/workers/scm/create_remote_repository_job.rb index 331f3176553..11c1ff90c3d 100644 --- a/app/workers/scm/create_remote_repository_job.rb +++ b/app/workers/scm/create_remote_repository_job.rb @@ -36,7 +36,9 @@ # creation and deletion of repositories BOTH on the database and filesystem. # Until then, a synchronous process is more failsafe. class Scm::CreateRemoteRepositoryJob < Scm::RemoteRepositoryJob - def perform + def perform(repository) + super(repository) + response = send_request(repository_request.merge(action: :create)) repository.root_url = response['path'] repository.url = response['url'] diff --git a/app/workers/scm/delete_remote_repository_job.rb b/app/workers/scm/delete_remote_repository_job.rb index 41a2a2e6d65..ce24eae415a 100644 --- a/app/workers/scm/delete_remote_repository_job.rb +++ b/app/workers/scm/delete_remote_repository_job.rb @@ -35,7 +35,9 @@ # creation and deletion of repositories BOTH on the database and filesystem. # Until then, a synchronous process is more failsafe. class Scm::DeleteRemoteRepositoryJob < Scm::RemoteRepositoryJob - def perform + + def perform(repository) + super send_request(repository_request.merge(action: :delete)) end end diff --git a/app/workers/scm/relocate_repository_job.rb b/app/workers/scm/relocate_repository_job.rb index 8e5d4e11368..5fe9a0cbf21 100644 --- a/app/workers/scm/relocate_repository_job.rb +++ b/app/workers/scm/relocate_repository_job.rb @@ -30,7 +30,9 @@ ## # Provides an asynchronous job to relocate a managed repository on the local or remote system class Scm::RelocateRepositoryJob < Scm::RemoteRepositoryJob - def perform + def perform(repository) + super(repository) + if repository.class.manages_remote? relocate_remote else diff --git a/app/workers/scm/remote_repository_job.rb b/app/workers/scm/remote_repository_job.rb index 4c160e16c25..cc78634a85e 100644 --- a/app/workers/scm/remote_repository_job.rb +++ b/app/workers/scm/remote_repository_job.rb @@ -40,17 +40,8 @@ require 'net/http' class Scm::RemoteRepositoryJob < ApplicationJob attr_reader :repository - ## - # Initialize the job, optionally saving the whole repository object - # (use only when not serializing the job.) - # As we're using the jobs majorly synchronously for the time being, it saves a db trip. - # When we have error handling for asynchronous tasks, refactor this. - def initialize(repository, perform_now: false) - if perform_now - @repository = repository - else - @repository_id = repository.id - end + def perform(repository) + @repository = repository end protected @@ -105,10 +96,6 @@ class Scm::RemoteRepositoryJob < ApplicationJob } end - def repository - @repository ||= Repository.find(@repository_id) - end - ## # For packager and snakeoil-ssl certificates, we need to provide the user # with an option to skip SSL certificate verification when communicating diff --git a/app/workers/scm/storage_updater_job.rb b/app/workers/scm/storage_updater_job.rb index 6550b9c0eee..226b15d9dc8 100644 --- a/app/workers/scm/storage_updater_job.rb +++ b/app/workers/scm/storage_updater_job.rb @@ -28,26 +28,20 @@ #++ class Scm::StorageUpdaterJob < ApplicationJob - def initialize(repository) - @id = repository.id + queue_with_priority :low + def perform(repository) unless repository.scm.storage_available? - raise OpenProject::Scm::Exceptions::ScmError.new( - I18n.t('repositories.storage.not_available') - ) + Rails.logger.warn "Storage is not available for repository #{repository.id}" + return end - end - def perform - repository = Repository.find @id bytes = repository.scm.count_repository! repository.update!( required_storage_bytes: bytes, storage_updated_at: Time.now, ) - rescue ActiveRecord::RecordNotFound - Rails.logger.warn("StorageUpdater requested for Repository ##{@id}, which could not be found.") end ## diff --git a/config/initializers/doorkeeper.rb b/config/initializers/doorkeeper.rb index 4cf134ede7b..9220e94a34d 100644 --- a/config/initializers/doorkeeper.rb +++ b/config/initializers/doorkeeper.rb @@ -193,7 +193,7 @@ Doorkeeper.configure do # after_successful_authorization do |controller| # Schedule a cleanup job to clean out over-TTL tokens and grants - Delayed::Job.enqueue ::OAuth::CleanupJob.new, priority: ::ApplicationJob.priority_number(:low) + ::OAuth::CleanupJob.perform_later end # Under some circumstances you might want to have applications auto-approved, diff --git a/config/initializers/user_invitation.rb b/config/initializers/user_invitation.rb index 9e93af06cc7..b68eb6d0413 100644 --- a/config/initializers/user_invitation.rb +++ b/config/initializers/user_invitation.rb @@ -2,9 +2,9 @@ # The default behaviour is to send the user a sign-up mail # when they were invited. OpenProject::Notifications.subscribe UserInvitation::Events.user_invited do |token| - Delayed::Job.enqueue DeliverInvitationJob.new(token.id), priority: ::ApplicationJob.priority_number(:high) + DeliverInvitationJob.perform_later(token) end OpenProject::Notifications.subscribe UserInvitation::Events.user_reinvited do |token| - Delayed::Job.enqueue DeliverInvitationJob.new(token.id), priority: ::ApplicationJob.priority_number(:high) + DeliverInvitationJob.perform_later(token) end diff --git a/lib/open_project/scm/manageable_repository.rb b/lib/open_project/scm/manageable_repository.rb index 650f43fea67..4fadec37cde 100644 --- a/lib/open_project/scm/manageable_repository.rb +++ b/lib/open_project/scm/manageable_repository.rb @@ -38,9 +38,8 @@ module OpenProject OpenProject::Notifications.subscribe('project_renamed') do |payload| repository = payload[:project].repository - if repository && repository.managed? - Delayed::Job.enqueue ::Scm::RelocateRepositoryJob.new(repository), - priority: ::ApplicationJob.priority_number(:low) + if repository&.managed? + ::Scm::RelocateRepositoryJob.perform_later(repository) end end end diff --git a/modules/documents/app/controllers/documents_controller.rb b/modules/documents/app/controllers/documents_controller.rb index a67b75ecd3e..32cea737250 100644 --- a/modules/documents/app/controllers/documents_controller.rb +++ b/modules/documents/app/controllers/documents_controller.rb @@ -112,7 +112,7 @@ class DocumentsController < ApplicationController if saved_attachments.present? && Setting.notified_events.include?('document_added') users = saved_attachments.first.container.recipients users.each do |user| - UserMailer.attachments_added(user, saved_attachments).deliver + UserMailer.attachments_added(user, saved_attachments).deliver_later end end redirect_to action: 'show', id: @document diff --git a/modules/webhooks/app/workers/work_package_webhook_job.rb b/modules/webhooks/app/workers/work_package_webhook_job.rb index 0ec3817b3c3..12e25d93c9c 100644 --- a/modules/webhooks/app/workers/work_package_webhook_job.rb +++ b/modules/webhooks/app/workers/work_package_webhook_job.rb @@ -36,13 +36,11 @@ class WorkPackageWebhookJob < WebhookJob attr_reader :journal_id attr_reader :event_name - def initialize(webhook_id, journal_id, event_name) + def perform(webhook_id, journal_id, event_name) @webhook_id = webhook_id @journal_id = journal_id @event_name = event_name - end - def perform return unless webhook.enabled_for_project?(work_package.project_id) body = request_body diff --git a/modules/webhooks/lib/open_project/webhooks/event_resources/work_package.rb b/modules/webhooks/lib/open_project/webhooks/event_resources/work_package.rb index af5e019defe..3547733cd70 100644 --- a/modules/webhooks/lib/open_project/webhooks/event_resources/work_package.rb +++ b/modules/webhooks/lib/open_project/webhooks/event_resources/work_package.rb @@ -23,7 +23,7 @@ module OpenProject::Webhooks::EventResources action = payload[:initial] ? "created" : "updated" event_name = prefixed_event_name(action) active_webhooks.with_event_name(event_name).pluck(:id).each do |id| - Delayed::Job.enqueue WorkPackageWebhookJob.new(id, payload[:journal_id], event_name) + WorkPackageWebhookJob.perform_later(id, payload[:journal_id], event_name) end end end diff --git a/spec/controllers/sys_controller_spec.rb b/spec/controllers/sys_controller_spec.rb index 9a3679c98b5..7d223355085 100644 --- a/spec/controllers/sys_controller_spec.rb +++ b/spec/controllers/sys_controller_spec.rb @@ -37,9 +37,9 @@ describe SysController, type: :controller do let(:valid_user_password) { 'Top Secret Password' } let(:valid_user) { FactoryBot.create(:user, - login: 'johndoe', - password: valid_user_password, - password_confirmation: valid_user_password) + login: 'johndoe', + password: valid_user_password, + password_confirmation: valid_user_password) } let(:api_key) { '12345678' } @@ -53,9 +53,9 @@ describe SysController, type: :controller do random_project = FactoryBot.create(:project, public: false) FactoryBot.create(:member, - user: valid_user, - roles: [browse_role], - project: random_project) + user: valid_user, + roles: [browse_role], + project: random_project) allow(Setting).to receive(:sys_api_key).and_return(api_key) allow(Setting).to receive(:sys_api_enabled?).and_return(true) allow(Setting).to receive(:repository_authentication_caching_enabled?).and_return(true) @@ -76,9 +76,9 @@ describe SysController, type: :controller do valid_user_password ) - post 'repo_auth', params: { key: api_key, - repository: 'without-access', - method: 'GET' } + post 'repo_auth', params: {key: api_key, + repository: 'without-access', + method: 'GET'} end it 'should respond 403 not allowed' do @@ -90,9 +90,9 @@ describe SysController, type: :controller do context 'for valid login and user has read permission (role reporter) for project' do before(:each) do FactoryBot.create(:member, - user: valid_user, - roles: [browse_role], - project: project) + user: valid_user, + roles: [browse_role], + project: project) request.env['HTTP_AUTHORIZATION'] = ActionController::HttpAuthentication::Basic.encode_credentials( @@ -102,17 +102,17 @@ describe SysController, type: :controller do end it 'should respond 200 okay dokay for GET' do - post 'repo_auth', params: { key: api_key, - repository: project.identifier, - method: 'GET' } + post 'repo_auth', params: {key: api_key, + repository: project.identifier, + method: 'GET'} expect(response.code).to eq('200') end it 'should respond 403 not allowed for POST' do - post 'repo_auth', params: { key: api_key, - repository: project.identifier, - method: 'POST' } + post 'repo_auth', params: {key: api_key, + repository: project.identifier, + method: 'POST'} expect(response.code).to eq('403') end @@ -121,9 +121,9 @@ describe SysController, type: :controller do context 'for valid login and user has rw permission (role developer) for project' do before(:each) do FactoryBot.create(:member, - user: valid_user, - roles: [commit_role], - project: project) + user: valid_user, + roles: [commit_role], + project: project) valid_user.save request.env['HTTP_AUTHORIZATION'] = ActionController::HttpAuthentication::Basic.encode_credentials( @@ -133,17 +133,17 @@ describe SysController, type: :controller do end it 'should respond 200 okay dokay for GET' do - post 'repo_auth', params: { key: api_key, - repository: project.identifier, - method: 'GET' } + post 'repo_auth', params: {key: api_key, + repository: project.identifier, + method: 'GET'} expect(response.code).to eq('200') end it 'should respond 200 okay dokay for POST' do - post 'repo_auth', params: { key: api_key, - repository: project.identifier, - method: 'POST' } + post 'repo_auth', params: {key: api_key, + repository: project.identifier, + method: 'POST'} expect(response.code).to eq('200') end @@ -152,18 +152,18 @@ describe SysController, type: :controller do context 'for invalid login and user has role manager for project' do before(:each) do FactoryBot.create(:member, - user: valid_user, - roles: [commit_role], - project: project) + user: valid_user, + roles: [commit_role], + project: project) request.env['HTTP_AUTHORIZATION'] = ActionController::HttpAuthentication::Basic.encode_credentials( valid_user.login, valid_user_password + 'made invalid' ) - post 'repo_auth', params: { key: api_key, - repository: project.identifier, - method: 'GET' } + post 'repo_auth', params: {key: api_key, + repository: project.identifier, + method: 'GET'} end it 'should respond 401 auth required' do @@ -179,9 +179,9 @@ describe SysController, type: :controller do valid_user_password ) - post 'repo_auth', params: { key: api_key, - repository: project.identifier, - method: 'GET' } + post 'repo_auth', params: {key: api_key, + repository: project.identifier, + method: 'GET'} end it 'should respond 403 not allowed' do @@ -195,9 +195,9 @@ describe SysController, type: :controller do before(:each) do random_project = FactoryBot.create(:project, public: false) FactoryBot.create(:member, - user: valid_user, - roles: [browse_role], - project: random_project) + user: valid_user, + roles: [browse_role], + project: random_project) request.env['HTTP_AUTHORIZATION'] = ActionController::HttpAuthentication::Basic.encode_credentials( @@ -205,9 +205,9 @@ describe SysController, type: :controller do valid_user_password ) - post 'repo_auth', params: { key: api_key, - repository: project.identifier, - method: 'GET' } + post 'repo_auth', params: {key: api_key, + repository: project.identifier, + method: 'GET'} end it 'should respond 200 OK' do @@ -217,9 +217,9 @@ describe SysController, type: :controller do context 'for invalid credentials' do before(:each) do - post 'repo_auth', params: { key: api_key, - repository: 'any-repo', - method: 'GET' } + post 'repo_auth', params: {key: api_key, + repository: 'any-repo', + method: 'GET'} end it 'should respond 401 auth required' do @@ -235,9 +235,9 @@ describe SysController, type: :controller do valid_user.login, valid_user_password ) - post 'repo_auth', params: { key: 'not_the_api_key', - repository: 'any-repo', - method: 'GET' } + post 'repo_auth', params: {key: 'not_the_api_key', + repository: 'any-repo', + method: 'GET'} expect(response.code).to eq('403') expect(response.body) @@ -251,9 +251,9 @@ describe SysController, type: :controller do 'invalid' ) - post 'repo_auth', params: { key: 'not_the_api_key', - repository: 'any-repo', - method: 'GET' } + post 'repo_auth', params: {key: 'not_the_api_key', + repository: 'any-repo', + method: 'GET'} expect(response.code).to eq('403') expect(response.body) @@ -274,12 +274,12 @@ describe SysController, type: :controller do valid_user_password ) - post 'repo_auth', params: { key: api_key, - repository: 'without-access', - method: 'GET', - git_smart_http: '1', - uri: '/git', - location: '/git' } + post 'repo_auth', params: {key: api_key, + repository: 'without-access', + method: 'GET', + git_smart_http: '1', + uri: '/git', + location: '/git'} end it 'should respond 403 not allowed' do @@ -291,9 +291,9 @@ describe SysController, type: :controller do context 'for valid login and user has read permission (role reporter) for project' do before(:each) do FactoryBot.create(:member, - user: valid_user, - roles: [browse_role], - project: project) + user: valid_user, + roles: [browse_role], + project: project) request.env['HTTP_AUTHORIZATION'] = ActionController::HttpAuthentication::Basic.encode_credentials( @@ -303,23 +303,23 @@ describe SysController, type: :controller do end it 'should respond 200 okay dokay for read-only access' do - post 'repo_auth', params: { key: api_key, - repository: project.identifier, - method: 'GET', - git_smart_http: '1', - uri: '/git', - location: '/git' } + post 'repo_auth', params: {key: api_key, + repository: project.identifier, + method: 'GET', + git_smart_http: '1', + uri: '/git', + location: '/git'} expect(response.code).to eq('200') end it 'should respond 403 not allowed for write (push)' do - post 'repo_auth', params: { key: api_key, - repository: project.identifier, - method: 'POST', - git_smart_http: '1', - uri: "/git/#{project.identifier}/git-receive-pack", - location: '/git' } + post 'repo_auth', params: {key: api_key, + repository: project.identifier, + method: 'POST', + git_smart_http: '1', + uri: "/git/#{project.identifier}/git-receive-pack", + location: '/git'} expect(response.code).to eq('403') end @@ -328,9 +328,9 @@ describe SysController, type: :controller do context 'for valid login and user has rw permission (role developer) for project' do before(:each) do FactoryBot.create(:member, - user: valid_user, - roles: [commit_role], - project: project) + user: valid_user, + roles: [commit_role], + project: project) valid_user.save request.env['HTTP_AUTHORIZATION'] = @@ -341,23 +341,23 @@ describe SysController, type: :controller do end it 'should respond 200 okay dokay for GET' do - post 'repo_auth', params: { key: api_key, - repository: project.identifier, - method: 'GET', - git_smart_http: '1', - uri: '/git', - location: '/git' } + post 'repo_auth', params: {key: api_key, + repository: project.identifier, + method: 'GET', + git_smart_http: '1', + uri: '/git', + location: '/git'} expect(response.code).to eq('200') end it 'should respond 200 okay dokay for POST' do - post 'repo_auth', params: { key: api_key, - repository: project.identifier, - method: 'POST', - git_smart_http: '1', - uri: "/git/#{project.identifier}/git-receive-pack", - location: '/git' } + post 'repo_auth', params: {key: api_key, + repository: project.identifier, + method: 'POST', + git_smart_http: '1', + uri: "/git/#{project.identifier}/git-receive-pack", + location: '/git'} expect(response.code).to eq('200') end @@ -366,9 +366,9 @@ describe SysController, type: :controller do context 'for invalid login and user has role manager for project' do before(:each) do FactoryBot.create(:member, - user: valid_user, - roles: [commit_role], - project: project) + user: valid_user, + roles: [commit_role], + project: project) request.env['HTTP_AUTHORIZATION'] = ActionController::HttpAuthentication::Basic.encode_credentials( @@ -376,12 +376,12 @@ describe SysController, type: :controller do valid_user_password + 'made invalid' ) - post 'repo_auth', params: { key: api_key, - repository: project.identifier, - method: 'GET', - git_smart_http: '1', - uri: '/git', - location: '/git' } + post 'repo_auth', params: {key: api_key, + repository: project.identifier, + method: 'GET', + git_smart_http: '1', + uri: '/git', + location: '/git'} end it 'should respond 401 auth required' do @@ -398,12 +398,12 @@ describe SysController, type: :controller do valid_user_password ) - post 'repo_auth', params: { key: api_key, - repository: project.identifier, - method: 'GET', - git_smart_http: '1', - uri: '/git', - location: '/git' } + post 'repo_auth', params: {key: api_key, + repository: project.identifier, + method: 'GET', + git_smart_http: '1', + uri: '/git', + location: '/git'} end it 'should respond 403 not allowed' do @@ -416,21 +416,21 @@ describe SysController, type: :controller do before(:each) do random_project = FactoryBot.create(:project, public: false) FactoryBot.create(:member, - user: valid_user, - roles: [browse_role], - project: random_project) + user: valid_user, + roles: [browse_role], + project: random_project) request.env['HTTP_AUTHORIZATION'] = ActionController::HttpAuthentication::Basic.encode_credentials( valid_user.login, valid_user_password ) - post 'repo_auth', params: { key: api_key, - repository: project.identifier, - method: 'GET', - git_smart_http: '1', - uri: '/git', - location: '/git' } + post 'repo_auth', params: {key: api_key, + repository: project.identifier, + method: 'GET', + git_smart_http: '1', + uri: '/git', + location: '/git'} end it 'should respond 200 OK' do @@ -440,12 +440,12 @@ describe SysController, type: :controller do context 'for invalid credentials' do before(:each) do - post 'repo_auth', params: { key: api_key, - repository: 'any-repo', - method: 'GET', - git_smart_http: '1', - uri: '/git', - location: '/git' } + post 'repo_auth', params: {key: api_key, + repository: 'any-repo', + method: 'GET', + git_smart_http: '1', + uri: '/git', + location: '/git'} end it 'should respond 401 auth required' do @@ -462,12 +462,12 @@ describe SysController, type: :controller do valid_user_password ) - post 'repo_auth', params: { key: 'not_the_api_key', - repository: 'any-repo', - method: 'GET', - git_smart_http: '1', - uri: '/git', - location: '/git' } + post 'repo_auth', params: {key: 'not_the_api_key', + repository: 'any-repo', + method: 'GET', + git_smart_http: '1', + uri: '/git', + location: '/git'} expect(response.code).to eq('403') expect(response.body) @@ -481,12 +481,12 @@ describe SysController, type: :controller do 'invalid' ) - post 'repo_auth', params: { key: 'not_the_api_key', - repository: 'any-repo', - method: 'GET', - git_smart_http: '1', - uri: '/git', - location: '/git' } + post 'repo_auth', params: {key: 'not_the_api_key', + repository: 'any-repo', + method: 'GET', + git_smart_http: '1', + uri: '/git', + location: '/git'} expect(response.code).to eq('403') expect(response.body) @@ -546,9 +546,9 @@ describe SysController, type: :controller do let(:last_updated) { nil } def request_storage - get 'update_required_storage', params: { key: apikey, - id: id, - force: force } + get 'update_required_storage', params: {key: apikey, + id: id, + force: force} end context 'missing project' do @@ -648,10 +648,9 @@ describe SysController, type: :controller do context 'outdated storage' do let(:last_updated) { 2.days.ago } - it 'updates the storage' do - expect(Delayed::Job) - .to receive(:enqueue).with(instance_of(::Scm::StorageUpdaterJob), any_args) + it 'updates the storage' do + expect(::Scm::StorageUpdaterJob).to receive(:perform_later) request_storage end end @@ -660,9 +659,7 @@ describe SysController, type: :controller do let(:last_updated) { 10.minutes.ago } it 'does not update to storage' do - expect(Delayed::Job) - .not_to receive(:enqueue).with(instance_of(::Scm::StorageUpdaterJob), any_args) - + expect(::Scm::StorageUpdaterJob).not_to receive(:perform_later) request_storage end end @@ -672,9 +669,7 @@ describe SysController, type: :controller do let(:last_updated) { 10.minutes.ago } it 'does update to storage' do - expect(Delayed::Job) - .to receive(:enqueue).with(instance_of(::Scm::StorageUpdaterJob), any_args) - + expect(::Scm::StorageUpdaterJob).to receive(:perform_later) request_storage end end diff --git a/spec/features/work_packages/table/queries/filter_spec.rb b/spec/features/work_packages/table/queries/filter_spec.rb index 1c558aaa4e1..2cd32b7f5ea 100644 --- a/spec/features/work_packages/table/queries/filter_spec.rb +++ b/spec/features/work_packages/table/queries/filter_spec.rb @@ -270,10 +270,10 @@ describe 'filter work packages', js: true do allow_any_instance_of(Plaintext::Resolver).to receive(:text).and_return('I am the first text $1.99.') wp_with_attachment_a - ExtractFulltextJob.new(attachment_a.id).perform + ExtractFulltextJob.perform_now(attachment_a.id) allow_any_instance_of(Plaintext::Resolver).to receive(:text).and_return('I am the second text.') wp_with_attachment_b - ExtractFulltextJob.new(attachment_b.id).perform + ExtractFulltextJob.perform_now(attachment_b.id) wp_without_attachment end diff --git a/spec/models/attachment_spec.rb b/spec/models/attachment_spec.rb index 7bdf27a102e..12b9def6989 100644 --- a/spec/models/attachment_spec.rb +++ b/spec/models/attachment_spec.rb @@ -176,9 +176,7 @@ describe Attachment, type: :model do end it 'adds no cleanup job' do - expect(Delayed::Job) - .not_to receive(:enqueue) - .with an_instance_of(Attachments::CleanupUncontaineredJob) + expect(Attachments::CleanupUncontaineredJob).not_to receive(:perform_later) attachment.save! end @@ -187,13 +185,7 @@ describe Attachment, type: :model do let(:container) { nil } it 'adds a cleanup job' do - allow(Delayed::Job) - .to receive(:enqueue) - - expect(Delayed::Job) - .to receive(:enqueue) - .with(an_instance_of(Attachments::CleanupUncontaineredJob), any_args) - + expect(Attachments::CleanupUncontaineredJob).to receive(:perform_later) attachment.save! end end diff --git a/spec/models/copy_project_job_spec.rb b/spec/models/copy_project_job_spec.rb index 400042da72f..7a7286c8148 100644 --- a/spec/models/copy_project_job_spec.rb +++ b/spec/models/copy_project_job_spec.rb @@ -32,7 +32,7 @@ describe CopyProjectJob, type: :model do let(:project) { FactoryBot.create(:project, public: false) } let(:user) { FactoryBot.create(:user) } let(:role) { FactoryBot.create(:role, permissions: [:copy_projects]) } - let(:params) { { name: 'Copy', identifier: 'copy' } } + let(:params) { {name: 'Copy', identifier: 'copy'} } let(:maildouble) { double('Mail::Message', deliver: true) } before do @@ -45,27 +45,22 @@ describe CopyProjectJob, type: :model do let(:target_project) { FactoryBot.create(:project) } let(:copy_job) do - CopyProjectJob.new user_id: user_de.id, - source_project_id: source_project.id, - target_project_params: {}, - associations_to_copy: [] - end - - before do - # 'Delayed Job' uses a work around to get Rails 3 mailers working with it - # (see https://github.com/collectiveidea/delayed_job#rails-3-mailers). - # Thus, we need to return a message object here, otherwise 'Delayed Job' - # will complain about an object without a method #deliver. - allow(ProjectMailer).to receive(:copy_project_failed).and_return(maildouble) + CopyProjectJob.new end it 'sets locale correctly' do - expect(copy_job).to receive(:create_project_copy) do |*_args| + expect(copy_job) + .to receive(:create_project_copy) + .and_wrap_original do |m, *args, &block| expect(I18n.locale).to eq(:de) - [nil, nil] + m.call(*args, &block) end - copy_job.perform + copy_job.perform user_id: user_de.id, + source_project_id: source_project.id, + target_project_params: {}, + associations_to_copy: [] + end end @@ -82,12 +77,15 @@ describe CopyProjectJob, type: :model do is_for_all: true) end let(:copy_job) do - CopyProjectJob.new user_id: admin.id, - source_project_id: source_project.id, - target_project_params: params, - associations_to_copy: [:work_packages] - end # send mails - let(:params) { { name: 'Copy', identifier: 'copy', type_ids: [type.id], work_package_custom_field_ids: [custom_field.id] } } + CopyProjectJob.new.tap do |job| + job.perform user_id: admin.id, + source_project_id: source_project.id, + target_project_params: params, + associations_to_copy: [:work_packages] + end + end + + let(:params) { {name: 'Copy', identifier: 'copy', type_ids: [type.id], work_package_custom_field_ids: [custom_field.id]} } let(:expected_error_message) { "#{WorkPackage.model_name.human} '#{work_package.type.name} #: #{work_package.subject}': #{custom_field.name} #{I18n.t('errors.messages.blank')}." } before do @@ -96,17 +94,8 @@ describe CopyProjectJob, type: :model do allow(User).to receive(:current).and_return(admin) - # 'Delayed Job' uses a work around to get Rails 3 mailers working with it - # (see https://github.com/collectiveidea/delayed_job#rails-3-mailers). - # Thus, we need to return a message object here, otherwise 'Delayed Job' - # will complain about an object without a method #deliver. - allow(ProjectMailer).to receive(:copy_project_succeeded).and_return(maildouble) - - @copied_project, @errors = copy_job.send(:create_project_copy, - source_project, - params, - [:work_packages], # associations - false) + @copied_project = copy_job.target_project + @errors = copy_job.errors end it 'copies the project' do @@ -130,13 +119,15 @@ describe CopyProjectJob, type: :model do project end - let(:copy_job) { - CopyProjectJob.new user_id: admin.id, - source_project_id: source_project.id, - target_project_params: params, - associations_to_copy: [:work_packages] - } # send mails - let(:params) { { name: 'Copy', identifier: 'copy' } } + let(:copy_job) do + CopyProjectJob.new.tap do |job| + job.perform user_id: admin.id, + source_project_id: source_project.id, + target_project_params: params, + associations_to_copy: [:work_packages] + end + end + let(:params) { {name: 'Copy', identifier: 'copy'} } before do allow(User).to receive(:current).and_return(admin) @@ -145,11 +136,8 @@ describe CopyProjectJob, type: :model do it 'saves without the repository' do expect(source_project).not_to be_valid - copied_project, errors = copy_job.send(:create_project_copy, - source_project, - params, - [:work_packages], # associations - false) + copied_project = copy_job.target_project + errors = copy_job.errors expect(errors).to be_empty expect(copied_project).to be_valid @@ -162,12 +150,15 @@ describe CopyProjectJob, type: :model do let(:admin) { FactoryBot.create(:admin) } let(:source_project) { FactoryBot.create(:project) } let(:copy_job) do - CopyProjectJob.new user_id: admin.id, - source_project_id: source_project.id, - target_project_params: params, - associations_to_copy: [:work_packages] - end # send mails - let(:params) { { name: 'Copy', identifier: 'copy' } } + CopyProjectJob.new.tap do |job| + job.perform user_id: admin.id, + source_project_id: source_project.id, + target_project_params: params, + associations_to_copy: [:work_packages] + end + end + + let(:params) { {name: 'Copy', identifier: 'copy'} } before do allow(User).to receive(:current).and_return(admin) @@ -177,20 +168,21 @@ describe CopyProjectJob, type: :model do it 'renders a error when unexpected errors occur' do expect(ProjectMailer) .to receive(:copy_project_failed) - .with(admin, source_project, 'Copy', [I18n.t('copy_project.failed_internal')]) - .and_return maildouble + .with(admin, source_project, 'Copy', [I18n.t('copy_project.failed_internal')]) + .and_return maildouble - expect { copy_job.perform }.not_to raise_error + expect { copy_job }.not_to raise_error end end shared_context 'copy project' do before do - copy_project_job = CopyProjectJob.new(user_id: user.id, - source_project_id: project_to_copy.id, - target_project_params: params, - associations_to_copy: [:members]) - copy_project_job.perform + CopyProjectJob.new.tap do |job| + job.perform user_id: user.id, + source_project_id: project_to_copy.id, + target_project_params: params, + associations_to_copy: [:members] + end end end @@ -201,7 +193,7 @@ describe CopyProjectJob, type: :model do end describe 'subproject' do - let(:params) { { name: 'Copy', identifier: 'copy', parent_id: project.id } } + let(:params) { {name: 'Copy', identifier: 'copy', parent_id: project.id} } let(:subproject) { FactoryBot.create(:project, parent: project) } describe 'invalid parent' do diff --git a/spec/models/journal_notification_mailer_spec.rb b/spec/models/journal_notification_mailer_spec.rb index e604306f2a1..db9ed4d9a17 100644 --- a/spec/models/journal_notification_mailer_spec.rb +++ b/spec/models/journal_notification_mailer_spec.rb @@ -55,20 +55,13 @@ describe JournalNotificationMailer do login_as(user) allow(Setting).to receive(:notified_events).and_return(notifications) - - allow(Delayed::Job).to receive(:enqueue) end shared_examples_for 'enqueues a regular notification' do it do - expect(Delayed::Job).to receive(:enqueue) - .with( - an_instance_of(EnqueueWorkPackageNotificationJob), - run_at: anything, priority: anything) + expect(EnqueueWorkPackageNotificationJob) + .to receive_message_chain(:set, :perform_later) - # immediate delivery is not part of regular notfications, it only covers an edge-case - expect(Delayed::Job).not_to receive(:enqueue) - .with(an_instance_of(DeliverWorkPackageNotificationJob), anything) call_listener end end @@ -94,14 +87,14 @@ describe JournalNotificationMailer do end it 'sends no notification' do - expect(Delayed::Job).not_to receive(:enqueue) + expect(EnqueueWorkPackageNotificationJob).not_to receive(:set) call_listener end end end it 'sends no notification' do - expect(Delayed::Job).not_to receive(:enqueue) + expect(EnqueueWorkPackageNotificationJob).not_to receive(:set) call_listener end end @@ -131,14 +124,14 @@ describe JournalNotificationMailer do let(:journal) { FactoryBot.create(:work_package).journals.first } it 'sends no notification' do - expect(Delayed::Job).not_to receive(:enqueue) + expect(EnqueueWorkPackageNotificationJob).not_to receive(:set) call_listener end end end it 'sends no notification' do - expect(Delayed::Job).not_to receive(:enqueue) + expect(EnqueueWorkPackageNotificationJob).not_to receive(:set) call_listener end end @@ -176,7 +169,7 @@ describe JournalNotificationMailer do let(:send_notification) { false } it 'sends no notification' do - expect(Delayed::Job).not_to receive(:enqueue) + expect(EnqueueWorkPackageNotificationJob).not_to receive(:set) call_listener end end @@ -273,18 +266,12 @@ describe JournalNotificationMailer do end it 'immediately delivers a mail on behalf of Journal 1' do - expect(Delayed::Job).to receive(:enqueue) - .with( - an_instance_of(DeliverWorkPackageNotificationJob), priority: anything) + expect(EnqueueWorkPackageNotificationJob).to receive_message_chain(:set, :perform_later) call_listener end it 'also enqueues a regular mail' do - expect(Delayed::Job).to receive(:enqueue) - .with( - an_instance_of(EnqueueWorkPackageNotificationJob), - priority: anything, - run_at: anything) + expect(EnqueueWorkPackageNotificationJob).to receive_message_chain(:set, :perform_later) call_listener end end @@ -325,7 +312,7 @@ end describe 'initialization' do it 'subscribes the listener' do - expect(JournalNotificationMailer).to receive(:distinguish_journals) + expect(EnqueueWorkPackageNotificationJob).to receive_message_chain(:set, :perform_later) FactoryBot.create(:work_package) end end diff --git a/spec/models/watcher_notification_mailer_spec.rb b/spec/models/watcher_notification_mailer_spec.rb index 89f0a1ef58d..be27098e0f1 100644 --- a/spec/models/watcher_notification_mailer_spec.rb +++ b/spec/models/watcher_notification_mailer_spec.rb @@ -36,8 +36,6 @@ describe WatcherNotificationMailer do before do # make sure no other calls are made due to WP creation/update allow(OpenProject::Notifications).to receive(:send) # ... and do nothing - - allow(Delayed::Job).to receive(:enqueue) end describe 'watcher setup' do @@ -83,7 +81,7 @@ describe WatcherNotificationMailer do let(:self_notified) { true } it 'notifies the watcher' do - expect(Delayed::Job).to receive(:enqueue) + expect(DeliverWatcherNotificationJob).to receive(:perform_later) call_listener(watcher, watcher_setter) end end @@ -93,7 +91,7 @@ describe WatcherNotificationMailer do let(:self_notified) { false } it 'notifies the watcher' do - expect(Delayed::Job).to receive(:enqueue) + expect(DeliverWatcherNotificationJob).to receive(:perform_later) call_listener(watcher, watcher_setter) end end @@ -104,7 +102,7 @@ describe WatcherNotificationMailer do let(:self_notified) { false } it 'does not notify the watcher' do - expect(Delayed::Job).to_not receive(:enqueue) + expect(DeliverWatcherNotificationJob).not_to receive(:perform_later) call_listener(watcher, watcher_setter) end end @@ -115,7 +113,7 @@ describe WatcherNotificationMailer do let(:self_notified) { true } it 'notifies the watcher' do - expect(Delayed::Job).to receive(:enqueue) + expect(DeliverWatcherNotificationJob).to receive(:perform_later) call_listener(watcher, watcher_setter) end end @@ -126,7 +124,7 @@ describe WatcherNotificationMailer do context 'when added by a different user' do it 'does not notify the watcher' do - expect(Delayed::Job).to_not receive(:enqueue) + expect(DeliverWatcherNotificationJob).not_to receive(:perform_later) call_listener(watcher, watcher_setter) end end @@ -136,7 +134,7 @@ describe WatcherNotificationMailer do let(:self_notified) { false } it 'does not notify the watcher' do - expect(Delayed::Job).to_not receive(:enqueue) + expect(DeliverWatcherNotificationJob).not_to receive(:perform_later) call_listener(watcher, watcher_setter) end end diff --git a/spec/requests/api/v3/project_resource_spec.rb b/spec/requests/api/v3/project_resource_spec.rb index a798b585f27..91c08f0370d 100644 --- a/spec/requests/api/v3/project_resource_spec.rb +++ b/spec/requests/api/v3/project_resource_spec.rb @@ -633,6 +633,9 @@ describe 'API v3 Project resource', type: :request, content_type: :json do setup delete path + + # run the deletion job + perform_enqueued_jobs end subject { last_response } diff --git a/spec/requests/api/v3/user/user_resource_spec.rb b/spec/requests/api/v3/user/user_resource_spec.rb index a7d3f2ca5bf..9648c8a0c11 100644 --- a/spec/requests/api/v3/user/user_resource_spec.rb +++ b/spec/requests/api/v3/user/user_resource_spec.rb @@ -64,19 +64,19 @@ describe 'API v3 User resource', type: :request, content_type: :json do it 'contains the user in the response' do expect(subject.body) .to be_json_eql(current_user.name.to_json) - .at_path('_embedded/elements/0/name') + .at_path('_embedded/elements/0/name') end it 'contains the current user in the response' do expect(subject.body) .to be_json_eql(user.name.to_json) - .at_path('_embedded/elements/1/name') + .at_path('_embedded/elements/1/name') end it 'has the users index path for link self href' do expect(subject.body) .to be_json_eql((api_v3_paths.users + '?offset=1&pageSize=30').to_json) - .at_path('_links/self/href') + .at_path('_links/self/href') end context 'if pageSize = 1 and offset = 2' do @@ -85,30 +85,30 @@ describe 'API v3 User resource', type: :request, content_type: :json do it 'contains the current user in the response' do expect(subject.body) .to be_json_eql(user.name.to_json) - .at_path('_embedded/elements/0/name') + .at_path('_embedded/elements/0/name') end end context 'on filtering for name' do let(:get_path) do - filter = [{ 'name' => { + filter = [{'name' => { 'operator' => '~', 'values' => [user.name] - } }] + }}] - "#{api_v3_paths.users}?#{{ filters: filter.to_json }.to_query}" + "#{api_v3_paths.users}?#{{filters: filter.to_json}.to_query}" end it 'contains the filtered user in the response' do expect(subject.body) .to be_json_eql(user.name.to_json) - .at_path('_embedded/elements/0/name') + .at_path('_embedded/elements/0/name') end it 'contains no more users' do expect(subject.body) .to be_json_eql(1.to_json) - .at_path('total') + .at_path('total') end end @@ -120,30 +120,30 @@ describe 'API v3 User resource', type: :request, content_type: :json do let(:get_path) do sort = [['name', 'desc']] - "#{api_v3_paths.users}?#{{ sortBy: sort.to_json }.to_query}" + "#{api_v3_paths.users}?#{{sortBy: sort.to_json}.to_query}" end it 'contains the first user as the first element' do expect(subject.body) .to be_json_eql(users_by_name_order[0].name.to_json) - .at_path('_embedded/elements/0/name') + .at_path('_embedded/elements/0/name') end it 'contains the first user as the second element' do expect(subject.body) .to be_json_eql(users_by_name_order[1].name.to_json) - .at_path('_embedded/elements/1/name') + .at_path('_embedded/elements/1/name') end end context 'on an invalid filter' do let(:get_path) do - filter = [{ 'name' => { + filter = [{'name' => { 'operator' => 'a', 'values' => [user.name] - } }] + }}] - "#{api_v3_paths.users}?#{{ filters: filter.to_json }.to_query}" + "#{api_v3_paths.users}?#{{filters: filter.to_json}.to_query}" end it 'returns an error' do @@ -225,6 +225,7 @@ describe 'API v3 User resource', type: :request, content_type: :json do allow(Setting).to receive(:users_deletable_by_self?).and_return(self_delete) delete path + user.reload end shared_examples 'deletion allowed' do @@ -232,8 +233,12 @@ describe 'API v3 User resource', type: :request, content_type: :json do expect(subject.status).to eq 202 end - it 'should delete the account' do - expect(User.exists?(user.id)).not_to be_truthy + it 'should lock the account and mark for deletion' do + expect(DeleteUserJob) + .to have_been_enqueued + .with(user) + + expect(user).to be_locked end context 'with a non-existent user' do diff --git a/spec/requests/api/v3/work_package_resource_spec.rb b/spec/requests/api/v3/work_package_resource_spec.rb index c08d593884a..8b36fe494d7 100644 --- a/spec/requests/api/v3/work_package_resource_spec.rb +++ b/spec/requests/api/v3/work_package_resource_spec.rb @@ -51,7 +51,7 @@ describe 'API v3 Work package resource', let(:current_user) do user = FactoryBot.create(:user, member_in_project: project, member_through_role: role) - FactoryBot.create(:user_preference, user: user, others: { no_self_notified: false }) + FactoryBot.create(:user_preference, user: user, others: {no_self_notified: false}) user end @@ -59,8 +59,8 @@ describe 'API v3 Work package resource', FactoryBot .create(:user, member_in_project: project, member_through_role: role) .tap do |user| - work_package.add_watcher(user) - end + work_package.add_watcher(user) + end end let(:unauthorize_user) { FactoryBot.create(:user) } let(:type) { FactoryBot.create(:type) } @@ -91,7 +91,7 @@ describe 'API v3 Work package resource', expect(subject.body) .to be_json_eql(api_v3_paths.work_package_schema(project.id, work_package.type.id).to_json) - .at_path('_embedded/schemas/_embedded/elements/0/_links/self/href') + .at_path('_embedded/schemas/_embedded/elements/0/_links/self/href') end context 'user not seeing any work packages' do @@ -133,11 +133,11 @@ describe 'API v3 Work package resource', subject(:parsed_response) { JSON.parse(last_response.body) } let!(:other_wp) do FactoryBot.create(:work_package, project_id: project.id, - status: closed_status) + status: closed_status) end let(:work_package) do FactoryBot.create(:work_package, project_id: project.id, - description: description) + description: description) end let(:description) do <<~DESCRIPTION @@ -259,7 +259,7 @@ describe 'API v3 Work package resource', shared_examples_for 'lock version updated' do it { expect(subject.body).to be_json_eql(work_package.reload.lock_version) - .at_path('lockVersion') + .at_path('lockVersion') } end @@ -269,31 +269,36 @@ describe 'API v3 Work package resource', before(:each) do allow(User).to receive(:current).and_return current_user work_package - ActionMailer::Base.deliveries.clear # throw away mails due to work package creation end include_context 'patch request' - subject { ActionMailer::Base.deliveries } - context 'not set' do let(:params) { update_params } - it { expect(subject.count).to eq(1) } + it { expect(EnqueueWorkPackageNotificationJob).to have_been_enqueued.at_least(1) } end context 'disabled' do let(:patch_path) { "#{api_v3_paths.work_package work_package.id}?notify=false" } let(:params) { update_params } - it { expect(subject).to be_empty } + it do + expect(EnqueueWorkPackageNotificationJob) + .to have_been_enqueued + .at_least(1) + end end context 'enabled' do let(:patch_path) { "#{api_v3_paths.work_package work_package.id}?notify=Something" } let(:params) { update_params } - it { expect(subject.count).to eq(1) } + it do + expect(EnqueueWorkPackageNotificationJob) + .to have_been_enqueued + .at_least(1) + end end end @@ -320,7 +325,7 @@ describe 'API v3 Work package resource', it 'has a readonly error' do expect(response.body) .to be_json_eql('urn:openproject-org:api:v3:errors:PropertyIsReadOnly'.to_json) - .at_path('errorIdentifier') + .at_path('errorIdentifier') end end end @@ -339,7 +344,7 @@ describe 'API v3 Work package resource', context 'w/o value (empty)' do let(:raw) { nil } let(:html) { '' } - let(:params) { valid_params.merge(description: { raw: nil }) } + let(:params) { valid_params.merge(description: {raw: nil}) } include_context 'patch request' @@ -353,7 +358,7 @@ describe 'API v3 Work package resource', let(:html) do '

Some text describing something...

' end - let(:params) { valid_params.merge(description: { raw: raw }) } + let(:params) { valid_params.merge(description: {raw: raw}) } include_context 'patch request' @@ -396,7 +401,7 @@ describe 'API v3 Work package resource', context 'status' do let(:target_status) { FactoryBot.create(:status) } let(:status_link) { api_v3_paths.status target_status.id } - let(:status_parameter) { { _links: { status: { href: status_link } } } } + let(:status_parameter) { {_links: {status: {href: status_link}}} } let(:params) { valid_params.merge(status_parameter) } before { allow(User).to receive(:current).and_return current_user } @@ -416,7 +421,7 @@ describe 'API v3 Work package resource', it 'should respond with updated work package status' do expect(subject.body).to be_json_eql(target_status.name.to_json) - .at_path('_embedded/status/name') + .at_path('_embedded/status/name') end it_behaves_like 'lock version updated' @@ -452,7 +457,7 @@ describe 'API v3 Work package resource', context 'type' do let(:target_type) { FactoryBot.create(:type) } let(:type_link) { api_v3_paths.type target_type.id } - let(:type_parameter) { { _links: { type: { href: type_link } } } } + let(:type_parameter) { {_links: {type: {href: type_link}}} } let(:params) { valid_params.merge(type_parameter) } before { allow(User).to receive(:current).and_return current_user } @@ -468,7 +473,7 @@ describe 'API v3 Work package resource', it 'should respond with updated work package type' do expect(subject.body).to be_json_eql(target_type.name.to_json) - .at_path('_embedded/type/name') + .at_path('_embedded/type/name') end it_behaves_like 'lock version updated' @@ -476,7 +481,7 @@ describe 'API v3 Work package resource', context 'valid type changing custom fields' do let(:custom_field) { FactoryBot.create(:work_package_custom_field) } - let(:custom_field_parameter) { { :"customField#{custom_field.id}" => true } } + let(:custom_field_parameter) { {:"customField#{custom_field.id}" => true} } let(:params) { valid_params.merge(type_parameter).merge(custom_field_parameter) } before do @@ -490,7 +495,7 @@ describe 'API v3 Work package resource', it 'responds with the new custom field having the desired value' do expect(subject.body) .to be_json_eql(true.to_json) - .at_path("customField#{custom_field.id}") + .at_path("customField#{custom_field.id}") end end @@ -523,7 +528,7 @@ describe 'API v3 Work package resource', FactoryBot.create(:project, public: false) end let(:project_link) { api_v3_paths.project target_project.id } - let(:project_parameter) { { _links: { project: { href: project_link } } } } + let(:project_parameter) { {_links: {project: {href: project_link}}} } let(:params) { valid_params.merge(project_parameter) } before do @@ -555,7 +560,7 @@ describe 'API v3 Work package resource', context 'with a custom field defined on the target project' do let(:custom_field) { FactoryBot.create(:work_package_custom_field) } - let(:custom_field_parameter) { { :"customField#{custom_field.id}" => true } } + let(:custom_field_parameter) { {:"customField#{custom_field.id}" => true} } let(:params) { valid_params.merge(project_parameter).merge(custom_field_parameter) } before do @@ -568,7 +573,7 @@ describe 'API v3 Work package resource', it 'responds with the new custom field having the desired value' do expect(subject.body) .to be_json_eql(true.to_json) - .at_path("customField#{custom_field.id}") + .at_path("customField#{custom_field.id}") end end end @@ -603,7 +608,7 @@ describe 'API v3 Work package resource', end shared_examples_for 'handling people' do |property| - let(:user_parameter) { { _links: { property => { href: user_href } } } } + let(:user_parameter) { {_links: {property => {href: user_href}}} } let(:href_path) { "_links/#{property}/href" } describe 'nil' do @@ -626,7 +631,7 @@ describe 'API v3 Work package resource', it { expect(response.body).to be_json_eql(title) - .at_path("_links/#{property}/title") + .at_path("_links/#{property}/title") } it_behaves_like 'lock version updated' @@ -724,7 +729,7 @@ describe 'API v3 Work package resource', context 'version' do let(:target_version) { FactoryBot.create(:version, project: project) } let(:version_link) { api_v3_paths.version target_version.id } - let(:version_parameter) { { _links: { version: { href: version_link } } } } + let(:version_parameter) { {_links: {version: {href: version_link}}} } let(:params) { valid_params.merge(version_parameter) } before { allow(User).to receive(:current).and_return current_user } @@ -736,7 +741,7 @@ describe 'API v3 Work package resource', it 'should respond with the work package assigned to the version' do expect(subject.body).to be_json_eql(target_version.name.to_json) - .at_path('_embedded/version/name') + .at_path('_embedded/version/name') end it_behaves_like 'lock version updated' @@ -752,7 +757,7 @@ describe 'API v3 Work package resource', it 'has a readonly error' do expect(response.body) .to be_json_eql('urn:openproject-org:api:v3:errors:PropertyIsReadOnly'.to_json) - .at_path('errorIdentifier') + .at_path('errorIdentifier') end end end @@ -760,7 +765,7 @@ describe 'API v3 Work package resource', context 'category' do let(:target_category) { FactoryBot.create(:category, project: project) } let(:category_link) { api_v3_paths.category target_category.id } - let(:category_parameter) { { _links: { category: { href: category_link } } } } + let(:category_parameter) { {_links: {category: {href: category_link}}} } let(:params) { valid_params.merge(category_parameter) } before { allow(User).to receive(:current).and_return current_user } @@ -772,7 +777,7 @@ describe 'API v3 Work package resource', it 'should respond with the work package assigned to the category' do expect(subject.body).to be_json_eql(target_category.name.to_json) - .at_path('_embedded/category/name') + .at_path('_embedded/category/name') end it_behaves_like 'lock version updated' @@ -782,7 +787,7 @@ describe 'API v3 Work package resource', context 'priority' do let(:target_priority) { FactoryBot.create(:priority) } let(:priority_link) { api_v3_paths.priority target_priority.id } - let(:priority_parameter) { { _links: { priority: { href: priority_link } } } } + let(:priority_parameter) { {_links: {priority: {href: priority_link}}} } let(:params) { valid_params.merge(priority_parameter) } before { allow(User).to receive(:current).and_return current_user } @@ -794,7 +799,7 @@ describe 'API v3 Work package resource', it 'should respond with the work package assigned to the priority' do expect(subject.body).to be_json_eql(target_priority.name.to_json) - .at_path('_embedded/priority/name') + .at_path('_embedded/priority/name') end it_behaves_like 'lock version updated' @@ -813,7 +818,7 @@ describe 'API v3 Work package resource', end let(:value_parameter) do - { _links: { custom_field.accessor_name.camelize(:lower) => { href: value_link } } } + {_links: {custom_field.accessor_name.camelize(:lower) => {href: value_link}}} end let(:params) { valid_params.merge(value_parameter) } @@ -830,7 +835,7 @@ describe 'API v3 Work package resource', it 'should respond with the work package assigned to the new value' do expect(subject.body).to be_json_eql(value_link.to_json) - .at_path("_links/#{custom_field.accessor_name.camelize(:lower)}/href") + .at_path("_links/#{custom_field.accessor_name.camelize(:lower)}/href") end it_behaves_like 'lock version updated' @@ -1040,7 +1045,7 @@ describe 'API v3 Work package resource', status.save! priority.save! - FactoryBot.create(:user_preference, user: current_user, others: { no_self_notified: false }) + FactoryBot.create(:user_preference, user: current_user, others: {no_self_notified: false}) post path, parameters.to_json, 'CONTENT_TYPE' => 'application/json' end @@ -1048,14 +1053,16 @@ describe 'API v3 Work package resource', let(:permissions) { %i[add_work_packages view_project view_work_packages] } it 'sends a mail by default' do - expect(ActionMailer::Base.deliveries.count).to eq(1) + expect(EnqueueWorkPackageNotificationJob) + .to have_been_enqueued + .at_least(1) end context 'without notifications' do let(:path) { "#{api_v3_paths.work_packages}?notify=false" } it 'should not send a mail' do - expect(ActionMailer::Base.deliveries.count).to eq(0) + expect(EnqueueWorkPackageNotificationJob).not_to have_been_enqueued end end @@ -1063,7 +1070,9 @@ describe 'API v3 Work package resource', let(:path) { "#{api_v3_paths.work_packages}?notify=true" } it 'should send a mail' do - expect(ActionMailer::Base.deliveries.count).to eq(1) + expect(EnqueueWorkPackageNotificationJob) + .to have_been_enqueued + .at_least(1) end end end diff --git a/spec/requests/api/v3/work_packages/work_packages_by_project_resource_spec.rb b/spec/requests/api/v3/work_packages/work_packages_by_project_resource_spec.rb index f881e267f09..e7137d89390 100644 --- a/spec/requests/api/v3/work_packages/work_packages_by_project_resource_spec.rb +++ b/spec/requests/api/v3/work_packages/work_packages_by_project_resource_spec.rb @@ -301,14 +301,14 @@ describe API::V3::WorkPackages::WorkPackagesByProjectAPI, type: :request do let(:permissions) { [:add_work_packages, :view_project, :view_work_packages] } it 'sends a mail by default' do - expect(ActionMailer::Base.deliveries.count).to eq(1) + expect(EnqueueWorkPackageNotificationJob).to have_been_enqueued end context 'without notifications' do let(:path) { "#{api_v3_paths.work_packages_by_project(project.id)}?notify=false" } it 'should not send a mail' do - expect(ActionMailer::Base.deliveries.count).to eq(0) + expect(EnqueueWorkPackageNotificationJob).not_to have_been_enqueued end end @@ -316,7 +316,7 @@ describe API::V3::WorkPackages::WorkPackagesByProjectAPI, type: :request do let(:path) { "#{api_v3_paths.work_packages_by_project(project.id)}?notify=true" } it 'should send a mail' do - expect(ActionMailer::Base.deliveries.count).to eq(1) + expect(EnqueueWorkPackageNotificationJob).to have_been_enqueued end end end diff --git a/spec/services/projects/schedule_deletion_service_spec.rb b/spec/services/projects/schedule_deletion_service_spec.rb index eacc00ecc99..8009f59a922 100644 --- a/spec/services/projects/schedule_deletion_service_spec.rb +++ b/spec/services/projects/schedule_deletion_service_spec.rb @@ -89,16 +89,9 @@ describe ::Projects::ScheduleDeletionService, type: :model do .to receive(:call) .and_return(archive_result) - job = double 'job' - expect(::Projects::DeleteProjectJob) - .to receive(:new) + .to receive(:perform_later) .with(user_id: user.id, project_id: project.id) - .and_return(job) - - expect(Delayed::Job) - .to receive(:enqueue) - .with(job, anything) expect(subject).to be_success end diff --git a/spec/services/scm/create_managed_repository_service_spec.rb b/spec/services/scm/create_managed_repository_service_spec.rb index 9c1e23d9a8f..4bb95fa3754 100644 --- a/spec/services/scm/create_managed_repository_service_spec.rb +++ b/spec/services/scm/create_managed_repository_service_spec.rb @@ -82,6 +82,10 @@ describe Scm::CreateManagedRepositoryService do repo = Repository::Subversion.new(scm_type: :managed) repo.project = project repo.configure(:managed, nil) + + # Ignore default creation + allow(repo).to receive(:create_managed_repository) + repo.save! repo } @@ -201,14 +205,16 @@ describe Scm::CreateManagedRepositoryService do } } - let(:job) { Scm::CreateRemoteRepositoryJob.new(repository, perform_now: true) } + let(:instance) { Scm::CreateRemoteRepositoryJob.new } + let(:job_call) { instance.perform(repository) } context 'with insecure option' do let(:insecure) { true } it_behaves_like 'calls the callback' it 'uses the insecure option' do - expect(job.send(:configured_verification)).to eq(OpenSSL::SSL::VERIFY_NONE) + job_call + expect(instance.send(:configured_verification)).to eq(OpenSSL::SSL::VERIFY_NONE) end end @@ -216,7 +222,8 @@ describe Scm::CreateManagedRepositoryService do let(:insecure) { false } it 'uses the insecure option' do - expect(job.send(:configured_verification)).to eq(OpenSSL::SSL::VERIFY_PEER) + job_call + expect(instance.send(:configured_verification)).to eq(OpenSSL::SSL::VERIFY_PEER) end end end diff --git a/spec/services/scm/delete_managed_repository_service_spec.rb b/spec/services/scm/delete_managed_repository_service_spec.rb index 4c5723cabaa..1e9151793e1 100644 --- a/spec/services/scm/delete_managed_repository_service_spec.rb +++ b/spec/services/scm/delete_managed_repository_service_spec.rb @@ -136,8 +136,9 @@ describe Scm::DeleteManagedRepositoryService do end it 'calls the callback' do - expect(Scm::DeleteRemoteRepositoryJob) - .to receive(:new).and_call_original + expect(::Scm::DeleteRemoteRepositoryJob) + .to receive(:perform_now) + .and_call_original expect(service.call).to be true expect(WebMock) @@ -154,8 +155,10 @@ describe Scm::DeleteManagedRepositoryService do end it 'calls the callback' do - expect(Scm::DeleteRemoteRepositoryJob) - .to receive(:new).and_call_original + expect(::Scm::DeleteRemoteRepositoryJob) + .to receive(:perform_now) + .and_call_original + expect(service.call).to be false diff --git a/spec/support/scm/countable_repository.rb b/spec/support/scm/countable_repository.rb index f4c8987ac4b..d0a1229bbd8 100644 --- a/spec/support/scm/countable_repository.rb +++ b/spec/support/scm/countable_repository.rb @@ -1,10 +1,9 @@ require 'open3' shared_examples_for 'is a countable repository' do - let(:job) { ::Scm::StorageUpdaterJob.new repository } + let(:job_call) { ::Scm::StorageUpdaterJob.perform_now repository } let(:cache_time) { 720 } before do - allow(::Scm::StorageUpdaterJob).to receive(:new).and_return(job) allow(Repository).to receive(:find).and_return(repository) allow(Setting).to receive(:repository_storage_cache_minutes).and_return(cache_time) end @@ -12,17 +11,6 @@ shared_examples_for 'is a countable repository' do expect(repository.scm).to be_storage_available end - context 'with vanished repository' do - before do - allow(Repository).to receive(:find).and_raise(ActiveRecord::RecordNotFound) - end - - it 'does not raise' do - expect(Rails.logger).to receive(:warn).with(/StorageUpdater requested for Repository/) - expect { job.perform }.not_to raise_error - end - end - context 'with patched counter' do let(:count) { 1234 } @@ -98,7 +86,7 @@ shared_examples_for 'is not a countable repository' do end it 'does not return or update the count' do - expect(::Scm::StorageUpdaterJob).not_to receive(:new) + expect(::Scm::StorageUpdaterJob).not_to receive(:perform_later) expect(repository.update_required_storage).to be false end end diff --git a/spec/support/scm/relocate_repository.rb b/spec/support/scm/relocate_repository.rb index 8585da62d38..5a59397c5cc 100644 --- a/spec/support/scm/relocate_repository.rb +++ b/spec/support/scm/relocate_repository.rb @@ -1,5 +1,5 @@ shared_examples_for 'repository can be relocated' do |vendor| - let(:job) { ::Scm::RelocateRepositoryJob.new repository } + let(:job_call) { ::Scm::RelocateRepositoryJob.perform_now repository } let(:project) { FactoryBot.build :project } let(:repository) { repo = FactoryBot.build("repository_#{vendor}".to_sym, @@ -13,7 +13,6 @@ shared_examples_for 'repository can be relocated' do |vendor| } before do - allow(::Scm::RelocateRepositoryJob).to receive(:new).and_return(job) allow(Repository).to receive(:find).and_return(repository) end @@ -30,7 +29,7 @@ shared_examples_for 'repository can be relocated' do |vendor| project.update!(identifier: 'somenewidentifier') repository.reload - job.perform + job_call # Confirm that all paths are updated expect(current_path).not_to eq(repository.managed_repository_path) @@ -65,7 +64,8 @@ shared_examples_for 'repository can be relocated' do |vendor| # Rename the project project.identifier = 'somenewidentifier' - job.perform + + job_call expect(WebMock) .to have_requested(:post, url) diff --git a/spec/workers/mail_notification_jobs/deliver_watcher_notification_job_spec.rb b/spec/workers/mail_notification_jobs/deliver_watcher_notification_job_spec.rb index ab84e036397..669a1f00eae 100644 --- a/spec/workers/mail_notification_jobs/deliver_watcher_notification_job_spec.rb +++ b/spec/workers/mail_notification_jobs/deliver_watcher_notification_job_spec.rb @@ -39,7 +39,7 @@ describe DeliverWatcherNotificationJob, type: :model do let(:work_package) { FactoryBot.build(:work_package, project: project) } let(:watcher) { FactoryBot.create(:watcher, watchable: work_package, user: watcher_user) } - subject { described_class.new(watcher.id, watcher_user.id, watcher_setter.id) } + subject { described_class.new.perform(watcher.id, watcher_user.id, watcher_setter.id) } before do # make sure no actual calls make it into the UserMailer @@ -51,7 +51,7 @@ describe DeliverWatcherNotificationJob, type: :model do expect(UserMailer).to receive(:work_package_watcher_added).with(work_package, watcher_user, watcher_setter) - subject.perform + subject end describe 'exceptions' do @@ -63,7 +63,7 @@ describe DeliverWatcherNotificationJob, type: :model do end it 'raises the error' do - expect { subject.perform }.to raise_error(SocketError) + expect { subject }.to raise_error(SocketError) end end end diff --git a/spec/workers/mail_notification_jobs/deliver_work_package_notification_job_spec.rb b/spec/workers/mail_notification_jobs/deliver_work_package_notification_job_spec.rb index 95cc4945ac2..8a8e286033f 100644 --- a/spec/workers/mail_notification_jobs/deliver_work_package_notification_job_spec.rb +++ b/spec/workers/mail_notification_jobs/deliver_work_package_notification_job_spec.rb @@ -42,7 +42,8 @@ describe DeliverWorkPackageNotificationJob, type: :model do author: author) } let(:journal) { work_package.journals.first } - subject { described_class.new(journal.id, recipient.id, author.id) } + let(:instance) { described_class.new } + subject { instance.perform(journal.id, recipient.id, author.id) } before do # make sure no actual calls make it into the UserMailer @@ -55,7 +56,7 @@ describe DeliverWorkPackageNotificationJob, type: :model do recipient, an_instance_of(Journal::AggregatedJournal), author) - subject.perform + subject end context 'non-existant journal' do @@ -65,7 +66,7 @@ describe DeliverWorkPackageNotificationJob, type: :model do it 'sends no mail' do expect(UserMailer).not_to receive(:work_package_added) - subject.perform + subject end end @@ -76,14 +77,14 @@ describe DeliverWorkPackageNotificationJob, type: :model do it 'sends a mail' do expect(UserMailer).to receive(:work_package_added) - subject.perform + subject end it 'uses the deleted user as author' do expect(UserMailer).to receive(:work_package_added) .with(anything, anything, DeletedUser.first) - subject.perform + subject end end @@ -95,7 +96,7 @@ describe DeliverWorkPackageNotificationJob, type: :model do end it 'raises no observable error' do - expect { subject.perform }.not_to raise_error + expect { subject }.not_to raise_error end end @@ -109,7 +110,7 @@ describe DeliverWorkPackageNotificationJob, type: :model do it 'sends an update mail' do expect(UserMailer).to receive(:work_package_updated) - subject.perform + subject end it 'sends a mail for the aggregated journal' do @@ -120,7 +121,7 @@ describe DeliverWorkPackageNotificationJob, type: :model do double('mail', deliver_now: nil) end - subject.perform + subject end end @@ -133,7 +134,7 @@ describe DeliverWorkPackageNotificationJob, type: :model do end end - it { subject.perform } + it { subject } end context 'for a known current user' do @@ -141,7 +142,7 @@ describe DeliverWorkPackageNotificationJob, type: :model do it 'resets to the previous current user after running' do User.current = current_user - subject.perform + subject expect(User.current).to eql(current_user) end end @@ -155,7 +156,7 @@ describe DeliverWorkPackageNotificationJob, type: :model do end it 'raises the error' do - expect { subject.perform }.to raise_error(SocketError) + expect { subject }.to raise_error(SocketError) end end @@ -165,7 +166,7 @@ describe DeliverWorkPackageNotificationJob, type: :model do end it 'swallows the error' do - expect { subject.perform }.not_to raise_error + expect { subject }.not_to raise_error end end end diff --git a/spec/workers/mail_notification_jobs/enqueue_work_package_notification_job_spec.rb b/spec/workers/mail_notification_jobs/enqueue_work_package_notification_job_spec.rb index 077a8e1c5c0..ff71ce0a06d 100644 --- a/spec/workers/mail_notification_jobs/enqueue_work_package_notification_job_spec.rb +++ b/spec/workers/mail_notification_jobs/enqueue_work_package_notification_job_spec.rb @@ -38,12 +38,13 @@ describe EnqueueWorkPackageNotificationJob, type: :model do let(:author) { FactoryBot.create(:user, login: "marktwain") } let(:work_package) do FactoryBot.create(:work_package, - project: project, - author: author, - assigned_to: recipient) + project: project, + author: author, + assigned_to: recipient) end let(:journal) { work_package.journals.first } - subject { described_class.new(journal.id, author.id) } + let(:instance) { described_class.new } + subject { instance.perform(journal.id, author.id) } before do # make sure no other calls are made due to WP creation/update @@ -51,8 +52,8 @@ describe EnqueueWorkPackageNotificationJob, type: :model do end it 'sends a mail' do - expect(Delayed::Job).to receive(:enqueue).with(an_instance_of(DeliverWorkPackageNotificationJob), any_args) - subject.perform + expect(DeliverNotificationJob).to receive(:perform_later) + subject end context 'non-existant journal' do @@ -61,8 +62,8 @@ describe EnqueueWorkPackageNotificationJob, type: :model do end it 'sends no mail' do - expect(Delayed::Job).not_to receive(:enqueue) - subject.perform + expect(DeliverNotificationJob).not_to receive(:perform_later) + subject end end @@ -72,10 +73,8 @@ describe EnqueueWorkPackageNotificationJob, type: :model do end it 'sends a mail' do - expect(Delayed::Job) - .to receive(:enqueue) - .with(an_instance_of(DeliverWorkPackageNotificationJob), any_args) - subject.perform + expect(DeliverNotificationJob).to receive(:perform_later) + subject end end @@ -87,8 +86,8 @@ describe EnqueueWorkPackageNotificationJob, type: :model do end it 'does not send any mails' do - expect(Delayed::Job).not_to receive(:enqueue) - subject.perform + expect(DeliverNotificationJob).not_to receive(:perform_later) + subject end end @@ -112,8 +111,8 @@ describe EnqueueWorkPackageNotificationJob, type: :model do # This is important since late exec of a Job might cause it to _not_ skip notifications before do - change = { subject: 'new subject' } - note = { journal_notes: 'a comment' } + change = {subject: 'new subject'} + note = {journal_notes: 'a comment'} allow(WorkPackages::UpdateContract).to receive(:new).and_return(NoopContract.new) service = WorkPackages::UpdateService.new(user: author, model: work_package) @@ -134,24 +133,18 @@ describe EnqueueWorkPackageNotificationJob, type: :model do # -> no special behaviour required it 'Job 1 sends one mail for journal 1' do - expect(Delayed::Job) - .to receive(:enqueue) - .with(an_instance_of(DeliverWorkPackageNotificationJob), any_args) - .once - described_class.new(journal_1.id, author.id).perform + expect(DeliverNotificationJob).to receive(:perform_later).once + instance.perform(journal_1.id, author.id) end it 'Job 2 sends no mails' do - expect(Delayed::Job).not_to receive(:enqueue) - described_class.new(journal_2.id, author.id).perform + expect(DeliverNotificationJob).not_to receive(:perform_later) + instance.perform(journal_2.id, author.id) end it 'Job 3 sends one mail for journal (2,3)' do - expect(Delayed::Job) - .to receive(:enqueue) - .with(an_instance_of(DeliverWorkPackageNotificationJob), any_args) - .once - described_class.new(journal_3.id, author.id).perform + expect(DeliverNotificationJob).to receive(:perform_later).once + instance.perform(journal_3.id, author.id) end end @@ -170,21 +163,18 @@ describe EnqueueWorkPackageNotificationJob, type: :model do end it 'Job 1 sends no mails' do - expect(Delayed::Job).not_to receive(:enqueue) - described_class.new(journal_1.id, author.id).perform + expect(DeliverNotificationJob).not_to receive(:perform_later) + instance.perform(journal_1.id, author.id) end it 'Job 2 sends no mails' do - expect(Delayed::Job).not_to receive(:enqueue) - described_class.new(journal_2.id, author.id).perform + expect(DeliverNotificationJob).not_to receive(:perform_later) + instance.perform(journal_2.id, author.id) end it 'Job 3 sends one mail for (2,3)' do - expect(Delayed::Job) - .to receive(:enqueue) - .with(an_instance_of(DeliverWorkPackageNotificationJob), any_args) - .once - described_class.new(journal_3.id, author.id).perform + expect(DeliverNotificationJob).to receive(:perform_later).once + instance.perform(journal_3.id, author.id) end end @@ -199,30 +189,26 @@ describe EnqueueWorkPackageNotificationJob, type: :model do end it 'Job 1 sends no mails' do - expect(Delayed::Job).not_to receive(:enqueue) - described_class.new(journal_1.id, author.id).perform + expect(DeliverNotificationJob).not_to receive(:perform_later) + instance.perform(journal_1.id, author.id) end it 'Job 2 sends one mail for journal (1, 2)' do - expect(Delayed::Job).to receive(:enqueue) - .with(an_instance_of(DeliverWorkPackageNotificationJob), any_args) - .once - described_class.new(journal_2.id, author.id).perform + expect(DeliverNotificationJob).to receive(:perform_later).once + instance.perform(journal_2.id, author.id) end it 'Job 3 sends one mail for journal 3' do - expect(Delayed::Job).to receive(:enqueue) - .with(an_instance_of(DeliverWorkPackageNotificationJob), any_args) - .once - described_class.new(journal_3.id, author.id).perform + expect(DeliverNotificationJob).to receive(:perform_later).once + instance.perform(journal_3.id, author.id) end end end describe "#text_for_mentions" do it "returns a text" do - subject.perform - expect(subject.send(:text_for_mentions)).to be_a String + subject + expect(instance.send(:text_for_mentions)).to be_a String end context "subject and description changed" do @@ -230,7 +216,7 @@ describe EnqueueWorkPackageNotificationJob, type: :model do let(:description) { 'New description' } let(:notes) { 'Nice notes!' } - subject { described_class.new(work_package.journals.last.id, author.id) } + subject { instance.perform(work_package.journals.last.id, author.id) } before do work_package.subject = title @@ -240,26 +226,25 @@ describe EnqueueWorkPackageNotificationJob, type: :model do work_package.journals.last.notes = notes work_package.journals.last.save - subject.perform + subject end it "returns notes, subject and description" do - expect(subject.send(:text_for_mentions)).not_to be_blank - expect(subject.send(:text_for_mentions)).to match title - expect(subject.send(:text_for_mentions)).to match description - expect(subject.send(:text_for_mentions)).to match notes + expect(instance.send(:text_for_mentions)).not_to be_blank + expect(instance.send(:text_for_mentions)).to match title + expect(instance.send(:text_for_mentions)).to match description + expect(instance.send(:text_for_mentions)).to match notes end end end describe "#mentioned" do subject do - instance = described_class.new(journal.id, author.id) - instance.perform + instance.perform(journal.id, author.id) allow(instance) .to receive(:text_for_mentions) - .and_return(added_text) + .and_return(added_text) instance.send(:mentioned) end @@ -279,9 +264,9 @@ describe EnqueueWorkPackageNotificationJob, type: :model do context "that is an email address" do let(:recipient) do FactoryBot.create(:user, - member_in_project: project, - member_through_role: role, - login: "foo@bar.com") + member_in_project: project, + member_through_role: role, + login: "foo@bar.com") end it "detects the user" do @@ -303,9 +288,9 @@ describe EnqueueWorkPackageNotificationJob, type: :model do context "the recipient turned off all mail notifications" do let(:recipient) do FactoryBot.create(:user, - member_in_project: project, - member_through_role: role, - mail_notification: 'none') + member_in_project: project, + member_through_role: role, + mail_notification: 'none') end let(:added_text) do @@ -322,7 +307,7 @@ describe EnqueueWorkPackageNotificationJob, type: :model do context "mentioned user is not allowed to view the work package" do let(:recipient) do FactoryBot.create(:user, - login: "foo@bar.com") + login: "foo@bar.com") end let(:added_text) do "Hello user:#{recipient.login}, hey user##{recipient.id}" @@ -343,9 +328,9 @@ describe EnqueueWorkPackageNotificationJob, type: :model do group.users << group_member FactoryBot.create(:member, - project: project, - principal: group, - roles: [role]) + project: project, + principal: group, + roles: [role]) end end @@ -382,9 +367,9 @@ describe EnqueueWorkPackageNotificationJob, type: :model do context 'but group member is allowed individually' do before do FactoryBot.create(:member, - project: project, - principal: group_member, - roles: [FactoryBot.create(:role, permissions: [:view_work_packages])]) + project: project, + principal: group_member, + roles: [FactoryBot.create(:role, permissions: [:view_work_packages])]) end it "group member gets detected" do diff --git a/spec/workers/scm/create_local_repository_job_spec.rb b/spec/workers/scm/create_local_repository_job_spec.rb index a887dedd0e2..7ae14157c7e 100644 --- a/spec/workers/scm/create_local_repository_job_spec.rb +++ b/spec/workers/scm/create_local_repository_job_spec.rb @@ -30,7 +30,8 @@ require 'spec_helper' describe Scm::CreateLocalRepositoryJob do - subject { described_class.new(repository) } + let(:instance) { described_class.new } + subject { instance.perform(repository) } # Allow to override configuration values to determine # whether to activate managed repositories @@ -61,7 +62,7 @@ describe Scm::CreateLocalRepositoryJob do shared_examples 'creates a directory with mode' do |expected| it 'creates the directory' do - subject.perform + subject expect(Dir.exists?(repository.root_url)).to be true file_mode = File.stat(repository.root_url).mode @@ -73,8 +74,8 @@ describe Scm::CreateLocalRepositoryJob do let(:mode) { 0770 } it 'uses the correct mode' do - expect(subject).to receive(:create).with(mode) - subject.perform + expect(instance).to receive(:create).with(mode) + subject end it_behaves_like 'creates a directory with mode', '0770' @@ -83,8 +84,8 @@ describe Scm::CreateLocalRepositoryJob do context 'with string mode' do let(:mode) { '0770' } it 'uses the correct mode' do - expect(subject).to receive(:create).with(0770) - subject.perform + expect(instance).to receive(:create).with(0770) + subject end it_behaves_like 'creates a directory with mode', '0770' @@ -93,8 +94,8 @@ describe Scm::CreateLocalRepositoryJob do context 'with no mode set' do let(:mode) { nil } it 'uses the default mode' do - expect(subject).to receive(:create).with(0700) - subject.perform + expect(instance).to receive(:create).with(0700) + subject end it_behaves_like 'creates a directory with mode', '0700'