Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[WIP] Support Rails 7.0 #133

Closed
wants to merge 8 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion .github/workflows/ci.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,9 @@ jobs:
- '2.7'
- '3.0'
- '3.1'
rails-version:
- '6.1'
- '7.0'
services:
postgres:
image: postgres:13
Expand All @@ -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
Expand All @@ -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:
Expand Down
10 changes: 9 additions & 1 deletion Gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
2 changes: 1 addition & 1 deletion activerecord-virtual_attributes.gemspec
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ Gem::Specification.new do |spec|

spec.require_paths = ["lib"]

spec.add_runtime_dependency "activerecord", "~> 6.1.0"
spec.add_runtime_dependency "activerecord", ">=6.1.7.6"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this was the main sticking point in #125 and is why I closed it - in production this unlocks all versions of Rails, but this gem is intentionally not semver and matches the version of Rails that it supports directly. That is, this gem at this version can't (and shouldn't) work with versions >= 6.1.

In my PR I tried to make this part dynamic and only for test, but it turns out you really can't have "dynamic" code in a gemspec because it won't be packaged properly.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

agreed, this needs to be:

Suggested change
spec.add_runtime_dependency "activerecord", ">=6.1.7.6"
spec.add_runtime_dependency "activerecord", "~>7.0.8"

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

and then also drop the 6.1 test suites.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

will do this once 6.1/7.0 get feature parity.


spec.add_development_dependency "byebug"
spec.add_development_dependency "database_cleaner-active_record", "~> 2.1"
Expand Down
64 changes: 33 additions & 31 deletions lib/active_record/virtual_attributes/virtual_fields.rb
Original file line number Diff line number Diff line change
Expand Up @@ -178,34 +178,46 @@ def grouped_records(orig_association, records, polymorphic_parent)
end
# rubocop:enable Style/BlockDelimiters, Lint/AmbiguousBlockAssociation, Style/MethodCallWithArgsParentheses
})
class Branch
prepend(Module.new {
def grouped_records
# binding.pry
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(self.association)

# 1 line optimization for single element array:
@association = association.first if association.kind_of?(Array)# && association.size == 1

# !!!!
@association = association.keys.first if association.kind_of?(Hash)

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
})
end if ActiveRecord.version >= Gem::Version.new(7.0)
end
end

class Relation
def without_virtual_includes
filtered_includes = includes_values && klass.replace_virtual_fields(includes_values)
if filtered_includes != includes_values
spawn.tap { |other| other.includes_values = filtered_includes }
else
self
end
end

include(Module.new {
# From ActiveRecord::FinderMethods
def apply_join_dependency(*args, **kargs, &block)
real = without_virtual_includes
if real.equal?(self)
super
else
real.apply_join_dependency(*args, **kargs, &block)
end
end

# From ActiveRecord::QueryMethods (rails 5.2 - 6.1)
def build_select(arel)
if select_values.any?
cols = arel_columns(select_values.uniq).map do |col|
cols = arel_columns(select_values).map do |col|
# if it is a virtual attribute, then add aliases to those columns
if col.kind_of?(Arel::Nodes::Grouping) && col.name
col.as(connection.quote_column_name(col.name))
Expand All @@ -221,7 +233,7 @@ def build_select(arel)

# from ActiveRecord::QueryMethods (rails 5.2 - 6.0)
# TODO: remove from rails 7.0
def arel_column(field, &block)
def arel_column(field)
if virtual_attribute?(field) && (arel = table[field])
arel
else
Expand All @@ -233,16 +245,6 @@ def construct_join_dependency(associations, join_type) # :nodoc:
associations = klass.replace_virtual_fields(associations)
super
end

# From ActiveRecord::Calculations
# introduces virtual includes support for calculate (we mostly use COUNT(*))
def calculate(operation, attribute_name)
# allow calculate to work with includes and a virtual attribute
real = without_virtual_includes
return super if real.equal?(self)

real.calculate(operation, attribute_name)
end
})
end
end
4 changes: 4 additions & 0 deletions spec/spec_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -41,3 +41,7 @@
end
end
end

require "active_record"
puts
puts "\e[93mUsing ActiveRecord #{ActiveRecord.version}\e[0m"
16 changes: 11 additions & 5 deletions spec/virtual_includes_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -240,9 +240,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
Expand Down Expand Up @@ -342,11 +340,13 @@
end

it "preloads virtual_reflection(:uses => :books => :bookmarks) (nothing virtual)" do
skip "ActiveRecord Preloader doesn't preload collection associations in rails 7+. See: https://www.github.com/rails/rails/pull/42654" if ActiveRecord.version >= Gem::Version.new(7.0)
bookmarked_book = Author.first.books.first
expect(Author.includes(:book_with_most_bookmarks)).to preload_values(:book_with_most_bookmarks, bookmarked_book)
end

it "preloads virtual_reflection(:uses => :books => :bookmarks, :uses => :books) (multiple overlapping relations)" do
skip "ActiveRecord Preloader doesn't preload collection associations in rails 7+. See: https://www.github.com/rails/rails/pull/42654" if ActiveRecord.version >= Gem::Version.new(7.0)
bookmarked_book = Author.first.books.first
expect(Author.includes(:book_with_most_bookmarks, :books)).to preload_values(:book_with_most_bookmarks, bookmarked_book)
end
Expand Down Expand Up @@ -404,6 +404,7 @@
end

it "preloads virtual_reflection(:uses => :books => :bookmarks) (nothing virtual)" do
skip "ActiveRecord Preloader doesn't preload collection associations in rails 7+. See: https://www.github.com/rails/rails/pull/42654" if ActiveRecord.version >= Gem::Version.new(7.0)
bookmarked_book = Author.first.books.first
expect(preloaded(Author.all.to_a, :book_with_most_bookmarks)).to preload_values(:book_with_most_bookmarks, bookmarked_book)
end
Expand Down Expand Up @@ -540,8 +541,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