diff --git a/app/workers/copy_project_job.rb b/app/workers/copy_project_job.rb index 3b285cf312d7..6dde92dc32ac 100644 --- a/app/workers/copy_project_job.rb +++ b/app/workers/copy_project_job.rb @@ -154,7 +154,7 @@ def copy_project(target_project_params, associations_to_copy, send_notifications end def enqueue_copy_project_folder_jobs(copied_storages, work_packages_map, only) - return unless only.intersect?("file_links", "storage_project_folders").any? + return unless only.intersect?(%w[file_links storage_project_folders]) Array(copied_storages).each do |storage_pair| batch.enqueue do diff --git a/modules/storages/app/models/storages/storage_file_info.rb b/modules/storages/app/models/storages/storage_file_info.rb index 2e0397f24ab3..e34ec0bab9a6 100644 --- a/modules/storages/app/models/storages/storage_file_info.rb +++ b/modules/storages/app/models/storages/storage_file_info.rb @@ -80,6 +80,16 @@ def initialize( ) end + def clean_location + return if location.nil? + + if location.starts_with? "/" + CGI.unescape(location) + else + CGI.unescape("/#{location}") + end + end + def self.from_id(file_id) new(id: file_id, status: "OK", status_code: 200) end diff --git a/modules/storages/app/services/storages/file_links/copy_file_links_service.rb b/modules/storages/app/services/storages/file_links/copy_file_links_service.rb index e282fe9ea356..a7b3cf33b875 100644 --- a/modules/storages/app/services/storages/file_links/copy_file_links_service.rb +++ b/modules/storages/app/services/storages/file_links/copy_file_links_service.rb @@ -45,12 +45,16 @@ def initialize(source:, target:, user:, work_packages_map:) end def call + OpenProject.logger.error "STORAGE CLASS: #{@source.storage.class}, #{@target.storage.class}" + source_file_links = FileLink .includes(:creator) - .where(container_id: @work_packages_map.keys, container_type: "WorkPackage") + .where(storage: @source.storage, + container_id: @work_packages_map.keys, + container_type: "WorkPackage") with_locale_for(@user) do - if @source.project_folder_automatic? + if @source.project_folder_mode == "automatic" create_managed_file_links(source_file_links) else create_unmanaged_file_links(source_file_links) @@ -67,6 +71,8 @@ def create_managed_file_links(source_file_links) ) source_file_links.find_each do |source_link| + next unless location_map.has_key?(source_link.origin_id) + attributes = source_link.dup.attributes attributes.merge!( "creator_id" => @user.id, @@ -80,17 +86,22 @@ def create_managed_file_links(source_file_links) end def build_location_map(source_files, target_location_map) - source_location_map = source_files.filter { |info| info.status == "OK" }.to_h do |info| - [info.id, info.location] + # We need this due to inconsistencies of how we represent the File Path + target_location_map.transform_keys! { |key| key.starts_with?("/") ? key : "/#{key}" } + + # Since right now we can't make the relevant call as a remote admin we need to filter out 403 responses + source_location_map = source_files.filter { |info| info.status_code.to_i == 200 }.to_h do |info| + [info.id.to_s, info.clean_location] end source_location_map.each_with_object({}) do |(id, location), output| target = location.gsub(@source.managed_project_folder_path, @target.managed_project_folder_path) - output[id] = target_location_map[target] + output[id] = target_location_map[target]&.id || id end end + # Known issue, this can lead to 403s. def source_files_info(source_file_links) Peripherals::Registry .resolve("#{@source.storage.short_provider_type}.queries.files_info") @@ -115,7 +126,8 @@ def create_unmanaged_file_links(source_file_links) end def log_errors(failure) - OpenProject.logger.warn failure.errors.inspect + OpenProject.logger.error failure.inspect + OpenProject.logger.error failure.errors.inspect end end end diff --git a/modules/storages/spec/workers/storages/copy_project_folders_job_spec.rb b/modules/storages/spec/workers/storages/copy_project_folders_job_spec.rb index 5f5b5c0962fb..6c48fc5a2447 100644 --- a/modules/storages/spec/workers/storages/copy_project_folders_job_spec.rb +++ b/modules/storages/spec/workers/storages/copy_project_folders_job_spec.rb @@ -56,7 +56,7 @@ let(:target_deep_file_ids) do source_file_links.each_with_object({}) do |fl, hash| - hash["#{target.managed_project_folder_path}#{fl.name}"] = "RANDOM_ID_#{fl.hash}" + hash["#{target.managed_project_folder_path}#{fl.name}"] = Storages::StorageFileInfo.from_id("RANDOM_ID_#{fl.hash}") end end @@ -64,7 +64,7 @@ let(:source_file_infos) do source_file_links.map do |fl| Storages::StorageFileInfo.new( - status: "ok", + status: "OK", status_code: 200, id: fl.origin_id, name: fl.name, @@ -156,7 +156,7 @@ GoodJob.perform_inline Storages::FileLink.where(container: target_work_packages).find_each do |file_link| - expect(file_link.origin_id).to eq(target_deep_file_ids["#{target.managed_project_folder_path}#{file_link.name}"]) + expect(file_link.origin_id).to eq(target_deep_file_ids["/#{target.managed_project_folder_path}#{file_link.name}"].id) end end end diff --git a/spec/services/projects/copy_service_integration_spec.rb b/spec/services/projects/copy_service_integration_spec.rb index ce67f36ea622..ff0f5fc06a14 100644 --- a/spec/services/projects/copy_service_integration_spec.rb +++ b/spec/services/projects/copy_service_integration_spec.rb @@ -102,6 +102,8 @@ boards overview storages + storage_project_folders + file_links ) ) end