Remove deprecated invalid default content types in Active Storage con…

…figurations
rails · Mar 3, 2023 · 4edaa41 · 4edaa41
1 parent 8241178
commit 4edaa41
Show file tree

Hide file tree

Showing 9 changed files with 25 additions and 87 deletions.
diff --git a/activestorage/CHANGELOG.md b/activestorage/CHANGELOG.md
@@ -1,3 +1,7 @@
+*   Remove deprecated invalid default content types in Active Storage configurations.
+
+    *Rafael Mendonça França*
+
 *   Add missing preview event to `ActiveStorage::LogSubscriber`
 
     A `preview` event is being instrumented in `ActiveStorage::Previewer`.

diff --git a/activestorage/app/models/active_storage/blob.rb b/activestorage/app/models/active_storage/blob.rb
@@ -327,31 +327,6 @@ def service
     services.fetch(service_name)
   end
 
-  def content_type=(value)
-    unless ActiveStorage.silence_invalid_content_types_warning
-      if INVALID_VARIABLE_CONTENT_TYPES_DEPRECATED_IN_RAILS_7.include?(value)
-        ActiveStorage.deprecator.warn(<<-MSG.squish)
-          #{value} is not a valid content type, it should not be used when creating a blob, and support for it will be removed in Rails 7.1.
-          If you want to keep supporting this content type past Rails 7.1, add it to `config.active_storage.variable_content_types`.
-          Dismiss this warning by setting `config.active_storage.silence_invalid_content_types_warning = true`.
-        MSG
-      end
-
-      if INVALID_VARIABLE_CONTENT_TYPES_TO_SERVE_AS_BINARY_DEPRECATED_IN_RAILS_7.include?(value)
-        ActiveStorage.deprecator.warn(<<-MSG.squish)
-          #{value} is not a valid content type, it should not be used when creating a blob, and support for it will be removed in Rails 7.1.
-          If you want to keep supporting this content type past Rails 7.1, add it to `config.active_storage.content_types_to_serve_as_binary`.
-          Dismiss this warning by setting `config.active_storage.silence_invalid_content_types_warning = true`.
-        MSG
-      end
-    end
-
-    super
-  end
-
-  INVALID_VARIABLE_CONTENT_TYPES_DEPRECATED_IN_RAILS_7 = ["image/jpg", "image/pjpeg"]
-  INVALID_VARIABLE_CONTENT_TYPES_TO_SERVE_AS_BINARY_DEPRECATED_IN_RAILS_7 = ["text/javascript"]
-
   private
     def compute_checksum_in_chunks(io)
       raise ArgumentError, "io must be rewindable" unless io.respond_to?(:rewind)

diff --git a/activestorage/lib/active_storage.rb b/activestorage/lib/active_storage.rb
@@ -363,7 +363,13 @@ module ActiveStorage
 
   mattr_accessor :video_preview_arguments, default: "-y -vframes 1 -f image2"
 
-  mattr_accessor :silence_invalid_content_types_warning, default: false
+  def self.silence_invalid_content_types_warning
+    ActiveStorage.deprecator.warn("config.active_storage.silence_invalid_content_types_warning is deprecated and has no effect.")
+  end
+
+  def self.silence_invalid_content_types_warning=(value)
+    ActiveStorage.deprecator.warn("config.active_storage.silence_invalid_content_types_warning is deprecated and has no effect.")
+  end
 
   module Transformers
     extend ActiveSupport::Autoload

