fix(cli): improve openapi code generation for naming and typing

- validated against openapi specs from https://www.berlin-group.org/psd2-access-to-bank-accounts

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;

packages/cli/generators/openapi/spec-helper.js

@@ -14,7 +14,6 @@ const {
   kebabCase,
   camelCase,
   escapeIdentifier,
-  escapePropertyOrMethodName,
 } = require('./utils');
 
 const HTTP_VERBS = [
@@ -134,10 +133,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) {

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,
 };

packages/cli/test/fixtures/openapi/3.0/customer.yaml

@@ -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'

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');
+  });
+});

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',
       },
     ]);