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

Replace calls to ActiveFedora::Base.uri_to_id and .id_to_uri (removes 6 refs to ActiveFedora) #3821

Closed
elrayle opened this issue Jun 17, 2019 · 0 comments

Comments

@elrayle
Copy link
Contributor

elrayle commented Jun 17, 2019

Descriptive summary

ActiveFedora::Base.uri_to_id and ActiveFedora::Base.id_to_uri are used to translate back and forth between the AF uri and AF id.

These calls will be replaced in two steps. The first step will replace the ActiveFedora::Base calls with the same calls defined in Hyrax::Base in app/models/hyrax/base.rb. This Hyrax::Base versions of these methods will simply call the ActiveFedora::Base versions. This will isolate the refs to ActiveFedora to a single class.

The second step will rewrite the methods to not have a dependency on ActiveFedora.

NOTE: If app/models/hyrax/base.rb does not exist, create it as part of this issue.

ActiveFedora code

Active Fedora call being replaced...

Example from app/services/hyrax/list_source_exporter.rb

          parent_id = ActiveFedora::Base.uri_to_id(parent_url)

To locate, search code for ActiveFedora::Base.uri_to_id.

Replacement code

First pass

At least initially, define a service class to do these transformations defined in Hyrax::Base (see above).

module Hyrax
  class Base
    def self.uri_to_id(uri)
      ActiveFedora::Base.uri_to_id uri
    end

    def self.id_to_uri(id)
      ActiveFedora::Base.id_to_uri id
    end
  end
end

Example from app/services/hyrax/list_source_exporter.rb

          parent_id = Hyrax::Base.uri_to_id(parent_url)

Second pass

How to change this into Valkyrie native code? Potential locations for the actual transformations...

  • wings metadata adapter defines uri_to_id and id_to_uri methods
    • Challenge - Other metadata adapters (e.g. postgres) do not define these methods. Note that the Valkyrie defined Fedora adapter does, but presumably are only used by the Fedora adapters/query service.
    • If the methods are moved to the wings metadata adapter, can the usage be isolated to the wings adapters/query service and removed from Hyrax general code?
  • resource defines uri_to_id and id_to_uri methods
    • Challenge - What will these do when the metadata adapter is postgres or some other adapter without URIs?
  • somewhere else?

For the wings AF metadata adapter, it can continue to make the ActiveFedora::Base calls to these methods. It is not clear how this will be handled for other metadata adapters. This may involve a translation between the valkyrie URI and the alternate id, and vice versa.

It is not clear that other metadata adapters will necessarily have URIs.

Exploration required.

Considerations

  • The Fedora metadata adapter in Valkyrie has similar methods defined there. But those are for internal use only. The question is can we remove all refs to uri_to_id and id_to_uri from Hyrax code and only reference it within the wings adapter. Each use will need to be evaluated to see if it can be removed.
  • Do NOT replace this call if found in lib/wings directories.

Impact on Testing

Not expected to require test changes. All tests should pass with the replacement code.

Related Work

PR #3804 does a similar first part creating a wrapper for ActiveFedora for ActiveFedora::SolrService. The second part of replacing the ActiveFedora code has not yet been addressed, at the time of the writing of this issue.

There will be similar issues created that are also replacing ActiveFedora::Base methods.

@no-reply no-reply added this to the Wings milestone Sep 23, 2019
mjgiarlo added a commit that referenced this issue Jan 23, 2020
Connects to #3821
Connects to #3824

This branch handles the "first pass" suggestion in two issues (#3821 and #3824), isolating certain `ActiveFedora::Base` class method calls within a new wrapper class (`Hyrax::Base`). Those calls are `.id_to_uri`, `.uri_to_id`, and `.uncached`. This provides a single location in the codebase where these calls might be refactored, replaced, or removed in the "second pass" work.
no-reply pushed a commit that referenced this issue Mar 18, 2021
no-reply pushed a commit that referenced this issue Mar 18, 2021
no-reply pushed a commit that referenced this issue Mar 18, 2021
no-reply pushed a commit that referenced this issue Mar 18, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants