Repository: Redocly/redocly-cli
Pull request #1694 (2024-08-29 08:02:38)

fix: clean up the discriminator mapping in the remove-x-internal decorator
## What/Why/How?

Fixed the `remove-x-internal` decorator, which was not removing the reference in the corresponding discriminator mapping while removing the original $ref.

## Reference

Fixes https://github.com/Redocly/redocly-cli/issues/1649

## Testing

## Screenshots (optional)

## Check yourself

- [ ] Code changed? - Tested with redoc/reference-docs/workflows (internal)
- [x] All new/updated code is covered with tests
- [ ] New package installed? - Tested in different environments (browser/node)

## Security

- [x] Security impact of change has been considered
- [x] Code follows company security practices and guidelines


changeset-bot[bot]: ###  🦋  Changeset detected

Latest commit: 9afd1554ca9aa024487beb81d24c86788a2627bf

**The changes in this PR will be included in the next version bump.**

<details><summary>This PR includes changesets to release 2 packages</summary>

  | Name                  | Type  |
| --------------------- | ----- |
| @redocly/openapi-core | Patch |
| @redocly/cli          | Patch |

</details>

Not sure what this means? [Click here  to learn what changesets are](https://github.com/changesets/changesets/blob/main/docs/adding-a-changeset.md).

[Click here if you're a maintainer who wants to add another changeset to this PR](https://github.com/Redocly/redocly-cli/new/fix/remove-x-internal-discriminator-mapping?filename=.changeset/grumpy-experts-allow.md&value=---%0A%22%40redocly%2Fopenapi-core%22%3A%20patch%0A---%0A%0Afix%3A%20remove%20the%20discriminator%20mapping%20in%20the%20remove-x-internal%20decorator%0A)


github-actions[bot]: | Command | Mean [s] | Min [s] | Max [s] | Relative |
|:---|---:|---:|---:|---:|
| `redocly lint packages/core/src/benchmark/benches/rebilly.yaml` | 1.007 ± 0.020 | 0.982 | 1.043 | 1.01 ± 0.02 |
| `redocly-next lint packages/core/src/benchmark/benches/rebilly.yaml` | 0.993 ± 0.013 | 0.974 | 1.015 | 1.00 |

<!-- thollander/actions-comment-pull-request "latest-vs-next-comparison" -->
github-actions[bot]: <!-- jest coverage report action for options with hash 3339ae53a38c44382c0fb4491bdb282d -->

## Coverage report 



| <div title="Status of coverage:&#10; 🟢 - ok&#10; 🟡 - slightly more than threshold&#10; 🔴 - under the threshold">St.<sup>:grey_question:</sup></div> | Category   | Percentage                                                     | Covered / Total |
| :----------------------------------------------------------------------------------------------------------------------------------------------------: | :--------- | :------------------------------------------------------------- | :-------------: |
|                                                                           🟡                                                                           | Statements | 77.75%                                                         |    4756/6117    |
|                                                                           🟡                                                                           | Branches   | <div title="Base coverage is: 65.11%">66.17% (+1.06% 🔼)</div> |    1981/2994    |
|                                                                           🟡                                                                           | Functions  | 72.06%                                                         |     784/1088    |
|                                                                           🟡                                                                           | Lines      | 77.96%                                                         |    4488/5757    |


## Test suite run success
766 tests passing in 108 suites.

<p align="right">Report generated by <a href="https://github.com/ArtiomTr/jest-coverage-report-action">🧪jest coverage report action</a> from 9afd1554ca9aa024487beb81d24c86788a2627bf</p>


Patch:
diff --git a/.changeset/seven-geckos-behave.md b/.changeset/seven-geckos-behave.md
new file mode 100644
index 00000000..d3bfc6a3
--- /dev/null
+++ b/.changeset/seven-geckos-behave.md
@@ -0,0 +1,6 @@
+---
+"@redocly/openapi-core": patch
+"@redocly/cli": patch
+---
+
+Fixed the `remove-x-internal` decorator, which was not removing the reference in the corresponding discriminator mapping while removing the original `$ref`.
diff --git a/packages/core/src/bundle.ts b/packages/core/src/bundle.ts
index 3df1e9df..4c2deaf2 100755
--- a/packages/core/src/bundle.ts
+++ b/packages/core/src/bundle.ts
@@ -446,7 +446,7 @@ function makeBundleVisitor(
 
   function saveComponent(
     componentType: string,
-    target: { node: any; location: Location },
+    target: { node: unknown; location: Location },
     ctx: UserContext
   ) {
     components[componentType] = components[componentType] || {};
@@ -464,8 +464,8 @@ function makeBundleVisitor(
   }
 
   function isEqualOrEqualRef(
-    node: any,
-    target: { node: any; location: Location },
+    node: unknown,
+    target: { node: unknown; location: Location },
     ctx: UserContext
   ) {
     if (
@@ -480,7 +480,7 @@ function makeBundleVisitor(
   }
 
   function getComponentName(
-    target: { node: any; location: Location },
+    target: { node: unknown; location: Location },
     componentType: string,
     ctx: UserContext
   ) {
diff --git a/packages/core/src/decorators/common/remove-x-internal.ts b/packages/core/src/decorators/common/remove-x-internal.ts
index 3974ceda..06d72d09 100644
--- a/packages/core/src/decorators/common/remove-x-internal.ts
+++ b/packages/core/src/decorators/common/remove-x-internal.ts
@@ -6,23 +6,35 @@ import type { UserContext } from '../../walk';
 
 const DEFAULT_INTERNAL_PROPERTY_NAME = 'x-internal';
 
-export const RemoveXInternal: Oas3Decorator | Oas2Decorator = ({ internalFlagProperty }) => {
-  const hiddenTag: string = internalFlagProperty || DEFAULT_INTERNAL_PROPERTY_NAME;
-
-  function removeInternal(node: any, ctx: UserContext) {
+export const RemoveXInternal: Oas3Decorator | Oas2Decorator = ({
+  internalFlagProperty = DEFAULT_INTERNAL_PROPERTY_NAME,
+}) => {
+  function removeInternal(
+    node: unknown,
+    ctx: UserContext,
+    originalMapping: Record<string, string>
+  ) {
     const { parent, key } = ctx;
     let didDelete = false;
     if (Array.isArray(node)) {
       for (let i = 0; i < node.length; i++) {
         if (isRef(node[i])) {
           const resolved = ctx.resolve(node[i]);
-          if (resolved.node?.[hiddenTag]) {
+          if (resolved.node?.[internalFlagProperty]) {
+            // First, remove the reference in the discriminator mapping, if it exists:
+            if (isPlainObject(parent.discriminator?.mapping)) {
+              for (const mapping in parent.discriminator.mapping) {
+                if (originalMapping?.[mapping] === node[i].$ref) {
+                  delete parent.discriminator.mapping[mapping];
+                }
+              }
+            }
             node.splice(i, 1);
             didDelete = true;
             i--;
           }
         }
-        if (node[i]?.[hiddenTag]) {
+        if (node[i]?.[internalFlagProperty]) {
           node.splice(i, 1);
           didDelete = true;
           i--;
@@ -30,15 +42,14 @@ export const RemoveXInternal: Oas3Decorator | Oas2Decorator = ({ internalFlagPro
       }
     } else if (isPlainObject(node)) {
       for (const key of Object.keys(node)) {
-        node = node as any;
         if (isRef(node[key])) {
-          const resolved = ctx.resolve<any>(node[key]);
-          if (resolved.node?.[hiddenTag]) {
+          const resolved = ctx.resolve(node[key]);
+          if (isPlainObject(resolved.node) && resolved.node?.[internalFlagProperty]) {
             delete node[key];
             didDelete = true;
           }
         }
-        if (node[key]?.[hiddenTag]) {
+        if (isPlainObject(node[key]) && node[key]?.[internalFlagProperty]) {
           delete node[key];
           didDelete = true;
         }
@@ -50,10 +61,16 @@ export const RemoveXInternal: Oas3Decorator | Oas2Decorator = ({ internalFlagPro
     }
   }
 
+  let originalMapping: Record<string, string> = {};
   return {
+    DiscriminatorMapping: {
+      enter: (mapping: Record<string, string>) => {
+        originalMapping = structuredClone(mapping);
+      },
+    },
     any: {
       enter: (node, ctx) => {
-        removeInternal(node, ctx);
+        removeInternal(node, ctx, originalMapping);
       },
     },
   };


Test patch:
diff --git a/__tests__/bundle/remove-x-internal/main.yaml b/__tests__/bundle/remove-x-internal/main.yaml
new file mode 100644
index 00000000..2b4cb06b
--- /dev/null
+++ b/__tests__/bundle/remove-x-internal/main.yaml
@@ -0,0 +1,25 @@
+openapi: 3.1.0
+info: {}
+paths:
+  /test:
+    post:
+      requestBody:
+        content:
+          application/json:
+            schema:
+              $ref: '#/components/schemas/christmas-tree'
+components:
+  schemas:
+    christmas-tree:
+      type: array
+      items:
+        discriminator:
+          propertyName: type
+          mapping:
+            candy-cane: schemas/candy-cane.yaml
+            popcorn: schemas/popcorn.yaml
+            cranberry: schemas/cranberry.yaml
+        anyOf:
+          - $ref: schemas/candy-cane.yaml
+          - $ref: schemas/popcorn.yaml
+          - $ref: schemas/cranberry.yaml
diff --git a/__tests__/bundle/remove-x-internal/redocly.yaml b/__tests__/bundle/remove-x-internal/redocly.yaml
new file mode 100644
index 00000000..ceaecf82
--- /dev/null
+++ b/__tests__/bundle/remove-x-internal/redocly.yaml
@@ -0,0 +1,5 @@
+apis:
+  main:
+    root: ./main.yaml
+    decorators:
+      remove-x-internal: on
diff --git a/__tests__/bundle/remove-x-internal/schemas/candy-cane.yaml b/__tests__/bundle/remove-x-internal/schemas/candy-cane.yaml
new file mode 100644
index 00000000..04b468f9
--- /dev/null
+++ b/__tests__/bundle/remove-x-internal/schemas/candy-cane.yaml
@@ -0,0 +1,7 @@
+x-internal: true
+title: candy-cane
+type: object
+properties:
+  type:
+    type: string
+    enum: [candy-cane]
diff --git a/__tests__/bundle/remove-x-internal/schemas/cranberry.yaml b/__tests__/bundle/remove-x-internal/schemas/cranberry.yaml
new file mode 100644
index 00000000..96556c84
--- /dev/null
+++ b/__tests__/bundle/remove-x-internal/schemas/cranberry.yaml
@@ -0,0 +1,5 @@
+type: object
+properties:
+  type:
+    type: string
+    enum: [cranberry]
diff --git a/__tests__/bundle/remove-x-internal/schemas/popcorn.yaml b/__tests__/bundle/remove-x-internal/schemas/popcorn.yaml
new file mode 100644
index 00000000..6b56263b
--- /dev/null
+++ b/__tests__/bundle/remove-x-internal/schemas/popcorn.yaml
@@ -0,0 +1,5 @@
+type: object
+properties:
+  type:
+    type: string
+    enum: [popcorn]
diff --git a/__tests__/bundle/remove-x-internal/snapshot.js b/__tests__/bundle/remove-x-internal/snapshot.js
new file mode 100644
index 00000000..3094c9f0
--- /dev/null
+++ b/__tests__/bundle/remove-x-internal/snapshot.js
@@ -0,0 +1,45 @@
+// Jest Snapshot v1, https://goo.gl/fbAQLP
+
+exports[`E2E bundle remove-x-internal 1`] = `
+openapi: 3.1.0
+info: {}
+paths:
+  /test:
+    post:
+      requestBody:
+        content:
+          application/json:
+            schema:
+              $ref: '#/components/schemas/christmas-tree'
+components:
+  schemas:
+    christmas-tree:
+      type: array
+      items:
+        discriminator:
+          propertyName: type
+          mapping:
+            popcorn: '#/components/schemas/popcorn'
+            cranberry: '#/components/schemas/cranberry'
+        anyOf:
+          - $ref: '#/components/schemas/popcorn'
+          - $ref: '#/components/schemas/cranberry'
+    popcorn:
+      type: object
+      properties:
+        type:
+          type: string
+          enum:
+            - popcorn
+    cranberry:
+      type: object
+      properties:
+        type:
+          type: string
+          enum:
+            - cranberry
+
+bundling ./main.yaml...
+📦 Created a bundle for ./main.yaml at stdout <test>ms.
+
+`;
diff --git a/packages/core/src/decorators/__tests__/remove-x-internal.test.ts b/packages/core/src/decorators/__tests__/remove-x-internal.test.ts
index 7035cd74..bbe11af8 100644
--- a/packages/core/src/decorators/__tests__/remove-x-internal.test.ts
+++ b/packages/core/src/decorators/__tests__/remove-x-internal.test.ts
@@ -271,6 +271,103 @@ describe('oas3 remove-x-internal', () => {
 
     `);
   });
+
+  it('should remove $refs and the corresponding discriminator mapping', async () => {
+    const testDoc = parseYamlToDocument(
+      outdent`
+        openapi: 3.1.0
+        info: {}
+        paths:
+          /test:
+            post:
+              requestBody:
+                content:
+                  application/json:
+                    schema:
+                      $ref: '#/components/schemas/christmas-tree'
+        components:
+          schemas:
+            christmas-tree:
+              type: array
+              items:
+                discriminator:
+                  propertyName: type
+                  mapping:
+                    candy-cane: '#/components/schemas/candy-cane'
+                    popcorn: '#/components/schemas/popcorn'
+                    cranberry: '#/components/schemas/cranberry'
+                anyOf:
+                  - $ref: '#/components/schemas/candy-cane'
+                  - $ref: '#/components/schemas/popcorn'
+                  - $ref: '#/components/schemas/cranberry'
+            candy-cane:
+              x-internal: true
+              title: candy-cane
+              type: object
+              properties:
+                type:
+                  type: string
+                  enum: [candy-cane]
+            popcorn:
+              type: object
+              properties:
+                type:
+                  type: string
+                  enum: [popcorn]
+            cranberry:
+              type: object
+              properties:
+                type:
+                  type: string
+                  enum: [cranberry]
+      `
+    );
+    const { bundle: res } = await bundleDocument({
+      document: testDoc,
+      externalRefResolver: new BaseResolver(),
+      config: await makeConfig({ rules: {}, decorators: { 'remove-x-internal': 'on' } }),
+    });
+    expect(res.parsed).toMatchInlineSnapshot(`
+      openapi: 3.1.0
+      info: {}
+      paths:
+        /test:
+          post:
+            requestBody:
+              content:
+                application/json:
+                  schema:
+                    $ref: '#/components/schemas/christmas-tree'
+      components:
+        schemas:
+          christmas-tree:
+            type: array
+            items:
+              discriminator:
+                propertyName: type
+                mapping:
+                  popcorn: '#/components/schemas/popcorn'
+                  cranberry: '#/components/schemas/cranberry'
+              anyOf:
+                - $ref: '#/components/schemas/popcorn'
+                - $ref: '#/components/schemas/cranberry'
+          popcorn:
+            type: object
+            properties:
+              type:
+                type: string
+                enum:
+                  - popcorn
+          cranberry:
+            type: object
+            properties:
+              type:
+                type: string
+                enum:
+                  - cranberry
+
+    `);
+  });
 });
 
 describe('oas2 remove-x-internal', () => {