diff --git a/lib/couch_potato/database.rb b/lib/couch_potato/database.rb index 2dc79f3..90d9a05 100644 --- a/lib/couch_potato/database.rb +++ b/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 diff --git a/spec/unit/caching_spec.rb b/spec/unit/caching_spec.rb index 5787a21..238c8dc 100644 --- a/spec/unit/caching_spec.rb +++ b/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 diff --git a/spec/unit/database_spec.rb b/spec/unit/database_spec.rb index 96eb0cf..fff7ac5 100644 --- a/spec/unit/database_spec.rb +++ b/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 }] }