diff --git a/openapi3/marsh.go b/openapi3/marsh.go index f895e6e40..72ca10930 100644 --- a/openapi3/marsh.go +++ b/openapi3/marsh.go @@ -17,15 +17,22 @@ func unmarshalError(jsonUnmarshalErr error) error { return jsonUnmarshalErr } -// unmarshal decodes data into v. It returns the document origin tree when -// includeOrigin is set and the data took the yaml path (json input carries no -// origins), so the caller can retain it (see Loader.originTrees). +// unmarshal decodes data into v. It returns the document origin tree when the +// data took the yaml path, so the caller can retain it (see Loader.originTrees). +// +// Only the yaml path records positions, so the order of the two attempts +// depends on includeOrigin: without it the json fast path runs first, with it +// the yaml path does, which is what gives json documents origins. Either way +// both parsers are tried before failing, so anything that loaded before still +// loads. func unmarshal(data []byte, v any, includeOrigin bool, location *url.URL) (*originTree, error) { var jsonErr, yamlErr error // See https://github.com/getkin/kin-openapi/issues/680 - if jsonErr = json.Unmarshal(data, v); jsonErr == nil { - return nil, nil + if !includeOrigin { + if jsonErr = json.Unmarshal(data, v); jsonErr == nil { + return nil, nil + } } // UnmarshalStrict(data, v) TODO: investigate how ymlv3 handles duplicate map keys @@ -43,6 +50,15 @@ func unmarshal(data []byte, v any, includeOrigin bool, location *url.URL) (*orig yamlErr = err } + // The yaml path was tried first and failed, so json has not run yet. It + // accepts documents yaml rejects (duplicate keys, which json resolves + // last-one-wins), so this keeps them loading, without origins. + if includeOrigin { + if jsonErr = json.Unmarshal(data, v); jsonErr == nil { + return nil, nil + } + } + // If both unmarshaling attempts fail, return a new error that includes both errors return nil, fmt.Errorf("failed to unmarshal data: json error: %v, yaml error: %v", jsonErr, yamlErr) } diff --git a/openapi3/origin_json_test.go b/openapi3/origin_json_test.go new file mode 100644 index 000000000..46d751578 --- /dev/null +++ b/openapi3/origin_json_test.go @@ -0,0 +1,104 @@ +package openapi3_test + +import ( + "testing" + + "github.com/stretchr/testify/require" + + "github.com/getkin/kin-openapi/openapi3" +) + +// A JSON document gets origins like a YAML one: only the yaml parser records +// positions, so the loader runs it first when origins are requested. +func TestOrigin_JSONSpec(t *testing.T) { + loader := openapi3.NewLoader() + loader.IncludeOrigin = true + loader.Context = t.Context() + + doc, err := loader.LoadFromFile("testdata/origin/simple.json") + require.NoError(t, err) + + require.NotNil(t, doc.Origin) + require.Equal(t, + openapi3.Location{ + File: "testdata/origin/simple.json", + Line: 2, + Column: 3, + Name: "openapi", + }, + doc.Origin.Fields["openapi"]) + + require.NotNil(t, doc.Info.Origin) + require.Equal(t, + &openapi3.Location{ + File: "testdata/origin/simple.json", + Line: 3, + Column: 3, + Name: "info", + EndLine: 6, + EndColumn: 4, + }, + doc.Info.Origin.Key) + + pathItem := doc.Paths.Find("/partner-api/test/some-method") + require.NotNil(t, pathItem.Origin) + require.Equal(t, + &openapi3.Location{ + File: "testdata/origin/simple.json", + Line: 8, + Column: 5, + Name: "/partner-api/test/some-method", + EndLine: 19, + EndColumn: 6, + }, + pathItem.Origin.Key) + + require.NotNil(t, pathItem.Get.Origin) + require.Equal(t, + &openapi3.Location{ + File: "testdata/origin/simple.json", + Line: 9, + Column: 7, + Name: "get", + EndLine: 18, + EndColumn: 8, + }, + pathItem.Get.Origin.Key) + + response := pathItem.Get.Responses.Value("200") + require.NotNil(t, response.Value.Origin) + require.Equal(t, + openapi3.Location{ + File: "testdata/origin/simple.json", + Line: 15, + Column: 13, + Name: "description", + }, + response.Value.Origin.Fields["description"]) +} + +// Origins are off, so the json fast path runs first and no positions are +// recorded. Pins that requesting origins is what changes the parser order. +func TestOrigin_JSONSpecWithoutIncludeOrigin(t *testing.T) { + loader := openapi3.NewLoader() + loader.Context = t.Context() + + doc, err := loader.LoadFromFile("testdata/origin/simple.json") + require.NoError(t, err) + require.Nil(t, doc.Origin) + require.Equal(t, "Test API", doc.Info.Title) +} + +// json permits duplicate keys and resolves them last-one-wins; yaml rejects +// them. Such a document must keep loading when origins are requested, since +// the loader falls back to json, and it simply has no origins. +func TestOrigin_JSONSpecDuplicateKeys(t *testing.T) { + loader := openapi3.NewLoader() + loader.IncludeOrigin = true + loader.Context = t.Context() + + doc, err := loader.LoadFromFile("testdata/origin/duplicate_keys.json") + require.NoError(t, err) + require.Equal(t, "Second", doc.Info.Title) + require.Nil(t, doc.Origin) +} diff --git a/openapi3/testdata/origin/duplicate_keys.json b/openapi3/testdata/origin/duplicate_keys.json new file mode 100644 index 000000000..7b4482a90 --- /dev/null +++ b/openapi3/testdata/origin/duplicate_keys.json @@ -0,0 +1,9 @@ +{ + "openapi": "3.0.1", + "info": { + "title": "First", + "title": "Second", + "version": "v1" + }, + "paths": {} +} diff --git a/openapi3/testdata/origin/simple.json b/openapi3/testdata/origin/simple.json new file mode 100644 index 000000000..8b495eb31 --- /dev/null +++ b/openapi3/testdata/origin/simple.json @@ -0,0 +1,21 @@ +{ + "openapi": "3.0.1", + "info": { + "title": "Test API", + "version": "v1" + }, + "paths": { + "/partner-api/test/some-method": { + "get": { + "tags": [ + "Test" + ], + "responses": { + "200": { + "description": "Success" + } + } + } + } + } +}