From 9660aa4543deff41c60d131e081137f84e771499 Mon Sep 17 00:00:00 2001 From: Eugen Rochko Date: Sun, 8 Mar 2020 23:56:18 +0100 Subject: Change local media attachments to perform heavy processing asynchronously (#13210) Fix #9106 --- app/controllers/api/v1/media_controller.rb | 29 ++++++++++++++---- app/controllers/api/v2/media_controller.rb | 12 ++++++++ app/javascript/mastodon/actions/compose.js | 23 +++++++++++++-- app/models/concerns/attachmentable.rb | 2 +- app/models/media_attachment.rb | 26 +++++++++++++++-- .../rest/media_attachment_serializer.rb | 4 ++- app/services/post_status_service.rb | 1 + app/workers/backup_worker.rb | 8 +++-- app/workers/post_process_media_worker.rb | 34 ++++++++++++++++++++++ 9 files changed, 124 insertions(+), 15 deletions(-) create mode 100644 app/controllers/api/v2/media_controller.rb create mode 100644 app/workers/post_process_media_worker.rb (limited to 'app') diff --git a/app/controllers/api/v1/media_controller.rb b/app/controllers/api/v1/media_controller.rb index d87d7b946..0bb3d0d27 100644 --- a/app/controllers/api/v1/media_controller.rb +++ b/app/controllers/api/v1/media_controller.rb @@ -3,25 +3,42 @@ class Api::V1::MediaController < Api::BaseController before_action -> { doorkeeper_authorize! :write, :'write:media' } before_action :require_user! + before_action :set_media_attachment, except: [:create] + before_action :check_processing, except: [:create] def create - @media = current_account.media_attachments.create!(media_params) - render json: @media, serializer: REST::MediaAttachmentSerializer + @media_attachment = current_account.media_attachments.create!(media_attachment_params) + render json: @media_attachment, serializer: REST::MediaAttachmentSerializer rescue Paperclip::Errors::NotIdentifiedByImageMagickError render json: file_type_error, status: 422 rescue Paperclip::Error render json: processing_error, status: 500 end + def show + render json: @media_attachment, serializer: REST::MediaAttachmentSerializer, status: status_code_for_media_attachment + end + def update - @media = current_account.media_attachments.where(status_id: nil).find(params[:id]) - @media.update!(media_params) - render json: @media, serializer: REST::MediaAttachmentSerializer + @media_attachment.update!(media_attachment_params) + render json: @media_attachment, serializer: REST::MediaAttachmentSerializer, status: status_code_for_media_attachment end private - def media_params + def status_code_for_media_attachment + @media_attachment.not_processed? ? 206 : 200 + end + + def set_media_attachment + @media_attachment = current_account.media_attachments.unattached.find(params[:id]) + end + + def check_processing + render json: processing_error, status: 422 if @media_attachment.processing_failed? + end + + def media_attachment_params params.permit(:file, :description, :focus) end diff --git a/app/controllers/api/v2/media_controller.rb b/app/controllers/api/v2/media_controller.rb new file mode 100644 index 000000000..0c1baf01d --- /dev/null +++ b/app/controllers/api/v2/media_controller.rb @@ -0,0 +1,12 @@ +# frozen_string_literal: true + +class Api::V2::MediaController < Api::V1::MediaController + def create + @media_attachment = current_account.media_attachments.create!({ delay_processing: true }.merge(media_attachment_params)) + render json: @media_attachment, serializer: REST::MediaAttachmentSerializer, status: 202 + rescue Paperclip::Errors::NotIdentifiedByImageMagickError + render json: file_type_error, status: 422 + rescue Paperclip::Error + render json: processing_error, status: 500 + end +end diff --git a/app/javascript/mastodon/actions/compose.js b/app/javascript/mastodon/actions/compose.js index c3c6ff1a1..68ef8fbd5 100644 --- a/app/javascript/mastodon/actions/compose.js +++ b/app/javascript/mastodon/actions/compose.js @@ -230,12 +230,31 @@ export function uploadCompose(files) { // Account for disparity in size of original image and resized data total += file.size - f.size; - return api(getState).post('/api/v1/media', data, { + return api(getState).post('/api/v2/media', data, { onUploadProgress: function({ loaded }){ progress[i] = loaded; dispatch(uploadComposeProgress(progress.reduce((a, v) => a + v, 0), total)); }, - }).then(({ data }) => dispatch(uploadComposeSuccess(data, f))); + }).then(({ status, data }) => { + // If server-side processing of the media attachment has not completed yet, + // poll the server until it is, before showing the media attachment as uploaded + + if (status === 200) { + dispatch(uploadComposeSuccess(data, f)); + } else if (status === 202) { + const poll = () => { + api(getState).get(`/api/v1/media/${data.id}`).then(response => { + if (response.status === 200) { + dispatch(uploadComposeSuccess(data, f)); + } else if (response.status === 206) { + setTimeout(() => poll(), 1000); + } + }).catch(error => dispatch(uploadComposeFail(error))); + }; + + poll(); + } + }); }).catch(error => dispatch(uploadComposeFail(error))); }; }; diff --git a/app/models/concerns/attachmentable.rb b/app/models/concerns/attachmentable.rb index 43ff8ac12..18b872c1e 100644 --- a/app/models/concerns/attachmentable.rb +++ b/app/models/concerns/attachmentable.rb @@ -74,7 +74,7 @@ module Attachmentable self.class.attachment_definitions.each_key do |attachment_name| attachment = send(attachment_name) - next if attachment.blank? || attachment.queued_for_write[:original].blank? + next if attachment.blank? || attachment.queued_for_write[:original].blank? || attachment.options[:preserve_files] attachment.instance_write :file_name, SecureRandom.hex(8) + File.extname(attachment.instance_read(:file_name)) end diff --git a/app/models/media_attachment.rb b/app/models/media_attachment.rb index 1e36625ca..3fd4f231c 100644 --- a/app/models/media_attachment.rb +++ b/app/models/media_attachment.rb @@ -19,12 +19,14 @@ # description :text # scheduled_status_id :bigint(8) # blurhash :string +# processing :integer # class MediaAttachment < ApplicationRecord self.inheritance_column = nil enum type: [:image, :gifv, :video, :unknown, :audio] + enum processing: [:queued, :in_progress, :complete, :failed], _prefix: true MAX_DESCRIPTION_LENGTH = 1_500 @@ -156,6 +158,10 @@ class MediaAttachment < ApplicationRecord remote_url.blank? end + def not_processed? + processing.present? && !processing_complete? + end + def needs_redownload? file.blank? && remote_url.present? end @@ -203,10 +209,18 @@ class MediaAttachment < ApplicationRecord "#{x},#{y}" end + attr_writer :delay_processing + + def delay_processing? + @delay_processing + end + + after_commit :enqueue_processing, on: :create after_commit :reset_parent_cache, on: :update before_create :prepare_description, unless: :local? before_create :set_shortcode + before_create :set_processing before_post_process :set_type_and_extension @@ -277,6 +291,10 @@ class MediaAttachment < ApplicationRecord end end + def set_processing + self.processing = delay_processing? ? :queued : :complete + end + def set_meta meta = populate_meta @@ -322,9 +340,11 @@ class MediaAttachment < ApplicationRecord }.compact end - def reset_parent_cache - return if status_id.nil? + def enqueue_processing + PostProcessMediaWorker.perform_async(id) if delay_processing? + end - Rails.cache.delete("statuses/#{status_id}") + def reset_parent_cache + Rails.cache.delete("statuses/#{status_id}") if status_id.present? end end diff --git a/app/serializers/rest/media_attachment_serializer.rb b/app/serializers/rest/media_attachment_serializer.rb index 1b3498ea4..cc10e3001 100644 --- a/app/serializers/rest/media_attachment_serializer.rb +++ b/app/serializers/rest/media_attachment_serializer.rb @@ -12,7 +12,9 @@ class REST::MediaAttachmentSerializer < ActiveModel::Serializer end def url - if object.needs_redownload? + if object.not_processed? + nil + elsif object.needs_redownload? media_proxy_url(object.id, :original) else full_asset_url(object.file.url(:original)) diff --git a/app/services/post_status_service.rb b/app/services/post_status_service.rb index 2301621ab..c61b3baa2 100644 --- a/app/services/post_status_service.rb +++ b/app/services/post_status_service.rb @@ -101,6 +101,7 @@ class PostStatusService < BaseService @media = @account.media_attachments.where(status_id: nil).where(id: @options[:media_ids].take(4).map(&:to_i)) raise Mastodon::ValidationError, I18n.t('media_attachments.validations.images_and_video') if @media.size > 1 && @media.find(&:audio_or_video?) + raise Mastodon::ValidationError, I18n.t('media_attachments.validations.not_ready') if @media.any?(&:not_processed?) end def language_from_option(str) diff --git a/app/workers/backup_worker.rb b/app/workers/backup_worker.rb index e4c609d70..7b0b52844 100644 --- a/app/workers/backup_worker.rb +++ b/app/workers/backup_worker.rb @@ -9,8 +9,12 @@ class BackupWorker backup_id = msg['args'].first ActiveRecord::Base.connection_pool.with_connection do - backup = Backup.find(backup_id) - backup&.destroy + begin + backup = Backup.find(backup_id) + backup.destroy + rescue ActiveRecord::RecordNotFound + true + end end end diff --git a/app/workers/post_process_media_worker.rb b/app/workers/post_process_media_worker.rb new file mode 100644 index 000000000..d3ebda194 --- /dev/null +++ b/app/workers/post_process_media_worker.rb @@ -0,0 +1,34 @@ +# frozen_string_literal: true + +class PostProcessMediaWorker + include Sidekiq::Worker + + sidekiq_options retry: 1, dead: false + + sidekiq_retries_exhausted do |msg| + media_attachment_id = msg['args'].first + + ActiveRecord::Base.connection_pool.with_connection do + begin + media_attachment = MediaAttachment.find(media_attachment_id) + media_attachment.processing = :failed + media_attachment.save + rescue ActiveRecord::RecordNotFound + true + end + end + + Sidekiq.logger.error("Processing media attachment #{media_attachment_id} failed with #{msg['error_message']}") + end + + def perform(media_attachment_id) + media_attachment = MediaAttachment.find(media_attachment_id) + media_attachment.processing = :in_progress + media_attachment.save + media_attachment.file.reprocess_original! + media_attachment.processing = :complete + media_attachment.save + rescue ActiveRecord::RecordNotFound + true + end +end -- cgit