From 9a592ee1572e81b7e26bed009fabedc00b0be8d1 Mon Sep 17 00:00:00 2001 From: Vincent Prigent Date: Thu, 16 Jan 2025 13:30:03 +1300 Subject: [PATCH 1/7] Add missing ostruct require --- spec/bullet/detector/n_plus_one_query_spec.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/spec/bullet/detector/n_plus_one_query_spec.rb b/spec/bullet/detector/n_plus_one_query_spec.rb index 7b981bf4..fe0f291f 100644 --- a/spec/bullet/detector/n_plus_one_query_spec.rb +++ b/spec/bullet/detector/n_plus_one_query_spec.rb @@ -1,6 +1,7 @@ # frozen_string_literal: true require 'spec_helper' +require 'ostruct' using Bullet::Ext::Object From e38fb42444a9f03e399e7ebb361e40bd4d13d6da Mon Sep 17 00:00:00 2001 From: Vincent Prigent Date: Thu, 16 Jan 2025 14:37:32 +1300 Subject: [PATCH 2/7] Update benchmark to use sqlite Fix typo in comment --- lib/bullet/detector/association.rb | 2 +- perf/benchmark.rb | 8 +++----- 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/lib/bullet/detector/association.rb b/lib/bullet/detector/association.rb index ab1b2cfd..a92ec6cc 100644 --- a/lib/bullet/detector/association.rb +++ b/lib/bullet/detector/association.rb @@ -81,7 +81,7 @@ def eager_loadings Thread.current.thread_variable_get(:bullet_eager_loadings) end - # cal_stacks keeps stacktraces where querie-objects were called from. + # call_stacks keeps stacktraces where querie-objects were called from. # e.g. { 'Object:111' => [SomeProject/app/controllers/...] } def call_stacks Thread.current.thread_variable_get(:bullet_call_stacks) diff --git a/perf/benchmark.rb b/perf/benchmark.rb index da07b00c..d7476494 100644 --- a/perf/benchmark.rb +++ b/perf/benchmark.rb @@ -30,10 +30,8 @@ class User < ActiveRecord::Base # create database bullet_benchmark; ActiveRecord::Base.establish_connection( - adapter: 'mysql2', - database: 'bullet_benchmark', - server: '/tmp/mysql.socket', - username: 'root' + adapter: 'sqlite3', + database: 'bullet_benchmark' ) ActiveRecord::Base.connection.tables.each { |table| ActiveRecord::Base.connection.drop_table(table) } @@ -81,7 +79,7 @@ class User < ActiveRecord::Base bm.report("Querying & Iterating #{posts_size} Posts with #{comments_size} Comments and #{users_size} Users") do 10.times do Bullet.start_request - Post.select('SQL_NO_CACHE *').includes(:user, comments: :user).each do |p| + Post.includes(:user, comments: :user).each do |p| p.title p.user.name p.comments.each do |c| From ad7d4c862d308e90e2e6ff0e71fb9e5ef42e520e Mon Sep 17 00:00:00 2001 From: Vincent Prigent Date: Thu, 16 Jan 2025 14:38:20 +1300 Subject: [PATCH 3/7] Remove remnants of Ruby 1.9 support in StackTraceFilter --- lib/bullet/stack_trace_filter.rb | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/lib/bullet/stack_trace_filter.rb b/lib/bullet/stack_trace_filter.rb index f46167eb..dfd51617 100644 --- a/lib/bullet/stack_trace_filter.rb +++ b/lib/bullet/stack_trace_filter.rb @@ -7,7 +7,6 @@ module Bullet module StackTraceFilter VENDOR_PATH = '/vendor' - IS_RUBY_19 = Gem::Version.new(RUBY_VERSION) < Gem::Version.new('2.0.0') # @param bullet_key[String] - use this to get stored call stack from call_stacks object. def caller_in_project(bullet_key = nil) @@ -56,13 +55,12 @@ def pattern_matches?(location, pattern) def location_as_path(location) return location if location.is_a?(String) - IS_RUBY_19 ? location : location.absolute_path.to_s + location.absolute_path.to_s end def select_caller_locations(bullet_key = nil) - return caller.select { |caller_path| yield caller_path } if IS_RUBY_19 - call_stack = bullet_key ? call_stacks[bullet_key] : caller_locations + call_stack.select { |location| yield location } end end From b10ed992aa4d5dc26ca6494e84de1741541a68b5 Mon Sep 17 00:00:00 2001 From: Vincent Prigent Date: Thu, 16 Jan 2025 14:40:53 +1300 Subject: [PATCH 4/7] Use memoization on refined class Avoid recreating strings within n_plus_one_query --- lib/bullet/detector/n_plus_one_query.rb | 10 +++++----- lib/bullet/ext/object.rb | 20 +++++++++----------- lib/bullet/ext/string.rb | 7 ++++++- 3 files changed, 20 insertions(+), 17 deletions(-) diff --git a/lib/bullet/detector/n_plus_one_query.rb b/lib/bullet/detector/n_plus_one_query.rb index 7a6600e1..650ea3c9 100644 --- a/lib/bullet/detector/n_plus_one_query.rb +++ b/lib/bullet/detector/n_plus_one_query.rb @@ -27,7 +27,7 @@ def call_association(object, associations) ) if !excluded_stacktrace_path? && conditions_met?(object, associations) Bullet.debug('detect n + 1 query', "object: #{object.bullet_key}, associations: #{associations}") - create_notification caller_in_project(object.bullet_key), object.class.to_s, associations + create_notification(caller_in_project(object.bullet_key), object.class.to_s, associations) end end @@ -38,16 +38,16 @@ def add_possible_objects(object_or_objects) objects = Array.wrap(object_or_objects) class_names_match_regex = true primary_key_values_are_empty = true - keys_joined = "" - objects.each do |obj| + + keys_joined = objects.map do |obj| unless obj.class.name =~ /^HABTM_/ class_names_match_regex = false end unless obj.bullet_primary_key_value.nil? primary_key_values_are_empty = false end - keys_joined += "#{(keys_joined.empty? ? '' : ', ')}#{obj.bullet_key}" - end + obj.bullet_key + end.join(", ") unless class_names_match_regex || primary_key_values_are_empty Bullet.debug('Detector::NPlusOneQuery#add_possible_objects', "objects: #{keys_joined}") objects.each { |object| possible_objects.add object.bullet_key } diff --git a/lib/bullet/ext/object.rb b/lib/bullet/ext/object.rb index 68dfbc87..f8e60cbb 100644 --- a/lib/bullet/ext/object.rb +++ b/lib/bullet/ext/object.rb @@ -4,22 +4,20 @@ module Bullet module Ext module Object refine ::Object do + attr_writer :bullet_key, :bullet_primary_key_value + def bullet_key - "#{self.class}:#{bullet_primary_key_value}" + @bullet_key ||= "#{self.class}:#{bullet_primary_key_value}" end def bullet_primary_key_value - return if respond_to?(:persisted?) && !persisted? - - if self.class.respond_to?(:primary_keys) && self.class.primary_keys - primary_key = self.class.primary_keys - elsif self.class.respond_to?(:primary_key) && self.class.primary_key - primary_key = self.class.primary_key - else - primary_key = :id - end + @bullet_primary_key_value ||= begin + return if respond_to?(:persisted?) && !persisted? - bullet_join_potential_composite_primary_key(primary_key) + primary_key = self.class.try(:primary_keys) || self.class.try(:primary_key) || :id + + bullet_join_potential_composite_primary_key(primary_key) + end end private diff --git a/lib/bullet/ext/string.rb b/lib/bullet/ext/string.rb index 1b9e9413..ed4ee285 100644 --- a/lib/bullet/ext/string.rb +++ b/lib/bullet/ext/string.rb @@ -4,8 +4,13 @@ module Bullet module Ext module String refine ::String do + attr_reader :bullet_class_name + def bullet_class_name - sub(/:[^:]*?$/, '') + @bullet_class_name ||= begin + last_colon = self.rindex(':') + last_colon ? self[0...last_colon] : self + end end end end From 42d24737d0998d623d626f6a8cf9b4bf065249a4 Mon Sep 17 00:00:00 2001 From: Vincent Prigent Date: Tue, 21 Jan 2025 16:45:15 +1300 Subject: [PATCH 5/7] Disable memoize on string refinement method Its causing all the specs to fail, however I was not able to figure out why it was doing so... --- lib/bullet/detector/n_plus_one_query.rb | 1 + lib/bullet/ext/string.rb | 8 ++------ 2 files changed, 3 insertions(+), 6 deletions(-) diff --git a/lib/bullet/detector/n_plus_one_query.rb b/lib/bullet/detector/n_plus_one_query.rb index 650ea3c9..4b27d4ef 100644 --- a/lib/bullet/detector/n_plus_one_query.rb +++ b/lib/bullet/detector/n_plus_one_query.rb @@ -48,6 +48,7 @@ def add_possible_objects(object_or_objects) end obj.bullet_key end.join(", ") + unless class_names_match_regex || primary_key_values_are_empty Bullet.debug('Detector::NPlusOneQuery#add_possible_objects', "objects: #{keys_joined}") objects.each { |object| possible_objects.add object.bullet_key } diff --git a/lib/bullet/ext/string.rb b/lib/bullet/ext/string.rb index ed4ee285..8319d537 100644 --- a/lib/bullet/ext/string.rb +++ b/lib/bullet/ext/string.rb @@ -4,13 +4,9 @@ module Bullet module Ext module String refine ::String do - attr_reader :bullet_class_name - def bullet_class_name - @bullet_class_name ||= begin - last_colon = self.rindex(':') - last_colon ? self[0...last_colon] : self - end + last_colon = self.rindex(':') + last_colon ? self[0...last_colon].dup : self.dup end end end From f9677abfc9f5f954d8b51147dd489a8ae6f6b3af Mon Sep 17 00:00:00 2001 From: Vincent Prigent Date: Wed, 22 Jan 2025 09:59:22 +1300 Subject: [PATCH 6/7] Fix test to avoid memoization bug I don't believe primary_key is ever changed within a request --- spec/bullet/ext/object_spec.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/spec/bullet/ext/object_spec.rb b/spec/bullet/ext/object_spec.rb index 13846f81..dbf03584 100644 --- a/spec/bullet/ext/object_spec.rb +++ b/spec/bullet/ext/object_spec.rb @@ -26,21 +26,21 @@ end it 'should return primary key value' do - post = Post.first Post.primary_key = 'name' + post = Post.first expect(post.bullet_primary_key_value).to eq(post.name) Post.primary_key = 'id' end it 'should return value for multiple primary keys from the composite_primary_key gem' do - post = Post.first allow(Post).to receive(:primary_keys).and_return(%i[category_id writer_id]) + post = Post.first expect(post.bullet_primary_key_value).to eq("#{post.category_id},#{post.writer_id}") end it 'should return value for multiple primary keys from ActiveRecord 7.1' do - post = Post.first allow(Post).to receive(:primary_key).and_return(%i[category_id writer_id]) + post = Post.first expect(post.bullet_primary_key_value).to eq("#{post.category_id},#{post.writer_id}") end From aa2158fbdee96679b9c588ad76ece43b73d11719 Mon Sep 17 00:00:00 2001 From: Vincent Prigent Date: Sat, 25 Jan 2025 18:35:51 +1300 Subject: [PATCH 7/7] Try debug the actions --- .github/workflows/main.yml | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index ab309ba9..f954cc14 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -29,6 +29,11 @@ jobs: ruby-version: 2.7 bundler: 1 bundler-cache: true + - name: Breakpoint if tests failed + if: failure() + uses: namespacelabs/breakpoint-action@v0 + with: + duration: 30m - name: Run tests run: bundle exec rake test_rails_5: