From ed2881f594c4ff415ddb21dd156b9b0f84496476 Mon Sep 17 00:00:00 2001 From: as-op Date: Thu, 7 May 2026 14:42:25 +0200 Subject: [PATCH] [#74746] Avoid additional journal background jobs to be started by Jira import job https://community.openproject.org/wp/74746 --- app/models/journal/event_configuration.rb | 91 +++++++++ app/services/journals/update_service.rb | 3 +- .../jira_fetch_and_import_projects_job.rb | 8 +- .../import/jira_finalize_import_job.rb | 12 +- config/initializers/subscribe_listeners.rb | 4 + .../lib/acts/journalized/save_hooks.rb | 3 +- .../journal/event_configuration_spec.rb | 178 ++++++++++++++++++ 7 files changed, 289 insertions(+), 10 deletions(-) create mode 100644 app/models/journal/event_configuration.rb create mode 100644 spec/models/journal/event_configuration_spec.rb diff --git a/app/models/journal/event_configuration.rb b/app/models/journal/event_configuration.rb new file mode 100644 index 00000000000..e2ef6505e63 --- /dev/null +++ b/app/models/journal/event_configuration.rb @@ -0,0 +1,91 @@ +# frozen_string_literal: true + +#-- copyright +# OpenProject is an open source project management software. +# Copyright (C) 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. +#++ + +class Journal::EventConfiguration + class << self + DEFAULT = true + + # Allows controlling whether event callbacks (workflows, aggregation jobs) are triggered + # for created/updated journals. After the block is executed, the setting is returned to + # its original state which is true by default. + # In case the method is called multiple times within itself, the first setting prevails. + def with(trigger_callbacks, &) + if trigger_callbacks.nil? + yield + elsif already_set? + log_warning(trigger_callbacks) + yield + else + with_first(trigger_callbacks, &) + end + end + + def active? + @active ||= Concurrent::ThreadLocalVar.new(DEFAULT) + @active.value + end + + protected + + def with_first(trigger_callbacks) + old_value = active? + self.already_set = true + + self.active = trigger_callbacks + + yield + ensure + self.active = old_value + self.already_set = false + end + + def log_warning(trigger_callbacks) + return if active? == trigger_callbacks + + message = <<~MSG + Ignoring setting journal event callbacks to '#{trigger_callbacks}' as a parent block already set it to #{active?} + MSG + Rails.logger.debug message.strip + end + + def active=(value) + @active.value = value + end + + def already_set? + @already_set ||= Concurrent::ThreadLocalVar.new(false) + @already_set.value + end + + def already_set=(value) + @already_set.value = value + end + end +end diff --git a/app/services/journals/update_service.rb b/app/services/journals/update_service.rb index f4060a9ae0c..ac3e5365c1a 100644 --- a/app/services/journals/update_service.rb +++ b/app/services/journals/update_service.rb @@ -40,7 +40,8 @@ module Journals OpenProject::Notifications.send(OpenProject::Events::JOURNAL_UPDATED, journal: call.result, - send_notification: Journal::NotificationConfiguration.active?) + send_notification: Journal::NotificationConfiguration.active?, + trigger_callbacks: Journal::EventConfiguration.active?) call end diff --git a/app/workers/import/jira_fetch_and_import_projects_job.rb b/app/workers/import/jira_fetch_and_import_projects_job.rb index 15767340bfc..fe458905704 100644 --- a/app/workers/import/jira_fetch_and_import_projects_job.rb +++ b/app/workers/import/jira_fetch_and_import_projects_job.rb @@ -23,7 +23,7 @@ # # 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. +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. # # See COPYRIGHT and LICENSE files for more details. #++ @@ -39,8 +39,10 @@ module Import fetch_and_save_users_data(jira_import) Journal::NotificationConfiguration.with(false) do - jira_import.import_users - Import::JiraImportProjectsJob.perform_now(jira_import_id) + Journal::EventConfiguration.with(false) do + jira_import.import_users + Import::JiraImportProjectsJob.perform_now(jira_import_id) + end end jira_import.transition_to!(:imported) diff --git a/app/workers/import/jira_finalize_import_job.rb b/app/workers/import/jira_finalize_import_job.rb index 7f0e8523118..85c27a0bbf4 100644 --- a/app/workers/import/jira_finalize_import_job.rb +++ b/app/workers/import/jira_finalize_import_job.rb @@ -23,7 +23,7 @@ # # 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. +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. # # See COPYRIGHT and LICENSE files for more details. #++ @@ -56,10 +56,12 @@ module Import op_user = ref.op_leg Journal::NotificationConfiguration.with(false) do - Users::UpdateService - .new(model: op_user, user: User.system, contract_class: Users::JiraImportUpdateContract) - .call(status: :active) - .on_failure { |result| raise result.message } + Journal::EventConfiguration.with(false) do + Users::UpdateService + .new(model: op_user, user: User.system, contract_class: Users::JiraImportUpdateContract) + .call(status: :active) + .on_failure { |result| raise result.message } + end end end end diff --git a/config/initializers/subscribe_listeners.rb b/config/initializers/subscribe_listeners.rb index 5d087000ca6..ab907b2238b 100644 --- a/config/initializers/subscribe_listeners.rb +++ b/config/initializers/subscribe_listeners.rb @@ -30,6 +30,8 @@ Rails.application.config.after_initialize do OpenProject::Notifications.subscribe(OpenProject::Events::JOURNAL_CREATED) do |payload| + next unless payload[:trigger_callbacks] + journal = payload[:journal] send_notifications = payload[:send_notification] @@ -52,6 +54,8 @@ Rails.application.config.after_initialize do end OpenProject::Notifications.subscribe(OpenProject::Events::JOURNAL_UPDATED) do |payload| + next unless payload[:trigger_callbacks] + # A job is scheduled immediately that creates notifications (in-app if # supported) right away and schedules jobs to be run for mail and digest # mails. diff --git a/lib_static/plugins/acts_as_journalized/lib/acts/journalized/save_hooks.rb b/lib_static/plugins/acts_as_journalized/lib/acts/journalized/save_hooks.rb index f307230e5b4..abb26fd62db 100644 --- a/lib_static/plugins/acts_as_journalized/lib/acts/journalized/save_hooks.rb +++ b/lib_static/plugins/acts_as_journalized/lib/acts/journalized/save_hooks.rb @@ -69,7 +69,8 @@ module Acts::Journalized OpenProject::Notifications.send(OpenProject::Events::JOURNAL_CREATED, changes: previous_changes, journal: create_call.result, - send_notification: Journal::NotificationConfiguration.active?) + send_notification: Journal::NotificationConfiguration.active?, + trigger_callbacks: Journal::EventConfiguration.active?) end create_call.success? diff --git a/spec/models/journal/event_configuration_spec.rb b/spec/models/journal/event_configuration_spec.rb new file mode 100644 index 00000000000..1b80dbf8af4 --- /dev/null +++ b/spec/models/journal/event_configuration_spec.rb @@ -0,0 +1,178 @@ +# frozen_string_literal: true + +#-- copyright +# OpenProject is an open source project management software. +# Copyright (C) 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. +#++ + +require "spec_helper" + +RSpec.describe Journal::EventConfiguration do + describe ".with" do + let!(:trigger_callbacks_before) { described_class.active? } + let!(:proc_called_counter) { OpenStruct.new called: false, trigger_callbacks: trigger_callbacks_before } # rubocop:disable Style/OpenStructUse + let(:proc) do + Proc.new do + proc_called_counter.called = true + proc_called_counter.trigger_callbacks = described_class.active? + end + end + + it "executes the block" do + described_class.with !trigger_callbacks_before, &proc + + expect(proc_called_counter.called) + .to be_truthy + end + + it "uses the provided trigger_callbacks value within the proc" do + described_class.with !trigger_callbacks_before, &proc + + expect(proc_called_counter.trigger_callbacks) + .to eql !trigger_callbacks_before + end + + it "resets the trigger_callbacks to the value before" do + described_class.with !trigger_callbacks_before, &proc + + expect(described_class.active?) + .to eql trigger_callbacks_before + end + + context "when called with nil" do + it "defaults to true for trigger_callbacks" do + described_class.with nil, &proc + expect(described_class.active?) + .to be(true) + end + end + + context "with nested calls" do + before do + allow(Rails.logger).to receive(:debug) + + described_class.with outer_call_value do + described_class.with inner_call_value, &proc + end + end + + context "when trigger_callbacks value of inner call is different from outer call" do + let(:outer_call_value) { !trigger_callbacks_before } + let(:inner_call_value) { trigger_callbacks_before } + + it "executes the block" do + expect(proc_called_counter.called) + .to be_truthy + end + + it "lets the outer block call dominate further block calls" do + expect(proc_called_counter.trigger_callbacks) + .to eq(outer_call_value) + end + + it "logs a debug message" do + expect(Rails.logger).to have_received(:debug) + .with("Ignoring setting journal event callbacks to '#{inner_call_value}' " \ + "as a parent block already set it to #{outer_call_value}") + end + end + + context "when trigger_callbacks value of inner call is the same as for outer call" do + let(:outer_call_value) { !trigger_callbacks_before } + let(:inner_call_value) { outer_call_value } + + it "does not log any debug messages" do + expect(Rails.logger).not_to have_received(:debug) + end + end + + context "when trigger_callbacks value of inner call is nil" do + let(:outer_call_value) { !trigger_callbacks_before } + let(:inner_call_value) { nil } + + it "executes the block" do + expect(proc_called_counter.called) + .to be_truthy + end + + it "keeps the value of the outer block call" do + expect(proc_called_counter.trigger_callbacks) + .to eq(outer_call_value) + end + + it "does not log any debug messages" do + expect(Rails.logger).not_to have_received(:debug) + end + end + + context "when trigger_callbacks value of outer call is nil" do + let(:outer_call_value) { nil } + let(:inner_call_value) { !trigger_callbacks_before } + + it "executes the block" do + expect(proc_called_counter.called) + .to be_truthy + end + + it "sets the value to the inner block call value" do + expect(proc_called_counter.trigger_callbacks) + .to eq(inner_call_value) + end + + it "does not log any debug messages" do + expect(Rails.logger).not_to have_received(:debug) + end + end + end + + it "is thread safe" do + thread = Thread.new do + described_class.with true do + inner = Thread.new do + described_class.with false do + described_class.active? + end + end + + [described_class.active?, inner.value] + end + end + + expect(thread.value) + .to eql [true, false] + end + + context "with an exception being raised within the block" do + it "raises the exception but always resets the trigger_callbacks value" do + expect { described_class.with(!trigger_callbacks_before) { raise ArgumentError } } + .to raise_error ArgumentError + + expect(described_class.active?) + .to eql trigger_callbacks_before + end + end + end +end