Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 29 additions & 0 deletions compilers/openapi/internal/lowering/lowering.go
Original file line number Diff line number Diff line change
Expand Up @@ -212,6 +212,35 @@ func (c Ctx) NamingByReference() Ctx {
// declaration replaces. See NamingByReference.
func (c Ctx) NamesByReference() bool { return c.namesByReference }

// NamingByReferenceAt is NamingByReference for a reference-or-declaration
// position: it marks c when declPtr is not the position usePtr addressed, and
// leaves it alone when the two agree.
//
// The two pointers agree exactly when the construct is declared where it is
// used. They differ when a $ref carried this lowering into a declaration some
// other position owns — a request body written under another operation, a
// header written under another response — and that is the case
// NamingByReference describes: the hint this lowering derives is a use-site
// name (an operationId, a headers-map key) for a node the use site does not own.
//
// It exists because $ref is resolved at these positions before the lowering
// starts, so the marking hoistSubSchema does for a schema-level $ref has no
// counterpart here: by the time a body or header is lowered, nothing downstream
// can still tell that a reference is what reached it. Without the mark both
// lowerings claim to be the declaration, and Intern — first-write-wins — hands
// the shared node to whichever ran first (GitHub #433).
//
// DeclarationHint already covers the case where the declaration is a top-level
// component entry, which is named the same from every use site. This covers the
// rest: a $ref may spell any pointer, and one naming a construct declared inline
// elsewhere is just as shared while matching no component shape.
func (c Ctx) NamingByReferenceAt(usePtr, declPtr string) Ctx {
if declPtr == usePtr {
return c
}
return c.NamingByReference()
}

// declaredSchemaNames collects the names under components/schemas, or nil when
// the document declares none.
func declaredSchemaNames(doc *soa.OpenAPI) map[string]bool {
Expand Down
27 changes: 27 additions & 0 deletions compilers/openapi/internal/lowering/lowering_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -326,3 +326,30 @@ func TestCtx_NamingByReferenceIsScopedToTheCopy(t *testing.T) {
assert.True(t, referencing.NamingByReference().NamesByReference(),
"and marking an already-marked context is a no-op rather than a toggle")
}

// TestCtx_NamingByReferenceAtMarksOnlyAForeignDeclaration pins the question the
// marking is derived from: whether the declaration a lowering is about to read
// sits at the position that reached it.
//
// The two pointers agree exactly when a construct is declared where it is used,
// and differ exactly when a $ref carried the lowering into a declaration another
// position owns. Deciding it here rather than at each call site is what keeps a
// request body and a response header answering it the same way (GitHub #433).
func TestCtx_NamingByReferenceAtMarksOnlyAForeignDeclaration(t *testing.T) {
t.Parallel()
const use = "/paths/~1b/post/requestBody"
declaring := lowering.Ctx{}

assert.False(t, declaring.NamingByReferenceAt(use, use).NamesByReference(),
"a construct declared where it is used is reached through its own declaration")

foreign := declaring.NamingByReferenceAt(use, "/paths/~1a/post/requestBody")
assert.True(t, foreign.NamesByReference(),
"a declaration another position owns is reached by reference, so its name is a placeholder")
assert.False(t, declaring.NamesByReference(),
"and the caller's context is left alone, as NamingByReference leaves it")

assert.True(t, foreign.NamingByReferenceAt(use, use).NamesByReference(),
"a marked context stays marked: the subtree under a $ref is named by reference throughout, "+
"however its own positions line up")
}
20 changes: 17 additions & 3 deletions compilers/openapi/internal/operation/content.go
Original file line number Diff line number Diff line change
Expand Up @@ -439,7 +439,10 @@ func reservedHeaderEntryDiag(c lowering.Ctx, name, hptr string) []ir.Diagnostic
// them (GitHub #116).
func lowerHeader(c lowering.Ctx, ts *compile.Types, anchors *schema.AnchorIndex, h *soa.Header, name, hptr, hdecl string) (ir.Property, []ir.Diagnostic) {
elected, diags := electTypeSpelling(c, h.GetSchema(), h.GetContent(), h.GetRootNode(), hdecl)
headerType, headerDiags := schema.CarriedRef(c, ts, anchors, schema.TopLevelDepth, elected.js, elected.pointer, ids.DeclarationHint(hdecl, name))
// name is this entry's map key, which names the shared node after this mount
// when the header is declared under another response (GitHub #433).
headerType, headerDiags := schema.CarriedRef(c.NamingByReferenceAt(hptr, hdecl), ts, anchors,
schema.TopLevelDepth, elected.js, elected.pointer, ids.DeclarationHint(hdecl, name))
diags = append(diags, headerDiags...)
p := ir.Property{
ID: ids.Prop(hptr),
Expand Down Expand Up @@ -730,12 +733,23 @@ func appendValuelessExample(c lowering.Ctx, out []ir.Example, proto ir.Example,
// content once at its component pointer rather than once per mount site
// (issue #107) — and under the component's name, since the operationId hint
// would otherwise name the shared node after one arbitrary referencing site.
//
// A body $ref'd from anywhere else takes the second half of that rule: the
// pointer is some other operation's, which DeclarationHint has no name for, so
// the lowering names by reference and leaves the owning operation to settle it
// (GitHub #433).
func lowerRequestBody(c lowering.Ctx, ts *compile.Types, anchors *schema.AnchorIndex, op *ir.Operation, hb *ir.HTTPBinding, src *soa.Operation, opDeclPtr string) []ir.Diagnostic {
rb, bodyPtr := resolve.ObjectAt[soa.RequestBody](c.RefScope(), src.GetRequestBody(), opDeclPtr+ids.Ptr("requestBody"))
usePtr := opDeclPtr + ids.Ptr("requestBody")
rb, bodyPtr := resolve.ObjectAt[soa.RequestBody](c.RefScope(), src.GetRequestBody(), usePtr)
if rb == nil {
return nil
}
payload, diags := lowerPayload(c, ts, anchors, rb.GetContent(), bodyPtr, ids.DeclarationHint(bodyPtr, requestBodyHint(src)))
// requestBodyHint spells this operation's ID, which names the shared node
// after this mount when the body is declared under another operation. Marking
// the lowering lets that operation's own pass replace the placeholder, in
// whichever order the two run (GitHub #433).
payload, diags := lowerPayload(c.NamingByReferenceAt(usePtr, bodyPtr), ts, anchors, rb.GetContent(),
bodyPtr, ids.DeclarationHint(bodyPtr, requestBodyHint(src)))
if payload == nil {
return diags
}
Expand Down
30 changes: 30 additions & 0 deletions testdata/openapi/shared_header_cross_response.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
# A $ref naming a header declared inline under another response. The header's
# hint is its map key at the use site, so the shared node must not take the key
# of whichever response lowered first (GitHub #433).
openapi: 3.1.0
info:
title: Header shared across responses by pointer
version: 1.0.0
paths:
/a:
get:
operationId: getA
responses:
"200":
description: declares the header
headers:
X-First:
schema:
type: object
properties:
detail:
type: string
/b:
get:
operationId: getB
responses:
"200":
description: names the header above
headers:
X-Second:
$ref: '#/paths/~1a/get/responses/200/headers/X-First'
30 changes: 30 additions & 0 deletions testdata/openapi/shared_request_body_cross_operation.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
# A $ref naming a request body declared inline under another operation. The
# body's hint is derived from the operation ID at its use site, so the shared
# node must not take the ID of whichever operation lowered first (GitHub #433).
openapi: 3.1.0
info:
title: Request body shared across operations by pointer
version: 1.0.0
paths:
/a:
post:
operationId: alpha
requestBody:
content:
application/json:
schema:
type: object
properties:
detail:
type: string
responses:
"204":
description: accepted
/b:
post:
operationId: beta
requestBody:
$ref: '#/paths/~1a/post/requestBody'
responses:
"204":
description: accepted
Loading