maxdome / swagger-combine

Combines multiple Swagger schemas into one dereferenced schema.
MIT License
132 stars 36 forks source link

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

Open kaleocheng opened 3 years ago

kaleocheng commented 3 years ago

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

fabsrc commented 3 years ago

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.

kaleocheng commented 3 years ago

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