redocly-cli icon indicating copy to clipboard operation
redocly-cli copied to clipboard

The remove-x-internal decorator does not fully remove internal paths which contain a path-level parameter

Open bojanz opened this issue 1 year ago • 3 comments

Describe the bug

Right now if an x-internal path has a path-level parameter, the path does not get fully removed. If the path is defined on the operation level (under post), the entire path is removed, as expected.

This might be a regression, we've had such paths before, and have been using the CLI since the beta days to remove them. However, I can't be certain.

To Reproduce

openapi.yaml:

paths:
  '/products/{product_id}/clowncopterize':
    parameters:
      - $ref: '#/components/parameters/ProductID'
    post:
      summary: Clowncopterize product
      description: Clowncopterizes the given product.
      operationId: product-clowncopterize
      x-internal: true
      tags:
        - Products
      responses:
        '204':
          description: OK

Result (redocly bundle openapi.yaml):

paths:
  /products/{product_id}/clowncopterize:
    parameters:
      - $ref: '#/components/parameters/ProductID'

My example uses $ref, but the same behavior is present if the parameter is defined directly.

Repo with full example: https://github.com/bojanz/cli-bug

Expected behavior

The entire path is removed.

Redocly Version(s)

1.19.0

Node.js Version(s)

v20.12.2

bojanz avatar Aug 20 '24 10:08 bojanz

Hi @bojanz! I'm not sure it's a bug as the decorator removes exactly what you've told it to remove. Technically, x-internal could be placed on any node, so it'd be quite challenging to figure out the context and clean up it appropriately. In your case I'd recommend using a custom decorator in addition to the existing one to remove the empty path items. You'll need to create a plugin for that:

my-plugin.js:

module.exports = {
  id: 'test',
  decorators: {
    oas3: {
      'cleanup-paths': () => ({
        Paths: {
          leave: (paths) => {
            for (const pathName in paths) {
              console.log(paths[pathName]);
              // Check if we can remove the path
              if (/* the path is empty... */) {
                delete paths[pathName];
              }
            }
          },
        },
      }),
    },
  },
};

Then use it in redocly.yaml:

plugins:
  - ./my-plugin.js # <- reference the filepath to your plugin here
decorators:
  remove-x-internal: on
  test/cleanup-paths: on # <- use the decorator

Please let me know if that helps.

tatomyr avatar Aug 21 '24 06:08 tatomyr

I totally get the argument for keeping the remove-x-internal decorator simple, since it currently doesn't care about which level it is operating on.

Perhaps it would make sense to add a separate decorator that removes paths with no operations? The two could then be combined to get the result expected by some users.

bojanz avatar Aug 21 '24 08:08 bojanz

I’d like to add one, but I don’t have the bandwidth at the moment. If you'd like, you can try adding the decorator yourself here, or contribute to our Cookbook.

tatomyr avatar Aug 21 '24 16:08 tatomyr

This is the decorator that I ended up using:

const id = "internal";
const httpVerbs = ['get', 'head', 'post', 'put', 'patch', 'delete', 'options', 'trace'];

/** @type {import('@redocly/cli').CustomRulesConfig} */
const decorators = {
  oas3: {
    "remove-empty-path": () => ({
      Paths: {
        leave: (paths) => {
          for (const pathName in paths) {
            const foundVerbs = Object.keys(paths[pathName]).filter((k) => httpVerbs.includes(k));
            if (foundVerbs.length == 0) {
              delete paths[pathName];
            }
          }
        },
      },
    }),
  },
};

module.exports = {
  id,
  decorators,
};

bojanz avatar Oct 10 '24 09:10 bojanz