Skip to content

Rails 4 compatibility & thread safety - #15

Merged
RDIL merged 12 commits into
masterfrom
reece/rails-4
Jul 17, 2026
Merged

RDIL merged 12 commits into
masterfrom
reece/rails-4

Conversation

@RDIL

@RDIL RDIL commented Mar 17, 2026 •

Copy link
Copy Markdown
Member

Fixes GEN-6734

This was really painful

This PR:

  • Adds Rails 3 and 4 specs running on Ruby 3
  • Updates for Rails 4
  • Makes the Gem thread safe

The signature changed in Rails 4, since the hash-style query syntax is deprecated. It now gets a proc or lambda for the scope.

Also it's thread safe now.
@RDIL

This comment was marked as resolved.

@RDIL RDIL closed this Mar 27, 2026
@RDIL RDIL reopened this Mar 27, 2026
RDIL added 10 commits April 6, 2026 12:59
See the documentation attached.
…ord-deprecated_finders`)

The lambda is instance_exec'd on `klass.unscoped`, which is a `Relation`, not the model class. So bare all doesn't resolve to `Model.all` (which returns a Relation); it hits deprecated_finders' `Relation#all`, whose body is `apply_finder_options(args.first, true).to_a`. The array cast then errors because it's not a scope.
```
# (new, scope-block style)
has_many :x, -> { where(...) }, class_name: 'Y'
# (legacy finder-option style)
has_many :x, conditions: '...', order: '...'
```

In the legacy form Ruby binds the options hash to the `scope` slot. We must not relocate it, `activerecord-deprecated_finders` only translates legacy finder options (:conditions/:order/:include/...) into a real scope when they arrive in the `scope` position. Parse the args non-destructively and forward them in their original shape so both deprecated_finders (reads `scope`) and downstream has_many overrides that use `extract_options!` (read the trailing arg) see the hash where they expect it.
(It's in the git history so there you have it)
It assumed a scope is present only if `args.first` is callable. But Rails 4.2 implements `has_and_belongs_to_many` by internally calling `has_many name, nil, hm_options` (note the explicit nil scope followed by the options hash).
@RDIL
RDIL marked this pull request as ready for review July 17, 2026 14:46
@RDIL
RDIL merged commit 7c3b9b2 into master Jul 17, 2026
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants