Fix file uploads in 3.1, remove file uploads nullability in 3.0 (#94)

swaggest · Dec 8, 2023 · ac93ece · ac93ece
1 parent fb05447
commit ac93ece
Show file tree

Hide file tree

Showing 14 changed files with 174 additions and 18 deletions.
diff --git a/internal/json_schema.go b/internal/json_schema.go
@@ -34,6 +34,7 @@ func sanitizeDefName(rc *jsonschema.ReflectContext) {
 
 // ReflectRequestBody reflects JSON schema of request body.
 func ReflectRequestBody(
+	is31 bool, // True if OpenAPI 3.1
 	r *jsonschema.Reflector,
 	cu openapi.ContentUnit,
 	httpMethod string,
@@ -121,21 +122,43 @@ func ReflectRequestBody(
 		jsonschema.PropertyNameMapping(mapping),
 		jsonschema.PropertyNameTag(tag, additionalTags...),
 		sanitizeDefName,
+		jsonschema.InterceptNullability(func(params jsonschema.InterceptNullabilityParams) {
+			if params.NullAdded {
+				vv := reflect.Zero(params.Schema.ReflectType).Interface()
+
+				foundFiles := false
+				if _, ok := vv.([]multipart.File); ok {
+					foundFiles = true
+				}
+
+				if _, ok := vv.([]*multipart.FileHeader); ok {
+					foundFiles = true
+				}
+
+				if foundFiles {
+					params.Schema.RemoveType(jsonschema.Null)
+				}
+			}
+		}),
 		jsonschema.InterceptSchema(func(params jsonschema.InterceptSchemaParams) (stop bool, err error) {
 			vv := params.Value.Interface()
 
-			found := false
+			foundFile := false
 			if _, ok := vv.(*multipart.File); ok {
-				found = true
+				foundFile = true
 			}
 
 			if _, ok := vv.(*multipart.FileHeader); ok {
-				found = true
+				foundFile = true
 			}
 
-			if found {
+			if foundFile {
 				params.Schema.AddType(jsonschema.String)
+				params.Schema.RemoveType(jsonschema.Null)
 				params.Schema.WithFormat("binary")
+				if is31 {
+					params.Schema.WithExtraPropertiesItem("contentMediaType", "application/octet-stream")
+				}
 
 				hasFileUpload = true
 

diff --git a/openapi3/reflect.go b/openapi3/reflect.go
@@ -321,6 +321,7 @@ func (r *Reflector) parseRequestBody(
 	additionalTags ...string,
 ) error {
 	schema, hasFileUpload, err := internal.ReflectRequestBody(
+		false,
 		r.JSONSchemaReflector(),
 		cu,
 		httpMethod,

diff --git a/openapi3/reflect_deprecated_test.go b/openapi3/reflect_deprecated_test.go
@@ -73,7 +73,7 @@ func TestReflector_SetRequest_uploadInterface(t *testing.T) {
 			"type":"object",
 			"properties":{"upload1":{"$ref":"#/components/schemas/MultipartFile"}}
 		  },
-		  "MultipartFile":{"type":"string","format":"binary","nullable":true}
+		  "MultipartFile":{"type":"string","format":"binary"}
 		}
 	  }
 	}`, s)
@@ -342,7 +342,7 @@ func TestReflector_SetupRequest(t *testing.T) {
 		}
 	  },
 	  "components":{
-		"schemas":{"MultipartFile":{"type":"string","format":"binary","nullable":true}}
+		"schemas":{"MultipartFile":{"type":"string","format":"binary"}}
 	  }
 	}`, s)
 }

diff --git a/openapi3/reflect_test.go b/openapi3/reflect_test.go
@@ -153,7 +153,7 @@ func TestReflector_AddOperation_uploadInterface(t *testing.T) {
 			"type":"object",
 			"properties":{"upload1":{"$ref":"#/components/schemas/MultipartFile"}}
 		  },
-		  "MultipartFile":{"type":"string","format":"binary","nullable":true}
+		  "MultipartFile":{"type":"string","format":"binary"}
 		}
 	  }
 	}`, reflector.Spec)
@@ -460,7 +460,7 @@ func TestReflector_AddOperation_setup_request(t *testing.T) {
 		}
 	  },
 	  "components":{
-		"schemas":{"MultipartFile":{"type":"string","format":"binary","nullable":true}}
+		"schemas":{"MultipartFile":{"type":"string","format":"binary"}}
 	  }
 	}`, s)
 }

diff --git a/openapi3/testdata/openapi.json b/openapi3/testdata/openapi.json
@@ -99,8 +99,8 @@
      "upload2":{"$ref":"#/components/schemas/MultipartFileHeader"}
     }
    },
-   "MultipartFile":{"type":"string","format":"binary","nullable":true},
-   "MultipartFileHeader":{"type":"string","format":"binary","nullable":true},
+   "MultipartFile":{"type":"string","format":"binary"},
+   "MultipartFileHeader":{"type":"string","format":"binary"},
    "Openapi3TestReq":{"type":"object","properties":{"in_body1":{"type":"integer"},"in_body2":{"type":"string"}}},
    "Openapi3TestResp":{
     "title":"Sample Response","type":"object",

diff --git a/openapi3/testdata/req_schema.json b/openapi3/testdata/req_schema.json
@@ -7,8 +7,8 @@
   "type":"object",
   "components":{
     "schemas":{
-      "MultipartFile":{"type":["string","null"],"format":"binary"},
-      "MultipartFileHeader":{"type":["string","null"],"format":"binary"}
+      "MultipartFile":{"type":"string","format":"binary"},
+      "MultipartFileHeader":{"type":"string","format":"binary"}
     }
   }
 }
diff --git a/openapi3/testdata/uploads.json b/openapi3/testdata/uploads.json
@@ -0,0 +1,25 @@
+{
+ "openapi":"3.0.3","info":{"title":"","version":""},
+ "paths":{
+  "/upload":{
+   "post":{
+    "requestBody":{"content":{"multipart/form-data":{"schema":{"$ref":"#/components/schemas/FormDataOpenapi3TestReq"}}}},
+    "responses":{"204":{"description":"No Content"}}
+   }
+  }
+ },
+ "components":{
+  "schemas":{
+   "FormDataOpenapi3TestReq":{
+    "type":"object",
+    "properties":{
+     "upload1":{"$ref":"#/components/schemas/MultipartFile"},
+     "upload2":{"$ref":"#/components/schemas/MultipartFileHeader"},
+     "uploads3":{"type":"array","items":{"$ref":"#/components/schemas/MultipartFile"}},
+     "uploads4":{"type":"array","items":{"$ref":"#/components/schemas/MultipartFileHeader"}}
+    }
+   },
+   "MultipartFile":{"type":"string","format":"binary"},"MultipartFileHeader":{"type":"string","format":"binary"}
+  }
+ }
+}
diff --git a/openapi3/upload_test.go b/openapi3/upload_test.go
@@ -0,0 +1,40 @@
+package openapi3_test
+
+import (
+	"mime/multipart"
+	"net/http"
+	"os"
+	"testing"
+
+	"github.com/stretchr/testify/require"
+	"github.com/swaggest/assertjson"
+	"github.com/swaggest/openapi-go/openapi3"
+)
+
+func TestNewReflector_uploads(t *testing.T) {
+	r := openapi3.NewReflector()
+
+	oc, err := r.NewOperationContext(http.MethodPost, "/upload")
+	require.NoError(t, err)
+
+	type req struct {
+		Upload1  multipart.File          `formData:"upload1"`
+		Upload2  *multipart.FileHeader   `formData:"upload2"`
+		Uploads3 []multipart.File        `formData:"uploads3"`
+		Uploads4 []*multipart.FileHeader `formData:"uploads4"`
+	}
+
+	oc.AddReqStructure(req{})
+
+	require.NoError(t, r.AddOperation(oc))
+
+	schema, err := assertjson.MarshalIndentCompact(r.SpecSchema(), "", " ", 120)
+	require.NoError(t, err)
+
+	require.NoError(t, os.WriteFile("testdata/uploads_last_run.json", schema, 0o600))
+
+	expected, err := os.ReadFile("testdata/uploads.json")
+	require.NoError(t, err)
+
+	assertjson.Equal(t, expected, schema)
+}
diff --git a/openapi31/reflect.go b/openapi31/reflect.go
@@ -268,6 +268,7 @@ func (r *Reflector) parseRequestBody(
 	additionalTags ...string,
 ) error {
 	schema, hasFileUpload, err := internal.ReflectRequestBody(
+		true,
 		r.JSONSchemaReflector(),
 		cu,
 		httpMethod,

diff --git a/openapi31/reflect_test.go b/openapi31/reflect_test.go
@@ -155,7 +155,7 @@ func TestReflector_AddOperation_uploadInterface(t *testing.T) {
 			"properties":{"upload1":{"$ref":"#/components/schemas/MultipartFile"}},
 			"type":"object"
 		  },
-		  "MultipartFile":{"format":"binary","type":["null","string"]}
+		  "MultipartFile":{"format":"binary","type":"string","contentMediaType": "application/octet-stream"}
 		}
 	  }
 	}`, reflector.Spec)
@@ -477,7 +477,7 @@ func TestReflector_AddOperation_setup_request(t *testing.T) {
 		  }
 		}
 	  },
-	  "components":{"schemas":{"MultipartFile":{"format":"binary","type":["null","string"]}}}
+	  "components":{"schemas":{"MultipartFile":{"format":"binary","type":"string", "contentMediaType": "application/octet-stream"}}}
 	}`, s)
 }
 

diff --git a/openapi31/testdata/openapi.json b/openapi31/testdata/openapi.json
@@ -99,8 +99,8 @@
     },
     "type":"object"
    },
-   "MultipartFile":{"format":"binary","type":["null","string"]},
-   "MultipartFileHeader":{"format":"binary","type":["null","string"]},
+   "MultipartFile":{"contentMediaType":"application/octet-stream","format":"binary","type":"string"},
+   "MultipartFileHeader":{"contentMediaType":"application/octet-stream","format":"binary","type":"string"},
    "Openapi31TestReq":{"properties":{"in_body1":{"type":"integer"},"in_body2":{"type":"string"}},"type":"object"},
    "Openapi31TestResp":{
     "description":"This is a sample response.",

diff --git a/openapi31/testdata/req_schema.json b/openapi31/testdata/req_schema.json
@@ -7,8 +7,8 @@
   "type":"object",
   "components":{
     "schemas":{
-      "MultipartFile":{"type":["null","string"],"format":"binary"},
-      "MultipartFileHeader":{"type":["null","string"],"format":"binary"}
+      "MultipartFile":{"type":"string","format":"binary","contentMediaType": "application/octet-stream"},
+      "MultipartFileHeader":{"type":"string","format":"binary","contentMediaType": "application/octet-stream"}
     }
   }
 }
diff --git a/openapi31/testdata/uploads.json b/openapi31/testdata/uploads.json
@@ -0,0 +1,26 @@
+{
+ "openapi":"3.1.0","info":{"title":"","version":""},
+ "paths":{
+  "/upload":{
+   "post":{
+    "requestBody":{"content":{"multipart/form-data":{"schema":{"$ref":"#/components/schemas/FormDataOpenapi31TestReq"}}}},
+    "responses":{"204":{"description":"No Content"}}
+   }
+  }
+ },
+ "components":{
+  "schemas":{
+   "FormDataOpenapi31TestReq":{
+    "properties":{
+     "upload1":{"$ref":"#/components/schemas/MultipartFile"},
+     "upload2":{"$ref":"#/components/schemas/MultipartFileHeader"},
+     "uploads3":{"items":{"$ref":"#/components/schemas/MultipartFile"},"type":"array"},
+     "uploads4":{"items":{"$ref":"#/components/schemas/MultipartFileHeader"},"type":"array"}
+    },
+    "type":"object"
+   },
+   "MultipartFile":{"contentMediaType":"application/octet-stream","format":"binary","type":"string"},
+   "MultipartFileHeader":{"contentMediaType":"application/octet-stream","format":"binary","type":"string"}
+  }
+ }
+}
diff --git a/openapi31/upload_test.go b/openapi31/upload_test.go
@@ -0,0 +1,40 @@
+package openapi31_test
+
+import (
+	"mime/multipart"
+	"net/http"
+	"os"
+	"testing"
+
+	"github.com/stretchr/testify/require"
+	"github.com/swaggest/assertjson"
+	"github.com/swaggest/openapi-go/openapi31"
+)
+
+func TestNewReflector_uploads(t *testing.T) {
+	r := openapi31.NewReflector()
+
+	oc, err := r.NewOperationContext(http.MethodPost, "/upload")
+	require.NoError(t, err)
+
+	type req struct {
+		Upload1  multipart.File          `formData:"upload1"`
+		Upload2  *multipart.FileHeader   `formData:"upload2"`
+		Uploads3 []multipart.File        `formData:"uploads3"`
+		Uploads4 []*multipart.FileHeader `formData:"uploads4"`
+	}
+
+	oc.AddReqStructure(req{})
+
+	require.NoError(t, r.AddOperation(oc))
+
+	schema, err := assertjson.MarshalIndentCompact(r.SpecSchema(), "", " ", 120)
+	require.NoError(t, err)
+
+	require.NoError(t, os.WriteFile("testdata/uploads_last_run.json", schema, 0o600))
+
+	expected, err := os.ReadFile("testdata/uploads.json")
+	require.NoError(t, err)
+
+	assertjson.Equal(t, expected, schema)
+}