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 a factory for Valkyrie::Resources generated from AF objects #3486

Merged
merged 7 commits into from
Jan 30, 2019
Merged

Conversation

no-reply
Copy link
Member

@no-reply no-reply commented Jan 29, 2019

This adds a Valkyrie dependency, and is part of the Hyrax::Valkyrie sprint at Penn State (see Wiki Entry)

Please discuss.

@samvera/hyrax-code-reviewers

Add a factory for Valkyrie::Resources generated from AF objects

2c08320

Pinning to Valkyrie ~> 1.4

ace6f62

In debugging and trouble-shooting all the things related to Valkyrie,
the recent Dry releases, this appears to get the specs written in the
prior commit (originally SHA1 5a2906a)
by tomjohnson@ucsb.edu with title:

"Add a factory for Valkyrie::Resources generated from AF objects"

Normalize ActiveFedora attributes when mapping to Valkyrie properties

f5ea689

Enabling dynamic classes to persist/reify via Valkyrie

b8f1587

Related to samvera/valkyrie#654

Based on the current implementation of the dynamic Valkyrie::Resource,
and the reported issue in Valkyrie (samvera/valkyrie#647)
I'm looking at the following steps:

Step #1: Add #to_s to dynamic Valkyrie::Resource; This method is used
to define the class_name hat is persisted in Postgres.
When Valkyrie reifies the object from Postgres, it maps the
persisted record to the persisted class_name. The #to_s
method, by convention, should use the given PCDM Object's class
as a prefix (eg. given a GenericWork object, the dynamic
Valkyrie::Resource.to_s should be something like
"GenericWorkAsValkyrieResource" )

Question: Could we store the value as GenericWork and allow the
          below lambda to resolve that as the dynamic
          Valkyrie::Resource for a GenericWork?
Answer: In discussion with @revgum, @no-reply, @kelynch we believe
        it to be helpful to say that the "class/object" that is
        sent to the ORM persistence adapter of Valkyrie should not
        be assumed to be the ruby class used to transport the data
        from the application to the persistence layer, but should
        instead be the model used in the application for
        representing the data. In other words, we may have multiple
        objects that are used to persist the same conceptual
        application data object.

Step #2: In a local change to Valkyrie (github-647 branch), I added a
lambda to allow customization of how to convert the given
class_name to a proper class (the default implementation is
class_name.constantize). The implementation for finding the
dynamic Valkyrie::Resource needs to be able to take the
persisted class_name and find the corresponding dynamic
Valkyrie::Resource; The resolved dynamic class for reification
need not have the same object_id that was used to persist the
data, but should be built using the same logic that was used to
create the Valkyrie::Resource that was used to persist data.

Move Valkyrie integration to Wings

9e69c6e

Let it be known: "Hyrax with Wings" is the name of the project.

Setting Valkyrie as a development dependency

7d789f9

Steps to test:

  • Generate a new Rails application (using defaults)
  • Add Hyrax to the Gemfile (as per README documentation)
  • Bundle new Rails application
  • Verify that bundle exec gem list | grep valkyrie is blank (eg.
    Valkyrie is not loaded as a dependency)
  • Spin up rails server on the new Rails application
  • Then review log/development.log and look for the info message from
    the Hyrax::Engine.initializer (e.g. "Hyrax::Engine.initializer did
    not load Valkyrie")

In particular, we do not want to force upstream dependencies on
Postgres.

This is related to:

Creating the Wings namespace file

f701944

Responsible for coordinating the wings related requires.

@revgum
Copy link
Contributor

revgum commented Jan 29, 2019

I like it.. a good starting point for the Valkyrie<->AF work and doesn't impact any existing code.

Tom Johnson and others added 3 commits January 30, 2019 06:31
In debugging and trouble-shooting all the things related to Valkyrie,
the recent Dry releases, this appears to get the specs written in the
prior commit (originally SHA1 5a2906a)
by tomjohnson@ucsb.edu with title:

"Add a factory for `Valkyrie::Resources` generated from AF objects"
Related to samvera/valkyrie#654

Based on the current implementation of the dynamic Valkyrie::Resource,
and the reported issue in Valkyrie (samvera/valkyrie#647)
I'm looking at the following steps:

Step #1: Add `#to_s` to dynamic Valkyrie::Resource; This method is used
        to define the `class_name` hat is persisted in Postgres.
        When Valkyrie reifies the object from Postgres, it maps the
        persisted record to the persisted `class_name`. The `#to_s`
        method, by convention, should use the given PCDM Object's class
        as a prefix (eg. given a GenericWork object, the dynamic
        Valkyrie::Resource.to_s should be something like
        "GenericWorkAsValkyrieResource"  )

    Question: Could we store the value as GenericWork and allow the
              below lambda to resolve that as the dynamic
              Valkyrie::Resource for a GenericWork?
    Answer: In discussion with @revgum, @no-reply, @kelynch we believe
            it to be helpful to say that the "class/object" that is
            sent to the ORM persistence adapter of Valkyrie should not
            be assumed to be the ruby class used to transport the data
            from the application to the persistence layer, but should
            instead be the model used in the application for
            representing the data. In other words, we may have multiple
            objects that are used to persist the same conceptual
            application data object.

Step #2: In a local change to Valkyrie (github-647 branch), I added a
        lambda to allow customization of how to convert the given
        `class_name` to a proper class (the default implementation is
        `class_name.constantize`). The implementation for finding the
        dynamic Valkyrie::Resource needs to be able to take the
        persisted `class_name` and find the corresponding dynamic
        Valkyrie::Resource; The resolved dynamic class for reification
        need not have the same `object_id` that was used to persist the
        data, but should be built using the same logic that was used to
        create the Valkyrie::Resource that was used to persist data.
Let it be known: "Hyrax with Wings" is the name of the project.
Steps to test:

* Generate a new Rails application (using defaults)
* Add Hyrax to the Gemfile (as per README documentation)
* Bundle new Rails application
* Verify that `bundle exec gem list | grep valkyrie` is blank (eg.
  Valkyrie is not loaded as a dependency)
* Spin up `rails server` on the new Rails application
* Then review `log/development.log` and look for the info message from
  the `Hyrax::Engine.initializer` (e.g. "Hyrax::Engine.initializer did
  not load Valkyrie")

In particular, we do not want to force upstream dependencies on
Postgres.

This is related to:

* samvera/valkyrie#640
* samvera/valkyrie#655
Responsible for coordinating the wings related requires.
Copy link
Contributor

@laritakr laritakr left a comment

Choose a reason for hiding this comment

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

Work is still pending on monkey patch but it isn't currently used in this PR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants