From 1e379be59b0cf3f5365a508b67c6e11730d4755a Mon Sep 17 00:00:00 2001 From: LaRita Robinson Date: Fri, 11 Oct 2024 17:26:16 -0400 Subject: [PATCH 1/4] Add failing specs to fix --- spec/wings/active_fedora_converter_spec.rb | 11 ++++++++++- spec/wings/attribute_transformer_spec.rb | 13 ++++++++++--- 2 files changed, 20 insertions(+), 4 deletions(-) diff --git a/spec/wings/active_fedora_converter_spec.rb b/spec/wings/active_fedora_converter_spec.rb index 17b7f1c687..fc4ca0aba5 100644 --- a/spec/wings/active_fedora_converter_spec.rb +++ b/spec/wings/active_fedora_converter_spec.rb @@ -124,7 +124,10 @@ end context 'when given a valkyrie native model' do - let(:resource) { klass.new(title: ['comet in moominland'], distant_relation: ['Snufkin']) } + let(:based_near) { "https://sws.geonames.org/4920808/" } + let(:resource) { klass.new(title: ['comet in moominland'], + distant_relation: ['Snufkin'], + based_near: [based_near]) } let(:klass) { Hyrax::Test::Converter::Resource } before do @@ -133,6 +136,7 @@ module Converter class Resource < Hyrax::Resource attribute :title, Valkyrie::Types::Array.of(Valkyrie::Types::String) attribute :distant_relation, Valkyrie::Types::String + attribute :based_near, Valkyrie::Types::Array.of(Valkyrie::Types::String) end end end @@ -154,6 +158,11 @@ class Resource < Hyrax::Resource .to have_attributes(title: ['comet in moominland'], distant_relation: ['Snufkin']) end + it 'converts based_near to an RDF::URI' do + converted_work = converter.convert + expect(converted_work.based_near.first).to be_a(Hyrax::ControlledVocabularies::Location) + end + it 'supports indexing' do expect(converter.convert.indexing_service).to be_a Hyrax::Indexers::ResourceIndexer end diff --git a/spec/wings/attribute_transformer_spec.rb b/spec/wings/attribute_transformer_spec.rb index 310014cb1f..9d8a460c79 100644 --- a/spec/wings/attribute_transformer_spec.rb +++ b/spec/wings/attribute_transformer_spec.rb @@ -6,20 +6,27 @@ RSpec.describe Wings::AttributeTransformer, :active_fedora do let(:id) { 'moomin123' } + let(:based_near) { Hyrax::ControlledVocabularies::Location.new("https://sws.geonames.org/4920808/") } let(:work) { GenericWork.new(id: id, **attributes) } let(:attributes) do { title: ['fake title', 'fake title 2'], contributor: ['user1'], - description: ['a description'] + description: ['a description'], + based_near:[based_near] } end - it "transform the attributes" do - expect(described_class.run(work)) + it "transforms the attributes" do + converted_work = described_class.run(work) + expect(converted_work) .to include title: work.title, contributor: work.contributor.first, description: work.description.first + # ensure that based_near is converted from RDF::URI to URI string during valkyrization + converted_based_near = Array.wrap(converted_work[:based_near]).first + expect(converted_based_near).to be_a(String) + expect(converted_based_near).to eq(work.based_near.first.id) end end From a89b634cbf11954a3e730da100423512022fedad Mon Sep 17 00:00:00 2001 From: LaRita Robinson Date: Fri, 11 Oct 2024 18:17:45 -0400 Subject: [PATCH 2/4] Maps location for wings migrations - Native Valkyrie objects contain a URI string, not an RDF::URI - Fedora contains Hyrax::ControlledVocabularies::Location objects Modifies the behavior of the `based_near` attribute in the `to_resource` and `from_resource` methods. When using wings to convert between resources & fedora works, we were previously storing the RDF::URI in Valkyrie. This PR updates the behavior to consistently return values consistent with what would be seen in native objects. --- lib/wings/active_fedora_converter.rb | 9 +++++++++ lib/wings/transformer_value_mapper.rb | 6 +++++- 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/lib/wings/active_fedora_converter.rb b/lib/wings/active_fedora_converter.rb index a9e207bf4a..2b41a53700 100644 --- a/lib/wings/active_fedora_converter.rb +++ b/lib/wings/active_fedora_converter.rb @@ -149,6 +149,7 @@ def parse_attributes(af_object) af_object.attributes = converted_attrs.except(:members, :files, :file_name) af_object.original_filename = converted_attrs[:file_name] if converted_attrs[:file_name] af_object.extracted_text = create_extrated_text(af_object) if resource.attributes[:extracted_text_id].present? + af_object.based_near = convert_based_near(converted_attrs[:based_near]) if converted_attrs[:based_near] perform_lease_conversion(af_object: af_object, resource: resource) perform_embargo_conversion(af_object: af_object, resource: resource) @@ -164,6 +165,14 @@ def parse_attributes(af_object) end # rubocop:enable Metrics/AbcSize + def convert_based_near(based_near_string) + converted_based_near = [] + based_near_string.each do |bn| + converted_based_near << Hyrax::ControlledVocabularies::Location.new(bn) + end + converted_based_near + end + def create_extrated_text(af_object) pcdm_et_file = af_object.extracted_text.presence || af_object.create_extracted_text pcdm_et_file.content = Hyrax.custom_queries.find_many_file_metadata_by_use(resource: resource, use: Hyrax::FileMetadata::Use::EXTRACTED_TEXT).first&.content diff --git a/lib/wings/transformer_value_mapper.rb b/lib/wings/transformer_value_mapper.rb index d67a72ebcc..0c2b29d55a 100644 --- a/lib/wings/transformer_value_mapper.rb +++ b/lib/wings/transformer_value_mapper.rb @@ -49,8 +49,12 @@ def self.handles?(value) end ## - # @return [RDF::Term] + # Valkyrie objects contain a URI string for location objects so we need + # to return just the string for mapping from Fedora + # + # @return [RDF::Term || String] def result + return value.id if value.is_a?(Hyrax::ControlledVocabularies::Location) value.to_term end end From a0ee2bf9e41a3250d2e8fce31423077e14dae8d2 Mon Sep 17 00:00:00 2001 From: LaRita Robinson Date: Fri, 11 Oct 2024 18:47:31 -0400 Subject: [PATCH 3/4] Pacify the cops --- lib/wings/active_fedora_converter.rb | 10 +++------- spec/wings/active_fedora_converter_spec.rb | 8 +++++--- spec/wings/attribute_transformer_spec.rb | 4 ++-- 3 files changed, 10 insertions(+), 12 deletions(-) diff --git a/lib/wings/active_fedora_converter.rb b/lib/wings/active_fedora_converter.rb index 2b41a53700..def499d2f3 100644 --- a/lib/wings/active_fedora_converter.rb +++ b/lib/wings/active_fedora_converter.rb @@ -143,7 +143,7 @@ def apply_attributes_to_model(af_object) end # rubocop:enable Metrics/CyclomaticComplexity, Metrics/MethodLength - # rubocop:disable Metrics/AbcSize + # rubocop:disable Metrics/AbcSize, Metrics/MethodLength def parse_attributes(af_object) converted_attrs = normal_attributes af_object.attributes = converted_attrs.except(:members, :files, :file_name) @@ -163,14 +163,10 @@ def parse_attributes(af_object) files = converted_attrs.delete(:files) af_object.files.build_or_set(files) if files end - # rubocop:enable Metrics/AbcSize + # rubocop:enable Metrics/AbcSize, Metrics/MethodLength def convert_based_near(based_near_string) - converted_based_near = [] - based_near_string.each do |bn| - converted_based_near << Hyrax::ControlledVocabularies::Location.new(bn) - end - converted_based_near + based_near_string.map { |bn| Hyrax::ControlledVocabularies::Location.new(bn) } end def create_extrated_text(af_object) diff --git a/spec/wings/active_fedora_converter_spec.rb b/spec/wings/active_fedora_converter_spec.rb index fc4ca0aba5..5fb8439735 100644 --- a/spec/wings/active_fedora_converter_spec.rb +++ b/spec/wings/active_fedora_converter_spec.rb @@ -125,9 +125,11 @@ context 'when given a valkyrie native model' do let(:based_near) { "https://sws.geonames.org/4920808/" } - let(:resource) { klass.new(title: ['comet in moominland'], - distant_relation: ['Snufkin'], - based_near: [based_near]) } + let(:resource) do + klass.new(title: ['comet in moominland'], + distant_relation: ['Snufkin'], + based_near: [based_near]) + end let(:klass) { Hyrax::Test::Converter::Resource } before do diff --git a/spec/wings/attribute_transformer_spec.rb b/spec/wings/attribute_transformer_spec.rb index 9d8a460c79..4130e14524 100644 --- a/spec/wings/attribute_transformer_spec.rb +++ b/spec/wings/attribute_transformer_spec.rb @@ -5,7 +5,7 @@ require 'wings/attribute_transformer' RSpec.describe Wings::AttributeTransformer, :active_fedora do - let(:id) { 'moomin123' } + let(:id) { 'moomin123' } let(:based_near) { Hyrax::ControlledVocabularies::Location.new("https://sws.geonames.org/4920808/") } let(:work) { GenericWork.new(id: id, **attributes) } @@ -14,7 +14,7 @@ title: ['fake title', 'fake title 2'], contributor: ['user1'], description: ['a description'], - based_near:[based_near] + based_near: [based_near] } end From cc7902cfe2a2350483c5cb0d310dfd389557d980 Mon Sep 17 00:00:00 2001 From: LaRita Robinson Date: Wed, 16 Oct 2024 13:22:14 -0400 Subject: [PATCH 4/4] Undo Active Fedora conversion of based_near This may still be needed, but this doesn't work correctly. A ticket has been created for this bug if this is something that is ever needed. https://github.com/samvera/hyrax/issues/6926 --- lib/wings/active_fedora_converter.rb | 9 ++------- spec/wings/active_fedora_converter_spec.rb | 13 +------------ 2 files changed, 3 insertions(+), 19 deletions(-) diff --git a/lib/wings/active_fedora_converter.rb b/lib/wings/active_fedora_converter.rb index def499d2f3..a9e207bf4a 100644 --- a/lib/wings/active_fedora_converter.rb +++ b/lib/wings/active_fedora_converter.rb @@ -143,13 +143,12 @@ def apply_attributes_to_model(af_object) end # rubocop:enable Metrics/CyclomaticComplexity, Metrics/MethodLength - # rubocop:disable Metrics/AbcSize, Metrics/MethodLength + # rubocop:disable Metrics/AbcSize def parse_attributes(af_object) converted_attrs = normal_attributes af_object.attributes = converted_attrs.except(:members, :files, :file_name) af_object.original_filename = converted_attrs[:file_name] if converted_attrs[:file_name] af_object.extracted_text = create_extrated_text(af_object) if resource.attributes[:extracted_text_id].present? - af_object.based_near = convert_based_near(converted_attrs[:based_near]) if converted_attrs[:based_near] perform_lease_conversion(af_object: af_object, resource: resource) perform_embargo_conversion(af_object: af_object, resource: resource) @@ -163,11 +162,7 @@ def parse_attributes(af_object) files = converted_attrs.delete(:files) af_object.files.build_or_set(files) if files end - # rubocop:enable Metrics/AbcSize, Metrics/MethodLength - - def convert_based_near(based_near_string) - based_near_string.map { |bn| Hyrax::ControlledVocabularies::Location.new(bn) } - end + # rubocop:enable Metrics/AbcSize def create_extrated_text(af_object) pcdm_et_file = af_object.extracted_text.presence || af_object.create_extracted_text diff --git a/spec/wings/active_fedora_converter_spec.rb b/spec/wings/active_fedora_converter_spec.rb index 5fb8439735..17b7f1c687 100644 --- a/spec/wings/active_fedora_converter_spec.rb +++ b/spec/wings/active_fedora_converter_spec.rb @@ -124,12 +124,7 @@ end context 'when given a valkyrie native model' do - let(:based_near) { "https://sws.geonames.org/4920808/" } - let(:resource) do - klass.new(title: ['comet in moominland'], - distant_relation: ['Snufkin'], - based_near: [based_near]) - end + let(:resource) { klass.new(title: ['comet in moominland'], distant_relation: ['Snufkin']) } let(:klass) { Hyrax::Test::Converter::Resource } before do @@ -138,7 +133,6 @@ module Converter class Resource < Hyrax::Resource attribute :title, Valkyrie::Types::Array.of(Valkyrie::Types::String) attribute :distant_relation, Valkyrie::Types::String - attribute :based_near, Valkyrie::Types::Array.of(Valkyrie::Types::String) end end end @@ -160,11 +154,6 @@ class Resource < Hyrax::Resource .to have_attributes(title: ['comet in moominland'], distant_relation: ['Snufkin']) end - it 'converts based_near to an RDF::URI' do - converted_work = converter.convert - expect(converted_work.based_near.first).to be_a(Hyrax::ControlledVocabularies::Location) - end - it 'supports indexing' do expect(converter.convert.indexing_service).to be_a Hyrax::Indexers::ResourceIndexer end