Skip to content

Commit

Permalink
Merge pull request #14992 from opf/chore/53122/replace-concurrency-ch…
Browse files Browse the repository at this point in the history
…eck-with-good-job

Replace manual job checking for GoodJob concurrency checks
  • Loading branch information
ba1ash committed Mar 18, 2024
2 parents 024376a + a5bcba9 commit 7b02616
Show file tree
Hide file tree
Showing 5 changed files with 110 additions and 7 deletions.
4 changes: 1 addition & 3 deletions app/contracts/settings/working_days_params_contract.rb
Expand Up @@ -42,9 +42,7 @@ def working_days_are_present
end

def unique_job
if GoodJob::Job
.where(finished_at: nil)
.exists?(job_class: WorkPackages::ApplyWorkingDaysChangeJob.name)
WorkPackages::ApplyWorkingDaysChangeJob.new.check_concurrency do
errors.add :base, :previous_working_day_changes_unprocessed
end
end
Expand Down
42 changes: 42 additions & 0 deletions app/workers/job_concurrency.rb
@@ -0,0 +1,42 @@
#-- copyright
# OpenProject is an open source project management software.
# Copyright (C) 2012-2024 the OpenProject GmbH
#
# This program is free software; you can redistribute it and/or
# modify it under the terms of the GNU General Public License version 3.
#
# OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows:
# Copyright (C) 2006-2013 Jean-Philippe Lang
# Copyright (C) 2010-2013 the ChiliProject Team
#
# This program is free software; you can redistribute it and/or
# modify it under the terms of the GNU General Public License
# as published by the Free Software Foundation; either version 2
# of the License, or (at your option) any later version.
#
# This program is distributed in the hope that it will be useful,
# but WITHOUT ANY WARRANTY; without even the implied warranty of
# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
# GNU General Public License for more details.
#
# You should have received a copy of the GNU General Public License
# along with this program; if not, write to the Free Software
# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
#
# See COPYRIGHT and LICENSE files for more details.
#++

module JobConcurrency
extend ActiveSupport::Concern

included do
include GoodJob::ActiveJobExtensions::Concurrency
end

##
# Run the concurrency check of good_job without actually trying to enqueue it
# Will call the provided block in case the job would be cancelled
def check_concurrency(&block)
good_job_enqueue_concurrency_check(self, on_abort: block, on_enqueue: nil)
end
end
5 changes: 5 additions & 0 deletions app/workers/work_packages/apply_working_days_change_job.rb
Expand Up @@ -27,8 +27,13 @@
#++

class WorkPackages::ApplyWorkingDaysChangeJob < ApplicationJob
include JobConcurrency
queue_with_priority :above_normal

good_job_control_concurrency_with(
total_limit: 1
)

def perform(user_id:, previous_working_days:, previous_non_working_days:)
user = User.find(user_id)

Expand Down
13 changes: 9 additions & 4 deletions spec/contracts/settings/working_days_params_contract_spec.rb
Expand Up @@ -31,8 +31,8 @@

RSpec.describe Settings::WorkingDaysParamsContract do
include_context 'ModelContract shared context'
shared_let(:current_user) { create(:admin) }
let(:setting) { Setting }
let(:current_user) { build_stubbed(:admin) }
let(:params) { { working_days: [1] } }
let(:contract) do
described_class.new(setting, current_user, params:)
Expand All @@ -46,11 +46,16 @@
include_examples 'contract is invalid', base: :working_days_are_missing
end

context 'with an ApplyWorkingDaysChangeJob already existing' do
context 'with an ApplyWorkingDaysChangeJob already existing',
with_good_job: WorkPackages::ApplyWorkingDaysChangeJob do
let(:params) { { working_days: [1, 2, 3] } }

before do
ActiveJob::Base.disable_test_adapter
WorkPackages::ApplyWorkingDaysChangeJob.perform_later
WorkPackages::ApplyWorkingDaysChangeJob
.set(wait: 10.minutes) # GoodJob executes inline job without wait immediately
.perform_later(user_id: current_user.id,
previous_non_working_days: [],
previous_working_days: [1, 2, 3, 4])
end

include_examples 'contract is invalid', base: :previous_working_day_changes_unprocessed
Expand Down
53 changes: 53 additions & 0 deletions spec/support/shared/with_good_job.rb
@@ -0,0 +1,53 @@
#-- copyright
# OpenProject is an open source project management software.
# Copyright (C) 2012-2024 the OpenProject GmbH
#
# This program is free software; you can redistribute it and/or
# modify it under the terms of the GNU General Public License version 3.
#
# OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows:
# Copyright (C) 2006-2013 Jean-Philippe Lang
# Copyright (C) 2010-2013 the ChiliProject Team
#
# This program is free software; you can redistribute it and/or
# modify it under the terms of the GNU General Public License
# as published by the Free Software Foundation; either version 2
# of the License, or (at your option) any later version.
#
# This program is distributed in the hope that it will be useful,
# but WITHOUT ANY WARRANTY; without even the implied warranty of
# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
# GNU General Public License for more details.
#
# You should have received a copy of the GNU General Public License
# along with this program; if not, write to the Free Software
# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
#
# See COPYRIGHT and LICENSE files for more details.
#++

# Disable the test adapter for the given classes
# allowing GoodJob to handle execution and scheduling,
# which in turn allows us to check concurrency controls etc.
RSpec.configure do |config|
config.around(:example, :with_good_job) do |example|
original_adapter = ActiveJob::Base.queue_adapter
good_job_adapter = GoodJob::Adapter.new(execution_mode: :inline)

begin
classes = Array(example.metadata[:with_good_job])
unless classes.all? { |cls| cls <= ApplicationJob }
raise ArgumentError.new("Pass the ApplicationJob subclasses you want to disable the test adapter on.")
end

classes.each(&:disable_test_adapter)
ActiveJob::Base.queue_adapter = good_job_adapter
example.run

ensure
ActiveJob::Base.queue_adapter = original_adapter
classes.each { |cls| cls.enable_test_adapter(original_adapter) }
good_job_adapter&.shutdown
end
end
end

0 comments on commit 7b02616

Please sign in to comment.