From 24efc6acc7927200ac48441e6835338cfed51e92 Mon Sep 17 00:00:00 2001 From: Wendy Chen Date: Wed, 5 Jun 2024 12:46:51 -0400 Subject: [PATCH 1/3] enable warning on app, and rsepc --- .rspec | 1 + lib/jit_preloader.rb | 1 + 2 files changed, 2 insertions(+) diff --git a/.rspec b/.rspec index 83e16f8..4f8a7dd 100644 --- a/.rspec +++ b/.rspec @@ -1,2 +1,3 @@ --color +--warning --require spec_helper diff --git a/lib/jit_preloader.rb b/lib/jit_preloader.rb index 5301378..637201c 100644 --- a/lib/jit_preloader.rb +++ b/lib/jit_preloader.rb @@ -22,6 +22,7 @@ require 'jit_preloader/preloader' module JitPreloader + Warning[:deprecated] = true def self.globally_enabled=(value) @enabled = value end From 5d2621f459e7e50b4d2ef76178f25c4c3690132b Mon Sep 17 00:00:00 2001 From: Wendy Chen Date: Wed, 5 Jun 2024 13:00:06 -0400 Subject: [PATCH 2/3] add test scenario to make sure deprecated warning is triggered --- lib/jit_preloader/preloader.rb | 4 ++++ spec/lib/jit_preloader/preloader_spec.rb | 5 +++++ 2 files changed, 9 insertions(+) diff --git a/lib/jit_preloader/preloader.rb b/lib/jit_preloader/preloader.rb index d8c9c5d..e9b8035 100644 --- a/lib/jit_preloader/preloader.rb +++ b/lib/jit_preloader/preloader.rb @@ -3,6 +3,10 @@ class Preloader < ActiveRecord::Associations::Preloader attr_accessor :records + def foo(**kwargs) + kwargs + end + 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| diff --git a/spec/lib/jit_preloader/preloader_spec.rb b/spec/lib/jit_preloader/preloader_spec.rb index 92e5c11..f110704 100644 --- a/spec/lib/jit_preloader/preloader_spec.rb +++ b/spec/lib/jit_preloader/preloader_spec.rb @@ -49,6 +49,11 @@ ->(event, data){ source_map[data[:source]] << data[:association] } end + + it "should warn about keyword arguments" do + expect(described_class.new(records: nil, associations: nil).foo({a: 1})).to eq({a: 1}) + 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") } From 9fc828981ae3c7bfe63112d6cc15727ef28e7d6d Mon Sep 17 00:00:00 2001 From: Wendy Chen Date: Wed, 5 Jun 2024 13:05:20 -0400 Subject: [PATCH 3/3] remove warning and test; update github workflow --- .github/workflows/ci.yml | 4 +- .github/workflows/gem-push.yml | 4 +- .rspec | 1 - Gemfile | 1 + Gemfile.5.2 | 6 -- Gemfile.5.2.lock | 72 ------------------- lib/jit_preloader.rb | 5 +- .../associations/preloader/ar5_association.rb | 66 ----------------- lib/jit_preloader/preloader.rb | 4 -- lib/jit_preloader/version.rb | 2 +- spec/lib/jit_preloader/preloader_spec.rb | 5 -- 11 files changed, 6 insertions(+), 164 deletions(-) delete mode 100644 Gemfile.5.2 delete mode 100644 Gemfile.5.2.lock delete mode 100644 lib/jit_preloader/active_record/associations/preloader/ar5_association.rb diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 7882d6c..72ba2ac 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -14,8 +14,6 @@ jobs: matrix: gemfile: - Gemfile - - Gemfile.5.2 - - Gemfile.6.0 - Gemfile.6.1 env: BUNDLE_GEMFILE: ${{ matrix.gemfile }} @@ -24,7 +22,7 @@ jobs: - name: Set up Ruby ${{ matrix.ruby-version }} uses: ruby/setup-ruby@v1 with: - ruby-version: 2.7 + ruby-version: 3.1 - name: Install dependencies run: bundle install - name: Run tests diff --git a/.github/workflows/gem-push.yml b/.github/workflows/gem-push.yml index 44ea83e..6bdf189 100644 --- a/.github/workflows/gem-push.yml +++ b/.github/workflows/gem-push.yml @@ -13,10 +13,10 @@ jobs: steps: - uses: actions/checkout@v4 - - name: Set up Ruby 2.7 + - name: Set up Ruby 3.1 uses: ruby/setup-ruby@v1 with: - ruby-version: 2.7 + ruby-version: 3.1 - name: Publish to RubyGems env: diff --git a/.rspec b/.rspec index 4f8a7dd..83e16f8 100644 --- a/.rspec +++ b/.rspec @@ -1,3 +1,2 @@ --color ---warning --require spec_helper diff --git a/Gemfile b/Gemfile index 6aad51b..d8f73d7 100644 --- a/Gemfile +++ b/Gemfile @@ -1,5 +1,6 @@ source 'https://rubygems.org' gem "activerecord", ">=7" +gem "sqlite3", "~> 1.4" # Specify your gem's dependencies in jit_preloader.gemspec gemspec diff --git a/Gemfile.5.2 b/Gemfile.5.2 deleted file mode 100644 index a6b6f20..0000000 --- a/Gemfile.5.2 +++ /dev/null @@ -1,6 +0,0 @@ -source 'https://rubygems.org' - -gem "activerecord", "~>5.2" - -# Specify your gem's dependencies in jit_preloader.gemspec -gemspec diff --git a/Gemfile.5.2.lock b/Gemfile.5.2.lock deleted file mode 100644 index 3febf6b..0000000 --- a/Gemfile.5.2.lock +++ /dev/null @@ -1,72 +0,0 @@ -PATH - remote: . - specs: - jit_preloader (1.0.3) - activerecord (>= 5.2, < 7) - activesupport - -GEM - remote: https://rubygems.org/ - specs: - activemodel (5.2.6) - activesupport (= 5.2.6) - activerecord (5.2.6) - activemodel (= 5.2.6) - activesupport (= 5.2.6) - arel (>= 9.0) - activesupport (5.2.6) - concurrent-ruby (~> 1.0, >= 1.0.2) - i18n (>= 0.7, < 2) - minitest (~> 5.1) - tzinfo (~> 1.1) - arel (9.0.0) - byebug (11.1.3) - concurrent-ruby (1.1.9) - database_cleaner (2.0.1) - database_cleaner-active_record (~> 2.0.0) - database_cleaner-active_record (2.0.1) - activerecord (>= 5.a) - database_cleaner-core (~> 2.0.0) - database_cleaner-core (2.0.1) - db-query-matchers (0.10.0) - activesupport (>= 4.0, < 7) - rspec (~> 3.0) - diff-lcs (1.4.4) - i18n (1.8.10) - concurrent-ruby (~> 1.0) - minitest (5.14.4) - rake (13.0.6) - rspec (3.10.0) - rspec-core (~> 3.10.0) - rspec-expectations (~> 3.10.0) - rspec-mocks (~> 3.10.0) - rspec-core (3.10.1) - rspec-support (~> 3.10.0) - rspec-expectations (3.10.1) - diff-lcs (>= 1.2.0, < 2.0) - rspec-support (~> 3.10.0) - rspec-mocks (3.10.2) - diff-lcs (>= 1.2.0, < 2.0) - rspec-support (~> 3.10.0) - rspec-support (3.10.2) - sqlite3 (1.4.2) - thread_safe (0.3.6) - tzinfo (1.2.9) - thread_safe (~> 0.1) - -PLATFORMS - x86_64-darwin-19 - -DEPENDENCIES - activerecord (~> 5.2) - bundler - byebug - database_cleaner - db-query-matchers - jit_preloader! - rake (~> 13.0) - rspec - sqlite3 - -BUNDLED WITH - 2.2.12 diff --git a/lib/jit_preloader.rb b/lib/jit_preloader.rb index 637201c..3efc2ab 100644 --- a/lib/jit_preloader.rb +++ b/lib/jit_preloader.rb @@ -11,10 +11,8 @@ 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.0.0") +elsif Gem::Version.new(ActiveRecord::VERSION::STRING) >= Gem::Version.new("6.1.0") require 'jit_preloader/active_record/associations/preloader/ar6_association' -elsif Gem::Version.new(ActiveRecord::VERSION::STRING) >= Gem::Version.new("5.2.2") - require 'jit_preloader/active_record/associations/preloader/ar5_association' else require 'jit_preloader/active_record/associations/preloader/collection_association' require 'jit_preloader/active_record/associations/preloader/singular_association' @@ -22,7 +20,6 @@ require 'jit_preloader/preloader' module JitPreloader - Warning[:deprecated] = true def self.globally_enabled=(value) @enabled = value end diff --git a/lib/jit_preloader/active_record/associations/preloader/ar5_association.rb b/lib/jit_preloader/active_record/associations/preloader/ar5_association.rb deleted file mode 100644 index 385a76e..0000000 --- a/lib/jit_preloader/active_record/associations/preloader/ar5_association.rb +++ /dev/null @@ -1,66 +0,0 @@ -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. - # - # def run(preloader) - # records = load_records do |record| - # owner = owners_by_key[convert_key(record[association_key_name])] - # association = owner.association(reflection.name) - # association.set_inverse_instance(record) - # end - - # owners.each do |owner| - # associate_records_to_owner(owner, records[convert_key(owner[owner_key_name])] || []) - # end - # end - - def run(preloader) - return unless (reflection.scope.nil? || reflection.scope.arity == 0) && klass.ancestors.include?(ActiveRecord::Base) - - super.tap do - if preloaded_records.any? && preloaded_records.none?(&:jit_preloader) - JitPreloader::Preloader.attach(preloaded_records) if owners.any?(&:jit_preloader) || JitPreloader.globally_enabled? - end - end - end - - # Original method: - # def associate_records_to_owner(owner, records) - # association = owner.association(reflection.name) - # association.loaded! - # if reflection.collection? - # association.target.concat(records) - # else - # association.target = records.first unless records.empty? - # end - # end - def associate_records_to_owner(owner, records) - association = owner.association(reflection.name) - association.loaded! - - if reflection.collection? - # 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 - association.target.concat(new_records) - association.loaded! - else - association.target = records.first unless records.empty? - end - end - - - def build_scope - super.tap do |scope| - scope.jit_preload! if owners.any?(&:jit_preloader) || JitPreloader.globally_enabled? - end - end - end -end - -ActiveRecord::Associations::Preloader::Association.prepend(JitPreloader::PreloaderAssociation) -ActiveRecord::Associations::Preloader::ThroughAssociation.prepend(JitPreloader::PreloaderAssociation) diff --git a/lib/jit_preloader/preloader.rb b/lib/jit_preloader/preloader.rb index e9b8035..d8c9c5d 100644 --- a/lib/jit_preloader/preloader.rb +++ b/lib/jit_preloader/preloader.rb @@ -3,10 +3,6 @@ class Preloader < ActiveRecord::Associations::Preloader attr_accessor :records - def foo(**kwargs) - kwargs - end - 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| diff --git a/lib/jit_preloader/version.rb b/lib/jit_preloader/version.rb index 13b1637..ee6b908 100644 --- a/lib/jit_preloader/version.rb +++ b/lib/jit_preloader/version.rb @@ -1,3 +1,3 @@ module JitPreloader - VERSION = "2.1.0" + VERSION = "3.0.0" end diff --git a/spec/lib/jit_preloader/preloader_spec.rb b/spec/lib/jit_preloader/preloader_spec.rb index f110704..92e5c11 100644 --- a/spec/lib/jit_preloader/preloader_spec.rb +++ b/spec/lib/jit_preloader/preloader_spec.rb @@ -49,11 +49,6 @@ ->(event, data){ source_map[data[:source]] << data[:association] } end - - it "should warn about keyword arguments" do - expect(described_class.new(records: nil, associations: nil).foo({a: 1})).to eq({a: 1}) - 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") }