Merge pull request #498 from supabase/merge_fields_new

Reimplement field merging

Author: olirice

Cargo.lock

@@ -25,6 +25,7 @@ uuid = "1"
 base64 = "0.13"
 lazy_static = "1"
 bimap = { version = "0.6.3", features = ["serde"] }
+indexmap = "2.2"
 
 [dev-dependencies]
 pgrx-tests = "=0.11.2"

Cargo.toml

@@ -8,6 +8,7 @@ use serde_json::json;
 mod builder;
 mod graphql;
 mod gson;
+mod merge;
 mod omit;
 mod parser_util;
 mod resolve;

src/builder.rs

@@ -0,0 +1,143 @@
+use std::{collections::HashMap, hash::Hash};
+
+use graphql_parser::query::{Field, Text, Value};
+use indexmap::IndexMap;
+
+use crate::parser_util::alias_or_name;
+
+/// Merges duplicates in a vector of fields. The fields in the vector are added to a
+/// map from field name to field. If a field with the same name already exists in the
+/// map, the existing and new field's children are combined into the existing field's
+/// children. These children will be merged later when they are normalized.
+///
+/// The map is an `IndexMap` to ensure iteration order of the fields is preserved.
+/// This prevents tests from being flaky due to field order changing between test runs.
+pub fn merge<'a, 'b, T>(fields: Vec<Field<'a, T>>) -> Result<Vec<Field<'a, T>>, String>
+where
+    T: Text<'a> + Eq + AsRef<str>,
+    T::Value: Hash,
+{
+    let mut merged: IndexMap<String, Field<'a, T>> = IndexMap::new();
+
+    for current_field in fields {
+        let response_key = alias_or_name(&current_field);
+        match merged.get_mut(&response_key) {
+            Some(existing_field) => {
+                if can_merge(&current_field, existing_field)? {
+                    existing_field
+                        .selection_set
+                        .items
+                        .extend(current_field.selection_set.items);
+                }
+            }
+            None => {
+                merged.insert(response_key, current_field);
+            }
+        }
+    }
+
+    let fields = merged.into_iter().map(|(_, field)| field).collect();
+
+    Ok(fields)
+}
+
+fn can_merge<'a, T>(field_a: &Field<'a, T>, field_b: &Field<'a, T>) -> Result<bool, String>
+where
+    T: Text<'a> + Eq + AsRef<str>,
+    T::Value: Hash,
+{
+    if field_a.name != field_b.name {
+        return Err(format!(
+            "Fields `{}` and `{}` are different",
+            field_a.name.as_ref(),
+            field_b.name.as_ref(),
+        ));
+    }
+    if !same_arguments(&field_a.arguments, &field_b.arguments) {
+        return Err(format!(
+            "Two fields named `{}` have different arguments",
+            field_a.name.as_ref(),
+        ));
+    }
+
+    Ok(true)
+}
+
+/// Compares two sets of arguments and returns true only if
+/// both are the same. `arguments_a` should not have two
+/// arguments with the same name. Similarly `arguments_b`
+/// should not have duplicates. It is assumed that [Argument
+/// Uniqueness] validation has already been run by the time
+/// this function is called.
+///
+/// [Argument Uniqueness]: https://spec.graphql.org/October2021/#sec-Argument-Uniqueness
+fn same_arguments<'a, 'b, T>(
+    arguments_a: &[(T::Value, Value<'a, T>)],
+    arguments_b: &[(T::Value, Value<'a, T>)],
+) -> bool
+where
+    T: Text<'a> + Eq + AsRef<str>,
+    T::Value: Hash,
+{
+    if arguments_a.len() != arguments_b.len() {
+        return false;
+    }
+
+    let mut arguments_a_map = HashMap::new();
+    for (arg_a_name, arg_a_val) in arguments_a {
+        arguments_a_map.insert(arg_a_name, arg_a_val);
+    }
+
+    for (arg_b_name, arg_b_val) in arguments_b {
+        match arguments_a_map.get(arg_b_name) {
+            Some(arg_a_val) => {
+                if *arg_a_val != arg_b_val {
+                    return false;
+                }
+            }
+            None => return false,
+        }
+    }
+
+    true
+}
+
+#[cfg(test)]
+mod tests {
+
+    use super::same_arguments;
+    use graphql_parser::query::Value;
+
+    #[test]
+    fn same_args_test() {
+        let arguments_a = vec![("a", Value::Int(1.into()))];
+        let arguments_b = vec![("a", Value::Int(1.into()))];
+        let result = same_arguments::<&str>(&arguments_a, &arguments_b);
+        assert!(result);
+
+        let arguments_a = vec![("a", Value::Int(1.into())), ("b", Value::Int(2.into()))];
+        let arguments_b = vec![("a", Value::Int(1.into())), ("b", Value::Int(2.into()))];
+        let result = same_arguments::<&str>(&arguments_a, &arguments_b);
+        assert!(result);
+
+        let arguments_a = vec![("a", Value::Int(1.into())), ("b", Value::Int(2.into()))];
+        let arguments_b = vec![("b", Value::Int(2.into())), ("a", Value::Int(1.into()))];
+        let result = same_arguments::<&str>(&arguments_a, &arguments_b);
+        assert!(result);
+
+        let arguments_a = vec![("a", Value::Int(1.into()))];
+        let arguments_b = vec![("a", Value::Int(2.into()))];
+        let result = same_arguments::<&str>(&arguments_a, &arguments_b);
+        assert!(!result);
+
+        let arguments_a = vec![("a", Value::Int(1.into())), ("b", Value::Int(1.into()))];
+        let arguments_b = vec![("a", Value::Int(1.into()))];
+        let result = same_arguments::<&str>(&arguments_a, &arguments_b);
+        assert!(!result);
+
+        let arguments_a = vec![("a", Value::Int(1.into()))];
+        let arguments_b = vec![("b", Value::Int(1.into()))];
+        let result = same_arguments::<&str>(&arguments_a, &arguments_b);
+        assert!(!result);
+    }
+}

src/lib.rs

@@ -1,7 +1,8 @@
 use crate::graphql::{EnumSource, __InputValue, __Type, ___Type};
-use crate::gson;
+use crate::{gson, merge::merge};
 use graphql_parser::query::*;
 use std::collections::HashMap;
+use std::hash::Hash;
 
 pub fn alias_or_name<'a, T>(query_field: &graphql_parser::query::Field<'a, T>) -> String
 where
@@ -19,11 +20,12 @@ pub fn normalize_selection_set<'a, 'b, T>(
     fragment_definitions: &'b Vec<FragmentDefinition<'a, T>>,
     type_name: &String,            // for inline fragments
     variables: &serde_json::Value, // for directives
-) -> Result<Vec<&'b Field<'a, T>>, String>
+) -> Result<Vec<Field<'a, T>>, String>
 where
-    T: Text<'a> + Eq + AsRef<str>,
+    T: Text<'a> + Eq + AsRef<str> + Clone,
+    T::Value: Hash,
 {
-    let mut selections: Vec<&'b Field<'a, T>> = vec![];
+    let mut selections: Vec<Field<'a, T>> = vec![];
 
     for selection in &selection_set.items {
         let sel = selection;
@@ -32,6 +34,7 @@ where
             Err(err) => return Err(err),
         }
     }
+    let selections = merge(selections)?;
     Ok(selections)
 }
 
@@ -135,19 +138,20 @@ pub fn normalize_selection<'a, 'b, T>(
     fragment_definitions: &'b Vec<FragmentDefinition<'a, T>>,
     type_name: &String,            // for inline fragments
     variables: &serde_json::Value, // for directives
-) -> Result<Vec<&'b Field<'a, T>>, String>
+) -> Result<Vec<Field<'a, T>>, String>
 where
-    T: Text<'a> + Eq + AsRef<str>,
+    T: Text<'a> + Eq + AsRef<str> + Clone,
+    T::Value: Hash,
 {
-    let mut selections: Vec<&Field<'a, T>> = vec![];
+    let mut selections: Vec<Field<'a, T>> = vec![];
 
     if selection_is_skipped(query_selection, variables)? {
         return Ok(selections);
     }
 
     match query_selection {
         Selection::Field(field) => {
-            selections.push(field);
+            selections.push(field.clone());
         }
         Selection::FragmentSpread(fragment_spread) => {
             let frag_name = &fragment_spread.fragment_name;
@@ -180,7 +184,7 @@ where
                 variables,
             );
             match frag_selections {
-                Ok(sels) => selections.extend(sels.iter()),
+                Ok(sels) => selections.extend(sels),
                 Err(err) => return Err(err),
             };
         }
@@ -199,7 +203,7 @@ where
                     type_name,
                     variables,
                 )?;
-                selections.extend(infrag_selections.iter());
+                selections.extend(infrag_selections);
             }
         }
     }

src/merge.rs

@@ -1,4 +1,5 @@
 use std::collections::HashSet;
+use std::hash::Hash;
 
 use crate::builder::*;
 use crate::graphql::*;
@@ -22,7 +23,8 @@ pub fn resolve_inner<'a, T>(
     schema: &__Schema,
 ) -> GraphQLResponse
 where
-    T: Text<'a> + Eq + AsRef<str>,
+    T: Text<'a> + Eq + AsRef<str> + Clone,
+    T::Value: Hash,
 {
     match variables {
         serde_json::Value::Object(_) => (),
@@ -130,7 +132,8 @@ fn resolve_query<'a, 'b, T>(
     fragment_definitions: Vec<FragmentDefinition<'a, T>>,
 ) -> GraphQLResponse
 where
-    T: Text<'a> + Eq + AsRef<str>,
+    T: Text<'a> + Eq + AsRef<str> + Clone,
+    T::Value: Hash,
 {
     let variable_definitions = &query.variable_definitions;
     resolve_selection_set(
@@ -150,7 +153,8 @@ fn resolve_selection_set<'a, 'b, T>(
     variable_definitions: &Vec<VariableDefinition<'a, T>>,
 ) -> GraphQLResponse
 where
-    T: Text<'a> + Eq + AsRef<str>,
+    T: Text<'a> + Eq + AsRef<str> + Clone,
+    T::Value: Hash,
 {
     use crate::graphql::*;
 
@@ -337,7 +341,8 @@ fn resolve_mutation<'a, 'b, T>(
     fragment_definitions: Vec<FragmentDefinition<'a, T>>,
 ) -> GraphQLResponse
 where
-    T: Text<'a> + Eq + AsRef<str>,
+    T: Text<'a> + Eq + AsRef<str> + Clone,
+    T::Value: Hash,
 {
     let variable_definitions = &query.variable_definitions;
     resolve_mutation_selection_set(
@@ -357,7 +362,8 @@ fn resolve_mutation_selection_set<'a, 'b, T>(
     variable_definitions: &Vec<VariableDefinition<'a, T>>,
 ) -> GraphQLResponse
 where
-    T: Text<'a> + Eq + AsRef<str>,
+    T: Text<'a> + Eq + AsRef<str> + Clone,
+    T::Value: Hash,
 {
     use crate::graphql::*;