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

🎁 Add Bulkrax.persistence_adapter #895

Merged
merged 1 commit into from
Jan 19, 2024

Conversation

jeremyf
Copy link
Contributor

@jeremyf jeremyf commented Jan 17, 2024

🎁 Add Bulkrax.persistence_adapter

At present we have not incorporated using the
Bulkrax.persistence_adapter into the application. It is instead put
forward for discussion and review.

Why introduce the adapter strategy? Because there are several instances
of Bulkrax that are installed along various points of Hyrax
versions as well as non-Hyrax versions; which means that there are some
cases where we'll be using ActiveFedora and some where we'll be moving
towards Valkyrie.

The goal of the adapter is two fold:

  • Clarify and define the data interface between Bulkrax and the Main App
  • Allow for ease of maintaining Bulkrax releative to varying versions of
    Hyrax and Hyku; thus allowing folks to upgrade to new features of
    Bulkrax without mandating an upgrade of their entire persistence
    strategy.

Related to:

@jeremyf jeremyf force-pushed the introducing-persistence-layer-for-bulkrax branch from 7c726f2 to 64d873e Compare January 17, 2024 15:36
@jeremyf jeremyf added the minor-ver for release notes label Jan 17, 2024
@jeremyf
Copy link
Contributor Author

jeremyf commented Jan 17, 2024

@orangewolf does this approach make sense to proceed further? It's also something to consider for the iiif_print gem.

At present we have not incorporated using the
`Bulkrax.persistence_adapter` into the application.  It is instead put
forward for discussion and review.

Why introduce the adapter strategy?  Because there are several instances
of Bulkrax that are installed along various points of Hyrax
versions as well as non-Hyrax versions; which means that there are some
cases where we'll be using ActiveFedora and some where we'll be moving
towards Valkyrie.

The goal of the adapter is two fold:

- Clarify and define the data interface between Bulkrax and the Main App
- Allow for ease of maintaining Bulkrax releative to varying versions of
  Hyrax and Hyku; thus allowing folks to upgrade to new features of
  Bulkrax without mandating an upgrade of their entire persistence
  strategy.

Related to:

