StorageManager: remove Post-specific code.

Refactor StorageManager to remove all image URL generation code. Instead
the image URL generation code lives in MediaAsset.

Now StorageManager is only concerned with how to read and write files to
remote storage backends like S3 or SFTP, not with how image URLs should
be generated. This way the file storage code isn't tightly coupled to
posts, so it can be used to store any kind of file, not just images
belonging to posts.

app/logical/storage_manager.rb

@@ -48,102 +48,8 @@ class StorageManager
     raise NotImplementedError, "open not implemented"
   end
 
-  # Store or replace the given file belonging to the given post.
+  def file_url(path)
-  # @param io [IO] the file to store
+    File.join(base_url, path)
-  # @param post [Post] the post the image belongs to
-  # @param type [Symbol] the image variant to store (:preview, :crop, :large, :original)
-  def store_file(io, post, type)
-    store(io, file_path(post.md5, post.file_ext, type))
-  end
-
-  # Delete the file belonging to the given post.
-  # @param post_id [Integer] the post's id
-  # @param md5 [String] the post's md5
-  # @param file_ext [String] the post's file extension
-  # @param type [Symbol] the image variant to delete (:preview, :crop, :large, :original)
-  def delete_file(post_id, md5, file_ext, type)
-    delete(file_path(md5, file_ext, type))
-  end
-
-  # Return a readonly copy of the image belonging to the given post.
-  # @param post [Post] the post
-  # @param type [Symbol] the image variant to open (:preview, :crop, :large, :original)
-  # @return [MediaFile] the image file
-  def open_file(post, type)
-    self.open(file_path(post.md5, post.file_ext, type))
-  end
-
-  # Generate the image URL for the given post.
-  # @param post [Post] the post
-  # @param type [Symbol] the post's image variant (:preview, :crop, :large, :original)
-  # @param tagged_filename [Boolean] whether the URL should contain the post's tags
-  # @return [String] the image URL
-  def file_url(post, type, tagged_filenames: false)
-    subdir = subdir_for(post.md5)
-    file = file_name(post.md5, post.file_ext, type)
-    seo_tags = seo_tags(post) if tagged_filenames
-
-    if type == :preview && !post.has_preview?
-      "#{root_url}/images/download-preview.png"
-    elsif type == :preview
-      "#{base_url}/preview/#{subdir}#{file}"
-    elsif type == :crop
-      "#{base_url}/crop/#{subdir}#{file}"
-    elsif type == :large && post.has_large?
-      "#{base_url}/sample/#{subdir}#{seo_tags}#{file}"
-    else
-      "#{base_url}/original/#{subdir}#{seo_tags}#{post.md5}.#{post.file_ext}"
-    end
-  end
-
-  def root_url
-    origin = Addressable::URI.parse(base_url).origin
-    origin = "" if origin == "null" # base_url was relative
-    origin
-  end
-
-  def file_path(post_or_md5, file_ext, type)
-    md5 = post_or_md5.is_a?(String) ? post_or_md5 : post_or_md5.md5
-    subdir = subdir_for(md5)
-    file = file_name(md5, file_ext, type)
-
-    case type
-    when :preview
-      "/preview/#{subdir}#{file}"
-    when :crop
-      "/crop/#{subdir}#{file}"
-    when :large
-      "/sample/#{subdir}#{file}"
-    when :original
-      "/original/#{subdir}#{file}"
-    end
-  end
-
-  def file_name(md5, file_ext, type)
-    large_file_ext = (file_ext == "zip") ? "webm" : "jpg"
-
-    case type
-    when :preview
-      "#{md5}.jpg"
-    when :crop
-      "#{md5}.jpg"
-    when :large
-      "sample-#{md5}.#{large_file_ext}"
-    when :original
-      "#{md5}.#{file_ext}"
-    end
-  end
-
-  def subdir_for(md5)
-    "#{md5[0..1]}/#{md5[2..3]}/"
-  end
-
-  # Generate the tags in the image URL.
-  def seo_tags(post)
-    return "" if !tagged_filenames
-
-    tags = post.presenter.humanized_essential_tag_string.gsub(/[^a-z0-9]+/, "_").gsub(/(?:^_+)|(?:_+$)/, "").gsub(/_{2,}/, "_")
-    "__#{tags}__"
   end
 
   def full_path(path)

