From f567f837d43561d2d33fa27374b627ef7636777e Mon Sep 17 00:00:00 2001 From: Shana Moore Date: Wed, 24 Jan 2024 10:16:06 -0800 Subject: [PATCH 01/15] :broom: relocates transactions from inititalizer file Issue: - https://github.com/samvera/bulkrax/issues/897 Co-Authored-By: LaRita Robinson --- .../concerns/bulkrax/has_mapping_ext.rb | 67 +++++++++++++ lib/bulkrax/engine.rb | 1 + lib/bulkrax/transactions.rb | 18 ++++ lib/bulkrax/transactions/container.rb | 36 +++++++ .../templates/config/initializers/bulkrax.rb | 97 +------------------ 5 files changed, 123 insertions(+), 96 deletions(-) create mode 100644 app/models/concerns/bulkrax/has_mapping_ext.rb create mode 100644 lib/bulkrax/transactions.rb create mode 100644 lib/bulkrax/transactions/container.rb diff --git a/app/models/concerns/bulkrax/has_mapping_ext.rb b/app/models/concerns/bulkrax/has_mapping_ext.rb new file mode 100644 index 00000000..45f18471 --- /dev/null +++ b/app/models/concerns/bulkrax/has_mapping_ext.rb @@ -0,0 +1,67 @@ +# frozen_string_literal: true + +module Bulkrax + module HasMappingExt + ## + # Field of the model that can be supported + def field_supported?(field) + field = field.gsub("_attributes", "") + + return false if excluded?(field) + return true if supported_bulkrax_fields.include?(field) + + property_defined = factory_class.singleton_methods.include?(:properties) && factory_class.properties[field].present? + + factory_class.method_defined?(field) && (Bulkrax::ValkyrieObjectFactory.schema_properties(factory_class).include?(field) || property_defined) + end + + ## + # Determine a multiple properties field + def multiple?(field) + @multiple_bulkrax_fields ||= + %W[ + file + remote_files + rights_statement + #{related_parents_parsed_mapping} + #{related_children_parsed_mapping} + ] + + return true if @multiple_bulkrax_fields.include?(field) + return false if field == "model" + + field_supported?(field) && (multiple_field?(field) || factory_class.singleton_methods.include?(:properties) && factory_class&.properties&.[](field)&.[]("multiple")) + end + + def multiple_field?(field) + form_definition = schema_form_definitions[field.to_sym] + form_definition.nil? ? false : form_definition.multiple? + end + + # override: we want to directly infer from a property being multiple that we should split when it's a String + # def multiple_metadata(content) + # return unless content + + # case content + # when Nokogiri::XML::NodeSet + # content&.content + # when Array + # content + # when Hash + # Array.wrap(content) + # when String + # String(content).strip.split(Bulkrax.multi_value_element_split_on) + # else + # Array.wrap(content) + # end + # end + + def schema_form_definitions + @schema_form_definitions ||= ::SchemaLoader.new.form_definitions_for(factory_class.name.underscore.to_sym) + end + end +end + +[Bulkrax::HasMatchers, Bulkrax::HasMatchers.singleton_class].each do |mod| + mod.prepend Bulkrax::HasMappingExt +end \ No newline at end of file diff --git a/lib/bulkrax/engine.rb b/lib/bulkrax/engine.rb index 063afddb..7fcd75af 100644 --- a/lib/bulkrax/engine.rb +++ b/lib/bulkrax/engine.rb @@ -17,6 +17,7 @@ class Engine < ::Rails::Engine require 'bulkrax/persistence_layer' require 'bulkrax/persistence_layer/active_fedora_adapter' if defined?(ActiveFedora) require 'bulkrax/persistence_layer/valkyrie_adapter' if defined?(Valkyrie) + require 'bulkrax/transactions' if defined?(Valkyrie) end config.generators do |g| diff --git a/lib/bulkrax/transactions.rb b/lib/bulkrax/transactions.rb new file mode 100644 index 00000000..44f817f8 --- /dev/null +++ b/lib/bulkrax/transactions.rb @@ -0,0 +1,18 @@ +# 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 \ No newline at end of file diff --git a/lib/bulkrax/transactions/container.rb b/lib/bulkrax/transactions/container.rb new file mode 100644 index 00000000..26407556 --- /dev/null +++ b/lib/bulkrax/transactions/container.rb @@ -0,0 +1,36 @@ +# frozen_string_literal: true +require 'dry/container' + +module Bulkrax + module Transactions + class Container + extend Dry::Container::Mixin + + namespace "work_resource" do |ops| + ops.register "create_with_bulk_behavior" do + steps = Hyrax::Transactions::WorkCreate::DEFAULT_STEPS.dup + steps[steps.index("work_resource.add_file_sets")] = "work_resource.add_bulkrax_files" + + Hyrax::Transactions::WorkCreate.new(steps: steps) + end + + ops.register "update_with_bulk_behavior" do + steps = Hyrax::Transactions::WorkUpdate::DEFAULT_STEPS.dup + steps[steps.index("work_resource.add_file_sets")] = "work_resource.add_bulkrax_files" + + Hyrax::Transactions::WorkUpdate.new(steps: steps) + end + + # TODO: uninitialized constant Bulkrax::Transactions::Container::InlineUploadHandler + # ops.register "add_file_sets" do + # Hyrax::Transactions::Steps::AddFileSets.new(handler: InlineUploadHandler) + # end + + ops.register "add_bulkrax_files" do + Bulkrax::Transactions::Steps::AddFiles.new + end + end + end + end +end +Hyrax::Transactions::Container.merge(Bulkrax::Transactions::Container) \ No newline at end of file diff --git a/lib/generators/bulkrax/templates/config/initializers/bulkrax.rb b/lib/generators/bulkrax/templates/config/initializers/bulkrax.rb index 7cf496a1..5b03b5c9 100644 --- a/lib/generators/bulkrax/templates/config/initializers/bulkrax.rb +++ b/lib/generators/bulkrax/templates/config/initializers/bulkrax.rb @@ -85,99 +85,4 @@ # rubocop:disable Style/IfUnlessModifier if Object.const_defined?(:Hyrax) && ::Hyrax::DashboardController&.respond_to?(:sidebar_partials) Hyrax::DashboardController.sidebar_partials[:repository_content] << "hyrax/dashboard/sidebar/bulkrax_sidebar_additions" -end - -# TODO: move outside of initializer? -class BulkraxTransactionContainer - extend Dry::Container::Mixin - - namespace "work_resource" do |ops| - ops.register "create_with_bulk_behavior" do - steps = Hyrax::Transactions::WorkCreate::DEFAULT_STEPS.dup - steps[steps.index("work_resource.add_file_sets")] = "work_resource.add_bulkrax_files" - - Hyrax::Transactions::WorkCreate.new(steps: steps) - end - - ops.register "update_with_bulk_behavior" do - steps = Hyrax::Transactions::WorkUpdate::DEFAULT_STEPS.dup - steps[steps.index("work_resource.add_file_sets")] = "work_resource.add_bulkrax_files" - - Hyrax::Transactions::WorkUpdate.new(steps: steps) - end - - # TODO: uninitialized constant BulkraxTransactionContainer::InlineUploadHandler - # ops.register "add_file_sets" do - # Hyrax::Transactions::Steps::AddFileSets.new(handler: InlineUploadHandler) - # end - - ops.register "add_bulkrax_files" do - Bulkrax::Transactions::Steps::AddFiles.new - end - end -end -Hyrax::Transactions::Container.merge(BulkraxTransactionContainer) - -# TODO: move outside of initializer? -module HasMappingExt - ## - # Field of the model that can be supported - def field_supported?(field) - field = field.gsub("_attributes", "") - - return false if excluded?(field) - return true if supported_bulkrax_fields.include?(field) - - property_defined = factory_class.singleton_methods.include?(:properties) && factory_class.properties[field].present? - - factory_class.method_defined?(field) && (Bulkrax::ValkyrieObjectFactory.schema_properties(factory_class).include?(field) || property_defined) - end - - ## - # Determine a multiple properties field - def multiple?(field) - @multiple_bulkrax_fields ||= - %W[ - file - remote_files - rights_statement - #{related_parents_parsed_mapping} - #{related_children_parsed_mapping} - ] - - return true if @multiple_bulkrax_fields.include?(field) - return false if field == "model" - - field_supported?(field) && (multiple_field?(field) || factory_class.singleton_methods.include?(:properties) && factory_class&.properties&.[](field)&.[]("multiple")) - end - - def multiple_field?(field) - form_definition = schema_form_definitions[field.to_sym] - form_definition.nil? ? false : form_definition.multiple? - end - - # override: we want to directly infer from a property being multiple that we should split when it's a String - # def multiple_metadata(content) - # return unless content - - # case content - # when Nokogiri::XML::NodeSet - # content&.content - # when Array - # content - # when Hash - # Array.wrap(content) - # when String - # String(content).strip.split(Bulkrax.multi_value_element_split_on) - # else - # Array.wrap(content) - # end - # end - - def schema_form_definitions - @schema_form_definitions ||= ::SchemaLoader.new.form_definitions_for(factory_class.name.underscore.to_sym) - end -end -[Bulkrax::HasMatchers, Bulkrax::HasMatchers.singleton_class].each do |mod| - mod.prepend HasMappingExt -end +end \ No newline at end of file From 6cbb00cb41783e07e8565c527d2880f5598f7554 Mon Sep 17 00:00:00 2001 From: Shana Moore Date: Wed, 24 Jan 2024 10:59:51 -0800 Subject: [PATCH 02/15] :broom: Add specs for container.rb, relocate files Co-Authored-By: LaRita Robinson --- .../bulkrax/transactions/steps/add_files.rb | 49 ------------------- app/transactions/bulkrax/container.rb | 34 +++++++++++++ app/transactions/bulkrax/steps/add_files.rb | 47 ++++++++++++++++++ .../transactions}/bulkrax/transactions.rb | 4 +- lib/bulkrax/engine.rb | 5 +- lib/bulkrax/transactions/container.rb | 36 -------------- spec/bulkrax/transactions/container_spec.rb | 31 ++++++++++++ 7 files changed, 118 insertions(+), 88 deletions(-) delete mode 100644 app/services/bulkrax/transactions/steps/add_files.rb create mode 100644 app/transactions/bulkrax/container.rb create mode 100644 app/transactions/bulkrax/steps/add_files.rb rename {lib => app/transactions}/bulkrax/transactions.rb (90%) delete mode 100644 lib/bulkrax/transactions/container.rb create mode 100644 spec/bulkrax/transactions/container_spec.rb diff --git a/app/services/bulkrax/transactions/steps/add_files.rb b/app/services/bulkrax/transactions/steps/add_files.rb deleted file mode 100644 index 2b9b1f62..00000000 --- a/app/services/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/container.rb b/app/transactions/bulkrax/container.rb new file mode 100644 index 00000000..ac595f26 --- /dev/null +++ b/app/transactions/bulkrax/container.rb @@ -0,0 +1,34 @@ +# frozen_string_literal: true +require 'dry/container' + +module Bulkrax + class Container + extend Dry::Container::Mixin + + namespace "work_resource" do |ops| + ops.register "create_with_bulk_behavior" do + steps = Hyrax::Transactions::WorkCreate::DEFAULT_STEPS.dup + steps[steps.index("work_resource.add_file_sets")] = "work_resource.add_bulkrax_files" + + Hyrax::Transactions::WorkCreate.new(steps: steps) + end + + ops.register "update_with_bulk_behavior" do + steps = Hyrax::Transactions::WorkUpdate::DEFAULT_STEPS.dup + steps[steps.index("work_resource.add_file_sets")] = "work_resource.add_bulkrax_files" + + Hyrax::Transactions::WorkUpdate.new(steps: steps) + end + + # TODO: uninitialized constant Bulkrax::Container::InlineUploadHandler + # ops.register "add_file_sets" do + # Hyrax::Transactions::Steps::AddFileSets.new(handler: InlineUploadHandler) + # end + + ops.register "add_bulkrax_files" do + Bulkrax::Steps::AddFiles.new + end + end + end +end +Hyrax::Transactions::Container.merge(Bulkrax::Container) \ No newline at end of file diff --git a/app/transactions/bulkrax/steps/add_files.rb b/app/transactions/bulkrax/steps/add_files.rb new file mode 100644 index 00000000..7e5c2607 --- /dev/null +++ b/app/transactions/bulkrax/steps/add_files.rb @@ -0,0 +1,47 @@ +# frozen_string_literal: true + +require "dry/monads" + +module Bulkrax + 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 diff --git a/lib/bulkrax/transactions.rb b/app/transactions/bulkrax/transactions.rb similarity index 90% rename from lib/bulkrax/transactions.rb rename to app/transactions/bulkrax/transactions.rb index 44f817f8..91f10b16 100644 --- a/lib/bulkrax/transactions.rb +++ b/app/transactions/bulkrax/transactions.rb @@ -1,5 +1,5 @@ # frozen_string_literal: true -require 'bulkrax/transactions/container' +require 'bulkrax/container' module Bulkrax ## @@ -15,4 +15,4 @@ module Bulkrax # @see https://dry-rb.org/gems/dry-transaction/ module Transactions end -end \ No newline at end of file +end diff --git a/lib/bulkrax/engine.rb b/lib/bulkrax/engine.rb index 7fcd75af..85eb11cf 100644 --- a/lib/bulkrax/engine.rb +++ b/lib/bulkrax/engine.rb @@ -5,6 +5,9 @@ module Bulkrax class Engine < ::Rails::Engine isolate_namespace Bulkrax + + config.eager_load_paths += %W[#{config.root}/app/transactions] + initializer :append_migrations do |app| if !app.root.to_s.match(root.to_s) && app.root.join('db/migrate').children.none? { |path| path.fnmatch?("*.bulkrax.rb") } config.paths["db/migrate"].expanded.each do |expanded_path| @@ -17,7 +20,7 @@ class Engine < ::Rails::Engine require 'bulkrax/persistence_layer' require 'bulkrax/persistence_layer/active_fedora_adapter' if defined?(ActiveFedora) require 'bulkrax/persistence_layer/valkyrie_adapter' if defined?(Valkyrie) - require 'bulkrax/transactions' if defined?(Valkyrie) + require 'bulkrax/transactions' if defined?(Hyrax::Transactions) end config.generators do |g| diff --git a/lib/bulkrax/transactions/container.rb b/lib/bulkrax/transactions/container.rb deleted file mode 100644 index 26407556..00000000 --- a/lib/bulkrax/transactions/container.rb +++ /dev/null @@ -1,36 +0,0 @@ -# frozen_string_literal: true -require 'dry/container' - -module Bulkrax - module Transactions - class Container - extend Dry::Container::Mixin - - namespace "work_resource" do |ops| - ops.register "create_with_bulk_behavior" do - steps = Hyrax::Transactions::WorkCreate::DEFAULT_STEPS.dup - steps[steps.index("work_resource.add_file_sets")] = "work_resource.add_bulkrax_files" - - Hyrax::Transactions::WorkCreate.new(steps: steps) - end - - ops.register "update_with_bulk_behavior" do - steps = Hyrax::Transactions::WorkUpdate::DEFAULT_STEPS.dup - steps[steps.index("work_resource.add_file_sets")] = "work_resource.add_bulkrax_files" - - Hyrax::Transactions::WorkUpdate.new(steps: steps) - end - - # TODO: uninitialized constant Bulkrax::Transactions::Container::InlineUploadHandler - # ops.register "add_file_sets" do - # Hyrax::Transactions::Steps::AddFileSets.new(handler: InlineUploadHandler) - # end - - ops.register "add_bulkrax_files" do - Bulkrax::Transactions::Steps::AddFiles.new - end - end - end - end -end -Hyrax::Transactions::Container.merge(Bulkrax::Transactions::Container) \ No newline at end of file diff --git a/spec/bulkrax/transactions/container_spec.rb b/spec/bulkrax/transactions/container_spec.rb new file mode 100644 index 00000000..f9a81ac4 --- /dev/null +++ b/spec/bulkrax/transactions/container_spec.rb @@ -0,0 +1,31 @@ +# frozen_string_literal: true + +require 'rails_helper' + +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::Steps::AddFiles } + end +end \ No newline at end of file From 02db0773e314036bc0adcb46cdcbcaf2f4991f08 Mon Sep 17 00:00:00 2001 From: Shana Moore Date: Wed, 24 Jan 2024 11:11:03 -0800 Subject: [PATCH 03/15] :broom: normalize magic strings into constants for referencing later MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Convert the create_with_bulk_behavior and update_with_bulk_behavior to a constant; that way we can reference it in IiifPrint and document the “magic” string. Co-Authored-By: LaRita Robinson --- app/factories/bulkrax/valkyrie_object_factory.rb | 8 ++++---- app/transactions/bulkrax/container.rb | 14 +++++++++----- 2 files changed, 13 insertions(+), 9 deletions(-) diff --git a/app/factories/bulkrax/valkyrie_object_factory.rb b/app/factories/bulkrax/valkyrie_object_factory.rb index 28af180c..eddbfe42 100644 --- a/app/factories/bulkrax/valkyrie_object_factory.rb +++ b/app/factories/bulkrax/valkyrie_object_factory.rb @@ -53,7 +53,7 @@ def create result = transaction .with_step_args( # "work_resource.add_to_parent" => {parent_id: @related_parents_parsed_mapping, user: @user}, - "work_resource.add_bulkrax_files" => { files: get_s3_files(remote_files: attributes["remote_files"]), user: @user }, + "work_resource.#{Bulkrax::Container::AddBulkraxFiles}" => { 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 } @@ -83,7 +83,7 @@ def update result = update_transaction .with_step_args( - "work_resource.add_bulkrax_files" => { files: get_s3_files(remote_files: attributes["remote_files"]), user: @user } + "work_resource.#{Bulkrax::Container::AddBulkraxFiles}" => { files: get_s3_files(remote_files: attributes["remote_files"]), user: @user } # TODO: uncomment when we upgrade Hyrax 4.x # 'work_resource.save_acl' => { permissions_params: [attrs.try('visibility') || 'open'].compact } @@ -165,12 +165,12 @@ def destroy_existing_files private def transaction - Hyrax::Transactions::Container["work_resource.create_with_bulk_behavior"] + Hyrax::Transactions::Container["work_resource.#{Bulkrax::Container::CreateWithBulkBehavior}"] end # Customize Hyrax::Transactions::WorkUpdate transaction with bulkrax def update_transaction - Hyrax::Transactions::Container["work_resource.update_with_bulk_behavior"] + Hyrax::Transactions::Container["work_resource.#{Bulkrax::Container::UpdateWithBulkBehavior}"] end # Query child FileSet in the resource/object diff --git a/app/transactions/bulkrax/container.rb b/app/transactions/bulkrax/container.rb index ac595f26..75c79b38 100644 --- a/app/transactions/bulkrax/container.rb +++ b/app/transactions/bulkrax/container.rb @@ -4,18 +4,22 @@ module Bulkrax class Container extend Dry::Container::Mixin + + CreateWithBulkBehavior = 'create_with_bulk_behavior'.freeze + UpdateWithBulkBehavior = 'update_with_bulk_behavior'.freeze + AddBulkraxFiles = 'add_bulkrax_files'.freeze namespace "work_resource" do |ops| - ops.register "create_with_bulk_behavior" do + ops.register CreateWithBulkBehavior do 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.#{Bulkrax::Container::AddBulkraxFiles}" Hyrax::Transactions::WorkCreate.new(steps: steps) end - ops.register "update_with_bulk_behavior" do + ops.register UpdateWithBulkBehavior do 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.#{Bulkrax::Container::AddBulkraxFiles}" Hyrax::Transactions::WorkUpdate.new(steps: steps) end @@ -25,7 +29,7 @@ class Container # Hyrax::Transactions::Steps::AddFileSets.new(handler: InlineUploadHandler) # end - ops.register "add_bulkrax_files" do + ops.register AddBulkraxFiles do Bulkrax::Steps::AddFiles.new end end From e8da6ae8bcc3f63cea5c2146e4da93d8a727d559 Mon Sep 17 00:00:00 2001 From: Shana Moore Date: Wed, 24 Jan 2024 11:44:03 -0800 Subject: [PATCH 04/15] :broom: correct camel case to constant notation for easier referencing Co-Authored-By: LaRita Robinson --- .../bulkrax/valkyrie_object_factory.rb | 8 ++--- app/transactions/bulkrax/container.rb | 36 ++++++++++--------- 2 files changed, 24 insertions(+), 20 deletions(-) diff --git a/app/factories/bulkrax/valkyrie_object_factory.rb b/app/factories/bulkrax/valkyrie_object_factory.rb index eddbfe42..634bca14 100644 --- a/app/factories/bulkrax/valkyrie_object_factory.rb +++ b/app/factories/bulkrax/valkyrie_object_factory.rb @@ -53,7 +53,7 @@ def create result = transaction .with_step_args( # "work_resource.add_to_parent" => {parent_id: @related_parents_parsed_mapping, user: @user}, - "work_resource.#{Bulkrax::Container::AddBulkraxFiles}" => { files: get_s3_files(remote_files: attributes["remote_files"]), user: @user }, + "work_resource.#{Bulkrax::Container::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 } @@ -83,7 +83,7 @@ def update result = update_transaction .with_step_args( - "work_resource.#{Bulkrax::Container::AddBulkraxFiles}" => { files: get_s3_files(remote_files: attributes["remote_files"]), user: @user } + "work_resource.#{Bulkrax::Container::ADD_BULKRAX_FILES}" => { files: get_s3_files(remote_files: attributes["remote_files"]), user: @user } # TODO: uncomment when we upgrade Hyrax 4.x # 'work_resource.save_acl' => { permissions_params: [attrs.try('visibility') || 'open'].compact } @@ -165,12 +165,12 @@ def destroy_existing_files private def transaction - Hyrax::Transactions::Container["work_resource.#{Bulkrax::Container::CreateWithBulkBehavior}"] + Hyrax::Transactions::Container["work_resource.#{Bulkrax::Container::CREATE_WITH_BULK_BEHAVIOR}"] end # Customize Hyrax::Transactions::WorkUpdate transaction with bulkrax def update_transaction - Hyrax::Transactions::Container["work_resource.#{Bulkrax::Container::UpdateWithBulkBehavior}"] + Hyrax::Transactions::Container["work_resource.#{Bulkrax::Container::UPDATE_WITH_BULK_BEHAVIOR}"] end # Query child FileSet in the resource/object diff --git a/app/transactions/bulkrax/container.rb b/app/transactions/bulkrax/container.rb index 75c79b38..09053544 100644 --- a/app/transactions/bulkrax/container.rb +++ b/app/transactions/bulkrax/container.rb @@ -4,24 +4,28 @@ module Bulkrax class Container extend Dry::Container::Mixin - - CreateWithBulkBehavior = 'create_with_bulk_behavior'.freeze - UpdateWithBulkBehavior = 'update_with_bulk_behavior'.freeze - AddBulkraxFiles = 'add_bulkrax_files'.freeze - namespace "work_resource" do |ops| - ops.register CreateWithBulkBehavior do - steps = Hyrax::Transactions::WorkCreate::DEFAULT_STEPS.dup - steps[steps.index("work_resource.add_file_sets")] = "work_resource.#{Bulkrax::Container::AddBulkraxFiles}" + ADD_BULKRAX_FILES = 'add_bulkrax_files' + CREATE_WITH_BULK_BEHAVIOR = 'create_with_bulk_behavior' + CREATE_WITH_BULK_BEHAVIOR_STEPS = begin + steps = Hyrax::Transactions::WorkCreate::DEFAULT_STEPS.dup + steps[steps.index("work_resource.add_file_sets")] = "work_resource.#{Bulkrax::Container::ADD_BULKRAX_FILES}" + steps + end.freeze + UPDATE_WITH_BULK_BEHAVIOR = 'update_with_bulk_behavior' + UPDATE_WITH_BULK_BEHAVIOR_STEPS = begin + steps = Hyrax::Transactions::WorkUpdate::DEFAULT_STEPS.dup + steps[steps.index("work_resource.add_file_sets")] = "work_resource.#{Bulkrax::Container::ADD_BULKRAX_FILES}" + steps + end.freeze - Hyrax::Transactions::WorkCreate.new(steps: steps) + 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 UpdateWithBulkBehavior do - steps = Hyrax::Transactions::WorkUpdate::DEFAULT_STEPS.dup - steps[steps.index("work_resource.add_file_sets")] = "work_resource.#{Bulkrax::Container::AddBulkraxFiles}" - - Hyrax::Transactions::WorkUpdate.new(steps: steps) + ops.register UPDATE_WITH_BULK_BEHAVIOR do + Hyrax::Transactions::WorkUpdate.new(steps: UPDATE_WITH_BULK_BEHAVIOR_STEPS) end # TODO: uninitialized constant Bulkrax::Container::InlineUploadHandler @@ -29,10 +33,10 @@ class Container # Hyrax::Transactions::Steps::AddFileSets.new(handler: InlineUploadHandler) # end - ops.register AddBulkraxFiles do + ops.register ADD_BULKRAX_FILES do Bulkrax::Steps::AddFiles.new end end end end -Hyrax::Transactions::Container.merge(Bulkrax::Container) \ No newline at end of file +Hyrax::Transactions::Container.merge(Bulkrax::Container) From e04d41bd074c032603014768bc4a743d601a10fb Mon Sep 17 00:00:00 2001 From: Shana Moore Date: Wed, 24 Jan 2024 11:49:38 -0800 Subject: [PATCH 05/15] :lipstick: rubocop fixes Co-Authored-By: LaRita Robinson --- app/models/concerns/bulkrax/has_mapping_ext.rb | 2 +- lib/generators/bulkrax/templates/config/initializers/bulkrax.rb | 2 +- spec/bulkrax/transactions/container_spec.rb | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/app/models/concerns/bulkrax/has_mapping_ext.rb b/app/models/concerns/bulkrax/has_mapping_ext.rb index 45f18471..61c94f2f 100644 --- a/app/models/concerns/bulkrax/has_mapping_ext.rb +++ b/app/models/concerns/bulkrax/has_mapping_ext.rb @@ -64,4 +64,4 @@ def schema_form_definitions [Bulkrax::HasMatchers, Bulkrax::HasMatchers.singleton_class].each do |mod| mod.prepend Bulkrax::HasMappingExt -end \ No newline at end of file +end diff --git a/lib/generators/bulkrax/templates/config/initializers/bulkrax.rb b/lib/generators/bulkrax/templates/config/initializers/bulkrax.rb index 5b03b5c9..731a35c5 100644 --- a/lib/generators/bulkrax/templates/config/initializers/bulkrax.rb +++ b/lib/generators/bulkrax/templates/config/initializers/bulkrax.rb @@ -85,4 +85,4 @@ # rubocop:disable Style/IfUnlessModifier if Object.const_defined?(:Hyrax) && ::Hyrax::DashboardController&.respond_to?(:sidebar_partials) Hyrax::DashboardController.sidebar_partials[:repository_content] << "hyrax/dashboard/sidebar/bulkrax_sidebar_additions" -end \ No newline at end of file +end diff --git a/spec/bulkrax/transactions/container_spec.rb b/spec/bulkrax/transactions/container_spec.rb index f9a81ac4..3dcecb8f 100644 --- a/spec/bulkrax/transactions/container_spec.rb +++ b/spec/bulkrax/transactions/container_spec.rb @@ -28,4 +28,4 @@ it { is_expected.to be_a Bulkrax::Steps::AddFiles } end -end \ No newline at end of file +end From 736ce793d73f95ebd2892ff98b81def569cc2c57 Mon Sep 17 00:00:00 2001 From: Jeremy Friesen Date: Wed, 24 Jan 2024 16:09:45 -0500 Subject: [PATCH 06/15] Update app/factories/bulkrax/valkyrie_object_factory.rb --- app/factories/bulkrax/valkyrie_object_factory.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/app/factories/bulkrax/valkyrie_object_factory.rb b/app/factories/bulkrax/valkyrie_object_factory.rb index 634bca14..be99b54a 100644 --- a/app/factories/bulkrax/valkyrie_object_factory.rb +++ b/app/factories/bulkrax/valkyrie_object_factory.rb @@ -164,6 +164,7 @@ def destroy_existing_files private + # TODO Rename to create_transaction def transaction Hyrax::Transactions::Container["work_resource.#{Bulkrax::Container::CREATE_WITH_BULK_BEHAVIOR}"] end From ed2ffb382e42861e99b700d6d057f2a63f56075c Mon Sep 17 00:00:00 2001 From: Jeremy Friesen Date: Wed, 24 Jan 2024 16:12:53 -0500 Subject: [PATCH 07/15] Update spec/bulkrax/transactions/container_spec.rb --- spec/bulkrax/transactions/container_spec.rb | 2 ++ 1 file changed, 2 insertions(+) diff --git a/spec/bulkrax/transactions/container_spec.rb b/spec/bulkrax/transactions/container_spec.rb index 3dcecb8f..1ccf7284 100644 --- a/spec/bulkrax/transactions/container_spec.rb +++ b/spec/bulkrax/transactions/container_spec.rb @@ -2,6 +2,8 @@ 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'] } From f573eb047ab41bb25ee7e19ab2927ef610fc9e63 Mon Sep 17 00:00:00 2001 From: LaRita Robinson Date: Wed, 24 Jan 2024 16:27:45 -0500 Subject: [PATCH 08/15] =?UTF-8?q?=F0=9F=A7=B9=20Move=20container=20&=20ste?= =?UTF-8?q?ps?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Match Hyrax convention by using bulkrax/transactions. --- .../bulkrax/valkyrie_object_factory.rb | 10 ++-- app/transactions/bulkrax/container.rb | 42 ---------------- app/transactions/bulkrax/steps/add_files.rb | 47 ------------------ app/transactions/bulkrax/transactions.rb | 2 +- .../bulkrax/transactions/container.rb | 44 +++++++++++++++++ .../bulkrax/transactions/steps/add_files.rb | 49 +++++++++++++++++++ spec/bulkrax/transactions/container_spec.rb | 2 +- 7 files changed, 100 insertions(+), 96 deletions(-) delete mode 100644 app/transactions/bulkrax/container.rb delete mode 100644 app/transactions/bulkrax/steps/add_files.rb create mode 100644 app/transactions/bulkrax/transactions/container.rb create mode 100644 app/transactions/bulkrax/transactions/steps/add_files.rb diff --git a/app/factories/bulkrax/valkyrie_object_factory.rb b/app/factories/bulkrax/valkyrie_object_factory.rb index be99b54a..97cbdb6c 100644 --- a/app/factories/bulkrax/valkyrie_object_factory.rb +++ b/app/factories/bulkrax/valkyrie_object_factory.rb @@ -53,7 +53,7 @@ def create result = transaction .with_step_args( # "work_resource.add_to_parent" => {parent_id: @related_parents_parsed_mapping, user: @user}, - "work_resource.#{Bulkrax::Container::ADD_BULKRAX_FILES}" => { files: get_s3_files(remote_files: attributes["remote_files"]), user: @user }, + "work_resource.#{Bulkrax::Transactions::Container::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 } @@ -83,7 +83,7 @@ def update result = update_transaction .with_step_args( - "work_resource.#{Bulkrax::Container::ADD_BULKRAX_FILES}" => { files: get_s3_files(remote_files: attributes["remote_files"]), user: @user } + "work_resource.#{Bulkrax::Transactions::Container::ADD_BULKRAX_FILES}" => { files: get_s3_files(remote_files: attributes["remote_files"]), user: @user } # TODO: uncomment when we upgrade Hyrax 4.x # 'work_resource.save_acl' => { permissions_params: [attrs.try('visibility') || 'open'].compact } @@ -164,14 +164,14 @@ def destroy_existing_files private - # TODO Rename to create_transaction + # TODO: Rename to create_transaction def transaction - Hyrax::Transactions::Container["work_resource.#{Bulkrax::Container::CREATE_WITH_BULK_BEHAVIOR}"] + Hyrax::Transactions::Container["work_resource.#{Bulkrax::Transactions::Container::CREATE_WITH_BULK_BEHAVIOR}"] end # Customize Hyrax::Transactions::WorkUpdate transaction with bulkrax def update_transaction - Hyrax::Transactions::Container["work_resource.#{Bulkrax::Container::UPDATE_WITH_BULK_BEHAVIOR}"] + Hyrax::Transactions::Container["work_resource.#{Bulkrax::Transactions::Container::UPDATE_WITH_BULK_BEHAVIOR}"] end # Query child FileSet in the resource/object diff --git a/app/transactions/bulkrax/container.rb b/app/transactions/bulkrax/container.rb deleted file mode 100644 index 09053544..00000000 --- a/app/transactions/bulkrax/container.rb +++ /dev/null @@ -1,42 +0,0 @@ -# frozen_string_literal: true -require 'dry/container' - -module Bulkrax - class Container - extend Dry::Container::Mixin - - ADD_BULKRAX_FILES = 'add_bulkrax_files' - CREATE_WITH_BULK_BEHAVIOR = 'create_with_bulk_behavior' - CREATE_WITH_BULK_BEHAVIOR_STEPS = begin - steps = Hyrax::Transactions::WorkCreate::DEFAULT_STEPS.dup - steps[steps.index("work_resource.add_file_sets")] = "work_resource.#{Bulkrax::Container::ADD_BULKRAX_FILES}" - steps - end.freeze - UPDATE_WITH_BULK_BEHAVIOR = 'update_with_bulk_behavior' - UPDATE_WITH_BULK_BEHAVIOR_STEPS = begin - steps = Hyrax::Transactions::WorkUpdate::DEFAULT_STEPS.dup - steps[steps.index("work_resource.add_file_sets")] = "work_resource.#{Bulkrax::Container::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: uninitialized constant Bulkrax::Container::InlineUploadHandler - # ops.register "add_file_sets" do - # Hyrax::Transactions::Steps::AddFileSets.new(handler: InlineUploadHandler) - # end - - ops.register ADD_BULKRAX_FILES do - Bulkrax::Steps::AddFiles.new - end - end - end -end -Hyrax::Transactions::Container.merge(Bulkrax::Container) diff --git a/app/transactions/bulkrax/steps/add_files.rb b/app/transactions/bulkrax/steps/add_files.rb deleted file mode 100644 index 7e5c2607..00000000 --- a/app/transactions/bulkrax/steps/add_files.rb +++ /dev/null @@ -1,47 +0,0 @@ -# frozen_string_literal: true - -require "dry/monads" - -module Bulkrax - 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 diff --git a/app/transactions/bulkrax/transactions.rb b/app/transactions/bulkrax/transactions.rb index 91f10b16..6efbedea 100644 --- a/app/transactions/bulkrax/transactions.rb +++ b/app/transactions/bulkrax/transactions.rb @@ -1,5 +1,5 @@ # frozen_string_literal: true -require 'bulkrax/container' +require 'bulkrax/transactions/container' module Bulkrax ## diff --git a/app/transactions/bulkrax/transactions/container.rb b/app/transactions/bulkrax/transactions/container.rb new file mode 100644 index 00000000..5d3f45d5 --- /dev/null +++ b/app/transactions/bulkrax/transactions/container.rb @@ -0,0 +1,44 @@ +# frozen_string_literal: true +require 'dry/container' + +module Bulkrax + module Transactions + class Container + extend Dry::Container::Mixin + + ADD_BULKRAX_FILES = 'add_bulkrax_files' + CREATE_WITH_BULK_BEHAVIOR = 'create_with_bulk_behavior' + CREATE_WITH_BULK_BEHAVIOR_STEPS = begin + steps = Hyrax::Transactions::WorkCreate::DEFAULT_STEPS.dup + steps[steps.index("work_resource.add_file_sets")] = "work_resource.#{Bulkrax::Transactions::Container::ADD_BULKRAX_FILES}" + steps + end.freeze + UPDATE_WITH_BULK_BEHAVIOR = 'update_with_bulk_behavior' + UPDATE_WITH_BULK_BEHAVIOR_STEPS = begin + steps = Hyrax::Transactions::WorkUpdate::DEFAULT_STEPS.dup + steps[steps.index("work_resource.add_file_sets")] = "work_resource.#{Bulkrax::Transactions::Container::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: uninitialized constant Bulkrax::Transactions::Container::InlineUploadHandler + # ops.register "add_file_sets" do + # Hyrax::Transactions::Steps::AddFileSets.new(handler: InlineUploadHandler) + # end + + ops.register ADD_BULKRAX_FILES do + Bulkrax::Transactions::Steps::AddFiles.new + end + 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 new file mode 100644 index 00000000..2b9b1f62 --- /dev/null +++ b/app/transactions/bulkrax/transactions/steps/add_files.rb @@ -0,0 +1,49 @@ +# 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/spec/bulkrax/transactions/container_spec.rb b/spec/bulkrax/transactions/container_spec.rb index 1ccf7284..3222a2d4 100644 --- a/spec/bulkrax/transactions/container_spec.rb +++ b/spec/bulkrax/transactions/container_spec.rb @@ -28,6 +28,6 @@ describe 'work_resource.add_bulkrax_files' do subject(:transaction_step) { described_class['work_resource.add_bulkrax_files'] } - it { is_expected.to be_a Bulkrax::Steps::AddFiles } + it { is_expected.to be_a Bulkrax::Transactions::Steps::AddFiles } end end From d78a16a875035ccd0f6d41ecf1b73f600e3f4be5 Mon Sep 17 00:00:00 2001 From: Shana Moore Date: Wed, 24 Jan 2024 14:42:00 -0800 Subject: [PATCH 09/15] restructure org to run specs locally receiving error when trying to run the entire spec suite due to restructuring files but not moving the spec file. --- spec/{ => transactions}/bulkrax/transactions/container_spec.rb | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename spec/{ => transactions}/bulkrax/transactions/container_spec.rb (100%) diff --git a/spec/bulkrax/transactions/container_spec.rb b/spec/transactions/bulkrax/transactions/container_spec.rb similarity index 100% rename from spec/bulkrax/transactions/container_spec.rb rename to spec/transactions/bulkrax/transactions/container_spec.rb From aa1f2f0361dab0c4b25af4857f23a91532df4822 Mon Sep 17 00:00:00 2001 From: Shana Moore Date: Wed, 24 Jan 2024 16:55:00 -0800 Subject: [PATCH 10/15] =?UTF-8?q?=F0=9F=9A=A7=20WIP:=20Consolidate=20HasMa?= =?UTF-8?q?tchers=20with=20HasMappingExt?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Remove HasMappingExt and consolidate logic within HasMatchers. HasMatchers should handle both cases, when objects are ActiveFedora vs Valkyrie. --- .../concerns/bulkrax/has_mapping_ext.rb | 67 ------------------- app/models/concerns/bulkrax/has_matchers.rb | 23 +++++-- spec/bulkrax/entry_spec_helper_spec.rb | 50 ++++++++++---- 3 files changed, 53 insertions(+), 87 deletions(-) delete mode 100644 app/models/concerns/bulkrax/has_mapping_ext.rb diff --git a/app/models/concerns/bulkrax/has_mapping_ext.rb b/app/models/concerns/bulkrax/has_mapping_ext.rb deleted file mode 100644 index 61c94f2f..00000000 --- a/app/models/concerns/bulkrax/has_mapping_ext.rb +++ /dev/null @@ -1,67 +0,0 @@ -# frozen_string_literal: true - -module Bulkrax - module HasMappingExt - ## - # Field of the model that can be supported - def field_supported?(field) - field = field.gsub("_attributes", "") - - return false if excluded?(field) - return true if supported_bulkrax_fields.include?(field) - - property_defined = factory_class.singleton_methods.include?(:properties) && factory_class.properties[field].present? - - factory_class.method_defined?(field) && (Bulkrax::ValkyrieObjectFactory.schema_properties(factory_class).include?(field) || property_defined) - end - - ## - # Determine a multiple properties field - def multiple?(field) - @multiple_bulkrax_fields ||= - %W[ - file - remote_files - rights_statement - #{related_parents_parsed_mapping} - #{related_children_parsed_mapping} - ] - - return true if @multiple_bulkrax_fields.include?(field) - return false if field == "model" - - field_supported?(field) && (multiple_field?(field) || factory_class.singleton_methods.include?(:properties) && factory_class&.properties&.[](field)&.[]("multiple")) - end - - def multiple_field?(field) - form_definition = schema_form_definitions[field.to_sym] - form_definition.nil? ? false : form_definition.multiple? - end - - # override: we want to directly infer from a property being multiple that we should split when it's a String - # def multiple_metadata(content) - # return unless content - - # case content - # when Nokogiri::XML::NodeSet - # content&.content - # when Array - # content - # when Hash - # Array.wrap(content) - # when String - # String(content).strip.split(Bulkrax.multi_value_element_split_on) - # else - # Array.wrap(content) - # end - # end - - def schema_form_definitions - @schema_form_definitions ||= ::SchemaLoader.new.form_definitions_for(factory_class.name.underscore.to_sym) - end - end -end - -[Bulkrax::HasMatchers, Bulkrax::HasMatchers.singleton_class].each do |mod| - mod.prepend Bulkrax::HasMappingExt -end diff --git a/app/models/concerns/bulkrax/has_matchers.rb b/app/models/concerns/bulkrax/has_matchers.rb index 3d50683b..9eb8933a 100644 --- a/app/models/concerns/bulkrax/has_matchers.rb +++ b/app/models/concerns/bulkrax/has_matchers.rb @@ -56,6 +56,10 @@ def add_metadata(node_name, node_content, index = nil) end end + def get_object_name(field) + mapping&.[](field)&.[]('object') + end + def set_parsed_data(name, value) return parsed_metadata[name] = value unless multiple?(name) @@ -125,9 +129,14 @@ def field_supported?(field) return false if excluded?(field) return true if supported_bulkrax_fields.include?(field) + property_defined = factory_class.singleton_methods.include?(:properties) && factory_class.properties[field].present? - factory_class.method_defined?(field) && (Bulkrax::ValkyrieObjectFactory.schema_properties(factory_class).include?(field) || property_defined) + if factory_class == Bulkrax::ValkyrieObjectFactory + factory_class.method_defined?(field) && (Bulkrax::ValkyrieObjectFactory.schema_properties(factory_class).include?(field) || property_defined) + else + factory_class.method_defined?(field) && factory_class.properties[field].present? + end end def supported_bulkrax_fields @@ -144,6 +153,8 @@ def supported_bulkrax_fields ] end + ## + # Determine a multiple properties field def multiple?(field) @multiple_bulkrax_fields ||= %W[ @@ -157,13 +168,17 @@ def multiple?(field) return true if @multiple_bulkrax_fields.include?(field) return false if field == 'model' - if factory.class.respond_to?(:schema) + if factory_class == Bulkrax::ValkyrieObjectFactory field_supported?(field) && valkyrie_multiple?(field) else field_supported?(field) && ar_multiple?(field) end end + def schema_form_definitions + @schema_form_definitions ||= ::SchemaLoader.new.form_definitions_for(factory_class.name.underscore.to_sym) + end + def ar_multiple?(field) factory_class.singleton_methods.include?(:properties) && factory_class&.properties&.[](field)&.[]("multiple") end @@ -174,10 +189,6 @@ def valkyrie_multiple?(field) factory_class.schema.key(sym_field).respond_to?(:of) if factory_class.fields.include?(sym_field) end - def get_object_name(field) - mapping&.[](field)&.[]('object') - end - # Hyrax field to use for the given import field # @param field [String] the importer field name # @return [Array] hyrax fields diff --git a/spec/bulkrax/entry_spec_helper_spec.rb b/spec/bulkrax/entry_spec_helper_spec.rb index 4ca872cc..f265e8ff 100644 --- a/spec/bulkrax/entry_spec_helper_spec.rb +++ b/spec/bulkrax/entry_spec_helper_spec.rb @@ -22,22 +22,44 @@ } end - let(:data) { { model: "Work", source_identifier: identifier, title: "If You Want to Go Far" } } - - it { is_expected.to be_a(Bulkrax::CsvEntry) } - - it "parses metadata" do - entry.build_metadata - - expect(entry.factory_class).to eq(Work) - { - "title" => ["If You Want to Go Far"], - "admin_set_id" => "admin_set/default", - "source" => [identifier] - }.each do |key, value| - expect(entry.parsed_metadata.fetch(key)).to eq(value) + context 'when ActiveFedora object' do + let(:data) { { model: "Work", source_identifier: identifier, title: "If You Want to Go Far" } } + + it { is_expected.to be_a(Bulkrax::CsvEntry) } + + it "parses metadata" do + entry.build_metadata + + expect(entry.factory_class).to eq(Work) + { + "title" => ["If You Want to Go Far"], + "admin_set_id" => "admin_set/default", + "source" => [identifier] + }.each do |key, value| + expect(entry.parsed_metadata.fetch(key)).to eq(value) + end end end + + # TODO: Add specs for when Bulkrax::ValkyrieObjectFactory is used + # context 'when Valkyrie object' do + # let(:data) { { model: "Work", source_identifier: identifier, title: "If You Want to Go Far" } } + + # it { is_expected.to be_a(Bulkrax::CsvEntry) } + + # it "parses metadata" do + # entry.build_metadata + + # expect(entry.factory_class).to eq(Work) + # { + # "title" => ["If You Want to Go Far"], + # "admin_set_id" => "admin_set/default", + # "source" => [identifier] + # }.each do |key, value| + # expect(entry.parsed_metadata.fetch(key)).to eq(value) + # end + # end + # end end context 'for parser_class_name: "Bulkrax::OaiDcParser"' do From f160e20178c9aae7de94f258b39b7e0ecfd1fd01 Mon Sep 17 00:00:00 2001 From: LaRita Robinson Date: Thu, 25 Jan 2024 12:51:09 -0500 Subject: [PATCH 11/15] =?UTF-8?q?=F0=9F=A7=B9=20Fix=20Specs=20&=20add=20Va?= =?UTF-8?q?lkyrie=20Specs?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- app/models/concerns/bulkrax/has_matchers.rb | 18 ++++--- spec/bulkrax/entry_spec_helper_spec.rb | 50 ++++++++++++------- spec/test_app/app/models/work_resource.rb | 6 +++ .../config/metadata/work_resource.yaml | 11 ++++ spec/test_app/db/schema.rb | 12 +++-- 5 files changed, 66 insertions(+), 31 deletions(-) create mode 100644 spec/test_app/app/models/work_resource.rb create mode 100644 spec/test_app/config/metadata/work_resource.yaml diff --git a/app/models/concerns/bulkrax/has_matchers.rb b/app/models/concerns/bulkrax/has_matchers.rb index 9eb8933a..c4bb1feb 100644 --- a/app/models/concerns/bulkrax/has_matchers.rb +++ b/app/models/concerns/bulkrax/has_matchers.rb @@ -130,10 +130,10 @@ def field_supported?(field) return false if excluded?(field) return true if supported_bulkrax_fields.include?(field) - property_defined = factory_class.singleton_methods.include?(:properties) && factory_class.properties[field].present? - - if factory_class == Bulkrax::ValkyrieObjectFactory - factory_class.method_defined?(field) && (Bulkrax::ValkyrieObjectFactory.schema_properties(factory_class).include?(field) || property_defined) + if Bulkrax.object_factory == Bulkrax::ValkyrieObjectFactory + # used in cases where we have a Fedora object class but use the Valkyrie object factory + property_defined = factory_class.singleton_methods.include?(:properties) && factory_class.properties[field].present? + factory_class.method_defined?(field) && (property_defined || Bulkrax::ValkyrieObjectFactory.schema_properties(factory_class).include?(field)) else factory_class.method_defined?(field) && factory_class.properties[field].present? end @@ -168,7 +168,7 @@ def multiple?(field) return true if @multiple_bulkrax_fields.include?(field) return false if field == 'model' - if factory_class == Bulkrax::ValkyrieObjectFactory + if Bulkrax.object_factory == Bulkrax::ValkyrieObjectFactory field_supported?(field) && valkyrie_multiple?(field) else field_supported?(field) && ar_multiple?(field) @@ -185,8 +185,12 @@ def ar_multiple?(field) def valkyrie_multiple?(field) # TODO: there has got to be a better way. Only array types have 'of' - sym_field = field.to_sym - factory_class.schema.key(sym_field).respond_to?(:of) if factory_class.fields.include?(sym_field) + if factory_class.respond_to?(:schema) + sym_field = field.to_sym + factory_class.schema.key(sym_field).respond_to?(:of) if factory_class.fields.include?(sym_field) + else + ar_multiple?(field) + end end # Hyrax field to use for the given import field diff --git a/spec/bulkrax/entry_spec_helper_spec.rb b/spec/bulkrax/entry_spec_helper_spec.rb index f265e8ff..d13cdd83 100644 --- a/spec/bulkrax/entry_spec_helper_spec.rb +++ b/spec/bulkrax/entry_spec_helper_spec.rb @@ -25,6 +25,10 @@ context 'when ActiveFedora object' do let(:data) { { model: "Work", source_identifier: identifier, title: "If You Want to Go Far" } } + before do + allow(Bulkrax).to receive(:object_factory).and_return(Bulkrax::ObjectFactory) + end + it { is_expected.to be_a(Bulkrax::CsvEntry) } it "parses metadata" do @@ -41,25 +45,33 @@ end end - # TODO: Add specs for when Bulkrax::ValkyrieObjectFactory is used - # context 'when Valkyrie object' do - # let(:data) { { model: "Work", source_identifier: identifier, title: "If You Want to Go Far" } } - - # it { is_expected.to be_a(Bulkrax::CsvEntry) } - - # it "parses metadata" do - # entry.build_metadata - - # expect(entry.factory_class).to eq(Work) - # { - # "title" => ["If You Want to Go Far"], - # "admin_set_id" => "admin_set/default", - # "source" => [identifier] - # }.each do |key, value| - # expect(entry.parsed_metadata.fetch(key)).to eq(value) - # end - # end - # end + context 'when using ValkyrieObjectFactory' do + ['Work', 'WorkResource'].each do |model_name| + + context "for #{model_name}" do + let(:data) { { model: model_name, source_identifier: identifier, title: "If You Want to Go Far" } } + + before do + allow(Bulkrax).to receive(:object_factory).and_return(Bulkrax::ValkyrieObjectFactory) + end + + it { is_expected.to be_a(Bulkrax::CsvEntry) } + + it "parses metadata" do + entry.build_metadata + + expect(entry.factory_class).to eq(model_name.constantize) + { + "title" => ["If You Want to Go Far"], + "admin_set_id" => "admin_set/default", + "source" => [identifier] + }.each do |key, value| + expect(entry.parsed_metadata.fetch(key)).to eq(value) + end + end + end + end + end end context 'for parser_class_name: "Bulkrax::OaiDcParser"' do diff --git a/spec/test_app/app/models/work_resource.rb b/spec/test_app/app/models/work_resource.rb new file mode 100644 index 00000000..a1ab399e --- /dev/null +++ b/spec/test_app/app/models/work_resource.rb @@ -0,0 +1,6 @@ +# frozen_string_literal: true + +class WorkResource < Hyrax::Work + include Hyrax::Schema(:basic_metadata) + include Hyrax::Schema(:work_resource) +end diff --git a/spec/test_app/config/metadata/work_resource.yaml b/spec/test_app/config/metadata/work_resource.yaml new file mode 100644 index 00000000..0a113b40 --- /dev/null +++ b/spec/test_app/config/metadata/work_resource.yaml @@ -0,0 +1,11 @@ +attributes: + source_identifier: + type: string + multiple: false + index_keys: + - "source_identifier_sim" + - "source_identifier_tesim" + form: + required: false + primary: false + multiple: false diff --git a/spec/test_app/db/schema.rb b/spec/test_app/db/schema.rb index 70580d6e..af0e23b5 100644 --- a/spec/test_app/db/schema.rb +++ b/spec/test_app/db/schema.rb @@ -2,11 +2,11 @@ # of editing this file, please use the migrations feature of Active Record to # incrementally modify your database, and then regenerate this schema definition. # -# Note that this schema.rb definition is the authoritative source for your -# database schema. If you need to create the application database on another -# system, you should be using db:schema:load, not running all the migrations -# from scratch. The latter is a flawed and unsustainable approach (the more migrations -# you'll amass, the slower it'll run and the greater likelihood for issues). +# This file is the source Rails uses to define your schema when running `bin/rails +# db:schema:load`. When creating a new database, `bin/rails db:schema:load` tends to +# be faster and is potentially less error prone than running all of your +# migrations from scratch. Old migrations may fail to apply correctly if those +# migrations use external dependencies or application code. # # It's strongly recommended that you check this file into your version control system. @@ -99,6 +99,8 @@ t.integer "total_file_set_entries", default: 0 t.integer "processed_works", default: 0 t.integer "failed_works", default: 0 + t.integer "processed_children", default: 0 + t.integer "failed_children", default: 0 t.index ["importer_id"], name: "index_bulkrax_importer_runs_on_importer_id" end From a239166d6d80da04298ab8136d41c8bf4ed26b14 Mon Sep 17 00:00:00 2001 From: LaRita Robinson Date: Thu, 25 Jan 2024 12:55:14 -0500 Subject: [PATCH 12/15] =?UTF-8?q?=20=F0=9F=A7=B9=20Fix=20Rubocop=20complai?= =?UTF-8?q?nt?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- spec/bulkrax/entry_spec_helper_spec.rb | 1 - 1 file changed, 1 deletion(-) diff --git a/spec/bulkrax/entry_spec_helper_spec.rb b/spec/bulkrax/entry_spec_helper_spec.rb index d13cdd83..1926366a 100644 --- a/spec/bulkrax/entry_spec_helper_spec.rb +++ b/spec/bulkrax/entry_spec_helper_spec.rb @@ -47,7 +47,6 @@ context 'when using ValkyrieObjectFactory' do ['Work', 'WorkResource'].each do |model_name| - context "for #{model_name}" do let(:data) { { model: model_name, source_identifier: identifier, title: "If You Want to Go Far" } } From 2ad87412dcab4dc938715c650ddd21a1ff4a179e Mon Sep 17 00:00:00 2001 From: LaRita Robinson Date: Thu, 25 Jan 2024 13:18:50 -0500 Subject: [PATCH 13/15] =?UTF-8?q?=F0=9F=A7=B9=20Address=20Valkyrie's=20det?= =?UTF-8?q?ermination=20of=20multiple=3F?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- app/models/concerns/bulkrax/has_matchers.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/app/models/concerns/bulkrax/has_matchers.rb b/app/models/concerns/bulkrax/has_matchers.rb index c4bb1feb..08873700 100644 --- a/app/models/concerns/bulkrax/has_matchers.rb +++ b/app/models/concerns/bulkrax/has_matchers.rb @@ -187,7 +187,8 @@ def valkyrie_multiple?(field) # TODO: there has got to be a better way. Only array types have 'of' if factory_class.respond_to?(:schema) sym_field = field.to_sym - factory_class.schema.key(sym_field).respond_to?(:of) if factory_class.fields.include?(sym_field) + return true if factory_class.schema.key(sym_field).primitive == Array + false else ar_multiple?(field) end From 3f90afe7fc3777909798cccad5eadb99df0e8882 Mon Sep 17 00:00:00 2001 From: LaRita Robinson Date: Thu, 25 Jan 2024 13:41:14 -0500 Subject: [PATCH 14/15] =?UTF-8?q?=F0=9F=A7=B9=20Address=20permitted=20attr?= =?UTF-8?q?ibutes?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In Valkyrie, we use the schema to identify the permitted attributes. All allowed attributes should be on the schema, so no additional attributes should be required. Also add a fallback for permitted attributes in case an ActiveFedora model class goes through the ValkyrieObjectFactory. This supports the case where we want to always force a Valkyrie resource to be created, regardless of the model name given. --- app/factories/bulkrax/valkyrie_object_factory.rb | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/app/factories/bulkrax/valkyrie_object_factory.rb b/app/factories/bulkrax/valkyrie_object_factory.rb index 97cbdb6c..d31b7128 100644 --- a/app/factories/bulkrax/valkyrie_object_factory.rb +++ b/app/factories/bulkrax/valkyrie_object_factory.rb @@ -109,15 +109,11 @@ def get_s3_files(remote_files: {}) end ## - # TODO: What else fields are necessary: %i[id edit_users edit_groups read_groups work_members_attributes]? - # Regardless of what the Parser gives us, these are the properties we are prepared to accept. + # We accept attributes based on the model schema def permitted_attributes - Bulkrax::ValkyrieObjectFactory.schema_properties(klass) + - %i[ - admin_set_id - title - visibility - ] + return Bulkrax::ValkyrieObjectFactory.schema_properties(klass) if klass.respond_to?(:schema) + # fallback to support ActiveFedora model name + klass.properties.keys.map(&:to_sym) + base_permitted_attributes end def apply_depositor_metadata(object, user) From b13f0bb4b94efff37db66cb98355a70b9b0c730a Mon Sep 17 00:00:00 2001 From: LaRita Robinson Date: Thu, 25 Jan 2024 13:49:38 -0500 Subject: [PATCH 15/15] =?UTF-8?q?=F0=9F=A7=B9=20Update=20TODO=20comment?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adjust TODO message because referring to a handler that doesn't exist anywhere is confusing. We may need to register steps for file sets once the behavior is implemented. --- app/transactions/bulkrax/transactions/container.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/transactions/bulkrax/transactions/container.rb b/app/transactions/bulkrax/transactions/container.rb index 5d3f45d5..7b6481f5 100644 --- a/app/transactions/bulkrax/transactions/container.rb +++ b/app/transactions/bulkrax/transactions/container.rb @@ -29,9 +29,9 @@ class Container Hyrax::Transactions::WorkUpdate.new(steps: UPDATE_WITH_BULK_BEHAVIOR_STEPS) end - # TODO: uninitialized constant Bulkrax::Transactions::Container::InlineUploadHandler + # TODO: Need to register step for uploads handler? # ops.register "add_file_sets" do - # Hyrax::Transactions::Steps::AddFileSets.new(handler: InlineUploadHandler) + # Hyrax::Transactions::Steps::AddFileSets.new # end ops.register ADD_BULKRAX_FILES do