Skip to content

Fix: Operation security should override top-level#186

Merged
wolfadex merged 5 commits intowolfadex:mainfrom
adamdicarlo0:adam/paths-security-should-override-global-security
Mar 4, 2025
Merged

Fix: Operation security should override top-level#186
wolfadex merged 5 commits intowolfadex:mainfrom
adamdicarlo0:adam/paths-security-should-override-global-security

Conversation

@adamdicarlo0
Copy link
Copy Markdown
Contributor

@adamdicarlo0 adamdicarlo0 commented Feb 19, 2025

Requires wolfadex/elm-open-api#11 (CI currently fails due to this)

In order to allow schema override files to also override (replace) individual operations' security values, we must special-case the JsonValue merging logic on key == "security". This is needed because otherwise security: [] in an override will just be merged into, e.g., security: [{ Token: [] }] in the schema's operation, not changing anything.

This also affects how top-level security is handled; that may sound weird, but I think it's good/necessary, because it gives the most flexibility to override files. But it is a special case, whereas all other override data are merged with the OAS data.

@wolfadex @miniBill What do you think?

@adamdicarlo0 adamdicarlo0 marked this pull request as ready for review March 4, 2025 19:23
@adamdicarlo0
Copy link
Copy Markdown
Contributor Author

I'd suggest adding -N to the diff command: "Treat absent files as empty"

(The diff failed due to permissions or whatnot, but it looks like it was empty, anyway, in spite of new files being generated -- it is not showing new files, now.)

@wolfadex wolfadex merged commit 013c1c6 into wolfadex:main Mar 4, 2025
1 of 2 checks passed
@adamdicarlo0 adamdicarlo0 deleted the adam/paths-security-should-override-global-security branch March 4, 2025 21:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants