Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Prevent loss of field descriptions when converting swagger specs by using refSiblings: 'preserve' #2207

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -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)."
}
}
}
}
}
50 changes: 50 additions & 0 deletions src/services/__tests__/models/FieldModel.test.ts
Original file line number Diff line number Diff line change
@@ -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({});

Expand Down Expand Up @@ -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);
}
});
});
});
42 changes: 42 additions & 0 deletions src/services/models/Field.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
2 changes: 1 addition & 1 deletion src/utils/loadAndBundleSpec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ export async function loadAndBundleSpec(specUrlOrObject: object | string): Promi
export function convertSwagger2OpenAPI(spec: any): Promise<OpenAPISpec> {
console.warn('[ReDoc Compatibility mode]: Converting OpenAPI 2.0 to OpenAPI 3.0');
return new Promise<OpenAPISpec>((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);
Expand Down