SERVER-29334 remove dotted field name check from BSONObj::okForStorage

4 files changed, 11 insertions, 100 deletions

diff --git a/src/mongo/bson/bsonobj.cpp b/src/mongo/bson/bsonobj.cpp
index d9af2c706a9..2f7133a2a63 100644
--- a/src/mongo/bson/bsonobj.cpp
+++ b/src/mongo/bson/bsonobj.cpp
@@ -376,7 +376,7 @@ BSONObj BSONObj::replaceFieldNames(const BSONObj& names) const {
     return b.obj();
 }
 
-Status BSONObj::_okForStorage(bool root, bool deep) const {
+Status BSONObj::storageValidEmbedded() const {
     BSONObjIterator i(*this);
 
     // The first field is special in the case of a DBRef where the first field must be $ref
@@ -413,39 +413,23 @@ Status BSONObj::_okForStorage(bool root, bool deep) const {
             }
         }
 
-        // Do not allow "." in the field name
-        if (strchr(name, '.')) {
-            return Status(ErrorCodes::DottedFieldName,
-                          str::stream() << name << " is not valid for storage.");
-        }
-
-        // (SERVER-9502) Do not allow storing an _id field with a RegEx type or
-        // Array type in a root document
-        if (root && (e.type() == RegEx || e.type() == Array || e.type() == Undefined) &&
-            str::equals(name, "_id")) {
-            return Status(ErrorCodes::InvalidIdField,
-                          str::stream() << name
-                                        << " is not valid for storage because it is of type "
-                                        << typeName(e.type()));
-        }
-
-        if (deep && e.mayEncapsulate()) {
+        if (e.mayEncapsulate()) {
             switch (e.type()) {
                 case Object:
                 case Array: {
-                    Status s = e.embeddedObject()._okForStorage(false, true);
+                    Status s = e.embeddedObject().storageValidEmbedded();
                     // TODO: combine field names for better error messages
                     if (!s.isOK())
                         return s;
                 } break;
                 case CodeWScope: {
-                    Status s = e.codeWScopeObject()._okForStorage(false, true);
+                    Status s = e.codeWScopeObject().storageValidEmbedded();
                     // TODO: combine field names for better error messages
                     if (!s.isOK())
                         return s;
                 } break;
                 default:
-                    uassert(12579, "unhandled cases in BSONObj okForStorage", 0);
+                    uassert(12579, "unhandled cases in BSONObj storageValidEmbedded", 0);
             }
         }
 
diff --git a/src/mongo/bson/bsonobj.h b/src/mongo/bson/bsonobj.h
index 22ab6ff182f..b1b3473c7d2 100644
--- a/src/mongo/bson/bsonobj.h
+++ b/src/mongo/bson/bsonobj.h
@@ -351,49 +351,11 @@ public:
         return x > 0 && x <= BSONObjMaxInternalSize;
     }
 
-    /** @return ok if it can be stored as a valid embedded doc.
-     *  Not valid if any field name:
-     *      - contains a "."
-     *      - starts with "$"
-     *          -- unless it is a dbref ($ref/$id/[$db]/...)
-     */
-    inline bool okForStorage() const {
-        return _okForStorage(false, true).isOK();
-    }
-
-    /** Same as above with the following extra restrictions
-     *  Not valid if:
-     *      - "_id" field is a
-     *          -- Regex
-     *          -- Array
-     */
-    inline bool okForStorageAsRoot() const {
-        return _okForStorage(true, true).isOK();
-    }
-
     /**
-     * Validates that this can be stored as an embedded document
-     * See details above in okForStorage
-     *
-     * If 'deep' is true then validation is done to children
-     *
-     * If not valid a user readable status message is returned.
+     * Validates that the element is okay to be stored in a collection.
+     * Recursively validates children.
      */
-    inline Status storageValidEmbedded(const bool deep = true) const {
-        return _okForStorage(false, deep);
-    }
-
-    /**
-     * Validates that this can be stored as a document (in a collection)
-     * See details above in okForStorageAsRoot
-     *
-     * If 'deep' is true then validation is done to children
-     *
-     * If not valid a user readable status message is returned.
-     */
-    inline Status storageValid(const bool deep = true) const {
-        return _okForStorage(true, deep);
-    }
+    Status storageValidEmbedded() const;
 
     /** @return true if object is empty -- i.e.,  {} */
     bool isEmpty() const {
@@ -605,14 +567,6 @@ private:
             _assertInvalid();
     }
 
-    /**
-     * Validate if the element is okay to be stored in a collection, maybe as the root element
-     *
-     * If 'root' is true then checks against _id are made.
-     * If 'deep' is false then do not traverse through children
-     */
-    Status _okForStorage(bool root, bool deep) const;
-
     const char* _objdata;
     ConstSharedBuffer _ownedBuffer;
 };
