Skip to content

Commit

Permalink
refactor: template, remove unnecessary key parameter in lookup filter
Browse files Browse the repository at this point in the history 
as the key is always the first column
  • Loading branch information
@jqnatividad
jqnatividad committed Nov 28, 2024
1 parent 50be4d5 commit 525de61
Show file tree
Hide file tree
Showing 2 changed files with 72 additions and 51 deletions.
88 changes: 52 additions & 36 deletions src/cmd/template.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,25 +17,36 @@ Otherwise, ALL the rendered rows will be sent to STDOUT or the designated --outp
Example:
data.csv:
"first name","last name",balance,"loyalty points",active
alice,jones,100.50,1000,true
bob,smith,200.75,2000,false
john,doe,10,1,true
"first name","last name",balance,"loyalty points",active,us_state
alice,jones,100.50,1000,true,TX
bob,smith,200.75,2000,false,CA
john,doe,10,1,true,TX
NOTE: All variables are of type String and will need to be cast with the `|float` or `|int`
filters for math operations and when a MiniJinja filter/function requires it.
qsv's custom filters (substr, format_float, human_count, human_float_count, round_banker &
str_to_bool) do not require casting for convenience.
template.tpl
{% set us_state_lookup_loaded = register_lookup("us_states", "dathere://us-states-example.csv") -%}
Dear {{ first_name|title }} {{ last_name|title }}!
Your account balance is {{ balance|format_float(2) }}
with {{ loyalty_points|human_count }} point{{ loyalty_points|int|pluralize }}!
{# This is a comment and will not be rendered. The closing minus sign in this
block tells MiniJinja to trim whitespaces -#}
{% set loyalty_value = loyalty_points|int / 100 -%}
Value of Points: {{ loyalty_value }}
Final Balance: {{ balance|int - loyalty_value }}
{% if us_state_lookup_loaded is true -%}
{% if us_state not in ["DE", "CA"] -%}
State: {{ us_state|lookup("us_states", "Name")|title }}
{% set loyalty_value = loyalty_points|int / 100 -%}
Value of Points: {{ loyalty_value }}
{% set tax_rate = us_state|lookup("us_states", "Sales Tax (2023)")|float -%}
{% set tax_amount = loyalty_value * tax_rate -%}
{% set loyalty_value = loyalty_value - tax_amount -%}
{% else %}
{% set loyalty_value = 0 -%}
{% endif %}
Final Balance: {{ balance|int - loyalty_value }}
{% endif %}
Status: {% if active|str_to_bool is true %}Active{% else %}Inactive{% endif %}
qsv template --template-file template.tpl data.csv
Expand Down Expand Up @@ -350,9 +361,7 @@ pub fn run(argv: &[&str]) -> CliResult<()> {
// Try to render just the register_lookup statements with empty context
match temp_env.render_str(&temp_template, minijinja::context! {}) {
Ok(s) => {
// eprintln!("{:#?}", LOOKUP_MAP.get().unwrap());

if s.contains("LOOKUP REGISTRATION ERROR:") {
if !s.is_empty() {
return fail_incorrectusage_clierror!("{s}");
}
},
Expand Down Expand Up @@ -589,7 +598,7 @@ fn str_to_bool(value: &str) -> bool {
/// * `lookup_name` - Name to register the lookup table under
/// * `lookup_table_uri` - Path/URI to the CSV file (supports local files, HTTP(S), CKAN resources)
/// * `cache_age_secs` - Optional cache duration in seconds for remote files. Defaults to 3600 (1
/// hour)
/// hour). Set to 0 to disable caching.
///
/// # Returns
///
Expand Down Expand Up @@ -625,20 +634,22 @@ fn register_lookup(
));
}

let cache_age_secs = cache_age_secs.unwrap_or(3600);

// Check if lookup_name already exists in LOOKUP_MAP
if let Some(lock) = LOOKUP_MAP.get() {
if let Ok(map) = lock.read() {
if map.contains_key(lookup_name) {
if map.contains_key(lookup_name) && cache_age_secs > 0 {
// Lookup table already registered
return Ok(true);
}
}
}

let lookup_opts = LookupTableOptions {
name: lookup_name.to_string(),
uri: lookup_table_uri.to_string(),
cache_dir: QSV_CACHE_DIR
name: lookup_name.to_string(),
uri: lookup_table_uri.to_string(),
cache_dir: QSV_CACHE_DIR
.get()
.ok_or_else(|| {
minijinja::Error::new(
Expand All @@ -647,11 +658,11 @@ fn register_lookup(
)
})?
.to_string(),
cache_age_secs: cache_age_secs.unwrap_or(3600),
delimiter: DELIMITER.get().copied().flatten(),
ckan_api_url: CKAN_API.get().cloned(),
ckan_token: CKAN_TOKEN.get().and_then(std::clone::Clone::clone),
timeout_secs: TIMEOUT_SECS.load(Ordering::Relaxed),
cache_age_secs,
delimiter: DELIMITER.get().copied().flatten(),
ckan_api_url: CKAN_API.get().cloned(),
ckan_token: CKAN_TOKEN.get().and_then(std::clone::Clone::clone),
timeout_secs: TIMEOUT_SECS.load(Ordering::Relaxed),
};

let lookup_table = lookup::load_lookup_table(&lookup_opts).map_err(|e| {
Expand All @@ -661,7 +672,19 @@ fn register_lookup(
)
})?;

let mut rdr = csv::Reader::from_path(&lookup_table.filepath).map_err(|e| {
if lookup_table.rowcount == 0 {
return Err(minijinja::Error::new(
minijinja::ErrorKind::InvalidOperation,
"lookup table is empty",
));
}

let lookup_config = Config::new(Some(lookup_table.filepath.clone()).as_ref())
.delimiter(lookup_opts.delimiter)
.comment(Some(b'#'))
.no_headers(false);

let mut rdr = lookup_config.reader().map_err(|e| {
minijinja::Error::new(
minijinja::ErrorKind::InvalidOperation,
format!(
Expand All @@ -686,7 +709,7 @@ fn register_lookup(

// Use the first column as the key by default
if let Some(key_value) = record.get(0) {
lookup_data.insert(key_value.to_string(), row_data);
lookup_data.insert(key_value.trim().to_string(), row_data);
}
}

Expand Down Expand Up @@ -714,14 +737,13 @@ fn register_lookup(
/// A filter function for looking up values in a registered lookup table.
///
/// This function is used as a template filter to look up values from a previously registered
/// lookup table. It searches for a record in the lookup table where the `lookup_key` column
/// lookup table. It searches for a record in the lookup table where the first column
/// matches the input `value`, and returns the corresponding value from the `field` column.
///
/// # Arguments
///
/// * `value` - The value to look up in the lookup table
/// * `lookup_name` - The name of the registered lookup table to search in
/// * `lookup_key` - The column name in the lookup table to match against the input value
/// * `field` - The column name in the lookup table whose value should be returned
/// * `case_sensitive` - Optional boolean to control case-sensitive matching (defaults to true)
///
Expand All @@ -735,14 +757,13 @@ fn register_lookup(
///
/// ```text
/// # Case-sensitive lookup (default)
/// {{ product_id|lookup('products', 'id', 'name') }}
/// {{ product_id|lookup('products', 'name') }}
/// # Case-insensitive lookup (supports Unicode)
/// {{ product_id|lookup('products', 'id', 'name', false) }}
/// {{ product_id|lookup('products', 'name', false) }}
/// ```
fn lookup_filter(
value: &str,
lookup_name: &str,
lookup_key: &str,
field: &str,
case_sensitive: Option<bool>,
) -> Result<String, minijinja::Error> {
Expand All @@ -753,13 +774,6 @@ fn lookup_filter(
));
}

if lookup_key.is_empty() {
return Err(minijinja::Error::new(
minijinja::ErrorKind::InvalidOperation,
"lookup key not provided",
));
}

if field.is_empty() {
return Err(minijinja::Error::new(
minijinja::ErrorKind::InvalidOperation,
Expand Down Expand Up @@ -790,7 +804,9 @@ fn lookup_filter(
let table = map.get(lookup_name)?;
// Find the matching row
if case_sensitive {
table.get(value).and_then(|row| row.get(field).map(String::from))
table
.get(value)
.and_then(|row| row.get(field).map(String::from))
} else {
table
.iter()
Expand All @@ -806,7 +822,7 @@ fn lookup_filter(
String::new()
} else {
format!(
r#"{filter_error} - lookup: "{lookup_name}" key: "{lookup_key}-{field}" not found for: "{value}""#
r#"{filter_error} - lookup: "{lookup_name}-{field}" not found for: "{value}""#
)
}
}))
Expand Down
35 changes: 20 additions & 15 deletions tests/test_template.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -793,8 +793,8 @@ fn template_lookup_filter_simple() {
.arg(concat!(
"{% set result = register_lookup('products', 'lookup.csv') %}",
"{% if result %}",
"{{product_id}}: {{product_id|lookup('products', 'id', 'name')}} - \
{{product_id|lookup('products', 'id', 'description')}}\n",
"{{product_id}}: {{product_id|lookup('products', 'name')}} - \
{{product_id|lookup('products', 'description')}}\n",
"{% else %}",
"Error: Failed to register lookup table 'products' {{ result.err }} \n",
"{% endif %}"
Expand All @@ -809,7 +809,7 @@ fn template_lookup_filter_simple() {
let expected = concat!(
"1: apple - A red fruit\n",
"2: banana - A yellow fruit\n",
r#"4: <not found> - lookup: "products" key: "id-name" not found for: "4" - <not found> - lookup: "products" key: "id-description" not found for: "4""#
r#"4: <not found> - lookup: "products-name" not found for: "4" - <not found> - lookup: "products-description" not found for: "4""#
);
assert_eq!(got, expected);
}
Expand Down Expand Up @@ -845,8 +845,8 @@ fn template_lookup_filter_invalid_field() {
.arg(concat!(
"{% set result = register_lookup('products', 'lookup.csv') %}",
"{% if result %}",
"{{product_id}}: {{product_id|lookup('products', 'id', 'name')}} - \
{{product_id|lookup('products', 'id', 'non_existent_column')}}\n",
"{{product_id}}: {{product_id|lookup('products', 'name')}} - \
{{product_id|lookup('products', 'non_existent_column')}}\n",
"{% else %}",
"Error: Failed to register lookup table 'products' {{ result.err }} \n",
"{% endif %}"
Expand All @@ -859,11 +859,11 @@ fn template_lookup_filter_invalid_field() {

let got: String = wrk.stdout(&mut cmd);
let expected = concat!(
r#"1: apple - <not found> - lookup: "products" key: "id-non_existent_column" not found for: "1"
r#"1: apple - <not found> - lookup: "products-non_existent_column" not found for: "1"
"#,
r#"2: banana - <not found> - lookup: "products" key: "id-non_existent_column" not found for: "2"
r#"2: banana - <not found> - lookup: "products-non_existent_column" not found for: "2"
"#,
r#"4: <not found> - lookup: "products" key: "id-name" not found for: "4" - <not found> - lookup: "products" key: "id-non_existent_column" not found for: "4""#
r#"4: <not found> - lookup: "products-name" not found for: "4" - <not found> - lookup: "products-non_existent_column" not found for: "4""#
);
assert_eq!(got, expected);
}
Expand Down Expand Up @@ -897,11 +897,16 @@ fn template_lookup_filter_errors() {
// Test unregistered lookup table
let mut cmd = wrk.command("template");
cmd.arg("--template")
.arg("{{id|lookup('id', 'name')}}")
.arg("{{id|lookup('non_existent lookup', 'name')}}")
.arg("data.csv");

wrk.assert_success(&mut cmd);

let got: String = wrk.stdout(&mut cmd);
assert!(got.contains("RENDERING ERROR"));
assert_eq!(
got,
"<FILTER_ERROR> - lookup: \"non_existent lookup-name\" not found for: \"1\""
);
}

#[test]
Expand Down Expand Up @@ -955,25 +960,25 @@ fn template_lookup_case_sensitivity() {
cmd.arg("--template")
.arg(concat!(
"{% if register_lookup('codes', 'lookup.csv') %}",
"{{code|lookup('codes', 'code', 'value')}}\n",
"{{code|lookup('codes', 'value')}}\n",
"{% endif %}"
))
.arg("data.csv");

let got: String = wrk.stdout(&mut cmd);
assert_eq!(
got,
r#"<FILTER_ERROR> - lookup: "codes" key: "code-value" not found for: "abc"
<FILTER_ERROR> - lookup: "codes" key: "code-value" not found for: "DEF"
<FILTER_ERROR> - lookup: "codes" key: "code-value" not found for: "ghi""#
r#"<FILTER_ERROR> - lookup: "codes-value" not found for: "abc"
<FILTER_ERROR> - lookup: "codes-value" not found for: "DEF"
<FILTER_ERROR> - lookup: "codes-value" not found for: "ghi""#
);

// Test case-insensitive lookup
let mut cmd = wrk.command("template");
cmd.arg("--template")
.arg(concat!(
"{% if register_lookup('codes', 'lookup.csv') %}",
"{{code|lookup('codes', 'code', 'value', false)}}\n",
"{{code|lookup('codes', 'value', false)}}\n",
"{% endif %}"
))
.arg("data.csv");
Expand Down

0 comments on commit 525de61

Please sign in to comment.