From 587946cf1b34dddce36f3479e8a36c6f2fadb33b Mon Sep 17 00:00:00 2001 From: sevenzing <41516657+sevenzing@users.noreply.github.com> Date: Mon, 2 Sep 2024 13:57:40 +0200 Subject: [PATCH 1/4] fix(env-collector): add .json support and add documentation --- libs/env-collector/Cargo.toml | 2 +- libs/env-collector/README.md | 60 ++++++++++++++++++++++ libs/env-collector/src/lib.rs | 94 ++++++++++++++++++++++++----------- 3 files changed, 125 insertions(+), 31 deletions(-) create mode 100644 libs/env-collector/README.md diff --git a/libs/env-collector/Cargo.toml b/libs/env-collector/Cargo.toml index ad098126a..2cb133384 100644 --- a/libs/env-collector/Cargo.toml +++ b/libs/env-collector/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "env-collector" -version = "0.1.0" +version = "0.1.1" edition = "2021" [dependencies] diff --git a/libs/env-collector/README.md b/libs/env-collector/README.md new file mode 100644 index 000000000..f8678bdde --- /dev/null +++ b/libs/env-collector/README.md @@ -0,0 +1,60 @@ +Env collector +=== + +This is a simple tool to collect possible environment variables from `Settings` structure and place it to `README.md` file. + +## Usage + +1. Add `env-collector` to your `server` crate: + ```toml + # Cargo.toml + [dependencies] + env-collector = { git = "https://github.com/blockscout/blockscout-rs", version = "0.1.1" } + ``` + +2. In your `server` crate create new binary file called `check-envs.rs` with the following content: + + ```rust + // check-envs.rs + use ::Settings; + use env_collector::run_env_collector_cli; + + fn main() { + run_env_collector_cli::( + "", + "README.md", + "", + &[""], + ); + } + ``` +3. In `README.md` file add special **anchors** lines to specify where to store the table with ENVs: + + ```markdown + ## Envs + + [anchor]: <> (anchors.envs.start) + [anchor]: <> (anchors.envs.end) + ``` + +4. (Optional) In your `justfile` add new command to run `check-envs.rs`: + + ```toml + # justfile + check-envs: + cargo run --bin check-envs + ``` + +5. Run command `just check-envs` to generate ENVs table in `README.md` file. + +6. Add github action to run this binary on every push to validate ENVs table in `README.md` file: + ```yaml + [... other steps of `test` job ...] + + - name: ENVs in doc tests + run: cargo run --bin check-envs + env: + VALIDATE_ONLY: true + + [... other steps of `test` job ...] + ``` \ No newline at end of file diff --git a/libs/env-collector/src/lib.rs b/libs/env-collector/src/lib.rs index ded3fa398..174346a7c 100644 --- a/libs/env-collector/src/lib.rs +++ b/libs/env-collector/src/lib.rs @@ -1,5 +1,5 @@ use anyhow::Context; -use config::{Config, File, FileFormat}; +use config::{Config, File}; use json_dotpath::DotPaths; use serde::{de::DeserializeOwned, Serialize}; use serde_json::Value; @@ -12,6 +12,7 @@ use std::{ const ANCHOR_START: &str = "anchors.envs.start"; const ANCHOR_END: &str = "anchors.envs.end"; +const VALIDATE_ONLY_ENV: &str = "VALIDATE_ONLY"; pub fn run_env_collector_cli( service_name: &str, @@ -25,7 +26,7 @@ pub fn run_env_collector_cli( config_path.into(), skip_vars.iter().map(|s| s.to_string()).collect(), ); - let validate_only = std::env::var("VALIDATE_ONLY") + let validate_only = std::env::var(VALIDATE_ONLY_ENV) .unwrap_or_default() .to_lowercase() .eq("true"); @@ -129,19 +130,16 @@ impl From> for Envs { } impl Envs { - pub fn from_example_toml( + pub fn from_example( service_prefix: &str, - example_toml_config_content: &str, + example_config_path: &str, skip_vars: Vec, ) -> Result where S: Serialize + DeserializeOwned, { let settings: S = Config::builder() - .add_source(File::from_str( - example_toml_config_content, - FileFormat::Toml, - )) + .add_source(File::with_name(example_config_path)) .build() .context("failed to build config")? .try_deserialize() @@ -228,11 +226,11 @@ fn find_missing_variables_in_markdown( where S: Serialize + DeserializeOwned, { - let example = Envs::from_example_toml::( + let example = Envs::from_example::( service_name, - std::fs::read_to_string(config_path) - .context("failed to read example file")? - .as_str(), + config_path + .to_str() + .expect("config path is not valid utf-8"), skip_vars, )?; let markdown: Envs = Envs::from_markdown( @@ -263,11 +261,11 @@ fn update_markdown_file( where S: Serialize + DeserializeOwned, { - let from_config = Envs::from_example_toml::( + let from_config = Envs::from_example::( service_name, - std::fs::read_to_string(config_path) - .context("failed to read config file")? - .as_str(), + config_path + .to_str() + .expect("config path is not valid utf-8"), skip_vars, )?; let mut markdown_config = Envs::from_markdown( @@ -408,12 +406,32 @@ mod tests { ) } - fn default_example() -> &'static str { - r#"test = "value" -test2 = 123 -[database.connect] -url = "test-url" -"# + fn tempfile_with_content(content: &str, format: &str) -> tempfile::NamedTempFile { + let mut file = tempfile::Builder::new().suffix(format).tempfile().unwrap(); + writeln!(file, "{}", content).unwrap(); + file + } + + fn default_config_example_file_toml() -> tempfile::NamedTempFile { + let content = r#"test = "value" + test2 = 123 + [database.connect] + url = "test-url" + "#; + tempfile_with_content(content, ".toml") + } + + fn default_config_example_file_json() -> tempfile::NamedTempFile { + let content = r#"{ + "test": "value", + "test2": 123, + "database": { + "connect": { + "url": "test-url" + } + } + }"#; + tempfile_with_content(content, ".json") } fn default_envs() -> Envs { @@ -434,7 +452,7 @@ url = "test-url" ])) } - fn default_markdown() -> &'static str { + fn default_markdown_content() -> &'static str { r#" [anchor]: <> (anchors.envs.start) @@ -450,17 +468,34 @@ url = "test-url" } #[test] - fn from_example_works() { - let vars = - Envs::from_example_toml::("TEST_SERVICE", default_example(), vec![]) - .unwrap(); + fn from_toml_example_works() { + let example_file = default_config_example_file_toml(); + let vars = Envs::from_example::( + "TEST_SERVICE", + example_file.path().to_str().unwrap(), + vec![], + ) + .unwrap(); + let expected = default_envs(); + assert_eq!(vars, expected); + } + + #[test] + fn from_json_example_works() { + let example_file = default_config_example_file_json(); + let vars = Envs::from_example::( + "TEST_SERVICE", + example_file.path().to_str().unwrap(), + vec![], + ) + .unwrap(); let expected = default_envs(); assert_eq!(vars, expected); } #[test] fn from_markdown_works() { - let markdown = default_markdown(); + let markdown = default_markdown_content(); let vars = Envs::from_markdown(markdown).unwrap(); let expected = default_envs(); assert_eq!(vars, expected); @@ -481,8 +516,7 @@ url = "test-url" ) .unwrap(); - let mut config = tempfile::NamedTempFile::new().unwrap(); - writeln!(config, "{}", default_example()).unwrap(); + let config = default_config_example_file_toml(); let collector = EnvCollector::::new( "TEST_SERVICE".to_string(), From f3bd1048e0e90283276581ad9a97d50bad3370c7 Mon Sep 17 00:00:00 2001 From: sevenzing <41516657+sevenzing@users.noreply.github.com> Date: Mon, 2 Sep 2024 18:11:04 +0200 Subject: [PATCH 2/4] feat(env-collector): place real default value in table, add simple description --- libs/env-collector/src/lib.rs | 91 ++++++++++++++++++++++++----------- 1 file changed, 62 insertions(+), 29 deletions(-) diff --git a/libs/env-collector/src/lib.rs b/libs/env-collector/src/lib.rs index 174346a7c..22c9062f8 100644 --- a/libs/env-collector/src/lib.rs +++ b/libs/env-collector/src/lib.rs @@ -6,7 +6,6 @@ use serde_json::Value; use std::{ collections::BTreeMap, marker::PhantomData, - ops::Not, path::{Path, PathBuf}, }; @@ -102,7 +101,7 @@ where } } -#[derive(Debug, Clone, Ord, PartialOrd, Eq)] +#[derive(Debug, Clone, Ord, PartialOrd, PartialEq, Eq)] pub struct EnvVariable { pub key: String, pub description: String, @@ -110,11 +109,9 @@ pub struct EnvVariable { pub default_value: Option, } -impl PartialEq for EnvVariable { - fn eq(&self, other: &Self) -> bool { - self.key == other.key - && self.required == other.required - && self.default_value == other.default_value +impl EnvVariable { + pub fn eq_with_ignores(&self, other: &Self) -> bool { + self.key == other.key && self.required == other.required } } @@ -149,14 +146,16 @@ impl Envs { .into_iter() .filter(|(key, _)| !skip_vars.iter().any(|s| key.starts_with(s))) .map(|(key, value)| { - let required = - var_is_required(&settings, &from_key_to_json_path(&key, service_prefix)); - let default_value = required.not().then_some(value); + let default_value = + default_of_var(&settings, &from_key_to_json_path(&key, service_prefix)); + let required = default_value.is_none(); + let description = try_get_description(&key, &value, &default_value); + let default_value = default_value.map(|v| v.to_string()); let var = EnvVariable { key: key.clone(), required, default_value, - description: Default::default(), + description, }; (key, var) @@ -244,7 +243,9 @@ where .iter() .filter(|(key, value)| { let maybe_markdown_var = markdown.vars.get(*key); - maybe_markdown_var.map(|var| var != *value).unwrap_or(true) + maybe_markdown_var + .map(|var| !var.eq_with_ignores(value)) + .unwrap_or(true) }) .map(|(_, value)| value.clone()) .collect(); @@ -292,14 +293,30 @@ where Ok(()) } -fn var_is_required(settings: &S, path: &str) -> bool +fn default_of_var(settings: &S, path: &str) -> Option where S: Serialize + DeserializeOwned, { - let mut json = serde_json::to_value(settings).unwrap(); - json.dot_remove(path).unwrap(); - let result = serde_json::from_value::(json); - result.is_err() + let mut json = serde_json::to_value(settings).expect("structure should be serializable"); + json.dot_remove(path).expect("value path not found"); + let settings_with_default_value = serde_json::from_value::(json).ok()?; + let json = serde_json::to_value(&settings_with_default_value) + .expect("structure should be serializable"); + json.dot_get(path).expect("value path not found") +} + +fn try_get_description(_key: &str, value: &str, default: &Option) -> String { + if value.is_empty() { + return Default::default(); + } + + if let Some(default) = default { + if *default == value { + return Default::default(); + } + } + + format!("e.g. `{}`", value) } fn from_key_to_json_path(key: &str, service_prefix: &str) -> String { @@ -389,19 +406,28 @@ mod tests { #[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq)] struct TestSettings { pub test: String, - #[serde(default)] + #[serde(default = "default_test2")] pub test2: i32, pub database: DatabaseSettings, } - fn var(key: &str, val: Option<&str>, required: bool) -> (String, EnvVariable) { + fn default_test2() -> i32 { + 1000 + } + + fn var( + key: &str, + val: Option<&str>, + required: bool, + description: &str, + ) -> (String, EnvVariable) { ( key.into(), EnvVariable { key: key.to_string(), default_value: val.map(str::to_string), required, - description: "".to_string(), + description: description.into(), }, ) } @@ -436,19 +462,26 @@ mod tests { fn default_envs() -> Envs { Envs::from(BTreeMap::from_iter(vec![ - var("TEST_SERVICE__TEST", None, true), + var("TEST_SERVICE__TEST", None, true, "e.g. `value`"), var( "TEST_SERVICE__DATABASE__CREATE_DATABASE", Some("false"), false, + "", ), var( "TEST_SERVICE__DATABASE__RUN_MIGRATIONS", Some("false"), false, + "", + ), + var("TEST_SERVICE__TEST2", Some("1000"), false, "e.g. `123`"), + var( + "TEST_SERVICE__DATABASE__CONNECT__URL", + None, + true, + "e.g. `test-url`", ), - var("TEST_SERVICE__TEST2", Some("123"), false), - var("TEST_SERVICE__DATABASE__CONNECT__URL", None, true), ])) } @@ -458,11 +491,11 @@ mod tests { | Variable | Required | Description | Default Value | |-------------------------------------------|-------------|-------------|---------------| -| `TEST_SERVICE__TEST` | true | | | +| `TEST_SERVICE__TEST` | true | e.g. `value` | | | `TEST_SERVICE__DATABASE__CREATE_DATABASE` | false | | `false` | | `TEST_SERVICE__DATABASE__RUN_MIGRATIONS` | false | | `false` | -| `TEST_SERVICE__TEST2` | false | | `123` | -| `TEST_SERVICE__DATABASE__CONNECT__URL` | true | | | +| `TEST_SERVICE__TEST2` | false | e.g. `123` | `1000` | +| `TEST_SERVICE__DATABASE__CONNECT__URL` | true | e.g. `test-url` | | [anchor]: <> (anchors.envs.end) "# } @@ -548,12 +581,12 @@ mod tests { | Variable | Required | Description | Default value | | --- | --- | --- | --- | | `SOME_EXTRA_VARS2` | true | | `example_value2` | -| `TEST_SERVICE__DATABASE__CONNECT__URL` | true | | | -| `TEST_SERVICE__TEST` | true | | | +| `TEST_SERVICE__DATABASE__CONNECT__URL` | true | e.g. `test-url` | | +| `TEST_SERVICE__TEST` | true | e.g. `value` | | | `SOME_EXTRA_VARS` | | comment should be saved. `kek` | `example_value` | | `TEST_SERVICE__DATABASE__CREATE_DATABASE` | | | `false` | | `TEST_SERVICE__DATABASE__RUN_MIGRATIONS` | | | `false` | -| `TEST_SERVICE__TEST2` | | | `123` | +| `TEST_SERVICE__TEST2` | | e.g. `123` | `1000` | [anchor]: <> (anchors.envs.end) "# From 773d33d6ee1261c561d8603a08cfa7a623f24ec4 Mon Sep 17 00:00:00 2001 From: sevenzing <41516657+sevenzing@users.noreply.github.com> Date: Mon, 2 Sep 2024 18:12:07 +0200 Subject: [PATCH 3/4] fix: typo --- libs/env-collector/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libs/env-collector/README.md b/libs/env-collector/README.md index f8678bdde..66a70114b 100644 --- a/libs/env-collector/README.md +++ b/libs/env-collector/README.md @@ -39,7 +39,7 @@ This is a simple tool to collect possible environment variables from `Settings` 4. (Optional) In your `justfile` add new command to run `check-envs.rs`: - ```toml + ```just # justfile check-envs: cargo run --bin check-envs From 8aac82516d98a5806e95f955af3c5099cab7aefe Mon Sep 17 00:00:00 2001 From: Kirill Ivanov <8144358+bragov4ik@users.noreply.github.com> Date: Tue, 3 Sep 2024 12:45:40 +0300 Subject: [PATCH 4/4] fix(libs): env-collector required field check (#1047) * test with options * fix: wrong checking that field is required --------- Co-authored-by: sevenzing <41516657+sevenzing@users.noreply.github.com> --- libs/env-collector/src/lib.rs | 44 +++++++++++++++++++++++++---------- 1 file changed, 32 insertions(+), 12 deletions(-) diff --git a/libs/env-collector/src/lib.rs b/libs/env-collector/src/lib.rs index 22c9062f8..72e6fc353 100644 --- a/libs/env-collector/src/lib.rs +++ b/libs/env-collector/src/lib.rs @@ -299,21 +299,26 @@ where { let mut json = serde_json::to_value(settings).expect("structure should be serializable"); json.dot_remove(path).expect("value path not found"); + let settings_with_default_value = serde_json::from_value::(json).ok()?; - let json = serde_json::to_value(&settings_with_default_value) + let json: serde_json::Value = serde_json::to_value(&settings_with_default_value) .expect("structure should be serializable"); - json.dot_get(path).expect("value path not found") + let default_value: serde_json::Value = json + .dot_get(path) + .expect("value path not found") + .unwrap_or_default(); + Some(default_value) } fn try_get_description(_key: &str, value: &str, default: &Option) -> String { if value.is_empty() { return Default::default(); } + let default_str = default.as_ref().map(|v| v.to_string()).unwrap_or_default(); - if let Some(default) = default { - if *default == value { - return Default::default(); - } + // If the value is the same as the default value, we don't need to show it in the description + if default_str == value { + return Default::default(); } format!("e.g. `{}`", value) @@ -408,6 +413,8 @@ mod tests { pub test: String, #[serde(default = "default_test2")] pub test2: i32, + pub test3_set: Option, + pub test4_not_set: Option, pub database: DatabaseSettings, } @@ -441,6 +448,7 @@ mod tests { fn default_config_example_file_toml() -> tempfile::NamedTempFile { let content = r#"test = "value" test2 = 123 + test3_set = false [database.connect] url = "test-url" "#; @@ -451,6 +459,7 @@ mod tests { let content = r#"{ "test": "value", "test2": 123, + "test3_set": false, "database": { "connect": { "url": "test-url" @@ -476,6 +485,13 @@ mod tests { "", ), var("TEST_SERVICE__TEST2", Some("1000"), false, "e.g. `123`"), + var( + "TEST_SERVICE__TEST3_SET", + Some("null"), + false, + "e.g. `false`", + ), + var("TEST_SERVICE__TEST4_NOT_SET", Some("null"), false, ""), var( "TEST_SERVICE__DATABASE__CONNECT__URL", None, @@ -489,12 +505,14 @@ mod tests { r#" [anchor]: <> (anchors.envs.start) -| Variable | Required | Description | Default Value | -|-------------------------------------------|-------------|-------------|---------------| -| `TEST_SERVICE__TEST` | true | e.g. `value` | | -| `TEST_SERVICE__DATABASE__CREATE_DATABASE` | false | | `false` | -| `TEST_SERVICE__DATABASE__RUN_MIGRATIONS` | false | | `false` | -| `TEST_SERVICE__TEST2` | false | e.g. `123` | `1000` | +| Variable | Required | Description | Default Value | +|-------------------------------------------|-------------|------------------|---------------| +| `TEST_SERVICE__TEST` | true | e.g. `value` | | +| `TEST_SERVICE__DATABASE__CREATE_DATABASE` | false | | `false` | +| `TEST_SERVICE__DATABASE__RUN_MIGRATIONS` | false | | `false` | +| `TEST_SERVICE__TEST2` | false | e.g. `123` | `1000` | +| `TEST_SERVICE__TEST3_SET` | false | e.g. `false` | `null` | +| `TEST_SERVICE__TEST4_NOT_SET` | false | | `null` | | `TEST_SERVICE__DATABASE__CONNECT__URL` | true | e.g. `test-url` | | [anchor]: <> (anchors.envs.end) "# @@ -587,6 +605,8 @@ mod tests { | `TEST_SERVICE__DATABASE__CREATE_DATABASE` | | | `false` | | `TEST_SERVICE__DATABASE__RUN_MIGRATIONS` | | | `false` | | `TEST_SERVICE__TEST2` | | e.g. `123` | `1000` | +| `TEST_SERVICE__TEST3_SET` | | e.g. `false` | `null` | +| `TEST_SERVICE__TEST4_NOT_SET` | | | `null` | [anchor]: <> (anchors.envs.end) "#