fix(query-parser): update DBRef stringify to preserve type information COMPASS-10987

packages/query-parser/src/index.spec.ts

@@ -443,6 +443,113 @@ e  s`,
         a: new bson.Code(code, { b: 1 }),
       });
     });
+
+    describe('DBRef', function () {
+      // The bson types say `oid` is an ObjectId, but a DBRef oid can hold any
+      // BSON value in practice, which is what we want to cover here.
+      const dbRef = (
+        collection: string,
+        oid: unknown,
+        db?: string,
+        fields?: Record<string, unknown>,
+      ) => new bson.DBRef(collection, oid as bson.ObjectId, db, fields);
+
+      it('preserves the oid type rather than flattening it to a string', function () {
+        assert.equal(
+          toJSString({ a: dbRef('col', 1) }, 0),
+          '{a:DBRef("col", 1)}',
+        );
+        assert.equal(
+          toJSString({ a: dbRef('col', 'abc') }, 0),
+          '{a:DBRef("col", \'abc\')}',
+        );
+        assert.equal(
+          toJSString(
+            { a: dbRef('col', new bson.ObjectId('507f191e810c19729de860ea')) },
+            0,
+          ),
+          '{a:DBRef("col", ObjectId(\'507f191e810c19729de860ea\'))}',
+        );
+      });
+
+      it('includes the db when present', function () {
+        assert.equal(
+          toJSString({ a: dbRef('col', 1, 'db') }, 0),
+          '{a:DBRef("col", 1, "db")}',
+        );
+      });
+
+      it('includes the fields when present', function () {
+        assert.equal(
+          toJSString({ a: dbRef('col', 1, 'db', { b: 1 }) }, 0),
+          '{a:DBRef("col", 1, "db", {b:1})}',
+        );
+      });
+
+      it('passes an undefined db when fields are present without one', function () {
+        assert.equal(
+          toJSString({ a: dbRef('col', 1, undefined, { b: 1 }) }, 0),
+          '{a:DBRef("col", 1, undefined, {b:1})}',
+        );
+      });
+
+      it('omits empty fields', function () {
+        assert.equal(
+          toJSString({ a: dbRef('col', 1, undefined, {}) }, 0),
+          '{a:DBRef("col", 1)}',
+        );
+      });
+
+      it('preserves BSON types inside fields', function () {
+        assert.equal(
+          toJSString(
+            {
+              a: dbRef('col', 1, 'db', {
+                b: new bson.ObjectId('507f191e810c19729de860ea'),
+              }),
+            },
+            0,
+          ),
+          '{a:DBRef("col", 1, "db", {b:ObjectId(\'507f191e810c19729de860ea\')})}',
+        );
+      });
+
+      it('escapes quotes in the collection and db', function () {
+        assert.equal(
+          toJSString({ a: dbRef("co'l", 1, 'd"b') }, 0),
+          '{a:DBRef("co\'l", 1, "d\\"b")}',
+        );
+      });
+
+      const roundTrips: [string, bson.DBRef][] = [
+        ['numeric oid', dbRef('col', 1)],
+        ['string oid', dbRef('col', 'abc')],
+        [
+          'ObjectId oid',
+          dbRef('col', new bson.ObjectId('507f191e810c19729de860ea')),
+        ],
+        ['db', dbRef('col', 1, 'db')],
+        ['quotes', dbRef("co'l", 1, 'd"b')],
+        ['double spaces', dbRef('a  b', 1)],
+        ['newline', dbRef('a\nb', 1)],
+        ['nested DBRef oid', dbRef('col', dbRef('inner', 1), 'db')],
+        ['fields', dbRef('col', 1, 'db', { b: 1 })],
+        ['fields but no db', dbRef('col', 1, undefined, { b: 1 })],
+        [
+          'BSON values in fields',
+          dbRef('col', 1, 'db', {
+            b: new bson.ObjectId('507f191e810c19729de860ea'),
+          }),
+        ],
+      ];
+
+      for (const [name, dbref] of roundTrips) {
+        it(`round-trips a DBRef with ${name}`, function () {
+          const jsString = toJSString({ a: dbref }, 0) as string;
+          assert.deepEqual(parseFilter(jsString), { a: dbref });
+        });
+      }
+    });
   });
 
   describe('toJSString with indent 0', function () {
@@ -568,21 +675,21 @@ e  s`,
     context('when providing a DBRef with (collection, oid)', function () {
       it('correctly converts to a DBRef', function () {
         const res = parseFilter("{dbref: DBRef('col', 1)}");
-        assert.equal(compactStringify(res), "{dbref:DBRef('col', '1')}");
+        assert.equal(compactStringify(res), '{dbref:DBRef("col", 1)}');
       });
     });
 
     context('when providing a DBRef with (db.collection, oid)', function () {
       it('correctly converts to a DBRef', function () {
         const res = parseFilter("{dbref: DBRef('db.col', 1)}");
-        assert.equal(compactStringify(res), "{dbref:DBRef('col', '1', 'db')}");
+        assert.equal(compactStringify(res), '{dbref:DBRef("col", 1, "db")}');
       });
     });
 
     context('when providing a DBRef with (collection, oid, db)', function () {
       it('correctly converts to a DBRef', function () {
         const res = parseFilter("{dbref: DBRef('col', 1, 'db')}");
-        assert.equal(compactStringify(res), "{dbref:DBRef('col', '1', 'db')}");
+        assert.equal(compactStringify(res), '{dbref:DBRef("col", 1, "db")}');
       });
     });
 

packages/query-parser/src/stringify.ts

@@ -80,11 +80,21 @@ const BSON_TO_JS_STRING = {
     return `BinData(${subType.toString(10)}, '${v.toString('base64')}')`;
   },
   DBRef: function (v: DBRef) {
-    if (v.db) {
-      return `DBRef('${v.collection}', '${v.oid.toString()}', '${v.db}')`;
+    // `toJSString` only returns undefined for values that stringify to
+    // nothing, which for an oid can only be `undefined` itself.
+    const oid = toJSString(v.oid, 0) ?? 'undefined';
+    const args = [JSON.stringify(v.collection), oid];
+    // `fields` defaults to an empty object rather than being unset, so only
+    // include it when it actually holds something.
+    const hasFields = !!v.fields && Object.keys(v.fields).length > 0;
+    if (v.db || hasFields) {
+      // `db` has to be present for `fields` to land in the right position.
+      args.push(v.db === undefined ? 'undefined' : JSON.stringify(v.db));
     }
-
-    return `DBRef('${v.collection}', '${v.oid.toString()}')`;
+    if (hasFields) {
+      args.push(toJSString(v.fields, 0) ?? '{}');
+    }
+    return `DBRef(${args.join(', ')})`;
   },
   Timestamp: function (v: Timestamp) {
     return `Timestamp({ t: ${v.high}, i: ${v.low} })`;

packages/shell-bson-parser/src/index.spec.ts

@@ -531,7 +531,7 @@ describe('@mongodb-js/shell-bson-parser', function () {
 
               // When constructing a date with no arguments, it will be set to the current date,
               // which is prone to race conditions for millisecond precision.
-              const allowedMillisecondDelta = args.length === 0 ? 3 : 0;
+              const allowedMillisecondDelta = args.length === 0 ? 9 : 0;
 
               expect(actual.getDate).to.equal(
                 new (Date as any)(...args).getDate(),