SERVER-1606: don't duplicate update names

3 files changed, 68 insertions, 21 deletions

diff --git a/src/mongo/db/ops/update_internal.cpp b/src/mongo/db/ops/update_internal.cpp
index 4d5de801fb9..8197c7bd7e0 100644
--- a/src/mongo/db/ops/update_internal.cpp
+++ b/src/mongo/db/ops/update_internal.cpp
@@ -720,7 +720,28 @@ namespace mongo {
         return mss;
     }
 
-    void ModState::appendForOpLog( BSONObjBuilder& b ) const {
+    const char* ModState::getOpLogName() const {
+        if ( dontApply ) {
+            return NULL;
+        }
+
+        if ( incType ) {
+            return "$set";
+        }
+
+        if ( m->op == Mod::RENAME_FROM ) {
+            return "$unset";
+        }
+
+        if ( m->op == Mod::RENAME_TO ) {
+            return "$set";
+        }
+
+        return fixedOpName ? fixedOpName : Mod::modNames[op()];
+    }
+
+
+    void ModState::appendForOpLog( BSONObjBuilder& bb ) const {
         // dontApply logic is deprecated for all but $rename.
         if ( dontApply ) {
             return;
@@ -729,23 +750,18 @@ namespace mongo {
         if ( incType ) {
             DEBUGUPDATE( "\t\t\t\t\t appendForOpLog inc fieldname: " << m->fieldName
                          << " short:" << m->shortFieldName );
-            BSONObjBuilder bb( b.subobjStart( "$set" ) );
             appendIncValue( bb , true );
-            bb.done();
             return;
         }
 
         if ( m->op == Mod::RENAME_FROM ) {
             DEBUGUPDATE( "\t\t\t\t\t appendForOpLog RENAME_FROM fieldName:" << m->fieldName );
-            BSONObjBuilder bb( b.subobjStart( "$unset" ) );
             bb.append( m->fieldName, 1 );
-            bb.done();
             return;
         }
 
         if ( m->op == Mod::RENAME_TO ) {
             DEBUGUPDATE( "\t\t\t\t\t appendForOpLog RENAME_TO fieldName:" << m->fieldName );
-            BSONObjBuilder bb( b.subobjStart( "$set" ) );
             bb.appendAs( newVal, m->fieldName );
             return;
         }
@@ -756,13 +772,10 @@ namespace mongo {
                      << " fn: " << m->fieldName );
 
         if (strcmp(name, "$unset") == 0) {
-            BSONObjBuilder bb(b.subobjStart(name));
             bb.append(m->fieldName, 1);
-            bb.done();
             return;
         }
 
-        BSONObjBuilder bb( b.subobjStart( name ) );
         if ( fixed ) {
             bb.appendAs( *fixed , m->fieldName );
         }
@@ -776,7 +789,32 @@ namespace mongo {
         else {
             bb.appendAs( m->elt , m->fieldName );
         }
-        bb.done();
+
+    }
+
+    typedef map<string, vector<ModState*> > NamedModMap;
+
+    BSONObj ModSetState::getOpLogRewrite() const {
+        NamedModMap names;
+        for ( ModStateHolder::const_iterator i = _mods.begin(); i != _mods.end(); ++i ) {
+            const char* name = i->second->getOpLogName();
+            if ( ! name )
+                continue;
+            names[name].push_back( i->second.get() );
+        }
+
+        BSONObjBuilder b;
+        for ( NamedModMap::const_iterator i = names.begin();
+              i != names.end();
+              ++i ) {
+            BSONObjBuilder bb( b.subobjStart( i->first ) );
+            const vector<ModState*>& mods = i->second;
+            for ( unsigned j = 0; j < mods.size(); j++ ) {
+                mods[j]->appendForOpLog( bb );
+            }
+            bb.doneFast();
+        }
+        return b.obj();
     }
 
     string ModState::toString() const {
diff --git a/src/mongo/db/ops/update_internal.h b/src/mongo/db/ops/update_internal.h
index 1b46bbd9e48..9680b0bc645 100644
--- a/src/mongo/db/ops/update_internal.h
+++ b/src/mongo/db/ops/update_internal.h
@@ -537,6 +537,7 @@ namespace mongo {
             }
         }
 
+        const char* getOpLogName() const;
         void appendForOpLog( BSONObjBuilder& b ) const;
 
         void apply( BSONBuilderBase& b , BSONElement in ) {
@@ -776,12 +777,7 @@ namespace mongo {
             return false;
         }
 
-        BSONObj getOpLogRewrite() const {
-            BSONObjBuilder b;
-            for ( ModStateHolder::const_iterator i = _mods.begin(); i != _mods.end(); i++ )
-                i->second->appendForOpLog( b );
-            return b.obj();
-        }
+        BSONObj getOpLogRewrite() const;
 
         bool DEPRECATED_haveArrayDepMod() const {
             for ( ModStateHolder::const_iterator i = _mods.begin(); i != _mods.end(); i++ )
diff --git a/src/mongo/dbtests/updatetests.cpp b/src/mongo/dbtests/updatetests.cpp
index 7f2b597e55a..96b7af74cfe 100644
--- a/src/mongo/dbtests/updatetests.cpp
+++ b/src/mongo/dbtests/updatetests.cpp
@@ -1726,7 +1726,7 @@ namespace UpdateTests {
                 auto_ptr<ModSetState> modSetState = modSet.prepare( obj );
                 ASSERT_FALSE( modSetState->canApplyInPlace() );
                 modSetState->createNewFromMods();
-                ASSERT_EQUALS( BSON( "$set" << BSON( "a" << 3 ) << "$set" << BSON("b" << 2)),
+                ASSERT_EQUALS( BSON( "$set" << BSON( "a" << 3 << "b" << 2)),
                                modSetState->getOpLogRewrite() );
             }
         };
@@ -1740,7 +1740,7 @@ namespace UpdateTests {
                 auto_ptr<ModSetState> modSetState = modSet.prepare( obj );
                 ASSERT_FALSE( modSetState->canApplyInPlace() );
                 modSetState->createNewFromMods();
-                ASSERT_EQUALS( BSON( "$set" << BSON( "a" << 1 ) << "$set" << BSON("b" << 2)),
+                ASSERT_EQUALS( BSON( "$set" << BSON( "a" << 1 << "b" << 2)),
                                modSetState->getOpLogRewrite() );
             }
         };
@@ -2025,7 +2025,7 @@ namespace UpdateTests {
                 auto_ptr<ModSetState> modSetState = modSet.prepare( obj );
                 ASSERT_FALSE( modSetState->canApplyInPlace() );
                 modSetState->createNewFromMods();
-                ASSERT_EQUALS( fromjson( "{ $set:{ 'a.b':[ 1 ] }, $set:{ 'a.c':[ 1 ] } }" ),
+                ASSERT_EQUALS( fromjson( "{ $set:{ 'a.b':[ 1 ] , 'a.c':[ 1 ] } }" ),
                                modSetState->getOpLogRewrite() );
             }
         };
@@ -2180,7 +2180,7 @@ namespace UpdateTests {
                                 auto_ptr<ModSetState> modSetState = modSet.prepare( obj );
                 ASSERT_FALSE( modSetState->canApplyInPlace() );
                 modSetState->createNewFromMods();
-                ASSERT_EQUALS( BSON( "$unset" << BSON( "a" << 1 ) << "$set" << BSON ( "b" << 100 ) ),
+                ASSERT_EQUALS( BSON( "$set" << BSON( "b" << 100 ) << "$unset" << BSON ( "a" << 1 ) ),
                                modSetState->getOpLogRewrite() );
             }
         };
@@ -2194,7 +2194,7 @@ namespace UpdateTests {
                                 auto_ptr<ModSetState> modSetState = modSet.prepare( obj );
                 ASSERT_FALSE( modSetState->canApplyInPlace() );
                 modSetState->createNewFromMods();
-                ASSERT_EQUALS( BSON( "$unset" << BSON( "a" << 1 ) << "$set" << BSON ( "b" << 100 ) ),
+                ASSERT_EQUALS( BSON( "$set" << BSON( "b" << 100 ) << "$unset" << BSON ( "a" << 1 ) ),
                                modSetState->getOpLogRewrite() );
             }
         };
@@ -2256,6 +2256,18 @@ namespace UpdateTests {
             }
         };
 
+        class MultiSets {
+        public:
+            void run() {
+                BSONObj obj = BSON( "_id" << 1 << "a" << 1 << "b" << 1 );
+                BSONObj mod = BSON( "$set" << BSON( "a" << 2 << "b" << 2 ) );
+                ModSet modSet( mod );
+                auto_ptr<ModSetState> modSetState = modSet.prepare( obj );
+                ASSERT_TRUE( modSetState->canApplyInPlace() );
+                ASSERT_EQUALS( mod, modSetState->getOpLogRewrite() );
+            }
+        };
+
         class PositionalWithoutElemMatchKey {
         public:
             void run() {
@@ -2724,6 +2736,7 @@ namespace UpdateTests {
             // add< ModSetTests::BitRewriteNonExistingField >();
             add< ModSetTests::SetIsNotRewritten >();
             add< ModSetTests::UnsetIsNotRewritten >();
+            add< ModSetTests::MultiSets >();
             add< ModSetTests::PositionalWithoutElemMatchKey >();
             add< ModSetTests::PositionalWithoutNestedElemMatchKey >();
             add< ModSetTests::DbrefPassesPositionalValidation >();