From 6850b752b9b2bc5d844125c859574d25ba5c9e68 Mon Sep 17 00:00:00 2001 From: Jan Sandbrink Date: Tue, 17 Dec 2024 15:51:35 +0100 Subject: [PATCH] Refactor ReplaceReferencesService Instead of expecting modules to selectively overwrite methods of this class, we offer an explicit extension point where replacements can be registered explicitly. Thus patching the implementation is not necessary anymore, instead it can be configured from initializers. --- .../principals/replace_references_service.rb | 103 ++++-------------- .../replace_references_service.rb | 54 +++++++++ .../lib/open_project/storages/engine.rb | 10 +- .../replace_references_service_patch.rb | 22 ---- 4 files changed, 81 insertions(+), 108 deletions(-) create mode 100644 config/initializers/replace_references_service.rb delete mode 100644 modules/storages/lib/open_project/storages/patches/replace_references_service_patch.rb diff --git a/app/services/principals/replace_references_service.rb b/app/services/principals/replace_references_service.rb index e1c0810a49c1..71c1b1198fb2 100644 --- a/app/services/principals/replace_references_service.rb +++ b/app/services/principals/replace_references_service.rb @@ -30,8 +30,25 @@ # No data is to be removed. module Principals class ReplaceReferencesService + class << self + attr_reader :replacements, :foreign_keys + + def add_replacement(class_name, attribute) + @replacements ||= {} + @replacements[class_name] ||= Set.new + @replacements[class_name] << attribute + + @foreign_keys ||= Set.new + @foreign_keys << attribute.to_s + end + end + def call(from:, to:) - rewrite_active_models(from, to) + self.class.replacements.each do |class_name, attributes| + klass = class_name.constantize + attributes.each { |attribute| rewrite(klass, attribute, from, to) } + end + rewrite_custom_value(from, to) rewrite_default_journals(from, to) rewrite_customizable_journals(from, to) @@ -41,17 +58,6 @@ def call(from:, to:) private - def rewrite_active_models(from, to) - rewrite_author(from, to) - rewrite_user(from, to) - rewrite_assigned_to(from, to) - rewrite_responsible(from, to) - rewrite_actor(from, to) - rewrite_owner(from, to) - rewrite_logged_by(from, to) - rewrite_presenter(from, to) - end - def rewrite_custom_value(from, to) CustomValue .where(custom_field_id: CustomField.where(field_format: "user")) @@ -61,7 +67,7 @@ def rewrite_custom_value(from, to) def rewrite_default_journals(from, to) journal_classes.each do |klass| - foreign_keys.each do |foreign_key| + self.class.foreign_keys.each do |foreign_key| if klass.column_names.include? foreign_key rewrite(klass, foreign_key, from, to) end @@ -77,81 +83,10 @@ def rewrite_customizable_journals(from, to) .update_all(value: to.id.to_s) end - def rewrite_author(from, to) - [WorkPackage, - Attachment, - WikiPage, - News, - Comment, - Message, - Budget, - MeetingAgenda, - MeetingMinutes, - MeetingAgendaItem].each do |klass| - rewrite(klass, :author_id, from, to) - end - end - - def rewrite_user(from, to) - [TimeEntry, - CostEntry, - ::Query, - Changeset, - CostQuery, - MeetingParticipant].each do |klass| - rewrite(klass, :user_id, from, to) - end - end - - def rewrite_actor(from, to) - [::Notification].each do |klass| - rewrite(klass, :actor_id, from, to) - end - end - - def rewrite_owner(from, to) - [::Doorkeeper::Application].each do |klass| - rewrite(klass, :owner_id, from, to) - end - end - - def rewrite_assigned_to(from, to) - [WorkPackage].each do |klass| - rewrite(klass, :assigned_to_id, from, to) - end - end - - def rewrite_responsible(from, to) - [WorkPackage].each do |klass| - rewrite(klass, :responsible_id, from, to) - end - end - - def rewrite_logged_by(from, to) - [ - TimeEntry, - CostEntry - ].each do |klass| - rewrite(klass, :logged_by_id, from, to) - end - end - - def rewrite_presenter(from, to) - [ - MeetingAgendaItem - ].each do |klass| - rewrite(klass, :presenter_id, from, to) - end - end - def journal_classes [Journal] + Journal::BaseJournal.subclasses end - def foreign_keys - %w[author_id user_id assigned_to_id responsible_id logged_by_id presenter_id] - end - def rewrite(klass, attribute, from, to) klass.where(attribute => from.id).update_all(attribute => to.id) end diff --git a/config/initializers/replace_references_service.rb b/config/initializers/replace_references_service.rb new file mode 100644 index 000000000000..39684a6c2708 --- /dev/null +++ b/config/initializers/replace_references_service.rb @@ -0,0 +1,54 @@ +#-- 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. +#++ + +Rails.application.reloader.to_prepare do + Principals::ReplaceReferencesService.add_replacement("AuthProvider", :creator_id) + Principals::ReplaceReferencesService.add_replacement("Attachment", :author_id) + Principals::ReplaceReferencesService.add_replacement("Budget", :author_id) + Principals::ReplaceReferencesService.add_replacement("Changeset", :user_id) + Principals::ReplaceReferencesService.add_replacement("Comment", :author_id) + Principals::ReplaceReferencesService.add_replacement("CostEntry", :logged_by_id) + Principals::ReplaceReferencesService.add_replacement("CostEntry", :user_id) + Principals::ReplaceReferencesService.add_replacement("CostQuery", :user_id) + Principals::ReplaceReferencesService.add_replacement("::Doorkeeper::Application", :owner_id) + Principals::ReplaceReferencesService.add_replacement("MeetingAgenda", :author_id) + Principals::ReplaceReferencesService.add_replacement("MeetingAgendaItem", :author_id) + Principals::ReplaceReferencesService.add_replacement("MeetingAgendaItem", :presenter_id) + Principals::ReplaceReferencesService.add_replacement("MeetingMinutes", :author_id) + Principals::ReplaceReferencesService.add_replacement("MeetingParticipant", :user_id) + Principals::ReplaceReferencesService.add_replacement("Message", :author_id) + Principals::ReplaceReferencesService.add_replacement("News", :author_id) + Principals::ReplaceReferencesService.add_replacement("::Notification", :actor_id) + Principals::ReplaceReferencesService.add_replacement("::Query", :user_id) + Principals::ReplaceReferencesService.add_replacement("TimeEntry", :logged_by_id) + Principals::ReplaceReferencesService.add_replacement("TimeEntry", :user_id) + Principals::ReplaceReferencesService.add_replacement("WikiPage", :author_id) + Principals::ReplaceReferencesService.add_replacement("WorkPackage", :author_id) + Principals::ReplaceReferencesService.add_replacement("WorkPackage", :assigned_to_id) + Principals::ReplaceReferencesService.add_replacement("WorkPackage", :responsible_id) +end diff --git a/modules/storages/lib/open_project/storages/engine.rb b/modules/storages/lib/open_project/storages/engine.rb index d60979349731..2e38777f9a5d 100644 --- a/modules/storages/lib/open_project/storages/engine.rb +++ b/modules/storages/lib/open_project/storages/engine.rb @@ -50,6 +50,14 @@ def self.external_file_permissions # please see comments inside ActsAsOpEngine class include OpenProject::Plugins::ActsAsOpEngine + initializer "openproject_storages.replace_references" do + Rails.application.reloader.to_prepare do + Principals::ReplaceReferencesService.add_replacement("::Storages::Storage", :creator_id) + Principals::ReplaceReferencesService.add_replacement("::Storages::ProjectStorage", :creator_id) + Principals::ReplaceReferencesService.add_replacement("::Storages::FileLink", :creator_id) + end + end + initializer "openproject_storages.feature_decisions" do OpenProject::FeatureDecisions.add :storage_file_picking_select_all end @@ -240,8 +248,6 @@ def self.external_file_permissions end end - patch_with_namespace :Principals, :ReplaceReferencesService - # This hook is executed when the module is loaded. config.to_prepare do # Allow the browser to connect to external servers for direct file uploads. diff --git a/modules/storages/lib/open_project/storages/patches/replace_references_service_patch.rb b/modules/storages/lib/open_project/storages/patches/replace_references_service_patch.rb deleted file mode 100644 index 2f8e8238d2e5..000000000000 --- a/modules/storages/lib/open_project/storages/patches/replace_references_service_patch.rb +++ /dev/null @@ -1,22 +0,0 @@ -module OpenProject::Storages::Patches::ReplaceReferencesServicePatch - def self.included(base) # :nodoc: - base.prepend InstanceMethods - end - - module InstanceMethods - private - - def rewrite_active_models(from, to) - super - rewrite_creator(from, to) - end - - def rewrite_creator(from, to) - [::Storages::Storage, - ::Storages::ProjectStorage, - ::Storages::FileLink].each do |klass| - klass.where(creator_id: from.id).update_all(creator_id: to.id) - end - end - end -end