From 3d414f3986c6488829da7734bd1be5085e405b3f Mon Sep 17 00:00:00 2001 From: Vipul A M Date: Mon, 27 Mar 2023 23:24:43 +0530 Subject: [PATCH] Remove mini_mime usage in favour of marcel We are using two libraries to do the same job. This commit removes the usage of mini_mime in favour of Marcel instead. Changes are as follows: - Replace MiniMime lookup by extension with Marcel Mimetype for lookup with extension - Replaces usage of MiniMime lookup by content type to fetch extension with usage of Marcel Magic lookup. Marcel has multiple extentions being returned, we pick the first one. MiniMime always returns just one - Removes specs which we specifically checking failing identification issue of MiniMine on jpeg images - Removes mini_mime from gemspec --- Gemfile.lock | 1 - activestorage/CHANGELOG.md | 7 +++++++ activestorage/activestorage.gemspec | 1 - .../active_storage/blob/representable.rb | 6 +++--- .../app/models/active_storage/variation.rb | 6 +++--- activestorage/test/engine_test.rb | 20 +++++-------------- activestorage/test/models/variant_test.rb | 4 ++-- 7 files changed, 20 insertions(+), 25 deletions(-) diff --git a/Gemfile.lock b/Gemfile.lock index 90c4af8b622..075fc1aedaf 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -72,7 +72,6 @@ PATH activerecord (= 7.1.0.alpha) activesupport (= 7.1.0.alpha) marcel (~> 1.0) - mini_mime (>= 1.1.0) activesupport (7.1.0.alpha) concurrent-ruby (~> 1.0, >= 1.0.2) connection_pool (>= 2.2.5) diff --git a/activestorage/CHANGELOG.md b/activestorage/CHANGELOG.md index e90e23066c3..79544063c33 100644 --- a/activestorage/CHANGELOG.md +++ b/activestorage/CHANGELOG.md @@ -1,3 +1,10 @@ +* Remove mini_mime usage in favour of marcel. + + We have two libraries that are have similar usage. This change removes + dependency on mini_mime and makes use of similar methods from marcel. + + *Vipul A M* + * Allow destroying active storage variants ```ruby diff --git a/activestorage/activestorage.gemspec b/activestorage/activestorage.gemspec index 79f6cc50f99..8adcc5a9314 100644 --- a/activestorage/activestorage.gemspec +++ b/activestorage/activestorage.gemspec @@ -38,5 +38,4 @@ Gem::Specification.new do |s| s.add_dependency "activerecord", version s.add_dependency "marcel", "~> 1.0" - s.add_dependency "mini_mime", ">= 1.1.0" end diff --git a/activestorage/app/models/active_storage/blob/representable.rb b/activestorage/app/models/active_storage/blob/representable.rb index a3a5af380ba..25485e0c7ce 100644 --- a/activestorage/app/models/active_storage/blob/representable.rb +++ b/activestorage/app/models/active_storage/blob/representable.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true -require "mini_mime" +require "marcel" module ActiveStorage::Blob::Representable extend ActiveSupport::Concern @@ -112,10 +112,10 @@ module ActiveStorage::Blob::Representable end def format - if filename.extension.present? && MiniMime.lookup_by_extension(filename.extension)&.content_type == content_type + if filename.extension.present? && Marcel::MimeType.for(extension: filename.extension) == content_type filename.extension else - MiniMime.lookup_by_content_type(content_type)&.extension + Marcel::Magic.new(content_type.to_s).extensions.first end end diff --git a/activestorage/app/models/active_storage/variation.rb b/activestorage/app/models/active_storage/variation.rb index 2925e9fbe67..d69467061ff 100644 --- a/activestorage/app/models/active_storage/variation.rb +++ b/activestorage/app/models/active_storage/variation.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true -require "mini_mime" +require "marcel" # A set of transformations that can be applied to a blob to create a variant. This class is exposed via # the ActiveStorage::Blob#variant method and should rarely be used directly. @@ -59,14 +59,14 @@ class ActiveStorage::Variation def format transformations.fetch(:format, :png).tap do |format| - if MiniMime.lookup_by_extension(format.to_s).nil? + if Marcel::Magic.by_extension(format.to_s).nil? raise ArgumentError, "Invalid variant format (#{format.inspect})" end end end def content_type - MiniMime.lookup_by_extension(format.to_s).content_type + Marcel::MimeType.for(extension: format.to_s) end # Returns a signed key for all the +transformations+ that this variation was instantiated with. diff --git a/activestorage/test/engine_test.rb b/activestorage/test/engine_test.rb index 32835dade57..8fb34191c40 100644 --- a/activestorage/test/engine_test.rb +++ b/activestorage/test/engine_test.rb @@ -4,31 +4,21 @@ require "test_helper" require "database/setup" class ActiveStorage::EngineTest < ActiveSupport::TestCase - test "all default content types are recognized by mini_mime" do - exceptions = ["image/bmp"] # see https://github.com/discourse/mini_mime/pull/45, once mini_mime is updated this can be removed - + test "all default content types are recognized by marcel" do ActiveStorage.variable_content_types.each do |content_type| - next if exceptions.include?(content_type) # remove this line when there is no exceptions - - assert_equal content_type, MiniMime.lookup_by_content_type(content_type)&.content_type + assert_equal content_type, Marcel::Magic.new(content_type).type end ActiveStorage.web_image_content_types.each do |content_type| - next if exceptions.include?(content_type) # remove this line when there is no exceptions - - assert_equal content_type, MiniMime.lookup_by_content_type(content_type)&.content_type + assert_equal content_type, Marcel::Magic.new(content_type).type end ActiveStorage.content_types_to_serve_as_binary.each do |content_type| - next if exceptions.include?(content_type) # remove this line when there is no exceptions - - assert_equal content_type, MiniMime.lookup_by_content_type(content_type)&.content_type + assert_equal content_type, Marcel::Magic.new(content_type).type end ActiveStorage.content_types_allowed_inline.each do |content_type| - next if exceptions.include?(content_type) # remove this line when there is no exceptions - - assert_equal content_type, MiniMime.lookup_by_content_type(content_type)&.content_type + assert_equal content_type, Marcel::Magic.new(content_type).type end end diff --git a/activestorage/test/models/variant_test.rb b/activestorage/test/models/variant_test.rb index 24461c99b2d..15beb3e6647 100644 --- a/activestorage/test/models/variant_test.rb +++ b/activestorage/test/models/variant_test.rb @@ -197,10 +197,10 @@ class ActiveStorage::VariantTest < ActiveSupport::TestCase end end - test "content_type not recognized by mini_mime isn't included as variable" do + test "content_type not recognized by marcel isn't included as variable" do blob = create_file_blob(filename: "racecar.jpg") - # image/jpg is not recognized by mini_mime (image/jpeg is correct) + # image/jpg is not recognized by marcel (image/jpeg is correct) blob.update(content_type: "image/jpg") assert_raises(ActiveStorage::InvariableError) do