fix(grpc): linking multiple proto files with the same package name (#…

…1733)

src/blueprint/operators/grpc.rs

@@ -166,8 +166,8 @@ pub fn compile_grpc(inputs: CompileGrpc) -> Valid<Expression, String> {
     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))

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<A> Deref for Content<A> {
 /// 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<Content<FileDescriptorSet>>,
+    /// Contains the file descriptor set resolved from the links to proto files
+    pub grpc_file_descriptor_set: Option<FileDescriptorSet>,
 
     /// Contains the contents of the JS file
     pub script: Option<String>,
@@ -63,23 +63,33 @@ 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 {
         !self.htpasswd.is_empty() || !self.jwks.is_empty()
     }
 }
 
+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<Config> 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()
+                    })
+                );
+            }
+        }
+    }
+}

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);

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(),
         )

src/grpc/protobuf.rs

@@ -79,17 +79,12 @@ impl ProtobufSet {
     }
 
     pub fn find_service(&self, grpc_method: &GrpcMethod) -> Result<ProtobufService> {
+        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(())

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(),
         )

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 } }
+```

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"
+      }
+    }
+  }
+}

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
+}