fix(plugin-mongodb): store fields named "" and top-level $ names on insert

CHANGELOG.md

@@ -569,6 +569,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - MongoDB collections could not be created from **New Table…**. (#3131)
 - A new or empty MongoDB collection showing only `_id` instead of the fields its validator declares.
 - MongoDB edits that stored dates and ObjectIds as text, rounded integers past 2^53, or missed a string `_id` that looks numeric.
+- MongoDB inserts from the query editor and the data grid failing on a field named `""`. (#3132)
 - MongoDB filters, validators and pipelines with a `$type` or `$regex` object refused as "not a document MongoDB can read".
 - **New Table…** offered on databases that cannot create a table, such as Redis and Kafka.
 - Executing indicator and Stop button carried over for a moment onto the query tab switched to.

Plugins/MongoDBDriverPlugin/MongoDBConnection+Documents.swift

@@ -38,12 +38,14 @@ extension MongoDBConnection {
         try await onClient { [self] client in
             let documentBson = try parsedBson(document)
             defer { bson_destroy(documentBson) }
+            let optionsBson = try parsedBson(MongoInsertOptions.json)
+            defer { bson_destroy(optionsBson) }
             let handle = try getCollection(client, database: database, collection: collection)
             defer { mongoc_collection_destroy(handle) }
             guard let reply = bson_new() else { throw MongoDBError.connectionFailed }
             defer { bson_destroy(reply) }
             var error = bson_error_t()
-            guard mongoc_collection_insert_one(handle, documentBson, nil, reply, &error) else {
+            guard mongoc_collection_insert_one(handle, documentBson, optionsBson, reply, &error) else {
                 if let failure = (try? canonicalText(of: reply)).flatMap(MongoWriteFailure.read(fromReply:)) {
                     throw MongoDBError(code: failure.code, message: failure.message)
                 }

Plugins/MongoDBDriverPlugin/MongoDBConnection+ScriptHelpers.swift

@@ -178,6 +178,11 @@ extension MongoDBConnection {
             identifiers.append("{\"$oid\": \"\(hex)\"}")
         }
 
+        guard let optsBson = jsonToBson(MongoInsertOptions.json) else {
+            throw MongoDBError(code: 0, message: MongoScriptText.invalidDocument(MongoInsertOptions.json))
+        }
+        defer { bson_destroy(optsBson) }
+
         try checkCancelled()
 
         var pointers: [OpaquePointer?] = prepared.map { Optional($0) }
@@ -187,7 +192,7 @@ extension MongoDBConnection {
 
         let ok = pointers.withUnsafeMutableBufferPointer { buffer -> Bool in
             guard let base = buffer.baseAddress else { return false }
-            return mongoc_collection_insert_many(handle, base, buffer.count, nil, reply, &error)
+            return mongoc_collection_insert_many(handle, base, buffer.count, optsBson, reply, &error)
         }
         guard ok else { throw makeError(error) }
         return identifiers

Plugins/MongoDBDriverPlugin/MongoDocumentText.swift

@@ -28,7 +28,6 @@ struct MongoDocumentText: Equatable, Sendable {
         case trailingContent
         case malformed(line: Int, column: Int)
         case duplicateField(String)
-        case operatorField(String)
         case tooDeep
 
         var errorDescription: String? {
@@ -47,11 +46,6 @@ struct MongoDocumentText: Equatable, Sendable {
                 )
             case .duplicateField(let name):
                 return String(format: String(localized: "The field \u{201C}%@\u{201D} appears more than once."), name)
-            case .operatorField(let name):
-                return String(
-                    format: String(localized: "The top-level field \u{201C}%@\u{201D} starts with $, which MongoDB reads as an operator."),
-                    name
-                )
             case .tooDeep:
                 return String(
                     format: String(localized: "The document is nested more than %d levels deep."),
@@ -78,9 +72,6 @@ struct MongoDocumentText: Equatable, Sendable {
         guard case .object(let members) = try reader.readValue(depth: 1) else { throw Refusal.notAnObject }
         reader.skipWhitespace()
         guard reader.isAtEnd else { throw Refusal.trailingContent }
-        if let operatorField = members.first(where: { $0.key.hasPrefix("$") }) {
-            throw Refusal.operatorField(operatorField.key)
-        }
         self.members = members
     }
 

Plugins/MongoDBDriverPlugin/MongoInsertOptions.swift

@@ -0,0 +1,21 @@
+//
+//  MongoInsertOptions.swift
+//  MongoDBDriverPlugin
+//
+
+import Foundation
+
+/// The options every insert hands libmongoc: Insert Document's, and those behind a script's
+/// `insertOne`, `insertMany`, `insert` and `save`, which is also how the grid writes a new row.
+///
+/// libmongoc checks a document's keys before it sends an insert, and its default check refuses a
+/// field named "" at any depth with "invalid document for insert: empty key". The server stores
+/// such a field, and mongosh writes one. `validate` is that default without the empty-name flag,
+/// libbson's `BSON_VALIDATE_UTF8` and `BSON_VALIDATE_UTF8_ALLOW_NULL`, so a key that is not UTF-8
+/// is still refused. It is never `false` or 0, which turns every check off, and for a replace that
+/// includes the one keeping an update operator out of the replacement.
+enum MongoInsertOptions {
+    static let validation = 1 | 8
+
+    static let json = #"{"validate":\#(validation)}"#
+}

Plugins/MongoDBDriverPlugin/MongoScriptPrelude.swift

@@ -42,6 +42,14 @@ enum MongoScriptPrelude {
             target[key] = value;
         }
     }
+
+    function __document(value) {
+        var document = {};
+        for (var key in value) {
+            if (Object.prototype.hasOwnProperty.call(value, key)) { __setMember(document, key, EJSON.__fromServer(value[key])); }
+        }
+        return document;
+    }
     """
 
     private static let values = """
@@ -418,7 +426,7 @@ enum MongoScriptPrelude {
         if (this.__exhausted) { return false; }
         this.__started = true;
         var page = __tp.call({ op: "cursorFetch", handle: this.__handle });
-        this.__batch = EJSON.__fromServer(page.docs);
+        this.__batch = page.docs.map(__document);
         this.__index = 0;
         this.__exhausted = page.done;
         return this.__batch.length > 0;
@@ -609,13 +617,13 @@ enum MongoScriptPrelude {
         if (!remove && (change === undefined || change === null)) {
             throw new Error("findOneAndUpdate needs an update document or pipeline");
         }
-        var reply = this.__reply("findAndModify", {
+        var reply = this.__call("findAndModify", {
             filter: __ejson(filter === undefined ? {} : filter),
             update: change === undefined ? null : __ejson(change),
             options: options === undefined ? null : __ejson(options),
             remove: remove
         });
-        return reply.value === undefined ? null : reply.value;
+        return reply.value === undefined || reply.value === null ? null : __document(reply.value);
     };
     DBCollection.prototype.findOneAndUpdate = function (filter, update, options) {
         return this.__findAndModify(filter, update, options, false);

TablePro/Resources/Localizable.xcstrings

@@ -184246,9 +184246,6 @@
     },
     "The field “%@” appears more than once." : {
 
-    },
-    "The top-level field “%@” starts with $, which MongoDB reads as an operator." : {
-
     },
     "The document is nested more than %d levels deep." : {
 

TableProTests/Core/MongoDB/MongoScriptPreludeTests.swift

@@ -318,6 +318,46 @@ struct MongoScriptPreludeTests {
         #expect(value?.toString() == "507f1f77bcf86cd799439011")
     }
 
+    @Test("A document whose only field is named like a wrapper reads back as that document")
+    func wrapperNamedDocumentStaysADocument() throws {
+        let host = RecordingHost()
+        host.replies = [
+            "1",
+            "{\"docs\": [{\"$oid\": \"not an id\"}, {\"$date\": \"2024\"}], \"done\": true}"
+        ]
+        let context = try makeContext(host)
+
+        let value = context.evaluateScript("""
+        db.orders.find({}, {_id: 0}).toArray().map(function (d) { return Object.keys(d)[0] + "=" + d[Object.keys(d)[0]]; }).join(",")
+        """)
+        #expect(context.exception == nil)
+        #expect(value?.toString() == "$oid=not an id,$date=2024")
+    }
+
+    @Test("A findOneAndUpdate document named like a wrapper comes back as that document")
+    func findAndModifyDocumentStaysADocument() throws {
+        let host = RecordingHost()
+        host.replies = ["{\"value\": {\"$oid\": \"507f1f77bcf86cd799439011\"}}"]
+        let context = try makeContext(host)
+
+        let value = context.evaluateScript("""
+        var found = db.orders.findOneAndUpdate({}, {$set: {a: 1}}, {projection: {_id: 0}}); typeof found.$oid
+        """)
+        #expect(context.exception == nil)
+        #expect(value?.toString() == "string")
+    }
+
+    @Test("A findOneAndUpdate that matches nothing returns null")
+    func findAndModifyWithNoMatchIsNull() throws {
+        let host = RecordingHost()
+        host.replies = ["{\"value\": null}"]
+        let context = try makeContext(host)
+
+        let value = context.evaluateScript("db.orders.findOneAndUpdate({}, {$set: {a: 1}}) === null")
+        #expect(context.exception == nil)
+        #expect(value?.toBool() == true)
+    }
+
     @Test("Variables and functions survive from one evaluated statement to the next")
     func shellStateSurvives() throws {
         let host = RecordingHost()

TableProTests/Plugins/MongoDocumentTextTests.swift

@@ -57,13 +57,22 @@ struct MongoDocumentTextTests {
         }
     }
 
-    @Test("A top-level field starting with $ is refused, a nested one is a wrapper")
-    func operatorFields() throws {
-        #expect(throws: MongoDocumentText.Refusal.operatorField("$set")) {
-            try MongoDocumentText(parsing: #"{"$set": {"a": 1}}"#)
+    @Test("An empty, blank, dotted or $-prefixed name reads as written, since an insert stores each one")
+    func namesAreReadAsWritten() throws {
+        let text = #"{"":1,"   ":2,"a.b":3,"$set":{"x":4},"$oid":"507f1f77bcf86cd799439011","n":{"":5}}"#
+        let document = try MongoDocumentText(parsing: text)
+        #expect(document.members.map(\.key) == ["", "   ", "a.b", "$set", "$oid", "n"])
+        #expect(document.compactText == text)
+
+        let wrapped = try MongoDocumentText(parsing: #"{"_id": {"$oid": "507f1f77bcf86cd799439011"}}"#)
+        #expect(wrapped.members.count == 1)
+    }
+
+    @Test("A field named \"\" is held to the rule against repeated fields")
+    func duplicateEmptyName() {
+        #expect(throws: MongoDocumentText.Refusal.duplicateField("")) {
+            try MongoDocumentText(parsing: #"{"": 1, "": 2}"#)
         }
-        let document = try MongoDocumentText(parsing: #"{"_id": {"$oid": "507f1f77bcf86cd799439011"}}"#)
-        #expect(document.members.count == 1)
     }
 
     @Test("Nesting deeper than MongoDB allows is refused without exhausting the stack")

TableProTests/Plugins/MongoDocumentWritePlanTests.swift

@@ -54,6 +54,22 @@ struct MongoDocumentWritePlanTests {
         #expect(!asked)
     }
 
+    @Test("Every name the server stores on insert reaches libbson and is sent as written")
+    func namesTheServerStores() throws {
+        let text = #"{"":1,"a":{"":2},"$set":{"x":1},"a.b":3,"$ref":"c","$id":1}"#
+        var asked = false
+        let spy: (String) throws -> String = { text in
+            asked = true
+            return text
+        }
+        let plan = try MongoDocumentWritePlan.make(
+            collection: "events", operation: .insert(document: text), canonicalize: spy
+        )
+        #expect(asked)
+        #expect(plan.document == text)
+        #expect(plan.statement == "db.events.insertOne(\(text))")
+    }
+
     @Test("A wrapper libbson cannot read is refused with libbson's reason")
     func libbsonRefusal() {
         struct Unreadable: Error {}

TableProTests/Plugins/MongoInsertOptionsTests.swift

@@ -0,0 +1,52 @@
+//
+//  MongoInsertOptionsTests.swift
+//  TableProTests
+//
+
+import Foundation
+import Testing
+
+struct MongoInsertOptionsTests {
+    /// libbson 1.28.1's `bson_validate_flags_t`, which the test target cannot import.
+    private enum LibbsonValidation {
+        static let utf8 = 1 << 0
+        static let dollarKeys = 1 << 1
+        static let dotKeys = 1 << 2
+        static let utf8AllowNull = 1 << 3
+        static let emptyKeys = 1 << 4
+    }
+
+    /// `_mongoc_default_insert_vflags` in libmongoc 1.28.1, used when an insert passes no options.
+    private let libmongocInsertDefault = LibbsonValidation.utf8 | LibbsonValidation.utf8AllowNull
+        | LibbsonValidation.emptyKeys
+
+    private func validate(in json: String) throws -> Int {
+        let data = try #require(json.data(using: .utf8))
+        let options = try #require(try JSONSerialization.jsonObject(with: data) as? [String: Any])
+        #expect(options.keys.sorted() == ["validate"])
+        let number = try #require(options["validate"] as? NSNumber)
+        #expect(CFGetTypeID(number) != CFBooleanGetTypeID())
+        return number.intValue
+    }
+
+    @Test("An insert asks for libmongoc's default check without the one that refuses a field named \"\"")
+    func dropsOnlyTheEmptyNameCheck() throws {
+        let validate = try validate(in: MongoInsertOptions.json)
+        #expect(validate & LibbsonValidation.emptyKeys == 0)
+        #expect(validate == libmongocInsertDefault & ~LibbsonValidation.emptyKeys)
+    }
+
+    @Test("An insert keeps the UTF-8 check and adds no key check of its own")
+    func keepsTheRestOfTheDefault() throws {
+        let validate = try validate(in: MongoInsertOptions.json)
+        #expect(validate != 0)
+        #expect(validate & LibbsonValidation.utf8 != 0)
+        #expect(validate & LibbsonValidation.utf8AllowNull != 0)
+        #expect(validate & (LibbsonValidation.dollarKeys | LibbsonValidation.dotKeys) == 0)
+    }
+
+    @Test("The options text carries the value the type names")
+    func textMatchesValue() throws {
+        #expect(try validate(in: MongoInsertOptions.json) == MongoInsertOptions.validation)
+    }
+}

docs/databases/mongodb.mdx

@@ -151,6 +151,8 @@ A field exists only in the documents that hold it, so a collection with no docum
 
 The text is Extended JSON: quote every field name, and write an ObjectId as `{"$oid": "…"}`, a date as `{"$date": "2024-05-01T10:00:00Z"}` and a decimal as `{"$numberDecimal": "1.10"}`. A whole number is stored as a 32-bit integer, or as a 64-bit one when it does not fit; `{"$numberLong": "5"}` stores a small 64-bit integer. A number with a decimal point is a double. Fields are stored in the order written. Leave out `_id` and the server generates one.
 
+Every field name is stored as typed, an empty name, a dotted name such as `a.b` and a top-level name starting with `$` included. MongoDB before 5.0 refuses the last two kinds. mongosh reads a document holding both `$ref` and `$id` at the top level as a DBRef, so keep that pair for references.
+
 ## Writing queries
 
 Queries run through JavaScriptCore, so a statement is JavaScript and the whole language is
@@ -301,6 +303,7 @@ New connections default to **Disabled**, and the driver has no TLS fallback: **P
 - A collection takes one text index. A second **FULLTEXT** row fails after the collection and the indexes before it are created: list every text field in one index instead.
 - **New Table…** writes the validator with the server's own level and action. To log bad documents instead of refusing them, run `db.runCommand({collMod: "articles", validationAction: "warn"})` after creating the collection.
 - Nested paths filter but do not sort. Sorting works on the grid's own columns.
+- Filtering, sorting or editing a field named `""` or starting with `$` fails in the grid. A dotted name such as `a.b` reads as the path to `b` inside `a`. Use `$getField` and `$setField` in the query editor.
 - **same element** covers a field one array deep. A path through an array inside another array needs nested `$elemMatch`, so those filter with dot notation only.
 - GridFS buckets are not browsable, and change streams are unsupported.
 - A `Code` scope holding an object that opens with `$type`, `$regex` or `$options`, such as `Code("f", {a: {$type: "binData"}})`, fails with `This is not a document MongoDB can read`. List another key of that object first.

project.yml

@@ -538,6 +538,7 @@ targets:
       - Plugins/MongoDBDriverPlugin/MongoExtendedJsonForm.swift
       - Plugins/MongoDBDriverPlugin/MongoNestedValueDiff.swift
       - Plugins/MongoDBDriverPlugin/MongoDocumentWritePlan.swift
+      - Plugins/MongoDBDriverPlugin/MongoInsertOptions.swift
       - Plugins/MongoDBDriverPlugin/MongoDBTimeoutPolicy.swift
       - Plugins/MongoDBDriverPlugin/MongoScriptCommandBuilder.swift
       - Plugins/MongoDBDriverPlugin/MongoShellCommandLine.swift