fix(jit): nested resolver handling (#2338)

Co-authored-by: Tushar Mathur <[email protected]>
Co-authored-by: Shashi Kant <[email protected]>

src/core/generator/generator.rs

@@ -181,7 +181,7 @@ mod test {
     fn should_generate_config_from_configs() -> anyhow::Result<()> {
         let cfg_module = Generator::default()
             .inputs(vec![Input::Config {
-                schema: std::fs::read_to_string(tailcall_fixtures::configs::USER_POSTS)?,
+                schema: std::fs::read_to_string(tailcall_fixtures::configs::JSONPLACEHOLDER)?,
                 source: crate::core::config::Source::GraphQL,
             }])
             .generate(true)?;
@@ -218,7 +218,7 @@ mod test {
 
         // Config input
         let config_input = Input::Config {
-            schema: std::fs::read_to_string(tailcall_fixtures::configs::USER_POSTS)?,
+            schema: std::fs::read_to_string(tailcall_fixtures::configs::JSONPLACEHOLDER)?,
             source: crate::core::config::Source::GraphQL,
         };
 

src/core/generator/snapshots/tailcall__core__generator__generator__test__should_generate_combined_config.snap

@@ -30,6 +30,21 @@ enum news__Status {
   PUBLISHED
 }
 
+type Album {
+  id: Int!
+  photos: [Photo] @http(path: "/albums/{{.value.id}}/photos?_limit=3")
+  title: Int
+  userId: Int!
+}
+
+type Comment {
+  body: String!
+  email: String!
+  id: Int!
+  name: String!
+  title: String! @expr(body: "{{.value.email}}: {{.value.name}}")
+}
+
 type F1 {
   campaignTemplates: Any
   colors: [Any]
@@ -47,12 +62,21 @@ type NewsNewsServiceGetMultipleNew @tag(id: "news.NewsList") {
   news: [News]
 }
 
+type Photo {
+  albumId: Int!
+  combinedId: String! @expr(body: "Album: {{.value.albumId}}, photo: {{.value.id}}")
+  id: Int!
+  title: String!
+}
+
 type Post {
   body: String!
+  comments: [Comment] @http(path: "/posts/{{.value.id}}/comments")
   id: Int!
   title: String!
   user: User @http(path: "/users/{{.value.userId}}")
   userId: Int!
+  users: [User] @http(path: "/users")
 }
 
 type Query {
@@ -64,16 +88,17 @@ type Query {
   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!): Post @http(path: "/posts/{{.args.id}}")
-  posts: [Post] @http(path: "/posts")
+  posts: [Post] @http(path: "/posts?_limit=11")
   user(id: Int!): User @http(path: "/users/{{.args.id}}")
   users: [User] @http(path: "/users")
 }
 
 type User {
+  albums: [Album] @http(path: "/users/{{.value.id}}/albums?_limit=2")
+  blog: String @expr(body: "https://test.blog/users/website/{{.value.username}}")
   email: String!
   id: Int!
   name: String!
   phone: String
   username: String!
-  website: String @expr(body: "/users/website/{{.value.username}}")
 }

src/core/generator/snapshots/tailcall__core__generator__generator__test__should_generate_config_from_configs.snap

@@ -1,31 +1,56 @@
 ---
 source: src/core/generator/generator.rs
-expression: cfg_module.config.to_sdl()
+expression: cfg_module.config().to_sdl()
 ---
 schema @server(hostname: "0.0.0.0", port: 8000) @upstream(baseURL: "http://jsonplaceholder.typicode.com", httpCache: 42) {
   query: Query
 }
 
+type Album {
+  id: Int!
+  photos: [Photo] @http(path: "/albums/{{.value.id}}/photos?_limit=3")
+  title: Int
+  userId: Int!
+}
+
+type Comment {
+  body: String!
+  email: String!
+  id: Int!
+  name: String!
+  title: String! @expr(body: "{{.value.email}}: {{.value.name}}")
+}
+
+type Photo {
+  albumId: Int!
+  combinedId: String! @expr(body: "Album: {{.value.albumId}}, photo: {{.value.id}}")
+  id: Int!
+  title: String!
+}
+
 type Post {
   body: String!
+  comments: [Comment] @http(path: "/posts/{{.value.id}}/comments")
   id: Int!
   title: String!
   user: User @http(path: "/users/{{.value.userId}}")
   userId: Int!
+  users: [User] @http(path: "/users")
 }
 
 type Query {
   post(id: Int!): Post @http(path: "/posts/{{.args.id}}")
-  posts: [Post] @http(path: "/posts")
+  posts: [Post] @http(path: "/posts?_limit=11")
   user(id: Int!): User @http(path: "/users/{{.args.id}}")
   users: [User] @http(path: "/users")
 }
 
 type User {
+  albums: [Album] @http(path: "/users/{{.value.id}}/albums?_limit=2")
+  blog: String @expr(body: "https://test.blog/users/website/{{.value.username}}")
   email: String!
   id: Int!
   name: String!
   phone: String
   username: String!
-  website: String @expr(body: "/users/website/{{.value.username}}")
 }

src/core/jit/common/json_placeholder.rs

@@ -1,12 +1,13 @@
 use std::collections::HashMap;
 
-use serde_json_borrow::Value;
+use async_graphql::Value;
 
 use crate::core::blueprint::Blueprint;
 use crate::core::config::{Config, ConfigModule};
 use crate::core::jit::builder::Builder;
 use crate::core::jit::store::{Data, Store};
-use crate::core::jit::synth::SynthBorrow;
+use crate::core::jit::synth::Synth;
+use crate::core::json::JsonLike;
 use crate::core::valid::Validator;
 
 /// NOTE: This is a bit of a boilerplate reducing module that is used in tests
@@ -18,28 +19,31 @@ impl JsonPlaceholder {
     const USERS: &'static str = include_str!("users.json");
     const CONFIG: &'static str = include_str!("../fixtures/jsonplaceholder-mutation.graphql");
 
-    pub fn init(query: &str) -> SynthBorrow {
+    pub fn init(query: &str) -> Synth {
         let posts = serde_json::from_str::<Vec<Value>>(Self::POSTS).unwrap();
         let users = serde_json::from_str::<Vec<Value>>(Self::USERS).unwrap();
 
         let user_map = users.iter().fold(HashMap::new(), |mut map, user| {
-            let id = user
-                .as_object()
-                .and_then(|user| user.get("id"))
-                .and_then(|u| u.as_u64());
+            let id = if let Value::Object(user) = user {
+                user.get("id").and_then(|u| u.as_u64_ok().ok())
+            } else {
+                None
+            };
 
             if let Some(id) = id {
                 map.insert(id, user);
             }
             map
         });
 
-        let users: Vec<Value<'static>> = posts
+        let users: HashMap<_, _> = posts
             .iter()
             .map(|post| {
-                let user_id = post
-                    .as_object()
-                    .and_then(|post| post.get("userId").and_then(|u| u.as_u64()));
+                let user_id = if let Value::Object(post) = post {
+                    post.get("userId").and_then(|u| u.as_u64_ok().ok())
+                } else {
+                    None
+                };
 
                 if let Some(user_id) = user_id {
                     if let Some(user) = user_map.get(&user_id) {
@@ -51,7 +55,10 @@ impl JsonPlaceholder {
                     Value::Null
                 }
             })
-            .collect::<Vec<Value<'static>>>();
+            .map(Ok)
+            .map(Data::Single)
+            .enumerate()
+            .collect();
 
         let config = ConfigModule::from(Config::from_sdl(Self::CONFIG).to_result().unwrap());
         let builder = Builder::new(
@@ -66,15 +73,15 @@ impl JsonPlaceholder {
             .id
             .to_owned();
         let store = [
-            (posts_id, Data::Single(Value::Array(posts))),
+            (posts_id, Data::Single(Ok(Value::List(posts)))),
             (users_id, Data::Multiple(users)),
         ]
         .into_iter()
-        .fold(Store::new(plan.size()), |mut store, (id, data)| {
-            store.set(id, data);
+        .fold(Store::new(), |mut store, (id, data)| {
+            store.set_data(id, data);
             store
         });
 
-        SynthBorrow::new(plan, store)
+        Synth::new(plan, store)
     }
 }

src/core/jit/exec.rs

@@ -1,3 +1,4 @@
+use std::fmt::Debug;
 use std::mem;
 use std::sync::{Arc, Mutex};
 
@@ -6,7 +7,7 @@ use futures_util::future::join_all;
 
 use super::context::Context;
 use super::synth::Synthesizer;
-use super::{Children, ExecutionPlan, Field, Request, Response, Store};
+use super::{Children, DataPath, ExecutionPlan, Field, Request, Response, Store};
 use crate::core::ir::model::IR;
 use crate::core::json::JsonLike;
 
@@ -21,7 +22,7 @@ pub struct Executor<Synth, IRExec> {
 
 impl<Input, Output, Error, Synth, Exec> Executor<Synth, Exec>
 where
-    Output: JsonLike<Output = Output> + Default,
+    Output: JsonLike<Output = Output> + Debug,
     Synth: Synthesizer<Value = Result<Output, Error>>,
     Exec: IRExecutor<Input = Input, Output = Output, Error = Error>,
 {
@@ -30,12 +31,11 @@ where
     }
 
     async fn execute_inner(&self, request: Request<Input>) -> Store<Result<Output, Error>> {
-        let store: Arc<Mutex<Store<Result<Output, Error>>>> =
-            Arc::new(Mutex::new(Store::new(self.plan.size())));
+        let store: Arc<Mutex<Store<Result<Output, Error>>>> = Arc::new(Mutex::new(Store::new()));
         let mut ctx = ExecutorInner::new(request, store.clone(), self.plan.to_owned(), &self.exec);
         ctx.init().await;
 
-        let store = mem::replace(&mut *store.lock().unwrap(), Store::new(0));
+        let store = mem::replace(&mut *store.lock().unwrap(), Store::new());
         store
     }
 
@@ -55,7 +55,7 @@ struct ExecutorInner<'a, Input, Output, Error, Exec> {
 
 impl<'a, Input, Output, Error, Exec> ExecutorInner<'a, Input, Output, Error, Exec>
 where
-    Output: JsonLike<Output = Output> + Default,
+    Output: JsonLike<Output = Output> + Debug,
     Exec: IRExecutor<Input = Input, Output = Output, Error = Error>,
 {
     fn new(
@@ -70,7 +70,7 @@ where
     async fn init(&mut self) {
         join_all(self.plan.as_children().iter().map(|field| async {
             let ctx = Context::new(&self.request);
-            self.execute(field, &ctx).await
+            self.execute(field, &ctx, DataPath::new()).await
         }))
         .await;
     }
@@ -79,39 +79,25 @@ where
         &'b self,
         field: &'b Field<Children>,
         ctx: &'b Context<'b, Input, Output>,
+        data_path: DataPath,
     ) -> Result<(), Error> {
         if let Some(ir) = &field.ir {
             let result = self.ir_exec.execute(ir, ctx).await;
+
             if let Ok(ref value) = result {
                 // Array
                 // Check if the field expects a list
                 if field.type_of.is_list() {
                     // Check if the value is an array
                     if let Ok(array) = value.as_array_ok() {
-                        let values = join_all(
-                            field
-                                .children()
-                                .iter()
-                                .filter_map(|field| field.ir.as_ref())
-                                .map(|ir| {
-                                    join_all(array.iter().map(|value| {
-                                        let ctx = ctx.with_value(value);
-                                        // TODO: doesn't handle nested values
-                                        async move { self.ir_exec.execute(ir, &ctx).await }
-                                    }))
-                                }),
-                        )
+                        join_all(field.children().iter().map(|field| {
+                            join_all(array.iter().enumerate().map(|(index, value)| {
+                                let ctx = ctx.with_value(value);
+                                let data_path = data_path.clone().with_index(index);
+                                async move { self.execute(field, &ctx, data_path).await }
+                            }))
+                        }))
                         .await;
-
-                        let mut store = self.store.lock().unwrap();
-                        for (field, values) in field
-                            .children()
-                            .iter()
-                            .filter(|field| field.ir.is_some())
-                            .zip(values)
-                        {
-                            store.set_multiple(&field.id, values)
-                        }
                     }
                     // TODO:  We should throw an error stating that we expected
                     // a list type here but because the `Error` is a
@@ -122,14 +108,18 @@ where
                 else {
                     join_all(field.children().iter().map(|child| {
                         let ctx = ctx.with_value(value);
-                        async move { self.execute(child, &ctx).await }
+                        let data_path = data_path.clone();
+                        async move { self.execute(child, &ctx, data_path).await }
                     }))
                     .await;
                 }
             }
 
-            self.store.lock().unwrap().set_single(&field.id, result)
+            let mut store = self.store.lock().unwrap();
+
+            store.set(&field.id, &data_path, result);
         }
+
         Ok(())
     }
 }