openapi3: record origins for specs written in JSON

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)
 }

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)
+}

openapi3/testdata/origin/duplicate_keys.json

@@ -0,0 +1,9 @@
+{
+  "openapi": "3.0.1",
+  "info": {
+    "title": "First",
+    "title": "Second",
+    "version": "v1"
+  },
+  "paths": {}
+}

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"
+          }
+        }
+      }
+    }
+  }
+}