From dbf16191031515b9c9f042508d269978974828df Mon Sep 17 00:00:00 2001 From: Mattia Roccoberton Date: Sun, 15 Mar 2026 12:10:56 +0100 Subject: [PATCH 1/6] chore: Remove unused body keyword The method was already using `request.body` directly; the parameter was never read. --- app/controllers/active_storage_db/files_controller.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/controllers/active_storage_db/files_controller.rb b/app/controllers/active_storage_db/files_controller.rb index bd6ae7c..a2635d9 100644 --- a/app/controllers/active_storage_db/files_controller.rb +++ b/app/controllers/active_storage_db/files_controller.rb @@ -16,7 +16,7 @@ def show def update if (token = decode_verified_token) - file_uploaded = upload_file(token, body: request.body) + file_uploaded = upload_file(token) head(file_uploaded ? :no_content : unprocessable) else head(:not_found) @@ -54,7 +54,7 @@ def serve_file(key, content_type:, disposition:) send_data(db_service.download(key), options) end - def upload_file(token, body:) # rubocop:disable Naming/PredicateMethod + def upload_file(token) # rubocop:disable Naming/PredicateMethod return false unless acceptable_content?(token) db_service.upload(token[:key], request.body, checksum: token[:checksum]) From 06d946fe43195ac10fb6db5589f9dab35b9cb35a Mon Sep 17 00:00:00 2001 From: Mattia Roccoberton Date: Sun, 15 Mar 2026 12:15:41 +0100 Subject: [PATCH 2/6] fix: Update adapter methods memoization Change `adapter_sqlite?` and `adapter_sqlserver?` from `||=` memoization to `defined?`-based memoization. The `||=` pattern doesn't memoize false, causing `active_storage_db_adapter_name` (which acquires a DB connection) to be re-evaluated on every call for non-SQLite/non-SQLServer adapters. --- lib/active_storage/service/db_service.rb | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/lib/active_storage/service/db_service.rb b/lib/active_storage/service/db_service.rb index 3628532..f977c28 100644 --- a/lib/active_storage/service/db_service.rb +++ b/lib/active_storage/service/db_service.rb @@ -109,11 +109,15 @@ def headers_for_direct_upload(_key, content_type:, **) private def adapter_sqlite? - @adapter_sqlite ||= active_storage_db_adapter_name == "SQLite" + return @adapter_sqlite if defined?(@adapter_sqlite) + + @adapter_sqlite = active_storage_db_adapter_name == "SQLite" end def adapter_sqlserver? - @adapter_sqlserver ||= active_storage_db_adapter_name == "SQLServer" + return @adapter_sqlserver if defined?(@adapter_sqlserver) + + @adapter_sqlserver = active_storage_db_adapter_name == "SQLServer" end def active_storage_db_adapter_name From 5eafaaba69c3c5a82a45402547dc92ac1393b39a Mon Sep 17 00:00:00 2001 From: Mattia Roccoberton Date: Sun, 15 Mar 2026 12:16:44 +0100 Subject: [PATCH 3/6] chore: service_name_for_token abstraction Extract the duplicated `respond_to?(:name) ? name : "db"`` expression into a `service_name_for_token`` private method, used in both `url_for_direct_upload`` and `generate_url`. --- lib/active_storage/service/db_service.rb | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/lib/active_storage/service/db_service.rb b/lib/active_storage/service/db_service.rb index f977c28..8e89c23 100644 --- a/lib/active_storage/service/db_service.rb +++ b/lib/active_storage/service/db_service.rb @@ -90,7 +90,7 @@ def url_for_direct_upload(key, expires_in:, content_type:, content_length:, chec content_type: content_type, content_length: content_length, checksum: checksum, - service_name: respond_to?(:name) ? name : "db" + service_name: service_name_for_token }, expires_in: expires_in, purpose: :blob_token @@ -108,6 +108,10 @@ def headers_for_direct_upload(_key, content_type:, **) private + def service_name_for_token + respond_to?(:name) ? name : "db" + end + def adapter_sqlite? return @adapter_sqlite if defined?(@adapter_sqlite) @@ -135,7 +139,7 @@ def generate_url(key, expires_in:, filename:, content_type:, disposition:) key: key, disposition: content_disposition, content_type: content_type, - service_name: respond_to?(:name) ? name : "db" + service_name: service_name_for_token }, expires_in: expires_in, purpose: :blob_key From dac3975b91f70f4355c1de399e12029fd5bbae8f Mon Sep 17 00:00:00 2001 From: Mattia Roccoberton Date: Sun, 15 Mar 2026 12:19:40 +0100 Subject: [PATCH 4/6] fix: Use sanitize_sql_like in search task Wrapped the search filename in `ActiveRecord::Base.sanitize_sql_like()` so that `%` and `_` characters in user input are treated as literals rather than SQL LIKE wildcards. --- lib/tasks/active_storage_db_tasks.rake | 30 +++++++++++++++----------- 1 file changed, 18 insertions(+), 12 deletions(-) diff --git a/lib/tasks/active_storage_db_tasks.rake b/lib/tasks/active_storage_db_tasks.rake index 30b10d3..b827af9 100644 --- a/lib/tasks/active_storage_db_tasks.rake +++ b/lib/tasks/active_storage_db_tasks.rake @@ -5,22 +5,25 @@ module ActiveStorage module_function def print_blob_header(digits: 0) - puts ['Size'.rjust(8), 'Date'.rjust(18), 'Id'.rjust(digits + 2), ' Filename'].join + puts ["Size".rjust(8), "Date".rjust(18), "Id".rjust(digits + 2), " Filename"].join end def print_blob(blob, digits: 0) size = (blob.byte_size / 1024).to_s.rjust(7) - date = blob.created_at.strftime('%Y-%m-%d %H:%M') + date = blob.created_at.strftime("%Y-%m-%d %H:%M") puts "#{size}K #{date} #{blob.id.to_s.rjust(digits)} #{blob.filename}" end end end namespace :asdb do - desc 'ActiveStorageDB: list attachments ordered by blob id desc' + desc "ActiveStorageDB: list attachments ordered by blob id desc" task list: [:environment] do |_t, _args| query = ActiveStorage::Blob.order(id: :desc).limit(100) - digits = query.ids.inject(0) { |ret, id| size = id.to_s.size; [size, ret].max } + digits = query.ids.inject(0) { |ret, id| + size = id.to_s.size + [size, ret].max + } ActiveStorage::Tasks.print_blob_header(digits: digits) query.each do |blob| @@ -28,14 +31,14 @@ namespace :asdb do end end - desc 'ActiveStorageDB: download attachment by blob id' + desc "ActiveStorageDB: download attachment by blob id" task :download, [:blob_id, :destination] => [:environment] do |_t, args| blob_id = args[:blob_id]&.strip destination = args[:destination]&.strip || Dir.pwd - abort('Required arguments: source blob id, destination path') if blob_id.blank? || destination.blank? + abort("Required arguments: source blob id, destination path") if blob_id.blank? || destination.blank? blob = ActiveStorage::Blob.find_by(id: blob_id) - abort('Source file not found') unless blob + abort("Source file not found") unless blob destination = "#{destination}/#{blob.filename}" if Dir.exist?(destination) dir = File.dirname(destination) @@ -45,20 +48,23 @@ namespace :asdb do puts "#{ret} bytes written - #{destination}" end - desc 'ActiveStorageDB: search attachment by filename (or part of it)' + desc "ActiveStorageDB: search attachment by filename (or part of it)" task :search, [:filename] => [:environment] do |_t, args| filename = args[:filename]&.strip - abort('Required arguments: filename') if filename.blank? + abort("Required arguments: filename") if filename.blank? - blobs = ActiveStorage::Blob.where('filename LIKE ?', "%#{filename}%").order(id: :desc) + blobs = ActiveStorage::Blob.where("filename LIKE ?", "%#{ActiveRecord::Base.sanitize_sql_like(filename)}%").order(id: :desc) if blobs.any? - digits = blobs.ids.inject(0) { |ret, id| size = id.to_s.size; [size, ret].max } + digits = blobs.ids.inject(0) { |ret, id| + size = id.to_s.size + [size, ret].max + } ActiveStorage::Tasks.print_blob_header(digits: digits) blobs.each do |blob| ActiveStorage::Tasks.print_blob(blob, digits: digits) end else - puts 'No results' + puts "No results" end end end From 40680973922fd587628f3ce80d697cab1d7ee05e Mon Sep 17 00:00:00 2001 From: Mattia Roccoberton Date: Sun, 15 Mar 2026 12:23:16 +0100 Subject: [PATCH 5/6] chore: Raise explicitly FileNotFoundError Change `compose` to use `find_by` + explicit `raise ActiveStorage::FileNotFoundError` instead of `find_by!` (which raises `ActiveRecord::RecordNotFound`). This makes the error type consistent with every other method in the service. --- lib/active_storage/service/db_service_rails70.rb | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/lib/active_storage/service/db_service_rails70.rb b/lib/active_storage/service/db_service_rails70.rb index 18e15a7..f4943cc 100644 --- a/lib/active_storage/service/db_service_rails70.rb +++ b/lib/active_storage/service/db_service_rails70.rb @@ -6,11 +6,13 @@ def compose(source_keys, destination_key, **) buffer = nil comment = "DBService#compose" source_keys.each do |source_key| - data = ::ActiveStorageDB::File.annotate(comment).find_by!(ref: source_key).data + record = ::ActiveStorageDB::File.annotate(comment).find_by(ref: source_key) + raise ActiveStorage::FileNotFoundError unless record + if buffer - buffer << data + buffer << record.data else - buffer = +data + buffer = +record.data end end ::ActiveStorageDB::File.create!(ref: destination_key, data: buffer) if buffer From d05986e644d27b8c8e4620b9cbb6b67ba8a27c9e Mon Sep 17 00:00:00 2001 From: Mattia Roccoberton Date: Sun, 15 Mar 2026 12:25:15 +0100 Subject: [PATCH 6/6] test: Fix attachments spec Added missing `else example.run`` to the `around` block. Without this, tests in the "with a new target entity" context were silently skipped on Rails < 7.1 (where `touch_attachment_records` doesn't exist). --- spec/integration/attachments_spec.rb | 2 ++ 1 file changed, 2 insertions(+) diff --git a/spec/integration/attachments_spec.rb b/spec/integration/attachments_spec.rb index d292791..692767b 100644 --- a/spec/integration/attachments_spec.rb +++ b/spec/integration/attachments_spec.rb @@ -19,6 +19,8 @@ ActiveStorage.touch_attachment_records = false example.run ActiveStorage.touch_attachment_records = touch_option + else + example.run end end