Repository: getkin/kin-openapi
Pull request #494 (2022-02-17 20:54:48)

distinguish form data in fromV3RequestBodies
This change fixes an issue in `openapi2conv.fromV3RequestBodies` where it didn't distinguish a "formData" parameter from a "body" parameter.

Given the following spec:
```json
{
  "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"
}
```
Would produce the following output:
```json
{
  "basePath": "/",
  "consumes": ["application/json"],
  "host": "localhost",
  "info": {
    "description": "Test Spec",
    "title": "Test Spec",
    "version": "0.0.0"
  },
  "parameters": {
    "message": { "in": "formData", "name": "message", "type": "string" }
  },
  "paths": {
    "/foo-path": {
      "post": {
        "parameters": [{ "$ref": "#/parameters/FooBody" }],
        "responses": { "202": { "description": "Test spec post." } },
        "summary": "Test spec path"
      }
    }
  },
  "schemes": ["http"],
  "swagger": "2.0"
}
```
Note that `"$ref": "#/parameters/FooBody"` does not resolve and parameters contains "formData"

after this change:
```json
{
  "basePath": "/",
  "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"
}
```
Note `"$ref": "#/parameters/FooBody"` resolves to the correct schema.

I also took the liberty to ~~remove the unused `consumes` set and~~ fix a minor typo ✨ 