app/models/media_asset.rb

@@ -38,7 +38,9 @@ class MediaAsset < ApplicationRecord
     end
 
     def open_file
-      storage_service.open(file_path)
+      file = storage_service.open(file_path)
+      frame_data = media_asset.pixiv_ugoira_frame_data&.data if media_asset.is_ugoira?
+      MediaFile.open(file, frame_data: frame_data)
     end
 
     def convert_file(media_file)

app/models/post.rb

@@ -74,36 +74,47 @@ class Post < ApplicationRecord
   module FileMethods
     extend ActiveSupport::Concern
 
-    def storage_manager
+    def seo_tags
-      Danbooru.config.storage_manager
+      presenter.humanized_essential_tag_string.gsub(/[^a-z0-9]+/, "_").gsub(/(?:^_+)|(?:_+$)/, "").gsub(/_{2,}/, "_")
     end
 
     def file(type = :original)
-      storage_manager.open_file(self, type)
+      media_asset.variant(type).open_file
     end
 
     def tagged_file_url(tagged_filenames: !CurrentUser.user.disable_tagged_filenames?)
-      storage_manager.file_url(self, :original, tagged_filenames: tagged_filenames)
+      slug = seo_tags if tagged_filenames
+      media_asset.variant(:original).file_url(slug)
     end
 
     def tagged_large_file_url(tagged_filenames: !CurrentUser.user.disable_tagged_filenames?)
-      storage_manager.file_url(self, :large, tagged_filenames: tagged_filenames)
+      slug = seo_tags if tagged_filenames
+
+      if media_asset.has_variant?(:sample)
+        media_asset.variant(:sample).file_url(slug)
+      else
+        media_asset.variant(:original).file_url(slug)
+      end
     end
 
     def file_url
-      storage_manager.file_url(self, :original)
+      media_asset.variant(:original).file_url
     end
 
     def large_file_url
-      storage_manager.file_url(self, :large)
+      if media_asset.has_variant?(:sample)
+        media_asset.variant(:sample).file_url
+      else
+        media_asset.variant(:original).file_url
+      end
     end
 
     def preview_file_url
-      storage_manager.file_url(self, :preview)
+      media_asset.variant(:preview).file_url
     end
 
     def crop_file_url
-      storage_manager.file_url(self, :crop)
+      media_asset.variant(:crop).file_url
     end
 
     def open_graph_image_url
@@ -1162,7 +1173,7 @@ class Post < ApplicationRecord
 
         ModAction.log("<@#{user.name}> regenerated IQDB for post ##{id}", :post_regenerate_iqdb, user)
       else
