Skip to content

Commit

Permalink
add database_collection to models (#162)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
langalex authored Feb 25, 2024
1 parent b2a8c7e commit 7bd890f
Show file tree
Hide file tree
Showing 5 changed files with 87 additions and 9 deletions.
2 changes: 2 additions & 0 deletions lib/couch_potato/database.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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

Expand Down
2 changes: 1 addition & 1 deletion lib/couch_potato/persistence.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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=

Expand Down
2 changes: 2 additions & 0 deletions lib/couch_potato/view/flex_view_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
63 changes: 55 additions & 8 deletions spec/unit/database_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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: '<map_code>', reduce_function: '<reduce_code>',
lib: { test: '<test_code>' },
Expand All @@ -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: '<map_code>', reduce_function: '<reduce_code>',
lib: nil, list_name: 'my_list', list_function: '<list_code>',
Expand All @@ -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: '<map_code>', reduce_function: '<reduce_code>',
list_name: nil, list_function: nil,
Expand All @@ -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: '<map_code>', reduce_function: '<reduce_code>',
lib: nil, list_name: nil, list_function: nil)
Expand All @@ -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)
Expand Down
27 changes: 27 additions & 0 deletions spec/unit/flex_view_spec_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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

0 comments on commit 7bd890f

Please sign in to comment.