Skip to content

Commit 55b6dca

Browse files
committed
Add a way to temporarily opt-out to #all now lazily making requests:
- ### Problem In 9372d5a we made a change that defer the http request submission until the collection is used. ```ruby # Before Product.all #=> HTTP request is made # After products = Product.all #=> No HTTP request is made products.each { ... } #=> HTTP request is made here ``` This create issues for codepaths that used to expect the `Product.all` to make the http request and this feels like a breaking change that should go through a deprecation cycle (too late now but let's have a way to opt-out to the new behaviour). ### Context We have a safety net in our application that does something like this: ```ruby products = allow_explicit_http_connections do Product.all end products.each { ... } ``` This is now broken because we make the request outside of the explictly allowed resiliency block. ### Solution Introduce a way to opt out to the new behaviour and trigger a deprecation warning.
1 parent e577c6d commit 55b6dca

File tree

2 files changed

+36
-1
lines changed

2 files changed

+36
-1
lines changed

lib/active_resource/base.rb

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -375,6 +375,7 @@ def self.logger=(logger)
375375
class_attribute :_format
376376
class_attribute :_collection_parser
377377
class_attribute :include_format_in_path
378+
class_attribute :find_all_lazy_load, default: true
378379
self.include_format_in_path = true
379380

380381
class_attribute :connection_class
@@ -1126,7 +1127,18 @@ def last(*args)
11261127
# This is an alias for find(:all). You can pass in all the same
11271128
# arguments to this method as you can to <tt>find(:all)</tt>
11281129
def all(*args)
1129-
WhereClause.new(self, *args)
1130+
if find_all_lazy_load
1131+
WhereClause.new(self, *args)
1132+
else
1133+
ActiveResource.deprecator.warn(<<~MSG)
1134+
In the next Active Resource release, calling ActiveResource::Base#all will defer the http request submission
1135+
until the collection is used.
1136+
1137+
To fix this warning, ensure your code doesn't rely on eagerly loading collection and
1138+
remove the `ActiveResource::Base.find_all_lazy_load` flag from your configuration.
1139+
MSG
1140+
find(:all, *args)
1141+
end
11301142
end
11311143

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

test/cases/finder_test.rb

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

64+
def test_all_loads_the_collection_lazily
65+
all = Person.all
66+
assert_empty(ActiveResource::HttpMock.requests)
67+
68+
assert_equal(2, all.size)
69+
assert_not_empty(ActiveResource::HttpMock.requests)
70+
end
71+
72+
def test_all_loads_the_collection_eagerly
73+
previous_value = ActiveResource::Base.find_all_lazy_load
74+
ActiveResource::Base.find_all_lazy_load = false
75+
76+
all = nil
77+
78+
assert_deprecated(/defer the http request submission/, ActiveResource.deprecator) do
79+
all = Person.all
80+
end
81+
assert_not_empty(ActiveResource::HttpMock.requests)
82+
assert_equal(2, all.size)
83+
ensure
84+
ActiveResource::Base.find_all_lazy_load = previous_value
85+
end
86+
6487
def test_all_with_params
6588
all = StreetAddress.all(params: { person_id: 1 })
6689
assert_equal 1, all.size

0 commit comments

Comments
 (0)