Merge pull request #70 from blocknotes/dev/internal-improvements

blocknotes · web-flow · commit 503b1a47ff04 · 2025-05-31T15:13:16.000+02:00
Dev &amp; Test internal improvements
diff --git a/.rubocop.yml b/.rubocop.yml
@@ -22,6 +22,10 @@ Lint/MissingSuper:
 Lint/UnusedMethodArgument:
   AllowUnusedKeywordArguments: true
 
+Naming/FileName:
+  Exclude:
+    - spec/dummy/db/*
+
 RSpec/DescribeClass:
   Enabled: false
 
diff --git a/Gemfile b/Gemfile
@@ -44,6 +44,7 @@ end
 gem 'concurrent-ruby', '1.3.4'
 
 gem 'bigdecimal'
+gem 'bullet'
 gem 'image_processing', '>= 1.2'
 gem 'mutex_m'
 gem 'puma'
diff --git a/Makefile b/Makefile
@@ -38,6 +38,9 @@ console:
 lint:
 	@docker compose -f extra/docker-compose.yml exec app_with_${DB} bin/rubocop
 
+seed:
+	@docker compose -f extra/docker-compose.yml exec app_with_${DB} bin/rails db:seed
+
 server:
 	@docker compose -f extra/docker-compose.yml exec app_with_${DB} bin/rails server -b 0.0.0.0 -p ${SERVER_PORT}
 
diff --git a/extra/Dockerfile b/extra/Dockerfile
@@ -5,9 +5,9 @@ ENV DEBIAN_FRONTEND=noninteractive
 ENV DEVEL=1
 ENV LANG=C.UTF-8
 
-RUN apt-get update -qq
-RUN apt-get install -yqq --no-install-recommends build-essential less nano netcat-traditional pkg-config
-RUN apt-get install -yqq --no-install-recommends freetds-dev libmariadb-dev libpq-dev libvips42 libyaml-dev sqlite3
+RUN apt-get update -qq && \
+  apt-get install -yqq --no-install-recommends build-essential less nano netcat-traditional pkg-config && \
+  apt-get install -yqq --no-install-recommends freetds-dev libmariadb-dev libpq-dev libvips42 libyaml-dev sqlite3
 
 ARG BUNDLER_VERSION
 RUN gem install bundler -v ${BUNDLER_VERSION}
diff --git a/lib/active_storage/service/db_service.rb b/lib/active_storage/service/db_service.rb
@@ -57,7 +57,8 @@ def download_chunk(key, range)
 
     def delete(key)
       instrument :delete, key: key do
-        record = ::ActiveStorageDB::File.find_by(ref: key)
+        comment = "DBService#delete"
+        record = ::ActiveStorageDB::File.annotate(comment).find_by(ref: key)
         record&.destroy
         # NOTE: Ignore files already deleted
         !record.nil?
@@ -66,15 +67,18 @@ def delete(key)
 
     def delete_prefixed(prefix)
       instrument :delete_prefixed, prefix: prefix do
-        ::ActiveStorageDB::File.where('ref LIKE ?', "#{ApplicationRecord.sanitize_sql_like(prefix)}%").destroy_all
+        comment = "DBService#delete_prefixed"
+        sanitized_prefix = "#{ApplicationRecord.sanitize_sql_like(prefix)}%"
+        ::ActiveStorageDB::File.annotate(comment).where('ref LIKE ?', sanitized_prefix).destroy_all
       end
     end
 
     def exist?(key)
       instrument :exist, key: key do |payload|
-        answer = ::ActiveStorageDB::File.where(ref: key).exists?
-        payload[:exist] = answer
-        answer
+        comment = "DBService#exist?"
+        result = ::ActiveStorageDB::File.annotate(comment).where(ref: key).exists?
+        payload[:exist] = result
+        result
       end
     end
 
@@ -152,7 +156,8 @@ def retrieve_file(key)
     end
 
     def object_for(key, fields: nil)
-      as_file = fields ? ::ActiveStorageDB::File.select(*fields) : ::ActiveStorageDB::File
+      comment = "DBService#object_for"
+      as_file = fields ? ::ActiveStorageDB::File.annotate(comment).select(*fields) : ::ActiveStorageDB::File
       as_file.find_by(ref: key)
     end
 
diff --git a/lib/active_storage/service/db_service_rails70.rb b/lib/active_storage/service/db_service_rails70.rb
@@ -4,8 +4,9 @@ module ActiveStorage
   module DBServiceRails70
     def compose(source_keys, destination_key, **)
       buffer = nil
+      comment = "DBService#compose"
       source_keys.each do |source_key|
-        data = ::ActiveStorageDB::File.find_by!(ref: source_key).data
+        data = ::ActiveStorageDB::File.annotate(comment).find_by!(ref: source_key).data
         if buffer
           buffer << data
         else
diff --git a/spec/dummy/app/assets/images/.keep b/spec/dummy/app/assets/images/.keep
diff --git a/spec/dummy/app/channels/application_cable/channel.rb b/spec/dummy/app/channels/application_cable/channel.rb
diff --git a/spec/dummy/app/channels/application_cable/connection.rb b/spec/dummy/app/channels/application_cable/connection.rb
diff --git a/spec/dummy/app/helpers/application_helper.rb b/spec/dummy/app/helpers/application_helper.rb
diff --git a/spec/dummy/app/mailers/application_mailer.rb b/spec/dummy/app/mailers/application_mailer.rb
diff --git a/spec/dummy/app/views/layouts/application.html.erb b/spec/dummy/app/views/layouts/application.html.erb
@@ -5,9 +5,23 @@
     <meta name="viewport" content="width=device-width,initial-scale=1">
     <%= csrf_meta_tags %>
     <%= csp_meta_tag %>
+
+    <link rel="stylesheet" href="https://fonts.googleapis.com/css?family=Roboto:300,300italic,700,700italic">
+    <link rel="stylesheet" href="https://cdnjs.cloudflare.com/ajax/libs/normalize/8.0.1/normalize.css">
+    <link rel="stylesheet" href="https://cdnjs.cloudflare.com/ajax/libs/milligram/1.4.1/milligram.css">
+
+    <style>
+      form.button_to { display: inline-block; }
+      .container { background: #fafafa; min-height: 100vh; }
+      .listing { list-style: none; }
+      .listing .button { margin-right: 5px; }
+      .post svg { opacity: 0.5; width: 128px; height: 128px; }
+    </style>
   </head>
 
   <body>
-    <%= yield %>
+    <div class="container">
+      <%= yield %>
+    </div>
   </body>
 </html>
diff --git a/spec/dummy/app/views/posts/_post.html.erb b/spec/dummy/app/views/posts/_post.html.erb
@@ -1,10 +1,10 @@
-<div>
-  <div class="item"><b>title</b>: <%= post.title %></div>
-  <div class="item"><b>content</b>: <%= post.content %></div>
-  <div class="item"><b>created_at</b>: <%= post.created_at %></div>
+<div class="post">
+  <div class="item"><b>Title</b>: <%= post.title %></div>
+  <div class="item"><b>Content</b>: <%= post.content %></div>
+  <div class="item"><b>Created at</b>: <%= post.created_at %></div>
   <% if post.some_file.attached?  %>
   <div class="item">
-    <pre><%= post.some_file.download %></pre>
+    <%== post.some_file.download %>
   </div>
   <% end %>
 </div>
diff --git a/spec/dummy/app/views/posts/index.html.erb b/spec/dummy/app/views/posts/index.html.erb
@@ -1,15 +1,23 @@
 <h1>Posts</h1>
 
-<ul>
+<ul class="listing">
 <% @posts.each do |post| %>
   <li>
-    <%= post.title %><br/>
+    <div>
+      <b><%= post.title %></b>
+    </div>
     <% if post.content %>
-      <%= post.content %><br/>
+      <div>
+        <%= post.content %>
+      </div>
     <% end %>
-    <%= link_to 'Show', post, class: 'button button-outline' %>
-    <%= link_to 'Edit', edit_post_path(post), class: 'button button-outline' %>
-    <%= link_to 'Destroy', post, data: { confirm: 'Are you sure?' }, method: :delete, class: 'button button-outline' %>
+    <%= link_to 'Show', post, class: 'button button-outline' %><%= link_to 'Edit', edit_post_path(post), class: 'button button-outline' %><%= button_to 'Destroy', post, data: { confirm: 'Are you sure?' }, method: :delete, class: 'button button-outline' %>
   </li>
 <% end %>
 </ul>
+
+<hr/>
+
+<div>
+  <%= link_to 'New', new_post_path, class: 'button button-outline' %>
+</div>
diff --git a/spec/dummy/config/cable.yml b/spec/dummy/config/cable.yml
diff --git a/spec/dummy/config/environments/development.rb b/spec/dummy/config/environments/development.rb
@@ -3,6 +3,15 @@
 require "active_support/core_ext/integer/time"
 
 Rails.application.configure do
+  config.after_initialize do
+    Bullet.enable        = true
+    Bullet.alert         = true
+    Bullet.bullet_logger = true
+    Bullet.console       = true
+    Bullet.rails_logger  = true
+    Bullet.add_footer    = true
+  end
+
   # Settings specified here will take precedence over those in config/application.rb.
 
   # In the development environment your application's code is reloaded any time
diff --git a/spec/dummy/config/environments/test.rb b/spec/dummy/config/environments/test.rb
@@ -8,6 +8,12 @@
 # and recreated between test runs. Don't rely on the data there!
 
 Rails.application.configure do
+  config.after_initialize do
+    Bullet.enable        = true
+    Bullet.bullet_logger = true
+    Bullet.raise         = true # raise an error if n+1 query occurs
+  end
+
   # Settings specified here will take precedence over those in config/application.rb.
 
   # Turn false under Spring and add config.action_view.cache_template_loading = true.
diff --git a/spec/dummy/db/seeds.rb b/spec/dummy/db/seeds.rb
@@ -0,0 +1,11 @@
+# frozen_string_literal: true
+
+4.times do |i|
+  io = Rails.root.join("public/globe.svg").open
+  Post.find_or_create_by!(title: "Post #{i + 1}") do |post|
+    post.assign_attributes(content: "Some content for post #{i + 1}")
+    if i.even?
+      post.some_file.attach(io: io, filename: "img#{i + 1}.svg")
+    end
+  end
+end
diff --git a/spec/dummy/lib/assets/.keep b/spec/dummy/lib/assets/.keep
diff --git a/spec/dummy/public/globe.svg b/spec/dummy/public/globe.svg
@@ -0,0 +1 @@
+<svg xmlns="http://www.w3.org/2000/svg" id="Outline" viewBox="0 0 24 24" width="512" height="512"><path d="M12,0A12,12,0,1,0,24,12,12.013,12.013,0,0,0,12,0Zm8.647,7H17.426a19.676,19.676,0,0,0-2.821-4.644A10.031,10.031,0,0,1,20.647,7ZM16.5,12a10.211,10.211,0,0,1-.476,3H7.976A10.211,10.211,0,0,1,7.5,12a10.211,10.211,0,0,1,.476-3h8.048A10.211,10.211,0,0,1,16.5,12ZM8.778,17h6.444A19.614,19.614,0,0,1,12,21.588,19.57,19.57,0,0,1,8.778,17Zm0-10A19.614,19.614,0,0,1,12,2.412,19.57,19.57,0,0,1,15.222,7ZM9.4,2.356A19.676,19.676,0,0,0,6.574,7H3.353A10.031,10.031,0,0,1,9.4,2.356ZM2.461,9H5.9a12.016,12.016,0,0,0-.4,3,12.016,12.016,0,0,0,.4,3H2.461a9.992,9.992,0,0,1,0-6Zm.892,8H6.574A19.676,19.676,0,0,0,9.4,21.644,10.031,10.031,0,0,1,3.353,17Zm11.252,4.644A19.676,19.676,0,0,0,17.426,17h3.221A10.031,10.031,0,0,1,14.605,21.644ZM21.539,15H18.1a12.016,12.016,0,0,0,.4-3,12.016,12.016,0,0,0-.4-3h3.437a9.992,9.992,0,0,1,0,6Z"/></svg>
diff --git a/spec/integration/attachments_spec.rb b/spec/integration/attachments_spec.rb
@@ -12,7 +12,7 @@
     end
     let(:test_post) { Post.new(title: 'A test post', some_file: uploaded_file) }
 
-    it 'creates the entity with the attached file', :aggregate_failures do
+    it 'creates the entity with the attached file', :aggregate_failures, :skip_bullet do
       expect { test_post.save! }.to change(ActiveStorageDB::File, :count).by(1)
 
       expect(test_post.some_file).to be_attached
diff --git a/spec/rails_helper.rb b/spec/rails_helper.rb
@@ -37,13 +37,6 @@
 support_files = File.expand_path('support/**/*.rb', __dir__)
 Dir[support_files].sort.each { |f| require f }
 
