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

Orders members when converting to AF objects. #3697

Merged
merged 8 commits into from
Apr 4, 2019
2 changes: 1 addition & 1 deletion lib/wings/active_fedora_converter.rb
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,7 @@ def result
def convert_members(af_object)
return unless resource.respond_to?(:member_ids) && resource.member_ids
# TODO: It would be better to find a way to add the members without resuming all the member AF objects
resource.member_ids.each { |valkyrie_id| af_object.members << ActiveFedora::Base.find(valkyrie_id.id) }
resource.member_ids.each { |valkyrie_id| af_object.ordered_members << ActiveFedora::Base.find(valkyrie_id.id) }
end

def convert_member_of_collections(af_object)
Expand Down
16 changes: 16 additions & 0 deletions lib/wings/hydra/pcdm/models/concerns/pcdm_valkyrie_behavior.rb
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,22 @@ def method_missing(name, *args, &block)
def respond_to_missing?(_name, _include_private = false)
true
end

# @param valkyrie [Boolean] Should the returned ids be for Valkyrie or AF objects?
# @return [Enumerable<Hydra::Works::Work>] The works this work is contained in
# NOTE: This method avoids using the Hydra::Works version of parent_works because of Issue #361
def parent_works(valkyrie: false)
af_child = Wings::ActiveFedoraConverter.new(resource: self).convert
af_parents = af_child.member_of_works
return af_parents unless valkyrie
af_parents.map(&:valkyrie_resource)
end

# @param valkyrie [Boolean] Should the returned ids be for Valkyrie or AF objects?
# @return [Enumerable<String> | Enumerable<Valkyrie::ID>] The ids of the works this work is contained in
def parent_work_ids(valkyrie: false)
parent_works(valkyrie: valkyrie).map(&:id)
end
end
end
end
12 changes: 0 additions & 12 deletions lib/wings/hydra/works/models/concerns/work_valkyrie_behavior.rb
Original file line number Diff line number Diff line change
Expand Up @@ -24,18 +24,6 @@ def file_set?
false
end

# @param valkyrie [Boolean] Should the returned resources be Valkyrie or AF objects?
# @return [Enumerable<Hydra::Works::Work>] The works this work is in
def parent_works(valkyrie: false)
parent_objects(valkyrie: valkyrie).select(&:work?)
end

# @param valkyrie [Boolean] Should the returned ids be for Valkyrie or AF objects?
# @return [Enumerable<String> | Enumerable<Valkerie::ID] The ids of the works this work is in
def parent_work_ids(valkyrie: false)
parent_works(valkyrie: valkyrie).map(&:id)
end

# @param valkyrie [Boolean] Should the returned resources be Valkyrie or AF objects?
# @return [Enumerable<Hydra::Works::Work>] The works this work contains
def child_works(valkyrie: false)
Expand Down
13 changes: 8 additions & 5 deletions lib/wings/model_transformer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -49,10 +49,12 @@ def self.for(pcdm_object)
#
# @return [Array<Symbol>]
def self.relationship_keys_for(reflections:)
reflections
.keys
.reject { |k| k.to_s.include?('id') }
.map { |k| k.to_s.singularize + '_ids' }
relationships = reflections
.keys
.reject { |k| k.to_s.include?('id') }
.map { |k| k.to_s.singularize + '_ids' }
relationships.delete(:member_ids) # Remove here. Members will be extracted as ordered_members in attributes method.
relationships
end

##
Expand Down Expand Up @@ -199,7 +201,8 @@ def attributes
updated_at: pcdm_object.try(:modified_date),
embargo_id: pcdm_object.try(:embargo)&.id,
lease_id: pcdm_object.try(:lease)&.id,
visibility: pcdm_object.try(:visibility))
visibility: pcdm_object.try(:visibility),
member_ids: pcdm_object.try(:ordered_member_ids)) # We want members in order, so extract from ordered_members.
end
end
# rubocop:enable Style/ClassVars
Expand Down
15 changes: 11 additions & 4 deletions lib/wings/valkyrie/query_service.rb
Original file line number Diff line number Diff line change
Expand Up @@ -55,18 +55,25 @@ def find_all_of_model(model:)
end
end

