From f0da3c52738371de6db8a3ad3fb6d18833f577dd Mon Sep 17 00:00:00 2001
From: Dave Transom <dave.transom@gmail.com>
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<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);