SERVER-27263 BSONExtractWithDefault more efficient in the 'no such key' case

2 files changed, 159 insertions, 87 deletions

diff --git a/src/mongo/bson/util/bson_extract.cpp b/src/mongo/bson/util/bson_extract.cpp
index b6ea2f8853e..0ac6bc3468b 100644
--- a/src/mongo/bson/util/bson_extract.cpp
+++ b/src/mongo/bson/util/bson_extract.cpp
@@ -32,24 +32,34 @@
 
 namespace mongo {
 
-Status bsonExtractField(const BSONObj& object, StringData fieldName, BSONElement* outElement) {
+namespace {
+
+Status bsonExtractFieldImpl(const BSONObj& object,
+                            StringData fieldName,
+                            BSONElement* outElement,
+                            bool withDefault) {
     BSONElement element = object.getField(fieldName);
-    if (element.eoo())
-        return Status(ErrorCodes::NoSuchKey,
-                      mongoutils::str::stream() << "Missing expected field \""
-                                                << fieldName.toString()
-                                                << "\" in object '"
-                                                << object
-                                                << "'.");
-    *outElement = element;
-    return Status::OK();
+
+    if (!element.eoo()) {
+        *outElement = element;
+        return Status::OK();
+    }
+    if (withDefault) {
+        static const Status kDefaultCase(ErrorCodes::NoSuchKey,
+                                         "bsonExtractFieldImpl default case no such key error");
+        return kDefaultCase;
+    }
+    return Status(ErrorCodes::NoSuchKey,
+                  mongoutils::str::stream() << "Missing expected field \"" << fieldName.toString()
+                                            << "\"");
 }
 
-Status bsonExtractTypedField(const BSONObj& object,
-                             StringData fieldName,
-                             BSONType type,
-                             BSONElement* outElement) {
-    Status status = bsonExtractField(object, fieldName, outElement);
+Status bsonExtractTypedFieldImpl(const BSONObj& object,
+                                 StringData fieldName,
+                                 BSONType type,
+                                 BSONElement* outElement,
+                                 bool withDefault) {
+    Status status = bsonExtractFieldImpl(object, fieldName, outElement, withDefault);
     if (!status.isOK())
         return status;
     if (type != outElement->type()) {
@@ -60,141 +70,170 @@ Status bsonExtractTypedField(const BSONObj& object,
                                                 << ", found "
                                                 << typeName(outElement->type()));
     }
-    return Status::OK();
+    return status;
 }
 
-Status bsonExtractBooleanField(const BSONObj& object, StringData fieldName, bool* out) {
+Status bsonExtractIntegerFieldImpl(const BSONObj& object,
+                                   StringData fieldName,
+                                   long long* out,
+                                   bool withDefault) {
     BSONElement element;
-    Status status = bsonExtractTypedField(object, fieldName, Bool, &element);
+    Status status = bsonExtractFieldImpl(object, fieldName, &element, withDefault);
     if (!status.isOK())
         return status;
-    *out = element.boolean();
-    return Status::OK();
+    if (!element.isNumber()) {
+        return Status(ErrorCodes::TypeMismatch,
+                      mongoutils::str::stream() << "Expected field \"" << fieldName
+                                                << "\" to have numeric type, but found "
+                                                << typeName(element.type()));
+    }
+    long long result = element.safeNumberLong();
+    if (result != element.numberDouble()) {
+        return Status(
+            ErrorCodes::BadValue,
+            mongoutils::str::stream() << "Expected field \"" << fieldName
+                                      << "\" to have a value "
+                                         "exactly representable as a 64-bit integer, but found "
+                                      << element);
+    }
+    *out = result;
+    return status;
+}
+
+Status bsonExtractDoubleFieldImpl(const BSONObj& object,
+                                  StringData fieldName,
+                                  double* out,
+                                  bool withDefault) {
+    BSONElement element;
+    Status status = bsonExtractField(object, fieldName, &element);
+    if (!status.isOK())
+        return status;
+    if (!element.isNumber()) {
+        return Status(ErrorCodes::TypeMismatch,
+                      mongoutils::str::stream() << "Expected field \"" << fieldName
+                                                << "\" to have numeric type, but found "
+                                                << typeName(element.type()));
+    }
+    *out = element.numberDouble();
+    return status;
+}
+}  // namespace
+
+
+Status bsonExtractField(const BSONObj& object, StringData fieldName, BSONElement* outElement) {
+    return bsonExtractFieldImpl(object, fieldName, outElement, false);
+}
+
+Status bsonExtractTypedField(const BSONObj& object,
+                             StringData fieldName,
+                             BSONType type,
+                             BSONElement* outElement) {
+    return bsonExtractTypedFieldImpl(object, fieldName, type, outElement, false);
+}
+
+Status bsonExtractBooleanField(const BSONObj& object, StringData fieldName, bool* out) {
+    BSONElement element;
+    Status status = bsonExtractTypedField(object, fieldName, Bool, &element);
+    if (status.isOK())
+        *out = element.boolean();
+    return status;
 }
 
 Status bsonExtractBooleanFieldWithDefault(const BSONObj& object,
                                           StringData fieldName,
                                           bool defaultValue,
                                           bool* out) {
-    BSONElement value;
-    Status status = bsonExtractField(object, fieldName, &value);
+    BSONElement element;
+    Status status = bsonExtractFieldImpl(object, fieldName, &element, true);
     if (status == ErrorCodes::NoSuchKey) {
         *out = defaultValue;
         return Status::OK();
-    } else if (!status.isOK()) {
+    }
+
+    if (!status.isOK())
         return status;
-    } else if (!value.isNumber() && !value.isBoolean()) {
+
+    if (!element.isNumber() && !element.isBoolean()) {
         return Status(ErrorCodes::TypeMismatch,
                       mongoutils::str::stream() << "Expected boolean or number type for field \""
                                                 << fieldName
                                                 << "\", found "
-                                                << typeName(value.type()));
-    } else {
-        *out = value.trueValue();
-        return Status::OK();
+                                                << typeName(element.type()));
     }
+    *out = element.trueValue();
+    return status;
 }
 
 Status bsonExtractStringField(const BSONObj& object, StringData fieldName, std::string* out) {
     BSONElement element;
     Status status = bsonExtractTypedField(object, fieldName, String, &element);
-    if (!status.isOK())
-        return status;
-    *out = element.str();
-    return Status::OK();
+    if (status.isOK())
+        *out = element.str();
+    return status;
 }
 
 Status bsonExtractTimestampField(const BSONObj& object, StringData fieldName, Timestamp* out) {
     BSONElement element;
     Status status = bsonExtractTypedField(object, fieldName, bsonTimestamp, &element);
-    if (!status.isOK())
-        return status;
-    *out = element.timestamp();
-    return Status::OK();
+    if (status.isOK())
+        *out = element.timestamp();
+    return status;
 }
 
 Status bsonExtractOIDField(const BSONObj& object, StringData fieldName, OID* out) {
     BSONElement element;
     Status status = bsonExtractTypedField(object, fieldName, jstOID, &element);
-    if (!status.isOK())
-        return status;
-    *out = element.OID();
-    return Status::OK();
+    if (status.isOK())
+        *out = element.OID();
+    return status;
 }
 
 Status bsonExtractOIDFieldWithDefault(const BSONObj& object,
                                       StringData fieldName,
                                       const OID& defaultValue,
                                       OID* out) {
-    Status status = bsonExtractOIDField(object, fieldName, out);
+    BSONElement element;
+    Status status = bsonExtractTypedFieldImpl(object, fieldName, jstOID, &element, true);
     if (status == ErrorCodes::NoSuchKey) {
         *out = defaultValue;
-    } else if (!status.isOK()) {
-        return status;
+        return Status::OK();
     }
-    return Status::OK();
+    if (status.isOK())
+        *out = element.OID();
+    return status;
 }
 
 Status bsonExtractStringFieldWithDefault(const BSONObj& object,
                                          StringData fieldName,
                                          StringData defaultValue,
                                          std::string* out) {
-    Status status = bsonExtractStringField(object, fieldName, out);
+    BSONElement element;
+    Status status = bsonExtractTypedFieldImpl(object, fieldName, String, &element, true);
     if (status == ErrorCodes::NoSuchKey) {
         *out = defaultValue.toString();
-    } else if (!status.isOK()) {
-        return status;
+        return Status::OK();
     }
-    return Status::OK();
+    if (status.isOK())
+        *out = element.str();
+    return status;
 }
 
 Status bsonExtractIntegerField(const BSONObj& object, StringData fieldName, long long* out) {
-    BSONElement value;
-    Status status = bsonExtractField(object, fieldName, &value);
-    if (!status.isOK())
-        return status;
-    if (!value.isNumber()) {
-        return Status(ErrorCodes::TypeMismatch,
-                      mongoutils::str::stream() << "Expected field \"" << fieldName
-                                                << "\" to have numeric type, but found "
-                                                << typeName(value.type()));
-    }
-    long long result = value.safeNumberLong();
-    if (result != value.numberDouble()) {
-        return Status(
-            ErrorCodes::BadValue,
-            mongoutils::str::stream() << "Expected field \"" << fieldName
-                                      << "\" to have a value "
-                                         "exactly representable as a 64-bit integer, but found "
-                                      << value);
-    }
-    *out = result;
-    return Status::OK();
+    return bsonExtractIntegerFieldImpl(object, fieldName, out, false);
 }
 
 Status bsonExtractDoubleField(const BSONObj& object, StringData fieldName, double* out) {
-    BSONElement value;
-    Status status = bsonExtractField(object, fieldName, &value);
-    if (!status.isOK())
-        return status;
-    if (!value.isNumber()) {
-        return Status(ErrorCodes::TypeMismatch,
-                      mongoutils::str::stream() << "Expected field \"" << fieldName
-                                                << "\" to have numeric type, but found "
-                                                << typeName(value.type()));
-    }
-    *out = value.numberDouble();
-    return Status::OK();
+    return bsonExtractDoubleFieldImpl(object, fieldName, out, false);
 }
 
 Status bsonExtractDoubleFieldWithDefault(const BSONObj& object,
                                          StringData fieldName,
                                          double defaultValue,
                                          double* out) {
-    Status status = bsonExtractDoubleField(object, fieldName, out);
+    Status status = bsonExtractDoubleFieldImpl(object, fieldName, out, true);
     if (status == ErrorCodes::NoSuchKey) {
         *out = defaultValue;
-        status = Status::OK();
+        return Status::OK();
     }
     return status;
 }
@@ -203,10 +242,10 @@ Status bsonExtractIntegerFieldWithDefault(const BSONObj& object,
                                           StringData fieldName,
                                           long long defaultValue,
                                           long long* out) {
-    Status status = bsonExtractIntegerField(object, fieldName, out);
+    Status status = bsonExtractIntegerFieldImpl(object, fieldName, out, true);
     if (status == ErrorCodes::NoSuchKey) {
         *out = defaultValue;
-        status = Status::OK();
+        return Status::OK();
     }
     return status;
 }
@@ -217,7 +256,7 @@ Status bsonExtractIntegerFieldWithDefaultIf(const BSONObj& object,
                                             stdx::function<bool(long long)> pred,
                                             const std::string& predDescription,
                                             long long* out) {
-    auto status = bsonExtractIntegerFieldWithDefault(object, fieldName, defaultValue, out);
+    Status status = bsonExtractIntegerFieldWithDefault(object, fieldName, defaultValue, out);
     if (!status.isOK()) {
         return status;
     }
@@ -228,7 +267,7 @@ Status bsonExtractIntegerFieldWithDefaultIf(const BSONObj& object,
                                       << ": "
                                       << predDescription);
     }
-    return Status::OK();
+    return status;
 }
 
 Status bsonExtractIntegerFieldWithDefaultIf(const BSONObj& object,
diff --git a/src/mongo/bson/util/bson_extract_test.cpp b/src/mongo/bson/util/bson_extract_test.cpp
index 9ef6a448e80..7681fbbf7a4 100644
--- a/src/mongo/bson/util/bson_extract_test.cpp
+++ b/src/mongo/bson/util/bson_extract_test.cpp
@@ -150,3 +150,36 @@ TEST(ExtractBSON, ExtractIntegerField) {
     ASSERT_EQUALS(ErrorCodes::BadValue,
                   bsonExtractIntegerFieldWithDefaultIf(BSON("a" << 1), "b", -1LL, pred, &v));
 }
+
+TEST(ExtractBSON, ExtractDoubleFieldWithDefault) {
+    double d;
+    ASSERT_EQUALS(ErrorCodes::NoSuchKey, bsonExtractDoubleField(BSON("a" << 1), "b", &d));
+    ASSERT_OK(bsonExtractDoubleFieldWithDefault(BSON("a" << 1), "b", 1.2, &d));
+    ASSERT_EQUALS(ErrorCodes::TypeMismatch, bsonExtractDoubleField(BSON("a" << false), "a", &d));
+    ASSERT_OK(bsonExtractDoubleField(BSON("a" << 5178.0), "a", &d));
+    ASSERT_EQUALS(5178.0, d);
+    ASSERT_OK(bsonExtractDoubleField(BSON("a" << 5178), "a", &d));
+    ASSERT_EQUALS(5178, d);
+    ASSERT_OK(bsonExtractDoubleField(BSON("a" << 5178), "a", &d));
+    ASSERT_EQUALS(5178.0, d);
+    ASSERT_OK(bsonExtractDoubleField(BSON("a" << 5178.0), "a", &d));
+    ASSERT_EQUALS(5178, d);
+    ASSERT_OK(bsonExtractDoubleFieldWithDefault(BSON("a" << 1.0), "b", 1, &d));
+    ASSERT_EQUALS(1.0, d);
+    ASSERT_OK(bsonExtractDoubleFieldWithDefault(BSON("a" << 2.0), "a", 1, &d));
+    ASSERT_EQUALS(2.0, d);
+}
+
+TEST(ExtractBSON, ExtractOIDFieldWithDefault) {
+    OID r;
+    OID def = OID();
+    ASSERT_EQUALS(ErrorCodes::NoSuchKey, bsonExtractOIDField(BSON("a" << 1), "b", &r));
+    ASSERT_OK(bsonExtractOIDFieldWithDefault(BSON("a" << 2), "b", def, &r));
+    ASSERT_EQUALS(ErrorCodes::TypeMismatch, bsonExtractOIDField(BSON("a" << false), "a", &r));
+    ASSERT_OK(bsonExtractOIDField(BSON("a" << def), "a", &r));
+    ASSERT_EQUALS(def, r);
+    ASSERT_OK(bsonExtractOIDFieldWithDefault(BSON("a" << 3.0), "b", def, &r));
+    ASSERT_EQUALS(def, r);
+    ASSERT_OK(bsonExtractOIDFieldWithDefault(BSON("a" << def), "a", OID(), &r));
+    ASSERT_EQUALS(def, r);
+}