Change local media attachments to perform heavy processing asynchronously (#13210)
Fix #9106
This commit is contained in:
		
					parent
					
						
							
								764b89939f
							
						
					
				
			
			
				commit
				
					
						9660aa4543
					
				
			
		
					 15 changed files with 165 additions and 17 deletions
				
			
		|  | @ -3,25 +3,42 @@ | ||||||
| class Api::V1::MediaController < Api::BaseController | class Api::V1::MediaController < Api::BaseController | ||||||
|   before_action -> { doorkeeper_authorize! :write, :'write:media' } |   before_action -> { doorkeeper_authorize! :write, :'write:media' } | ||||||
|   before_action :require_user! |   before_action :require_user! | ||||||
|  |   before_action :set_media_attachment, except: [:create] | ||||||
|  |   before_action :check_processing, except: [:create] | ||||||
| 
 | 
 | ||||||
|   def create |   def create | ||||||
|     @media = current_account.media_attachments.create!(media_params) |     @media_attachment = current_account.media_attachments.create!(media_attachment_params) | ||||||
|     render json: @media, serializer: REST::MediaAttachmentSerializer |     render json: @media_attachment, serializer: REST::MediaAttachmentSerializer | ||||||
|   rescue Paperclip::Errors::NotIdentifiedByImageMagickError |   rescue Paperclip::Errors::NotIdentifiedByImageMagickError | ||||||
|     render json: file_type_error, status: 422 |     render json: file_type_error, status: 422 | ||||||
|   rescue Paperclip::Error |   rescue Paperclip::Error | ||||||
|     render json: processing_error, status: 500 |     render json: processing_error, status: 500 | ||||||
|   end |   end | ||||||
| 
 | 
 | ||||||
|  |   def show | ||||||
|  |     render json: @media_attachment, serializer: REST::MediaAttachmentSerializer, status: status_code_for_media_attachment | ||||||
|  |   end | ||||||
|  | 
 | ||||||
|   def update |   def update | ||||||
|     @media = current_account.media_attachments.where(status_id: nil).find(params[:id]) |     @media_attachment.update!(media_attachment_params) | ||||||
|     @media.update!(media_params) |     render json: @media_attachment, serializer: REST::MediaAttachmentSerializer, status: status_code_for_media_attachment | ||||||
|     render json: @media, serializer: REST::MediaAttachmentSerializer |  | ||||||
|   end |   end | ||||||
| 
 | 
 | ||||||
|   private |   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) |     params.permit(:file, :description, :focus) | ||||||
|   end |   end | ||||||
| 
 | 
 | ||||||
|  |  | ||||||
							
								
								
									
										12
									
								
								app/controllers/api/v2/media_controller.rb
									
										
									
									
									
										Normal file
									
								
							
							
						
						
									
										12
									
								
								app/controllers/api/v2/media_controller.rb
									
										
									
									
									
										Normal file
									
								
							|  | @ -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 | ||||||
|  | @ -230,12 +230,31 @@ export function uploadCompose(files) { | ||||||
|         // Account for disparity in size of original image and resized data
 |         // Account for disparity in size of original image and resized data
 | ||||||
|         total += file.size - f.size; |         total += file.size - f.size; | ||||||
| 
 | 
 | ||||||
|         return api(getState).post('/api/v1/media', data, { |         return api(getState).post('/api/v2/media', data, { | ||||||
|           onUploadProgress: function({ loaded }){ |           onUploadProgress: function({ loaded }){ | ||||||
|             progress[i] = loaded; |             progress[i] = loaded; | ||||||
|             dispatch(uploadComposeProgress(progress.reduce((a, v) => a + v, 0), total)); |             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))); |       }).catch(error => dispatch(uploadComposeFail(error))); | ||||||
|     }; |     }; | ||||||
|   }; |   }; | ||||||
|  |  | ||||||
|  | @ -74,7 +74,7 @@ module Attachmentable | ||||||
|     self.class.attachment_definitions.each_key do |attachment_name| |     self.class.attachment_definitions.each_key do |attachment_name| | ||||||
|       attachment = send(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)) |       attachment.instance_write :file_name, SecureRandom.hex(8) + File.extname(attachment.instance_read(:file_name)) | ||||||
|     end |     end | ||||||
|  |  | ||||||
|  | @ -19,12 +19,14 @@ | ||||||
| #  description         :text | #  description         :text | ||||||
| #  scheduled_status_id :bigint(8) | #  scheduled_status_id :bigint(8) | ||||||
| #  blurhash            :string | #  blurhash            :string | ||||||
|  | #  processing          :integer | ||||||
| # | # | ||||||
| 
 | 
 | ||||||