-# begin
-#   ActiveRecord::Migration.maintain_test_schema!
-# rescue ActiveRecord::PendingMigrationError => e
-#   puts e.to_s.strip
-#   exit 1
-# end
-
 RSpec.configure do |config|
   config.filter_rails_from_backtrace!
   config.infer_spec_type_from_file_location!
@@ -68,4 +61,15 @@
 
     RSpec.configuration.reporter.message(intro)
   end
+
+  config.before do |example|
+    Bullet.start_request unless example.metadata[:skip_bullet]
+  end
+
+  config.after do |example|
+    unless example.metadata[:skip_bullet]
+      Bullet.perform_out_of_channel_notifications if Bullet.notification?
+      Bullet.end_request
+    end
+  end
 end
diff --git a/spec/service/active_storage/service/db_service_spec.rb b/spec/service/active_storage/service/db_service_spec.rb
@@ -141,6 +141,15 @@
           expect(download_block).to eq fixture_data
         end
       end
+
+      context "with download a block and when the attachment is not found" do
+        let(:download_block) do
+          service.download("#{key}!") do |_data| # rubocop:disable Lint/EmptyBlock
+          end
+        end
+
+        it { expect { download_block }.to raise_exception(ActiveStorage::FileNotFoundError) }
+      end
     end
   end
 
@@ -154,6 +163,12 @@
     after { service.delete(key) }
 
     it { is_expected.to eq fixture_data[range] }
+
+    context "when the attachment is not found" do
+      subject(:download_chunk) { service.download_chunk("#{key}!", range) }
+
+      it { expect { download_chunk }.to raise_exception(ActiveStorage::FileNotFoundError) }
+    end
   end
 
   describe '.exist?' do
