Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Replace manual job checking for GoodJob concurrency checks #14992

Merged
merged 1 commit into from
Mar 18, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 1 addition & 3 deletions app/contracts/settings/working_days_params_contract.rb
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
@@ -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
Loading