# Find an array of record using Valkyrie IDs, and map them to Valkyrie Resources
# Find an array of record using Valkyrie IDs, and map them to Valkyrie Resources maintaining order based on given ids
# @param [Array<Valkyrie::ID, String>] ids
# @return [Array<Valkyrie::Resource>]
# NOTE: Ignores non-existent ids.
def find_many_by_ids(ids:)
ids.each do |id|
id = ::Valkyrie::ID.new(id.to_s) if id.is_a?(String)
validate_id(id)
end
ids = ids.uniq.map(&:to_s)
ActiveFedora::Base.where("id: (#{ids.join(' OR ')})").map do |obj|
resource_factory.to_resource(object: obj)
resources = []
ids.uniq.map(&:to_s).each do |id|
begin
af_object = ActiveFedora::Base.find(id)
resources << resource_factory.to_resource(object: af_object)
rescue ::ActiveFedora::ObjectNotFoundError, Ldp::Gone
next
end
end
resources
end

def find_by_alternate_identifier(alternate_identifier:)
Expand Down
10 changes: 7 additions & 3 deletions spec/wings/active_fedora_converter_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
require 'wings'
require 'wings/active_fedora_converter'

RSpec.describe Wings::ActiveFedoraConverter do
RSpec.describe Wings::ActiveFedoraConverter, :clean_repo do
subject(:converter) { described_class.new(resource: resource) }
let(:adapter) { Valkyrie::Persistence::Memory::MetadataAdapter.new }
let(:attributes) { { id: id } }
Expand Down Expand Up @@ -74,12 +74,16 @@
let(:work3) { build(:work, id: 'wk3', title: ['Work 3']) }

before do
work1.members = [work2, work3]
work1.ordered_members = [work2, work3]
work1.save!
end

it 'converts member_of_collection_ids back to af_object' do
expect(converter.convert.members.map(&:id)).to match_array [work2.id, work3.id]
expect(converter.convert.members.map(&:id)).to match_array [work3.id, work2.id]
end

it 'preserves order across conversion' do
expect(converter.convert.ordered_member_ids).to eq [work2.id, work3.id]
end
end
end
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@
let(:parent_work_resource) { resource }

before do
work1.members = [work2, work3, fileset1, fileset2]
work1.ordered_members = [work2, work3, fileset1, fileset2]
work1.save!
end

Expand Down Expand Up @@ -111,7 +111,7 @@
let(:parent_work_resource) { resource }

before do
work1.members = [work2, work3, fileset1, fileset2]
work1.ordered_members = [work2, work3, fileset1, fileset2]
work1.save!
end

Expand Down Expand Up @@ -147,7 +147,7 @@
let(:pcdm_object) { collection1 }

before do
collection1.members = [work1, work2, collection2, collection3]
collection1.ordered_members = [work1, work2, collection2, collection3]
collection1.save!
end

Expand Down Expand Up @@ -181,7 +181,7 @@
let(:pcdm_object) { collection1 }

before do
collection1.members = [work1, work2, collection2, collection3]
collection1.ordered_members = [work1, work2, collection2, collection3]
collection1.save!
end

Expand Down Expand Up @@ -213,8 +213,8 @@
let(:child_resource) { resource }

before do
work1.members = [work3, fileset1]
work2.members = [work3, fileset2]
work1.ordered_members = [work3, fileset1]
work2.ordered_members = [work3, fileset2]
work1.save!
work2.save!
end
Expand Down Expand Up @@ -254,9 +254,9 @@
let(:child_resource) { resource }

before do
collection1.members = [work1, collection3]
collection2.members = [work1, work2]
work3.members = [work1]
collection1.ordered_members = [work1, collection3]
collection2.ordered_members = [work1, work2]
work3.ordered_members = [work1]
collection1.save!
collection2.save!
end
Expand Down Expand Up @@ -296,9 +296,9 @@
let(:child_resource) { resource }

