Skip to content

Commit

Permalink
Isolate select AF::Base calls
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
mjgiarlo committed Jan 23, 2020
1 parent 1b0afbd commit 1707bc0
Show file tree
Hide file tree
Showing 11 changed files with 65 additions and 12 deletions.
2 changes: 1 addition & 1 deletion app/actors/hyrax/actors/apply_order_actor.rb
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ def apply_order(curation_concern, new_order)
proxy.prev.next = curation_concern.ordered_member_proxies.last.next
break
end
proxy.proxy_for = ActiveFedora::Base.id_to_uri(new_order[index])
proxy.proxy_for = Hyrax::Base.id_to_uri(new_order[index])
proxy.target = nil
end
curation_concern.list_source.order_will_change!
Expand Down
18 changes: 18 additions & 0 deletions app/models/hyrax/base.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
# frozen_string_literal: true

module Hyrax
# Isolate calls to ActiveFedora::Base
class Base
def self.uncached(&block)
ActiveFedora::Base.uncached(&block)
end

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
6 changes: 3 additions & 3 deletions app/services/hyrax/adapters/nesting_index_adapter.rb
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ def self.find_preservation_document_by(id:)
def self.find_preservation_parent_ids_for(id:)
# Not everything is guaranteed to have library_collection_ids
# If it doesn't have it, what do we do?
fedora_object = ActiveFedora::Base.uncached do
fedora_object = Hyrax::Base.uncached do
fedora_object = ActiveFedora::Base.find(id)
end

Expand All @@ -47,7 +47,7 @@ def self.find_index_document_by(id:)
# rubocop:disable Lint/UnusedMethodArgument
def self.each_perservation_document_id_and_parent_ids(&block)
ActiveFedora::Base.descendant_uris(ActiveFedora.fedora.base_uri, exclude_uri: true).each do |uri|
id = ActiveFedora::Base.uri_to_id(uri)
id = Hyrax::Base.uri_to_id(uri)
object = ActiveFedora::Base.find(id)
parent_ids = object.try(:member_of_collection_ids) || []

Expand All @@ -70,7 +70,7 @@ def self.each_perservation_document_id_and_parent_ids(&block)
# @param nesting_document [Samvera::NestingIndexer::Documents::IndexDocument]
# @return Hash - the attributes written to the indexing layer
def self.write_nesting_document_to_index_layer(nesting_document:)
solr_doc = ActiveFedora::Base.uncached do
solr_doc = Hyrax::Base.uncached do
ActiveFedora::Base.find(nesting_document.id).to_solr # What is the current state of the solr document
end

Expand Down
2 changes: 1 addition & 1 deletion app/services/hyrax/graph_exporter.rb
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ def connection
# This method is called once for each statement in the graph.
def replacer
lambda do |resource_id, graph|
url = ActiveFedora::Base.id_to_uri(resource_id)
url = Hyrax::Base.id_to_uri(resource_id)
klass = graph.query([:s, ActiveFedora::RDF::Fcrepo::Model.hasModel, :o]).first.object.to_s.constantize

# if the subject URL matches
Expand Down
2 changes: 1 addition & 1 deletion app/services/hyrax/list_source_exporter.rb
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ def connection
# This method is called once for each statement in the graph.
def replacer
lambda do |resource_id, _graph|
parent_id = ActiveFedora::Base.uri_to_id(parent_url)
parent_id = Hyrax::Base.uri_to_id(parent_url)
return parent_url + resource_id.sub(parent_id, '') if resource_id.start_with?(parent_id)
Rails.application.routes.url_helpers.solr_document_url(resource_id, host: hostname)
end
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ def self.call(content, directives)
def self.retrieve_file_set(directives)
uri = URI(directives.fetch(:url))
raise ArgumentError, "#{uri} is not an http(s) uri" unless uri.is_a?(URI::HTTP)
Hyrax.query_service.find_by_alternate_identifier(alternate_identifier: ActiveFedora::Base.uri_to_id(uri.to_s), use_valkyrie: false)
Hyrax.query_service.find_by_alternate_identifier(alternate_identifier: Hyrax::Base.uri_to_id(uri.to_s), use_valkyrie: false)
end
private_class_method :retrieve_file_set

Expand Down
2 changes: 1 addition & 1 deletion app/services/hyrax/versioning_service.rb
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ def latest_version_of(file)
def versioned_file_id(file)
versions = file.versions.all
if versions.present?
ActiveFedora::File.uri_to_id versions.last.uri
Hyrax::Base.uri_to_id(versions.last.uri)
else
file.id
end
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
end

Riiif::Image.file_resolver.id_to_uri = lambda do |id|
ActiveFedora::Base.id_to_uri(CGI.unescape(id)).tap do |url|
Hyrax::Base.id_to_uri(CGI.unescape(id)).tap do |url|
Rails.logger.info "Riiif resolved #{id} to #{url}"
end
end
Expand Down
4 changes: 2 additions & 2 deletions lib/wings/valkyrie/query_service.rb
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@ def find_inverse_references_by(resource: nil, id: nil, property:)
raise ArgumentError, "Provide resource or id" unless resource || id
id ||= resource.alternate_ids.first
raise ArgumentError, "Resource has no id; is it persisted?" unless id
uri = ActiveFedora::Base.id_to_uri(id.to_s)
uri = Hyrax::Base.id_to_uri(id.to_s)
ActiveFedora::Base.where("+(#{property}_ssim: \"#{uri}\" OR #{property}_ssim: \"#{id}\")").map do |obj|
resource_factory.to_resource(object: obj)
end
Expand Down Expand Up @@ -160,7 +160,7 @@ def validate_id(id)
end

def find_id_for(reference)
return ::ActiveFedora::Base.uri_to_id(reference.id) if reference.class == ActiveTriples::Resource
return ::Hyrax::Base.uri_to_id(reference.id) if reference.class == ActiveTriples::Resource
return reference if reference.class == String
# not a supported type
''
Expand Down
35 changes: 35 additions & 0 deletions spec/models/hyrax/base_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
# frozen_string_literal: true

RSpec.describe Hyrax::Base do
describe '.uncached' do
let(:block) { proc { puts 'I do nothing' } }

it 'calls ActiveFedora::Base.uncached' do
allow(ActiveFedora::Base).to receive(:uncached)
described_class.uncached(&block)
expect(ActiveFedora::Base).to have_received(:uncached) do |&passed_block|
expect(passed_block).to eq(block)
end
end
end

describe '.id_to_uri' do
let(:id) { 'abc123' }

it 'calls ActiveFedora::Base.id_to_uri' do
allow(ActiveFedora::Base).to receive(:id_to_uri)
described_class.id_to_uri(id)
expect(ActiveFedora::Base).to have_received(:id_to_uri).with(id).once
end
end

describe '.uri_to_id' do
let(:uri) { 'https://foo.bar/abc123' }

it 'calls ActiveFedora::Base.uri_to_id' do
allow(ActiveFedora::Base).to receive(:uri_to_id)
described_class.uri_to_id(uri)
expect(ActiveFedora::Base).to have_received(:uri_to_id).with(uri).once
end
end
end
2 changes: 1 addition & 1 deletion spec/presenters/hyrax/file_set_presenter_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -313,7 +313,7 @@ def uri_segment_escape(uri)
let(:solr_document) { SolrDocument.new(file_set.to_solr) }
let(:request) { double(base_url: 'http://test.host') }
let(:presenter) { described_class.new(solr_document, ability, request) }
let(:id) { ActiveFedora::File.uri_to_id(file_set.original_file.versions.last.uri) }
let(:id) { Hyrax::Base.uri_to_id(file_set.original_file.versions.last.uri) }
let(:read_permission) { true }

before do
Expand Down

0 comments on commit 1707bc0

Please sign in to comment.