From 3f46a2189f307f83760374daa044851228c2e2cb Mon Sep 17 00:00:00 2001 From: Shana Moore Date: Fri, 1 Nov 2024 10:48:11 -0700 Subject: [PATCH 1/5] =?UTF-8?q?=E2=99=BB=EF=B8=8F=20revert=20uploads=20con?= =?UTF-8?q?troller?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The refactor still caused bugs. I've live tested the original implementation on the server and no longer see the retried jobs at this point. --- app/controllers/hyrax/uploads_controller.rb | 61 ++++++--------------- 1 file changed, 16 insertions(+), 45 deletions(-) diff --git a/app/controllers/hyrax/uploads_controller.rb b/app/controllers/hyrax/uploads_controller.rb index a18b1c1044..112bb2cefd 100644 --- a/app/controllers/hyrax/uploads_controller.rb +++ b/app/controllers/hyrax/uploads_controller.rb @@ -1,16 +1,28 @@ # frozen_string_literal: true - module Hyrax class UploadsController < ApplicationController load_and_authorize_resource class: Hyrax::UploadedFile def create if params[:id].blank? - handle_new_upload + @upload.attributes = { file: params[:files].first, + user: current_user } else - handle_chunked_upload + @upload = Hyrax::UploadedFile.find(params[:id]) + unpersisted_upload = Hyrax::UploadedFile.new(file: params[:files].first, user: current_user) + + # deal with chunks + current_size = @upload.file.size + content_range = request.headers['CONTENT-RANGE'] + begin_of_chunk = content_range[/\ (.*?)-/,1].to_i # "bytes 100-999999/1973660678" will return '100' + + # Add the following chunk to the incomplete upload + if @upload.file.present? && begin_of_chunk == current_size + File.open(@upload.file.path, "ab") { |f| f.write(params[:files].first.read) } + else + @upload.file = unpersisted_upload.file + end end - @upload.save! end @@ -18,46 +30,5 @@ def destroy @upload.destroy head :no_content end - - private - - def handle_new_upload - @upload.attributes = { file: params[:files].first, user: current_user } - end - - def chunk_valid?(upload) - content_range = request.headers['CONTENT-RANGE'] - return false unless content_range - - begin_of_chunk = content_range[/\ (.*?)-/, 1].to_i - current_size = upload.file.size - - upload.file.present? && begin_of_chunk == current_size - end - - def handle_chunked_upload - @upload = Hyrax::UploadedFile.find(params[:id]) - unpersisted_upload = Hyrax::UploadedFile.new(file: params[:files].first, user: current_user) - - if chunk_valid?(@upload) - append_chunk(@upload) - else - replace_file(@upload, unpersisted_upload) - end - end - - def append_chunk(upload) - File.open(upload.file.path, "ab") do |f| - f.write(params[:files].first.read) - end - - upload.reload - end - - def replace_file(upload, unpersisted_upload) - upload.file = unpersisted_upload.file - upload.save! - upload.reload - end end end From 03819cb3c6184e43f4265d44a3722676738fff01 Mon Sep 17 00:00:00 2001 From: Shana Moore Date: Fri, 1 Nov 2024 11:45:47 -0700 Subject: [PATCH 2/5] =?UTF-8?q?=F0=9F=92=84=20rubocop=20fix?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- app/controllers/hyrax/uploads_controller.rb | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/app/controllers/hyrax/uploads_controller.rb b/app/controllers/hyrax/uploads_controller.rb index 112bb2cefd..40512d86af 100644 --- a/app/controllers/hyrax/uploads_controller.rb +++ b/app/controllers/hyrax/uploads_controller.rb @@ -3,28 +3,32 @@ module Hyrax class UploadsController < ApplicationController load_and_authorize_resource class: Hyrax::UploadedFile + # rubocop:disable Metrics/AbcSize + # rubocop:disable Metrics/MethodLength def create if params[:id].blank? @upload.attributes = { file: params[:files].first, - user: current_user } + user: current_user } else @upload = Hyrax::UploadedFile.find(params[:id]) - unpersisted_upload = Hyrax::UploadedFile.new(file: params[:files].first, user: current_user) + unpersisted_upload = Hyrax::UploadedFile.new(file: params[:files].first, user: current_user) # deal with chunks current_size = @upload.file.size content_range = request.headers['CONTENT-RANGE'] - begin_of_chunk = content_range[/\ (.*?)-/,1].to_i # "bytes 100-999999/1973660678" will return '100' - + begin_of_chunk = content_range[/\ (.*?)-/, 1].to_i # "bytes 100-999999/1973660678" will return '100' + # Add the following chunk to the incomplete upload if @upload.file.present? && begin_of_chunk == current_size - File.open(@upload.file.path, "ab") { |f| f.write(params[:files].first.read) } + File.open(@upload.file.path, "ab") { |f| f.write(params[:files].first.read) } else @upload.file = unpersisted_upload.file end end @upload.save! end + # rubocop:enable Metrics/AbcSize + # rubocop:enable Metrics/MethodLength def destroy @upload.destroy From 449a33a7a1ab137ebbc69984d8c5153170ed433b Mon Sep 17 00:00:00 2001 From: Shana Moore Date: Fri, 1 Nov 2024 11:56:02 -0700 Subject: [PATCH 3/5] Refactor: to avoid rubocop Metrics/AbcSize and Metrics/MethodLength errors --- app/controllers/hyrax/uploads_controller.rb | 38 +++++++++++---------- 1 file changed, 20 insertions(+), 18 deletions(-) diff --git a/app/controllers/hyrax/uploads_controller.rb b/app/controllers/hyrax/uploads_controller.rb index 40512d86af..378437d527 100644 --- a/app/controllers/hyrax/uploads_controller.rb +++ b/app/controllers/hyrax/uploads_controller.rb @@ -3,36 +3,38 @@ module Hyrax class UploadsController < ApplicationController load_and_authorize_resource class: Hyrax::UploadedFile - # rubocop:disable Metrics/AbcSize - # rubocop:disable Metrics/MethodLength def create if params[:id].blank? @upload.attributes = { file: params[:files].first, user: current_user } else - @upload = Hyrax::UploadedFile.find(params[:id]) - unpersisted_upload = Hyrax::UploadedFile.new(file: params[:files].first, user: current_user) - - # deal with chunks - current_size = @upload.file.size - content_range = request.headers['CONTENT-RANGE'] - begin_of_chunk = content_range[/\ (.*?)-/, 1].to_i # "bytes 100-999999/1973660678" will return '100' - - # Add the following chunk to the incomplete upload - if @upload.file.present? && begin_of_chunk == current_size - File.open(@upload.file.path, "ab") { |f| f.write(params[:files].first.read) } - else - @upload.file = unpersisted_upload.file - end + upload_with_chunking end @upload.save! end - # rubocop:enable Metrics/AbcSize - # rubocop:enable Metrics/MethodLength def destroy @upload.destroy head :no_content end + + private + + def upload_with_chunking + @upload = Hyrax::UploadedFile.find(params[:id]) + unpersisted_upload = Hyrax::UploadedFile.new(file: params[:files].first, user: current_user) + + # deal with chunks + current_size = @upload.file.size + content_range = request.headers['CONTENT-RANGE'] + begin_of_chunk = content_range[/\ (.*?)-/, 1].to_i # "bytes 100-999999/1973660678" will return '100' + + # Add the following chunk to the incomplete upload + if @upload.file.present? && begin_of_chunk == current_size + File.open(@upload.file.path, "ab") { |f| f.write(params[:files].first.read) } + else + @upload.file = unpersisted_upload.file + end + end end end From 3423cf85b44da9481cfdd9ebe160377cbbb9e9ca Mon Sep 17 00:00:00 2001 From: Shana Moore Date: Fri, 1 Nov 2024 13:18:00 -0700 Subject: [PATCH 4/5] fix for failing specs --- app/controllers/hyrax/uploads_controller.rb | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/app/controllers/hyrax/uploads_controller.rb b/app/controllers/hyrax/uploads_controller.rb index 378437d527..010eb363d1 100644 --- a/app/controllers/hyrax/uploads_controller.rb +++ b/app/controllers/hyrax/uploads_controller.rb @@ -23,12 +23,15 @@ def destroy def upload_with_chunking @upload = Hyrax::UploadedFile.find(params[:id]) unpersisted_upload = Hyrax::UploadedFile.new(file: params[:files].first, user: current_user) - + + # Check if CONTENT-RANGE header is present + content_range = request.headers['CONTENT-RANGE'] + return @upload.file = unpersisted_upload.file if content_range.nil? + # deal with chunks current_size = @upload.file.size - content_range = request.headers['CONTENT-RANGE'] begin_of_chunk = content_range[/\ (.*?)-/, 1].to_i # "bytes 100-999999/1973660678" will return '100' - + # Add the following chunk to the incomplete upload if @upload.file.present? && begin_of_chunk == current_size File.open(@upload.file.path, "ab") { |f| f.write(params[:files].first.read) } From 7c72302938e4852123429a523a04a55dafca7761 Mon Sep 17 00:00:00 2001 From: Shana Moore Date: Fri, 1 Nov 2024 13:44:37 -0700 Subject: [PATCH 5/5] Update uploads_controller.rb --- app/controllers/hyrax/uploads_controller.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/app/controllers/hyrax/uploads_controller.rb b/app/controllers/hyrax/uploads_controller.rb index 010eb363d1..fb8ad050f8 100644 --- a/app/controllers/hyrax/uploads_controller.rb +++ b/app/controllers/hyrax/uploads_controller.rb @@ -23,15 +23,15 @@ def destroy def upload_with_chunking @upload = Hyrax::UploadedFile.find(params[:id]) unpersisted_upload = Hyrax::UploadedFile.new(file: params[:files].first, user: current_user) - + # Check if CONTENT-RANGE header is present content_range = request.headers['CONTENT-RANGE'] return @upload.file = unpersisted_upload.file if content_range.nil? - + # deal with chunks current_size = @upload.file.size begin_of_chunk = content_range[/\ (.*?)-/, 1].to_i # "bytes 100-999999/1973660678" will return '100' - + # Add the following chunk to the incomplete upload if @upload.file.present? && begin_of_chunk == current_size File.open(@upload.file.path, "ab") { |f| f.write(params[:files].first.read) }