add caching bulk loads

lib/couch_potato/database.rb

@@ -1,5 +1,7 @@
 # frozen_string_literal: true
 
+require 'active_support/core_ext/enumerable'
+
 module CouchPotato
   class Database
     class ValidationsFailedError < ::StandardError; end
@@ -143,7 +145,9 @@ def destroy_document(document)
     # returns nil if the single document could not be found. when passing an array and some documents
     # could not be found these are omitted from the returned array
     def load_document(id)
-      cached = cache && id.is_a?(String) && cache[id]
+      return load_documents(id) if id.is_a?(Array)
+
+      cached = cache && cache[id]
       if cache
         if cached
           ActiveSupport::Notifications.instrument('couch_potato.load.cached') do
@@ -159,6 +163,23 @@ def load_document(id)
     end
     alias load load_document
 
+    def load_documents(ids)
+      return [] if ids.empty?
+
+      uncached_ids = ids - (cache&.keys || [])
+      uncached_docs_by_id = bulk_load(uncached_ids).index_by {|doc| doc.id if doc.respond_to?(:id) }
+      if cache
+        uncached_ids.each do |id|
+          doc = uncached_docs_by_id[id]
+          cache[id] = doc if doc
+        end
+      end
+      cached_docs_by_id = ActiveSupport::Notifications.instrument('couch_potato.load.cached') do
+        cache&.slice(*ids) || {}
+      end
+      ids.filter_map { |id| (cached_docs_by_id[id]) || uncached_docs_by_id[id] }
+    end
+
     # loads one or more documents by its id(s)
     # behaves like #load except it raises a CouchPotato::NotFound if any of the documents could not be found
     def load!(id)
@@ -248,14 +269,10 @@ def load_document_without_caching(id)
       raise "Can't load a document without an id (got nil)" if id.nil?
 
       ActiveSupport::Notifications.instrument('couch_potato.load') do
-        if id.is_a?(Array)
-          bulk_load id
-        else
-          instance = couchrest_database.get(id)
-          instance.database = self if instance
-          instance
-        end
-      end
+        instance = couchrest_database.get(id)
+        instance.database = self if instance
+        instance
+    end
     end
 
     def view_cache_id(spec)
@@ -294,11 +311,13 @@ def save_document_without_conflict_handling(document, validate = true)
     def bulk_load(ids)
       return [] if ids.empty?
 
-      response = couchrest_database.bulk_load ids
-      docs = response['rows'].map { |row| row['doc'] }.compact
-      docs.each do |doc|
-        doc.database = self if doc.respond_to?(:database=)
-        doc.database_collection = docs if doc.respond_to?(:database_collection=)
+      ActiveSupport::Notifications.instrument('couch_potato.load') do
+        response = couchrest_database.bulk_load ids
+        docs = response['rows'].map { |row| row['doc'] }.compact
+        docs.each do |doc|
+          doc.database = self if doc.respond_to?(:database=)
+          doc.database_collection = docs if doc.respond_to?(:database_collection=)
+        end
       end
     end
 

spec/unit/caching_spec.rb

@@ -18,41 +18,92 @@
     {}
   end
 
-  it 'gets an object from the cache the 2nd time via #load_documemt' do
-    expect(couchrest_db).to receive(:get).with('1').exactly(1).times
+  context 'for a single document' do
+    it 'gets an object from the cache the 2nd time via #load_documemt' do
+      expect(couchrest_db).to receive(:get).with('1').exactly(1).times
 
-    db.load_document '1'
-    db.load_document '1'
-  end
+      db.load_document '1'
+      db.load_document '1'
+    end
 
-  it 'gets an object from the cache the 2nd time via #load' do
-    expect(couchrest_db).to receive(:get).with('1').exactly(1).times
+    it 'gets an object from the cache the 2nd time via #load' do
+      expect(couchrest_db).to receive(:get).with('1').exactly(1).times
 
-    db.load '1'
-    db.load '1'
-  end
+      db.load '1'
+      db.load '1'
+    end
 
-  it 'gets an object from the cache the 2nd time via #load!' do
-    expect(couchrest_db).to receive(:get).with('1').exactly(1).times
+    it 'gets an object from the cache the 2nd time via #load!' do
+      expect(couchrest_db).to receive(:get).with('1').exactly(1).times
 
-    db.load! '1'
-    db.load! '1'
-  end
+      db.load! '1'
+      db.load! '1'
+    end
 
-  it 'returns the correct object' do
-    doc = double(:doc, 'database=': nil)
-    allow(couchrest_db).to receive_messages(get: doc)
+    it 'returns the correct object' do
+      doc = double(:doc, 'database=': nil)
+      allow(couchrest_db).to receive_messages(get: doc)
 
-    db.load_document '1'
-    expect(db.load_document('1')).to eql(doc)
+      db.load_document '1'
+      expect(db.load_document('1')).to eql(doc)
+    end
   end
 
-  it 'does not cache bulk loads' do
-    allow(couchrest_db).to receive_messages(bulk_load: {'rows' => []})
-    expect(couchrest_db).to receive(:bulk_load).with(['1']).exactly(2).times
+  context 'for multiple documents' do
+    let(:doc1) { double(:doc1, 'database=': nil, id: '1') }
+    let(:doc2) { double(:doc12, 'database=': nil, id: '2') }
+
+    it 'only loads uncached documents' do
+      allow(couchrest_db).to receive(:bulk_load).with(['1']).and_return('rows' => [{'doc' => doc1}])
+      allow(couchrest_db).to receive(:bulk_load).with(['2']).and_return('rows' => [{'doc' => doc2}])
+
+
+      db.load_document(['1'])
+      db.load_document(['1', '2'])
+
+      expect(couchrest_db).to have_received(:bulk_load).with(['1']).exactly(1).times
+      expect(couchrest_db).to have_received(:bulk_load).with(['2']).exactly(1).times
+    end
+
+    it 'loads nothing if all documents are cached' do
+      allow(couchrest_db).to receive(:bulk_load).with(['1', '2'])
+        .and_return('rows' => [{'doc' => doc1}, {'doc' => doc2}])
+
+      db.load_document(['1', '2'])
+      db.load_document(['1', '2'])
+
+      expect(couchrest_db).to have_received(:bulk_load).with(['1', '2']).exactly(1).times
+    end
+
+    it 'returns all requested documents' do
+      allow(couchrest_db).to receive(:bulk_load).with(['1']).and_return('rows' => [{'doc' => doc1}])
+      allow(couchrest_db).to receive(:bulk_load).with(['2']).and_return('rows' => [{'doc' => doc2}])
+
+
+      db.load_document(['1'])
+      result = db.load_document(['1', '2'])
+
+      expect(result).to eql([doc1, doc2])
+    end
+
+    it 'does not cache documents that do not respond to id' do
+      doc1 = {
+        'id' => '1',
+      }
+      doc2 = {
+        'id' => '2',
+      }
+      allow(couchrest_db).to receive(:bulk_load).with(['1', '2'])
+        .and_return('rows' => [{'doc' => doc1}, {'doc' => doc1}])
+
+      db.load_document(['1', '2'])
+      db.load_document(['1', '2'])
+
+      expect(couchrest_db).to have_received(:bulk_load).with(['1', '2']).exactly(2).times
+    end
+  end
 
-    db.load_document ['1']
-    db.load_document ['1']
+  context 'when switching the database' do
   end
 
   it 'clears the cache when destroying a document via #destroy_document' do

spec/unit/database_spec.rb

@@ -72,8 +72,8 @@ class Child
   end
 
   context 'when several ids given' do
-    let(:doc1) { DbTestUser.new }
-    let(:doc2) { DbTestUser.new }
+    let(:doc1) { DbTestUser.new(id: '1') }
+    let(:doc2) { DbTestUser.new(id: '2') }
     let(:response) do
       { 'rows' => [{ 'doc' => nil }, { 'doc' => doc1 }, { 'doc' => doc2 }] }
     end
@@ -98,7 +98,7 @@ class Child
     end
 
     it 'does not write itself to a document that has no database= method' do
-      doc1 = double(:doc1)
+      doc1 = double(:doc1, id: '1')
       allow(doc1).to receive(:respond_to?) { false }
       allow(couchrest_db).to receive(:bulk_load) do
         { 'rows' => [{ 'doc' => doc1 }] }
@@ -116,7 +116,7 @@ class Child
     end
 
     it 'does not set database_collection on a document that has no database_collection= method' do
-      doc1 = double(:doc1)
+      doc1 = double(:doc1, id: '1')
       allow(doc1).to receive(:respond_to?) { false }
       allow(couchrest_db).to receive(:bulk_load) do
         { 'rows' => [{ 'doc' => doc1 }] }