Skip to content

feat: allow methods to control strictObjectIDCoercion #467

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
virkt25 merged 1 commit into from
Sep 19, 2018
Merged

feat: allow methods to control strictObjectIDCoercion #467

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 15 additions & 2 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -206,9 +206,10 @@ The results will be output in `./benchmarks/results.md`.

## strictObjectIDCoercion flag

In version 1.17.0, the id of string type is being converted to ObjectID, when the string length is 12 or 24 and has the format of an ObjectID i.e /^[0-9a-fA-F]{24}$/. To avoid this issue, the strictObjectIDCoercion flag should be set to true in the model-definition file.
In version 1.17.0, the id of string type is being converted to ObjectID, when the string length is 12 or 24 and has the format of an ObjectID i.e /^[0-9a-fA-F]{24}$/.
To avoid this issue, the strictObjectIDCoercion flag should be set to true in the model-definition file. It is also possible to enable this flag on a per method bases by passing it in as part of the options object.

model-definition.js
### model-definition.js

```js
{
Expand Down Expand Up @@ -264,6 +265,18 @@ module.exports = function(app) {
};
```

### Per method basis

```js
myModelName.find(
{where: {id: {inq: ['59460487e9532ae90c324b59', '59460487e9532ae90c324b5a']}}},
{strictObjectIDCoercion: true},
function(err, result) {
// ...
}
)
```

## Release notes

* 1.1.7 - Do not return MongoDB-specific _id to client API, except if specifically specified in the model definition
66 changes: 35 additions & 31 deletions lib/mongodb.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -116,14 +116,14 @@ exports.initialize = function initializeDataSource(dataSource, callback) {
// compatibility of model connector hooks, this maps the new
// commands to previous names for the observors of this command.
const COMMAND_MAPPINGS = {
'insertOne': 'insert',
'updateOne': 'save',
'findOneAndUpdate': 'findAndModify',
'deleteOne': 'delete',
'deleteMany': 'delete',
'replaceOne': 'update',
'updateMany': 'update',
'countDocuments': 'count',
insertOne: 'insert',
updateOne: 'save',
findOneAndUpdate: 'findAndModify',
deleteOne: 'delete',
deleteMany: 'delete',
replaceOne: 'update',
updateMany: 'update',
countDocuments: 'count',
};

exports.MongoDB = MongoDB;
Expand Down Expand Up @@ -475,7 +475,7 @@ MongoDB.prototype.execute = function(model, command) {
}
};

