Skip to content

Commit

Permalink
fix(cli): keep or escape property names for models
Browse files Browse the repository at this point in the history 
We need to maintain the property names as `@property` does not support
name customization yet.
  • Loading branch information
@raymondfeng
raymondfeng committed Apr 12, 2019
1 parent 9e7ea17 commit 3639388
Show file tree
Hide file tree
Showing 6 changed files with 68 additions and 46 deletions.
10 changes: 5 additions & 5 deletions packages/cli/generators/openapi/schema-helper.js
View file
Expand Up @@ -9,7 +9,7 @@ const {
isExtension,
titleCase,
kebabCase,
escapeIdentifier,
escapePropertyName,
toJsonStr,
} = require('./utils');

Expand Down Expand Up @@ -159,12 +159,12 @@ function mapObjectType(schema, options) {
}),
);
// The property name might have chars such as `-`
const propName = escapeIdentifier(p);
const propName = escapePropertyName(p);

let propDecoration = `@property({name: '${p}'})`;
let propDecoration = `@property()`;

if (required.includes(p)) {
propDecoration = `@property({name: '${p}', required: true})`;
propDecoration = `@property({required: true})`;
}

if (propertyType.itemType) {
Expand All @@ -177,7 +177,7 @@ function mapObjectType(schema, options) {
getJSType(propertyType.itemType.name);
if (itemType) {
// Use `@property.array` for array types
propDecoration = `@property.array(${itemType}, {name: '${p}'})`;
propDecoration = `@property.array(${itemType})`;
if (propertyType.itemType.className) {
// The referenced item type is either a class or type
collectImports(typeSpec, propertyType.itemType);
Expand Down
12 changes: 12 additions & 0 deletions packages/cli/generators/openapi/utils.js
View file
Expand Up @@ -151,6 +151,17 @@ function escapeIdentifier(name) {
return name;
}

/**
* Escape the property if it's not a valid JavaScript identifer
* @param {string} name
*/
function escapePropertyName(name) {
if (JS_KEYWORDS.includes(name) || !name.match(SAFE_IDENTIFER)) {
return toJsonStr(name);
}
return name;
}

function toJsonStr(val) {
return json5.stringify(val, null, 2);
}
Expand All @@ -163,6 +174,7 @@ module.exports = {
kebabCase: utils.kebabCase,
camelCase: _.camelCase,
escapeIdentifier,
escapePropertyName,
toJsonStr,
validateUrlOrFile,
};
4 changes: 2 additions & 2 deletions packages/cli/test/integration/generators/openapi-petstore.integration.js
View file
Expand Up @@ -73,10 +73,10 @@ describe('openapi-generator specific files', function() {
assert.fileContent(newPetModel, `@model({name: 'NewPet'})`);
assert.fileContent(
newPetModel,
`@property({name: 'name', required: true})`,
`@property({required: true})`,
);
assert.fileContent(newPetModel, `name: string;`);
assert.fileContent(newPetModel, `@property({name: 'tag'})`);
assert.fileContent(newPetModel, `@property()`);
assert.fileContent(newPetModel, `tag?: string`);
assert.file(errorModel);
});
Expand Down
7 changes: 2 additions & 5 deletions packages/cli/test/integration/generators/swagger-petstore.integration.js
View file
Expand Up @@ -69,12 +69,9 @@ describe('openapi-generator specific files', () => {
assert.fileContent(newPetModel, `export class NewPet {`);
assert.fileContent(newPetModel, `constructor(data?: Partial<NewPet>) {`);
assert.fileContent(newPetModel, `@model({name: 'NewPet'})`);
assert.fileContent(
newPetModel,
`@property({name: 'name', required: true})`,
);
assert.fileContent(newPetModel, `@property({required: true})`);
assert.fileContent(newPetModel, `name: string;`);
assert.fileContent(newPetModel, `@property({name: 'tag'})`);
assert.fileContent(newPetModel, `@property()`);
assert.fileContent(newPetModel, `tag?: string`);
assert.file(errorModel);
});
Expand Down
13 changes: 13 additions & 0 deletions packages/cli/test/unit/openapi/openapi-utils.unit.js
View file
Expand Up @@ -22,4 +22,17 @@ describe('openapi utils', () => {
expect(utils.escapeIdentifier('fooBar')).to.eql('fooBar');
expect(utils.escapeIdentifier('Foobar')).to.eql('Foobar');
});

it('escapes property names with illegal chars', () => {
expect(utils.escapePropertyName('customer-id')).to.eql("'customer-id'");
expect(utils.escapePropertyName('customer id')).to.eql("'customer id'");
expect(utils.escapePropertyName('customer.id')).to.eql("'customer.id'");
expect(utils.escapePropertyName('default')).to.eql("'default'");
});

it('does not escape property names with legal chars', () => {
expect(utils.escapePropertyName('customerId')).to.eql('customerId');
expect(utils.escapePropertyName('customer_id')).to.eql('customer_id');
expect(utils.escapePropertyName('customerid')).to.eql('customerid');
});
});
68 changes: 34 additions & 34 deletions packages/cli/test/unit/openapi/schema-model.unit.js
View file
Expand Up @@ -50,14 +50,14 @@ describe('schema to model', () => {
{
name: 'total',
signature: 'total?: number;',
decoration: "@property({name: 'total'})",
decoration: '@property()',
},
{
name: 'apis',
signature:
'apis?: {\n apiKey?: string;\n apiVersionNumber?: string;\n' +
' apiUrl?: string;\n apiDocumentationUrl?: string;\n}[];',
decoration: "@property({name: 'apis'})",
decoration: '@property()',
},
],
declaration:
Expand All @@ -81,14 +81,14 @@ describe('schema to model', () => {
{
name: 'total',
signature: 'total?: number;',
decoration: "@property({name: 'total'})",
decoration: '@property()',
},
{
name: 'apis',
signature:
'apis?: {\n apiKey?: string;\n apiVersionNumber?: string;\n ' +
'apiUrl?: string;\n apiDocumentationUrl?: string;\n}[];',
decoration: "@property({name: 'apis'})",
decoration: '@property()',
},
],
imports: [],
Expand All @@ -109,7 +109,7 @@ describe('schema to model', () => {
{
name: 'criteria',
signature: "criteria: string = '*:*';",
decoration: "@property({name: 'criteria', required: true})",
decoration: '@property({required: true})',
description:
'Uses Lucene Query Syntax in the format of propertyName:value, ' +
'propertyName:[num1 TO num2] and date range format: ' +
Expand All @@ -121,13 +121,13 @@ describe('schema to model', () => {
{
name: 'start',
signature: 'start?: number = 0;',
decoration: "@property({name: 'start'})",
decoration: '@property()',
description: 'Starting record number. Default value is 0.',
},
{
name: 'rows',
signature: 'rows?: number = 100;',
decoration: "@property({name: 'rows'})",
decoration: '@property()',
description:
'Specify number of rows to be returned. If you run the search ' +
"with default values, in the response you will see 'numFound' " +
Expand Down Expand Up @@ -188,12 +188,12 @@ describe('schema to model', () => {
{
name: 'name',
signature: 'name: string;',
decoration: "@property({name: 'name', required: true})",
decoration: '@property({required: true})',
},
{
name: 'tag',
signature: 'tag?: string;',
decoration: "@property({name: 'tag'})",
decoration: '@property()',
},
],
imports: [],
Expand All @@ -208,7 +208,7 @@ describe('schema to model', () => {
{
name: 'id',
signature: 'id: number;',
decoration: "@property({name: 'id', required: true})",
decoration: '@property({required: true})',
},
],
signature: '{\n id: number;\n}',
Expand All @@ -228,12 +228,12 @@ describe('schema to model', () => {
{
name: 'name',
signature: 'name: string;',
decoration: "@property({name: 'name', required: true})",
decoration: '@property({required: true})',
},
{
name: 'tag',
signature: 'tag?: string;',
decoration: "@property({name: 'tag'})",
decoration: '@property()',
},
],
imports: [],
Expand All @@ -251,12 +251,12 @@ describe('schema to model', () => {
{
name: 'code',
signature: 'code: number;',
decoration: "@property({name: 'code', required: true})",
decoration: '@property({required: true})',
},
{
name: 'message',
signature: 'message: string;',
decoration: "@property({name: 'message', required: true})",
decoration: '@property({required: true})',
},
],
imports: [],
Expand Down Expand Up @@ -312,22 +312,22 @@ describe('schema to model', () => {
{
name: 'street',
signature: 'street?: string;',
decoration: "@property({name: 'street'})",
decoration: '@property()',
},
{
name: 'city',
signature: 'city?: string;',
decoration: "@property({name: 'city'})",
decoration: '@property()',
},
{
name: 'state',
signature: 'state?: string;',
decoration: "@property({name: 'state'})",
decoration: '@property()',
},
{
name: 'zipCode',
signature: 'zipCode?: string;',
decoration: "@property({name: 'zipCode'})",
decoration: '@property()',
},
],
imports: [],
Expand All @@ -347,22 +347,22 @@ describe('schema to model', () => {
{
name: 'street',
signature: 'street?: string;',
decoration: "@property({name: 'street'})",
decoration: '@property()',
},
{
name: 'city',
signature: 'city?: string;',
decoration: "@property({name: 'city'})",
decoration: '@property()',
},
{
name: 'state',
signature: 'state?: string;',
decoration: "@property({name: 'state'})",
decoration: '@property()',
},
{
name: 'zipCode',
signature: 'zipCode?: string;',
decoration: "@property({name: 'zipCode'})",
decoration: '@property()',
},
],
imports: [],
Expand All @@ -381,37 +381,37 @@ describe('schema to model', () => {
{
name: 'id',
signature: 'id: number;',
decoration: "@property({name: 'id', required: true})",
decoration: '@property({required: true})',
},
{
name: 'first-name',
signature: 'firstName?: string;',
decoration: "@property({name: 'first-name'})",
signature: "'first-name'?: string;",
decoration: '@property()',
},
{
name: 'last-name',
signature: 'lastName?: Name;',
decoration: "@property({name: 'last-name'})",
signature: "'last-name'?: Name;",
decoration: '@property()',
},
{
name: 'profiles',
signature: 'profiles?: ProfileId[];',
decoration: "@property.array(String, {name: 'profiles'})",
decoration: "@property.array(String)",
},
{
name: 'emails',
signature: 'emails?: string[];',
decoration: "@property.array(String, {name: 'emails'})",
decoration: "@property.array(String)",
},
{
name: 'addresses',
signature: 'addresses?: AddressList;',
decoration: "@property.array(Address, {name: 'addresses'})",
decoration: "@property.array(Address)",
},
{
name: 'us-office',
signature: 'usOffice?: Address;',
decoration: "@property({name: 'us-office'})",
signature: "'us-office'?: Address;",
decoration: '@property()',
},
],
imports: [
Expand All @@ -423,9 +423,9 @@ describe('schema to model', () => {
import: "import {Customer} from './customer.model';",
kind: 'class',
declaration:
'{\n id: number;\n firstName?: string;\n lastName?: Name;\n' +
"{\n id: number;\n 'first-name'?: string;\n 'last-name'?: Name;\n" +
' profiles?: ProfileId[];\n emails?: string[];\n' +
' addresses?: AddressList;\n usOffice?: Address;\n}',
" addresses?: AddressList;\n 'us-office'?: Address;\n}",
signature: 'Customer',
},
]);
Expand Down

0 comments on commit 3639388

Please sign in to comment.