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

Remove carrierwave dependency from Spotlight, relying on ActiveStorage #2739

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft
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
13 changes: 10 additions & 3 deletions app/models/spotlight/attachment.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,18 @@ module Spotlight
##
# Sir-trevor image upload attachments
class Attachment < ActiveRecord::Base
# Open to alternatives on how to do this, but this works
include Rails.application.routes.url_helpers

belongs_to :exhibit
mount_uploader :file, Spotlight::AttachmentUploader
has_one_attached :file

def as_json(options = nil)
file.as_json(options).merge(name: name, uid: uid, attachment: to_global_id)
def as_json(_options = nil)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is super weird, but seems like we need to provide the accessible url at this point for our uploader forms to work correctly.

# as_json has problems with single has_one_attached content
# https://github.com/rails/rails/issues/33036
{
name: name, uid: uid, attachment: to_global_id, url: rails_blob_path(file, only_path: true)
}
end
end
end
2 changes: 1 addition & 1 deletion app/models/spotlight/bulk_update.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

module Spotlight
class BulkUpdate < ActiveRecord::Base
mount_uploader :file, Spotlight::BulkUpdatesUploader
has_one_attached :file
belongs_to :exhibit
end
end
15 changes: 3 additions & 12 deletions app/models/spotlight/featured_image.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,16 +4,14 @@ module Spotlight
##
# Featured images for browse categories, feature pages, and exhibits
class FeaturedImage < ActiveRecord::Base
mount_uploader :image, Spotlight::FeaturedImageUploader
has_one_attached :image

before_validation do
next unless upload_id.present? && source == 'remote'

# copy the image from the temp upload
temp_image = Spotlight::TemporaryImage.find(upload_id)
self.image = CarrierWave::SanitizedFile.new tempfile: StringIO.new(temp_image.image.read),
filename: temp_image.image.filename || temp_image.image.identifier,
content_type: temp_image.image.content_type
image.attach(temp_image.image.blob)

# Unset the incoming iiif_tilesource, which points at the temp image
self.iiif_tilesource = nil
Expand All @@ -24,13 +22,6 @@ class FeaturedImage < ActiveRecord::Base
Spotlight::TemporaryImage.find(upload_id).delete if upload_id.present?
end

after_save do
if image.present?
image.cache! unless image.cached?
image.store!
end
end

attr_accessor :upload_id

def iiif_url
Expand Down Expand Up @@ -60,7 +51,7 @@ def document
end

def file_present?
image.file.present?
image.blob.present?
end

def iiif_tilesource
Expand Down
4 changes: 2 additions & 2 deletions app/services/spotlight/carrierwave_file_resolver.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,10 @@ def initialize
end

def pattern(id)
uploaded_file = Spotlight::FeaturedImage.find(id).image.file
uploaded_file = Spotlight::FeaturedImage.find(id).image.blob
raise Riiif::ImageNotFoundError, "unable to find file for #{id}" if uploaded_file.nil?

uploaded_file.file
ActiveStorage::Blob.service.path_for(uploaded_file.key)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not sure how this will work with other types of ActiveStorage::Service, but should those people just write their own file resolver?

Copy link
Member

Choose a reason for hiding this comment

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

Yeah.. I think we might have to adapt RIIIF to handle some of this better. I think ActiveStorage::Blob.service.path_for won't work with some of the remote storage options, right?

(but... the whole download-with-a-block API won't play nice with RIIIF)

end
end
end
16 changes: 8 additions & 8 deletions app/services/spotlight/exhibit_import_export_service.rb
Original file line number Diff line number Diff line change
Expand Up @@ -186,9 +186,9 @@ def from_hash!(hash)

# dedupe by something??
ar = exhibit.attachments.build(attr)
ar.file = CarrierWave::SanitizedFile.new tempfile: StringIO.new(Base64.decode64(file[:content])),
filename: file[:filename],
content_type: file[:content_type]
ar.file.attach io: StringIO.new(Base64.decode64(file[:content])),
filename: file[:filename],
content_type: file[:content_type]
end

hash[:languages].each do |attr|
Expand All @@ -212,9 +212,9 @@ def deserialize_featured_image(obj, method, data)
image = obj.public_send("build_#{method}")
image.update(data)
if file
image.image = CarrierWave::SanitizedFile.new tempfile: StringIO.new(Base64.decode64(file[:content])),
filename: file[:filename],
content_type: file[:content_type]
image.image.attach io: StringIO.new(Base64.decode64(file[:content])),
filename: file[:filename],
content_type: file[:content_type]
# Unset the iiif_tilesource field as the new image should be different, because
# the source has been reloaded
image.iiif_tilesource = nil
Expand Down Expand Up @@ -301,11 +301,11 @@ def attach_featured_images(json)

def serialize_featured_image(id)
image = Spotlight::FeaturedImage.find(id)
file = image.image.file
file = image.image
if file
img = {
image: {
filename: file.filename, content_type: file.content_type, content: Base64.encode64(file.read)
filename: file.filename, content_type: file.content_type, content: Base64.encode64(file.download)
}
}
end
Expand Down
15 changes: 0 additions & 15 deletions app/uploaders/spotlight/attachment_uploader.rb

This file was deleted.

7 changes: 0 additions & 7 deletions app/uploaders/spotlight/bulk_updates_uploader.rb

This file was deleted.

17 changes: 0 additions & 17 deletions app/uploaders/spotlight/featured_image_uploader.rb

This file was deleted.

1 change: 0 additions & 1 deletion blacklight-spotlight.gemspec
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@ these collections.)
s.add_dependency 'bootstrap_form', '~> 4.1'
s.add_dependency 'breadcrumbs_on_rails', '>= 3.0', '< 5'
s.add_dependency 'cancancan'
s.add_dependency 'carrierwave', '~> 2.2'
s.add_dependency 'clipboard-rails', '~> 1.5'
s.add_dependency 'devise', '~> 4.1'
s.add_dependency 'devise_invitable'
Expand Down
5 changes: 5 additions & 0 deletions lib/generators/spotlight/install_generator.rb
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,11 @@ def add_js
run 'bundle exec rails webpacker:install'
end

