diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index e017e25..0179446 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -27,5 +27,6 @@ jobs: - name: Install dependencies run: bundle install - name: Run tests - run: - bundle exec rspec \ No newline at end of file + run: bundle exec rspec + - name: Rubocop + run: bundle exec rubocop --parallel \ No newline at end of file diff --git a/.rubocop.yml b/.rubocop.yml new file mode 100644 index 0000000..4c50b1a --- /dev/null +++ b/.rubocop.yml @@ -0,0 +1,49 @@ +# Omakase Ruby styling for Rails +inherit_gem: { rubocop-rails-omakase: rubocop.yml } + +AllCops: + TargetRubyVersion: 3.1 + +Layout/AssignmentIndentation: + Enabled: true + +Layout/DefEndAlignment: + Enabled: true + EnforcedStyleAlignWith: def + AutoCorrect: true + +Layout/EmptyLineBetweenDefs: + Enabled: true + NumberOfEmptyLines: 1 + +Layout/FirstArrayElementIndentation: + Enabled: true + EnforcedStyle: consistent + +Layout/HeredocIndentation: + Enabled: true + +Layout/IndentationConsistency: + Enabled: true + EnforcedStyle: normal + +Layout/IndentationStyle: + Enabled: true + +Layout/IndentationWidth: + Enabled: true + Width: 2 + +Style/ClassAndModuleChildren: + Enabled: true + EnforcedStyle: nested + +Lint/Debugger: + Enabled: true + +Style/FrozenStringLiteralComment: + Enabled: true + +Style/StringLiterals: + Enabled: true + EnforcedStyle: single_quotes diff --git a/Gemfile b/Gemfile index d8f73d7..6d5d66f 100644 --- a/Gemfile +++ b/Gemfile @@ -1,6 +1,9 @@ +# frozen_string_literal: true + source 'https://rubygems.org' -gem "activerecord", ">=7" -gem "sqlite3", "~> 1.4" +gem 'activerecord', '>=7' +gem 'sqlite3', '~> 1.4' +gem 'rubocop-rails-omakase', '~> 1.1', require: false # Specify your gem's dependencies in jit_preloader.gemspec gemspec diff --git a/README.md b/README.md index 20d9223..48b81c0 100644 --- a/README.md +++ b/README.md @@ -90,16 +90,16 @@ This gem will publish an `n_plus_one_query` event via ActiveSupport::Notificatio You could implement some basic tracking. This will let you measure the extent of the N+1 query problems in your app: ```ruby -ActiveSupport::Notifications.subscribe("n_plus_one_query") do |event, data| +ActiveSupport::Notifications.subscribe('n_plus_one_query') do |event, data| statsd.increment "web.#{Rails.env}.n_plus_one_queries.global" end ``` You could log the N+1 queries. In your development environment, you could throw N+1 queries into the logs along with a stack trace: ```ruby -ActiveSupport::Notifications.subscribe("n_plus_one_query") do |event, data| +ActiveSupport::Notifications.subscribe('n_plus_one_query') do |event, data| message = "N+1 Query detected: #{data[:association]} on #{data[:source].class}" - backtrace = caller.select{|r| r.starts_with?(Rails.root.to_s) } + backtrace = caller.select { |r| r.starts_with?(Rails.root.to_s) } Rails.logger.debug("\n\n#{message}\n#{backtrace.join("\n")}\n".red) end ``` @@ -113,7 +113,7 @@ config.around(:each) do |example| raise QueryError.new(message) end end - ActiveSupport::Notifications.subscribed(callback, "n_plus_one_query") do + ActiveSupport::Notifications.subscribed(callback, 'n_plus_one_query') do example.run end end @@ -144,7 +144,7 @@ There is now a `has_many_aggregate` method available for ActiveRecord::Base. Thi ```ruby # old Contact.all.each do |contact| - contact.addresses.maximum("LENGTH(street)") + contact.addresses.maximum('LENGTH(street)') contact.addresses.count end # SELECT * FROM contacts @@ -159,8 +159,8 @@ end #new class Contact < ActiveRecord::Base has_many :addresses - has_many_aggregate :addresses, :max_street_length, :maximum, "LENGTH(street)", default: nil - has_many_aggregate :addresses, :count_all, :count, "*" + has_many_aggregate :addresses, :max_street_length, :maximum, 'LENGTH(street)', default: nil + has_many_aggregate :addresses, :count_all, :count, '*' end Contact.jit_preload.each do |contact| @@ -177,7 +177,7 @@ Furthermore, there is an argument `max_ids_per_query` setting max ids per query. ```ruby class Contact < ActiveRecord::Base has_many :addresses - has_many_aggregate :addresses, :count_all, :count, "*", max_ids_per_query: 10 + has_many_aggregate :addresses, :count_all, :count, '*', max_ids_per_query: 10 end Contact.jit_preload.each do |contact| @@ -197,7 +197,7 @@ This is a method `preload_scoped_relation` that is available that can handle thi #old class Contact < ActiveRecord::Base has_many :addresses - has_many :usa_addresses, ->{ where(country: Country.find_by_name("USA")) } + has_many :usa_addresses, -> { where(country: Country.find_by_name('USA')) } end Contact.jit_preload.all.each do |contact| @@ -205,18 +205,18 @@ Contact.jit_preload.all.each do |contact| contact.usa_addresses # This will preload as the entire addresses association, and filters it in memory - contact.addresses.select{|address| address.country == Country.find_by_name("USA") } + contact.addresses.select { |address| address.country == Country.find_by_name('USA') } # This is an N+1 query - contact.addresses.where(country: Country.find_by_name("USA")) + contact.addresses.where(country: Country.find_by_name('USA')) end # New Contact.jit_preload.all.each do |contact| contact.preload_scoped_relation( - name: "USA Addresses", + name: 'USA Addresses', base_association: :addresses, - preload_scope: Address.where(country: Country.find_by_name("USA")) + preload_scope: Address.where(country: Country.find_by_name('USA')) ) end # SELECT * FROM contacts @@ -236,14 +236,14 @@ JitPreloader.globally_enabled = true # Can also be given anything that responds to `call`. # You could build a kill switch with Redis (or whatever you'd like) # so that you can turn it on or off dynamically. -JitPreloader.globally_enabled = ->{ $redis.get('always_jit_preload') == 'on' } +JitPreloader.globally_enabled = -> { $redis.get('always_jit_preload') == 'on' } # Setting global max ids constraint on all aggregation methods. JitPreloader.max_ids_per_query = 10 class Contact < ActiveRecord::Base has_many :emails - has_many_aggregate :emails, :count_all, :count, "*" + has_many_aggregate :emails, :count_all, :count, '*' end # When enabled globally, this would not generate an N+1 query. diff --git a/Rakefile b/Rakefile index 809eb56..7398a90 100644 --- a/Rakefile +++ b/Rakefile @@ -1,2 +1,3 @@ -require "bundler/gem_tasks" +# frozen_string_literal: true +require 'bundler/gem_tasks' diff --git a/jit_preloader.gemspec b/jit_preloader.gemspec index fb5875a..b485e7c 100644 --- a/jit_preloader.gemspec +++ b/jit_preloader.gemspec @@ -1,34 +1,38 @@ # coding: utf-8 +# frozen_string_literal: true + lib = File.expand_path('../lib', __FILE__) $LOAD_PATH.unshift(lib) unless $LOAD_PATH.include?(lib) require 'jit_preloader/version' Gem::Specification.new do |spec| - spec.name = "jit_preloader" + spec.name = 'jit_preloader' spec.version = JitPreloader::VERSION - spec.authors = ["Kyle d'Oliveira"] - spec.email = ["kyle.doliveira@clio.com"] - spec.summary = %q{Tool to understand N+1 queries and to remove them} - spec.description = %q{The JitPreloader has the ability to send notifications when N+1 queries occur to help guage how problematic they are for your code base and a way to remove all of the commons explicitly or automatically} - spec.homepage = "https://github.com/clio/jit_preloader" - spec.metadata["homepage_uri"] = spec.homepage - spec.metadata["source_code_uri"] = spec.homepage + spec.authors = [ "Kyle d'Oliveira" ] + spec.email = [ 'kyle.doliveira@clio.com' ] + spec.summary = 'Tool to understand N+1 queries and to remove them' + spec.description = 'The JitPreloader has the ability to send notifications when N+1 queries occur to help guage how problematic they are for your code base and a way to remove all of the commons explicitly or automatically' + spec.homepage = 'https://github.com/clio/jit_preloader' + spec.metadata['homepage_uri'] = spec.homepage + spec.metadata['source_code_uri'] = spec.homepage + spec.required_ruby_version = '>= 3.0.0' - spec.license = "MIT" + spec.license = 'MIT' - spec.files = `git ls-files -z`.split("\x0") + spec.files = Dir.glob('lib/**/*.rb') + [ File.basename(__FILE__) ] spec.executables = spec.files.grep(%r{^bin/}) { |f| File.basename(f) } spec.test_files = spec.files.grep(%r{^(test|spec|features)/}) - spec.require_paths = ["lib"] + spec.require_paths = [ 'lib' ] - spec.add_dependency "activerecord", "< 8" - spec.add_dependency "activesupport" + spec.add_dependency 'activerecord', '< 8' + spec.add_dependency 'activesupport' - spec.add_development_dependency "bundler" - spec.add_development_dependency "rake", "~> 13.0" - spec.add_development_dependency "rspec" - spec.add_development_dependency "database_cleaner" - spec.add_development_dependency "sqlite3" - spec.add_development_dependency "byebug" - spec.add_development_dependency "db-query-matchers" + spec.add_development_dependency 'bundler' + spec.add_development_dependency 'rake', '~> 13.0' + spec.add_development_dependency 'rspec' + spec.add_development_dependency 'database_cleaner' + spec.add_development_dependency 'sqlite3' + spec.add_development_dependency 'rubocop-rails_config' + spec.add_development_dependency 'byebug' + spec.add_development_dependency 'db-query-matchers' end diff --git a/lib/jit_preloader.rb b/lib/jit_preloader.rb index 3efc2ab..6992add 100644 --- a/lib/jit_preloader.rb +++ b/lib/jit_preloader.rb @@ -1,17 +1,19 @@ +# frozen_string_literal: true + require 'active_support/concern' require 'active_support/core_ext/module/delegation' require 'active_support/notifications' require 'active_record' -require "jit_preloader/version" +require 'jit_preloader/version' require 'jit_preloader/active_record/base' require 'jit_preloader/active_record/relation' require 'jit_preloader/active_record/associations/collection_association' require 'jit_preloader/active_record/associations/singular_association' -if Gem::Version.new(ActiveRecord::VERSION::STRING) >= Gem::Version.new("7.0.0") +if Gem::Version.new(ActiveRecord::VERSION::STRING) >= Gem::Version.new('7.0.0') require 'jit_preloader/active_record/associations/preloader/ar7_association' require 'jit_preloader/active_record/associations/preloader/ar7_branch' -elsif Gem::Version.new(ActiveRecord::VERSION::STRING) >= Gem::Version.new("6.1.0") +elsif Gem::Version.new(ActiveRecord::VERSION::STRING) >= Gem::Version.new('6.1.0') require 'jit_preloader/active_record/associations/preloader/ar6_association' else require 'jit_preloader/active_record/associations/preloader/collection_association' @@ -41,5 +43,4 @@ def self.globally_enabled? @enabled end end - end diff --git a/lib/jit_preloader/active_record/associations/collection_association.rb b/lib/jit_preloader/active_record/associations/collection_association.rb index 940c9ea..d65303b 100644 --- a/lib/jit_preloader/active_record/associations/collection_association.rb +++ b/lib/jit_preloader/active_record/associations/collection_association.rb @@ -1,6 +1,7 @@ +# frozen_string_literal: true + module JitPreloader module ActiveRecordAssociationsCollectionAssociation - def load_target was_loaded = loaded? @@ -17,10 +18,10 @@ def load_target JitPreloader::Preloader.attach(records) if records.any? && !jit_loaded && JitPreloader.globally_enabled? # If the records were not pre_loaded - records.each{ |record| record.jit_n_plus_one_tracking = true } + records.each { |record| record.jit_n_plus_one_tracking = true } if !jit_loaded && owner.jit_n_plus_one_tracking - ActiveSupport::Notifications.publish("n_plus_one_query", + ActiveSupport::Notifications.publish('n_plus_one_query', source: owner, association: reflection.name) end end diff --git a/lib/jit_preloader/active_record/associations/preloader/ar6_association.rb b/lib/jit_preloader/active_record/associations/preloader/ar6_association.rb index 6d953cb..97d7689 100644 --- a/lib/jit_preloader/active_record/associations/preloader/ar6_association.rb +++ b/lib/jit_preloader/active_record/associations/preloader/ar6_association.rb @@ -1,6 +1,7 @@ +# frozen_string_literal: true + module JitPreloader module PreloaderAssociation - # A monkey patch to ActiveRecord. The old method looked like the snippet # below. Our changes here are that we remove records that are already # part of the target, then attach all of the records to a new jit preloader. diff --git a/lib/jit_preloader/active_record/associations/preloader/ar7_association.rb b/lib/jit_preloader/active_record/associations/preloader/ar7_association.rb index e603b7e..af8748b 100644 --- a/lib/jit_preloader/active_record/associations/preloader/ar7_association.rb +++ b/lib/jit_preloader/active_record/associations/preloader/ar7_association.rb @@ -1,6 +1,7 @@ +# frozen_string_literal: true + module JitPreloader module PreloaderAssociation - # A monkey patch to ActiveRecord. The old method looked like the snippet # below. Our changes here are that we remove records that are already # part of the target, then attach all of the records to a new jit preloader. diff --git a/lib/jit_preloader/active_record/associations/preloader/ar7_branch.rb b/lib/jit_preloader/active_record/associations/preloader/ar7_branch.rb index 7960a77..e06032d 100644 --- a/lib/jit_preloader/active_record/associations/preloader/ar7_branch.rb +++ b/lib/jit_preloader/active_record/associations/preloader/ar7_branch.rb @@ -1,6 +1,8 @@ +# frozen_string_literal: true + module JitPreloader module PreloaderBranch - """ + ''" ActiveRecord version >= 7.x.x introduced an improvement for preloading associations in batches: https://github.com/rails/rails/blob/main/activerecord/lib/active_record/associations/preloader.rb#L121 @@ -9,7 +11,7 @@ module PreloaderBranch But this change breaks that behaviour because now Batch is calling `klass.base_class` (a method defined by ActiveRecord::Base) before we have a chance to filter out the non-AR classes. This patch for AR 7.x makes the Branch class ignore any association loaders that aren't for ActiveRecord::Base subclasses. - """ + "'' def loaders @loaders = super.find_all do |loader| diff --git a/lib/jit_preloader/active_record/associations/preloader/collection_association.rb b/lib/jit_preloader/active_record/associations/preloader/collection_association.rb index 38f2843..0ceeb56 100644 --- a/lib/jit_preloader/active_record/associations/preloader/collection_association.rb +++ b/lib/jit_preloader/active_record/associations/preloader/collection_association.rb @@ -1,34 +1,42 @@ -class ActiveRecord::Associations::Preloader::CollectionAssociation - private - # A monkey patch to ActiveRecord. The old method looked like the snippet - # below. Our changes here are that we remove records that are already - # part of the target, then attach all of the records to a new jit preloader. - # - # def preload(preloader) - # associated_records_by_owner(preloader).each do |owner, records| - # association = owner.association(reflection.name) - # association.loaded! - # association.target.concat(records) - # records.each { |record| association.set_inverse_instance(record) } - # end - # end +# frozen_string_literal: true - def preload(preloader) - return unless (reflection.scope.nil? || reflection.scope.arity == 0) && klass.ancestors.include?(ActiveRecord::Base) - all_records = [] - associated_records_by_owner(preloader).each do |owner, records| - association = owner.association(reflection.name) - association.loaded! - # It is possible that some of the records are loaded already. - # We don't want to duplicate them, but we also want to preserve - # the original copy so that we don't blow away in-memory changes. - new_records = association.target.any? ? records - association.target : records +module ActiveRecord + module Associations + module Preloader + class CollectionAssociation + private + # A monkey patch to ActiveRecord. The old method looked like the snippet + # below. Our changes here are that we remove records that are already + # part of the target, then attach all of the records to a new jit preloader. + # + # def preload(preloader) + # associated_records_by_owner(preloader).each do |owner, records| + # association = owner.association(reflection.name) + # association.loaded! + # association.target.concat(records) + # records.each { |record| association.set_inverse_instance(record) } + # end + # end - association.target.concat(new_records) - new_records.each { |record| association.set_inverse_instance(record) } + def preload(preloader) + return unless (reflection.scope.nil? || reflection.scope.arity == 0) && klass.ancestors.include?(ActiveRecord::Base) + all_records = [] + associated_records_by_owner(preloader).each do |owner, records| + association = owner.association(reflection.name) + association.loaded! + # It is possible that some of the records are loaded already. + # We don't want to duplicate them, but we also want to preserve + # the original copy so that we don't blow away in-memory changes. + new_records = association.target.any? ? records - association.target : records - all_records.concat(records) if owner.jit_preloader || JitPreloader.globally_enabled? + association.target.concat(new_records) + new_records.each { |record| association.set_inverse_instance(record) } + + all_records.concat(records) if owner.jit_preloader || JitPreloader.globally_enabled? + end + JitPreloader::Preloader.attach(all_records) if all_records.any? + end + end end - JitPreloader::Preloader.attach(all_records) if all_records.any? end end diff --git a/lib/jit_preloader/active_record/associations/preloader/singular_association.rb b/lib/jit_preloader/active_record/associations/preloader/singular_association.rb index 353d79b..aa3c107 100644 --- a/lib/jit_preloader/active_record/associations/preloader/singular_association.rb +++ b/lib/jit_preloader/active_record/associations/preloader/singular_association.rb @@ -1,29 +1,37 @@ -class ActiveRecord::Associations::Preloader::SingularAssociation - private - # A monkey patch to ActiveRecord. The old method looked like the snippet - # below. Our changes here are that we don't assign the record if the - # target has already been set, and we attach all of the records to a new - # jit preloader. - # - # def preload(preloader) - # associated_records_by_owner(preloader).each do |owner, associated_records| - # record = associated_records.first - # association = owner.association(reflection.name) - # association.target = record - # end - # end +# frozen_string_literal: true - def preload(preloader) - return unless (reflection.scope.nil? || reflection.scope.arity == 0) && klass.ancestors.include?(ActiveRecord::Base) - all_records = [] +module ActiveRecord + module Associations + module Preloader + class SingularAssociation + private + # A monkey patch to ActiveRecord. The old method looked like the snippet + # below. Our changes here are that we don't assign the record if the + # target has already been set, and we attach all of the records to a new + # jit preloader. + # + # def preload(preloader) + # associated_records_by_owner(preloader).each do |owner, associated_records| + # record = associated_records.first + # association = owner.association(reflection.name) + # association.target = record + # end + # end - associated_records_by_owner(preloader).each do |owner, associated_records| - record = associated_records.first + def preload(preloader) + return unless (reflection.scope.nil? || reflection.scope.arity == 0) && klass.ancestors.include?(ActiveRecord::Base) + all_records = [] - association = owner.association(reflection.name) - association.target ||= record - all_records.push(record) if record && (owner.jit_preloader || JitPreloader.globally_enabled?) + associated_records_by_owner(preloader).each do |owner, associated_records| + record = associated_records.first + + association = owner.association(reflection.name) + association.target ||= record + all_records.push(record) if record && (owner.jit_preloader || JitPreloader.globally_enabled?) + end + JitPreloader::Preloader.attach(all_records) if all_records.any? + end + end end - JitPreloader::Preloader.attach(all_records) if all_records.any? end end diff --git a/lib/jit_preloader/active_record/associations/singular_association.rb b/lib/jit_preloader/active_record/associations/singular_association.rb index 6bcecf0..907984d 100644 --- a/lib/jit_preloader/active_record/associations/singular_association.rb +++ b/lib/jit_preloader/active_record/associations/singular_association.rb @@ -1,6 +1,7 @@ +# frozen_string_literal: true + module JitPreloader module ActiveRecordAssociationsSingularAssociation - def load_target was_loaded = loaded? @@ -19,16 +20,16 @@ def load_target record.jit_n_plus_one_tracking ||= owner.jit_n_plus_one_tracking if record if !jit_loaded && owner.jit_n_plus_one_tracking && !is_polymorphic_association_without_type - ActiveSupport::Notifications.publish("n_plus_one_query", + ActiveSupport::Notifications.publish('n_plus_one_query', source: owner, association: reflection.name) end end end end - + private def is_polymorphic_association_without_type - self.is_a?(ActiveRecord::Associations::BelongsToPolymorphicAssociation) && self.klass.nil? - end + self.is_a?(ActiveRecord::Associations::BelongsToPolymorphicAssociation) && self.klass.nil? + end end end diff --git a/lib/jit_preloader/active_record/base.rb b/lib/jit_preloader/active_record/base.rb index c9ea1b1..4518aea 100644 --- a/lib/jit_preloader/active_record/base.rb +++ b/lib/jit_preloader/active_record/base.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + module JitPreloadExtension attr_accessor :jit_preloader attr_accessor :jit_n_plus_one_tracking @@ -18,12 +20,12 @@ def clear_jit_preloader! end end - if Gem::Version.new(ActiveRecord::VERSION::STRING) >= Gem::Version.new("7.0.0") + if Gem::Version.new(ActiveRecord::VERSION::STRING) >= Gem::Version.new('7.0.0') def preload_scoped_relation(name:, base_association:, preload_scope: nil) return jit_preload_scoped_relations[name] if jit_preload_scoped_relations&.key?(name) base_association = base_association.to_sym - records = jit_preloader&.records || [self] + records = jit_preloader&.records || [ self ] previous_association_values = {} records.each do |record| @@ -35,7 +37,7 @@ def preload_scoped_relation(name:, base_association:, preload_scope: nil) end preloader_association = ActiveRecord::Associations::Preloader.new( - records: records, + records:, associations: base_association, scope: preload_scope ).call.first @@ -57,7 +59,7 @@ def preload_scoped_relation(name:, base_association:, preload_scope: nil) return jit_preload_scoped_relations[name] if jit_preload_scoped_relations&.key?(name) base_association = base_association.to_sym - records = jit_preloader&.records || [self] + records = jit_preloader&.records || [ self ] previous_association_values = {} records.each do |record| @@ -95,19 +97,19 @@ class << base def has_many_aggregate(assoc, name, aggregate, field, table_alias_name: nil, default: 0, max_ids_per_query: nil) method_name = "#{assoc}_#{name}" - define_method(method_name) do |conditions={}| + define_method(method_name) do |conditions = {}| self.jit_preload_aggregates ||= {} key = "#{method_name}|#{conditions.sort.hash}" return jit_preload_aggregates[key] if jit_preload_aggregates.key?(key) if jit_preloader reflection = association(assoc).reflection - primary_ids = jit_preloader.records.collect{|r| r[reflection.active_record_primary_key] } + primary_ids = jit_preloader.records.collect { |r| r[reflection.active_record_primary_key] } max_ids_per_query = max_ids_per_query || JitPreloader.max_ids_per_query if max_ids_per_query slices = primary_ids.each_slice(max_ids_per_query) else - slices = [primary_ids] + slices = [ primary_ids ] end klass = reflection.klass diff --git a/lib/jit_preloader/active_record/relation.rb b/lib/jit_preloader/active_record/relation.rb index 0c210cf..f036743 100644 --- a/lib/jit_preloader/active_record/relation.rb +++ b/lib/jit_preloader/active_record/relation.rb @@ -1,6 +1,7 @@ +# frozen_string_literal: true + module JitPreloader module ActiveRecordRelation - def jit_preload(*args) spawn.jit_preload!(*args) end @@ -16,7 +17,7 @@ def jit_preload? def calculate(*args) if respond_to?(:proxy_association) && proxy_association.owner && proxy_association.owner.jit_n_plus_one_tracking - ActiveSupport::Notifications.publish("n_plus_one_query", + ActiveSupport::Notifications.publish('n_plus_one_query', source: proxy_association.owner, association: "#{proxy_association.reflection.name}.#{args.first}") end @@ -27,14 +28,13 @@ def calculate(*args) def exec_queries super.tap do |records| if limit_value != 1 - records.each{ |record| record.jit_n_plus_one_tracking = true } + records.each { |record| record.jit_n_plus_one_tracking = true } if jit_preload? || JitPreloader.globally_enabled? JitPreloader::Preloader.attach(records) end end end end - end end diff --git a/lib/jit_preloader/preloader.rb b/lib/jit_preloader/preloader.rb index d8c9c5d..9bc8d11 100644 --- a/lib/jit_preloader/preloader.rb +++ b/lib/jit_preloader/preloader.rb @@ -1,9 +1,10 @@ +# frozen_string_literal: true + module JitPreloader class Preloader < ActiveRecord::Associations::Preloader - attr_accessor :records - if Gem::Version.new(ActiveRecord::VERSION::STRING) >= Gem::Version.new("7.0.0") + if Gem::Version.new(ActiveRecord::VERSION::STRING) >= Gem::Version.new('7.0.0') def self.attach(records) new(records: records.dup, associations: nil).tap do |loader| records.each do |record| @@ -15,12 +16,12 @@ def self.attach(records) def jit_preload(associations) # It is possible that the records array has multiple different classes (think single table inheritance). # Thus, it is possible that some of the records don't have an association. - records_with_association = records.reject{|r| r.class.reflect_on_association(associations).nil? } + records_with_association = records.reject { |r| r.class.reflect_on_association(associations).nil? } # Some of the records may already have the association loaded and we should not load them again - records_requiring_loading = records_with_association.select{|r| !r.association(associations).loaded? } + records_requiring_loading = records_with_association.select { |r| !r.association(associations).loaded? } - self.class.new(records: records_requiring_loading, associations: associations).call + self.class.new(records: records_requiring_loading, associations:).call end else def self.attach(records) @@ -35,10 +36,10 @@ def self.attach(records) def jit_preload(associations) # It is possible that the records array has multiple different classes (think single table inheritance). # Thus, it is possible that some of the records don't have an association. - records_with_association = records.reject{ |record| record.class.reflect_on_association(associations).nil? } + records_with_association = records.reject { |record| record.class.reflect_on_association(associations).nil? } # Some of the records may already have the association loaded and we should not load them again - records_requiring_loading = records_with_association.select{ |record| !record.association(associations).loaded? } + records_with_association.select { |record| !record.association(associations).loaded? } preload records_with_association, associations end end @@ -51,12 +52,11 @@ def jit_preload(associations) # each object would dump N+1 objects which means you'll end up storing O(N^2) memory. Thats no good. # So instead, we will just nullify the jit_preloader on load def _dump(level) - "" + '' end def self._load(args) nil end - end end diff --git a/lib/jit_preloader/version.rb b/lib/jit_preloader/version.rb index f201357..9fcd164 100644 --- a/lib/jit_preloader/version.rb +++ b/lib/jit_preloader/version.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + module JitPreloader - VERSION = "3.1.0" + VERSION = '3.1.0' end diff --git a/spec/lib/jit_preloader/active_record/base_spec.rb b/spec/lib/jit_preloader/active_record/base_spec.rb index 7e47afa..3dba016 100644 --- a/spec/lib/jit_preloader/active_record/base_spec.rb +++ b/spec/lib/jit_preloader/active_record/base_spec.rb @@ -1,41 +1,42 @@ -require 'spec_helper' -require "db-query-matchers" +# frozen_string_literal: true -RSpec.describe "ActiveRecord::Base Extensions" do +require 'spec_helper' +require 'db-query-matchers' - let(:canada) { Country.create(name: "Canada") } - let(:usa) { Country.create(name: "U.S.A") } +RSpec.describe 'ActiveRecord::Base Extensions' do + let(:canada) { Country.create(name: 'Canada') } + let(:usa) { Country.create(name: 'U.S.A') } - describe "#preload_scoped_relation" do + describe '#preload_scoped_relation' do def call(contact) contact.preload_scoped_relation( - name: "American Addresses", + name: 'American Addresses', base_association: :addresses, preload_scope: Address.where(country: usa) ) end before do - Contact.create(name: "Bar", addresses: [ - Address.new(street: "123 Fake st", country: canada), - Address.new(street: "21 Jump st", country: usa), - Address.new(street: "90210 Beverly Hills", country: usa) + Contact.create(name: 'Bar', addresses: [ + Address.new(street: '123 Fake st', country: canada), + Address.new(street: '21 Jump st', country: usa), + Address.new(street: '90210 Beverly Hills', country: usa) ]) - Contact.create(name: "Foo", addresses: [ - Address.new(street: "1 First st", country: canada), - Address.new(street: "10 Tenth Ave", country: usa) + Contact.create(name: 'Foo', addresses: [ + Address.new(street: '1 First st', country: canada), + Address.new(street: '10 Tenth Ave', country: usa) ]) end - context "when operating on a single object" do - it "will load the objects for that object" do + context 'when operating on a single object' do + it 'will load the objects for that object' do contact = Contact.first expect(call(contact)).to match_array contact.addresses.where(country: usa).to_a end end - it "memoizes the result" do + it 'memoizes the result' do contacts = Contact.jit_preload.limit(2).to_a expect do @@ -44,8 +45,8 @@ def call(contact) end.to make_database_queries(count: 1) end - context "when reloading the object" do - it "clears the memoization" do + context 'when reloading the object' do + it 'clears the memoization' do contacts = Contact.jit_preload.limit(2).to_a expect do @@ -58,7 +59,7 @@ def call(contact) end end - it "will issue one query for the group of objects" do + it 'will issue one query for the group of objects' do contacts = Contact.jit_preload.limit(2).to_a usa_addresses = contacts.first.addresses.where(country: usa).to_a @@ -83,23 +84,23 @@ def call(contact) expect(contacts.last.association(:addresses)).to_not be_loaded end - context "when the association is already loaded" do + context 'when the association is already loaded' do it "doesn't change the value of the association" do contacts = Contact.jit_preload.limit(2).to_a - contacts.each{|contact| contact.addresses.to_a } - contacts.each{|contact| call(contact) } + contacts.each { |contact| contact.addresses.to_a } + contacts.each { |contact| call(contact) } expect(contacts.first.association(:addresses)).to be_loaded expect(contacts.last.association(:addresses)).to be_loaded end end - context "when no records exist for the association" do + context 'when no records exist for the association' do let!(:record) { Parent.create } - it "returns an empty array" do + it 'returns an empty array' do value = record.preload_scoped_relation( - name: "Parent Children", + name: 'Parent Children', base_association: :parents_child ) diff --git a/spec/lib/jit_preloader/preloader_spec.rb b/spec/lib/jit_preloader/preloader_spec.rb index 92e5c11..a193af5 100644 --- a/spec/lib/jit_preloader/preloader_spec.rb +++ b/spec/lib/jit_preloader/preloader_spec.rb @@ -1,147 +1,148 @@ -require "spec_helper" -require "db-query-matchers" +# frozen_string_literal: true + +require 'spec_helper' +require 'db-query-matchers' RSpec.describe JitPreloader::Preloader do let!(:contact1) do addresses = [ - Address.new(street: "123 Fake st", country: canada), - Address.new(street: "21 Jump st", country: usa), - Address.new(street: "90210 Beverly Hills", country: usa) + Address.new(street: '123 Fake st', country: canada), + Address.new(street: '21 Jump st', country: usa), + Address.new(street: '90210 Beverly Hills', country: usa) ] phones = [ - PhoneNumber.new(phone: "4445556666"), - PhoneNumber.new(phone: "2223333444") + PhoneNumber.new(phone: '4445556666'), + PhoneNumber.new(phone: '2223333444') ] - Contact.create(name: "Only Addresses", addresses: addresses, phone_numbers: phones) + Contact.create(name: 'Only Addresses', addresses:, phone_numbers: phones) end let!(:contact2) do - Contact.create(name: "Only Emails", email_address: EmailAddress.new(address: "woot@woot.com")) + Contact.create(name: 'Only Emails', email_address: EmailAddress.new(address: 'woot@woot.com')) end let!(:contact3) do addresses = [ - Address.new(street: "1 First st", country: canada), - Address.new(street: "10 Tenth Ave", country: usa) + Address.new(street: '1 First st', country: canada), + Address.new(street: '10 Tenth Ave', country: usa) ] Contact.create( - name: "Both!", - addresses: addresses, - email_address: EmailAddress.new(address: "woot@woot.com"), - phone_numbers: [PhoneNumber.new(phone: "1234567890")] + name: 'Both!', + addresses:, + email_address: EmailAddress.new(address: 'woot@woot.com'), + phone_numbers: [ PhoneNumber.new(phone: '1234567890') ] ) end let!(:contact_owner) do contact3.contact_owner_id = contact1.id - contact3.contact_owner_type = "Address" + contact3.contact_owner_type = 'Address' contact3.save! ContactOwner.create( - contacts: [contact1, contact2], + contacts: [ contact1, contact2 ], ) end - let(:canada) { Country.create(name: "Canada") } - let(:usa) { Country.create(name: "U.S.A") } + let(:canada) { Country.create(name: 'Canada') } + let(:usa) { Country.create(name: 'U.S.A') } - let(:source_map) { Hash.new{|h,k| h[k]= Array.new } } + let(:source_map) { Hash.new { |h, k| h[k] = Array.new } } let(:callback) do - ->(event, data){ source_map[data[:source]] << data[:association] } + ->(event, data) { source_map[data[:source]] << data[:association] } end - context "for single table inheritance" do - context "when preloading an aggregate for a child model" do - let!(:contact_book) { ContactBook.create(name: "The Yellow Pages") } - let!(:company1) { Company.create(name: "Company1", contact_book: contact_book) } - let!(:company2) { Company.create(name: "Company2", contact_book: contact_book) } + context 'for single table inheritance' do + context 'when preloading an aggregate for a child model' do + let!(:contact_book) { ContactBook.create(name: 'The Yellow Pages') } + let!(:company1) { Company.create(name: 'Company1', contact_book:) } + let!(:company2) { Company.create(name: 'Company2', contact_book:) } - it "can handle queries" do + it 'can handle queries' do contact_books = ContactBook.jit_preload.to_a expect(contact_books.first.companies_count).to eq 2 end end - context "when preloading an aggregate of a child model through its base model" do - let!(:contact_book) { ContactBook.create(name: "The Yellow Pages") } - let!(:contact) { Contact.create(name: "Contact", contact_book: contact_book) } - let!(:company1) { Company.create(name: "Company1", contact_book: contact_book) } - let!(:company2) { Company.create(name: "Company2", contact_book: contact_book) } - let!(:contact_employee1) { Employee.create(name: "Contact Employee1", contact: contact) } - let!(:contact_employee2) { Employee.create(name: "Contact Employee2", contact: contact) } - let!(:company_employee1) { Employee.create(name: "Company Employee1", contact: company1) } - let!(:company_employee2) { Employee.create(name: "Company Employee2", contact: company2) } - - it "can handle queries" do + context 'when preloading an aggregate of a child model through its base model' do + let!(:contact_book) { ContactBook.create(name: 'The Yellow Pages') } + let!(:contact) { Contact.create(name: 'Contact', contact_book:) } + let!(:company1) { Company.create(name: 'Company1', contact_book:) } + let!(:company2) { Company.create(name: 'Company2', contact_book:) } + let!(:contact_employee1) { Employee.create(name: 'Contact Employee1', contact:) } + let!(:contact_employee2) { Employee.create(name: 'Contact Employee2', contact:) } + let!(:company_employee1) { Employee.create(name: 'Company Employee1', contact: company1) } + let!(:company_employee2) { Employee.create(name: 'Company Employee2', contact: company2) } + + it 'can handle queries' do contact_books = ContactBook.jit_preload.to_a expect(contact_books.first.employees_count).to eq 4 end end - context "when preloading an aggregate of a nested child model through another child model" do - let!(:contact_book) { ContactBook.create(name: "The Yellow Pages") } - let!(:contact) { Contact.create(name: "Contact", contact_book: contact_book) } - let!(:company1) { Company.create(name: "Company1", contact_book: contact_book) } - let!(:company2) { Company.create(name: "Company2", contact_book: contact_book) } - let!(:contact_employee1) { Employee.create(name: "Contact Employee1", contact: contact) } - let!(:contact_employee2) { Employee.create(name: "Contact Employee2", contact: contact) } - let!(:company_employee1) { Employee.create(name: "Company Employee1", contact: company1) } - let!(:company_employee2) { Employee.create(name: "Company Employee2", contact: company2) } - - it "can handle queries" do + context 'when preloading an aggregate of a nested child model through another child model' do + let!(:contact_book) { ContactBook.create(name: 'The Yellow Pages') } + let!(:contact) { Contact.create(name: 'Contact', contact_book:) } + let!(:company1) { Company.create(name: 'Company1', contact_book:) } + let!(:company2) { Company.create(name: 'Company2', contact_book:) } + let!(:contact_employee1) { Employee.create(name: 'Contact Employee1', contact:) } + let!(:contact_employee2) { Employee.create(name: 'Contact Employee2', contact:) } + let!(:company_employee1) { Employee.create(name: 'Company Employee1', contact: company1) } + let!(:company_employee2) { Employee.create(name: 'Company Employee2', contact: company2) } + + it 'can handle queries' do contact_books = ContactBook.jit_preload.to_a expect(contact_books.first.company_employees_count).to eq 2 end end - context "when preloading an aggregate of a nested child model through a many-to-many relationship with another child model" do - let!(:contact_book) { ContactBook.create(name: "The Yellow Pages") } - let!(:child1) { Child.create(name: "Child1") } - let!(:child2) { Child.create(name: "Child2") } - let!(:child3) { Child.create(name: "Child3") } - let!(:parent1) { Parent.create(name: "Parent1", contact_book: contact_book, children: [child1, child2]) } - let!(:parent2) { Parent.create(name: "Parent2", contact_book: contact_book, children: [child2, child3]) } + context 'when preloading an aggregate of a nested child model through a many-to-many relationship with another child model' do + let!(:contact_book) { ContactBook.create(name: 'The Yellow Pages') } + let!(:child1) { Child.create(name: 'Child1') } + let!(:child2) { Child.create(name: 'Child2') } + let!(:child3) { Child.create(name: 'Child3') } + let!(:parent1) { Parent.create(name: 'Parent1', contact_book:, children: [ child1, child2 ]) } + let!(:parent2) { Parent.create(name: 'Parent2', contact_book:, children: [ child2, child3 ]) } - it "can handle queries" do + it 'can handle queries' do contact_books = ContactBook.jit_preload.to_a expect(contact_books.first.children_count).to eq 4 expect(contact_books.first.children).to include(child1, child2, child3) end end - context "when preloading an aggregate for a child model scoped by another join table" do - let!(:contact_book) { ContactBook.create(name: "The Yellow Pages") } - let!(:contact1) { Company.create(name: "Without Email", contact_book: contact_book) } - let!(:contact2) { Company.create(name: "With Blank Email", email_address: EmailAddress.new(address: ""), contact_book: contact_book) } - let!(:contact3) { Company.create(name: "With Email", email_address: EmailAddress.new(address: "a@a.com"), contact_book: contact_book) } + context 'when preloading an aggregate for a child model scoped by another join table' do + let!(:contact_book) { ContactBook.create(name: 'The Yellow Pages') } + let!(:contact1) { Company.create(name: 'Without Email', contact_book:) } + let!(:contact2) { Company.create(name: 'With Blank Email', email_address: EmailAddress.new(address: ''), contact_book:) } + let!(:contact3) { Company.create(name: 'With Email', email_address: EmailAddress.new(address: 'a@a.com'), contact_book:) } - it "can handle queries" do + it 'can handle queries' do contact_books = ContactBook.jit_preload.to_a expect(contact_books.first.companies_with_blank_email_address_count).to eq 1 - expect(contact_books.first.companies_with_blank_email_address).to eq [contact2] + expect(contact_books.first.companies_with_blank_email_address).to eq [ contact2 ] end end end - context "when preloading an aggregate as polymorphic" do - let(:contact_owner_counts) { [2] } + context 'when preloading an aggregate as polymorphic' do + let(:contact_owner_counts) { [ 2 ] } - context "without jit preload" do - it "generates N+1 query notifications for each one" do - ActiveSupport::Notifications.subscribed(callback, "n_plus_one_query") do + context 'without jit preload' do + it 'generates N+1 query notifications for each one' do + ActiveSupport::Notifications.subscribed(callback, 'n_plus_one_query') do ContactOwner.all.each_with_index do |c, i| expect(c.contacts_count).to eql contact_owner_counts[i] end end - contact_owner_queries = [contact_owner].product([["contacts.count"]]) + contact_owner_queries = [ contact_owner ].product([ [ 'contacts.count' ] ]) expect(source_map).to eql(Hash[contact_owner_queries]) end end - context "with jit_preload" do - - it "does NOT generate N+1 query notifications" do - ActiveSupport::Notifications.subscribed(callback, "n_plus_one_query") do + context 'with jit_preload' do + it 'does NOT generate N+1 query notifications' do + ActiveSupport::Notifications.subscribed(callback, 'n_plus_one_query') do ContactOwner.jit_preload.each_with_index do |c, i| expect(c.contacts_count).to eql contact_owner_counts[i] end @@ -150,7 +151,7 @@ expect(source_map).to eql({}) end - it "can handle queries" do + it 'can handle queries' do ContactOwner.jit_preload.each_with_index do |c, i| expect(c.contacts_count).to eql contact_owner_counts[i] end @@ -158,22 +159,22 @@ context "when a record has a polymorphic association type that's not an ActiveRecord" do before do - contact1.update!(contact_owner_type: "NilClass", contact_owner_id: nil) + contact1.update!(contact_owner_type: 'NilClass', contact_owner_id: nil) end it "doesn't die while trying to load the association" do - expect(Contact.jit_preload.map(&:contact_owner)).to eq [nil, ContactOwner.first, Address.first] + expect(Contact.jit_preload.map(&:contact_owner)).to eq [ nil, ContactOwner.first, Address.first ] end end - context "when a record has a polymorphic association type is nil" do + context 'when a record has a polymorphic association type is nil' do before do contact1.update!(contact_owner_type: nil, contact_owner_id: nil) end - it "successfully load the rest of association values and does not publish a n+1 notification" do + it 'successfully load the rest of association values and does not publish a n+1 notification' do contacts = Contact.jit_preload.to_a - ActiveSupport::Notifications.subscribed(callback, "n_plus_one_query") do + ActiveSupport::Notifications.subscribed(callback, 'n_plus_one_query') do expect(contacts.first.contact_owner).to eq(nil) end @@ -192,25 +193,25 @@ end end - context "when preloading an aggregate on a has_many through relationship" do - let(:country_contacts_counts) { [2, 3] } + context 'when preloading an aggregate on a has_many through relationship' do + let(:country_contacts_counts) { [ 2, 3 ] } - context "without jit preload" do - it "generates N+1 query notifications for each one" do - ActiveSupport::Notifications.subscribed(callback, "n_plus_one_query") do + context 'without jit preload' do + it 'generates N+1 query notifications for each one' do + ActiveSupport::Notifications.subscribed(callback, 'n_plus_one_query') do Country.all.each_with_index do |c, i| expect(c.contacts_count).to eql country_contacts_counts[i] end end - country_contact_queries = [canada, usa].product([["contacts.count"]]) + country_contact_queries = [ canada, usa ].product([ [ 'contacts.count' ] ]) expect(source_map).to eql(Hash[country_contact_queries]) end end - context "with jit_preload" do - it "does NOT generate N+1 query notifications" do - ActiveSupport::Notifications.subscribed(callback, "n_plus_one_query") do + context 'with jit_preload' do + it 'does NOT generate N+1 query notifications' do + ActiveSupport::Notifications.subscribed(callback, 'n_plus_one_query') do Country.all.jit_preload.each_with_index do |c, i| expect(c.contacts_count).to eql country_contacts_counts[i] end @@ -219,7 +220,7 @@ expect(source_map).to eql({}) end - it "can handle queries" do + it 'can handle queries' do Country.all.jit_preload.each_with_index do |c, i| expect(c.contacts_count).to eql country_contacts_counts[i] end @@ -227,41 +228,41 @@ end end - context "when accessing an association with a scope that has a parameter" do - let!(:contact_book) { ContactBook.create(name: "The Yellow Pages") } - let!(:contact) { Contact.create(name: "Contact", contact_book: contact_book) } - let!(:company1) { Company.create(name: "Company1", contact_book: contact_book) } + context 'when accessing an association with a scope that has a parameter' do + let!(:contact_book) { ContactBook.create(name: 'The Yellow Pages') } + let!(:contact) { Contact.create(name: 'Contact', contact_book:) } + let!(:company1) { Company.create(name: 'Company1', contact_book:) } - it "is unable to be preloaded" do - ActiveSupport::Notifications.subscribed(callback, "n_plus_one_query") do + it 'is unable to be preloaded' do + ActiveSupport::Notifications.subscribed(callback, 'n_plus_one_query') do ContactBook.all.jit_preload.each do |contact_book| - expect(contact_book.contacts_with_scope.to_a).to eql [company1, contact] + expect(contact_book.contacts_with_scope.to_a).to eql [ company1, contact ] end end - expect(source_map).to eql(Hash[contact_book, [:contacts_with_scope]]) + expect(source_map).to eql(Hash[contact_book, [ :contacts_with_scope ]]) end end - context "when preloading an aggregate on a polymorphic has_many through relationship" do - let(:contact_owner_addresses_counts) { [3] } + context 'when preloading an aggregate on a polymorphic has_many through relationship' do + let(:contact_owner_addresses_counts) { [ 3 ] } - context "without jit preload" do - it "generates N+1 query notifications for each one" do - ActiveSupport::Notifications.subscribed(callback, "n_plus_one_query") do + context 'without jit preload' do + it 'generates N+1 query notifications for each one' do + ActiveSupport::Notifications.subscribed(callback, 'n_plus_one_query') do ContactOwner.all.each_with_index do |c, i| expect(c.addresses_count).to eql contact_owner_addresses_counts[i] end end - contact_owner_addresses_queries = [contact_owner].product([["addresses.count"]]) + contact_owner_addresses_queries = [ contact_owner ].product([ [ 'addresses.count' ] ]) expect(source_map).to eql(Hash[contact_owner_addresses_queries]) end end - context "with jit_preload" do - it "does NOT generate N+1 query notifications" do - ActiveSupport::Notifications.subscribed(callback, "n_plus_one_query") do + context 'with jit_preload' do + it 'does NOT generate N+1 query notifications' do + ActiveSupport::Notifications.subscribed(callback, 'n_plus_one_query') do ContactOwner.all.jit_preload.each_with_index do |c, i| expect(c.addresses_count).to eql contact_owner_addresses_counts[i] end @@ -270,7 +271,7 @@ expect(source_map).to eql({}) end - it "can handle queries" do + it 'can handle queries' do ContactOwner.all.jit_preload.each_with_index do |c, i| expect(c.addresses_count).to eql contact_owner_addresses_counts[i] end @@ -278,25 +279,25 @@ end end - context "when preloading a has_many through polymorphic aggregate where the through class has a polymorphic relationship to the target class" do - let(:contact_owner_counts) { [1, 2] } + context 'when preloading a has_many through polymorphic aggregate where the through class has a polymorphic relationship to the target class' do + let(:contact_owner_counts) { [ 1, 2 ] } - context "without jit preload" do - it "generates N+1 query notifications for each one" do - ActiveSupport::Notifications.subscribed(callback, "n_plus_one_query") do + context 'without jit preload' do + it 'generates N+1 query notifications for each one' do + ActiveSupport::Notifications.subscribed(callback, 'n_plus_one_query') do Country.all.each_with_index do |c, i| expect(c.contact_owners_count).to eql contact_owner_counts[i] end end - contact_owner_queries = [canada, usa].product([["contact_owners.count"]]) + contact_owner_queries = [ canada, usa ].product([ [ 'contact_owners.count' ] ]) expect(source_map).to eql(Hash[contact_owner_queries]) end end - context "with jit_preload" do - it "does NOT generate N+1 query notifications" do - ActiveSupport::Notifications.subscribed(callback, "n_plus_one_query") do + context 'with jit_preload' do + it 'does NOT generate N+1 query notifications' do + ActiveSupport::Notifications.subscribed(callback, 'n_plus_one_query') do Country.all.jit_preload.each_with_index do |c, i| expect(c.contact_owners_count).to eql contact_owner_counts[i] end @@ -305,7 +306,7 @@ expect(source_map).to eql({}) end - it "can handle queries" do + it 'can handle queries' do Country.all.jit_preload.each_with_index do |c, i| expect(c.contact_owners_count).to eql contact_owner_counts[i] end @@ -313,30 +314,30 @@ end end - context "when a singular association id changes after preload" do - let!(:contact_book1) { ContactBook.create(name: "The Yellow Pages") } - let!(:contact_book2) { ContactBook.create(name: "The White Pages") } - let!(:company1) { Company.create(name: "Company1", contact_book: contact_book1) } - let!(:company2) { Company.create(name: "Company2", contact_book: contact_book1) } + context 'when a singular association id changes after preload' do + let!(:contact_book1) { ContactBook.create(name: 'The Yellow Pages') } + let!(:contact_book2) { ContactBook.create(name: 'The White Pages') } + let!(:company1) { Company.create(name: 'Company1', contact_book: contact_book1) } + let!(:company2) { Company.create(name: 'Company2', contact_book: contact_book1) } - it "allows the association to be reloaded" do - companies = Company.where(id: [company1.id, company2.id]).jit_preload.all.to_a - expect(companies.map(&:contact_book)).to match_array [contact_book1, contact_book1] + it 'allows the association to be reloaded' do + companies = Company.where(id: [ company1.id, company2.id ]).jit_preload.all.to_a + expect(companies.map(&:contact_book)).to match_array [ contact_book1, contact_book1 ] - company = companies.each {|c| c.contact_book_id = contact_book2.id } + companies.each { |c| c.contact_book_id = contact_book2.id } - expect(companies.map(&:contact_book)).to match_array [contact_book2, contact_book2] + expect(companies.map(&:contact_book)).to match_array [ contact_book2, contact_book2 ] end end - context "when preloading an aggregate" do - let(:addresses_counts) { [3, 0, 2] } - let(:phone_number_counts) { [2, 0, 1] } - let(:maxes) { [19, 0, 12] } + context 'when preloading an aggregate' do + let(:addresses_counts) { [ 3, 0, 2 ] } + let(:phone_number_counts) { [ 2, 0, 1 ] } + let(:maxes) { [ 19, 0, 12 ] } - context "without jit_preload" do - it "generates N+1 query notifications for each one" do - ActiveSupport::Notifications.subscribed(callback, "n_plus_one_query") do + context 'without jit_preload' do + it 'generates N+1 query notifications for each one' do + ActiveSupport::Notifications.subscribed(callback, 'n_plus_one_query') do Contact.all.each_with_index do |c, i| expect(c.addresses_count).to eql addresses_counts[i] expect(c.addresses_max_street_length).to eql maxes[i] @@ -344,17 +345,17 @@ end end - contact_queries = [contact1, contact2, contact3].product([["addresses.count", "addresses.maximum", "phone_numbers.count"]]) + contact_queries = [ contact1, contact2, contact3 ].product([ [ 'addresses.count', 'addresses.maximum', 'phone_numbers.count' ] ]) expect(source_map).to eql(Hash[contact_queries]) end end - context "with jit_preload" do - let(:usa_addresses_counts) { [2, 0, 1] } - let(:can_addresses_counts) { [1, 0, 1] } + context 'with jit_preload' do + let(:usa_addresses_counts) { [ 2, 0, 1 ] } + let(:can_addresses_counts) { [ 1, 0, 1 ] } - it "does NOT generate N+1 query notifications" do - ActiveSupport::Notifications.subscribed(callback, "n_plus_one_query") do + it 'does NOT generate N+1 query notifications' do + ActiveSupport::Notifications.subscribed(callback, 'n_plus_one_query') do Contact.jit_preload.each_with_index do |c, i| expect(c.addresses_count).to eql addresses_counts[i] expect(c.addresses_max_street_length).to eql maxes[i] @@ -365,7 +366,7 @@ expect(source_map).to eql({}) end - it "can handle dynamic queries" do + it 'can handle dynamic queries' do Contact.jit_preload.each_with_index do |c, i| expect(c.addresses_count(country: usa)).to eql usa_addresses_counts[i] expect(c.addresses_count(country: canada)).to eql can_addresses_counts[i] @@ -374,10 +375,10 @@ end end - context "when we marshal dump the active record object" do - it "nullifes the jit_preloader reference" do + context 'when we marshal dump the active record object' do + it 'nullifes the jit_preloader reference' do contacts = Contact.jit_preload.to_a - reloaded_contacts = contacts.collect{|r| Marshal.load(Marshal.dump(r)) } + reloaded_contacts = contacts.collect { |r| Marshal.load(Marshal.dump(r)) } contacts.each do |c| expect(c.jit_preloader).to_not be_nil end @@ -387,7 +388,7 @@ end end - context "when the preloader is globally enabled" do + context 'when the preloader is globally enabled' do around do |example| JitPreloader.globally_enabled = true example.run @@ -395,127 +396,127 @@ end it "doesn't reference the same records array that is returned" do contacts = Contact.all.to_a - contacts << "A string" + contacts << 'A string' expect(contacts.first.jit_preloader.records).to eql Contact.all.to_a end context "when grabbing all of the address'es contries and email addresses" do it "doesn't generate an N+1 query ntoification" do - ActiveSupport::Notifications.subscribed(callback, "n_plus_one_query") do - Contact.all.collect{|c| c.addresses.collect(&:country); c.email_address } + ActiveSupport::Notifications.subscribed(callback, 'n_plus_one_query') do + Contact.all.collect { |c| c.addresses.collect(&:country); c.email_address } end expect(source_map).to eql({}) end end - context "when we perform aggregate functions on the data" do - it "generates N+1 query notifications for each one" do - ActiveSupport::Notifications.subscribed(callback, "n_plus_one_query") do - Contact.all.each{|c| c.addresses.count; c.addresses.sum(:id) } + context 'when we perform aggregate functions on the data' do + it 'generates N+1 query notifications for each one' do + ActiveSupport::Notifications.subscribed(callback, 'n_plus_one_query') do + Contact.all.each { |c| c.addresses.count; c.addresses.sum(:id) } end - contact_queries = [contact1,contact2, contact3].product([["addresses.count", "addresses.sum"]]) + contact_queries = [ contact1, contact2, contact3 ].product([ [ 'addresses.count', 'addresses.sum' ] ]) expect(source_map).to eql(Hash[contact_queries]) end end end - context "when the preloader is not globally enabled" do - context "when we perform aggregate functions on the data" do - it "generates N+1 query notifications for each one" do - ActiveSupport::Notifications.subscribed(callback, "n_plus_one_query") do - Contact.all.each{|c| c.addresses.count; c.addresses.sum(:id) } + context 'when the preloader is not globally enabled' do + context 'when we perform aggregate functions on the data' do + it 'generates N+1 query notifications for each one' do + ActiveSupport::Notifications.subscribed(callback, 'n_plus_one_query') do + Contact.all.each { |c| c.addresses.count; c.addresses.sum(:id) } end - contact_queries = [contact1,contact2, contact3].product([["addresses.count", "addresses.sum"]]) + contact_queries = [ contact1, contact2, contact3 ].product([ [ 'addresses.count', 'addresses.sum' ] ]) expect(source_map).to eql(Hash[contact_queries]) end end - context "when explicitly finding a contact" do - it "generates N+1 query notifications for the country" do - ActiveSupport::Notifications.subscribed(callback, "n_plus_one_query") do - Contact.find(contact1.id).tap{|c| c.addresses.collect(&:country); c.email_address } + context 'when explicitly finding a contact' do + it 'generates N+1 query notifications for the country' do + ActiveSupport::Notifications.subscribed(callback, 'n_plus_one_query') do + Contact.find(contact1.id).tap { |c| c.addresses.collect(&:country); c.email_address } end - address_queries = Address.where(contact_id: 1).to_a.product([[:country]]) + address_queries = Address.where(contact_id: 1).to_a.product([ [ :country ] ]) expect(source_map).to eql(Hash[address_queries]) end end - context "when explicitly finding multiple contacts" do - it "generates N+1 query notifications for the country" do - ActiveSupport::Notifications.subscribed(callback, "n_plus_one_query") do - Contact.find(contact1.id, contact2.id).each{|c| c.addresses.collect(&:country); c.email_address } + context 'when explicitly finding multiple contacts' do + it 'generates N+1 query notifications for the country' do + ActiveSupport::Notifications.subscribed(callback, 'n_plus_one_query') do + Contact.find(contact1.id, contact2.id).each { |c| c.addresses.collect(&:country); c.email_address } end - contact_queries = [contact1,contact2].product([[:addresses, :email_address]]) - address_queries = Address.where(contact_id: contact1.id).to_a.product([[:country]]) + contact_queries = [ contact1, contact2 ].product([ [ :addresses, :email_address ] ]) + address_queries = Address.where(contact_id: contact1.id).to_a.product([ [ :country ] ]) expect(source_map).to eql(Hash[address_queries.concat(contact_queries)]) end end context "when grabbing the email address and address's country of the first contact" do - it "generates N+1 query notifications for the country" do - ActiveSupport::Notifications.subscribed(callback, "n_plus_one_query") do - Contact.first.tap{|c| c.addresses.collect(&:country); c.email_address } + it 'generates N+1 query notifications for the country' do + ActiveSupport::Notifications.subscribed(callback, 'n_plus_one_query') do + Contact.first.tap { |c| c.addresses.collect(&:country); c.email_address } end - address_queries = Address.where(contact_id: contact1.id).to_a.product([[:country]]) + address_queries = Address.where(contact_id: contact1.id).to_a.product([ [ :country ] ]) expect(source_map).to eql(Hash[address_queries]) end end context "when grabbing all of the address'es contries and email addresses" do - it "generates an N+1 query for each association on the contacts" do - ActiveSupport::Notifications.subscribed(callback, "n_plus_one_query") do - Contact.all.each{|c| c.addresses.collect(&:country); c.email_address } + it 'generates an N+1 query for each association on the contacts' do + ActiveSupport::Notifications.subscribed(callback, 'n_plus_one_query') do + Contact.all.each { |c| c.addresses.collect(&:country); c.email_address } end - contact_queries = [contact1,contact2,contact3].product([[:addresses, :email_address]]) - address_queries = Address.all.to_a.product([[:country]]) + contact_queries = [ contact1, contact2, contact3 ].product([ [ :addresses, :email_address ] ]) + address_queries = Address.all.to_a.product([ [ :country ] ]) expect(source_map).to eql(Hash[address_queries.concat(contact_queries)]) end - context "and we use regular preload for addresses" do - it "generates an N+1 query for only the email addresses on the contacts" do - ActiveSupport::Notifications.subscribed(callback, "n_plus_one_query") do - Contact.preload(:addresses).each{|c| c.addresses.collect(&:country); c.email_address } + context 'and we use regular preload for addresses' do + it 'generates an N+1 query for only the email addresses on the contacts' do + ActiveSupport::Notifications.subscribed(callback, 'n_plus_one_query') do + Contact.preload(:addresses).each { |c| c.addresses.collect(&:country); c.email_address } end - contact_queries = [contact1,contact2,contact3].product([[:email_address]]) - address_queries = Address.all.to_a.product([[:country]]) + contact_queries = [ contact1, contact2, contact3 ].product([ [ :email_address ] ]) + address_queries = Address.all.to_a.product([ [ :country ] ]) expect(source_map).to eql(Hash[address_queries.concat(contact_queries)]) end end - context "and we use jit preload" do - it "generates no n+1 queries" do - ActiveSupport::Notifications.subscribed(callback, "n_plus_one_query") do - Contact.jit_preload.each{|c| c.addresses.collect(&:country); c.email_address } + context 'and we use jit preload' do + it 'generates no n+1 queries' do + ActiveSupport::Notifications.subscribed(callback, 'n_plus_one_query') do + Contact.jit_preload.each { |c| c.addresses.collect(&:country); c.email_address } end expect(source_map).to eql({}) end end - context "reload" do - it "clears the jit_preload_aggregates" do + context 'reload' do + it 'clears the jit_preload_aggregates' do contact = Contact.jit_preload.first contact.addresses_count - expect { contact.reload }.to change{ contact.jit_preload_aggregates }.to({}) + expect { contact.reload }.to change { contact.jit_preload_aggregates }.to({}) end end end - context "with dive limit set" do - let!(:contact_book_1) { ContactBook.create(name: "The Yellow Pages") } - let!(:contact_book_2) { ContactBook.create(name: "The Yellow Pages") } - let!(:contact_book_3) { ContactBook.create(name: "The Yellow Pages") } - let!(:company1) { Company.create(name: "Company1", contact_book: contact_book_1) } - let!(:company2) { Company.create(name: "Company2", contact_book: contact_book_1) } - let!(:company3) { Company.create(name: "Company2", contact_book: contact_book_2) } - let!(:company4) { Company.create(name: "Company4", contact_book: contact_book_3) } - let!(:company5) { Company.create(name: "Company5", contact_book: contact_book_3) } - - context "from the global value" do + context 'with dive limit set' do + let!(:contact_book_1) { ContactBook.create(name: 'The Yellow Pages') } + let!(:contact_book_2) { ContactBook.create(name: 'The Yellow Pages') } + let!(:contact_book_3) { ContactBook.create(name: 'The Yellow Pages') } + let!(:company1) { Company.create(name: 'Company1', contact_book: contact_book_1) } + let!(:company2) { Company.create(name: 'Company2', contact_book: contact_book_1) } + let!(:company3) { Company.create(name: 'Company2', contact_book: contact_book_2) } + let!(:company4) { Company.create(name: 'Company4', contact_book: contact_book_3) } + let!(:company5) { Company.create(name: 'Company5', contact_book: contact_book_3) } + + context 'from the global value' do before do JitPreloader.max_ids_per_query = 2 end @@ -524,7 +525,7 @@ JitPreloader.max_ids_per_query = nil end - it "can handle queries" do + it 'can handle queries' do contact_books = ContactBook.jit_preload.to_a expect(contact_books.first.companies_count).to eq 2 @@ -532,7 +533,7 @@ expect(contact_books.last.companies_count).to eq 2 end - it "makes the right number of queries based on dive limit" do + it 'makes the right number of queries based on dive limit' do contact_books = ContactBook.jit_preload.to_a expect do contact_books.first.companies_count @@ -545,8 +546,8 @@ end end - context "from aggregate argument" do - it "can handle queries" do + context 'from aggregate argument' do + it 'can handle queries' do contact_books = ContactBook.jit_preload.to_a expect(contact_books.first.companies_count_with_max_ids_set).to eq 2 @@ -554,7 +555,7 @@ expect(contact_books.last.companies_count_with_max_ids_set).to eq 2 end - it "makes the right number of queries based on dive limit" do + it 'makes the right number of queries based on dive limit' do contact_books = ContactBook.jit_preload.to_a expect do contact_books.first.companies_count_with_max_ids_set @@ -568,5 +569,4 @@ end end end - end diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 78c9872..1f96017 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + require 'bundler/setup' Bundler.setup diff --git a/spec/support/database.rb b/spec/support/database.rb index 7f09eb6..09cd487 100644 --- a/spec/support/database.rb +++ b/spec/support/database.rb @@ -1,14 +1,16 @@ +# frozen_string_literal: true + class Database def self.tables [ - "CREATE TABLE contact_books (id INTEGER NOT NULL PRIMARY KEY, name VARCHAR(255))", - "CREATE TABLE contacts (id INTEGER NOT NULL PRIMARY KEY, type VARCHAR(255), contact_book_id INTEGER, contact_id INTEGER, name VARCHAR(255), contact_owner_id INTEGER, contact_owner_type VARCHAR(255))", - "CREATE TABLE contact_owners (id INTEGER NOT NULL PRIMARY KEY, name VARCHAR(255))", - "CREATE TABLE addresses (id INTEGER NOT NULL PRIMARY KEY, contact_id INTEGER NOT NULL, country_id INTEGER NOT NULL, street VARCHAR(255))", - "CREATE TABLE email_addresses (id INTEGER NOT NULL PRIMARY KEY, contact_id INTEGER NOT NULL, address VARCHAR(255))", - "CREATE TABLE phone_numbers (id INTEGER NOT NULL PRIMARY KEY, contact_id INTEGER NOT NULL, phone VARCHAR(10))", - "CREATE TABLE countries (id INTEGER NOT NULL PRIMARY KEY, name VARCHAR(255))", - "CREATE TABLE parents_children (id INTEGER NOT NULL PRIMARY KEY, parent_id INTEGER, child_id INTEGER)", + 'CREATE TABLE contact_books (id INTEGER NOT NULL PRIMARY KEY, name VARCHAR(255))', + 'CREATE TABLE contacts (id INTEGER NOT NULL PRIMARY KEY, type VARCHAR(255), contact_book_id INTEGER, contact_id INTEGER, name VARCHAR(255), contact_owner_id INTEGER, contact_owner_type VARCHAR(255))', + 'CREATE TABLE contact_owners (id INTEGER NOT NULL PRIMARY KEY, name VARCHAR(255))', + 'CREATE TABLE addresses (id INTEGER NOT NULL PRIMARY KEY, contact_id INTEGER NOT NULL, country_id INTEGER NOT NULL, street VARCHAR(255))', + 'CREATE TABLE email_addresses (id INTEGER NOT NULL PRIMARY KEY, contact_id INTEGER NOT NULL, address VARCHAR(255))', + 'CREATE TABLE phone_numbers (id INTEGER NOT NULL PRIMARY KEY, contact_id INTEGER NOT NULL, phone VARCHAR(10))', + 'CREATE TABLE countries (id INTEGER NOT NULL PRIMARY KEY, name VARCHAR(255))', + 'CREATE TABLE parents_children (id INTEGER NOT NULL PRIMARY KEY, parent_id INTEGER, child_id INTEGER)' ] end @@ -19,7 +21,6 @@ def self.build! end def self.connect! - ActiveRecord::Base.establish_connection adapter: 'sqlite3', database: ":memory:" + ActiveRecord::Base.establish_connection adapter: 'sqlite3', database: ':memory:' end - end diff --git a/spec/support/models.rb b/spec/support/models.rb index 05f132b..ad25505 100644 --- a/spec/support/models.rb +++ b/spec/support/models.rb @@ -1,6 +1,8 @@ +# frozen_string_literal: true + class ContactBook < ActiveRecord::Base has_many :contacts - has_many :contacts_with_scope, ->(_contact_book) { desc }, class_name: "Contact", foreign_key: :contact_book_id + has_many :contacts_with_scope, ->(_contact_book) { desc }, class_name: 'Contact', foreign_key: :contact_book_id has_many :employees, through: :contacts has_many :companies @@ -9,14 +11,14 @@ class ContactBook < ActiveRecord::Base has_many :parents has_many :children, through: :parents - has_many_aggregate :companies, :count, :count, "*" - has_many_aggregate :companies, :count_with_max_ids_set, :count, "*", max_ids_per_query: 2 - has_many_aggregate :employees, :count, :count, "*" - has_many_aggregate :company_employees, :count, :count, "*" - has_many_aggregate :children, :count, :count, "*" + has_many_aggregate :companies, :count, :count, '*' + has_many_aggregate :companies, :count_with_max_ids_set, :count, '*', max_ids_per_query: 2 + has_many_aggregate :employees, :count, :count, '*' + has_many_aggregate :company_employees, :count, :count, '*' + has_many_aggregate :children, :count, :count, '*' - has_many :companies_with_blank_email_address, -> { joins(:email_address).where(email_addresses: { address: "" }) }, class_name: "Company" - has_many_aggregate :companies_with_blank_email_address, :count, :count, "*", table_alias_name: "contacts" + has_many :companies_with_blank_email_address, -> { joins(:email_address).where(email_addresses: { address: '' }) }, class_name: 'Company' + has_many_aggregate :companies_with_blank_email_address, :count, :count, '*', table_alias_name: 'contacts' end class Contact < ActiveRecord::Base @@ -28,11 +30,11 @@ class Contact < ActiveRecord::Base has_one :email_address has_many :employees - has_many_aggregate :addresses, :max_street_length, :maximum, "LENGTH(street)" - has_many_aggregate :phone_numbers, :count, :count, "id" - has_many_aggregate :addresses, :count, :count, "*" + has_many_aggregate :addresses, :max_street_length, :maximum, 'LENGTH(street)' + has_many_aggregate :phone_numbers, :count, :count, 'id' + has_many_aggregate :addresses, :count, :count, '*' - scope :desc, ->{ order(id: :desc) } + scope :desc, -> { order(id: :desc) } end class Company < Contact @@ -75,14 +77,14 @@ class Country < ActiveRecord::Base has_many :contacts, through: :addresses has_many :contact_owners, through: :contacts, source_type: 'ContactOwner' - has_many_aggregate :contacts, :count, :count, "*" - has_many_aggregate :contact_owners, :count, :count, "*" + has_many_aggregate :contacts, :count, :count, '*' + has_many_aggregate :contact_owners, :count, :count, '*' end class ContactOwner < ActiveRecord::Base has_many :contacts, as: :contact_owner has_many :addresses, through: :contacts - has_many_aggregate :contacts, :count, :count, "*" - has_many_aggregate :addresses, :count, :count, "*" + has_many_aggregate :contacts, :count, :count, '*' + has_many_aggregate :addresses, :count, :count, '*' end