Skip to content

Commit dbb84fc

Browse files
committed
Revert to immediately retrieved collections by default
Closes #460 Introduce both the `config.active_resource.lazy_collections` configuration, as well as the `Base.lazy_collections` class attribute. Restore the original behavior of immediately retrieving collections by defaulting the initial value to `false`, but deprecate the behavior and encourage migrating to setting the value to `true`.
1 parent e577c6d commit dbb84fc

File tree

6 files changed

+65
-7
lines changed

6 files changed

+65
-7
lines changed

README.md

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -192,6 +192,29 @@ people = Person.where(last_name: "Durden")
192192
people.first # => <Person::xxx 'first_name' => 'Tyler' ...>
193193
```
194194

195+
Collections are retrieved immediately by default. To defer the loading of the
196+
records until their first access, set `Base.lazy_collections = true`:
197+
198+
```ruby
199+
Person.lazy_collections = true
200+
201+
# Builds a lazy collection with an initial filter
202+
people = Person.where(first_name: "Tyler")
203+
204+
# Modifies the lazy collection with an additional filter
205+
people = people.where(last_name: "Durden")
206+
207+
# Expects a response of
208+
#
209+
# [
210+
# {"id":1,"first_name":"Tyler","last_name":"Durden"},
211+
# ]
212+
#
213+
# for GET http://api.people.com:3000/people.json?first_name=Tyler&last_name=Durden
214+
215+
people.first # => <Person::xxx 'first_name' => 'Tyler' ...>
216+
```
217+
195218
### Create
196219

197220
Creating a new resource submits the JSON form of the resource as the body of the request and expects

lib/active_resource/base.rb

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -380,6 +380,8 @@ def self.logger=(logger)
380380
class_attribute :connection_class
381381
self.connection_class = Connection
382382

383+
class_attribute :lazy_collections, instance_accessor: false, default: false
384+
383385
class << self
384386
include ThreadsafeAttributes
385387
threadsafe_attribute :_headers, :_connection, :_user, :_password, :_bearer_token, :_site, :_proxy
@@ -1126,7 +1128,18 @@ def last(*args)
11261128
# This is an alias for find(:all). You can pass in all the same
11271129
# arguments to this method as you can to <tt>find(:all)</tt>
11281130
def all(*args)
1129-
WhereClause.new(self, *args)
1131+
if lazy_collections
1132+
WhereClause.new(self, *args)
1133+
else
1134+
ActiveResource.deprecator.warn(<<~MSG)
1135+
In the next Active Resource release, calling ActiveResource::Base#all will defer the http request submission
1136+
until the collection is used.
1137+
To fix this warning, ensure your code doesn't rely on eagerly loading collection and
1138+
remove the `ActiveResource::Base.lazy_collections` flag from your configuration.
1139+
MSG
1140+
1141+
find(:all, *args)
1142+
end
11301143
end
11311144

11321145
# This is an alias for all. You can pass in all the same

lib/active_resource/railtie.rb

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ class Railtie < Rails::Railtie
88
config.eager_load_namespaces << ActiveResource
99

1010
config.active_resource = ActiveSupport::OrderedOptions.new
11+
config.active_resource.lazy_collections = false
1112

1213
initializer "active_resource.set_configs" do |app|
1314
ActiveSupport.on_load(:active_resource) do

test/cases/finder_test.rb

Lines changed: 21 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,22 @@ def test_all
6161
assert_equal "David", all.last.name
6262
end
6363

64+
def test_all_immediately_finds_by_default
65+
assert_deprecated(/defer the http request submission/, ActiveResource.deprecator) do
66+
Person.all
67+
end
68+
69+
request, *other_requests = ActiveResource::HttpMock.requests
70+
assert_equal "/people.json", request.path
71+
assert_empty other_requests
72+
end
73+
74+
def test_all_with_lazy_collections
75+
DeferredPerson.all
76+
77+
assert_empty ActiveResource::HttpMock.requests
78+
end
79+
6480
def test_all_with_params
6581
all = StreetAddress.all(params: { person_id: 1 })
6682
assert_equal 1, all.size
@@ -84,7 +100,7 @@ def test_where_with_clauses
84100
def test_where_with_multiple_where_clauses
85101
ActiveResource::HttpMock.respond_to.get "/people.json?id=2&name=david", {}, @people_david
86102

87-
people = Person.where(id: 2).where(name: "david")
103+
people = DeferredPerson.where(id: 2).where(name: "david")
88104
assert_equal 1, people.size
89105
assert_kind_of Person, people.first
90106
assert_equal 2, people.first.id
@@ -94,7 +110,7 @@ def test_where_with_multiple_where_clauses
94110
def test_where_chained_from_all
95111
ActiveResource::HttpMock.respond_to.get "/records.json?id=2", {}, @people_david
96112

97-
people = Person.all(from: "/records.json").where(id: 2)
113+
people = DeferredPerson.all(from: "/records.json").where(id: 2)
98114
assert_equal 1, people.size
99115
assert_kind_of Person, people.first
100116
assert_equal 2, people.first.id
@@ -104,7 +120,7 @@ def test_where_chained_from_all
104120
def test_where_with_chained_into_all
105121
ActiveResource::HttpMock.respond_to.get "/records.json?id=2&name=david", {}, @people_david
106122

107-
people = Person.where(id: 2).all(from: "/records.json", params: { name: "david" })
123+
people = DeferredPerson.where(id: 2).all(from: "/records.json", params: { name: "david" })
108124
assert_equal 1, people.size
109125
assert_kind_of Person, people.first
110126
assert_equal 2, people.first.id
@@ -113,7 +129,7 @@ def test_where_with_chained_into_all
113129

114130
def test_where_loading
115131
ActiveResource::HttpMock.respond_to.get "/people.json?id=2", {}, @people_david
116-
people = Person.where(id: 2)
132+
people = DeferredPerson.where(id: 2)
117133

118134
assert_changes -> { ActiveResource::HttpMock.requests.count }, from: 0, to: 1 do
119135
people.load
@@ -125,7 +141,7 @@ def test_where_loading
125141

126142
def test_where_reloading
127143
ActiveResource::HttpMock.respond_to.get "/people.json?id=2", {}, @people_david
128-
people = Person.where(id: 2)
144+
people = DeferredPerson.where(id: 2)
129145

130146
assert_changes -> { ActiveResource::HttpMock.requests.count }, from: 0, to: 1 do
131147
assert_equal 1, people.size

test/cases/notifications_test.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ def setup
1515
end
1616

1717
def test_get_request_with_params
18-
payload = capture_notifications { Person.where(name: "Matz").load }
18+
payload = capture_notifications { Person.where(name: "Matz") }
1919

2020
assert_equal :get, payload[:method]
2121
assert_equal "http://37s.sunrise.i:3000/people.json?name=Matz", payload[:request_uri]

test/fixtures/person.rb

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,11 @@ class Person < ActiveResource::Base
44
self.site = "http://37s.sunrise.i:3000"
55
end
66

7+
class DeferredPerson < Person
8+
self.element_name = "person"
9+
self.lazy_collections = true
10+
end
11+
712
module External
813
class Person < ActiveResource::Base
914
self.site = "http://atq.caffeine.intoxication.it"

0 commit comments

Comments
 (0)