From 176273dec06b85a8b382f2704570b8cf93a3dadc Mon Sep 17 00:00:00 2001 From: James Smith Date: Thu, 12 Sep 2024 12:34:14 +0100 Subject: [PATCH 1/6] move shrine temp to its own folder within rails tmp --- config/initializers/shrine.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/config/initializers/shrine.rb b/config/initializers/shrine.rb index 20f3c58fb..8223a8bcc 100644 --- a/config/initializers/shrine.rb +++ b/config/initializers/shrine.rb @@ -8,7 +8,7 @@ Shrine.plugin :rack_response Shrine.storages = { - cache: Shrine::Storage::FileSystem.new("tmp/cache") + cache: Shrine::Storage::FileSystem.new("tmp/shrine") } Rails.application.config.after_initialize do From fcdae9c15503cefc2d071eaf554f0bd6d085cabd Mon Sep 17 00:00:00 2001 From: James Smith Date: Thu, 12 Sep 2024 12:57:19 +0100 Subject: [PATCH 2/6] rewrite unzip method to use shrine cache location --- app/jobs/process_uploaded_file_job.rb | 24 ++++++++++++++---------- 1 file changed, 14 insertions(+), 10 deletions(-) diff --git a/app/jobs/process_uploaded_file_job.rb b/app/jobs/process_uploaded_file_job.rb index 8a1635737..cc61a4f07 100644 --- a/app/jobs/process_uploaded_file_job.rb +++ b/app/jobs/process_uploaded_file_job.rb @@ -45,18 +45,22 @@ def perform(library_id, uploaded_file, owner: nil, creator_id: nil, collection_i def unzip(model, uploaded_file) Shrine.with_file(uploaded_file) do |archive| - Dir.mktmpdir do |tmpdir| - strip = count_common_path_components(archive) - Archive::Reader.open_filename(archive.path, strip_components: strip) do |reader| - reader.each_entry do |entry| - next if !entry.file? || entry.size > SiteSettings.max_file_extract_size - next if SiteSettings.ignored_file?(entry.pathname) - filename = entry.pathname # Stored because pathname gets mutated by the extract and we want the original - reader.extract(entry, Archive::EXTRACT_SECURE, destination: tmpdir) - model.model_files.create(filename: filename, attachment: File.open(entry.pathname)) - end + tmpdir = Shrine.find_storage(:cache).directory.join(SecureRandom.uuid) + tmpdir.mkdir + strip = count_common_path_components(archive) + Archive::Reader.open_filename(archive.path, strip_components: strip) do |reader| + reader.each_entry do |entry| + next if !entry.file? || entry.size > SiteSettings.max_file_extract_size + next if SiteSettings.ignored_file?(entry.pathname) + filename = entry.pathname # Stored because pathname gets mutated by the extract and we want the original + reader.extract(entry, Archive::EXTRACT_SECURE, destination: tmpdir.to_s) + model.model_files.create(filename: filename, attachment: File.open(entry.pathname)) + # Clean up file + File.delete(entry.pathname) end end + # Clean up temp folder + Dir.rmdir(tmpdir) end end From a319a98c85c987bbe073774e1ccde0d4921c5e1c Mon Sep 17 00:00:00 2001 From: James Smith Date: Thu, 12 Sep 2024 14:13:16 +0100 Subject: [PATCH 3/6] clean up extracted files if they still exist --- app/jobs/process_uploaded_file_job.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/jobs/process_uploaded_file_job.rb b/app/jobs/process_uploaded_file_job.rb index cc61a4f07..2e921169c 100644 --- a/app/jobs/process_uploaded_file_job.rb +++ b/app/jobs/process_uploaded_file_job.rb @@ -56,11 +56,11 @@ def unzip(model, uploaded_file) reader.extract(entry, Archive::EXTRACT_SECURE, destination: tmpdir.to_s) model.model_files.create(filename: filename, attachment: File.open(entry.pathname)) # Clean up file - File.delete(entry.pathname) + File.delete(entry.pathname) if File.exist?(entry.pathname) end end # Clean up temp folder - Dir.rmdir(tmpdir) + Dir.rmdir(tmpdir) if Dir.empty?(tmpdir) end end From 40d0b0268e1b6a030423fd05f6a9ff44abd67443 Mon Sep 17 00:00:00 2001 From: James Smith Date: Thu, 12 Sep 2024 14:13:56 +0100 Subject: [PATCH 4/6] Set all filesystem libraries to "move" mode on boot Won't happen dynamically for new libraries, but I don't think that really matters too much? --- config/initializers/shrine.rb | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/config/initializers/shrine.rb b/config/initializers/shrine.rb index 8223a8bcc..69ed10927 100644 --- a/config/initializers/shrine.rb +++ b/config/initializers/shrine.rb @@ -13,6 +13,13 @@ Rails.application.config.after_initialize do Library.register_all_storage + + upload_options = {cache: {move: true}} + Library.all.map do |l| + upload_options[l.storage_key.to_sym] = {move: true} if l.storage_service == "filesystem" + end + Shrine.plugin :upload_options, **upload_options + begin Sidekiq.set_schedule("sweep", {every: "1h", class: "CacheSweepJob"}) rescue RedisClient::CannotConnectError From 5013d40de4ff7b8257cc6f649de0472eeabe649a Mon Sep 17 00:00:00 2001 From: James Smith Date: Thu, 12 Sep 2024 14:28:23 +0100 Subject: [PATCH 5/6] don't try to move files in test - they're not real --- config/initializers/shrine.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/config/initializers/shrine.rb b/config/initializers/shrine.rb index 69ed10927..553ae246f 100644 --- a/config/initializers/shrine.rb +++ b/config/initializers/shrine.rb @@ -18,7 +18,7 @@ Library.all.map do |l| upload_options[l.storage_key.to_sym] = {move: true} if l.storage_service == "filesystem" end - Shrine.plugin :upload_options, **upload_options + Shrine.plugin :upload_options, **upload_options unless Rails.env.test? begin Sidekiq.set_schedule("sweep", {every: "1h", class: "CacheSweepJob"}) From b2950c4b40a671cd79eeb0aa8728429500d68ff0 Mon Sep 17 00:00:00 2001 From: James Smith Date: Thu, 12 Sep 2024 14:52:32 +0100 Subject: [PATCH 6/6] don't try to set up libraries during migrations --- config/initializers/shrine.rb | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/config/initializers/shrine.rb b/config/initializers/shrine.rb index 553ae246f..b8b769f34 100644 --- a/config/initializers/shrine.rb +++ b/config/initializers/shrine.rb @@ -14,11 +14,15 @@ Rails.application.config.after_initialize do Library.register_all_storage - upload_options = {cache: {move: true}} - Library.all.map do |l| - upload_options[l.storage_key.to_sym] = {move: true} if l.storage_service == "filesystem" + begin + upload_options = {cache: {move: true}} + Library.all.map do |l| + upload_options[l.storage_key.to_sym] = {move: true} if l.storage_service == "filesystem" + end + Shrine.plugin :upload_options, **upload_options unless Rails.env.test? + rescue ActiveRecord::StatementInvalid, NameError + nil # migrations probably haven't run yet to create library table end - Shrine.plugin :upload_options, **upload_options unless Rails.env.test? begin Sidekiq.set_schedule("sweep", {every: "1h", class: "CacheSweepJob"})