From 42dedfcaccf99cdcd95c26608c23cc2a543f3bfe Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miroslav=20Bajto=C5=A1?= Date: Wed, 15 Oct 2014 13:54:21 +0200 Subject: [PATCH 1/3] Move `convertText` to `typeConverter` Create a new module `lib/type-converter.js`. Move `routeHelper.convertText` to this new module. --- lib/class-helper.js | 4 ++-- lib/route-helper.js | 19 ++++--------------- lib/type-converter.js | 14 ++++++++++++++ 3 files changed, 20 insertions(+), 17 deletions(-) create mode 100644 lib/type-converter.js diff --git a/lib/class-helper.js b/lib/class-helper.js index 1e9c128..c03cb1b 100644 --- a/lib/class-helper.js +++ b/lib/class-helper.js @@ -4,7 +4,7 @@ * Module dependencies. */ var modelHelper = require('./model-helper'); -var routeHelper = require('./route-helper'); +var typeConverter = require('./type-converter'); var urlJoin = require('./url-join'); /** @@ -47,7 +47,7 @@ var classHelper = module.exports = { return { path: aClass.http.path, - description: routeHelper.convertText(description) + description: typeConverter.convertText(description) }; } }; diff --git a/lib/route-helper.js b/lib/route-helper.js index f52ed4c..add9775 100644 --- a/lib/route-helper.js +++ b/lib/route-helper.js @@ -8,6 +8,7 @@ var debug = require('debug')('loopback:explorer:routeHelpers'); var _cloneDeep = require('lodash.clonedeep'); var translateDataTypeKeys = require('./translate-data-type-keys'); var modelHelper = require('./model-helper'); +var typeConverter = require('./type-converter'); /** * Export the routeHelper singleton. @@ -142,8 +143,8 @@ var routeHelper = module.exports = { parameters: accepts, // TODO(schoon) - We don't have descriptions for this yet. responseMessages: [], - summary: routeHelper.convertText(route.description), - notes: routeHelper.convertText(route.notes), + summary: typeConverter.convertText(route.description), + notes: typeConverter.convertText(route.notes), deprecated: route.deprecated })] }; @@ -200,7 +201,7 @@ var routeHelper = module.exports = { var out = { paramType: paramType || type, name: name, - description: routeHelper.convertText(accepts.description), + description: typeConverter.convertText(accepts.description), type: accepts.type, required: !!accepts.required, defaultValue: accepts.defaultValue, @@ -239,18 +240,6 @@ var routeHelper = module.exports = { obj[key] = typeDesc[key]; }); return obj; - }, - - /** - * Convert a text value that can be expressed either as a string or - * as an array of strings. - * @param {string|Array} value - * @returns {string} - */ - convertText: function(value) { - if (Array.isArray(value)) - return value.join('\n'); - return value; } }; diff --git a/lib/type-converter.js b/lib/type-converter.js new file mode 100644 index 0000000..c7eb686 --- /dev/null +++ b/lib/type-converter.js @@ -0,0 +1,14 @@ +var typeConverter = module.exports = { + + /** + * Convert a text value that can be expressed either as a string or + * as an array of strings. + * @param {string|Array} value + * @returns {string} + */ + convertText: function(value) { + if (Array.isArray(value)) + return value.join('\n'); + return value; + } +}; From dc815a84211fb7d8ab3946dc27416eaa9908d704 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miroslav=20Bajto=C5=A1?= Date: Wed, 15 Oct 2014 13:58:13 +0200 Subject: [PATCH 2/3] Refactor conversion of data types Refactor the way how loopback types are converted to swagger data types. - `modelHelper.LDLPropToSwaggerDataType` is responsible for producing a valid Swagger Data Type object from LDL object (be it a property, accepts item or returns item). - LDLPropToSwaggerDataType picks only fields that are part of the swagger spec, everything else is excluded from the result. It's up to the caller to add extra fields like `description`. - refactor `routeHelper.extendWithType` to accept an additional arg: the original LDL object. This way it's possible to copy all type-specific fields to the output object and don't add anything else. --- lib/model-helper.js | 68 +++++++++++++++++++++------------ lib/route-helper.js | 53 ++++++++++--------------- lib/translate-data-type-keys.js | 1 - package.json | 2 + test/model-helper.test.js | 27 +++++++++++-- test/route-helper.test.js | 15 ++++++++ 6 files changed, 104 insertions(+), 62 deletions(-) diff --git a/lib/model-helper.js b/lib/model-helper.js index 1cff049..508df88 100644 --- a/lib/model-helper.js +++ b/lib/model-helper.js @@ -4,7 +4,9 @@ * Module dependencies. */ var _cloneDeep = require('lodash.clonedeep'); +var _pick = require('lodash.pick'); var translateDataTypeKeys = require('./translate-data-type-keys'); +var typeConverter = require('./type-converter'); /** * Export the modelHelper singleton. @@ -57,20 +59,20 @@ var modelHelper = module.exports = { } // Eke a type out of the constructors we were passed. - prop = modelHelper.LDLPropToSwaggerDataType(prop); + var swaggerType = modelHelper.LDLPropToSwaggerDataType(prop); + + var desc = typeConverter.convertText(prop.description || prop.doc); + if (desc) swaggerType.description = desc; // Required props sit in a per-model array. if (prop.required || (prop.id && !prop.generated)) { required.push(key); } - // Change mismatched keys. - prop = translateDataTypeKeys(prop); - // Assign this back to the properties object. - properties[key] = prop; + properties[key] = swaggerType; - var propType = def.properties[key].type; + var propType = prop.type; if (typeof propType === 'function' && propType.modelName) { if (referencedModels.indexOf(propType) === -1) { referencedModels.push(propType); @@ -134,34 +136,52 @@ var modelHelper = module.exports = { // Converts a prop defined with the LDL spec to one conforming to the // Swagger spec. // https://github.com/wordnik/swagger-spec/blob/master/versions/1.2.md#431-primitives - LDLPropToSwaggerDataType: function LDLPropToSwaggerDataType(prop) { - var out = _cloneDeep(prop); - out.type = modelHelper.getPropType(out.type); + LDLPropToSwaggerDataType: function LDLPropToSwaggerDataType(ldlType) { + var SWAGGER_DATA_TYPE_FIELDS = [ + 'format', + 'defaultValue', + 'enum', + 'minimum', + 'maximum', + 'uniqueItems', + // loopback-explorer extensions + 'length', + // https://www.npmjs.org/package/swagger-validation + 'pattern' + ]; - if (out.type === 'array') { - var hasItemType = Array.isArray(prop.type) && prop.type.length; - var arrayItem = hasItemType && prop.type[0]; + // Rename LoopBack keys to Swagger keys + ldlType = translateDataTypeKeys(ldlType); + + // Pick only keys supported by Swagger + var swaggerType = _pick(ldlType, SWAGGER_DATA_TYPE_FIELDS); + + swaggerType.type = modelHelper.getPropType(ldlType.type); + + if (swaggerType.type === 'array') { + var hasItemType = Array.isArray(ldlType.type) && ldlType.type.length; + var arrayItem = hasItemType && ldlType.type[0]; if (arrayItem) { if(typeof arrayItem === 'object') { - out.items = modelHelper.LDLPropToSwaggerDataType(arrayItem); + swaggerType.items = modelHelper.LDLPropToSwaggerDataType(arrayItem); } else { - out.items = { type: modelHelper.getPropType(arrayItem) }; + swaggerType.items = { type: modelHelper.getPropType(arrayItem) }; } } else { // NOTE: `any` is not a supported type in swagger 1.2 - out.items = { type: 'any' }; + swaggerType.items = { type: 'any' }; } - } else if (out.type === 'date') { - out.type = 'string'; - out.format = 'date'; - } else if (out.type === 'buffer') { - out.type = 'string'; - out.format = 'byte'; - } else if (out.type === 'number') { - out.format = 'double'; // Since all JS numbers are doubles + } else if (swaggerType.type === 'date') { + swaggerType.type = 'string'; + swaggerType.format = 'date'; + } else if (swaggerType.type === 'buffer') { + swaggerType.type = 'string'; + swaggerType.format = 'byte'; + } else if (swaggerType.type === 'number') { + swaggerType.format = 'double'; // Since all JS numbers are doubles } - return out; + return swaggerType; } }; diff --git a/lib/route-helper.js b/lib/route-helper.js index add9775..d148cea 100644 --- a/lib/route-helper.js +++ b/lib/route-helper.js @@ -6,7 +6,7 @@ var debug = require('debug')('loopback:explorer:routeHelpers'); var _cloneDeep = require('lodash.clonedeep'); -var translateDataTypeKeys = require('./translate-data-type-keys'); +var _assign = require('lodash.assign'); var modelHelper = require('./model-helper'); var typeConverter = require('./type-converter'); @@ -71,9 +71,6 @@ var routeHelper = module.exports = { return true; }); - // Translate LDL keys to Swagger keys. - accepts = accepts.map(translateDataTypeKeys); - // Turn accept definitions in to parameter docs. accepts = accepts.map(routeHelper.acceptToParameter(route)); @@ -98,19 +95,19 @@ var routeHelper = module.exports = { } } - // Translate LDL keys to Swagger keys. - var returns = routeReturns.map(translateDataTypeKeys); - - // Convert `returns` into a single object for later conversion into an + // Convert `returns` into a single object for later conversion into an // operation object. - if (returns && returns.length > 1) { + if (routeReturns && routeReturns.length > 1) { // TODO ad-hoc model definition in the case of multiple return values. - returns = {model: 'object'}; + routeReturns = { type: 'object' }; } else { - returns = returns[0] || {}; + // Per the spec: + // https://github.com/wordnik/swagger-spec/blob/master/versions/1.2.md#523-operation-object + // This is the only object that may have a type of 'void'. + routeReturns = routeReturns[0] || { type: 'void' }; } - return returns; + return routeReturns; }, /** @@ -119,9 +116,7 @@ var routeHelper = module.exports = { * See https://github.com/wordnik/swagger-spec/blob/master/versions/1.2.md#523-operation-object */ routeToAPIDoc: function routeToAPIDoc(route, classDef) { - var returnDesc; - - // Some parameters need to be altered; eventually most of this should + // Some parameters need to be altered; eventually most of this should // be removed. var accepts = routeHelper.convertAcceptsToSwagger(route, classDef); var returns = routeHelper.convertReturnsToSwagger(route, classDef); @@ -136,17 +131,13 @@ var routeHelper = module.exports = { method: routeHelper.convertVerb(route.verb), // [rfeng] Swagger UI doesn't escape '.' for jQuery selector nickname: route.method.replace(/\./g, '_'), - // Per the spec: - // https://github.com/wordnik/swagger-spec/blob/master/versions/1.2.md#523-operation-object - // This is the only object that may have a type of 'void'. - type: returns.model || returns.type || 'void', parameters: accepts, // TODO(schoon) - We don't have descriptions for this yet. responseMessages: [], summary: typeConverter.convertText(route.description), notes: typeConverter.convertText(route.notes), deprecated: route.deprecated - })] + }, returns)] }; return apiDoc; @@ -202,15 +193,11 @@ var routeHelper = module.exports = { paramType: paramType || type, name: name, description: typeConverter.convertText(accepts.description), - type: accepts.type, required: !!accepts.required, - defaultValue: accepts.defaultValue, - minimum: accepts.minimum, - maximum: accepts.maximum, allowMultiple: false }; - out = routeHelper.extendWithType(out); + out = routeHelper.extendWithType(out, accepts); // HACK: Derive the type from model if(out.name === 'data' && out.type === 'object') { @@ -222,23 +209,23 @@ var routeHelper = module.exports = { }, /** - * Extends an Operation Object or Parameter object with + * Extends an Operation Object or Parameter object with * a proper Swagger type and optional `format` and `items` fields. * Does not modify original object. * @param {Object} obj Object to extend. - * @return {Object} Extended object. + * @param {Object} ldlType LDL type definition + * @return {Object} Extended object. */ - extendWithType: function extendWithType(obj) { + extendWithType: function extendWithType(obj, ldlType) { obj = _cloneDeep(obj); // Format the `type` property using our LDL converter. - var typeDesc = modelHelper - .LDLPropToSwaggerDataType({type: obj.model || obj.type}); + var typeDesc = modelHelper.LDLPropToSwaggerDataType(ldlType); + // The `typeDesc` may have additional attributes, such as // `format` for non-primitive types. - Object.keys(typeDesc).forEach(function(key){ - obj[key] = typeDesc[key]; - }); + _assign(obj, typeDesc); + return obj; } }; diff --git a/lib/translate-data-type-keys.js b/lib/translate-data-type-keys.js index a47c85d..d1e84a7 100644 --- a/lib/translate-data-type-keys.js +++ b/lib/translate-data-type-keys.js @@ -9,7 +9,6 @@ var _cloneDeep = require('lodash.clonedeep'); // Keys that are different between LDL and Swagger var KEY_TRANSLATIONS = { // LDL : Swagger - 'doc': 'description', 'default': 'defaultValue', 'min': 'minimum', 'max': 'maximum' diff --git a/package.json b/package.json index 0543493..86e6dc7 100644 --- a/package.json +++ b/package.json @@ -34,8 +34,10 @@ "cors": "^2.4.2", "debug": "~1.0.3", "express": "3.x", + "lodash.assign": "^2.4.1", "lodash.clonedeep": "^2.4.1", "lodash.defaults": "^2.4.1", + "lodash.pick": "^2.4.1", "swagger-ui": "~2.0.18" } } diff --git a/test/model-helper.test.js b/test/model-helper.test.js index 98fd4b1..299f5ae 100644 --- a/test/model-helper.test.js +++ b/test/model-helper.test.js @@ -1,6 +1,7 @@ 'use strict'; var modelHelper = require('../lib/model-helper'); +var _assign = require('lodash.assign'); var loopback = require('loopback'); var expect = require('chai').expect; @@ -122,6 +123,22 @@ describe('model-helper', function() { }); }); + + it('converts model property field `doc`', function() { + var def = buildSwaggerModels({ + name: { type: String, doc: 'a-description' } + }); + var nameProp = def.properties.name; + expect(nameProp).to.have.property('description', 'a-description'); + }); + + it('converts model property field `description`', function() { + var def = buildSwaggerModels({ + name: { type: String, description: 'a-description' } + }); + var nameProp = def.properties.name; + expect(nameProp).to.have.property('description', 'a-description'); + }); }); describe('related models', function() { @@ -200,15 +217,17 @@ function buildSwaggerModels(model) { return modelHelper.generateModelDefinition(aClass.ctor, {}).testModel; } -function createModelCtor(model) { - Object.keys(model).forEach(function(name) { - model[name] = {type: model[name]}; +function createModelCtor(properties) { + Object.keys(properties).forEach(function(name) { + var type = properties[name]; + if (typeof type !== 'object' || Array.isArray(type)) + properties[name] = { type: type }; }); var aClass = { ctor: { definition: { name: 'testModel', - properties: model + properties: properties } } }; diff --git a/test/route-helper.test.js b/test/route-helper.test.js index 6714480..e5a85e8 100644 --- a/test/route-helper.test.js +++ b/test/route-helper.test.js @@ -143,6 +143,21 @@ describe('route-helper', function() { expect(params.length).to.equal(0); }); + it('preserves `enum` accepts arg metadata', function() { + var doc = createAPIDoc({ + accepts: [{ name: 'arg', type: 'number', enum: [1,2,3] }] + }); + expect(doc.operations[0].parameters[0]) + .to.have.property('enum').eql([1,2,3]); + }); + + it('preserves `enum` returns arg metadata', function() { + var doc = createAPIDoc({ + returns: [{ name: 'arg', root: true, type: 'number', enum: [1,2,3] }] + }); + expect(doc.operations[0]) + .to.have.property('enum').eql([1,2,3]); + }); }); // Easy wrapper around createRoute From 060354cff8e80b81ac7f22c936523a774332b974 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miroslav=20Bajto=C5=A1?= Date: Wed, 15 Oct 2014 14:07:23 +0200 Subject: [PATCH 3/3] models: include model's `description` --- lib/model-helper.js | 1 + test/model-helper.test.js | 25 +++++++++++++++++-------- 2 files changed, 18 insertions(+), 8 deletions(-) diff --git a/lib/model-helper.js b/lib/model-helper.js index 508df88..51fbf53 100644 --- a/lib/model-helper.js +++ b/lib/model-helper.js @@ -90,6 +90,7 @@ var modelHelper = module.exports = { out[name] = { id: name, + description: typeConverter.convertText(def.description), properties: properties, required: required }; diff --git a/test/model-helper.test.js b/test/model-helper.test.js index 299f5ae..b183400 100644 --- a/test/model-helper.test.js +++ b/test/model-helper.test.js @@ -1,7 +1,7 @@ 'use strict'; var modelHelper = require('../lib/model-helper'); -var _assign = require('lodash.assign'); +var _defaults = require('lodash.defaults'); var loopback = require('loopback'); var expect = require('chai').expect; @@ -139,6 +139,11 @@ describe('model-helper', function() { var nameProp = def.properties.name; expect(nameProp).to.have.property('description', 'a-description'); }); + + it('converts model field `description`', function() { + var def = buildSwaggerModels({}, { description: 'a-description' }); + expect(def).to.have.property('description', 'a-description'); + }); }); describe('related models', function() { @@ -212,23 +217,27 @@ describe('model-helper', function() { }); // Simulates the format of a remoting class. -function buildSwaggerModels(model) { - var aClass = createModelCtor(model); +function buildSwaggerModels(modelProperties, modelOptions) { + var aClass = createModelCtor(modelProperties, modelOptions); return modelHelper.generateModelDefinition(aClass.ctor, {}).testModel; } -function createModelCtor(properties) { +function createModelCtor(properties, modelOptions) { Object.keys(properties).forEach(function(name) { var type = properties[name]; if (typeof type !== 'object' || Array.isArray(type)) properties[name] = { type: type }; }); + + var definition = { + name: 'testModel', + properties: properties + }; + _defaults(definition, modelOptions); + var aClass = { ctor: { - definition: { - name: 'testModel', - properties: properties - } + definition: definition } }; return aClass;