From 7bd890fce08870d5dcc412ee54b00c9ebbc38c95 Mon Sep 17 00:00:00 2001 From: Alexander Lang <2173+langalex@users.noreply.github.com> Date: Sun, 25 Feb 2024 17:44:40 +0100 Subject: [PATCH] add database_collection to models (#162) this allows code using this library to detect and avoid n+1 requests when loading associated models: if database_collection is set on a model, instead of loading just one associated model by its id, code should load all associated models for all models in database_collection. --- lib/couch_potato/database.rb | 2 + lib/couch_potato/persistence.rb | 2 +- lib/couch_potato/view/flex_view_spec.rb | 2 + spec/unit/database_spec.rb | 63 +++++++++++++++++++++---- spec/unit/flex_view_spec_spec.rb | 27 +++++++++++ 5 files changed, 87 insertions(+), 9 deletions(-) diff --git a/lib/couch_potato/database.rb b/lib/couch_potato/database.rb index 9f3c7a9..2dc79f3 100644 --- a/lib/couch_potato/database.rb +++ b/lib/couch_potato/database.rb @@ -224,6 +224,7 @@ def process_view_results(results, spec) elsif processed_results.respond_to?(:each) processed_results.each do |document| document.database = self if document.respond_to?(:database=) + document.database_collection = processed_results if document.respond_to?(:database_collection=) end end processed_results @@ -297,6 +298,7 @@ def 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 diff --git a/lib/couch_potato/persistence.rb b/lib/couch_potato/persistence.rb index 7973e86..9cef8a1 100644 --- a/lib/couch_potato/persistence.rb +++ b/lib/couch_potato/persistence.rb @@ -27,7 +27,7 @@ def self.included(base) #:nodoc: ForbiddenAttributesProtection, Revisions base.send :include, Validation base.class_eval do - attr_accessor :_id, :_rev, :_deleted, :database + attr_accessor :_id, :_rev, :_deleted, :database, :database_collection alias_method :id, :_id alias_method :id=, :_id= diff --git a/lib/couch_potato/view/flex_view_spec.rb b/lib/couch_potato/view/flex_view_spec.rb index e1c2716..bb49b23 100644 --- a/lib/couch_potato/view/flex_view_spec.rb +++ b/lib/couch_potato/view/flex_view_spec.rb @@ -73,9 +73,11 @@ def reduce_count end def docs + all_docs = rows.map { |r| r['doc'] } rows.map do |row| doc = row['doc'] doc.database = database if doc.respond_to?(:database=) + doc.database_collection = all_docs if doc.respond_to?(:database_collection=) doc end end diff --git a/spec/unit/database_spec.rb b/spec/unit/database_spec.rb index e836c9c..96eb0cf 100644 --- a/spec/unit/database_spec.rb +++ b/spec/unit/database_spec.rb @@ -56,6 +56,16 @@ class Child db.load '1' end + it 'does not set database_collection on the model' do + user = double('user', 'database_collection=': nil).as_null_object + allow(DbTestUser).to receive(:new).and_return(user) + allow(couchrest_db).to receive(:get).and_return DbTestUser.json_create({ JSON.create_id => 'DbTestUser' }) + + db.load '1' + + expect(user).not_to have_received(:database_collection=).with(db) + end + it 'should load namespaced models' do allow(couchrest_db).to receive(:get).and_return Parent::Child.json_create({ JSON.create_id => 'Parent::Child' }) expect(db.load('1').class).to eq(Parent::Child) @@ -89,7 +99,7 @@ class Child it 'does not write itself to a document that has no database= method' do doc1 = double(:doc1) - allow(doc1).to receive(:respond_to?).with(:database=) { false } + allow(doc1).to receive(:respond_to?) { false } allow(couchrest_db).to receive(:bulk_load) do { 'rows' => [{ 'doc' => doc1 }] } end @@ -99,6 +109,24 @@ class Child db.load(['1']) end + it 'sets database_collection on each of the documents' do + db.load(%w[1 2]).each do |doc| + expect(doc.database_collection).to eql([doc1, doc2]) + end + end + + it 'does not set database_collection on a document that has no database_collection= method' do + doc1 = double(:doc1) + allow(doc1).to receive(:respond_to?) { false } + allow(couchrest_db).to receive(:bulk_load) do + { 'rows' => [{ 'doc' => doc1 }] } + end + + expect(doc1).not_to receive(:database_collection=) + + db.load(['1']) + end + it 'returns an empty array when passing an empty array' do expect(db.load([])).to eq([]) end @@ -336,7 +364,7 @@ def set_errors allow(CouchPotato::View::ViewQuery).to receive_messages(new: double('view query', query_view!: { 'rows' => [@result] })) end - it 'initialzes a view query with map/reduce/list/lib funtions' do + it 'initializes a view query with map/reduce/list/lib funtions' do allow(@spec).to receive_messages(design_document: 'design_doc', view_name: 'my_view', map_function: '', reduce_function: '', lib: { test: '' }, @@ -355,7 +383,7 @@ def set_errors @db.view(@spec) end - it 'initialzes a view query with map/reduce/list funtions' do + it 'initializes a view query with map/reduce/list funtions' do allow(@spec).to receive_messages(design_document: 'design_doc', view_name: 'my_view', map_function: '', reduce_function: '', lib: nil, list_name: 'my_list', list_function: '', @@ -374,7 +402,7 @@ def set_errors @db.view(@spec) end - it 'initialzes a view query with only map/reduce/lib functions' do + it 'initializes a view query with only map/reduce/lib functions' do allow(@spec).to receive_messages(design_document: 'design_doc', view_name: 'my_view', map_function: '', reduce_function: '', list_name: nil, list_function: nil, @@ -390,7 +418,7 @@ def set_errors @db.view(@spec) end - it 'initialzes a view query with only map/reduce functions' do + it 'initializes a view query with only map/reduce functions' do allow(@spec).to receive_messages(design_document: 'design_doc', view_name: 'my_view', map_function: '', reduce_function: '', lib: nil, list_name: nil, list_function: nil) @@ -405,18 +433,37 @@ def set_errors @db.view(@spec) end - it 'sets itself on returned results that have an accessor' do + it 'sets itself on returned docs that have an accessor' do + allow(@result).to receive(:respond_to?).and_return(false) allow(@result).to receive(:respond_to?).with(:database=).and_return(true) expect(@result).to receive(:database=).with(@db) @db.view(@spec) end - it "does not set itself on returned results that don't have an accessor" do - allow(@result).to receive(:respond_to?).with(:database=).and_return(false) + it "does not set itself on returned docs that don't have an accessor" do + allow(@result).to receive(:respond_to?).and_return(false) expect(@result).not_to receive(:database=).with(@db) @db.view(@spec) end + it 'sets the result of the view call on each returned doc' do + allow(@result).to receive(:respond_to?).and_return(false) + allow(@result).to receive(:respond_to?).with(:database_collection=).and_return(true) + allow(@result).to receive(:database_collection=) + + @db.view(@spec) + + expect(@result).to have_received(:database_collection=).with([@result]) + end + + it "does not set the result of the view call on docs that don't have an accessor" do + allow(@result).to receive(:respond_to?).and_return(false) + + @db.view(@spec) + + expect(@result).not_to receive(:database_collection=).with([@result]) + end + it 'does not try to set itself on result sets that are not collections' do expect do allow(@spec).to receive_messages(process_results: 1) diff --git a/spec/unit/flex_view_spec_spec.rb b/spec/unit/flex_view_spec_spec.rb index b57c5be..bb4de87 100644 --- a/spec/unit/flex_view_spec_spec.rb +++ b/spec/unit/flex_view_spec_spec.rb @@ -15,3 +15,30 @@ expect(result.reduce_count).to eq(0) end end + +RSpec.describe CouchPotato::View::FlexViewSpec::Results, '#docs' do + it 'sets the database on each doc' do + db = double('db') + doc = double('doc', 'database=': nil) + + result = CouchPotato::View::FlexViewSpec::Results.new 'rows' => [{ 'doc' => doc }] + result.database = db + + result.docs + + expect(doc).to have_received(:database=).with(db) + end + + it 'sets all docs as database_collection on each doc' do + doc = double('doc', 'database_collection=': nil) + + result = CouchPotato::View::FlexViewSpec::Results.new 'rows' => [{ 'doc' => doc }] + + result.docs + + expect(doc).to have_received(:database_collection=).with([doc]) + end + + it 'returns the docs' do + end +end \ No newline at end of file