From 9f0079537089f49c088982a755de6f84cb9f993d Mon Sep 17 00:00:00 2001 From: Jason Cruz Date: Tue, 24 Jan 2023 11:23:34 -0800 Subject: [PATCH] fix(numericids): update based on issue pr --- lib/model.js | 116 +++++++++++++++++++++++----------- lib/persisted-model.js | 22 +++++-- test/relations.integration.js | 4 +- test/remoting.integration.js | 38 +++++------ test/util/model-tests.js | 11 ++-- 5 files changed, 120 insertions(+), 71 deletions(-) diff --git a/lib/model.js b/lib/model.js index bcb9db08..c662bd4a 100644 --- a/lib/model.js +++ b/lib/model.js @@ -108,6 +108,40 @@ module.exports = function(registry) { Model.registry = registry; + + /** + * Fix for numeric IDs + */ + function getRemotingType(prop) { + if (prop.type === Number) { + return 'number'; + } + if (prop.type === boolean) { + return 'number' + } + + return 'string'; + } + + Model._getRemotingIdType = function getIdType(ModelCtor) { + var idName = ModelCtor.definition.idName(); + var idProp = ModelCtor.definition.properties[idName]; + return getRemotingType(idProp); + }; + + Model._getRemotingFkType = function getForeignKeyType(ModelCtor, relationName) { + var relation = ModelCtor.relations[relationName]; + var prop; + if (relation.type === 'belongsTo') { + prop = relation.modelFrom.definition.properties[relation.keyFrom]; + } else if (relation.modelThrough) { + prop = relation.modelThrough.definition.properties[relation.keyTo]; + } else { + prop = relation.modelTo.definition.properties[relation.keyTo]; + } + return getRemotingType(prop); + }; + /** * The `loopback.Model.extend()` method calls this when you create a model that extends another model. * Add any setup or configuration code you want executed when the model is created. @@ -183,9 +217,38 @@ module.exports = function(registry) { } }; + sharedClass.resolve(function resolver(define) { + var relations = ModelCtor.relations || {}; + + // get the relations + for (var relationName in relations) { + var relation = relations[relationName]; + if (relation.type === 'belongsTo') { + ModelCtor.belongsToRemoting(relationName, relation, define); + } else if ( + relation.type === 'hasOne' || + relation.type === 'embedsOne' + ) { + ModelCtor.hasOneRemoting(relationName, relation, define); + } else if ( + relation.type === 'hasMany' || + relation.type === 'embedsMany' || + relation.type === 'referencesMany') { + ModelCtor.hasManyRemoting(relationName, relation, define); + } + } + + // handle scopes + var scopes = ModelCtor.scopes || {}; + for (var scopeName in scopes) { + ModelCtor.scopeRemoting(scopeName, scopes[scopeName], define); + } + }); + var idDesc = ModelCtor.modelName + ' id'; + var idType = ModelCtor._getRemotingIdType(ModelCtor); ModelCtor.sharedCtor.accepts = [ - {arg: 'id', type: 'string', required: true, http: {source: 'path'}, + {arg: 'id', type: idType, required: true, http: {source: 'path'}, description: idDesc} // {arg: 'instance', type: 'object', http: {source: 'body'}} ]; @@ -196,6 +259,11 @@ module.exports = function(registry) { ModelCtor.sharedCtor.returns = {root: true}; + ModelCtor.once('dataSourceAttached', function() { + var idType = ModelCtor._getRemotingIdType(ModelCtor); + ModelCtor.sharedCtor.accepts[0].type = idType; + }); + // before remote hook ModelCtor.beforeRemote = function(name, fn) { var className = this.modelName; @@ -234,36 +302,6 @@ module.exports = function(registry) { }); }; - // resolve relation functions - sharedClass.resolve(function resolver(define) { - - var relations = ModelCtor.relations || {}; - - // get the relations - for (var relationName in relations) { - var relation = relations[relationName]; - if (relation.type === 'belongsTo') { - ModelCtor.belongsToRemoting(relationName, relation, define); - } else if ( - relation.type === 'hasOne' || - relation.type === 'embedsOne' - ) { - ModelCtor.hasOneRemoting(relationName, relation, define); - } else if ( - relation.type === 'hasMany' || - relation.type === 'embedsMany' || - relation.type === 'referencesMany') { - ModelCtor.hasManyRemoting(relationName, relation, define); - } - } - - // handle scopes - var scopes = ModelCtor.scopes || {}; - for (var scopeName in scopes) { - ModelCtor.scopeRemoting(scopeName, scopes[scopeName], define); - } - }); - return ModelCtor; }; @@ -515,11 +553,12 @@ module.exports = function(registry) { var pathName = (relation.options.http && relation.options.http.path) || relationName; var toModelName = relation.modelTo.modelName; + var fkType = this._getRemotingFkType(this, relationName); var findByIdFunc = this.prototype['__findById__' + relationName]; define('__findById__' + relationName, { isStatic: false, http: {verb: 'get', path: '/' + pathName + '/:fk'}, - accepts: {arg: 'fk', type: 'any', + accepts: {arg: 'fk', type: fkType, description: format('Foreign key for %s', relationName), required: true, http: {source: 'path'}}, @@ -533,7 +572,7 @@ module.exports = function(registry) { define('__destroyById__' + relationName, { isStatic: false, http: {verb: 'delete', path: '/' + pathName + '/:fk'}, - accepts: { arg: 'fk', type: 'any', + accepts: { arg: 'fk', type: fkType, description: format('Foreign key for %s', relationName), required: true, http: {source: 'path'}}, @@ -547,7 +586,7 @@ module.exports = function(registry) { isStatic: false, http: {verb: 'put', path: '/' + pathName + '/:fk'}, accepts: [ - {arg: 'fk', type: 'any', + {arg: 'fk', type: fkType, description: format('Foreign key for %s', relationName), required: true, http: { source: 'path' }}, @@ -571,7 +610,7 @@ module.exports = function(registry) { define('__link__' + relationName, { isStatic: false, http: {verb: 'put', path: '/' + pathName + '/rel/:fk'}, - accepts: [{ arg: 'fk', type: 'any', + accepts: [{ arg: 'fk', type: fkType, description: format('Foreign key for %s', relationName), required: true, http: {source: 'path'}}].concat(accepts), @@ -584,7 +623,7 @@ module.exports = function(registry) { define('__unlink__' + relationName, { isStatic: false, http: {verb: 'delete', path: '/' + pathName + '/rel/:fk'}, - accepts: {arg: 'fk', type: 'any', + accepts: {arg: 'fk', type: fkType, description: format('Foreign key for %s', relationName), required: true, http: {source: 'path'}}, @@ -599,7 +638,7 @@ module.exports = function(registry) { define('__exists__' + relationName, { isStatic: false, http: {verb: 'head', path: '/' + pathName + '/rel/:fk'}, - accepts: {arg: 'fk', type: 'any', + accepts: {arg: 'fk', type: fkType, description: format('Foreign key for %s', relationName), required: true, http: {source: 'path'}}, @@ -720,7 +759,8 @@ module.exports = function(registry) { httpPath = pathName + '/:' + paramName; acceptArgs = [ { - arg: paramName, type: 'any', http: { source: 'path' }, + arg: paramName, type: self._getRemotingFkType(self, relationName), + http: { source: 'path' }, description: format('Foreign key for %s.', relation.name), required: true, }, diff --git a/lib/persisted-model.js b/lib/persisted-model.js index 3ed3c737..63fa4cd0 100644 --- a/lib/persisted-model.js +++ b/lib/persisted-model.js @@ -47,6 +47,13 @@ module.exports = function(registry) { var PersistedModel = this; + PersistedModel.once('dataSourceAttached', function() { + // Defer the remoting setup so that PK/FK types are configured + // by connectors + console.log('Set up remoting for %s', PersistedModel.modelName); + PersistedModel.setupRemoting(); + }); + // enable change tracking (usually for replication) if (this.settings.trackChanges) { PersistedModel._defineChangeModel(); @@ -625,6 +632,7 @@ module.exports = function(registry) { var PersistedModel = this; var typeName = PersistedModel.modelName; var options = PersistedModel.settings; + var idType = this._getRemotingIdType(this); // This is just for LB 2.x options.replaceOnPUT = options.replaceOnPUT === true; @@ -692,7 +700,7 @@ module.exports = function(registry) { setRemoting(PersistedModel, 'exists', { description: 'Check whether a model instance exists in the data source.', accessType: 'READ', - accepts: {arg: 'id', type: 'string', description: 'Model id', required: true}, + accepts: {arg: 'id', type: idType, description: 'Model id', required: true}, returns: {arg: 'exists', type: 'boolean'}, http: [ {verb: 'get', path: '/:id/exists'}, @@ -724,7 +732,7 @@ module.exports = function(registry) { description: 'Find a model instance by {{id}} from the data source.', accessType: 'READ', accepts: [ - { arg: 'id', type: 'string', description: 'Model id', required: true, + { arg: 'id', type: idType, description: 'Model id', required: true, http: {source: 'path'}}, { arg: 'filter', type: 'object', description: 'Filter defining fields and include' }, @@ -738,7 +746,7 @@ module.exports = function(registry) { description: 'Replace attributes for a model instance and persist it into the data source.', accessType: 'WRITE', accepts: [ - { arg: 'id', type: 'string', description: 'Model id', required: true, + { arg: 'id', type: idType, description: 'Model id', required: true, http: { source: 'path' }}, { arg: 'data', type: 'object', model: typeName, http: { source: 'body' }, description: 'Model instance data' }, @@ -807,7 +815,7 @@ module.exports = function(registry) { aliases: ['destroyById', 'removeById'], description: 'Delete a model instance by {{id}} from the data source.', accessType: 'WRITE', - accepts: {arg: 'id', type: 'string', description: 'Model id', required: true, + accepts: {arg: 'id', type: idType, description: 'Model id', required: true, http: {source: 'path'}}, http: {verb: 'del', path: '/:id'}, returns: {arg: 'count', type: 'object', root: true} @@ -906,7 +914,7 @@ module.exports = function(registry) { description: 'Get the most recent change record for this instance.', accessType: 'READ', accepts: { - arg: 'id', type: 'string', required: true, http: { source: 'path' }, + arg: 'id', type: idType, required: true, http: { source: 'path' }, description: 'Model id' }, returns: { arg: 'result', type: this.Change.modelName, root: true }, @@ -920,7 +928,7 @@ module.exports = function(registry) { accessType: 'WRITE', accepts: [ { - arg: 'id', type: 'string', required: true, http: { source: 'path' }, + arg: 'id', type: idType, required: true, http: { source: 'path' }, description: 'Model id' }, { @@ -945,7 +953,7 @@ module.exports = function(registry) { setRemoting(PersistedModel, 'rectifyChange', { description: 'Tell loopback that a change to the model with the given id has occurred.', accessType: 'WRITE', - accepts: {arg: 'id', type: 'string', http: {source: 'path'}}, + accepts: {arg: 'id', type: idType, http: {source: 'path'}}, http: {verb: 'post', path: '/:id/rectify-change'} }); } diff --git a/test/relations.integration.js b/test/relations.integration.js index 9871edae..731d46ac 100644 --- a/test/relations.integration.js +++ b/test/relations.integration.js @@ -1599,12 +1599,12 @@ describe('relations - integration', function() { it('has a basic error handler', function(done) { var test = this; - this.get('/api/books/unknown/pages/' + test.page.id + '/notes') + this.get('/api/books/999/pages/' + test.page.id + '/notes') .expect(404, function(err, res) { if (err) return done(err); expect(res.body.error).to.be.an.object; - var expected = 'could not find a model with id unknown'; + var expected = 'could not find a model with id 999'; expect(res.body.error.message).to.equal(expected); expect(res.body.error.code).to.be.equal('MODEL_NOT_FOUND'); diff --git a/test/remoting.integration.js b/test/remoting.integration.js index 41ec366e..eb91810c 100644 --- a/test/remoting.integration.js +++ b/test/remoting.integration.js @@ -91,14 +91,14 @@ describe('remoting - integration', function() { 'upsert(data:object:store):store PATCH /stores', 'upsert(data:object:store):store PUT /stores', 'replaceOrCreate(data:object:store):store POST /stores/replaceOrCreate', - 'exists(id:any):boolean GET /stores/:id/exists', + 'exists(id:number):boolean GET /stores/:id/exists', 'findById(id:any,filter:object):store GET /stores/:id', 'prototype.updateAttributes(data:object:store):store PUT /stores/:id', - 'replaceById(id:any,data:object:store):store POST /stores/:id/replace', + 'replaceById(id:number,data:object:store):store POST /stores/:id/replace', 'find(filter:object):store GET /stores', 'findOne(filter:object):store GET /stores/findOne', 'updateAll(where:object,data:object:store):object POST /stores/update', - 'deleteById(id:any):object DELETE /stores/:id', + 'deleteById(id:number):object DELETE /stores/:id', 'count(where:object):number GET /stores/count', 'prototype.updateAttributes(data:object:store):store PATCH /stores/:id', 'createChangeStream(options:object):ReadableStream POST /stores/change-stream', @@ -143,11 +143,11 @@ describe('remoting - integration', function() { var methods = getFormattedPrototypeMethods(physicianClass.methods); var expectedMethods = [ - 'prototype.__findById__widgets(fk:any):widget ' + + 'prototype.__findById__widgets(fk:number):widget ' + 'GET /stores/:id/widgets/:fk', - 'prototype.__destroyById__widgets(fk:any) ' + + 'prototype.__destroyById__widgets(fk:number) ' + 'DELETE /stores/:id/widgets/:fk', - 'prototype.__updateById__widgets(fk:any,data:object:widget):widget ' + + 'prototype.__updateById__widgets(fk:number,data:object:widget):widget ' + 'PUT /stores/:id/widgets/:fk', 'prototype.__get__widgets(filter:object):widget ' + 'GET /stores/:id/widgets', @@ -167,17 +167,17 @@ describe('remoting - integration', function() { var methods = getFormattedPrototypeMethods(physicianClass.methods); var expectedMethods = [ - 'prototype.__findById__patients(fk:any):patient ' + + 'prototype.__findById__patients(fk:number):patient ' + 'GET /physicians/:id/patients/:fk', - 'prototype.__destroyById__patients(fk:any) ' + + 'prototype.__destroyById__patients(fk:number) ' + 'DELETE /physicians/:id/patients/:fk', - 'prototype.__updateById__patients(fk:any,data:object:patient):patient ' + + 'prototype.__updateById__patients(fk:number,data:object:patient):patient ' + 'PUT /physicians/:id/patients/:fk', - 'prototype.__link__patients(fk:any,data:object:appointment):appointment ' + + 'prototype.__link__patients(fk:number,data:object:appointment):appointment ' + 'PUT /physicians/:id/patients/rel/:fk', - 'prototype.__unlink__patients(fk:any) ' + + 'prototype.__unlink__patients(fk:number) ' + 'DELETE /physicians/:id/patients/rel/:fk', - 'prototype.__exists__patients(fk:any):boolean ' + + 'prototype.__exists__patients(fk:number):boolean ' + 'HEAD /physicians/:id/patients/rel/:fk', 'prototype.__get__patients(filter:object):patient ' + 'GET /physicians/:id/patients', @@ -220,14 +220,14 @@ describe('With model.settings.replaceOnPUT false', function() { 'upsert(data:object:storeWithReplaceOnPUTfalse):storeWithReplaceOnPUTfalse PATCH /stores-updating', 'replaceOrCreate(data:object:storeWithReplaceOnPUTfalse):storeWithReplaceOnPUTfalse POST /stores-updating/replaceOrCreate', 'upsertWithWhere(where:object,data:object:storeWithReplaceOnPUTfalse):storeWithReplaceOnPUTfalse POST /stores-updating/upsertWithWhere', - 'exists(id:any):boolean GET /stores-updating/:id/exists', - 'exists(id:any):boolean HEAD /stores-updating/:id', - 'findById(id:any,filter:object):storeWithReplaceOnPUTfalse GET /stores-updating/:id', - 'replaceById(id:any,data:object:storeWithReplaceOnPUTfalse):storeWithReplaceOnPUTfalse POST /stores-updating/:id/replace', + 'exists(id:number):boolean GET /stores-updating/:id/exists', + 'exists(id:number):boolean HEAD /stores-updating/:id', + 'findById(id:number,filter:object):storeWithReplaceOnPUTfalse GET /stores-updating/:id', + 'replaceById(id:number,data:object:storeWithReplaceOnPUTfalse):storeWithReplaceOnPUTfalse POST /stores-updating/:id/replace', 'find(filter:object):storeWithReplaceOnPUTfalse GET /stores-updating', 'findOne(filter:object):storeWithReplaceOnPUTfalse GET /stores-updating/findOne', 'updateAll(where:object,data:object:storeWithReplaceOnPUTfalse):object POST /stores-updating/update', - 'deleteById(id:any):object DELETE /stores-updating/:id', + 'deleteById(id:number):object DELETE /stores-updating/:id', 'count(where:object):number GET /stores-updating/count', 'prototype.updateAttributes(data:object:storeWithReplaceOnPUTfalse):storeWithReplaceOnPUTfalse PUT /stores-updating/:id', 'prototype.updateAttributes(data:object:storeWithReplaceOnPUTfalse):storeWithReplaceOnPUTfalse PATCH /stores-updating/:id', @@ -254,8 +254,8 @@ describe('With model.settings.replaceOnPUT true', function() { 'upsert(data:object:storeWithReplaceOnPUTtrue):storeWithReplaceOnPUTtrue PATCH /stores-replacing', 'replaceOrCreate(data:object:storeWithReplaceOnPUTtrue):storeWithReplaceOnPUTtrue POST /stores-replacing/replaceOrCreate', 'replaceOrCreate(data:object:storeWithReplaceOnPUTtrue):storeWithReplaceOnPUTtrue PUT /stores-replacing', - 'replaceById(id:any,data:object:storeWithReplaceOnPUTtrue):storeWithReplaceOnPUTtrue POST /stores-replacing/:id/replace', - 'replaceById(id:any,data:object:storeWithReplaceOnPUTtrue):storeWithReplaceOnPUTtrue PUT /stores-replacing/:id', + 'replaceById(id:number,data:object:storeWithReplaceOnPUTtrue):storeWithReplaceOnPUTtrue POST /stores-replacing/:id/replace', + 'replaceById(id:number,data:object:storeWithReplaceOnPUTtrue):storeWithReplaceOnPUTtrue PUT /stores-replacing/:id', 'prototype.updateAttributes(data:object:storeWithReplaceOnPUTtrue):storeWithReplaceOnPUTtrue PATCH /stores-replacing/:id', ]; diff --git a/test/util/model-tests.js b/test/util/model-tests.js index cac6ffca..0f6cfa17 100644 --- a/test/util/model-tests.js +++ b/test/util/model-tests.js @@ -55,6 +55,7 @@ module.exports = function defineModelTestsWithDataSource(options) { enableRemoteReplication: options.enableRemoteReplication, }); + User.setup(); User.attachTo(dataSource); User.handleChangeError = function(err) { console.warn('WARNING: unhandled change-tracking error'); @@ -183,11 +184,11 @@ module.exports = function defineModelTestsWithDataSource(options) { describe('Model.upsert(data, callback)', function() { it('Update when record with id=data.id found, insert otherwise', function(done) { - User.upsert({first: 'joe', id: 7}, function(err, user) { + User.upsert({first: 'joe', id: '7'}, function(err, user) { assert(!err); assert.equal(user.first, 'joe'); - User.upsert({first: 'bob', id: 7}, function(err, updatedUser) { + User.upsert({first: 'bob', id: '7'}, function(err, updatedUser) { assert(!err); assert.equal(updatedUser.first, 'bob'); @@ -236,9 +237,9 @@ module.exports = function defineModelTestsWithDataSource(options) { describe('Model.findById(id, callback)', function() { it('Find an instance by id', function(done) { - User.create({first: 'michael', last: 'jordan', id: 23}, function() { - User.findById(23, function(err, user) { - assert.equal(user.id, 23); + User.create({first: 'michael', last: 'jordan', id: '23'}, function() { + User.findById('23', function(err, user) { + assert.equal(user.id, '23'); assert.equal(user.first, 'michael'); assert.equal(user.last, 'jordan');