From f0da3c52738371de6db8a3ad3fb6d18833f577dd Mon Sep 17 00:00:00 2001 From: Dave Transom Date: Thu, 17 Nov 2022 10:44:57 +1300 Subject: [PATCH] Use refSiblings: 'preserve' when converting `swagger` As per https://github.com/Redocly/redoc/issues/2067, `swagger2openapi` needs `refSiblings: 'preserve'` to bring additional fields into the converted open-api object. This makes the description field available to the `fieldSchema` objects, but it also needs to be merged into the field model to be shown in the UI. --- .../2.0/complexFieldDescriptions.json | 65 +++++++++++++++++++ .../__tests__/models/FieldModel.test.ts | 50 ++++++++++++++ src/services/models/Field.ts | 42 ++++++++++++ src/utils/loadAndBundleSpec.ts | 2 +- 4 files changed, 158 insertions(+), 1 deletion(-) create mode 100644 src/services/__tests__/fixtures/2.0/complexFieldDescriptions.json diff --git a/src/services/__tests__/fixtures/2.0/complexFieldDescriptions.json b/src/services/__tests__/fixtures/2.0/complexFieldDescriptions.json new file mode 100644 index 0000000000..36e41750c5 --- /dev/null +++ b/src/services/__tests__/fixtures/2.0/complexFieldDescriptions.json @@ -0,0 +1,65 @@ +{ + "swagger": "2.0", + "info": { + "title": "AA", + "version": "1.0" + }, + "paths": { + "/test": { + "get": { + "operationId": "test", + "responses": { + "200": { + "description": "OK", + "schema": { + "$ref": "#/components/schemas/Box" + } + } + } + } + } + }, + "definitions": { + "Measurement": { + "type": "object", + "description": "Reusable measurement type.", + "properties": { + "value": { + "format": "double", + "description": "The numeric value of the measurement.", + "type": "number" + }, + "uom": { + "description": "The unit-of-measurement. \r\ne.g. kg, m³", + "type": "string" + } + } + }, + "Box": { + "type": "object", + "description": "Model which contains a reused type as a field e.g. a box with dimensions.", + "properties": { + "weight": { + "$ref": "#/definitions/Measurement", + "description": "The gross weight of the box and its contents (in kg)." + }, + "volume": { + "$ref": "#/definitions/Measurement", + "description": "The volume of the box (in m3)." + }, + "height": { + "$ref": "#/definitions/Measurement", + "description": "The height of the box (in mm)." + }, + "width": { + "$ref": "#/definitions/Measurement", + "description": "The width of the box (in mm)." + }, + "depth": { + "$ref": "#/definitions/Measurement", + "description": "The depth of the box (in mm)." + } + } + } + } +} diff --git a/src/services/__tests__/models/FieldModel.test.ts b/src/services/__tests__/models/FieldModel.test.ts index 0fe3bb75cc..a5556960f4 100644 --- a/src/services/__tests__/models/FieldModel.test.ts +++ b/src/services/__tests__/models/FieldModel.test.ts @@ -1,6 +1,8 @@ import { FieldModel } from '../../models/Field'; +import { SchemaModel } from '../../models'; import { OpenAPIParser } from '../../OpenAPIParser'; import { RedocNormalizedOptions } from '../../RedocNormalizedOptions'; +import { convertSwagger2OpenAPI } from '../../../utils/loadAndBundleSpec'; const opts = new RedocNormalizedOptions({}); @@ -107,5 +109,53 @@ describe('Models', () => { expect(field.name).toEqual('Test-Header'); }); + + test('field uses field description for complex objects', async () => { + // eslint-disable-next-line @typescript-eslint/no-var-requires + const source = require('../fixtures/2.0/complexFieldDescriptions.json'); + + // check incoming source + expect(source.swagger).toEqual('2.0'); + + const spec = await convertSwagger2OpenAPI(source); + + // sanity check for swagger 2.0 => 3.0 + expect(spec?.openapi).toEqual('3.0.0'); + + const parser = new OpenAPIParser(spec, undefined, opts); + if (!spec.components?.schemas?.Box) { + throw Error('spec.components.schemas.Box is not a defined schema.'); + } + + const boxSchema = new SchemaModel( + parser, + spec.components.schemas.Box, + '#/components/schemas/Box', + opts, + ); + + if (!boxSchema.fields?.length) { + throw Error('No fields defined on the box schema.'); + } + + // expected on the measurement _type_ only. + const measurementSchemaDescription = 'Reusable measurement type.'; + + // expected on the weight _field_ only. + const weightField = boxSchema.fields[0]; + + expect(weightField.name).toBe('weight'); + expect(weightField.description).toBe('The gross weight of the box and its contents (in kg).'); + + expect(weightField.schema.type).toBe('object'); + expect(weightField.schema.title).toBe('Measurement'); + expect(weightField.schema.description).toBe(measurementSchemaDescription); + + // ensure all fields (they're all Measurements) don't inherit the schema's description. + for (const field of boxSchema.fields) { + expect(field.schema.title).toBe('Measurement'); + expect(field.description).not.toBe(measurementSchemaDescription); + } + }); }); }); diff --git a/src/services/models/Field.ts b/src/services/models/Field.ts index 537fdbeeba..3569bce097 100644 --- a/src/services/models/Field.ts +++ b/src/services/models/Field.ts @@ -84,6 +84,48 @@ export class FieldModel { this.schema = new SchemaModel(parser, fieldSchema || {}, pointer, options, false, refsStack); this.description = info.description === undefined ? this.schema.description || '' : info.description; + + /* + Merge the description on the field/property itself, with the description of the model/schema - this + helps where a model is reused more than once, and each field gives added context to its use. + + It requires `refSiblings: "preserve"` when parsing swagger using `convertSwagger2OpenAPI`. + + There is a similar test in `src\utils\loadAndBundleSpec.ts` called + "should override description from $ref of the referenced component, when sibling description exists" + which tests a similar behaviour from the `src\services\__tests__\fixtures\siblingRefDescription.json` file. + However, that test is for open-api 3.1.0, where is this applies to the process of converting swagger 2.0 + file to open-api. + */ + if (fieldSchema?.description) { + /* + 2 options here, either: + a) Use the `fieldSchema.description` verbatim if it's defined, or + b) Concatenate the field description with the schema description. + However, option b might be considered unintended behaviour. + + Should this be an option in `RedocNormalizedOptions`? + */ + + // option a) + this.description = fieldSchema.description; + + /* + // option b) + if (this.description.includes(fieldSchema.description)) { + // already found inside the current description, so avoid a duplication of content - no change. + } else if (!this.description || fieldSchema.description.includes(this.description)) { + // the current description already contains the fields description, so prefer the field version only. + this.description = fieldSchema.description; + } else { + // otherwise, concatenate them - either "\r\n\r\n" for a markdown paragraph, or " \r\n" for a + // markdown line break. Safest approach is just add a bit of whitespace, as we can't be sure of what + // other formatting might be in place in either description. + this.description = fieldSchema.description + '\r\n\r\n' + this.description; + } + */ + } + this.example = info.example || this.schema.example; if (info.examples !== undefined || this.schema.examples !== undefined) { diff --git a/src/utils/loadAndBundleSpec.ts b/src/utils/loadAndBundleSpec.ts index cc9ff6e364..0555c4a771 100644 --- a/src/utils/loadAndBundleSpec.ts +++ b/src/utils/loadAndBundleSpec.ts @@ -41,7 +41,7 @@ export async function loadAndBundleSpec(specUrlOrObject: object | string): Promi export function convertSwagger2OpenAPI(spec: any): Promise { console.warn('[ReDoc Compatibility mode]: Converting OpenAPI 2.0 to OpenAPI 3.0'); return new Promise((resolve, reject) => - convertObj(spec, { patch: true, warnOnly: true, text: '{}', anchors: true }, (err, res) => { + convertObj(spec, { patch: true, warnOnly: true, text: '{}', anchors: true, refSiblings: 'preserve' }, (err, res) => { // TODO: log any warnings if (err) { return reject(err);