diff --git a/app/logical/media_file.rb b/app/logical/media_file.rb index d7991c61b..ac4dd8bcd 100644 --- a/app/logical/media_file.rb +++ b/app/logical/media_file.rb @@ -19,6 +19,8 @@ class MediaFile # @param options [Hash] extra options for the MediaFile subclass. # @return [MediaFile] the media file def self.open(file, **options) + return file.dup if file.is_a?(MediaFile) + file = Kernel.open(file, "r", binmode: true) unless file.respond_to?(:read) case file_ext(file) diff --git a/app/logical/media_file/ugoira.rb b/app/logical/media_file/ugoira.rb index 893457731..cfd765131 100644 --- a/app/logical/media_file/ugoira.rb +++ b/app/logical/media_file/ugoira.rb @@ -7,7 +7,7 @@ # zip file, so it must be passed around separately. class MediaFile::Ugoira < MediaFile class Error < StandardError; end - attr_reader :frame_data + attr_accessor :frame_data def initialize(file, frame_data: {}, **options) super(file, **options) diff --git a/app/logical/sources/strategies/base.rb b/app/logical/sources/strategies/base.rb index 89ec4124f..6fe42e0aa 100644 --- a/app/logical/sources/strategies/base.rb +++ b/app/logical/sources/strategies/base.rb @@ -276,10 +276,6 @@ module Sources image_url end - def data - {} - end - def tags (@tags || []).uniq end diff --git a/app/logical/sources/strategies/pixiv.rb b/app/logical/sources/strategies/pixiv.rb index fb80f8e6b..af7650438 100644 --- a/app/logical/sources/strategies/pixiv.rb +++ b/app/logical/sources/strategies/pixiv.rb @@ -199,6 +199,12 @@ module Sources tag.gsub(/\d+users入り\z/i, "") end + def download_file!(url = image_url) + file = super(url) + file.frame_data = ugoira_frame_data if is_ugoira? + file + end + def translate_tag(tag) translated_tags = super(tag) @@ -297,8 +303,9 @@ module Sources end end - def data - { ugoira_frame_data: api_ugoira[:frames] } + def ugoira_frame_data + return nil unless is_ugoira? + api_ugoira[:frames] end def ugoira_content_type diff --git a/app/logical/upload_service.rb b/app/logical/upload_service.rb index 12e95d90f..9e694cd64 100644 --- a/app/logical/upload_service.rb +++ b/app/logical/upload_service.rb @@ -43,7 +43,7 @@ class UploadService @upload.update(status: "processing") - @upload.file = Utils.get_file_for_upload(@upload, file: @upload.file&.tempfile) + @upload.file = Utils.get_file_for_upload(@upload.source_url, @upload.referer_url, @upload.file&.tempfile) Utils.process_file(upload, @upload.file) @upload.save! @@ -64,14 +64,6 @@ class UploadService @post = convert_to_post(upload) @post.save! - if upload.context && upload.context["ugoira"] - PixivUgoiraFrameData.create( - post_id: @post.id, - data: upload.context["ugoira"]["frame_data"], - content_type: upload.context["ugoira"]["content_type"] - ) - end - if upload.has_commentary? @post.create_artist_commentary( :original_title => upload.artist_commentary_title, diff --git a/app/logical/upload_service/preprocessor.rb b/app/logical/upload_service/preprocessor.rb index f8b298e17..a90444df5 100644 --- a/app/logical/upload_service/preprocessor.rb +++ b/app/logical/upload_service/preprocessor.rb @@ -71,7 +71,7 @@ class UploadService begin upload.update(status: "preprocessing") - file = Utils.get_file_for_upload(upload, file: params[:file]&.tempfile) + file = Utils.get_file_for_upload(upload.source_url, upload.referer_url, params[:file]&.tempfile) Utils.process_file(upload, file, original_post_id: original_post_id) upload.rating = params[:rating] diff --git a/app/logical/upload_service/replacer.rb b/app/logical/upload_service/replacer.rb index 739d7c161..684c032e9 100644 --- a/app/logical/upload_service/replacer.rb +++ b/app/logical/upload_service/replacer.rb @@ -61,69 +61,44 @@ class UploadService undoer.process! end - def source_strategy(upload) - Sources::Strategies.find(upload.source, upload.referer_url) - end - - def find_replacement_url(repl, upload) - if repl.replacement_file.present? - return "file://#{repl.replacement_file.original_filename}" + def replacement_url + if replacement.replacement_file.present? + "file://#{replacement.replacement_file.original_filename}" + else + Sources::Strategies.find(replacement.replacement_url).canonical_url end - - if upload.source.blank? - raise "No source found in upload for replacement" - end - - if source_strategy(upload).canonical_url.present? - return source_strategy(upload).canonical_url - end - - upload.source end def process! - preprocessor = Preprocessor.new( - rating: post.rating, - tag_string: replacement.tags, - source: replacement.replacement_url, - file: replacement.replacement_file, - replaced_post: post, - original_post_id: post.id - ) - upload = preprocessor.start! - raise Error, upload.status if upload.is_errored? - upload = preprocessor.finish!(upload) - raise Error, upload.status if upload.is_errored? - md5_changed = upload.md5 != post.md5 + media_file = Utils::get_file_for_upload(replacement.replacement_url, nil, replacement.replacement_file&.tempfile) - replacement.replacement_url = find_replacement_url(replacement, upload) - - if md5_changed - post.queue_delete_files(PostReplacement::DELETION_GRACE_PERIOD) + if media_file.md5 == post.md5 + raise Error, "Can't replace a post with itself; regenerate the post instead" + elsif Post.exists?(md5: media_file.md5) + raise Error, "Duplicate: post with md5 #{media_file.md5} already exists" end - replacement.file_ext = upload.file_ext - replacement.file_size = upload.file_size - replacement.image_height = upload.image_height - replacement.image_width = upload.image_width - replacement.md5 = upload.md5 + media_asset = MediaAsset.upload!(media_file) + post.queue_delete_files(PostReplacement::DELETION_GRACE_PERIOD) - post.md5 = upload.md5 - post.file_ext = upload.file_ext - post.image_width = upload.image_width - post.image_height = upload.image_height - post.file_size = upload.file_size + replacement.replacement_url = replacement_url + replacement.file_ext = media_asset.file_ext + replacement.file_size = media_asset.file_size + replacement.image_height = media_asset.image_height + replacement.image_width = media_asset.image_width + replacement.md5 = media_asset.md5 + + post.md5 = media_asset.md5 + post.file_ext = media_asset.file_ext + post.image_width = media_asset.image_width + post.image_height = media_asset.image_height + post.file_size = media_asset.file_size post.source = replacement.final_source.presence || replacement.replacement_url - post.tag_string = upload.tag_string + post.tag_string = replacement.tags rescale_notes(post) - update_ugoira_frame_data(post, upload) - if md5_changed - CurrentUser.scoped(User.system) { Comment.create!(post: post, creator: User.system, updater: User.system, body: comment_replacement_message(post, replacement), do_not_bump_post: true, creator_ip_addr: "127.0.0.1") } - else - purge_cached_urls(post) - end + CurrentUser.scoped(User.system) { Comment.create!(post: post, creator: User.system, updater: User.system, body: comment_replacement_message(post, replacement), do_not_bump_post: true, creator_ip_addr: "127.0.0.1") } replacement.save! post.save! @@ -131,18 +106,6 @@ class UploadService post.update_iqdb end - def purge_cached_urls(post) - urls = [ - post.preview_file_url, - post.large_file_url, - post.file_url, - post.tagged_large_file_url, - post.tagged_file_url - ] - - CloudflareService.new.purge_cache(urls) - end - def rescale_notes(post) x_scale = post.image_width.to_f / post.image_width_was.to_f y_scale = post.image_height.to_f / post.image_height_was.to_f @@ -151,19 +114,5 @@ class UploadService note.rescale!(x_scale, y_scale) end end - - def update_ugoira_frame_data(post, upload) - post.pixiv_ugoira_frame_data.destroy if post.pixiv_ugoira_frame_data.present? - - unless post.is_ugoira? - return - end - - PixivUgoiraFrameData.create( - post_id: post.id, - data: upload.context["ugoira"]["frame_data"], - content_type: upload.context["ugoira"]["content_type"] - ) - end end end diff --git a/app/logical/upload_service/utils.rb b/app/logical/upload_service/utils.rb index ed23c8fc9..f5a219706 100644 --- a/app/logical/upload_service/utils.rb +++ b/app/logical/upload_service/utils.rb @@ -7,9 +7,9 @@ class UploadService end def process_file(upload, file, original_post_id: nil) - upload.file = file - media_file = upload.media_file + media_file = MediaFile.open(file) + upload.file = media_file upload.file_ext = media_file.file_ext.to_s upload.file_size = media_file.file_size upload.md5 = media_file.md5 @@ -28,25 +28,14 @@ class UploadService tags.join(" ") end - def get_file_for_upload(upload, file: nil) - return file if file.present? - raise "No file or source URL provided" if upload.source_url.blank? + def get_file_for_upload(source_url, referer_url, file) + return MediaFile.open(file) if file.present? + raise "No file or source URL provided" if source_url.blank? - strategy = Sources::Strategies.find(upload.source_url, upload.referer_url) + strategy = Sources::Strategies.find(source_url, referer_url) raise NotImplementedError, "No login credentials configured for #{strategy.site_name}." unless strategy.class.enabled? - file = strategy.download_file! - - if strategy.data[:ugoira_frame_data].present? - upload.context = { - "ugoira" => { - "frame_data" => strategy.data[:ugoira_frame_data], - "content_type" => "image/jpeg" - } - } - end - - file + strategy.download_file! end end end diff --git a/app/models/media_asset.rb b/app/models/media_asset.rb index dff00ebf1..3be6214e5 100644 --- a/app/models/media_asset.rb +++ b/app/models/media_asset.rb @@ -1,5 +1,7 @@ class MediaAsset < ApplicationRecord has_one :media_metadata, dependent: :destroy + has_one :pixiv_ugoira_frame_data, class_name: "PixivUgoiraFrameData", dependent: :destroy, foreign_key: :md5, primary_key: :md5 + delegate :metadata, to: :media_metadata delegate :is_non_repeating_animation?, :is_greyscale?, :is_rotated?, to: :metadata @@ -136,6 +138,7 @@ class MediaAsset < ApplicationRecord self.image_height = media_file.height self.duration = media_file.duration self.media_metadata = MediaMetadata.new(file: media_file) + self.pixiv_ugoira_frame_data = PixivUgoiraFrameData.new(data: media_file.frame_data, content_type: "image/jpeg") if is_ugoira? end def delete_files! diff --git a/app/models/pixiv_ugoira_frame_data.rb b/app/models/pixiv_ugoira_frame_data.rb index bbb5a5498..89b7d97b0 100644 --- a/app/models/pixiv_ugoira_frame_data.rb +++ b/app/models/pixiv_ugoira_frame_data.rb @@ -1,5 +1,6 @@ class PixivUgoiraFrameData < ApplicationRecord - belongs_to :post + belongs_to :post, optional: true, foreign_key: :md5, primary_key: :md5 + belongs_to :media_asset, foreign_key: :md5, primary_key: :md5 serialize :data before_validation :normalize_data, on: :create @@ -9,7 +10,7 @@ class PixivUgoiraFrameData < ApplicationRecord end def self.search(params) - q = search_attributes(params, :id, :data, :content_type, :post) + q = search_attributes(params, :id, :data, :content_type, :post, :md5) q.apply_default_order(params) end diff --git a/app/models/post.rb b/app/models/post.rb index d1e11e3f5..e32ea6e4a 100644 --- a/app/models/post.rb +++ b/app/models/post.rb @@ -40,7 +40,7 @@ class Post < ApplicationRecord has_one :media_asset, foreign_key: :md5, primary_key: :md5 has_one :upload, :dependent => :destroy has_one :artist_commentary, :dependent => :destroy - has_one :pixiv_ugoira_frame_data, :class_name => "PixivUgoiraFrameData", :dependent => :destroy + has_one :pixiv_ugoira_frame_data, class_name: "PixivUgoiraFrameData", foreign_key: :md5, primary_key: :md5 has_many :flags, :class_name => "PostFlag", :dependent => :destroy has_many :appeals, :class_name => "PostAppeal", :dependent => :destroy has_many :votes, :class_name => "PostVote", :dependent => :destroy diff --git a/app/models/upload.rb b/app/models/upload.rb index 665dacbd5..12d636f65 100644 --- a/app/models/upload.rb +++ b/app/models/upload.rb @@ -19,7 +19,7 @@ class Upload < ApplicationRecord end def validate_integrity(record) - if record.media_file.is_corrupt? + if record.file.is_corrupt? record.errors.add(:file, "is corrupted") end end @@ -55,7 +55,7 @@ class Upload < ApplicationRecord end def validate_video_duration(record) - if !record.uploader.is_admin? && record.media_file.is_video? && record.media_file.duration > MAX_VIDEO_DURATION + if !record.uploader.is_admin? && record.file.is_video? && record.file.duration > MAX_VIDEO_DURATION record.errors.add(:base, "video must not be longer than #{MAX_VIDEO_DURATION.seconds.inspect}") end end @@ -105,10 +105,6 @@ class Upload < ApplicationRecord end concerning :FileMethods do - def media_file - @media_file ||= MediaFile.open(file, frame_data: context.to_h.dig("ugoira", "frame_data")) - end - def delete_files # md5 is blank if the upload errored out before downloading the file. if is_completed? || md5.blank? || Upload.exists?(md5: md5) || Post.exists?(md5: md5) diff --git a/db/migrate/20211018062916_change_post_id_to_nullable_on_pixiv_ugoira_frame_data.rb b/db/migrate/20211018062916_change_post_id_to_nullable_on_pixiv_ugoira_frame_data.rb new file mode 100644 index 000000000..0be2470eb --- /dev/null +++ b/db/migrate/20211018062916_change_post_id_to_nullable_on_pixiv_ugoira_frame_data.rb @@ -0,0 +1,5 @@ +class ChangePostIdToNullableOnPixivUgoiraFrameData < ActiveRecord::Migration[6.1] + def change + change_column_null :pixiv_ugoira_frame_data, :post_id, true + end +end diff --git a/db/structure.sql b/db/structure.sql index 8b8a2443e..318c286e7 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -1327,7 +1327,7 @@ ALTER SEQUENCE public.notes_id_seq OWNED BY public.notes.id; CREATE TABLE public.pixiv_ugoira_frame_data ( id integer NOT NULL, - post_id integer NOT NULL, + post_id integer, data text NOT NULL, content_type character varying NOT NULL, md5 character varying @@ -5048,6 +5048,7 @@ INSERT INTO "schema_migrations" (version) VALUES ('20211013011619'), ('20211014063943'), ('20211015223510'), -('20211018045429'); +('20211018045429'), +('20211018062916'); diff --git a/test/unit/downloads/pixiv_test.rb b/test/unit/downloads/pixiv_test.rb index 00248c1d6..f651a77e4 100644 --- a/test/unit/downloads/pixiv_test.rb +++ b/test/unit/downloads/pixiv_test.rb @@ -131,9 +131,10 @@ module Downloads context "An ugoira site for pixiv" do should "capture the data" do @strategy = Sources::Strategies.find("http://www.pixiv.net/member_illust.php?mode=medium&illust_id=62247364") + media_file = @strategy.download_file! - assert_equal(2, @strategy.data[:ugoira_frame_data].size) - assert_equal([{"file" => "000000.jpg", "delay" => 125}, {"file" => "000001.jpg", "delay" => 125}], @strategy.data[:ugoira_frame_data]) + assert_equal(2, media_file.frame_data.size) + assert_equal([{"file" => "000000.jpg", "delay" => 125}, {"file" => "000001.jpg", "delay" => 125}], media_file.frame_data) end end end diff --git a/test/unit/sources/pixiv_test.rb b/test/unit/sources/pixiv_test.rb index 87b89f9d9..146a4292e 100644 --- a/test/unit/sources/pixiv_test.rb +++ b/test/unit/sources/pixiv_test.rb @@ -48,10 +48,10 @@ module Sources end should "capture the frame data" do - ugoira_frame_data = @site.data[:ugoira_frame_data] + media_file = @site.download_file! - assert_equal(2, ugoira_frame_data.size) - assert_equal([{"file" => "000000.jpg", "delay" => 125}, {"file" => "000001.jpg", "delay" => 125}], ugoira_frame_data) + assert_equal(2, media_file.frame_data.size) + assert_equal([{"file" => "000000.jpg", "delay" => 125}, {"file" => "000001.jpg", "delay" => 125}], media_file.frame_data) end end diff --git a/test/unit/upload_service_test.rb b/test/unit/upload_service_test.rb index 56eb59339..aa86d5a80 100644 --- a/test/unit/upload_service_test.rb +++ b/test/unit/upload_service_test.rb @@ -23,59 +23,6 @@ class UploadServiceTest < ActiveSupport::TestCase end context "::Utils" do - context "#get_file_for_upload" do - context "for a non-source site" do - setup do - @source = "https://upload.wikimedia.org/wikipedia/commons/c/c5/Moraine_Lake_17092005.jpg" - @upload = Upload.new - @upload.source = @source - end - - should "work on a jpeg" do - file = UploadService::Utils.get_file_for_upload(@upload) - assert_operator(File.size(file.path), :>, 0) - file.close - end - end - - context "for a pixiv" do - setup do - skip "Pixiv credentials not configured" unless Sources::Strategies::Pixiv.enabled? - - @source = "http://www.pixiv.net/member_illust.php?mode=medium&illust_id=62247350" - @upload = Upload.new - @upload.source = @source - end - - should "work on an ugoira url" do - begin - file = UploadService::Utils.get_file_for_upload(@upload) - assert_operator(File.size(file.path), :>, 0) - file.close - end - end - end - - context "for a pixiv ugoira" do - setup do - skip "Pixiv credentials not configured" unless Sources::Strategies::Pixiv.enabled? - - @source = "https://i.pximg.net/img-zip-ugoira/img/2017/04/04/08/57/38/62247364_ugoira1920x1080.zip" - @referer = "http://www.pixiv.net/member_illust.php?mode=medium&illust_id=62247364" - @upload = Upload.new - @upload.source = @source - @upload.referer_url = @referer - end - - should "work on an ugoira url" do - file = UploadService::Utils.get_file_for_upload(@upload) - - assert_not_nil(@upload.context["ugoira"]) - assert_operator(File.size(file.path), :>, 0) - end - end - end - context ".process_file" do setup do @upload = FactoryBot.build(:jpg_upload) @@ -286,12 +233,6 @@ class UploadServiceTest < ActiveSupport::TestCase subject { UploadService::Replacer.new(post: @post, replacement: @replacement) } context "#process!" do - should "create a new upload" do - assert_difference(-> { Upload.count }) do - as(@user) { subject.process! } - end - end - should "create a comment" do assert_difference(-> { @post.comments.count }) do as(@user) { subject.process! } @@ -342,22 +283,13 @@ class UploadServiceTest < ActiveSupport::TestCase end context "a post with the same file" do - should "not raise a duplicate error" do + should "raise an error" do upload_file("test/files/test.png") do |file| - assert_nothing_raised do + assert_raises(UploadService::Replacer::Error) do as(@user) { @post.replace!(replacement_file: file, replacement_url: "") } end end end - - should "not queue a deletion or log a comment" do - upload_file("test/files/test.png") do |file| - assert_no_difference(-> { @post.comments.count }) do - as(@user) { @post.replace!(replacement_file: file, replacement_url: "") } - @post.reload - end - end - end end end @@ -407,9 +339,10 @@ class UploadServiceTest < ActiveSupport::TestCase subject { UploadService::Replacer.new(post: @post, replacement: @replacement) } context "when replacing with its own source" do - should "work" do - as(@user) { @post.replace!(replacement_url: @post.source) } - assert_equal(@post_md5, @post.md5) + should "raise an error" do + assert_raises(UploadService::Replacer::Error) do + as(@user) { @post.replace!(replacement_url: @post.source) } + end end end @@ -480,12 +413,6 @@ class UploadServiceTest < ActiveSupport::TestCase end context "#process!" do - should "create a new upload" do - assert_difference(-> { Upload.count }) do - as(@user) { subject.process! } - end - end - should "create a comment" do assert_difference(-> { @post.comments.count }) do as(@user) { subject.process! } @@ -911,20 +838,6 @@ class UploadServiceTest < ActiveSupport::TestCase end end - context "for a pixiv ugoira" do - setup do - @upload = FactoryBot.create(:ugoira_upload, file_size: 1000, md5: "12345", file_ext: "jpg", image_width: 100, image_height: 100, context: UGOIRA_CONTEXT) - end - - should "create a post" do - assert_difference(-> { PixivUgoiraFrameData.count }) do - post = subject.new({}).create_post_from_upload(@upload) - assert_equal([], post.errors.full_messages) - assert_not_nil(post.id) - end - end - end - context "for nijie" do should "record the canonical source" do page_url = "https://nijie.info/view.php?id=728995"