Skip to content

Commit

Permalink
feat: add mongodb.dataType to property options
Browse files Browse the repository at this point in the history 
Adds mongodb.dataType to model property options
  • Loading branch information
Hage Yaapa committed May 16, 2019
1 parent 2ba8cc7 commit 305e0e0
Show file tree
Hide file tree
Showing 2 changed files with 205 additions and 35 deletions.
100 changes: 65 additions & 35 deletions lib/mongodb.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -411,19 +411,19 @@ MongoDB.prototype.fromDatabase = function(model, data) {
* @param {String} model The model name
* @param {Object} data The JSON data to convert
*/
MongoDB.prototype.toDatabase = function(model, data) {
const modelInstance = this._models[model].model;
var props = this._models[model].properties;
MongoDB.prototype.toDatabase = function(modelName, data) {
const model = this._models[modelName];
const props = model.properties;

if (this.settings.enableGeoIndexing !== true) {
visitAllProperties(data, modelInstance, coerceDecimalProperty);
visitAllProperties(data, model, coerceProperties);
// Override custom column names
data = this.fromPropertyToDatabaseNames(model, data);
data = this.fromPropertyToDatabaseNames(modelName, data);
return data;
}

for (var p in props) {
var prop = props[p];
for (const p in props) {
const prop = props[p];
const isGeoPoint = data[p] && prop && prop.type && prop.type.name === 'GeoPoint';
if (isGeoPoint) {
data[p] = {
Expand All @@ -433,9 +433,9 @@ MongoDB.prototype.toDatabase = function(model, data) {
}
}

visitAllProperties(data, modelInstance, coerceDecimalProperty);
visitAllProperties(data, model, coerceProperties);
// Override custom column names
data = this.fromPropertyToDatabaseNames(model, data);
data = this.fromPropertyToDatabaseNames(modelName, data);
if (debug.enabled) debug('toDatabase data: ', util.inspect(data));
return data;
};
Expand Down Expand Up @@ -942,12 +942,12 @@ MongoDB.prototype.buildWhere = function(model, where, options) {

var prop = self.getPropertyDefinition(model, propName);

const isDecimal = prop &&
prop.mongodb && prop.mongodb.dataType &&
prop.mongodb.dataType.toLowerCase() === 'decimal128';
if (isDecimal) {
cond = Decimal128.fromString(cond);
debug('buildWhere decimal value: %s, constructor name: %s', cond, cond.constructor.name);
if (prop && prop.mongodb && typeof prop.mongodb.dataType === 'string') {
const isDecimal = prop.mongodb.dataType.toLowerCase() === 'decimal128';
if (isDecimal) {
cond = Decimal128.fromString(cond);
debug('buildWhere decimal value: %s, constructor name: %s', cond, cond.constructor.name);
}
}

// Convert property to database column name
Expand Down Expand Up @@ -1881,14 +1881,25 @@ MongoDB.prototype.isObjectIDProperty = function(model, prop, value, options) {
) {
return true;
} else if ('string' === typeof value) {
// values doesn't look like an ObjectID
if (!value.match(/^[0-9a-fA-F]{24}$/)) return false;

// value looks like an ObjectID, and it should be treated as one
// because `prop.mongodb.dataType` is set to 'ObjectID'
// this condition overrides the `strictObjectIDCoercion` setting
if (prop.mongodb && prop.mongodb.dataType === ObjectID) return true;

// value looks like an ObjectID, determine its nature based on `strictObjectIDCoercion`
var settings = this._models[model] && this._models[model].settings;

options = options || {};
var strict =
(settings && 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 return true;
} else {
return false;
}
Expand Down Expand Up @@ -2000,15 +2011,14 @@ function optimizedFindOrCreate(model, filter, data, options, callback) {
}

/**
*
* @param {*} data Plain Data Object for the matching property definition(s)
* @param {*} modelCtorOrDef Model constructor or definition
* @param {*} model Model of the data
* @param {*} visitor A callback function which takes a property value and
* definition to apply custom property coercion
*/
function visitAllProperties(data, modelCtorOrDef, visitor) {
function visitAllProperties(data, model, visitor) {
if (data === null || data === undefined) return;
const modelProps = modelCtorOrDef.properties ? modelCtorOrDef.properties : modelCtorOrDef.definition.properties;
const modelProps = model.properties ? model.properties : model.definition.properties;
const allProps = new Set(Object.keys(data).concat(Object.keys(modelProps)));
for (const p of allProps) {
const value = data[p];
Expand All @@ -2023,28 +2033,49 @@ function visitAllProperties(data, modelCtorOrDef, visitor) {
visitAllProperties(value, def.type.definition, visitor);
}
} else {
visitor(value, def, (newValue) => { data[p] = newValue; });
visitor(model, value, def, (newValue) => { data[p] = newValue; });
}
continue;
}
}

/**
*
* @param {*} propValue Property value to coerce into a Decimal128 value
* @param {*} propDef Property definition to check if property is MongoDB
* Decimal128 type
* @param {*} model Model containing the property
* @param {*} propValue Property value to coerce into special types supported by the connector
* @param {*} propDef Property definition to check if property is for MongoDB
*/
function coerceDecimalProperty(propValue, propDef, setValue) {
let updatedValue;
if (hasDataType('decimal128', propDef)) {
if (Array.isArray(propValue)) {
updatedValue = propValue.map(val => Decimal128.fromString(val));
return setValue(updatedValue);
} else {
updatedValue = Decimal128.fromString(propValue);
return setValue(updatedValue);
function coerceProperties(model, propValue, propDef, setValue) {
let coercedValue;
// Process only mongo-specific data types
if (propDef && propDef.mongodb && propDef.mongodb.dataType) {
const dataType = propDef.mongodb.dataType;
// Data types specified as string values
// Yaapa: should we implement all type specifications using objects instead of strings?
if (typeof dataType === 'string') {
if (hasDataType('decimal128', propDef)) {
if (Array.isArray(propValue)) {
coercedValue = propValue.map(val => Decimal128.fromString(val));
return setValue(coercedValue);
} else {
coercedValue = Decimal128.fromString(propValue);
return setValue(coercedValue);
}
}
// ObjectID data type
} else if (dataType === ObjectID) {
if (!model.settings.strictObjectIDCoercion) {
coercedValue = ObjectID(propValue);
setValue(coercedValue);
}
}
} else {
// Apply strictObjectIDCoercion setting to ObjectID-like string values
if (typeof propValue === 'string' && !model.settings.strictObjectIDCoercion && propValue.match(/^[0-9a-fA-F]{24}$/)) {
coercedValue = ObjectID(propValue);
return setValue(coercedValue);
}

return setValue(propValue);
}
}

Expand All @@ -2071,4 +2102,3 @@ function hasDataType(dataType, propertyDef) {
propertyDef.mongodb.dataType &&
propertyDef.mongodb.dataType.toLowerCase() === dataType.toLowerCase();
}

140 changes: 140 additions & 0 deletions test/id.test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -247,3 +247,143 @@ describe('mongodb default id name', function() {
);
});
});

describe('strictObjectIDCoercion', function() {
var ObjectID = ds.connector.getDefaultIdType();
var objectIdLikeString = '7cd2ad46ffc580ba45d3cb1f';

context('set to false (default)', function() {
var User = ds.createModel(
'user1',
{
id: {type: String, id: true},
name: String,
}
);

beforeEach(function(done) {
User.deleteAll(done);
});

it('should find model with ObjectID id', async function() {
const user = await User.create({name: 'John'});
user.id.should.be.an.instanceOf(ds.ObjectID);
const found = await User.findById(user.id);
found.should.not.be.null();
});

it('should find model with ObjectID-like id', async function() {
const user = await User.create({id: objectIdLikeString, name: 'John'});
user.id.should.be.an.instanceOf(ds.ObjectID);
user.id.should.eql(ObjectID(objectIdLikeString));
const found = await User.findById(objectIdLikeString);
found.should.not.be.null();
});

it('should find model with string id', async function() {
const user = await User.create({id: 'a', name: 'John'});
user.id.should.be.an.instanceOf(String);
user.id.should.equal('a');
const found = await User.findById('a');
found.should.not.be.null();
});
});

context('set to true', function() {
var User = ds.createModel(
'user2',
{
id: {type: String, id: true},
name: String,
},
{strictObjectIDCoercion: true}
);

beforeEach(function(done) {
User.deleteAll(done);
});

it('should not find model with ObjectID id', async function() {
const user = await User.create({name: 'John'});
user.id.should.not.be.an.instanceOf(ds.ObjectID);
const found = await User.findById(user.id);
(found === null).should.be.true();
});

it('should find model with ObjectID-like string id', async function() {
const user = await User.create({id: objectIdLikeString, name: 'John'});
user.id.should.not.be.an.instanceOf(ds.ObjectID);
user.id.should.eql(objectIdLikeString);
const found = await User.findById(objectIdLikeString);
found.should.not.be.empty();
});

it('should find model with string id', async function() {
const user = await User.create({id: 'a', name: 'John'});
user.id.should.be.an.instanceOf(String);
user.id.should.equal('a');
const found = await User.findById('a');
found.should.not.be.empty();
});
});

context('set to true, id type set to `ObjectID`', function() {
var User = ds.createModel(
'user3',
{
id: {type: String, id: true, mongodb: {dataType: ObjectID}},
name: String,
},
{strictObjectIDCoercion: true}
);

beforeEach(function(done) {
User.deleteAll(done);
});

it('should find model with ObjectID id', async function() {
const user = await User.create({name: 'John'});
user.id.should.be.an.instanceOf(ds.ObjectID);
const found = await User.findById(user.id);
found.should.not.be.null();
});

// This works by coercing string to ObjectID, overriding `strictObjectIDCoercion: true`
it('should find model with ObjectID-like id', async function() {
const user = await User.create({id: objectIdLikeString, name: 'John'});
user.id.should.be.an.instanceOf(ds.ObjectID);
user.id.should.eql(ObjectID(objectIdLikeString));
const found = await User.findById(objectIdLikeString);
found.should.not.be.null();
});

it('should find model with string id', async function() {
const user = await User.create({id: 'a', name: 'John'});
user.id.should.be.an.instanceOf(String);
user.id.should.equal('a');
const found = await User.findById('a');
found.should.not.be.null();
});

it('should update model with ObjectID id', async function() {
const user = await User.create({name: 'John'});
await User.update({id: user.id, name: 'Jon'});
const found = await User.findById(user.id);
found.name.should.equal('Jon');
});

it('should update model with ObjectID-like id', async function() {
const user = await User.create({id: objectIdLikeString, name: 'John'});
await User.update({id: objectIdLikeString, name: 'Jon'});
const found = await User.findById(objectIdLikeString);
found.name.should.equal('Jon');
});

it('should update model with string id', async function() {
const user = await User.create({id: 'a', name: 'John'});
await User.update({id: 'a', name: 'Jon'});
const found = await User.findById('a');
found.name.should.equal('Jon');
});
});
});

0 comments on commit 305e0e0

Please sign in to comment.