Skip to content

Commit

Permalink
A simpler / faster dispatcher fetch. The spec "should delegate fetch …
Browse files Browse the repository at this point in the history
…to the first cacheable strategy" is already proven by various other specs, e.g. by spec "should not hit the cache when an :id where clause is defined".
  • Loading branch information
lawrencepit committed Jan 8, 2012
1 parent 041cbde commit aceab0d
Show file tree
Hide file tree
Showing 4 changed files with 7 additions and 37 deletions.
5 changes: 3 additions & 2 deletions lib/record_cache/datastore/active_record_30.rb
Expand Up @@ -42,8 +42,9 @@ def find_by_sql_with_record_cache(sql)

def try_record_cache(sql, arel)
query = arel ? RecordCache::Arel::QueryVisitor.new.accept(arel.ast) : nil
cacheable = query && record_cache.cacheable?(query)
cacheable ? record_cache.fetch(query) : connection.send(:select, sql, "#{name} Load")
record_cache.fetch(query) do
connection.send(:select, sql, "#{name} Load")
end
end

end
Expand Down
5 changes: 1 addition & 4 deletions lib/record_cache/datastore/active_record_31.rb
Expand Up @@ -44,10 +44,7 @@ def find_by_sql_with_record_cache(sql, binds = [])

def try_record_cache(arel, sql, binds)
query = arel ? RecordCache::Arel::QueryVisitor.new(binds).accept(arel.ast) : nil
cacheable = query && record_cache.cacheable?(query)
if cacheable
record_cache.fetch(query)
else
record_cache.fetch(query) do
sql = connection.visitor.accept(sql.ast) if sql.respond_to?(:ast)
connection.send(:select, sql, "#{name} Load")
end
Expand Down
21 changes: 3 additions & 18 deletions lib/record_cache/dispatcher.rb
Expand Up @@ -39,15 +39,10 @@ def [](attribute)
@strategy_by_attribute[attribute]
end

# Can the cache retrieve the records based on this query?
def cacheable?(query)
!!first_cacheable_strategy(query)
end

# retrieve the record(s) based on the given query (check with cacheable?(query) first)
def fetch(query)
# fetch the results using the first strategy that accepts this query
fetch_from_first_cacheable_strategy(query)
def fetch(query, &block)
strategy = query && ordered_strategies.detect { |strategy| strategy.cacheable?(query) }
strategy ? strategy.fetch(query) : yield
end

# Update the version store and the record store (used by callbacks)
Expand Down Expand Up @@ -79,16 +74,6 @@ def record_store(store)
RecordCache::MultiRead.test(store)
end

# Retrieve the data from the first strategy that can handle the query.
def fetch_from_first_cacheable_strategy(query)
first_cacheable_strategy(query).fetch(query)
end

# Find the first strategy that can handle this query.
def first_cacheable_strategy(query)
ordered_strategies.detect { |strategy| strategy.cacheable?(query) }
end

# Retrieve all strategies ordered by the fastest strategy first (currently :id, :unique, :index)
def ordered_strategies
@ordered_strategies ||= begin
Expand Down
13 changes: 0 additions & 13 deletions spec/lib/dispatcher_spec.rb
Expand Up @@ -30,19 +30,6 @@
@apple_dispatcher[:unknown].should == nil
end

it "should return cacheable? true if there is a cacheable strategy that accepts the query" do
query = RecordCache::Query.new
mock(@apple_dispatcher).first_cacheable_strategy(query) { Object.new }
@apple_dispatcher.cacheable?(query).should == true
end

it "should delegate fetch to the first cacheable strategy" do
query = RecordCache::Query.new
banana_dispatcher = Banana.record_cache
mock(banana_dispatcher).first_cacheable_strategy(query) { mock(Object.new).fetch(query) }
banana_dispatcher.fetch(query)
end

context "record_change" do
it "should dispatch record_change to all strategies" do
apple = Apple.first
Expand Down

0 comments on commit aceab0d

Please sign in to comment.