fix: clean up the discriminator mapping in the remove-x-internal deco…

…rator (#1694)

* fix: remove the discriminator mapping in the remove-x-internal decorator

* add a test for discriminators with $refs to files

* collect original discriminator mapping before resolving to compare $refs to the actual files

* add check for mapping to be an object

.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`.

__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

__tests__/bundle/remove-x-internal/redocly.yaml

@@ -0,0 +1,5 @@
+apis:
+  main:
+    root: ./main.yaml
+    decorators:
+      remove-x-internal: on

__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]

__tests__/bundle/remove-x-internal/schemas/cranberry.yaml

@@ -0,0 +1,5 @@
+type: object
+properties:
+  type:
+    type: string
+    enum: [cranberry]

__tests__/bundle/remove-x-internal/schemas/popcorn.yaml

@@ -0,0 +1,5 @@
+type: object
+properties:
+  type:
+    type: string
+    enum: [popcorn]

__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.
+
+`;

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
   ) {

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', () => {

packages/core/src/decorators/common/remove-x-internal.ts

@@ -6,39 +6,50 @@ 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--;
         }
       }
     } 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);
       },
     },
   };