feat!(NODE-3094): improve dots-and-dollars in keys experience (#2863)

Sets the BSON `checkKeys` option to false by default

Author: addaleax

src/bulk/common.ts

@@ -977,7 +977,7 @@ export abstract class BulkOperationBase {
  // Fundamental error
  err: undefined,
  // check keys
- checkKeys: typeof options.checkKeys === 'boolean' ? options.checkKeys : true
+ checkKeys: typeof options.checkKeys === 'boolean' ? options.checkKeys : false
  };
 
  // bypass Validation

src/cmap/commands.ts

@@ -104,7 +104,7 @@ export class Query {
  this.ignoreUndefined =
  typeof options.ignoreUndefined === 'boolean' ? options.ignoreUndefined : false;
  this.maxBsonSize = options.maxBsonSize || 1024 * 1024 * 16;
- this.checkKeys = typeof options.checkKeys === 'boolean' ? options.checkKeys : true;
+ this.checkKeys = typeof options.checkKeys === 'boolean' ? options.checkKeys : false;
  this.batchSize = this.numberToReturn;
 
  // Flags
@@ -686,7 +686,7 @@ export class Msg {
  typeof options.serializeFunctions === 'boolean' ? options.serializeFunctions : false;
  this.ignoreUndefined =
  typeof options.ignoreUndefined === 'boolean' ? options.ignoreUndefined : false;
- this.checkKeys = typeof options.checkKeys === 'boolean' ? options.checkKeys : true;
+ this.checkKeys = typeof options.checkKeys === 'boolean' ? options.checkKeys : false;
  this.maxBsonSize = options.maxBsonSize || 1024 * 1024 * 16;
 
  // flags

src/operations/insert.ts

@@ -19,7 +19,7 @@ export class InsertOperation extends CommandOperation<Document> {
 
  constructor(ns: MongoDBNamespace, documents: Document[], options: BulkWriteOptions) {
  super(undefined, options);
- this.options = { ...options, checkKeys: options.checkKeys ?? true };
+ this.options = { ...options, checkKeys: options.checkKeys ?? false };
  this.ns = ns;
  this.documents = documents;
  }

test/functional/collection.test.js

@@ -163,92 +163,6 @@ describe('Collection', function () {
  });
  });
 
- it('should fail to insert due to illegal keys', function (done) {
- db.createCollection('test_invalid_key_names', (err, collection) => {
- // Legal inserts
- collection.insertMany(
- [{ hello: 'world' }, { hello: { hello: 'world' } }],
- configuration.writeConcernMax(),
- err => {
- expect(err).to.not.exist;
-
- // Illegal insert for key
- collection.insertOne({ $hello: 'world' }, configuration.writeConcernMax(), err => {
- expect(err).to.be.an.instanceof(Error);
- expect(err.message).to.equal("key $hello must not start with '$'");
-
- collection.insertOne(
- { hello: { $hello: 'world' } },
- configuration.writeConcernMax(),
- err => {
- expect(err).to.be.an.instanceof(Error);
- expect(err.message).to.equal("key $hello must not start with '$'");
-
- collection.insertOne(
- { he$llo: 'world' },
- configuration.writeConcernMax(),
- err => {
- expect(err).to.not.exist;
-
- collection.insertOne(
- { hello: { hell$o: 'world' } },
- configuration.writeConcernMax(),
- err => {
- expect(err).to.not.exist;
-
- collection.insertOne(
- { '.hello': 'world' },
- configuration.writeConcernMax(),
- err => {
- expect(err).to.be.an.instanceof(Error);
- expect(err.message).to.equal("key .hello must not contain '.'");
-
- collection.insertOne(
- { hello: { '.hello': 'world' } },
- configuration.writeConcernMax(),
- err => {
- expect(err).to.be.an.instanceof(Error);
- expect(err.message).to.equal("key .hello must not contain '.'");
-
- collection.insertOne(
- { 'hello.': 'world' },
- configuration.writeConcernMax(),
- err => {
- expect(err).to.be.an.instanceof(Error);
- expect(err.message).to.equal(
- "key hello. must not contain '.'"
- );
-
- collection.insertOne(
- { hello: { 'hello.': 'world' } },
- configuration.writeConcernMax(),
- err => {
- expect(err).to.be.an.instanceof(Error);
- expect(err.message).to.equal(
- "key hello. must not contain '.'"
- );
- // Let's close the db
- done();
- }
- );
- }
- );
- }
- );
- }
- );
- }
- );
- }
- );
- }
- );
- });
- }
- );
- });
- });
-
  it('should permit insert of dot and dollar keys if requested', function () {
  const collection = db.collection('test_invalid_key_names');
  return Promise.all([

test/functional/insert.test.js

@@ -900,34 +900,6 @@ describe('Insert', function () {
  }
  });
 
- it('Should fail on insert due to key starting with $', {
- // Add a tag that our runner can trigger on
- // in this case we are setting that node needs to be higher than 0.10.X to run
- metadata: {
- requires: { topology: ['single', 'replicaset', 'sharded', 'ssl', 'heap', 'wiredtiger'] }
- },
-
- test: function (done) {
- var configuration = this.configuration;
- var doc = {
- _id: new ObjectId('4e886e687ff7ef5e00000162'),
- $key: 'foreign'
- };
-
- var client = configuration.newClient(configuration.writeConcernMax(), { maxPoolSize: 1 });
- client.connect(function (err, client) {
- var db = client.db(configuration.db);
- var collection = db.collection('Should_fail_on_insert_due_to_key_starting_with');
- collection.insert(doc, configuration.writeConcernMax(), function (err, result) {
- test.ok(err != null);
- expect(result).to.not.exist;
-
- client.close(done);
- });
- });
- }
- });
-
  it('Should Correctly allow for control of serialization of functions on command level', {
  // Add a tag that our runner can trigger on
  // in this case we are setting that node needs to be higher than 0.10.X to run

test/functional/unified-spec-runner/operations.ts

@@ -251,7 +251,7 @@ operations.set('find', async ({ entities, operation }) => {
 operations.set('findOneAndReplace', async ({ entities, operation }) => {
  const collection = entities.getEntity('collection', operation.object);
  const { filter, replacement, ...opts } = operation.arguments;
- return collection.findOneAndReplace(filter, replacement, translateOptions(opts));
+ return (await collection.findOneAndReplace(filter, replacement, translateOptions(opts))).value;
 });
 
 operations.set('findOneAndUpdate', async ({ entities, operation }) => {

test/functional/unified-spec-runner/runner.ts

@@ -152,7 +152,7 @@ export async function runUnifiedTest(
 
  expect(documents).to.have.lengthOf(collectionData.documents.length);
  for (const [expected, actual] of zip(collectionData.documents, documents)) {
- expect(actual).to.include(expected, 'Test outcome did not match expected');
+ expect(actual).to.deep.include(expected);
  }
  }
  }