- scientist-softserv/hykuup_knapsack#35
@jeremyf jeremyf force-pushed the introducing-persistence-layer-for-bulkrax branch from 64d873e to 9652f27 Compare January 17, 2024 17:35
@jeremyf jeremyf merged commit 1f2cd7d into main Jan 19, 2024
6 checks passed
@jeremyf jeremyf deleted the introducing-persistence-layer-for-bulkrax branch January 19, 2024 17:19
jeremyf added a commit that referenced this pull request Jan 24, 2024
* main: (24 commits)
  Retry and delete take 2 (#894)
  🎁 Add `Bulkrax.persistence_adapter` (#895)
  💸 Mint v6.0.1 (#892)
  🐛 Fix #work_identifier_search_field logic (#891)
  💸 Bump to v6.0.0 (#889)
  make search string used to look up objects configurable (#884)
  💸 v5.5.0 (#888)
  unpin dry-monads. its not a dependency of bulkrax (#885)
  fix syntax error in ERB (#883)
  add support for Rails 6, Hyrax 4, and Blacklight 7 (#782)
  Reduce SQL calls when incrementing/decrementing run counters (#881)
  Update readme to remove references to samvera-labs (#880)
  add Compatibility section to readme (#879)
  🐛 Fix tabs for Hydra application (#875)
  Nav-tabs event scoping (#874)
  📚 Update docs in preparation for best practices seminar (#873)
  use the `GlobalID` library tooling to determine global id (#869)
  Avoid NoMethodError in Bulkrax::Importers::Controller#create. (#870)
  preparing to deploy v5.4.1 (#868)
  5.4.0-bug-fixes (#865)
  ...
@orangewolf
Copy link
Member

well... I got here eventually. I don't understand what this does that is different than the ObjectFactory. I'm not groking the need for both. Its not hurting anything at the moment, but we should talk soon before it takes off to far =-)

@jeremyf
Copy link
Contributor Author

jeremyf commented Feb 7, 2024

Thanks for the prompt, I think these methods could be added as class methods to the object factory and we'd be in good shape.

@jeremyf
Copy link
Contributor Author

jeremyf commented Feb 7, 2024

Here's an example of where we need a class method on the object factory:

def find_child_file_sets(work_ids)
work_ids.each do |id|
ActiveFedora::Base.find(id).file_set_ids.each { |fs_id| @file_set_ids << fs_id }
end
end

jeremyf added a commit that referenced this pull request Feb 7, 2024
In review of the code and in brief discussion with @orangewolf, the
methods of the persistence layer could be added to the object factory.

We already were configuring the corresponding object factory for each
implementation of Bulkrax; so leveraging that configuration made
tremendous sense.

The methods on the persistence layer remain helpful (perhaps necessary)
for documented reasons in the `Bulkrax::ObjectFactoryInterface` module.

See:

- #895 and its discussion
jeremyf added a commit that referenced this pull request Feb 7, 2024
* ♻️ Migrate persistence layer methods to object factory

In review of the code and in brief discussion with @orangewolf, the
methods of the persistence layer could be added to the object factory.

We already were configuring the corresponding object factory for each
implementation of Bulkrax; so leveraging that configuration made
tremendous sense.

The methods on the persistence layer remain helpful (perhaps necessary)
for documented reasons in the `Bulkrax::ObjectFactoryInterface` module.

See:

- #895 and its discussion

* 🎁 Add Valkyrie object factory interface methods

* 🧹 Favor interface based exception

Given that we are not directly exposing ActiveFedora nor Hyrax nor
Valkyrie objects, we want to translate/transform exceptions into a
common exception based on an interface.

That way downstream implementers can catch the Bulkrax specific error
and not need to do things such as `if
defined?(ActiveFedora::RecordInvalid) rescue
ActiveFedora::RecordInvalid`

It's just funny looking.
ShanaLMoore added a commit that referenced this pull request Apr 2, 2024
* create an object factory that supports Valkyrie

All code in this commit has been adapted from Surfliner:
https://github.com/surfliner/surfliner-mirror

* temp gem conflict workaround

* ⚙️ upgrade dry-monads dependency to ~> 1.5.0

Hyrax 4.0.0 requires a dependency upgrade for
dry-monads. I could not upgrade GBH's bulkrax without
doing this change.

- Issue:
- scientist-softserv/ams#77
- Ref:
- https://github.com/samvera/hyrax/blob/cbe9278b919485f90a37630d3f3157ecef59cd7c/hyrax.gemspec#L48

* 🧹 Add extra parameter for fill_in_blank_source_identifiers

gbh got an error that we were passing too many arguments
when setting the source_identifier in the bulkrax config.
ref:
- https://github.com/samvera-labs/bulkrax/wiki/Configuring-Bulkrax#source-identifier

* Revert ":broom: Add extra parameter for fill_in_blank_source_identifiers"

This reverts commit df96de6.

* 🧹 delegate create_parent_child_relationships from importer to parser

* allow ruby 3 syntax in migrations

* 🧹 change exists? to exist? to support Ruby 3.2

* 🚧 add support for Hyrax 5, valkyrie and ruby 3.2

* add temp workaround for blank title and creator

* ⚙️ Switch find methods with custom queries for Valkyrie

* hyrax 4 permission service does both valk and non-valk

* new bagit

* handle validation failure

* better failure detection for vaklyrie object

* fix validation message

* importer failure helpers

* improve multiple detection in matchers

* fix matcher on missing field

* rob cant remember that its include?

* Appeasing rubocop

* ♻️ Handle exist? and/or exists? for finding objects

See inline comments

* Add dry/monads require for specs

* I897 Bulkrax readiness for Hyku 6 and Hyrax 4 & 5 (#898)

* 🧹 relocates transactions from inititalizer file

Issue:
- #897

Co-Authored-By: LaRita Robinson <laritakr@users.noreply.github.com>

* 🧹 Add specs for container.rb, relocate files

Co-Authored-By: LaRita Robinson <laritakr@users.noreply.github.com>

* 🧹 normalize magic strings into constants for referencing later

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 <laritakr@users.noreply.github.com>

* 🧹 correct camel case to constant notation for easier referencing

Co-Authored-By: LaRita Robinson <laritakr@users.noreply.github.com>

* 💄 rubocop fixes

Co-Authored-By: LaRita Robinson <laritakr@users.noreply.github.com>

* Update app/factories/bulkrax/valkyrie_object_factory.rb

* Update spec/bulkrax/transactions/container_spec.rb

* 🧹 Move container & steps

Match Hyrax convention by using bulkrax/transactions.

* 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.

* 🚧 WIP: Consolidate HasMatchers with HasMappingExt

Remove HasMappingExt and consolidate logic within HasMatchers. HasMatchers should handle both cases, when objects are ActiveFedora vs Valkyrie.

* 🧹 Fix Specs & add Valkyrie Specs

* 🧹 Fix Rubocop complaint

* 🧹 Address Valkyrie's determination of multiple?

* 🧹 Address permitted attributes

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.

* 🧹 Update TODO comment

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.

---------

Co-authored-by: LaRita Robinson <laritakr@users.noreply.github.com>
Co-authored-by: Jeremy Friesen <jeremy.n.friesen@gmail.com>
Co-authored-by: LaRita Robinson <larita@scientist.com>

* 📚 Adding documentation for configuration (#896)

This builds on a [question asked in Slack][1]

[1]: https://samvera.slack.com/archives/C03S9FS60KW/p1705681632335919

* ♻️ Extract Bulkrax::FactoryClassFinder (#900)

This refactor introduces consolidating logic for determining an entry's
factory_class.

The goal is to begin to allow for us to have a CSV record that says
"model = Work" and to use a "WorkResource".

Note, there are downstream implementations that overwrite
`factory_class` and we'll need to consider how we approach that.

* 🐛 [i134] - Fix missing translations

Missing translations were evaluating to false.

Issue:
- scientist-softserv/hykuup_knapsack#134

* Renaming method for parity

* ♻️ Favor Bulkrax's persistence layer

Instead of direct calls to a deprecated service favor a persistence
layer call; one that defines an interface.

Note this means we need to implement the methods in the Valkyrie
adapter; but those should be trivial.

* ♻️ Favor Bulkrax.persistence_adapter over ActiveFedora::Base

* Moving methods to adapter pattern

* use find_by_source_identifier instead of find_by_bulkrax_identifier (#907)

* i903 - move bulkrax identifier custom queries into bulkrax

move bulkrax identifier custom queries into bulkrax

Issue:
- scientist-softserv/hykuup_knapsack#136

* make find_by_source_identifier dynamic

Import a csv with child works. The forming of relationships is not working. Part of the problem is the find_by_bulkrax_identifier call.

From GBH, this used to be find_by_bulkrax_identifier which not all clients will configure as their source identifier. Instead we need to ask for the source identifier and use that for the sql query. This commit goes along with a PR from Hyku which currently has the find_by_source_identifier.rb files defined.

Issue:
- scientist-softserv/hykuup_knapsack#128

Co-Authored-By: Kirk Wang <k3wang@gmail.com>

* remove files: they live in Hyku for now

Co-Authored-By: Kirk Wang <k3wang@gmail.com>

* 🧹 Place custom queries back in Bulkrax

* 🧹 remove misleading comment

Co-Authored-By: Kirk Wang <k3wang@gmail.com>

* 🧹 Entry is a required argument when initializing ObjectFactory

Fix for broken specs

Co-Authored-By: Kirk Wang <k3wang@gmail.com>

* revert changes to pass Entry arg

The object factory already has work_identifier: parser.work_identifier. we don't need the entry argument after all.

ref:
- https://github.com/samvera/bulkrax/blob/main/app/models/concerns/bulkrax/import_behavior.rb#L181

Co-Authored-By: Kirk Wang <k3wang@gmail.com>

---------

Co-authored-by: Kirk Wang <k3wang@gmail.com>
Co-authored-by: Kirk Wang <kirk.wang@scientist.com>

* 🧹 Make CreateRelationshipJob work for Valkyrie (#908)

* 🧹 Make the relationships job work for Valkyrie

This will add a relationships path for Valkyrie objects.  It also will
add a transactions call so set child flag will fire off in IIIF Print.

ref:
  - scientist-softserv/hykuup_knapsack#141

* 💄 rubocop fix

Co-Authored-By: Kirk Wang <k3wang@gmail.com>

* ♻️ Adjust rescue logic to move closer to error

This also adds some consideration for refactoring the queries to instead
use the persistence layer.

* Adding notes about transactions

---------

Co-authored-by: Shana Moore <shana@scientist.com>
Co-authored-by: Jeremy Friesen <jeremy.n.friesen@gmail.com>

* Add todo comment

Co-Authored-By: Kirk Wang <k3wang@gmail.com>

* 🎁 Switch transaction to listener

This commit will switch the membership transaction to a listener.

* ♻️ Migrate persistence layer methods to object factory (#911)

* ♻️ Migrate persistence layer methods to object factory

In review of the code and in brief discussion with @orangewolf, the
methods of the persistence layer could be added to the object factory.

We already were configuring the corresponding object factory for each
implementation of Bulkrax; so leveraging that configuration made
tremendous sense.

The methods on the persistence layer remain helpful (perhaps necessary)
for documented reasons in the `Bulkrax::ObjectFactoryInterface` module.

See:

- #895 and its discussion

* 🎁 Add Valkyrie object factory interface methods

* 🧹 Favor interface based exception

Given that we are not directly exposing ActiveFedora nor Hyrax nor
Valkyrie objects, we want to translate/transform exceptions into a
common exception based on an interface.

That way downstream implementers can catch the Bulkrax specific error
and not need to do things such as `if
defined?(ActiveFedora::RecordInvalid) rescue
ActiveFedora::RecordInvalid`

It's just funny looking.

* 🧹 Get exporters to work

This commit contains various changes to get the
exporters to work correctly.

* make updates work

* 🧹 Make DeleteJob work wth new class method .find (#912)

* 🧹 Make DeleteJob work wth new class method .find

The DeleteJob previously was not working with the
old factory#find method because when it is doing a
delete action, the parsed_metadata does not get
generated like during a regular import.  Because
of this, the #search_by_identifier method fails to
find anything because we don't have a
`work_identifier` field which would have came from
the parsed_metadata.  So instead, we are using the
new class method .find which will take an id
(which we find on the raw_metadata) to find the
object.  We make sure to reindex and publish the
action to any relevant listeners.

* 🎁 Implement a #delete method for the ObjectFactory

This commit will add a delete method to the ObjectFactory and the
ValkyrieObjectFactory so we can avoid unnecessary conditionals.

* 🧹 Rework factories to implement delete method

This cuts down on the method chaining.

* ♻️ Remove constant

This creates hard to parse chatter, and is not needed as we were relying
on it for IIIF Print to be able to reference.

* ♻️ Reworking structure

The Hyrax transactions create a lot of pre-amble and post-amble for
performing the save.  This commit attempts to consolidate logic to
reduce redundancy of that boilerplate.

Further, it adds handling for creating collections.

We still need to handle form validation.

* Adding index to schema

* ♻️ Favor asking about model_name over class (#934)

Given our effort at lazy migration in Bulkrax we want to do a bit more
sniffing regarding the objects.  This is not quite adequate for the
general case of Collections but it is an improvement.

Ideally we should be interrogating the class and asking
`klass.collection?` but there are some confounding edge cases around
routing that we are in this pickle.

```ruby
irb(main):002:0> CollectionResource.model_name
=>
 @collection="collections",
 @element="collection",
 @Human="Collection",
 @i18n_key=:collection,
 @klass=CollectionResource,
 @name="CollectionResource",
 @param_key="collection",
 @plural="collections",
 @route_key="collections",
 @Singular="collection",
 @singular_route_key="collection">
irb(main):003:0> Collection.model_name
=>
 @collection="collections",
 @element="collection",
 @Human="Collection",
 @i18n_key=:collection,
 @klass=Collection,
 @name="Collection",
 @param_key="collection",
 @plural="collections",
 @route_key="collections",
 @Singular="collection",
 @singular_route_key="collection">
irb(main):004:0>
```

* Favor object factory for find

* ♻️ Fix return value of transaction create

* Correct Hyrax.object_factory -> Bulkrax.object_factory

* Download cloud files later (#930)

* 🎁 Reschedule ImporterJob if downloads aren't done

This commit will add a check in the `ImporterJob` to see if the cloud
files finished downloading.  If they haven't, the job will be
rescheduled until they are.

* 🎁 Download Cloud Files later

This commit will bring in changes from `5.3.1-british_library` to move
the download of cloud files to a background job.

---------

Co-authored-by: Jeremy Friesen <jeremy.n.friesen@gmail.com>

* ♻️ Favor configuration over hard-coding and reaching assumptions

The main "flip" of logic is that we can remove the `curation_concern?`
method because we can instead ask "if Collection || FileSet" and infer
when that is false that we have a work.

This means removing the very reaching assumption of Hyku and it's
implementation foibles for work types.

* ♻️ Extract Bulkrax.collection_class_method

Instead of relying on the hard-coding, allow for configuration.

Co-authored-by: Shana Moore <shana.lavina.moore@gmail.com>

* ♻️ Favor Bulkrax.collection_model_class

Co-authored-by: Shana Moore <shana@scientist.com>

* ♻️ Favor Bulkrax.object_factory.find

Instead of relying on the direct call to a constant.

Co-authored-by: Shana Moore <shana@scientist.com>

* ♻️ Extract Bulkrax.object_factory.save! method for

We have a place where we try to call save! directly.  We do need to pass
a user for save event; hence the added method.

* ♻️ Favor using object_factory for save!

Co-authored-by: Shana Moore <shana@scientist.com>

* ♻️ Extract Hyrax.object_factory.search_by_property

There is a duplication of this logic elsewhere, but I first wanted to
extract common logic then begin extracting full replacement and
conforming object interface for Valkyrie.

* ♻️ Extract method for Valkyrization

We cannot directly query the class.  But must instead favor the
object_factory.

* 🎁 Adding query for find_by_model_and_property_value

* ♻️ Remove custom Valkyrie search_by_identifer

The super method was refined to use the class object factory; making it
redundant and flexible in the same manner as
`Bulkrax::ObjectFactory#search_by_identifer`.

* ♻️ Favor internal_resource definitions (when available)

* ♻️ Extract internal_resources method for curation concerns

* ♻️ Favor Bulkrax.object_factory and add fault tolerance

* Addressing TODO and minor refactoring

* I161 import collection resources (#933)

* 🚧 WIP: Import Collection Resource

A user should be able to import a collection resource. In this commit, we are able to successfully import and create collection resources. From the console we can see the collection formed relatioships with works, but the frontend's count and display shows 0 relationships. Additionally, we are unable to re run the importer without receiving errors on the collection entry.

TODO: specs, refactor,

Issue:
- scientist-softserv/hykuup_knapsack#161

* remove unused code

* refactor #conditionally_destroy_existing_files

This refactor was necessary because even though klass == ImageResource, which inherits from Valkyrie::Resouce through it's chain, klass === Valkyrie::Resource was returning false.

* exclude CollectionResource class from #destroy_existing_files

* WIP - try to import filesets with valkyrie resources

* Revert "WIP - try to import filesets with valkyrie resources"

This reverts commit 4ab31b6.

* 💄 rubocop fix

* i162 - import valkyrie works with filesets (#936)

* Revert "WIP - try to import filesets with valkyrie resources"

This reverts commit 4ab31b6.

* WIP

* WIP - try to import filesets with valkyrie resources

* 🚧 WIP: get filesets to import via bulkrax x valkyrie

* 🎉 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.

* 💄 rubocop fixes

* uncomment #get_s3_files call and add collections to configuration

* Update object_factory.rb

* ♻️ Move method and remove single instance definition

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.

* Revert changes due to refactor coming in from main

* address errors post big refactor

* Refactoring for consistent method signatures

Also avoiding setting an unused instance variable

* 🐛 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 🎉

* 🎁 Adding a new transaction step to handle different association

* ♻️ Extract update_index method to object factory

* ♻️ Extract object factory method

* ♻️ Extract add_resource_to_collection method

* ♻️ XIT out the mockery and stubbery of a spec

* ♻️ Extract method publish and add_child_to_parent_work

* ♻️ Rename method as it's not conditional

Yes, it is conditional but it operates on arrays that could be empty.

* Remove add to collection step

* 🐛 Fix publish parameter mismatch

* Removing custom transaction container.  We weren't using it

* Favor keyword args instead of hashes

* 💄fixing typo

* 🎁 Add update_collection to valkyrie object factory

* 💄 endless and ever appeasing of the coppers

---------

Co-authored-by: Jeremy Friesen <jeremy.n.friesen@gmail.com>

---------

Co-authored-by: Jeremy Friesen <jeremy.n.friesen@gmail.com>

* ♻️ Extract logic for add_user_to_collection_permissions

* 📚 Tidying documentation

* ♻️ Refactor Object Factories to leverage more inheritance

* ♻️ Extract abstract class for ObjectFactory

In constructing object inheritance, a more robust strategy is to create
an abstract class and then have classes directly extend that abstract
class.

This helps define and narrow an interface.

* ♻️ Move method to interface

This is used in both ObjectFactory and ValkyrieObjectFactory

* ♻️ Organizing code for Valkyrie Object Factory

* Refactoring method names for sorting order

* ♻️ Handle Valkyrie::Resource situation

* ♻️ Puzzling through implementation details

* ♻️ Extract method to enable removal of conditionals

* ♻️ Extract FileFactory::InnerWorkings

The goal of this extraction is to minimize the exposed interface of what
is quite complicated and state dependent logic.

* ♻️ Refactor to extract local variable

* Adding class attribute for Bulkrax::FileFactory

* ♻️ Adding inner methods for file factory interaction

* 🐛🏳️ post Big refactor fixes

Refactoring caused some bugs. At this point we are able to successfully import CSV works again.

* fix typo

* 🧹 Add case for `'collectionresource'`

In Valkyrie Hyku we're using CollectionResource and this was not being
recognized by the CSV parser.

* reload the object before calling persisted? on it

resolves failure saying that errors is undefined. object.persisted? returned false even though we could see that they got created in the UI.

* 💄 rubocop fix

* 🐛 Add return in ObjectFactory if valkyrie

Adding this early return here so we don't go down to the the #where and
trigger a NoMethodError. What it seems like it's doing is checking
Postgres for the object but if it doesn't find it then tries in Fedora,
however, Valkyrie object don't respond to #where so it throws an error.

* save parent object to establish relationships

This fixes the reason why works weren't forming relationships with other works

* Add FileSet branch to coercer conditional

This is in prep to handle Hyrax::FileSets being imports as rows.

* Add commit to clarify casecmp in CsvParser

* 🎁 Add ability to use tar.gz files

This commit will allow users to use tar.gz files as well as zip files
for importing.

* 🐛 Changing guard to #respond_to?(:where)

A spec was failing with the previous way we were checking.

* 🎁 Change glyphicon to font awesome

Hyrax 4+ applications use font awesome and not glyphicon. This commit
will convert all glyphicon to font awesome.

* Add require ruby-progressbar (#942)

Update bulkrax_tasks.rake

Fixes #941

* 🐛 Ensure we include visibility and other keywords for collection

Related to:

- scientist-softserv/hykuup_knapsack#182

Co-authored-by: LaRita Robinson <laritakr@users.noreply.github.com>

* 🐛 Fix visibility check on the object

This commit will add a guard for visibility because it is not on a
valkyrie resource.

* 🐛 Save provided visibility from CSV

CSV provided visibility was being clobbered in the ImportCollectionJob.

Refs scientist-softserv/hykuup_knapsack#182

* ♻️ Extract methods for better composition

* ♻️ Extracting object factory methods

I want to avoid having conditionals regarding object factories.  This
violates the polymorphism and means that other implementors that choose
a different `Bulkrax.object_factory` will have unintended consequences.

* 💄 endless and ever appeasing of the coppers

* ♻️ Favor object factory over hard-coded

* Amend the see/refer documentation for parser

* 💄 endless and ever appeasing of the coppers

* Updating test schema

* Remove transactions from initialization

* ♻️ Remove explicit calls to AdminSet

* 📚 Adding TODO items

---------

Co-authored-by: Benjamin Kiah Stroud <32469930+bkiahstroud@users.noreply.github.com>
Co-authored-by: Rob Kaufman <rob@notch8.com>
Co-authored-by: Kirk Wang <kirk.wang@scientist.com>
Co-authored-by: Jeremy Friesen <jeremy.n.friesen@gmail.com>
Co-authored-by: LaRita Robinson <laritakr@users.noreply.github.com>
Co-authored-by: LaRita Robinson <larita@scientist.com>
Co-authored-by: Kirk Wang <k3wang@gmail.com>
Co-authored-by: Dan Kerchner <kerchner@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
minor-ver for release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants