-
Notifications
You must be signed in to change notification settings - Fork 332
fix(cli-generator): unwrap anyOf/oneOf when detecting multipart file fields #17304
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,11 @@ | ||
| # yaml-language-server: $schema=../../../../fern-changes-yml.schema.json | ||
|
|
||
| - summary: | | ||
| Fix optional file uploads being sent as text parts. A `multipart/form-data` property | ||
| typed `anyOf: [{type: string, format: binary}, {type: "null"}]` — the shape every | ||
| optional file gets — was not recognized as a file, so the CLI sent the filename as a | ||
| plain text part and the API rejected the call (e.g. `422 Expected UploadFile, received: | ||
| <class 'str'>`). File classification now unwraps `anyOf`/`oneOf` (resolving `$ref` | ||
| branches and ignoring the `null` branch), so nullable binary fields and nullable arrays | ||
| of binary fields upload the file contents. | ||
| type: fix |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3484,28 +3484,47 @@ fn classify_multipart_property( | |
| prop | ||
| }; | ||
|
|
||
| let ty = resolved.schema_type(); | ||
| let fmt = resolved.format.as_deref(); | ||
| if is_binary_schema(resolved) { | ||
| return (true, Some("application/octet-stream".to_string())); | ||
| } | ||
|
|
||
| // `type: string, format: binary` or legacy `type: file` | ||
| if (ty == Some("string") && fmt == Some("binary")) || ty == Some("file") { | ||
| let ct = Some("application/octet-stream".to_string()); | ||
| return (true, ct); | ||
| // Optional / nullable uploads wrap the binary schema in a composition: | ||
| // `anyOf: [{type: string, format: binary}, {type: "null"}]`. Unwrap the | ||
| // non-null branches (resolving `$ref`) and classify on those. | ||
| for branch in resolved.one_of.iter().chain(resolved.any_of.iter()) { | ||
| let effective = branch | ||
| .schema_ref | ||
| .as_ref() | ||
| .and_then(|r| component_schemas.get(&strip_ref_prefix(r))) | ||
| .unwrap_or(branch); | ||
| if is_null_sentinel(effective) { | ||
| continue; | ||
| } | ||
| if is_binary_schema(effective) { | ||
| return (true, Some("application/octet-stream".to_string())); | ||
| } | ||
| } | ||
|
Comment on lines
+3491
to
+3506
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔵 suggestion Only one level of composition is unwrapped: an |
||
|
|
||
| (false, None) | ||
| } | ||
|
|
||
| /// `true` when the schema is a binary payload: `type: string, format: binary`, | ||
| /// the legacy `type: file`, or an array whose items are either of those. | ||
| fn is_binary_schema(schema: &OpenApiSchemaObject) -> bool { | ||
| let ty = schema.schema_type(); | ||
| if (ty == Some("string") && schema.format.as_deref() == Some("binary")) || ty == Some("file") { | ||
| return true; | ||
| } | ||
|
|
||
| // Array of binary files (e.g. `type: array, items: { type: string, format: binary }`) | ||
| if ty == Some("array") { | ||
| if let Some(items) = &resolved.items { | ||
| if (items.schema_type() == Some("string") | ||
| if let Some(items) = &schema.items { | ||
| return (items.schema_type() == Some("string") | ||
| && items.format.as_deref() == Some("binary")) | ||
| || items.schema_type() == Some("file") | ||
| { | ||
| return (true, Some("application/octet-stream".to_string())); | ||
| } | ||
| || items.schema_type() == Some("file"); | ||
| } | ||
| } | ||
|
Comment on lines
3519
to
3525
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟡 warning
|
||
|
|
||
| (false, None) | ||
| false | ||
| } | ||
|
|
||
| /// Recursively walk an object schema and emit one body-located [`MethodParameter`] | ||
|
|
@@ -4502,6 +4521,98 @@ paths: | |
| assert!(!purpose_field.required); | ||
| } | ||
|
|
||
| #[test] | ||
| fn test_multipart_nullable_anyof_binary_is_file_part() { | ||
| let yaml = r#" | ||
| openapi: "3.0.0" | ||
| info: { title: T, version: "1.0" } | ||
| servers: [{ url: "https://x.com" }] | ||
| paths: | ||
| /speech-to-text: | ||
| post: | ||
| x-fern-sdk-group-name: stt | ||
| x-fern-sdk-method-name: convert | ||
| operationId: sttConvert | ||
| requestBody: | ||
| content: | ||
| multipart/form-data: | ||
| schema: | ||
| type: object | ||
| required: [model_id] | ||
| properties: | ||
| model_id: | ||
| type: string | ||
| file: | ||
| anyOf: | ||
| - type: string | ||
| format: binary | ||
| - type: "null" | ||
| responses: { "200": { description: ok } } | ||
| "#; | ||
| let doc = load_openapi_spec(yaml, "t").unwrap(); | ||
| let convert = &doc.resources["stt"].methods["convert"]; | ||
| let file_field = convert | ||
| .multipart_fields | ||
| .iter() | ||
| .find(|f| f.wire_name == "file") | ||
| .expect("file field missing"); | ||
| assert!( | ||
| file_field.is_file, | ||
| "anyOf[binary, null] must be classified as a file part" | ||
| ); | ||
| assert_eq!( | ||
| file_field.content_type.as_deref(), | ||
| Some("application/octet-stream") | ||
| ); | ||
| assert!(!file_field.required); | ||
| } | ||
|
|
||
| #[test] | ||
| fn test_multipart_nullable_oneof_ref_binary_is_file_part() { | ||
| let yaml = r#" | ||
| openapi: "3.0.0" | ||
| info: { title: T, version: "1.0" } | ||
| servers: [{ url: "https://x.com" }] | ||
| paths: | ||
| /dubbing: | ||
| post: | ||
| x-fern-sdk-group-name: dubbing | ||
| x-fern-sdk-method-name: create | ||
| operationId: dubbingCreate | ||
| requestBody: | ||
| content: | ||
| multipart/form-data: | ||
| schema: | ||
| type: object | ||
| properties: | ||
| csv_file: | ||
| oneOf: | ||
| - $ref: '#/components/schemas/Upload' | ||
| - type: "null" | ||
| clips: | ||
| anyOf: | ||
| - type: array | ||
| items: { type: string, format: binary } | ||
| - type: "null" | ||
| responses: { "200": { description: ok } } | ||
| components: | ||
| schemas: | ||
| Upload: | ||
| type: string | ||
| format: binary | ||
| "#; | ||
| let doc = load_openapi_spec(yaml, "t").unwrap(); | ||
| let create = &doc.resources["dubbing"].methods["create"]; | ||
| for name in ["csv_file", "clips"] { | ||
| let field = create | ||
| .multipart_fields | ||
| .iter() | ||
| .find(|f| f.wire_name == name) | ||
| .unwrap_or_else(|| panic!("{name} field missing")); | ||
| assert!(field.is_file, "{name} must be classified as a file part"); | ||
| } | ||
| } | ||
|
|
||
| #[test] | ||
| fn test_multipart_form_data_with_ref_schema() { | ||
| let yaml = r#" | ||
|
|
||
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🔵 suggestion
Some("application/octet-stream".to_string())is now built in two places. Hoist it into aconst OCTET_STREAM: &str(or a tinyfn file_part()helper) so the two return paths can't drift.