search

active-hash / active_hash

A readonly ActiveRecord-esque base class that lets you use a hash, a Yaml file or a custom file as the datasource
MIT License
1.19k stars 178 forks source link

Relation: Make `where` return a fresh relation to avoid leaking scopes #265

Closed pfeiffer closed 1 year ago

pfeiffer commented 1 year ago

This PR fixes issue #257 where scopes are leaking. The issue can lead to wildly unexpected behavior.

Previously calling .where(..) would mutate the original relation.

Before this change

expect(countries.size).to eq 2
expect(countries.where(name: "US").size).to eq 1
expect(countries.size).to eq 2 # ❌ Fails as previous line mutated the `countries` object

After this change

expect(countries.size).to eq 2
expect(countries.where(name: "US").size).to eq 1
expect(countries.size).to eq 2 # ✅ Passes as the previous line returned a new relation keeping the original

The change also makes the "dirty tracking" in Relation unnecessary as Relations are immutable.

pfeiffer commented 1 year ago

See also the #268 as an alternative which addresses the root of the issue.

kbrock commented 1 year ago

Going with #268