From 92760aca64ab89a5c8aba56ee00cd84746634296 Mon Sep 17 00:00:00 2001 From: Jim Crossley Date: Thu, 7 Nov 2024 12:46:52 -0500 Subject: [PATCH 1/5] Revert the dot-notation in queries Instead, provide a means to associate query field names to keys in json object. Signed-off-by: Jim Crossley --- common/src/db/query.rs | 2 +- common/src/db/query/columns.rs | 64 ++++++++++++++++++++-------------- 2 files changed, 39 insertions(+), 27 deletions(-) diff --git a/common/src/db/query.rs b/common/src/db/query.rs index b14396c0..94250709 100644 --- a/common/src/db/query.rs +++ b/common/src/db/query.rs @@ -111,7 +111,7 @@ impl Query { fn parse(&self) -> Vec { // regex for filters: {field}{op}{value} - const RE: &str = r"^(?[[:word:]\.]+)(?=|!=|~|!~|>=|>|<=|<)(?.*)$"; + const RE: &str = r"^(?[[:word:]]+)(?=|!=|~|!~|>=|>|<=|<)(?.*)$"; static LOCK: OnceLock = OnceLock::new(); #[allow(clippy::unwrap_used)] let regex = LOCK.get_or_init(|| (Regex::new(RE).unwrap())); diff --git a/common/src/db/query/columns.rs b/common/src/db/query/columns.rs index b8398708..bcd99a50 100644 --- a/common/src/db/query/columns.rs +++ b/common/src/db/query/columns.rs @@ -1,3 +1,4 @@ +use std::collections::HashMap; use std::fmt::{Display, Formatter}; use sea_orm::entity::ColumnDef; @@ -10,6 +11,7 @@ use sea_query::{Alias, ColumnRef, Expr, IntoColumnRef, IntoIden}; pub struct Columns { columns: Vec<(ColumnRef, ColumnDef)>, translator: Option, + json_keys: HashMap<&'static str, &'static str>, } impl Display for Columns { @@ -73,6 +75,7 @@ impl Columns { Self { columns, translator: None, + json_keys: HashMap::new(), } } @@ -124,6 +127,14 @@ impl Columns { self } + /// Declare which query fields are the nested keys of a JSON column + pub fn json_keys(mut self, column: &'static str, fields: &[&'static str]) -> Self { + for each in fields { + self.json_keys.insert(each, column); + } + self + } + pub fn iter(&self) -> impl Iterator { self.columns.iter() } @@ -139,29 +150,27 @@ impl Columns { if name.to_string().eq_ignore_ascii_case(tgt)) } } - match field.split_once('.') { - None => self - .columns - .iter() - .find(name_match(field)) - .map(|(r, d)| (Expr::col(r.clone()), d.clone())), - Some((col, key)) => self - .columns - .iter() - .filter(|(_, def)| { - matches!( - def.get_column_type(), - ColumnType::Json | ColumnType::JsonBinary - ) - }) - .find(name_match(col)) - .map(|(r, d)| { - ( - Expr::expr(Expr::col(r.clone()).cast_json_field(key)), - d.clone(), - ) - }), - } + self.columns + .iter() + .find(name_match(field)) + .map(|(r, d)| (Expr::col(r.clone()), d.clone())) + .or_else(|| { + self.columns + .iter() + .filter(|(_, def)| { + matches!( + def.get_column_type(), + ColumnType::Json | ColumnType::JsonBinary + ) + }) + .find(name_match(self.json_keys.get(field).map_or("", |v| v))) + .map(|(r, d)| { + ( + Expr::expr(Expr::col(r.clone()).cast_json_field(field)), + d.clone(), + ) + }) + }) } pub(crate) fn translate(&self, field: &str, op: &str, value: &str) -> Option { @@ -320,8 +329,11 @@ mod tests { .select_only() .column(advisory::Column::Id) .filtering_with( - q("purl.name~log4j&purl.version>1.0&purl.ty=maven").sort("purl.name"), - advisory::Entity.columns().alias("advisory", "foo"), + q("name~log4j&version>1.0&type=maven").sort("name"), + advisory::Entity + .columns() + .alias("advisory", "x") + .json_keys("purl", &["name", "type", "version"]), )? .build(sea_orm::DatabaseBackend::Postgres) .to_string() @@ -332,7 +344,7 @@ mod tests { assert_eq!( clause, - r#"(("foo"."purl" ->> 'name') ILIKE '%log4j%') AND ("foo"."purl" ->> 'version') > '1.0' AND ("foo"."purl" ->> 'ty') = 'maven' ORDER BY "foo"."purl" ->> 'name' ASC"# + r#"(("x"."purl" ->> 'name') ILIKE '%log4j%') AND ("x"."purl" ->> 'version') > '1.0' AND ("x"."purl" ->> 'type') = 'maven' ORDER BY "x"."purl" ->> 'name' ASC"# ); Ok(()) From d22729d47adca46788e796fd55e426d333baf363 Mon Sep 17 00:00:00 2001 From: Jim Crossley Date: Thu, 7 Nov 2024 12:48:16 -0500 Subject: [PATCH 2/5] Consolidate error handling inside the common fn, for_field Signed-off-by: Jim Crossley --- common/src/db/query/columns.rs | 7 ++++++- common/src/db/query/filter.rs | 4 +--- common/src/db/query/sort.rs | 7 +------ 3 files changed, 8 insertions(+), 10 deletions(-) diff --git a/common/src/db/query/columns.rs b/common/src/db/query/columns.rs index bcd99a50..b9f9cc4f 100644 --- a/common/src/db/query/columns.rs +++ b/common/src/db/query/columns.rs @@ -6,6 +6,8 @@ use sea_orm::{sea_query, ColumnTrait, ColumnType, EntityTrait, IntoIdentity, Ite use sea_query::extension::postgres::PgExpr; use sea_query::{Alias, ColumnRef, Expr, IntoColumnRef, IntoIden}; +use super::Error; + /// Context of columns which can be used for filtering and sorting. #[derive(Default, Debug, Clone)] pub struct Columns { @@ -140,7 +142,7 @@ impl Columns { } /// Look up the column context for a given simple field name. - pub(crate) fn for_field(&self, field: &str) -> Option<(Expr, ColumnDef)> { + pub(crate) fn for_field(&self, field: &str) -> Result<(Expr, ColumnDef), Error> { fn name_match(tgt: &str) -> impl Fn(&&(ColumnRef, ColumnDef)) -> bool + '_ { |(col, _)| { matches!(col, @@ -171,6 +173,9 @@ impl Columns { ) }) }) + .ok_or(Error::SearchSyntax(format!( + "Invalid field name: '{field}'" + ))) } pub(crate) fn translate(&self, field: &str, op: &str, value: &str) -> Option { diff --git a/common/src/db/query/filter.rs b/common/src/db/query/filter.rs index d35a2237..74bc5ef0 100644 --- a/common/src/db/query/filter.rs +++ b/common/src/db/query/filter.rs @@ -30,9 +30,7 @@ impl TryFrom<(&str, Operator, &Vec, &Columns)> for Filter { type Error = Error; fn try_from(tuple: (&str, Operator, &Vec, &Columns)) -> Result { let (ref field, operator, values, columns) = tuple; - let (expr, col_def) = columns.for_field(field).ok_or(Error::SearchSyntax(format!( - "Invalid field name for filter: '{field}'" - )))?; + let (expr, col_def) = columns.for_field(field)?; Ok(Filter { operator: match operator { Operator::NotLike | Operator::NotEqual => Operator::And, diff --git a/common/src/db/query/sort.rs b/common/src/db/query/sort.rs index a30ff738..afc2ec5f 100644 --- a/common/src/db/query/sort.rs +++ b/common/src/db/query/sort.rs @@ -22,12 +22,7 @@ impl Sort { match columns.translate(field, &order, "") { Some(s) => Sort::parse(&s, columns), None => Ok(Self { - field: columns - .for_field(field) - .ok_or(Error::SearchSyntax(format!( - "Invalid sort field: '{field}'" - )))? - .0, + field: columns.for_field(field)?.0, order: match order.as_str() { "asc" => Order::Asc, "desc" => Order::Desc, From 6f0c7167d494eefc08d4b7bf6bd5d9f4737ec733 Mon Sep 17 00:00:00 2001 From: Jim Crossley Date: Thu, 7 Nov 2024 13:55:32 -0500 Subject: [PATCH 3/5] Add a few more tests around nested json fields Signed-off-by: Jim Crossley --- common/src/db/query.rs | 2 +- common/src/db/query/columns.rs | 42 +++++++++++++++++++--------------- 2 files changed, 25 insertions(+), 19 deletions(-) diff --git a/common/src/db/query.rs b/common/src/db/query.rs index 94250709..4aca1bac 100644 --- a/common/src/db/query.rs +++ b/common/src/db/query.rs @@ -171,7 +171,7 @@ pub struct Query { pub sort: String, } -#[derive(Debug, thiserror::Error)] +#[derive(Debug, PartialEq, thiserror::Error)] pub enum Error { #[error("query syntax error: {0}")] SearchSyntax(String), diff --git a/common/src/db/query/columns.rs b/common/src/db/query/columns.rs index b9f9cc4f..c6d6f2f2 100644 --- a/common/src/db/query/columns.rs +++ b/common/src/db/query/columns.rs @@ -330,27 +330,33 @@ mod tests { #[test(tokio::test)] async fn json_queries() -> Result<(), anyhow::Error> { - let clause = advisory::Entity::find() - .select_only() - .column(advisory::Column::Id) - .filtering_with( - q("name~log4j&version>1.0&type=maven").sort("name"), - advisory::Entity - .columns() - .alias("advisory", "x") - .json_keys("purl", &["name", "type", "version"]), - )? - .build(sea_orm::DatabaseBackend::Postgres) - .to_string() - .split("WHERE ") - .last() - .unwrap() - .to_string(); + fn clause(query: Query) -> Result { + Ok(advisory::Entity::find() + .filtering_with( + query, + advisory::Entity + .columns() + .json_keys("purl", &["name", "type", "version"]), + )? + .build(sea_orm::DatabaseBackend::Postgres) + .to_string() + .split("WHERE ") + .last() + .unwrap() + .to_string()) + } assert_eq!( - clause, - r#"(("x"."purl" ->> 'name') ILIKE '%log4j%') AND ("x"."purl" ->> 'version') > '1.0' AND ("x"."purl" ->> 'type') = 'maven' ORDER BY "x"."purl" ->> 'name' ASC"# + clause(q("name~log4j&version>1.0"))?, + r#"(("advisory"."purl" ->> 'name') ILIKE '%log4j%') AND ("advisory"."purl" ->> 'version') > '1.0'"# + ); + assert_eq!( + clause(q("name=log4j").sort("name"))?, + r#"("advisory"."purl" ->> 'name') = 'log4j' ORDER BY "advisory"."purl" ->> 'name' ASC"# ); + assert!(clause(q("missing=gone")).is_err()); + assert!(clause(q("").sort("name")).is_ok()); + assert!(clause(q("").sort("nope")).is_err()); Ok(()) } From 7e9719dd3971c6500012549e0877e8607669dd26 Mon Sep 17 00:00:00 2001 From: Jim Crossley Date: Thu, 7 Nov 2024 14:10:30 -0500 Subject: [PATCH 4/5] clippiness Signed-off-by: Jim Crossley --- common/src/db/query/filter.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/common/src/db/query/filter.rs b/common/src/db/query/filter.rs index 74bc5ef0..515083d6 100644 --- a/common/src/db/query/filter.rs +++ b/common/src/db/query/filter.rs @@ -29,7 +29,7 @@ impl Filter { impl TryFrom<(&str, Operator, &Vec, &Columns)> for Filter { type Error = Error; fn try_from(tuple: (&str, Operator, &Vec, &Columns)) -> Result { - let (ref field, operator, values, columns) = tuple; + let (field, operator, values, columns) = tuple; let (expr, col_def) = columns.for_field(field)?; Ok(Filter { operator: match operator { From b273952f53f6ccdb62e89327976c5f65de047e5c Mon Sep 17 00:00:00 2001 From: Jim Crossley Date: Thu, 7 Nov 2024 14:27:38 -0500 Subject: [PATCH 5/5] Deal with missing keys more elegantly Signed-off-by: Jim Crossley --- common/src/db/query/columns.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/common/src/db/query/columns.rs b/common/src/db/query/columns.rs index c6d6f2f2..d8942fe3 100644 --- a/common/src/db/query/columns.rs +++ b/common/src/db/query/columns.rs @@ -165,7 +165,7 @@ impl Columns { ColumnType::Json | ColumnType::JsonBinary ) }) - .find(name_match(self.json_keys.get(field).map_or("", |v| v))) + .find(name_match(self.json_keys.get(field)?)) .map(|(r, d)| { ( Expr::expr(Expr::col(r.clone()).cast_json_field(field)), @@ -357,6 +357,7 @@ mod tests { assert!(clause(q("missing=gone")).is_err()); assert!(clause(q("").sort("name")).is_ok()); assert!(clause(q("").sort("nope")).is_err()); + assert!(clause(q("q=x")).is_err()); Ok(()) }