diff --git a/README.md b/README.md index d3b4e77..0b67705 100755 --- a/README.md +++ b/README.md @@ -1,5 +1,6 @@ # oe-common-mixins + - [Introduction](#introduction) * [dependency](#dependency) * [Getting Started](#getting-started) @@ -17,6 +18,10 @@ + [Loading Mixin using app-list.json](#loading-mixin-using-app-listjson-1) + [Loading Mixin pragmatically](#loading-mixin-pragmatically-1) * [Developer Considerations](#developer-considerations-1) + * [EmbedsOne Relation](#embedsone-relation) + + [POST - create parent record](#post---create-parent-record) + + [PUT - Update parent record](#put---update-parent-record) + + [PUT - Updating embedded record](#put---updating-embedded-record) - [Soft Delete Mixin](#soft-delete-mixin) * [Using SoftDeleteMixin](#using-softdeletemixin) + [Loading Mixin using model-config.json](#loading-mixin-using-model-configjson-1) @@ -272,6 +277,109 @@ This can be confusing because for some models you will pass version while deleti * If programmer calls updateAll method in javascript, version checking would not be possible as multiple records are getting updated. For such models where version mixin is enabled, you should disable updateAll method. If it is not disabled or it is somehow gets called, concurrent update of same records could be possible. +## EmbedsOne Relation +When model is Embedded, version mixin behavior is little different and you need to take care of special case. + +### POST - create parent record + +When you are creating record in parent model and passing data of embedded model along, you don't have to worry about anything special. + +Once record is created in parent model, _version will automatically generated and same _version field will be populated for both parent and embedded record. Response of such request will look like + +POST /api/Books +``` +{ + "name": "a", + "id": "a", + "_isDeleted": false, + "_version": "679d0579-67d2-4e12-83a1-f1c8c20981c9", + "publisher": { + "name": "a", + "age": 0, + "id": "a", + "_isDeleted": false, + "_version": "679d0579-67d2-4e12-83a1-f1c8c20981c9" + } +} +``` +**Validation** : Child record and parent record both are validated. + +### PUT - Update parent record + +For this operation, _version field must be populated in your request body. _version must be existing version of the existing record that you are trying to modify. +If no record with _version is found then oecloud will throw error. + +PUT /api/Books/a +``` +{ + "name": "a-changed", + "id": "a", + "_version": "679d0579-67d2-4e12-83a1-f1c8c20981c9" +} +``` +Response + +``` +{ + "name": "a-changed", + "id": "a", + "_isDeleted": false, + "_oldVersion": "679d0579-67d2-4e12-83a1-f1c8c20981c9", + "_version": "c289c6d2-4bdc-482a-8adc-76d215a402f5", + "publisher": { + "name": "a", + "age": 0, + "id": "a", + "_isDeleted": false, + "_version": "679d0579-67d2-4e12-83a1-f1c8c20981c9" + } +} +``` +Note that only parent record's _version is updated with new version. This will make _version field in embedded model and parent model go **out of sync**. + +**Validation** : only parent data is validated. + +### PUT - Updating embedded record + +This is tricky operation. Here you want to just update embedded model record. But since embedded data is not residing in separate collection (or table), you are really modifying parent record. Therefore you need to supply parent record's version as well as child record's version. Parent record's version is supplied in _parentVersion field. + +/api/Books/a/personRel +``` +{ + "name": "publsher updatted", + "age": 11, + "id": "n", + "_version": "679d0579-67d2-4e12-83a1-f1c8c20981c9", + "_parentVersion" :"c289c6d2-4bdc-482a-8adc-76d215a402f5" +} +``` +As you can see above, you will see both _version and _parentVersion is supplied. This becomes important because parent record's version and embedded record version went out of sync. + +However when both are in 'sync', you may either supply only _parentVersion or _version. +However it is good practice to supply both of these fields. + +Response of such request will look like + +``` +{ + "name": "publsher updatted", + "age": 11, + "id": "n", + "_isDeleted": false, + "_version": "486c7ea9-f1b6-413a-afdf-64210e72e81e", + "_parentVersion": "c289c6d2-4bdc-482a-8adc-76d215a402f5" +} +``` +if you go to database, you will find _version field of both parent and embedded record is same. + +**Validation** : All fields in embedded model are validated. However _version is validated for parent Model also. **before save** hooks on embedded Model and parent model are called. + + +**Embedded model without Version Mixin** +In above examples, we have assumed that embedded model will too have VersionMixin enabled. But if that is not the case, then it will not be possible to pass _parentVersion as **strict** flag on model is true by default. + + + # Soft Delete Mixin diff --git a/common/mixins/history-mixin.js b/common/mixins/history-mixin.js index 9625aeb..5d0b322 100755 --- a/common/mixins/history-mixin.js +++ b/common/mixins/history-mixin.js @@ -210,6 +210,9 @@ function createHistoryData(ctx, next) { if (!historyModel) { return next(); } + if (ctx && ctx.embedsOne ) { + return next(); + } if (ctx.currentInstance) { ctx.hookState.historyData = [ctx.currentInstance.toObject()]; return next(); @@ -238,13 +241,14 @@ function createHistoryData(ctx, next) { if (err) { return next(err); } + if (!data) { var e = new Error('Model ID error ' + id); e.message = 'Model ID error ' + id; return next(e); } ctx.hookState.historyData = [data.toObject()]; - ctx.currentInstance = data; + // ctx.currentInstance = data; return next(); }); } else { diff --git a/common/mixins/version-mixin.js b/common/mixins/version-mixin.js index 801d9eb..aa274e1 100755 --- a/common/mixins/version-mixin.js +++ b/common/mixins/version-mixin.js @@ -17,6 +17,7 @@ const uuidv4 = require('uuid/v4'); const oecloudutil = require('oe-cloud/lib/common/util'); +const isMixinEnabled = require('../../lib/utils').isMixinEnabled; module.exports = function VersionMixin(Model) { if (Model.modelName === 'BaseEntity') { @@ -42,6 +43,11 @@ module.exports = function VersionMixin(Model) { type: String }); + Model.defineProperty('_parentVersion', { + type: String + }); + + // Model.settings._versioning = true; // Model.settings.updateOnLoad = true; @@ -112,18 +118,48 @@ module.exports = function VersionMixin(Model) { data._version = data._newVersion || data._version || uuidv4(); delete data._oldVersion; delete data._newVersion; + } + if (ctx.Model.relations) { + var relations = ctx.Model.relations; + for (var r in ctx.Model.relations) { + if (relations[r].type !== 'embedsOne' && relations[r].type !== 'embedsMany') { + continue; + } + var keyFrom = relations[r].keyFrom; + if (!data._version && !ctx.isNewInstance && data[keyFrom] && typeof data[keyFrom] === 'object') { + if (Array.isArray(data[keyFrom]) && data[keyFrom].length > 0 ) { + // Atul : For embeds many, it will be array + data._version = data[keyFrom][0]._parentVersion || data[keyFrom][0]._version; + } else { + data._version = data[keyFrom]._parentVersion || data[keyFrom]._version; + } + break; + } else if (ctx.isNewInstance && isMixinEnabled(relations[r].modelTo, 'VersionMixin')) { + if (relations[r].type === 'embedsOne' && data[keyFrom]) { + data[keyFrom]._version = data._version; + } else if (relations[r].type === 'embedsMany' && Array.isArray(data[keyFrom]) && data[keyFrom].length) { + data[keyFrom].forEach(function (item) { + item._version = data._version; + }); + } + } + } + } + if (ctx.isNewInstance) { return next(); } + var error; var id = oecloudutil.getIdValue(ctx.Model, data); var _version = data._version; + if (!data._version) { error = new Error(); Object.assign(error, { name: 'Data Error', message: 'Version must be defined. id ' + id + ' for model ' + Model.modelName, code: 'DATA_ERROR_071', type: 'DataModifiedError', retriable: false, status: 422 }); return next(error); } if (ctx.currentInstance) { - if (ctx.currentInstance._version !== ctx.data._version) { + if (ctx.currentInstance._version !== data._version) { error = new Error(); Object.assign(error, { name: 'Data Error', message: 'Version must be be same. id ' + id + ' for model ' + Model.modelName + ' Version ' + _version + ' <> ' + ctx.currentInstance._version, code: 'DATA_ERROR_071', type: 'DataModifiedError', retriable: false, status: 422 }); return next(error); diff --git a/lib/utils.js b/lib/utils.js index dfefa4b..8f85a82 100755 --- a/lib/utils.js +++ b/lib/utils.js @@ -1,8 +1,8 @@ -/** - * - * ©2018-2019 EdgeVerve Systems Limited (a fully owned Infosys subsidiary), - * Bangalore, India. All Rights Reserved. - * +/** + * + * ©2018-2019 EdgeVerve Systems Limited (a fully owned Infosys subsidiary), + * Bangalore, India. All Rights Reserved. + * */ /** * This is file contains utility function to check if version of record being updated @@ -12,9 +12,10 @@ * @author : Atul Pandit */ -// Author : Atul +// Author : Atul const log = require('oe-logger')('oe-common-mixins-utils'); const oeutils = require('oe-cloud/lib/common/util'); +const loopback = require('loopback'); module.exports.checkIfVersionMatched = function checkIfVersionMatched(Model, id, version, next) { if (!version) { var error = new Error(); @@ -52,4 +53,27 @@ module.exports.checkIfVersionMatched = function checkIfVersionMatched(Model, id, }); }; +module.exports.isMixinEnabled = function isMixinEnabled(model, mixin) { + var Model; + + if (typeof model === 'string') { + Model = loopback.findModel(model); + } else { + Model = model; + } + if (!Model.settings || !Model.settings.mixins) { + return false; + } + var flag = Model.settings.mixins[mixin]; + if (!flag) { + return false; + } + if (flag) { + if (Model.settings.overridingMixins && !Model.settings.overridingMixins[mixin]) { + return false; + } + } + return flag; +}; + diff --git a/lib/wrapper.js b/lib/wrapper.js index fc707f4..2d80cc4 100755 --- a/lib/wrapper.js +++ b/lib/wrapper.js @@ -13,6 +13,9 @@ const oeutils = require('oe-cloud/lib/common/util.js'); const uuidv4 = require('uuid/v4'); const g = require('strong-globalize')(); const utils = require('./utils'); +const EmbedsOne = require('loopback-datasource-juggler/lib/relation-definition').EmbedsOne; +const isMixinEnabled = require('./utils').isMixinEnabled; + module.exports = function (app) { var _setup = DataSource.prototype.setup; @@ -21,29 +24,7 @@ module.exports = function (app) { var _upsert = {}; var _generateContextData = {}; var _destroyAll = {}; - - function isMixinEnabled(model, mixin) { - var Model; - - if (typeof model === 'string') { - Model = loopback.findModel(model); - } else { - Model = model; - } - if (!Model.settings || !Model.settings.mixins) { - return false; - } - var flag = Model.settings.mixins[mixin]; - if (!flag) { - return false; - } - if (flag) { - if (Model.settings.overridingMixins && !Model.settings.overridingMixins[mixin]) { - return false; - } - } - return flag; - } + var _save = {}; function updateWithWhere(self, model, where, data, options, cb, throwError, caller) { self.update(model, where, data, options, function (err, results) { @@ -79,7 +60,8 @@ module.exports = function (app) { function callWithVersion(fn, model, id, data, options, cb) { var Model = loopback.findModel(model); if (!isMixinEnabled(model, 'VersionMixin')) { - if (fn.name === 'updateOrCreate') { + var functionName = fn.functionName || fn.name; + if (functionName === 'updateOrCreate' || functionName === 'save') { return fn.call(this, model, data, options, cb); } @@ -93,6 +75,29 @@ module.exports = function (app) { where.and.push(o, { _version: data._version }); data._oldVersion = data._version; data._version = data._newVersion || uuidv4(); + + if (Model.relations) { + var relations = Model.relations; + for (var r in Model.relations) { + if (relations[r].type !== 'embedsOne' && relations[r].type !== 'embedsMany') { + continue; + } + if (!isMixinEnabled(relations[r].modelTo, 'VersionMixin')) { + continue; + } + var keyFrom = relations[r].keyFrom; + if (!keyFrom || !data[keyFrom]) { + continue; + } + if (relations[r].type === 'embedsOne') { + data[keyFrom]._version = data._version; + } else if (data[keyFrom].length) { + data[keyFrom].forEach(function (item) { + item._version = data._version; + }); + } + } + } return updateWithWhere(this, model, where, data, options, cb, true, 'update'); } @@ -105,14 +110,33 @@ module.exports = function (app) { _updateAttributes[this.name] = connector.updateAttributes; connector.updateAttributes = function (model, id, data, options, cb) { var fn = _updateAttributes[this.dataSource.name]; + fn.functionName = 'updateAttributes'; callWithVersion.call(this, fn, model, id, data, options, cb); }; } + + if (this.name && !_save[this.name] && connector.save) { + _save[this.name] = connector.save; + connector.save = function (model, data, options, cb) { + var fn = _save[this.dataSource.name]; + fn.functionName = 'save'; + if ( !cb && typeof options === 'function' ) { + cb = options; + options = {}; + } + var Model = loopback.findModel(model); + var idField = oeutils.idName(Model); + callWithVersion.call(this, fn, model, data[idField], data, options, cb); + }; + } + + if (this.name && !_destroyAll[this.name]) { _destroyAll[this.name] = connector.destroyAll; connector.destroyAll = function (model, where, options, cb) { var fn = _destroyAll[this.dataSource.name]; + fn.functionName = 'destroyAll'; convertDestroyToUpdate.call(this, fn, model, where, options, cb); }; } @@ -121,6 +145,7 @@ module.exports = function (app) { _replaceById[this.name] = connector.replaceById; connector.replaceById = function (model, id, data, options, cb) { var fn = _replaceById[this.dataSource.name]; + fn.functionName = 'replaceById'; callWithVersion.call(this, fn, model, id, data, options, cb); }; } @@ -138,7 +163,10 @@ module.exports = function (app) { _upsert[this.name] = connector.updateOrCreate; connector.updateOrCreate = function (model, data, options, cb) { var fn = _upsert[this.dataSource.name]; - callWithVersion.call(this, fn, model, data.id, data, options, cb); + fn.functionName = 'updateOrCreate'; + var Model = loopback.findModel(model); + var idField = oeutils.idName(Model); + callWithVersion.call(this, fn, model, data[idField], data, options, cb); }; } }); @@ -173,6 +201,24 @@ module.exports = function (app) { }); return; }; -}; + // Atul : Overriding to update version field with parent version + const _embedUpdate = EmbedsOne.prototype.update; + EmbedsOne.prototype.update = function (targetModelData, options, cb) { + if (!isMixinEnabled(this.modelInstance.constructor, 'VersionMixin')) { + return _embedUpdate.call(this, targetModelData, options, cb); + } + this.modelInstance._version = targetModelData._parentVersion || targetModelData._version; + return _embedUpdate.call(this, targetModelData, options, cb); + }; + + const _embedCreate = EmbedsOne.prototype.create; + EmbedsOne.prototype.create = function (targetModelData, options, cb) { + if (!isMixinEnabled(this.modelInstance.constructor, 'VersionMixin')) { + return _embedCreate.call(this, targetModelData, options, cb); + } + this.modelInstance._version = targetModelData._parentVersion || targetModelData._version; + return _embedCreate.call(this, targetModelData, options, cb); + }; +}; diff --git a/package.json b/package.json index 1c9c6cd..66d5f8a 100755 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "oe-common-mixins", - "version": "2.2.0", + "version": "2.3.0", "description": "oe-cloud modularization project aka oecloud.io", "engines": { "node": ">=6" diff --git a/test/common/models/Book.json b/test/common/models/Book.json new file mode 100644 index 0000000..bf3daa8 --- /dev/null +++ b/test/common/models/Book.json @@ -0,0 +1,28 @@ +{ + "name": "Book", + "base": "BaseEntity", + "idInjection": true, + "properties": { + "name": { + "type": "string", + "unique": true, + "required" : true + } + }, + "validations": [], + "acls": [], + "mixins": { + "SoftDeleteMixin": true, + "HistoryMixin": true, + "VersionMixin": true + }, + "relations": { + "personRel" : { + "type" : "embedsOne", + "model" : "Person", + "property" : "publisher" + } + }, + "methods": {} +} + diff --git a/test/common/models/Page.json b/test/common/models/Page.json new file mode 100644 index 0000000..072a81b --- /dev/null +++ b/test/common/models/Page.json @@ -0,0 +1,22 @@ +{ + "name": "Page", + "base": "BaseEntity", + "idInjection": true, + "properties": { + "pagename": { + "type": "string", + "unique": true, + "required" : true + } + }, + "mixins": { + "SoftDeleteMixin": true, + "VersionMixin": true + }, + "validations": [], + "acls": [], + "relations": {}, + "methods": {} + +} + diff --git a/test/common/models/Person.json b/test/common/models/Person.json index f6c7dc6..cbd9968 100755 --- a/test/common/models/Person.json +++ b/test/common/models/Person.json @@ -5,10 +5,12 @@ "properties": { "name": { "type": "string", - "unique": true + "unique": true, + "required" : true }, "age": { - "type": "number" + "type": "number", + "required" : true } }, "mixins": { diff --git a/test/history-mixin-test.js b/test/history-mixin-test.js index 77f019f..5bcdc48 100755 --- a/test/history-mixin-test.js +++ b/test/history-mixin-test.js @@ -11,11 +11,7 @@ var loopback = require('loopback'); var bootstrap = require('./bootstrap'); const uuidv4 = require('uuid/v4'); var debug = require('debug')('history-mixin-test'); -<<<<<<< HEAD -/*var oecloud = require('oe-cloud'); -======= /* var oecloud = require('oe-cloud'); ->>>>>>> 7f557916e89c01383cb8281e7b91dc879dda06f7 var loopback = require('loopback'); oecloud.observe('loaded', function (ctx, next) { @@ -268,40 +264,6 @@ describe(chalk.blue('History Mixin Test Started'), function (done) { }); -<<<<<<< HEAD - it('t6 (oecloud 2.x test) create record and then update using replacebyid', function (done) { - this.timeout(50000); - var postData = { - id : 123, - name : "Atul", - age : 30 - }; - var customerModel = loopback.findModel('Customer'); - customerModel.create(postData, globalCtx, function (err, customer) { - if (err) { - return done(err); - } else { - customerModel.replaceById(customer.id, {name: "Atul111", age : 31, _version : customer._version}, globalCtx, function(err, customer2){ - if(err){ - return done(err); - } - var newData = customer2.toObject(); - newData.name = 'Atul222'; - customerModel.replaceOrCreate(newData, globalCtx, function(err, customer3){ - if(err){ - return done(err); - } - customer3.updateAttributes({name : "Atul3333", age : 35, id : customer3.id, _version : customer3._version}, globalCtx, function(err, customer4){ - if(err){ - return done(err); - } - if(customer4.name !== 'Atul3333' || customer4.age !== 35){ - return done(new Error("data not matching. expetcing name change")); - } - var url = basePath + '/Customers/history?filter={"where" : { "_modelId" : 123 } }'; - - api -======= it('t6 (oecloud 2.x test) create record and then update using replacebyid', function (done) { this.timeout(50000); var postData = { @@ -334,7 +296,6 @@ describe(chalk.blue('History Mixin Test Started'), function (done) { var url = basePath + '/Customers/history?filter={"where" : { "_modelId" : 123 } }'; api ->>>>>>> 7f557916e89c01383cb8281e7b91dc879dda06f7 .get(url) .send() .expect(200).end(function (err, historyRes) { @@ -346,19 +307,10 @@ describe(chalk.blue('History Mixin Test Started'), function (done) { return done(); } }); -<<<<<<< HEAD - }) - }) - }) - } - }); - }); -======= }) }) }) } }); }); ->>>>>>> 7f557916e89c01383cb8281e7b91dc879dda06f7 }); diff --git a/test/model-config.json b/test/model-config.json index 6ee30fa..6ca2d3c 100755 --- a/test/model-config.json +++ b/test/model-config.json @@ -53,5 +53,13 @@ "Migration": { "dataSource": "db", "public": true + }, + "Book" : { + "public" : true, + "dataSource" : "db" + }, + "Page" : { + "public" : true, + "dataSource" : "db" } } diff --git a/test/version-mixin-test.js b/test/version-mixin-test.js index 0229286..8907057 100755 --- a/test/version-mixin-test.js +++ b/test/version-mixin-test.js @@ -83,6 +83,12 @@ describe(chalk.blue('Version Mixin Test Started'), function (done) { done(); }); + it('waiting to start', function(done){ + setTimeout(function () { + return done(); + }, 5000); + }) + it('t1-0 create user admin/admin with /default tenant', function (done) { var url = basePath + '/users'; api.set('Accept', 'application/json') @@ -740,6 +746,158 @@ describe(chalk.blue('Version Mixin Test Started'), function (done) { }); }); }); + + it('t12 - 1 - EmbedsOne test case - POST in parent', function (done) { + + var d = { + "name": "n", + "id": "n", + "publisher": { + "name": "n", + "age": 0, + "id": "n" + } + }; + + const bookModel = loopback.findModel('Book'); + var url = basePath + '/books'; + api.set('Accept', 'application/json') + .post(url) + .send(d) + .end(function (err, response) { + if(err){ + return done(err); + } + var result = response.body; + expect(result.id).to.be.defined; + expect(result._version).to.be.defined; + expect(result.publisher._version).to.be.defined; + return done(); + }); + }); + + it('t12 - 2 - EmbedsOne test case - POST in embedded', function (done) { + var d = { + "name": "n-new", + "age": 0, + "id": "n" + }; + + const bookModel = loopback.findModel('Book'); + bookModel.find({where : {id : "n"}}, function(err, data){ + + if(err){ + return done(err); + } + d._version = data[0]._version; + var url = basePath + '/books/n/personRel'; + api.set('Accept', 'application/json') + .post(url) + .send(d) + .end(function (err, response) { + if(err){ + return done(err); + } + + var result = response.body; + if(result.error){ + return done(result); + } + expect(result.name).to.be.equal('n-new'); + return done(); + }); + }) + }) + + it('t12 - 3 - EmbedsOne test case - PUT in parent', function (done) { + var d = { + "name": "book name modified", + "id": "n" + }; + const bookModel = loopback.findModel('Book'); + bookModel.find({where : {id : "n"}}, function(err, data){ + if(err){ + return done(err); + } + d._version = data[0]._version; + var url = basePath + '/books/n'; + api.set('Accept', 'application/json') + .put(url) + .send(d) + .end(function (err, response) { + if(err){ + return done(err); + } + var result = response.body; + if(result.error){ + return done(result); + } + expect(result.name).to.be.equal('book name modified'); + expect(result.publisher._version).to.be.defined; + return done(); + }); + }) + }) + + it('t12 - 4 - EmbedsOne test case - PUT in embedded model (Version must match for embedded model)', function (done) { + var d = { + "name": "publisher modified", + "id": "n" + }; + const bookModel = loopback.findModel('Book'); + bookModel.find({where : {id : "n"}}, function(err, data){ + if(err){ + return done(err); + } + d._version = data[0]._version; + var url = basePath + '/books/n/personRel'; + api.set('Accept', 'application/json') + .put(url) + .send(d) + .end(function (err, response) { + if(err){ + return done(err) + } + var result = response.body; + if(!result.error){ + return done(new Error("Expecting error as version is out of sync and parent version must be provided")); + } + return done(); + }); + }) + }) + + it('t12 - 5 - EmbedsOne test case - PUT in embedded model (parentVersion must pass)', function (done) { + var d = { + "name": "publisher modified", + "id": "n" + }; + const bookModel = loopback.findModel('Book'); + bookModel.find({where : {id : "n"}}, function(err, data){ + if(err){ + return done(err); + } + d._parentVersion = data[0]._version; + d._version = data[0].publisher._version; + var url = basePath + '/books/n/personRel'; + api.set('Accept', 'application/json') + .put(url) + .send(d) + .end(function (err, response) { + if(err){ + return done(err) + } + var result = response.body; + if(result.error){ + return done(new Error(result.error)); + } + expect(result.name).to.be.equal('publisher modified'); + expect(result._version).to.be.defined; + return done(); + }); + }) + }) + });