SERVER-19068: treat NumberLong()/$numberLong as long long, always

4 files changed, 49 insertions, 41 deletions

diff --git a/src/mongo/bson/json.cpp b/src/mongo/bson/json.cpp
index d318a9bc0e8..55297486ef3 100644
--- a/src/mongo/bson/json.cpp
+++ b/src/mongo/bson/json.cpp
@@ -634,7 +634,7 @@ Status JParse::numberLongObject(StringData fieldName, BSONObjBuilder& builder) {
         return ret;
     }
 
-    builder.appendNumber(fieldName, numberLong);
+    builder.append(fieldName, numberLong);
     return Status::OK();
 }
 
@@ -819,7 +819,7 @@ Status JParse::numberLong(StringData fieldName, BSONObjBuilder& builder) {
     if (!readToken(RPAREN)) {
         return parseError("Expecting ')'");
     }
-    builder.appendNumber(fieldName, static_cast<long long int>(val));
+    builder.append(fieldName, static_cast<long long int>(val));
     return Status::OK();
 }
 
diff --git a/src/mongo/client/fetcher.cpp b/src/mongo/client/fetcher.cpp
index f45d0c99be5..18f9e02828a 100644
--- a/src/mongo/client/fetcher.cpp
+++ b/src/mongo/client/fetcher.cpp
@@ -79,13 +79,11 @@ Status parseCursorResponse(const BSONObj& obj,
                       str::stream() << "cursor response must contain '" << kCursorFieldName << "."
                                     << kCursorIdFieldName << "' field: " << obj);
     }
-    if (!(cursorIdElement.type() == mongo::NumberLong ||
-          cursorIdElement.type() == mongo::NumberInt)) {
+    if (cursorIdElement.type() != mongo::NumberLong) {
         return Status(ErrorCodes::FailedToParse,
-                      str::stream()
-                          << "'" << kCursorFieldName << "." << kCursorIdFieldName
-                          << "' field must be a integral number of type 'int' or 'long' but was a '"
-                          << typeName(cursorIdElement.type()) << "': " << obj);
+                      str::stream() << "'" << kCursorFieldName << "." << kCursorIdFieldName
+                                    << "' field must be a 'long' but was a '"
+                                    << typeName(cursorIdElement.type()) << "': " << obj);
     }
     batchData->cursorId = cursorIdElement.numberLong();
 
diff --git a/src/mongo/db/repl/data_replicator_test.cpp b/src/mongo/db/repl/data_replicator_test.cpp
index aae1d9cc52b..7116930557e 100644
--- a/src/mongo/db/repl/data_replicator_test.cpp
+++ b/src/mongo/db/repl/data_replicator_test.cpp
@@ -312,12 +312,12 @@ protected:
             const long long cursorId = cmdElem.numberLong();
             if (isGetMore && cursorId == 1LL) {
                 // process getmore requests from the oplog fetcher
-                auto respBSON = fromjson(str::stream()
-                                         << "{ok:1, cursor:{id:1, ns:'local.oplog.rs', nextBatch:["
-                                            "{ts:Timestamp(" << ++c
-                                         << ",1), h:1, ns:'test.a', v:2, op:'u', o2:{_id:" << c
-                                         << "}, o:{$set:{a:1}}}"
-                                            "]}}");
+                auto respBSON =
+                    fromjson(str::stream() << "{ok:1, cursor:{id:NumberLong(1), ns:'local.oplog.rs'"
+                                              " , nextBatch:[{ts:Timestamp(" << ++c
+                                           << ",1), h:1, ns:'test.a', v:2, op:'u', o2:{_id:" << c
+                                           << "}, o:{$set:{a:1}}}"
+                                              "]}}");
                 net->scheduleResponse(
                     noi,
                     net->now(),
@@ -354,7 +354,7 @@ protected:
     }
 
     void verifySync(Status s = Status::OK()) {
-        ASSERT_EQ(_isbr->getResult().getStatus(), s) << "status objects differ";
+        verifySync(s.code());
     }
 
     void verifySync(ErrorCodes::Error code) {
@@ -392,12 +392,12 @@ TEST_F(InitialSyncTest, Complete) {
     const std::vector<BSONObj> responses = {
         // get latest oplog ts
         fromjson(
-            "{ok:1, cursor:{id:0, ns:'local.oplog.rs', firstBatch:["
+            "{ok:1, cursor:{id:NumberLong(0), ns:'local.oplog.rs', firstBatch:["
             "{ts:Timestamp(1,1), h:1, ns:'a.a', v:2, op:'i', o:{_id:1, a:1}}"
             "]}}"),
         // oplog fetcher find
         fromjson(
-            "{ok:1, cursor:{id:1, ns:'local.oplog.rs', firstBatch:["
+            "{ok:1, cursor:{id:NumberLong(1), ns:'local.oplog.rs', firstBatch:["
             "{ts:Timestamp(1,1), h:1, ns:'a.a', v:2, op:'i', o:{_id:1, a:1}}"
             "]}}"),
         // Clone Start
@@ -405,23 +405,23 @@ TEST_F(InitialSyncTest, Complete) {
         fromjson("{ok:1, databases:[{name:'a'}]}"),
         // listCollections for "a"
         fromjson(
-            "{ok:1, cursor:{id:0, ns:'a.$cmd.listCollections', firstBatch:["
+            "{ok:1, cursor:{id:NumberLong(0), ns:'a.$cmd.listCollections', firstBatch:["
             "{name:'a', options:{}} "
             "]}}"),
         // listIndexes:a
         fromjson(
-            "{ok:1, cursor:{id:0, ns:'a.$cmd.listIndexes.a', firstBatch:["
+            "{ok:1, cursor:{id:NumberLong(0), ns:'a.$cmd.listIndexes.a', firstBatch:["
             "{v:1, key:{_id:1}, name:'_id_', ns:'a.a'}"
             "]}}"),
         // find:a
         fromjson(
-            "{ok:1, cursor:{id:0, ns:'a.a', firstBatch:["
+            "{ok:1, cursor:{id:NumberLong(0), ns:'a.a', firstBatch:["
             "{_id:1, a:1} "
             "]}}"),
         // Clone Done
         // get latest oplog ts
         fromjson(
-            "{ok:1, cursor:{id:0, ns:'local.oplog.rs', firstBatch:["
+            "{ok:1, cursor:{id:NumberLong(0), ns:'local.oplog.rs', firstBatch:["
             "{ts:Timestamp(2,2), h:1, ns:'b.c', v:2, op:'i', o:{_id:1, c:1}}"
             "]}}"),
         // Applier starts ...
@@ -445,12 +445,12 @@ TEST_F(InitialSyncTest, MissingDocOnApplyCompletes) {
     const std::vector<BSONObj> responses = {
         // get latest oplog ts
         fromjson(
-            "{ok:1, cursor:{id:0, ns:'local.oplog.rs', firstBatch:["
+            "{ok:1, cursor:{id:NumberLong(0), ns:'local.oplog.rs', firstBatch:["
             "{ts:Timestamp(1,1), h:1, ns:'a.a', v:2, op:'i', o:{_id:1, a:1}}"
             "]}}"),
         // oplog fetcher find
         fromjson(
-            "{ok:1, cursor:{id:1, ns:'local.oplog.rs', firstBatch:["
+            "{ok:1, cursor:{id:NumberLong(1), ns:'local.oplog.rs', firstBatch:["
             "{ts:Timestamp(1,1), h:1, ns:'a.a', v:2, op:'u', o2:{_id:1}, o:{$set:{a:1}}}"
             "]}}"),
         // Clone Start
@@ -458,26 +458,26 @@ TEST_F(InitialSyncTest, MissingDocOnApplyCompletes) {
         fromjson("{ok:1, databases:[{name:'a'}]}"),
         // listCollections for "a"
         fromjson(
-            "{ok:1, cursor:{id:0, ns:'a.$cmd.listCollections', firstBatch:["
+            "{ok:1, cursor:{id:NumberLong(0), ns:'a.$cmd.listCollections', firstBatch:["
             "{name:'a', options:{}} "
             "]}}"),
         // listIndexes:a
         fromjson(
-            "{ok:1, cursor:{id:0, ns:'a.$cmd.listIndexes.a', firstBatch:["
+            "{ok:1, cursor:{id:NumberLong(0), ns:'a.$cmd.listIndexes.a', firstBatch:["
             "{v:1, key:{_id:1}, name:'_id_', ns:'a.a'}"
             "]}}"),
         // find:a -- empty
-        fromjson("{ok:1, cursor:{id:0, ns:'a.a', firstBatch:[]}}"),
+        fromjson("{ok:1, cursor:{id:NumberLong(0), ns:'a.a', firstBatch:[]}}"),
         // Clone Done
         // get latest oplog ts
         fromjson(
-            "{ok:1, cursor:{id:0, ns:'local.oplog.rs', firstBatch:["
+            "{ok:1, cursor:{id:NumberLong(0), ns:'local.oplog.rs', firstBatch:["
             "{ts:Timestamp(2,2), h:1, ns:'b.c', v:2, op:'i', o:{_id:1, c:1}}"
             "]}}"),
         // Applier starts ...
         // missing doc fetch -- find:a {_id:1}
         fromjson(
-            "{ok:1, cursor:{id:0, ns:'a.a', firstBatch:["
+            "{ok:1, cursor:{id:NumberLong(0), ns:'a.a', firstBatch:["
             "{_id:1, a:1} "
             "]}}"),
     };
@@ -521,12 +521,12 @@ TEST_F(InitialSyncTest, FailsOnClone) {
     const std::vector<BSONObj> responses = {
         // get latest oplog ts
         fromjson(
-            "{ok:1, cursor:{id:0, ns:'local.oplog.rs', firstBatch:["
+            "{ok:1, cursor:{id:NumberLong(0), ns:'local.oplog.rs', firstBatch:["
             "{ts:Timestamp(1,1), h:1, ns:'a.a', v:2, op:'i', o:{_id:1, a:1}}"
             "]}}"),
         // oplog fetcher find
         fromjson(
-            "{ok:1, cursor:{id:1, ns:'local.oplog.rs', firstBatch:["
+            "{ok:1, cursor:{id:NumberLong(1), ns:'local.oplog.rs', firstBatch:["
             "{ts:Timestamp(1,1), h:1, ns:'a.a', v:2, op:'i', o:{_id:1, a:1}}"
             "]}}"),
         // Clone Start
@@ -728,7 +728,7 @@ TEST_F(SteadyStateTest, ChooseNewSyncSourceAfterFailedNetworkRequest) {
 TEST_F(SteadyStateTest, RemoteOplogEmptyRollbackSucceeded) {
     _setUpOplogFetcherFailed();
     auto oplogFetcherResponse =
-        fromjson("{ok:1, cursor:{id:0, ns:'local.oplog.rs', firstBatch: []}}");
+        fromjson("{ok:1, cursor:{id:NumberLong(0), ns:'local.oplog.rs', firstBatch: []}}");
     _testOplogFetcherFailed(oplogFetcherResponse,
                             Status::OK(),
                             HostAndPort("host-0", -1),  // rollback source
@@ -742,7 +742,7 @@ TEST_F(SteadyStateTest, RemoteOplogEmptyRollbackSucceeded) {
 TEST_F(SteadyStateTest, RemoteOplogEmptyRollbackFailed) {
     _setUpOplogFetcherFailed();
     auto oplogFetcherResponse =
-        fromjson("{ok:1, cursor:{id:0, ns:'local.oplog.rs', firstBatch: []}}");
+        fromjson("{ok:1, cursor:{id:NumberLong(0), ns:'local.oplog.rs', firstBatch: []}}");
     _testOplogFetcherFailed(oplogFetcherResponse,
                             Status(ErrorCodes::OperationFailed, "rollback failed"),
                             HostAndPort("host-0", -1),  // rollback source
@@ -756,7 +756,7 @@ TEST_F(SteadyStateTest, RemoteOplogEmptyRollbackFailed) {
 TEST_F(SteadyStateTest, RemoteOplogFirstOperationMissingTimestampRollbackFailed) {
     _setUpOplogFetcherFailed();
     auto oplogFetcherResponse =
-        fromjson("{ok:1, cursor:{id:0, ns:'local.oplog.rs', firstBatch: [{}]}}");
+        fromjson("{ok:1, cursor:{id:NumberLong(0), ns:'local.oplog.rs', firstBatch: [{}]}}");
     _testOplogFetcherFailed(oplogFetcherResponse,
                             Status(ErrorCodes::OperationFailed, "rollback failed"),
                             HostAndPort("host-0", -1),  // rollback source
@@ -769,8 +769,8 @@ TEST_F(SteadyStateTest, RemoteOplogFirstOperationMissingTimestampRollbackFailed)
 
 TEST_F(SteadyStateTest, RemoteOplogFirstOperationTimestampDoesNotMatchRollbackFailed) {
     _setUpOplogFetcherFailed();
-    auto oplogFetcherResponse =
-        fromjson("{ok:1, cursor:{id:0, ns:'local.oplog.rs', firstBatch: [{ts:Timestamp(1,1)}]}}");
+    auto oplogFetcherResponse = fromjson(
+        "{ok:1, cursor:{id:NumberLong(0), ns:'local.oplog.rs', firstBatch:[{ts:Timestamp(1,1)}]}}");
     _testOplogFetcherFailed(oplogFetcherResponse,
                             Status(ErrorCodes::OperationFailed, "rollback failed"),
                             HostAndPort("host-0", -1),  // rollback source
@@ -784,7 +784,7 @@ TEST_F(SteadyStateTest, RemoteOplogFirstOperationTimestampDoesNotMatchRollbackFa
 TEST_F(SteadyStateTest, RollbackTwoSyncSourcesBothFailed) {
     _setUpOplogFetcherFailed();
     auto oplogFetcherResponse =
-        fromjson("{ok:1, cursor:{id:0, ns:'local.oplog.rs', firstBatch: []}}");
+        fromjson("{ok:1, cursor:{id:NumberLong(0), ns:'local.oplog.rs', firstBatch: []}}");
 
     _testOplogFetcherFailed(oplogFetcherResponse,
                             Status(ErrorCodes::OperationFailed, "rollback failed"),
@@ -808,7 +808,7 @@ TEST_F(SteadyStateTest, RollbackTwoSyncSourcesBothFailed) {
 TEST_F(SteadyStateTest, RollbackTwoSyncSourcesSecondRollbackSucceeds) {
     _setUpOplogFetcherFailed();
     auto oplogFetcherResponse =
-        fromjson("{ok:1, cursor:{id:0, ns:'local.oplog.rs', firstBatch: []}}");
+        fromjson("{ok:1, cursor:{id:NumberLong(0), ns:'local.oplog.rs', firstBatch: []}}");
 
     _testOplogFetcherFailed(oplogFetcherResponse,
                             Status(ErrorCodes::OperationFailed, "rollback failed"),
diff --git a/src/mongo/dbtests/jsontests.cpp b/src/mongo/dbtests/jsontests.cpp
index 199bbde4881..b5bf68b6af6 100644
--- a/src/mongo/dbtests/jsontests.cpp
+++ b/src/mongo/dbtests/jsontests.cpp
@@ -163,6 +163,15 @@ public:
     }
 };
 
+class NumberLongStrictZero {
+public:
+    void run() {
+        BSONObjBuilder b;
+        b.append("a", 0LL);
+        ASSERT_EQUALS("{ \"a\" : { \"$numberLong\" : \"0\" } }", b.done().jsonString(Strict));
+    }
+};
+
 class NumberLongStrict {
 public:
     void run() {
@@ -1698,7 +1707,7 @@ class DateNegative : public Base {
 class NumberLongTest : public Base {
     virtual BSONObj bson() const {
         BSONObjBuilder b;
-        b.appendNumber("a", 20000LL);
+        b.append("a", 20000LL);
         return b.obj();
     }
     virtual string json() const {
@@ -1709,7 +1718,7 @@ class NumberLongTest : public Base {
 class NumberLongMin : public Base {
     virtual BSONObj bson() const {
         BSONObjBuilder b;
-        b.appendNumber("a", std::numeric_limits<long long>::min());
+        b.append("a", std::numeric_limits<long long>::min());
         return b.obj();
     }
     virtual string json() const {
@@ -1735,7 +1744,7 @@ class NumberIntTest : public Base {
 class NumberLongNeg : public Base {
     virtual BSONObj bson() const {
         BSONObjBuilder b;
-        b.appendNumber("a", -20000LL);
+        b.append("a", -20000LL);
         return b.obj();
     }
     virtual string json() const {
@@ -2477,7 +2486,7 @@ public:
 class NumericLongMin : public Base {
     virtual BSONObj bson() const {
         BSONObjBuilder b;
-        b.appendNumber("a", std::numeric_limits<long long>::min());
+        b.append("a", std::numeric_limits<long long>::min());
         return b.obj();
     }
     virtual string json() const {
@@ -2682,6 +2691,7 @@ public:
         add<JsonStringTests::InvalidNumbers>();
         add<JsonStringTests::NumberPrecision>();
         add<JsonStringTests::NegativeNumber>();
+        add<JsonStringTests::NumberLongStrictZero>();
         add<JsonStringTests::NumberLongStrict>();
         add<JsonStringTests::NumberLongStrictLarge>();
         add<JsonStringTests::NumberLongStrictNegative>();