Merge pull request #13789 from prikeshsavla/13537-upgrade-active-storage-validations-gem

Upgraded gem active_storage_validations to 3.0.2 and fixed any upgrade related issues

Gemfile.lock

@@ -104,11 +104,12 @@ GEM
       rails-html-sanitizer (~> 1.6)
     active_model_serializers (0.8.4)
       activemodel (>= 3.0)
-    active_storage_validations (1.1.4)
-      activejob (>= 5.2.0)
-      activemodel (>= 5.2.0)
-      activestorage (>= 5.2.0)
-      activesupport (>= 5.2.0)
+    active_storage_validations (3.0.2)
+      activejob (>= 6.1.4)
+      activemodel (>= 6.1.4)
+      activestorage (>= 6.1.4)
+      activesupport (>= 6.1.4)
+      marcel (>= 1.0.3)
     activejob (7.1.6)
       activesupport (= 7.1.6)
       globalid (>= 0.3.6)

app/models/enterprise.rb

@@ -111,14 +111,14 @@ class Enterprise < ApplicationRecord
   end
 
   validates :logo,
-            processable_image: true,
-            content_type: %r{\Aimage/(png|jpeg|gif|jpg|svg\+xml|webp)\Z}
+            processable_file: true,
+            content_type: ::Spree::Image::ACCEPTED_CONTENT_TYPES
   validates :promo_image,
-            processable_image: true,
-            content_type: %r{\Aimage/(png|jpeg|gif|jpg|svg\+xml|webp)\Z}
+            processable_file: true,
+            content_type: ::Spree::Image::ACCEPTED_CONTENT_TYPES
   validates :white_label_logo,
