From ce33aafebd55b0e8684e1040fe55279cc3271666 Mon Sep 17 00:00:00 2001 From: Luis Fernando Planella Gonzalez Date: Wed, 14 Aug 2019 13:27:49 -0300 Subject: [PATCH] Model referenced in oneOf in path is ignored Fixes #31 --- lib/gen-type.ts | 10 ++- lib/service.ts | 6 +- templates/requestBuilder.handlebars | 2 +- test/all-types.json | 120 ++++++++++++++++++++++++-- test/all-types.spec.ts | 128 +++++++++++++++++++++++++--- 5 files changed, 242 insertions(+), 24 deletions(-) diff --git a/lib/gen-type.ts b/lib/gen-type.ts index ed59ffe..29eb60a 100644 --- a/lib/gen-type.ts +++ b/lib/gen-type.ts @@ -41,7 +41,7 @@ export abstract class GenType { this.additionalDependencies = [...this._additionalDependencies]; } - protected collectImports(schema: SchemaObject | ReferenceObject | undefined, additional?: true): void { + protected collectImports(schema: SchemaObject | ReferenceObject | undefined, additional = false, processOneOf = false): void { if (!schema) { return; } else if (schema.$ref) { @@ -55,12 +55,18 @@ export abstract class GenType { schema = schema as SchemaObject; (schema.allOf || []).forEach(i => this.collectImports(i, additional)); (schema.anyOf || []).forEach(i => this.collectImports(i, additional)); + if (processOneOf) { + (schema.oneOf || []).forEach(i => this.collectImports(i, additional)); + } if (schema.items) { this.collectImports(schema.items, additional); } if (schema.properties) { const properties = schema.properties; - Object.keys(properties).forEach(p => this.collectImports(properties[p], additional)); + Object.keys(properties).forEach(p => { + const prop = properties[p]; + this.collectImports(prop, additional, true); + }); } if (typeof schema.additionalProperties === 'object') { this.collectImports(schema.additionalProperties, additional); diff --git a/lib/service.ts b/lib/service.ts index a7e8e2d..2bc3bd7 100644 --- a/lib/service.ts +++ b/lib/service.ts @@ -25,7 +25,7 @@ export class Service extends GenType { // Collect the imports for (const operation of operations) { for (const parameter of operation.parameters) { - this.collectImports(parameter.spec.schema); + this.collectImports(parameter.spec.schema, false, true); } for (const securityGroup of operation.security) { securityGroup.forEach(security => this.collectImports(security.spec.schema)); @@ -36,9 +36,9 @@ export class Service extends GenType { } } for (const response of operation.allResponses) { - const additional = response === operation.successResponse ? undefined : true; + const additional = response !== operation.successResponse; for (const content of response.content) { - this.collectImports(content.spec.schema, additional); + this.collectImports(content.spec.schema, additional, true); } } } diff --git a/templates/requestBuilder.handlebars b/templates/requestBuilder.handlebars index 71ed0c3..9fb0820 100644 --- a/templates/requestBuilder.handlebars +++ b/templates/requestBuilder.handlebars @@ -165,7 +165,7 @@ export class {{ requestBuilderClass }} { const value = this._header.get(param); if (value instanceof Array) { for (const item of value) { - httpHeaders = httpHeaders.append(param, (item !== undefined && item !== null ? item || '').toString()); + httpHeaders = httpHeaders.append(param, (item !== undefined && item !== null ? item : '').toString()); } } else { httpHeaders = httpHeaders.set(param, (value !== undefined && value !== null ? value : '').toString()); diff --git a/test/all-types.json b/test/all-types.json index cdf3a52..bfc15fd 100644 --- a/test/all-types.json +++ b/test/all-types.json @@ -5,7 +5,7 @@ "version": "1.0" }, "paths": { - "/test": { + "/foo": { "get": { "responses": { "200": { @@ -19,6 +19,66 @@ } } } + }, + "/bar": { + "parameters": [ + { + "name": "param", + "in": "query", + "required": false, + "schema": { + "nullable": true, + "oneOf": [ + { + "$ref": "#/components/schemas/ReferencedInParamOneOf1" + }, + { + "$ref": "#/components/schemas/ReferencedInParamOneOf2" + } + ] + } + } + ], + "get": { + "responses": { + "200": { + "content": { + "application/json": { + "schema": { + "nullable": true, + "oneOf": [ + { + "$ref": "#/components/schemas/ReferencedInServiceOneOf1" + }, + { + "$ref": "#/components/schemas/ReferencedInServiceOneOf2" + } + ] + } + } + } + } + } + } + }, + "/baz": { + "get": { + "responses": { + "200": { + "content": { + "application/json": { + "schema": { + "oneOf": [ + { + "$ref": "#/components/schemas/Disjunct" + } + ] + } + } + } + } + } + } } }, "components": { @@ -70,8 +130,13 @@ "Disjunct": { "type": "object", "properties": { - "id": { - "type": "string" + "ref": { + "nullable": true, + "oneOf": [ + { + "$ref": "#/components/schemas/ReferencedInNullableOneOf" + } + ] } }, "oneOf": [ @@ -94,6 +159,46 @@ } } }, + "ReferencedInNullableOneOf": { + "type": "object", + "properties": { + "name": { + "type": "string" + } + } + }, + "ReferencedInServiceOneOf1": { + "type": "object", + "properties": { + "name": { + "type": "string" + } + } + }, + "ReferencedInServiceOneOf2": { + "type": "object", + "properties": { + "name": { + "type": "string" + } + } + }, + "ReferencedInParamOneOf1": { + "type": "object", + "properties": { + "name": { + "type": "string" + } + } + }, + "ReferencedInParamOneOf2": { + "type": "object", + "properties": { + "name": { + "type": "string" + } + } + }, "Containers": { "type": "array", "items": { @@ -210,6 +315,12 @@ "description": "Property of type array of any type", "items": {} }, + "dynamic": { + "type": "object", + "additionalProperties": { + "$ref": "#/components/schemas/OtherObject" + } + }, "nestedObject": { "type": "object", "properties": { @@ -245,9 +356,6 @@ } } } - }, - "additionalProperties": { - "$ref": "#/components/schemas/OtherObject" } } ] diff --git a/test/all-types.spec.ts b/test/all-types.spec.ts index cb1cfb5..c37c48a 100644 --- a/test/all-types.spec.ts +++ b/test/all-types.spec.ts @@ -1,13 +1,34 @@ import { OpenAPIObject } from '@loopback/openapi-v3-types'; -import { InterfaceDeclaration, TypeAliasDeclaration, TypescriptParser, EnumDeclaration } from 'typescript-parser'; +import { ClassDeclaration, EnumDeclaration, InterfaceDeclaration, TypeAliasDeclaration, TypescriptParser } from 'typescript-parser'; import { NgOpenApiGen } from '../lib/ng-openapi-gen'; +import { Options } from '../lib/options'; import options from './all-types.config.json'; import allTypesSpec from './all-types.json'; -import { Options } from '../lib/options'; const gen = new NgOpenApiGen(allTypesSpec as OpenAPIObject, options as Options); gen.generate(); +it('Api', done => { + const api = gen.services.get('ApiService'); + expect(api).toBeDefined(); + if (api) { + const ts = gen.templates.apply('service', api); + const parser = new TypescriptParser(); + parser.parseSource(ts).then(ast => { + expect(ast.declarations.length).toBe(1); + expect(ast.declarations[0]).toEqual(jasmine.any(ClassDeclaration)); + const cls = ast.declarations[0] as ClassDeclaration; + expect(cls.methods.length).toEqual(3 * 2); // foo, bar, baz, in 2 variants each + // Should have imported referenced-in-service-one-of-1/2 + expect(ast.imports.find(i => i.libraryName === '../models/referenced-in-service-one-of-1')).withContext('ref1 import').toBeDefined(); + expect(ast.imports.find(i => i.libraryName === '../models/referenced-in-service-one-of-2')).withContext('ref2 import').toBeDefined(); + // But not referenced-in-one-of, as it is nested within an object schema + expect(ast.imports.find(i => i.libraryName === '../models/referenced-in-one-of')).withContext('ref import').toBeUndefined(); + done(); + }); + } +}); + describe('Generation tests using all-types.json', () => { it('RefEnum model', done => { const refString = gen.models.get('RefEnum'); @@ -84,19 +105,19 @@ describe('Generation tests using all-types.json', () => { const ts = gen.templates.apply('model', disjunct); const parser = new TypescriptParser(); parser.parseSource(ts).then(ast => { - expect(ast.imports.length).toBe(0); + expect(ast.imports.length).toBe(1); + expect(ast.imports.find(i => i.libraryName === './referenced-in-nullable-one-of')).withContext('referenced-in-nullable-one-of import').toBeDefined(); expect(ast.declarations.length).toBe(1); expect(ast.declarations[0]).toEqual(jasmine.any(InterfaceDeclaration)); const decl = ast.declarations[0] as InterfaceDeclaration; expect(decl.name).toBe('Disjunct'); expect(decl.properties.length).toBe(1); - expect(decl.properties[0].name).toBe('id'); - expect(decl.properties[0].type).toBe('string'); + expect(decl.properties[0].name).toBe('ref'); + expect(decl.properties[0].type).toBe('null | ReferencedInNullableOneOf'); done(); }); }); - it('ReferencedInOneOf model', done => { const ref = gen.models.get('ReferencedInOneOf'); const ts = gen.templates.apply('model', ref); @@ -114,6 +135,91 @@ describe('Generation tests using all-types.json', () => { }); }); + it('ReferencedInNullableOneOf model', done => { + const ref = gen.models.get('ReferencedInNullableOneOf'); + const ts = gen.templates.apply('model', ref); + const parser = new TypescriptParser(); + parser.parseSource(ts).then(ast => { + expect(ast.imports.length).toBe(0); + expect(ast.declarations.length).toBe(1); + expect(ast.declarations[0]).toEqual(jasmine.any(InterfaceDeclaration)); + const decl = ast.declarations[0] as InterfaceDeclaration; + expect(decl.name).toBe('ReferencedInNullableOneOf'); + expect(decl.properties.length).toBe(1); + expect(decl.properties[0].name).toBe('name'); + expect(decl.properties[0].type).toBe('string'); + done(); + }); + }); + + it('ReferencedInServiceOneOf1 model', done => { + const ref = gen.models.get('ReferencedInServiceOneOf1'); + const ts = gen.templates.apply('model', ref); + const parser = new TypescriptParser(); + parser.parseSource(ts).then(ast => { + expect(ast.imports.length).toBe(0); + expect(ast.declarations.length).toBe(1); + expect(ast.declarations[0]).toEqual(jasmine.any(InterfaceDeclaration)); + const decl = ast.declarations[0] as InterfaceDeclaration; + expect(decl.name).toBe('ReferencedInServiceOneOf1'); + expect(decl.properties.length).toBe(1); + expect(decl.properties[0].name).toBe('name'); + expect(decl.properties[0].type).toBe('string'); + done(); + }); + }); + + it('ReferencedInServiceOneOf2 model', done => { + const ref = gen.models.get('ReferencedInServiceOneOf2'); + const ts = gen.templates.apply('model', ref); + const parser = new TypescriptParser(); + parser.parseSource(ts).then(ast => { + expect(ast.imports.length).toBe(0); + expect(ast.declarations.length).toBe(1); + expect(ast.declarations[0]).toEqual(jasmine.any(InterfaceDeclaration)); + const decl = ast.declarations[0] as InterfaceDeclaration; + expect(decl.name).toBe('ReferencedInServiceOneOf2'); + expect(decl.properties.length).toBe(1); + expect(decl.properties[0].name).toBe('name'); + expect(decl.properties[0].type).toBe('string'); + done(); + }); + }); + + it('ReferencedInParamOneOf1 model', done => { + const ref = gen.models.get('ReferencedInParamOneOf1'); + const ts = gen.templates.apply('model', ref); + const parser = new TypescriptParser(); + parser.parseSource(ts).then(ast => { + expect(ast.imports.length).toBe(0); + expect(ast.declarations.length).toBe(1); + expect(ast.declarations[0]).toEqual(jasmine.any(InterfaceDeclaration)); + const decl = ast.declarations[0] as InterfaceDeclaration; + expect(decl.name).toBe('ReferencedInParamOneOf1'); + expect(decl.properties.length).toBe(1); + expect(decl.properties[0].name).toBe('name'); + expect(decl.properties[0].type).toBe('string'); + done(); + }); + }); + + it('ReferencedInParamOneOf2 model', done => { + const ref = gen.models.get('ReferencedInParamOneOf2'); + const ts = gen.templates.apply('model', ref); + const parser = new TypescriptParser(); + parser.parseSource(ts).then(ast => { + expect(ast.imports.length).toBe(0); + expect(ast.declarations.length).toBe(1); + expect(ast.declarations[0]).toEqual(jasmine.any(InterfaceDeclaration)); + const decl = ast.declarations[0] as InterfaceDeclaration; + expect(decl.name).toBe('ReferencedInParamOneOf2'); + expect(decl.properties.length).toBe(1); + expect(decl.properties[0].name).toBe('name'); + expect(decl.properties[0].type).toBe('string'); + done(); + }); + }); + it('Container model', done => { const container = gen.models.get('Container'); const ts = gen.templates.apply('model', container); @@ -122,13 +228,14 @@ describe('Generation tests using all-types.json', () => { expect(ast.imports.length).toBe(5); expect(ast.imports.find(i => i.libraryName === './ref-enum')).withContext('ref-enum import').toBeDefined(); expect(ast.imports.find(i => i.libraryName === './ref-object')).withContext('ref-object import').toBeDefined(); - expect(ast.imports.find(i => i.libraryName === './other-object')).withContext('other-object import').toBeDefined(); expect(ast.imports.find(i => i.libraryName === './union')).withContext('union import').toBeDefined(); + expect(ast.imports.find(i => i.libraryName === './disjunct')).withContext('disjunct import').toBeDefined(); + expect(ast.imports.find(i => i.libraryName === './other-object')).withContext('other-object import').toBeDefined(); expect(ast.declarations.length).toBe(1); expect(ast.declarations[0]).toEqual(jasmine.any(InterfaceDeclaration)); const decl = ast.declarations[0] as InterfaceDeclaration; expect(decl.name).toBe('Container'); - expect(decl.properties.length).toBe(18); + expect(decl.properties.length).toBe(19); // Assert the simple types function assertProperty(name: string, type: string, required = false) { @@ -158,10 +265,7 @@ describe('Generation tests using all-types.json', () => { assertProperty('arrayOfAnyProp', 'Array'); assertProperty('nestedObject', '{ \'p1\': string, \'p2\': number, ' + '\'deeper\': { \'d1\': RefObject, \'d2\': string | Array | number } }'); - - // There's no support for additional properties in typescript-parser. Check as text. - const text = ts.substring(decl.start || 0, decl.end || ts.length); - expect(text).toContain('[key: string]: OtherObject'); + assertProperty('dynamic', '{ [key: string]: OtherObject }'); done(); });