Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
kaleocheng opened this issue Oct 21, 2020 · 2 comments
Open

Comments

@kaleocheng
Copy link

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
Copy link
Contributor

fabsrc commented Oct 24, 2020

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
Copy link
Author

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants