Merge pull request hasura#23 from hasura/karthikeyan/misc-relationshi…

…p-fixes Misc relationship fixes GitOrigin-RevId: be63dc0a3e4d325677b002a81408391dea20e8d6
hgiasac · Dec 19, 2023 · 508f4b5 · 508f4b5
1 parent 96cd535
commit 508f4b5
Show file tree

Hide file tree

Showing 7 changed files with 86 additions and 33 deletions.
diff --git a/v3/gds/src/schema/operations.rs b/v3/gds/src/schema/operations.rs
@@ -60,4 +60,15 @@ pub enum Error {
     RemoteRelationshipsAreNotSupported,
     #[error("{description}")]
     InternalError { description: String },
+    #[error("Type mapping not found for the type name {type_name:} while executing the relationship {relationship_name:}")]
+    TypeMappingNotFoundForRelationship {
+        type_name: data_specification::TypeName,
+        relationship_name: data_specification::RelationshipName,
+    },
+    #[error("Field mapping not found for the field {field_name:} of type {type_name:} while executing the relationship {relationship_name:}")]
+    FieldMappingNotFoundForRelationship {
+        type_name: data_specification::TypeName,
+        relationship_name: data_specification::RelationshipName,
+        field_name: data_specification::FieldName,
+    },
 }
diff --git a/v3/gds/src/schema/operations/relationships.rs b/v3/gds/src/schema/operations/relationships.rs
@@ -3,11 +3,35 @@ use ndc_client;
 use open_dds::data_specification;
 use std::collections::BTreeMap;
 
-use crate::metadata::resolved;
+use crate::metadata::resolved::{self, FieldMapping};
 use crate::schema::operations;
 use crate::schema::types::Annotation;
 use crate::schema::GDS;
 
+fn get_field_mapping_of_field_name(
+    model_source: &resolved::ModelSource,
+    type_name: &data_specification::TypeName,
+    relationship_name: &data_specification::RelationshipName,
+    field_name: &data_specification::FieldName,
+) -> Result<FieldMapping, operations::Error> {
+    let source_type_mapping = model_source.type_mappings.get(type_name).ok_or_else(|| {
+        operations::Error::TypeMappingNotFoundForRelationship {
+            type_name: type_name.clone(),
+            relationship_name: relationship_name.clone(),
+        }
+    })?;
+    match source_type_mapping {
+        resolved::TypeMapping::Object { field_mappings } => Ok(field_mappings
+            .get(field_name)
+            .ok_or_else(|| operations::Error::FieldMappingNotFoundForRelationship {
+                type_name: type_name.clone(),
+                relationship_name: relationship_name.clone(),
+                field_name: field_name.clone(),
+            })?
+            .clone()),
+    }
+}
+
 /// Collects all the relationships in the 'relationships_collector'
 /// present in the selection set.
 pub fn collect_relationships_from_selection_set<'s>(
