From 5f07ebe618c9acc7acdd3fd574a696b9e9c1d35e Mon Sep 17 00:00:00 2001 From: Raminder Singh Date: Tue, 27 Feb 2024 12:34:22 +0530 Subject: [PATCH] fixed a bug in can_merge function Also changed the comparison algorithm to O(n + m) from O(nm) --- src/builder.rs | 16 +++++++++++ src/merge.rs | 69 +++++++++++++++++++++++++++++++++++++++++++--- src/parser_util.rs | 3 ++ src/resolve.rs | 6 ++++ 4 files changed, 90 insertions(+), 4 deletions(-) diff --git a/src/builder.rs b/src/builder.rs index 3847ef63..4ed5df54 100644 --- a/src/builder.rs +++ b/src/builder.rs @@ -5,6 +5,7 @@ use crate::sql_types::*; use graphql_parser::query::*; use serde::Serialize; use std::collections::HashMap; +use std::hash::Hash; use std::ops::Deref; use std::str::FromStr; use std::sync::Arc; @@ -264,6 +265,7 @@ pub fn to_insert_builder<'a, T>( ) -> Result where T: Text<'a> + Eq + AsRef + Clone, + T::Value: Hash, { let type_ = field.type_().unmodified_type(); let type_name = type_ @@ -429,6 +431,7 @@ pub fn to_update_builder<'a, T>( ) -> Result where T: Text<'a> + Eq + AsRef + Clone, + T::Value: Hash, { let type_ = field.type_().unmodified_type(); let type_name = type_ @@ -534,6 +537,7 @@ pub fn to_delete_builder<'a, T>( ) -> Result where T: Text<'a> + Eq + AsRef + Clone, + T::Value: Hash, { let type_ = field.type_().unmodified_type(); let type_name = type_ @@ -643,6 +647,7 @@ pub fn to_function_call_builder<'a, T>( ) -> Result where T: Text<'a> + Eq + AsRef + Clone, + T::Value: Hash, { let type_ = field.type_().unmodified_type(); let alias = alias_or_name(query_field); @@ -1337,6 +1342,7 @@ pub fn to_connection_builder<'a, T>( ) -> Result where T: Text<'a> + Eq + AsRef + Clone, + T::Value: Hash, { let type_ = field.type_().unmodified_type(); let type_ = type_.return_type(); @@ -1510,6 +1516,7 @@ fn to_page_info_builder<'a, T>( ) -> Result where T: Text<'a> + Eq + AsRef + Clone, + T::Value: Hash, { let type_ = field.type_().unmodified_type(); let type_name = type_.name().ok_or(format!( @@ -1572,6 +1579,7 @@ fn to_edge_builder<'a, T>( ) -> Result where T: Text<'a> + Eq + AsRef + Clone, + T::Value: Hash, { let type_ = field.type_().unmodified_type(); let type_name = type_.name().ok_or(format!( @@ -1639,6 +1647,7 @@ pub fn to_node_builder<'a, T>( ) -> Result where T: Text<'a> + Eq + AsRef + Clone, + T::Value: Hash, { let type_ = field.type_().unmodified_type(); @@ -1982,6 +1991,7 @@ impl __Schema { ) -> Result<__EnumValueBuilder, String> where T: Text<'a> + Eq + AsRef + Clone, + T::Value: Hash, { let selection_fields = normalize_selection_set( &query_field.selection_set, @@ -2034,6 +2044,7 @@ impl __Schema { ) -> Result<__InputValueBuilder, String> where T: Text<'a> + Eq + AsRef + Clone, + T::Value: Hash, { let selection_fields = normalize_selection_set( &query_field.selection_set, @@ -2099,6 +2110,7 @@ impl __Schema { ) -> Result<__FieldBuilder, String> where T: Text<'a> + Eq + AsRef + Clone, + T::Value: Hash, { let selection_fields = normalize_selection_set( &query_field.selection_set, @@ -2175,6 +2187,7 @@ impl __Schema { ) -> Result, String> where T: Text<'a> + Eq + AsRef + Clone, + T::Value: Hash, { if field.type_.unmodified_type() != __Type::__Type(__TypeType {}) { return Err("can not build query for non-__type type".to_string()); @@ -2227,6 +2240,7 @@ impl __Schema { ) -> Result<__TypeBuilder, String> where T: Text<'a> + Eq + AsRef + Clone, + T::Value: Hash, { let field_map = field_map(&__Type::__Type(__TypeType {})); @@ -2421,6 +2435,7 @@ impl __Schema { ) -> Result<__DirectiveBuilder, String> where T: Text<'a> + Eq + AsRef + Clone, + T::Value: Hash, { let selection_fields = normalize_selection_set( &query_field.selection_set, @@ -2490,6 +2505,7 @@ impl __Schema { ) -> Result<__SchemaBuilder, String> where T: Text<'a> + Eq + AsRef + Clone, + T::Value: Hash, { let type_ = field.type_.unmodified_type(); let type_name = type_ diff --git a/src/merge.rs b/src/merge.rs index 87a4f22b..fb7ca945 100644 --- a/src/merge.rs +++ b/src/merge.rs @@ -1,3 +1,5 @@ +use std::{collections::HashMap, hash::Hash}; + use graphql_parser::query::{Field, Text, Value}; use indexmap::IndexMap; @@ -5,7 +7,7 @@ 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 fields' children are combined into the existing field's +/// 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. @@ -13,6 +15,7 @@ use crate::parser_util::alias_or_name; pub fn merge<'a, 'b, T>(fields: Vec>) -> Result>, String> where T: Text<'a> + Eq + AsRef, + T::Value: Hash, { let mut merged: IndexMap> = IndexMap::new(); @@ -41,6 +44,7 @@ where fn can_merge<'a, T>(field_a: &Field<'a, T>, field_b: &Field<'a, T>) -> Result where T: Text<'a> + Eq + AsRef, + T::Value: Hash, { if field_a.name != field_b.name { return Err(format!( @@ -59,24 +63,81 @@ where 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, + 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 { - for (arg_b_name, arg_b_val) in arguments_b { - if arg_a_name == arg_b_name && arg_a_val != arg_b_val { - return false; + 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); + } +} diff --git a/src/parser_util.rs b/src/parser_util.rs index e40a8c17..35a9914a 100644 --- a/src/parser_util.rs +++ b/src/parser_util.rs @@ -2,6 +2,7 @@ use crate::graphql::{EnumSource, __InputValue, __Type, ___Type}; 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 @@ -22,6 +23,7 @@ pub fn normalize_selection_set<'a, 'b, T>( ) -> Result>, String> where T: Text<'a> + Eq + AsRef + Clone, + T::Value: Hash, { let mut selections: Vec> = vec![]; @@ -139,6 +141,7 @@ pub fn normalize_selection<'a, 'b, T>( ) -> Result>, String> where T: Text<'a> + Eq + AsRef + Clone, + T::Value: Hash, { let mut selections: Vec> = vec![]; diff --git a/src/resolve.rs b/src/resolve.rs index 7395b8b4..9a87e77e 100644 --- a/src/resolve.rs +++ b/src/resolve.rs @@ -1,4 +1,5 @@ use std::collections::HashSet; +use std::hash::Hash; use crate::builder::*; use crate::graphql::*; @@ -23,6 +24,7 @@ pub fn resolve_inner<'a, T>( ) -> GraphQLResponse where T: Text<'a> + Eq + AsRef + Clone, + T::Value: Hash, { match variables { serde_json::Value::Object(_) => (), @@ -131,6 +133,7 @@ fn resolve_query<'a, 'b, T>( ) -> GraphQLResponse where T: Text<'a> + Eq + AsRef + Clone, + T::Value: Hash, { let variable_definitions = &query.variable_definitions; resolve_selection_set( @@ -151,6 +154,7 @@ fn resolve_selection_set<'a, 'b, T>( ) -> GraphQLResponse where T: Text<'a> + Eq + AsRef + Clone, + T::Value: Hash, { use crate::graphql::*; @@ -338,6 +342,7 @@ fn resolve_mutation<'a, 'b, T>( ) -> GraphQLResponse where T: Text<'a> + Eq + AsRef + Clone, + T::Value: Hash, { let variable_definitions = &query.variable_definitions; resolve_mutation_selection_set( @@ -358,6 +363,7 @@ fn resolve_mutation_selection_set<'a, 'b, T>( ) -> GraphQLResponse where T: Text<'a> + Eq + AsRef + Clone, + T::Value: Hash, { use crate::graphql::*;