Repository navigation
Conversation
…nRender MarkdownRender listed changed operations without any detail when the only change was in the security scheme definitions (e.g. OAuth2 flow authorizationUrl/tokenUrl), and the final result still reported a breaking change, which made the output misleading. Add a 'Security Requirements' section to changed operations that renders added/deleted requirements and, for changed ones, the scheme-level changes (type, in, scheme, bearerFormat, openIdConnectUrl, scopes, description) and OAuth flow URL changes (authorizationUrl/tokenUrl/refreshUrl old -> new). The compatibility verdict is untouched: it keeps following the diff model, where OAuth2/OpenID Connect URL changes are configurable via incompatible.security.scheme.oauth2.*.url.changed properties.
There was a problem hiding this comment.
All reported issues were addressed across 4 files
Reply with feedback, questions, or to request a fix.
View guided diff | Re-trigger cubic
…d schemes in Markdown - render each security requirement's scopes (scheme: [scope, ...]) instead of bare scheme names, so OAuth2: [read] is distinguishable from OAuth2: [] - render increased/missing schemes of a changed security requirement (New/Deleted security scheme) in the Markdown details - add a blank line between the changed-requirement heading and the nested scheme bullets so they do not merge into one paragraph - extend issue-893 fixtures with a multi-scheme requirement covering added/deleted requirements, unmatched schemes and changed scopes
There was a problem hiding this comment.
All reported issues were addressed across 4 files (changes from recent commits).
Reply with feedback, questions, or to request a fix.
View guided diff | Re-trigger cubic
Per the OpenAPI security-requirement rules only oauth2 and openIdConnect schemes may list scope names, so the HTTP basic requirement now uses an empty array (BasicAuth: []) and the apiKey requirement likewise (NewApiKey: []). The OAuth2 authorizationCode flow declares the read and write scopes its requirements reference, keeping named-scope rendering covered. Expected Markdown assertions updated to the corrected fixtures. The diff model already treats increased OAuth2 scopes as backward incompatible by default (incompatible.security.scheme.scopes.increased), so the compatibility assertions follow the model unchanged.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #893
Problem
When the only change between two specs is inside a
securitySchemesdefinition (e.g. OAuth2 flowauthorizationUrl/tokenUrl), the diff model correctly attachesChangedSecurityRequirementsto the affected operation, butMarkdownRenderonly renders Operation ID / Parameters / Request / Return Type sections. The result is a changed-operation header with no explanation, followed by "API changes broke backward compatibility" (as diagnosed by @DrSatyr in the issue).Root cause
MarkdownRender.listEndpoints(...)never rendersChangedOperation#getSecurityRequirements()— the model has the data, the renderer drops it.What changed
MarkdownRendernow renders a Security Requirements section for changed operations whenoperation.resultSecurityRequirements().isDifferent():type,in,scheme,bearerFormat,openIdConnectUrl, scopes, description) and OAuth flow changes (authorizationUrl/tokenUrl/refreshUrl, old → new).Repro (specs from the issue,
''→'some_value'forauthorizationUrl/tokenUrl)Before:
After:
Breaking-ness decision
No model changes: the compatibility verdict keeps following the diff model.
ChangedOAuthFlowmarks OAuth2 URL changes as incompatible only when theincompatible.security.scheme.oauth2.auth.url.changed/...token.url.changed/...refresh.url.changedproperties are enabled, and they are enabled by default by design since #552 (covered byOAuthFlowBCTest). So by default the output still reports the change as breaking — but now it shows why. When a user disables those properties, the same change renders as "API changes are backward compatible" while still listing the URL changes. This is covered by the new tests.Renderers not touched
AsciidocRenderandConsoleRenderhave the same gap (no security section at all);JsonRenderserializes the whole diff model and is unaffected. I deliberately did not refactor those in this PR — happy to follow up if desired.Tests
New fixtures
issue-893-oauth-1.yaml/issue-893-oauth-2.yaml(minimal version of the issue's specs, differing only in the OAuth2 flow URLs) and two tests inMarkdownRenderTest:issue893_renderChangedSecuritySchemeDetailsInMarkdown— default config: security section rendered with the actual URL changes; result follows the model (incompatible).issue893_securitySchemeUrlChangeIsCompatibleWhenConfigDisabled— with the twoincompatible.security.scheme.oauth2.*properties disabled: diff is compatible, output says "API changes are backward compatible" and still renders the URL changes.mvn -pl core test: 272 tests, 0 failures.Summary by cubic
Fixes the Markdown renderer so changed security requirements (for example, OAuth2 flow URL updates) are now explained under a Security Requirements section, instead of showing only a changed-operation header with no detail.
openIdConnectUrl, scopes, description), and OAuth flow URLs.Written for commit 113fe7f. Summary will update on new commits.