diff --git a/packages/cli/generators/openapi/schema-helper.js b/packages/cli/generators/openapi/schema-helper.js index 62c8d11288c1..3275d646780b 100644 --- a/packages/cli/generators/openapi/schema-helper.js +++ b/packages/cli/generators/openapi/schema-helper.js @@ -9,7 +9,7 @@ const { isExtension, titleCase, kebabCase, - escapePropertyOrMethodName, + escapeIdentifier, toJsonStr, } = require('./utils'); @@ -159,7 +159,7 @@ function mapObjectType(schema, options) { }), ); // The property name might have chars such as `-` - const propName = escapePropertyOrMethodName(p); + const propName = escapeIdentifier(p); let propDecoration = `@property({name: '${p}'})`; @@ -171,11 +171,17 @@ function mapObjectType(schema, options) { const itemType = propertyType.itemType.kind === 'class' ? propertyType.itemType.className - : getJSType(propertyType.itemType.name); + : // The item type can be an alias such as `export type ID = string` + getJSType(propertyType.itemType.declaration) || + // The item type can be `string` + getJSType(propertyType.itemType.name); if (itemType) { // Use `@property.array` for array types propDecoration = `@property.array(${itemType}, {name: '${p}'})`; - collectImports(typeSpec, propertyType.itemType); + if (propertyType.itemType.className) { + // The referenced item type is either a class or type + collectImports(typeSpec, propertyType.itemType); + } } } const propSpec = { @@ -277,7 +283,7 @@ const JSTypeMapping = { */ function getJSType(type) { const ctor = JSTypeMapping[type]; - return (ctor && ctor.name) || type; + return ctor && ctor.name; } /** @@ -319,13 +325,12 @@ function mapSchemaType(schema, options) { * @param {object} schema */ function resolveSchema(schemaMapping, schema) { - if (schema['x-$ref']) { - const resolved = schemaMapping[schema['x-$ref']]; - if (resolved) { - return resolved; - } + if (!schema['x-$ref']) return schema; + let resolved = schema; + while (resolved && resolved['x-$ref']) { + resolved = schemaMapping[resolved['x-$ref']]; } - return schema; + return resolved || schema; } /** @@ -350,7 +355,10 @@ function generateModelSpecs(apiSpec, options) { // `model` is `undefined` for primitive types if (model == null) continue; if (model.className) { - models.push(model); + // The model might be a $ref + if (!models.includes(model)) { + models.push(model); + } } } return models; diff --git a/packages/cli/generators/openapi/spec-helper.js b/packages/cli/generators/openapi/spec-helper.js index df6dacc0d4fd..cd29953376ab 100644 --- a/packages/cli/generators/openapi/spec-helper.js +++ b/packages/cli/generators/openapi/spec-helper.js @@ -14,7 +14,6 @@ const { kebabCase, camelCase, escapeIdentifier, - escapePropertyOrMethodName, } = require('./utils'); const HTTP_VERBS = [ @@ -98,8 +97,9 @@ function groupOperationsByController(apiSpec) { let controllers = ['OpenApiController']; if (op['x-controller-name']) { controllers = [op['x-controller-name']]; - } else if (op.tags) { - controllers = op.tags.map(t => titleCase(t + 'Controller')); + } else if (Array.isArray(op.tags) && op.tags.length) { + // Only add the operation to first tag to avoid duplicate routes + controllers = [titleCase(op.tags[0] + 'Controller')]; } controllers.forEach((c, index) => { /** @@ -134,10 +134,7 @@ function groupOperationsByController(apiSpec) { * @param {object} opSpec OpenAPI operation spec */ function getMethodName(opSpec) { - return ( - opSpec['x-operation-name'] || - escapePropertyOrMethodName(camelCase(opSpec.operationId)) - ); + return opSpec['x-operation-name'] || escapeIdentifier(opSpec.operationId); } function registerAnonymousSchema(names, schema, typeRegistry) { @@ -206,7 +203,13 @@ function buildMethodSpec(controllerSpec, op, options) { const pType = mapSchemaType(p.schema, options); addImportsForType(pType); comments.push(`@param ${name} ${p.description || ''}`); - return `@param({name: '${p.name}', in: '${p.in}'}) ${name}: ${ + + // Normalize parameter name to match `\w` + let paramName = p.name; + if (p.in === 'path') { + paramName = paramName.replace(/[^\w]+/g, '_'); + } + return `@param({name: '${paramName}', in: '${p.in}'}) ${name}: ${ pType.signature }`; }); @@ -288,10 +291,17 @@ function buildMethodSpec(controllerSpec, op, options) { returnType.signature }>`; comments.unshift(op.spec.description || '', '\n'); + + // Normalize path variable names to alphanumeric characters including the + // underscore (Equivalent to [A-Za-z0-9_]). Please note `@loopback/rest` + // does not allow other characters that don't match `\w`. + const opPath = op.path.replace(/\{[^\}]+\}/g, varName => + varName.replace(/[^\w\{\}]+/g, '_'), + ); const methodSpec = { description: op.spec.description, comments, - decoration: `@operation('${op.verb}', '${op.path}')`, + decoration: `@operation('${op.verb}', '${opPath}')`, signature, }; if (op.spec['x-implementation']) { diff --git a/packages/cli/generators/openapi/utils.js b/packages/cli/generators/openapi/utils.js index 38d5397ff5d0..055129dfc7da 100644 --- a/packages/cli/generators/openapi/utils.js +++ b/packages/cli/generators/openapi/utils.js @@ -146,25 +146,7 @@ function escapeIdentifier(name) { return '_' + name; } if (!name.match(SAFE_IDENTIFER)) { - return _.camelCase(name); - } - return name; -} - -/** - * Escape a property/method name by quoting it if it's not a safe JavaScript - * identifier - * - * For example, - * - `default` -> `'default'` - * - `my-name` -> `'my-name'` - * - * @param {string} name - */ -function escapePropertyOrMethodName(name) { - if (JS_KEYWORDS.includes(name) || !name.match(SAFE_IDENTIFER)) { - // Encode the name with ', for example: 'my-name' - return `'${name}'`; + name = _.camelCase(name); } return name; } @@ -181,6 +163,6 @@ module.exports = { kebabCase: utils.kebabCase, camelCase: _.camelCase, escapeIdentifier, - escapePropertyOrMethodName, toJsonStr, + validateUrlOrFile, }; diff --git a/packages/cli/test/fixtures/openapi/3.0/customer.yaml b/packages/cli/test/fixtures/openapi/3.0/customer.yaml index 8ee276e16fb2..ca94b9ca5181 100644 --- a/packages/cli/test/fixtures/openapi/3.0/customer.yaml +++ b/packages/cli/test/fixtures/openapi/3.0/customer.yaml @@ -69,7 +69,7 @@ paths: application/json: schema: $ref: '#/components/schemas/Customer' - /customers/{id}: + /customers/{customer-id}: get: tags: - Customer @@ -77,7 +77,7 @@ paths: x-implementation: "return {id: id, 'first-name': 'John', last-name: 'Smith'};" operationId: find customer by id parameters: - - name: id + - name: customer-id in: path description: ID of customer to fetch required: true @@ -95,6 +95,8 @@ components: schemas: Name: type: string + profileId: + type: string AddressList: items: $ref: '#/components/schemas/Address' @@ -110,6 +112,8 @@ components: type: string zipCode: type: string + USAddress: + $ref: '#/components/schemas/Address' Customer: required: - id @@ -121,9 +125,15 @@ components: type: string last-name: $ref: '#/components/schemas/Name' + profiles: + type: array + items: + $ref: '#/components/schemas/profileId' emails: type: array items: type: string addresses: $ref: '#/components/schemas/AddressList' + us-office: + $ref: '#/components/schemas/USAddress' diff --git a/packages/cli/test/unit/openapi/controller-spec.unit.js b/packages/cli/test/unit/openapi/controller-spec.unit.js index bdd55dc3f624..d56e126f37ac 100644 --- a/packages/cli/test/unit/openapi/controller-spec.unit.js +++ b/packages/cli/test/unit/openapi/controller-spec.unit.js @@ -59,13 +59,13 @@ describe('openapi to controllers/models', () => { comments: [ 'Returns a customer based on a single ID', '\n', - '@param id ID of customer to fetch', + '@param customerId ID of customer to fetch', '@returns customer response', ], - decoration: "@operation('get', '/customers/{id}')", + decoration: "@operation('get', '/customers/{customer_id}')", signature: - "async findCustomerById(@param({name: 'id', in: 'path'}) " + - 'id: number): Promise', + "async findCustomerById(@param({name: 'customer_id', " + + "in: 'path'}) customerId: number): Promise", implementation: "return {id: id, 'first-name': 'John', last-name: 'Smith'};", }, diff --git a/packages/cli/test/unit/openapi/openapi-utils.unit.js b/packages/cli/test/unit/openapi/openapi-utils.unit.js new file mode 100644 index 000000000000..c30ef6e0b6cd --- /dev/null +++ b/packages/cli/test/unit/openapi/openapi-utils.unit.js @@ -0,0 +1,25 @@ +// Copyright IBM Corp. 2019. All Rights Reserved. +// Node module: @loopback/cli +// This file is licensed under the MIT License. +// License text available at https://opensource.org/licenses/MIT + +const expect = require('@loopback/testlab').expect; +const utils = require('../../../generators/openapi/utils'); + +describe('openapi utils', () => { + it('escapes keywords for an identifier', () => { + expect(utils.escapeIdentifier('for')).to.eql('_for'); + }); + + it('escapes illegal chars for an identifier', () => { + expect(utils.escapeIdentifier('foo bar')).to.eql('fooBar'); + expect(utils.escapeIdentifier('foo-bar')).to.eql('fooBar'); + expect(utils.escapeIdentifier('foo.bar')).to.eql('fooBar'); + }); + + it('does not escape legal chars for an identifier', () => { + expect(utils.escapeIdentifier('foobar')).to.eql('foobar'); + expect(utils.escapeIdentifier('fooBar')).to.eql('fooBar'); + expect(utils.escapeIdentifier('Foobar')).to.eql('Foobar'); + }); +}); diff --git a/packages/cli/test/unit/openapi/schema-model.unit.js b/packages/cli/test/unit/openapi/schema-model.unit.js index 1e08873fe6b0..602a8150672e 100644 --- a/packages/cli/test/unit/openapi/schema-model.unit.js +++ b/packages/cli/test/unit/openapi/schema-model.unit.js @@ -282,6 +282,17 @@ describe('schema to model', () => { declaration: 'string', signature: 'Name', }, + { + description: 'profileId', + name: 'profileId', + className: 'ProfileId', + fileName: 'profile-id.model.ts', + properties: [], + imports: [], + import: "import {ProfileId} from './profile-id.model';", + declaration: 'string', + signature: 'ProfileId', + }, { description: 'AddressList', name: 'Address[]', @@ -374,14 +385,19 @@ describe('schema to model', () => { }, { name: 'first-name', - signature: "'first-name'?: string;", + signature: 'firstName?: string;', decoration: "@property({name: 'first-name'})", }, { name: 'last-name', - signature: "'last-name'?: Name;", + signature: 'lastName?: Name;', decoration: "@property({name: 'last-name'})", }, + { + name: 'profiles', + signature: 'profiles?: ProfileId[];', + decoration: "@property.array(String, {name: 'profiles'})", + }, { name: 'emails', signature: 'emails?: string[];', @@ -392,17 +408,24 @@ describe('schema to model', () => { signature: 'addresses?: AddressList;', decoration: "@property.array(Address, {name: 'addresses'})", }, + { + name: 'us-office', + signature: 'usOffice?: Address;', + decoration: "@property({name: 'us-office'})", + }, ], imports: [ "import {Name} from './name.model';", + "import {ProfileId} from './profile-id.model';", "import {Address} from './address.model';", "import {AddressList} from './address-list.model';", ], import: "import {Customer} from './customer.model';", kind: 'class', declaration: - "{\n id: number;\n 'first-name'?: string;\n 'last-name'?: Name;\n" + - ' emails?: string[];\n addresses?: AddressList;\n}', + '{\n id: number;\n firstName?: string;\n lastName?: Name;\n' + + ' profiles?: ProfileId[];\n emails?: string[];\n' + + ' addresses?: AddressList;\n usOffice?: Address;\n}', signature: 'Customer', }, ]);