From 69953eb71bdad5c674602b206a5fae015816c190 Mon Sep 17 00:00:00 2001 From: Emily Stolfo Date: Thu, 26 Mar 2015 12:33:50 +0100 Subject: [PATCH 1/6] RUBY-882 Generate ids for documents inserted with Insert WP msg --- lib/mongo/operation/result.rb | 2 +- lib/mongo/operation/write.rb | 1 + lib/mongo/operation/write/idable.rb | 41 ++++++++++++++++++++++ lib/mongo/operation/write/insert.rb | 5 +-- lib/mongo/operation/write/insert/result.rb | 18 ++++++++++ spec/mongo/operation/write/insert_spec.rb | 27 ++++++++++++++ 6 files changed, 91 insertions(+), 3 deletions(-) create mode 100644 lib/mongo/operation/write/idable.rb diff --git a/lib/mongo/operation/result.rb b/lib/mongo/operation/result.rb index 00c8f5c582..d5dbb8bcd0 100644 --- a/lib/mongo/operation/result.rb +++ b/lib/mongo/operation/result.rb @@ -119,7 +119,7 @@ def each(&block) documents.each(&block) end - # Initialize a new result result. + # Initialize a new result. # # @example Instantiate the result. # Result.new(replies) diff --git a/lib/mongo/operation/write.rb b/lib/mongo/operation/write.rb index 97aa66fb06..1bd08d007a 100644 --- a/lib/mongo/operation/write.rb +++ b/lib/mongo/operation/write.rb @@ -12,6 +12,7 @@ # See the License for the specific language governing permissions and # limitations under the License. +require 'mongo/operation/write/idable' require 'mongo/operation/write/bulk' require 'mongo/operation/write/delete' require 'mongo/operation/write/insert' diff --git a/lib/mongo/operation/write/idable.rb b/lib/mongo/operation/write/idable.rb new file mode 100644 index 0000000000..ef708c4342 --- /dev/null +++ b/lib/mongo/operation/write/idable.rb @@ -0,0 +1,41 @@ +# Copyright (C) 2014-2015 MongoDB, Inc. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +module Mongo + module Operation + module Write + module Idable + + private + + def id(doc) + doc.respond_to?(:id) ? doc.id : (doc['_id'] || doc[:_id]) + end + + def has_id?(doc) + id(doc) + end + + def ensure_ids(documents) + @inserted_ids = [] + documents.collect do |doc| + doc_with_id = has_id?(doc) ? doc : doc.merge(_id: BSON::ObjectId.new) + @inserted_ids << id(doc_with_id) + doc_with_id + end + end + end + end + end +end \ No newline at end of file diff --git a/lib/mongo/operation/write/insert.rb b/lib/mongo/operation/write/insert.rb index a163cc9868..6391396b55 100644 --- a/lib/mongo/operation/write/insert.rb +++ b/lib/mongo/operation/write/insert.rb @@ -44,6 +44,7 @@ module Write class Insert include Executable include Specifiable + include Idable # Execute the insert operation. # @@ -71,7 +72,7 @@ def execute_write_command(context) def execute_message(context) context.with_connection do |connection| - Result.new(connection.dispatch([ message, gle ].compact)).validate! + Result.new(connection.dispatch([ message, gle ].compact), @inserted_ids).validate! end end @@ -82,7 +83,7 @@ def initialize_copy(original) def message opts = !!options[:continue_on_error] ? { :flags => [:continue_on_error] } : {} - Protocol::Insert.new(db_name, coll_name, documents, opts) + Protocol::Insert.new(db_name, coll_name, ensure_ids(documents), opts) end end end diff --git a/lib/mongo/operation/write/insert/result.rb b/lib/mongo/operation/write/insert/result.rb index 5ad544f5f7..b7f3ad36b2 100644 --- a/lib/mongo/operation/write/insert/result.rb +++ b/lib/mongo/operation/write/insert/result.rb @@ -25,6 +25,24 @@ class Insert # @since 2.0.0 class Result < Operation::Result + # Get the ids of the inserted documents. + # + # @since 2.0.0 + attr_reader :inserted_ids + + # Initialize a new result. + # + # @example Instantiate the result. + # Result.new(replies, inserted_ids) + # + # @param [ Protocol::Reply ] replies The wire protocol replies. + # @params [ Array ] ids The ids of the inserted documents. + # + # @since 2.0.0 + def initialize(replies, ids) + @replies = replies.is_a?(Protocol::Reply) ? [ replies ] : replies + @inserted_ids = ids + end end end end diff --git a/spec/mongo/operation/write/insert_spec.rb b/spec/mongo/operation/write/insert_spec.rb index 1936c3c840..a88016aa9b 100644 --- a/spec/mongo/operation/write/insert_spec.rb +++ b/spec/mongo/operation/write/insert_spec.rb @@ -15,6 +15,10 @@ } end + after do + authorized_collection.find.delete_many + end + let(:insert) do described_class.new(spec) end @@ -80,6 +84,29 @@ end end + describe 'document ids' do + + context 'when documents do not contain an id' do + + let(:documents) do + [{ 'field' => 'test' }, + { 'field' => 'test' }] + end + + let(:inserted_ids) do + insert.execute(authorized_primary.context).inserted_ids + end + + let(:collection_ids) do + authorized_collection.find(field: 'test').collect { |d| d['_id'] } + end + + it 'adds an id to the documents' do + expect(inserted_ids).to eq(collection_ids) + end + end + end + describe '#execute' do before do From 79474f2710b33f4569543916b63da8e3f2d1ab19 Mon Sep 17 00:00:00 2001 From: Emily Stolfo Date: Thu, 26 Mar 2015 12:54:27 +0100 Subject: [PATCH 2/6] RUBY-882 Generate ids for documents inserted with Insert Write Command --- lib/mongo/operation/write/idable.rb | 4 ++-- lib/mongo/operation/write/insert.rb | 5 +++-- spec/mongo/operation/write/command/insert_spec.rb | 2 +- 3 files changed, 6 insertions(+), 5 deletions(-) diff --git a/lib/mongo/operation/write/idable.rb b/lib/mongo/operation/write/idable.rb index ef708c4342..24b2b1364b 100644 --- a/lib/mongo/operation/write/idable.rb +++ b/lib/mongo/operation/write/idable.rb @@ -28,10 +28,10 @@ def has_id?(doc) end def ensure_ids(documents) - @inserted_ids = [] + @ids = [] documents.collect do |doc| doc_with_id = has_id?(doc) ? doc : doc.merge(_id: BSON::ObjectId.new) - @inserted_ids << id(doc_with_id) + @ids << id(doc_with_id) doc_with_id end end diff --git a/lib/mongo/operation/write/insert.rb b/lib/mongo/operation/write/insert.rb index 6391396b55..a4f623379b 100644 --- a/lib/mongo/operation/write/insert.rb +++ b/lib/mongo/operation/write/insert.rb @@ -67,12 +67,13 @@ def execute(context) private def execute_write_command(context) - Result.new(Command::Insert.new(spec).execute(context)).validate! + command_spec = spec.merge(:documents => ensure_ids(documents)) + Result.new(Command::Insert.new(command_spec).execute(context), @ids).validate! end def execute_message(context) context.with_connection do |connection| - Result.new(connection.dispatch([ message, gle ].compact), @inserted_ids).validate! + Result.new(connection.dispatch([ message, gle ].compact), @ids).validate! end end diff --git a/spec/mongo/operation/write/command/insert_spec.rb b/spec/mongo/operation/write/command/insert_spec.rb index 8a51430c55..668d367729 100644 --- a/spec/mongo/operation/write/command/insert_spec.rb +++ b/spec/mongo/operation/write/command/insert_spec.rb @@ -3,7 +3,7 @@ describe Mongo::Operation::Write::Command::Insert do include_context 'operation' - let(:documents) { [{ :foo => 1 }] } + let(:documents) { [{ :_id => 1, :foo => 1 }] } let(:spec) do { :documents => documents, :db_name => db_name, From 10ccd6bd094893abdd8be125dbbdfb9de8830506 Mon Sep 17 00:00:00 2001 From: Emily Stolfo Date: Thu, 26 Mar 2015 13:10:09 +0100 Subject: [PATCH 3/6] RUBY-882 Generate ids for documents inserted with Bulk Insert op --- lib/mongo/operation/write/bulk/bulk_insert.rb | 14 ++++--- .../write/bulk/bulk_insert/result.rb | 38 +++++++++++++++++++ 2 files changed, 46 insertions(+), 6 deletions(-) diff --git a/lib/mongo/operation/write/bulk/bulk_insert.rb b/lib/mongo/operation/write/bulk/bulk_insert.rb index 516bae3a83..618e22a234 100644 --- a/lib/mongo/operation/write/bulk/bulk_insert.rb +++ b/lib/mongo/operation/write/bulk/bulk_insert.rb @@ -47,6 +47,7 @@ module Write # @since 2.0.0 class BulkInsert include Specifiable + include Idable # Execute the bulk insert operation. # @@ -69,21 +70,22 @@ def execute(context) private def execute_write_command(context) - Result.new(Command::Insert.new(spec).execute(context)) + command_spec = spec.merge(:documents => ensure_ids(documents)) + Result.new(Command::Insert.new(command_spec).execute(context), @ids) end def execute_message(context) replies = [] messages.map do |m| context.with_connection do |connection| - result = LegacyResult.new(connection.dispatch([ m, gle ].compact)) + result = LegacyResult.new(connection.dispatch([ m, gle ].compact), @ids) replies << result.reply if stop_sending?(result) - return LegacyResult.new(replies) + return LegacyResult.new(replies, @ids) end end end - LegacyResult.new(replies.compact.empty? ? nil : replies) + LegacyResult.new(replies.compact.empty? ? nil : replies, @ids) end def stop_sending?(result) @@ -117,10 +119,10 @@ def gle def messages if ordered? || gle documents.collect do |doc| - Protocol::Insert.new(db_name, coll_name, [ doc ], options) + Protocol::Insert.new(db_name, coll_name, ensure_ids([ doc ]), options) end else - [ Protocol::Insert.new(db_name, coll_name, documents, { :flags => [:continue_on_error] }) ] + [ Protocol::Insert.new(db_name, coll_name, ensure_ids(documents), { :flags => [:continue_on_error] }) ] end end end diff --git a/lib/mongo/operation/write/bulk/bulk_insert/result.rb b/lib/mongo/operation/write/bulk/bulk_insert/result.rb index d426a23b3c..435067ef91 100644 --- a/lib/mongo/operation/write/bulk/bulk_insert/result.rb +++ b/lib/mongo/operation/write/bulk/bulk_insert/result.rb @@ -26,6 +26,25 @@ class BulkInsert class Result < Operation::Result include BulkMergable + # Get the ids of the inserted documents. + # + # @since 2.0.0 + attr_reader :inserted_ids + + # Initialize a new result. + # + # @example Instantiate the result. + # Result.new(replies, inserted_ids) + # + # @param [ Protocol::Reply ] replies The wire protocol replies. + # @params [ Array ] ids The ids of the inserted documents. + # + # @since 2.0.0 + def initialize(replies, ids) + @replies = replies.is_a?(Protocol::Reply) ? [ replies ] : replies + @inserted_ids = ids + end + # Gets the number of documents inserted. # # @example Get the number of documents inserted. @@ -46,6 +65,25 @@ def n_inserted class LegacyResult < Operation::Result include LegacyBulkMergable + # Get the ids of the inserted documents. + # + # @since 2.0.0 + attr_reader :inserted_ids + + # Initialize a new result. + # + # @example Instantiate the result. + # Result.new(replies, inserted_ids) + # + # @param [ Protocol::Reply ] replies The wire protocol replies. + # @params [ Array ] ids The ids of the inserted documents. + # + # @since 2.0.0 + def initialize(replies, ids) + @replies = replies.is_a?(Protocol::Reply) ? [ replies ] : replies + @inserted_ids = ids + end + # Gets the number of documents inserted. # # @example Get the number of documents inserted. From 3ee28c960ef8492d9da895670afbae7e96dab65d Mon Sep 17 00:00:00 2001 From: Emily Stolfo Date: Thu, 26 Mar 2015 13:10:38 +0100 Subject: [PATCH 4/6] RUBY-882 list of ids should be appended to --- lib/mongo/operation/write/idable.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/mongo/operation/write/idable.rb b/lib/mongo/operation/write/idable.rb index 24b2b1364b..3bbc6f518c 100644 --- a/lib/mongo/operation/write/idable.rb +++ b/lib/mongo/operation/write/idable.rb @@ -28,7 +28,7 @@ def has_id?(doc) end def ensure_ids(documents) - @ids = [] + @ids ||= [] documents.collect do |doc| doc_with_id = has_id?(doc) ? doc : doc.merge(_id: BSON::ObjectId.new) @ids << id(doc_with_id) From 009b9add1b3fb9b1bc326bc73bc36c09d0b478fa Mon Sep 17 00:00:00 2001 From: Emily Stolfo Date: Thu, 26 Mar 2015 13:11:13 +0100 Subject: [PATCH 5/6] Put bulk specs in their own folder --- .../write/{ => bulk}/bulk_delete_spec.rb | 0 .../write/{ => bulk}/bulk_insert_spec.rb | 27 +++++++++++++++++++ .../write/{ => bulk}/bulk_update_spec.rb | 0 3 files changed, 27 insertions(+) rename spec/mongo/operation/write/{ => bulk}/bulk_delete_spec.rb (100%) rename spec/mongo/operation/write/{ => bulk}/bulk_insert_spec.rb (90%) rename spec/mongo/operation/write/{ => bulk}/bulk_update_spec.rb (100%) diff --git a/spec/mongo/operation/write/bulk_delete_spec.rb b/spec/mongo/operation/write/bulk/bulk_delete_spec.rb similarity index 100% rename from spec/mongo/operation/write/bulk_delete_spec.rb rename to spec/mongo/operation/write/bulk/bulk_delete_spec.rb diff --git a/spec/mongo/operation/write/bulk_insert_spec.rb b/spec/mongo/operation/write/bulk/bulk_insert_spec.rb similarity index 90% rename from spec/mongo/operation/write/bulk_insert_spec.rb rename to spec/mongo/operation/write/bulk/bulk_insert_spec.rb index c7d94c9019..4e7af67ff0 100644 --- a/spec/mongo/operation/write/bulk_insert_spec.rb +++ b/spec/mongo/operation/write/bulk/bulk_insert_spec.rb @@ -19,6 +19,10 @@ described_class.new(spec) end + after do + authorized_collection.find.delete_many + end + describe '#initialize' do context 'spec' do @@ -81,6 +85,29 @@ end end + describe 'document ids' do + + context 'when documents do not contain an id' do + + let(:documents) do + [{ 'field' => 'test' }, + { 'field' => 'test' }] + end + + let(:inserted_ids) do + op.execute(authorized_primary.context).inserted_ids + end + + let(:collection_ids) do + authorized_collection.find(field: 'test').collect { |d| d['_id'] } + end + + it 'adds an id to the documents' do + expect(inserted_ids).to eq(collection_ids) + end + end + end + describe '#execute' do before do diff --git a/spec/mongo/operation/write/bulk_update_spec.rb b/spec/mongo/operation/write/bulk/bulk_update_spec.rb similarity index 100% rename from spec/mongo/operation/write/bulk_update_spec.rb rename to spec/mongo/operation/write/bulk/bulk_update_spec.rb From b45688110f0213ed53a1e49d7c5396969ee848d6 Mon Sep 17 00:00:00 2001 From: Emily Stolfo Date: Thu, 26 Mar 2015 13:32:09 +0100 Subject: [PATCH 6/6] Add inserted_id method on Insert::Result and update CRUD specs --- .../write/bulk/bulk_insert/result.rb | 24 +++++++++++++++++++ lib/mongo/operation/write/insert/result.rb | 12 ++++++++++ spec/support/crud/write.rb | 8 +++---- 3 files changed, 39 insertions(+), 5 deletions(-) diff --git a/lib/mongo/operation/write/bulk/bulk_insert/result.rb b/lib/mongo/operation/write/bulk/bulk_insert/result.rb index 435067ef91..b4fd14b6ce 100644 --- a/lib/mongo/operation/write/bulk/bulk_insert/result.rb +++ b/lib/mongo/operation/write/bulk/bulk_insert/result.rb @@ -56,6 +56,18 @@ def initialize(replies, ids) def n_inserted written_count end + + # Gets the id of the document inserted. + # + # @example Get id of the document inserted. + # result.inserted_id + # + # @return [ Object ] The id of the document inserted. + # + # @since 2.0.0 + def inserted_id + inserted_ids.first + end end # Defines custom behaviour of results when inserting. @@ -99,6 +111,18 @@ def n_inserted n end end + + # Gets the id of the document inserted. + # + # @example Get id of the document inserted. + # result.inserted_id + # + # @return [ Object ] The id of the document inserted. + # + # @since 2.0.0 + def inserted_id + inserted_ids.first + end end end end diff --git a/lib/mongo/operation/write/insert/result.rb b/lib/mongo/operation/write/insert/result.rb index b7f3ad36b2..e6c3d873c9 100644 --- a/lib/mongo/operation/write/insert/result.rb +++ b/lib/mongo/operation/write/insert/result.rb @@ -43,6 +43,18 @@ def initialize(replies, ids) @replies = replies.is_a?(Protocol::Reply) ? [ replies ] : replies @inserted_ids = ids end + + # Gets the id of the document inserted. + # + # @example Get id of the document inserted. + # result.inserted_id + # + # @return [ Object ] The id of the document inserted. + # + # @since 2.0.0 + def inserted_id + inserted_ids.first + end end end end diff --git a/spec/support/crud/write.rb b/spec/support/crud/write.rb index 62f1801005..3e48365d35 100644 --- a/spec/support/crud/write.rb +++ b/spec/support/crud/write.rb @@ -121,15 +121,13 @@ def delete_one(collection) end def insert_many(collection) - collection.insert_many(documents) - # returning inserted_ids is optional - { 'insertedIds' => documents.collect { |d| d['_id'] } } + result = collection.insert_many(documents) + { 'insertedIds' => result.inserted_ids } end def insert_one(collection) result = collection.insert_one(document) - # returning inserted_id is optional - { 'insertedId' => document['_id'] } + { 'insertedId' => result.inserted_id } end def update_return_doc(result)