diff --git a/lib/model-builder.js b/lib/model-builder.js index 6faf2c1e..62aab2fe 100644 --- a/lib/model-builder.js +++ b/lib/model-builder.js @@ -185,16 +185,8 @@ ModelBuilder.prototype.define = function defineClass(className, properties, sett // Create the ModelClass if it doesn't exist or it's resolved (override) // TODO: [rfeng] We need to decide what names to use for built-in models such as User. if (!ModelClass || !ModelClass.settings.unresolved) { - // every class can receive hash of data as optional param - ModelClass = function ModelConstructor(data, options) { - if (!(this instanceof ModelConstructor)) { - return new ModelConstructor(data, options); - } - if (ModelClass.settings.unresolved) { - throw new Error(g.f('Model %s is not defined.', ModelClass.modelName)); - } - ModelBaseClass.apply(this, arguments); - }; + ModelClass = createModelClassCtor(className, ModelBaseClass); + // mix in EventEmitter (don't inherit from) var events = new EventEmitter(); // The model can have more than 10 listeners for lazy relationship setup @@ -663,6 +655,44 @@ ModelBuilder.prototype.define = function defineClass(className, properties, sett return ModelClass; }; +function createModelClassCtor(name, ModelBaseClass) { + // A simple sanitization to handle most common characters + // that are used in model names but cannot be used as a function/class name. + // Note that the rules for valid JS indentifiers are way too complex, + // implementing a fully spec-compliant sanitization is not worth the effort. + // See https://mathiasbynens.be/notes/javascript-identifiers-es6 + name = name.replace(/[-.:]/g, '_'); + + try { + // It is not possible to access closure variables like "ModelBaseClass" + // from a dynamically defined function. The solution is to + // create a dynamically defined factory function that accepts + // closure variables as arguments. + const factory = new Function('ModelBaseClass', ` + // every class can receive hash of data as optional param + return function ${name}(data, options) { + if (!(this instanceof ${name})) { + return new ${name}(data, options); + } + if (${name}.settings.unresolved) { + throw new Error(g.f('Model %s is not defined.', ${JSON.stringify(name)})); + } + ModelBaseClass.apply(this, arguments); + };`); + + return factory(ModelBaseClass); + } catch (err) { + // modelName is not a valid function/class name, e.g. 'grand-child' + // and our simple sanitization was not good enough. + // Falling back to legacy 'ModelConstructor' name. + if (err.name === 'SyntaxError') { + return createModelClassCtor('ModelConstructor', ModelBaseClass); + } else { + throw err; + } + } +} + // DataType for Date function DateType(arg) { if (arg === null) return null; diff --git a/test/model-builder.test.js b/test/model-builder.test.js new file mode 100644 index 00000000..90093195 --- /dev/null +++ b/test/model-builder.test.js @@ -0,0 +1,56 @@ +// Copyright IBM Corp. 2018. All Rights Reserved. +// Node module: loopback-datasource-juggler +// This file is licensed under the MIT License. +// License text available at https://opensource.org/licenses/MIT + +'use strict'; + +const should = require('./init.js'); + +const juggler = require('../'); +var ModelBuilder = juggler.ModelBuilder; + +describe('ModelBuilder', () => { + describe('define()', () => { + let builder; + + beforeEach(givenModelBuilderInstance); + + it('sets correct "modelName" property', () => { + const MyModel = builder.define('MyModel'); + MyModel.should.have.property('modelName', 'MyModel'); + }); + + it('sets correct "name" property on model constructor', () => { + const MyModel = builder.define('MyModel'); + MyModel.should.have.property('name', 'MyModel'); + }); + + describe('model class name sanitization', () => { + it('converts "-" to "_"', () => { + const MyModel = builder.define('Grand-child'); + MyModel.should.have.property('name', 'Grand_child'); + }); + + it('converts "." to "_"', () => { + const MyModel = builder.define('Grand.child'); + MyModel.should.have.property('name', 'Grand_child'); + }); + + it('converts ":" to "_"', () => { + const MyModel = builder.define('local:User'); + MyModel.should.have.property('name', 'local_User'); + }); + + it('falls back to legacy "ModelConstructor" in other cases', () => { + const MyModel = builder.define('Grand\tchild'); + MyModel.should.have.property('name', 'ModelConstructor'); + }); + }); + + function givenModelBuilderInstance() { + builder = new ModelBuilder(); + } + }); +}); +