MongoDB.prototype.coerceId = function(model, id) {
MongoDB.prototype.coerceId = function(model, id, options) {
// See https://github.com/strongloop/loopback-connector-mongodb/issues/206
if (id == null) return id;
var self = this;
Expand All @@ -493,7 +493,7 @@ MongoDB.prototype.coerceId = function(model, id) {
}
}

if (self.isObjectIDProperty(model, idProp, idValue)) {
if (self.isObjectIDProperty(model, idProp, idValue, options)) {
idValue = ObjectID(idValue);
}
}
Expand All @@ -517,7 +517,7 @@ MongoDB.prototype.create = function(model, data, options, callback) {
if (idValue === null) {
delete data[idName]; // Allow MongoDB to generate the id
} else {
var oid = self.coerceId(model, idValue); // Is it an Object ID?c
var oid = self.coerceId(model, idValue, options); // Is it an Object ID?c
data._id = oid; // Set it to _id
if (idName !== '_id') {
delete data[idName];
Expand All @@ -534,7 +534,7 @@ MongoDB.prototype.create = function(model, data, options, callback) {
return callback(err);
}
idValue = result.ops[0]._id;
idValue = self.coerceId(model, idValue);
idValue = self.coerceId(model, idValue, options);
// Wrap it to process.nextTick as delete data._id seems to be interferring
// with mongo insert
process.nextTick(function() {
Expand All @@ -560,7 +560,7 @@ MongoDB.prototype.save = function(model, data, options, callback) {
var idValue = self.getIdValue(model, data);
var idName = self.idName(model);

var oid = self.coerceId(model, idValue);
var oid = self.coerceId(model, idValue, options);
data._id = oid;
if (idName !== '_id') {
delete data[idName];
Expand Down Expand Up @@ -613,7 +613,7 @@ MongoDB.prototype.exists = function(model, id, options, callback) {
if (self.debug) {
debug('exists', model, id);
}
id = self.coerceId(model, id);
id = self.coerceId(model, id, options);
this.execute(model, 'findOne', {_id: id}, function(err, data) {
if (self.debug) {
debug('exists.callback', model, id, err, data);
Expand All @@ -634,7 +634,7 @@ MongoDB.prototype.find = function find(model, id, options, callback) {
debug('find', model, id);
}
var idName = self.idName(model);
var oid = self.coerceId(model, id);
var oid = self.coerceId(model, id, options);
this.execute(model, 'findOne', {_id: oid}, function(err, data) {
if (self.debug) {
debug('find.callback', model, id, err, data);
Expand Down Expand Up @@ -744,7 +744,7 @@ MongoDB.prototype.updateOrCreate = function updateOrCreate(

var id = self.getIdValue(model, data);
var idName = self.idName(model);
var oid = self.coerceId(model, id);
var oid = self.coerceId(model, id, options);
delete data[idName];

data = self.toDatabase(model, data);
Expand Down Expand Up @@ -806,7 +806,7 @@ MongoDB.prototype.replaceOrCreate = function(model, data, options, cb) {
if (this.debug) debug('replaceOrCreate', model, data);

var id = this.getIdValue(model, data);
var oid = this.coerceId(model, id);
var oid = this.coerceId(model, id, options);
var idName = this.idName(model);
data._id = data[idName];
delete data[idName];
Expand All @@ -824,7 +824,7 @@ MongoDB.prototype.destroy = function destroy(model, id, options, callback) {
if (self.debug) {
debug('delete', model, id);
}
id = self.coerceId(model, id);
id = self.coerceId(model, id, options);
this.execute(model, 'deleteOne', {_id: id}, function(err, result) {
if (self.debug) {
debug('delete.callback', model, id, err, result);
Expand Down Expand Up @@ -902,9 +902,9 @@ MongoDB.prototype.buildWhere = function(model, where, options) {
k = self.getDatabaseColumnName(model, k);

var spec = false;
var options = null;
var regexOptions = null;
if (cond && cond.constructor.name === 'Object') {
options = cond.options;
regexOptions = cond.options;
spec = Object.keys(cond)[0];
cond = cond[spec];
}
Expand All @@ -915,29 +915,31 @@ MongoDB.prototype.buildWhere = function(model, where, options) {
cond = [].concat(cond || []);
query[k] = {
$in: cond.map(function(x) {
if (self.isObjectIDProperty(model, prop, x)) return ObjectID(x);
if (self.isObjectIDProperty(model, prop, x, options))
return ObjectID(x);
return x;
}),
};
} else if (spec === 'nin') {
cond = [].concat(cond || []);
query[k] = {
$nin: cond.map(function(x) {
if (self.isObjectIDProperty(model, prop, x)) return ObjectID(x);
if (self.isObjectIDProperty(model, prop, x, options))
return ObjectID(x);
return x;
}),
};
} else if (spec === 'like') {
if (cond instanceof RegExp) {
query[k] = {$regex: cond};
} else {
query[k] = {$regex: new RegExp(cond, options)};
query[k] = {$regex: new RegExp(cond, regexOptions)};
}
} else if (spec === 'nlike') {
if (cond instanceof RegExp) {
query[k] = {$not: cond};
} else {
query[k] = {$not: new RegExp(cond, options)};
query[k] = {$not: new RegExp(cond, regexOptions)};
}
} else if (spec === 'neq') {
query[k] = {$ne: cond};
Expand All @@ -956,7 +958,7 @@ MongoDB.prototype.buildWhere = function(model, where, options) {
// Null: 10
query[k] = {$type: 10};
} else {
if (self.isObjectIDProperty(model, prop, cond)) {
if (self.isObjectIDProperty(model, prop, cond, options)) {
cond = ObjectID(cond);
}
query[k] = cond;
Expand Down Expand Up @@ -1387,7 +1389,7 @@ MongoDB.prototype.count = function count(model, where, options, callback) {
*/
MongoDB.prototype.replaceById = function replace(model, id, data, options, cb) {
if (this.debug) debug('replace', model, id, data);
var oid = this.coerceId(model, id);
var oid = this.coerceId(model, id, options);
this.replaceWithOptions(model, oid, data, {upsert: false}, function(
err,
data
Expand Down Expand Up @@ -1480,7 +1482,7 @@ MongoDB.prototype.updateAttributes = function updateAttrs(
return;
}

var oid = self.coerceId(model, id);
var oid = self.coerceId(model, id, options);
var idName = this.idName(model);

this.execute(
Expand Down Expand Up @@ -1813,7 +1815,7 @@ MongoDB.prototype.ping = function(cb) {
* Check whether the property is an ObjectID (or Array thereof)
*
*/
MongoDB.prototype.isObjectIDProperty = function(model, prop, value) {
MongoDB.prototype.isObjectIDProperty = function(model, prop, value, options) {
if (
prop &&
(prop.type === ObjectID ||
Expand All @@ -1822,9 +1824,11 @@ MongoDB.prototype.isObjectIDProperty = function(model, prop, value) {
return true;
} else if ('string' === typeof value) {
var settings = this._models[model] && this._models[model].settings;
options = options || {};
var strict =
(settings && settings.strictObjectIDCoercion) ||
this.settings.strictObjectIDCoercion;
this.settings.strictObjectIDCoercion ||
options.strictObjectIDCoercion;
if (strict) return false; // unless explicitly typed, don't coerce
return /^[0-9a-fA-F]{24}$/.test(value);
} else {
Expand Down Expand Up @@ -1873,7 +1877,7 @@ function optimizedFindOrCreate(model, filter, data, options, callback) {
if (idValue == null) {
delete data[idName]; // Allow MongoDB to generate the id
} else {
var oid = self.coerceId(model, idValue); // Is it an Object ID?
var oid = self.coerceId(model, idValue, options); // Is it an Object ID?
data._id = oid; // Set it to _id
if (idName !== '_id') {
delete data[idName];
Expand All @@ -1886,7 +1890,7 @@ function optimizedFindOrCreate(model, filter, data, options, callback) {
if (filter.where[idName]) {
var id = filter.where[idName];
delete filter.where[idName];
id = self.coerceId(model, id);
id = self.coerceId(model, id, options);
filter.where._id = id;
}
query = self.buildWhere(model, filter.where, options);
Expand Down
15 changes: 15 additions & 0 deletions test/objectid.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,4 +53,19 @@ describe('ObjectID', function() {
var id = 123;
ObjectID(id).should.be.equal(123);
});

it('coerces ObjectID', function() {
const coercedId = db.connector.isObjectIDProperty('Book', {}, '52fcef5c0325ace8dcb7a0bd');
coercedId.should.be.True();
});

it('given strictObjectIDCoercion: true, does not coerce ObjectID', function() {
const coercedId = db.connector.isObjectIDProperty(
'Book',
{},
'52fcef5c0325ace8dcb7a0bd',
{strictObjectIDCoercion: true}
);
coercedId.should.be.False();
});
});