-            processable_image: true,
-            content_type: %r{\Aimage/(png|jpeg|gif|jpg|svg\+xml|webp)\Z}
+            processable_file: true,
+            content_type: ::Spree::Image::ACCEPTED_CONTENT_TYPES
   validates :terms_and_conditions, content_type: {
     in: "application/pdf",
     message: I18n.t(:enterprise_terms_and_conditions_type_error),

app/models/enterprise_group.rb

@@ -29,11 +29,11 @@ class EnterpriseGroup < ApplicationRecord
   has_one_attached :promo_image, service: image_service
 
   validates :logo,
-            processable_image: true,
-            content_type: %r{\Aimage/(png|jpeg|gif|jpg|svg\+xml|webp)\Z}
+            processable_file: true,
+            content_type: ::Spree::Image::ACCEPTED_CONTENT_TYPES
   validates :promo_image,
-            processable_image: true,
-            content_type: %r{\Aimage/(png|jpeg|gif|jpg|svg\+xml|webp)\Z}
+            processable_file: true,
+            content_type: ::Spree::Image::ACCEPTED_CONTENT_TYPES
 
   scope :by_position, -> { order('position ASC') }
   scope :on_front_page, -> { where(on_front_page: true) }

app/models/spree/image.rb

@@ -2,6 +2,8 @@
 
 module Spree
   class Image < Asset
+    ACCEPTED_CONTENT_TYPES = %r{\Aimage/(png|jpeg|gif|jpg|svg\+xml|webp)\Z}
+
     has_one_attached :attachment, service: image_service do |attachment|
       attachment.variant :mini, resize_to_fill: [48, 48]
       attachment.variant :small, resize_to_fill: [227, 227]
@@ -11,8 +13,8 @@ module Spree
 
     validates :attachment,
               attached: true,
-              processable_image: true,
-              content_type: %r{\Aimage/(png|jpeg|gif|jpg|svg\+xml|webp)\Z}
+              processable_file: true,
+              content_type: ACCEPTED_CONTENT_TYPES
     validate :no_attachment_errors
 
     def self.default_image_url(size)

app/services/image_importer.rb

@@ -11,7 +11,9 @@ class ImageImporter
 
     image = Spree::Image.create do |img|
       PrivateAddressCheck.only_public_connections do
-        img.attachment.attach(io: valid_url.open, filename:, metadata:)
+        io = valid_url.open
+        content_type = Marcel::MimeType.for(io)
+        img.attachment.attach(io:, filename:, metadata:, content_type:)
       end
     end
     product.image = image if image

config/locales/en.yml

@@ -138,26 +138,66 @@ en:
   # Used by active_storage_validations
   errors:
     messages:
-      content_type_invalid: "has an invalid content type"
-      file_size_out_of_range: "size %{file_size} is not between required range"
-      limit_out_of_range: "total number is out of range"
-      image_metadata_missing: "is not a valid image"
-      dimension_min_inclusion: "must be greater than or equal to %{width} x %{height} pixel."
-      dimension_max_inclusion: "must be less than or equal to %{width} x %{height} pixel."
-      dimension_width_inclusion: "width is not included between %{min} and %{max} pixel."
-      dimension_height_inclusion: "height is not included between %{min} and %{max} pixel."
-      dimension_width_greater_than_or_equal_to: "width must be greater than or equal to %{length} pixel."
-      dimension_height_greater_than_or_equal_to: "height must be greater than or equal to %{length} pixel."
-      dimension_width_less_than_or_equal_to: "width must be less than or equal to %{length} pixel."
-      dimension_height_less_than_or_equal_to: "height must be less than or equal to %{length} pixel."
-      dimension_width_equal_to: "width must be equal to %{length} pixel."
-      dimension_height_equal_to: "height must be equal to %{length} pixel."
-      aspect_ratio_not_square: "must be a square image"
-      aspect_ratio_not_portrait: "must be a portrait image"
-      aspect_ratio_not_landscape: "must be a landscape image"
-      aspect_ratio_is_not: "must have an aspect ratio of %{aspect_ratio}"
-      aspect_ratio_unknown: "has an unknown aspect ratio"
-      image_not_processable: "is not a valid image"
+      content_type_invalid:
+        one: "has an invalid content type (authorized content type is %{authorized_human_content_types})"
+        other: "has an invalid content type (authorized content types are %{authorized_human_content_types})"
+      content_type_spoofed:
+        one: "has a content type that is not equivalent to the one that is detected through its content (authorized content type is %{authorized_human_content_types})"
+        other: "has a content type that is not equivalent to the one that is detected through its content (authorized content types are %{authorized_human_content_types})"
+      file_size_not_less_than: "file size must be less than %{max} (current size is %{file_size})"
+      file_size_not_less_than_or_equal_to: "file size must be less than or equal to %{max} (current size is %{file_size})"
+      file_size_not_greater_than: "file size must be greater than %{min} (current size is %{file_size})"
+      file_size_not_greater_than_or_equal_to: "file size must be greater than or equal to %{min} (current size is %{file_size})"
+      file_size_not_between: "file size must be between %{min} and %{max} (current size is %{file_size})"
+      file_size_not_equal_to: "file size must be equal to %{exact} (current size is %{file_size})"
+      total_file_size_not_less_than: "total file size must be less than %{max} (current size is %{total_file_size})"
+      total_file_size_not_less_than_or_equal_to: "total file size must be less than or equal to %{max} (current size is %{total_file_size})"
+      total_file_size_not_greater_than: "total file size must be greater than %{min} (current size is %{total_file_size})"
+      total_file_size_not_greater_than_or_equal_to: "total file size must be greater than or equal to %{min} (current size is %{total_file_size})"
+      total_file_size_not_between: "total file size must be between %{min} and %{max} (current size is %{total_file_size})"
+      total_file_size_not_equal_to: "total file size must be equal to %{exact} (current size is %{total_file_size})"
+      duration_not_less_than: "duration must be less than %{max} (current duration is %{duration})"
+      duration_not_less_than_or_equal_to: "duration must be less than or equal to %{max} (current duration is %{duration})"
+      duration_not_greater_than: "duration must be greater than %{min} (current duration is %{duration})"
+      duration_not_greater_than_or_equal_to: "duration must be greater than or equal to %{min} (current duration is %{duration})"
+      duration_not_between: "duration must be between %{min} and %{max} (current duration is %{duration})"
+      duration_not_equal_to: "duration must be equal to %{exact} (current duration is %{duration})"
+      limit_out_of_range:
+        zero: "no files attached (must have between %{min} and %{max} files)"
+        one: "only 1 file attached (must have between %{min} and %{max} files)"
+        other: "total number of files must be between %{min} and %{max} files (there are %{count} files attached)"
+      limit_min_not_reached:
+        zero: "no files attached (must have at least %{min} files)"
+        one: "only 1 file attached (must have at least %{min} files)"
+        other: "%{count} files attached (must have at least %{min} files)"
+      limit_max_exceeded:
+        zero: "no files attached (maximum is %{max} files)"
+        one: "too many files attached (maximum is %{max} files, got %{count})"
+        other: "too many files attached (maximum is %{max} files, got %{count})"
+      attachment_missing: "is missing its attachment"
+      media_metadata_missing: "is not a valid media file"
+      dimension_min_not_included_in: "must be greater than or equal to %{width} x %{height} pixels"
+      dimension_max_not_included_in: "must be less than or equal to %{width} x %{height} pixels"
+      dimension_width_not_included_in: "width is not included between %{min} and %{max} pixels"
+      dimension_height_not_included_in: "height is not included between %{min} and %{max} pixels"
+      dimension_width_not_greater_than_or_equal_to: "width must be greater than or equal to %{length} pixels"
+      dimension_height_not_greater_than_or_equal_to: "height must be greater than or equal to %{length} pixels"
+      dimension_width_not_less_than_or_equal_to: "width must be less than or equal to %{length} pixels"
+      dimension_height_not_less_than_or_equal_to: "height must be less than or equal to %{length} pixels"
+      dimension_width_not_equal_to: "width must be equal to %{length} pixels"
+      dimension_height_not_equal_to: "height must be equal to %{length} pixels"
+      aspect_ratio_not_square: "must be square (current file is %{width}x%{height}px)"
+      aspect_ratio_not_portrait: "must be portrait (current file is %{width}x%{height}px)"
+      aspect_ratio_not_landscape: "must be landscape (current file is %{width}x%{height}px)"
+      aspect_ratio_not_x_y: "must be %{authorized_aspect_ratios} (current file is %{width}x%{height}px)"
+      aspect_ratio_invalid: "has an invalid aspect ratio (valid aspect ratios are %{authorized_aspect_ratios})"
+      file_not_processable: "is not identified as a valid media file"
+      pages_not_less_than: "page count must be less than %{max} (current page count is %{pages})"
+      pages_not_less_than_or_equal_to: "page count must be less than or equal to %{max} (current page count is %{pages})"
+      pages_not_greater_than: "page count must be greater than %{min} (current page count is %{pages})"
+      pages_not_greater_than_or_equal_to: "page count must be greater than or equal to %{min} (current page count is %{pages})"
+      pages_not_between: "page count must be between %{min} and %{max} (current page count is %{pages})"
+      pages_not_equal_to: "page count must be equal to %{exact} (current page count is %{pages})"
     not_found:
       title: "The page you were looking for doesn't exist (404)"
       message_html: "<b>Please try again</b>

engines/dfc_provider/app/services/image_builder.rb

@@ -19,12 +19,14 @@ class ImageBuilder < DfcBuilder
 
   def self.import(image_link)
     url = URI.parse(image_link)
-    filename = File.basename(image_link)
+    filename = File.basename(url.path)
     metadata = { custom: { origin: image_link } }
 
     Spree::Image.new.tap do |image|
       PrivateAddressCheck.only_public_connections do
-        image.attachment.attach(io: url.open, filename:, metadata:)
+        io = url.open
+        content_type = Marcel::MimeType.for(io)
+        image.attachment.attach(io:, filename:, metadata:, content_type:)
       end
     end
   rescue StandardError

engines/dfc_provider/spec/services/supplied_product_importer_spec.rb

@@ -251,7 +251,7 @@ RSpec.describe SuppliedProductImporter do
         supplied_product.isVariantOf << tomatoes
 
         imported_product = importer.import_variant(supplied_product, supplier).product
-        expect(imported_product.image.attachment.filename).to eq "tomato.png?v=1"
+        expect(imported_product.image.attachment.filename).to eq "tomato.png"
 
         expect {
           importer.import_variant(supplied_product, supplier).product
@@ -266,7 +266,7 @@ RSpec.describe SuppliedProductImporter do
         }
           .to change { imported_product.image }
 
-        expect(imported_product.image.attachment.filename).to eq "tomato.png?v=2"
+        expect(imported_product.image.attachment.filename).to eq "tomato.png"
       end
 
       context "when spree_product_uri doesn't match the server host" do

spec/controllers/api/v0/product_images_controller_spec.rb

@@ -45,7 +45,8 @@ RSpec.describe Api::V0::ProductImagesController do
       expect(response).to have_http_status :unprocessable_entity
       expect(product_without_image.image).to be_nil
       expect(json_response["id"]).to eq nil
-      expect(json_response["errors"]).to include "Attachment has an invalid content type"
+      expect(json_response["errors"]).to include "Attachment is " \
+                                                 "not identified as a valid media file"
     end
   end
 end

spec/models/enterprise_spec.rb

@@ -396,24 +396,8 @@ RSpec.describe Enterprise do
       let(:content_type) { 'image/png' }
 
       before do
-        blob = instance_double(
-          "ActiveStorage::Blob",
-          filename: ActiveStorage::Filename.new('white-label-logo.png'),
-          content_type:,
-          byte_size: 1024
-        )
-
-        # InstanceDouble is not working for attachment case as the blob method is not yet defined
-        # on instantiation.
-        attachment = double(
-          "ActiveStorage::Attached::One",
-          blank?: false,
-          attached?: true,
-          blob:
-        )
-
-        allow(enterprise)
-          .to receive(:white_label_logo).and_return(attachment)
+        blob = Rack::Test::UploadedFile.new('spec/fixtures/files/logo.png', content_type)
+        enterprise.white_label_logo.attach(blob)
       end
 
       context 'when the file attached is a PNG image' do
@@ -424,6 +408,12 @@ RSpec.describe Enterprise do
 
       context 'when the file attached is a BMP image' do
         let(:content_type) { 'image/bmp' }
+
+        before do
+          blob = Rack::Test::UploadedFile.new('spec/fixtures/files/logo.bmp', content_type)
+          enterprise.white_label_logo.attach(blob)
+        end
+
         it 'is not valid' do
           expect(enterprise).not_to be_valid
         end

spec/services/image_importer_spec.rb

@@ -7,7 +7,7 @@ RSpec.describe ImageImporter do
   let(:product) { create(:product) }
 
   describe "#import" do
-    it "downloads from the Internet", :vcr do
+    it "downloads from the Internet", :vcr, :aggregate_failures do
       expect {
         subject.import(ofn_url, product)
       }.to change {

spec/system/admin/products_spec.rb

@@ -604,7 +604,7 @@ RSpec.describe '
         click_button "Create"
 
         expect(page).to have_text "Attachment has an invalid content type"
-        expect(page).to have_text "Attachment is not a valid image"
+        expect(page).to have_text "Attachment is not identified as a valid media file"
       end
 
       it "deleting product images" do

spec/system/admin/products_v3/update_spec.rb

@@ -770,12 +770,12 @@ RSpec.describe 'As an enterprise user, I can update my products' do
         end
       end
 
-      it 'shows a modal telling not a valid image when uploading a non valid image file' do
+      it 'shows a modal telling not a valid image when uploading an invalid image file' do
         within ".reveal-modal" do
           attach_file 'image[attachment]',
                       Rails.public_path.join('invalid_image.jpg'),
                       visible: false
-          expect(page).to have_content /Attachment is not a valid image/
+          expect(page).to have_content /Attachment is not identified as a valid media file/
         end
       end
     end