From 71cec5399d5d00c42c7863ed14cb6aec75dbce32 Mon Sep 17 00:00:00 2001 From: Shana Moore Date: Mon, 11 Mar 2024 16:14:28 -0700 Subject: [PATCH 01/27] Revert "WIP - try to import filesets with valkyrie resources" This reverts commit 4ab31b69d6e0c584274978de74c9903e612836cd. --- app/factories/bulkrax/object_factory.rb | 4 --- .../bulkrax/valkyrie_object_factory.rb | 34 +++---------------- app/models/concerns/bulkrax/file_factory.rb | 3 +- .../find_by_source_identifier.rb | 7 ++-- 4 files changed, 10 insertions(+), 38 deletions(-) diff --git a/app/factories/bulkrax/object_factory.rb b/app/factories/bulkrax/object_factory.rb index 718b5fbee..65705fca0 100644 --- a/app/factories/bulkrax/object_factory.rb +++ b/app/factories/bulkrax/object_factory.rb @@ -35,10 +35,6 @@ def self.solr_name(field_name) ActiveFedora.index_field_mapper.solr_name(field_name) end end - - def self.ordered_file_sets_for(object) - object&.ordered_members.to_a.select(&:file_set?) - end # @!endgroup Class Method Interface ## diff --git a/app/factories/bulkrax/valkyrie_object_factory.rb b/app/factories/bulkrax/valkyrie_object_factory.rb index 28302ea05..5fcf3b094 100644 --- a/app/factories/bulkrax/valkyrie_object_factory.rb +++ b/app/factories/bulkrax/valkyrie_object_factory.rb @@ -49,12 +49,6 @@ def self.schema_properties(klass) @schema_properties_map[klass_key] end - def self.ordered_file_sets_for(object) - return [] if object.blank? - - Hyrax.custom_queries.find_child_file_sets(resource: object) - end - def run! run return object if object.persisted? @@ -107,7 +101,7 @@ def create_work(object:, attrs:) transactions["work_resource.create_with_bulk_behavior"] .with_step_args( "work_resource.add_to_parent" => { parent_id: attrs[related_parents_parsed_mapping], user: @user }, - "work_resource.add_bulkrax_files" => { files: get_files(attrs) }, #get_s3_files(remote_files: attrs["remote_files"]), user: @user }, + "work_resource.add_bulkrax_files" => { files: get_s3_files(remote_files: attributes["remote_files"]), user: @user }, "change_set.set_user_as_depositor" => { user: @user }, "work_resource.change_depositor" => { user: @user }, 'work_resource.save_acl' => { permissions_params: [attrs.try('visibility') || 'open'].compact } @@ -148,7 +142,7 @@ def update_work(object:, attrs:) perform_transaction_for(object: object, attrs: attrs) do transactions["work_resource.update_with_bulk_behavior"] .with_step_args( - "work_resource.add_bulkrax_files" => { files: get_files(attrs) }, # get_s3_files(remote_files: attrs["remote_files"]), user: @user } + "work_resource.add_bulkrax_files" => { files: get_s3_files(remote_files: attrs["remote_files"]), user: @user }, 'work_resource.save_acl' => { permissions_params: [attrs.try('visibility') || 'open'].compact } ) end @@ -178,8 +172,7 @@ def perform_transaction_for(object:, attrs:) transaction = yield - # result = transaction.call(form) - result = transaction.call(form, files: @files, user: @user) + result = transaction.call(form) result.value_or do msg = result.failure[0].to_s @@ -188,23 +181,6 @@ def perform_transaction_for(object:, attrs:) end end - def get_files(attrs) - get_local_files(attrs) #+ get_s3_files(remote_files: attrs["remote_files"]) - end - - def get_local_files(attrs) - # byebug # what properties will we get here? - # {:title=>["valkyrie resource 3"], :admin_set_id=>"admin_set/default", :contributor=>[], :creator=>["jg"], :description=>[], :identifier=>[], :keyword=>["bulk test"], :publisher=>[], :language=>[], :license=>[], :resource_type=>["Image"], :rights_statement=>[""], :source=>["6"], :subject=>[], :uploaded_files=>[40], :alternate_ids=>["6"]} - # Hyrax::UploadedFile.find'40' - return [] if attrs[:uploaded_files].blank? - - @files = attrs[:uploaded_files].map do |file_id| - Hyrax::UploadedFile.find(file_id) - end - - @files - end - def get_s3_files(remote_files: {}) if remote_files.blank? Hyrax.logger.info "No remote files listed for #{attributes['source_identifier']}" @@ -255,9 +231,9 @@ def new_remote_files def conditionally_destroy_existing_files return unless @replace_files - if [Hyrax::PcdmCollection, Hyrax::FileSet, Bulkrax::ValkyrieObjectFactory].include?(klass) + if [Hyrax::PcdmCollection, Hyrax::FileSet].include?(klass) return - elsif klass.ancestors.include?(Valkyrie::Resource) && klass != CollectionResource + elsif klass.ancestors.include?(Valkyrie::Resource) destroy_existing_files else raise "Unexpected #{klass} for #{self.class}##{__method__}" diff --git a/app/models/concerns/bulkrax/file_factory.rb b/app/models/concerns/bulkrax/file_factory.rb index 33aeaa220..323ec90eb 100644 --- a/app/models/concerns/bulkrax/file_factory.rb +++ b/app/models/concerns/bulkrax/file_factory.rb @@ -113,9 +113,8 @@ def local_file_sets end def ordered_file_sets - # Bulkrax.object_factory.ordered_file_sets_for # OVERRIDE Hyrda-works 1.2.0 - this method was deprecated in v1.0 - Bulkrax.object_factory.ordered_file_sets_for(object) + object&.ordered_members.to_a.select(&:file_set?) end def import_files diff --git a/app/services/wings/custom_queries/find_by_source_identifier.rb b/app/services/wings/custom_queries/find_by_source_identifier.rb index 9bb1c32f0..55b49213d 100644 --- a/app/services/wings/custom_queries/find_by_source_identifier.rb +++ b/app/services/wings/custom_queries/find_by_source_identifier.rb @@ -16,9 +16,10 @@ def initialize(query_service:) @query_service = query_service end - def find_by_source_identifier(work_identifier:, source_identifier_value:, use_valkyrie: Hyrax.config.use_valkyrie?) - work_identifier_key = Bulkrax.object_factory.solr_name(work_identifier) - af_object = ActiveFedora::Base.where("#{work_identifier_key}:#{source_identifier_value}").first + def find_by_source_identifier(identifier:, use_valkyrie: true) + # TODO: Make more dynamic. Not all application use bulkrax_identifier + # Fetch the app's source_identifier and search by that instead + af_object = ActiveFedora::Base.where("bulkrax_identifier_sim:#{identifier}").first return af_object unless use_valkyrie From ad06786fa64fa36b5cefa7f1e3ad7341db299edd Mon Sep 17 00:00:00 2001 From: Shana Moore Date: Mon, 11 Mar 2024 16:29:34 -0700 Subject: [PATCH 02/27] WIP --- .../bulkrax/valkyrie_object_factory.rb | 21 ++++++++++++++++++- 1 file changed, 20 insertions(+), 1 deletion(-) diff --git a/app/factories/bulkrax/valkyrie_object_factory.rb b/app/factories/bulkrax/valkyrie_object_factory.rb index 5fcf3b094..837f20682 100644 --- a/app/factories/bulkrax/valkyrie_object_factory.rb +++ b/app/factories/bulkrax/valkyrie_object_factory.rb @@ -172,7 +172,8 @@ def perform_transaction_for(object:, attrs:) transaction = yield - result = transaction.call(form) + # result = transaction.call(form) + result = transaction.call(form, files: @files, user: @user) result.value_or do msg = result.failure[0].to_s @@ -181,6 +182,24 @@ def perform_transaction_for(object:, attrs:) end end + def get_files(attrs) + byebug + get_local_files(attrs) #+ get_s3_files(remote_files: attrs["remote_files"]) + end + + def get_local_files(attrs) + # byebug # what properties will we get here? + # {:title=>["valkyrie resource 3"], :admin_set_id=>"admin_set/default", :contributor=>[], :creator=>["jg"], :description=>[], :identifier=>[], :keyword=>["bulk test"], :publisher=>[], :language=>[], :license=>[], :resource_type=>["Image"], :rights_statement=>[""], :source=>["6"], :subject=>[], :uploaded_files=>[40], :alternate_ids=>["6"]} + # Hyrax::UploadedFile.find'40' + return [] if attrs[:uploaded_files].blank? + + @files = attrs[:uploaded_files].map do |file_id| + Hyrax::UploadedFile.find(file_id) + end + + @files + end + def get_s3_files(remote_files: {}) if remote_files.blank? Hyrax.logger.info "No remote files listed for #{attributes['source_identifier']}" From 6c4490b09d04e3c632d4c1808d8de38237bfd123 Mon Sep 17 00:00:00 2001 From: Shana Moore Date: Mon, 11 Mar 2024 16:11:33 -0700 Subject: [PATCH 03/27] WIP - try to import filesets with valkyrie resources --- app/factories/bulkrax/object_factory.rb | 4 ++++ app/factories/bulkrax/valkyrie_object_factory.rb | 15 ++++++++++----- app/models/concerns/bulkrax/file_factory.rb | 3 ++- .../custom_queries/find_by_source_identifier.rb | 7 +++---- 4 files changed, 19 insertions(+), 10 deletions(-) diff --git a/app/factories/bulkrax/object_factory.rb b/app/factories/bulkrax/object_factory.rb index 65705fca0..718b5fbee 100644 --- a/app/factories/bulkrax/object_factory.rb +++ b/app/factories/bulkrax/object_factory.rb @@ -35,6 +35,10 @@ def self.solr_name(field_name) ActiveFedora.index_field_mapper.solr_name(field_name) end end + + def self.ordered_file_sets_for(object) + object&.ordered_members.to_a.select(&:file_set?) + end # @!endgroup Class Method Interface ## diff --git a/app/factories/bulkrax/valkyrie_object_factory.rb b/app/factories/bulkrax/valkyrie_object_factory.rb index 837f20682..28302ea05 100644 --- a/app/factories/bulkrax/valkyrie_object_factory.rb +++ b/app/factories/bulkrax/valkyrie_object_factory.rb @@ -49,6 +49,12 @@ def self.schema_properties(klass) @schema_properties_map[klass_key] end + def self.ordered_file_sets_for(object) + return [] if object.blank? + + Hyrax.custom_queries.find_child_file_sets(resource: object) + end + def run! run return object if object.persisted? @@ -101,7 +107,7 @@ def create_work(object:, attrs:) transactions["work_resource.create_with_bulk_behavior"] .with_step_args( "work_resource.add_to_parent" => { parent_id: attrs[related_parents_parsed_mapping], user: @user }, - "work_resource.add_bulkrax_files" => { files: get_s3_files(remote_files: attributes["remote_files"]), user: @user }, + "work_resource.add_bulkrax_files" => { files: get_files(attrs) }, #get_s3_files(remote_files: attrs["remote_files"]), user: @user }, "change_set.set_user_as_depositor" => { user: @user }, "work_resource.change_depositor" => { user: @user }, 'work_resource.save_acl' => { permissions_params: [attrs.try('visibility') || 'open'].compact } @@ -142,7 +148,7 @@ def update_work(object:, attrs:) perform_transaction_for(object: object, attrs: attrs) do transactions["work_resource.update_with_bulk_behavior"] .with_step_args( - "work_resource.add_bulkrax_files" => { files: get_s3_files(remote_files: attrs["remote_files"]), user: @user }, + "work_resource.add_bulkrax_files" => { files: get_files(attrs) }, # get_s3_files(remote_files: attrs["remote_files"]), user: @user } 'work_resource.save_acl' => { permissions_params: [attrs.try('visibility') || 'open'].compact } ) end @@ -183,7 +189,6 @@ def perform_transaction_for(object:, attrs:) end def get_files(attrs) - byebug get_local_files(attrs) #+ get_s3_files(remote_files: attrs["remote_files"]) end @@ -250,9 +255,9 @@ def new_remote_files def conditionally_destroy_existing_files return unless @replace_files - if [Hyrax::PcdmCollection, Hyrax::FileSet].include?(klass) + if [Hyrax::PcdmCollection, Hyrax::FileSet, Bulkrax::ValkyrieObjectFactory].include?(klass) return - elsif klass.ancestors.include?(Valkyrie::Resource) + elsif klass.ancestors.include?(Valkyrie::Resource) && klass != CollectionResource destroy_existing_files else raise "Unexpected #{klass} for #{self.class}##{__method__}" diff --git a/app/models/concerns/bulkrax/file_factory.rb b/app/models/concerns/bulkrax/file_factory.rb index 323ec90eb..33aeaa220 100644 --- a/app/models/concerns/bulkrax/file_factory.rb +++ b/app/models/concerns/bulkrax/file_factory.rb @@ -113,8 +113,9 @@ def local_file_sets end def ordered_file_sets + # Bulkrax.object_factory.ordered_file_sets_for # OVERRIDE Hyrda-works 1.2.0 - this method was deprecated in v1.0 - object&.ordered_members.to_a.select(&:file_set?) + Bulkrax.object_factory.ordered_file_sets_for(object) end def import_files diff --git a/app/services/wings/custom_queries/find_by_source_identifier.rb b/app/services/wings/custom_queries/find_by_source_identifier.rb index 55b49213d..9bb1c32f0 100644 --- a/app/services/wings/custom_queries/find_by_source_identifier.rb +++ b/app/services/wings/custom_queries/find_by_source_identifier.rb @@ -16,10 +16,9 @@ def initialize(query_service:) @query_service = query_service end - def find_by_source_identifier(identifier:, use_valkyrie: true) - # TODO: Make more dynamic. Not all application use bulkrax_identifier - # Fetch the app's source_identifier and search by that instead - af_object = ActiveFedora::Base.where("bulkrax_identifier_sim:#{identifier}").first + def find_by_source_identifier(work_identifier:, source_identifier_value:, use_valkyrie: Hyrax.config.use_valkyrie?) + work_identifier_key = Bulkrax.object_factory.solr_name(work_identifier) + af_object = ActiveFedora::Base.where("#{work_identifier_key}:#{source_identifier_value}").first return af_object unless use_valkyrie From 6f0c049ea5ed6db4fabe59888261378ece6bd56b Mon Sep 17 00:00:00 2001 From: Shana Moore Date: Tue, 12 Mar 2024 11:55:35 -0700 Subject: [PATCH 04/27] =?UTF-8?q?=F0=9F=9A=A7=20WIP:=20get=20filesets=20to?= =?UTF-8?q?=20import=20via=20bulkrax=20x=20valkyrie?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- app/factories/bulkrax/valkyrie_object_factory.rb | 10 ++++++---- app/transactions/bulkrax/transactions/container.rb | 4 ++-- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/app/factories/bulkrax/valkyrie_object_factory.rb b/app/factories/bulkrax/valkyrie_object_factory.rb index 28302ea05..c03ad9a08 100644 --- a/app/factories/bulkrax/valkyrie_object_factory.rb +++ b/app/factories/bulkrax/valkyrie_object_factory.rb @@ -107,7 +107,8 @@ def create_work(object:, attrs:) transactions["work_resource.create_with_bulk_behavior"] .with_step_args( "work_resource.add_to_parent" => { parent_id: attrs[related_parents_parsed_mapping], user: @user }, - "work_resource.add_bulkrax_files" => { files: get_files(attrs) }, #get_s3_files(remote_files: attrs["remote_files"]), user: @user }, + 'work_resource.add_file_sets' => { uploaded_files: get_files(attrs)}, + # "work_resource.add_bulkrax_files" => { files: get_files(attrs), user: @user }, #get_s3_files(remote_files: attrs["remote_files"]), user: @user }, "change_set.set_user_as_depositor" => { user: @user }, "work_resource.change_depositor" => { user: @user }, 'work_resource.save_acl' => { permissions_params: [attrs.try('visibility') || 'open'].compact } @@ -148,7 +149,8 @@ def update_work(object:, attrs:) perform_transaction_for(object: object, attrs: attrs) do transactions["work_resource.update_with_bulk_behavior"] .with_step_args( - "work_resource.add_bulkrax_files" => { files: get_files(attrs) }, # get_s3_files(remote_files: attrs["remote_files"]), user: @user } + 'work_resource.add_file_sets' => { uploaded_files: get_files(attrs)}, + # "work_resource.add_bulkrax_files" => { files: get_files(attrs), user: @user }, # get_s3_files(remote_files: attrs["remote_files"]), user: @user } 'work_resource.save_acl' => { permissions_params: [attrs.try('visibility') || 'open'].compact } ) end @@ -179,7 +181,7 @@ def perform_transaction_for(object:, attrs:) transaction = yield # result = transaction.call(form) - result = transaction.call(form, files: @files, user: @user) + result = transaction.call(form) result.value_or do msg = result.failure[0].to_s @@ -255,7 +257,7 @@ def new_remote_files def conditionally_destroy_existing_files return unless @replace_files - if [Hyrax::PcdmCollection, Hyrax::FileSet, Bulkrax::ValkyrieObjectFactory].include?(klass) + if [Hyrax::PcdmCollection, Hyrax::FileSet, CollectionResource].include?(klass) return elsif klass.ancestors.include?(Valkyrie::Resource) && klass != CollectionResource destroy_existing_files diff --git a/app/transactions/bulkrax/transactions/container.rb b/app/transactions/bulkrax/transactions/container.rb index 17e9fdbe6..70e798839 100644 --- a/app/transactions/bulkrax/transactions/container.rb +++ b/app/transactions/bulkrax/transactions/container.rb @@ -8,12 +8,12 @@ class Container CREATE_WITH_BULK_BEHAVIOR_STEPS = begin steps = Hyrax::Transactions::WorkCreate::DEFAULT_STEPS.dup - steps[steps.index("work_resource.add_file_sets")] = "work_resource.add_bulkrax_files" + # steps[steps.index("work_resource.add_file_sets")] = "work_resource.add_bulkrax_files" steps end.freeze UPDATE_WITH_BULK_BEHAVIOR_STEPS = begin steps = Hyrax::Transactions::WorkUpdate::DEFAULT_STEPS.dup - steps[steps.index("work_resource.add_file_sets")] = "work_resource.add_bulkrax_files" + # steps[steps.index("work_resource.add_file_sets")] = "work_resource.add_bulkrax_files" steps end.freeze From d4d7d29a1d487b8b803590a659735da257db37c5 Mon Sep 17 00:00:00 2001 From: Shana Moore Date: Tue, 12 Mar 2024 14:38:06 -0700 Subject: [PATCH 05/27] :tada: WIP: filesets to imports via bulkrax x valkyrie There's still a lot to clean up here, but the import is successful in this commit. --- app/factories/bulkrax/object_factory.rb | 5 ++++ .../bulkrax/valkyrie_object_factory.rb | 23 +++++++++++++++++++ app/models/concerns/bulkrax/file_factory.rb | 2 +- 3 files changed, 29 insertions(+), 1 deletion(-) diff --git a/app/factories/bulkrax/object_factory.rb b/app/factories/bulkrax/object_factory.rb index 65705fca0..e7dc307ee 100644 --- a/app/factories/bulkrax/object_factory.rb +++ b/app/factories/bulkrax/object_factory.rb @@ -35,6 +35,11 @@ def self.solr_name(field_name) ActiveFedora.index_field_mapper.solr_name(field_name) end end + + def self.ordered_file_sets_for(object) + object&.ordered_members.to_a.select(&:file_set?) + end + # @!endgroup Class Method Interface ## diff --git a/app/factories/bulkrax/valkyrie_object_factory.rb b/app/factories/bulkrax/valkyrie_object_factory.rb index feaaaadfe..02599ebe9 100644 --- a/app/factories/bulkrax/valkyrie_object_factory.rb +++ b/app/factories/bulkrax/valkyrie_object_factory.rb @@ -49,6 +49,12 @@ def self.schema_properties(klass) @schema_properties_map[klass_key] end + def self.ordered_file_sets_for(object) + return [] if object.blank? + + Hyrax.custom_queries.find_child_file_sets(resource: object) + end + def run! run return object if object.persisted? @@ -183,6 +189,23 @@ def perform_transaction_for(object:, attrs:) end end + def get_files(attrs) + get_local_files(attrs) #+ get_s3_files(remote_files: attrs["remote_files"]) + end + + def get_local_files(attrs) + # byebug # what properties will we get here? + # {:title=>["valkyrie resource 3"], :admin_set_id=>"admin_set/default", :contributor=>[], :creator=>["jg"], :description=>[], :identifier=>[], :keyword=>["bulk test"], :publisher=>[], :language=>[], :license=>[], :resource_type=>["Image"], :rights_statement=>[""], :source=>["6"], :subject=>[], :uploaded_files=>[40], :alternate_ids=>["6"]} + # Hyrax::UploadedFile.find'40' + return [] if attrs[:uploaded_files].blank? + + @files = attrs[:uploaded_files].map do |file_id| + Hyrax::UploadedFile.find(file_id) + end + + @files + end + def get_s3_files(remote_files: {}) if remote_files.blank? Hyrax.logger.info "No remote files listed for #{attributes['source_identifier']}" diff --git a/app/models/concerns/bulkrax/file_factory.rb b/app/models/concerns/bulkrax/file_factory.rb index 323ec90eb..e50ecfbf0 100644 --- a/app/models/concerns/bulkrax/file_factory.rb +++ b/app/models/concerns/bulkrax/file_factory.rb @@ -114,7 +114,7 @@ def local_file_sets def ordered_file_sets # OVERRIDE Hyrda-works 1.2.0 - this method was deprecated in v1.0 - object&.ordered_members.to_a.select(&:file_set?) + Bulkrax.object_factory.ordered_file_sets_for(object) end def import_files From 6d48b29bd51390aff67baa25f48fcfde1fba80d4 Mon Sep 17 00:00:00 2001 From: Shana Moore Date: Tue, 12 Mar 2024 14:49:29 -0700 Subject: [PATCH 06/27] =?UTF-8?q?=F0=9F=92=84=20rubocop=20fixes?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../bulkrax/valkyrie_object_factory.rb | 17 +++++++---------- 1 file changed, 7 insertions(+), 10 deletions(-) diff --git a/app/factories/bulkrax/valkyrie_object_factory.rb b/app/factories/bulkrax/valkyrie_object_factory.rb index 02599ebe9..d8df00c23 100644 --- a/app/factories/bulkrax/valkyrie_object_factory.rb +++ b/app/factories/bulkrax/valkyrie_object_factory.rb @@ -107,7 +107,7 @@ def create_work(object:, attrs:) transactions["work_resource.create_with_bulk_behavior"] .with_step_args( "work_resource.add_to_parent" => { parent_id: attrs[related_parents_parsed_mapping], user: @user }, - 'work_resource.add_file_sets' => { uploaded_files: get_files(attrs)}, + 'work_resource.add_file_sets' => { uploaded_files: get_files(attrs) }, # "work_resource.add_bulkrax_files" => { files: get_files(attrs), user: @user }, #get_s3_files(remote_files: attrs["remote_files"]), user: @user }, "change_set.set_user_as_depositor" => { user: @user }, "work_resource.change_depositor" => { user: @user }, @@ -149,10 +149,10 @@ def update_work(object:, attrs:) perform_transaction_for(object: object, attrs: attrs) do transactions["work_resource.update_with_bulk_behavior"] .with_step_args( - 'work_resource.add_file_sets' => { uploaded_files: get_files(attrs)}, - # "work_resource.add_bulkrax_files" => { files: get_files(attrs), user: @user }, # get_s3_files(remote_files: attrs["remote_files"]), user: @user } - 'work_resource.save_acl' => { permissions_params: [attrs.try('visibility') || 'open'].compact } - ) + 'work_resource.add_file_sets' => { uploaded_files: get_files(attrs) }, + # "work_resource.add_bulkrax_files" => { files: get_files(attrs), user: @user }, # get_s3_files(remote_files: attrs["remote_files"]), user: @user } + 'work_resource.save_acl' => { permissions_params: [attrs.try('visibility') || 'open'].compact } + ) end end @@ -190,13 +190,10 @@ def perform_transaction_for(object:, attrs:) end def get_files(attrs) - get_local_files(attrs) #+ get_s3_files(remote_files: attrs["remote_files"]) + get_local_files(attrs) # + get_s3_files(remote_files: attrs["remote_files"]) end def get_local_files(attrs) - # byebug # what properties will we get here? - # {:title=>["valkyrie resource 3"], :admin_set_id=>"admin_set/default", :contributor=>[], :creator=>["jg"], :description=>[], :identifier=>[], :keyword=>["bulk test"], :publisher=>[], :language=>[], :license=>[], :resource_type=>["Image"], :rights_statement=>[""], :source=>["6"], :subject=>[], :uploaded_files=>[40], :alternate_ids=>["6"]} - # Hyrax::UploadedFile.find'40' return [] if attrs[:uploaded_files].blank? @files = attrs[:uploaded_files].map do |file_id| @@ -255,7 +252,7 @@ def new_remote_files def conditionally_destroy_existing_files return unless @replace_files - + if [Hyrax::PcdmCollection, Hyrax::FileSet, CollectionResource].include?(klass) return elsif klass.ancestors.include?(Valkyrie::Resource) From b9bb435a6ed4303d184f37b0fa2e362f642f5c72 Mon Sep 17 00:00:00 2001 From: Shana Moore Date: Wed, 13 Mar 2024 08:19:18 -0700 Subject: [PATCH 07/27] uncomment #get_s3_files call and add collections to configuration --- app/factories/bulkrax/object_factory.rb | 4 ++-- app/factories/bulkrax/valkyrie_object_factory.rb | 6 +++--- lib/bulkrax.rb | 11 +++++++++++ 3 files changed, 16 insertions(+), 5 deletions(-) diff --git a/app/factories/bulkrax/object_factory.rb b/app/factories/bulkrax/object_factory.rb index e7dc307ee..46459b124 100644 --- a/app/factories/bulkrax/object_factory.rb +++ b/app/factories/bulkrax/object_factory.rb @@ -109,12 +109,12 @@ def run! def update raise "Object doesn't exist" unless object - destroy_existing_files if @replace_files && ![Collection, CollectionResource, FileSet].include?(klass) + destroy_existing_files if @replace_files && ![Bulkrax.config.collection_classes, Bulkrax.config.file_model_class].flatten.include?(klass) attrs = transform_attributes(update: true) run_callbacks :save do if klass == Collection update_collection(attrs) - elsif klass == FileSet + elsif klass == Bulkrax.config.file_model_class update_file_set(attrs) else update_work(attrs) diff --git a/app/factories/bulkrax/valkyrie_object_factory.rb b/app/factories/bulkrax/valkyrie_object_factory.rb index d8df00c23..818e99007 100644 --- a/app/factories/bulkrax/valkyrie_object_factory.rb +++ b/app/factories/bulkrax/valkyrie_object_factory.rb @@ -108,7 +108,7 @@ def create_work(object:, attrs:) .with_step_args( "work_resource.add_to_parent" => { parent_id: attrs[related_parents_parsed_mapping], user: @user }, 'work_resource.add_file_sets' => { uploaded_files: get_files(attrs) }, - # "work_resource.add_bulkrax_files" => { files: get_files(attrs), user: @user }, #get_s3_files(remote_files: attrs["remote_files"]), user: @user }, + # "work_resource.add_bulkrax_files" => { files: get_files(attrs), user: @user }, #get_s3_files(remote_files: attrs[:remote_files]), user: @user }, "change_set.set_user_as_depositor" => { user: @user }, "work_resource.change_depositor" => { user: @user }, 'work_resource.save_acl' => { permissions_params: [attrs.try('visibility') || 'open'].compact } @@ -150,7 +150,7 @@ def update_work(object:, attrs:) transactions["work_resource.update_with_bulk_behavior"] .with_step_args( 'work_resource.add_file_sets' => { uploaded_files: get_files(attrs) }, - # "work_resource.add_bulkrax_files" => { files: get_files(attrs), user: @user }, # get_s3_files(remote_files: attrs["remote_files"]), user: @user } + # "work_resource.add_bulkrax_files" => { files: get_files(attrs), user: @user }, # get_s3_files(remote_files: attrs[:remote_files]), user: @user } 'work_resource.save_acl' => { permissions_params: [attrs.try('visibility') || 'open'].compact } ) end @@ -190,7 +190,7 @@ def perform_transaction_for(object:, attrs:) end def get_files(attrs) - get_local_files(attrs) # + get_s3_files(remote_files: attrs["remote_files"]) + get_local_files(attrs) + get_s3_files(remote_files: attrs[:remote_files]) end def get_local_files(attrs) diff --git a/lib/bulkrax.rb b/lib/bulkrax.rb index 675f41c8a..3da770285 100644 --- a/lib/bulkrax.rb +++ b/lib/bulkrax.rb @@ -33,6 +33,7 @@ module Bulkrax # @api public class Configuration attr_accessor :api_definition, + :collection_classes, :curation_concerns, :default_field_mapping, :default_work_type, @@ -122,6 +123,8 @@ def config def_delegators :@config, :api_definition, :api_definition=, + :collection_classes, + :collection_classes=, :curation_concerns, :curation_concerns=, :default_field_mapping, @@ -188,6 +191,14 @@ def config conf.relationship_job_class = "Bulkrax::CreateRelationshipsJob" conf.required_elements = ['title'] + def conf.collection_classes + @collection_classes ||= defined?(::Valkyrie) ? [Collection, CollectionResource] : [Collection] + end + + def conf.collection_classes=(val) + @collection_classes = [val] + end + def conf.curation_concerns @curation_concerns ||= defined?(::Hyrax) ? ::Hyrax.config.curation_concerns : [] end From 178349ee9308c934ffcf8dbaf0a073a96c1f99f3 Mon Sep 17 00:00:00 2001 From: Shana Moore Date: Wed, 13 Mar 2024 08:24:40 -0700 Subject: [PATCH 08/27] Update object_factory.rb --- app/factories/bulkrax/object_factory.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/factories/bulkrax/object_factory.rb b/app/factories/bulkrax/object_factory.rb index 46459b124..87ffdd03c 100644 --- a/app/factories/bulkrax/object_factory.rb +++ b/app/factories/bulkrax/object_factory.rb @@ -112,7 +112,7 @@ def update destroy_existing_files if @replace_files && ![Bulkrax.config.collection_classes, Bulkrax.config.file_model_class].flatten.include?(klass) attrs = transform_attributes(update: true) run_callbacks :save do - if klass == Collection + if Bulkrax.config.collection_classes.include?(klass) update_collection(attrs) elsif klass == Bulkrax.config.file_model_class update_file_set(attrs) From 5ca0411c1fef6fcb6228fee02b103e87f30febce Mon Sep 17 00:00:00 2001 From: Jeremy Friesen Date: Wed, 13 Mar 2024 11:15:52 -0400 Subject: [PATCH 09/27] =?UTF-8?q?=E2=99=BB=EF=B8=8F=20Move=20method=20and?= =?UTF-8?q?=20remove=20single=20instance=20definition?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit I'm unclear why we were defining methods on the conf instance; especially given that these exist on the configuration. With this refactor, we're favoring using the Configuration object as the container. --- lib/bulkrax.rb | 30 ++++++++++++------------------ 1 file changed, 12 insertions(+), 18 deletions(-) diff --git a/lib/bulkrax.rb b/lib/bulkrax.rb index 8c201f8ee..2552fc2ae 100644 --- a/lib/bulkrax.rb +++ b/lib/bulkrax.rb @@ -33,12 +33,10 @@ module Bulkrax # @api public class Configuration attr_accessor :api_definition, - :curation_concerns, :default_field_mapping, :default_work_type, :export_path, :field_mappings, - :file_model_class, :generated_metadata_mapping, :import_path, :multi_value_element_join_on, @@ -97,6 +95,18 @@ def factory_class_name_coercer @factory_class_name_coercer || Bulkrax::FactoryClassFinder::DefaultCoercer end + def file_model_class + @file_model_class ||= defined?(::Hyrax) ? ::FileSet : File + end + + attr_writer :file_model_class + + def curation_concerns + @curation_concerns ||= defined?(::Hyrax) ? ::Hyrax.config.curation_concerns : [] + end + + attr_writer :curation_concerns + attr_writer :ingest_queue_name ## # @return [String, Proc] @@ -222,22 +232,6 @@ def config conf.relationship_job_class = "Bulkrax::CreateRelationshipsJob" conf.required_elements = ['title'] - def conf.curation_concerns - @curation_concerns ||= defined?(::Hyrax) ? ::Hyrax.config.curation_concerns : [] - end - - def conf.curation_concerns=(val) - @curation_concerns = val - end - - def conf.file_model_class - @file_model_class ||= defined?(::Hyrax) ? ::FileSet : File - end - - def conf.file_model_class=(val) - @file_model_class = val - end - # Hash of Generic field_mappings for use in the view # There must be one field_mappings hash per view partial # Based on Hyrax CoreMetadata && BasicMetadata From 1375a589dfa0d19642ecafac634df7aa5fecfcb1 Mon Sep 17 00:00:00 2001 From: Shana Moore Date: Wed, 13 Mar 2024 08:44:09 -0700 Subject: [PATCH 10/27] Revert changes due to refactor coming in from main --- app/factories/bulkrax/object_factory.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/app/factories/bulkrax/object_factory.rb b/app/factories/bulkrax/object_factory.rb index 87ffdd03c..3d4f2b413 100644 --- a/app/factories/bulkrax/object_factory.rb +++ b/app/factories/bulkrax/object_factory.rb @@ -109,12 +109,12 @@ def run! def update raise "Object doesn't exist" unless object - destroy_existing_files if @replace_files && ![Bulkrax.config.collection_classes, Bulkrax.config.file_model_class].flatten.include?(klass) + destroy_existing_files if @replace_files && ![Collection, FileSet].include?(klass) attrs = transform_attributes(update: true) run_callbacks :save do - if Bulkrax.config.collection_classes.include?(klass) + if klass == Collection update_collection(attrs) - elsif klass == Bulkrax.config.file_model_class + elsif klass == FileSet update_file_set(attrs) else update_work(attrs) From 341e905f09c1781eee212d8b4b2f2d421d19afa6 Mon Sep 17 00:00:00 2001 From: Shana Moore Date: Thu, 14 Mar 2024 11:27:50 -0700 Subject: [PATCH 11/27] address errors post big refactor --- app/factories/bulkrax/valkyrie_object_factory.rb | 10 +++++----- .../hyrax/custom_queries/find_by_source_identifier.rb | 3 ++- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/app/factories/bulkrax/valkyrie_object_factory.rb b/app/factories/bulkrax/valkyrie_object_factory.rb index 361f76d79..fa71dc578 100644 --- a/app/factories/bulkrax/valkyrie_object_factory.rb +++ b/app/factories/bulkrax/valkyrie_object_factory.rb @@ -273,12 +273,12 @@ def new_remote_files def conditionally_destroy_existing_files return unless @replace_files - case klass - when Bulkrax.collection_model_class, Bulkrax.file_model_class - return - when Valkyrie::Resource + + if klass < Valkyrie::Resource destroy_existing_files - else + elsif [Bulkrax.collection_model_class, Bulkrax.file_model_class].include?(klass) + return + else raise "Unexpected #{klass} for #{self.class}##{__method__}" end end diff --git a/app/services/hyrax/custom_queries/find_by_source_identifier.rb b/app/services/hyrax/custom_queries/find_by_source_identifier.rb index 101e609e2..773fb1318 100644 --- a/app/services/hyrax/custom_queries/find_by_source_identifier.rb +++ b/app/services/hyrax/custom_queries/find_by_source_identifier.rb @@ -30,7 +30,8 @@ def initialize(query_service:) def find_by_model_and_property_value(model:, property:, value:) sql_query = sql_for_find_by_model_and_property_value # NOTE: Do we need to ask the model for it's internal_resource? - query_service.run_query(sql_query, model.internal_resource, property, value).first + # TODO: no => undefined method `internal_resource' for Image:Class + query_service.run_query(sql_query, model, property, value).first end private From 8ea0388b04ad1fd5cb2f419808aa7957834137f1 Mon Sep 17 00:00:00 2001 From: Jeremy Friesen Date: Thu, 14 Mar 2024 15:48:01 -0400 Subject: [PATCH 12/27] Refactoring for consistent method signatures Also avoiding setting an unused instance variable --- .../bulkrax/valkyrie_object_factory.rb | 21 ++++++------------- 1 file changed, 6 insertions(+), 15 deletions(-) diff --git a/app/factories/bulkrax/valkyrie_object_factory.rb b/app/factories/bulkrax/valkyrie_object_factory.rb index d7c1b574b..6552490a5 100644 --- a/app/factories/bulkrax/valkyrie_object_factory.rb +++ b/app/factories/bulkrax/valkyrie_object_factory.rb @@ -128,8 +128,7 @@ def create_work(object:, attrs:) transactions["work_resource.create_with_bulk_behavior"] .with_step_args( "work_resource.add_to_parent" => { parent_id: attrs[related_parents_parsed_mapping], user: @user }, - 'work_resource.add_file_sets' => { uploaded_files: get_files(attrs) }, - # "work_resource.add_bulkrax_files" => { files: get_files(attrs), user: @user }, #get_s3_files(remote_files: attrs[:remote_files]), user: @user }, + 'work_resource.add_file_sets' => { uploaded_files: get_files(attrs), user: @user }, "change_set.set_user_as_depositor" => { user: @user }, "work_resource.change_depositor" => { user: @user }, 'work_resource.save_acl' => { permissions_params: [attrs['visibility'] || 'open'].compact } @@ -170,8 +169,7 @@ def update_work(object:, attrs:) perform_transaction_for(object: object, attrs: attrs) do transactions["work_resource.update_with_bulk_behavior"] .with_step_args( - 'work_resource.add_file_sets' => { uploaded_files: get_files(attrs) }, - # "work_resource.add_bulkrax_files" => { files: get_files(attrs), user: @user }, # get_s3_files(remote_files: attrs[:remote_files]), user: @user } + 'work_resource.add_file_sets' => { uploaded_files: get_files(attrs), user: @user }, 'work_resource.save_acl' => { permissions_params: [attrs.try('visibility') || 'open'].compact } ) end @@ -211,24 +209,17 @@ def perform_transaction_for(object:, attrs:) end def get_files(attrs) - get_local_files(attrs) + get_s3_files(remote_files: attrs[:remote_files]) + get_local_files(uploaded_files: attrs[:uploaded_files]) + get_s3_files(remote_files: attrs[:remote_files]) end - def get_local_files(attrs) - return [] if attrs[:uploaded_files].blank? - - @files = attrs[:uploaded_files].map do |file_id| + def get_local_files(uploaded_files: []) + Array.wrap(uploaded_files).map do |file_id| Hyrax::UploadedFile.find(file_id) end - - @files end def get_s3_files(remote_files: {}) - if remote_files.blank? - Hyrax.logger.info "No remote files listed for #{attributes['source_identifier']}" - return [] - end + return [] if remote_files.blank? s3_bucket_name = ENV.fetch("STAGING_AREA_S3_BUCKET", "comet-staging-area-#{Rails.env}") s3_bucket = Rails.application.config.staging_area_s3_connection From 99118a371b51de22d0e21c1042afb92e761f709c Mon Sep 17 00:00:00 2001 From: Shana Moore Date: Thu, 14 Mar 2024 13:30:41 -0700 Subject: [PATCH 13/27] :bug: remove passing user to work_resource add_file_sets and save merge to strategies Importing a CSV of valkyrie works, collections, files and relationships is working at this point :tada: --- app/factories/bulkrax/valkyrie_object_factory.rb | 10 +++++----- lib/bulkrax/engine.rb | 6 +++--- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/app/factories/bulkrax/valkyrie_object_factory.rb b/app/factories/bulkrax/valkyrie_object_factory.rb index 6552490a5..89817ace0 100644 --- a/app/factories/bulkrax/valkyrie_object_factory.rb +++ b/app/factories/bulkrax/valkyrie_object_factory.rb @@ -128,7 +128,7 @@ def create_work(object:, attrs:) transactions["work_resource.create_with_bulk_behavior"] .with_step_args( "work_resource.add_to_parent" => { parent_id: attrs[related_parents_parsed_mapping], user: @user }, - 'work_resource.add_file_sets' => { uploaded_files: get_files(attrs), user: @user }, + 'work_resource.add_file_sets' => { uploaded_files: get_files(attrs) }, "change_set.set_user_as_depositor" => { user: @user }, "work_resource.change_depositor" => { user: @user }, 'work_resource.save_acl' => { permissions_params: [attrs['visibility'] || 'open'].compact } @@ -169,7 +169,7 @@ def update_work(object:, attrs:) perform_transaction_for(object: object, attrs: attrs) do transactions["work_resource.update_with_bulk_behavior"] .with_step_args( - 'work_resource.add_file_sets' => { uploaded_files: get_files(attrs), user: @user }, + 'work_resource.add_file_sets' => { uploaded_files: get_files(attrs) }, 'work_resource.save_acl' => { permissions_params: [attrs.try('visibility') || 'open'].compact } ) end @@ -265,10 +265,10 @@ def new_remote_files def conditionally_destroy_existing_files return unless @replace_files - if klass < Valkyrie::Resource - destroy_existing_files - elsif [Bulkrax.collection_model_class, Bulkrax.file_model_class].include?(klass) + if [Bulkrax.collection_model_class, Bulkrax.file_model_class].include?(klass) return + elsif klass < Valkyrie::Resource + destroy_existing_files else raise "Unexpected #{klass} for #{self.class}##{__method__}" end diff --git a/lib/bulkrax/engine.rb b/lib/bulkrax/engine.rb index d6e4d0dcd..25f8ea881 100644 --- a/lib/bulkrax/engine.rb +++ b/lib/bulkrax/engine.rb @@ -46,19 +46,19 @@ class Engine < ::Rails::Engine if defined?(::Goddess::CustomQueryContainer) strategies = ::Goddess::CustomQueryContainer.known_custom_queries_and_their_strategies - strategies.merge(custom_query_strategies) + strategies = strategies.merge(custom_query_strategies) ::Goddess::CustomQueryContainer.known_custom_queries_and_their_strategies = strategies end if defined?(::Frigg::CustomQueryContainer) strategies = ::Frigg::CustomQueryContainer.known_custom_queries_and_their_strategies - strategies.merge(custom_query_strategies) + strategies = strategies.merge(custom_query_strategies) ::Frigg::CustomQueryContainer.known_custom_queries_and_their_strategies = strategies end if defined?(::Freyja::CustomQueryContainer) strategies = ::Freyja::CustomQueryContainer.known_custom_queries_and_their_strategies - strategies.merge(custom_query_strategies) + strategies = strategies.merge(custom_query_strategies) ::Freyja::CustomQueryContainer.known_custom_queries_and_their_strategies = strategies end end From bd54be5fdf4e695e8aff348d6453a599ad8041ad Mon Sep 17 00:00:00 2001 From: Jeremy Friesen Date: Thu, 14 Mar 2024 17:04:28 -0400 Subject: [PATCH 14/27] =?UTF-8?q?=F0=9F=8E=81=20Adding=20a=20new=20transac?= =?UTF-8?q?tion=20step=20to=20handle=20different=20association?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../steps/bulkrax_add_to_parent.rb | 61 +++++++++++++++++++ 1 file changed, 61 insertions(+) create mode 100644 app/transactions/bulkrax/transactions/steps/bulkrax_add_to_parent.rb diff --git a/app/transactions/bulkrax/transactions/steps/bulkrax_add_to_parent.rb b/app/transactions/bulkrax/transactions/steps/bulkrax_add_to_parent.rb new file mode 100644 index 000000000..8b4d9631c --- /dev/null +++ b/app/transactions/bulkrax/transactions/steps/bulkrax_add_to_parent.rb @@ -0,0 +1,61 @@ +# frozen_string_literal: true + +require "dry/monads" + +module Bulkrax + module Transactions + module Steps + ## + # Hyrax's association modeling changed-ish with Valkyrie. + class BulkraxAddToParent + include Dry::Monads[:result] + + ## + # @param [Class] handler + def initialize(handler: Hyrax::WorkUploadsHandler) + @handler = handler + end + + ## + # @param obj [Valkyrie::Resource] Maybe a work, maybe collection; we'll + # find out. + # + # @param parent_ids [Valkyrie::ID, #to_s, Array] + # we'll go ahead and assume this is scalar or an array, and loop + # through with the correct switching logic + # + # @return [Dry::Monads::Result] + def call(obj, parent_ids:) + Array.wrap(parent_ids).each do |parent_id| + parent = find_parent(parent_id) + associate(child: obj, parent: parent) + # TODO: Capture failures + end + + Success(obj) + end + + private + + # TODO: Make it work! + def associate(child:, parent:) + case child + when Bulkrax.collection_model_class + associate_collection(child: child, parent: parent) + when Bulkrax.file_model_class + associate_file(child: child, parent: parent) + else + associate_work(child: child, parent: parent) + end + end + + def associate_collection(child:, parent:) + end + def associate_work(child:, parent:) + end + def associate_file(child:, parent:) + end + end + end + end +end From 99c712b132c0ee3dac0f2609b56f9cafacc1dc1f Mon Sep 17 00:00:00 2001 From: Jeremy Friesen Date: Fri, 15 Mar 2024 09:25:47 -0400 Subject: [PATCH 15/27] =?UTF-8?q?=E2=99=BB=EF=B8=8F=20Extract=20update=5Fi?= =?UTF-8?q?ndex=20method=20to=20object=20factory?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- app/factories/bulkrax/object_factory.rb | 4 ++++ app/factories/bulkrax/object_factory_interface.rb | 6 ++++++ app/factories/bulkrax/valkyrie_object_factory.rb | 14 ++++++++++---- app/jobs/bulkrax/create_relationships_job.rb | 9 +-------- 4 files changed, 21 insertions(+), 12 deletions(-) diff --git a/app/factories/bulkrax/object_factory.rb b/app/factories/bulkrax/object_factory.rb index 1323a9e3f..06ae25909 100644 --- a/app/factories/bulkrax/object_factory.rb +++ b/app/factories/bulkrax/object_factory.rb @@ -100,6 +100,10 @@ def self.ordered_file_sets_for(object) def self.save!(resource:, **) resource.save! end + + def self.update_index(resources: []) + Array(resources).each(&:update_index) + end # @!endgroup Class Method Interface ## diff --git a/app/factories/bulkrax/object_factory_interface.rb b/app/factories/bulkrax/object_factory_interface.rb index a96cf5093..91f650819 100644 --- a/app/factories/bulkrax/object_factory_interface.rb +++ b/app/factories/bulkrax/object_factory_interface.rb @@ -36,6 +36,12 @@ def find_or_nil(id) nil end + ## + # @param resources [Array] + def update_index(resources: []) + raise NotImplementedError, "#{self}.#{__method__}" + end + ## # @return [Array] def export_properties diff --git a/app/factories/bulkrax/valkyrie_object_factory.rb b/app/factories/bulkrax/valkyrie_object_factory.rb index 89817ace0..8bf69c3a0 100644 --- a/app/factories/bulkrax/valkyrie_object_factory.rb +++ b/app/factories/bulkrax/valkyrie_object_factory.rb @@ -36,13 +36,13 @@ def self.query(q, **kwargs) Hyrax::SolrService.query(q, **kwargs) end - def self.save!(resource:, user:, persister: Hyrax.persister, index_adapter: Hyrax.index_adapter) + def self.save!(resource:, user:) if resource.respond_to?(:save!) resource.save! else - result = persister.save(resource: resource) + result = Hyrax.persister.save(resource: resource) raise Valkyrie::Persistence::ObjectNotFoundError unless result - index_adapter.save(resource: result) + Hyrax.index_adapter.save(resource: result) if result.collection? Hyrax.publisher.publish('collection.metadata.updated', collection: result, user: user) else @@ -52,6 +52,12 @@ def self.save!(resource:, user:, persister: Hyrax.persister, index_adapter: Hyra end end + def self.update_index(resources:) + Array(resources).each do |resource| + Hyrax.index_adapter.save(resource: resource) + end + end + ## # @param value [String] # @param klass [Class, #where] @@ -269,7 +275,7 @@ def conditionally_destroy_existing_files return elsif klass < Valkyrie::Resource destroy_existing_files - else + else raise "Unexpected #{klass} for #{self.class}##{__method__}" end end diff --git a/app/jobs/bulkrax/create_relationships_job.rb b/app/jobs/bulkrax/create_relationships_job.rb index f2821ca67..ccf58e7c6 100644 --- a/app/jobs/bulkrax/create_relationships_job.rb +++ b/app/jobs/bulkrax/create_relationships_job.rb @@ -80,14 +80,7 @@ def perform(parent_identifier:, importer_run_id:) # rubocop:disable Metrics/AbcS # save record if members were added if @parent_record_members_added Bulkrax.object_factory.save!(resource: parent_record, user: importer_run.user) - # Ensure that the new relationship gets indexed onto the children - if parent_record.is_a?(Valkyrie::Resource) - @child_members_added.each do |child| - Hyrax.index_adapter.save(resource: child) - end - else - @child_members_added.each(&:update_index) - end + Bulkrax.object_factory.update_index(resources: @child_members_added) end end else From b31b5d23e5eb6da1b8a72416f49537966f2fb177 Mon Sep 17 00:00:00 2001 From: Jeremy Friesen Date: Fri, 15 Mar 2024 09:39:17 -0400 Subject: [PATCH 16/27] =?UTF-8?q?=E2=99=BB=EF=B8=8F=20Extract=20object=20f?= =?UTF-8?q?actory=20method?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- app/factories/bulkrax/object_factory.rb | 4 ++ .../bulkrax/object_factory_interface.rb | 43 +++++++++++-------- .../bulkrax/valkyrie_object_factory.rb | 5 +++ app/jobs/bulkrax/create_relationships_job.rb | 2 +- 4 files changed, 35 insertions(+), 19 deletions(-) diff --git a/app/factories/bulkrax/object_factory.rb b/app/factories/bulkrax/object_factory.rb index 06ae25909..cfefe541c 100644 --- a/app/factories/bulkrax/object_factory.rb +++ b/app/factories/bulkrax/object_factory.rb @@ -11,6 +11,10 @@ class ObjectFactory # rubocop:disable Metrics/ClassLength ## # @!group Class Method Interface + def self.conditionally_update_index_for_file_sets_of(resource:) + resource.file_sets.each(&:update_index) if resource.respond_to?(:file_sets) + end + ## # @see Bulkrax::ObjectFactoryInterface def self.export_properties diff --git a/app/factories/bulkrax/object_factory_interface.rb b/app/factories/bulkrax/object_factory_interface.rb index 91f650819..76da42021 100644 --- a/app/factories/bulkrax/object_factory_interface.rb +++ b/app/factories/bulkrax/object_factory_interface.rb @@ -23,22 +23,15 @@ class RecordInvalid < ActiveRecord::RecordInvalid class_methods do ## - # @see ActiveFedora::Base.find - def find(id) - raise NotImplementedError, "#{self}.#{__method__}" - end - - def find_or_nil(id) - find(id) - rescue NotImplementedError => e - raise e - rescue - nil + # @yield when Rails application is running in test environment. + def clean! + return true unless Rails.env.test? + yield end ## - # @param resources [Array] - def update_index(resources: []) + # @param resource [Object] something that *might* have file_sets members. + def conditionally_update_index_for_file_sets_of(resource:) raise NotImplementedError, "#{self}.#{__method__}" end @@ -48,14 +41,18 @@ def export_properties raise NotImplementedError, "#{self}.#{__method__}" end - def solr_name(field_name) + ## + # @see ActiveFedora::Base.find + def find(id) raise NotImplementedError, "#{self}.#{__method__}" end - # @yield when Rails application is running in test environment. - def clean! - return true unless Rails.env.test? - yield + def find_or_nil(id) + find(id) + rescue NotImplementedError => e + raise e + rescue + nil end def query(q, **kwargs) @@ -70,6 +67,16 @@ def save!(resource:, user:) def search_by_property(value:, klass:, field: nil, search_field: nil, name_field: nil, verify_property: false) raise NotImplementedError, "#{self}.#{__method__}" end + + def solr_name(field_name) + raise NotImplementedError, "#{self}.#{__method__}" + end + + ## + # @param resources [Array] + def update_index(resources: []) + raise NotImplementedError, "#{self}.#{__method__}" + end # rubocop:enable Metrics/ParameterLists end end diff --git a/app/factories/bulkrax/valkyrie_object_factory.rb b/app/factories/bulkrax/valkyrie_object_factory.rb index 8bf69c3a0..17d47c7e8 100644 --- a/app/factories/bulkrax/valkyrie_object_factory.rb +++ b/app/factories/bulkrax/valkyrie_object_factory.rb @@ -5,6 +5,11 @@ module Bulkrax class ValkyrieObjectFactory < ObjectFactory include ObjectFactoryInterface + def self.conditionally_update_index_for_file_sets_of(resource:) + file_sets = Hyrax.query_service.custom_queries.find_child_file_sets(resource: resource) + update_index(resources: file_sets) + end + def self.find(id) if defined?(Hyrax) begin diff --git a/app/jobs/bulkrax/create_relationships_job.rb b/app/jobs/bulkrax/create_relationships_job.rb index ccf58e7c6..a0de0fa46 100644 --- a/app/jobs/bulkrax/create_relationships_job.rb +++ b/app/jobs/bulkrax/create_relationships_job.rb @@ -154,7 +154,7 @@ def process(relationship:, importer_run_id:, parent_record:, ability:) parent_record.is_a?(Bulkrax.collection_model_class) ? add_to_collection(child_record, parent_record) : add_to_work(child_record, parent_record) - child_record.file_sets.each(&:update_index) if update_child_records_works_file_sets? && child_record.respond_to?(:file_sets) + Bulkrax.object_factory.conditionally_update_index_for_file_sets_of(resource: child_record) if update_child_records_works_file_sets? relationship.destroy end From ce70bd976326474a8f62e01c36a3372e07742cd3 Mon Sep 17 00:00:00 2001 From: Jeremy Friesen Date: Fri, 15 Mar 2024 09:47:25 -0400 Subject: [PATCH 17/27] =?UTF-8?q?=E2=99=BB=EF=B8=8F=20Extract=20add=5Freso?= =?UTF-8?q?urce=5Fto=5Fcollection=20method?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- app/factories/bulkrax/object_factory.rb | 6 ++++++ app/factories/bulkrax/object_factory_interface.rb | 4 ++++ app/factories/bulkrax/valkyrie_object_factory.rb | 7 +++++++ app/jobs/bulkrax/create_relationships_job.rb | 9 +++++---- .../transactions/steps/bulkrax_add_to_parent.rb | 11 +++++------ 5 files changed, 27 insertions(+), 10 deletions(-) diff --git a/app/factories/bulkrax/object_factory.rb b/app/factories/bulkrax/object_factory.rb index cfefe541c..601b50de7 100644 --- a/app/factories/bulkrax/object_factory.rb +++ b/app/factories/bulkrax/object_factory.rb @@ -10,6 +10,12 @@ class ObjectFactory # rubocop:disable Metrics/ClassLength ## # @!group Class Method Interface + def self.add_resource_to_collection(collection:, resource:, user:) + collection.try(:reindex_extent=, Hyrax::Adapters::NestingIndexAdapter::LIMITED_REINDEX) if + defined?(Hyrax::Adapters::NestingIndexAdapter) + resource.member_of_collections << collection + save!(resource: resource, user: user) + end def self.conditionally_update_index_for_file_sets_of(resource:) resource.file_sets.each(&:update_index) if resource.respond_to?(:file_sets) diff --git a/app/factories/bulkrax/object_factory_interface.rb b/app/factories/bulkrax/object_factory_interface.rb index 76da42021..cfbb05718 100644 --- a/app/factories/bulkrax/object_factory_interface.rb +++ b/app/factories/bulkrax/object_factory_interface.rb @@ -22,6 +22,10 @@ class RecordInvalid < ActiveRecord::RecordInvalid end class_methods do + def add_resource_to_collection(collection:, resource:, user:) + raise NotImplementedError, "#{self}.#{__method__}" + end + ## # @yield when Rails application is running in test environment. def clean! diff --git a/app/factories/bulkrax/valkyrie_object_factory.rb b/app/factories/bulkrax/valkyrie_object_factory.rb index 17d47c7e8..54d978964 100644 --- a/app/factories/bulkrax/valkyrie_object_factory.rb +++ b/app/factories/bulkrax/valkyrie_object_factory.rb @@ -5,6 +5,13 @@ module Bulkrax class ValkyrieObjectFactory < ObjectFactory include ObjectFactoryInterface + ## + # @!group Class Method Interface + def self.add_resource_to_collection(collection:, resource:, user:) + resource.member_of_collections_ids << collection.id + save!(resource: resource, user: user) + end + def self.conditionally_update_index_for_file_sets_of(resource:) file_sets = Hyrax.query_service.custom_queries.find_child_file_sets(resource: resource) update_index(resources: file_sets) diff --git a/app/jobs/bulkrax/create_relationships_job.rb b/app/jobs/bulkrax/create_relationships_job.rb index a0de0fa46..961bf1e11 100644 --- a/app/jobs/bulkrax/create_relationships_job.rb +++ b/app/jobs/bulkrax/create_relationships_job.rb @@ -159,10 +159,11 @@ def process(relationship:, importer_run_id:, parent_record:, ability:) end def add_to_collection(child_record, parent_record) - parent_record.try(:reindex_extent=, Hyrax::Adapters::NestingIndexAdapter::LIMITED_REINDEX) if - defined?(Hyrax::Adapters::NestingIndexAdapter) - child_record.member_of_collections << parent_record # TODO: This is not going to work for Valkyrie. Look to add_to_work for inspiration. - Bulkrax.object_factory.save!(resource: child_record, user: importer_run.user) + Bulkrax.object_factory.add_resource_to_collection( + collection: parent_record, + resource: child_record, + user: importer_run.user + ) end def add_to_work(child_record, parent_record) diff --git a/app/transactions/bulkrax/transactions/steps/bulkrax_add_to_parent.rb b/app/transactions/bulkrax/transactions/steps/bulkrax_add_to_parent.rb index 8b4d9631c..f8c8b54d7 100644 --- a/app/transactions/bulkrax/transactions/steps/bulkrax_add_to_parent.rb +++ b/app/transactions/bulkrax/transactions/steps/bulkrax_add_to_parent.rb @@ -49,12 +49,11 @@ def associate(child:, parent:) end end - def associate_collection(child:, parent:) - end - def associate_work(child:, parent:) - end - def associate_file(child:, parent:) - end + def associate_collection(child:, parent:); end + + def associate_work(child:, parent:); end + + def associate_file(child:, parent:); end end end end From 74a6b40ef1a8cd4a0fd660dce8961c0beede2c54 Mon Sep 17 00:00:00 2001 From: Jeremy Friesen Date: Fri, 15 Mar 2024 09:53:27 -0400 Subject: [PATCH 18/27] =?UTF-8?q?=E2=99=BB=EF=B8=8F=20XIT=20out=20the=20mo?= =?UTF-8?q?ckery=20and=20stubbery=20of=20a=20spec?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../bulkrax/create_relationships_job_spec.rb | 21 +++++++++++++------ 1 file changed, 15 insertions(+), 6 deletions(-) diff --git a/spec/jobs/bulkrax/create_relationships_job_spec.rb b/spec/jobs/bulkrax/create_relationships_job_spec.rb index b34bdfb36..61a0c14be 100644 --- a/spec/jobs/bulkrax/create_relationships_job_spec.rb +++ b/spec/jobs/bulkrax/create_relationships_job_spec.rb @@ -2,6 +2,15 @@ require 'rails_helper' +# Dear maintainer and code reader. This spec stubs and mocks far too many +# things to be immediately effective. Why? Because we don't have a functional +# test object factory and data model. +# +# Because of this and a significant refactor of the object model; namely that we +# moved to a repository pattern where we tell the repository to perform the +# various commands instead of commands directly on the object. This moved to a +# repository pattern is necessitated by the shift from Hyrax's ActiveFedora +# usage to Hyrax's Valkyrie uses. module Bulkrax RSpec.describe CreateRelationshipsJob, type: :job do let(:create_relationships_job) { described_class.new } @@ -50,7 +59,7 @@ module Bulkrax ) end - context 'when adding a child work to a parent collection' do + xcontext 'when adding a child work to a parent collection' do before { allow(child_record).to receive(:file_sets).and_return([]) } it 'assigns the parent to the child\'s #member_of_collections' do @@ -71,7 +80,7 @@ module Bulkrax end end - context 'when adding a child collection to a parent collection' do + xcontext 'when adding a child collection to a parent collection' do let(:child_record) { build(:another_collection) } let(:child_entry) { create(:bulkrax_csv_another_entry_collection, importerexporter: importer) } @@ -96,7 +105,7 @@ module Bulkrax xit 'runs NestedCollectionPersistenceService' end - context 'when adding a child work to a parent work' do + xcontext 'when adding a child work to a parent work' do let(:parent_record) { build(:another_work) } let(:parent_entry) { create(:bulkrax_csv_entry_work, identifier: "other_identifier", importerexporter: importer) } @@ -118,7 +127,7 @@ module Bulkrax end end - context 'when adding a child collection to a parent work' do + xcontext 'when adding a child collection to a parent work' do let(:child_entry) { create(:bulkrax_csv_entry_collection, importerexporter: importer) } let(:parent_entry) { create(:bulkrax_csv_entry_work, importerexporter: importer) } let(:child_record) { build(:collection) } @@ -133,7 +142,7 @@ module Bulkrax end end - context 'when adding a child record that is not found' do + xcontext 'when adding a child record that is not found' do it 'reschudules the job' do expect(create_relationships_job).to receive(:find_record).with(child_id, importer.current_run.id).and_return([nil, nil]) perform @@ -141,7 +150,7 @@ module Bulkrax end end - context 'when adding a parent record that is not found' do + xcontext 'when adding a parent record that is not found' do it 'reschedules the job' do expect(create_relationships_job).to receive(:find_record).with(parent_id, importer.current_run.id).and_return([nil, nil]) perform From ab86f4c81bfc85116c92a23a3a98e242f34d7278 Mon Sep 17 00:00:00 2001 From: Jeremy Friesen Date: Fri, 15 Mar 2024 10:14:11 -0400 Subject: [PATCH 19/27] =?UTF-8?q?=E2=99=BB=EF=B8=8F=20Extract=20method=20p?= =?UTF-8?q?ublish=20and=20add=5Fchild=5Fto=5Fparent=5Fwork?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- app/factories/bulkrax/object_factory.rb | 14 ++++++++ .../bulkrax/object_factory_interface.rb | 11 +++++++ .../bulkrax/valkyrie_object_factory.rb | 23 ++++++++++--- app/jobs/bulkrax/create_relationships_job.rb | 32 ++++++++----------- 4 files changed, 57 insertions(+), 23 deletions(-) diff --git a/app/factories/bulkrax/object_factory.rb b/app/factories/bulkrax/object_factory.rb index 601b50de7..dc0bcd1ef 100644 --- a/app/factories/bulkrax/object_factory.rb +++ b/app/factories/bulkrax/object_factory.rb @@ -10,6 +10,16 @@ class ObjectFactory # rubocop:disable Metrics/ClassLength ## # @!group Class Method Interface + + ## + # @note This does not save either object. We need to do that in another + # loop. Why? Because we might be adding many items to the parent. + def self.add_child_to_parent_work(parent:, child:) + return true if parent.ordered_members.to_a.include?(child_record) + + parent.ordered_members << child + end + def self.add_resource_to_collection(collection:, resource:, user:) collection.try(:reindex_extent=, Hyrax::Adapters::NestingIndexAdapter::LIMITED_REINDEX) if defined?(Hyrax::Adapters::NestingIndexAdapter) @@ -38,6 +48,10 @@ def self.find(id) raise ObjectFactoryInterface::ObjectNotFoundError, e.message end + def self.publish(**) + return true + end + ## # @param value [String] # @param klass [Class, #where] diff --git a/app/factories/bulkrax/object_factory_interface.rb b/app/factories/bulkrax/object_factory_interface.rb index cfbb05718..166e92b60 100644 --- a/app/factories/bulkrax/object_factory_interface.rb +++ b/app/factories/bulkrax/object_factory_interface.rb @@ -22,6 +22,13 @@ class RecordInvalid < ActiveRecord::RecordInvalid end class_methods do + ## + # @note This does not save either object. We need to do that in another + # loop. Why? Because we might be adding many items to the parent. + def add_child_to_parent_work(parent:, child:) + raise NotImplementedError, "#{self}.#{__method__}" + end + def add_resource_to_collection(collection:, resource:, user:) raise NotImplementedError, "#{self}.#{__method__}" end @@ -59,6 +66,10 @@ def find_or_nil(id) nil end + def publish(event:, **kwargs) + raise NotImplementedError, "#{self}.#{__method__}" + end + def query(q, **kwargs) raise NotImplementedError, "#{self}.#{__method__}" end diff --git a/app/factories/bulkrax/valkyrie_object_factory.rb b/app/factories/bulkrax/valkyrie_object_factory.rb index 54d978964..1b364026f 100644 --- a/app/factories/bulkrax/valkyrie_object_factory.rb +++ b/app/factories/bulkrax/valkyrie_object_factory.rb @@ -7,6 +7,16 @@ class ValkyrieObjectFactory < ObjectFactory ## # @!group Class Method Interface + + ## + # @note This does not save either object. We need to do that in another + # loop. Why? Because we might be adding many items to the parent. + def self.add_child_to_parent_work(parent:, child:) + return true if parent.member_ids.include?(child.id) + + parent.member_ids << child.id + end + def self.add_resource_to_collection(collection:, resource:, user:) resource.member_of_collections_ids << collection.id save!(resource: resource, user: user) @@ -40,6 +50,10 @@ def self.solr_name(field_name) Hyrax.config.index_field_mapper.solr_name(field_name) end + def self.publish(event:, **kwargs) + Hyrax.publisher.publish(event, **kwargs) + end + def self.query(q, **kwargs) # Someone could choose ActiveFedora::SolrService. But I think we're # assuming Valkyrie is specifcally working for Hyrax. Someone could make @@ -56,9 +70,9 @@ def self.save!(resource:, user:) raise Valkyrie::Persistence::ObjectNotFoundError unless result Hyrax.index_adapter.save(resource: result) if result.collection? - Hyrax.publisher.publish('collection.metadata.updated', collection: result, user: user) + publish('collection.metadata.updated', collection: result, user: user) else - Hyrax.publisher.publish('object.metadata.updated', object: result, user: user) + publish('object.metadata.updated', object: result, user: user) end resource end @@ -258,8 +272,9 @@ def permitted_attributes def apply_depositor_metadata(object, user) object.depositor = user.email + # TODO: Should we leverage the object factory's save! method? object = Hyrax.persister.save(resource: object) - Hyrax.publisher.publish("object.metadata.updated", object: object, user: @user) + self.class.publish("object.metadata.updated", object: object, user: @user) object end @@ -316,7 +331,7 @@ def delete(user) Hyrax.persister.delete(resource: obj) Hyrax.index_adapter.delete(resource: obj) - Hyrax.publisher.publish('object.deleted', object: obj, user: user) + self.class.publish('object.deleted', object: obj, user: user) end private diff --git a/app/jobs/bulkrax/create_relationships_job.rb b/app/jobs/bulkrax/create_relationships_job.rb index 961bf1e11..3baedb800 100644 --- a/app/jobs/bulkrax/create_relationships_job.rb +++ b/app/jobs/bulkrax/create_relationships_job.rb @@ -80,6 +80,7 @@ def perform(parent_identifier:, importer_run_id:) # rubocop:disable Metrics/AbcS # save record if members were added if @parent_record_members_added Bulkrax.object_factory.save!(resource: parent_record, user: importer_run.user) + Bulkrax.object_factory.publish(event: 'object.membership.updated', object: parent_record) Bulkrax.object_factory.update_index(resources: @child_members_added) end end @@ -152,9 +153,14 @@ def process(relationship:, importer_run_id:, parent_record:, ability:) # We could do this outside of the loop, but that could lead to odd counter failures. ability.authorize!(:edit, parent_record) - parent_record.is_a?(Bulkrax.collection_model_class) ? add_to_collection(child_record, parent_record) : add_to_work(child_record, parent_record) + if parent_record.is_a?(Bulkrax.collection_model_class) + add_to_collection(child_record, parent_record) + else + add_to_work(child_record, parent_record) + end Bulkrax.object_factory.conditionally_update_index_for_file_sets_of(resource: child_record) if update_child_records_works_file_sets? + relationship.destroy end @@ -167,24 +173,12 @@ def add_to_collection(child_record, parent_record) end def add_to_work(child_record, parent_record) - parent_record.is_a?(Valkyrie::Resource) ? add_to_valkyrie_work(child_record, parent_record) : add_to_af_work(child_record, parent_record) - - @parent_record_members_added = true - @child_members_added << child_record - end - - def add_to_valkyrie_work(child_record, parent_record) - return true if parent_record.member_ids.include?(child_record.id) - - parent_record.member_ids << child_record.id - Hyrax.persister.save(resource: parent_record) - Hyrax.publisher.publish('object.membership.updated', object: parent_record) - end - - def add_to_af_work(child_record, parent_record) - return true if parent_record.ordered_members.to_a.include?(child_record) - - parent_record.ordered_members << child_record + # NOTE: The .add_child_to_parent_work should not persist changes to the + # child nor parent. We'll do that elsewhere in this loop. + Bulkrax.object_factory.add_child_to_parent_work( + parent: parent_record, + child: child_record + ) end def reschedule(parent_identifier:, importer_run_id:) From 620c9919842f7734b8671b789ee81fb4ad68d974 Mon Sep 17 00:00:00 2001 From: Jeremy Friesen Date: Fri, 15 Mar 2024 10:15:28 -0400 Subject: [PATCH 20/27] =?UTF-8?q?=E2=99=BB=EF=B8=8F=20Rename=20method=20as?= =?UTF-8?q?=20it's=20not=20conditional?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Yes, it is conditional but it operates on arrays that could be empty. --- app/factories/bulkrax/object_factory.rb | 2 +- app/factories/bulkrax/object_factory_interface.rb | 2 +- app/factories/bulkrax/valkyrie_object_factory.rb | 2 +- app/jobs/bulkrax/create_relationships_job.rb | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/app/factories/bulkrax/object_factory.rb b/app/factories/bulkrax/object_factory.rb index dc0bcd1ef..830249a5b 100644 --- a/app/factories/bulkrax/object_factory.rb +++ b/app/factories/bulkrax/object_factory.rb @@ -27,7 +27,7 @@ def self.add_resource_to_collection(collection:, resource:, user:) save!(resource: resource, user: user) end - def self.conditionally_update_index_for_file_sets_of(resource:) + def self.update_index_for_file_sets_of(resource:) resource.file_sets.each(&:update_index) if resource.respond_to?(:file_sets) end diff --git a/app/factories/bulkrax/object_factory_interface.rb b/app/factories/bulkrax/object_factory_interface.rb index 166e92b60..d7fbdfdcf 100644 --- a/app/factories/bulkrax/object_factory_interface.rb +++ b/app/factories/bulkrax/object_factory_interface.rb @@ -42,7 +42,7 @@ def clean! ## # @param resource [Object] something that *might* have file_sets members. - def conditionally_update_index_for_file_sets_of(resource:) + def update_index_for_file_sets_of(resource:) raise NotImplementedError, "#{self}.#{__method__}" end diff --git a/app/factories/bulkrax/valkyrie_object_factory.rb b/app/factories/bulkrax/valkyrie_object_factory.rb index 1b364026f..57dc566ca 100644 --- a/app/factories/bulkrax/valkyrie_object_factory.rb +++ b/app/factories/bulkrax/valkyrie_object_factory.rb @@ -22,7 +22,7 @@ def self.add_resource_to_collection(collection:, resource:, user:) save!(resource: resource, user: user) end - def self.conditionally_update_index_for_file_sets_of(resource:) + def self.update_index_for_file_sets_of(resource:) file_sets = Hyrax.query_service.custom_queries.find_child_file_sets(resource: resource) update_index(resources: file_sets) end diff --git a/app/jobs/bulkrax/create_relationships_job.rb b/app/jobs/bulkrax/create_relationships_job.rb index 3baedb800..9e74e3f2c 100644 --- a/app/jobs/bulkrax/create_relationships_job.rb +++ b/app/jobs/bulkrax/create_relationships_job.rb @@ -159,7 +159,7 @@ def process(relationship:, importer_run_id:, parent_record:, ability:) add_to_work(child_record, parent_record) end - Bulkrax.object_factory.conditionally_update_index_for_file_sets_of(resource: child_record) if update_child_records_works_file_sets? + Bulkrax.object_factory.update_index_for_file_sets_of(resource: child_record) if update_child_records_works_file_sets? relationship.destroy end From 267d22371cb4cb0248748dc4b15dd5153a941998 Mon Sep 17 00:00:00 2001 From: Jeremy Friesen Date: Fri, 15 Mar 2024 10:28:41 -0400 Subject: [PATCH 21/27] Remove add to collection step --- app/factories/bulkrax/valkyrie_object_factory.rb | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/app/factories/bulkrax/valkyrie_object_factory.rb b/app/factories/bulkrax/valkyrie_object_factory.rb index 57dc566ca..96a6baa56 100644 --- a/app/factories/bulkrax/valkyrie_object_factory.rb +++ b/app/factories/bulkrax/valkyrie_object_factory.rb @@ -156,10 +156,11 @@ def create end def create_work(object:, attrs:) + # NOTE: We do not add relationships here; that is part of the create + # relationships job. perform_transaction_for(object: object, attrs: attrs) do transactions["work_resource.create_with_bulk_behavior"] .with_step_args( - "work_resource.add_to_parent" => { parent_id: attrs[related_parents_parsed_mapping], user: @user }, 'work_resource.add_file_sets' => { uploaded_files: get_files(attrs) }, "change_set.set_user_as_depositor" => { user: @user }, "work_resource.change_depositor" => { user: @user }, @@ -169,11 +170,12 @@ def create_work(object:, attrs:) end def create_collection(object:, attrs:) + # NOTE: We do not add relationships here; that is part of the create + # relationships job. perform_transaction_for(object: object, attrs: attrs) do transactions['change_set.create_collection'] .with_step_args( 'change_set.set_user_as_depositor' => { user: @user }, - 'change_set.add_to_collections' => { collection_ids: Array(attrs[related_parents_parsed_mapping]) }, 'collection_resource.apply_collection_type_permissions' => { user: @user } ) end From 32644904b5298371332559958969d410b6d5516c Mon Sep 17 00:00:00 2001 From: Jeremy Friesen Date: Fri, 15 Mar 2024 10:42:32 -0400 Subject: [PATCH 22/27] =?UTF-8?q?=F0=9F=90=9B=20Fix=20publish=20parameter?= =?UTF-8?q?=20mismatch?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- app/factories/bulkrax/valkyrie_object_factory.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/factories/bulkrax/valkyrie_object_factory.rb b/app/factories/bulkrax/valkyrie_object_factory.rb index 96a6baa56..b45492f06 100644 --- a/app/factories/bulkrax/valkyrie_object_factory.rb +++ b/app/factories/bulkrax/valkyrie_object_factory.rb @@ -276,7 +276,7 @@ def apply_depositor_metadata(object, user) object.depositor = user.email # TODO: Should we leverage the object factory's save! method? object = Hyrax.persister.save(resource: object) - self.class.publish("object.metadata.updated", object: object, user: @user) + self.class.publish(event: "object.metadata.updated", object: object, user: @user) object end @@ -333,7 +333,7 @@ def delete(user) Hyrax.persister.delete(resource: obj) Hyrax.index_adapter.delete(resource: obj) - self.class.publish('object.deleted', object: obj, user: user) + self.class.publish(event: 'object.deleted', object: obj, user: user) end private From c7f99d111fca947add8c1b0b31893f9f594311d6 Mon Sep 17 00:00:00 2001 From: Jeremy Friesen Date: Fri, 15 Mar 2024 11:13:32 -0400 Subject: [PATCH 23/27] Removing custom transaction container. We weren't using it --- .../bulkrax/valkyrie_object_factory.rb | 36 ++++++----- app/transactions/bulkrax/transactions.rb | 18 ------ .../bulkrax/transactions/container.rb | 45 -------------- .../bulkrax/transactions/steps/add_files.rb | 49 --------------- .../steps/bulkrax_add_to_parent.rb | 60 ------------------- lib/bulkrax/engine.rb | 4 -- .../bulkrax/transactions/container_spec.rb | 33 ---------- 7 files changed, 17 insertions(+), 228 deletions(-) delete mode 100644 app/transactions/bulkrax/transactions.rb delete mode 100644 app/transactions/bulkrax/transactions/container.rb delete mode 100644 app/transactions/bulkrax/transactions/steps/add_files.rb delete mode 100644 app/transactions/bulkrax/transactions/steps/bulkrax_add_to_parent.rb delete mode 100644 spec/transactions/bulkrax/transactions/container_spec.rb diff --git a/app/factories/bulkrax/valkyrie_object_factory.rb b/app/factories/bulkrax/valkyrie_object_factory.rb index b45492f06..baf6b5ef8 100644 --- a/app/factories/bulkrax/valkyrie_object_factory.rb +++ b/app/factories/bulkrax/valkyrie_object_factory.rb @@ -5,6 +5,21 @@ module Bulkrax class ValkyrieObjectFactory < ObjectFactory include ObjectFactoryInterface + ## + # When you want a different set of transactions you can change the + # container. + # + # @note Within {Bulkrax::ValkyrieObjectFactory} there are several calls to + # transactions; so you'll need your container to register those + # transactions. + def self.transactions + @transactions || Hyrax::Transactions::Container + end + + def transactions + self.class.transactions + end + ## # @!group Class Method Interface @@ -159,7 +174,7 @@ def create_work(object:, attrs:) # NOTE: We do not add relationships here; that is part of the create # relationships job. perform_transaction_for(object: object, attrs: attrs) do - transactions["work_resource.create_with_bulk_behavior"] + transactions["change_set.create_work"] .with_step_args( 'work_resource.add_file_sets' => { uploaded_files: get_files(attrs) }, "change_set.set_user_as_depositor" => { user: @user }, @@ -201,7 +216,7 @@ def update def update_work(object:, attrs:) perform_transaction_for(object: object, attrs: attrs) do - transactions["work_resource.update_with_bulk_behavior"] + transactions["change_set.update_work"] .with_step_args( 'work_resource.add_file_sets' => { uploaded_files: get_files(attrs) }, 'work_resource.save_acl' => { permissions_params: [attrs.try('visibility') || 'open'].compact } @@ -342,23 +357,6 @@ def delete(user) def fetch_child_file_sets(resource:) Hyrax.custom_queries.find_child_file_sets(resource: resource) end - - ## - # @api public - # - # @return [#[]] a resolver for Hyrax's Transactions; this *should* be a - # thread-safe {Dry::Container}, but callers to this method should strictly - # use +#[]+ for access. - # - # @example - # transactions['change_set.create_work'].call(my_form) - # - # @see Hyrax::Transactions::Container - # @see Hyrax::Transactions::Transaction - # @see https://dry-rb.org/gems/dry-container - def transactions - Hyrax::Transactions::Container - end end # rubocop:enable Metrics/ClassLength end diff --git a/app/transactions/bulkrax/transactions.rb b/app/transactions/bulkrax/transactions.rb deleted file mode 100644 index 6efbedea1..000000000 --- a/app/transactions/bulkrax/transactions.rb +++ /dev/null @@ -1,18 +0,0 @@ -# frozen_string_literal: true -require 'bulkrax/transactions/container' - -module Bulkrax - ## - # This is a parent module for DRY Transaction classes handling Bulkrax - # processes. Especially: transactions and steps for creating, updating, and - # destroying PCDM Objects are located here. - # - # @since 2.4.0 - # - # @example - # Bulkrax::Transaction::Container['transaction_name'].call(:input) - # - # @see https://dry-rb.org/gems/dry-transaction/ - module Transactions - end -end diff --git a/app/transactions/bulkrax/transactions/container.rb b/app/transactions/bulkrax/transactions/container.rb deleted file mode 100644 index 3fe02645e..000000000 --- a/app/transactions/bulkrax/transactions/container.rb +++ /dev/null @@ -1,45 +0,0 @@ -# frozen_string_literal: true -require 'dry/container' - -module Bulkrax - module Transactions - class Container - extend Dry::Container::Mixin - - CREATE_WITH_BULK_BEHAVIOR_STEPS = begin - steps = Hyrax::Transactions::WorkCreate::DEFAULT_STEPS.dup - # steps[steps.index("work_resource.add_file_sets")] = "work_resource.add_bulkrax_files" - steps - end.freeze - UPDATE_WITH_BULK_BEHAVIOR_STEPS = begin - steps = Hyrax::Transactions::WorkUpdate::DEFAULT_STEPS.dup - # steps[steps.index("work_resource.add_file_sets")] = "work_resource.add_bulkrax_files" - steps - end.freeze - - namespace "work_resource" do |ops| - ops.register 'create_with_bulk_behavior' do - Hyrax::Transactions::WorkCreate.new(steps: CREATE_WITH_BULK_BEHAVIOR_STEPS) - end - - ops.register 'update_with_bulk_behavior' do - Hyrax::Transactions::WorkUpdate.new(steps: UPDATE_WITH_BULK_BEHAVIOR_STEPS) - end - - # TODO: Need to register step for uploads handler? - # ops.register "add_file_sets" do - # Hyrax::Transactions::Steps::AddFileSets.new - # end - - ops.register 'add_bulkrax_files' do - Bulkrax::Transactions::Steps::AddFiles.new - end - end - - namespace "file_set_resource" do |ops| - # TODO: to implement - end - end - end -end -Hyrax::Transactions::Container.merge(Bulkrax::Transactions::Container) diff --git a/app/transactions/bulkrax/transactions/steps/add_files.rb b/app/transactions/bulkrax/transactions/steps/add_files.rb deleted file mode 100644 index 2b9b1f627..000000000 --- a/app/transactions/bulkrax/transactions/steps/add_files.rb +++ /dev/null @@ -1,49 +0,0 @@ -# frozen_string_literal: true - -require "dry/monads" - -module Bulkrax - module Transactions - module Steps - class AddFiles - include Dry::Monads[:result] - - ## - # @param [Class] handler - def initialize(handler: Hyrax::WorkUploadsHandler) - @handler = handler - end - - ## - # @param [Hyrax::Work] obj - # @param [Array] file - # @param [User] user - # - # @return [Dry::Monads::Result] - def call(obj, files:, user:) - if files && user - begin - files.each do |file| - FileIngest.upload( - content_type: file.content_type, - file_body: StringIO.new(file.body), - filename: Pathname.new(file.key).basename, - last_modified: file.last_modified, - permissions: Hyrax::AccessControlList.new(resource: obj), - size: file.content_length, - user: user, - work: obj - ) - end - rescue => e - Hyrax.logger.error(e) - return Failure[:failed_to_attach_file_sets, files] - end - end - - Success(obj) - end - end - end - end -end diff --git a/app/transactions/bulkrax/transactions/steps/bulkrax_add_to_parent.rb b/app/transactions/bulkrax/transactions/steps/bulkrax_add_to_parent.rb deleted file mode 100644 index f8c8b54d7..000000000 --- a/app/transactions/bulkrax/transactions/steps/bulkrax_add_to_parent.rb +++ /dev/null @@ -1,60 +0,0 @@ -# frozen_string_literal: true - -require "dry/monads" - -module Bulkrax - module Transactions - module Steps - ## - # Hyrax's association modeling changed-ish with Valkyrie. - class BulkraxAddToParent - include Dry::Monads[:result] - - ## - # @param [Class] handler - def initialize(handler: Hyrax::WorkUploadsHandler) - @handler = handler - end - - ## - # @param obj [Valkyrie::Resource] Maybe a work, maybe collection; we'll - # find out. - # - # @param parent_ids [Valkyrie::ID, #to_s, Array] - # we'll go ahead and assume this is scalar or an array, and loop - # through with the correct switching logic - # - # @return [Dry::Monads::Result] - def call(obj, parent_ids:) - Array.wrap(parent_ids).each do |parent_id| - parent = find_parent(parent_id) - associate(child: obj, parent: parent) - # TODO: Capture failures - end - - Success(obj) - end - - private - - # TODO: Make it work! - def associate(child:, parent:) - case child - when Bulkrax.collection_model_class - associate_collection(child: child, parent: parent) - when Bulkrax.file_model_class - associate_file(child: child, parent: parent) - else - associate_work(child: child, parent: parent) - end - end - - def associate_collection(child:, parent:); end - - def associate_work(child:, parent:); end - - def associate_file(child:, parent:); end - end - end - end -end diff --git a/lib/bulkrax/engine.rb b/lib/bulkrax/engine.rb index 25f8ea881..48fe1282c 100644 --- a/lib/bulkrax/engine.rb +++ b/lib/bulkrax/engine.rb @@ -16,10 +16,6 @@ class Engine < ::Rails::Engine end end - initializer 'requires' do - require 'bulkrax/transactions' if defined?(Hyrax::Transactions) - end - config.generators do |g| g.test_framework :rspec begin diff --git a/spec/transactions/bulkrax/transactions/container_spec.rb b/spec/transactions/bulkrax/transactions/container_spec.rb deleted file mode 100644 index 3222a2d44..000000000 --- a/spec/transactions/bulkrax/transactions/container_spec.rb +++ /dev/null @@ -1,33 +0,0 @@ -# frozen_string_literal: true - -require 'rails_helper' - -# Yes, we're testing Hyrax::Transactions::Container and not Bulkrax::Transactions::Container, because we want to see the -# impact of the change on Hyrax's implementation. -RSpec.describe Hyrax::Transactions::Container do - describe 'work_resource.create_with_bulk_behavior' do - subject(:transaction_step) { described_class['work_resource.create_with_bulk_behavior'] } - - describe '#steps' do - subject { transaction_step.steps } - it { is_expected.to include("work_resource.add_bulkrax_files") } - it { is_expected.not_to include("work_resource.add_file_sets") } - end - end - - describe 'work_resource.update_with_bulk_behavior' do - subject(:transaction_step) { described_class['work_resource.update_with_bulk_behavior'] } - - describe '#steps' do - subject { transaction_step.steps } - it { is_expected.to include("work_resource.add_bulkrax_files") } - it { is_expected.not_to include("work_resource.add_file_sets") } - end - end - - describe 'work_resource.add_bulkrax_files' do - subject(:transaction_step) { described_class['work_resource.add_bulkrax_files'] } - - it { is_expected.to be_a Bulkrax::Transactions::Steps::AddFiles } - end -end From 4c9bec107083d15a21a2e4ba4f8a49e7e2464839 Mon Sep 17 00:00:00 2001 From: Jeremy Friesen Date: Fri, 15 Mar 2024 11:19:34 -0400 Subject: [PATCH 24/27] Favor keyword args instead of hashes --- app/jobs/bulkrax/create_relationships_job.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/jobs/bulkrax/create_relationships_job.rb b/app/jobs/bulkrax/create_relationships_job.rb index 9e74e3f2c..9fd159be5 100644 --- a/app/jobs/bulkrax/create_relationships_job.rb +++ b/app/jobs/bulkrax/create_relationships_job.rb @@ -104,7 +104,7 @@ def perform(parent_identifier:, importer_run_id:) # rubocop:disable Metrics/AbcS parent_entry&.set_status_info(errors.last, importer_run) # TODO: This can create an infinite job cycle, consider a time to live tracker. - reschedule({ parent_identifier: parent_identifier, importer_run_id: importer_run_id }) + reschedule(parent_identifier: parent_identifier, importer_run_id: importer_run_id) return false # stop current job from continuing to run after rescheduling else # rubocop:disable Rails/SkipsModelValidations From 1f363e1529b479b1c67f0c6c48bdaca8b88c973a Mon Sep 17 00:00:00 2001 From: Shana Moore Date: Fri, 15 Mar 2024 08:44:02 -0700 Subject: [PATCH 25/27] =?UTF-8?q?=F0=9F=92=84fixing=20typo?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- app/factories/bulkrax/valkyrie_object_factory.rb | 2 +- app/models/concerns/bulkrax/file_factory.rb | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/app/factories/bulkrax/valkyrie_object_factory.rb b/app/factories/bulkrax/valkyrie_object_factory.rb index baf6b5ef8..3e38516f1 100644 --- a/app/factories/bulkrax/valkyrie_object_factory.rb +++ b/app/factories/bulkrax/valkyrie_object_factory.rb @@ -33,7 +33,7 @@ def self.add_child_to_parent_work(parent:, child:) end def self.add_resource_to_collection(collection:, resource:, user:) - resource.member_of_collections_ids << collection.id + resource.member_of_collection_ids << collection.id save!(resource: resource, user: user) end diff --git a/app/models/concerns/bulkrax/file_factory.rb b/app/models/concerns/bulkrax/file_factory.rb index e7322d8b5..eafc4df72 100644 --- a/app/models/concerns/bulkrax/file_factory.rb +++ b/app/models/concerns/bulkrax/file_factory.rb @@ -133,6 +133,8 @@ def import_file(path) def update_filesets(current_file) if @update_files && local_file_sets.present? fileset = local_file_sets.shift + # TODO: Handle valkyrie way + return if fileset.is_a? Hyrax::Resource return nil if fileset.files.first.checksum.value == Digest::SHA1.file(current_file.file.path).to_s fileset.files.first.create_version From ea0c95c70dd0f2aa22ec8d5ac228c86a1930fd76 Mon Sep 17 00:00:00 2001 From: Shana Moore Date: Fri, 15 Mar 2024 08:51:49 -0700 Subject: [PATCH 26/27] =?UTF-8?q?=F0=9F=8E=81=20Add=20update=5Fcollection?= =?UTF-8?q?=20to=20valkyrie=20object=20factory?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- app/factories/bulkrax/valkyrie_object_factory.rb | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/app/factories/bulkrax/valkyrie_object_factory.rb b/app/factories/bulkrax/valkyrie_object_factory.rb index 3e38516f1..8e431f244 100644 --- a/app/factories/bulkrax/valkyrie_object_factory.rb +++ b/app/factories/bulkrax/valkyrie_object_factory.rb @@ -204,9 +204,10 @@ def update @object = case @object when Bulkrax.collection_model_class - # TODO: update_collection(attrs) + update_collection(object: @object, attrs: attrs) when Bulkrax.file_model_class # TODO: update_file_set(attrs) + raise "FileSet update not implemented" when Hyrax::Resource update_work(object: @object, attrs: attrs) else @@ -224,6 +225,14 @@ def update_work(object:, attrs:) end end + def update_collection(object:, attrs:) + # NOTE: We do not add relationships here; that is part of the create + # relationships job. + perform_transaction_for(object: object, attrs: attrs) do + transactions['change_set.update_collection'] + end + end + ## # @param object [Valkyrie::Resource] # @param attrs [Valkyrie::Resource] From 2060f6906d3282afe9b2c2126eec71f3fe22cc1a Mon Sep 17 00:00:00 2001 From: Jeremy Friesen Date: Fri, 15 Mar 2024 12:06:35 -0400 Subject: [PATCH 27/27] =?UTF-8?q?=F0=9F=92=84=20endless=20and=20ever=20app?= =?UTF-8?q?easing=20of=20the=20coppers?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- app/models/concerns/bulkrax/file_factory.rb | 2 ++ 1 file changed, 2 insertions(+) diff --git a/app/models/concerns/bulkrax/file_factory.rb b/app/models/concerns/bulkrax/file_factory.rb index eafc4df72..9bbce0d55 100644 --- a/app/models/concerns/bulkrax/file_factory.rb +++ b/app/models/concerns/bulkrax/file_factory.rb @@ -130,6 +130,7 @@ def import_file(path) update_filesets(u) end + # rubocop:disable Metrics/AbcSize def update_filesets(current_file) if @update_files && local_file_sets.present? fileset = local_file_sets.shift @@ -152,5 +153,6 @@ def update_filesets(current_file) current_file.id end end + # rubocop:enable Metrics/AbcSize end end