Repository: getkin/kin-openapi (go)

Issue:
Resolving refs is not cached
From what I can see in swagger_loader.go, if a document has 10 references to the same document, the document will be fetched and parsed 10 times. If we were to cache by URL, we could significantly speed up document loading.

Pull request:
feat: cache resolved refs, improve URI reader extensibility
This introduces simple URI-based caching.

A basic map-based cache implementation is provided, which may cover many
simple use cases. This caching is now used by default in the Loader.

File and HTTP URI readers are extracted from the loader internals so
that they may be extended, replaced, or configured. ReadFromHTTP allows
specifying the http.Client used to read HTTP $refs, for example.

Supersedes #117 and fixes #334.

Patch:
diff --git a/openapi2conv/openapi2_conv.go b/openapi2conv/openapi2_conv.go
index 35cf43d..387a05a 100644
--- a/openapi2conv/openapi2_conv.go
+++ b/openapi2conv/openapi2_conv.go
@@ -698,27 +698,29 @@ func fromV3RequestBodies(name string, requestBodyRef *openapi3.RequestBodyRef, c
 		return
 	}
 
-	//Only select one formData or request body for an individual requesstBody as OpenAPI 2 does not support multiples
+	//Only select one formData or request body for an individual requestBody as OpenAPI 2 does not support multiples
 	if requestBodyRef.Value != nil {
 		for contentType, mediaType := range requestBodyRef.Value.Content {
 			if consumes == nil {
 				consumes = make(map[string]struct{})
 			}
 			consumes[contentType] = struct{}{}
-			if formParams := FromV3RequestBodyFormData(mediaType); len(formParams) != 0 {
-				formParameters = formParams
-			} else {
-				paramName := name
-				if originalName, ok := requestBodyRef.Value.Extensions["x-originalParamName"]; ok {
-					json.Unmarshal(originalName.(json.RawMessage), &paramName)
-				}
+			if contentType == "application/x-www-form-urlencoded" || contentType == "multipart/form-data" {
+				formParameters = FromV3RequestBodyFormData(mediaType)
+				continue
+			}
 
-				var r *openapi2.Parameter
-				if r, err = FromV3RequestBody(paramName, requestBodyRef, mediaType, components); err != nil {
-					return
-				}
-				bodyOrRefParameters = append(bodyOrRefParameters, r)
+			paramName := name
+			if originalName, ok := requestBodyRef.Value.Extensions["x-originalParamName"]; ok {
+				json.Unmarshal(originalName.(json.RawMessage), &paramName)
 			}
+
+			var r *openapi2.Parameter
+			if r, err = FromV3RequestBody(paramName, requestBodyRef, mediaType, components); err != nil {
+				return
+			}
+
+			bodyOrRefParameters = append(bodyOrRefParameters, r)
 		}
 	}
 	return
diff --git a/openapi3/loader.go b/openapi3/loader.go
index 0b8d0e1..8af733c 100644
--- a/openapi3/loader.go
+++ b/openapi3/loader.go
@@ -5,8 +5,6 @@ import (
 	"encoding/json"
 	"errors"
 	"fmt"
-	"io/ioutil"
-	"net/http"
 	"net/url"
 	"path"
 	"path/filepath"
@@ -31,7 +29,7 @@ type Loader struct {
 	IsExternalRefsAllowed bool
 
 	// ReadFromURIFunc allows overriding the any file/URL reading func
-	ReadFromURIFunc func(loader *Loader, url *url.URL) ([]byte, error)
+	ReadFromURIFunc ReadFromURIFunc
 
 	Context context.Context
 
@@ -121,22 +119,7 @@ func (loader *Loader) readURL(location *url.URL) ([]byte, error) {
 	if f := loader.ReadFromURIFunc; f != nil {
 		return f(loader, location)
 	}
-
-	if location.Scheme != "" && location.Host != "" {
-		resp, err := http.Get(location.String())
-		if err != nil {
-			return nil, err
-		}
-		defer resp.Body.Close()
-		if resp.StatusCode > 399 {
-			return nil, fmt.Errorf("error loading %q: request returned status code %d", location.String(), resp.StatusCode)
-		}
-		return ioutil.ReadAll(resp.Body)
-	}
-	if location.Scheme != "" || location.Host != "" || location.RawQuery != "" {
-		return nil, fmt.Errorf("unsupported URI: %q", location.String())
-	}
-	return ioutil.ReadFile(location.Path)
+	return DefaultReadFromURI(loader, location)
 }
 
 // LoadFromData loads a spec from a byte array
diff --git a/openapi3/loader_uri_reader.go b/openapi3/loader_uri_reader.go
new file mode 100644
index 0000000..8357a98
--- /dev/null
+++ b/openapi3/loader_uri_reader.go
@@ -0,0 +1,104 @@
+package openapi3
+
+import (
+	"errors"
+	"fmt"
+	"io/ioutil"
+	"net/http"
+	"net/url"
+	"path/filepath"
+)
+
+// ReadFromURIFunc defines a function which reads the contents of a resource
+// located at a URI.
+type ReadFromURIFunc func(loader *Loader, url *url.URL) ([]byte, error)
+
+// ErrURINotSupported indicates the ReadFromURIFunc does not know how to handle a
+// given URI.
+var ErrURINotSupported = errors.New("unsupported URI")
+
+// ReadFromURIs returns a ReadFromURIFunc which tries to read a URI using the
+// given reader functions, in the same order. If a reader function does not
+// support the URI and returns ErrURINotSupported, the next function is checked
+// until a match is found, or the URI is not supported by any.
+func ReadFromURIs(readers ...ReadFromURIFunc) ReadFromURIFunc {
+	return func(loader *Loader, url *url.URL) ([]byte, error) {
+		for i := range readers {
+			buf, err := readers[i](loader, url)
+			if err == ErrURINotSupported {
+				continue
+			} else if err != nil {
+				return nil, err
+			}
+			return buf, nil
+		}
+		return nil, ErrURINotSupported
+	}
+}
+
+// DefaultReadFromURI returns a caching ReadFromURIFunc which can read remote
+// HTTP URIs and local file URIs.
+var DefaultReadFromURI = URIMapCache(ReadFromURIs(ReadFromHTTP(http.DefaultClient), ReadFromFile))
+
+// ReadFromHTTP returns a ReadFromURIFunc which uses the given http.Client to
+// read the contents from a remote HTTP URI. This client may be customized to
+// implement timeouts, RFC 7234 caching, etc.
+func ReadFromHTTP(cl *http.Client) ReadFromURIFunc {
+	return func(loader *Loader, location *url.URL) ([]byte, error) {
+		if location.Scheme == "" || location.Host == "" {
+			return nil, ErrURINotSupported
+		}
+		req, err := http.NewRequest("GET", location.String(), nil)
+		if err != nil {
+			return nil, err
+		}
+		resp, err := cl.Do(req)
+		if err != nil {
+			return nil, err
+		}
+		defer resp.Body.Close()
+		if resp.StatusCode > 399 {
+			return nil, fmt.Errorf("error loading %q: request returned status code %d", location.String(), resp.StatusCode)
+		}
+		return ioutil.ReadAll(resp.Body)
+	}
+}
+
+// ReadFromFile is a ReadFromURIFunc which reads local file URIs.
+func ReadFromFile(loader *Loader, location *url.URL) ([]byte, error) {
+	if location.Host != "" {
+		return nil, ErrURINotSupported
+	}
+	if location.Scheme != "" && location.Scheme != "file" {
+		return nil, ErrURINotSupported
+	}
+	return ioutil.ReadFile(location.Path)
+}
+
+// URIMapCache returns a ReadFromURIFunc that caches the contents read from URI
+// locations in a simple map. This cache implementation is suitable for
+// short-lived processes such as command-line tools which process OpenAPI
+// documents.
+func URIMapCache(reader ReadFromURIFunc) ReadFromURIFunc {
+	cache := map[string][]byte{}
+	return func(loader *Loader, location *url.URL) (buf []byte, err error) {
+		if location.Scheme == "" || location.Scheme == "file" {
+			if !filepath.IsAbs(location.Path) {
+				// Do not cache relative file paths; this can cause trouble if
+				// the current working directory changes when processing
+				// multiple top-level documents.
+				return reader(loader, location)
+			}
+		}
+		uri := location.String()
+		var ok bool
+		if buf, ok = cache[uri]; ok {
+			return
+		}
+		if buf, err = reader(loader, location); err != nil {
+			return
+		}
+		cache[uri] = buf
+		return
+	}
+}


Test patch:
diff --git a/openapi2conv/openapi2_conv_test.go b/openapi2conv/openapi2_conv_test.go
index 24ead56..9edce54 100644
--- a/openapi2conv/openapi2_conv_test.go
+++ b/openapi2conv/openapi2_conv_test.go
@@ -31,6 +31,26 @@ func TestConvOpenAPIV3ToV2(t *testing.T) {
 	require.JSONEq(t, exampleV2, string(data))
 }
 
+func TestConvOpenAPIV3ToV2WithReqBody(t *testing.T) {
+	var doc3 openapi3.T
+	err := json.Unmarshal([]byte(exampleRequestBodyV3), &doc3)
+	require.NoError(t, err)
+	{
+		// Refs need resolving before we can Validate
+		sl := openapi3.NewLoader()
+		err = sl.ResolveRefsIn(&doc3, nil)
+		require.NoError(t, err)
+		err = doc3.Validate(context.Background())
+		require.NoError(t, err)
+	}
+
+	doc2, err := FromV3(&doc3)
+	require.NoError(t, err)
+	data, err := json.Marshal(doc2)
+	require.NoError(t, err)
+	require.JSONEq(t, exampleRequestBodyV2, string(data))
+}
+
 func TestConvOpenAPIV2ToV3(t *testing.T) {
 	var doc2 openapi2.T
 	err := json.Unmarshal([]byte(exampleV2), &doc2)
@@ -766,3 +786,74 @@ const exampleV3 = `
 	"x-root2": "root extension 2"
 }
 `
+
+const exampleRequestBodyV3 = `{
+	"info": {
+		"description": "Test Spec",
+		"title": "Test Spec",
+		"version": "0.0.0"
+	},
+	"components": {
+		"requestBodies": {
+			"FooBody": {
+				"content": {
+					"application/json": {
+						"schema": {
+							"properties": { "message": { "type": "string" } },
+							"type": "object"
+			  			}
+					}
+		  		},
+		  		"description": "test spec request body.",
+		  		"required": true
+			}
+		}
+	},
+	"paths": {
+		"/foo-path": {
+			"post": {
+				"requestBody": { "$ref": "#/components/requestBodies/FooBody" },
+				"responses": { "202": { "description": "Test spec post." } },
+				"summary": "Test spec path"
+			}
+		}
+	},
+	"servers": [{ "url": "http://localhost/" }],
+	"openapi": "3.0.3"
+}
+`
+
+const exampleRequestBodyV2 = `{
+	"basePath": "/",
+	"consumes": ["application/json"],
+	"host": "localhost",
+	"info": {
+		"description": "Test Spec",
+		"title": "Test Spec",
+		"version": "0.0.0"
+	},
+	"parameters": {
+		"FooBody": {
+			"description": "test spec request body.",
+			"in": "body",
+			"name": "FooBody",
+			"required": true,
+			"schema": {
+				"properties": { "message": { "type": "string" } },
+				"type": "object"
+			}
+		}
+	},
+	"paths": {
+		"/foo-path": {
+			"post": {
+				"parameters": [{ "$ref": "#/parameters/FooBody" }],
+				"responses": { "202": { "description": "Test spec post." } },
+				"summary": "Test spec path"
+			}
+		}
+	},
+	"schemes": ["http"],
+	"swagger": "2.0"
+}
+`