Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: minor internal improvements #36

Merged
merged 3 commits into from
Nov 1, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions .rubocop.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions active_storage_db.gemspec
Original file line number Diff line number Diff line change
Expand Up @@ -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
14 changes: 7 additions & 7 deletions lib/tasks/active_storage_db_tasks.rake
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -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)
Expand All @@ -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'
Expand Down
2 changes: 1 addition & 1 deletion spec/models/active_storage_db/file_spec.rb
Original file line number Diff line number Diff line change
@@ -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
Expand Down
4 changes: 2 additions & 2 deletions spec/rails_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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/
Expand Down Expand Up @@ -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

Expand Down
6 changes: 3 additions & 3 deletions spec/requests/file_controller_spec.rb
Original file line number Diff line number Diff line change
@@ -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),
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions spec/requests/file_url_spec.rb
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
# 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 }
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
Expand Down
20 changes: 10 additions & 10 deletions spec/service/active_storage/service/db_service_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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_kind_of ::ActiveStorageDB::File
expect(compose.data).to eq [db_file1.data, db_file2.data, db_file3.data].join
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 [first_db_file.data, second_db_file.data, third_db_file.data].join
end
end
end
Expand All @@ -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

Expand All @@ -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

Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down
12 changes: 4 additions & 8 deletions spec/tasks/active_storage_db_tasks_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Loading