@@ -31,21 +55,29 @@ pub fn collect_relationships_from_selection_set<'s>(
                             resolved::RelationshipMappingSource::Value(_v),
                             resolved::RelationshipMappingTarget::Argument(_argument_name),
                         ) => {
-                            unimplemented!("Value to argument mappings is not supported yet.")
+                            return Err(operations::Error::InternalError {
+                                description: "Value to argument mappings is not supported yet."
+                                    .to_string(),
+                            })
                         }
                         (
                             resolved::RelationshipMappingSource::FieldPath(_field_path),
                             resolved::RelationshipMappingTarget::Argument(_argument_name),
                         ) => {
-                            unimplemented!("Field path to argument mapping is not supported yet.")
+                            return Err(operations::Error::InternalError {
+                                description: "Field path to argument mapping is not supported yet."
+                                    .to_string(),
+                            })
                         }
                         (
                             resolved::RelationshipMappingSource::Value(..),
                             resolved::RelationshipMappingTarget::ModelField(..),
                         ) => {
-                            unimplemented!(
-                                "Value expression to model field mapping not supported yet."
-                            )
+                            return Err(operations::Error::InternalError {
+                                description:
+                                    "Value expression to model field mapping not supported yet."
+                                        .to_string(),
+                            })
                         }
                         (
                             resolved::RelationshipMappingSource::FieldPath(source_field_path),
@@ -66,11 +98,21 @@ pub fn collect_relationships_from_selection_set<'s>(
                             if current_model_source.data_source != relationship_target.data_source {
                                 return Err(operations::Error::RemoteRelationshipsAreNotSupported);
                             } else {
+                                let source_column = get_field_mapping_of_field_name(
+                                    current_model_source,
+                                    &relationship.source,
+                                    &relationship.name,
+                                    &source_field_path.tail.field_name,
+                                )?;
+                                let target_column = get_field_mapping_of_field_name(
+                                    relationship_target,
+                                    &relationship.target_typename,
+                                    &relationship.name,
+                                    &target_field_path.tail.field_name,
+                                )?;
+
                                 if column_mapping
-                                    .insert(
-                                        source_field_path.tail.field_name.0.clone(),
-                                        target_field_path.tail.field_name.0.clone(),
-                                    )
+                                    .insert(source_column.column, target_column.column)
                                     .is_some()
                                 {
                                     return Err(operations::Error::MappingExistsInRelationship {

diff --git a/v3/gds/tests/execute/introspection/expected.json b/v3/gds/tests/execute/introspection/expected.json
@@ -131,31 +131,31 @@
             "deprecationReason": null
           },
           {
-            "name": "first_name",
+            "name": "author_id",
             "description": null,
             "args": [],
             "type": {
               "kind": "NON_NULL",
               "name": null,
               "ofType": {
                 "kind": "SCALAR",
-                "name": "String",
+                "name": "Int",
                 "ofType": null
               }
             },
             "isDeprecated": false,
             "deprecationReason": null
           },
           {
-            "name": "id",
+            "name": "first_name",
             "description": null,
             "args": [],
             "type": {
               "kind": "NON_NULL",
               "name": null,
               "ofType": {
                 "kind": "SCALAR",
-                "name": "Int",
+                "name": "String",
                 "ofType": null
               }
             },
@@ -296,7 +296,7 @@
             "description": null,
             "args": [
               {
-                "name": "id",
+                "name": "author_id",
                 "description": null,
                 "type": {
                   "kind": "NON_NULL",

diff --git a/v3/gds/tests/execute/relationships/expected.json b/v3/gds/tests/execute/relationships/expected.json
@@ -1,13 +1,13 @@
 {
   "AuthorByID": {
-    "id": 1,
+    "author_id": 1,
     "first_name": "Peter",
     "nested_relationship_articles_with_alias": [
       {
         "id": 1,
         "title": "The Next 700 Programming Languages",
         "Author": {
-          "id": 1,
+          "author_id": 1,
           "first_name": "Peter"
         }
       }

diff --git a/v3/gds/tests/execute/relationships/request.gql b/v3/gds/tests/execute/relationships/request.gql
@@ -1,12 +1,12 @@
 query {
-    AuthorByID(id: 1) {
-        id
+    AuthorByID(author_id: 1) {
+        author_id
         first_name
         nested_relationship_articles_with_alias: Articles {
             id
             title
             Author {
-              id
+              author_id
               first_name
             }
         }

diff --git a/v3/gds/tests/schema.json b/v3/gds/tests/schema.json
@@ -180,7 +180,7 @@
         "name": "author",
         "fields": [
             {
-                "name": "id",
+                "name": "author_id",
                 "type": "Int"
             },
             {
@@ -224,7 +224,7 @@
         "typeMappings": {
             "author": {
                 "fieldMappings": {
-                    "id": {
+                    "author_id": {
                         "column": "id"
                     }
                 }
@@ -264,7 +264,7 @@
       "typeName": "author",
       "outputPermissions": {
           "admin": {
-              "fields": ["id", "first_name"]
+              "fields": ["author_id", "first_name"]
           }
       }
     },
@@ -296,7 +296,7 @@
             {
                 "queryRootField": "AuthorByID",
                 "uniqueIdentifier": [
-                    "id"
+                    "author_id"
                 ]
             }
         ]
@@ -406,7 +406,7 @@
                 "source": {
                     "fieldPath": [
                         {
-                            "fieldName": "id"
+                            "fieldName": "author_id"
                         }
                     ]
                 },
@@ -440,11 +440,11 @@
                 "target": {
                     "modelField": [
                         {
-                            "fieldName": "id"
+                            "fieldName": "author_id"
                         }
                     ]
                 }
             }
         ]
     }
-]
+]
diff --git a/v3/open-dds/examples/reference.json b/v3/open-dds/examples/reference.json
@@ -164,7 +164,7 @@
         "name": "author",
         "fields": [
             {
-                "name": "id",
+                "name": "author_id",
                 "type": "Int"
             },
             {
@@ -208,7 +208,7 @@
         "typeMappings": {
             "author": {
                 "fieldMappings": {
-                    "id": {
+                    "author_id": {
                         "column": "id"
                     },
                     "first_name": {
@@ -252,7 +252,7 @@
       "typeName": "author",
       "outputPermissions": {
           "admin": {
-              "fields": ["id", "first_name", "last_name"]
+              "fields": ["author_id", "first_name", "last_name"]
           }
       }
     },
@@ -284,7 +284,7 @@
             {
                 "queryRootField": "AuthorByID",
                 "uniqueIdentifier": [
-                    "id"
+                    "author_id"
                 ]
             }
         ]
@@ -314,7 +314,7 @@
                 "source": {
                     "fieldPath": [
                         {
-                            "fieldName": "id"
+                            "fieldName": "author_id"
                         }
                     ]
                 },
@@ -348,7 +348,7 @@
                 "target": {
                     "modelField": [
                         {
-                            "fieldName": "id"
+                            "fieldName": "author_id"
                         }
                     ]
                 }