-        media_file = MediaFile.open(file, frame_data: pixiv_ugoira_frame_data&.data.to_a)
+        media_file = media_asset.variant(:original).open_file
         media_asset.distribute_files!(media_file)
 
         update!(

lib/tasks/danbooru.rake

@@ -70,8 +70,7 @@ namespace :danbooru do
 
       MediaMetadata.joins(:media_asset).where(metadata: {}).find_each do |metadata|
         asset = metadata.media_asset
-        file = sm.open(sm.file_path(asset.md5, asset.file_ext, :original))
+        media_file = asset.variant(:original).open_file
-        media_file = MediaFile.open(file)
 
         metadata.update!(metadata: media_file.metadata)
         puts "metadata[id=#{metadata.id}, md5=#{asset.md5}]: #{media_file.metadata.count}"

test/functional/post_regenerations_controller_test.rb

@@ -40,8 +40,8 @@ class PostRegenerationsControllerTest < ActionDispatch::IntegrationTest
 
       context "for an image sample regeneration" do
         should "regenerate missing thumbnails" do
-          @preview_file_size = @post.file(:preview).size
+          @preview_file_size = @post.media_asset.variant(:preview).open_file.size
-          @post.storage_manager.delete_file(@post.id, @post.md5, @post.file_ext, :preview)
+          @post.media_asset.variant(:preview).delete_file!
           assert_raise(Errno::ENOENT) { @post.file(:preview) }
 
           post_auth post_regenerations_path, @mod, params: { post_id: @post.id }

test/unit/post_test.rb

@@ -1626,14 +1626,11 @@ class PostTest < ActiveSupport::TestCase
 
   context "URLs:" do
     should "generate the correct urls for animated gifs" do
-      manager = StorageManager::Local.new(base_url: "https://test.com/data", base_dir: "/")
+      @post = create(:post_with_file, filename: "test-animated-86x52.gif")
-      Danbooru.config.stubs(:storage_manager).returns(manager)
 
-      @post = build(:post, md5: "deadbeef", file_ext: "gif", tag_string: "animated_gif")
+      assert_equal("https://www.example.com/data/preview/77/d8/77d89bda37ea3af09158ed3282f8334f.jpg", @post.preview_file_url)
-
+      assert_equal("https://www.example.com/data/original/77/d8/77d89bda37ea3af09158ed3282f8334f.gif", @post.large_file_url)
-      assert_equal("https://test.com/data/preview/de/ad/deadbeef.jpg", @post.preview_file_url)
+      assert_equal("https://www.example.com/data/original/77/d8/77d89bda37ea3af09158ed3282f8334f.gif", @post.file_url)
-      assert_equal("https://test.com/data/original/de/ad/deadbeef.gif", @post.large_file_url)
-      assert_equal("https://test.com/data/original/de/ad/deadbeef.gif", @post.file_url)
     end
   end
 
@@ -1642,7 +1639,6 @@ class PostTest < ActiveSupport::TestCase
 
     setup do
       @post = FactoryBot.create(:post)
-      @post.stubs(:queue_delete_files)
     end
 
     should "update the post" do

test/unit/storage_manager_test.rb

@@ -37,58 +37,5 @@ class StorageManagerTest < ActiveSupport::TestCase
         assert_nothing_raised { @storage_manager.delete("dne.txt") }
       end
     end
-
-    context "#store_file and #delete_file methods" do
-      setup do
-        @post = FactoryBot.create(:post, file_ext: "png")
-
-        @storage_manager.store_file(StringIO.new("data"), @post, :preview)
-        @storage_manager.store_file(StringIO.new("data"), @post, :large)
-        @storage_manager.store_file(StringIO.new("data"), @post, :original)
-        subdir = "#{@post.md5[0..1]}/#{@post.md5[2..3]}"
-
-        @file_path = "#{@temp_dir}/preview/#{subdir}/#{@post.md5}.jpg"
-        @large_file_path = "#{@temp_dir}/sample/#{subdir}/sample-#{@post.md5}.jpg"
-        @preview_file_path = "#{@temp_dir}/original/#{subdir}/#{@post.md5}.#{@post.file_ext}"
-      end
-
-      should "store the files at the correct path" do
-        assert(File.exist?(@file_path))
-        assert(File.exist?(@large_file_path))
-        assert(File.exist?(@preview_file_path))
-      end
-
-      should "delete the files" do
-        @storage_manager.delete_file(@post.id, @post.md5, @post.file_ext, :preview)
-        @storage_manager.delete_file(@post.id, @post.md5, @post.file_ext, :large)
-        @storage_manager.delete_file(@post.id, @post.md5, @post.file_ext, :original)
-
-        assert_not(File.exist?(@file_path))
-        assert_not(File.exist?(@large_file_path))
-        assert_not(File.exist?(@preview_file_path))
-      end
-    end
-
-    context "#file_url method" do
-      should "return the correct urls" do
-        @post = FactoryBot.create(:post, file_ext: "png")
-        @storage_manager.stubs(:tagged_filenames).returns(false)
-        subdir = "#{@post.md5[0..1]}/#{@post.md5[2..3]}"
-
-        assert_equal("/data/original/#{subdir}/#{@post.md5}.png", @storage_manager.file_url(@post, :original))
-        assert_equal("/data/sample/#{subdir}/sample-#{@post.md5}.jpg", @storage_manager.file_url(@post, :large))
-        assert_equal("/data/preview/#{subdir}/#{@post.md5}.jpg", @storage_manager.file_url(@post, :preview))
-      end
-
-      should "return the correct url for flash files" do
-        @post = FactoryBot.create(:post, file_ext: "swf")
-
-        @storage_manager.stubs(:base_url).returns("/data")
-        assert_equal("/images/download-preview.png", @storage_manager.file_url(@post, :preview))
-
-        @storage_manager.stubs(:base_url).returns("http://localhost/data")
-        assert_equal("http://localhost/images/download-preview.png", @storage_manager.file_url(@post, :preview))
-      end
-    end
   end
 end