[ISSUE-57] Fixes bug where CollectionParser is not chainable (#59)

* [ISSUE-56] Do not cache when cache is disable

Disable cache completely, do not use it when it is disabled.

* [ISSUE-57] Make collection_parser methods chainable

lib/cached_resource/caching.rb

@@ -37,13 +37,14 @@ def find_via_cache(key, *arguments)
         cache_read(key) || find_via_reload(key, *arguments)
       end
 
-      # Re/send the request to fetch the resource. Cache the response
-      # for the request.
+      # 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
+
         cache_collection_synchronize(object, *arguments) if cached_resource.collection_synchronize
         return object if !cached_resource.cache_collections && is_any_collection?(*arguments)
-        cache_write(key, object)
+        cache_write(key, object, *arguments)
         cache_read(key)
       end
 
@@ -52,29 +53,29 @@ def find_via_reload(key, *arguments)
       # otherwise update an existing collection if possible.
       def cache_collection_synchronize(object, *arguments)
         if object.is_a? Enumerable
-          update_singles_cache(object)
+          update_singles_cache(object, *arguments)
           # update the collection only if this is a subset of it
-          update_collection_cache(object) unless is_collection?(*arguments)
+          update_collection_cache(object, *arguments) unless is_collection?(*arguments)
         else
-          update_collection_cache(object)
+          update_collection_cache(object, *arguments)
         end
       end
 
       # Update the cache of singles with an array of updates.
-      def update_singles_cache(updates)
+      def update_singles_cache(updates, *arguments)
         updates = Array(updates)
-        updates.each { |object| cache_write(cache_key(object.send(primary_key)), object) }
+        updates.each { |object| cache_write(cache_key(object.send(primary_key)), object, *arguments) }
       end
 
       # Update the "mother" collection with an array of updates.
-      def update_collection_cache(updates)
+      def update_collection_cache(updates, *arguments)
         updates = Array(updates)
         collection = cache_read(cache_key(cached_resource.collection_arguments))
 
         if collection && !updates.empty?
           index = collection.inject({}) { |hash, object| hash[object.send(primary_key)] = object; hash }
           updates.each { |object| index[object.send(primary_key)] = object }
-          cache_write(cache_key(cached_resource.collection_arguments), index.values)
+          cache_write(cache_key(cached_resource.collection_arguments), index.values, *arguments)
         end
       end
 
@@ -101,7 +102,10 @@ def cache_read(key)
             if cache.is_a? Enumerable
               restored = cache.map { |record| full_dup(record) }
               next restored unless respond_to?(:collection_parser)
-              collection_parser.new(restored)
+              collection_parser.new(restored).tap do |parser|
+                parser.resource_class = self
+                parser.original_params = json['original_params']
+              end
             else
               full_dup(cache)
             end
@@ -112,8 +116,12 @@ def cache_read(key)
       end
 
       # Write an entry to the cache for the given key and value.
-      def cache_write(key, object)
-        result = cached_resource.cache.write(key, object_to_json(object), :race_condition_ttl => cached_resource.race_condition_ttl, :expires_in => cached_resource.generate_ttl)
+      def cache_write(key, object, *arguments)
+        options = arguments[1] || {}
+        params = options[:params]
+        prefix_options, query_options = split_options(params)
+
+        result = cached_resource.cache.write(key, object_to_json(object, prefix_options, query_options), :race_condition_ttl => cached_resource.race_condition_ttl, :expires_in => cached_resource.generate_ttl)
         result && cached_resource.logger.info("#{CachedResource::Configuration::LOGGER_PREFIX} WRITE #{key}")
         result
       end
@@ -150,21 +158,34 @@ def full_dup(record)
       end
 
       def json_to_object(json)
-        if json.is_a? Array
-          json.map { |attrs|
-            self.new(attrs["object"], attrs["persistence"]) }
+        resource = json['resource']
+        if resource.is_a? Array
+          resource.map do |attrs|
+            self.new(attrs["object"], attrs["persistence"]).tap do |resource|
+              resource.prefix_options = json['prefix_options']
+            end
+          end
         else
-          self.new(json["object"], json["persistence"])
+          self.new(resource["object"], resource["persistence"]).tap do |resource|
+            resource.prefix_options = json['prefix_options']
+          end
         end
       end
 
-      def object_to_json(object)
+      def object_to_json(object, prefix_options, query_options)
         if object.is_a? Enumerable
-           object.map { |o| { :object => o, :persistence => o.persisted? } }.to_json
+          {
+            :resource => object.map { |o| { :object => o, :persistence => o.persisted? } },
+            :prefix_options => prefix_options,
+            :original_params => query_options
+          }.to_json
         elsif object.nil?
           nil.to_json
         else
-          { :object => object, :persistence => object.persisted? }.to_json
+          {
+            :resource => { :object => object, :persistence => object.persisted? },
+            :prefix_options => prefix_options
+          }.to_json
         end
       end
     end

spec/cached_resource/caching_spec.rb

@@ -18,6 +18,8 @@ class NotTheThing < ActiveResource::Base
     end
 
     @thing = {:thing => {:id => 1, :name => "Ada"}}
+    @thing_collection = [{:id => 1, :name => "Ada"}, {:id => 2, :name => "Ada", :major => 'CS'}]
+    @thing_collection2 = [{:id => 2, :name => "Ada", :major => 'CS'}]
     @other_thing = {:thing => {:id => 1, :name => "Ari"}}
     @thing2 = {:thing => {:id => 2, :name => "Joe"}}
     @other_thing2 = {:thing => {:id => 2, :name => "Jeb"}}
@@ -215,13 +217,32 @@ class NotTheThing < ActiveResource::Base
           cached.should be_instance_of(ActiveResource::Collection)
         end
 
-        it "should return an instance of the collection_parser" do
+        it "should return a chainable instance of the collection_parser" do
           Thing.cached_resource.cache.clear
           class CustomCollection < ActiveResource::Collection; end
           Thing.collection_parser = CustomCollection
-          Thing.all
-          cached = read_from_cache("thing/all")
+
+          ActiveResource::HttpMock.respond_to do |mock|
+            mock.get "/things.json?name=ada", {}, @thing_collection.to_json
+            mock.get "/things.json?major=CS&name=ada", {}, @thing_collection2.to_json
+          end
+
+          non_cached = Thing.where(name: 'ada')
+          non_cached.original_params.should == { 'name' => 'ada' }
+          non_cached.map(&:id).should == @thing_collection.map { |h| h[:id]}
+
+          cached = read_from_cache('thing/all/{:params=>{:name=>"ada"}}')
           cached.should be_instance_of(CustomCollection)
+          cached.original_params.should == { 'name' => 'ada' }
+          cached.map(&:id).should == @thing_collection.map { |h| h[:id]}
+
+          non_cached = cached.where(major: 'CS')
+          non_cached.original_params.should == { 'name' => 'ada', 'major' => 'CS' }
+          non_cached.map(&:id).should == @thing_collection2.map { |h| h[:id]}
+
+          cached = read_from_cache('thing/all/{:params=>{"name"=>"ada",:major=>"cs"}}')
+          cached.original_params.should == { 'name' => 'ada', 'major' => 'CS' }
+          cached.map(&:id).should == @thing_collection2.map { |h| h[:id]}
         end
       else
         it "should return an Array" do
@@ -505,14 +526,9 @@ class CustomCollection < ActiveResource::Collection; end
       end
     end
 
-    it "should cache a response" do
+    it "should not cache a response" do
       result = Thing.find(1)
-      read_from_cache("thing/1").should == result
-    end
-
-    it "should cache a response for a string primary key" do
-      result = Thing.find("fded")
-      read_from_cache("thing/fded").should == result
+      read_from_cache("thing/1").should be_nil
     end
 
     it "should always remake the request" do
@@ -528,42 +544,6 @@ class CustomCollection < ActiveResource::Collection; end
       Thing.find("fded")
       ActiveResource::HttpMock.requests.length.should == 2
     end
-
-    it "should rewrite the cache for each request" do
-      Thing.find(1)
-      old_result = read_from_cache("thing/1")
-
-      # change the response
-      ActiveResource::HttpMock.reset!
-      ActiveResource::HttpMock.respond_to do |mock|
-        mock.get "/things/1.json", {}, @other_thing_json
-      end
-
-      Thing.find(1)
-      new_result = read_from_cache("thing/1")
-      # since active resources are equal if and only if they
-      # are the same object or an instance of the same class,
-      # not new?, and have the same id.
-      new_result.name.should_not == old_result.name
-    end
-
-    it "should rewrite the cache for each request for a string primary key" do
-      Thing.find("fded")
-      old_result = read_from_cache("thing/fded")
-
-      # change the response
-      ActiveResource::HttpMock.reset!
-      ActiveResource::HttpMock.respond_to do |mock|
-        mock.get "/things/fded.json", {}, @other_string_thing_json
-      end
-
-      Thing.find("fded")
-      new_result = read_from_cache("thing/fded")
-      # since active resources are equal if and only if they
-      # are the same object or an instance of the same class,
-      # not new?, and have the same id.
-      new_result.name.should_not == old_result.name
-    end
   end
 
   describe "when cache_collections is disabled" do