From fd99c6dc6e3cb71d4da861ae605971b17332b19f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miroslav=20Bajto=C5=A1?= Date: Fri, 25 Jan 2019 09:46:31 +0100 Subject: [PATCH] Support nested properties with class type When converting plain-data object values into model instances, correctly handle the case where the constructor functions is a class constructor and must be invoked via `new`. --- lib/model-builder.js | 17 ++++++++++++----- lib/model-utils.js | 18 +++++++++++------- lib/utils.js | 5 +++++ test/datatype.test.js | 31 +++++++++++++++++++++++++++++++ test/model-builder.test.js | 30 +++++++++++++++++++++++++++++- 5 files changed, 88 insertions(+), 13 deletions(-) diff --git a/lib/model-builder.js b/lib/model-builder.js index cd831f43..20cc3454 100644 --- a/lib/model-builder.js +++ b/lib/model-builder.js @@ -17,10 +17,13 @@ const deprecated = require('depd')('loopback-datasource-juggler'); const DefaultModelBaseClass = require('./model.js'); const List = require('./list.js'); const ModelDefinition = require('./model-definition.js'); -const deepMerge = require('./utils').deepMerge; -const deepMergeProperty = require('./utils').deepMergeProperty; -const rankArrayElements = require('./utils').rankArrayElements; const MixinProvider = require('./mixins'); +const { + deepMerge, + deepMergeProperty, + rankArrayElements, + isClass, +} = require('./utils'); // Set up types require('./types')(ModelBuilder); @@ -591,11 +594,15 @@ ModelBuilder.prototype.define = function defineClass(className, properties, sett this.__data[propertyName] = value; } else { if (DataType === List) { - this.__data[propertyName] = DataType(value, properties[propertyName].type, this.__data); + this.__data[propertyName] = isClass(DataType) ? + new DataType(value, properties[propertyName].type, this.__data) : + DataType(value, properties[propertyName].type, this.__data); } else { // Assume the type constructor handles Constructor() call // If not, we should call new DataType(value).valueOf(); - this.__data[propertyName] = (value instanceof DataType) ? value : DataType(value); + this.__data[propertyName] = (value instanceof DataType) ? + value : + isClass(DataType) ? new DataType(value) : DataType(value); } } } diff --git a/lib/model-utils.js b/lib/model-utils.js index 9e8f9698..a83974f6 100644 --- a/lib/model-utils.js +++ b/lib/model-utils.js @@ -14,9 +14,13 @@ module.exports = ModelUtils; */ const g = require('strong-globalize')(); const geo = require('./geo'); -const utils = require('./utils'); -const fieldsToArray = utils.fieldsToArray; -const sanitizeQueryOrData = utils.sanitizeQuery; +const { + fieldsToArray, + sanitizeQuery: sanitizeQueryOrData, + isPlainObject, + isClass, + toRegExp, +} = require('./utils'); const BaseModel = require('./model'); /** @@ -212,7 +216,7 @@ function coerceArray(val) { return val; } - if (!utils.isPlainObject(val)) { + if (!isPlainObject(val)) { throw new Error(g.f('Value is not an {{array}} or {{object}} with sequential numeric indices')); } @@ -474,7 +478,7 @@ ModelUtils._coerce = function(where, options) { } break; case 'regexp': - val = utils.toRegExp(val); + val = toRegExp(val); if (val instanceof Error) { val.statusCode = 400; throw val; @@ -499,7 +503,7 @@ ModelUtils._coerce = function(where, options) { for (let i = 0; i < val.length; i++) { if (val[i] !== null && val[i] !== undefined) { if (!(val[i] instanceof RegExp)) { - val[i] = DataType(val[i]); + val[i] = isClass(DataType) ? new DataType(val[i]) : DataType(val[i]); } } } @@ -532,7 +536,7 @@ ModelUtils._coerce = function(where, options) { throw err; } } - val = DataType(val); + val = isClass(DataType) ? new DataType(val) : DataType(val); } } } diff --git a/lib/utils.js b/lib/utils.js index a2a08f9e..7568aae2 100644 --- a/lib/utils.js +++ b/lib/utils.js @@ -27,6 +27,7 @@ exports.collectTargetIds = collectTargetIds; exports.idName = idName; exports.rankArrayElements = rankArrayElements; exports.idsHaveDuplicates = idsHaveDuplicates; +exports.isClass = isClass; const g = require('strong-globalize')(); const traverse = require('traverse'); @@ -801,3 +802,7 @@ function idsHaveDuplicates(ids) { } return hasDuplicates === true; } + +function isClass(fn) { + return fn && fn.toString().startsWith('class '); +} diff --git a/test/datatype.test.js b/test/datatype.test.js index ee069191..41b9fa3d 100644 --- a/test/datatype.test.js +++ b/test/datatype.test.js @@ -11,6 +11,12 @@ const should = require('./init.js'); let db, Model; +class NestedClass { + constructor(roleName) { + this.roleName = roleName; + } +} + describe('datatypes', function() { before(function(done) { db = getSchema(); @@ -23,6 +29,7 @@ describe('datatypes', function() { list: {type: [String]}, arr: Array, nested: Nested, + nestedClass: NestedClass, }; Model = db.define('Model', modelTableSchema); db.automigrate(['Model'], done); @@ -101,6 +108,30 @@ describe('datatypes', function() { } }); + it('should create nested object defined by a class when reading data from db', async () => { + const d = new Date('2015-01-01T12:00:00'); + let id; + const created = await Model.create({ + date: d, + list: ['test'], + arr: [1, 'str'], + nestedClass: new NestedClass('admin'), + }); + created.list.should.deepEqual(['test']); + created.arr.should.deepEqual([1, 'str']); + created.date.should.be.an.instanceOf(Date); + created.date.toString().should.equal(d.toString(), 'Time must match'); + created.nestedClass.should.have.property('roleName', 'admin'); + + const found = await Model.findById(created.id); + should.exist(found); + found.list.should.deepEqual(['test']); + found.arr.should.deepEqual([1, 'str']); + found.date.should.be.an.instanceOf(Date); + found.date.toString().should.equal(d.toString(), 'Time must match'); + found.nestedClass.should.have.property('roleName', 'admin'); + }); + it('should respect data types when updating attributes', function(done) { const d = new Date; let id; diff --git a/test/model-builder.test.js b/test/model-builder.test.js index 58c4faab..4bd2613a 100644 --- a/test/model-builder.test.js +++ b/test/model-builder.test.js @@ -48,9 +48,37 @@ describe('ModelBuilder', () => { }); }); + describe('model with nested properties as function', () => { + const Role = function(roleName) {}; + it('sets correct nested properties', () => { + const User = builder.define('User', { + role: { + type: typeof Role, + default: null, + }, + }); + should.equal(User.getPropertyType('role'), 'ModelConstructor'); + }); + }); + + describe('model with nested properties as class', () => { + class Role { + constructor(roleName) {} + } + it('sets correct nested properties', () => { + const User = builder.define('UserWithClass', { + role: { + type: Role, + default: null, + }, + }); + User.registerProperty('role'); + should.equal(User.getPropertyType('role'), 'Role'); + }); + }); + function givenModelBuilderInstance() { builder = new ModelBuilder(); } }); }); -