diff --git a/src/mongo/dbtests/jsobjtests.cpp b/src/mongo/dbtests/jsobjtests.cpp
index 917488bd1c0..beef2906811 100644
--- a/src/mongo/dbtests/jsobjtests.cpp
+++ b/src/mongo/dbtests/jsobjtests.cpp
@@ -2038,7 +2038,7 @@ public:
     }
 
     void good(BSONObj o) {
-        if (o.okForStorageAsRoot())
+        if (o.storageValidEmbedded().isOK())
             return;
         throw UserException(12528, (string) "should be ok for storage:" + o.toString());
     }
@@ -2048,7 +2048,7 @@ public:
     }
 
     void bad(BSONObj o) {
-        if (!o.okForStorageAsRoot())
+        if (!o.storageValidEmbedded().isOK())
             return;
         throw UserException(12529, (string) "should NOT be ok for storage:" + o.toString());
     }
@@ -2059,10 +2059,6 @@ public:
         good("{x:1}");
         good("{x:{a:2}}");
 
-        // no dots allowed
-        bad("{'x.y':1}");
-        bad("{'x\\.y':1}");
-
         // Check for $
         bad("{x:{'$a':2}}");
         good("{'a$b':2}");
@@ -2072,7 +2068,6 @@ public:
 
         // Queries are not ok
         bad("{num: {$gt: 1}}");
-        bad("{_id: {$regex:'test'}}");
         bad("{$gt: 2}");
         bad("{a : { oo: [ {$bad:1}, {good:1}] }}");
         good("{a : { oo: [ {'\\\\$good':1}, {good:1}] }}");
@@ -2127,28 +2122,6 @@ public:
                              << 1
                              << "$hater"
                              << 1)));
-        bad(BSON("a" << BSON("$ref"
-                             << "coll"
-                             << "$id"
-                             << 1
-                             << "dot.dot"
-                             << 1)));
-
-        // _id isn't a RegEx, or Array
-        good("{_id: 0}");
-        good("{_id: {a:1, b:1}}");
-        good("{_id: {rx: /a/}}");
-        good("{_id: {rx: {$regex: 'a'}}}");
-        bad("{_id: /a/ }");
-        bad("{_id: /a/, other:1}");
-        bad("{hi:1, _id: /a/ }");
-        bad("{_id: /a/i }");
-        bad("{first:/f/i, _id: /a/i }");
-        // Not really a regex type
-        bad("{_id: {$regex: 'a'} }");
-        bad("{_id: {$regex: 'a', $options:'i'} }");
-        bad("{_id:  [1,2]}");
-        bad("{_id:  [1]}");
     }
 };
 
diff --git a/src/mongo/s/shard_key_pattern.cpp b/src/mongo/s/shard_key_pattern.cpp
index 4cef9efdec4..c71a23f0133 100644
--- a/src/mongo/s/shard_key_pattern.cpp
+++ b/src/mongo/s/shard_key_pattern.cpp
@@ -107,7 +107,7 @@ bool isShardKeyElement(const BSONElement& element, bool allowRegex) {
     if (!allowRegex && element.type() == RegEx)
         return false;
 
-    if (element.type() == Object && !element.embeddedObject().okForStorage())
+    if (element.type() == Object && !element.embeddedObject().storageValidEmbedded().isOK())
         return false;
 
     return true;