From 8d639a7342b85953bcf707c2db43d34d43a0870e Mon Sep 17 00:00:00 2001 From: Mathieu EUSTACHY Date: Wed, 1 Jan 2025 17:08:18 +0100 Subject: [PATCH] [Validator] Improve performance leveraging ActiveStorage::Blob#metadata metadata (#340) --- README.md | 1 + lib/active_storage_validations.rb | 5 +-- .../duration_validator.rb | 2 +- .../extensors/asv_blob_metadatable.rb | 18 ++++++++++ .../asv_marcelable.rb} | 0 lib/active_storage_validations/railtie.rb | 5 +++ .../shared/asv_analyzable.rb | 12 ++++++- .../shared/asv_attachable.rb | 2 +- .../validator/is_performance_optimized.rb | 17 ++++++++++ .../validator/is_performance_optimized.rb | 17 ++++++++++ .../validator/is_performance_optimized.rb | 17 ++++++++++ .../validator/is_performance_optimized.rb | 17 ++++++++++ test/dummy/db/schema.rb | 12 +++++++ test/extensors/asv_blob_metadatable_test.rb | 16 +++++++++ .../validators/aspect_ratio_validator_test.rb | 13 +++++++ test/validators/duration_validator_test.rb | 13 +++++++ .../processable_file_validator_test.rb | 13 +++++++ .../is_performance_optimized.rb | 34 +++++++++++++++++++ 18 files changed, 209 insertions(+), 5 deletions(-) create mode 100644 lib/active_storage_validations/extensors/asv_blob_metadatable.rb rename lib/active_storage_validations/{marcel_extensor.rb => extensors/asv_marcelable.rb} (100%) create mode 100644 test/dummy/app/models/aspect_ratio/validator/is_performance_optimized.rb create mode 100644 test/dummy/app/models/dimension/validator/is_performance_optimized.rb create mode 100644 test/dummy/app/models/duration/validator/is_performance_optimized.rb create mode 100644 test/dummy/app/models/processable_file/validator/is_performance_optimized.rb create mode 100644 test/extensors/asv_blob_metadatable_test.rb create mode 100644 test/validators/shared_examples/is_performance_optimized.rb diff --git a/README.md b/README.md index b78ce304..2e316435 100755 --- a/README.md +++ b/README.md @@ -598,6 +598,7 @@ Added features: - `dimension` validator now supports videos - `aspect_ratio` validator now supports videos - `processable_image` validator is now `processable_file` validator and supports image/video/audio +- Major performance improvment have been added: we now only perform the expensive io analysis operation on the newly attached files. For previously attached files, we validate them using Rails `ActiveStorage::Blob#metadata` internal mecanism ([more here](https://github.com/rails/rails/blob/main/activestorage/app/models/active_storage/blob/analyzable.rb)). - All error messages have been given an upgrade and new variables that you can use But this major version bump also comes with some breaking changes. Below are the main breaking changes you need to be aware of: diff --git a/lib/active_storage_validations.rb b/lib/active_storage_validations.rb index 9c44c217..dcbf1909 100644 --- a/lib/active_storage_validations.rb +++ b/lib/active_storage_validations.rb @@ -11,6 +11,9 @@ require 'active_storage_validations/analyzer/video_analyzer' require 'active_storage_validations/analyzer/audio_analyzer' +require 'active_storage_validations/extensors/asv_blob_metadatable' +require 'active_storage_validations/extensors/asv_marcelable' + require 'active_storage_validations/railtie' require 'active_storage_validations/engine' require 'active_storage_validations/attached_validator' @@ -23,8 +26,6 @@ require 'active_storage_validations/size_validator' require 'active_storage_validations/total_size_validator' -require 'active_storage_validations/marcel_extensor' - ActiveSupport.on_load(:active_record) do send :include, ActiveStorageValidations end diff --git a/lib/active_storage_validations/duration_validator.rb b/lib/active_storage_validations/duration_validator.rb index 91676726..762817c6 100644 --- a/lib/active_storage_validations/duration_validator.rb +++ b/lib/active_storage_validations/duration_validator.rb @@ -21,7 +21,7 @@ def validate_each(record, attribute, _value) flat_options = set_flat_options(record) attachables_and_blobs(record, attribute).each do |attachable, blob| - duration = metadata_for(attachable)[:duration] + duration = metadata_for(blob, attachable)&.fetch(:duration, nil) if duration.to_i <= 0 errors_options = initialize_error_options(options, attachable) diff --git a/lib/active_storage_validations/extensors/asv_blob_metadatable.rb b/lib/active_storage_validations/extensors/asv_blob_metadatable.rb new file mode 100644 index 00000000..8779895f --- /dev/null +++ b/lib/active_storage_validations/extensors/asv_blob_metadatable.rb @@ -0,0 +1,18 @@ +# frozen_string_literal: true + +module ActiveStorageValidations + module ASVBlobMetadatable + extend ActiveSupport::Concern + + included do + def active_storage_validations_metadata + metadata.dig('custom', 'active_storage_validations') + end + + def active_storage_validations_metadata=(value) + metadata['custom'] ||= {} + metadata['custom']['active_storage_validations'] = value + end + end + end +end diff --git a/lib/active_storage_validations/marcel_extensor.rb b/lib/active_storage_validations/extensors/asv_marcelable.rb similarity index 100% rename from lib/active_storage_validations/marcel_extensor.rb rename to lib/active_storage_validations/extensors/asv_marcelable.rb diff --git a/lib/active_storage_validations/railtie.rb b/lib/active_storage_validations/railtie.rb index 6d614ad7..266db300 100644 --- a/lib/active_storage_validations/railtie.rb +++ b/lib/active_storage_validations/railtie.rb @@ -2,5 +2,10 @@ module ActiveStorageValidations class Railtie < ::Rails::Railtie + initializer 'active_storage_validations.extend_active_storage_blob' do + Rails.application.config.to_prepare do + ActiveStorage::Blob.include(ActiveStorageValidations::ASVBlobMetadatable) + end + end end end diff --git a/lib/active_storage_validations/shared/asv_analyzable.rb b/lib/active_storage_validations/shared/asv_analyzable.rb index 5b871ccc..4feaabc1 100644 --- a/lib/active_storage_validations/shared/asv_analyzable.rb +++ b/lib/active_storage_validations/shared/asv_analyzable.rb @@ -12,7 +12,17 @@ module ASVAnalyzable private - def metadata_for(attachable) + # Retrieve the ASV metadata from the blob. + # If the blob has not been analyzed by our gem yet, the gem will analyze the + # attachable with the corresponding analyzer and set the metadata in the + # blob. + def metadata_for(blob, attachable) + return blob.active_storage_validations_metadata if blob.active_storage_validations_metadata.present? + + blob.active_storage_validations_metadata = generate_metadata_for(attachable) + end + + def generate_metadata_for(attachable) analyzer_for(attachable).metadata end diff --git a/lib/active_storage_validations/shared/asv_attachable.rb b/lib/active_storage_validations/shared/asv_attachable.rb index 70a68885..1481ce58 100644 --- a/lib/active_storage_validations/shared/asv_attachable.rb +++ b/lib/active_storage_validations/shared/asv_attachable.rb @@ -18,7 +18,7 @@ module ASVAttachable # to perform file analyses. def validate_changed_files_from_metadata(record, attribute) attachables_and_blobs(record, attribute).each do |attachable, blob| - is_valid?(record, attribute, attachable, metadata_for(attachable)) + is_valid?(record, attribute, attachable, metadata_for(blob, attachable)) end end diff --git a/test/dummy/app/models/aspect_ratio/validator/is_performance_optimized.rb b/test/dummy/app/models/aspect_ratio/validator/is_performance_optimized.rb new file mode 100644 index 00000000..7180f3a6 --- /dev/null +++ b/test/dummy/app/models/aspect_ratio/validator/is_performance_optimized.rb @@ -0,0 +1,17 @@ +# frozen_string_literal: true + +# == Schema Information +# +# Table name: aspect_ratio_validator_is_performance_optimizeds +# +# id :integer not null, primary key +# created_at :datetime not null +# updated_at :datetime not null +# + +class AspectRatio::Validator::IsPerformanceOptimized < ApplicationRecord + has_one_attached :is_performance_optimized + has_many_attached :is_performance_optimizeds + validates :is_performance_optimized, aspect_ratio: :square + validates :is_performance_optimizeds, aspect_ratio: :square +end diff --git a/test/dummy/app/models/dimension/validator/is_performance_optimized.rb b/test/dummy/app/models/dimension/validator/is_performance_optimized.rb new file mode 100644 index 00000000..11a2cc0d --- /dev/null +++ b/test/dummy/app/models/dimension/validator/is_performance_optimized.rb @@ -0,0 +1,17 @@ +# frozen_string_literal: true + +# == Schema Information +# +# Table name: dimension_validator_is_performance_optimizeds +# +# id :integer not null, primary key +# created_at :datetime not null +# updated_at :datetime not null +# + +class Dimension::Validator::IsPerformanceOptimized < ApplicationRecord + has_one_attached :is_performance_optimized + has_many_attached :is_performance_optimizeds + validates :is_performance_optimized, dimension: { width: 150, height: 150 } + validates :is_performance_optimizeds, dimension: { width: 150, height: 150 } +end diff --git a/test/dummy/app/models/duration/validator/is_performance_optimized.rb b/test/dummy/app/models/duration/validator/is_performance_optimized.rb new file mode 100644 index 00000000..faedd6cf --- /dev/null +++ b/test/dummy/app/models/duration/validator/is_performance_optimized.rb @@ -0,0 +1,17 @@ +# frozen_string_literal: true + +# == Schema Information +# +# Table name: duration_validator_is_performance_optimizeds +# +# id :integer not null, primary key +# created_at :datetime not null +# updated_at :datetime not null +# + +class Duration::Validator::IsPerformanceOptimized < ApplicationRecord + has_one_attached :is_performance_optimized + has_many_attached :is_performance_optimizeds + validates :is_performance_optimized, duration: { less_than: 2.seconds } + validates :is_performance_optimizeds, duration: { less_than: 2.seconds } +end diff --git a/test/dummy/app/models/processable_file/validator/is_performance_optimized.rb b/test/dummy/app/models/processable_file/validator/is_performance_optimized.rb new file mode 100644 index 00000000..fcdab67b --- /dev/null +++ b/test/dummy/app/models/processable_file/validator/is_performance_optimized.rb @@ -0,0 +1,17 @@ +# frozen_string_literal: true + +# == Schema Information +# +# Table name: processable_file_validator_is_performance_optimizeds +# +# id :integer not null, primary key +# created_at :datetime not null +# updated_at :datetime not null +# + +class ProcessableFile::Validator::IsPerformanceOptimized < ApplicationRecord + has_one_attached :is_performance_optimized + has_many_attached :is_performance_optimizeds + validates :is_performance_optimized, processable_file: true + validates :is_performance_optimizeds, processable_file: true +end diff --git a/test/dummy/db/schema.rb b/test/dummy/db/schema.rb index 794ba478..b128b467 100644 --- a/test/dummy/db/schema.rb +++ b/test/dummy/db/schema.rb @@ -95,6 +95,18 @@ end end + %i( + aspect_ratio + dimension + duration + processable_file + ).each do |validator| + create_table :"#{validator}_validator_is_performance_optimizeds", force: :cascade do |t| + t.datetime :created_at, null: false + t.datetime :updated_at, null: false + end + end + %w(proc_option invalid_named_argument invalid_is_xy_argument).each do |invalid_case| create_table :"aspect_ratio_validator_check_validity_#{invalid_case.pluralize}", force: :cascade do |t| t.datetime :created_at, null: false diff --git a/test/extensors/asv_blob_metadatable_test.rb b/test/extensors/asv_blob_metadatable_test.rb new file mode 100644 index 00000000..9a3e0a49 --- /dev/null +++ b/test/extensors/asv_blob_metadatable_test.rb @@ -0,0 +1,16 @@ +# frozen_string_literal: true + +require 'test_helper' + +describe ActiveStorageValidations::ASVBlobMetadatable do + let(:blob) { ActiveStorage::Blob.new } + + it "adds our gem's getter method to ActiveStorage::Blob custom metadata" do + assert blob.respond_to?(:active_storage_validations_metadata) + end + + it "adds our gem's setter method to ActiveStorage::Blob custom metadata" do + blob.active_storage_validations_metadata = { 'duration' => '1.0' } + assert blob.active_storage_validations_metadata == { 'duration' => '1.0' } + end +end diff --git a/test/validators/aspect_ratio_validator_test.rb b/test/validators/aspect_ratio_validator_test.rb index cc02dfe2..82b75a6e 100644 --- a/test/validators/aspect_ratio_validator_test.rb +++ b/test/validators/aspect_ratio_validator_test.rb @@ -2,6 +2,7 @@ require 'test_helper' require 'validators/shared_examples/checks_validator_validity' +require 'validators/shared_examples/is_performance_optimized' require 'validators/shared_examples/works_fine_with_attachables' require 'validators/shared_examples/works_with_all_rails_common_validation_options' @@ -242,6 +243,18 @@ end end + describe 'Blob Metadata' do + let(:attachable) do + { + io: File.open(Rails.root.join('public', 'image_150x150.png')), + filename: 'image_150x150.png', + content_type: 'image/png' + } + end + + include IsPerformanceOptimized + end + describe 'Rails options' do include WorksWithAllRailsCommonValidationOptions end diff --git a/test/validators/duration_validator_test.rb b/test/validators/duration_validator_test.rb index 72786a8b..45774fd2 100644 --- a/test/validators/duration_validator_test.rb +++ b/test/validators/duration_validator_test.rb @@ -2,6 +2,7 @@ require 'test_helper' require 'validators/shared_examples/checks_validator_validity' +require 'validators/shared_examples/is_performance_optimized' require 'validators/shared_examples/works_with_all_rails_common_validation_options' describe ActiveStorageValidations::DurationValidator do @@ -248,6 +249,18 @@ end end + describe 'Blob Metadata' do + let(:attachable) do + { + io: File.open(Rails.root.join('public', 'audio.mp3')), + filename: 'audio.mp3', + content_type: 'audio/mp3' + } + end + + include IsPerformanceOptimized + end + describe 'Rails options' do include WorksWithAllRailsCommonValidationOptions end diff --git a/test/validators/processable_file_validator_test.rb b/test/validators/processable_file_validator_test.rb index 63f27c52..99452846 100644 --- a/test/validators/processable_file_validator_test.rb +++ b/test/validators/processable_file_validator_test.rb @@ -1,6 +1,7 @@ # frozen_string_literal: true require 'test_helper' +require 'validators/shared_examples/is_performance_optimized' require 'validators/shared_examples/works_fine_with_attachables' require 'validators/shared_examples/works_with_all_rails_common_validation_options' @@ -55,6 +56,18 @@ end end + describe 'Blob Metadata' do + let(:attachable) do + { + io: File.open(Rails.root.join('public', 'audio.mp3')), + filename: 'audio.mp3', + content_type: 'audio/mp3' + } + end + + include IsPerformanceOptimized + end + describe 'Rails options' do include WorksWithAllRailsCommonValidationOptions end diff --git a/test/validators/shared_examples/is_performance_optimized.rb b/test/validators/shared_examples/is_performance_optimized.rb new file mode 100644 index 00000000..0a07c636 --- /dev/null +++ b/test/validators/shared_examples/is_performance_optimized.rb @@ -0,0 +1,34 @@ +# frozen_string_literal: true + +module IsPerformanceOptimized + extend ActiveSupport::Concern + + included do + subject { validator_test_class::IsPerformanceOptimized.new(params) } + + let(:validator_class) { "ActiveStorageValidations::#{validator_test_class.name.delete('::')}".constantize } + + describe "when the attachable blob has not been analyzed by our gem yet" do + before { subject.is_performance_optimized.attach(attachable) } + + it "calls the corresponding media analyzer (expensive operation) once" do + assert_called_on_instance_of(validator_class, :generate_metadata_for, times: 1, returns: {}) do + subject.valid? + end + end + end + + describe "when an attachable blob has already been analyzed by our gem" do + before do + subject.is_performance_optimizeds.attach(attachable) + subject.save! + end + + it "only calls the corresponding media analyzer (expensive operation) on the new attachable" do + assert_called_on_instance_of(validator_class, :generate_metadata_for, times: 1, returns: {}) do + subject.is_performance_optimizeds.attach(attachable) + end + end + end + end +end