diff --git a/src/blueprint/operators/grpc.rs b/src/blueprint/operators/grpc.rs index 4d97cc20be..e33cfd0cab 100644 --- a/src/blueprint/operators/grpc.rs +++ b/src/blueprint/operators/grpc.rs @@ -166,8 +166,8 @@ pub fn compile_grpc(inputs: CompileGrpc) -> Valid { Valid::from(GrpcMethod::try_from(grpc.method.as_str())) .and_then(|method| { Valid::from_option( - config_module.extensions.get_file_descriptor_set(&method), - format!("File descriptor not found for method: {}", grpc.method), + config_module.extensions.get_file_descriptor_set(), + "Protobuf files were not specified in the config".to_string(), ) .and_then(|file_descriptor_set| to_operation(&method, file_descriptor_set.clone())) .fuse(to_url(grpc, &method, config_module)) diff --git a/src/config/config_module.rs b/src/config/config_module.rs index 266c687b2f..bab6be8642 100644 --- a/src/config/config_module.rs +++ b/src/config/config_module.rs @@ -7,10 +7,10 @@ use jsonwebtoken::jwk::JwkSet; use prost_reflect::prost_types::FileDescriptorSet; use rustls_pki_types::{CertificateDer, PrivateKeyDer}; -use crate::blueprint::GrpcMethod; use crate::config::Config; use crate::macros::MergeRight; use crate::merge_right::MergeRight; +use crate::proto_reader::ProtoMetadata; use crate::rest::{EndpointSet, Unchecked}; use crate::scalar; @@ -42,8 +42,8 @@ impl Deref for Content { /// an IO operation, i.e., reading a file, making an HTTP call, etc. #[derive(Clone, Debug, Default, MergeRight)] pub struct Extensions { - /// Contains the file descriptor sets resolved from the links - pub grpc_file_descriptors: Vec>, + /// Contains the file descriptor set resolved from the links to proto files + pub grpc_file_descriptor_set: Option, /// Contains the contents of the JS file pub script: Option, @@ -63,16 +63,18 @@ pub struct Extensions { } impl Extensions { - pub fn get_file_descriptor_set(&self, grpc: &GrpcMethod) -> Option<&FileDescriptorSet> { - self.grpc_file_descriptors - .iter() - .find(|content| { - content - .file - .iter() - .any(|file| file.package == Some(grpc.package.to_owned())) - }) - .map(|a| &a.content) + pub fn add_proto(&mut self, metadata: ProtoMetadata) { + if let Some(set) = self.grpc_file_descriptor_set.as_mut() { + set.file.extend(metadata.descriptor_set.file); + } else { + let _ = self + .grpc_file_descriptor_set + .insert(metadata.descriptor_set); + } + } + + pub fn get_file_descriptor_set(&self) -> Option<&FileDescriptorSet> { + self.grpc_file_descriptor_set.as_ref() } pub fn has_auth(&self) -> bool { @@ -80,6 +82,14 @@ impl Extensions { } } +impl MergeRight for FileDescriptorSet { + fn merge_right(mut self, other: Self) -> Self { + self.file.extend(other.file); + + self + } +} + impl Deref for ConfigModule { type Target = Config; fn deref(&self) -> &Self::Target { @@ -153,3 +163,93 @@ impl From for ConfigModule { ConfigModule { config, input_types, output_types, ..Default::default() } } } + +#[cfg(test)] +mod tests { + mod extensions { + mod merge_right { + use std::path::Path; + + use prost_reflect::prost_types::FileDescriptorSet; + + use crate::config::Extensions; + use crate::merge_right::MergeRight; + + #[test] + fn grpc_file_descriptor_set_none() { + let extensions1 = Extensions::default(); + let extensions2 = Extensions::default(); + + assert_eq!( + extensions1 + .merge_right(extensions2) + .grpc_file_descriptor_set, + None + ); + } + + #[test] + fn grpc_file_descriptor_set_single() { + let greetings_path = Path::new("src/grpc/tests/proto/greetings.proto"); + + let file_descriptor_set = protox::compile([greetings_path], ["."]).unwrap(); + let extensions1 = Extensions { + grpc_file_descriptor_set: Some(file_descriptor_set.clone()), + ..Default::default() + }; + let extensions2 = Extensions::default(); + + assert_eq!( + extensions1 + .merge_right(extensions2) + .grpc_file_descriptor_set, + Some(file_descriptor_set.clone()) + ); + + let extensions1 = Extensions::default(); + let extensions2 = Extensions { + grpc_file_descriptor_set: Some(file_descriptor_set.clone()), + ..Default::default() + }; + + assert_eq!( + extensions1 + .merge_right(extensions2) + .grpc_file_descriptor_set, + Some(file_descriptor_set) + ); + } + + #[test] + fn grpc_file_descriptor_set_both() { + let greetings_path = Path::new("src/grpc/tests/proto/greetings.proto"); + let news_path = Path::new("src/grpc/tests/proto/news.proto"); + + let file_descriptor_set_greetings = + protox::compile([greetings_path], ["."]).unwrap(); + let file_descriptor_set_news = protox::compile([news_path], ["."]).unwrap(); + let extensions1 = Extensions { + grpc_file_descriptor_set: Some(file_descriptor_set_greetings.clone()), + ..Default::default() + }; + let extensions2 = Extensions { + grpc_file_descriptor_set: Some(file_descriptor_set_news.clone()), + ..Default::default() + }; + + assert_eq!( + extensions1 + .merge_right(extensions2) + .grpc_file_descriptor_set, + Some(FileDescriptorSet { + file: file_descriptor_set_greetings + .file + .into_iter() + .chain(file_descriptor_set_news.file) + .collect() + }) + ); + } + } + } +} diff --git a/src/config/reader.rs b/src/config/reader.rs index e9bd206860..93b6273c3c 100644 --- a/src/config/reader.rs +++ b/src/config/reader.rs @@ -81,13 +81,7 @@ impl ConfigReader { LinkType::Protobuf => { let path = Self::resolve_path(&link.src, parent_dir); let meta = self.proto_reader.read(path).await?; - config_module - .extensions - .grpc_file_descriptors - .push(Content { - id: link.id.clone(), - content: meta.descriptor_set.clone(), - }); + config_module.extensions.add_proto(meta); } LinkType::Script => { config_module.extensions.script = Some(content); diff --git a/src/grpc/data_loader_request.rs b/src/grpc/data_loader_request.rs index 891e760c9b..70c9a15751 100644 --- a/src/grpc/data_loader_request.rs +++ b/src/grpc/data_loader_request.rs @@ -98,7 +98,7 @@ mod tests { let protobuf_set = ProtobufSet::from_proto_file( config_module .extensions - .get_file_descriptor_set(&method) + .get_file_descriptor_set() .unwrap() .clone(), ) diff --git a/src/grpc/protobuf.rs b/src/grpc/protobuf.rs index 47fe97cec7..ec9ed9fca6 100644 --- a/src/grpc/protobuf.rs +++ b/src/grpc/protobuf.rs @@ -79,17 +79,12 @@ impl ProtobufSet { } pub fn find_service(&self, grpc_method: &GrpcMethod) -> Result { + let service_name = format!("{}.{}", grpc_method.package, grpc_method.service); + let service_descriptor = self .descriptor_pool - .get_service_by_name( - format!("{}.{}", grpc_method.package, grpc_method.service).as_str(), - ) - .with_context(|| { - format!( - "Couldn't find definitions for service {}", - grpc_method.service - ) - })?; + .get_service_by_name(&service_name) + .with_context(|| format!("Couldn't find definitions for service {service_name}"))?; Ok(ProtobufService { service_descriptor }) } @@ -304,7 +299,7 @@ pub mod tests { .resolve(config, None) .await? .extensions - .get_file_descriptor_set(&method) + .get_file_descriptor_set() .unwrap() .to_owned()) } @@ -353,7 +348,7 @@ pub mod tests { assert_eq!( error.to_string(), - "Couldn't find definitions for service _unknown" + "Couldn't find definitions for service greetings._unknown" ); Ok(()) diff --git a/src/grpc/request_template.rs b/src/grpc/request_template.rs index f5d01b205a..c09049340f 100644 --- a/src/grpc/request_template.rs +++ b/src/grpc/request_template.rs @@ -169,7 +169,7 @@ mod tests { .await .unwrap() .extensions - .get_file_descriptor_set(&method) + .get_file_descriptor_set() .unwrap() .clone(), ) diff --git a/tests/execution/grpc-proto-with-same-package.md b/tests/execution/grpc-proto-with-same-package.md new file mode 100644 index 0000000000..93d6d613e4 --- /dev/null +++ b/tests/execution/grpc-proto-with-same-package.md @@ -0,0 +1,83 @@ +# Grpc when multiple proto files have the same package name + +```protobuf @file:foo.proto +syntax = "proto3"; + +import "google/protobuf/empty.proto"; + +package test; + +message Foo { + string foo = 1; +} + +service FooService { + rpc GetFoo (google.protobuf.Empty) returns (Foo) {} +} +``` + +```protobuf @file:bar.proto +syntax = "proto3"; + +package test; + +message Input { + +} + +message Bar { + string bar = 1; +} + +service BarService { + rpc GetBar (Input) returns (Bar) {} +} +``` + +```graphql @server +schema + @server(port: 8000, graphiql: true) + @upstream(baseURL: "http://localhost:50051") + @link(src: "foo.proto", type: Protobuf) + @link(src: "bar.proto", type: Protobuf) { + query: Query +} + +type Query { + foo: Foo! @grpc(method: "test.FooService.GetFoo") + bar: Bar! @grpc(method: "test.BarService.GetBar") +} + +type Foo { + foo: String +} + +type Bar { + bar: String +} +``` + +```yml @mock +- request: + method: POST + url: http://localhost:50051/test.FooService/GetFoo + body: null + response: + status: 200 + body: \0\0\0\0\n\n\x08test-foo + +- request: + method: POST + url: http://localhost:50051/test.BarService/GetBar + body: null + response: + status: 200 + body: \0\0\0\0\n\n\x08test-bar +``` + +```yml @assert +- method: POST + url: http://localhost:8080/graphql + body: + query: query { foo { foo } bar { bar } } +``` diff --git a/tests/snapshots/execution_spec__grpc-proto-with-same-package.md_assert_0.snap b/tests/snapshots/execution_spec__grpc-proto-with-same-package.md_assert_0.snap new file mode 100644 index 0000000000..9b046b6ceb --- /dev/null +++ b/tests/snapshots/execution_spec__grpc-proto-with-same-package.md_assert_0.snap @@ -0,0 +1,20 @@ +--- +source: tests/execution_spec.rs +expression: response +--- +{ + "status": 200, + "headers": { + "content-type": "application/json" + }, + "body": { + "data": { + "foo": { + "foo": "test-foo" + }, + "bar": { + "bar": "test-bar" + } + } + } +} diff --git a/tests/snapshots/execution_spec__grpc-proto-with-same-package.md_client.snap b/tests/snapshots/execution_spec__grpc-proto-with-same-package.md_client.snap new file mode 100644 index 0000000000..a909f59e7d --- /dev/null +++ b/tests/snapshots/execution_spec__grpc-proto-with-same-package.md_client.snap @@ -0,0 +1,32 @@ +--- +source: tests/execution_spec.rs +expression: client +--- +type Bar { + bar: String +} + +scalar Date + +scalar Email + +scalar Empty + +type Foo { + foo: String +} + +scalar JSON + +scalar PhoneNumber + +type Query { + bar: Bar! + foo: Foo! +} + +scalar Url + +schema { + query: Query +} diff --git a/tests/snapshots/execution_spec__grpc-proto-with-same-package.md_merged.snap b/tests/snapshots/execution_spec__grpc-proto-with-same-package.md_merged.snap new file mode 100644 index 0000000000..f35c9b06e8 --- /dev/null +++ b/tests/snapshots/execution_spec__grpc-proto-with-same-package.md_merged.snap @@ -0,0 +1,20 @@ +--- +source: tests/execution_spec.rs +expression: merged +--- +schema @server(graphiql: true, port: 8000) @upstream(baseURL: "http://localhost:50051") @link(src: "foo.proto", type: Protobuf) @link(src: "bar.proto", type: Protobuf) { + query: Query +} + +type Bar { + bar: String +} + +type Foo { + foo: String +} + +type Query { + bar: Bar! @grpc(method: "test.BarService.GetBar") + foo: Foo! @grpc(method: "test.FooService.GetFoo") +} diff --git a/tests/snapshots/execution_spec__test-grpc-invalid-proto-id.md_errors.snap b/tests/snapshots/execution_spec__test-grpc-invalid-proto-id.md_errors.snap index 80c9a4c08f..3aa8391073 100644 --- a/tests/snapshots/execution_spec__test-grpc-invalid-proto-id.md_errors.snap +++ b/tests/snapshots/execution_spec__test-grpc-invalid-proto-id.md_errors.snap @@ -4,7 +4,7 @@ expression: errors --- [ { - "message": "File descriptor not found for method: abc.NewsService.GetAllNews", + "message": "Protobuf files were not specified in the config", "trace": [ "Query", "news", diff --git a/tests/snapshots/execution_spec__test-grpc-service.md_errors.snap b/tests/snapshots/execution_spec__test-grpc-service.md_errors.snap index c08d1c5a76..42cbd1e0b1 100644 --- a/tests/snapshots/execution_spec__test-grpc-service.md_errors.snap +++ b/tests/snapshots/execution_spec__test-grpc-service.md_errors.snap @@ -4,7 +4,7 @@ expression: errors --- [ { - "message": "Couldn't find definitions for service YourServiceName", + "message": "Couldn't find definitions for service news.YourServiceName", "trace": [ "Query", "news",