Patch:
diff --git a/openapi2conv/openapi2_conv.go b/openapi2conv/openapi2_conv.go
index 4177f1f..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
@@ -1124,17 +1126,33 @@ func FromV3SecurityScheme(doc3 *openapi3.T, ref *openapi3.SecuritySchemeRef) (*o
 		if flows != nil {
 			var flow *openapi3.OAuthFlow
 			// TODO: Is this the right priority? What if multiple defined?
-			if flow = flows.Implicit; flow != nil {
+			switch {
+			case flows.Implicit != nil:
 				result.Flow = "implicit"
-			} else if flow = flows.AuthorizationCode; flow != nil {
+				flow = flows.Implicit
+				result.AuthorizationURL = flow.AuthorizationURL
+
+			case flows.AuthorizationCode != nil:
 				result.Flow = "accessCode"
-			} else if flow = flows.Password; flow != nil {
+				flow = flows.AuthorizationCode
+				result.AuthorizationURL = flow.AuthorizationURL
+				result.TokenURL = flow.TokenURL
+
+			case flows.Password != nil:
 				result.Flow = "password"
-			} else if flow = flows.ClientCredentials; flow != nil {
+				flow = flows.Password
+				result.TokenURL = flow.TokenURL
+
+			case flows.ClientCredentials != nil:
 				result.Flow = "application"
-			} else {
+				flow = flows.ClientCredentials
+				result.TokenURL = flow.TokenURL
+
+			default:
 				return nil, nil
 			}
+
+			result.Scopes = make(map[string]string, len(flow.Scopes))
 			for scope, desc := range flow.Scopes {
 				result.Scopes[scope] = desc
 			}
diff --git a/openapi3/parameter.go b/openapi3/parameter.go
index 2081e4e..a0b8ed1 100644
--- a/openapi3/parameter.go
+++ b/openapi3/parameter.go
@@ -252,6 +252,10 @@ func (value *Parameter) Validate(ctx context.Context) error {
 		return fmt.Errorf("parameter can't have 'in' value %q", value.In)
 	}
 
+	if in == ParameterInPath && !value.Required {
+		return fmt.Errorf("path parameter %q must be required", value.Name)
+	}
+
 	// Validate a parameter's serialization method.
 	sm, err := value.SerializationMethod()
 	if err != nil {
diff --git a/openapi3/schema_formats.go b/openapi3/schema_formats.go
index 8a8a940..29fbd51 100644
--- a/openapi3/schema_formats.go
+++ b/openapi3/schema_formats.go
@@ -90,7 +90,7 @@ func init() {
 	DefineStringFormat("date", `^[0-9]{4}-(0[0-9]|10|11|12)-([0-2][0-9]|30|31)$`)
 
 	// date-time
-	DefineStringFormat("date-time", `^[0-9]{4}-(0[0-9]|10|11|12)-([0-2][0-9]|30|31)T[0-9]{2}:[0-9]{2}:[0-9]{2}(.[0-9]+)?(Z|(\+|-)[0-9]{2}:[0-9]{2})?$`)
+	DefineStringFormat("date-time", `^[0-9]{4}-(0[0-9]|10|11|12)-([0-2][0-9]|30|31)T[0-9]{2}:[0-9]{2}:[0-9]{2}(\.[0-9]+)?(Z|(\+|-)[0-9]{2}:[0-9]{2})?$`)
 
 }
 


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"
+}
+`
diff --git a/openapi3/loader_test.go b/openapi3/loader_test.go
index eb29314..384e54d 100644
--- a/openapi3/loader_test.go
+++ b/openapi3/loader_test.go
@@ -412,6 +412,7 @@ paths:
       parameters:
         - name: id,
           in: path
+          required: true
           schema:
             type: string
       responses:
diff --git a/openapi3/schema_issue492_test.go b/openapi3/schema_issue492_test.go
new file mode 100644
index 0000000..535c82a
--- /dev/null
+++ b/openapi3/schema_issue492_test.go
@@ -0,0 +1,41 @@
+package openapi3
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/require"
+)
+
+func TestIssue492(t *testing.T) {
+	spec := []byte(`components:
+  schemas:
+    Server:
+      properties:
+        time:
+          $ref: "#/components/schemas/timestamp"
+        name:
+          type: string
+      type: object
+    timestamp:
+      type: string
+      format: date-time
+openapi: "3.0.1"
+`)
+
+	s, err := NewLoader().LoadFromData(spec)
+	require.NoError(t, err)
+
+	// verify that the expected format works
+	err = s.Components.Schemas["Server"].Value.VisitJSON(map[string]interface{}{
+		"name": "kin-openapi",
+		"time": "2001-02-03T04:05:06.789Z",
+	})
+	require.NoError(t, err)
+
+	// verify that the issue is fixed
+	err = s.Components.Schemas["Server"].Value.VisitJSON(map[string]interface{}{
+		"name": "kin-openapi",
+		"time": "2001-02-03T04:05:06:789Z",
+	})
+	require.EqualError(t, err, "Error at \"/time\": string doesn't match the format \"date-time\" (regular expression \"^[0-9]{4}-(0[0-9]|10|11|12)-([0-2][0-9]|30|31)T[0-9]{2}:[0-9]{2}:[0-9]{2}(\\.[0-9]+)?(Z|(\\+|-)[0-9]{2}:[0-9]{2})?$\")\nSchema:\n  {\n    \"format\": \"date-time\",\n    \"type\": \"string\"\n  }\n\nValue:\n  \"2001-02-03T04:05:06:789Z\"\n")
+}
diff --git a/openapi3filter/req_resp_decoder_test.go b/openapi3filter/req_resp_decoder_test.go
index 34e6371..f40a7a5 100644
--- a/openapi3filter/req_resp_decoder_test.go
+++ b/openapi3filter/req_resp_decoder_test.go
@@ -908,6 +908,7 @@ func TestDecodeParameter(t *testing.T) {
 					path := "/test"
 					if tc.path != "" {
 						path += "/{" + tc.param.Name + "}"
+						tc.param.Required = true
 					}
 
 					info := &openapi3.Info{
diff --git a/openapi3filter/validation_test.go b/openapi3filter/validation_test.go
index d4d0a12..2f7cf80 100644
--- a/openapi3filter/validation_test.go
+++ b/openapi3filter/validation_test.go
@@ -62,9 +62,10 @@ func TestFilter(t *testing.T) {
 					Parameters: openapi3.Parameters{
 						{
 							Value: &openapi3.Parameter{
-								In:     "path",
-								Name:   "pathArg",
-								Schema: openapi3.NewStringSchema().WithMaxLength(2).NewRef(),
+								In:       "path",
+								Name:     "pathArg",
+								Schema:   openapi3.NewStringSchema().WithMaxLength(2).NewRef(),
+								Required: true,
 							},
 						},
 						{