From 05955479187015585ccc38331e99b5c7d48c97f0 Mon Sep 17 00:00:00 2001 From: Joe Rafaniello Date: Tue, 16 Jan 2024 16:24:40 -0500 Subject: [PATCH 1/7] Also test with rails 7, find all the broken things --- .github/workflows/ci.yaml | 6 +++++- Gemfile | 10 +++++++++- spec/spec_helper.rb | 4 ++++ 3 files changed, 18 insertions(+), 2 deletions(-) diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index 4d3b7067..63127d24 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -15,6 +15,9 @@ jobs: - '2.7' - '3.0' - '3.1' + rails-version: + - '6.1' + - '7.0' services: postgres: image: postgres:13 @@ -39,6 +42,7 @@ jobs: PGPORT: 5432 PGUSER: postgres PGPASSWORD: password + TEST_RAILS_VERSION: ${{ matrix.rails-version }} # for the mysql cli (mysql, mysqladmin) MYSQL_HOST: 127.0.0.1 MYSQL_PWD: password @@ -64,7 +68,7 @@ jobs: DB: mysql2 run: bundle exec rake - name: Report code coverage - if: ${{ github.ref == 'refs/heads/master' && matrix.ruby-version == '3.1' }} + if: ${{ github.ref == 'refs/heads/master' && matrix.ruby-version == '3.1' && matrix.rails-version == '6.1' }} continue-on-error: true uses: paambaati/codeclimate-action@v8 env: diff --git a/Gemfile b/Gemfile index 7e28e19c..cf092a24 100644 --- a/Gemfile +++ b/Gemfile @@ -2,7 +2,15 @@ source "https://rubygems.org" -gem "activerecord", "~> 6.1.7" +minimum_version = + case ENV['TEST_RAILS_VERSION'] + when "7.0" + "~>7.0.8" + else + "~>6.1.4" + end + +gem "activerecord", minimum_version gem "mysql2" gem "pg" gem "sqlite3", "< 2" diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 5440eda2..3b13f215 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -41,3 +41,7 @@ end end end + +require "active_record" +puts +puts "\e[93mUsing ActiveRecord #{ActiveRecord.version}\e[0m" From 877352ab7b129b0f77069baf1baa2423dc56aa98 Mon Sep 17 00:00:00 2001 From: Joe Rafaniello Date: Thu, 18 Jan 2024 15:37:37 -0500 Subject: [PATCH 2/7] Use Rails 7 interface for the Preloader See rails commit: e3b9779cb701c63012bc1af007c71dc5a888d35a --- spec/virtual_includes_spec.rb | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/spec/virtual_includes_spec.rb b/spec/virtual_includes_spec.rb index a8b8afb0..c3f4564c 100644 --- a/spec/virtual_includes_spec.rb +++ b/spec/virtual_includes_spec.rb @@ -243,9 +243,7 @@ expect(books.last.author).to be_nil # the book just created does not have an author # the second time preloading throws an error - preloader = ActiveRecord::Associations::Preloader.new - preloader.preload(books, :author => :books) - + preloaded(books, :author => :books) expect(books.size).to be(4) end end @@ -548,8 +546,13 @@ end def preloaded(records, associations, preload_scope = nil) - preloader = ActiveRecord::Associations::Preloader.new - preloader.preload(records, associations, preload_scope) + if ActiveRecord::Associations::Preloader.instance_methods.include?(:preload) + preloader = ActiveRecord::Associations::Preloader.new + preloader.preload(records, associations, preload_scope) + else + # Rails 7+ interface, see rails commit: e3b9779cb701c63012bc1af007c71dc5a888d35a + ActiveRecord::Associations::Preloader.new(records: records, associations: associations, scope: preload_scope).call + end records end end From 1b20dc5f13d7679db5b879b51b21ee0f9a7fdc0a Mon Sep 17 00:00:00 2001 From: Keenan Brock Date: Tue, 30 Jul 2024 11:53:30 -0400 Subject: [PATCH 3/7] Match rails 6.1/7.0 interface... no need to bind a block --- lib/active_record/virtual_attributes/virtual_fields.rb | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/lib/active_record/virtual_attributes/virtual_fields.rb b/lib/active_record/virtual_attributes/virtual_fields.rb index 43a53bd8..2769cc03 100644 --- a/lib/active_record/virtual_attributes/virtual_fields.rb +++ b/lib/active_record/virtual_attributes/virtual_fields.rb @@ -270,9 +270,8 @@ def build_select(arel) end end - # from ActiveRecord::QueryMethods (rails 5.2 - 6.0) - # TODO: remove from rails 7.0 - def arel_column(field, &block) + # from ActiveRecord::QueryMethods (rails 5.2 - 7.0) + def arel_column(field) if virtual_attribute?(field) && (arel = table[field]) arel else From d4d2df56d9e4de0b75f2e8a3c6bf0139949e7d90 Mon Sep 17 00:00:00 2001 From: Keenan Brock Date: Tue, 30 Jul 2024 13:44:03 -0400 Subject: [PATCH 4/7] Bring over rails 7 grouped_records remove extra gook from grouped_records the "Fix includes" commit resolves any other issues that were handled in here --- .../virtual_attributes/virtual_fields.rb | 90 ++----------------- 1 file changed, 6 insertions(+), 84 deletions(-) diff --git a/lib/active_record/virtual_attributes/virtual_fields.rb b/lib/active_record/virtual_attributes/virtual_fields.rb index 2769cc03..25db89ad 100644 --- a/lib/active_record/virtual_attributes/virtual_fields.rb +++ b/lib/active_record/virtual_attributes/virtual_fields.rb @@ -151,97 +151,19 @@ class Base module Associations class Preloader - prepend(Module.new { - # preloader.rb active record 6.0 - # changed: - # since grouped_records can return a hash/array, we need to handle those 2 new cases - def preloaders_for_reflection(reflection, records, scope, polymorphic_parent) - case reflection - when Array - reflection.flat_map { |ref| preloaders_on(ref, records, scope, polymorphic_parent) } - when Hash - preloaders_on(reflection, records, scope, polymorphic_parent) - else - super(reflection, records, scope) - end - end - - # rubocop:disable Style/BlockDelimiters, Lint/AmbiguousBlockAssociation, Style/MethodCallWithArgsParentheses - # preloader.rb active record 6.0 - # changed: - # passing polymorphic around (and makes 5.2 more similar to 6.0) - def preloaders_for_hash(association, records, scope, polymorphic_parent) - association.flat_map { |parent, child| - grouped_records(parent, records, polymorphic_parent).flat_map do |reflection, reflection_records| - loaders = preloaders_for_reflection(reflection, reflection_records, scope, polymorphic_parent) - recs = loaders.flat_map(&:preloaded_records).uniq - child_polymorphic_parent = reflection && reflection.respond_to?(:options) && reflection.options[:polymorphic] - loaders.concat Array.wrap(child).flat_map { |assoc| - preloaders_on assoc, recs, scope, child_polymorphic_parent - } - loaders - end - } - end - - # preloader.rb active record 6.0 - # changed: - # passing polymorphic_parent to preloaders_for_reflection - def preloaders_for_one(association, records, scope, polymorphic_parent) - grouped_records(association, records, polymorphic_parent) - .flat_map do |reflection, reflection_records| - preloaders_for_reflection(reflection, reflection_records, scope, polymorphic_parent) - end - end - - # preloader.rb active record 6.0, 6.1 - def grouped_records(orig_association, records, polymorphic_parent) - h = {} - records.each do |record| - # The virtual_field lookup can return Symbol/Nil/Other (typically a Hash) - # so the case statement and the cases for Nil/Other are new - - # each class can resolve virtual_{attributes,includes} differently - association = record.class.replace_virtual_fields(orig_association) - # 1 line optimization for single element array: - association = association.first if association.kind_of?(Array) && association.size == 1 - - case association - when Symbol, String - reflection = record.class._reflect_on_association(association) - next if polymorphic_parent && !reflection || !record.association(association).klass - when nil - next - else # need parent (preloaders_for_{hash,one}) to handle this Array/Hash - reflection = association - end - (h[reflection] ||= []) << record - end - h - end - # rubocop:enable Style/BlockDelimiters, Lint/AmbiguousBlockAssociation, Style/MethodCallWithArgsParentheses - }) class Branch prepend(Module.new { + # from branched.rb 7.0 def grouped_records h = {} polymorphic_parent = !root? && parent.polymorphic? source_records.each do |record| - # each class can resolve virtual_{attributes,includes} differently - @association = record.class.replace_virtual_fields(association) - - # 1 line optimization for single element array: - @association = association.first if association.kind_of?(Array) # && association.size == 1 + # begin virtual_attributes changes + association = record.class.replace_virtual_fields(self.association) + # end virtual_attributes changes - case association - when Symbol, String - reflection = record.class._reflect_on_association(association) - next if polymorphic_parent && !reflection || !record.association(association).klass - when nil - next - else # need parent (preloaders_for_{hash,one}) to handle this Array/Hash - reflection = association - end + reflection = record.class._reflect_on_association(association) + next if polymorphic_parent && !reflection || !record.association(association).klass (h[reflection] ||= []) << record end h From ed0ed0cf4d127c2d494a5118a88633f9ea627b4b Mon Sep 17 00:00:00 2001 From: Keenan Brock Date: Mon, 29 Jul 2024 22:33:43 -0400 Subject: [PATCH 5/7] Fix includes The preloading components moved into Branch It also brought a bunch of variables into class level and put resolution into instances So our approach of returning a hash and resolving accordingly no longer worked. Since association was stored in the class, we needed to derive the values multiple times --- .../virtual_attributes/virtual_fields.rb | 46 +++++++++++++++++++ 1 file changed, 46 insertions(+) diff --git a/lib/active_record/virtual_attributes/virtual_fields.rb b/lib/active_record/virtual_attributes/virtual_fields.rb index 25db89ad..c2ecd6f9 100644 --- a/lib/active_record/virtual_attributes/virtual_fields.rb +++ b/lib/active_record/virtual_attributes/virtual_fields.rb @@ -151,6 +151,30 @@ class Base module Associations class Preloader + prepend(Module.new { + # preloader is called with virtual attributes - need to resolve + def call + # Possibly overkill since all records probably have the same class and associations + # use a cache so we only convert includes once per base class + assoc_cache = Hash.new { |h, klass| h[klass] = klass.replace_virtual_fields(associations) } + + # convert the includes with virtual attributes to includes with proper associations + records_by_assoc = records.group_by { |rec| assoc_cache[rec.class] } + # if these are the same includes, then do the preloader work + return super if records_by_assoc.size == 1 && records_by_assoc.keys.first == associations + + # for each of the associations, run a preloader + records_by_assoc.each do |klass_associations, klass_records| + next if klass_associations.blank? + + Array[klass_associations].each do |klass_association| # rubocop:disable Style/RedundantArrayConstructor + # this calls back into itself, but it will take the short circuit + Preloader.new(:records => klass_records, :associations => klass_association, :scope => scope).call + end + end + end + }) + class Branch prepend(Module.new { # from branched.rb 7.0 @@ -168,6 +192,28 @@ def grouped_records end h end + + # branched.rb 7.0 + def preloaders_for_reflection(reflection, reflection_records) + reflection_records.group_by do |record| + # begin virtual_attributes changes + needed_association = record.class.replace_virtual_fields(association) + # end virtual_attributes changes + + klass = record.association(needed_association).klass + + if reflection.scope && reflection.scope.arity != 0 + # For instance dependent scopes, the scope is potentially + # different for each record. To allow this we'll group each + # object separately into its own preloader + reflection_scope = reflection.join_scopes(klass.arel_table, klass.predicate_builder, klass, record).inject(&:merge!) + end + + [klass, reflection_scope] + end.map do |(rhs_klass, reflection_scope), rs| + preloader_for(reflection).new(rhs_klass, rs, reflection, scope, reflection_scope, associate_by_default) + end + end }) end if ActiveRecord.version >= Gem::Version.new(7.0) end From 20b6106dc31928bfdb2e2c0a4b460038ecf63639 Mon Sep 17 00:00:00 2001 From: Keenan Brock Date: Mon, 29 Jul 2024 22:25:46 -0400 Subject: [PATCH 6/7] Require Rails 7.0 --- .github/workflows/ci.yaml | 6 +----- Gemfile | 10 +--------- .../virtual_attributes/version.rb | 2 +- .../virtual_attributes/virtual_fields.rb | 20 ++++++------------- spec/virtual_includes_spec.rb | 9 ++------- 5 files changed, 11 insertions(+), 36 deletions(-) diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index 63127d24..4d3b7067 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -15,9 +15,6 @@ jobs: - '2.7' - '3.0' - '3.1' - rails-version: - - '6.1' - - '7.0' services: postgres: image: postgres:13 @@ -42,7 +39,6 @@ jobs: PGPORT: 5432 PGUSER: postgres PGPASSWORD: password - TEST_RAILS_VERSION: ${{ matrix.rails-version }} # for the mysql cli (mysql, mysqladmin) MYSQL_HOST: 127.0.0.1 MYSQL_PWD: password @@ -68,7 +64,7 @@ jobs: DB: mysql2 run: bundle exec rake - name: Report code coverage - if: ${{ github.ref == 'refs/heads/master' && matrix.ruby-version == '3.1' && matrix.rails-version == '6.1' }} + if: ${{ github.ref == 'refs/heads/master' && matrix.ruby-version == '3.1' }} continue-on-error: true uses: paambaati/codeclimate-action@v8 env: diff --git a/Gemfile b/Gemfile index cf092a24..49ec9d52 100644 --- a/Gemfile +++ b/Gemfile @@ -2,15 +2,7 @@ source "https://rubygems.org" -minimum_version = - case ENV['TEST_RAILS_VERSION'] - when "7.0" - "~>7.0.8" - else - "~>6.1.4" - end - -gem "activerecord", minimum_version +gem "activerecord", "~>7.0.8" gem "mysql2" gem "pg" gem "sqlite3", "< 2" diff --git a/lib/active_record/virtual_attributes/version.rb b/lib/active_record/virtual_attributes/version.rb index 159ca698..67274c35 100644 --- a/lib/active_record/virtual_attributes/version.rb +++ b/lib/active_record/virtual_attributes/version.rb @@ -1,5 +1,5 @@ module ActiveRecord module VirtualAttributes - VERSION = "6.1.2".freeze + VERSION = "7.0.0".freeze end end diff --git a/lib/active_record/virtual_attributes/virtual_fields.rb b/lib/active_record/virtual_attributes/virtual_fields.rb index c2ecd6f9..4a6cf5f7 100644 --- a/lib/active_record/virtual_attributes/virtual_fields.rb +++ b/lib/active_record/virtual_attributes/virtual_fields.rb @@ -124,20 +124,12 @@ def assert_klass_has_instance_method(klass, instance_method) raise NameError, msg end -if ActiveRecord.version >= Gem::Version.new(7.0) # Rails 7.0 expected methods to patch - %w[ - grouped_records - ].each { |method| assert_klass_has_instance_method(ActiveRecord::Associations::Preloader::Branch, method) } -elsif ActiveRecord.version >= Gem::Version.new(6.1) # Rails 6.1 methods to patch - %w[ - preloaders_for_reflection - preloaders_for_hash - preloaders_for_one - grouped_records - ].each { |method| assert_klass_has_instance_method(ActiveRecord::Associations::Preloader, method) } -end +# Expect these methods to exist. (Otherwise we are patching the wrong methods) +%w[ + grouped_records + preloaders_for_reflection +].each { |method| assert_klass_has_instance_method(ActiveRecord::Associations::Preloader::Branch, method) } -# Expected methods to patch on any version %w[ build_select arel_column @@ -215,7 +207,7 @@ def preloaders_for_reflection(reflection, reflection_records) end end }) - end if ActiveRecord.version >= Gem::Version.new(7.0) + end end end diff --git a/spec/virtual_includes_spec.rb b/spec/virtual_includes_spec.rb index c3f4564c..6fff3246 100644 --- a/spec/virtual_includes_spec.rb +++ b/spec/virtual_includes_spec.rb @@ -546,13 +546,8 @@ end def preloaded(records, associations, preload_scope = nil) - if ActiveRecord::Associations::Preloader.instance_methods.include?(:preload) - preloader = ActiveRecord::Associations::Preloader.new - preloader.preload(records, associations, preload_scope) - else - # Rails 7+ interface, see rails commit: e3b9779cb701c63012bc1af007c71dc5a888d35a - ActiveRecord::Associations::Preloader.new(records: records, associations: associations, scope: preload_scope).call - end + # Rails 7+ interface, see rails commit: e3b9779cb701c63012bc1af007c71dc5a888d35a + ActiveRecord::Associations::Preloader.new(:records => records, :associations => associations, :scope => preload_scope).call records end end From 2905c1cc6bc2543d4e27ceebcb4ddefcef9b60e1 Mon Sep 17 00:00:00 2001 From: Keenan Brock Date: Mon, 29 Jul 2024 22:43:41 -0400 Subject: [PATCH 7/7] version 7.0.0 changelog --- CHANGELOG.md | 16 +++++++++++++++- activerecord-virtual_attributes.gemspec | 2 +- 2 files changed, 16 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index fc1462dc..2f499167 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,19 @@ The versioning of this gem follows ActiveRecord versioning, and does not follow ## [Unreleased] +## [7.0.0] - 2024-08-01 + +* Use Arel.literal [#154](https://github.com/ManageIQ/activerecord-virtual_attributes/pull/154) +* drop attribute_builder [#153](https://github.com/ManageIQ/activerecord-virtual_attributes/pull/153) +* dropped virtual_aggregate [#152](https://github.com/ManageIQ/activerecord-virtual_attributes/pull/152) +* resolve rubocops (also fix bin/console) [#151](https://github.com/ManageIQ/activerecord-virtual_attributes/pull/151) +* Rails 7.0 support / dropping 6.1 [#150](https://github.com/ManageIQ/activerecord-virtual_attributes/pull/150) +* condense includes produced by replace_virtual_fields [#149](https://github.com/ManageIQ/activerecord-virtual_attributes/pull/149) +* fix bin/console [#148](https://github.com/ManageIQ/activerecord-virtual_attributes/pull/148) +* Rails 7.0 support pt1 [#146](https://github.com/ManageIQ/activerecord-virtual_attributes/pull/146) +* Fix sqlite3 v2 and rails [#140](https://github.com/ManageIQ/activerecord-virtual_attributes/pull/140) +* Use custom Arel node [#114](https://github.com/ManageIQ/activerecord-virtual_attributes/pull/114) + ## [6.1.2] - 2023-10-26 * Fix bind variables for joins with static strings [#124](https://github.com/ManageIQ/activerecord-virtual_attributes/pull/124) @@ -93,7 +106,8 @@ The versioning of this gem follows ActiveRecord versioning, and does not follow * Initial Release * Extracted from ManageIQ/manageiq -[Unreleased]: https://github.com/ManageIQ/activerecord-virtual_attributes/compare/v6.1.2...HEAD +[Unreleased]: https://github.com/ManageIQ/activerecord-virtual_attributes/compare/v7.0.0...HEAD +[7.0.0]: https://github.com/ManageIQ/activerecord-virtual_attributes/compare/v6.1.2...v7.0.0 [6.1.2]: https://github.com/ManageIQ/activerecord-virtual_attributes/compare/v6.1.1...v6.1.2 [6.1.1]: https://github.com/ManageIQ/activerecord-virtual_attributes/compare/v6.1.0...v6.1.1 [6.1.0]: https://github.com/ManageIQ/activerecord-virtual_attributes/compare/v3.0.0...v6.1.0 diff --git a/activerecord-virtual_attributes.gemspec b/activerecord-virtual_attributes.gemspec index 5f9afa7d..738e0059 100644 --- a/activerecord-virtual_attributes.gemspec +++ b/activerecord-virtual_attributes.gemspec @@ -28,7 +28,7 @@ Gem::Specification.new do |spec| spec.require_paths = ["lib"] - spec.add_runtime_dependency "activerecord", ">=6.1.7.8", "<7.1" + spec.add_runtime_dependency "activerecord", "~> 7.0" spec.add_development_dependency "byebug" spec.add_development_dependency "database_cleaner-active_record", "~> 2.1"