fix: handle required for lists for proto (#2681)

Co-authored-by: Tushar Mathur <[email protected]>
tailcallhq · Aug 16, 2024 · 315b06d · 315b06d · github-actions · Aug 16, 2024
1 parent 4a2c557
commit 315b06d
Show file tree

Hide file tree

Showing 53 changed files with 307 additions and 134 deletions.
diff --git a/examples/grpc-reflection.graphql b/examples/grpc-reflection.graphql
@@ -32,5 +32,5 @@ input NewsInput {
 }
 
 type NewsData {
-  news: [News]!
+  news: [News]
 }
diff --git a/examples/grpc.graphql b/examples/grpc.graphql
@@ -32,5 +32,5 @@ input NewsInput {
 }
 
 type NewsData {
-  news: [News]!
+  news: [News]
 }
diff --git a/examples/telemetry-otlp.graphql b/examples/telemetry-otlp.graphql
@@ -58,5 +58,5 @@ enum Status {
 }
 
 type NewsData {
-  news: [News]!
+  news: [News]
 }
diff --git a/src/core/blueprint/from_config.rs b/src/core/blueprint/from_config.rs
@@ -1,4 +1,4 @@
-use std::collections::{BTreeMap, BTreeSet, HashMap};
+use std::collections::{BTreeMap, BTreeSet};
 
 use async_graphql::dynamic::SchemaBuilder;
 
@@ -71,7 +71,7 @@ pub fn to_json_schema_for_field(field: &Field, config: &Config) -> JsonSchema {
     to_json_schema(field, config)
 }
 pub fn to_json_schema_for_args(args: &BTreeMap<String, Arg>, config: &Config) -> JsonSchema {
-    let mut schema_fields = HashMap::new();
+    let mut schema_fields = BTreeMap::new();
     for (name, arg) in args.iter() {
         schema_fields.insert(name.clone(), to_json_schema(arg, config));
     }
@@ -87,7 +87,7 @@ where
     let type_ = config.find_type(type_of);
     let type_enum_ = config.find_enum(type_of);
     let schema = if let Some(type_) = type_ {
-        let mut schema_fields = HashMap::new();
+        let mut schema_fields = BTreeMap::new();
         for (name, field) in type_.fields.iter() {
             if field.resolver.is_none() {
                 schema_fields.insert(name.clone(), to_json_schema_for_field(field, config));

diff --git a/src/core/blueprint/operators/grpc.rs b/src/core/blueprint/operators/grpc.rs
@@ -73,15 +73,14 @@ fn validate_schema(
 
     Valid::from(JsonSchema::try_from(input_type))
         .zip(Valid::from(JsonSchema::try_from(output_type)))
-        .and_then(|(_input_schema, output_schema)| {
+        .and_then(|(_input_schema, sub_type)| {
             // TODO: add validation for input schema - should compare result grpc.body to
             // schema
-            let fields = field_schema.field;
-            let _args = field_schema.args;
+            let super_type = field_schema.field;
             // TODO: all of the fields in protobuf are optional actually
             // and if we want to mark some fields as required in GraphQL
             // JsonSchema won't match and the validation will fail
-            fields.compare(&output_schema, name)
+            sub_type.is_a(&super_type, name)
         })
 }
 
@@ -108,10 +107,9 @@ fn validate_group_by(
             // TODO: add validation for input schema - should compare result grpc.body to
             // schema considering repeated message type
             let fields = &field_schema.field;
-            let args = &field_schema.args;
-            let fields = JsonSchema::Arr(Box::new(fields.to_owned()));
-            let _args = JsonSchema::Arr(Box::new(args.to_owned()));
-            fields.compare(&output_schema, group_by[0].as_str())
+            // we're treating List types for gRPC as optional.
+            let fields = JsonSchema::Opt(Box::new(JsonSchema::Arr(Box::new(fields.to_owned()))));
+            fields.is_a(&output_schema, group_by[0].as_str())
         })
 }
 

diff --git a/...call__core__config__transformer__ambiguous_type__tests__resolve_ambiguous_news_types.snap b/...call__core__config__transformer__ambiguous_type__tests__resolve_ambiguous_news_types.snap
@@ -29,12 +29,12 @@ enum news__Status {
 }
 
 type Query {
-  news__NewsService__AddNews(news: news__NewsInput!): news__News! @grpc(body: "{{.args.news}}", method: "news.NewsService.AddNews")
-  news__NewsService__DeleteNews(newsId: news__NewsId!): Empty! @grpc(body: "{{.args.newsId}}", method: "news.NewsService.DeleteNews")
-  news__NewsService__EditNews(news: news__NewsInput!): news__News! @grpc(body: "{{.args.news}}", method: "news.NewsService.EditNews")
-  news__NewsService__GetAllNews: news__NewsList! @grpc(method: "news.NewsService.GetAllNews")
-  news__NewsService__GetMultipleNews(multipleNewsId: news__MultipleNewsId!): news__NewsList! @grpc(body: "{{.args.multipleNewsId}}", method: "news.NewsService.GetMultipleNews")
-  news__NewsService__GetNews(newsId: news__NewsId!): news__News! @grpc(body: "{{.args.newsId}}", method: "news.NewsService.GetNews")
+  news__NewsService__AddNews(news: news__NewsInput!): news__News @grpc(body: "{{.args.news}}", method: "news.NewsService.AddNews")
+  news__NewsService__DeleteNews(newsId: news__NewsId!): Empty @grpc(body: "{{.args.newsId}}", method: "news.NewsService.DeleteNews")
+  news__NewsService__EditNews(news: news__NewsInput!): news__News @grpc(body: "{{.args.news}}", method: "news.NewsService.EditNews")
+  news__NewsService__GetAllNews: news__NewsList @grpc(method: "news.NewsService.GetAllNews")
+  news__NewsService__GetMultipleNews(multipleNewsId: news__MultipleNewsId!): news__NewsList @grpc(body: "{{.args.multipleNewsId}}", method: "news.NewsService.GetMultipleNews")
+  news__NewsService__GetNews(newsId: news__NewsId!): news__News @grpc(body: "{{.args.newsId}}", method: "news.NewsService.GetNews")
 }
 
 type news__News {

diff --git a/src/core/generator/from_proto.rs b/src/core/generator/from_proto.rs
@@ -362,7 +362,6 @@ impl Context {
                     .into_object_type()
                     .to_string();
                 cfg_field.type_of = output_ty;
-                cfg_field.required = true;
 
                 cfg_field.resolver = Some(Resolver::Grpc(Grpc {
                     base_url: None,

diff --git a/src/core/generator/snapshots/tailcall__core__generator__from_proto__test__from_proto.snap b/src/core/generator/snapshots/tailcall__core__generator__from_proto__test__from_proto.snap
@@ -43,17 +43,17 @@ type Query {
   """
   Sends a greeting
   """
-  greetings_a__b__Greeter__SayHello(helloRequest: greetings_a__b__HelloRequest!): greetings_a__b__HelloReply! @grpc(body: "{{.args.helloRequest}}", method: "greetings_a.b.Greeter.SayHello")
+  greetings_a__b__Greeter__SayHello(helloRequest: greetings_a__b__HelloRequest!): greetings_a__b__HelloReply @grpc(body: "{{.args.helloRequest}}", method: "greetings_a.b.Greeter.SayHello")
   """
   Sends a greeting
   """
-  greetings_b__c__Greeter__SayHello(helloRequest: greetings__HelloRequest!): greetings__HelloReply! @grpc(body: "{{.args.helloRequest}}", method: "greetings_b.c.Greeter.SayHello")
-  news__NewsService__AddNews(news: news__NewsInput!): news__News! @grpc(body: "{{.args.news}}", method: "news.NewsService.AddNews")
-  news__NewsService__DeleteNews(newsId: news__NewsId!): Empty! @grpc(body: "{{.args.newsId}}", method: "news.NewsService.DeleteNews")
-  news__NewsService__EditNews(news: news__NewsInput!): news__News! @grpc(body: "{{.args.news}}", method: "news.NewsService.EditNews")
-  news__NewsService__GetAllNews: news__NewsList! @grpc(method: "news.NewsService.GetAllNews")
-  news__NewsService__GetMultipleNews(multipleNewsId: news__MultipleNewsId!): news__NewsList! @grpc(body: "{{.args.multipleNewsId}}", method: "news.NewsService.GetMultipleNews")
-  news__NewsService__GetNews(newsId: news__NewsId!): news__News! @grpc(body: "{{.args.newsId}}", method: "news.NewsService.GetNews")
+  greetings_b__c__Greeter__SayHello(helloRequest: greetings__HelloRequest!): greetings__HelloReply @grpc(body: "{{.args.helloRequest}}", method: "greetings_b.c.Greeter.SayHello")
+  news__NewsService__AddNews(news: news__NewsInput!): news__News @grpc(body: "{{.args.news}}", method: "news.NewsService.AddNews")
+  news__NewsService__DeleteNews(newsId: news__NewsId!): Empty @grpc(body: "{{.args.newsId}}", method: "news.NewsService.DeleteNews")
+  news__NewsService__EditNews(news: news__NewsInput!): news__News @grpc(body: "{{.args.news}}", method: "news.NewsService.EditNews")
+  news__NewsService__GetAllNews: news__NewsList @grpc(method: "news.NewsService.GetAllNews")
+  news__NewsService__GetMultipleNews(multipleNewsId: news__MultipleNewsId!): news__NewsList @grpc(body: "{{.args.multipleNewsId}}", method: "news.NewsService.GetMultipleNews")
+  news__NewsService__GetNews(newsId: news__NewsId!): news__News @grpc(body: "{{.args.newsId}}", method: "news.NewsService.GetNews")
 }
 
 type greetings__HelloReply {

diff --git a/...erator/snapshots/tailcall__core__generator__from_proto__test__from_proto_no_pkg_file.snap b/...erator/snapshots/tailcall__core__generator__from_proto__test__from_proto_no_pkg_file.snap
@@ -18,5 +18,5 @@ type NewsList {
 }
 
 type Query {
-  NewsService__GetAllNews: NewsList! @grpc(method: "NewsService.GetAllNews")
+  NewsService__GetAllNews: NewsList @grpc(method: "NewsService.GetAllNews")
 }
diff --git a/...enerator/snapshots/tailcall__core__generator__from_proto__test__greetings_proto_file.snap b/...enerator/snapshots/tailcall__core__generator__from_proto__test__greetings_proto_file.snap
@@ -11,7 +11,7 @@ input greetings__HelloRequest {
 }
 
 type Query {
-  greetings__Greeter__SayHello(helloRequest: greetings__HelloRequest!): greetings__HelloReply! @grpc(body: "{{.args.helloRequest}}", method: "greetings.Greeter.SayHello")
+  greetings__Greeter__SayHello(helloRequest: greetings__HelloRequest!): greetings__HelloReply @grpc(body: "{{.args.helloRequest}}", method: "greetings.Greeter.SayHello")
 }
 
 type greetings__HelloReply {

diff --git a/src/core/generator/snapshots/tailcall__core__generator__from_proto__test__map_types.snap b/src/core/generator/snapshots/tailcall__core__generator__from_proto__test__map_types.snap
@@ -11,7 +11,7 @@ input map__MapRequest {
 }
 
 type Query {
-  map__MapService__GetMap(mapRequest: map__MapRequest!): map__MapResponse! @grpc(body: "{{.args.mapRequest}}", method: "map.MapService.GetMap")
+  map__MapService__GetMap(mapRequest: map__MapRequest!): map__MapResponse @grpc(body: "{{.args.mapRequest}}", method: "map.MapService.GetMap")
 }
 
 type map__MapResponse {

diff --git a/src/core/generator/snapshots/tailcall__core__generator__from_proto__test__movies.snap b/src/core/generator/snapshots/tailcall__core__generator__from_proto__test__movies.snap
@@ -196,19 +196,19 @@ type Query {
   """
   get all movies
   """
-  movies__AnotherExample__GetMovies(movieRequest: movies__MovieRequest!): movies__MoviesResult! @grpc(body: "{{.args.movieRequest}}", method: "movies.AnotherExample.GetMovies")
+  movies__AnotherExample__GetMovies(movieRequest: movies__MovieRequest!): movies__MoviesResult @grpc(body: "{{.args.movieRequest}}", method: "movies.AnotherExample.GetMovies")
   """
   search movies by the name of the cast
   """
-  movies__AnotherExample__SearchMoviesByCast(searchByCastRequest: movies__SearchByCastRequest!): movies__Movie! @grpc(body: "{{.args.searchByCastRequest}}", method: "movies.AnotherExample.SearchMoviesByCast")
+  movies__AnotherExample__SearchMoviesByCast(searchByCastRequest: movies__SearchByCastRequest!): movies__Movie @grpc(body: "{{.args.searchByCastRequest}}", method: "movies.AnotherExample.SearchMoviesByCast")
   """
   get all movies
   """
-  movies__Example__GetMovies(movieRequest: movies__MovieRequest!): movies__MoviesResult! @grpc(body: "{{.args.movieRequest}}", method: "movies.Example.GetMovies")
+  movies__Example__GetMovies(movieRequest: movies__MovieRequest!): movies__MoviesResult @grpc(body: "{{.args.movieRequest}}", method: "movies.Example.GetMovies")
   """
   search movies by the name of the cast
   """
-  movies__Example__SearchMoviesByCast(searchByCastRequest: movies__SearchByCastRequest!): movies__Movie! @grpc(body: "{{.args.searchByCastRequest}}", method: "movies.Example.SearchMoviesByCast")
+  movies__Example__SearchMoviesByCast(searchByCastRequest: movies__SearchByCastRequest!): movies__Movie @grpc(body: "{{.args.searchByCastRequest}}", method: "movies.Example.SearchMoviesByCast")
 }
 
   """

diff --git a/src/core/generator/snapshots/tailcall__core__generator__from_proto__test__nested_types.snap b/src/core/generator/snapshots/tailcall__core__generator__from_proto__test__nested_types.snap
@@ -17,7 +17,7 @@ enum nested__types__Result__NestedEnum {
 }
 
 type Query {
-  nested__types__Example__Get(veryNested: nested__types__Result__Nested__VeryNestedInput!): nested__types__Result! @grpc(body: "{{.args.veryNested}}", method: "nested.types.Example.Get")
+  nested__types__Example__Get(veryNested: nested__types__Result__Nested__VeryNestedInput!): nested__types__Result @grpc(body: "{{.args.veryNested}}", method: "nested.types.Example.Get")
 }
 
 type nested__types__Result {

diff --git a/src/core/generator/snapshots/tailcall__core__generator__from_proto__test__oneof_types.snap b/src/core/generator/snapshots/tailcall__core__generator__from_proto__test__oneof_types.snap
@@ -67,7 +67,7 @@ union oneof__Request = oneof__Request__Var0__Var | oneof__Request__Var0__Var0 |
 union oneof__Response = oneof__Response__Var | oneof__Response__Var0 | oneof__Response__Var1 | oneof__Response__Var2
 
 type Query {
-  oneof__OneOfService__GetOneOf(request: oneof__Request!): oneof__Response! @grpc(body: "{{.args.request}}", method: "oneof.OneOfService.GetOneOf")
+  oneof__OneOfService__GetOneOf(request: oneof__Request!): oneof__Response @grpc(body: "{{.args.request}}", method: "oneof.OneOfService.GetOneOf")
 }
 
 type oneof__Command {

diff --git a/...ore/generator/snapshots/tailcall__core__generator__from_proto__test__optional_fields.snap b/...ore/generator/snapshots/tailcall__core__generator__from_proto__test__optional_fields.snap
@@ -32,7 +32,7 @@ enum type__Status {
 }
 
 type Query {
-  type__TypeService__Get(type: type__TypeInput!): type__Type! @grpc(body: "{{.args.type}}", method: "type.TypeService.Get")
+  type__TypeService__Get(type: type__TypeInput!): type__Type @grpc(body: "{{.args.type}}", method: "type.TypeService.Get")
 }
 
 type type__Type {

diff --git a/...core/generator/snapshots/tailcall__core__generator__from_proto__test__required_types.snap b/...core/generator/snapshots/tailcall__core__generator__from_proto__test__required_types.snap
@@ -7,7 +7,7 @@ schema @server @upstream {
 }
 
 type Query {
-  person__PersonService__GetPerson: person__Person! @grpc(method: "person.PersonService.GetPerson")
+  person__PersonService__GetPerson: person__Person @grpc(method: "person.PersonService.GetPerson")
 }
 
   """

diff --git a/src/core/generator/snapshots/tailcall__core__generator__from_proto__test__scalar_types.snap b/src/core/generator/snapshots/tailcall__core__generator__from_proto__test__scalar_types.snap
@@ -25,7 +25,7 @@ input scalars__ItemInput {
 }
 
 type Query {
-  scalars__Example__Get(item: scalars__ItemInput!): scalars__Result! @grpc(body: "{{.args.item}}", method: "scalars.Example.Get")
+  scalars__Example__Get(item: scalars__ItemInput!): scalars__Result @grpc(body: "{{.args.item}}", method: "scalars.Example.Get")
 }
 
 type scalars__Item {

diff --git a/...napshots/tailcall__core__generator__generator__test__should_generate_combined_config.snap b/...napshots/tailcall__core__generator__generator__test__should_generate_combined_config.snap
@@ -79,12 +79,12 @@ type Post {
 
 type Query {
   f1: F1 @http(path: "/")
-  news__NewsService__AddNews(news: news__NewsInput!): News! @grpc(body: "{{.args.news}}", method: "news.NewsService.AddNews")
-  news__NewsService__DeleteNews(newsId: news__NewsId!): Empty! @grpc(body: "{{.args.newsId}}", method: "news.NewsService.DeleteNews")
-  news__NewsService__EditNews(news: news__NewsInput!): News! @grpc(body: "{{.args.news}}", method: "news.NewsService.EditNews")
-  news__NewsService__GetAllNews: NewsNewsServiceGetMultipleNew! @grpc(method: "news.NewsService.GetAllNews")
-  news__NewsService__GetMultipleNews(multipleNewsId: news__MultipleNewsId!): NewsNewsServiceGetMultipleNew! @grpc(body: "{{.args.multipleNewsId}}", method: "news.NewsService.GetMultipleNews")
-  news__NewsService__GetNews(newsId: news__NewsId!): News! @grpc(body: "{{.args.newsId}}", method: "news.NewsService.GetNews")
+  news__NewsService__AddNews(news: news__NewsInput!): News @grpc(body: "{{.args.news}}", method: "news.NewsService.AddNews")
+  news__NewsService__DeleteNews(newsId: news__NewsId!): Empty @grpc(body: "{{.args.newsId}}", method: "news.NewsService.DeleteNews")
+  news__NewsService__EditNews(news: news__NewsInput!): News @grpc(body: "{{.args.news}}", method: "news.NewsService.EditNews")
+  news__NewsService__GetAllNews: NewsNewsServiceGetMultipleNew @grpc(method: "news.NewsService.GetAllNews")
+  news__NewsService__GetMultipleNews(multipleNewsId: news__MultipleNewsId!): NewsNewsServiceGetMultipleNew @grpc(body: "{{.args.multipleNewsId}}", method: "news.NewsService.GetMultipleNews")
+  news__NewsService__GetNews(newsId: news__NewsId!): News @grpc(body: "{{.args.newsId}}", method: "news.NewsService.GetNews")
   post(id: Int! = 1): Post @http(path: "/posts/{{.args.id}}")
   posts: [Post] @http(path: "/posts?_limit=11")
   user(id: Int!): User @http(path: "/users/{{.args.id}}")

diff --git a/...pshots/tailcall__core__generator__generator__test__should_generate_config_from_proto.snap b/...pshots/tailcall__core__generator__generator__test__should_generate_config_from_proto.snap
@@ -29,12 +29,12 @@ enum news__Status {
 }
 
 type Query {
-  news__NewsService__AddNews(news: news__NewsInput!): news__News! @grpc(body: "{{.args.news}}", method: "news.NewsService.AddNews")
-  news__NewsService__DeleteNews(newsId: news__NewsId!): Empty! @grpc(body: "{{.args.newsId}}", method: "news.NewsService.DeleteNews")
-  news__NewsService__EditNews(news: news__NewsInput!): news__News! @grpc(body: "{{.args.news}}", method: "news.NewsService.EditNews")
-  news__NewsService__GetAllNews: news__NewsList! @grpc(method: "news.NewsService.GetAllNews")
-  news__NewsService__GetMultipleNews(multipleNewsId: news__MultipleNewsId!): news__NewsList! @grpc(body: "{{.args.multipleNewsId}}", method: "news.NewsService.GetMultipleNews")
-  news__NewsService__GetNews(newsId: news__NewsId!): news__News! @grpc(body: "{{.args.newsId}}", method: "news.NewsService.GetNews")
+  news__NewsService__AddNews(news: news__NewsInput!): news__News @grpc(body: "{{.args.news}}", method: "news.NewsService.AddNews")
+  news__NewsService__DeleteNews(newsId: news__NewsId!): Empty @grpc(body: "{{.args.newsId}}", method: "news.NewsService.DeleteNews")
+  news__NewsService__EditNews(news: news__NewsInput!): news__News @grpc(body: "{{.args.news}}", method: "news.NewsService.EditNews")
+  news__NewsService__GetAllNews: news__NewsList @grpc(method: "news.NewsService.GetAllNews")
+  news__NewsService__GetMultipleNews(multipleNewsId: news__MultipleNewsId!): news__NewsList @grpc(body: "{{.args.multipleNewsId}}", method: "news.NewsService.GetMultipleNews")
+  news__NewsService__GetNews(newsId: news__NewsId!): news__News @grpc(body: "{{.args.newsId}}", method: "news.NewsService.GetNews")
 }
 
 type news__News {
-Original file line number
+Diff line change
@@ Expand Up / @@ -7,7 +7,7 @@ schema @server @upstream { @@
     }
     type Query {
-      person__PersonService__GetPerson: person__Person! @grpc(method: "person.PersonService.GetPerson")
+      person__PersonService__GetPerson: person__Person @grpc(method: "person.PersonService.GetPerson")
     }
       """
@@ Expand Down @@
Thread Stats	Avg	Stdev	Max	+/- Stdev
Latency	7.38ms	3.65ms	112.57ms	75.67%
Req/Sec	3.44k	199.69	5.39k	91.25%