From afde464e71cadbf2418c4a142f957e4b0d33766e Mon Sep 17 00:00:00 2001 From: Sandipsinh Dilipsinh Rathod <62684960+ssddOnTop@users.noreply.github.com> Date: Tue, 9 Jul 2024 17:50:59 +0530 Subject: [PATCH] fix(grpc): allow users to pass value along with mustache template (#2383) --- generated/.tailcallrc.graphql | 4 +- generated/.tailcallrc.schema.json | 6 +- src/core/blueprint/mustache.rs | 11 ++- src/core/blueprint/operators/grpc.rs | 2 +- src/core/config/config.rs | 2 +- src/core/generator/from_proto.rs | 3 +- src/core/grpc/request_template.rs | 30 +++++- src/core/helpers/body.rs | 28 ++++-- src/core/proto_reader/fetch.rs | 6 +- tests/core/snapshots/grpc-json.md_0.snap | 17 ++++ tests/core/snapshots/grpc-json.md_1.snap | 17 ++++ tests/core/snapshots/grpc-json.md_2.snap | 17 ++++ tests/core/snapshots/grpc-json.md_client.snap | 58 ++++++++++++ tests/core/snapshots/grpc-json.md_merged.snap | 28 ++++++ tests/execution/grpc-json.md | 91 +++++++++++++++++++ 15 files changed, 293 insertions(+), 27 deletions(-) create mode 100644 tests/core/snapshots/grpc-json.md_0.snap create mode 100644 tests/core/snapshots/grpc-json.md_1.snap create mode 100644 tests/core/snapshots/grpc-json.md_2.snap create mode 100644 tests/core/snapshots/grpc-json.md_client.snap create mode 100644 tests/core/snapshots/grpc-json.md_merged.snap create mode 100644 tests/execution/grpc-json.md diff --git a/generated/.tailcallrc.graphql b/generated/.tailcallrc.graphql index 53f9728e17..ec77d65704 100644 --- a/generated/.tailcallrc.graphql +++ b/generated/.tailcallrc.graphql @@ -109,7 +109,7 @@ directive @grpc( or use Mustache template for dynamic parameters. These parameters will be added in the body in `protobuf` format. """ - body: String + body: JSON """ The `headers` parameter allows you to customize the headers of the HTTP request made by the `@grpc` operator. It is used by specifying a key-value map of header names @@ -754,7 +754,7 @@ input Grpc { or use Mustache template for dynamic parameters. These parameters will be added in the body in `protobuf` format. """ - body: String + body: JSON """ The `headers` parameter allows you to customize the headers of the HTTP request made by the `@grpc` operator. It is used by specifying a key-value map of header names diff --git a/generated/.tailcallrc.schema.json b/generated/.tailcallrc.schema.json index 5f11afb62a..4e2248eef8 100644 --- a/generated/.tailcallrc.schema.json +++ b/generated/.tailcallrc.schema.json @@ -608,11 +608,7 @@ } }, "body": { - "description": "This refers to the arguments of your gRPC call. You can pass it as a static object or use Mustache template for dynamic parameters. These parameters will be added in the body in `protobuf` format.", - "type": [ - "string", - "null" - ] + "description": "This refers to the arguments of your gRPC call. You can pass it as a static object or use Mustache template for dynamic parameters. These parameters will be added in the body in `protobuf` format." }, "headers": { "description": "The `headers` parameter allows you to customize the headers of the HTTP request made by the `@grpc` operator. It is used by specifying a key-value map of header names and their values. Note: content-type is automatically set to application/grpc", diff --git a/src/core/blueprint/mustache.rs b/src/core/blueprint/mustache.rs index 6d39b2c7be..b1546c7232 100644 --- a/src/core/blueprint/mustache.rs +++ b/src/core/blueprint/mustache.rs @@ -158,9 +158,14 @@ impl FieldDefinition { ) .and_then(|_| { if let Some(body) = &req_template.body { - Valid::from_iter(body.expression_segments(), |parts| { - parts_validator.validate(parts, true).trace("body") - }) + if let Some(mustache) = &body.mustache { + Valid::from_iter(mustache.expression_segments(), |parts| { + parts_validator.validate(parts, true).trace("body") + }) + } else { + // TODO: needs review + Valid::succeed(Default::default()) + } } else { Valid::succeed(Default::default()) } diff --git a/src/core/blueprint/operators/grpc.rs b/src/core/blueprint/operators/grpc.rs index ffac9bdc42..f2bb3795c9 100644 --- a/src/core/blueprint/operators/grpc.rs +++ b/src/core/blueprint/operators/grpc.rs @@ -174,7 +174,7 @@ pub fn compile_grpc(inputs: CompileGrpc) -> Valid { to_operation(&method, file_descriptor_set) .fuse(to_url(grpc, &method, config_module)) .fuse(helpers::headers::to_mustache_headers(&grpc.headers)) - .fuse(helpers::body::to_body(grpc.body.as_deref())) + .fuse(helpers::body::to_body(grpc.body.as_ref())) .into() }) .and_then(|(operation, url, headers, body)| { diff --git a/src/core/config/config.rs b/src/core/config/config.rs index d8826612ea..c89fd063c1 100644 --- a/src/core/config/config.rs +++ b/src/core/config/config.rs @@ -665,7 +665,7 @@ pub struct Grpc { /// This refers to the arguments of your gRPC call. You can pass it as a /// static object or use Mustache template for dynamic parameters. These /// parameters will be added in the body in `protobuf` format. - pub body: Option, + pub body: Option, #[serde(rename = "batchKey", default, skip_serializing_if = "is_default")] /// The key path in the response which should be used to group multiple requests. For instance `["news","id"]`. For more details please refer out [n + 1 guide](https://tailcall.run/docs/guides/n+1#solving-using-batching). pub group_by: Vec, diff --git a/src/core/generator/from_proto.rs b/src/core/generator/from_proto.rs index b0ed4b130e..55b7fffe6b 100644 --- a/src/core/generator/from_proto.rs +++ b/src/core/generator/from_proto.rs @@ -6,6 +6,7 @@ use prost_reflect::prost_types::field_descriptor_proto::Label; use prost_reflect::prost_types::{ DescriptorProto, EnumDescriptorProto, FileDescriptorSet, ServiceDescriptorProto, SourceCodeInfo, }; +use serde_json::Value; use super::graphql_type::{GraphQLType, Unparsed}; use super::proto::comments_builder::CommentsBuilder; @@ -355,7 +356,7 @@ impl Context { default_value: None, }; - body = Some(format!("{{{{.args.{key}}}}}")); + body = Some(Value::String(format!("{{{{.args.{key}}}}}"))); cfg_field.args.insert(key, val); } diff --git a/src/core/grpc/request_template.rs b/src/core/grpc/request_template.rs index acd8bd3a67..71a42efd40 100644 --- a/src/core/grpc/request_template.rs +++ b/src/core/grpc/request_template.rs @@ -23,11 +23,27 @@ static GRPC_MIME_TYPE: HeaderValue = HeaderValue::from_static("application/grpc" pub struct RequestTemplate { pub url: Mustache, pub headers: MustacheHeaders, - pub body: Option, + pub body: Option, pub operation: ProtobufOperation, pub operation_type: GraphQLOperationType, } +#[derive(Default, Debug, Clone, PartialEq, Setters)] +pub struct RequestBody { + pub mustache: Option, + pub value: String, +} + +impl RequestBody { + pub fn render(&self, ctx: &C) -> String { + if let Some(mustache) = &self.mustache { + mustache.render(ctx) + } else { + self.value.to_string() + } + } +} + #[derive(Debug, Clone, PartialEq, Eq)] pub struct RenderedRequestTemplate { pub url: Url, @@ -123,7 +139,7 @@ mod tests { use pretty_assertions::assert_eq; use tailcall_fixtures::protobuf; - use super::RequestTemplate; + use super::{RequestBody, RequestTemplate}; use crate::core::blueprint::GrpcMethod; use crate::core::config::reader::ConfigReader; use crate::core::config::{Config, Field, GraphQLOperationType, Grpc, Link, LinkType, Type}; @@ -237,7 +253,10 @@ mod tests { url: Mustache::parse("http://localhost:3000/").unwrap(), headers: vec![], operation: get_protobuf_op().await, - body: Some(Mustache::parse(r#"{ "name": "test" }"#).unwrap()), + body: Some(RequestBody { + mustache: Some(Mustache::parse(r#"{ "name": "test" }"#).unwrap()), + value: Default::default(), + }), operation_type: GraphQLOperationType::Query, }; let ctx = Context::default(); @@ -254,7 +273,10 @@ mod tests { url: Mustache::parse("http://localhost:3000/").unwrap(), headers: vec![], operation: get_protobuf_op().await, - body: Some(Mustache::parse(body_str).unwrap()), + body: Some(RequestBody { + mustache: Some(Mustache::parse(body_str).unwrap()), + value: Default::default(), + }), operation_type: GraphQLOperationType::Query, } } diff --git a/src/core/helpers/body.rs b/src/core/helpers/body.rs index 5ca32640fc..8538c65bbd 100644 --- a/src/core/helpers/body.rs +++ b/src/core/helpers/body.rs @@ -1,21 +1,27 @@ +use serde_json::Value; + +use crate::core::grpc::request_template::RequestBody; use crate::core::mustache::Mustache; -use crate::core::valid::{Valid, ValidationError}; +use crate::core::valid::Valid; -pub fn to_body(body: Option<&str>) -> Valid, String> { +pub fn to_body(body: Option<&Value>) -> Valid, String> { let Some(body) = body else { return Valid::succeed(None); }; - Valid::from( - Mustache::parse(body) - .map(Some) - .map_err(|e| ValidationError::new(e.to_string())), - ) + let mut req_body = RequestBody::default(); + + let value = body.to_string(); + if let Ok(mustache) = Mustache::parse(&value) { + req_body = req_body.mustache(Some(mustache)); + } + Valid::succeed(Some(req_body.value(value))) } #[cfg(test)] mod tests { use super::to_body; + use crate::core::grpc::request_template::RequestBody; use crate::core::mustache::Mustache; use crate::core::valid::Valid; @@ -28,11 +34,15 @@ mod tests { #[test] fn body_parse_success() { - let result = to_body(Some("content")); + let value = serde_json::Value::String("content".to_string()); + let result = to_body(Some(&value)); assert_eq!( result, - Valid::succeed(Some(Mustache::parse("content").unwrap())) + Valid::succeed(Some(RequestBody { + mustache: Some(Mustache::parse(value.to_string().as_str()).unwrap()), + value: value.to_string() + })) ); } } diff --git a/src/core/proto_reader/fetch.rs b/src/core/proto_reader/fetch.rs index 6d70c87420..b7f63579f7 100644 --- a/src/core/proto_reader/fetch.rs +++ b/src/core/proto_reader/fetch.rs @@ -11,6 +11,7 @@ use serde_json::json; use crate::core::blueprint::GrpcMethod; use crate::core::config::ConfigReaderContext; use crate::core::grpc::protobuf::ProtobufSet; +use crate::core::grpc::request_template::RequestBody; use crate::core::grpc::RequestTemplate; use crate::core::mustache::Mustache; use crate::core::runtime::TargetRuntime; @@ -133,7 +134,10 @@ impl GrpcReflection { HeaderName::from_static("content-type"), Mustache::parse("application/grpc+proto")?, )], - body: Mustache::parse(body.to_string().as_str()).ok(), + body: Some(RequestBody { + mustache: Mustache::parse(body.to_string().as_str()).ok(), + value: Default::default(), + }), operation: operation.clone(), operation_type: Default::default(), }; diff --git a/tests/core/snapshots/grpc-json.md_0.snap b/tests/core/snapshots/grpc-json.md_0.snap new file mode 100644 index 0000000000..889f672334 --- /dev/null +++ b/tests/core/snapshots/grpc-json.md_0.snap @@ -0,0 +1,17 @@ +--- +source: tests/core/spec.rs +expression: response +--- +{ + "status": 200, + "headers": { + "content-type": "application/json" + }, + "body": { + "data": { + "newsById": { + "id": 2 + } + } + } +} diff --git a/tests/core/snapshots/grpc-json.md_1.snap b/tests/core/snapshots/grpc-json.md_1.snap new file mode 100644 index 0000000000..39d361b21d --- /dev/null +++ b/tests/core/snapshots/grpc-json.md_1.snap @@ -0,0 +1,17 @@ +--- +source: tests/core/spec.rs +expression: response +--- +{ + "status": 200, + "headers": { + "content-type": "application/json" + }, + "body": { + "data": { + "newsByIdMustache": { + "id": 2 + } + } + } +} diff --git a/tests/core/snapshots/grpc-json.md_2.snap b/tests/core/snapshots/grpc-json.md_2.snap new file mode 100644 index 0000000000..f312fe8482 --- /dev/null +++ b/tests/core/snapshots/grpc-json.md_2.snap @@ -0,0 +1,17 @@ +--- +source: tests/core/spec.rs +expression: response +--- +{ + "status": 200, + "headers": { + "content-type": "application/json" + }, + "body": { + "data": { + "newsByIdMustacheAndJson": { + "id": 2 + } + } + } +} diff --git a/tests/core/snapshots/grpc-json.md_client.snap b/tests/core/snapshots/grpc-json.md_client.snap new file mode 100644 index 0000000000..e53a55797e --- /dev/null +++ b/tests/core/snapshots/grpc-json.md_client.snap @@ -0,0 +1,58 @@ +--- +source: tests/core/spec.rs +expression: formatted +--- +scalar Bytes + +scalar Date + +scalar Email + +scalar Empty + +scalar Int128 + +scalar Int16 + +scalar Int32 + +scalar Int64 + +scalar Int8 + +scalar JSON + +type News { + body: String + id: Int + postImage: String + title: String +} + +input NewsInput { + id: Int +} + +scalar PhoneNumber + +type Query { + newsById: News! + newsByIdMustache(news: NewsInput!): News! + newsByIdMustacheAndJson(news: NewsInput!): News! +} + +scalar UInt128 + +scalar UInt16 + +scalar UInt32 + +scalar UInt64 + +scalar UInt8 + +scalar Url + +schema { + query: Query +} diff --git a/tests/core/snapshots/grpc-json.md_merged.snap b/tests/core/snapshots/grpc-json.md_merged.snap new file mode 100644 index 0000000000..68c32fb712 --- /dev/null +++ b/tests/core/snapshots/grpc-json.md_merged.snap @@ -0,0 +1,28 @@ +--- +source: tests/core/spec.rs +expression: formatter +--- +schema + @server(port: 8000) + @upstream(baseURL: "http://localhost:50051") + @link(id: "news", src: "news.proto", type: Protobuf) { + query: Query +} + +input NewsInput { + id: Int +} + +type News { + body: String + id: Int + postImage: String + title: String +} + +type Query { + newsById: News! @grpc(body: {id: 2}, method: "news.NewsService.GetNews") + newsByIdMustache(news: NewsInput!): News! @grpc(body: "{{.args.news}}", method: "news.NewsService.GetNews") + newsByIdMustacheAndJson(news: NewsInput!): News! + @grpc(body: {id: "{{.args.news.id}}"}, method: "news.NewsService.GetNews") +} diff --git a/tests/execution/grpc-json.md b/tests/execution/grpc-json.md new file mode 100644 index 0000000000..fa80cd023a --- /dev/null +++ b/tests/execution/grpc-json.md @@ -0,0 +1,91 @@ +# Grpc datasource + +```protobuf @file:news.proto +syntax = "proto3"; + +import "google/protobuf/empty.proto"; + +package news; + +message News { + int32 id = 1; + string title = 2; + string body = 3; + string postImage = 4; +} + +service NewsService { + rpc GetAllNews (google.protobuf.Empty) returns (NewsList) {} + rpc GetNews (NewsId) returns (News) {} + rpc GetMultipleNews (MultipleNewsId) returns (NewsList) {} + rpc DeleteNews (NewsId) returns (google.protobuf.Empty) {} + rpc EditNews (News) returns (News) {} + rpc AddNews (News) returns (News) {} +} + +message NewsId { + int32 id = 1; +} + +message MultipleNewsId { + repeated NewsId ids = 1; +} + +message NewsList { + repeated News news = 1; +} +``` + +```graphql @config +schema + @server(port: 8000) + @upstream(baseURL: "http://localhost:50051") + @link(id: "news", src: "news.proto", type: Protobuf) { + query: Query +} + +type Query { + newsById: News! @grpc(method: "news.NewsService.GetNews", body: {id: 2}) + newsByIdMustache(news: NewsInput!): News! @grpc(method: "news.NewsService.GetNews", body: "{{.args.news}}") + newsByIdMustacheAndJson(news: NewsInput!): News! + @grpc(method: "news.NewsService.GetNews", body: {id: "{{.args.news.id}}"}) +} + +input NewsInput { + id: Int +} + +type News { + id: Int + title: String + body: String + postImage: String +} +``` + +```yml @mock +- request: + method: POST + url: http://localhost:50051/news.NewsService/GetNews + response: + status: 200 + textBody: \0\0\0\0#\x08\x02\x12\x06Note 2\x1a\tContent 2\"\x0cPost image 2 + expectedHits: 3 +``` + +```yml @test +- method: POST + url: http://localhost:8080/graphql + body: + query: query { newsById { id } } + +- method: POST + url: http://localhost:8080/graphql + body: + query: "query { newsByIdMustache(news: {id: 2}) { id } }" + +- method: POST + url: http://localhost:8080/graphql + body: + query: "query { newsByIdMustacheAndJson(news: {id: 2}) { id } }" +```