swagger-combine icon indicating copy to clipboard operation
swagger-combine copied to clipboard

Prioritize exclude over include instead of 'if include else exclude'

Open kaleocheng opened this issue 5 years ago • 2 comments

the current strategy ignores exclude if include exists: https://github.com/maxdome/swagger-combine/blob/master/src/SwaggerCombine.js#L87-L99

It may be confusing if someone has the configuration like this: (include all paths under /api/products excpet for /api/products/{id}/recommendation)

{
  "openapi": "3.0.0",
   .....
  "apis": [
    {
      "url": "./docs/api.yaml",
      "paths": {
        "exclude": [
          "^/api/products/{id}/recommendation"
        ],
        "include": [
          "^/api/products(/.*|)$"
        ]
      }
    }
   ]
}

To me it makes more sense to use

if (include.paths) {
....
}

if (exclude.paths) {
....
}

instead of if include else if exclude

kaleocheng avatar Oct 21 '20 13:10 kaleocheng

I think this was implemented when includes/excludes with regular expressions were not possible yet. Back then this implementation made sense, because you would either include certain paths or exclude them.

Could you create a pull request for this? If you do, please make sure to add some additional tests and update documentation accordingly.

Probably makes sense to put this into a new major version of the package then, as it is a breaking change.

fabsrc avatar Oct 24 '20 16:10 fabsrc

I'm kind of busy recently but sure, I can create a PR later. 😄

kaleocheng avatar Oct 26 '20 06:10 kaleocheng