From a9c8e9cd3c59cbad77cf39f816181d7f9610c281 Mon Sep 17 00:00:00 2001 From: Rob Kaufman Date: Wed, 13 Nov 2024 21:02:36 -0800 Subject: [PATCH] move valkyrie resource migration code from hyku to hyrax (#6973) * move valkyrie resource migration code from hyku to hyrax * prevent works from becoming inaccessible during valkyrie transition * more cross compatible fix for collection thumbnail issue * make adminset wings identifiable --- .../hyrax/works_controller_behavior.rb | 2 + app/forms/hyrax/forms/pcdm_collection_form.rb | 9 +++ app/jobs/migrate_resources_job.rb | 34 ++++++++++++ app/jobs/valkyrie_create_derivatives_job.rb | 3 +- app/models/admin_set.rb | 1 + app/models/hyrax/resource.rb | 5 ++ app/services/migrate_resource_service.rb | 55 +++++++++++++++++++ lib/freyja/persister.rb | 5 +- .../dashboard/collections_controller_spec.rb | 3 - spec/jobs/migrate_resources_job_spec.rb | 29 ++++++++++ 10 files changed, 140 insertions(+), 6 deletions(-) create mode 100644 app/jobs/migrate_resources_job.rb create mode 100644 app/services/migrate_resource_service.rb create mode 100644 spec/jobs/migrate_resources_job_spec.rb diff --git a/app/controllers/concerns/hyrax/works_controller_behavior.rb b/app/controllers/concerns/hyrax/works_controller_behavior.rb index e65dd1ca32..107acf4cae 100644 --- a/app/controllers/concerns/hyrax/works_controller_behavior.rb +++ b/app/controllers/concerns/hyrax/works_controller_behavior.rb @@ -263,6 +263,8 @@ def hash_key_for_curation_concern def contextual_path(presenter, parent_presenter) ::Hyrax::ContextualPath.new(presenter, parent_presenter).show + rescue NoMethodError + '' end ## diff --git a/app/forms/hyrax/forms/pcdm_collection_form.rb b/app/forms/hyrax/forms/pcdm_collection_form.rb index 32e6cdc0f2..43abbdc33e 100644 --- a/app/forms/hyrax/forms/pcdm_collection_form.rb +++ b/app/forms/hyrax/forms/pcdm_collection_form.rb @@ -81,6 +81,15 @@ def display_additional_fields? secondary_terms.any? end + ## + # This feature is not supported in Valkyrie collections and should be removed as part of #5764 + # However, the depreciated method is still needed for some specs + # @return [] always empty. + def select_files + Deprecation.warn "`Hyrax::PcdmCollection` does not currently support thumbnail_id. Collection thumbnails need to be redesigned as part of issue #5764" + [] + end + private def _form_field_definitions diff --git a/app/jobs/migrate_resources_job.rb b/app/jobs/migrate_resources_job.rb new file mode 100644 index 0000000000..0d4addd576 --- /dev/null +++ b/app/jobs/migrate_resources_job.rb @@ -0,0 +1,34 @@ +# frozen_string_literal: true + +# migrates models from AF to valkyrie +class MigrateResourcesJob < ApplicationJob + attr_writer :errors + # input [Array>>String] Array of ActiveFedora model names to migrate to valkyrie objects + # defaults to AdminSet & Collection models if empty + def perform(ids: [], models: ['AdminSet', 'Collection']) + if ids.blank? + models.each do |model| + model.constantize.find_each do |item| + migrate(item.id) + end + end + else + ids.each do |id| + migrate(id) + end + end + raise errors.inspect if errors.present? + end + + def errors + @errors ||= [] + end + + def migrate(id) + resource = Hyrax.query_service.find_by(id: id) + return unless resource.wings? # this resource has already been converted + result = MigrateResourceService.new(resource: resource).call + errors << result unless result.success? + result + end +end diff --git a/app/jobs/valkyrie_create_derivatives_job.rb b/app/jobs/valkyrie_create_derivatives_job.rb index 8bc078709e..3895705378 100644 --- a/app/jobs/valkyrie_create_derivatives_job.rb +++ b/app/jobs/valkyrie_create_derivatives_job.rb @@ -16,8 +16,9 @@ def perform(file_set_id, file_id, _filepath = nil) def reindex_parent(file_set_id) file_set = Hyrax.query_service.find_by(id: file_set_id) + return unless file_set parent = Hyrax.custom_queries.find_parent_work(resource: file_set) - return unless parent.thumbnail_id == file_set.id + return unless parent&.thumbnail_id == file_set.id Hyrax.logger.debug { "Reindexing #{parent.id} due to creation of thumbnail derivatives." } Hyrax.index_adapter.save(resource: parent) end diff --git a/app/models/admin_set.rb b/app/models/admin_set.rb index e2700f9af6..9f6acb8168 100644 --- a/app/models/admin_set.rb +++ b/app/models/admin_set.rb @@ -17,6 +17,7 @@ # @see Hyrax::DefaultAdminSetActor # @see Hyrax::ApplyPermissionTemplateActor class AdminSet < ActiveFedora::Base + include Hydra::PCDM::CollectionBehavior include Hydra::AccessControls::Permissions include Hyrax::Noid include Hyrax::HumanReadableType diff --git a/app/models/hyrax/resource.rb b/app/models/hyrax/resource.rb index a0499e96f2..176f900848 100644 --- a/app/models/hyrax/resource.rb +++ b/app/models/hyrax/resource.rb @@ -149,6 +149,11 @@ def work? self.class.work? end + # Its nice to know if a record is still in AF or not + def wings? + respond_to?(:head) && respond_to?(:tail) + end + def ==(other) attributes.except(:created_at, :updated_at) == other.attributes.except(:created_at, :updated_at) if other.respond_to?(:attributes) end diff --git a/app/services/migrate_resource_service.rb b/app/services/migrate_resource_service.rb new file mode 100644 index 0000000000..a8d2b2b41f --- /dev/null +++ b/app/services/migrate_resource_service.rb @@ -0,0 +1,55 @@ +# frozen_string_literal: true + +# migrates models from AF to valkyrie +class MigrateResourceService + attr_accessor :resource + def initialize(resource:) + @resource = resource + end + + def model + @model || Wings::ModelRegistry.lookup(resource.class).to_s + end + + def call + prep_resource + Hyrax::Transactions::Container[model_events(model)] + .with_step_args(**model_steps(model)).call(resource_form) + end + + def prep_resource + case model + when 'FileSet' + resource.creator << ::User.batch_user.email if resource.creator.blank? + end + end + + def resource_form + @resource_form ||= Hyrax::Forms::ResourceForm.for(resource: resource) + end + + def model_events(model) + { + 'AdminSet' => 'admin_set_resource.update', + 'Collection' => 'change_set.update_collection', + 'FileSet' => 'change_set.update_file_set' + }[model] || 'change_set.update_work' + end + + def model_steps(model) + { + 'AdminSet' => {}, + 'Collection' => { + 'collection_resource.save_collection_banner' => { banner_unchanged_indicator: true }, + 'collection_resource.save_collection_logo' => { logo_unchanged_indicator: true } + }, + 'FileSet' => { + 'file_set.save_acl' => {} + } + }[model] || { + 'work_resource.add_file_sets' => { uploaded_files: [], file_set_params: [] }, + 'work_resource.update_work_members' => { work_members_attributes: [] }, + 'work_resource.save_acl' => { permissions_params: [] } + } + end +end diff --git a/lib/freyja/persister.rb b/lib/freyja/persister.rb index 0c7052a347..a4122e7ac2 100644 --- a/lib/freyja/persister.rb +++ b/lib/freyja/persister.rb @@ -32,8 +32,9 @@ def save(resource:, external_resource: false, perform_af_validation: false) def convert_and_migrate_resource(orm_object) new_resource = resource_factory.to_resource(object: orm_object) - if Hyrax.config.valkyrie_transition? && new_resource.is_a?(Hyrax::FileSet) && new_resource.file_ids.size == 1 && new_resource.file_ids.first.id.to_s.match('/files/') - MigrateFilesToValkyrieJob.perform_later(new_resource) + if Hyrax.config.valkyrie_transition? + MigrateFilesToValkyrieJob.perform_later(new_resource) if new_resource.is_a?(Hyrax::FileSet) && new_resource.file_ids.size == 1 && new_resource.file_ids.first.id.to_s.match('/files/') + MigrateResourcesJob.perform_later(ids: new_resource.member_ids) if new_resource.is_a?(Hyrax::Work) end new_resource end diff --git a/spec/controllers/hyrax/dashboard/collections_controller_spec.rb b/spec/controllers/hyrax/dashboard/collections_controller_spec.rb index bfe2af3492..da829a1722 100644 --- a/spec/controllers/hyrax/dashboard/collections_controller_spec.rb +++ b/spec/controllers/hyrax/dashboard/collections_controller_spec.rb @@ -805,9 +805,6 @@ before { sign_in user } it 'shows a list of member files' do - pending "update implementation to work with CollectionResource" if - model.safe_constantize == CollectionResource - get :files, params: { id: collection }, format: :json expect(response).to be_successful diff --git a/spec/jobs/migrate_resources_job_spec.rb b/spec/jobs/migrate_resources_job_spec.rb new file mode 100644 index 0000000000..cd1c42f585 --- /dev/null +++ b/spec/jobs/migrate_resources_job_spec.rb @@ -0,0 +1,29 @@ +# frozen_string_literal: true + +require 'freyja/persister' +RSpec.describe MigrateResourcesJob, index_adapter: :solr_index, valkyrie_adapter: :freyja_adapter do + let(:af_file_set) { create(:file_set, title: ['TestFS']) } + + let!(:af_admin_set) do + as = AdminSet.new(title: ['AF Admin Set']) + as.save + AdminSet.find(as.id) + end + + describe '#perform' do + it "migrates admin sets to valkyrie", active_fedora_to_valkyrie: true do + expect(Valkyrie::Persistence::Postgres::ORM::Resource.find_by(id: af_admin_set.id.to_s)).to be_nil + + MigrateResourcesJob.perform_now(ids: [af_admin_set.id]) + expect(Valkyrie::Persistence::Postgres::ORM::Resource.find_by(id: af_admin_set.id.to_s)).to be_present + end + + it "migrates a file set by its id", active_fedora_to_valkyrie: true do + expect(Valkyrie::Persistence::Postgres::ORM::Resource.find_by(id: af_file_set.id.to_s)).to be_nil + + MigrateResourcesJob.perform_now(ids: [af_file_set.id]) + + expect(Valkyrie::Persistence::Postgres::ORM::Resource.find_by(id: af_file_set.id.to_s)).to be_present + end + end +end