From 9fd4a34fd812025420e9fb81c5d2275f90b9f455 Mon Sep 17 00:00:00 2001 From: Emily Date: Mon, 15 Feb 2016 17:18:21 +0200 Subject: [PATCH 1/6] RUBY-1089 Fix aggregated upserted count reported by Bulk Update operation --- lib/mongo/bulk_write/result.rb | 2 +- .../operation/write/bulk/delete/result.rb | 6 +- .../operation/write/bulk/update/result.rb | 12 +- spec/mongo/bulk_write_spec.rb | 559 ++++++++++++- spec/support/shared/bulk_write.rb | 747 ------------------ 5 files changed, 545 insertions(+), 781 deletions(-) delete mode 100644 spec/support/shared/bulk_write.rb diff --git a/lib/mongo/bulk_write/result.rb b/lib/mongo/bulk_write/result.rb index 8719a6339a..4cefc39119 100644 --- a/lib/mongo/bulk_write/result.rb +++ b/lib/mongo/bulk_write/result.rb @@ -166,7 +166,7 @@ def upserted_count # # @since 2.1.0 def upserted_ids - @results[UPSERTED_IDS] + @results[UPSERTED_IDS] || [] end # Validates the bulk write result. diff --git a/lib/mongo/operation/write/bulk/delete/result.rb b/lib/mongo/operation/write/bulk/delete/result.rb index 18baa3808e..16a5297b93 100644 --- a/lib/mongo/operation/write/bulk/delete/result.rb +++ b/lib/mongo/operation/write/bulk/delete/result.rb @@ -34,7 +34,11 @@ module Aggregatable def n_removed return 0 unless acknowledged? @replies.reduce(0) do |n, reply| - n += reply.documents.first[Result::N] + if reply.documents.first[Result::N] + n += reply.documents.first[Result::N] + else + n + end end end end diff --git a/lib/mongo/operation/write/bulk/update/result.rb b/lib/mongo/operation/write/bulk/update/result.rb index 6463762772..80eae90ff4 100644 --- a/lib/mongo/operation/write/bulk/update/result.rb +++ b/lib/mongo/operation/write/bulk/update/result.rb @@ -46,9 +46,9 @@ def n_upserted return 0 unless acknowledged? @replies.reduce(0) do |n, reply| if upsert?(reply) - n += 1 + n += reply.documents.first[UPSERTED].size else - n += 0 + n end end end @@ -65,9 +65,13 @@ def n_matched return 0 unless acknowledged? @replies.reduce(0) do |n, reply| if upsert?(reply) - n += 0 + n else - n += reply.documents.first[N] + if reply.documents.first[N] + n += reply.documents.first[N] + else + n + end end end end diff --git a/spec/mongo/bulk_write_spec.rb b/spec/mongo/bulk_write_spec.rb index e168c08ded..1d81c566b7 100644 --- a/spec/mongo/bulk_write_spec.rb +++ b/spec/mongo/bulk_write_spec.rb @@ -8,7 +8,7 @@ after do authorized_collection.delete_many - collection_with_validator.drop() + collection_with_validator.drop end let(:collection_with_validator) do @@ -18,6 +18,10 @@ end end + let(:collection_invalid_write_concern) do + authorized_collection.client.with(write: { w: (WRITE_CONCERN[:w] + 1) })[authorized_collection.name] + end + describe '#execute' do shared_examples_for 'an executable bulk write' do @@ -81,6 +85,32 @@ expect(result.inserted_count).to eq(1) expect(authorized_collection.find(_id: 0).count).to eq(1) end + + it 'only inserts that document' do + result + expect(authorized_collection.find.first['_id']).to eq(0) + end + + context 'when there is a write concern error' do + + context 'when the server version is < 2.6' do + + it 'raises a BulkWriteError', if: !write_command_enabled? && standalone? do + expect { + bulk_write_invalid_write_concern.execute + }.to raise_error(Mongo::Error::BulkWriteError) + end + end + + context 'when the server version has write commands enabled' do + + it 'raises an OperationFailure', if: write_command_enabled? && standalone? do + expect { + bulk_write_invalid_write_concern.execute + }.to raise_error(Mongo::Error::OperationFailure) + end + end + end end context 'when provided multiple insert ones' do @@ -101,6 +131,40 @@ expect(result.inserted_count).to eq(3) expect(authorized_collection.find(_id: { '$in'=> [ 0, 1, 2 ]}).count).to eq(3) end + + context 'when there is a write failure' do + + let(:requests) do + [{ insert_one: { _id: 1 }}, { insert_one: { _id: 1 }}] + end + + it 'raises a BulkWriteError' do + expect { + bulk_write.execute + }.to raise_error(Mongo::Error::BulkWriteError) + end + end + + context 'when there is a write concern error' do + + context 'when the server version is < 2.6' do + + it 'raises a BulkWriteError', if: !write_command_enabled? && standalone? do + expect { + bulk_write_invalid_write_concern.execute + }.to raise_error(Mongo::Error::BulkWriteError) + end + end + + context 'when the server version has write commands enabled' do + + it 'raises an OperationFailure', if: write_command_enabled? && standalone? do + expect { + bulk_write_invalid_write_concern.execute + }.to raise_error(Mongo::Error::OperationFailure) + end + end + end end context 'when provided a single delete one' do @@ -121,6 +185,47 @@ expect(result.deleted_count).to eq(1) expect(authorized_collection.find(_id: 0).count).to eq(0) end + + context 'when there is a write concern error' do + + context 'when the server version is < 2.6' do + + it 'raises a BulkWriteError', if: !write_command_enabled? && standalone? do + expect { + bulk_write_invalid_write_concern.execute + }.to raise_error(Mongo::Error::BulkWriteError) + end + end + + context 'when the server version has write commands enabled' do + + it 'raises an OperationFailure', if: write_command_enabled? && standalone? do + expect { + bulk_write_invalid_write_concern.execute + }.to raise_error(Mongo::Error::OperationFailure) + end + end + end + + context 'when multiple documents match delete selector' do + + before do + authorized_collection.insert_many([{ a: 1 }, { a: 1 }]) + end + + let(:requests) do + [{ delete_one: { filter: { a: 1 }}}] + end + + it 'reports n_removed correctly' do + expect(bulk_write.execute.deleted_count).to eq(1) + end + + it 'deletes only matching documents' do + bulk_write.execute + expect(authorized_collection.find(a: 1).count).to eq(1) + end + end end context 'when provided multiple delete ones' do @@ -147,6 +252,27 @@ expect(result.deleted_count).to eq(3) expect(authorized_collection.find(_id: { '$in'=> [ 0, 1, 2 ]}).count).to eq(0) end + + context 'when there is a write concern error' do + + context 'when the server version is < 2.6' do + + it 'raises a BulkWriteError', if: !write_command_enabled? && standalone? do + expect { + bulk_write_invalid_write_concern.execute + }.to raise_error(Mongo::Error::BulkWriteError) + end + end + + context 'when the server version has write commands enabled' do + + it 'raises an OperationFailure', if: write_command_enabled? && standalone? do + expect { + bulk_write_invalid_write_concern.execute + }.to raise_error(Mongo::Error::OperationFailure) + end + end + end end context 'when provided a single delete many' do @@ -167,6 +293,27 @@ expect(result.deleted_count).to eq(1) expect(authorized_collection.find(_id: 0).count).to eq(0) end + + context 'when there is a write concern error' do + + context 'when the server version is < 2.6' do + + it 'raises a BulkWriteError', if: !write_command_enabled? && standalone? do + expect { + bulk_write_invalid_write_concern.execute + }.to raise_error(Mongo::Error::BulkWriteError) + end + end + + context 'when the server version has write commands enabled' do + + it 'raises an OperationFailure', if: write_command_enabled? && standalone? do + expect { + bulk_write_invalid_write_concern.execute + }.to raise_error(Mongo::Error::OperationFailure) + end + end + end end context 'when provided multiple delete many ops' do @@ -193,6 +340,27 @@ expect(result.deleted_count).to eq(3) expect(authorized_collection.find(_id: { '$in'=> [ 0, 1, 2 ]}).count).to eq(0) end + + context 'when there is a write concern error' do + + context 'when the server version is < 2.6' do + + it 'raises a BulkWriteError', if: !write_command_enabled? && standalone? do + expect { + bulk_write_invalid_write_concern.execute + }.to raise_error(Mongo::Error::BulkWriteError) + end + end + + context 'when the server version has write commands enabled' do + + it 'raises an OperationFailure', if: write_command_enabled? && standalone? do + expect { + bulk_write_invalid_write_concern.execute + }.to raise_error(Mongo::Error::OperationFailure) + end + end + end end context 'when providing a single replace one' do @@ -213,59 +381,386 @@ expect(result.modified_count).to eq(1) expect(authorized_collection.find(_id: 0).first[:name]).to eq('test') end + + context 'when there is a write concern error' do + + context 'when the server version is < 2.6' do + + it 'raises a BulkWriteError', if: !write_command_enabled? && standalone? do + expect { + bulk_write_invalid_write_concern.execute + }.to raise_error(Mongo::Error::BulkWriteError) + end + end + + context 'when the server version has write commands enabled' do + + it 'raises an OperationFailure', if: write_command_enabled? && standalone? do + expect { + bulk_write_invalid_write_concern.execute + }.to raise_error(Mongo::Error::OperationFailure) + end + end + end end context 'when providing a single update one' do - let(:requests) do - [{ update_one: { filter: { _id: 0 }, update: { "$set" => { name: 'test' }}}}] - end + context 'when upsert is false' do - let(:result) do - bulk_write.execute + let(:requests) do + [{ update_one: { filter: { _id: 0 }, update: { "$set" => { name: 'test' }}}}] + end + + let(:result) do + bulk_write.execute + end + + before do + authorized_collection.insert_one({ _id: 0 }) + end + + it 'updates the document' do + result + expect(authorized_collection.find(_id: 0).first[:name]).to eq('test') + end + + it 'reports the upserted id' do + expect(result.upserted_ids).to eq([]) + end + + it 'reports the upserted count' do + expect(result.upserted_count).to eq(0) + end + + it 'reports the modified count' do + expect(result.modified_count).to eq(1) + end + + context 'when the number of updates exceeds the max batch size', if: write_command_enabled? do + + let(:requests) do + 1001.times.collect do |i| + { update_one: { filter: { a: i }, update: { "$set" => { a: i, b: 3 }}, upsert: true }} + end + end + + it 'updates the documents and reports the correct number of upserted ids' do + expect(result.upserted_ids.size).to eq(1001) + expect(authorized_collection.find(b: 3).count).to eq(1001) + end + end + + context 'when there is a write concern error' do + + context 'when the server version is < 2.6' do + + it 'raises a BulkWriteError', if: !write_command_enabled? && standalone? do + expect { + bulk_write_invalid_write_concern.execute + }.to raise_error(Mongo::Error::BulkWriteError) + end + end + + context 'when the server version has write commands enabled' do + + it 'raises an OperationFailure', if: write_command_enabled? && standalone? do + expect { + bulk_write_invalid_write_concern.execute + }.to raise_error(Mongo::Error::OperationFailure) + end + end + end end - before do - authorized_collection.insert_one({ _id: 0 }) + context 'when upsert is true' do + + let(:requests) do + [{ update_one: { filter: { _id: 0 }, update: { "$set" => { name: 'test' } }, upsert: true }}] + end + + let(:result) do + bulk_write.execute + end + + it 'updates the document' do + result + expect(authorized_collection.find(_id: 0).first[:name]).to eq('test') + end + + it 'reports the upserted count' do + expect(result.upserted_count).to eq(1) + end + + it 'reports the modified count' do + expect(result.modified_count).to eq(0) + end + + context 'when not using write commands', unless: write_command_enabled? do + + it 'does not report the upserted id' do + expect(result.upserted_ids).to eq([]) + end + end + + context 'when using write commands', if: write_command_enabled? do + + it 'reports the upserted id' do + expect(result.upserted_ids).to eq([0]) + end + end + + context 'when there is a write concern error' do + + context 'when the server version is < 2.6' do + + it 'raises a BulkWriteError', if: !write_command_enabled? && standalone? do + expect { + bulk_write_invalid_write_concern.execute + }.to raise_error(Mongo::Error::BulkWriteError) + end + end + + context 'when the server version has write commands enabled' do + + it 'raises an OperationFailure', if: write_command_enabled? && standalone? do + expect { + bulk_write_invalid_write_concern.execute + }.to raise_error(Mongo::Error::OperationFailure) + end + end + end end + end - it 'updates the document' do - expect(result.modified_count).to eq(1) - expect(authorized_collection.find(_id: 0).first[:name]).to eq('test') + context 'when providing multiple update ones' do + + context 'when upsert is false' do + + let(:requests) do + [{ update_one: { filter: { _id: 0 }, update: { "$set" => { name: 'test' }}}}, + { update_one: { filter: { _id: 1 }, update: { "$set" => { name: 'test' }}}}] + end + + let(:result) do + bulk_write.execute + end + + before do + authorized_collection.insert_many([{ _id: 0 }, { _id: 1 }]) + end + + it 'updates the document' do + result + expect(authorized_collection.find(name: 'test').count).to eq(2) + end + + it 'reports the upserted id' do + expect(result.upserted_ids).to eq([]) + end + + it 'reports the upserted count' do + expect(result.upserted_count).to eq(0) + end + + it 'reports the modified count' do + expect(result.modified_count).to eq(2) + end + + context 'when there is a write concern error' do + + context 'when the server version is < 2.6' do + + it 'raises a BulkWriteError', if: !write_command_enabled? && standalone? do + expect { + bulk_write_invalid_write_concern.execute + }.to raise_error(Mongo::Error::BulkWriteError) + end + end + + context 'when the server version has write commands enabled' do + + it 'raises an OperationFailure', if: write_command_enabled? && standalone? do + expect { + bulk_write_invalid_write_concern.execute + }.to raise_error(Mongo::Error::OperationFailure) + end + end + end end - context 'when the number of updates exceeds the max batch size', if: write_command_enabled? do + context 'when upsert is true' do let(:requests) do - 1001.times.collect do |i| - { update_one: { filter: { a: i }, update: { "$set" => { a: i, b: 3 }}, upsert: true }} + [{ update_one: { filter: { _id: 0 }, update: { "$set" => { name: 'test' }}, upsert: true }}, + { update_one: { filter: { _id: 1 }, update: { "$set" => { name: 'test1' }}, upsert: true }}] + end + + let(:result) do + bulk_write.execute + end + + it 'updates the document' do + expect(result.modified_count).to eq(0) + expect(authorized_collection.find(name: { '$in' => ['test', 'test1'] }).count).to eq(2) + end + + it 'reports the upserted count' do + expect(result.upserted_count).to eq(2) + end + + it 'reports the modified count' do + expect(result.modified_count).to eq(0) + end + + context 'when not using write commands', unless: write_command_enabled? do + + it 'does not report the upserted id' do + expect(result.upserted_ids).to eq([]) end end - it 'updates the documents and reports the correct number of upserted ids' do - expect(result.upserted_ids.size).to eq(1001) - expect(authorized_collection.find(b: 3).count).to eq(1001) + context 'when using write commands', if: write_command_enabled? do + + it 'reports the upserted id' do + expect(result.upserted_ids).to eq([0, 1]) + end + end + + context 'when there is a write concern error' do + + context 'when the server version is < 2.6' do + + it 'raises a BulkWriteError', if: !write_command_enabled? && standalone? do + expect { + bulk_write_invalid_write_concern.execute + }.to raise_error(Mongo::Error::BulkWriteError) + end + end + + context 'when the server version has write commands enabled' do + + it 'raises an OperationFailure', if: write_command_enabled? && standalone? do + expect { + bulk_write_invalid_write_concern.execute + }.to raise_error(Mongo::Error::OperationFailure) + end + end end end end context 'when providing a single update many' do - let(:requests) do - [{ update_many: { filter: { _id: 0 }, update: { "$set" => { name: 'test' }}}}] - end + context 'when upsert is false' do - let(:result) do - bulk_write.execute - end + let(:requests) do + [{ update_many: { filter: { a: 0 }, update: { "$set" => { name: 'test' }}}}] + end - before do - authorized_collection.insert_one({ _id: 0 }) + let(:result) do + bulk_write.execute + end + + before do + authorized_collection.insert_many([{ a: 0 }, { a: 0 }]) + end + + it 'updates the documents' do + expect(authorized_collection.find(a: 0).count).to eq(2) + end + + it 'reports the upserted ids' do + expect(result.upserted_ids).to eq([]) + end + + it 'reports the upserted count' do + expect(result.upserted_count).to eq(0) + end + + it 'reports the modified count' do + expect(result.modified_count).to eq(2) + end + + context 'when there is a write concern error' do + + context 'when the server version is < 2.6' do + + it 'raises a BulkWriteError', if: !write_command_enabled? && standalone? do + expect { + bulk_write_invalid_write_concern.execute + }.to raise_error(Mongo::Error::BulkWriteError) + end + end + + context 'when the server version has write commands enabled' do + + it 'raises an OperationFailure', if: write_command_enabled? && standalone? do + expect { + bulk_write_invalid_write_concern.execute + }.to raise_error(Mongo::Error::OperationFailure) + end + end + end end - it 'updates the documents' do - expect(result.modified_count).to eq(1) - expect(authorized_collection.find(_id: 0).first[:name]).to eq('test') + context 'when upsert is true' do + + let(:requests) do + [{ update_many: { filter: { _id: 0 }, update: { "$set" => { name: 'test' }}, upsert: true }}] + end + + let(:result) do + bulk_write.execute + end + + it 'updates the document' do + result + expect(authorized_collection.find(name: 'test').count).to eq(1) + end + + it 'reports the upserted count' do + expect(result.upserted_count).to eq(1) + end + + it 'reports the modified count' do + expect(result.modified_count).to eq(0) + end + + context 'when not using write commands', unless: write_command_enabled? do + + it 'does not report the upserted id' do + expect(result.upserted_ids).to eq([]) + end + end + + context 'when using write commands', if: write_command_enabled? do + + it 'reports the upserted id' do + expect(result.upserted_ids).to eq([0]) + end + end + + context 'when there is a write concern error' do + + context 'when the server version is < 2.6' do + + it 'raises a BulkWriteError', if: !write_command_enabled? && standalone? do + expect { + bulk_write_invalid_write_concern.execute + }.to raise_error(Mongo::Error::BulkWriteError) + end + end + + context 'when the server version has write commands enabled' do + + it 'raises an OperationFailure', if: write_command_enabled? && standalone? do + expect { + bulk_write_invalid_write_concern.execute + }.to raise_error(Mongo::Error::OperationFailure) + end + end + end end end end @@ -347,6 +842,10 @@ described_class.new(authorized_collection, requests, ordered: false) end + let(:bulk_write_invalid_write_concern) do + described_class.new(collection_invalid_write_concern, requests, ordered: false) + end + it_behaves_like 'an executable bulk write' end @@ -356,6 +855,10 @@ described_class.new(authorized_collection, requests, ordered: true) end + let(:bulk_write_invalid_write_concern) do + described_class.new(collection_invalid_write_concern, requests, ordered: true) + end + it_behaves_like 'an executable bulk write' end end diff --git a/spec/support/shared/bulk_write.rb b/spec/support/shared/bulk_write.rb deleted file mode 100644 index e5e94f4b9d..0000000000 --- a/spec/support/shared/bulk_write.rb +++ /dev/null @@ -1,747 +0,0 @@ -shared_examples 'a bulk write object' do - - context 'when no operations are provided' do - - let(:operations) do - [] - end - - it 'raises an error' do - expect { - bulk.execute - }.to raise_error(ArgumentError) - end - end - - context 'when invalid operations are provided' do - - let(:operations) do - [{ :not_an_op => {}}] - end - - it 'raises an error' do - expect { - bulk.execute - }.to raise_error(Mongo::Error::InvalidBulkOperationType) - end - end - - context 'when an insert_one operation is provided' do - - context 'when there is a write failure' do - - let(:operations) do - [{ insert_one: { _id: 1 }}, { insert_one: { _id: 1 }}] - end - - it 'raises a BulkWriteError' do - expect { - bulk.execute - }.to raise_error(Mongo::Error::BulkWriteError) - end - end - - context 'when a document is provided' do - - let(:operations) do - [{ insert_one: { name: 'test' }}] - end - - it 'returns n_inserted of 1' do - expect(bulk.execute.inserted_count).to eq(1) - end - - it 'only inserts that document' do - bulk.execute - expect(authorized_collection.find.first['name']).to eq('test') - end - - context 'when there is a write concern error' do - - context 'when the server version is < 2.6' do - - it 'raises a BulkWriteError', if: !write_command_enabled? && standalone? do - expect { - bulk_invalid_write_concern.execute - }.to raise_error(Mongo::Error::BulkWriteError) - end - end - - context 'when the server version has write commands enabled' do - - it 'raises an OperationFailure', if: write_command_enabled? && standalone? do - expect { - bulk_invalid_write_concern.execute - }.to raise_error(Mongo::Error::OperationFailure) - end - end - end - end - - context 'when an invalid object is provided' do - - let(:operations) do - [{ insert_one: [] }] - end - - it 'raises an exception' do - expect { - bulk.execute - }.to raise_error(Mongo::Error::InvalidBulkOperation) - end - end - end - - context 'delete_one' do - - let(:docs) do - [ { a: 1 }, { a: 1 } ] - end - - let(:expected) do - [{ 'a' => 1 }] - end - - before do - authorized_collection.insert_many(docs) - end - - let(:operations) do - [ { delete_one: { a: 1 }}, - { delete_one: { a: 2 }} - ] - end - - context 'when no selector is specified' do - - let(:operations) do - [{ delete_one: nil }] - end - - it 'raises an exception' do - expect { - bulk.execute - }.to raise_exception(Mongo::Error::InvalidBulkOperation) - end - end - - context 'when multiple documents match delete selector' do - - it 'reports n_removed correctly' do - expect(bulk.execute.deleted_count).to eq(1) - end - - it 'deletes only matching documents' do - bulk.execute - expect(authorized_collection.find.projection(_id: 0).to_a).to eq(expected) - end - - context 'when there is a write concern error' do - - context 'when the server version is < 2.6' do - - it 'raises a BulkWriteError', if: !write_command_enabled? && standalone? do - expect { - bulk_invalid_write_concern.execute - }.to raise_error(Mongo::Error::BulkWriteError) - end - end - - context 'when the server version has write commands enabled' do - - it 'raises an OperationFailure', if: write_command_enabled? && standalone? do - expect { - bulk_invalid_write_concern.execute - }.to raise_error(Mongo::Error::OperationFailure) - end - end - end - end - end - - context 'when a delete_many operation is provided' do - - let(:docs) do - [{ a: 1 }, { a: 1 }] - end - - before do - authorized_collection.insert_many(docs) - end - - let(:operations) do - [{ delete_many: { a: 1 }}] - end - - context 'when no selector is specified' do - - let(:operations) do - [{ delete_many: nil }] - end - - it 'raises an exception' do - expect { - bulk.execute - }.to raise_exception(Mongo::Error::InvalidBulkOperation) - end - end - - context 'when a selector is specified' do - - context 'when multiple documents match delete selector' do - - it 'reports n_removed correctly' do - expect(bulk.execute.deleted_count).to eq(2) - end - - it 'deletes all matching documents' do - bulk.execute - expect(authorized_collection.find.to_a).to be_empty - end - - context 'when there is a write concern error' do - - context 'when the server version is < 2.6' do - - it 'raises a BulkWriteError', if: !write_command_enabled? && standalone? do - expect { - bulk_invalid_write_concern.execute - }.to raise_error(Mongo::Error::BulkWriteError) - end - end - - context 'when the server version has write commands enabled' do - - it 'raises an OperationFailure', if: write_command_enabled? && standalone? do - expect { - bulk_invalid_write_concern.execute - }.to raise_error(Mongo::Error::OperationFailure) - end - end - end - end - - context 'when only one document matches delete selector' do - - let(:docs) do - [{ a: 1 }, { a: 2 }] - end - - let(:expected) do - [{ 'a' => 2 }] - end - - it 'reports n_removed correctly' do - expect(bulk.execute.deleted_count).to eq(1) - end - - it 'deletes all matching documents' do - bulk.execute - expect(authorized_collection.find.projection(_id: 0).to_a).to eq(expected) - end - end - end - end - - context 'when a replace_one operation is provided' do - - let(:docs) do - [{ a: 1 }, { a: 1 }] - end - - let(:expected) do - [{ 'a' => 2 }, { 'a' => 1 }] - end - - before do - authorized_collection.insert_many(docs) - end - - let(:replacement) do - { a: 2 } - end - - let(:operations) do - [{ replace_one: { find: { a: 1 }, - replacement: replacement, - upsert: false } - }] - end - - context 'when a replace document is not specified' do - - let(:operations) do - [{ replace_one: { find: { a: 1 }, - replacement: nil, - upsert: false } - }] - end - - it 'raises an exception' do - expect { - bulk.execute - }.to raise_exception(Mongo::Error::InvalidBulkOperation) - end - end - - context 'when there are $-operator top-level keys' do - - let(:operations) do - [{ replace_one: { find: { a: 1 }, - replacement: { :$set => { a: 3 }}, - upsert: false } - }] - end - - it 'raises an exception' do - expect { - bulk.execute - }.to raise_exception(Mongo::Error::InvalidBulkOperation) - end - - end - - context 'when a replace document is specified' do - - it 'applies the replacement to only one matching document' do - bulk.execute - expect(authorized_collection.find(replacement).count).to eq(1) - end - - it 'reports n_matched correctly' do - expect(bulk.execute.matched_count).to eq(1) - end - - it 'only applies the replacement to one matching document' do - bulk.execute - expect(authorized_collection.find.projection(_id: 0).to_a).to eq(expected) - end - - context 'when there is a write concern error' do - - context 'when the server version is < 2.6' do - - it 'raises a BulkWriteError', if: !write_command_enabled? && standalone? do - expect { - bulk_invalid_write_concern.execute - }.to raise_error(Mongo::Error::BulkWriteError) - end - end - - context 'when the server version has write commands enabled' do - - it 'raises an OperationFailure', if: write_command_enabled? && standalone? do - expect { - bulk_invalid_write_concern.execute - }.to raise_error(Mongo::Error::OperationFailure) - end - end - end - - context 'when upsert is true' do - - let(:operations) do - [{ replace_one: { find: { a: 4 }, - replacement: replacement, - upsert: true } - }] - end - - let(:expected) do - [{ 'a' => 1 }, { 'a' => 1 }, { 'a' => 2 }] - end - - it 'upserts the replacement document' do - bulk.execute - expect(authorized_collection.find(replacement).count).to eq(1) - end - - it 'reports n_matched correctly' do - expect(bulk.execute.matched_count).to eq(0) - end - - it 'does not replace any documents' do - bulk.execute - expect(authorized_collection.find.projection(_id: 0).to_a).to eq(expected) - end - end - end - end - - context 'when an update_one operation is provided' do - - let(:docs) do - [{ a: 1 }, { a: 1 }] - end - - let(:update) do - { :$set => { a: 2 }} - end - - let(:operations) do - [{ update_one: { find: { a: 1 }, - update: update, - upsert: false } - }] - end - - before do - authorized_collection.insert_many(docs) - end - - let(:expected) do - [{ 'a' => 2 }, { 'a' => 1 }] - end - - context 'when there is a write failure' do - - let(:operations) do - [{ update_one: { find: { a: 1 }, - update: { '$set' => { field: 'blah' } }, - upsert: false } - }] - end - - it 'raises a BulkWriteError' do - expect { - bulk.execute - }.to raise_error(Mongo::Error::BulkWriteError) - end - end - - context 'when an update document is not specified' do - - let(:operations) do - [{ update_one: [{ a: 1 }]}] - end - - let(:operations) do - [{ update_one: { find: { a: 1 }, - upsert: false } - }] - end - - it 'raises an exception' do - expect { - bulk.execute - }.to raise_exception(Mongo::Error::InvalidBulkOperation) - end - end - - context 'when an invalid update document is specified' do - - let(:update) do - { a: 2 } - end - - it 'raises an exception' do - expect { - bulk.execute - }.to raise_exception(Mongo::Error::InvalidBulkOperation) - end - end - - context 'when a valid update document is specified' do - - it 'reports n_modified correctly', if: write_command_enabled? do - expect(bulk.execute.modified_count).to eq(1) - end - - it 'reports n_modified correctly', unless: write_command_enabled? do - expect(bulk.execute.modified_count).to eq(nil) - end - - it 'reports n_upserted correctly' do - expect(bulk.execute.upserted_count).to eq(0) - end - - it 'reports n_matched correctly' do - expect(bulk.execute.matched_count).to eq(1) - end - - it 'applies the correct writes' do - bulk.execute - expect(authorized_collection.find.projection(_id: 0).to_a).to eq(expected) - end - - context 'when there is a write concern error' do - - context 'when the server version is < 2.6' do - - it 'raises a BulkWriteError', if: !write_command_enabled? && standalone? do - expect { - bulk_invalid_write_concern.execute - }.to raise_error(Mongo::Error::BulkWriteError) - end - end - - context 'when the server version has write commands enabled' do - - it 'raises an OperationFailure', if: write_command_enabled? && standalone? do - expect { - bulk_invalid_write_concern.execute - }.to raise_error(Mongo::Error::OperationFailure) - end - end - end - - context 'when upsert is true' do - - let(:operations) do - [{ update_one: { find: { a: 3 }, - update: update, - upsert: true } - }] - end - - let(:expected) do - [{ 'a' => 1 }, { 'a' => 1 }, { 'a' => 2 }] - end - - it 'reports n_modified correctly', if: write_command_enabled? do - expect(bulk.execute.modified_count).to eq(0) - end - - it 'reports n_modified correctly', unless: write_command_enabled? do - expect(bulk.execute.modified_count).to eq(nil) - end - - it 'reports n_upserted correctly' do - expect(bulk.execute.upserted_count).to eq(1) - end - - it 'returns the upserted ids', if: write_command_enabled? do - expect(bulk.execute.upserted_ids.size).to eq(1) - end - - it 'reports n_matched correctly' do - expect(bulk.execute.matched_count).to eq(0) - end - - it 'applies the correct writes' do - bulk.execute - expect(authorized_collection.find.projection(_id: 0).to_a).to eq(expected) - end - - context 'when the number of updates exceeds the max batch size' do - - let(:operations) do - 1001.times.collect do |i| - { update_one: { find: { _id: i }, - update: { '$set' => { _id: i } }, - upsert: true } - } - end - - it 'reports the full number of upserted ids' do - expect(bulk.execute.upserted_ids.size).to eq(1001) - end - end - end - end - end - end - - context 'when an update_many operation is provided' do - - let(:docs) do - [{ a: 1 }, { a: 1 }] - end - - let(:update) do - { :$set => { a: 2 }} - end - - let(:operations) do - [{ update_many: { find: { a: 1 }, - update: update, - upsert: false } - }] - end - - let(:expected) do - [{ 'a' => 2 }, { 'a' => 2 }] - end - - before do - authorized_collection.insert_many(docs) - end - - context 'when there is a write failure' do - - let(:operations) do - [{ update_many: { find: { a: 1 }, - update: { '$st' => { field: 'blah' } }, - upsert: false } - }] - end - - it 'raises an BulkWriteError' do - expect { - bulk.execute - }.to raise_error(Mongo::Error::BulkWriteError) - end - end - - context 'when an update document is not specified' do - - let(:operations) do - [{ update_many: { find: { a: 1 }, - upsert: false } - }] - end - - it 'raises an exception' do - expect do - bulk.execute - end.to raise_exception(Mongo::Error::InvalidBulkOperation) - end - end - - context 'when an invalid update document is specified' do - - let(:update) do - { a: 2 } - end - - it 'raises an exception' do - expect do - bulk.execute - end.to raise_exception(Mongo::Error::InvalidBulkOperation) - end - end - - context 'when a valid update document is specified' do - - it 'reports n_modified correctly', if: write_command_enabled? do - expect(bulk.execute.modified_count).to eq(2) - end - - it 'reports n_modified correctly', unless: write_command_enabled? do - expect(bulk.execute.modified_count).to eq(nil) - end - - it 'reports n_upserted correctly' do - expect(bulk.execute.upserted_count).to eq(0) - end - - it 'reports n_matched correctly' do - expect(bulk.execute.matched_count).to eq(2) - end - - it 'applies the correct writes' do - bulk.execute - expect(authorized_collection.find.projection(_id: 0).to_a).to eq(expected) - end - - context 'when there is a write concern error' do - - context 'when the server version is < 2.6' do - - it 'raises a BulkWriteError', if: !write_command_enabled? && standalone? do - expect { - bulk_invalid_write_concern.execute - }.to raise_error(Mongo::Error::BulkWriteError) - end - end - - context 'when the server version has write commands enabled' do - - it 'raises an OperationFailure', if: write_command_enabled? && standalone? do - expect { - bulk_invalid_write_concern.execute - }.to raise_error(Mongo::Error::OperationFailure) - end - end - end - - context 'when upsert is true' do - - let(:operations) do - [{ update_many: { find: { a: 3 }, - update: update, - upsert: true } - }] - end - - let(:expected) do - [ { 'a' => 1 }, { 'a' => 1 }, { 'a' => 2 } ] - end - - it 'reports n_modified correctly', if: write_command_enabled? do - expect(bulk.execute.modified_count).to eq(0) - end - - it 'reports n_modified correctly', unless: write_command_enabled? do - expect(bulk.execute.modified_count).to eq(nil) - end - - it 'reports n_upserted correctly' do - expect(bulk.execute.upserted_count).to eq(1) - end - - it 'reports n_matched correctly' do - expect(bulk.execute.matched_count).to eq(0) - end - - it 'applies the correct writes' do - bulk.execute - expect(authorized_collection.find.projection(_id: 0).to_a).to eq(expected) - end - end - end - end - - context 'when the operations need to be split' do - - before do - 6000.times do |i| - authorized_collection.insert_one(x: i) - end - end - - let(:operations) do - [].tap do |ops| - 3000.times do |i| - ops << { update_one: { find: { x: i }, - update: { '$set' => { x: 6000-i } }, - upsert: false } - } - end - ops << { :insert_one => { test: 'emily' } } - 3000.times do |i| - ops << { update_one: { find: { x: 3000+i }, - update: { '$set' => { x: 3000-i } }, - upsert: false } - } - end - end - end - - it 'completes all operations' do - bulk.execute - expect(authorized_collection.find(x: { '$lte' => 3000 }).to_a.size).to eq(3000) - end - - context 'when there is a write concern error' do - - context 'when the server version is < 2.6' do - - it 'raises a BulkWriteError', if: !write_command_enabled? && standalone? do - expect { - bulk_invalid_write_concern.execute - }.to raise_error(Mongo::Error::BulkWriteError) - end - end - - context 'when the server version has write commands enabled' do - - it 'raises an OperationFailure', if: write_command_enabled? && standalone? do - expect { - bulk_invalid_write_concern.execute - }.to raise_error(Mongo::Error::OperationFailure) - end - end - end - end -end From 4c7b761c5d7e676e2dde0b69e757ed0fc279356e Mon Sep 17 00:00:00 2001 From: Emily Date: Mon, 15 Feb 2016 20:03:05 +0200 Subject: [PATCH 2/6] RUBY-1089 Fix Bulk result modified and matched count --- .../operation/write/bulk/update/result.rb | 22 +++- spec/mongo/bulk_write_spec.rb | 115 ++++++++++++++++++ 2 files changed, 135 insertions(+), 2 deletions(-) diff --git a/lib/mongo/operation/write/bulk/update/result.rb b/lib/mongo/operation/write/bulk/update/result.rb index 80eae90ff4..8b15411976 100644 --- a/lib/mongo/operation/write/bulk/update/result.rb +++ b/lib/mongo/operation/write/bulk/update/result.rb @@ -65,7 +65,7 @@ def n_matched return 0 unless acknowledged? @replies.reduce(0) do |n, reply| if upsert?(reply) - n + reply.documents.first[N] - n_upserted else if reply.documents.first[N] n += reply.documents.first[N] @@ -159,7 +159,25 @@ def n_matched end end end - alias :n_modified :n_matched + + # Gets the number of documents modified. + # + # @example Get the modified count. + # result.n_modified + # + # @return [ Integer ] The number of documents modified. + # + # @since 2.2.3 + def n_modified + return 0 unless acknowledged? + @replies.reduce(0) do |n, reply| + if upsert?(reply) + n + else + updated_existing?(reply) ? n += reply.documents.first[N] : n + end + end + end private diff --git a/spec/mongo/bulk_write_spec.rb b/spec/mongo/bulk_write_spec.rb index 1d81c566b7..e55961a63e 100644 --- a/spec/mongo/bulk_write_spec.rb +++ b/spec/mongo/bulk_write_spec.rb @@ -437,6 +437,37 @@ expect(result.modified_count).to eq(1) end + it 'reports the matched count' do + expect(result.matched_count).to eq(1) + end + + context 'when documents match but are not modified' do + + before do + authorized_collection.insert_one({ a: 0 }) + end + + let(:requests) do + [{ update_one: { filter: { a: 0 }, update: { "$set" => { a: 0 }}}}] + end + + it 'reports the upserted id' do + expect(result.upserted_ids).to eq([]) + end + + it 'reports the upserted count' do + expect(result.upserted_count).to eq(0) + end + + it 'reports the modified count' do + #expect(result.modified_count).to eq(0) + end + + it 'reports the matched count' do + expect(result.matched_count).to eq(1) + end + end + context 'when the number of updates exceeds the max batch size', if: write_command_enabled? do let(:requests) do @@ -567,6 +598,48 @@ expect(result.modified_count).to eq(2) end + it 'reports the matched count' do + expect(result.modified_count).to eq(2) + end + + + context 'when there is a mix of updates and matched without an update' do + + let(:requests) do + [{ update_one: { filter: { a: 0 }, update: { "$set" => { a: 1 }}}}, + { update_one: { filter: { a: 2 }, update: { "$set" => { a: 2 }}}}] + end + + let(:result) do + bulk_write.execute + end + + before do + authorized_collection.insert_many([{ a: 0 }, { a: 2 }]) + end + + it 'updates the document' do + result + expect(authorized_collection.find(a: { '$lt' => 3 }).count).to eq(2) + end + + it 'reports the upserted id' do + expect(result.upserted_ids).to eq([]) + end + + it 'reports the upserted count' do + expect(result.upserted_count).to eq(0) + end + + it 'reports the modified count' do + #expect(result.modified_count).to eq(1) + end + + it 'reports the matched count' do + expect(result.matched_count).to eq(2) + end + end + context 'when there is a write concern error' do context 'when the server version is < 2.6' do @@ -613,6 +686,10 @@ expect(result.modified_count).to eq(0) end + it 'reports the matched count' do + expect(result.matched_count).to eq(0) + end + context 'when not using write commands', unless: write_command_enabled? do it 'does not report the upserted id' do @@ -627,6 +704,44 @@ end end + context 'when there is a mix of updates, upsert, and matched without an update' do + + let(:requests) do + [{ update_one: { filter: { a: 0 }, update: { "$set" => { a: 1 }}}}, + { update_one: { filter: { a: 2 }, update: { "$set" => { a: 2 }}}}, + { update_one: { filter: { _id: 3 }, update: { "$set" => { a: 4 }}, upsert: true }}] + end + + let(:result) do + bulk_write.execute + end + + before do + authorized_collection.insert_many([{ a: 0 }, { a: 2 }]) + end + + it 'updates the document' do + result + expect(authorized_collection.find(a: { '$lt' => 3 }).count).to eq(2) + end + + it 'reports the upserted id' do + #expect(result.upserted_ids).to eq([3]) + end + + it 'reports the upserted count' do + expect(result.upserted_count).to eq(1) + end + + it 'reports the modified count' do + #expect(result.modified_count).to eq(1) + end + + it 'reports the matched count' do + expect(result.matched_count).to eq(2) + end + end + context 'when there is a write concern error' do context 'when the server version is < 2.6' do From c2bc70180d279ad3c918f39b41c8a4fcafec0173 Mon Sep 17 00:00:00 2001 From: Emily Date: Tue, 16 Feb 2016 11:45:06 +0200 Subject: [PATCH 3/6] RUBY-1089 Fix modified count for bulk legacy --- .../operation/write/bulk/update/result.rb | 3 +- spec/mongo/bulk_write_spec.rb | 66 +++++++------------ 2 files changed, 24 insertions(+), 45 deletions(-) diff --git a/lib/mongo/operation/write/bulk/update/result.rb b/lib/mongo/operation/write/bulk/update/result.rb index 8b15411976..bec5410f66 100644 --- a/lib/mongo/operation/write/bulk/update/result.rb +++ b/lib/mongo/operation/write/bulk/update/result.rb @@ -182,7 +182,8 @@ def n_modified private def upsert?(reply) - !updated_existing?(reply) && reply.documents.first[N] == 1 + reply.documents.first[BulkWrite::Result::UPSERTED] || + (!updated_existing?(reply) && reply.documents.first[N] == 1) end def updated_existing?(reply) diff --git a/spec/mongo/bulk_write_spec.rb b/spec/mongo/bulk_write_spec.rb index e55961a63e..cd27f6e121 100644 --- a/spec/mongo/bulk_write_spec.rb +++ b/spec/mongo/bulk_write_spec.rb @@ -459,8 +459,8 @@ expect(result.upserted_count).to eq(0) end - it 'reports the modified count' do - #expect(result.modified_count).to eq(0) + it 'reports the modified count', if: write_command_enabled? do + expect(result.modified_count).to eq(0) end it 'reports the matched count' do @@ -527,18 +527,8 @@ expect(result.modified_count).to eq(0) end - context 'when not using write commands', unless: write_command_enabled? do - - it 'does not report the upserted id' do - expect(result.upserted_ids).to eq([]) - end - end - - context 'when using write commands', if: write_command_enabled? do - - it 'reports the upserted id' do - expect(result.upserted_ids).to eq([0]) - end + it 'reports the upserted id', if: write_command_enabled? do + expect(result.upserted_ids).to eq([0]) end context 'when there is a write concern error' do @@ -631,8 +621,12 @@ expect(result.upserted_count).to eq(0) end - it 'reports the modified count' do - #expect(result.modified_count).to eq(1) + it 'reports the modified count', if: write_command_enabled? do + expect(result.modified_count).to eq(1) + end + + it 'reports the modified count', unless: write_command_enabled? do + expect(result.modified_count).to eq(2) end it 'reports the matched count' do @@ -690,18 +684,8 @@ expect(result.matched_count).to eq(0) end - context 'when not using write commands', unless: write_command_enabled? do - - it 'does not report the upserted id' do - expect(result.upserted_ids).to eq([]) - end - end - - context 'when using write commands', if: write_command_enabled? do - - it 'reports the upserted id' do - expect(result.upserted_ids).to eq([0, 1]) - end + it 'reports the upserted id', if: write_command_enabled? do + expect(result.upserted_ids).to eq([0, 1]) end context 'when there is a mix of updates, upsert, and matched without an update' do @@ -725,16 +709,20 @@ expect(authorized_collection.find(a: { '$lt' => 3 }).count).to eq(2) end - it 'reports the upserted id' do - #expect(result.upserted_ids).to eq([3]) + it 'reports the upserted id', if: write_command_enabled? do + expect(result.upserted_ids).to eq([3]) end it 'reports the upserted count' do expect(result.upserted_count).to eq(1) end - it 'reports the modified count' do - #expect(result.modified_count).to eq(1) + it 'reports the modified count', if: write_command_enabled? do + expect(result.modified_count).to eq(1) + end + + it 'reports the modified count', unless: write_command_enabled? do + expect(result.modified_count).to eq(2) end it 'reports the matched count' do @@ -842,18 +830,8 @@ expect(result.modified_count).to eq(0) end - context 'when not using write commands', unless: write_command_enabled? do - - it 'does not report the upserted id' do - expect(result.upserted_ids).to eq([]) - end - end - - context 'when using write commands', if: write_command_enabled? do - - it 'reports the upserted id' do - expect(result.upserted_ids).to eq([0]) - end + it 'reports the upserted id', if: write_command_enabled? do + expect(result.upserted_ids).to eq([0]) end context 'when there is a write concern error' do From 57ce2dfc0211bb5e4c6a2e31079779feff4fc2f2 Mon Sep 17 00:00:00 2001 From: Emily Date: Tue, 16 Feb 2016 11:50:42 +0200 Subject: [PATCH 4/6] RUBY-1089 Explicitly check for upserted document in spec --- spec/mongo/bulk_write_spec.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/spec/mongo/bulk_write_spec.rb b/spec/mongo/bulk_write_spec.rb index cd27f6e121..1d226fe931 100644 --- a/spec/mongo/bulk_write_spec.rb +++ b/spec/mongo/bulk_write_spec.rb @@ -704,9 +704,10 @@ authorized_collection.insert_many([{ a: 0 }, { a: 2 }]) end - it 'updates the document' do + it 'updates the documents' do result expect(authorized_collection.find(a: { '$lt' => 3 }).count).to eq(2) + expect(authorized_collection.find(a: 4).count).to eq(1) end it 'reports the upserted id', if: write_command_enabled? do From 60884cd3ec254e77ca112410ec25c9dc0294046b Mon Sep 17 00:00:00 2001 From: Emily Date: Wed, 17 Feb 2016 17:10:25 +0100 Subject: [PATCH 5/6] RUBY-1089 Account for nModified in a mixed cluster --- lib/mongo/bulk_write/result_combiner.rb | 4 ++-- lib/mongo/operation/write/bulk/update/result.rb | 10 +++++++++- 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/lib/mongo/bulk_write/result_combiner.rb b/lib/mongo/bulk_write/result_combiner.rb index 4928ad14cb..21f6d67501 100644 --- a/lib/mongo/bulk_write/result_combiner.rb +++ b/lib/mongo/bulk_write/result_combiner.rb @@ -77,8 +77,8 @@ def result def combine_counts!(result) Result::FIELDS.each do |field| - if result.respond_to?(field) - results.merge!(field => (results[field] || 0) + result.send(field)) + if result.respond_to?(field) && value = result.send(field) + results.merge!(field => (results[field] || 0) + value) end end end diff --git a/lib/mongo/operation/write/bulk/update/result.rb b/lib/mongo/operation/write/bulk/update/result.rb index bec5410f66..076d85c0ef 100644 --- a/lib/mongo/operation/write/bulk/update/result.rb +++ b/lib/mongo/operation/write/bulk/update/result.rb @@ -77,6 +77,10 @@ def n_matched end # Gets the number of documents modified. + # Not that in a mixed sharded cluster a call to + # update could return nModified (>= 2.6) or not (<= 2.4). + # If any call does not return nModified we can't report + # a valid final count so set the field to nil. # # @example Get the modified count. # result.n_modified @@ -87,7 +91,11 @@ def n_matched def n_modified return 0 unless acknowledged? @replies.reduce(0) do |n, reply| - n += reply.documents.first[MODIFIED] || 0 + if n && reply.documents.first[MODIFIED] + n += reply.documents.first[MODIFIED] + else + nil + end end end From 37255a13433ee640ddb0ec5e533fed9b0622c364 Mon Sep 17 00:00:00 2001 From: Emily Date: Wed, 17 Feb 2016 17:34:26 +0100 Subject: [PATCH 6/6] RUBY-1089 Add a few missing matched count tests --- spec/mongo/bulk_write_spec.rb | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/spec/mongo/bulk_write_spec.rb b/spec/mongo/bulk_write_spec.rb index 1d226fe931..5cb59e7a71 100644 --- a/spec/mongo/bulk_write_spec.rb +++ b/spec/mongo/bulk_write_spec.rb @@ -523,6 +523,10 @@ expect(result.upserted_count).to eq(1) end + it 'reports the matched count' do + expect(result.modified_count).to eq(0) + end + it 'reports the modified count' do expect(result.modified_count).to eq(0) end @@ -786,6 +790,10 @@ expect(result.modified_count).to eq(2) end + it 'reports the matched count' do + expect(result.modified_count).to eq(2) + end + context 'when there is a write concern error' do context 'when the server version is < 2.6' do @@ -827,6 +835,10 @@ expect(result.upserted_count).to eq(1) end + it 'reports the matched count' do + expect(result.matched_count).to eq(0) + end + it 'reports the modified count' do expect(result.modified_count).to eq(0) end