| class MediaAttachment < ApplicationRecord | class MediaAttachment < ApplicationRecord | ||||||
|   self.inheritance_column = nil |   self.inheritance_column = nil | ||||||
| 
 | 
 | ||||||
|   enum type: [:image, :gifv, :video, :unknown, :audio] |   enum type: [:image, :gifv, :video, :unknown, :audio] | ||||||
|  |   enum processing: [:queued, :in_progress, :complete, :failed], _prefix: true | ||||||
| 
 | 
 | ||||||
|   MAX_DESCRIPTION_LENGTH = 1_500 |   MAX_DESCRIPTION_LENGTH = 1_500 | ||||||
| 
 | 
 | ||||||
|  | @ -156,6 +158,10 @@ class MediaAttachment < ApplicationRecord | ||||||
|     remote_url.blank? |     remote_url.blank? | ||||||
|   end |   end | ||||||
| 
 | 
 | ||||||
|  |   def not_processed? | ||||||
|  |     processing.present? && !processing_complete? | ||||||
|  |   end | ||||||
|  | 
 | ||||||
|   def needs_redownload? |   def needs_redownload? | ||||||
|     file.blank? && remote_url.present? |     file.blank? && remote_url.present? | ||||||
|   end |   end | ||||||
|  | @ -203,10 +209,18 @@ class MediaAttachment < ApplicationRecord | ||||||
|     "#{x},#{y}" |     "#{x},#{y}" | ||||||
|   end |   end | ||||||
| 
 | 
 | ||||||
|  |   attr_writer :delay_processing | ||||||
|  | 
 | ||||||
|  |   def delay_processing? | ||||||
|  |     @delay_processing | ||||||
|  |   end | ||||||
|  | 
 | ||||||
|  |   after_commit :enqueue_processing, on: :create | ||||||
|   after_commit :reset_parent_cache, on: :update |   after_commit :reset_parent_cache, on: :update | ||||||
| 
 | 
 | ||||||
|   before_create :prepare_description, unless: :local? |   before_create :prepare_description, unless: :local? | ||||||
|   before_create :set_shortcode |   before_create :set_shortcode | ||||||
|  |   before_create :set_processing | ||||||
| 
 | 
 | ||||||
|   before_post_process :set_type_and_extension |   before_post_process :set_type_and_extension | ||||||
| 
 | 
 | ||||||
|  | @ -277,6 +291,10 @@ class MediaAttachment < ApplicationRecord | ||||||
|     end |     end | ||||||
|   end |   end | ||||||
| 
 | 
 | ||||||
|  |   def set_processing | ||||||
|  |     self.processing = delay_processing? ? :queued : :complete | ||||||
|  |   end | ||||||
|  | 
 | ||||||
|   def set_meta |   def set_meta | ||||||
|     meta = populate_meta |     meta = populate_meta | ||||||
| 
 | 
 | ||||||
|  | @ -322,9 +340,11 @@ class MediaAttachment < ApplicationRecord | ||||||
|     }.compact |     }.compact | ||||||
|   end |   end | ||||||
| 
 | 
 | ||||||
|   def reset_parent_cache |   def enqueue_processing | ||||||
|     return if status_id.nil? |     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 | ||||||
| end | end | ||||||
|  |  | ||||||
|  | @ -12,7 +12,9 @@ class REST::MediaAttachmentSerializer < ActiveModel::Serializer | ||||||
|   end |   end | ||||||
| 
 | 
 | ||||||
