diff --git a/apollo-router/tests/fixtures/file_upload/multiple_subgraph.graphql b/apollo-router/tests/fixtures/file_upload/multiple_subgraph.graphql deleted file mode 100644 index 369b305118..0000000000 --- a/apollo-router/tests/fixtures/file_upload/multiple_subgraph.graphql +++ /dev/null @@ -1,122 +0,0 @@ -schema - @link(url: "https://specs.apollo.dev/link/v1.0") - @link(url: "https://specs.apollo.dev/join/v0.3", for: EXECUTION) -{ - query: Query - mutation: Mutation -} - -directive @join__enumValue(graph: join__Graph!) repeatable on ENUM_VALUE - -directive @join__field(graph: join__Graph, requires: join__FieldSet, provides: join__FieldSet, type: String, external: Boolean, override: String, usedOverridden: Boolean) repeatable on FIELD_DEFINITION | INPUT_FIELD_DEFINITION - -directive @join__graph(name: String!, url: String!) on ENUM_VALUE - -directive @join__implements(graph: join__Graph!, interface: String!) repeatable on OBJECT | INTERFACE - -directive @join__type(graph: join__Graph!, key: join__FieldSet, extension: Boolean! = false, resolvable: Boolean! = true, isInterfaceObject: Boolean! = false) repeatable on OBJECT | INTERFACE | UNION | ENUM | INPUT_OBJECT | SCALAR - -directive @join__unionMember(graph: join__Graph!, member: String!) repeatable on UNION - -directive @link(url: String, as: String, for: link__Purpose, import: [link__Import]) repeatable on SCHEMA - -scalar join__FieldSet - -enum join__Graph { - ACCOUNTS @join__graph(name: "accounts", url: "https://accounts.demo.starstuff.dev/") - INVENTORY @join__graph(name: "inventory", url: "https://inventory.demo.starstuff.dev/") - PRODUCTS @join__graph(name: "products", url: "https://products.demo.starstuff.dev/") - REVIEWS @join__graph(name: "reviews", url: "https://reviews.demo.starstuff.dev/") - UPLOADS1 @join__graph(name: "uploads1", url: "http://127.0.0.1:4005/s1") - UPLOADS2 @join__graph(name: "uploads2", url: "http://127.0.0.1:4005/s2") -} - -scalar link__Import - -enum link__Purpose { - """ - `SECURITY` features provide metadata necessary to securely resolve fields. - """ - SECURITY - - """ - `EXECUTION` features provide metadata necessary for operation execution. - """ - EXECUTION -} - -type Product - @join__type(graph: INVENTORY, key: "upc") - @join__type(graph: PRODUCTS, key: "upc") - @join__type(graph: REVIEWS, key: "upc") -{ - upc: String! - weight: Int @join__field(graph: INVENTORY, external: true) @join__field(graph: PRODUCTS) - price: Int @join__field(graph: INVENTORY, external: true) @join__field(graph: PRODUCTS) - inStock: Boolean @join__field(graph: INVENTORY) - shippingEstimate: Int @join__field(graph: INVENTORY, requires: "price weight") - name: String @join__field(graph: PRODUCTS) - reviews: [Review] @join__field(graph: REVIEWS) - reviewsForAuthor(authorID: ID!): [Review] @join__field(graph: REVIEWS) -} - -type Query - @join__type(graph: ACCOUNTS) - @join__type(graph: INVENTORY) - @join__type(graph: PRODUCTS) - @join__type(graph: REVIEWS) -{ - me: User @join__field(graph: ACCOUNTS) - topProducts(first: Int = 5): [Product] @join__field(graph: PRODUCTS) -} - -type Review - @join__type(graph: REVIEWS, key: "id") -{ - id: ID! - body: String - author: User @join__field(graph: REVIEWS, provides: "username") - product: Product -} - -type User - @join__type(graph: ACCOUNTS, key: "id") - @join__type(graph: REVIEWS, key: "id") -{ - id: ID! - name: String @join__field(graph: ACCOUNTS) - username: String @join__field(graph: ACCOUNTS) @join__field(graph: REVIEWS, external: true) - reviews: [Review] @join__field(graph: REVIEWS) -} - - -scalar Upload1 - @join__type(graph: UPLOADS1) - -scalar Upload2 - @join__type(graph: UPLOADS2) - -type File1 - @join__type(graph: UPLOADS1) -{ - filename: String! - mimetype: String! - encoding: String! - body: String! -} -type File2 - @join__type(graph: UPLOADS2) -{ - filename: String! - mimetype: String! - encoding: String! - body: String! -} - -type Mutation - @join__type(graph: UPLOADS1) - @join__type(graph: UPLOADS2) -{ - singleUpload1(file: Upload1): File1 @join__field(graph: UPLOADS1) - singleUpload2(file: Upload2): File2 @join__field(graph: UPLOADS2) -} diff --git a/apollo-router/tests/fixtures/file_upload/single_subgraph.graphql b/apollo-router/tests/fixtures/file_upload/schema.graphql similarity index 50% rename from apollo-router/tests/fixtures/file_upload/single_subgraph.graphql rename to apollo-router/tests/fixtures/file_upload/schema.graphql index 23d2733cb4..07e301012c 100644 --- a/apollo-router/tests/fixtures/file_upload/single_subgraph.graphql +++ b/apollo-router/tests/fixtures/file_upload/schema.graphql @@ -23,11 +23,8 @@ directive @link(url: String, as: String, for: link__Purpose, import: [link__Impo scalar join__FieldSet enum join__Graph { - ACCOUNTS @join__graph(name: "accounts", url: "https://accounts.demo.starstuff.dev/") - INVENTORY @join__graph(name: "inventory", url: "https://inventory.demo.starstuff.dev/") - PRODUCTS @join__graph(name: "products", url: "https://products.demo.starstuff.dev/") - REVIEWS @join__graph(name: "reviews", url: "https://reviews.demo.starstuff.dev/") UPLOADS @join__graph(name: "uploads", url: "http://127.0.0.1:4005/") + UPLOADS_CLONE @join__graph(name: "uploads_clone", url: "http://127.0.0.1:4006/") } scalar link__Import @@ -44,53 +41,11 @@ enum link__Purpose { EXECUTION } -type Product - @join__type(graph: INVENTORY, key: "upc") - @join__type(graph: PRODUCTS, key: "upc") - @join__type(graph: REVIEWS, key: "upc") -{ - upc: String! - weight: Int @join__field(graph: INVENTORY, external: true) @join__field(graph: PRODUCTS) - price: Int @join__field(graph: INVENTORY, external: true) @join__field(graph: PRODUCTS) - inStock: Boolean @join__field(graph: INVENTORY) - shippingEstimate: Int @join__field(graph: INVENTORY, requires: "price weight") - name: String @join__field(graph: PRODUCTS) - reviews: [Review] @join__field(graph: REVIEWS) - reviewsForAuthor(authorID: ID!): [Review] @join__field(graph: REVIEWS) -} - -type Query - @join__type(graph: ACCOUNTS) - @join__type(graph: INVENTORY) - @join__type(graph: PRODUCTS) - @join__type(graph: REVIEWS) -{ - me: User @join__field(graph: ACCOUNTS) - topProducts(first: Int = 5): [Product] @join__field(graph: PRODUCTS) -} - -type Review - @join__type(graph: REVIEWS, key: "id") -{ - id: ID! - body: String - author: User @join__field(graph: REVIEWS, provides: "username") - product: Product -} - -type User - @join__type(graph: ACCOUNTS, key: "id") - @join__type(graph: REVIEWS, key: "id") -{ - id: ID! - name: String @join__field(graph: ACCOUNTS) - username: String @join__field(graph: ACCOUNTS) @join__field(graph: REVIEWS, external: true) - reviews: [Review] @join__field(graph: REVIEWS) -} - scalar Upload @join__type(graph: UPLOADS) +scalar UploadClone + @join__type(graph: UPLOADS_CLONE) type File @join__type(graph: UPLOADS) @@ -100,9 +55,34 @@ type File encoding: String! body: String! } +type FileClone + @join__type(graph: UPLOADS_CLONE) +{ + filename: String! + mimetype: String! + encoding: String! + body: String! +} + +input NestedUpload + @join__type(graph: UPLOADS) +{ + file: Upload! +} + +type Query + @join__type(graph: UPLOADS) + @join__type(graph: UPLOADS_CLONE) +{ + empty: String +} type Mutation @join__type(graph: UPLOADS) + @join__type(graph: UPLOADS_CLONE) { singleUpload(file: Upload): File @join__field(graph: UPLOADS) + singleUploadClone(file: UploadClone): FileClone @join__field(graph: UPLOADS_CLONE) + multiUpload(files: [Upload!]!): [File!]! @join__field(graph: UPLOADS) + nestedUpload(nested: NestedUpload): File @join__field(graph: UPLOADS) } diff --git a/apollo-router/tests/integration/file_upload.rs b/apollo-router/tests/integration/file_upload.rs index cef8bd1d81..d1ff7a97c8 100644 --- a/apollo-router/tests/integration/file_upload.rs +++ b/apollo-router/tests/integration/file_upload.rs @@ -1,5 +1,4 @@ use std::collections::BTreeMap; -use std::path::PathBuf; use bytes::Bytes; use http::header::CONTENT_ENCODING; @@ -40,12 +39,6 @@ async fn it_uploads_a_single_file() -> Result<(), BoxError> { .handler(make_handler!(helper::echo_single_file)) .request(request) .subgraph_mapping("uploads", "/") - .supergraph(PathBuf::from_iter([ - "tests", - "fixtures", - "file_upload", - "single_subgraph.graphql", - ])) .build() .run_test(|response| { insta::assert_json_snapshot!(response, @r###" @@ -100,12 +93,6 @@ async fn it_uploads_multiple_files() -> Result<(), BoxError> { .handler(make_handler!(helper::echo_files)) .request(request) .subgraph_mapping("uploads", "/") - .supergraph(PathBuf::from_iter([ - "tests", - "fixtures", - "file_upload", - "single_subgraph.graphql", - ])) .build() .run_test(move |response| { insta::assert_json_snapshot!(response, @r###" @@ -160,12 +147,6 @@ async fn it_uploads_a_massive_file() -> Result<(), BoxError> { .handler(make_handler!(helper::verify_stream).with_state((TEN_GB, 0xAA))) .request(request) .subgraph_mapping("uploads", "/") - .supergraph(PathBuf::from_iter([ - "tests", - "fixtures", - "file_upload", - "single_subgraph.graphql", - ])) .build() .run_test(|response| { insta::assert_json_snapshot!(response, @r###" @@ -193,9 +174,9 @@ async fn it_uploads_to_multiple_subgraphs() -> Result<(), BoxError> { "operations", Part::text( serde_json::json!({ - "query": "mutation SomeMutation($file0: Upload1, $file1: Upload2) { - file0: singleUpload1(file: $file0) { filename body } - file1: singleUpload2(file: $file1) { filename body } + "query": "mutation SomeMutation($file0: Upload, $file1: UploadClone) { + file0: singleUpload(file: $file0) { filename body } + file1: singleUploadClone(file: $file1) { filename body } }", "variables": { "file0": null, @@ -226,14 +207,8 @@ async fn it_uploads_to_multiple_subgraphs() -> Result<(), BoxError> { "/s2" => helper::echo_single_file )) .request(request) - .subgraph_mapping("uploads1", "/s1") - .subgraph_mapping("uploads2", "/s2") - .supergraph(PathBuf::from_iter([ - "tests", - "fixtures", - "file_upload", - "multiple_subgraph.graphql", - ])) + .subgraph_mapping("uploads", "/s1") + .subgraph_mapping("uploads_clone", "/s2") .build() .run_test(|response| { insta::assert_json_snapshot!(response, @r###" @@ -381,12 +356,6 @@ async fn it_supports_compression() -> Result<(), BoxError> { .handler(make_handler!(helper::echo_single_file)) .request(Form::new()) // Gets overwritten by the `compress` transformer .subgraph_mapping("uploads", "/") - .supergraph(PathBuf::from_iter([ - "tests", - "fixtures", - "file_upload", - "single_subgraph.graphql", - ])) .transformer(compress) .build() .run_test(|request| { @@ -404,6 +373,132 @@ async fn it_supports_compression() -> Result<(), BoxError> { .await } +#[tokio::test(flavor = "multi_thread")] +async fn it_supports_nested_file() -> Result<(), BoxError> { + use reqwest::multipart::Form; + use reqwest::multipart::Part; + + // Construct a manual request that sets up a nested structure containing a file to upload + let request = Form::new() + .part( + "operations", + Part::text( + serde_json::json!({ + "query": "mutation SomeMutation($file0: NestedUpload) { + file0: nestedUpload(nested: $file0) { filename body } + }", + "variables": { + "file0": { + "file": null, + }, + }, + }) + .to_string(), + ), + ) + .part( + "map", + Part::text( + serde_json::json!({ + "0": ["variables.file0.file"], + }) + .to_string(), + ), + ) + .part("0", Part::text("file0 contents").file_name("file0")); + + helper::FileUploadTestServer::builder() + .config(FILE_CONFIG) + .handler(make_handler!(helper::echo_single_file)) + .request(request) + .subgraph_mapping("uploads", "/") + .build() + .run_test(|request| { + insta::assert_json_snapshot!(request, @r###" + { + "data": { + "file0": { + "filename": "file0", + "body": "file0 contents" + } + } + } + "###); + }) + .await +} + +#[tokio::test(flavor = "multi_thread")] +async fn it_supports_nested_file_list() -> Result<(), BoxError> { + use reqwest::multipart::Form; + use reqwest::multipart::Part; + + // Construct a manual request that sets up a nested structure containing a file to upload + let request = Form::new() + .part( + "operations", + Part::text( + serde_json::json!({ + "query": "mutation SomeMutation($files: [Upload!]!) { + files: multiUpload(files: $files) { filename body } + }", + "variables": { + "files": { + "0": null, + "1": null, + "2": null, + }, + }, + }) + .to_string(), + ), + ) + .part( + "map", + Part::text( + serde_json::json!({ + "0": ["variables.files.0"], + "1": ["variables.files.1"], + "2": ["variables.files.2"], + }) + .to_string(), + ), + ) + .part("0", Part::text("file0 contents").file_name("file0")) + .part("1", Part::text("file1 contents").file_name("file1")) + .part("2", Part::text("file2 contents").file_name("file2")); + + helper::FileUploadTestServer::builder() + .config(FILE_CONFIG) + .handler(make_handler!(helper::echo_file_list)) + .request(request) + .subgraph_mapping("uploads", "/") + .build() + .run_test(|request| { + insta::assert_json_snapshot!(request, @r###" + { + "data": { + "files": [ + { + "filename": "file0", + "body": "file0 contents" + }, + { + "filename": "file1", + "body": "file1 contents" + }, + { + "filename": "file2", + "body": "file2 contents" + } + ] + } + } + "###); + }) + .await +} + #[tokio::test(flavor = "multi_thread")] async fn it_fails_upload_without_file() -> Result<(), BoxError> { // Construct a request with no attached files @@ -415,12 +510,6 @@ async fn it_fails_upload_without_file() -> Result<(), BoxError> { .handler(make_handler!(helper::always_fail)) .request(request) .subgraph_mapping("uploads", "/") - .supergraph(PathBuf::from_iter([ - "tests", - "fixtures", - "file_upload", - "single_subgraph.graphql", - ])) .build() .run_test(|response| { insta::assert_json_snapshot!(response, @r###" @@ -461,12 +550,6 @@ async fn it_fails_with_file_count_limits() -> Result<(), BoxError> { .handler(make_handler!(helper::always_fail)) .request(request) .subgraph_mapping("uploads", "/") - .supergraph(PathBuf::from_iter([ - "tests", - "fixtures", - "file_upload", - "single_subgraph.graphql", - ])) .build() .run_test(|response| { insta::assert_json_snapshot!(response, @r###" @@ -505,12 +588,6 @@ async fn it_fails_with_file_size_limit() -> Result<(), BoxError> { .handler(make_handler!(helper::always_fail)) .request(request) .subgraph_mapping("uploads", "/") - .supergraph(PathBuf::from_iter([ - "tests", - "fixtures", - "file_upload", - "single_subgraph.graphql", - ])) .build() .run_test(|response| { insta::assert_json_snapshot!(response, @r###" @@ -561,12 +638,6 @@ async fn it_fails_invalid_multipart_order() -> Result<(), BoxError> { .handler(make_handler!(helper::always_fail)) .request(request) .subgraph_mapping("uploads", "/") - .supergraph(PathBuf::from_iter([ - "tests", - "fixtures", - "file_upload", - "single_subgraph.graphql", - ])) .build() .run_test(|response| { insta::assert_json_snapshot!(response, @r###" @@ -596,9 +667,9 @@ async fn it_fails_invalid_file_order() -> Result<(), BoxError> { "operations", Part::text( serde_json::json!({ - "query": "mutation ($file0: Upload1, $file1: Upload2) { - file0: singleUpload1(file: $file0) { filename body } - file1: singleUpload2(file: $file1) { filename body } + "query": "mutation ($file0: Upload, $file1: UploadClone) { + file0: singleUpload(file: $file0) { filename body } + file1: singleUploadClone(file: $file1) { filename body } }", "variables": { "file0": null, @@ -629,14 +700,8 @@ async fn it_fails_invalid_file_order() -> Result<(), BoxError> { "/s2" => helper::always_fail )) .request(request) - .subgraph_mapping("uploads1", "/s1") - .subgraph_mapping("uploads2", "/s2") - .supergraph(PathBuf::from_iter([ - "tests", - "fixtures", - "file_upload", - "multiple_subgraph.graphql", - ])) + .subgraph_mapping("uploads", "/s1") + .subgraph_mapping("uploads_clone", "/s2") .build() .run_test(|response| { insta::assert_json_snapshot!(response, @r###" @@ -650,12 +715,12 @@ async fn it_fails_invalid_file_order() -> Result<(), BoxError> { }, "errors": [ { - "message": "HTTP fetch failed from 'uploads2': HTTP fetch failed from 'uploads2': error from user's HttpBody stream: error reading a body from connection: Missing files in the request: '1'.", + "message": "HTTP fetch failed from 'uploads_clone': HTTP fetch failed from 'uploads_clone': error from user's HttpBody stream: error reading a body from connection: Missing files in the request: '1'.", "path": [], "extensions": { "code": "SUBREQUEST_HTTP_ERROR", - "service": "uploads2", - "reason": "HTTP fetch failed from 'uploads2': error from user's HttpBody stream: error reading a body from connection: Missing files in the request: '1'." + "service": "uploads_clone", + "reason": "HTTP fetch failed from 'uploads_clone': error from user's HttpBody stream: error reading a body from connection: Missing files in the request: '1'." } } ] @@ -689,12 +754,6 @@ async fn it_fails_with_no_boundary_in_multipart() -> Result<(), BoxError> { .handler(make_handler!(helper::always_fail)) .request(request) .subgraph_mapping("uploads", "/") - .supergraph(PathBuf::from_iter([ - "tests", - "fixtures", - "file_upload", - "single_subgraph.graphql", - ])) .transformer(strip_boundary) .build() .run_test(|response| { @@ -727,9 +786,9 @@ async fn it_fails_incompatible_query_order() -> Result<(), BoxError> { "operations", Part::text( serde_json::json!({ - "query": "mutation SomeMutation($file0: Upload2, $file1: Upload1) { - file1: singleUpload1(file: $file1) { filename } - file0: singleUpload2(file: $file0) { filename } + "query": "mutation SomeMutation($file0: UploadClone, $file1: Upload) { + file1: singleUpload(file: $file1) { filename } + file0: singleUploadClone(file: $file0) { filename } }", "variables": { "file0": null, @@ -760,14 +819,8 @@ async fn it_fails_incompatible_query_order() -> Result<(), BoxError> { "/s2" => helper::always_fail )) .request(request) - .subgraph_mapping("uploads1", "/s1") - .subgraph_mapping("uploads2", "/s2") - .supergraph(PathBuf::from_iter([ - "tests", - "fixtures", - "file_upload", - "multiple_subgraph.graphql", - ])) + .subgraph_mapping("uploads", "/s1") + .subgraph_mapping("uploads_clone", "/s2") .build() .run_test(|response| { insta::assert_json_snapshot!(response, @r###" @@ -834,7 +887,6 @@ mod helper { handler: Router, request: Form, subgraph_mappings: HashMap, - supergraph: PathBuf, transformer: Option reqwest::Request>, } @@ -851,7 +903,6 @@ mod helper { handler: Router, subgraph_mappings: HashMap, request: Form, - supergraph: PathBuf, transformer: Option reqwest::Request>, ) -> Self { Self { @@ -859,7 +910,6 @@ mod helper { handler, request, subgraph_mappings, - supergraph, transformer, } } @@ -894,7 +944,12 @@ mod helper { .map(|(name, path)| (name, format!("{bound_url}{path}"))) .collect(), ) - .supergraph(self.supergraph) + .supergraph(PathBuf::from_iter([ + "tests", + "fixtures", + "file_upload", + "schema.graphql", + ])) .build() .await; @@ -959,6 +1014,9 @@ mod helper { #[error("expected a file with name '{0}' but found nothing")] MissingFile(String), + #[error("expected a list of files but found nothing")] + MissingList, + #[error("expected a set of mappings but found nothing")] MissingMapping, @@ -1141,6 +1199,69 @@ mod helper { }))) } + /// Handler that echos back the contents of the list of files that it receives + pub async fn echo_file_list( + mut request: Request, + ) -> Result, FileUploadError> { + let (operation, map, mut multipart) = decode_request(&mut request).await?; + + // Make sure that we have some mappings + if map.is_empty() { + return Err(FileUploadError::MissingMapping); + } + + // Make sure that we have one list input + let file_list = { + let Some((_, list)) = operation.variables.first_key_value() else { + return Err(FileUploadError::MissingList); + }; + + let Some(list) = list.as_object() else { + return Err(FileUploadError::MissingList); + }; + + list + }; + + // Make sure that the list has the correct amount of slots for the files + if file_list.len() != map.len() { + return Err(FileUploadError::VariableMismatch( + map.len(), + file_list.len(), + )); + } + + // Extract all of the files + let mut files = Vec::new(); + for file_mapping in map.into_keys() { + let f = multipart + .next_field() + .await? + .ok_or(FileUploadError::MissingFile(file_mapping.clone()))?; + + let field_name = f + .name() + .and_then(|name| (name == file_mapping).then_some(name)) + .ok_or(FileUploadError::UnexpectedField( + file_mapping, + f.name().map(String::from), + ))?; + let file_name = f.file_name().unwrap_or(field_name).to_string(); + let body = f.bytes().await?; + + files.push(Upload { + filename: Some(file_name), + body: Some(String::from_utf8_lossy(&body).to_string()), + }); + } + + Ok(Json(json!({ + "data": { + "files": files, + }, + }))) + } + /// A handler that always fails. Useful for tests that should not reach the subgraph at all. pub async fn always_fail(mut request: Request) -> Result, FileUploadError> { // Consume the stream