From ba8b4a0f2a7a48e415891f2d36a4c9f9761f705d Mon Sep 17 00:00:00 2001 From: Ritchie Martori Date: Mon, 30 Jun 2014 14:39:29 -0700 Subject: [PATCH 1/6] Add unique id base methods --- models.json | 41 ++++++++++++++++++++--------- models/component-definition.js | 16 +++++++----- models/definition.js | 5 ++-- models/workspace-entity.js | 48 +++++++++++++++++++++++----------- test/workspace-entity.js | 16 +++++------- 5 files changed, 81 insertions(+), 45 deletions(-) diff --git a/models.json b/models.json index 965ba551..76489cdc 100644 --- a/models.json +++ b/models.json @@ -16,7 +16,8 @@ }, "PackageDefinition": { "properties": { - "componentName": {"type": "string", "required": true} + "componentName": {"type": "string", "required": true}, + "name": {"id": true, "type": "string"} }, "public": false, "dataSource": "db", @@ -30,6 +31,7 @@ }, "ComponentDefinition": { "properties": { + "name": {"id": true, "type": "string"}, "defaultPermission": {"type": "string"} }, "public": true, @@ -62,7 +64,8 @@ }, "ComponentModel": { "properties": { - "name": {"type": "string", "id": true} + "id": {"type": "string"}, + "name": {"type": "string"} }, "dataSource": "db", "options": { @@ -90,6 +93,7 @@ }, "ModelDefinition": { "properties": { + "id": "string", "plural": "string", "strict": "boolean", "scopes": "array", @@ -149,6 +153,7 @@ }, "ModelMethod": { "properties": { + "id": "string", "aliases": {"type": "array"}, "isStatic": {"type": "boolean"}, "accepts": {"type": "array"}, @@ -160,11 +165,13 @@ "public": true, "dataSource": "db", "options": { - "base": "WorkspaceEntity" + "base": "WorkspaceEntity", + "includeIdPart": "modelName" } }, "ModelRelation": { "properties": { + "id": {"type": "string"}, "type": {"type": "string"}, "as": {"type": "string"}, "foreignKey": {"type": "string"}, @@ -173,11 +180,13 @@ "public": true, "dataSource": "db", "options": { - "base": "WorkspaceEntity" + "base": "WorkspaceEntity", + "includeIdPart": "modelName" } }, "ModelAccessControl": { "properties": { + "id": "string", "property": {"type": "string"}, "route": {"type": "string"}, "principalId": {"type": "string"}, @@ -187,11 +196,13 @@ "public": true, "dataSource": "db", "options": { - "base": "WorkspaceEntity" + "base": "WorkspaceEntity", + "includeIdPart": "modelName" } }, "ModelProperty": { "properties": { + "id": "string", "type": {"type": "string"}, "name": {"type": "string", "id": true}, "generated": {"type": "boolean"}, @@ -210,15 +221,18 @@ "model": "PropertyValid", "foreignKey": "property" } - } + }, + "includeIdPart": "modelName" } }, "DatabaseColumn": { - "connector": {"type": "string"}, - "columnName": {"type": "string"}, - "dataType": {"type": "string"}, - "dataLength": {"type": "string"}, - "nullable": {"type": "boolean"}, + "properties": { + "connector": {"type": "string"}, + "columnName": {"type": "string"}, + "dataType": {"type": "string"}, + "dataLength": {"type": "string"}, + "nullable": {"type": "boolean"} + }, "options": { "base": "WorkspaceEntity" }, @@ -227,6 +241,7 @@ }, "PropertyValidation": { "properties": { + "id": {"type": "string"}, "type": {"type": "string"}, "message": {"type": "string"}, "min": {"type": "number"}, @@ -238,7 +253,8 @@ "public": true, "dataSource": "db", "options": { - "base": "WorkspaceEntity" + "base": "WorkspaceEntity", + "includeIdPart": "modelName" } }, "ViewDefinition": { @@ -265,6 +281,7 @@ }, "DataSourceDefinition": { "properties": { + "id": {"type": "string"}, "host": {"type": "string"}, "port": {"type": "number"}, "url": {"type": "string"}, diff --git a/models/component-definition.js b/models/component-definition.js index 6b67b87f..cb3969fa 100644 --- a/models/component-definition.js +++ b/models/component-definition.js @@ -44,7 +44,7 @@ ComponentDefinition.loadIntoCache = function(cache, componentName, components, c packageFile.load(cb); }, function(cb) { packageFile.data.componentName = componentName; - PackageDefinition.addToCache(cache, componentName, packageFile.data || {}); + PackageDefinition.addToCache(cache, packageFile.data || {}); cb(); }); } @@ -57,7 +57,7 @@ ComponentDefinition.loadIntoCache = function(cache, componentName, components, c component.data.configFile = component.path; component.data.name = componentName; debug('adding to cache component file [%s]', component.path); - ComponentDefinition.addToCache(cache, componentName, component.data); + ComponentDefinition.addToCache(cache, component.data); cb(); }); } else { @@ -86,7 +86,7 @@ ComponentDefinition.loadIntoCache = function(cache, componentName, components, c var componentModel = modelDefs[modelName]; componentModel.componentName = componentName; componentModel.name = modelName; - ComponentModel.addToCache(cache, modelName, componentModel); + ComponentModel.addToCache(cache, componentModel); }); cb(); @@ -106,8 +106,8 @@ ComponentDefinition.loadIntoCache = function(cache, componentName, components, c debug('loading [%s] model definition into cache', def.name); - ModelDefinition.addToCache(cache, def.name, def); - ModelDefinition.addRelatedToCache(cache, def.name, def); + ModelDefinition.addToCache(cache, def); + ModelDefinition.addRelatedToCache(cache, def); }); cb(); }); @@ -126,7 +126,7 @@ ComponentDefinition.loadIntoCache = function(cache, componentName, components, c def.name = dataSourceName; def.componentName = componentName; debug('loading [%s] dataSource into cache', dataSourceName); - DataSourceDefinition.addToCache(cache, dataSourceName, def); + DataSourceDefinition.addToCache(cache, def); }); cb(); @@ -244,3 +244,7 @@ ComponentDefinition.hasApp = function(componentDef) { // e.g. package.json > loopback-workspace > app: true|false return componentDef.name !== '.'; }; + +ComponentDefinition.prototype.getUniqueId = function() { + return this.name || null; +} diff --git a/models/definition.js b/models/definition.js index 1613cb65..7b10cf7f 100644 --- a/models/definition.js +++ b/models/definition.js @@ -15,7 +15,6 @@ var Definition = app.model('Definition', { "properties": { "name": { "type": "string", - "id": true, "required": true }, "dir": { @@ -117,7 +116,7 @@ Definition.addRelatedToCache = function(cache, name, fileData) { var id = config[relation.foreignKey] || config.id; config[relation.foreignKey] = name; debug('addRelatedToCache %s %s %j', relation.model, id, config); - Entity.addToCache(cache, id, config); + Entity.addToCache(cache, config); }); } else if(relatedData) { Object.keys(relatedData).forEach(function(id) { @@ -125,7 +124,7 @@ Definition.addRelatedToCache = function(cache, name, fileData) { config[Entity.dataSource.idName(Entity.modelName)] = id; config[relation.foreignKey] = name; debug('addRelatedToCache %s %s %j', relation.model, id, config); - Entity.addToCache(cache, id, config); + Entity.addToCache(cache, config); }); } }); diff --git a/models/workspace-entity.js b/models/workspace-entity.js index d95cf65d..3735b01c 100644 --- a/models/workspace-entity.js +++ b/models/workspace-entity.js @@ -3,27 +3,42 @@ var app = require('../app'); var WorkspaceEntity = app.model('WorkspaceEntity', { "properties": { "configFile": {"type": "string"}, - "scriptFile": {"type": "string"}, - "configLineNum": {"type": "number"}, - "locked": {"type": "boolean"} + "componentName": {"type": "string"} /* , required: true */ }, "public": false, "dataSource": "db" }); -/** - * Get the file location of the entity. - * - * @returns {String} location For example - * `"/foo/bar/bat/baz.json:237"` - */ - -WorkspaceEntity.prototype.getLocation = function() { - throwMustImplement('getLocation', this.constructor); +WorkspaceEntity.prototype.getUniqueId = function() { + var sep = this.constructor.settings.idSeparator || '.'; + var parts = this.getUniqueIdParts(); + if(parts.length >= 1) { + return parts.join(sep); + } + return null; } -function throwMustImplement(name, constructor) { - throw new Error('must be implemented by ', constructor.name); +WorkspaceEntity.prototype.getUniqueIdParts = function() { + var settings = this.constructor.settings; + var includeIdPart = settings.includeIdPart; + var parts = []; + var requiredParts = 2; + + if(includeIdPart) requiredParts++; + if(this.componentName) { + if(this.componentName === '.') { + requiredParts--; + } else { + parts.push(this.componentName); + } + } + if(includeIdPart && this[includeIdPart]) { + parts.push(this[includeIdPart]); + } + if(this.name) parts.push(this.name); + + if(parts.length === requiredParts) return parts; + return []; } /** @@ -41,7 +56,10 @@ WorkspaceEntity.clearCache = function(cache) { cache[this.modelName] = {}; } -WorkspaceEntity.addToCache = function(cache, id, val) { +WorkspaceEntity.addToCache = function(cache, val) { + var Entity = this; + var entity = new Entity(val); + var id = entity.getUniqueId(); cache[this.modelName][id] = JSON.stringify(val); } diff --git a/test/workspace-entity.js b/test/workspace-entity.js index 2027b5e3..aed77679 100644 --- a/test/workspace-entity.js +++ b/test/workspace-entity.js @@ -3,15 +3,13 @@ var WorkspaceEntity = app.models.WorkspaceEntity; var TestDataBuilder = require('loopback-testing').TestDataBuilder; describe('WorkspaceEntity', function() { - describe('workspaceEntity.getLocation()', function() { - it('Get the file location of the entity.', function() { - - }); - }); - - describe('WorkspaceEntity.getWorkspaceDir()', function() { - it('Get the Workspace directory.', function() { - + describe('workspaceEntity.getUniqueId()', function() { + it('get the unique identifier of the entity', function() { + var bar = new WorkspaceEntity({ + componentName: 'foo', + name: 'bar' + }); + expect(bar.getUniqueId()).to.equal('foo.bar'); }); }); }); From e2c69147ebd6b7b8f2f6d1603e1a92b0c0c91981 Mon Sep 17 00:00:00 2001 From: Ritchie Martori Date: Tue, 1 Jul 2014 14:42:34 -0700 Subject: [PATCH 2/6] Fix embed identifiers - allow `WorkspaceEntity` to determine the unique id - `connector.create` should callback with an id - all models should not have the correct `id` property - pass the correct identifier data to `addRelatedCache` so it can set the `componentName` and the correct foreign key - clean related data for both embedded arrays and objects - remove `componentName` and `id` for all embedded config data - ensure the value being set to the cache has the correct id - add id tests (could use more for other model types) --- connector.js | 32 ++++++++++++++++++++++++++++++++ models.json | 19 ++++++++++--------- models/component-definition.js | 4 +++- models/definition.js | 19 ++++++++++--------- models/model-definition.js | 10 +++++++--- models/workspace-entity.js | 1 + test/component-definition.js | 17 +++++++++++++++-- test/model-property.js | 17 +++++++++++++---- test/workspace-entity.js | 3 ++- 9 files changed, 93 insertions(+), 29 deletions(-) diff --git a/connector.js b/connector.js index a5fd824f..8555f1d1 100644 --- a/connector.js +++ b/connector.js @@ -1,4 +1,5 @@ var app = require('./app'); +var loopback = require('loopback'); var connector = app.dataSources.db.connector; var ComponentDefinition = app.models.ComponentDefinition; var ConfigFile = app.models.ConfigFile; @@ -89,3 +90,34 @@ connector.all = function(model) { originalAll.apply(connector, args); }); } + +connector.getIdValue = function(model, data) { + var Entity = loopback.getModel(model); + var entity = new Entity(data); + return entity.getUniqueId(); +} + +connector.create = function create(model, data, callback) { + var Entity = loopback.getModel(model); + var entity = new Entity(data); + var id = entity.getUniqueId(); + + this.setIdValue(model, data, id); + + if(!this.cache[model]) { + this.cache[model] = {}; + } + + this.cache[model][id] = serialize(data); + this.saveToFile(id, function(err) { + if(err) return callback(err); + callback(null, id); + }); +}; + +function serialize(obj) { + if(obj === null || obj === undefined) { + return obj; + } + return JSON.stringify(obj); +} diff --git a/models.json b/models.json index 76489cdc..e9d58403 100644 --- a/models.json +++ b/models.json @@ -64,7 +64,7 @@ }, "ComponentModel": { "properties": { - "id": {"type": "string"}, + "id": {"type": "string", "id": true}, "name": {"type": "string"} }, "dataSource": "db", @@ -93,7 +93,8 @@ }, "ModelDefinition": { "properties": { - "id": "string", + "id": {"type": "string", "id": true}, + "name": "string", "plural": "string", "strict": "boolean", "scopes": "array", @@ -153,7 +154,7 @@ }, "ModelMethod": { "properties": { - "id": "string", + "id": {"type": "string", "id": true}, "aliases": {"type": "array"}, "isStatic": {"type": "boolean"}, "accepts": {"type": "array"}, @@ -171,7 +172,7 @@ }, "ModelRelation": { "properties": { - "id": {"type": "string"}, + "id": {"type": "string", "id": true}, "type": {"type": "string"}, "as": {"type": "string"}, "foreignKey": {"type": "string"}, @@ -186,7 +187,7 @@ }, "ModelAccessControl": { "properties": { - "id": "string", + "id": {"type": "string", "id": true}, "property": {"type": "string"}, "route": {"type": "string"}, "principalId": {"type": "string"}, @@ -202,9 +203,9 @@ }, "ModelProperty": { "properties": { - "id": "string", + "id": {"type": "string", "id": true}, "type": {"type": "string"}, - "name": {"type": "string", "id": true}, + "name": {"type": "string"}, "generated": {"type": "boolean"}, "required": {"type": "boolean"}, "index": {"type": "boolean"}, @@ -241,7 +242,7 @@ }, "PropertyValidation": { "properties": { - "id": {"type": "string"}, + "id": {"type": "string", "id": true}, "type": {"type": "string"}, "message": {"type": "string"}, "min": {"type": "number"}, @@ -281,7 +282,7 @@ }, "DataSourceDefinition": { "properties": { - "id": {"type": "string"}, + "id": {"type": "string", "id": true}, "host": {"type": "string"}, "port": {"type": "number"}, "url": {"type": "string"}, diff --git a/models/component-definition.js b/models/component-definition.js index cb3969fa..7932b019 100644 --- a/models/component-definition.js +++ b/models/component-definition.js @@ -103,11 +103,13 @@ ComponentDefinition.loadIntoCache = function(cache, componentName, components, c var def = configFile.data || {}; def.componentName = componentName; def.configFile = configFile.path; + var modelDef = new ModelDefinition(def); debug('loading [%s] model definition into cache', def.name); ModelDefinition.addToCache(cache, def); - ModelDefinition.addRelatedToCache(cache, def); + ModelDefinition.addRelatedToCache(cache, def, componentName + , modelDef.getUniqueId()); }); cb(); }); diff --git a/models/definition.js b/models/definition.js index 7b10cf7f..0348d8f6 100644 --- a/models/definition.js +++ b/models/definition.js @@ -105,25 +105,26 @@ Definition.getEmbededRelations = function() { return results; } -Definition.addRelatedToCache = function(cache, name, fileData) { +Definition.addRelatedToCache = function(cache, fileData, componentName, fk) { var Definition = this; this.getEmbededRelations().forEach(function(relation) { var relatedData = fileData[relation.as]; var Entity = loopback.getModel(relation.model); + var entity; if(Array.isArray(relatedData)) { relatedData.forEach(function(config) { - var id = config[relation.foreignKey] || config.id; - config[relation.foreignKey] = name; - debug('addRelatedToCache %s %s %j', relation.model, id, config); + var entity = new Entity(config); + config[relation.foreignKey] = fk; + config.componentName = componentName; Entity.addToCache(cache, config); }); } else if(relatedData) { - Object.keys(relatedData).forEach(function(id) { - var config = relatedData[id]; - config[Entity.dataSource.idName(Entity.modelName)] = id; - config[relation.foreignKey] = name; - debug('addRelatedToCache %s %s %j', relation.model, id, config); + Object.keys(relatedData).forEach(function(embedId) { + var config = relatedData[embedId]; + config[relation.foreignKey] = fk; + config[relation.embed.key] = embedId; + config.componentName = componentName; Entity.addToCache(cache, config); }); } diff --git a/models/model-definition.js b/models/model-definition.js index 09d384b9..a070c18e 100644 --- a/models/model-definition.js +++ b/models/model-definition.js @@ -51,7 +51,6 @@ function getRelated(cache, id, relation) { function formatRelatedData(relation, relatedData) { var result; assert(relation.embed && relation.embed.as, 'embed requires "as"'); - cleanRelatedData(relatedData, relation); switch(relation.embed.as) { case 'object': assert(relation.embed.key, 'embed as object requires "key"'); @@ -59,11 +58,12 @@ function formatRelatedData(relation, relatedData) { relatedData.forEach(function(related) { var key = related[relation.embed.key]; result[key] = related; - delete related[relation.embed.key]; }); + cleanRelatedData(relatedData, relation); return result; break; case 'array': + cleanRelatedData(relatedData, relation); return relatedData; break; } @@ -95,6 +95,10 @@ ModelDefinition.toFilename = function(name) { function cleanRelatedData(relatedData, relation) { relatedData.forEach(function(obj) { assert(relation.foreignKey, 'embeded relation must have foreignKey'); - delete obj[relation.foreignKey]; + delete obj[relation.foreignKey]; + delete obj[relation.embed.key]; + // TODO(ritch) we can probably generalize these + delete obj.id; + delete obj.componentName; }); } diff --git a/models/workspace-entity.js b/models/workspace-entity.js index 3735b01c..f357442f 100644 --- a/models/workspace-entity.js +++ b/models/workspace-entity.js @@ -60,6 +60,7 @@ WorkspaceEntity.addToCache = function(cache, val) { var Entity = this; var entity = new Entity(val); var id = entity.getUniqueId(); + val[this.dataSource.idName(Entity.modelName)] = id; cache[this.modelName][id] = JSON.stringify(val); } diff --git a/test/component-definition.js b/test/component-definition.js index a17513e9..f1f7af74 100644 --- a/test/component-definition.js +++ b/test/component-definition.js @@ -1,8 +1,21 @@ var fs = require('fs-extra'); +var ComponentDefinition = require('../app').models.ComponentDefinition; -describe('ComponentDefinition', function() { - describe('componentDefinition.saveToFs', function() { +describe('ComponentDefinition', function () { + describe('ComponentDefinition.create(def, cb)', function () { beforeEach(givenBasicWorkspace); + + it('should use name as the id', function (done) { + ComponentDefinition.create({ + name: 'foo' + }, function(err, def) { + expect(err).to.not.exist; + expect(def).to.not.have.property('id'); + expect(def.name).to.equal('foo'); + done(); + }); + }); + it('omits `name` in config.json', function() { var content = fs.readJsonFileSync(SANDBOX + '/rest/config.json'); expect(content).to.not.have.property('name'); diff --git a/test/model-property.js b/test/model-property.js index 5a41b9b5..d98b3be5 100644 --- a/test/model-property.js +++ b/test/model-property.js @@ -18,7 +18,8 @@ describe('ModelProperty', function() { var property = { name: test.propertyName, type: 'String', - modelName: 'user' + modelName: 'user', + componentName: 'rest' }; ModelProperty.create(property, function(err, property) { if(err) return done(err); @@ -36,6 +37,9 @@ describe('ModelProperty', function() { expect(properties).to.have.property(this.propertyName); expect(properties[this.propertyName]).to.eql({type: type}); }); + it('should have the correct id', function () { + expect(this.property.id).to.equal('rest.user.myProperty'); + }); }); describe('ModelProperty.find(filter, cb)', function (done) { @@ -76,11 +80,16 @@ describe('ModelProperty', function() { // every query triggers a reload ModelProperty.all(function(err, list) { if (err) return done(err); - expect(list[0].toObject()).to.eql(new ModelProperty({ + var actual = list[0].toObject(); + var expected = new ModelProperty({ name: this.propertyName, type: 'String', - modelName: 'user' - }).toObject()); + modelName: 'rest.user', + componentName: 'rest', + id: 'rest.rest.user.myProperty' + }).toObject(); + + expect(actual).to.eql(expected); done(); }.bind(this)); }); diff --git a/test/workspace-entity.js b/test/workspace-entity.js index aed77679..df01eaf8 100644 --- a/test/workspace-entity.js +++ b/test/workspace-entity.js @@ -1,10 +1,11 @@ var app = require('../app'); var WorkspaceEntity = app.models.WorkspaceEntity; +var ComponentDefinition = app.models.ComponentDefinition; var TestDataBuilder = require('loopback-testing').TestDataBuilder; describe('WorkspaceEntity', function() { describe('workspaceEntity.getUniqueId()', function() { - it('get the unique identifier of the entity', function() { + it('gets the unique identifier of the entity', function() { var bar = new WorkspaceEntity({ componentName: 'foo', name: 'bar' From fc66deefe7dc3a7286799ac478d764972710877a Mon Sep 17 00:00:00 2001 From: Ritchie Martori Date: Wed, 2 Jul 2014 10:06:38 -0700 Subject: [PATCH 3/6] Add componentName to correct models and mark required --- models.json | 21 ++++++++++++++------- models/definition.js | 2 ++ models/workspace-entity.js | 3 +-- models/workspace.js | 1 + 4 files changed, 18 insertions(+), 9 deletions(-) diff --git a/models.json b/models.json index e9d58403..ee8d9d12 100644 --- a/models.json +++ b/models.json @@ -65,7 +65,8 @@ "ComponentModel": { "properties": { "id": {"type": "string", "id": true}, - "name": {"type": "string"} + "name": {"type": "string"}, + "componentName": {"type": "string", "required": true} }, "dataSource": "db", "options": { @@ -161,7 +162,8 @@ "returns": {"type": "array"}, "description": {"type": "string"}, "http": {"type": "object"}, - "shared": {"type": "boolean"} + "shared": {"type": "boolean"}, + "componentName": {"type": "string", "required": true} }, "public": true, "dataSource": "db", @@ -176,7 +178,8 @@ "type": {"type": "string"}, "as": {"type": "string"}, "foreignKey": {"type": "string"}, - "model": {"type": "string"} + "model": {"type": "string"}, + "componentName": {"type": "string", "required": true} }, "public": true, "dataSource": "db", @@ -192,7 +195,8 @@ "route": {"type": "string"}, "principalId": {"type": "string"}, "principalType": {"type": "string"}, - "permission": {"type": "string", "required": true} + "permission": {"type": "string", "required": true}, + "componentName": {"type": "string", "required": true} }, "public": true, "dataSource": "db", @@ -210,7 +214,8 @@ "required": {"type": "boolean"}, "index": {"type": "boolean"}, "modelName": "string", - "desc": {"type": "string"} + "desc": {"type": "string"}, + "componentName": {"type": "string", "required": true} }, "public": true, "dataSource": "db", @@ -249,7 +254,8 @@ "max": {"type": "string"}, "int": {"type": "string"}, "number": {"type": "string"}, - "with": {"type": "string"} + "with": {"type": "string"}, + "componentName": {"type": "string", "required": true} }, "public": true, "dataSource": "db", @@ -288,7 +294,8 @@ "url": {"type": "string"}, "database": {"type": "string"}, "username": {"type": "string"}, - "password": {"type": "string"} + "password": {"type": "string"}, + "componentName": {"type": "string", "required": true} }, "public": true, "dataSource": "db", diff --git a/models/definition.js b/models/definition.js index 0348d8f6..371ba298 100644 --- a/models/definition.js +++ b/models/definition.js @@ -117,6 +117,7 @@ Definition.addRelatedToCache = function(cache, fileData, componentName, fk) { var entity = new Entity(config); config[relation.foreignKey] = fk; config.componentName = componentName; + debug('addRelatedToCache %s %j', relation.model, config); Entity.addToCache(cache, config); }); } else if(relatedData) { @@ -125,6 +126,7 @@ Definition.addRelatedToCache = function(cache, fileData, componentName, fk) { config[relation.foreignKey] = fk; config[relation.embed.key] = embedId; config.componentName = componentName; + debug('addRelatedToCache %s %j', relation.model, config); Entity.addToCache(cache, config); }); } diff --git a/models/workspace-entity.js b/models/workspace-entity.js index f357442f..b344bb82 100644 --- a/models/workspace-entity.js +++ b/models/workspace-entity.js @@ -2,8 +2,7 @@ var path = require('path'); var app = require('../app'); var WorkspaceEntity = app.model('WorkspaceEntity', { "properties": { - "configFile": {"type": "string"}, - "componentName": {"type": "string"} /* , required: true */ + "configFile": {"type": "string"} }, "public": false, "dataSource": "db" diff --git a/models/workspace.js b/models/workspace.js index ae057767..d0cc3db5 100644 --- a/models/workspace.js +++ b/models/workspace.js @@ -129,6 +129,7 @@ Workspace.addComponent = function(options, cb) { } if(template.relations) { + setComponentName(template.relations); steps.push(function(cb) { async.each(template.relations, ModelRelation.create.bind(ModelRelation), cb); From efd215ed8d64f96125707279a342f912d08d7831 Mon Sep 17 00:00:00 2001 From: Ritchie Martori Date: Wed, 2 Jul 2014 12:48:55 -0700 Subject: [PATCH 4/6] Refactor id creation to use belongsTo foreign key - rename model foreign key - remove modelName property from all entities - remove componentName from entities where it was made redundant by the parent id (eg. modelProperty.modelId has the componentName) - allow for simpler id building by adding belongsTo relationships - use belongs to relations to build the id - parentIds (belongsTo foreignKeys) are now required - cleanup data (eg. dataSourceDef.id) leaking into config files --- models.json | 73 +++++++++++++++++++++++----------- models/component-definition.js | 9 +++-- models/model-definition.js | 3 +- models/package-definition.js | 5 +++ models/workspace-entity.js | 40 +++++++++++++------ templates/rest/component.js | 14 +++++++ test/data-source-definition.js | 18 ++++++--- test/model-definition.js | 2 +- test/model-property.js | 7 ++-- test/workspace-entity.js | 8 +++- test/workspace.js | 10 ++--- 11 files changed, 130 insertions(+), 59 deletions(-) create mode 100644 models/package-definition.js diff --git a/models.json b/models.json index ee8d9d12..1e53e19d 100644 --- a/models.json +++ b/models.json @@ -115,40 +115,45 @@ ], "configExtensions": ["json"], "relations": { + "component": { + "type": "belongsTo", + "model": "ComponentDefinition", + "foreignKey": "componentName" + }, "properties": { "embed": {"as": "object", "key": "name"}, "type": "hasMany", "model": "ModelProperty", - "foreignKey": "modelName" + "foreignKey": "modelId" }, "validations": { "embed": {"as": "array"}, "type": "hasMany", "model": "PropertyValidation", - "foreignKey": "modelName" + "foreignKey": "modelId" }, "relations": { "embed": {"as": "object", "key": "as"}, "type": "hasMany", "model": "ModelRelation", - "foreignKey": "fromModel" + "foreignKey": "fromModelId" }, "accessControls": { "embed": {"name": "acls", "as": "array"}, "type": "hasMany", "model": "ModelAccessControl", - "foreignKey": "model" + "foreignKey": "modelId" }, "methods": { "embed": {"as": "array"}, "type": "hasMany", "model": "ModelMethod", - "foreignKey": "model" + "foreignKey": "modelId" }, "views": { "type": "hasMany", "model": "ViewDefinition", - "foreignKey": "model" + "foreignKey": "modelId" } } } @@ -156,66 +161,76 @@ "ModelMethod": { "properties": { "id": {"type": "string", "id": true}, + "modelId": {"type": "string", "required": true}, "aliases": {"type": "array"}, "isStatic": {"type": "boolean"}, "accepts": {"type": "array"}, "returns": {"type": "array"}, "description": {"type": "string"}, "http": {"type": "object"}, - "shared": {"type": "boolean"}, - "componentName": {"type": "string", "required": true} + "shared": {"type": "boolean"} }, "public": true, "dataSource": "db", "options": { - "base": "WorkspaceEntity", - "includeIdPart": "modelName" + "base": "WorkspaceEntity" } }, "ModelRelation": { "properties": { "id": {"type": "string", "id": true}, + "modelId": {"type": "string", "required": true}, "type": {"type": "string"}, "as": {"type": "string"}, "foreignKey": {"type": "string"}, - "model": {"type": "string"}, - "componentName": {"type": "string", "required": true} + "model": {"type": "string"} }, "public": true, "dataSource": "db", "options": { "base": "WorkspaceEntity", - "includeIdPart": "modelName" + "relations": { + "model": { + "type": "belongsTo", + "model": "ModelDefinition", + "foreignKey": "modelId" + } + } } }, "ModelAccessControl": { "properties": { "id": {"type": "string", "id": true}, + "modelId": {"type": "string", "required": true}, "property": {"type": "string"}, "route": {"type": "string"}, "principalId": {"type": "string"}, "principalType": {"type": "string"}, - "permission": {"type": "string", "required": true}, - "componentName": {"type": "string", "required": true} + "permission": {"type": "string", "required": true} }, "public": true, "dataSource": "db", "options": { "base": "WorkspaceEntity", - "includeIdPart": "modelName" + "relations": { + "model": { + "type": "belongsTo", + "model": "ModelDefinition", + "foreignKey": "modelId" + } + } } }, "ModelProperty": { "properties": { "id": {"type": "string", "id": true}, + "modelId": {"type": "string", "required": true}, "type": {"type": "string"}, "name": {"type": "string"}, "generated": {"type": "boolean"}, "required": {"type": "boolean"}, "index": {"type": "boolean"}, - "modelName": "string", - "desc": {"type": "string"}, - "componentName": {"type": "string", "required": true} + "desc": {"type": "string"} }, "public": true, "dataSource": "db", @@ -226,9 +241,13 @@ "type": "hasMany", "model": "PropertyValid", "foreignKey": "property" + }, + "model": { + "type": "belongsTo", + "model": "ModelDefinition", + "foreignKey": "modelId" } - }, - "includeIdPart": "modelName" + } } }, "DatabaseColumn": { @@ -248,20 +267,26 @@ "PropertyValidation": { "properties": { "id": {"type": "string", "id": true}, + "propertyId": {"type": "string", "required": true}, "type": {"type": "string"}, "message": {"type": "string"}, "min": {"type": "number"}, "max": {"type": "string"}, "int": {"type": "string"}, "number": {"type": "string"}, - "with": {"type": "string"}, - "componentName": {"type": "string", "required": true} + "with": {"type": "string"} }, "public": true, "dataSource": "db", "options": { "base": "WorkspaceEntity", - "includeIdPart": "modelName" + "relations": { + "model": { + "type": "belongsTo", + "model": "ModelProperty", + "foreignKey": "propertyId" + } + } } }, "ViewDefinition": { diff --git a/models/component-definition.js b/models/component-definition.js index 7932b019..ae219a2e 100644 --- a/models/component-definition.js +++ b/models/component-definition.js @@ -186,11 +186,11 @@ ComponentDefinition.saveToFs = function(cache, componentDef, cb) { var cachedDataSources = DataSourceDefinition.allFromCache(cache); cachedDataSources.forEach(function(dataSourceDef) { - if (dataSourceDef.componentName === componentName) { + if(dataSourceDef.componentName === componentName) { dataSourcePath = DataSourceDefinition.getPath(componentName, dataSourceDef); dataSoureConfig[dataSourceDef.name] = dataSourceDef; - // remove extra data that shouldn't be persisted to the fs delete dataSourceDef.name; + delete dataSourceDef.id; delete dataSourceDef.componentName; } }); @@ -206,10 +206,9 @@ ComponentDefinition.saveToFs = function(cache, componentDef, cb) { var componentModelsConfig = componentModelFile.data = {}; cachedComponentModels.forEach(function(componentModel) { - if (componentModel.componentName !== componentName) return; componentModelsConfig[componentModel.name] = componentModel; - // remove extra data that shouldn't be persisted to the fs delete componentModel.name; + delete componentModel.id; delete componentModel.componentName; }); @@ -224,6 +223,8 @@ ComponentDefinition.saveToFs = function(cache, componentDef, cb) { delete modelDef.dataSource; var modelConfigFile = ModelDefinition.getConfigFile(componentName, modelDef); modelConfigFile.data = ModelDefinition.getConfigData(cache, modelDef); + delete modelDef.componentName; + delete modelDef.id; filesToSave.push(modelConfigFile); } }); diff --git a/models/model-definition.js b/models/model-definition.js index a070c18e..d0ebd93d 100644 --- a/models/model-definition.js +++ b/models/model-definition.js @@ -31,7 +31,7 @@ ModelDefinition.getConfigData = function(cache, modelDef) { configData.name = modelDef.name; relations.forEach(function(relation) { - var relatedData = getRelated(cache, modelDef.name, relation); + var relatedData = getRelated(cache, modelDef.id, relation); configData[relation.as] = formatRelatedData(relation, relatedData); }); @@ -100,5 +100,6 @@ function cleanRelatedData(relatedData, relation) { // TODO(ritch) we can probably generalize these delete obj.id; delete obj.componentName; + delete obj.modelName; }); } diff --git a/models/package-definition.js b/models/package-definition.js new file mode 100644 index 00000000..c0c4200e --- /dev/null +++ b/models/package-definition.js @@ -0,0 +1,5 @@ +var PackageDefinition = require('../app').models.PackageDefinition; + +PackageDefinition.prototype.getUniqueId = function() { + return this.name || null; +} diff --git a/models/workspace-entity.js b/models/workspace-entity.js index b344bb82..fb0c3726 100644 --- a/models/workspace-entity.js +++ b/models/workspace-entity.js @@ -19,25 +19,41 @@ WorkspaceEntity.prototype.getUniqueId = function() { WorkspaceEntity.prototype.getUniqueIdParts = function() { var settings = this.constructor.settings; - var includeIdPart = settings.includeIdPart; + var parentPropertyName = this.constructor.getParentPropertyName(); var parts = []; - var requiredParts = 2; + var parentId = parentPropertyName && this[parentPropertyName]; + var splitParentId = parentId && parentId.split('.'); + var parentIdIsNotRootComponent = parentId !== '.'; - if(includeIdPart) requiredParts++; - if(this.componentName) { - if(this.componentName === '.') { - requiredParts--; + if(parentPropertyName) { + if(parentId) { + if(parentIdIsNotRootComponent) { + parts.push.apply(parts, splitParentId); + } } else { - parts.push(this.componentName); + // cannot construct the id without the parent id + return []; } } - if(includeIdPart && this[includeIdPart]) { - parts.push(this[includeIdPart]); - } + if(this.name) parts.push(this.name); - if(parts.length === requiredParts) return parts; - return []; + return parts; +} + +WorkspaceEntity.getParentPropertyName = function() { + var relations = this.relations; + if(!relations) return; + + var relationNames = Object.keys(relations); + var relation; + + for(var i = 0; i < relationNames.length; i++) { + relation = relations[relationNames[i]]; + if(relation.type === 'belongsTo') { + return relation.keyFrom; + } + } } /** diff --git a/templates/rest/component.js b/templates/rest/component.js index 441a2c4d..977a6702 100644 --- a/templates/rest/component.js +++ b/templates/rest/component.js @@ -38,6 +38,20 @@ template.componentModels = [ ]; template.relations = [ + { + fromModel: 'user', + model: 'access-token', + type: 'hasMany', + foreignKey: 'userId', + modelId: 'rest.user' + }, + { + fromModel: 'role', + type: 'hasMany', + model: 'roleMapping', + foreignKey: 'roleId', + modelId: 'rest.role' + } ]; template.datasources = [ diff --git a/test/data-source-definition.js b/test/data-source-definition.js index 3317421b..8098bdbc 100644 --- a/test/data-source-definition.js +++ b/test/data-source-definition.js @@ -40,11 +40,19 @@ describe('DataSourceDefinition', function() { done(); }); }); - it('should create a config file', function(done) { - this.configFile.exists(function(err, exists) { - expect(err).to.not.exist; - expect(exists).to.equal(true); - done(); + describe('config file', function () { + it('should be created', function(done) { + this.configFile.exists(function(err, exists) { + expect(err).to.not.exist; + expect(exists).to.equal(true); + done(); + }); + }); + it('should not contain id properties', function () { + var configData = this.configFile.data; + var dsConfig = configData.foo; + expect(dsConfig).to.not.have.property('id'); + expect(dsConfig).to.not.have.property('componentName'); }); }); it('shoulb be persist multiple to the config file', function (done) { diff --git a/test/model-definition.js b/test/model-definition.js index c78f0efb..0098ac67 100644 --- a/test/model-definition.js +++ b/test/model-definition.js @@ -105,7 +105,7 @@ describe('ModelDefinition', function() { }) .define('aclx', ModelAccessControl, { method: 'ALL', - model: ref('model.name') + modelId: ref('model.id') }) .buildTo(this, function(err) { if (err) return done(err); diff --git a/test/model-property.js b/test/model-property.js index d98b3be5..093e850b 100644 --- a/test/model-property.js +++ b/test/model-property.js @@ -18,8 +18,7 @@ describe('ModelProperty', function() { var property = { name: test.propertyName, type: 'String', - modelName: 'user', - componentName: 'rest' + modelId: 'rest.user' }; ModelProperty.create(property, function(err, property) { if(err) return done(err); @@ -84,9 +83,9 @@ describe('ModelProperty', function() { var expected = new ModelProperty({ name: this.propertyName, type: 'String', - modelName: 'rest.user', componentName: 'rest', - id: 'rest.rest.user.myProperty' + id: 'rest.user.myProperty', + modelId: 'rest.user' }).toObject(); expect(actual).to.eql(expected); diff --git a/test/workspace-entity.js b/test/workspace-entity.js index df01eaf8..1230c7b8 100644 --- a/test/workspace-entity.js +++ b/test/workspace-entity.js @@ -6,7 +6,13 @@ var TestDataBuilder = require('loopback-testing').TestDataBuilder; describe('WorkspaceEntity', function() { describe('workspaceEntity.getUniqueId()', function() { it('gets the unique identifier of the entity', function() { - var bar = new WorkspaceEntity({ + var MyWorkspaceEntity = WorkspaceEntity.extend('MyWorkspaceEntity'); + MyWorkspaceEntity.attachTo(app.dataSources.db); + MyWorkspaceEntity.belongsTo(ComponentDefinition, { + as: 'component', + foreignKey: 'componentName' + }); + var bar = new MyWorkspaceEntity({ componentName: 'foo', name: 'bar' }); diff --git a/test/workspace.js b/test/workspace.js index 92692359..303fa2f4 100644 --- a/test/workspace.js +++ b/test/workspace.js @@ -65,18 +65,18 @@ describe('Workspace', function() { }); }); - it('it should create a set of component definitions', function() { + it('should create a set of component definitions', function() { var componentNames = toNames(this.components); expect(componentNames).to.contain('rest'); expect(componentNames).to.contain('.'); expect(componentNames).to.contain('server'); }); - it('it should not create a set of model definitions', function() { + it('should not create a set of model definitions', function() { expect(this.models).to.be.empty; }); - it('it should create a set of data source definitions', function() { + it('should create a set of data source definitions', function() { var dataSourceNames = toNames(this.dataSources); expect(dataSourceNames).to.contain('db'); }); @@ -88,10 +88,6 @@ describe('Workspace', function() { }); }); - describe('Workspace.listUseableConnectors(cb)', function () { - it('should return a list of connectors in package.json'); - }); - describe('project.listAvailableConnectors(cb)', function() { before(function(done) { Workspace.listAvailableConnectors(function(err, list) { From ecf8f77172f3ec49f65ead083b4192f0aa5ee7d2 Mon Sep 17 00:00:00 2001 From: Ritchie Martori Date: Thu, 3 Jul 2014 11:41:37 -0700 Subject: [PATCH 5/6] Ensure component models are saved to the correct component --- models/component-definition.js | 12 +++++++----- models/definition.js | 1 - 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/models/component-definition.js b/models/component-definition.js index ae219a2e..e925af31 100644 --- a/models/component-definition.js +++ b/models/component-definition.js @@ -67,7 +67,7 @@ ComponentDefinition.loadIntoCache = function(cache, componentName, components, c configFile: path.join(componentName, 'config.json') }; debug('adding to cache component entry [%s]', componentData.configFile); - ComponentDefinition.addToCache(cache, componentName, componentData); + ComponentDefinition.addToCache(cache, componentData); cb(); }); } @@ -206,10 +206,12 @@ ComponentDefinition.saveToFs = function(cache, componentDef, cb) { var componentModelsConfig = componentModelFile.data = {}; cachedComponentModels.forEach(function(componentModel) { - componentModelsConfig[componentModel.name] = componentModel; - delete componentModel.name; - delete componentModel.id; - delete componentModel.componentName; + if(componentModel.componentName === componentName) { + componentModelsConfig[componentModel.name] = componentModel; + delete componentModel.name; + delete componentModel.id; + delete componentModel.componentName; + } }); filesToSave.push(componentModelFile); diff --git a/models/definition.js b/models/definition.js index 371ba298..9e39ee5c 100644 --- a/models/definition.js +++ b/models/definition.js @@ -114,7 +114,6 @@ Definition.addRelatedToCache = function(cache, fileData, componentName, fk) { if(Array.isArray(relatedData)) { relatedData.forEach(function(config) { - var entity = new Entity(config); config[relation.foreignKey] = fk; config.componentName = componentName; debug('addRelatedToCache %s %j', relation.model, config); From 1b8da9c48e7b5a76394d295da5bd6d5cdf2ade21 Mon Sep 17 00:00:00 2001 From: Ritchie Martori Date: Thu, 3 Jul 2014 11:52:03 -0700 Subject: [PATCH 6/6] Refactor getUniqueId into a static method --- models/component-definition.js | 4 ++-- models/workspace-entity.js | 24 ++++++++++++++---------- test/workspace-entity.js | 5 ++++- 3 files changed, 20 insertions(+), 13 deletions(-) diff --git a/models/component-definition.js b/models/component-definition.js index e925af31..918a1bb9 100644 --- a/models/component-definition.js +++ b/models/component-definition.js @@ -250,6 +250,6 @@ ComponentDefinition.hasApp = function(componentDef) { return componentDef.name !== '.'; }; -ComponentDefinition.prototype.getUniqueId = function() { - return this.name || null; +ComponentDefinition.getUniqueId = function(data) { + return data.name || null; } diff --git a/models/workspace-entity.js b/models/workspace-entity.js index fb0c3726..553c6665 100644 --- a/models/workspace-entity.js +++ b/models/workspace-entity.js @@ -8,22 +8,27 @@ var WorkspaceEntity = app.model('WorkspaceEntity', { "dataSource": "db" }); -WorkspaceEntity.prototype.getUniqueId = function() { - var sep = this.constructor.settings.idSeparator || '.'; - var parts = this.getUniqueIdParts(); +WorkspaceEntity.getUniqueId = function(data) { + var sep = this.settings.idSeparator || '.'; + var parts = this.getUniqueIdParts(data); if(parts.length >= 1) { return parts.join(sep); } return null; } -WorkspaceEntity.prototype.getUniqueIdParts = function() { - var settings = this.constructor.settings; - var parentPropertyName = this.constructor.getParentPropertyName(); +WorkspaceEntity.prototype.getUniqueId = function() { + return this.constructor.getUniqueId(this); +} + +WorkspaceEntity.getUniqueIdParts = function(data) { + var settings = this.settings; + var parentPropertyName = this.getParentPropertyName(); var parts = []; - var parentId = parentPropertyName && this[parentPropertyName]; + var parentId = parentPropertyName && data[parentPropertyName]; var splitParentId = parentId && parentId.split('.'); var parentIdIsNotRootComponent = parentId !== '.'; + var name = data.name; if(parentPropertyName) { if(parentId) { @@ -36,7 +41,7 @@ WorkspaceEntity.prototype.getUniqueIdParts = function() { } } - if(this.name) parts.push(this.name); + if(name) parts.push(name); return parts; } @@ -73,8 +78,7 @@ WorkspaceEntity.clearCache = function(cache) { WorkspaceEntity.addToCache = function(cache, val) { var Entity = this; - var entity = new Entity(val); - var id = entity.getUniqueId(); + var id = Entity.getUniqueId(val); val[this.dataSource.idName(Entity.modelName)] = id; cache[this.modelName][id] = JSON.stringify(val); } diff --git a/test/workspace-entity.js b/test/workspace-entity.js index 1230c7b8..a1cf3cdc 100644 --- a/test/workspace-entity.js +++ b/test/workspace-entity.js @@ -16,7 +16,10 @@ describe('WorkspaceEntity', function() { componentName: 'foo', name: 'bar' }); - expect(bar.getUniqueId()).to.equal('foo.bar'); + var expected = 'foo.bar'; + expect(bar.getUniqueId()).to.equal(expected); + expect(MyWorkspaceEntity.getUniqueId(bar)).to.equal(expected); + expect(MyWorkspaceEntity.getUniqueId(bar.toObject())).to.equal(expected); }); }); });