Merge pull request #1834 from strongloop/embedded-parent2

Add __parent reference to embedded models

lib/list.js

@@ -8,6 +8,9 @@
 const g = require('strong-globalize')();
 const util = require('util');
 const Any = require('./types').Types.Any;
+const {
+  applyParentProperty,
+} = require('./utils');
 
 module.exports = List;
 
@@ -61,6 +64,7 @@ function List(items, itemType, parent) {
   });
 
   if (parent) {
+    // List constructor now called with actual model instance
     Object.defineProperty(arr, 'parent', {
       writable: true,
       enumerable: false,
@@ -74,6 +78,7 @@ function List(items, itemType, parent) {
     } else {
       arr[i] = item;
     }
+    if (parent && arr[i] && typeof arr[i] === 'object') applyParentProperty(arr[i], parent);
   });
 
   return arr;
@@ -100,6 +105,7 @@ List.prototype.toItem = function(item) {
 
 List.prototype.push = function(obj) {
   const item = this.itemType && (obj instanceof this.itemType) ? obj : this.toItem(obj);
+  if (item && typeof item === 'object' && this.parent) applyParentProperty(item, this.parent);
   _push.call(this, item);
   return item;
 };

lib/model-builder.js

@@ -24,6 +24,7 @@ const {
   deepMergeProperty,
   rankArrayElements,
   isClass,
+  applyParentProperty,
 } = require('./utils');
 
 // Set up types
@@ -596,14 +597,18 @@ ModelBuilder.prototype.define = function defineClass(className, properties, sett
           } else {
             if (DataType === List) {
               this.__data[propertyName] = isClass(DataType) ?
-                new DataType(value, properties[propertyName].type, this.__data) :
-                DataType(value, properties[propertyName].type, this.__data);
+                new DataType(value, properties[propertyName].type, this) :
+                DataType(value, properties[propertyName].type, this);
             } else {
               // Assume the type constructor handles Constructor() call
               // If not, we should call new DataType(value).valueOf();
               this.__data[propertyName] = (value instanceof DataType) ?
                 value :
                 isClass(DataType) ? new DataType(value) : DataType(value);
+              if (value && this.__data[propertyName] instanceof DefaultModelBaseClass) {
+                // we are dealing with an embedded model, apply parent
+                applyParentProperty(this.__data[propertyName], this);
+              }
             }
           }
         }

lib/model.js

@@ -353,6 +353,7 @@ ModelBaseClass.prototype._initProperties = function(data, options) {
             typeof self.__data[p] === 'object' &&
             self.__data[p] !== null) {
           self.__data[p] = new type(self.__data[p]);
+          utils.applyParentProperty(self.__data[p], this);
         }
       } else if (type.name === 'Array' || Array.isArray(type)) {
         if (!(self.__data[p] instanceof List) &&

lib/utils.js

@@ -30,12 +30,25 @@ exports.rankArrayElements = rankArrayElements;
 exports.idsHaveDuplicates = idsHaveDuplicates;
 exports.isClass = isClass;
 exports.escapeRegExp = escapeRegExp;
+exports.applyParentProperty = applyParentProperty;
 
 const g = require('strong-globalize')();
 const traverse = require('traverse');
 const assert = require('assert');
 const debug = require('debug')('loopback:juggler:utils');
 
+/**
+ * The name of the property in modelBuilder settings that will enable the child parent reference functionality
+ * @type {string}
+ */
+const BUILDER_PARENT_SETTING = 'parentRef';
+
+/**
+ * The property name that should be defined on each child instance if parent feature flag enabled
+ * @type {string}
+ */
+const PARENT_PROPERTY_NAME = '__parent';
+
 function safeRequire(module) {
   try {
     return require(module);
@@ -842,3 +855,41 @@ function idsHaveDuplicates(ids) {
 function isClass(fn) {
   return fn && fn.toString().startsWith('class ');
 }
+
+/**
+ * Accept an element, and attach the __parent property to it, unless no object given, while also
+ * making sure to check for already created properties
+ *
+ * @param {object} element
+ * @param {Model} parent
+ */
+function applyParentProperty(element, parent) {
+  assert.strictEqual(typeof element, 'object', 'Non object element given to assign parent');
+  const {constructor: {modelBuilder: {settings: builderSettings} = {}} = {}} = element;
+  if (!builderSettings || !builderSettings[BUILDER_PARENT_SETTING]) {
+    // parentRef flag not enabled on ModelBuilder settings
+    return;
+  }
+
+  if (element.hasOwnProperty(PARENT_PROPERTY_NAME)) {
+    // property already created on model, just assign
+    const existingParent = element[PARENT_PROPERTY_NAME];
+    if (existingParent && existingParent !== parent) {
+      // parent re-assigned (child model assigned to other model instance)
+      g.warn('Re-assigning child model instance to another parent than the original!\n' +
+        'Although supported, this is not a recommended practice: ' +
+        `${element.constructor.name} -> ${parent.constructor.name}\n` +
+        'You should create an independent copy of the child model using `new Model(CHILD)` OR ' +
+        '`new Model(CHILD.toJSON())` and assign to new parent');
+    }
+    element[PARENT_PROPERTY_NAME] = parent;
+  } else {
+    // first time defining the property on the element
+    Object.defineProperty(element, PARENT_PROPERTY_NAME, {
+      value: parent,
+      writable: true,
+      enumerable: false,
+      configurable: false,
+    });
+  }
+}

test/basic-querying.test.js

@@ -11,6 +11,7 @@ const async = require('async');
 const bdd = require('./helpers/bdd-if');
 const should = require('./init.js');
 const uid = require('./helpers/uid-generator');
+const createTestSetupForParentRef = require('./helpers/setup-parent-ref');
 
 let db, User;
 
@@ -958,6 +959,29 @@ describe('basic-querying', function() {
         null, // databases representing `undefined` as `null` (e.g. SQL)
       ]);
     });
+
+    describe('check __parent relationship in embedded models', () => {
+      createTestSetupForParentRef(() => User.modelBuilder);
+      it('should fill the parent in embedded model', async () => {
+        const user = await User.findOne({where: {name: 'John Lennon'}});
+        user.should.have.property('address');
+        should(user.address).have.property('__parent');
+        should(user.address.__parent).be.instanceof(User).and.equal(user);
+      });
+      it('should assign the container model as parent in list property', async () => {
+        const user = await User.findOne({where: {name: 'John Lennon'}});
+        user.should.have.property('friends');
+        should(user.friends).have.property('parent');
+        should(user.friends.parent).be.instanceof(User).and.equal(user);
+      });
+      it('should have the complete chain of parents available in embedded list element', async () => {
+        const user = await User.findOne({where: {name: 'John Lennon'}});
+        user.friends.forEach((userFriend) => {
+          userFriend.should.have.property('__parent');
+          should(userFriend.__parent).equal(user);
+        });
+      });
+    });
   });
 
   describe('count', function() {

test/helpers/setup-parent-ref.js

@@ -0,0 +1,24 @@
+'use strict';
+
+const assert = require('assert');
+
+/**
+ * Helper function that when called should return the current instance of the modelBuilder
+ * @param {function: ModelBuilder} getBuilder
+ */
+const createTestSetupForParentRef = (getBuilder) => {
+  assert.strictEqual(typeof getBuilder, 'function', 'Missing getter function for model builder');
+  const settingProperty = 'parentRef';
+  beforeEach('enabling parentRef for given modelBuilder', () => {
+    const modelBuilder = getBuilder();
+    assert(modelBuilder && typeof modelBuilder === 'object', 'Invalid modelBuilder instance');
+    modelBuilder.settings[settingProperty] = true;
+  });
+  afterEach('Disabling parentRef for given modelBuilder', () => {
+    const modelBuilder = getBuilder();
+    assert(modelBuilder && typeof modelBuilder === 'object', 'Invalid modelBuilder instance');
+    modelBuilder.settings[settingProperty] = false;
+  });
+};
+
+module.exports = createTestSetupForParentRef;

test/list.test.js

@@ -7,6 +7,10 @@
 
 const should = require('./init.js');
 const List = require('../lib/list');
+const parentRefHelper = require('./helpers/setup-parent-ref');
+const {ModelBuilder} = require('../lib/model-builder');
+
+const builder = new ModelBuilder(); // dummy builder instance for tests
 
 /**
  * Phone as a class
@@ -25,6 +29,12 @@ class Phone {
   }
 }
 
+/**
+ * Dummy property for testing parent reference
+ * @type {ModelBuilder}
+ */
+Phone.modelBuilder = builder;
+
 /**
  * Phone as a constructor function
  * @param {string} label
@@ -38,6 +48,12 @@ function PhoneCtor(label, num) {
   this.num = num;
 }
 
+/**
+ * Dummy property for testing parent reference
+ * @type {ModelBuilder}
+ */
+PhoneCtor.modelBuilder = builder;
+
 describe('Does not break default Array functionality', function() {
   it('allows creating an empty length with a specified length', function() {
     const list = new List(4);
@@ -49,6 +65,7 @@ describe('Does not break default Array functionality', function() {
 });
 
 describe('list of items typed by a class', function() {
+  parentRefHelper(() => builder);
   it('allows itemType to be a class', function() {
     const phones = givenPhones();
 
@@ -78,9 +95,29 @@ describe('list of items typed by a class', function() {
     list.push(phones[0]);
     list[0].should.be.an.instanceOf(Phone);
   });
+
+  it('should assign the list\'s parent as parent to every child element', () => {
+    const phones = givenPhones();
+    const listParent = {name: 'PhoneBook'};
+    const list = new List(phones, Phone, listParent);
+    list.forEach((listItem) => {
+      listItem.should.have.property('__parent').which.equals(listParent);
+    });
+  });
+
+  it('should assign the list\'s parent as element parent with push', () => {
+    const phones = givenPhonesAsJSON();
+    const listParent = {name: 'PhoneBook'};
+    const list = new List([], Phone, listParent);
+    list.push(phones[0], phones[1]);
+    list.forEach((listItem) => {
+      listItem.should.have.property('__parent').which.equals(listParent);
+    });
+  });
 });
 
 describe('list of items typed by a ctor', function() {
+  parentRefHelper(() => builder);
   it('allows itemType to be a ctor', function() {
     const phones = givenPhonesWithCtor();
 
@@ -110,6 +147,25 @@ describe('list of items typed by a ctor', function() {
     list.push(phones[0]);
     list[0].should.be.an.instanceOf(PhoneCtor);
   });
+
+  it('should assign the list\'s parent as parent to every child element', () => {
+    const phones = givenPhones();
+    const listParent = {name: 'PhoneBook'};
+    const list = new List(phones, PhoneCtor, listParent);
+    list.forEach((listItem) => {
+      listItem.should.have.property('__parent').which.equals(listParent);
+    });
+  });
+
+  it('should assign the list\'s parent as element parent with push', () => {
+    const phones = givenPhonesAsJSON();
+    const listParent = {name: 'PhoneBook'};
+    const list = new List([], PhoneCtor, listParent);
+    list.push(phones[0], phones[1]);
+    list.forEach((listItem) => {
+      listItem.should.have.property('__parent').which.equals(listParent);
+    });
+  });
 });
 
 function givenPhones() {

test/model-builder.test.js

@@ -9,6 +9,8 @@ const should = require('./init.js');
 
 const juggler = require('../');
 const ModelBuilder = juggler.ModelBuilder;
+const {StrongGlobalize} = require('strong-globalize');
+const parentRefHelper = require('./helpers/setup-parent-ref');
 
 describe('ModelBuilder', () => {
   describe('define()', () => {
@@ -77,6 +79,123 @@ describe('ModelBuilder', () => {
       });
     });
 
+    describe('model with nested properties as embedded model', () => {
+      let Address, Person;
+      const originalWarn = StrongGlobalize.prototype.warn;
+      parentRefHelper(() => builder);
+      before('create stub for warning check', () => {
+        StrongGlobalize.prototype.warn = function gWarnWrapper(...args) {
+          StrongGlobalize.prototype.warn.called++;
+          return originalWarn.apply(this, args);
+        };
+        StrongGlobalize.prototype.warn.called = 0;
+      });
+      beforeEach('Define models', () => {
+        Address = builder.define('Address', {
+          street: {type: 'string'},
+          number: {type: 'number'},
+        });
+        Person = builder.define('Person', {
+          name: {type: 'string'},
+          address: {type: 'Address'},
+          other: {type: 'object'},
+        });
+      });
+      after('restore warning stub', () => {
+        StrongGlobalize.prototype.warn = originalWarn;
+      });
+      it('should properly add the __parent relationship when instantiating parent model', () => {
+        const person = new Person({
+          name: 'Mitsos',
+          address: {street: 'kopria', number: 11},
+        });
+        person.should.have.propertyByPath('address', '__parent').which.equals(person);
+      });
+      it('should add _parent property when setting embedded model after instantiation', () => {
+        const person = new Person({
+          name: 'Mitsos',
+        });
+        person.address = {street: 'kopria', number: 11};
+        person.should.have.propertyByPath('address', '__parent').which.equals(person);
+      });
+      it('should handle nullish embedded property values', () => {
+        const person = new Person({
+          name: 'Mitsos',
+          address: null,
+        });
+        person.should.have.property('address').which.equals(null);
+      });
+      it('should change __parent reference and WARN when moving a child instance to an other parent', () => {
+        const person1 = new Person({
+          name: 'Mitsos',
+          address: {street: 'kopria', number: 11},
+        });
+        const {address} = person1;
+        address.should.be.instanceof(Address).and.have.property('__parent').which.equals(person1);
+        StrongGlobalize.prototype.warn.should.have.property('called', 0); // check that no warn yet
+        const person2 = new Person({
+          name: 'Allos',
+          address,
+        });
+        address.should.have.property('__parent').which.equals(person2);
+        StrongGlobalize.prototype.warn.should.have.property('called', 1); // check we had a warning
+      });
+      it('should NOT provide the __parent property to any serialization of the instance', () => {
+        const person = new Person({
+          name: 'Mitsos',
+          address: {street: 'kopria', number: 11},
+        });
+        person.toJSON().should.not.have.propertyByPath('address', '__parent');
+        person.toObject().should.not.have.propertyByPath('address', '__parent');
+      });
+      it('should NOT provide __parent property in plain object properties', () => {
+        const person = new Person({
+          name: 'Mitsos',
+          address: {street: 'kopria', number: 11},
+          other: {some: 'object'},
+        });
+        person.should.have.property('other').which.eql({some: 'object'}).and.not.has
+          .property('__parent');
+      });
+    });
+
+    describe('Model with properties as list of embedded models', () => {
+      let Person, Address;
+      beforeEach('Define models', () => {
+        Address = builder.define('Address', {
+          street: {type: 'string'},
+          number: {type: 'number'},
+        });
+        Person = builder.define('Person', {
+          name: {type: 'string'},
+          addresses: {type: ['Address']}, // array of addresses
+        });
+      });
+      it('should pass the container model instance as parent to the list item', () => {
+        const person = new Person({
+          name: 'mitsos',
+          addresses: [{
+            street: 'kapou oraia',
+            number: 100,
+          }],
+        });
+        person.should.have.property('addresses').which.has.property('parent')
+          .which.is.instanceof(Person).and.equals(person);
+      });
+      it('should pass the container model instance as parent to the list, when assigning to ' +
+        'the list property', () => {
+        const person = new Person({
+          name: 'mitsos',
+        });
+        person.addresses = [{
+          street: 'kapou oraia',
+          number: 100,
+        }];
+        person.should.have.property('addresses').which.has.property('parent')
+          .which.is.instanceof(Person).and.equals(person);
+      });
+    });
+
     function givenModelBuilderInstance() {
       builder = new ModelBuilder();
     }