From 336c4b208d4c02517a9170697879e7363155c34c Mon Sep 17 00:00:00 2001 From: Tushar Mathur Date: Sun, 23 Jun 2024 19:29:39 +0530 Subject: [PATCH] Revert "chore: dropped pub fields from config module" (#2271) --- Cargo.lock | 12 ----- Cargo.toml | 1 - benches/handle_request_bench.rs | 2 +- src/cli/server/http_server.rs | 2 +- src/cli/tc.rs | 10 ++-- src/core/blueprint/auth.rs | 4 +- src/core/blueprint/definitions.rs | 8 ++-- src/core/blueprint/operators/call.rs | 2 +- src/core/blueprint/operators/grpc.rs | 2 +- src/core/blueprint/operators/js.rs | 2 +- src/core/blueprint/operators/protected.rs | 4 +- src/core/blueprint/server.rs | 8 ++-- src/core/blueprint/upstream.rs | 2 +- src/core/config/config_module.rs | 46 +++---------------- src/core/config/into_document.rs | 4 +- src/core/config/reader.rs | 33 ++++++------- src/core/config/transformer/ambiguous_type.rs | 2 +- src/core/generator/generator.rs | 10 ++-- .../generator/tests/json_to_config_spec.rs | 2 +- src/core/grpc/data_loader_request.rs | 2 +- src/core/grpc/protobuf.rs | 2 +- src/core/grpc/request_template.rs | 2 +- tailcall-aws-lambda/src/main.rs | 2 +- tests/core/parse.rs | 2 +- 24 files changed, 61 insertions(+), 105 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 284cf567dc..8a2f1a6a14 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1235,17 +1235,6 @@ dependencies = [ "powerfmt", ] -[[package]] -name = "derive-getters" -version = "0.4.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "0a6433aac097572ea8ccc60b3f2e756c661c9aeed9225cdd4d0cb119cb7ff6ba" -dependencies = [ - "proc-macro2", - "quote", - "syn 2.0.67", -] - [[package]] name = "derive_builder" version = "0.13.1" @@ -5094,7 +5083,6 @@ dependencies = [ "convert_case", "criterion", "datatest-stable", - "derive-getters", "derive_setters", "dotenvy", "exitcode", diff --git a/Cargo.toml b/Cargo.toml index 7f81450d91..aae4bd02d7 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -161,7 +161,6 @@ Inflector = "0.11.4" path-clean = "=1.0.1" pathdiff = "0.2.1" num = "0.4.3" -derive-getters = "0.4.0" [dev-dependencies] tailcall-prettier = { path = "tailcall-prettier" } diff --git a/benches/handle_request_bench.rs b/benches/handle_request_bench.rs index 5f0aee1cf2..0c4940a274 100644 --- a/benches/handle_request_bench.rs +++ b/benches/handle_request_bench.rs @@ -17,7 +17,7 @@ pub fn benchmark_handle_request(c: &mut Criterion) { let config_module: ConfigModule = Config::from_sdl(sdl.as_str()).to_result().unwrap().into(); let blueprint = Blueprint::try_from(&config_module).unwrap(); - let endpoints = config_module.into_extensions().endpoint_set; + let endpoints = config_module.extensions.endpoint_set; let server_config = tokio_runtime .block_on(ServerConfig::new(blueprint, endpoints)) diff --git a/src/cli/server/http_server.rs b/src/cli/server/http_server.rs index d815f567ea..d27cfeaab8 100644 --- a/src/cli/server/http_server.rs +++ b/src/cli/server/http_server.rs @@ -36,7 +36,7 @@ impl Server { let server_config = Arc::new( ServerConfig::new( blueprint.clone(), - self.config_module.into_extensions().endpoint_set, + self.config_module.extensions.endpoint_set, ) .await?, ); diff --git a/src/cli/tc.rs b/src/cli/tc.rs index 5ef89951ec..1e3e140433 100644 --- a/src/cli/tc.rs +++ b/src/cli/tc.rs @@ -48,15 +48,15 @@ pub async fn run() -> Result<()> { match cli.command { Command::Start { file_paths } => { let config_module = config_reader.read_all(&file_paths).await?; - log_endpoint_set(&config_module.extensions().endpoint_set); - Fmt::log_n_plus_one(false, config_module.config()); + log_endpoint_set(&config_module.extensions.endpoint_set); + Fmt::log_n_plus_one(false, &config_module.config); let server = Server::new(config_module); server.fork_start().await?; Ok(()) } Command::Check { file_paths, n_plus_one_queries, schema, format } => { let config_module = (config_reader.read_all(&file_paths)).await?; - log_endpoint_set(&config_module.extensions().endpoint_set); + log_endpoint_set(&config_module.extensions.endpoint_set); if let Some(format) = format { Fmt::display(format.encode(&config_module)?); } @@ -65,10 +65,10 @@ pub async fn run() -> Result<()> { match blueprint { Ok(blueprint) => { tracing::info!("Config {} ... ok", file_paths.join(", ")); - Fmt::log_n_plus_one(n_plus_one_queries, config_module.config()); + Fmt::log_n_plus_one(n_plus_one_queries, &config_module.config); // Check the endpoints' schema let _ = config_module - .into_extensions() + .extensions .endpoint_set .into_checked(&blueprint, runtime) .await?; diff --git a/src/core/blueprint/auth.rs b/src/core/blueprint/auth.rs index 9ca378103a..563ba24c7d 100644 --- a/src/core/blueprint/auth.rs +++ b/src/core/blueprint/auth.rs @@ -34,13 +34,13 @@ pub enum Auth { impl Auth { pub fn make(config_module: &ConfigModule) -> Valid, String> { - let htpasswd = config_module.extensions().htpasswd.iter().map(|htpasswd| { + let htpasswd = config_module.extensions.htpasswd.iter().map(|htpasswd| { Auth::Provider(Provider::Basic(Basic { htpasswd: htpasswd.content.clone(), })) }); - let jwks = config_module.extensions().jwks.iter().map(|jwks| { + let jwks = config_module.extensions.jwks.iter().map(|jwks| { Auth::Provider(Provider::Jwt(Jwt { jwks: jwks.content.clone(), // TODO: read those options from link instead of using defaults diff --git a/src/core/blueprint/definitions.rs b/src/core/blueprint/definitions.rs index 018419ce37..64f9344877 100644 --- a/src/core/blueprint/definitions.rs +++ b/src/core/blueprint/definitions.rs @@ -520,8 +520,8 @@ pub fn to_field_definition( pub fn to_definitions<'a>() -> TryFold<'a, ConfigModule, Vec, String> { TryFold::, String>::new(|config_module, _| { - let output_types = config_module.output_types(); - let input_types = config_module.input_types(); + let output_types = &config_module.output_types; + let input_types = &config_module.input_types; Valid::from_iter(config_module.types.iter(), |(name, type_)| { let dbl_usage = input_types.contains(name) && output_types.contains(name); @@ -534,9 +534,9 @@ pub fn to_definitions<'a>() -> TryFold<'a, ConfigModule, Vec, String .trace(name) .and_then(|definition| match definition.clone() { Definition::Object(object_type_definition) => { - if config_module.input_types().contains(name) { + if config_module.input_types.contains(name) { to_input_object_type_definition(object_type_definition).trace(name) - } else if config_module.interface_types().contains(name) { + } else if config_module.interface_types.contains(name) { to_interface_type_definition(object_type_definition).trace(name) } else { Valid::succeed(definition) diff --git a/src/core/blueprint/operators/call.rs b/src/core/blueprint/operators/call.rs index 0763cfb697..8beaa6b5ce 100644 --- a/src/core/blueprint/operators/call.rs +++ b/src/core/blueprint/operators/call.rs @@ -151,7 +151,7 @@ fn get_field_and_field_name<'a>( ) .and_then(|(type_name, field_name)| { Valid::from_option( - config_module.config().find_type(&type_name), + config_module.config.find_type(&type_name), format!("{} type not found on config", type_name), ) .and_then(|query_type| { diff --git a/src/core/blueprint/operators/grpc.rs b/src/core/blueprint/operators/grpc.rs index ffac9bdc42..bb676d72e3 100644 --- a/src/core/blueprint/operators/grpc.rs +++ b/src/core/blueprint/operators/grpc.rs @@ -165,7 +165,7 @@ pub fn compile_grpc(inputs: CompileGrpc) -> Valid { Valid::from(GrpcMethod::try_from(grpc.method.as_str())) .and_then(|method| { - let file_descriptor_set = config_module.extensions().get_file_descriptor_set(); + let file_descriptor_set = config_module.extensions.get_file_descriptor_set(); if file_descriptor_set.file.is_empty() { return Valid::fail("Protobuf files were not specified in the config".to_string()); diff --git a/src/core/blueprint/operators/js.rs b/src/core/blueprint/operators/js.rs index 0687f15b42..6e08f097d6 100644 --- a/src/core/blueprint/operators/js.rs +++ b/src/core/blueprint/operators/js.rs @@ -25,7 +25,7 @@ pub fn update_js_field<'a>( return Valid::succeed(b_field); }; - compile_js(CompileJs { script: &module.extensions().script, name: &js.name }) + compile_js(CompileJs { script: &module.extensions.script, name: &js.name }) .map(|resolver| b_field.resolver(Some(resolver))) }, ) diff --git a/src/core/blueprint/operators/protected.rs b/src/core/blueprint/operators/protected.rs index 5dff9c50f2..02381d230d 100644 --- a/src/core/blueprint/operators/protected.rs +++ b/src/core/blueprint/operators/protected.rs @@ -17,11 +17,11 @@ pub fn update_protected<'a>( .and_then(|type_| type_.protected.as_ref()) .is_some() { - if config.input_types().contains(type_name) { + if config.input_types.contains(type_name) { return Valid::fail("Input types can not be protected".to_owned()); } - if !config.extensions().has_auth() { + if !config.extensions.has_auth() { return Valid::fail( "@protected operator is used but there is no @link definitions for auth providers".to_owned(), ); diff --git a/src/core/blueprint/server.rs b/src/core/blueprint/server.rs index 28fc99c773..25cf5a21e5 100644 --- a/src/core/blueprint/server.rs +++ b/src/core/blueprint/server.rs @@ -91,15 +91,15 @@ impl TryFrom for Server { let http_server = match config_server.clone().get_version() { HttpVersion::HTTP2 => { - if config_module.extensions().cert.is_empty() { + if config_module.extensions.cert.is_empty() { return Valid::fail("Certificate is required for HTTP2".to_string()) .to_result(); } - let cert = config_module.extensions().cert.clone(); + let cert = config_module.extensions.cert.clone(); let key_file: PrivateKeyDer<'_> = config_module - .extensions() + .extensions .keys .first() .ok_or_else(|| ValidationError::new("Key is required for HTTP2".to_string()))? @@ -160,7 +160,7 @@ impl TryFrom for Server { } fn to_script(config_module: &crate::core::config::ConfigModule) -> Valid, String> { - config_module.extensions().script.as_ref().map_or_else( + config_module.extensions.script.as_ref().map_or_else( || Valid::succeed(None), |script| { Valid::succeed(Some(Script { diff --git a/src/core/blueprint/upstream.rs b/src/core/blueprint/upstream.rs index 6f1abc872e..75a90f5ceb 100644 --- a/src/core/blueprint/upstream.rs +++ b/src/core/blueprint/upstream.rs @@ -57,7 +57,7 @@ impl TryFrom<&ConfigModule> for Upstream { let mut allowed_headers = config_upstream.get_allowed_headers(); - if config_module.extensions().has_auth() { + if config_module.extensions.has_auth() { // force add auth specific headers to use it to make actual validation allowed_headers.insert(hyper::header::AUTHORIZATION.to_string()); } diff --git a/src/core/config/config_module.rs b/src/core/config/config_module.rs index 5b05d7ab05..27b7bbda60 100644 --- a/src/core/config/config_module.rs +++ b/src/core/config/config_module.rs @@ -2,7 +2,7 @@ use std::collections::{HashMap, HashSet}; use std::ops::Deref; use std::sync::Arc; -use derive_getters::Getters; +use derive_setters::Setters; use jsonwebtoken::jwk::JwkSet; use prost_reflect::prost_types::{FileDescriptorProto, FileDescriptorSet}; use rustls_pki_types::{CertificateDer, PrivateKeyDer}; @@ -15,45 +15,13 @@ use crate::core::rest::{EndpointSet, Unchecked}; /// A wrapper on top of Config that contains all the resolved extensions and /// computed values. -#[derive(Clone, Debug, Default, Getters, MergeRight)] +#[derive(Clone, Debug, Default, Setters, MergeRight)] pub struct ConfigModule { - config: Config, - extensions: Extensions, - input_types: HashSet, - output_types: HashSet, - interface_types: HashSet, -} - -impl ConfigModule { - // recompute the values of `input_types`, `output_types` and `interface_types` - pub fn recompute_types(self) -> Self { - let input_types = self.config().input_types(); - let output_types = self.config().output_types(); - let interface_types = self.config().interface_types(); - Self { - config: self.config, - extensions: self.extensions, - input_types, - output_types, - interface_types, - } - } - - pub fn get_config_mut(&mut self) -> &mut Config { - &mut self.config - } - - pub fn get_extensions_mut(&mut self) -> &mut Extensions { - &mut self.extensions - } - - pub fn into_extensions(self) -> Extensions { - self.extensions - } - - pub fn into_config(self) -> Config { - self.config - } + pub config: Config, + pub extensions: Extensions, + pub input_types: HashSet, + pub output_types: HashSet, + pub interface_types: HashSet, } #[derive(Clone, Debug, Default)] diff --git a/src/core/config/into_document.rs b/src/core/config/into_document.rs index d56c31973d..bd2ce0f42c 100644 --- a/src/core/config/into_document.rs +++ b/src/core/config/into_document.rs @@ -59,7 +59,7 @@ fn config_document(config: &ConfigModule) -> ServiceDocument { }; definitions.push(TypeSystemDefinition::Schema(pos(schema_definition))); for (type_name, type_def) in config.types.iter() { - let kind = if config.interface_types().contains(type_name) { + let kind = if config.interface_types.contains(type_name) { TypeKind::Interface(InterfaceType { implements: type_def .implements @@ -91,7 +91,7 @@ fn config_document(config: &ConfigModule) -> ServiceDocument { }) .collect::>>(), }) - } else if config.input_types().contains(type_name) { + } else if config.input_types.contains(type_name) { TypeKind::InputObject(InputObjectType { fields: type_def .fields diff --git a/src/core/config/reader.rs b/src/core/config/reader.rs index d6c4eb4522..71fe0fddf5 100644 --- a/src/core/config/reader.rs +++ b/src/core/config/reader.rs @@ -41,7 +41,7 @@ impl ConfigReader { parent_dir: Option<&'async_recursion Path>, ) -> anyhow::Result { let links: Vec = config_module - .config() + .config .links .clone() .iter() @@ -81,40 +81,38 @@ impl ConfigReader { } LinkType::Protobuf => { let meta = self.proto_reader.read(path).await?; - config_module.get_extensions_mut().add_proto(meta); + config_module.extensions.add_proto(meta); } LinkType::Script => { let source = self.resource_reader.read_file(&path).await?; let content = source.content; - config_module.get_extensions_mut().script = Some(content); + config_module.extensions.script = Some(content); } LinkType::Cert => { let source = self.resource_reader.read_file(&path).await?; let content = source.content; config_module - .get_extensions_mut() + .extensions .cert .extend(self.load_cert(content).await?); } LinkType::Key => { let source = self.resource_reader.read_file(&path).await?; let content = source.content; - config_module.get_extensions_mut().keys = - Arc::new(self.load_private_key(content).await?) + config_module.extensions.keys = Arc::new(self.load_private_key(content).await?) } LinkType::Operation => { let source = self.resource_reader.read_file(&path).await?; let content = source.content; - config_module.get_extensions_mut().endpoint_set = - EndpointSet::try_new(&content)?; + config_module.extensions.endpoint_set = EndpointSet::try_new(&content)?; } LinkType::Htpasswd => { let source = self.resource_reader.read_file(&path).await?; let content = source.content; config_module - .get_extensions_mut() + .extensions .htpasswd .push(Content { id: link.id.clone(), content }); } @@ -124,7 +122,7 @@ impl ConfigReader { let de = &mut serde_json::Deserializer::from_str(&content); - config_module.get_extensions_mut().jwks.push(Content { + config_module.extensions.jwks.push(Content { id: link.id.clone(), content: serde_path_to_error::deserialize(de)?, }) @@ -133,14 +131,17 @@ impl ConfigReader { let meta = self.proto_reader.fetch(link.src.as_str()).await?; for m in meta { - config_module.get_extensions_mut().add_proto(m); + config_module.extensions.add_proto(m); } } } } - let config_module = config_module.recompute_types(); - Ok(config_module) + // Recreating the ConfigModule in order to recompute the values of + // `input_types`, `output_types` and `interface_types` + let mut final_config_module = ConfigModule::from(config_module.config); + final_config_module.extensions = config_module.extensions; + Ok(final_config_module) } /// Reads the certificate from a given file @@ -218,7 +219,7 @@ impl ConfigReader { // Extend it with the links let mut config_module = self.ext_links(config_module, parent_dir).await?; - let server = &config_module.config().server; + let server = &mut config_module.config.server; let reader_ctx = ConfigReaderContext { runtime: &self.runtime, vars: &server @@ -230,7 +231,7 @@ impl ConfigReader { }; config_module - .get_config_mut() + .config .telemetry .render_mustache(&reader_ctx)?; @@ -359,7 +360,7 @@ mod reader_tests { let path = format!("{}/examples/scripts/echo.js", cargo_manifest); let content = file_rt.read(&path).await; - assert_eq!(content.unwrap(), config.into_extensions().script.unwrap()); + assert_eq!(content.unwrap(), config.extensions.script.unwrap()); } #[test] diff --git a/src/core/config/transformer/ambiguous_type.rs b/src/core/config/transformer/ambiguous_type.rs index 6477be8672..c2e0e913a8 100644 --- a/src/core/config/transformer/ambiguous_type.rs +++ b/src/core/config/transformer/ambiguous_type.rs @@ -241,7 +241,7 @@ mod tests { .generate(false)?; let mut config = AmbiguousType::default() - .transform(cfg_module.into_config()) + .transform(cfg_module.config) .to_result()?; // remove links since they break snapshot tests diff --git a/src/core/generator/generator.rs b/src/core/generator/generator.rs index 2933765c85..ba91707df9 100644 --- a/src/core/generator/generator.rs +++ b/src/core/generator/generator.rs @@ -173,7 +173,7 @@ mod test { })]) .generate(false)?; - insta::assert_snapshot!(cfg_module.config().to_sdl()); + insta::assert_snapshot!(cfg_module.config.to_sdl()); Ok(()) } @@ -186,7 +186,7 @@ mod test { }]) .generate(true)?; - insta::assert_snapshot!(cfg_module.config().to_sdl()); + insta::assert_snapshot!(cfg_module.config.to_sdl()); Ok(()) } @@ -202,7 +202,7 @@ mod test { }]) .transformers(vec![Box::new(Preset::default())]) .generate(true)?; - insta::assert_snapshot!(cfg_module.config().to_sdl()); + insta::assert_snapshot!(cfg_module.config.to_sdl()); Ok(()) } @@ -238,7 +238,7 @@ mod test { .generate(true)?; // Assert the combined output - insta::assert_snapshot!(cfg_module.config().to_sdl()); + insta::assert_snapshot!(cfg_module.config.to_sdl()); Ok(()) } @@ -264,7 +264,7 @@ mod test { .inputs(inputs) .transformers(vec![Box::new(Preset::default())]) .generate(true)?; - insta::assert_snapshot!(cfg_module.config().to_sdl()); + insta::assert_snapshot!(cfg_module.config.to_sdl()); Ok(()) } } diff --git a/src/core/generator/tests/json_to_config_spec.rs b/src/core/generator/tests/json_to_config_spec.rs index d14e223be6..e445d7f6c2 100644 --- a/src/core/generator/tests/json_to_config_spec.rs +++ b/src/core/generator/tests/json_to_config_spec.rs @@ -44,7 +44,7 @@ fn test_spec(path: &Path, url: Url, body: Value) -> anyhow::Result<()> { field_name: "f1".to_string(), }]) .generate(true)? - .into_config(); + .config; let snapshot_name = path.file_name().unwrap().to_str().unwrap(); insta::assert_snapshot!(snapshot_name, config.to_sdl()); diff --git a/src/core/grpc/data_loader_request.rs b/src/core/grpc/data_loader_request.rs index 39547f3e1a..cbacf2e787 100644 --- a/src/core/grpc/data_loader_request.rs +++ b/src/core/grpc/data_loader_request.rs @@ -90,7 +90,7 @@ mod tests { let config_module = reader.resolve(config, None).await.unwrap(); let protobuf_set = - ProtobufSet::from_proto_file(config_module.extensions().get_file_descriptor_set()) + ProtobufSet::from_proto_file(config_module.extensions.get_file_descriptor_set()) .unwrap(); let service = protobuf_set.find_service(&method).unwrap(); diff --git a/src/core/grpc/protobuf.rs b/src/core/grpc/protobuf.rs index 7c22202cf2..3561cbcc5e 100644 --- a/src/core/grpc/protobuf.rs +++ b/src/core/grpc/protobuf.rs @@ -278,7 +278,7 @@ pub mod tests { Ok(reader .resolve(config, None) .await? - .extensions() + .extensions .get_file_descriptor_set()) } diff --git a/src/core/grpc/request_template.rs b/src/core/grpc/request_template.rs index 2ef5e2fb66..eb89101787 100644 --- a/src/core/grpc/request_template.rs +++ b/src/core/grpc/request_template.rs @@ -162,7 +162,7 @@ mod tests { .resolve(config, None) .await .unwrap() - .extensions() + .extensions .get_file_descriptor_set(), ) .unwrap(); diff --git a/tailcall-aws-lambda/src/main.rs b/tailcall-aws-lambda/src/main.rs index 9cbf9d0218..6965a7c1cd 100644 --- a/tailcall-aws-lambda/src/main.rs +++ b/tailcall-aws-lambda/src/main.rs @@ -35,7 +35,7 @@ async fn main() -> Result<(), Error> { .await?; let blueprint = Blueprint::try_from(&config)?; let endpoints = config - .into_extensions() + .extensions .endpoint_set .into_checked(&blueprint, runtime.clone()) .await?; diff --git a/tests/core/parse.rs b/tests/core/parse.rs index fb65a281f8..10fef0528a 100644 --- a/tests/core/parse.rs +++ b/tests/core/parse.rs @@ -303,7 +303,7 @@ impl ExecutionSpec { }; let endpoints = config - .extensions() + .extensions .endpoint_set .clone() .into_checked(&blueprint, runtime.clone())