fix kamil's reported issues

the-guild-org · Dec 25, 2023 · 5f481b0 · 5f481b0
1 parent f65472a
commit 5f481b0
Show file tree

Hide file tree

Showing 4 changed files with 33 additions and 32 deletions.
diff --git a/libs/federation_query_planner/src/lib.rs b/libs/federation_query_planner/src/lib.rs
@@ -22,12 +22,11 @@ pub async fn execute_federation(
     supergraph: &Supergraph,
     parsed_user_query: Document<'static, String>,
 ) -> Result<String> {
-    // println!("parse_user_query: {:#?}", user_query);
+    // println!("parsed_user_query: {:#?}", user_query);
     let mut user_query = parse_user_query(parsed_user_query)?;
-    // println!("query_plan: {:#?}", user_query);
     let query_plan = plan_for_user_query(supergraph, &mut user_query)?;
 
-    // println!("given query: {:#?}", user_query);
+    // println!("query plan: {:#?}", query_plan);
 
     let response_vec = execute_query_plan(&query_plan, supergraph).await?;
 

diff --git a/libs/federation_query_planner/src/query_planner.rs b/libs/federation_query_planner/src/query_planner.rs
@@ -3,9 +3,10 @@ use serde::{Deserialize, Serialize};
 use std::{collections::HashMap, vec};
 
 use crate::{
+    constants::CONDUCTOR_INTERNAL_SERVICE_RESOLVER,
     graphql_query_builder::{batch_subqueries, generate_query_for_field},
     supergraph::{GraphQLType, Supergraph},
-    user_query::{FieldNode, GraphQLFragment, UserQuery}, constants::CONDUCTOR_INTERNAL_SERVICE_RESOLVER,
+    user_query::{FieldNode, GraphQLFragment, UserQuery},
 };
 
 pub type EntityQueryNeeds = Option<EntityQuerySearch>;
@@ -267,8 +268,7 @@ pub fn plan_for_user_query(
     let mut mappings: Vec<(String, String, EntityQueryNeeds)> = vec![];
 
     for field in &mut user_query.fields {
-        let parent_source = determine_owner(&field.sources, None, None);
-        build_fields_mappings_to_subgraphs(field, &parent_source, &mut mappings, supergraph);
+        build_fields_mappings_to_subgraphs(field, None, &mut mappings, supergraph);
     }
 
     // TODO: that `.rev()` might be expensive!
@@ -300,13 +300,13 @@ pub fn plan_for_user_query(
 
 fn build_fields_mappings_to_subgraphs(
     field: &mut FieldNode,
-    parent_source: &str,
+    parent_source: Option<&str>,
     results: &mut Vec<(String, String, EntityQueryNeeds)>,
     supergraph: &Supergraph,
 ) {
     resolve_children(
         field,
-        Some(parent_source),
+        parent_source,
         results,
         false,
         supergraph,
@@ -324,11 +324,6 @@ fn resolve_children(
     _supergraph: &Supergraph,
     (persisted_parent_type_name, shared_parent_type_name_field): ParentInfo,
 ) -> String {
-    // println!("{}", field.field);
-    // if field.field.starts_with("...") {
-    //     return String::with_capacity(0);
-    // }
-
     let current_source = determine_owner(&field.sources, field.owner.as_ref(), parent_source);
 
     let children_results: Vec<_> = field
@@ -378,7 +373,10 @@ fn resolve_children(
     if !nested && !res.is_empty() {
         let current_source_str = current_source.to_string();
 
-        let (result, entity_key_map) = if field.key_fields.is_some() {
+        let (result, entity_key_map) = if field.key_fields.is_some()
+            // don't do an entity query on a root Query resolvable field
+            && field.parent_type_name != None
+        {
             // If no children, populate the current field
             if !field.children.is_empty() {
                 field.relevant_sub_queries.get_or_insert(vec![]).push((

diff --git a/libs/federation_query_planner/src/supergraph.rs b/libs/federation_query_planner/src/supergraph.rs
@@ -122,16 +122,16 @@ pub fn parse_supergraph(supergraph_schema: &str) -> Result<Supergraph, Box<dyn E
                                     match k.as_str() {
                                         // 5. Get the field's subgraph owner
                                         "graph" => {
-                                            if field_directive
-                                                .arguments
-                                                .iter()
-                                                // We're excluding `@join__field(external: true)` because we want the owning subgraph not the one referencing it
-                                                .any(|(key, val)| {
-                                                    key == "external" && val.to_string() == "true"
-                                                })
-                                            {
-                                                graphql_type_field.sources = vec![v.to_string()];
-                                            }
+                                            // if field_directive
+                                            //     .arguments
+                                            //     .iter()
+                                            //     // We're excluding `@join__field(external: true)` because we want the owning subgraph not the one referencing it
+                                            //     .any(|(key, val)| {
+                                            //         key == "external" && val.to_string() == "true"
+                                            //     })
+                                            // {
+                                            graphql_type_field.sources = vec![v.to_string()];
+                                            // }
                                         }
                                         // 6. Get other useful directives
                                         "requires" => {

diff --git a/test_config/supergraph.graphql b/test_config/supergraph.graphql
@@ -46,11 +46,17 @@ directive @link(
 scalar join__FieldSet
 
 enum join__Graph {
-  ACCOUNTS @join__graph(name: "accounts", url: "http://localhost:5000/graphql")
+  ACCOUNTS
+    @join__graph(name: "accounts", url: "https://accounts.demo.starstuff.dev/")
   INVENTORY
-    @join__graph(name: "inventory", url: "http://localhost:5001/graphql")
-  PRODUCTS @join__graph(name: "products", url: "http://localhost:5002/graphql")
-  REVIEWS @join__graph(name: "reviews", url: "http://localhost:5003/graphql")
+    @join__graph(
+      name: "inventory"
+      url: "https://inventory.demo.starstuff.dev/"
+    )
+  PRODUCTS
+    @join__graph(name: "products", url: "https://products.demo.starstuff.dev/")
+  REVIEWS
+    @join__graph(name: "reviews", url: "https://reviews.demo.starstuff.dev/")
 }
 
 scalar link__Import
@@ -82,6 +88,7 @@ type Product
   shippingEstimate: Int @join__field(graph: INVENTORY, requires: "price weight")
   name: String @join__field(graph: PRODUCTS)
   reviews: [Review] @join__field(graph: REVIEWS)
+  reviewsForAuthor(authorID: ID!): [Review] @join__field(graph: REVIEWS)
 }
 
 type Query
@@ -90,16 +97,14 @@ type Query
   @join__type(graph: PRODUCTS)
   @join__type(graph: REVIEWS) {
   me: User @join__field(graph: ACCOUNTS)
-  user(id: ID!): User @join__field(graph: ACCOUNTS)
-  users: [User] @join__field(graph: ACCOUNTS)
   topProducts(first: Int = 5): [Product] @join__field(graph: PRODUCTS)
 }
 
 type Review @join__type(graph: REVIEWS, key: "id") {
   id: ID!
   body: String
-  product: Product
   author: User @join__field(graph: REVIEWS, provides: "username")
+  product: Product
 }
 
 type User
@@ -110,6 +115,5 @@ type User
   username: String
     @join__field(graph: ACCOUNTS)
     @join__field(graph: REVIEWS, external: true)
-  birthday: Int @join__field(graph: ACCOUNTS)
   reviews: [Review] @join__field(graph: REVIEWS)
 }