Skip to content

Commit

Permalink
Lazy migrate characterization along with file (#6914)
Browse files Browse the repository at this point in the history
* Add spec to test characterization migrating

* work through infra issues around migration spec

* load hyrax custom queries in to postgres adapter

* use model registry to handle valkyrie class names, reducing repeats

* properly handle files when using goddess adapters in specs

* fix file characterization migration

---------

Co-authored-by: Rob Kaufman <rob@notch8.com>
  • Loading branch information
laritakr and orangewolf authored Oct 14, 2024
1 parent f77472d commit be73c6f
Show file tree
Hide file tree
Showing 7 changed files with 117 additions and 55 deletions.
70 changes: 19 additions & 51 deletions app/jobs/migrate_files_to_valkyrie_job.rb
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,21 @@ def perform(resource)
migrate_files!(resource: resource)
end

def attribute_mapping
return @attribute_mapping if @attribute_mapping
@attribute_mapping = %w[
aspect_ratio bit_depth bit_rate byte_order capture_device channels character_count character_set
checksum color_map color_space compression creator data_format duration exif_version file_title
fits_version format_label frame_rate gps_timestamp graphics_count height image_producer language
latitude line_count longitude markup_basis markup_language offset orientation page_count
paragraph_count profile_name profile_version recorded_size sample_rate scanning_software
table_count well_formed width word_count ].inject({}) { |j, i| j[i] = i; j}
@attribute_mapping['recorded_size'] = 'file_size'
@attribute_mapping['channels'] = 'alpha_channels'
@attribute_mapping['checksum'] = 'original_checksum'
@attribute_mapping
end

private

def migrate_derivatives!(resource:)
Expand Down Expand Up @@ -61,11 +76,11 @@ def migrate_files!(resource:)
end

def copy_attributes(valkyrie_file:, original_file:)
TRANSFERABLE_ATTRIBUTES.each do |attr|
valkyrie_file.set_value(attr, original_file[attr])
attribute_mapping.each do |k, v|
valkyrie_file.set_value(k, original_file.send(v))
end
valkyrie_file.set_value(:channels, original_file.alpha_channels) if valkyrie_file.channels.blank?
valkyrie_file.set_value(:checksum, original_file.original_checksum)
# Special case as this property isn't in the characterization proxy
valkyrie_file.set_value('alternate_ids', original_file.alternate_ids)
valkyrie_file
end

Expand All @@ -89,51 +104,4 @@ def container_for(filename)
'service_file'
end
end

TRANSFERABLE_ATTRIBUTES = %w[
alternate_ids
aspect_ratio
bit_depth
bit_rate
byte_order
capture_device
channels
character_count
character_set
checksum
color_map
color_space
compression
creator
data_format
duration
exif_version
file_title
fits_version
format_label
frame_rate
gps_timestamp
graphics_count
height
image_producer
language
latitude
line_count
longitude
markup_basis
markup_language
offset
orientation
page_count
paragraph_count
profile_name
profile_version
recorded_size
sample_rate
scanning_software
table_count
well_formed
width
word_count
]
end
4 changes: 3 additions & 1 deletion lib/freyja/persister.rb
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,9 @@ def save(resource:, external_resource: false, perform_af_validation: false)

def convert_and_migrate_resource(orm_object)
new_resource = resource_factory.to_resource(object: orm_object)
MigrateFilesToValkyrieJob.perform_later(new_resource) if new_resource.is_a?(Hyrax::FileSet) && new_resource.file_ids.size == 1 && new_resource.file_ids.first.id.to_s.match('/files/')
if Hyrax.config.valkyrie_transition? && new_resource.is_a?(Hyrax::FileSet) && new_resource.file_ids.size == 1 && new_resource.file_ids.first.id.to_s.match('/files/')
MigrateFilesToValkyrieJob.perform_later(new_resource)
end
new_resource
end
end
Expand Down
25 changes: 25 additions & 0 deletions lib/goddess/query.rb
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,31 @@ def find_single(method_name)
# @param [QueryService] query_service
def initialize(*services)
@services = services
setup_custom_queries
end

# rubocop:disable Metrics/MethodLength
def setup_custom_queries
# load all the sql based custom queries
[
Hyrax::CustomQueries::Navigators::CollectionMembers,
Hyrax::CustomQueries::Navigators::ChildCollectionsNavigator,
Hyrax::CustomQueries::Navigators::ParentCollectionsNavigator,
Hyrax::CustomQueries::Navigators::ChildFileSetsNavigator,
Hyrax::CustomQueries::Navigators::ChildWorksNavigator,
Hyrax::CustomQueries::Navigators::FindFiles,
Hyrax::CustomQueries::FindAccessControl,
Hyrax::CustomQueries::FindCollectionsByType,
Hyrax::CustomQueries::FindFileMetadata,
Hyrax::CustomQueries::FindIdsByModel,
Hyrax::CustomQueries::FindManyByAlternateIds,
Hyrax::CustomQueries::FindModelsByAccess,
Hyrax::CustomQueries::FindCountBy,
Hyrax::CustomQueries::FindByDateRange
].each do |handler|
services[0].custom_queries.register_query_handler(handler)
end
end
# rubocop:enable Metrics/MethodLength
end
end
9 changes: 9 additions & 0 deletions lib/wings/setup.rb
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,15 @@ def build_or_set(attributes, &block)
Valkyrie.config.metadata_adapter.query_service.custom_queries.register_query_handler(query_handler)
end

Valkyrie.config.resource_class_resolver = lambda do |resource_klass_name|
return resource_klass_name.constantize unless defined?(Wings)
klass_name = resource_klass_name.gsub(/Resource$/, '')
# Second one should throw a name error because we do not know what you want if
# it isn't one of these two options
klass = klass_name.safe_constantize || resource_klass_name.constantize
Wings::ModelRegistry.reverse_lookup(klass) || klass
end

Wings::ModelRegistry.register(Hyrax::AccessControl, Hydra::AccessControl)
Wings::ModelRegistry.register(Hyrax.config.admin_set_class_for_wings, AdminSet)
Wings::ModelRegistry.register(Hyrax.config.collection_class_for_wings, ::Collection)
Expand Down
39 changes: 39 additions & 0 deletions spec/jobs/migrate_files_to_valkyrie_job_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
# frozen_string_literal: true
require 'wings'

RSpec.describe MigrateFilesToValkyrieJob, valkyrie_adapter: :freyja_adapter, perform_enqueued: [MigrateFilesToValkyrieJob] do
let(:user) { create(:user) }
let(:content) { File.open(fixture_path + '/' + label) }
let(:uploaded_file1) { build(:uploaded_file, file:) }
let(:fedora_file_set) { create(:file_set, title: ['Some title'], label: label, content: content) }
let!(:pcdm_file) do pcdm_file = fedora_file_set.files.first
pcdm_file.content = 'foo'
pcdm_file.original_name = label
pcdm_file.height = '111'
pcdm_file.width = '222'
pcdm_file.file_size = '123456'
pcdm_file.format_label = ["Portable Network Graphics"]
pcdm_file.original_checksum = ['checksum123']
pcdm_file.save!
pcdm_file
end
let(:label) { 'image.jpg' }
let(:wings_file_set) { Hyrax.query_service.find_by(id: fedora_file_set.id) }
let(:migrated_file_set) { Hyrax.persister.save(resource: wings_file_set) }

before do
allow(Hyrax.config).to receive(:valkyrie_transition?).and_return(true)
# allow(ActiveFedora::Base).to receive(:find).and_call_original
# allow(ActiveFedora::Base).to receive(:find).with(migrated_file_set.original_file.file_identifier.to_s).and_return(file_with_characterization)
# allow(File_Set).to receive(:find).with(fedora_file_set.id).and_return(fedora_file_set)
end

it "it migrates all derivatives along with a file" do
described_class.perform_now(migrated_file_set)

valkyrized_file_set = Hyrax.query_service.find_by(id: migrated_file_set.id)
described_class.new.attribute_mapping.each do |k, v|
expect(valkyrized_file_set.original_file.send(k)).to match_array(pcdm_file.send(v))
end
end
end
7 changes: 6 additions & 1 deletion spec/lib/freyja/persister_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
require 'wings'
require 'freyja/metadata_adapter'

RSpec.describe Freyja::Persister, :active_fedora, :clean_repo do
RSpec.describe Freyja::Persister, :active_fedora, :clean_repo, valkyrie_adapter: :freyja_adapter do
subject(:persister) { described_class.new(adapter: adapter) }
let(:adapter) { Freyja::MetadataAdapter.new }
let(:query_service) { adapter.query_service }
Expand Down Expand Up @@ -36,8 +36,13 @@ class Custom < ActiveFedora::Base
accepts_nested_attributes_for :nested_resource
include Hydra::Works::WorkBehavior
end

Wings::ModelRegistry.register(CustomResource, Custom)
end

after do
Wings::ModelRegistry.unregister(CustomResource)

Object.send(:remove_const, :CustomResource)
Object.send(:remove_const, :Custom)
end
Expand Down
18 changes: 16 additions & 2 deletions spec/spec_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,8 @@ def ci_build?
.register(Valkyrie::Persistence::Memory::MetadataAdapter.new, :test_adapter)
Valkyrie::MetadataAdapter
.register(Valkyrie::Persistence::Postgres::MetadataAdapter.new, :postgres_adapter)
Valkyrie::MetadataAdapter
.register(Freyja::MetadataAdapter.new, :freyja_adapter)
version_path = Rails.root / 'tmp' / 'test_adapter_uploads'
Valkyrie::StorageAdapter.register(
Valkyrie::Storage::VersionedDisk.new(base_path: version_path),
Expand Down Expand Up @@ -238,11 +240,13 @@ def clean_active_fedora_repository
config.include Warden::Test::Helpers, type: :feature

config.before(:each, type: :feature) do |example|
adapter_name = example.metadata[:valkyrie_adapter]

clean_active_fedora_repository unless
# trust that clean_repo performed the clean if present
example.metadata[:clean_repo] ||
# don't run for adapters other than wings
(example.metadata[:valkyrie_adapter].present? && example.metadata[:valkyrie_adapter] != :wings_adapter)
(adapter_name.present? && ![:wings_adapter, :freyja_adapter, :frigg_adapter].include?(adapter_name))
end

config.append_after(:each, type: :feature) do
Expand Down Expand Up @@ -350,8 +354,18 @@ def clean_active_fedora_repository
config.prepend_before(:example, :valkyrie_adapter) do |example|
adapter_name = example.metadata[:valkyrie_adapter]

if adapter_name == :wings_adapter
if [:wings_adapter, :freyja_adapter, :frigg_adapter].include?(adapter_name)
skip("Don't test Wings when it is dasabled") if Hyrax.config.disable_wings
unless adapter_name == :wings_adapter
Valkyrie::StorageAdapter.register(
Valkyrie::Storage::Disk.new(base_path: Rails.root.join("tmp", "storage", "files"),
file_mover: FileUtils.method(:cp)),
:disk
)
allow(Valkyrie.config)
.to receive(:storage_adapter)
.and_return(Valkyrie::StorageAdapter.find(:disk))
end
else
allow(Hyrax.config).to receive(:disable_wings).and_return(true)
hide_const("Wings") # disable_wings=true removes the Wings constant
Expand Down

0 comments on commit be73c6f

Please sign in to comment.