Repository navigation
feat(ir)!: mark inbound operations and give a path its own docs - #1366
Open
OmarAlJarrah wants to merge 10 commits into
Open
OmarAlJarrah wants to merge 10 commits into
OmarAlJarrah wants to merge 10 commits into
Conversation
A callback operation sat beside client operations with nothing saying the API sends it: only a reverse lookup through its declaring operation's Callbacks showed it, and the callback's name lived only inside an operation ID. A webhook inherited the service's security requirement, the client's API key, though the API sends it. And a path item's summary and description had no field, so they were kept under Unmodeled on each operation with an info report. - HTTPBinding.IsWebhook becomes Inbound, a closed webhook | callback, empty for an operation the client sends. The OpenAPI compiler marks callback operations, and Callback gains Name, the callbacks-map key. irverify reports an undeclared kind (ir/unknown-http-inbound) and a callback whose two halves disagree (ir/callback-unlisted, ir/callback-unmarked). - An inbound operation inherits no requirement: its Auth is what its own declaration states, read as how the API authenticates to the receiver (docs/ir/auth.md#inbound-operations). - HTTPBinding gains PathDocs, the path's own documentation, written on every operation the path mounts as URITemplate is. The openapi:pathItemSummary and openapi:pathItemDescription keys and their report are retired. - A callback's correlation key now uses its name and method, so a rewritten runtime expression keeps it. BREAKING CHANGE: IRVersion moves to 0.29.0. A consumer pinned to 0.28.0 refuses the document, finds no isWebhook, and no longer finds the path-item keys. One that read a webhook with no Auth as inheriting now reads it as stating none.
3 of 4 tasks
…at/ir-inbound-operations
…at/ir-inbound-operations
checkCallbacks walked each operation's bindings inline, past the cognitive-complexity cap, behind a type check its own type assertion already makes, which left a branch no walk can take. The walk now asserts the type once and hands the operation to noteCallbacks.
17 tasks done
…at/ir-inbound-operations # Conflicts: # compilers/openapi/e2e/conformance_pathitems_test.go # compilers/openapi/e2e/conformance_unmodeled_test.go # compilers/openapi/internal/operation/pathitem.go # compilers/openapi/internal/operation/pathitem_internal_test.go # compilers/openapi/internal/operation/pathitem_test.go # compilers/openapi/internal/operation/pathitemref.go # compilers/openapi/internal/operation/pathitemref_collisions_test.go # compilers/openapi/internal/operation/pathitemref_fields_test.go # compilers/openapi/internal/operation/pathitemref_test.go # docs/compilers/openapi.md # ir/document.go # testdata/conformance/openapi/path-item-docs.golden.json # testdata/conformance/openapi/path-item-ref-siblings.golden.json # testdata/conformance/openapi/yaml-merge-keys.golden.json
…at/ir-inbound-operations
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.
Summary
Three facts about where an operation sits and who sends it had no field.
isWebhook: false. Only a reverse lookup through its declaring operation'sCallbacksshowed that the API sends it, and the callback's name lived only inside an operation ID. Every consumer reads an inbound operation the other way round: read-only and write-only projections flip, and so does a type's usage.HTTPBinding.IsWebhookbecomesInbound, a closedwebhook | callback, empty for an operation the client sends. The OpenAPI compiler marks every callback operation, andCallbackgainsName, the callbacks-map key.irverifyholds both halves of a callback to agree: a marked operation nobody lists (ir/callback-unlisted) and a listed one that is not marked (ir/callback-unmarked) are each reported, and so is an undeclared kind (ir/unknown-http-inbound).docs/ir/http-binding.md#inbound-operationsreplaces the webhook section.securityinherited the service's requirement, the client's API key, though the API sends the request and the receiver never sees that key.docs/ir/auth.md#inbound-operationsdecides it: an inbound operation'sAuthis what its own declaration states, read as how the API authenticates to the receiver, and nil states none. Both alternatives are weighed there. Inheriting matches every other operation, and is wrong for the one direction the requirement does not cover. Refusing an inbound operation anyAuthdrops what a document says about how deliveries are signed. No lowering changes, since the compiler already wrote nil; the meaning of that nil does.HTTPBinding.PathDocsholds a Path Item'ssummaryanddescription. It is written on every operation the path mounts, asURITemplateis, and read layer by layer so a$refsite's text stands in front of its target's. Theopenapi:pathItem/summaryandopenapi:pathItem/descriptionentries an operation held, with their info report, are retired; a path item mounting no operation still keeps its pair verbatim on the nearest node, as it keeps everything else it writes. The issue weighed three homes. A secondDocson the operation would sit empty in every format without paths. A path entity would make every route a node to hold two strings. Declining would leave the text under a key named for one format. The binding is where the route already lives, and a format that documents its grouping construct writesOperationGroup.DocsorChannel.Docsinstead.The golden spec has no webhooks, no callbacks and no path-item documentation (spec @319dc756), so its IR changes only in
isWebhookbecoming absent. That is the rule-3 ranking all three issues carry.Closes #625
Closes #626
Closes #1319
Breaking
IRVersionmoves to 0.32.0, after #1361's 0.31.0 onmain's 0.30.0. Earlier commits name 0.29.0 and 0.30.0, from before #1333 and #1327 landed; the merges renumbered. A consumer pinned to the generation before this one refuses the document. It also finds noisWebhook, and noopenapi:pathItem/summaryoropenapi:pathItem/descriptionentry on an operation, and one that read a webhook with noAuthas inheriting now reads it as stating none.Stack
Base: #1361, on #1355, #1354, #1351 and the emitter-design stack. Merge in order.
#1322 and #1327 landed on
mainwhile this was open. Its merge of them keeps their structure: a mount computes what its path item keeps once and copies it onto each operation, and the path's documentation is computed with it and written into each operation'sPathDocs. An unmounted item keeps its pair as #1322 does, soprecedencetakes nodocsflag.Test plan
go testonir/...,compilers/...,cmd/...,engine/...,pass/...andinternal/.... The goldens, shape pin and census were regenerated.irverify.TestVerify_UndeclaredInboundKindIsAViolation, andTestVerify_ACallbacksTwoHalvesAgree(six rows, a dangling listing included).TestVerify_WebhookRouteIsInTheFieldItsFlagSelectsgains the callback rows.operation.TestCallbacks_NameWithSlashKeepsItsOwnKeynow asserts each callback's name, listing and mark.operation.TestPathItem_DocsKeptOnEveryRoute,TestPathItem_DocsAddNoMarkerandTestApplyPathItemDocs_WritesTheBindingItIsGiven.$ref-sibling, collision, chain and YAML-merge cases now readPathDocs.cedesfor the summary is an equivalent mutant: layers apply backward, so the use site writes last and wins anyway. The check stays because it states the precedence rather than leaning on that order.TestVerify_ACleanComponentCostsNothingfirst caught the callback check spelling a path for every operation. It now spells one only for a marked operation and a listing entry.skills_lintclean.