def setup_activestorage
run 'bundle exec rails active_storage:install'
run 'bundle exec rails db:migrate'
end

def inject_spotlight_routes
route "mount Spotlight::Engine, at: 'spotlight'"
gsub_file 'config/routes.rb', /^\s*root.*/ do |match|
Expand Down
1 change: 0 additions & 1 deletion lib/spotlight/engine.rb
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,6 @@ class Engine < ::Rails::Engine
end
end

require 'carrierwave'
require 'underscore-rails'
require 'github/markup'
require 'sir_trevor_rails'
Expand Down
36 changes: 36 additions & 0 deletions lib/tasks/spotlight_tasks.rake
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,42 @@ namespace :spotlight do
Migration::PageLanguage.run
end

desc 'Migrate CarrierWave content to ActiveStorage'
task migrate_carrier_wave: :environment do
puts 'Mirating Spotlight::Attachment'
Spotlight::Attachment.find_each do |attachment|
next if attachment.file.blob

attachment.file.attach(
io: File.open(Rails.root.join("public/uploads/spotlight/attachment/file/#{attachment.attributes['file']}")),
filename: attachment.attributes['file'],
content_type: Mime::Type.lookup_by_extension(File.extname(attachment.attributes['file'])[1..])
)
end
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Should we clean up the old files?


puts 'Mirating Spotlight::FeaturedImage'
Spotlight::FeaturedImage.find_each do |featured_image|
next if featured_image.image.blob

featured_image.image.attach(
io: File.open(Rails.root.join("public/uploads/spotlight/featured_image/image/#{featured_image.attributes['image']}")),
filename: featured_image.attributes['image'],
content_type: Mime::Type.lookup_by_extension(File.extname(featured_image.attributes['image'])[1..])
)
end

puts 'Mirating Spotlight::BulkUpdate'
Spotlight::BulkUpdate.find_each do |bulk_update|
next if bulk_update.file.blob

bulk_update.file.attach(
io: File.open(Rails.root.join("public/uploads/#{bulk_update.attributes['file']}")),
filename: bulk_update.attributes['file'],
content_type: Mime::Type.lookup_by_extension(File.extname(bulk_update.attributes['file'])[1..])
)
end
end

def prompt_to_create_user
Spotlight::Engine.user_class.find_or_create_by!(email: prompt_for_email) do |u|
puts 'User not found. Enter a password to create the user.'
Expand Down
8 changes: 0 additions & 8 deletions spec/test_app_templates/carrierwave.rb

This file was deleted.

5 changes: 1 addition & 4 deletions spec/test_app_templates/lib/generators/test_app_generator.rb
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ def run_blacklight_generator

def run_spotlight_migrations
rake 'spotlight:install:migrations'
rake 'active_storage:install'
rake 'db:migrate'
end

Expand All @@ -40,10 +41,6 @@ def add_rake_tasks_to_app
rakefile 'spotlight_test.rake', File.read(find_in_source_paths('spotlight_test.rake'))
end

def disable_carrierwave_processing
copy_file 'carrierwave.rb', 'config/initializers/carrierwave.rb'
end

def add_theme_assets
copy_file 'fixture.png', 'app/assets/images/spotlight/themes/default_preview.png'
copy_file 'fixture.png', 'app/assets/images/spotlight/themes/modern_preview.png'
Expand Down
27 changes: 0 additions & 27 deletions spec/uploaders/spotlight/attachment_uploader_spec.rb

This file was deleted.

33 changes: 0 additions & 33 deletions spec/uploaders/spotlight/featured_image_uploader_spec.rb

This file was deleted.