|   def url |   def url | ||||||
|     if object.needs_redownload? |     if object.not_processed? | ||||||
|  |       nil | ||||||
|  |     elsif object.needs_redownload? | ||||||
|       media_proxy_url(object.id, :original) |       media_proxy_url(object.id, :original) | ||||||
|     else |     else | ||||||
|       full_asset_url(object.file.url(:original)) |       full_asset_url(object.file.url(:original)) | ||||||
|  |  | ||||||
|  | @ -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)) |     @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.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 |   end | ||||||
| 
 | 
 | ||||||
|   def language_from_option(str) |   def language_from_option(str) | ||||||
|  |  | ||||||
|  | @ -9,8 +9,12 @@ class BackupWorker | ||||||
|     backup_id = msg['args'].first |     backup_id = msg['args'].first | ||||||
| 
 | 
 | ||||||
|     ActiveRecord::Base.connection_pool.with_connection do |     ActiveRecord::Base.connection_pool.with_connection do | ||||||
|       backup = Backup.find(backup_id) |       begin | ||||||
|       backup&.destroy |         backup = Backup.find(backup_id) | ||||||
|  |         backup.destroy | ||||||
|  |       rescue ActiveRecord::RecordNotFound | ||||||
|  |         true | ||||||
|  |       end | ||||||
|     end |     end | ||||||
|   end |   end | ||||||
| 
 | 
 | ||||||
|  |  | ||||||
							
								
								
									
										34
									
								
								app/workers/post_process_media_worker.rb
									
										
									
									
									
										Normal file
									
								
							
							
						
						
									
										34
									
								
								app/workers/post_process_media_worker.rb
									
										
									
									
									
										Normal file
									
								
							|  | @ -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 | ||||||
|  | @ -7,6 +7,7 @@ require 'rails/all' | ||||||
| Bundler.require(*Rails.groups) | Bundler.require(*Rails.groups) | ||||||
| 
 | 
 | ||||||
| require_relative '../app/lib/exceptions' | require_relative '../app/lib/exceptions' | ||||||
|  | require_relative '../lib/paperclip/attachment_extensions' | ||||||
| require_relative '../lib/paperclip/lazy_thumbnail' | require_relative '../lib/paperclip/lazy_thumbnail' | ||||||
| require_relative '../lib/paperclip/gif_transcoder' | require_relative '../lib/paperclip/gif_transcoder' | ||||||
| require_relative '../lib/paperclip/video_transcoder' | require_relative '../lib/paperclip/video_transcoder' | ||||||
|  |  | ||||||
|  | @ -853,6 +853,7 @@ en: | ||||||
|   media_attachments: |   media_attachments: | ||||||
|     validations: |     validations: | ||||||
|       images_and_video: Cannot attach a video to a status that already contains images |       images_and_video: Cannot attach a video to a status that already contains images | ||||||
|  |       not_ready: Cannot attach files that have not finished processing. Try again in a moment! | ||||||
|       too_many: Cannot attach more than 4 files |       too_many: Cannot attach more than 4 files | ||||||
|   migrations: |   migrations: | ||||||
|     acct: Moved to |     acct: Moved to | ||||||
|  |  | ||||||
|  | @ -343,7 +343,7 @@ Rails.application.routes.draw do | ||||||
|         end |         end | ||||||
|       end |       end | ||||||
| 
 | 
 | ||||||
|       resources :media,        only: [:create, :update] |       resources :media,        only: [:create, :update, :show] | ||||||
|       resources :blocks,       only: [:index] |       resources :blocks,       only: [:index] | ||||||
|       resources :mutes,        only: [:index] |       resources :mutes,        only: [:index] | ||||||
|       resources :favourites,   only: [:index] |       resources :favourites,   only: [:index] | ||||||
|  | @ -455,6 +455,7 @@ Rails.application.routes.draw do | ||||||
|     end |     end | ||||||
| 
 | 
 | ||||||
|     namespace :v2 do |     namespace :v2 do | ||||||
|  |       resources :media, only: [:create] | ||||||
|       get '/search', to: 'search#index', as: :search |       get '/search', to: 'search#index', as: :search | ||||||
|     end |     end | ||||||
| 
 | 
 | ||||||
|  |  | ||||||
|  | @ -0,0 +1,5 @@ | ||||||
|  | class AddProcessingToMediaAttachments < ActiveRecord::Migration[5.2] | ||||||
|  |   def change | ||||||
|  |     add_column :media_attachments, :processing, :integer | ||||||
|  |   end | ||||||
|  | end | ||||||
|  | @ -10,7 +10,7 @@ | ||||||
| # | # | ||||||
| # It's strongly recommended that you check this file into your version control system. | # It's strongly recommended that you check this file into your version control system. | ||||||
| 
 | 
 | ||||||
| ActiveRecord::Schema.define(version: 2020_01_26_203551) do | ActiveRecord::Schema.define(version: 2020_03_06_035625) do | ||||||
| 
 | 
 | ||||||
|   # These are extensions that must be enabled in order to support this database |   # These are extensions that must be enabled in order to support this database | ||||||
|   enable_extension "plpgsql" |   enable_extension "plpgsql" | ||||||
|  | @ -459,6 +459,7 @@ ActiveRecord::Schema.define(version: 2020_01_26_203551) do | ||||||
|     t.text "description" |     t.text "description" | ||||||
|     t.bigint "scheduled_status_id" |     t.bigint "scheduled_status_id" | ||||||
|     t.string "blurhash" |     t.string "blurhash" | ||||||
|  |     t.integer "processing" | ||||||
|     t.index ["account_id"], name: "index_media_attachments_on_account_id" |     t.index ["account_id"], name: "index_media_attachments_on_account_id" | ||||||
|     t.index ["scheduled_status_id"], name: "index_media_attachments_on_scheduled_status_id" |     t.index ["scheduled_status_id"], name: "index_media_attachments_on_scheduled_status_id" | ||||||
|     t.index ["shortcode"], name: "index_media_attachments_on_shortcode", unique: true |     t.index ["shortcode"], name: "index_media_attachments_on_shortcode", unique: true | ||||||
|  |  | ||||||
							
								
								
									
										30
									
								
								lib/paperclip/attachment_extensions.rb
									
										
									
									
									
										Normal file
									
								
							
							
						
						
									
										30
									
								
								lib/paperclip/attachment_extensions.rb
									
										
									
									
									
										Normal file
									
								
							|  | @ -0,0 +1,30 @@ | ||||||
|  | # frozen_string_literal: true | ||||||
|  | 
 | ||||||
|  | module Paperclip | ||||||
|  |   module AttachmentExtensions | ||||||
|  |     # We overwrite this method to support delayed processing in | ||||||
|  |     # Sidekiq. Since we process the original file to reduce disk | ||||||
|  |     # usage, and we still want to generate thumbnails straight | ||||||
|  |     # away, it's the only style we need to exclude | ||||||
|  |     def process_style?(style_name, style_args) | ||||||
|  |       if style_name == :original && instance.respond_to?(:delay_processing?) && instance.delay_processing? | ||||||
|  |         false | ||||||
|  |       else | ||||||
|  |         style_args.empty? || style_args.include?(style_name) | ||||||
|  |       end | ||||||
|  |     end | ||||||
|  | 
 | ||||||
|  |     def reprocess_original! | ||||||
|  |       old_original_path = path(:original) | ||||||
|  |       reprocess!(:original) | ||||||
|  |       new_original_path = path(:original) | ||||||
|  | 
 | ||||||
|  |       if new_original_path != old_original_path | ||||||
|  |         @queued_for_delete << old_original_path | ||||||
|  |         flush_deletes | ||||||
|  |       end | ||||||
|  |     end | ||||||
|  |   end | ||||||
|  | end | ||||||
|  | 
 | ||||||
|  | Paperclip::Attachment.prepend(Paperclip::AttachmentExtensions) | ||||||
		Loading…
	
	Add table
		Add a link
		
	
		Reference in a new issue