diff --git a/activestorage/lib/active_storage/engine.rb b/activestorage/lib/active_storage/engine.rb
@@ -35,9 +35,7 @@ class Engine < Rails::Engine # :nodoc:
     config.active_storage.variable_content_types = %w(
       image/png
       image/gif
-      image/jpg
       image/jpeg
-      image/pjpeg
       image/tiff
       image/bmp
       image/vnd.adobe.photoshop
@@ -51,13 +49,11 @@ class Engine < Rails::Engine # :nodoc:
     config.active_storage.web_image_content_types = %w(
       image/png
       image/jpeg
-      image/jpg
       image/gif
     )
 
     config.active_storage.content_types_to_serve_as_binary = %w(
       text/html
-      text/javascript
       image/svg+xml
       application/postscript
       application/x-shockwave-flash
@@ -71,7 +67,6 @@ class Engine < Rails::Engine # :nodoc:
     config.active_storage.content_types_allowed_inline = %w(
       image/png
       image/gif
-      image/jpg
       image/jpeg
       image/tiff
       image/bmp
@@ -121,7 +116,9 @@ class Engine < Rails::Engine # :nodoc:
         ActiveStorage.binary_content_type = app.config.active_storage.binary_content_type || "application/octet-stream"
         ActiveStorage.video_preview_arguments = app.config.active_storage.video_preview_arguments || "-y -vframes 1 -f image2"
 
-        ActiveStorage.silence_invalid_content_types_warning = app.config.active_storage.silence_invalid_content_types_warning || false
+        unless app.config.active_storage.silence_invalid_content_types_warning.nil?
+          ActiveStorage.silence_invalid_content_types_warning = app.config.active_storage.silence_invalid_content_types_warning
+        end
 
         ActiveStorage.replace_on_assign_to_many = app.config.active_storage.replace_on_assign_to_many || false
         ActiveStorage.track_variants = app.config.active_storage.track_variants || false

diff --git a/activestorage/test/engine_test.rb b/activestorage/test/engine_test.rb
@@ -5,30 +5,28 @@
 
 class ActiveStorage::EngineTest < ActiveSupport::TestCase
   test "all default content types are recognized by mini_mime" do
-    exceptions = ActiveStorage::Blob::INVALID_VARIABLE_CONTENT_TYPES_DEPRECATED_IN_RAILS_7 +
-                  ActiveStorage::Blob::INVALID_VARIABLE_CONTENT_TYPES_TO_SERVE_AS_BINARY_DEPRECATED_IN_RAILS_7 +
-                  ["image/bmp"] # see https://github.com/discourse/mini_mime/pull/45, once mini_mime is updated this can be removed
+    exceptions = ["image/bmp"] # see https://github.com/discourse/mini_mime/pull/45, once mini_mime is updated this can be removed
 
     ActiveStorage.variable_content_types.each do |content_type|
-      next if exceptions.include?(content_type) # remove this line in Rails 7.1
+      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
     end
 
     ActiveStorage.web_image_content_types.each do |content_type|
-      next if exceptions.include?(content_type) # remove this line in Rails 7.1
+      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
     end
 
     ActiveStorage.content_types_to_serve_as_binary.each do |content_type|
-      next if exceptions.include?(content_type) # remove this line in Rails 7.1
+      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
     end
 
     ActiveStorage.content_types_allowed_inline.each do |content_type|
-      next if exceptions.include?(content_type) # remove this line in Rails 7.1
+      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
     end

diff --git a/activestorage/test/models/blob_test.rb b/activestorage/test/models/blob_test.rb
@@ -354,40 +354,6 @@ class ActiveStorage::BlobTest < ActiveSupport::TestCase
     end
   end
 
-  test "warning if blob is created with invalid mime type" do
-    assert_deprecated(ActiveStorage.deprecator) do
-      create_blob(filename: "funky.jpg", content_type: "image/jpg")
-    end
-
-    assert_not_deprecated(ActiveStorage.deprecator) do
-      create_blob(filename: "funky.jpg", content_type: "image/jpeg")
-    end
-
-    assert_not_deprecated(ActiveStorage.deprecator) do
-      create_file_blob(filename: "colors.bmp", content_type: "image/bmp")
-    end
-  end
-
-  test "warning if blob is created with invalid mime type can be disabled" do
-    warning_was = ActiveStorage.silence_invalid_content_types_warning
-    ActiveStorage.silence_invalid_content_types_warning = true
-
-    assert_not_deprecated(ActiveStorage.deprecator) do
-      create_blob(filename: "funky.jpg", content_type: "image/jpg")
-    end
-
-    assert_not_deprecated(ActiveStorage.deprecator) do
-      create_blob(filename: "funky.jpg", content_type: "image/jpeg")
-    end
-
-    assert_not_deprecated(ActiveStorage.deprecator) do
-      create_file_blob(filename: "colors.bmp", content_type: "image/bmp")
-    end
-
-  ensure
-    ActiveStorage.silence_invalid_content_types_warning = warning_was
-  end
-
   private
     def expected_url_for(blob, disposition: :attachment, filename: nil, content_type: nil, service_name: :local)
       filename ||= blob.filename

diff --git a/activestorage/test/models/variant_test.rb b/activestorage/test/models/variant_test.rb
@@ -197,15 +197,13 @@ class ActiveStorage::VariantTest < ActiveSupport::TestCase
     end
   end
 
-  test "doesn't crash content_type not recognized by mini_mime" do
+  test "content_type not recognized by mini_mime ins't included as variable" do
     blob = create_file_blob(filename: "racecar.jpg")
 
-    # image/jpg is not recognised by mini_mime (image/jpeg is correct)
-    assert_deprecated(ActiveStorage.deprecator) do
-      blob.update(content_type: "image/jpg")
-    end
+    # image/jpg is not recognized by mini_mime (image/jpeg is correct)
+    blob.update(content_type: "image/jpg")
 
-    assert_nothing_raised do
+    assert_raises(ActiveStorage::InvariableError) do
       blob.variant(resize_to_limit: [100, 100])
     end
 

diff --git a/guides/source/7_1_release_notes.md b/guides/source/7_1_release_notes.md
@@ -120,6 +120,8 @@ Please refer to the [Changelog][active-storage] for detailed changes.
 
 ### Removals
 
+*   Remove deprecated invalid default content types in Active Storage configurations.
+
 ### Deprecations
 
 ### Notable changes

diff --git a/guides/source/configuring.md b/guides/source/configuring.md
@@ -2590,14 +2590,6 @@ By default, this is defined as:
 config.active_storage.content_types_allowed_inline` = %w(image/png image/gif image/jpeg image/tiff image/vnd.adobe.photoshop image/vnd.microsoft.icon application/pdf)
 ```
 
-#### `config.active_storage.silence_invalid_content_types_warning`
-
-Since Rails 7, Active Storage will warn if you use an invalid content type that was incorrectly supported in Rails 6. You can use this config to turn the warning off.
-
-```ruby
-config.active_storage.silence_invalid_content_types_warning = false
-```
-
 #### `config.active_storage.queues.analysis`
 
 Accepts a symbol indicating the Active Job queue to use for analysis jobs. When this option is `nil`, analysis jobs are sent to the default Active Job queue (see `config.active_job.default_queue_name`).