From 52f8fb34160114be6ca45dd0b4fe092d0f5b45fc Mon Sep 17 00:00:00 2001 From: rashmihunt Date: Tue, 9 May 2017 16:31:54 -0700 Subject: [PATCH 1/4] handle excludeBaseProperties --- lib/model-builder.js | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/lib/model-builder.js b/lib/model-builder.js index 4a8aaee4..3de4a64a 100644 --- a/lib/model-builder.js +++ b/lib/model-builder.js @@ -256,8 +256,18 @@ ModelBuilder.prototype.define = function defineClass(className, properties, sett ModelClass.setter = {}; for (var p in properties) { - // Remove properties that reverted by the subclass - if (properties[p] === null || properties[p] === false) { + // e.g excludePropertyList = ['id'] - base properties listed in excludePropertyList will be excluded from the model. + // excludeBaseProperties is introduced in SOAP model generation only for now and below logic + // handles excludeBaseProperties. Generated SOAP model has base as 'Model' which means 'id' property gets added + // automatically and 'id' property shouldn't be there for SOAP models. idInjection = false will not work + // for SOAP generator case, since base 'Model' has already id property. 'id: false' at the property level will not + // work either for SOAP generator case since generators use ModelDefinition.create to create property in the model + // dynamically, that execution path has strict validation where doesn't accept 'id: false' in a property. + // See https://github.com/strongloop/loopback-workspace/issues/486 for some more details. + var excludePropertyList = settings['excludeBaseProperties']; + // Remove properties that reverted by the subclass of the property from excludePropertyList + if (properties[p] === null || properties[p] === false || + (excludePropertyList != null && excludePropertyList.indexOf(p) != -1)) { // Hide the base property delete properties[p]; } From 5aee1fe17e0f7179c7019f7a077916d1bbc7260d Mon Sep 17 00:00:00 2001 From: rashmihunt Date: Wed, 10 May 2017 12:00:24 -0700 Subject: [PATCH 2/4] test case to exclude base props --- test/exclude-base-props.test.js | 47 +++++++++++++++++++++++++++++++++ 1 file changed, 47 insertions(+) create mode 100644 test/exclude-base-props.test.js diff --git a/test/exclude-base-props.test.js b/test/exclude-base-props.test.js new file mode 100644 index 00000000..40135542 --- /dev/null +++ b/test/exclude-base-props.test.js @@ -0,0 +1,47 @@ +// Copyright IBM Corp. 2013,2016. 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 + +// This test written in mocha+should.js +'use strict'; +var should = require('./init.js'); +var assert = require('assert'); + +var jdb = require('../'); +var ModelBuilder = jdb.ModelBuilder; + +describe('exclude properties ', function() { + it('from base model', function(done) { + var ds = new ModelBuilder(); + // this excludes id property from 'base: Model' We still need to pass in idInjection: false since User model tries to + // add id again to the model. + var User = ds.define('User', {name: String, password: String}, + {idInjection: false, excludeBaseProperties: ['id']}); + // User will have these properties: name, password + var properties = User.definition.properties; + + var notFound = true; + for (var p in properties) { + if (p == 'id') { + notFound = false; // id should not be found in the properties list + } + } + assert.equal(notFound, true); + + // this excludes id property from the base model and and password property coming from base 'User' model since customer is + // extended from User. + var Customer = User.extend('Customer', {vip: {type: String}}, + {idInjection: false, excludeBaseProperties: ['password']}); + // Customer will have these properties: name, vip + properties = Customer.definition.properties; + notFound = true; + for (p in properties) { + if (p == 'id' || p == 'password') { + notFound = false; // id and password properties should not be found in the properties list + } + } + assert.equal(notFound, true); + done(); + }); +}); From fed0396d9e3d6ae384da7021defe14e549698c57 Mon Sep 17 00:00:00 2001 From: rashmihunt Date: Thu, 11 May 2017 09:59:39 -0700 Subject: [PATCH 3/4] code review, better asserts --- test/exclude-base-props.test.js | 24 +++++++----------------- 1 file changed, 7 insertions(+), 17 deletions(-) diff --git a/test/exclude-base-props.test.js b/test/exclude-base-props.test.js index 40135542..8bc6f2d9 100644 --- a/test/exclude-base-props.test.js +++ b/test/exclude-base-props.test.js @@ -20,28 +20,18 @@ describe('exclude properties ', function() { {idInjection: false, excludeBaseProperties: ['id']}); // User will have these properties: name, password var properties = User.definition.properties; - - var notFound = true; - for (var p in properties) { - if (p == 'id') { - notFound = false; // id should not be found in the properties list - } - } - assert.equal(notFound, true); - + assert(('name', 'password' in properties)); + // id should not be found in the properties list + assert(!('id' in properties)); // this excludes id property from the base model and and password property coming from base 'User' model since customer is // extended from User. var Customer = User.extend('Customer', {vip: {type: String}}, {idInjection: false, excludeBaseProperties: ['password']}); - // Customer will have these properties: name, vip + // Customer will have these properties: name(from UserModel) & vip properties = Customer.definition.properties; - notFound = true; - for (p in properties) { - if (p == 'id' || p == 'password') { - notFound = false; // id and password properties should not be found in the properties list - } - } - assert.equal(notFound, true); + assert(('name', 'vip' in properties)); + // id and password properties should not be found in the properties list + assert(!('id', 'password' in properties)); done(); }); }); From 45bf569ec4b3f28b32c3102b0425d188622cdd06 Mon Sep 17 00:00:00 2001 From: rashmihunt Date: Fri, 12 May 2017 11:47:50 -0700 Subject: [PATCH 4/4] fix assert, make the test case more clear --- test/exclude-base-props.test.js | 33 ++++++++++++++++++--------------- 1 file changed, 18 insertions(+), 15 deletions(-) diff --git a/test/exclude-base-props.test.js b/test/exclude-base-props.test.js index 8bc6f2d9..14064093 100644 --- a/test/exclude-base-props.test.js +++ b/test/exclude-base-props.test.js @@ -14,24 +14,27 @@ var ModelBuilder = jdb.ModelBuilder; describe('exclude properties ', function() { it('from base model', function(done) { var ds = new ModelBuilder(); - // this excludes id property from 'base: Model' We still need to pass in idInjection: false since User model tries to - // add id again to the model. - var User = ds.define('User', {name: String, password: String}, - {idInjection: false, excludeBaseProperties: ['id']}); - // User will have these properties: name, password + // create a base model User which has name and password properties. id property gets + // internally created for the User Model + var User = ds.define('User', {name: String, password: String}); var properties = User.definition.properties; - assert(('name', 'password' in properties)); - // id should not be found in the properties list - assert(!('id' in properties)); - // this excludes id property from the base model and and password property coming from base 'User' model since customer is - // extended from User. + // User should have id, name & password properties + assert(('id' in properties) && ('password' in properties) && ('name' in properties), + 'User should have id, name & password properties'); + // Create sub model Customer with vip as property. id property gets automatically created here as well. + // Customer will inherit name, password and id from base User model. + // With excludeBaseProperties, 'password' and 'id' gets excluded from base User model + // With idInjection: false - id property of sub Model Customer gets excluded. At the end + // User will have these 2 properties: name (inherited from User model) and vip (from customer Model). var Customer = User.extend('Customer', {vip: {type: String}}, - {idInjection: false, excludeBaseProperties: ['password']}); - // Customer will have these properties: name(from UserModel) & vip + {idInjection: false, excludeBaseProperties: ['password', 'id']}); + // Customer should have these properties: name(from UserModel) & vip properties = Customer.definition.properties; - assert(('name', 'vip' in properties)); - // id and password properties should not be found in the properties list - assert(!('id', 'password' in properties)); + assert(('name' in properties) && ('vip' in properties), + 'Customer should have name and vip properties'); + // id or password properties should not be found in the properties list + assert(!(('id' in properties) || ('password' in properties)), + 'Customer should not have id or password properties'); done(); }); });