Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

♻️ Extract Bulkrax::FactoryClassFinder #900

Merged
merged 1 commit into from
Jan 25, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 1 addition & 3 deletions app/models/bulkrax/csv_collection_entry.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,7 @@

module Bulkrax
class CsvCollectionEntry < CsvEntry
def factory_class
Collection
end
self.default_work_type = "Collection"

# Use identifier set by CsvParser#unique_collection_identifier, which falls back
# on the Collection's first title if record[source_identifier] is not present
Expand Down
2 changes: 2 additions & 0 deletions app/models/bulkrax/entry.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ class OAIError < RuntimeError; end
class Entry < ApplicationRecord
include Bulkrax::HasMatchers
include Bulkrax::ImportBehavior
self.class_attribute :default_work_type, default: Bulkrax.default_work_type

include Bulkrax::ExportBehavior
include Bulkrax::StatusInfo
include Bulkrax::HasLocalProcessing
Expand Down
4 changes: 1 addition & 3 deletions app/models/bulkrax/oai_set_entry.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,7 @@

module Bulkrax
class OaiSetEntry < OaiEntry
def factory_class
Collection
end
self.default_work_type = "Collection"

def build_metadata
self.parsed_metadata = self.raw_metadata
Expand Down
5 changes: 1 addition & 4 deletions app/models/bulkrax/rdf_collection_entry.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

module Bulkrax
class RdfCollectionEntry < RdfEntry
self.default_work_type = "Collection"
def record
@record ||= self.raw_metadata
end
Expand All @@ -11,9 +12,5 @@ def build_metadata
add_local
return self.parsed_metadata
end

def factory_class
Collection
end
end
end
6 changes: 4 additions & 2 deletions app/models/concerns/bulkrax/file_set_entry_behavior.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,10 @@

module Bulkrax
module FileSetEntryBehavior
def factory_class
::FileSet
extend ActiveSupport::Concern

included do
self.default_work_type = "::FileSet"
end

def file_reference
Expand Down
20 changes: 4 additions & 16 deletions app/models/concerns/bulkrax/import_behavior.rb
Original file line number Diff line number Diff line change
Expand Up @@ -189,22 +189,10 @@ def factory
end

def factory_class
fc = if self.parsed_metadata&.[]('model').present?
self.parsed_metadata&.[]('model').is_a?(Array) ? self.parsed_metadata&.[]('model')&.first : self.parsed_metadata&.[]('model')
elsif self.mapping&.[]('work_type').present?
self.parsed_metadata&.[]('work_type').is_a?(Array) ? self.parsed_metadata&.[]('work_type')&.first : self.parsed_metadata&.[]('work_type')
else
Bulkrax.default_work_type
end

# return the name of the collection or work
fc.tr!(' ', '_')
fc.downcase! if fc.match?(/[-_]/)
fc.camelcase.constantize
rescue NameError
nil
rescue
Bulkrax.default_work_type.constantize
# ATTENTION: Do not memoize this here; tests should catch the problem, but through out the
# lifecycle of parsing a CSV row or what not, we end up having different factory classes based
# on the encountered metadata.
FactoryClassFinder.find(entry: self)
end
end
end
56 changes: 56 additions & 0 deletions app/services/bulkrax/factory_class_finder.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
# frozen_string_literal: true

module Bulkrax
class FactoryClassFinder
##
# @param entry [Bulkrax::Entry]
# @return [Class]
def self.find(entry:)
new(entry: entry).find
end

def initialize(entry:)
@entry = entry
end
attr_reader :entry

##
# @return [Class] when we are able to derive the class based on the {#name}.
# @return [Nil] when we encounter errors with constantizing the {#name}.
# @see #name
def find
# TODO: We have a string, now we want to consider how we coerce. Let's say we have Work and
# WorkResource in our upstream application. Work extends ActiveFedora::Base and is legacy.
# And WorkResource extends Valkyrie::Resource and is where we want to be moving. We may want
# to coerce the "Work" name into "WorkResource"
name.constantize
rescue NameError
nil
rescue
entry.default_work_type.constantize
end

##
# @api private
# @return [String]
def name
fc = if entry.parsed_metadata&.[]('model').present?
Array.wrap(entry.parsed_metadata['model']).first
elsif entry.importerexporter&.mapping&.[]('work_type').present?
# Because of delegation's nil guard, we're reaching rather far into the implementation
# details.
Array.wrap(entry.parsed_metadata['work_type']).first
else
# The string might be frozen, so lets duplicate
entry.default_work_type.dup
end

# Let's coerce this into the right shape.
fc.tr!(' ', '_')
fc.downcase! if fc.match?(/[-_]/)
fc.camelcase
rescue
entry.default_work_type
end
end
end
2 changes: 0 additions & 2 deletions lib/bulkrax.rb
Original file line number Diff line number Diff line change
Expand Up @@ -40,8 +40,6 @@ class Configuration
attr_accessor :fill_in_blank_source_identifiers

##
# Configure which persistence adapter you'd prefer to favor.
#
# @param adapter [Class<Bulkrax::PersistenceLayer::AbstractAdapter>]
attr_writer :persistence_adapter

Expand Down
5 changes: 5 additions & 0 deletions spec/models/bulkrax/csv_file_set_entry_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,11 @@ module Bulkrax
RSpec.describe CsvFileSetEntry, type: :model do
subject(:entry) { described_class.new }

describe '#default_work_type' do
subject { entry.default_work_type }
it { is_expected.to eq("::FileSet") }
end

describe '#file_reference' do
context 'when parsed_metadata includes the "file" property' do
before do
Expand Down
17 changes: 17 additions & 0 deletions spec/models/bulkrax/rdf_file_set_entry_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
# frozen_string_literal: true

require 'rails_helper'

module Bulkrax
RSpec.describe RdfFileSetEntry, type: :model do
describe '#default_work_type' do
subject { described_class.new.default_work_type }
it { is_expected.to eq("::FileSet") }
end

describe '#factory_class' do
subject { described_class.new.factory_class }
it { is_expected.to eq(::FileSet) }
end
end
end
2 changes: 2 additions & 0 deletions spec/spec_helper.rb
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
# frozen_string_literal: true

require 'dry/monads'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jeremyf this dry-monads is now a dev dependency of bulkrax with this change. but I dont see where you are actually using it? can this come out or do we need to add it to the gemspec?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Without the require 'dry/monads' the specs failed to load. This was the quickest adjustment.

Looking at it now, my assumption is that dry/monads is a development dependency in that we are creating transactions and merging those transactions.

So the shorter solution would be to move this to a development dependency.


# This file was generated by the `rails generate rspec:install` command. Conventionally, all
# specs live under a `spec` directory, which RSpec adds to the `$LOAD_PATH`.
# The generated `.rspec` file contains `--require spec_helper` which will cause
Expand Down
Loading