Set model constructor name to model name

Rework the code building model constructors to leverage `Function` class
and dynamically emit a constructor function named after the model.

Before this change, all model classes were called "ModelConstructor",
which made debugging difficult.

After this change, a model class for model "User" is called "User.

Because not all valid model names are also valid JavaScript identifiers,
we implement a simple sanitization technique (replacing characters like
"-", "." and ":" with underscore "_") and fall back to legacy
"ModelConstructor" if the model name is still not a valid JS identifier.

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 = createModelClassCtor(className, ModelBaseClass);
-    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);
-    };
     // 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;

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();
+    }
+  });
+});
+