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

thumbnails and other derivatives were not stored in the path we expected them to be #6091

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 25 additions & 0 deletions app/services/hyrax/derivative_bucketed_storage.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
# frozen_string_literal: true
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would have subclassed the override here but Valkyrie::Storage::Disk::BucketStorage is a inner class. That makes me think it is best to replace instead of extend.


# this class overrides the Valkyrie::Storage::Disk::BucketStorage class so that file paths match
module Hyrax
class DerivativeBucketedStorage
attr_reader :base_path

def initialize(base_path:)
@base_path = base_path
end

# rubocop:disable Lint/UnusedMethodArgument
def generate(resource:, file:, original_filename:)
raise ArgumentError, "original_filename must be provided" unless original_filename
Pathname.new(base_path).join(*bucketed_path(resource.id)).join(original_filename)
end
# rubocop:enable Lint/UnusedMethodArgument

def bucketed_path(id)
# We want to use the same code the derivative process uses so that items end up
# stored in the place we expect them.
Hyrax::DerivativePath.new(id.to_s).pair_directory
end
end
end
16 changes: 12 additions & 4 deletions app/services/hyrax/derivative_path.rb
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,18 @@ def all_paths
end
end

def pairs
@pairs ||= id.split('').each_slice(2).map(&:join)
end

def pair_directory
pairs[0..-2]
end

def pair_path
(pair_directory + pairs[-1..-1]).join('/')
end

private

# @return [String] Returns the root path where derivatives will be generated into.
Expand All @@ -47,10 +59,6 @@ def path_prefix
Pathname.new(Hyrax.config.derivatives_path).join(pair_path)
end

def pair_path
id.split('').each_slice(2).map(&:join).join('/')
end

def file_name
return unless destination_name
destination_name + extension
Expand Down
6 changes: 4 additions & 2 deletions app/services/hyrax/valkyrie_persist_derivatives.rb
Original file line number Diff line number Diff line change
Expand Up @@ -25,11 +25,13 @@ def self.call(stream,
# responds to #path -- here we only have a StringIO, so some
# transformation is in order
tmpfile = Tempfile.new(file_set.id, encoding: 'ascii-8bit')
tmpfile.write stream.read
stream.rewind
output = tmpfile.write(stream.read)
tmpfile.flush
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the adapter does not flush the buffer first, so we need to make absolutely sure the file is in the tmp location.

raise 'blank file detected' if output.zero?

filename = filename(directives)
Hyrax.logger.debug "Uploading thumbnail for FileSet #{file_set.id} as #{filename}"

uploader.upload(
io: tmpfile,
filename: filename,
Expand Down
2 changes: 1 addition & 1 deletion config/initializers/storage_adapter_initializer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,6 @@
)

Valkyrie::StorageAdapter.register(
Valkyrie::Storage::Disk.new(base_path: Hyrax.config.derivatives_path),
Valkyrie::Storage::Disk.new(base_path: Hyrax.config.derivatives_path, path_generator: Hyrax::DerivativeBucketedStorage),
:derivatives_disk
)