Skip to content

Commit e376632

Browse files
addaleaxaddaleax
authored
feat!(NODE-3094): improve dots-and-dollars in keys experience (#2863)
Sets the BSON `checkKeys` option to false by default
1 parent 1cdb860 commit e376632

33 files changed

+6671
-148
lines changed

‎src/bulk/common.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -977,7 +977,7 @@ export abstract class BulkOperationBase {
977977
// Fundamental error
978978
err: undefined,
979979
// check keys
980-
checkKeys: typeof options.checkKeys === 'boolean' ? options.checkKeys : true
980+
checkKeys: typeof options.checkKeys === 'boolean' ? options.checkKeys : false
981981
};
982982

983983
// bypass Validation

‎src/cmap/commands.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -104,7 +104,7 @@ export class Query {
104104
this.ignoreUndefined =
105105
typeof options.ignoreUndefined === 'boolean' ? options.ignoreUndefined : false;
106106
this.maxBsonSize = options.maxBsonSize || 1024 * 1024 * 16;
107-
this.checkKeys = typeof options.checkKeys === 'boolean' ? options.checkKeys : true;
107+
this.checkKeys = typeof options.checkKeys === 'boolean' ? options.checkKeys : false;
108108
this.batchSize = this.numberToReturn;
109109

110110
// Flags
@@ -686,7 +686,7 @@ export class Msg {
686686
typeof options.serializeFunctions === 'boolean' ? options.serializeFunctions : false;
687687
this.ignoreUndefined =
688688
typeof options.ignoreUndefined === 'boolean' ? options.ignoreUndefined : false;
689-
this.checkKeys = typeof options.checkKeys === 'boolean' ? options.checkKeys : true;
689+
this.checkKeys = typeof options.checkKeys === 'boolean' ? options.checkKeys : false;
690690
this.maxBsonSize = options.maxBsonSize || 1024 * 1024 * 16;
691691

692692
// flags

‎src/operations/insert.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ export class InsertOperation extends CommandOperation<Document> {
1919

2020
constructor(ns: MongoDBNamespace, documents: Document[], options: BulkWriteOptions) {
2121
super(undefined, options);
22-
this.options = { ...options, checkKeys: options.checkKeys ?? true };
22+
this.options = { ...options, checkKeys: options.checkKeys ?? false };
2323
this.ns = ns;
2424
this.documents = documents;
2525
}

‎test/functional/collection.test.js‎

Lines changed: 0 additions & 86 deletions
Original file line numberDiff line numberDiff line change
@@ -163,92 +163,6 @@ describe('Collection', function () {
163163
});
164164
});
165165

166-
it('should fail to insert due to illegal keys', function (done) {
167-
db.createCollection('test_invalid_key_names', (err, collection) => {
168-
// Legal inserts
169-
collection.insertMany(
170-
[{ hello: 'world' }, { hello: { hello: 'world' } }],
171-
configuration.writeConcernMax(),
172-
err => {
173-
expect(err).to.not.exist;
174-
175-
// Illegal insert for key
176-
collection.insertOne({ $hello: 'world' }, configuration.writeConcernMax(), err => {
177-
expect(err).to.be.an.instanceof(Error);
178-
expect(err.message).to.equal("key $hello must not start with '$'");
179-
180-
collection.insertOne(
181-
{ hello: { $hello: 'world' } },
182-
configuration.writeConcernMax(),
183-
err => {
184-
expect(err).to.be.an.instanceof(Error);
185-
expect(err.message).to.equal("key $hello must not start with '$'");
186-
187-
collection.insertOne(
188-
{ he$llo: 'world' },
189-
configuration.writeConcernMax(),
190-
err => {
191-
expect(err).to.not.exist;
192-
193-
collection.insertOne(
194-
{ hello: { hell$o: 'world' } },
195-
configuration.writeConcernMax(),
196-
err => {
197-
expect(err).to.not.exist;
198-
199-
collection.insertOne(
200-
{ '.hello': 'world' },
201-
configuration.writeConcernMax(),
202-
err => {
203-
expect(err).to.be.an.instanceof(Error);
204-
expect(err.message).to.equal("key .hello must not contain '.'");
205-
206-
collection.insertOne(
207-
{ hello: { '.hello': 'world' } },
208-
configuration.writeConcernMax(),
209-
err => {
210-
expect(err).to.be.an.instanceof(Error);
211-
expect(err.message).to.equal("key .hello must not contain '.'");
212-
213-
collection.insertOne(
214-
{ 'hello.': 'world' },
215-
configuration.writeConcernMax(),
216-
err => {
217-
expect(err).to.be.an.instanceof(Error);
218-
expect(err.message).to.equal(
219-
"key hello. must not contain '.'"
220-
);
221-
222-
collection.insertOne(
223-
{ hello: { 'hello.': 'world' } },
224-
configuration.writeConcernMax(),
225-
err => {
226-
expect(err).to.be.an.instanceof(Error);
227-
expect(err.message).to.equal(
228-
"key hello. must not contain '.'"
229-
);
230-
// Let's close the db
231-
done();
232-
}
233-
);
234-
}
235-
);
236-
}
237-
);
238-
}
239-
);
240-
}
241-
);
242-
}
243-
);
244-
}
245-
);
246-
});
247-
}
248-
);
249-
});
250-
});
251-
252166
it('should permit insert of dot and dollar keys if requested', function () {
253167
const collection = db.collection('test_invalid_key_names');
254168
return Promise.all([

‎test/functional/insert.test.js‎

Lines changed: 0 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -900,34 +900,6 @@ describe('Insert', function () {
900900
}
901901
});
902902

903-
it('Should fail on insert due to key starting with $', {
904-
// Add a tag that our runner can trigger on
905-
// in this case we are setting that node needs to be higher than 0.10.X to run
906-
metadata: {
907-
requires: { topology: ['single', 'replicaset', 'sharded', 'ssl', 'heap', 'wiredtiger'] }
908-
},
909-
910-
test: function (done) {
911-
var configuration = this.configuration;
912-
var doc = {
913-
_id: new ObjectId('4e886e687ff7ef5e00000162'),
914-
$key: 'foreign'
915-
};
916-
917-
var client = configuration.newClient(configuration.writeConcernMax(), { maxPoolSize: 1 });
918-
client.connect(function (err, client) {
919-
var db = client.db(configuration.db);
920-
var collection = db.collection('Should_fail_on_insert_due_to_key_starting_with');
921-
collection.insert(doc, configuration.writeConcernMax(), function (err, result) {
922-
test.ok(err != null);
923-
expect(result).to.not.exist;
924-
925-
client.close(done);
926-
});
927-
});
928-
}
929-
});
930-
931903
it('Should Correctly allow for control of serialization of functions on command level', {
932904
// Add a tag that our runner can trigger on
933905
// 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‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -251,7 +251,7 @@ operations.set('find', async ({ entities, operation }) => {
251251
operations.set('findOneAndReplace', async ({ entities, operation }) => {
252252
const collection = entities.getEntity('collection', operation.object);
253253
const { filter, replacement, ...opts } = operation.arguments;
254-
return collection.findOneAndReplace(filter, replacement, translateOptions(opts));
254+
return (await collection.findOneAndReplace(filter, replacement, translateOptions(opts))).value;
255255
});
256256

257257
operations.set('findOneAndUpdate', async ({ entities, operation }) => {

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -152,7 +152,7 @@ export async function runUnifiedTest(
152152

153153
expect(documents).to.have.lengthOf(collectionData.documents.length);
154154
for (const [expected, actual] of zip(collectionData.documents, documents)) {
155-
expect(actual).to.include(expected, 'Test outcome did not match expected');
155+
expect(actual).to.deep.include(expected);
156156
}
157157
}
158158
}

0 commit comments

Comments
 (0)