Skip to content

Commit

Permalink
[ISSUE-62]: Avoid cache nil or empty array responses (#63)
Browse files Browse the repository at this point in the history
* Refactor `find_via_reload(key, *arguments)`

    - To avoid cache `nil` or `[]` responses/objects
    - Also refactor rspecs

* Add consecuent request on rspec

    - To check that after non cached `nil` or `[]` response we are
      caching valid vulues.

* Revert last commit
  • Loading branch information
italijancic-th authored Mar 27, 2024
1 parent cf17385 commit ca51141
Show file tree
Hide file tree
Showing 2 changed files with 16 additions and 4 deletions.
8 changes: 7 additions & 1 deletion lib/cached_resource/caching.rb
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ def find_via_cache(key, *arguments)
# Re/send the request to fetch the resource
def find_via_reload(key, *arguments)
object = find_without_cache(*arguments)
return object unless cached_resource.enabled
return object unless should_cache?(object)

cache_collection_synchronize(object, *arguments) if cached_resource.collection_synchronize
return object if !cached_resource.cache_collections && is_any_collection?(*arguments)
Expand Down Expand Up @@ -79,6 +79,12 @@ def update_collection_cache(updates, *arguments)
end
end

# Avoid cache nil or [] objects
def should_cache?(object)
return false unless cached_resource.enabled
object.respond_to?(:empty?) ? !object.empty? : !!object
end

# Determine if the given arguments represent
# the entire collection of objects.
def is_collection?(*arguments)
Expand Down
12 changes: 9 additions & 3 deletions spec/cached_resource/caching_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ class NotTheThing < ActiveResource::Base
@other_string_thing_json = @other_string_thing.to_json
@date_thing_json = @date_thing.to_json
@nil_thing = nil.to_json
@empty_array_thing = [].to_json
@not_the_thing = {:not_the_thing => {:id => 1, :name => "Not"}}
@not_the_thing_json = @not_the_thing.to_json
end
Expand All @@ -59,22 +60,27 @@ class NotTheThing < ActiveResource::Base
mock.get "/things/1.json?foo=bar", {}, @thing_json
mock.get "/things/fded.json", {}, @string_thing_json
mock.get "/things.json?name=42", {}, @nil_thing, 404
mock.get "/things.json?name=43", {}, @empty_array_thing
mock.get "/things/4.json", {}, @date_thing_json
mock.get "/not_the_things/1.json", {}, @not_the_thing_json
end
end

it "should cache a response" do
result = Thing.find(1)

read_from_cache("thing/1").should == result
end

it "should cache a nil response" do
result = Thing.find(:all, :params => { :name => '42' })
it "shouldn't cache nil response" do
Thing.find(:all, :params => { :name => '42' })
read_from_cache("thing/all/name/42").should == nil
end

it "shouldn't cache [] response" do
Thing.find(:all, :params => { :name => '43' })
read_from_cache("thing/all/name/43").should == nil
end

it "should cache a response for a string primary key" do
result = Thing.find("fded")
read_from_cache("thing/fded").should == result
Expand Down

0 comments on commit ca51141

Please sign in to comment.