From 25a6aa3d713281aa024521f31ea79c4ab53db79d Mon Sep 17 00:00:00 2001 From: Mattia Roccoberton Date: Wed, 1 Nov 2023 09:40:33 +0100 Subject: [PATCH 1/3] feat: apply RuboCop auto correct --- lib/tasks/active_storage_db_tasks.rake | 14 +++++++------- spec/rails_helper.rb | 4 ++-- spec/requests/file_controller_spec.rb | 4 ++-- spec/requests/file_url_spec.rb | 2 +- .../active_storage/service/db_service_spec.rb | 8 ++++---- 5 files changed, 16 insertions(+), 16 deletions(-) diff --git a/lib/tasks/active_storage_db_tasks.rake b/lib/tasks/active_storage_db_tasks.rake index 36f23c3..30b10d3 100644 --- a/lib/tasks/active_storage_db_tasks.rake +++ b/lib/tasks/active_storage_db_tasks.rake @@ -19,12 +19,12 @@ end namespace :asdb do desc 'ActiveStorageDB: list attachments ordered by blob id desc' task list: [:environment] do |_t, _args| - query = ::ActiveStorage::Blob.order(id: :desc).limit(100) + query = ActiveStorage::Blob.order(id: :desc).limit(100) digits = query.ids.inject(0) { |ret, id| size = id.to_s.size; [size, ret].max } - ::ActiveStorage::Tasks.print_blob_header(digits: digits) + ActiveStorage::Tasks.print_blob_header(digits: digits) query.each do |blob| - ::ActiveStorage::Tasks.print_blob(blob, digits: digits) + ActiveStorage::Tasks.print_blob(blob, digits: digits) end end @@ -34,7 +34,7 @@ namespace :asdb do destination = args[:destination]&.strip || Dir.pwd abort('Required arguments: source blob id, destination path') if blob_id.blank? || destination.blank? - blob = ::ActiveStorage::Blob.find_by(id: blob_id) + blob = ActiveStorage::Blob.find_by(id: blob_id) abort('Source file not found') unless blob destination = "#{destination}/#{blob.filename}" if Dir.exist?(destination) @@ -50,12 +50,12 @@ namespace :asdb do filename = args[:filename]&.strip abort('Required arguments: filename') if filename.blank? - blobs = ::ActiveStorage::Blob.where('filename LIKE ?', "%#{filename}%").order(id: :desc) + blobs = ActiveStorage::Blob.where('filename LIKE ?', "%#{filename}%").order(id: :desc) if blobs.any? digits = blobs.ids.inject(0) { |ret, id| size = id.to_s.size; [size, ret].max } - ::ActiveStorage::Tasks.print_blob_header(digits: digits) + ActiveStorage::Tasks.print_blob_header(digits: digits) blobs.each do |blob| - ::ActiveStorage::Tasks.print_blob(blob, digits: digits) + ActiveStorage::Tasks.print_blob(blob, digits: digits) end else puts 'No results' diff --git a/spec/rails_helper.rb b/spec/rails_helper.rb index 90d62c9..94c6f74 100644 --- a/spec/rails_helper.rb +++ b/spec/rails_helper.rb @@ -20,7 +20,7 @@ add_filter %r{^/spec/} add_filter %r{^/vendor/} - case ENV['RAILS'] + case ENV.fetch('RAILS', nil) when '6.0' then add_filter /_rails61|_rails70/ when '6.1' then add_filter /_rails60|_rails70/ when '7.0' then add_filter /_rails60|_rails61/ @@ -49,7 +49,7 @@ RSpec.configure do |config| config.filter_rails_from_backtrace! - config.fixture_path = "#{::Rails.root}/spec/fixtures" + config.fixture_path = "#{Rails.root}/spec/fixtures" config.infer_spec_type_from_file_location! config.use_transactional_fixtures = true diff --git a/spec/requests/file_controller_spec.rb b/spec/requests/file_controller_spec.rb index e1952df..71748ba 100644 --- a/spec/requests/file_controller_spec.rb +++ b/spec/requests/file_controller_spec.rb @@ -29,7 +29,7 @@ def create_blob_before_direct_upload(byte_size:, checksum:, filename: 'hello.txt } end let(:host) { "#{url_options[:protocol]}#{url_options[:host]}:#{url_options[:port]}" } - let(:engine_url_helpers) { ::ActiveStorageDB::Engine.routes.url_helpers } + let(:engine_url_helpers) { ActiveStorageDB::Engine.routes.url_helpers } before do if ActiveStorage::Current.respond_to? :url_options @@ -142,7 +142,7 @@ def create_blob_before_direct_upload(byte_size:, checksum:, filename: 'hello.txt let(:invalid_file) { create(:active_storage_db_file, data: 'Some other data') } before do - allow(::ActiveStorageDB::File).to receive(:find_by).and_return(invalid_file) + allow(ActiveStorageDB::File).to receive(:find_by).and_return(invalid_file) end it 'fails to upload' do diff --git a/spec/requests/file_url_spec.rb b/spec/requests/file_url_spec.rb index 402bef9..9f75650 100644 --- a/spec/requests/file_url_spec.rb +++ b/spec/requests/file_url_spec.rb @@ -2,7 +2,7 @@ RSpec.describe 'File URL', type: :request do let(:app_url_helpers) { Rails.application.routes.url_helpers } - let(:engine_url_helpers) { ::ActiveStorageDB::Engine.routes.url_helpers } + let(:engine_url_helpers) { ActiveStorageDB::Engine.routes.url_helpers } it 'has the service URL in the engine URL helpers' do expect(app_url_helpers).to respond_to :active_storage_db_path diff --git a/spec/service/active_storage/service/db_service_spec.rb b/spec/service/active_storage/service/db_service_spec.rb index bff9d6e..9af4131 100644 --- a/spec/service/active_storage/service/db_service_spec.rb +++ b/spec/service/active_storage/service/db_service_spec.rb @@ -57,8 +57,8 @@ let!(:db_file3) { create(:active_storage_db_file, ref: 'key3', data: 'third file') } it 'composes the source files' do - expect { compose }.to change { ::ActiveStorageDB::File.where(ref: 'dest_key').count }.by(1) - expect(compose).to be_kind_of ::ActiveStorageDB::File + expect { compose }.to change { ActiveStorageDB::File.where(ref: 'dest_key').count }.by(1) + expect(compose).to be_a ActiveStorageDB::File expect(compose.data).to eq [db_file1.data, db_file2.data, db_file3.data].join end end @@ -154,7 +154,7 @@ describe '.upload' do it 'uploads the data' do - expect(upload).to be_kind_of ActiveStorageDB::File + expect(upload).to be_a ActiveStorageDB::File ensure service.delete(key) end @@ -163,7 +163,7 @@ let(:upload) { service.upload(key, StringIO.new(fixture_data), checksum: checksum) } it 'uploads the data' do - expect(upload).to be_kind_of ActiveStorageDB::File + expect(upload).to be_a ActiveStorageDB::File ensure service.delete(key) end From 2a8c15df19a318655dfd58534769f547f71f4479 Mon Sep 17 00:00:00 2001 From: Mattia Roccoberton Date: Wed, 1 Nov 2023 09:48:59 +0100 Subject: [PATCH 2/3] feat: correct RuboCop offenses --- .rubocop.yml | 3 +-- active_storage_db.gemspec | 4 ++-- spec/models/active_storage_db/file_spec.rb | 2 +- spec/requests/file_controller_spec.rb | 2 +- spec/requests/file_url_spec.rb | 2 +- .../active_storage/service/db_service_spec.rb | 12 ++++++------ spec/tasks/active_storage_db_tasks_spec.rb | 12 ++++-------- 7 files changed, 16 insertions(+), 21 deletions(-) diff --git a/.rubocop.yml b/.rubocop.yml index aa4094b..cb27cc7 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -22,8 +22,7 @@ Lint/UnusedMethodArgument: AllowUnusedKeywordArguments: true RSpec/DescribeClass: - Exclude: - - spec/tasks/active_storage_db_tasks_spec.rb + Enabled: false RSpec/ExampleLength: # Default is 5 diff --git a/active_storage_db.gemspec b/active_storage_db.gemspec index 6c8aeec..0aa183e 100644 --- a/active_storage_db.gemspec +++ b/active_storage_db.gemspec @@ -27,6 +27,6 @@ Gem::Specification.new do |spec| spec.add_dependency 'activestorage', '>= 6.0' spec.add_dependency 'rails', '>= 6.0' - spec.add_development_dependency 'appraisal', '~> 2.4' - spec.add_development_dependency 'factory_bot_rails', '~> 6.1' + spec.add_development_dependency 'appraisal', '~> 2.4' # rubocop:disable Gemspec/DevelopmentDependencies + spec.add_development_dependency 'factory_bot_rails', '~> 6.1' # rubocop:disable Gemspec/DevelopmentDependencies end diff --git a/spec/models/active_storage_db/file_spec.rb b/spec/models/active_storage_db/file_spec.rb index 93868ea..51545f6 100644 --- a/spec/models/active_storage_db/file_spec.rb +++ b/spec/models/active_storage_db/file_spec.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true -RSpec.describe ActiveStorageDB::File, type: :model do +RSpec.describe ActiveStorageDB::File do let(:file) { create(:active_storage_db_file, ref: 'just_some_key') } context 'when creating a file with an already existing key' do diff --git a/spec/requests/file_controller_spec.rb b/spec/requests/file_controller_spec.rb index 71748ba..2e8828b 100644 --- a/spec/requests/file_controller_spec.rb +++ b/spec/requests/file_controller_spec.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true -RSpec.describe 'File controller', type: :request do +RSpec.describe 'File controller' do def create_blob(data: 'Hello world!', filename: 'hello.txt', content_type: 'text/plain', identify: true, record: nil) ActiveStorage::Blob.create_and_upload!( io: StringIO.new(data), diff --git a/spec/requests/file_url_spec.rb b/spec/requests/file_url_spec.rb index 9f75650..dd7d392 100644 --- a/spec/requests/file_url_spec.rb +++ b/spec/requests/file_url_spec.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true -RSpec.describe 'File URL', type: :request do +RSpec.describe 'File URL' do let(:app_url_helpers) { Rails.application.routes.url_helpers } let(:engine_url_helpers) { ActiveStorageDB::Engine.routes.url_helpers } diff --git a/spec/service/active_storage/service/db_service_spec.rb b/spec/service/active_storage/service/db_service_spec.rb index 9af4131..86fe38d 100644 --- a/spec/service/active_storage/service/db_service_spec.rb +++ b/spec/service/active_storage/service/db_service_spec.rb @@ -52,14 +52,14 @@ describe '.compose' do subject(:compose) { service.compose(%w[key1 key2 key3], 'dest_key') } - let!(:db_file1) { create(:active_storage_db_file, ref: 'key1', data: 'first file') } - let!(:db_file2) { create(:active_storage_db_file, ref: 'key2', data: 'second file') } - let!(:db_file3) { create(:active_storage_db_file, ref: 'key3', data: 'third file') } + let!(:first_db_file) { create(:active_storage_db_file, ref: 'key1', data: 'first file') } + let!(:second_db_file) { create(:active_storage_db_file, ref: 'key2', data: 'second file') } + let!(:third_db_file) { create(:active_storage_db_file, ref: 'key3', data: 'third file') } it 'composes the source files' do expect { compose }.to change { ActiveStorageDB::File.where(ref: 'dest_key').count }.by(1) expect(compose).to be_a ActiveStorageDB::File - expect(compose.data).to eq [db_file1.data, db_file2.data, db_file3.data].join + expect(compose.data).to eq [first_db_file.data, second_db_file.data, third_db_file.data].join end end end @@ -70,7 +70,7 @@ before { upload } it 'deletes the file' do - expect { delete }.to change { ActiveStorageDB::File.count }.from(1).to(0) + expect { delete }.to change(ActiveStorageDB::File, :count).from(1).to(0) end end @@ -80,7 +80,7 @@ before { upload } it 'deletes the files' do - expect { delete_prefixed }.to change { ActiveStorageDB::File.count }.from(1).to(0) + expect { delete_prefixed }.to change(ActiveStorageDB::File, :count).from(1).to(0) end end diff --git a/spec/tasks/active_storage_db_tasks_spec.rb b/spec/tasks/active_storage_db_tasks_spec.rb index cbbec45..b9f59c9 100644 --- a/spec/tasks/active_storage_db_tasks_spec.rb +++ b/spec/tasks/active_storage_db_tasks_spec.rb @@ -6,16 +6,12 @@ describe 'asdb:list' do subject(:task) { execute_task('asdb:list') } - let(:file1) { create(:active_storage_blob, filename: 'some file 1', created_at: Time.now - 1.hour) } - let(:file2) { create(:active_storage_blob, filename: 'some file 2', created_at: Time.now - 5.hour) } - let(:file3) { create(:active_storage_blob, filename: 'some file 3', created_at: Time.now - 3.hour) } - - before do - [file1, file2, file3] - end + let!(:first_file) { create(:active_storage_blob, filename: 'some file 1', created_at: Time.now - 1.hour) } + let!(:second_file) { create(:active_storage_blob, filename: 'some file 2', created_at: Time.now - 5.hour) } + let!(:third_file) { create(:active_storage_blob, filename: 'some file 3', created_at: Time.now - 3.hour) } it 'prints the columns header + the list of 3 files', :aggregate_failures do - pattern = /#{file3.id} #{file3.filename}.+#{file2.id} #{file2.filename}.+#{file1.id} #{file1.filename}/m + pattern = /#{third_file.id} #{third_file.filename}.+#{second_file.id} #{second_file.filename}.+#{first_file.id} #{first_file.filename}/m expect(task).to match pattern expect(task.split("\n").size).to be 4 end From cf69436bd660636653d27e9ef3cece7e0c1ef42f Mon Sep 17 00:00:00 2001 From: Mattia Roccoberton Date: Wed, 1 Nov 2023 10:35:53 +0100 Subject: [PATCH 3/3] test: rename factories' files --- .../{active_storage_blob_factory.rb => active_storage_blobs.rb} | 0 ...tive_storage_db_file_factory.rb => active_storage_db_files.rb} | 0 2 files changed, 0 insertions(+), 0 deletions(-) rename spec/factories/{active_storage_blob_factory.rb => active_storage_blobs.rb} (100%) rename spec/factories/{active_storage_db_file_factory.rb => active_storage_db_files.rb} (100%) diff --git a/spec/factories/active_storage_blob_factory.rb b/spec/factories/active_storage_blobs.rb similarity index 100% rename from spec/factories/active_storage_blob_factory.rb rename to spec/factories/active_storage_blobs.rb diff --git a/spec/factories/active_storage_db_file_factory.rb b/spec/factories/active_storage_db_files.rb similarity index 100% rename from spec/factories/active_storage_db_file_factory.rb rename to spec/factories/active_storage_db_files.rb