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

Chaining named scope is no longer leaking to class level querying methods #32380

Merged
merged 1 commit into from Feb 5, 2019

Conversation

kamipo
Copy link
Member

@kamipo kamipo commented Mar 30, 2018

Active Record uses scoping to delegate to named scopes from relations
for propagating the chaining source scope. It was needed to restore the
source scope in named scopes, but it was caused undesired behavior that
pollute all class level querying methods.

Example:

class Topic < ActiveRecord::Base
  scope :toplevel, -> { where(parent_id: nil) }
  scope :children, -> { where.not(parent_id: nil) }
  scope :has_children, -> { where(id: Topic.children.select(:parent_id)) }
end

# Works as expected.
Topic.toplevel.where(id: Topic.children.select(:parent_id))

# Doesn't work due to leaking `toplevel` to `Topic.children`.
Topic.toplevel.has_children

Since #29301, the receiver in named scopes has changed from the model
class to the chaining source scope, so the polluting class level
querying methods is no longer required for that purpose.

Fixes #14003.

…hods

Active Record uses `scoping` to delegate to named scopes from relations
for propagating the chaining source scope. It was needed to restore the
source scope in named scopes, but it was caused undesired behavior that
pollute all class level querying methods.

Example:

```ruby
class Topic < ActiveRecord::Base
  scope :toplevel, -> { where(parent_id: nil) }
  scope :children, -> { where.not(parent_id: nil) }
  scope :has_children, -> { where(id: Topic.children.select(:parent_id)) }
end

# Works as expected.
Topic.toplevel.where(id: Topic.children.select(:parent_id))

# Doesn't work due to leaking `toplevel` to `Topic.children`.
Topic.toplevel.has_children
```

Since rails#29301, the receiver in named scopes has changed from the model
class to the chaining source scope, so the polluting class level
querying methods is no longer required for that purpose.

Fixes rails#14003.
@kamipo kamipo changed the title Chaining named scope is no longer leaking to querying class methods Chaining named scope is no longer leaking to class level querying methods Feb 5, 2019
@kamipo kamipo merged commit 92c75c3 into rails:master Feb 5, 2019
@kamipo kamipo deleted the fix_leaking_scope branch February 5, 2019 17:30
kamipo added a commit to kamipo/rails that referenced this pull request Feb 13, 2019
…ying methods"

This reverts rails#32380, since this may cause that silently leaking
information when people upgrade the app.

We need deprecation first before making this.
kamipo added a commit to kamipo/rails that referenced this pull request Feb 15, 2019
…garded as leaked

This deprecates using class level querying methods if the receiver scope
regarded as leaked, since rails#32380 and rails#35186 may cause that silently
leaking information when people upgrade the app.

We need deprecation first before making those.
@equivalent
Copy link
Contributor

consequently this model_object.has_many_associations.scope_name.create! stop working (I think) due to this here is related ticket #38881

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Leaking unrelated scopes
2 participants