From 01fad33fcb720f2249de13715f035f4871628d7c Mon Sep 17 00:00:00 2001 From: laststylebender Date: Thu, 5 Sep 2024 23:20:59 +0530 Subject: [PATCH 1/6] - upon type change, handle it in all types. --- .../config/transformer/improve_type_names.rs | 35 +-------- src/core/config/transformer/rename_types.rs | 75 +++++++++++++++++-- ...test__should_generate_combined_config.snap | 10 +-- ...nerator__gen_json_proto_mix_config.md.snap | 10 +-- 4 files changed, 82 insertions(+), 48 deletions(-) diff --git a/src/core/config/transformer/improve_type_names.rs b/src/core/config/transformer/improve_type_names.rs index 93f270eda2..6da657de8c 100644 --- a/src/core/config/transformer/improve_type_names.rs +++ b/src/core/config/transformer/improve_type_names.rs @@ -2,6 +2,7 @@ use std::collections::{BTreeMap, HashSet}; use convert_case::{Case, Casing}; +use super::RenameTypes; use crate::core::config::Config; use crate::core::transform::Transform; use crate::core::valid::Valid; @@ -113,42 +114,12 @@ impl<'a> CandidateGeneration<'a> { #[derive(Default)] pub struct ImproveTypeNames; -impl ImproveTypeNames { - /// Generates type names based on inferred candidates from the provided - /// configuration. - fn generate_type_names(&self, mut config: Config) -> Config { - let finalized_candidates = CandidateGeneration::new(&config).generate().converge(); - - for (old_type_name, new_type_name) in finalized_candidates { - if let Some(type_) = config.types.remove(old_type_name.as_str()) { - // Add newly generated type. - config.types.insert(new_type_name.to_owned(), type_); - - // Replace all the instances of old name in config. - for actual_type in config.types.values_mut() { - for actual_field in actual_type.fields.values_mut() { - if actual_field.type_of.name() == &old_type_name { - // Update the field's type with the new name - actual_field.type_of = actual_field - .type_of - .clone() - .with_name(new_type_name.to_owned()); - } - } - } - } - } - config - } -} - impl Transform for ImproveTypeNames { type Value = Config; type Error = String; fn transform(&self, config: Config) -> Valid { - let config = self.generate_type_names(config); - - Valid::succeed(config) + let finalized_candidates = CandidateGeneration::new(&config).generate().converge(); + RenameTypes::new(finalized_candidates.iter()).transform(config.clone()) } } diff --git a/src/core/config/transformer/rename_types.rs b/src/core/config/transformer/rename_types.rs index 07a0f90526..73640aeb47 100644 --- a/src/core/config/transformer/rename_types.rs +++ b/src/core/config/transformer/rename_types.rs @@ -1,3 +1,5 @@ +use std::collections::HashSet; + use indexmap::IndexMap; use crate::core::config::Config; @@ -28,12 +30,11 @@ impl Transform for RenameTypes { // Ensure all types exist in the configuration Valid::from_iter(self.0.iter(), |(existing_name, suggested_name)| { - if !config.types.contains_key(existing_name) { - Valid::fail(format!( - "Type '{}' not found in configuration.", - existing_name - )) - } else { + if config.types.contains_key(existing_name) + || config.enums.contains_key(existing_name) + || config.unions.contains_key(existing_name) + { + // handle for the types. if let Some(type_info) = config.types.remove(existing_name) { config.types.insert(suggested_name.to_string(), type_info); lookup.insert(existing_name.clone(), suggested_name.clone()); @@ -46,7 +47,24 @@ impl Transform for RenameTypes { } } + // handle for the enums. + if let Some(type_info) = config.enums.remove(existing_name) { + config.enums.insert(suggested_name.to_string(), type_info); + lookup.insert(existing_name.clone(), suggested_name.clone()); + } + + // handle for the union. + if let Some(type_info) = config.unions.remove(existing_name) { + config.unions.insert(suggested_name.to_string(), type_info); + lookup.insert(existing_name.clone(), suggested_name.clone()); + } + Valid::succeed(()) + } else { + Valid::fail(format!( + "Type '{}' not found in configuration.", + existing_name + )) } }) .map(|_| { @@ -65,6 +83,51 @@ impl Transform for RenameTypes { } } } + + // replace in interface. + type_.implements = type_ + .implements + .iter() + .filter_map(|interface_type_name| { + lookup + .get(interface_type_name) + .cloned() + .or(Some(interface_type_name.clone())) + }) + .collect(); + } + + // replace in the union as well. + for union_type_ in config.unions.values_mut() { + // Collect changes to be made + let mut types_to_remove = HashSet::new(); + let mut types_to_add = HashSet::new(); + + for type_name in union_type_.types.iter() { + if let Some(merge_into_type_name) = lookup.get(type_name) { + types_to_remove.insert(type_name.clone()); + types_to_add.insert(merge_into_type_name.clone()); + } + } + // Apply changes + for type_name in types_to_remove { + union_type_.types.remove(&type_name); + } + + for type_name in types_to_add { + union_type_.types.insert(type_name); + } + } + + // replace in union as well. + for union_type_ in config.unions.values_mut() { + union_type_.types = union_type_ + .types + .iter() + .filter_map(|type_name| { + lookup.get(type_name).cloned().or(Some(type_name.clone())) + }) + .collect(); } config diff --git a/src/core/generator/snapshots/tailcall__core__generator__generator__test__should_generate_combined_config.snap b/src/core/generator/snapshots/tailcall__core__generator__generator__test__should_generate_combined_config.snap index 98e1f7616a..991be951e4 100644 --- a/src/core/generator/snapshots/tailcall__core__generator__generator__test__should_generate_combined_config.snap +++ b/src/core/generator/snapshots/tailcall__core__generator__generator__test__should_generate_combined_config.snap @@ -18,11 +18,11 @@ input news__NewsInput { body: String id: Int postImage: String - status: news__Status + status: Status title: String } -enum news__Status { +enum Status { DELETED DRAFT PUBLISHED @@ -52,7 +52,7 @@ type News { body: String id: Int postImage: String - status: news__Status + status: Status title: String } @@ -80,11 +80,11 @@ type Post { type Query { inCompatibleProperties: InCompatibleProperty @http(path: "/") news__NewsService__AddNews(news: news__NewsInput!): News @grpc(body: "{{.args.news}}", method: "news.NewsService.AddNews") - news__NewsService__DeleteNews(newsId: news__NewsId!): Empty @grpc(body: "{{.args.newsId}}", method: "news.NewsService.DeleteNews") + news__NewsService__DeleteNews(newsId: Id!): Empty @grpc(body: "{{.args.newsId}}", method: "news.NewsService.DeleteNews") news__NewsService__EditNews(news: news__NewsInput!): News @grpc(body: "{{.args.news}}", method: "news.NewsService.EditNews") news__NewsService__GetAllNews: NewsNewsServiceGetMultipleNew @grpc(method: "news.NewsService.GetAllNews") 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") + news__NewsService__GetNews(newsId: Id!): News @grpc(body: "{{.args.newsId}}", method: "news.NewsService.GetNews") post(id: Int! = 1): Post @http(path: "/posts/{{.args.id}}") posts: [Post] @http(path: "/posts?_limit=11") user(id: Int!): User @http(path: "/users/{{.args.id}}") diff --git a/tests/cli/snapshots/cli_spec__test__generator_spec__tests__cli__fixtures__generator__gen_json_proto_mix_config.md.snap b/tests/cli/snapshots/cli_spec__test__generator_spec__tests__cli__fixtures__generator__gen_json_proto_mix_config.md.snap index 74ddc5e649..a0394a0dc8 100644 --- a/tests/cli/snapshots/cli_spec__test__generator_spec__tests__cli__fixtures__generator__gen_json_proto_mix_config.md.snap +++ b/tests/cli/snapshots/cli_spec__test__generator_spec__tests__cli__fixtures__generator__gen_json_proto_mix_config.md.snap @@ -18,11 +18,11 @@ input news__NewsInput { body: String id: Int postImage: String - status: news__Status + status: Status title: String } -enum news__Status { +enum Status { DELETED DRAFT PUBLISHED @@ -51,7 +51,7 @@ type News { body: String id: Int postImage: String - status: news__Status + status: Status title: String } @@ -61,11 +61,11 @@ type NewsNewsServiceGetMultipleNew { type Query { news__NewsService__AddNews(news: news__NewsInput!): News @grpc(body: "{{.args.news}}", method: "news.NewsService.AddNews") - news__NewsService__DeleteNews(newsId: news__NewsId!): Empty @grpc(body: "{{.args.newsId}}", method: "news.NewsService.DeleteNews") + news__NewsService__DeleteNews(newsId: Id!): Empty @grpc(body: "{{.args.newsId}}", method: "news.NewsService.DeleteNews") news__NewsService__EditNews(news: news__NewsInput!): News @grpc(body: "{{.args.news}}", method: "news.NewsService.EditNews") news__NewsService__GetAllNews: NewsNewsServiceGetMultipleNew @grpc(method: "news.NewsService.GetAllNews") 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") + news__NewsService__GetNews(newsId: Id!): News @grpc(body: "{{.args.newsId}}", method: "news.NewsService.GetNews") users: [User] @http(path: "/users") } From 72e0e3c6580cb168e970f746297f0408dba98c8d Mon Sep 17 00:00:00 2001 From: laststylebender Date: Thu, 5 Sep 2024 23:38:49 +0530 Subject: [PATCH 2/6] - add test cases --- src/core/config/transformer/rename_types.rs | 13 ++++++++++--- ...ransformer__rename_types__test__rename_type.snap | 8 ++++++++ 2 files changed, 18 insertions(+), 3 deletions(-) diff --git a/src/core/config/transformer/rename_types.rs b/src/core/config/transformer/rename_types.rs index 73640aeb47..3a0781d859 100644 --- a/src/core/config/transformer/rename_types.rs +++ b/src/core/config/transformer/rename_types.rs @@ -155,14 +155,20 @@ mod test { id: ID! name: String } + type B { + name: String + username: String + } + union FooBar = A | B type Post { id: ID! title: String body: String } - type B { - name: String - username: String + enum Status { + PENDING + STARTED, + COMPLETED } type Query { posts: [Post] @http(path: "/posts") @@ -179,6 +185,7 @@ mod test { "A" => "User", "B" => "InputUser", "Mutation" => "UserMutation", + "Status" => "TaskStatus" } .iter(), ) diff --git a/src/core/config/transformer/snapshots/tailcall__core__config__transformer__rename_types__test__rename_type.snap b/src/core/config/transformer/snapshots/tailcall__core__config__transformer__rename_types__test__rename_type.snap index 72c1602857..6f3669a884 100644 --- a/src/core/config/transformer/snapshots/tailcall__core__config__transformer__rename_types__test__rename_type.snap +++ b/src/core/config/transformer/snapshots/tailcall__core__config__transformer__rename_types__test__rename_type.snap @@ -11,6 +11,14 @@ input InputUser { username: String } +union FooBar = InputUser | User + +enum TaskStatus { + COMPLETED + PENDING + STARTED +} + type Post { body: String id: ID! From 1812dc34b0283c9a0444366399e209c2aaf52a2b Mon Sep 17 00:00:00 2001 From: laststylebender Date: Thu, 5 Sep 2024 23:48:44 +0530 Subject: [PATCH 3/6] - added unit test for interface --- src/core/config/transformer/rename_types.rs | 26 +++++++++++++++++++ ..._rename_types__test__inferface_rename.snap | 20 ++++++++++++++ 2 files changed, 46 insertions(+) create mode 100644 src/core/config/transformer/snapshots/tailcall__core__config__transformer__rename_types__test__inferface_rename.snap diff --git a/src/core/config/transformer/rename_types.rs b/src/core/config/transformer/rename_types.rs index 3a0781d859..694c89589c 100644 --- a/src/core/config/transformer/rename_types.rs +++ b/src/core/config/transformer/rename_types.rs @@ -254,4 +254,30 @@ mod test { let expected = Err(b_err.combine(c_err)); assert_eq!(actual, expected); } + + #[test] + fn test_inferface_rename() { + let sdl = r#" + schema { + query: Query + } + interface Node { + id: ID + } + type Post implements Node { + id: ID + title: String + } + type Query { + posts: [Post] @http(path: "/posts") + } + "#; + let config = Config::from_sdl(sdl).to_result().unwrap(); + + let result = RenameTypes::new(hashmap! {"Node" => "NodeTest"}.iter()) + .transform(config) + .to_result() + .unwrap(); + insta::assert_snapshot!(result.to_sdl()) + } } diff --git a/src/core/config/transformer/snapshots/tailcall__core__config__transformer__rename_types__test__inferface_rename.snap b/src/core/config/transformer/snapshots/tailcall__core__config__transformer__rename_types__test__inferface_rename.snap new file mode 100644 index 0000000000..06f20b4e3e --- /dev/null +++ b/src/core/config/transformer/snapshots/tailcall__core__config__transformer__rename_types__test__inferface_rename.snap @@ -0,0 +1,20 @@ +--- +source: src/core/config/transformer/rename_types.rs +expression: result.to_sdl() +--- +schema @server @upstream { + query: Query +} + +interface NodeTest { + id: ID +} + +type Post implements NodeTest { + id: ID + title: String +} + +type Query { + posts: [Post] @http(path: "/posts") +} From e861c3f0412f3d38ac8b7d9f1b27284a07e579f4 Mon Sep 17 00:00:00 2001 From: laststylebender Date: Thu, 5 Sep 2024 23:51:13 +0530 Subject: [PATCH 4/6] - consume config instead of cloning --- src/core/config/transformer/improve_type_names.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/core/config/transformer/improve_type_names.rs b/src/core/config/transformer/improve_type_names.rs index 6da657de8c..a780a40750 100644 --- a/src/core/config/transformer/improve_type_names.rs +++ b/src/core/config/transformer/improve_type_names.rs @@ -119,7 +119,7 @@ impl Transform for ImproveTypeNames { type Error = String; fn transform(&self, config: Config) -> Valid { let finalized_candidates = CandidateGeneration::new(&config).generate().converge(); - RenameTypes::new(finalized_candidates.iter()).transform(config.clone()) + RenameTypes::new(finalized_candidates.iter()).transform(config) } } From 4aa3ce3aa904ee94300c66543e221f2b8b03f43b Mon Sep 17 00:00:00 2001 From: laststylebender Date: Thu, 5 Sep 2024 23:53:35 +0530 Subject: [PATCH 5/6] - variable rename --- src/core/config/transformer/rename_types.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/core/config/transformer/rename_types.rs b/src/core/config/transformer/rename_types.rs index 694c89589c..d35b4d63a7 100644 --- a/src/core/config/transformer/rename_types.rs +++ b/src/core/config/transformer/rename_types.rs @@ -104,9 +104,9 @@ impl Transform for RenameTypes { let mut types_to_add = HashSet::new(); for type_name in union_type_.types.iter() { - if let Some(merge_into_type_name) = lookup.get(type_name) { + if let Some(new_type_name) = lookup.get(type_name) { types_to_remove.insert(type_name.clone()); - types_to_add.insert(merge_into_type_name.clone()); + types_to_add.insert(new_type_name.clone()); } } // Apply changes From 484a0e14ebfdd5a5315ae984788e944ce65c5174 Mon Sep 17 00:00:00 2001 From: laststylebender Date: Mon, 16 Sep 2024 12:01:52 +0530 Subject: [PATCH 6/6] - eval unwraps lazy --- src/core/config/transformer/rename_types.rs | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/src/core/config/transformer/rename_types.rs b/src/core/config/transformer/rename_types.rs index d35b4d63a7..1ff72b9c2c 100644 --- a/src/core/config/transformer/rename_types.rs +++ b/src/core/config/transformer/rename_types.rs @@ -88,11 +88,11 @@ impl Transform for RenameTypes { type_.implements = type_ .implements .iter() - .filter_map(|interface_type_name| { + .map(|interface_type_name| { lookup .get(interface_type_name) .cloned() - .or(Some(interface_type_name.clone())) + .unwrap_or_else(|| interface_type_name.to_owned()) }) .collect(); } @@ -124,8 +124,11 @@ impl Transform for RenameTypes { union_type_.types = union_type_ .types .iter() - .filter_map(|type_name| { - lookup.get(type_name).cloned().or(Some(type_name.clone())) + .map(|type_name| { + lookup + .get(type_name) + .cloned() + .unwrap_or_else(|| type_name.to_owned()) }) .collect(); }