Skip to content

Commit a34baf0

Browse files
reuvenharrisonreuvenharrison
authored
openapi3: delete origin keys only when IncludeOrigin=true (#1055)
* replicate issue 1051 * remove pre-existing origin keys iff IncludeOrigin=true * update docs * fix raw quotes * udpate unit tests
1 parent 2d3e67a commit a34baf0

File tree

4 files changed

+108
-24
lines changed

4 files changed

+108
-24
lines changed

‎.github/docs/openapi3.txt

+5-3
Original file line numberDiff line numberDiff line change
@@ -90,6 +90,11 @@ var ErrURINotSupported = errors.New("unsupported URI")
9090
ErrURINotSupported indicates the ReadFromURIFunc does not know how to handle
9191
a given URI.
9292

93+
var IncludeOrigin = false
94+
IncludeOrigin specifies whether to include the origin of the OpenAPI
95+
elements Set this to true before loading a spec to include the origin of the
96+
OpenAPI elements Note it is global and affects all loaders
97+
9398

9499
FUNCTIONS
95100

@@ -790,9 +795,6 @@ type Loader struct {
790795
// IsExternalRefsAllowed enables visiting other files
791796
IsExternalRefsAllowed bool
792797

793-
// IncludeOrigin specifies whether to include the origin of the OpenAPI elements
794-
IncludeOrigin bool
795-
796798
// ReadFromURIFunc allows overriding the any file/URL reading func
797799
ReadFromURIFunc ReadFromURIFunc
798800

‎openapi3/loader.go

+9-7
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,11 @@ import (
1515
"strings"
1616
)
1717

18+
// IncludeOrigin specifies whether to include the origin of the OpenAPI elements
19+
// Set this to true before loading a spec to include the origin of the OpenAPI elements
20+
// Note it is global and affects all loaders
21+
var IncludeOrigin = false
22+
1823
func foundUnresolvedRef(ref string) error {
1924
return fmt.Errorf("found unresolved ref: %q", ref)
2025
}
@@ -28,9 +33,6 @@ type Loader struct {
2833
// IsExternalRefsAllowed enables visiting other files
2934
IsExternalRefsAllowed bool
3035

31-
// IncludeOrigin specifies whether to include the origin of the OpenAPI elements
32-
IncludeOrigin bool
33-
3436
// ReadFromURIFunc allows overriding the any file/URL reading func
3537
ReadFromURIFunc ReadFromURIFunc
3638

@@ -106,7 +108,7 @@ func (loader *Loader) loadSingleElementFromURI(ref string, rootPath *url.URL, el
106108
if err != nil {
107109
return nil, err
108110
}
109-
if err := unmarshal(data, element, loader.IncludeOrigin); err != nil {
111+
if err := unmarshal(data, element, IncludeOrigin); err != nil {
110112
return nil, err
111113
}
112114

@@ -142,7 +144,7 @@ func (loader *Loader) LoadFromIoReader(reader io.Reader) (*T, error) {
142144
func (loader *Loader) LoadFromData(data []byte) (*T, error) {
143145
loader.resetVisitedPathItemRefs()
144146
doc := &T{}
145-
if err := unmarshal(data, doc, loader.IncludeOrigin); err != nil {
147+
if err := unmarshal(data, doc, IncludeOrigin); err != nil {
146148
return nil, err
147149
}
148150
if err := loader.ResolveRefsIn(doc, nil); err != nil {
@@ -171,7 +173,7 @@ func (loader *Loader) loadFromDataWithPathInternal(data []byte, location *url.UR
171173
doc := &T{}
172174
loader.visitedDocuments[uri] = doc
173175

174-
if err := unmarshal(data, doc, loader.IncludeOrigin); err != nil {
176+
if err := unmarshal(data, doc, IncludeOrigin); err != nil {
175177
return nil, err
176178
}
177179

@@ -425,7 +427,7 @@ func (loader *Loader) resolveComponent(doc *T, ref string, path *url.URL, resolv
425427
if err2 != nil {
426428
return nil, nil, err
427429
}
428-
if err2 = unmarshal(data, &cursor, loader.IncludeOrigin); err2 != nil {
430+
if err2 = unmarshal(data, &cursor, IncludeOrigin); err2 != nil {
429431
return nil, nil, err
430432
}
431433
if cursor, err2 = drill(cursor); err2 != nil || cursor == nil {

‎openapi3/origin_test.go

+78-10
Original file line numberDiff line numberDiff line change
@@ -7,10 +7,17 @@ import (
77
"github.com/stretchr/testify/require"
88
)
99

10+
func unsetIncludeOrigin() {
11+
IncludeOrigin = false
12+
}
13+
1014
func TestOrigin_Info(t *testing.T) {
1115
loader := NewLoader()
1216
loader.IsExternalRefsAllowed = true
13-
loader.IncludeOrigin = true
17+
18+
IncludeOrigin = true
19+
defer unsetIncludeOrigin()
20+
1421
loader.Context = context.Background()
1522

1623
doc, err := loader.LoadFromFile("testdata/origin/simple.yaml")
@@ -42,7 +49,10 @@ func TestOrigin_Info(t *testing.T) {
4249
func TestOrigin_Paths(t *testing.T) {
4350
loader := NewLoader()
4451
loader.IsExternalRefsAllowed = true
45-
loader.IncludeOrigin = true
52+
53+
IncludeOrigin = true
54+
defer unsetIncludeOrigin()
55+
4656
loader.Context = context.Background()
4757

4858
doc, err := loader.LoadFromFile("testdata/origin/simple.yaml")
@@ -78,7 +88,10 @@ func TestOrigin_Paths(t *testing.T) {
7888
func TestOrigin_RequestBody(t *testing.T) {
7989
loader := NewLoader()
8090
loader.IsExternalRefsAllowed = true
81-
loader.IncludeOrigin = true
91+
92+
IncludeOrigin = true
93+
defer unsetIncludeOrigin()
94+
8295
loader.Context = context.Background()
8396

8497
doc, err := loader.LoadFromFile("testdata/origin/request_body.yaml")
@@ -105,7 +118,10 @@ func TestOrigin_RequestBody(t *testing.T) {
105118
func TestOrigin_Responses(t *testing.T) {
106119
loader := NewLoader()
107120
loader.IsExternalRefsAllowed = true
108-
loader.IncludeOrigin = true
121+
122+
IncludeOrigin = true
123+
defer unsetIncludeOrigin()
124+
109125
loader.Context = context.Background()
110126

111127
doc, err := loader.LoadFromFile("testdata/origin/simple.yaml")
@@ -140,7 +156,10 @@ func TestOrigin_Responses(t *testing.T) {
140156
func TestOrigin_Parameters(t *testing.T) {
141157
loader := NewLoader()
142158
loader.IsExternalRefsAllowed = true
143-
loader.IncludeOrigin = true
159+
160+
IncludeOrigin = true
161+
defer unsetIncludeOrigin()
162+
144163
loader.Context = context.Background()
145164

146165
doc, err := loader.LoadFromFile("testdata/origin/parameters.yaml")
@@ -173,7 +192,10 @@ func TestOrigin_Parameters(t *testing.T) {
173192
func TestOrigin_SchemaInAdditionalProperties(t *testing.T) {
174193
loader := NewLoader()
175194
loader.IsExternalRefsAllowed = true
176-
loader.IncludeOrigin = true
195+
196+
IncludeOrigin = true
197+
defer unsetIncludeOrigin()
198+
177199
loader.Context = context.Background()
178200

179201
doc, err := loader.LoadFromFile("testdata/origin/additional_properties.yaml")
@@ -201,7 +223,10 @@ func TestOrigin_SchemaInAdditionalProperties(t *testing.T) {
201223
func TestOrigin_ExternalDocs(t *testing.T) {
202224
loader := NewLoader()
203225
loader.IsExternalRefsAllowed = true
204-
loader.IncludeOrigin = true
226+
227+
IncludeOrigin = true
228+
defer unsetIncludeOrigin()
229+
205230
loader.Context = context.Background()
206231

207232
doc, err := loader.LoadFromFile("testdata/origin/external_docs.yaml")
@@ -235,7 +260,10 @@ func TestOrigin_ExternalDocs(t *testing.T) {
235260
func TestOrigin_Security(t *testing.T) {
236261
loader := NewLoader()
237262
loader.IsExternalRefsAllowed = true
238-
loader.IncludeOrigin = true
263+
264+
IncludeOrigin = true
265+
defer unsetIncludeOrigin()
266+
239267
loader.Context = context.Background()
240268

241269
doc, err := loader.LoadFromFile("testdata/origin/security.yaml")
@@ -283,7 +311,10 @@ func TestOrigin_Security(t *testing.T) {
283311
func TestOrigin_Example(t *testing.T) {
284312
loader := NewLoader()
285313
loader.IsExternalRefsAllowed = true
286-
loader.IncludeOrigin = true
314+
315+
IncludeOrigin = true
316+
defer unsetIncludeOrigin()
317+
287318
loader.Context = context.Background()
288319

289320
doc, err := loader.LoadFromFile("testdata/origin/example.yaml")
@@ -319,7 +350,10 @@ func TestOrigin_Example(t *testing.T) {
319350
func TestOrigin_XML(t *testing.T) {
320351
loader := NewLoader()
321352
loader.IsExternalRefsAllowed = true
322-
loader.IncludeOrigin = true
353+
354+
IncludeOrigin = true
355+
defer unsetIncludeOrigin()
356+
323357
loader.Context = context.Background()
324358

325359
doc, err := loader.LoadFromFile("testdata/origin/xml.yaml")
@@ -348,3 +382,37 @@ func TestOrigin_XML(t *testing.T) {
348382
},
349383
base.Origin.Fields["prefix"])
350384
}
385+
386+
// TestOrigin_OriginExistsInProperties verifies that loading fails when a specification
387+
// contains a property named "__origin__", highlighting a limitation in the current implementation.
388+
func TestOrigin_OriginExistsInProperties(t *testing.T) {
389+
var data = `
390+
paths:
391+
/foo:
392+
get:
393+
responses:
394+
"200":
395+
description: OK
396+
content:
397+
application/json:
398+
schema:
399+
$ref: "#/components/schemas/Foo"
400+
components:
401+
schemas:
402+
Foo:
403+
type: object
404+
properties:
405+
__origin__:
406+
type: string
407+
`
408+
409+
loader := NewLoader()
410+
411+
IncludeOrigin = true
412+
defer unsetIncludeOrigin()
413+
414+
_, err := loader.LoadFromData([]byte(data))
415+
require.Error(t, err)
416+
require.Equal(t, `failed to unmarshal data: json error: invalid character 'p' looking for beginning of value, yaml error: error converting YAML to JSON: yaml: unmarshal errors:
417+
line 0: mapping key "__origin__" already defined at line 17`, err.Error())
418+
}

‎openapi3/stringmap.go

+16-4
Original file line numberDiff line numberDiff line change
@@ -18,11 +18,10 @@ func unmarshalStringMapP[V any](data []byte) (map[string]*V, *Origin, error) {
1818
return nil, nil, err
1919
}
2020

21-
origin, err := deepCast[Origin](m[originKey])
21+
origin, err := popOrigin(m, originKey)
2222
if err != nil {
2323
return nil, nil, err
2424
}
25-
delete(m, originKey)
2625

2726
result := make(map[string]*V, len(m))
2827
for k, v := range m {
@@ -43,11 +42,10 @@ func unmarshalStringMap[V any](data []byte) (map[string]V, *Origin, error) {
4342
return nil, nil, err
4443
}
4544

46-
origin, err := deepCast[Origin](m[originKey])
45+
origin, err := popOrigin(m, originKey)
4746
if err != nil {
4847
return nil, nil, err
4948
}
50-
delete(m, originKey)
5149

5250
result := make(map[string]V, len(m))
5351
for k, v := range m {
@@ -74,3 +72,17 @@ func deepCast[V any](value any) (*V, error) {
7472
}
7573
return &result, nil
7674
}
75+
76+
// popOrigin removes the origin from the map and returns it.
77+
func popOrigin(m map[string]any, key string) (*Origin, error) {
78+
if !IncludeOrigin {
79+
return nil, nil
80+
}
81+
82+
origin, err := deepCast[Origin](m[key])
83+
if err != nil {
84+
return nil, err
85+
}
86+
delete(m, key)
87+
return origin, nil
88+
}

0 commit comments

Comments
 (0)