before do
collection1.members = [work1, collection3]
collection2.members = [work1, work2]
work3.members = [work1]
collection1.ordered_members = [work1, collection3]
collection2.ordered_members = [work1, work2]
work3.ordered_members = [work1]
collection1.save!
collection2.save!
work3.save!
Expand Down Expand Up @@ -330,4 +330,69 @@
expect { resource.a_missing_method }.to raise_error NoMethodError
end
end

describe '#parent_works' do
let(:pcdm_object) { work3 }
let(:child_work_resource) { resource }

before do
work1.ordered_members = [work3]
work2.ordered_members = [work3]
work1.save!
work2.save!
end

context 'when valkyrie resources requested' do
it 'returns parent works as valkyrie resources through pcdm_valkyrie_behavior' do
resources = child_work_resource.parent_works(valkyrie: true)
expect(resources.map(&:work?)).to all(be true)
expect(resources.map(&:id)).to match_valkyrie_ids_with_active_fedora_ids([work1.id, work2.id])
end
end
context 'when active fedora objects requested' do
it 'returns parent works as fedora objects through pcdm_valkyrie_behavior' do
af_objects = child_work_resource.parent_works(valkyrie: false)
expect(af_objects.map(&:work?)).to all(be true)
expect(af_objects.map(&:id)).to match_array [work1.id, work2.id]
end
end
context 'when return type is not specified' do
it 'returns parent works as fedora objects through pcdm_valkyrie_behavior' do
af_objects = child_work_resource.parent_works
expect(af_objects.map(&:work?)).to all(be true)
expect(af_objects.map(&:id)).to match_array [work1.id, work2.id]
end
end
end

describe '#parent_work_ids' do
let(:pcdm_object) { work3 }
let(:child_work_resource) { resource }

before do
work1.ordered_members = [work3]
work2.ordered_members = [work3]
work1.save!
work2.save!
end

context 'when valkyrie resources requested' do
it 'returns parent works as valkyrie resources through pcdm_valkyrie_behavior' do
resource_ids = child_work_resource.parent_work_ids(valkyrie: true)
expect(resource_ids).to match_valkyrie_ids_with_active_fedora_ids([work1.id, work2.id])
end
end
context 'when active fedora objects requested' do
it 'returns parent works as fedora objects through pcdm_valkyrie_behavior' do
af_object_ids = child_work_resource.parent_work_ids(valkyrie: false)
expect(af_object_ids).to match_array [work1.id, work2.id]
end
end
context 'when return type is not specified' do
it 'returns parent works as fedora objects through pcdm_valkyrie_behavior' do
af_object_ids = child_work_resource.parent_work_ids
expect(af_object_ids).to match_array [work1.id, work2.id]
end
end
end
end
Original file line number Diff line number Diff line change
Expand Up @@ -28,15 +28,14 @@
let(:child_file_set_resource) { resource }

before do
work1.members = [fileset1]
work2.members = [fileset1]
work1.ordered_members = [fileset1]
work2.ordered_members = [fileset1]
work1.save!
work2.save!
end

context 'when valkyrie resources requested' do
it 'returns parent works as valkyrie resources through file_set_valkyrie_behavior' do
pending "TODO: Implementation of this method for valkyrie"
resources = child_file_set_resource.parent_works(valkyrie: true)
expect(resources.map(&:work?)).to all(be true)
expect(resources.map(&:id)).to match_valkyrie_ids_with_active_fedora_ids([work1.id, work2.id])
Expand All @@ -63,15 +62,14 @@
let(:child_file_set_resource) { resource }

before do
work1.members = [fileset1]
work2.members = [fileset1]
work1.ordered_members = [fileset1]
work2.ordered_members = [fileset1]
work1.save!
work2.save!
end

context 'when valkyrie resources requested' do
it 'returns parent works as valkyrie resources through file_set_valkyrie_behavior' do
pending "TODO: Implementation of this method for valkyrie"
resource_ids = child_file_set_resource.parent_work_ids(valkyrie: true)
expect(resource_ids).to match_valkyrie_ids_with_active_fedora_ids([work1.id, work2.id])
end
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@
work4.save!
work5.ordered_members << work1
work5.save!
work1.members = [work2, work3, fileset1, fileset2]
work1.ordered_members = [work2, work3, fileset1, fileset2]
work1.save!
end

Expand Down